From e5aa7d96b436d57fd59953deaccb66f0573a6a21 Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Wed, 26 Jun 2024 10:59:12 +0100 Subject: [PATCH 1/3] rpc: strip derivation paths from psbt during combine Previously setting bip32derivs to false with walletprocesspsbt does not include bip32_derivs for inputs, but does include bip32_derivs for outputs. User may want to strip all bip32 derivation paths for privacy reasons however. It may make sense to do this after signing your inputs and outputs during a manual coinjoin for example. Therefore add functionality to `combinepsbt` permitting stripping of all bip32_derivation paths found in all provided psbts. As this RPC can be called with a single psbt, it can be used to strip derivation paths from any psbt. --- src/psbt.cpp | 13 +++++++++++++ src/psbt.h | 1 + src/rpc/client.cpp | 1 + src/rpc/rawtransaction.cpp | 6 ++++++ 4 files changed, 21 insertions(+) diff --git a/src/psbt.cpp b/src/psbt.cpp index 19d855e4c78..e494dacd7c1 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -66,6 +66,19 @@ bool PartiallySignedTransaction::AddOutput(const CTxOut& txout, const PSBTOutput return true; } +void PartiallySignedTransaction::StripDerivationPaths() +{ + // Loop over the inputs and strip m_tap_bip32_paths + for (unsigned int i = 0; i < inputs.size(); ++i) { + inputs[i].hd_keypaths.clear(); + } + + // Loop over the outputs and strip m_tap_bip32_paths + for (unsigned int i = 0; i < outputs.size(); ++i) { + outputs[i].hd_keypaths.clear(); + } +} + bool PartiallySignedTransaction::GetInputUTXO(CTxOut& utxo, int input_index) const { const PSBTInput& input = inputs[input_index]; diff --git a/src/psbt.h b/src/psbt.h index 6d49864b3cd..206b3845b61 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -968,6 +968,7 @@ struct PartiallySignedTransaction bool AddInput(const CTxIn& txin, PSBTInput& psbtin); bool AddOutput(const CTxOut& txout, const PSBTOutput& psbtout); PartiallySignedTransaction() = default; + void StripDerivationPaths(); explicit PartiallySignedTransaction(const CMutableTransaction& tx); /** * Finds the UTXO for a given input index diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 601e4fa7bf5..ad2b3740c93 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -178,6 +178,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "createpsbt", 2, "locktime" }, { "createpsbt", 3, "replaceable" }, { "combinepsbt", 0, "txs"}, + { "combinepsbt", 1, "stripderivs"}, { "joinpsbts", 0, "txs"}, { "finalizepsbt", 1, "extract"}, { "converttopsbt", 1, "permitsigdata"}, diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 65e6e40b0dc..b269b82f062 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1464,6 +1464,7 @@ static RPCHelpMan combinepsbt() {"psbt", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A base64 string of a PSBT"}, }, }, + {"stripderivs", RPCArg::Type::BOOL, RPCArg::Default{false}, "Strip BIP 32 derivation paths for out inputs and outputs if they are present"}, }, RPCResult{ RPCResult::Type::STR, "", "The base64-encoded partially signed transaction" @@ -1473,6 +1474,8 @@ static RPCHelpMan combinepsbt() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + bool strip_derivation_paths = request.params[1].isNull() ? false : request.params[1].get_bool(); + // Unserialize the transactions std::vector psbtxs; UniValue txs = request.params[0].get_array(); @@ -1485,6 +1488,9 @@ static RPCHelpMan combinepsbt() if (!DecodeBase64PSBT(psbtx, txs[i].get_str(), error)) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, strprintf("TX decode failed %s", error)); } + if (strip_derivation_paths) { + psbtx.StripDerivationPaths(); + } psbtxs.push_back(psbtx); } From 6f5b910876600fb8466f74333477ea8756a659e8 Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Wed, 26 Jun 2024 11:09:34 +0100 Subject: [PATCH 2/3] test: exercise bip32_deriv stripping during psbt combine These asserts check that when calling `combinepsbt` RPC with `stripderivs=true`, all derivation paths are stripped from all inputs and outputs. --- .../wallet_multisig_descriptor_psbt.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/functional/wallet_multisig_descriptor_psbt.py b/test/functional/wallet_multisig_descriptor_psbt.py index 2e0b0d1a41b..e53e5788920 100755 --- a/test/functional/wallet_multisig_descriptor_psbt.py +++ b/test/functional/wallet_multisig_descriptor_psbt.py @@ -128,10 +128,35 @@ class WalletMultisigDescriptorPSBTTest(BitcoinTestFramework): psbts.append(partially_signed_psbt["psbt"]) self.log.info("Finally, collect the signed PSBTs with combinepsbt, finalizepsbt, then broadcast the resulting transaction...") + combined = coordinator_wallet.combinepsbt(psbts) + combined_stripped = coordinator_wallet.combinepsbt(psbts, stripderivs=True) + + combined_decoded = coordinator_wallet.decodepsbt(combined) + combined_decoded_stripped = coordinator_wallet.decodepsbt(combined_stripped) + + self.log.info("Check that we can strip bip32 derivation information from all inputs and outputs using combinepsbt") + for key in ['inputs', 'outputs']: + # Test for presence + for item in combined_decoded[key]: + # Only check these those that are not empty + if item: + assert "bip32_derivs" in item, f"{key} must contain 'bip32_derivs'" + # Test for absence + for item in combined_decoded_stripped[key]: + if item: + assert "bip32_derivs" not in item, f"'bip32_derivs' should not be in {key}" + finalized = coordinator_wallet.finalizepsbt(combined) coordinator_wallet.sendrawtransaction(finalized["hex"]) + self.log.info("Check that we can strip bip32 derivation information from a single psbt using combinepsbt") + psbts2 = [psbts[0],] + combined_single_stripped = coordinator_wallet.combinepsbt(psbts2, stripderivs=True) + combined_single_stripped_decoded = coordinator_wallet.decodepsbt(combined_single_stripped) + assert "bip32_derivs" not in combined_single_stripped_decoded["inputs"][0] + assert "bip32_derivs" not in combined_single_stripped_decoded["outputs"][0] + self.log.info("Check that balances are correct after the transaction has been included in a block.") self.generate(self.nodes[0], 1) assert_approx(participants["multisigs"][0].getbalance(), deposit_amount - value, vspan=0.001) From f3e2a7f7ff172cef9d28d4c3de5696f59c0f3aad Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Wed, 26 Jun 2024 11:13:42 +0100 Subject: [PATCH 3/3] doc: detail psbt combiner derivation strip ability. --- doc/psbt.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/psbt.md b/doc/psbt.md index 0f31cb8eba6..bc66b527939 100644 --- a/doc/psbt.md +++ b/doc/psbt.md @@ -39,8 +39,10 @@ Generally, each of the above (excluding Creator and Extractor) will simply add more and more data to a particular PSBT, until all inputs are fully signed. In a naive workflow, they all have to operate sequentially, passing the PSBT from one to the next, until the Extractor can convert it to a real transaction. -In order to permit parallel operation, **Combiners** can be employed which merge -metadata from different PSBTs for the same unsigned transaction. +In order to permit parallel operation, **Combiners** can be employed which +merge metadata from different PSBTs for the same unsigned transaction. They can +also optionally remove bip32 derivation information from all inputs and outputs +to increase privacy. The names above in bold are the names of the roles defined in BIP174. They're useful in understanding the underlying steps, but in practice, software and