From 6edefe559bf7cf4e72dce71b976d9b574d6e61cb Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 1 Aug 2025 11:06:54 +0200 Subject: [PATCH 01/55] wallet: don't import external keys at creation if blank There's no need to treat external signer wallets different in this regard. When the user sets the 'blank' flag, don't generate or import keys. For multisig setups that involve an external signer, it may be useful to start from a blank wallet and manually import descriptors. --- src/wallet/wallet.cpp | 8 ++++---- test/functional/wallet_signer.py | 7 +++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8d502049287d..5667633c7649 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3688,10 +3688,10 @@ void CWallet::SetupDescriptorScriptPubKeyMans() void CWallet::SetupWalletGeneration() { AssertLockHeld(cs_wallet); - // Skip setup for non-external-signer wallets that are either blank - // or have private keys disabled (not having private keys implies blank). - if (!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER) && - (IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET) || IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS))) { + // Skip setup for blank wallets. Non-external-signer wallets with disabled + // private keys also skip setup (not having private keys implies blank). + if (IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET) || + (!IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER) && IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS))) { return; } SetupDescriptorScriptPubKeyMans(); diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 896169d4276a..ba678888b0ad 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -72,6 +72,13 @@ def test_valid_signer(self): hww = self.nodes[1].get_wallet_rpc('hww') assert_equal(hww.getwalletinfo()["external_signer"], True) + # A blank external signer wallet does not auto-import any keys. + self.nodes[1].createwallet(wallet_name='hww_blank', disable_private_keys=True, external_signer=True, blank=True) + hww_blank = self.nodes[1].get_wallet_rpc('hww_blank') + assert_equal(hww_blank.getwalletinfo()["keypoolsize"], 0) + assert_equal(hww_blank.listdescriptors()["descriptors"], []) + self.nodes[1].unloadwallet('hww_blank') + # Flag can't be set afterwards (could be added later for non-blank descriptor based watch-only wallets) self.nodes[1].createwallet(wallet_name='not_hww', disable_private_keys=True, external_signer=False) not_hww = self.nodes[1].get_wallet_rpc('not_hww') From 507292e2eb4dca99fd2b919db1c96f589b654beb Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 1 Aug 2025 14:51:53 +0200 Subject: [PATCH 02/55] wallet: avoid signing via createTransaction() with external signer External signer enabled wallets should always use the process PSBT flow. Avoid going through CreateTransaction. This has no effect until a later commit where WALLET_FLAG_EXTERNAL_SIGNER no longer implies WALLET_FLAG_DISABLE_PRIVATE_KEYS. Without this change signing with the GUI would break for external signers with private keys enabled. --- src/qt/walletmodel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index a9142b930d56..5c7587808ab4 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -203,7 +203,7 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact try { auto& newTx = transaction.getWtx(); - const auto& res = m_wallet->createTransaction(vecSend, coinControl, /*sign=*/!wallet().privateKeysDisabled(), /*change_pos=*/std::nullopt); + const auto& res = m_wallet->createTransaction(vecSend, coinControl, /*sign=*/!wallet().privateKeysDisabled() && !wallet().hasExternalSigner(), /*change_pos=*/std::nullopt); if (!res) { Q_EMIT message(tr("Send Coins"), QString::fromStdString(util::ErrorString(res).translated), CClientUIInterface::MSG_ERROR); From b66b41c3b3a5af5e372affe45931130c8cf94cb0 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 1 Aug 2025 10:45:49 +0200 Subject: [PATCH 03/55] wallet: make watch-only optional for external signer Before this change the external_signer flag required the wallet to be watch-only. This precludes multisig setups in which we hold a hot key. Remove this as a requirement, but disable private keys by default. This leaves the typical (and only documented) use case of a single external signer unaffected. --- src/qt/createwalletdialog.cpp | 28 ++++++++-------------------- src/wallet/rpc/wallet.cpp | 19 ++++++++++++++----- src/wallet/wallet.cpp | 7 ------- test/functional/wallet_signer.py | 29 +++++++++++++++++++---------- 4 files changed, 41 insertions(+), 42 deletions(-) diff --git a/src/qt/createwalletdialog.cpp b/src/qt/createwalletdialog.cpp index 59c6f51a27ec..b73c9728240e 100644 --- a/src/qt/createwalletdialog.cpp +++ b/src/qt/createwalletdialog.cpp @@ -26,34 +26,25 @@ CreateWalletDialog::CreateWalletDialog(QWidget* parent) : }); connect(ui->encrypt_wallet_checkbox, &QCheckBox::toggled, [this](bool checked) { - // Disable the disable_privkeys_checkbox and external_signer_checkbox when isEncryptWalletChecked is + // Disable the disable_privkeys_checkbox when isEncryptWalletChecked is // set to true, enable it when isEncryptWalletChecked is false. ui->disable_privkeys_checkbox->setEnabled(!checked); -#ifdef ENABLE_EXTERNAL_SIGNER - ui->external_signer_checkbox->setEnabled(m_has_signers && !checked); -#endif + // When the disable_privkeys_checkbox is disabled, uncheck it. if (!ui->disable_privkeys_checkbox->isEnabled()) { ui->disable_privkeys_checkbox->setChecked(false); } - - // When the external_signer_checkbox box is disabled, uncheck it. - if (!ui->external_signer_checkbox->isEnabled()) { - ui->external_signer_checkbox->setChecked(false); - } - }); connect(ui->external_signer_checkbox, &QCheckBox::toggled, [this](bool checked) { - ui->encrypt_wallet_checkbox->setEnabled(!checked); - ui->blank_wallet_checkbox->setEnabled(!checked); - ui->disable_privkeys_checkbox->setEnabled(!checked); + // In the basic use case all keys will be on the external signer + // device and the wallet should be watch-only. Makes this the + // default suggestion. + ui->disable_privkeys_checkbox->setChecked(checked); - // The external signer checkbox is only enabled when a device is detected. - // In that case it is checked by default. Toggling it restores the other - // options to their default. + // The external signer box is checked by default when a device is + // detected. Toggling it restores the other options to their default. ui->encrypt_wallet_checkbox->setChecked(false); - ui->disable_privkeys_checkbox->setChecked(checked); ui->blank_wallet_checkbox->setChecked(false); }); @@ -103,12 +94,9 @@ void CreateWalletDialog::setSigners(const std::vectorexternal_signer_checkbox->setEnabled(true); ui->external_signer_checkbox->setChecked(true); - ui->encrypt_wallet_checkbox->setEnabled(false); ui->encrypt_wallet_checkbox->setChecked(false); // The order matters, because connect() is called when toggling a checkbox: - ui->blank_wallet_checkbox->setEnabled(false); ui->blank_wallet_checkbox->setChecked(false); - ui->disable_privkeys_checkbox->setEnabled(false); ui->disable_privkeys_checkbox->setChecked(true); const std::string label = signers[0]->getName(); ui->wallet_name_line_edit->setText(QString::fromStdString(label)); diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 67b2f9e33baa..743f0c292d36 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -352,13 +352,13 @@ static RPCMethod createwallet() "Creates and loads a new wallet.\n", { {"wallet_name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name for the new wallet. If this is a path, the wallet will be created at the path location."}, - {"disable_private_keys", RPCArg::Type::BOOL, RPCArg::Default{false}, "Disable the possibility of private keys (only watchonlys are possible in this mode)."}, + {"disable_private_keys", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false unless external_signer is set"}, "Disable the possibility of private keys (only watchonlys are possible in this mode)."}, {"blank", RPCArg::Type::BOOL, RPCArg::Default{false}, "Create a blank wallet. A blank wallet has no keys."}, {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Encrypt the wallet with this passphrase."}, {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{false}, "Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind."}, {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{true}, "If set, must be \"true\""}, {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, - {"external_signer", RPCArg::Type::BOOL, RPCArg::Default{false}, "Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true."}, + {"external_signer", RPCArg::Type::BOOL, RPCArg::Default{false}, "Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -380,9 +380,8 @@ static RPCMethod createwallet() { WalletContext& context = EnsureWalletContext(request.context); uint64_t flags = 0; - if (!request.params[1].isNull() && request.params[1].get_bool()) { - flags |= WALLET_FLAG_DISABLE_PRIVATE_KEYS; - } + + std::optional disable_private_keys{self.MaybeArg("disable_private_keys")}; if (!request.params[2].isNull() && request.params[2].get_bool()) { flags |= WALLET_FLAG_BLANK_WALLET; @@ -408,11 +407,21 @@ static RPCMethod createwallet() if (!request.params[7].isNull() && request.params[7].get_bool()) { #ifdef ENABLE_EXTERNAL_SIGNER flags |= WALLET_FLAG_EXTERNAL_SIGNER; + if (!disable_private_keys.has_value()) { + // In the basic use case all keys will be on the external signer + // device and the wallet should be watch-only. Makes this the + // default. + disable_private_keys = true; + } #else throw JSONRPCError(RPC_WALLET_ERROR, "Compiled without external signing support (required for external signing)"); #endif } + if (disable_private_keys.value_or(false)) { + flags |= WALLET_FLAG_DISABLE_PRIVATE_KEYS; + } + DatabaseOptions options; DatabaseStatus status; ReadDatabaseArgs(*context.args, options); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5667633c7649..f8b8bd6018d0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -393,13 +393,6 @@ std::shared_ptr CreateWallet(WalletContext& context, const std::string& options.require_format = DatabaseFormat::SQLITE; - // Private keys must be disabled for an external signer wallet - if ((wallet_creation_flags & WALLET_FLAG_EXTERNAL_SIGNER) && !(wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { - error = Untranslated("Private keys must be disabled when using an external signer"); - status = DatabaseStatus::FAILED_CREATE; - return nullptr; - } - // Do not allow a passphrase when private keys are disabled if (born_encrypted && (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { error = Untranslated("Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled."); diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index ba678888b0ad..1aa4e39579d3 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -66,29 +66,38 @@ def test_valid_signer(self): self.log.debug(f"-signer={self.mock_signer_path()}") # Create new wallets for an external signer. - # disable_private_keys and descriptors must be true: - assert_raises_rpc_error(-4, "Private keys must be disabled when using an external signer", self.nodes[1].createwallet, wallet_name='not_hww', disable_private_keys=False, external_signer=True) - self.nodes[1].createwallet(wallet_name='hww', disable_private_keys=True, external_signer=True) + self.nodes[1].createwallet(wallet_name='hww', external_signer=True) hww = self.nodes[1].get_wallet_rpc('hww') assert_equal(hww.getwalletinfo()["external_signer"], True) + # Private keys are disabled by default + assert_equal(hww.getwalletinfo()["private_keys_enabled"], False) + + # Private keys can be explicitly enabled for external signer wallets + self.nodes[1].createwallet(wallet_name='hww_hot', external_signer=True, disable_private_keys=False) + hww_hot = self.nodes[1].get_wallet_rpc('hww_hot') + assert_equal(hww_hot.getwalletinfo()["external_signer"], True) + assert_equal(hww_hot.getwalletinfo()["private_keys_enabled"], True) + self.nodes[1].unloadwallet('hww_hot') + # A blank external signer wallet does not auto-import any keys. - self.nodes[1].createwallet(wallet_name='hww_blank', disable_private_keys=True, external_signer=True, blank=True) + self.nodes[1].createwallet(wallet_name='hww_blank', external_signer=True, blank=True) hww_blank = self.nodes[1].get_wallet_rpc('hww_blank') assert_equal(hww_blank.getwalletinfo()["keypoolsize"], 0) assert_equal(hww_blank.listdescriptors()["descriptors"], []) self.nodes[1].unloadwallet('hww_blank') # Flag can't be set afterwards (could be added later for non-blank descriptor based watch-only wallets) - self.nodes[1].createwallet(wallet_name='not_hww', disable_private_keys=True, external_signer=False) + self.nodes[1].createwallet(wallet_name='not_hww', external_signer=False) not_hww = self.nodes[1].get_wallet_rpc('not_hww') assert_equal(not_hww.getwalletinfo()["external_signer"], False) + # Without external_signer, private keys are enabled by default + assert_equal(not_hww.getwalletinfo()["private_keys_enabled"], True) assert_raises_rpc_error(-8, "Wallet flag is immutable: external_signer", not_hww.setwalletflag, "external_signer", True) - self.set_mock_result(self.nodes[1], '0 {"invalid json"}') assert_raises_rpc_error(-1, 'Unable to parse JSON', - self.nodes[1].createwallet, wallet_name='hww2', disable_private_keys=True, external_signer=True + self.nodes[1].createwallet, wallet_name='hww2', external_signer=True ) self.clear_mock_result(self.nodes[1]) @@ -232,7 +241,7 @@ def test_disconnected_signer(self): self.log.info('Test disconnected external signer') # First create a wallet with the signer connected - self.nodes[1].createwallet(wallet_name='hww_disconnect', disable_private_keys=True, external_signer=True) + self.nodes[1].createwallet(wallet_name='hww_disconnect', external_signer=True) hww = self.nodes[1].get_wallet_rpc('hww_disconnect') assert_equal(hww.getwalletinfo()["external_signer"], True) @@ -253,13 +262,13 @@ def test_disconnected_signer(self): def test_invalid_signer(self): self.log.debug(f"-signer={self.mock_invalid_signer_path()}") self.log.info('Test invalid external signer') - assert_raises_rpc_error(-1, "Invalid descriptor", self.nodes[1].createwallet, wallet_name='hww_invalid', disable_private_keys=True, external_signer=True) + assert_raises_rpc_error(-1, "Invalid descriptor", self.nodes[1].createwallet, wallet_name='hww_invalid', external_signer=True) def test_multiple_signers(self): self.log.debug(f"-signer={self.mock_multi_signers_path()}") self.log.info('Test multiple external signers') - assert_raises_rpc_error(-1, "More than one external signer found", self.nodes[1].createwallet, wallet_name='multi_hww', disable_private_keys=True, external_signer=True) + assert_raises_rpc_error(-1, "More than one external signer found", self.nodes[1].createwallet, wallet_name='multi_hww', external_signer=True) if __name__ == '__main__': WalletSignerTest(__file__).main() From 3aa88b42a863c9101c7a7aca89cf1f951c40cb9c Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 1 Aug 2025 10:46:50 +0200 Subject: [PATCH 04/55] wallet: make external_signer flag mutable With the removal of legacy wallets and the relaxing of restrictions in the previous commit, it's no longer a problem to toggle this flag. --- src/wallet/wallet.h | 3 ++- test/functional/wallet_signer.py | 13 +++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 47701d7e3a90..342802e91d32 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -156,7 +156,8 @@ static constexpr uint64_t KNOWN_WALLET_FLAGS = | WALLET_FLAG_EXTERNAL_SIGNER; static constexpr uint64_t MUTABLE_WALLET_FLAGS = - WALLET_FLAG_AVOID_REUSE; + WALLET_FLAG_AVOID_REUSE + | WALLET_FLAG_EXTERNAL_SIGNER; static const std::map WALLET_FLAG_TO_STRING{ {WALLET_FLAG_AVOID_REUSE, "avoid_reuse"}, diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index 1aa4e39579d3..aa285d79e0d0 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -87,13 +87,14 @@ def test_valid_signer(self): assert_equal(hww_blank.listdescriptors()["descriptors"], []) self.nodes[1].unloadwallet('hww_blank') - # Flag can't be set afterwards (could be added later for non-blank descriptor based watch-only wallets) - self.nodes[1].createwallet(wallet_name='not_hww', external_signer=False) - not_hww = self.nodes[1].get_wallet_rpc('not_hww') - assert_equal(not_hww.getwalletinfo()["external_signer"], False) + # Flag can be set afterwards + self.nodes[1].createwallet(wallet_name='not_hww_initially', external_signer=False) + not_hww_initially = self.nodes[1].get_wallet_rpc('not_hww_initially') + assert_equal(not_hww_initially.getwalletinfo()["external_signer"], False) # Without external_signer, private keys are enabled by default - assert_equal(not_hww.getwalletinfo()["private_keys_enabled"], True) - assert_raises_rpc_error(-8, "Wallet flag is immutable: external_signer", not_hww.setwalletflag, "external_signer", True) + assert_equal(not_hww_initially.getwalletinfo()["private_keys_enabled"], True) + not_hww_initially.setwalletflag("external_signer", True) + assert_equal(not_hww_initially.getwalletinfo()["external_signer"], True) self.set_mock_result(self.nodes[1], '0 {"invalid json"}') assert_raises_rpc_error(-1, 'Unable to parse JSON', From e13c29fd4360a74efa7bca8e28ab960e4b3116c4 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 6 May 2026 11:03:37 +0200 Subject: [PATCH 05/55] test: move mock signer path helper to the test framework Both rpc_signer.py and wallet_signer.py defined identical mock_signer_path() helpers; the upcoming wallet_signer_musig2.py test needs the same helper. Move it to BitcoinTestFramework. --- test/functional/rpc_signer.py | 5 ---- .../test_framework/test_framework.py | 5 ++++ test/functional/wallet_signer.py | 29 ++++--------------- 3 files changed, 11 insertions(+), 28 deletions(-) diff --git a/test/functional/rpc_signer.py b/test/functional/rpc_signer.py index 51a012494d5b..812edcaf28aa 100755 --- a/test/functional/rpc_signer.py +++ b/test/functional/rpc_signer.py @@ -9,7 +9,6 @@ """ import os import platform -import sys from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -19,10 +18,6 @@ class RPCSignerTest(BitcoinTestFramework): - def mock_signer_path(self): - path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'signer.py') - return sys.executable + " " + path - def set_test_params(self): self.num_nodes = 4 diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 64dcbfd7ec58..b44bab0120f0 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1107,6 +1107,11 @@ def skip_if_no_external_signer(self): if not self.is_external_signer_compiled(): raise SkipTest("external signer support has not been compiled.") + def mock_signer_path(self, name='signer.py'): + """Return a command that invokes a mock external signer script under test/functional/mocks/.""" + path = os.path.join(os.path.dirname(os.path.realpath(__file__)), '..', 'mocks', name) + return sys.executable + " " + os.path.realpath(path) + def skip_if_running_under_valgrind(self): """Skip the running test if Valgrind is being used.""" if self.options.valgrind: diff --git a/test/functional/wallet_signer.py b/test/functional/wallet_signer.py index aa285d79e0d0..556565ce289b 100755 --- a/test/functional/wallet_signer.py +++ b/test/functional/wallet_signer.py @@ -8,7 +8,6 @@ See also rpc_signer.py for tests without wallet context. """ import os -import sys from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -19,22 +18,6 @@ class WalletSignerTest(BitcoinTestFramework): - def mock_signer_path(self): - path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'signer.py') - return sys.executable + " " + path - - def mock_no_connected_signer_path(self): - path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'no_signer.py') - return sys.executable + " " + path - - def mock_invalid_signer_path(self): - path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'invalid_signer.py') - return sys.executable + " " + path - - def mock_multi_signers_path(self): - path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'mocks', 'multi_signers.py') - return sys.executable + " " + path - def set_test_params(self): self.num_nodes = 2 @@ -57,9 +40,9 @@ def clear_mock_result(self, node): def run_test(self): self.test_valid_signer() self.test_disconnected_signer() - self.restart_node(1, [f"-signer={self.mock_invalid_signer_path()}", "-keypool=10"]) + self.restart_node(1, [f"-signer={self.mock_signer_path('invalid_signer.py')}", "-keypool=10"]) self.test_invalid_signer() - self.restart_node(1, [f"-signer={self.mock_multi_signers_path()}", "-keypool=10"]) + self.restart_node(1, [f"-signer={self.mock_signer_path('multi_signers.py')}", "-keypool=10"]) self.test_multiple_signers() def test_valid_signer(self): @@ -251,8 +234,8 @@ def test_disconnected_signer(self): self.generate(self.nodes[0], 1) # Restart node with no signer connected - self.log.debug(f"-signer={self.mock_no_connected_signer_path()}") - self.restart_node(1, [f"-signer={self.mock_no_connected_signer_path()}", "-keypool=10"]) + self.log.debug(f"-signer={self.mock_signer_path('no_signer.py')}") + self.restart_node(1, [f"-signer={self.mock_signer_path('no_signer.py')}", "-keypool=10"]) self.nodes[1].loadwallet('hww_disconnect') hww = self.nodes[1].get_wallet_rpc('hww_disconnect') @@ -261,12 +244,12 @@ def test_disconnected_signer(self): assert_raises_rpc_error(-25, "External signer not found", hww.send, outputs=[{dest:0.5}]) def test_invalid_signer(self): - self.log.debug(f"-signer={self.mock_invalid_signer_path()}") + self.log.debug(f"-signer={self.mock_signer_path('invalid_signer.py')}") self.log.info('Test invalid external signer') assert_raises_rpc_error(-1, "Invalid descriptor", self.nodes[1].createwallet, wallet_name='hww_invalid', external_signer=True) def test_multiple_signers(self): - self.log.debug(f"-signer={self.mock_multi_signers_path()}") + self.log.debug(f"-signer={self.mock_signer_path('multi_signers.py')}") self.log.info('Test multiple external signers') assert_raises_rpc_error(-1, "More than one external signer found", self.nodes[1].createwallet, wallet_name='multi_hww', external_signer=True) From 10aa22325dc52400c7d12a291f597fb2d88c96ac Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 1 May 2026 12:33:36 +0200 Subject: [PATCH 06/55] test: add MuSig2 external signer wallet test --- test/functional/mocks/signer.py | 1 + test/functional/test_runner.py | 1 + test/functional/wallet_signer_musig2.py | 89 +++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100755 test/functional/wallet_signer_musig2.py diff --git a/test/functional/mocks/signer.py b/test/functional/mocks/signer.py index a13c97d1216f..f3734b4629b7 100755 --- a/test/functional/mocks/signer.py +++ b/test/functional/mocks/signer.py @@ -49,6 +49,7 @@ def displayaddress(args): "sh(wpkh([00000001/49h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7))#kz9y5w82": "2N2gQKzjUe47gM8p1JZxaAkTcoHPXV6YyVp", "pkh([00000001/44h/1h/0h/0/0]02c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#q3pqd8wh": "n1LKejAadN6hg2FrBXoU1KrwX4uK16mco9", "tr([00000001/86h/1h/0h/0/0]c97dc3f4420402e01a113984311bf4a1b8de376cac0bdcfaf1b3ac81f13433c7)#puqqa90m": "tb1phw4cgpt6cd30kz9k4wkpwm872cdvhss29jga2xpmftelhqll62mscq0k4g", + "tr([296d4e60/0/0]d5b810749f280dac8114cdb1c44fdcd6a725cb73669477b478f2172f0b32129a)#7luf7va9": "bcrt1p6s0wuy6g4ggh4qds9cju533wve82fnazzp4xkpk3p780gz30m5us9dqyaa", "wpkh([00000001/84h/1h/0h/0/1]03a20a46308be0b8ded6dff0a22b10b4245c587ccf23f3b4a303885be3a524f172)#aqpjv5xr": "wrong_address", } if args.desc not in expected_desc: diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 5bfc7d86239b..7b5e6f86a41a 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -158,6 +158,7 @@ 'p2p_timeouts.py --v2transport', 'rpc_signer.py', 'wallet_signer.py', + 'wallet_signer_musig2.py', 'mempool_limit.py', 'rpc_txoutproof.py', 'rpc_orphans.py', diff --git a/test/functional/wallet_signer_musig2.py b/test/functional/wallet_signer_musig2.py new file mode 100755 index 000000000000..d925bc656eef --- /dev/null +++ b/test/functional/wallet_signer_musig2.py @@ -0,0 +1,89 @@ +#!/usr/bin/env python3 +# Copyright (c) 2026-present The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test MuSig2 descriptors in an external-signer wallet.""" + +from test_framework.descriptors import descsum_create +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal + + +# The device side of the MuSig2 setup. +DEVICE_ORIGIN = "[00000001/84h/1h/0h]" +DEVICE_XPUB = "tpubDCxzhZZE31g2EqSv1UajMAw5Hd62htydz9r2XBkrccHgBh8uw3n62zr6Zjmj64tfTk8Tjxo6VctjUMAh5DXWTErfQPC6RmQhTdtNnXuTXTQ" + +# Hardcode the local MuSig participant's account xprv so the test can import a +# descriptor with one hot-wallet key and one external-signer key. +LOCAL_ORIGIN = "[ec63add0/84h/1h/0h]" +LOCAL_XPRV = "tprv8gauGKtmnH4cxv22ZtmEKqDDAeqnm2b4srPWuFsugMuVc79KDEHRTWgKGdAhACqjZQytU1o9gcc91TSW8L1s18PgFUHAJ8p8iY1GwaUEn9u" + +# The hot wallet in this test does not contain an HD key. After +# bitcoin/bitcoin#29136 it could, and after bitcoin/bitcoin#32784 we could +# derive the 84h/1h/0h derivation xprv. A subsequent improvement to +# importdescriptors could avoid the need to handle xprvs, by recognizing +# an xpub for which it already has the matching private key material. + +class WalletSignerMuSig2Test(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 1 + self.extra_args = [ + [f"-signer={self.mock_signer_path()}", '-keypool=10'], + ] + + def skip_test_if_missing_module(self): + self.skip_if_no_external_signer() + self.skip_if_no_wallet() + + def run_test(self): + self.test_create_wallet() + self.test_display_address() + + def test_create_wallet(self): + self.log.info('Create an external-signer wallet with a MuSig2 descriptor') + + musig_descriptor = f"tr(musig({LOCAL_ORIGIN}{LOCAL_XPRV},{DEVICE_ORIGIN}{DEVICE_XPUB})/<0;1>/*)" + + # Blank wallet so createwallet doesn't import the device's single-sig + # descriptors (which would clutter the wallet). + self.nodes[0].createwallet( + wallet_name='hww_musig', + external_signer=True, + disable_private_keys=False, + blank=True, + ) + hww_musig = self.nodes[0].get_wallet_rpc('hww_musig') + + result = hww_musig.importdescriptors([{ + "desc": descsum_create(musig_descriptor), + "active": True, + "timestamp": "now", + }]) + assert_equal(result[0]["success"], True) + + # Reload so the imported descriptor is managed by + # ExternalSignerScriptPubKeyMan rather than the plain + # DescriptorScriptPubKeyMan it was added to on import. + self.nodes[0].unloadwallet('hww_musig') + self.nodes[0].loadwallet('hww_musig') + + descs = self.nodes[0].get_wallet_rpc('hww_musig').listdescriptors()["descriptors"] + active_musig = [d for d in descs if d["active"] and d["desc"].startswith("tr(musig(")] + # One active descriptor each for receive and change. + assert_equal(len(active_musig), 2) + + def test_display_address(self): + self.log.info('Display an address from the MuSig2 descriptor') + hww_musig = self.nodes[0].get_wallet_rpc('hww_musig') + + addr = hww_musig.getnewaddress(address_type="bech32m") + addr_info = hww_musig.getaddressinfo(addr) + assert_equal(addr_info["ismine"], True) + assert_equal(addr_info["solvable"], True) + # This is not expected to work on a real device, which needs additional + # information such as a BIP388 policy. + assert_equal(hww_musig.walletdisplayaddress(addr), {"address": addr}) + + +if __name__ == '__main__': + WalletSignerMuSig2Test(__file__).main() From 3dfe347989753df0772692c3a90f61a2dd8152fa Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 4 Jul 2025 15:38:13 +0200 Subject: [PATCH 07/55] wallet: add option to avoid script path spends --- src/common/types.h | 5 +++++ src/psbt.cpp | 2 +- src/script/sign.cpp | 8 ++++++++ src/script/sign.h | 3 +++ src/wallet/wallet.h | 1 + 5 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/common/types.h b/src/common/types.h index b9ebca15e84f..338435197466 100644 --- a/src/common/types.h +++ b/src/common/types.h @@ -49,6 +49,11 @@ struct PSBTFillOptions { * Whether to fill in bip32 derivation information if available. */ bool bip32_derivs{true}; + + /** + * Only sign the key path (for taproot inputs). + */ + bool taproot_keypath_only{false}; }; } // namespace common diff --git a/src/psbt.cpp b/src/psbt.cpp index 8f2e9ab16f3e..e9ef6564c865 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -730,7 +730,7 @@ PSBTError SignPSBTInput(const SigningProvider& provider, PartiallySignedTransact if (txdata == nullptr) { sig_complete = ProduceSignature(provider, DUMMY_SIGNATURE_CREATOR, utxo.scriptPubKey, sigdata); } else { - MutableTransactionSignatureCreator creator(tx, index, utxo.nValue, txdata, {.sighash_type = sighash}); + MutableTransactionSignatureCreator creator(tx, index, utxo.nValue, txdata, {.sighash_type = sighash, .taproot_keypath_only = options.taproot_keypath_only}); sig_complete = ProduceSignature(provider, creator, utxo.scriptPubKey, sigdata); } // Verify that a witness signature was produced in case one was required. diff --git a/src/script/sign.cpp b/src/script/sign.cpp index f38ac2bee8e1..d83d3b7e7588 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -559,9 +559,13 @@ static bool SignTaproot(const SigningProvider& provider, const BaseSignatureCrea { TaprootSpendData spenddata; TaprootBuilder builder; + const bool taproot_keypath_only{creator.Options().taproot_keypath_only}; // Gather information about this output. if (provider.GetTaprootSpendData(output, spenddata)) { + // Avoid merging newly provided script path data. Existing taproot + // script path fields in sigdata (e.g. from a PSBT) are left intact. + if (taproot_keypath_only) spenddata.scripts.clear(); sigdata.tr_spenddata.Merge(spenddata); } if (provider.GetTaprootBuilder(output, builder)) { @@ -613,6 +617,10 @@ static bool SignTaproot(const SigningProvider& provider, const BaseSignatureCrea } } + // Key path signing failed. In keypath-only mode, stop here instead of + // attempting a script path signature. + if (taproot_keypath_only) return false; + // Try script path spending. std::vector> smallest_result_stack; for (const auto& [key, control_blocks] : sigdata.tr_spenddata.scripts) { diff --git a/src/script/sign.h b/src/script/sign.h index 107abb9e8492..558d7749f3f8 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -33,6 +33,7 @@ struct SignatureData; struct SignOptions { int sighash_type{SIGHASH_DEFAULT}; + bool taproot_keypath_only{false}; }; /** Interface for signature creators. */ @@ -40,6 +41,7 @@ class BaseSignatureCreator { public: virtual ~BaseSignatureCreator() = default; virtual const BaseSignatureChecker& Checker() const =0; + virtual SignOptions Options() const { return {}; } /** Create a singular (non-script) signature. */ virtual bool CreateSig(const SigningProvider& provider, std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const =0; @@ -65,6 +67,7 @@ class MutableTransactionSignatureCreator : public BaseSignatureCreator MutableTransactionSignatureCreator(const CMutableTransaction& tx LIFETIMEBOUND, unsigned int input_idx, const CAmount& amount, const SignOptions& options); MutableTransactionSignatureCreator(const CMutableTransaction& tx LIFETIMEBOUND, unsigned int input_idx, const CAmount& amount, const PrecomputedTransactionData* txdata, const SignOptions& options); const BaseSignatureChecker& Checker() const override { return checker; } + SignOptions Options() const override { return m_options; } bool CreateSig(const SigningProvider& provider, std::vector& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override; bool CreateSchnorrSig(const SigningProvider& provider, std::vector& sig, const XOnlyPubKey& pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion) const override; std::vector CreateMuSig2Nonce(const SigningProvider& provider, const CPubKey& aggregate_pubkey, const CPubKey& script_pubkey, const CPubKey& part_pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion, const SignatureData& sigdata) const override; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 184603bc2aaf..f2f25b991cd1 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -132,6 +132,7 @@ static const bool DEFAULT_WALLET_RBF = true; static const bool DEFAULT_WALLETBROADCAST = true; static const bool DEFAULT_DISABLE_WALLET = false; static const bool DEFAULT_WALLETCROSSCHAIN = false; +static constexpr bool DEFAULT_SIGN_TAPROOT_KEYPATH_ONLY{false}; //! -maxtxfee default constexpr CAmount DEFAULT_TRANSACTION_MAXFEE{COIN / 10}; //! Discourage users to set fees higher than this amount (in satoshis) per kB From 5e312ffed8b0ea2f2a3e182ad4a28ae9f392a11a Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 3 Jul 2025 14:37:15 +0200 Subject: [PATCH 08/55] rpc: add keypath_only to walletprocesspsbt --- src/rpc/client.cpp | 1 + src/wallet/rpc/spend.cpp | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 4741df8a9470..ee2b0a56d03e 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -214,6 +214,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "walletprocesspsbt", 2, "sighashtype", ParamFormat::STRING }, { "walletprocesspsbt", 3, "bip32derivs" }, { "walletprocesspsbt", 4, "finalize" }, + { "walletprocesspsbt", 5, "keypath_only"}, { "descriptorprocesspsbt", 0, "psbt", ParamFormat::STRING }, { "descriptorprocesspsbt", 1, "descriptors"}, { "descriptorprocesspsbt", 2, "sighashtype", ParamFormat::STRING }, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 352340d212b5..604dbf7be5a0 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1610,6 +1610,7 @@ RPCMethod walletprocesspsbt() " \"SINGLE|ANYONECANPAY\""}, {"bip32derivs", RPCArg::Type::BOOL, RPCArg::Default{true}, "Include BIP 32 derivation paths for public keys if we know them"}, {"finalize", RPCArg::Type::BOOL, RPCArg::Default{true}, "Also finalize inputs if possible"}, + {"keypath_only", RPCArg::Type::BOOL, RPCArg::Default{DEFAULT_SIGN_TAPROOT_KEYPATH_ONLY}, "Only sign the key path (for taproot inputs)."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -1646,11 +1647,13 @@ RPCMethod 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 finalize = request.params[4].isNull() ? true : request.params[4].get_bool(); + bool keypath_only{request.params[5].isNull() ? DEFAULT_SIGN_TAPROOT_KEYPATH_ONLY : request.params[5].get_bool()}; + bool complete = true; if (sign) EnsureWalletIsUnlocked(*pwallet); - const auto err{wallet.FillPSBT(psbtx, {.sign = sign, .sighash_type = nHashType, .finalize = finalize, .bip32_derivs = bip32derivs}, complete)}; + const auto err{wallet.FillPSBT(psbtx, {.sign = sign, .sighash_type = nHashType, .finalize = finalize, .bip32_derivs = bip32derivs, .taproot_keypath_only = keypath_only}, complete)}; if (err) { throw JSONRPCPSBTError(*err); } From 9460402e907e16302fab9842afcf542c0a76ae12 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 3 Jul 2025 14:36:42 +0200 Subject: [PATCH 09/55] test: cover keypath_only in wallet_taproot.py Expand taproot tests to cover avoid_script_path in walletprocesspsbt. When avoiding script paths, there's no need for the workaround that increases fee_rate to compensate for the wallet's inability to estimate fees for script path spends. We use this to indirectly test that key path was used. We also check that taproot_script_path_sigs is not set. Finally, for transactions that can't be signed using their key path, we try again by allowing the script path. Additional test extended private keys were extracted from other tests. Co-authored-by: rkrux --- test/functional/wallet_taproot.py | 38 +++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py index b312d4e1536d..35c24bcc08b1 100755 --- a/test/functional/wallet_taproot.py +++ b/test/functional/wallet_taproot.py @@ -171,8 +171,8 @@ def do_test_sendtoaddress(self, comment, pattern, privmap, treefn, keys_pay, key assert rpc_online.gettransaction(txid)["confirmations"] > 0 rpc_online.unloadwallet() - def do_test_psbt(self, comment, pattern, privmap, treefn, keys_pay, keys_change): - self.log.info("Testing %s through PSBT" % comment) + def do_test_psbt(self, comment, pattern, privmap, treefn, keys_pay, keys_change, keypath_only): + self.log.info(f"Testing {comment} through PSBT { '(key path only)' if keypath_only else '' }") # Create wallets wallet_uuid = uuid.uuid4().hex @@ -212,12 +212,16 @@ def do_test_psbt(self, comment, pattern, privmap, treefn, keys_pay, keys_change) self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) test_balance = int(psbt_online.getbalance() * 100000000) ret_amnt = random.randrange(100000, test_balance) - # Increase fee_rate to compensate for the wallet's inability to estimate fees for script path spends. - psbt = psbt_online.walletcreatefundedpsbt([], [{self.boring.getnewaddress(): Decimal(ret_amnt) / 100000000}], None, {"subtractFeeFromOutputs":[0], "fee_rate": 200, "change_type": address_type})['psbt'] - res = psbt_offline.walletprocesspsbt(psbt=psbt, finalize=False) + fee_rate = 1 + if not keypath_only: + # Increase fee_rate to compensate for the wallet's inability to estimate fees for script path spends. + fee_rate = 200 + psbt = psbt_online.walletcreatefundedpsbt([], [{self.boring.getnewaddress(): Decimal(ret_amnt) / 100000000}], None, {"subtractFeeFromOutputs":[0], "fee_rate": fee_rate, "change_type": address_type})['psbt'] + res = psbt_offline.walletprocesspsbt(psbt=psbt, finalize=False, keypath_only=keypath_only) for wallet in [psbt_offline, key_only_wallet]: - res = wallet.walletprocesspsbt(psbt=psbt, finalize=False) + res = wallet.walletprocesspsbt(psbt=psbt, finalize=False, keypath_only=keypath_only) + retry = False decoded = wallet.decodepsbt(res["psbt"]) if pattern.startswith("tr("): for psbtin in decoded["inputs"]: @@ -225,13 +229,25 @@ def do_test_psbt(self, comment, pattern, privmap, treefn, keys_pay, keys_change) assert "witness_utxo" in psbtin assert "taproot_internal_key" in psbtin assert "taproot_bip32_derivs" in psbtin - assert "taproot_key_path_sig" in psbtin or "taproot_script_path_sigs" in psbtin + if keypath_only: + assert "taproot_script_path_sigs" not in psbtin + if "taproot_key_path_sig" not in psbtin: + retry = True + else: + assert "taproot_key_path_sig" in psbtin or "taproot_script_path_sigs" in psbtin if "taproot_script_path_sigs" in psbtin: assert "taproot_merkle_root" in psbtin assert "taproot_scripts" in psbtin + if retry: + self.log.debug("Retry with script path") + fee_rate = 200 + psbt = psbt_online.walletcreatefundedpsbt([], [{self.boring.getnewaddress(): Decimal(ret_amnt) / 100000000}], None, {"subtractFeeFromOutputs":[0], "fee_rate": fee_rate, "change_type": address_type})['psbt'] + res = wallet.walletprocesspsbt(psbt=psbt, finalize=False, keypath_only=False) + rawtx = self.nodes[0].finalizepsbt(res['psbt'])['hex'] res = self.nodes[0].testmempoolaccept([rawtx]) + self.log.debug(res) assert res[0]["allowed"] txid = self.nodes[0].sendrawtransaction(rawtx) @@ -250,13 +266,15 @@ def do_test_psbt(self, comment, pattern, privmap, treefn, keys_pay, keys_change) def do_test(self, comment, pattern, privmap, treefn): nkeys = len(privmap) - keys = random.sample(self.keys, nkeys * 4) + keys = random.sample(self.keys, nkeys * 6) self.do_test_addr(comment, pattern, privmap, treefn, keys[0:nkeys]) self.do_test_sendtoaddress(comment, pattern, privmap, treefn, keys[0:nkeys], keys[nkeys:2*nkeys]) - self.do_test_psbt(comment, pattern, privmap, treefn, keys[2*nkeys:3*nkeys], keys[3*nkeys:4*nkeys]) + self.do_test_psbt(comment, pattern, privmap, treefn, keys[2*nkeys:3*nkeys], keys[3*nkeys:4*nkeys], keypath_only=False) + if 'tr' in pattern: + self.do_test_psbt(comment, pattern, privmap, treefn, keys[4*nkeys:5*nkeys], keys[5*nkeys:6*nkeys], keypath_only=True) def generate_test_keys(self): - xprvs = [ExtendedPrivateKey.generate() for _ in range(0, 13)] + xprvs = [ExtendedPrivateKey.generate() for _ in range(0, 18)] return [{ "xprv": xprv.to_string(), "xpub": xprv.pubkey().to_string(), From 201da29135738100921587429f2b1b2add4ce1ad Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Thu, 3 Jul 2025 14:43:05 +0200 Subject: [PATCH 10/55] rpc: add keypath_only to send and sendall --- src/rpc/client.cpp | 2 ++ src/wallet/rpc/spend.cpp | 9 ++++++-- test/functional/wallet_taproot.py | 34 +++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index ee2b0a56d03e..d3941b5a3794 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -267,6 +267,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "send", 4, "replaceable"}, { "send", 4, "solving_data"}, { "send", 4, "max_tx_weight"}, + { "send", 4, "keypath_only"}, { "send", 5, "version"}, { "sendall", 0, "recipients" }, { "sendall", 1, "conf_target" }, @@ -282,6 +283,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendall", 4, "send_max"}, { "sendall", 4, "minconf"}, { "sendall", 4, "maxconf"}, + { "sendall", 4, "keypath_only"}, { "sendall", 4, "conf_target"}, { "sendall", 4, "replaceable"}, { "sendall", 4, "solving_data"}, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 604dbf7be5a0..a93dcb8545fa 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -111,11 +111,13 @@ static UniValue FinishTransaction(const std::shared_ptr pwallet, const // Make a blank psbt PartiallySignedTransaction psbtx(rawTx, /*version=*/2); + bool keypath_only{options.exists("keypath_only") ? options["keypath_only"].get_bool() : DEFAULT_SIGN_TAPROOT_KEYPATH_ONLY}; + // First fill transaction with our data without signing, // so external signers are not asked to sign more than once. bool complete; - pwallet->FillPSBT(psbtx, {.sign = false, .bip32_derivs = true}, complete); - const auto err{pwallet->FillPSBT(psbtx, {.sign = true, .bip32_derivs = false}, complete)}; + pwallet->FillPSBT(psbtx, {.sign = false, .bip32_derivs = true, .taproot_keypath_only = keypath_only}, complete); + const auto err{pwallet->FillPSBT(psbtx, {.sign = true, .bip32_derivs = false, .taproot_keypath_only = keypath_only}, complete)}; if (err) { throw JSONRPCPSBTError(*err); } @@ -499,6 +501,7 @@ CreatedTransactionResult FundTransaction(CWallet& wallet, const CMutableTransact {"includeWatching", UniValueType(UniValue::VBOOL)}, {"include_watching", UniValueType(UniValue::VBOOL)}, {"inputs", UniValueType(UniValue::VARR)}, + {"keypath_only", UniValueType(UniValue::VBOOL)}, {"lockUnspents", UniValueType(UniValue::VBOOL)}, {"lock_unspents", UniValueType(UniValue::VBOOL)}, {"locktime", UniValueType(UniValue::VNUM)}, @@ -1224,6 +1227,7 @@ RPCMethod send() }}, }, }, + {"keypath_only", RPCArg::Type::BOOL, RPCArg::Default{DEFAULT_SIGN_TAPROOT_KEYPATH_ONLY}, "Only sign the key path (for taproot inputs)."}, {"locktime", RPCArg::Type::NUM, RPCArg::DefaultHint{"locktime close to block height to prevent fee sniping"}, "Raw locktime. Non-0 value also locktime-activates inputs"}, {"lock_unspents", RPCArg::Type::BOOL, RPCArg::Default{false}, "Lock selected unspent outputs"}, {"psbt", RPCArg::Type::BOOL, RPCArg::DefaultHint{"automatic"}, "Always return a PSBT, implies add_to_wallet=false."}, @@ -1348,6 +1352,7 @@ RPCMethod sendall() {"send_max", RPCArg::Type::BOOL, RPCArg::Default{false}, "When true, only use UTXOs that can pay for their own fees to maximize the output amount. When 'false' (default), no UTXO is left behind. send_max is incompatible with providing specific inputs."}, {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "Require inputs with at least this many confirmations."}, {"maxconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "Require inputs with at most this many confirmations."}, + {"keypath_only", RPCArg::Type::BOOL, RPCArg::Default{DEFAULT_SIGN_TAPROOT_KEYPATH_ONLY}, "Only sign the key path (for taproot inputs)."}, {"version", RPCArg::Type::NUM, RPCArg::Default{DEFAULT_WALLET_TX_VERSION}, "Transaction version"}, }, FundTxDoc() diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py index 35c24bcc08b1..a2452a9f3f91 100755 --- a/test/functional/wallet_taproot.py +++ b/test/functional/wallet_taproot.py @@ -254,6 +254,40 @@ def do_test_psbt(self, comment, pattern, privmap, treefn, keys_pay, keys_change, self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op) assert psbt_online.gettransaction(txid)['confirmations'] > 0 + if keypath_only: + self.log.info("Testing send with keypath_only") + + def assert_no_script_path(psbt): + decoded = psbt_online.decodepsbt(psbt) + for psbtin in decoded["inputs"]: + assert "taproot_script_path_sigs" not in psbtin + + test_balance = int(psbt_online.getbalance() * 100000000) + outputs = {self.boring.getnewaddress(): Decimal(test_balance // 2) / 100000000} + res = psbt_online.send( + outputs=outputs, + options={ + "keypath_only": True, + "psbt": True, + "add_to_wallet": False, + "fee_rate": 1, + "subtract_fee_from_outputs": [0], + }, + ) + assert_equal(res["complete"], False) + assert_no_script_path(res["psbt"]) + + self.log.info("Testing sendall with keypath_only") + res = psbt_online.sendall( + recipients=[self.boring.getnewaddress()], + keypath_only=True, + psbt=True, + add_to_wallet=False, + fee_rate=1, + ) + assert_equal(res["complete"], False) + assert_no_script_path(res["psbt"]) + # Cleanup psbt = psbt_online.sendall(recipients=[self.boring.getnewaddress()], psbt=True)["psbt"] res = psbt_offline.walletprocesspsbt(psbt=psbt, finalize=False) From 0a89ada4d155789c862f6c0d72d77929a7658874 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 8 Jun 2026 16:36:55 +0200 Subject: [PATCH 11/55] rpc: add keypath_only to descriptorprocesspsbt --- src/rpc/client.cpp | 1 + src/rpc/rawtransaction.cpp | 21 ++++++++++++--- test/functional/rpc_psbt.py | 51 +++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index d3941b5a3794..9361c332f219 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -220,6 +220,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "descriptorprocesspsbt", 2, "sighashtype", ParamFormat::STRING }, { "descriptorprocesspsbt", 3, "bip32derivs" }, { "descriptorprocesspsbt", 4, "finalize" }, + { "descriptorprocesspsbt", 5, "keypath_only"}, { "createpsbt", 0, "inputs" }, { "createpsbt", 1, "outputs" }, { "createpsbt", 2, "locktime" }, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 31a877b8f048..169d9ac22fb7 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -125,7 +125,13 @@ static std::vector CreateTxDoc() // Update PSBT with information from the mempool, the UTXO set, the txindex, and the provided descriptors. // Optionally, sign the inputs that we can using information from the descriptors. -PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std::any& context, const HidingSigningProvider& provider, std::optional sighash_type, bool finalize) +PartiallySignedTransaction ProcessPSBT( + const std::string& psbt_string, + const std::any& context, + const HidingSigningProvider& provider, + std::optional sighash_type, + bool finalize, + bool taproot_keypath_only) { // Unserialize the transactions util::Result psbt_res = DecodeBase64PSBT(psbt_string); @@ -196,7 +202,10 @@ PartiallySignedTransaction ProcessPSBT(const std::string& psbt_string, const std // We only actually care about those if our signing provider doesn't hide private // information, as is the case with `descriptorprocesspsbt` // Only error for mismatching sighash types as it is critical that the sighash to sign with matches the PSBT's - if (SignPSBTInput(provider, psbtx, /*index=*/i, &txdata, {.sighash_type = sighash_type, .finalize = finalize}, /*out_sigdata=*/nullptr) == common::PSBTError::SIGHASH_MISMATCH) { + if (SignPSBTInput(provider, psbtx, /*index=*/i, &txdata, { + .sighash_type = sighash_type, + .finalize = finalize, + .taproot_keypath_only = taproot_keypath_only}, /*out_sigdata=*/nullptr) == common::PSBTError::SIGHASH_MISMATCH) { throw JSONRPCPSBTError(common::PSBTError::SIGHASH_MISMATCH); } } @@ -1842,7 +1851,8 @@ static RPCMethod utxoupdatepsbt() request.context, HidingSigningProvider(&provider, /*hide_secret=*/true, /*hide_origin=*/false), /*sighash_type=*/std::nullopt, - /*finalize=*/false); + /*finalize=*/false, + /*taproot_keypath_only=*/false); DataStream ssTx{}; ssTx << psbtx; @@ -2089,6 +2099,7 @@ RPCMethod descriptorprocesspsbt() " \"SINGLE|ANYONECANPAY\""}, {"bip32derivs", RPCArg::Type::BOOL, RPCArg::Default{true}, "Include BIP 32 derivation paths for public keys if we know them"}, {"finalize", RPCArg::Type::BOOL, RPCArg::Default{true}, "Also finalize inputs if possible"}, + {"keypath_only", RPCArg::Type::BOOL, RPCArg::Default{false}, "Only sign the key path (for taproot inputs)."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -2115,13 +2126,15 @@ RPCMethod descriptorprocesspsbt() std::optional sighash_type = ParseSighashString(request.params[2]); bool bip32derivs = request.params[3].isNull() ? true : request.params[3].get_bool(); bool finalize = request.params[4].isNull() ? true : request.params[4].get_bool(); + bool keypath_only{request.params[5].isNull() ? false : request.params[5].get_bool()}; const PartiallySignedTransaction& psbtx = ProcessPSBT( request.params[0].get_str(), request.context, HidingSigningProvider(&provider, /*hide_secret=*/false, !bip32derivs), sighash_type, - finalize); + finalize, + keypath_only); // Check whether or not all of the inputs are now signed bool complete = true; diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 37218f32753b..de37395aff0e 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -1410,6 +1410,57 @@ def test_psbt_input_keys(psbt_input, keys): # Broadcast transaction self.nodes[2].sendrawtransaction(processed_psbt['hex']) + self.log.info("Test descriptorprocesspsbt keypath_only skips taproot script path signing") + taproot_key = get_generate_key() + taproot_descriptor = descsum_create(f"tr({H_POINT},pk({taproot_key.privkey}))") + taproot_public_descriptor = descsum_create(f"tr({H_POINT},pk({taproot_key.pubkey}))") + taproot_address = self.nodes[2].deriveaddresses(taproot_descriptor)[0] + taproot_utxo = self.create_outpoints(self.nodes[0], outputs=[{taproot_address: 1}])[0] + self.sync_all() + + taproot_psbt = self.nodes[2].createpsbt([taproot_utxo], {self.nodes[0].getnewaddress(): 0.99999}) + keypath_only_psbt = self.nodes[2].descriptorprocesspsbt( + psbt=taproot_psbt, + descriptors=[taproot_descriptor], + finalize=False, + keypath_only=True, + ) + decoded = self.nodes[2].decodepsbt(keypath_only_psbt["psbt"]) + assert "taproot_scripts" not in decoded["inputs"][0] + assert "taproot_key_path_sig" not in decoded["inputs"][0] + assert "taproot_script_path_sigs" not in decoded["inputs"][0] + + prepopulated_psbt = self.nodes[2].descriptorprocesspsbt( + psbt=taproot_psbt, + descriptors=[taproot_public_descriptor], + finalize=False, + ) + decoded = self.nodes[2].decodepsbt(prepopulated_psbt["psbt"]) + assert "taproot_scripts" in decoded["inputs"][0] + assert "taproot_script_path_sigs" not in decoded["inputs"][0] + + keypath_only_prepopulated_psbt = self.nodes[2].descriptorprocesspsbt( + psbt=prepopulated_psbt["psbt"], + descriptors=[taproot_descriptor], + finalize=False, + keypath_only=True, + ) + decoded = self.nodes[2].decodepsbt(keypath_only_prepopulated_psbt["psbt"]) + assert "taproot_scripts" in decoded["inputs"][0] + assert "taproot_key_path_sig" not in decoded["inputs"][0] + assert "taproot_script_path_sigs" not in decoded["inputs"][0] + + signed_psbt = self.nodes[2].descriptorprocesspsbt( + psbt=taproot_psbt, + descriptors=[taproot_descriptor], + finalize=False, + keypath_only=False, + ) + decoded = self.nodes[2].decodepsbt(signed_psbt["psbt"]) + assert "taproot_script_path_sigs" in decoded["inputs"][0] + rawtx = self.nodes[2].finalizepsbt(signed_psbt["psbt"])["hex"] + assert self.nodes[2].testmempoolaccept([rawtx])[0]["allowed"] + self.log.info("Test descriptorprocesspsbt raises if an invalid sighashtype is passed") assert_raises_rpc_error(-8, "'all' is not a valid sighash parameter.", self.nodes[2].descriptorprocesspsbt, psbt=psbt, descriptors=[descriptor], sighashtype="all") From d04ee7541c67ff4137ec410839d49c77df913cb9 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 9 Jun 2026 17:42:09 +0200 Subject: [PATCH 12/55] test: cover keypath_only in wallet_musig.py Extract the descriptor with both MuSig key and script paths into key_and_script_path_musigs. Reuse it for the existing cases and give those cases descriptions. Expand the same scenario with keypath_only=true. The new case asks walletprocesspsbt to avoid script-path signing. Co-authored-by: rkrux --- test/functional/wallet_musig.py | 37 ++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/test/functional/wallet_musig.py b/test/functional/wallet_musig.py index fbf0558ec99c..1d909bf84f5e 100755 --- a/test/functional/wallet_musig.py +++ b/test/functional/wallet_musig.py @@ -194,7 +194,16 @@ def test_failure_case_3(self, comment, pat): assert "musig2_pubnonces" in dec["inputs"][0] assert "musig2_partial_sigs" not in dec["inputs"][0] - def test_success_case(self, comment, pattern, sighash_type=None, scriptpath=False, nosign_wallets=None, only_one_musig_wallet=False): + def test_success_case( + self, + comment, + pattern, + sighash_type=None, + scriptpath=False, + keypath_only=False, + nosign_wallets=None, + only_one_musig_wallet=False, + ): self.log.info(f"Testing {comment}") has_internal = MULTIPATH_TWO_RE.search(pattern) is not None @@ -218,6 +227,9 @@ def test_success_case(self, comment, pattern, sighash_type=None, scriptpath=Fals continue if musig_partial_sigs is not None: expected_partial_sigs += musig_partial_sigs + if keypath_only: + # The first MuSig aggregate is the key path, so do not count script-path MuSigs. + break # Check that the wallets agree on the same musig address addr = None @@ -281,7 +293,7 @@ def test_success_case(self, comment, pattern, sighash_type=None, scriptpath=Fals if nosign_wallets and i in nosign_wallets: continue for psbt_list in [nonce_psbts, nonce_psbts2]: - proc = wallet.walletprocesspsbt(psbt=psbt, sighashtype=sighash_type) + proc = wallet.walletprocesspsbt(psbt=psbt, sighashtype=sighash_type, keypath_only=keypath_only) assert_equal(proc["complete"], False) psbt_list.append(proc["psbt"]) @@ -302,7 +314,7 @@ def test_success_case(self, comment, pattern, sighash_type=None, scriptpath=Fals if nosign_wallets and i in nosign_wallets: continue for psbt, psbt_list in [(comb_nonce_psbt, psig_psbts), (comb_nonce_psbt2, psig_psbts2)]: - proc = wallet.walletprocesspsbt(psbt=psbt, sighashtype=sighash_type) + proc = wallet.walletprocesspsbt(psbt=psbt, sighashtype=sighash_type, keypath_only=keypath_only) assert_equal(proc["complete"], False) psbt_list.append(proc["psbt"]) @@ -346,8 +358,23 @@ def run_test(self): self.test_success_case("tr(H,pk(musig/*))", "tr($H,pk(musig($0,$1,$2)/<0;1>/*))", scriptpath=True) self.test_success_case("tr(H,{pk(musig/*), pk(musig/*)})", "tr($H,{pk(musig($0,$1,$2)/<0;1>/*),pk(musig($3,$4,$5)/0/*)})", scriptpath=True) self.test_success_case("tr(H,{pk(musig/*), pk(same keys different musig/*)})", "tr($H,{pk(musig($0,$1,$2)/<0;1>/*),pk(musig($1,$2)/0/*)})", scriptpath=True) - self.test_success_case("tr(musig/*,{pk(partial keys diff musig-1/*),pk(partial keys diff musig-2/*)})}", "tr(musig($0,$1,$2)/<3;4>/*,{pk(musig($0,$1)/<5;6>/*),pk(musig($1,$2)/7/*)})") - self.test_success_case("tr(musig/*,{pk(partial keys diff musig-1/*),pk(partial keys diff musig-2/*)})} script-path", "tr(musig($0,$1,$2)/<3;4>/*,{pk(musig($0,$1)/<5;6>/*),pk(musig($1,$2)/7/*)})", scriptpath=True, nosign_wallets=[0]) + # Descriptor with one MuSig key path and two MuSig script paths. + key_and_script_path_musigs = "tr(musig($0,$1,$2)/<3;4>/*,{pk(musig($0,$1)/<5;6>/*),pk(musig($1,$2)/7/*)})" + self.test_success_case( + "tr() with MuSig key path and different MuSig script paths", + key_and_script_path_musigs, + ) + self.test_success_case( + "tr() with MuSig script path when key path cannot sign", + key_and_script_path_musigs, + scriptpath=True, + nosign_wallets=[0], + ) + self.test_success_case( + "tr() with MuSig key path and keypath_only", + key_and_script_path_musigs, + keypath_only=True, + ) self.test_success_case("tr(H,and(pk(musig/*),after(1)))", "tr($H,and_v(v:pk(musig($0,$1,$2)/<0;1>/*),after(1)))", scriptpath=True) self.test_success_case("tr(H,and(pk_k(musig/*),after(1)))", "tr($H,and_v(vc:pk_k(musig($0,$1,$2)/<0;1>/*),after(1)))", scriptpath=True) self.test_success_case("tr(H,and(pkh(musig/*),after(1)))", "tr($H,and_v(v:pkh(musig($0,$1,$2)/<0;1>/*),after(1)))", scriptpath=True) From 1070ba8c1083a449b1739d7d2af7de18f1c97743 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 16 Jun 2026 17:56:18 +0200 Subject: [PATCH 13/55] doc: add release note for keypath_only --- doc/release-notes-32857.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 doc/release-notes-32857.md diff --git a/doc/release-notes-32857.md b/doc/release-notes-32857.md new file mode 100644 index 000000000000..26687e61192c --- /dev/null +++ b/doc/release-notes-32857.md @@ -0,0 +1,6 @@ +Updated RPCs +------------ + +- The `send`, `sendall`, `walletprocesspsbt`, and `descriptorprocesspsbt` RPCs + now accept a `keypath_only` option that only signs the key path for taproot + inputs. (#32857) From 707b6132af3713aa40dcda4a9a52c18d68e5e85f Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 29 Jun 2026 16:31:57 +0200 Subject: [PATCH 14/55] test: remove unused wallet_taproot init_wallet --- test/functional/wallet_taproot.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py index a2452a9f3f91..f818e896c25e 100755 --- a/test/functional/wallet_taproot.py +++ b/test/functional/wallet_taproot.py @@ -64,9 +64,6 @@ def skip_test_if_missing_module(self): def setup_network(self): self.setup_nodes() - def init_wallet(self, *, node): - pass - @staticmethod def make_desc(pattern, privmap, keys, pub_only = False): pat = pattern.replace("$H", H_POINT) From fdfb6cae47b8a118a9620ac7764fc45aff76239a Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 20 Jun 2025 10:13:31 +0200 Subject: [PATCH 15/55] key: add DeriveExtKey() helper Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com> --- src/key.cpp | 15 +++++++++++++++ src/key.h | 9 +++++++++ src/test/bip32_tests.cpp | 30 ++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/src/key.cpp b/src/key.cpp index cc03df1cd946..6ab458a7c7b9 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -16,6 +16,8 @@ #include #include +#include + static secp256k1_context* secp256k1_context_sign = nullptr; /** These functions are taken from the libsecp256k1 distribution and are very ugly. */ @@ -365,6 +367,19 @@ bool CExtKey::Derive(CExtKey &out, unsigned int _nChild) const { return key.Derive(out.key, out.chaincode, _nChild, chaincode); } +std::optional> DeriveExtKey(const CExtKey& ext_key, const std::vector& path) +{ + CExtKey descendant = ext_key; + KeyOriginInfo origin; + const CKeyID id = ext_key.key.GetPubKey().GetID(); + std::copy(id.begin(), id.begin() + sizeof(origin.fingerprint), origin.fingerprint); + origin.path = path; + for (uint32_t i : path) { + if (!descendant.Derive(descendant, i)) return std::nullopt; + } + return std::make_pair(descendant, origin); +} + void CExtKey::SetSeed(std::span seed) { static const unsigned char hashkey[] = {'B','i','t','c','o','i','n',' ','s','e','e','d'}; diff --git a/src/key.h b/src/key.h index cd77dcd0ee2a..537e76b7e0f2 100644 --- a/src/key.h +++ b/src/key.h @@ -8,11 +8,14 @@ #define BITCOIN_KEY_H #include +#include