Merge f3e2a7f7ff into 8fa10edcd1

These asserts check that when calling `combinepsbt` RPC with
`stripderivs=true`, all derivation paths are stripped from all inputs
and outputs.

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

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];

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

src/rpc/client.cpp

@@ -181,6 +181,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"},

src/rpc/rawtransaction.cpp

@@ -1453,6 +1453,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"
@@ -1462,6 +1463,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<PartiallySignedTransaction> psbtxs;
     UniValue txs = request.params[0].get_array();
@@ -1474,6 +1477,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);
     }
 

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)