Merge #23106: Ensure wallet is unlocked before signing PSBT with walletprocesspsbt and GUI

MarcoFalke · MarcoFalke · commit a9d0cec4991a · 2021-09-28T09:49:44.000+02:00
7e3ee4c GUI: Ask user to unlock wallet before signing psbt (Samuel Dobson) 0f3acec Add test that walletprocesspsbt requires unlocked wallet when signing (Samuel Dobson) 0e89521 Ensure wallet is unlocked before signing in walletprocesspsbt (Samuel Dobson) Pull request description: If signing a PSBT, we need to ensure the wallet is unlocked. Fixes #22874, fixes bitcoin-core/gui#312 ACKs for top commit: achow101: ACK 7e3ee4c lsilva01: Code Review ACK 7e3ee4c benthecarman: ACK 7e3ee4c Tree-SHA512: 6726a873582747900ab454ea21153a92be86808a4c1517dc2856b389876a2da9e8df1ffa9b567b6bd017038342c3544ecf5ca3c97744e7debe0a5ee65563687d
diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp
@@ -71,6 +71,9 @@ void PSBTOperationsDialog::signTransaction()
 {
     bool complete;
     size_t n_signed;
+
+    WalletModel::UnlockContext ctx(m_wallet_model->requestUnlock());
+
     TransactionError err = m_wallet_model->wallet().fillPSBT(SIGHASH_ALL, true /* sign */, true /* bip32derivs */, &n_signed, m_transaction_data, complete);
 
     if (err != TransactionError::OK) {
@@ -81,7 +84,9 @@ void PSBTOperationsDialog::signTransaction()
 
     updateTransactionDisplay();
 
-    if (!complete && n_signed < 1) {
+    if (!complete && !ctx.isValid()) {
+        showStatus(tr("Cannot sign inputs while wallet is locked."), StatusLevel::WARN);
+    } else if (!complete && n_signed < 1) {
         showStatus(tr("Could not sign any more inputs."), StatusLevel::WARN);
     } else if (!complete) {
         showStatus(tr("Signed %1 inputs, but more signatures are still required.").arg(n_signed),
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -4420,7 +4420,7 @@ static RPCHelpMan walletprocesspsbt()
         HELP_REQUIRING_PASSPHRASE,
                 {
                     {"psbt", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction base64 string"},
-                    {"sign", RPCArg::Type::BOOL, RPCArg::Default{true}, "Also sign the transaction when updating"},
+                    {"sign", RPCArg::Type::BOOL, RPCArg::Default{true}, "Also sign the transaction when updating (requires wallet to be unlocked)"},
                     {"sighashtype", RPCArg::Type::STR, RPCArg::Default{"DEFAULT"}, "The signature hash type to sign with if not specified by the PSBT. Must be one of\n"
             "       \"DEFAULT\"\n"
             "       \"ALL\"\n"
@@ -4467,6 +4467,9 @@ static RPCHelpMan walletprocesspsbt()
     bool sign = request.params[1].isNull() ? true : request.params[1].get_bool();
     bool bip32derivs = request.params[3].isNull() ? true : request.params[3].get_bool();
     bool complete = true;
+
+    if (sign) EnsureWalletIsUnlocked(*pwallet);
+
     const TransactionError err{wallet.FillPSBT(psbtx, complete, nHashType, sign, bip32derivs)};
     if (err != TransactionError::OK) {
         throw JSONRPCTransactionError(err);
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
@@ -108,6 +108,16 @@ def run_test(self):
         psbtx = self.nodes[1].walletprocesspsbt(psbtx1)['psbt']
         assert_equal(psbtx1, psbtx)
 
+        # Node 0 should not be able to sign the transaction with the wallet is locked
+        self.nodes[0].encryptwallet("password")
+        assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first", self.nodes[0].walletprocesspsbt, psbtx)
+
+        # Node 0 should be able to process without signing though
+        unsigned_tx = self.nodes[0].walletprocesspsbt(psbtx, False)
+        assert_equal(unsigned_tx['complete'], False)
+
+        self.nodes[0].walletpassphrase(passphrase="password", timeout=1000000)
+
         # Sign the transaction and send
         signed_tx = self.nodes[0].walletprocesspsbt(psbtx)['psbt']
         final_tx = self.nodes[0].finalizepsbt(signed_tx)['hex']
