diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index ed0602594b7..32c0402d450 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -522,7 +522,9 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) questionString.append(tr("Warning: This may pay the additional fee by reducing change outputs or adding inputs, when necessary. It may add a new change output if one does not already exist. These changes may potentially leak privacy.")); } - auto confirmationDialog = new SendConfirmationDialog(tr("Confirm fee bump"), questionString, "", "", SEND_CONFIRM_DELAY, !m_wallet->privateKeysDisabled(), getOptionsModel()->getEnablePSBTControls(), nullptr); + const bool enable_send{!wallet().privateKeysDisabled() || wallet().hasExternalSigner()}; + const bool always_show_unsigned{getOptionsModel()->getEnablePSBTControls()}; + auto confirmationDialog = new SendConfirmationDialog(tr("Confirm fee bump"), questionString, "", "", SEND_CONFIRM_DELAY, enable_send, always_show_unsigned, nullptr); confirmationDialog->setAttribute(Qt::WA_DeleteOnClose); // TODO: Replace QDialog::exec() with safer QDialog::show(). const auto retval = static_cast(confirmationDialog->exec()); @@ -540,6 +542,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) // Short-circuit if we are returning a bumped transaction PSBT to clipboard if (retval == QMessageBox::Save) { + // "Create Unsigned" clicked PartiallySignedTransaction psbtx(mtx); bool complete = false; const TransactionError err = wallet().fillPSBT(SIGHASH_ALL, /*sign=*/false, /*bip32derivs=*/true, nullptr, psbtx, complete); @@ -555,7 +558,7 @@ bool WalletModel::bumpFee(uint256 hash, uint256& new_hash) return true; } - assert(!m_wallet->privateKeysDisabled()); + assert(!m_wallet->privateKeysDisabled() || wallet().hasExternalSigner()); // sign bumped transaction if (!m_wallet->signBumpTransaction(mtx)) { diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 6d7bb299cca..7343e8860bc 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -293,7 +293,22 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) { LOCK(wallet.cs_wallet); - return wallet.SignTransaction(mtx); + + if (wallet.IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { + // Make a blank psbt + PartiallySignedTransaction psbtx(mtx); + + // First fill transaction with our data without signing, + // so external signers are not asked to sign more than once. + bool complete; + wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, false /* sign */, true /* bip32derivs */); + const TransactionError err = wallet.FillPSBT(psbtx, complete, SIGHASH_ALL, true /* sign */, false /* bip32derivs */); + if (err != TransactionError::OK) return false; + complete = FinalizeAndExtractPSBT(psbtx, mtx); + return complete; + } else { + return wallet.SignTransaction(mtx); + } } Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransaction&& mtx, std::vector& errors, uint256& bumped_txid) diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index fb5e9a425e1..24bcae40e07 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -82,10 +82,10 @@ static UniValue FinishTransaction(const std::shared_ptr pwallet, const PartiallySignedTransaction psbtx(rawTx); // First fill transaction with our data without signing, - // so external signers are not asked sign more than once. + // so external signers are not asked to sign more than once. bool complete; - pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, false, true); - const TransactionError err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, true, false)}; + pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/false, /*bip32derivs=*/true); + const TransactionError err{pwallet->FillPSBT(psbtx, complete, SIGHASH_DEFAULT, /*sign=*/true, /*bip32derivs=*/false)}; if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } @@ -993,7 +993,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !want_psbt) { + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER) && !want_psbt) { throw JSONRPCError(RPC_WALLET_ERROR, "bumpfee is not available with wallets that have private keys disabled. Use psbtbumpfee instead."); } @@ -1071,6 +1071,9 @@ static RPCHelpMan bumpfee_helper(std::string method_name) // For psbtbumpfee, return the base64-encoded unsigned PSBT of the new transaction. if (!want_psbt) { if (!feebumper::SignTransaction(*pwallet, mtx)) { + if (pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { + throw JSONRPCError(RPC_WALLET_ERROR, "Transaction incomplete. Try psbtbumpfee instead."); + } throw JSONRPCError(RPC_WALLET_ERROR, "Can't sign transaction."); } @@ -1667,7 +1670,7 @@ RPCHelpMan walletcreatefundedpsbt() // Fill transaction with out data but don't sign bool bip32derivs = request.params[4].isNull() ? true : request.params[4].get_bool(); bool complete = true; - const TransactionError err{wallet.FillPSBT(psbtx, complete, 1, false, bip32derivs)}; + const TransactionError err{wallet.FillPSBT(psbtx, complete, 1, /*sign=*/false, /*bip32derivs=*/bip32derivs)}; if (err != TransactionError::OK) { throw JSONRPCTransactionError(err); } diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 9ab7154424e..0b63ba84910 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -13,6 +13,7 @@ import platform from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, ) @@ -169,7 +170,7 @@ class WalletSignerTest(BitcoinTestFramework): assert_equal(result[1], {'success': True}) assert_equal(mock_wallet.getwalletinfo()["txcount"], 1) dest = self.nodes[0].getnewaddress(address_type='bech32') - mock_psbt = mock_wallet.walletcreatefundedpsbt([], {dest:0.5}, 0, {}, True)['psbt'] + mock_psbt = mock_wallet.walletcreatefundedpsbt([], {dest:0.5}, 0, {'replaceable': True}, True)['psbt'] mock_psbt_signed = mock_wallet.walletprocesspsbt(psbt=mock_psbt, sign=True, sighashtype="ALL", bip32derivs=True) mock_psbt_final = mock_wallet.finalizepsbt(mock_psbt_signed["psbt"]) mock_tx = mock_psbt_final["hex"] @@ -209,6 +210,7 @@ class WalletSignerTest(BitcoinTestFramework): self.log.info('Test send using hww1') + # Don't broadcast transaction yet so the RPC returns the raw hex res = hww.send(outputs={dest:0.5},options={"add_to_wallet": False}) assert(res["complete"]) assert_equal(res["hex"], mock_tx) @@ -218,6 +220,25 @@ class WalletSignerTest(BitcoinTestFramework): res = hww.sendall(recipients=[{dest:0.5}, hww.getrawchangeaddress()],options={"add_to_wallet": False}) assert(res["complete"]) assert_equal(res["hex"], mock_tx) + # Broadcast transaction so we can bump the fee + hww.sendrawtransaction(res["hex"]) + + self.log.info('Prepare fee bumped mock PSBT') + + # Now that the transaction is broadcast, bump fee in mock wallet: + orig_tx_id = res["txid"] + mock_psbt_bumped = mock_wallet.psbtbumpfee(orig_tx_id)["psbt"] + mock_psbt_bumped_signed = mock_wallet.walletprocesspsbt(psbt=mock_psbt_bumped, sign=True, sighashtype="ALL", bip32derivs=True) + + with open(os.path.join(self.nodes[1].cwd, "mock_psbt"), "w", encoding="utf8") as f: + f.write(mock_psbt_bumped_signed["psbt"]) + + self.log.info('Test bumpfee using hww1') + + # Bump fee + res = hww.bumpfee(orig_tx_id) + assert_greater_than(res["fee"], res["origfee"]) + assert_equal(res["errors"], []) # # Handle error thrown by script # self.set_mock_result(self.nodes[4], "2")