From 0cb884e6df8d9037ec8e85412a82fcb79df98307 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 18 Mar 2026 13:54:16 -0700 Subject: [PATCH 01/32] psbt: Fill hash preimages and taproot builder from SignatureData Filling these fields was missing. --- src/psbt.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/psbt.cpp b/src/psbt.cpp index cfc720b06d9..3086ae7775e 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -210,6 +210,18 @@ void PSBTInput::FromSignatureData(const SignatureData& sigdata) for (const auto& [agg_key_lh, psigs] : sigdata.musig2_partial_sigs) { m_musig2_partial_sigs[agg_key_lh].insert(psigs.begin(), psigs.end()); } + for (const auto& [hash, preimage] : sigdata.ripemd160_preimages) { + ripemd160_preimages.emplace(std::vector(hash.begin(), hash.end()), preimage); + } + for (const auto& [hash, preimage] : sigdata.sha256_preimages) { + sha256_preimages.emplace(std::vector(hash.begin(), hash.end()), preimage); + } + for (const auto& [hash, preimage] : sigdata.hash160_preimages) { + hash160_preimages.emplace(std::vector(hash.begin(), hash.end()), preimage); + } + for (const auto& [hash, preimage] : sigdata.hash256_preimages) { + hash256_preimages.emplace(std::vector(hash.begin(), hash.end()), preimage); + } } void PSBTInput::Merge(const PSBTInput& input) @@ -268,6 +280,7 @@ void PSBTOutput::FillSignatureData(SignatureData& sigdata) const sigdata.tr_spenddata.internal_key = m_tap_internal_key; sigdata.tr_spenddata.Merge(spenddata); + sigdata.tr_builder = builder; } for (const auto& [pubkey, leaf_origin] : m_tap_bip32_paths) { sigdata.taproot_misc_pubkeys.emplace(pubkey, leaf_origin); From 001877500dc9edda64e3aeaf8e6fb4f50065c7bd Mon Sep 17 00:00:00 2001 From: David Gumberg Date: Thu, 5 Mar 2026 16:04:48 -0500 Subject: [PATCH 02/32] test: construct psbt with unknown field programmatically --- test/functional/data/rpc_psbt.json | 7 ------ test/functional/rpc_psbt.py | 37 +++++++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/test/functional/data/rpc_psbt.json b/test/functional/data/rpc_psbt.json index abc09d523a8..9e8131dbeb2 100644 --- a/test/functional/data/rpc_psbt.json +++ b/test/functional/data/rpc_psbt.json @@ -239,13 +239,6 @@ "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAAiAgLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU210cwRAIgYxqYn+c4qSrQGYYCMxLBkhT+KAKznly8GsNniAbGksMCIDnbbDh70mdxbf2z1NjaULjoXSEzJrp8faqkwM5B65IjAQEDBAEAAAABBEdSIQKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgfyEC2rYf9JoU22p9ArDNH7t4/EsYMStbTlTa5Nui+/71NtdSriIGApWDvzmuCmCXR60Zmt3WNPphCFWdbFzTm0whg/GrluB/ENkMak8AAACAAAAAgAAAAIAiBgLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU21xDZDGpPAAAAgAAAAIABAACAAAEBIADC6wsAAAAAF6kUt/X69A49QKWkWbHbNTXyty+pIeiHIgICOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnNHMEQCIGX0W6WZi1mif/4ae+0BavHx+Q1Us6qPdFCqX1aiUQO9AiB/ckcDrR7blmgLKEtW1P/LiPf7dZ6rvgiqMPKbhROD0gEBAwQBAAAAAQQiACCMI1MXN0O1ld+0oHtyuo5C43l9p06H/n2ddJfjsgKJAwEFR1IhAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcIQI63ZBPPW3PWd25BrDe4jUpt/+57VDl6GFRkmhgIh8Oc1KuIgYCOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnMQ2QxqTwAAAIAAAACAAwAAgCIGAwidwQx6xttU+RMpr2FzM9s4jOrQwjH3IzedG5kDCwLcENkMak8AAACAAAAAgAIAAIAAIgIDqaTDf1mW06ol26xrVwrwZQOUSSlCRgs1R1Ptnuylh3EQ2QxqTwAAAIAAAACABAAAgAAiAgJ/Y5l1fS7/VaE2rQLGhLGDi2VW5fG2s0KCqUtrUAUQlhDZDGpPAAAAgAAAAIAFAACAAA==" ], "result" : "cHNidP8BAJoCAAAAAljoeiG1ba8MI76OcHBFbDNvfLqlyHV5JPVFiHuyq911AAAAAAD/////g40EJ9DsZQpoqka7CwmK6kQiwHGyyng1Kgd5WdB86h0BAAAAAP////8CcKrwCAAAAAAWABTYXCtx0AYLCcmIauuBXlCZHdoSTQDh9QUAAAAAFgAUAK6pouXw+HaliN9VRuh0LR2HAI8AAAAAAAEAuwIAAAABqtc5MQGL0l+ErkALaISL4J23BurCrBgpi6vucatlb4sAAAAASEcwRAIgWPb8fGoz4bMVSNSByCbAFb0wE1qtQs1neQ2rZtKtJDsCIEoc7SYExnNbY5PltBaR3XiwDwxZQvufdRhW+qk4FX26Af7///8CgPD6AgAAAAAXqRQPuUY0IWlrgsgzryQceMF9295JNIfQ8gonAQAAABepFCnKdPigj4GZlCgYXJe12FLkBj9hh2UAAAAiAgKVg785rgpgl0etGZrd1jT6YQhVnWxc05tMIYPxq5bgf0cwRAIgdAGK1BgAl7hzMjwAFXILNoTMgSOJEEjn282bVa1nnJkCIHPTabdA4+tT3O+jOCPIBwUUylWn3ZVE8VfBZ5EyYRGMASICAtq2H/SaFNtqfQKwzR+7ePxLGDErW05U2uTbovv+9TbXRzBEAiBjGpif5zipKtAZhgIzEsGSFP4oArOeXLwaw2eIBsaSwwIgOdtsOHvSZ3Ft/bPU2NpQuOhdITMmunx9qqTAzkHrkiMBAQMEAQAAAAEER1IhApWDvzmuCmCXR60Zmt3WNPphCFWdbFzTm0whg/GrluB/IQLath/0mhTban0CsM0fu3j8SxgxK1tOVNrk26L7/vU211KuIgYClYO/Oa4KYJdHrRma3dY0+mEIVZ1sXNObTCGD8auW4H8Q2QxqTwAAAIAAAACAAAAAgCIGAtq2H/SaFNtqfQKwzR+7ePxLGDErW05U2uTbovv+9TbXENkMak8AAACAAAAAgAEAAIAAAQEgAMLrCwAAAAAXqRS39fr0Dj1ApaRZsds1NfK3L6kh6IciAgMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3EcwRAIgYut6VWEHp8c/RaxKtaHd329wdfsSdZaafzg+//eEvLICIAwF27dHDb8vCFV901bHMlwe0wkT6ZbNOECUXbEiKNpfASICAjrdkE89bc9Z3bkGsN7iNSm3/7ntUOXoYVGSaGAiHw5zRzBEAiBl9FulmYtZon/+GnvtAWrx8fkNVLOqj3RQql9WolEDvQIgf3JHA60e25ZoCyhLVtT/y4j3+3Weq74IqjDym4UTg9IBAQMEAQAAAAEEIgAgjCNTFzdDtZXftKB7crqOQuN5fadOh/59nXSX47ICiQMBBUdSIQMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3CECOt2QTz1tz1nduQaw3uI1Kbf/ue1Q5ehhUZJoYCIfDnNSriIGAjrdkE89bc9Z3bkGsN7iNSm3/7ntUOXoYVGSaGAiHw5zENkMak8AAACAAAAAgAMAAIAiBgMIncEMesbbVPkTKa9hczPbOIzq0MIx9yM3nRuZAwsC3BDZDGpPAAAAgAAAAIACAACAACICA6mkw39ZltOqJdusa1cK8GUDlEkpQkYLNUdT7Z7spYdxENkMak8AAACAAAAAgAQAAIAAIgICf2OZdX0u/1WhNq0CxoSxg4tlVuXxtrNCgqlLa1AFEJYQ2QxqTwAAAIAAAACABQAAgAA=" - }, - { - "combine" : [ - "cHNidP8BAD8CAAAAAf//////////////////////////////////////////AAAAAAD/////AQAAAAAAAAAAA2oBAAAAAAAKDwECAwQFBgcICQ8BAgMEBQYHCAkKCwwNDg8ACg8BAgMEBQYHCAkPAQIDBAUGBwgJCgsMDQ4PAAoPAQIDBAUGBwgJDwECAwQFBgcICQoLDA0ODwA=", - "cHNidP8BAD8CAAAAAf//////////////////////////////////////////AAAAAAD/////AQAAAAAAAAAAA2oBAAAAAAAKDwECAwQFBgcIEA8BAgMEBQYHCAkKCwwNDg8ACg8BAgMEBQYHCBAPAQIDBAUGBwgJCgsMDQ4PAAoPAQIDBAUGBwgQDwECAwQFBgcICQoLDA0ODwA=" - ], - "result" : "cHNidP8BAD8CAAAAAf//////////////////////////////////////////AAAAAAD/////AQAAAAAAAAAAA2oBAAAAAAAKDwECAwQFBgcICQ8BAgMEBQYHCAkKCwwNDg8KDwECAwQFBgcIEA8BAgMEBQYHCAkKCwwNDg8ACg8BAgMEBQYHCAkPAQIDBAUGBwgJCgsMDQ4PCg8BAgMEBQYHCBAPAQIDBAUGBwgJCgsMDQ4PAAoPAQIDBAUGBwgJDwECAwQFBgcICQoLDA0ODwoPAQIDBAUGBwgQDwECAwQFBgcICQoLDA0ODwA=" } ], "finalizer" : [ diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 6b1bc414a9f..e85a3372187 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -76,6 +76,26 @@ class PSBTTest(BitcoinTestFramework): def skip_test_if_missing_module(self): self.skip_if_no_wallet() + @staticmethod + def create_psbt(inputs=None, outputs=None): + if inputs is None: + inputs = {} + if outputs is None: + outputs = {} + tx = CTransaction() + tx.vin = [CTxIn(outpoint=COutPoint(hash=0, n=0))] + tx.vout = [CTxOut(nValue=1, scriptPubKey=CScript([OP_TRUE]))] + + # https://github.com/bitcoin/bips/blob/master/bip-0174.mediawiki#specification + psbt = PSBT() + # global map + psbt.g = PSBTMap({PSBT_GLOBAL_UNSIGNED_TX: tx.serialize()}) + # input map + psbt.i = [PSBTMap(inputs)] + # output map + psbt.o = [PSBTMap(outputs)] + return psbt + def test_psbt_incomplete_after_invalid_modification(self): self.log.info("Check that PSBT is correctly marked as incomplete after invalid modification") node = self.nodes[2] @@ -791,13 +811,24 @@ class PSBTTest(BitcoinTestFramework): assert_raises_rpc_error(-4, "Insufficient funds", wunsafe.walletcreatefundedpsbt, [], [{self.nodes[0].getnewaddress(): 1}]) wunsafe.walletcreatefundedpsbt([], [{self.nodes[0].getnewaddress(): 1}], 0, {"include_unsafe": True}) - # BIP 174 Test Vectors + self.log.info("Test that unknown values are just passed through") + unknown_type = b'\xf0' + unknown_key = unknown_type + bytes(range(1, 10)) + unknown_value = bytes(range(1, 16)) - # Check that unknown values are just passed through - unknown_psbt = "cHNidP8BAD8CAAAAAf//////////////////////////////////////////AAAAAAD/////AQAAAAAAAAAAA2oBAAAAAAAACg8BAgMEBQYHCAkPAQIDBAUGBwgJCgsMDQ4PAAA=" + unknown_psbt = self.create_psbt(inputs={unknown_key: unknown_value}).to_base64() unknown_out = self.nodes[0].walletprocesspsbt(unknown_psbt)['psbt'] assert_equal(unknown_psbt, unknown_out) + unknown_key2 = unknown_type + bytes(range(1, 9)) + bytes(11) + unknown_psbt2 = self.create_psbt(inputs={unknown_key2: unknown_value}).to_base64() + merged_unknown = self.nodes[0].combinepsbt([unknown_psbt, unknown_psbt2]) + merged_unknown_psbt = PSBT.from_base64(merged_unknown) + assert_equal(merged_unknown_psbt.i[0].map[unknown_key], unknown_value) + assert_equal(merged_unknown_psbt.i[0].map[unknown_key2], unknown_value) + + # BIP 174 Test Vectors + # Open the data file with open(os.path.join(os.path.dirname(os.path.realpath(__file__)), 'data/rpc_psbt.json')) as f: d = json.load(f) From 88384180d3e7a20155eed65290a23b2e603683db Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 17 Mar 2026 15:36:09 -0700 Subject: [PATCH 03/32] test: PSBTs should roundtrip through RPCs that do nothing --- test/functional/rpc_psbt.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index e85a3372187..cd47f61ada6 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -426,6 +426,21 @@ class PSBTTest(BitcoinTestFramework): self.log.info("PSBT parameter handling test completed successfully") + def test_psbt_roundtrip(self): + self.log.info("Test that PSBTs roundtrip when RPC does nothing") + utxo = self.nodes[0].listunspent()[0] + psbt = self.nodes[0].walletcreatefundedpsbt(inputs=[utxo], outputs=[{self.nodes[0].getnewaddress(): utxo["amount"] / 2}])["psbt"] + + rt_psbts = [ + self.nodes[0].combinepsbt([psbt, psbt]), + self.nodes[0].finalizepsbt(psbt)["psbt"], + self.nodes[0].utxoupdatepsbt(psbt), + self.nodes[0].descriptorprocesspsbt(psbt, [])["psbt"], + self.nodes[0].walletprocesspsbt(psbt, sign=False)["psbt"], + ] + for p in rt_psbts: + assert_equal(psbt, p) + def run_test(self): # Create and fund a raw tx for sending 10 BTC psbtx1 = self.nodes[0].walletcreatefundedpsbt([], {self.nodes[2].getnewaddress():10})['psbt'] @@ -1294,6 +1309,7 @@ class PSBTTest(BitcoinTestFramework): self.test_sighash_mismatch() self.test_sighash_adding() self.test_psbt_named_parameter_handling() + self.test_psbt_roundtrip() if __name__ == '__main__': PSBTTest(__file__).main() From 1e2d146b47cd65c16795ae1d1294cf79f7030278 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 18 Mar 2026 10:30:26 -0700 Subject: [PATCH 04/32] psbt: Refactor duplicate key lookup and size checks Every key has a duplicate key lookup check, and many keys have fixed size checks. These can be refactored to reduce code duplication. Co-Authored-By: David Gumberg --- src/psbt.h | 224 +++++++---------------------- test/functional/data/rpc_psbt.json | 4 +- 2 files changed, 53 insertions(+), 175 deletions(-) diff --git a/src/psbt.h b/src/psbt.h index 6348bd15ea5..719d68d9213 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -158,9 +158,6 @@ void DeserializeHDKeypaths(Stream& s, const std::vector& key, std if (!pubkey.IsFullyValid()) { throw std::ios_base::failure("Invalid pubkey"); } - if (hd_keypaths.contains(pubkey)) { - throw std::ios_base::failure("Duplicate Key, pubkey derivation path already provided"); - } KeyOriginInfo keypath; DeserializeHDKeypath(s, keypath); @@ -257,6 +254,12 @@ void DeserializeMuSig2ParticipantDataIdentifier(Stream& skey, CPubKey& agg_pub, } } +static inline void ExpectedKeySize(const std::string& key_name, const std::vector& key, uint64_t expected_size) { + if (key.size() != expected_size) { + throw std::ios_base::failure(tfm::format("Size of key was not %d for the type %s", expected_size, key_name)); + } +} + /** A structure for PSBTs which contain per-input information */ struct PSBTInput { @@ -494,6 +497,11 @@ struct PSBTInput break; } + // Duplicate keys are not permitted + if (!key_lookup.emplace(key).second) { + throw std::ios_base::failure(tfm::format("Duplicate Key, input key \"%s\" already provided", HexStr(key))); + } + // "skey" is used so that "key" is unchanged after reading keytype below SpanReader skey{key}; // keytype is of the format compact size uint at the beginning of "key" @@ -504,21 +512,13 @@ struct PSBTInput switch(type) { case PSBT_IN_NON_WITNESS_UTXO: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input non-witness utxo already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Non-witness utxo key is more than one byte type"); - } + ExpectedKeySize("Input Non-witness UTXO", key, 1); // Set the stream to unserialize with witness since this is always a valid network transaction UnserializeFromVector(s, TX_WITH_WITNESS(non_witness_utxo)); break; } case PSBT_IN_WITNESS_UTXO: - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input witness utxo already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Witness utxo key is more than one byte type"); - } + ExpectedKeySize("Input Witness UTXO", key, 1); UnserializeFromVector(s, witness_utxo); break; case PSBT_IN_PARTIAL_SIG: @@ -532,9 +532,6 @@ struct PSBTInput if (!pubkey.IsFullyValid()) { throw std::ios_base::failure("Invalid pubkey"); } - if (partial_sigs.contains(pubkey.GetID())) { - throw std::ios_base::failure("Duplicate Key, input partial signature for pubkey already provided"); - } // Read in the signature from value std::vector sig; @@ -550,32 +547,20 @@ struct PSBTInput break; } case PSBT_IN_SIGHASH: - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input sighash type already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Sighash type key is more than one byte type"); - } + ExpectedKeySize("Input Sighash Type", key, 1); int sighash; UnserializeFromVector(s, sighash); sighash_type = sighash; break; case PSBT_IN_REDEEMSCRIPT: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input redeemScript already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Input redeemScript key is more than one byte type"); - } + ExpectedKeySize("Input redeemScript", key, 1); s >> redeem_script; break; } case PSBT_IN_WITNESSSCRIPT: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input witnessScript already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Input witnessScript key is more than one byte type"); - } + ExpectedKeySize("Input witnessScript", key, 1); s >> witness_script; break; } @@ -586,36 +571,22 @@ struct PSBTInput } case PSBT_IN_SCRIPTSIG: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input final scriptSig already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Final scriptSig key is more than one byte type"); - } + ExpectedKeySize("Input Final scriptSig", key, 1); s >> final_script_sig; break; } case PSBT_IN_SCRIPTWITNESS: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input final scriptWitness already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Final scriptWitness key is more than one byte type"); - } + ExpectedKeySize("Input Final scriptWitness", key, 1); UnserializeFromVector(s, final_script_witness.stack); break; } case PSBT_IN_RIPEMD160: { - // Make sure that the key is the size of a ripemd160 hash + 1 - if (key.size() != CRIPEMD160::OUTPUT_SIZE + 1) { - throw std::ios_base::failure("Size of key was not the expected size for the type ripemd160 preimage"); - } + ExpectedKeySize("Input RIPEMD160 Preimage", key, CRIPEMD160::OUTPUT_SIZE + 1); // Read in the hash from key std::vector hash_vec(key.begin() + 1, key.end()); uint160 hash(hash_vec); - if (ripemd160_preimages.contains(hash)) { - throw std::ios_base::failure("Duplicate Key, input ripemd160 preimage already provided"); - } // Read in the preimage from value std::vector preimage; @@ -627,16 +598,10 @@ struct PSBTInput } case PSBT_IN_SHA256: { - // Make sure that the key is the size of a sha256 hash + 1 - if (key.size() != CSHA256::OUTPUT_SIZE + 1) { - throw std::ios_base::failure("Size of key was not the expected size for the type sha256 preimage"); - } + ExpectedKeySize("Input SHA256 Preimage", key, CSHA256::OUTPUT_SIZE + 1); // Read in the hash from key std::vector hash_vec(key.begin() + 1, key.end()); uint256 hash(hash_vec); - if (sha256_preimages.contains(hash)) { - throw std::ios_base::failure("Duplicate Key, input sha256 preimage already provided"); - } // Read in the preimage from value std::vector preimage; @@ -648,16 +613,10 @@ struct PSBTInput } case PSBT_IN_HASH160: { - // Make sure that the key is the size of a hash160 hash + 1 - if (key.size() != CHash160::OUTPUT_SIZE + 1) { - throw std::ios_base::failure("Size of key was not the expected size for the type hash160 preimage"); - } + ExpectedKeySize("Input Hash160 Preimage", key, CHash160::OUTPUT_SIZE + 1); // Read in the hash from key std::vector hash_vec(key.begin() + 1, key.end()); uint160 hash(hash_vec); - if (hash160_preimages.contains(hash)) { - throw std::ios_base::failure("Duplicate Key, input hash160 preimage already provided"); - } // Read in the preimage from value std::vector preimage; @@ -669,16 +628,10 @@ struct PSBTInput } case PSBT_IN_HASH256: { - // Make sure that the key is the size of a hash256 hash + 1 - if (key.size() != CHash256::OUTPUT_SIZE + 1) { - throw std::ios_base::failure("Size of key was not the expected size for the type hash256 preimage"); - } + ExpectedKeySize("Input Hash256 Preimage", key, CHash256::OUTPUT_SIZE + 1); // Read in the hash from key std::vector hash_vec(key.begin() + 1, key.end()); uint256 hash(hash_vec); - if (hash256_preimages.contains(hash)) { - throw std::ios_base::failure("Duplicate Key, input hash256 preimage already provided"); - } // Read in the preimage from value std::vector preimage; @@ -690,11 +643,7 @@ struct PSBTInput } case PSBT_IN_TAP_KEY_SIG: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input Taproot key signature already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Input Taproot key signature key is more than one byte type"); - } + ExpectedKeySize("Input Taproot Key Path Signature", key, 1); s >> m_tap_key_sig; if (m_tap_key_sig.size() < 64) { throw std::ios_base::failure("Input Taproot key path signature is shorter than 64 bytes"); @@ -705,11 +654,7 @@ struct PSBTInput } case PSBT_IN_TAP_SCRIPT_SIG: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input Taproot script signature already provided"); - } else if (key.size() != 65) { - throw std::ios_base::failure("Input Taproot script signature key is not 65 bytes"); - } + ExpectedKeySize("Input Taproot Script Path Signature", key, 65); SpanReader s_key{std::span{key}.subspan(1)}; XOnlyPubKey xonly; uint256 hash; @@ -727,10 +672,8 @@ struct PSBTInput } case PSBT_IN_TAP_LEAF_SCRIPT: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input Taproot leaf script already provided"); - } else if (key.size() < 34) { - throw std::ios_base::failure("Taproot leaf script key is not at least 34 bytes"); + if (key.size() < 34) { + throw std::ios_base::failure("Input Taproot leaf script key is not at least 34 bytes"); } else if ((key.size() - 2) % 32 != 0) { throw std::ios_base::failure("Input Taproot leaf script key's control block size is not valid"); } @@ -747,11 +690,7 @@ struct PSBTInput } case PSBT_IN_TAP_BIP32_DERIVATION: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input Taproot BIP32 keypath already provided"); - } else if (key.size() != 33) { - throw std::ios_base::failure("Input Taproot BIP32 keypath key is not at 33 bytes"); - } + ExpectedKeySize("Input Taproot BIP32 Keypath", key, 33); SpanReader s_key{std::span{key}.subspan(1)}; XOnlyPubKey xonly; s_key >> xonly; @@ -770,39 +709,25 @@ struct PSBTInput } case PSBT_IN_TAP_INTERNAL_KEY: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input Taproot internal key already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Input Taproot internal key key is more than one byte type"); - } + ExpectedKeySize("Input Taproot Internal Key", key, 1); UnserializeFromVector(s, m_tap_internal_key); break; } case PSBT_IN_TAP_MERKLE_ROOT: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input Taproot merkle root already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Input Taproot merkle root key is more than one byte type"); - } + ExpectedKeySize("Input Taproot Merkle Root", key, 1); UnserializeFromVector(s, m_tap_merkle_root); break; } case PSBT_IN_MUSIG2_PARTICIPANT_PUBKEYS: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input participant pubkeys for an aggregate key already provided"); - } else if (key.size() != CPubKey::COMPRESSED_SIZE + 1) { - throw std::ios_base::failure("Input musig2 participants pubkeys aggregate key is not 34 bytes"); - } + ExpectedKeySize("Input MuSig2 Participants Pubkeys", key, CPubKey::COMPRESSED_SIZE + 1); DeserializeMuSig2ParticipantPubkeys(s, skey, m_musig2_participants, std::string{"Input"}); break; } case PSBT_IN_MUSIG2_PUB_NONCE: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input musig2 pubnonce already provided"); - } else if (key.size() != 2 * CPubKey::COMPRESSED_SIZE + 1 && key.size() != 2 * CPubKey::COMPRESSED_SIZE + CSHA256::OUTPUT_SIZE + 1) { + if (key.size() != 2 * CPubKey::COMPRESSED_SIZE + 1 && key.size() != 2 * CPubKey::COMPRESSED_SIZE + CSHA256::OUTPUT_SIZE + 1) { throw std::ios_base::failure("Input musig2 pubnonce key is not expected size of 67 or 99 bytes"); } CPubKey agg_pub, part_pub; @@ -820,9 +745,7 @@ struct PSBTInput } case PSBT_IN_MUSIG2_PARTIAL_SIG: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, input musig2 partial sig already provided"); - } else if (key.size() != 2 * CPubKey::COMPRESSED_SIZE + 1 && key.size() != 2 * CPubKey::COMPRESSED_SIZE + CSHA256::OUTPUT_SIZE + 1) { + if (key.size() != 2 * CPubKey::COMPRESSED_SIZE + 1 && key.size() != 2 * CPubKey::COMPRESSED_SIZE + CSHA256::OUTPUT_SIZE + 1) { throw std::ios_base::failure("Input musig2 partial sig key is not expected size of 67 or 99 bytes"); } CPubKey agg_pub, part_pub; @@ -842,18 +765,12 @@ struct PSBTInput this_prop.subtype = ReadCompactSize(skey); this_prop.key = key; - if (m_proprietary.contains(this_prop)) { - throw std::ios_base::failure("Duplicate Key, proprietary key already found"); - } s >> this_prop.value; m_proprietary.insert(this_prop); break; } // Unknown stuff default: - if (unknown.contains(key)) { - throw std::ios_base::failure("Duplicate Key, key for unknown value already provided"); - } // Read in the value std::vector val_bytes; s >> val_bytes; @@ -986,6 +903,11 @@ struct PSBTOutput break; } + // Duplicate keys are not permitted + if (!key_lookup.emplace(key).second) { + throw std::ios_base::failure(tfm::format("Duplicate Key, output key \"%s\" already provided", HexStr(key))); + } + // "skey" is used so that "key" is unchanged after reading keytype below SpanReader skey{key}; // keytype is of the format compact size uint at the beginning of "key" @@ -996,21 +918,13 @@ struct PSBTOutput switch(type) { case PSBT_OUT_REDEEMSCRIPT: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, output redeemScript already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Output redeemScript key is more than one byte type"); - } + ExpectedKeySize("Output redeemScript", key, 1); s >> redeem_script; break; } case PSBT_OUT_WITNESSSCRIPT: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, output witnessScript already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Output witnessScript key is more than one byte type"); - } + ExpectedKeySize("Output witnessScript", key, 1); s >> witness_script; break; } @@ -1021,21 +935,13 @@ struct PSBTOutput } case PSBT_OUT_TAP_INTERNAL_KEY: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, output Taproot internal key already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Output Taproot internal key key is more than one byte type"); - } + ExpectedKeySize("Output Taproot Internal Key", key, 1); UnserializeFromVector(s, m_tap_internal_key); break; } case PSBT_OUT_TAP_TREE: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, output Taproot tree already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Output Taproot tree key is more than one byte type"); - } + ExpectedKeySize("Output Taproot Tree Key", key, 1); std::vector tree_v; s >> tree_v; SpanReader s_tree{tree_v}; @@ -1066,11 +972,7 @@ struct PSBTOutput } case PSBT_OUT_TAP_BIP32_DERIVATION: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, output Taproot BIP32 keypath already provided"); - } else if (key.size() != 33) { - throw std::ios_base::failure("Output Taproot BIP32 keypath key is not at 33 bytes"); - } + ExpectedKeySize("Output Taproot BIP32 Keypath", key, 33); XOnlyPubKey xonly(uint256(std::span(key).last(32))); std::set leaf_hashes; uint64_t value_len = ReadCompactSize(s); @@ -1087,11 +989,7 @@ struct PSBTOutput } case PSBT_OUT_MUSIG2_PARTICIPANT_PUBKEYS: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, output participant pubkeys for an aggregate key already provided"); - } else if (key.size() != CPubKey::COMPRESSED_SIZE + 1) { - throw std::ios_base::failure("Output musig2 participants pubkeys aggregate key is not 34 bytes"); - } + ExpectedKeySize("Output MuSig2 Participants Pubkeys", key, CPubKey::COMPRESSED_SIZE + 1); DeserializeMuSig2ParticipantPubkeys(s, skey, m_musig2_participants, std::string{"Output"}); break; } @@ -1102,18 +1000,12 @@ struct PSBTOutput this_prop.subtype = ReadCompactSize(skey); this_prop.key = key; - if (m_proprietary.contains(this_prop)) { - throw std::ios_base::failure("Duplicate Key, proprietary key already found"); - } s >> this_prop.value; m_proprietary.insert(this_prop); break; } // Unknown stuff default: { - if (unknown.contains(key)) { - throw std::ios_base::failure("Duplicate Key, key for unknown value already provided"); - } // Read in the value std::vector val_bytes; s >> val_bytes; @@ -1252,6 +1144,11 @@ struct PartiallySignedTransaction break; } + // Duplicate keys are not permitted + if (!key_lookup.emplace(key).second) { + throw std::ios_base::failure(tfm::format("Duplicate Key, global key \"%s\" already provided", HexStr(key))); + } + // "skey" is used so that "key" is unchanged after reading keytype below SpanReader skey{key}; // keytype is of the format compact size uint at the beginning of "key" @@ -1262,11 +1159,7 @@ struct PartiallySignedTransaction switch(type) { case PSBT_GLOBAL_UNSIGNED_TX: { - if (!key_lookup.emplace(key).second) { - throw std::ios_base::failure("Duplicate Key, unsigned tx already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Global unsigned tx key is more than one byte type"); - } + ExpectedKeySize("Global Unsigned TX", key, 1); CMutableTransaction mtx; // Set the stream to serialize with non-witness since this should always be non-witness UnserializeFromVector(s, TX_NO_WITNESS(mtx)); @@ -1281,18 +1174,13 @@ struct PartiallySignedTransaction } case PSBT_GLOBAL_XPUB: { - if (key.size() != BIP32_EXTKEY_WITH_VERSION_SIZE + 1) { - throw std::ios_base::failure("Size of key was not the expected size for the type global xpub"); - } + ExpectedKeySize("Global XPUB", key, BIP32_EXTKEY_WITH_VERSION_SIZE + 1); // Read in the xpub from key CExtPubKey xpub; xpub.DecodeWithVersion(&key.data()[1]); if (!xpub.pubkey.IsFullyValid()) { throw std::ios_base::failure("Invalid pubkey"); } - if (global_xpubs.contains(xpub)) { - throw std::ios_base::failure("Duplicate key, global xpub already provided"); - } global_xpubs.insert(xpub); // Read in the keypath from stream KeyOriginInfo keypath; @@ -1311,11 +1199,7 @@ struct PartiallySignedTransaction } case PSBT_GLOBAL_VERSION: { - if (m_version) { - throw std::ios_base::failure("Duplicate Key, version already provided"); - } else if (key.size() != 1) { - throw std::ios_base::failure("Global version key is more than one byte type"); - } + ExpectedKeySize("Global PSBT Version", key, 1); uint32_t v; UnserializeFromVector(s, v); m_version = v; @@ -1331,18 +1215,12 @@ struct PartiallySignedTransaction this_prop.subtype = ReadCompactSize(skey); this_prop.key = key; - if (m_proprietary.contains(this_prop)) { - throw std::ios_base::failure("Duplicate Key, proprietary key already found"); - } s >> this_prop.value; m_proprietary.insert(this_prop); break; } // Unknown stuff default: { - if (unknown.contains(key)) { - throw std::ios_base::failure("Duplicate Key, key for unknown value already provided"); - } // Read in the value std::vector val_bytes; s >> val_bytes; diff --git a/test/functional/data/rpc_psbt.json b/test/functional/data/rpc_psbt.json index 9e8131dbeb2..fd46312af4d 100644 --- a/test/functional/data/rpc_psbt.json +++ b/test/functional/data/rpc_psbt.json @@ -53,7 +53,7 @@ ], [ "cHNidP8BAFICAAAAAVaG3/QAFl9OBApYVfZYCTRyybz4EIsnKl0x8YH3tP+xAQAAAAD9////ARjd9QUAAAAAFgAUyRI+BujX8JZsXRzQ+TMALU63V80AAAAAAAEBKwDh9QUAAAAAIlEgC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9QhFgtY4zeqTThSqMKTh8QkCNjPvjphOl45fgqfAaX7cQfUBQAmgN1uIRY0a5lZM1cQfJ00Weneuo0+r0TmY2yFx/hT65C6UujNAAUAWAsIhyEWT6/WX4FpGG/Cv9siM8d+Yw0QvigKJMcWXAmidhF3XCwFAMMkmoIhFvkwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5BQB91lWSIRoLWOM3qk04UqjCk4fEJAjYz746YTpeOX4KnwGl+3EH1GMCNGuZWTNXEHydNFnp3rqNPq9E5mNshcf4U+uQulLozQACT6/WX4FpGG/Cv9siM8d+Yw0QvigKJMcWXAmidhF3XCwC+TCKAZJYwxBJNE+F+J1SKbUxyEWDb5mwhgHxE7zgNvkAAA==", - "Input musig2 participants pubkeys aggregate key is not 34 bytes" + "Size of key was not 34 for the type Input MuSig2 Participants Pubkeys" ], [ "cHNidP8BADMAAZJuAQAAAAAAAAAAAABMHQD/AAAAAAAAAAAA//////9BAB4AjIwAAAD5////AAAA/NwAAQErYQIAAAAAAAAiUSBw/G0rYgJicCsrtgAA2P//+HN0AAIA+f//7gAF++8AACIaCAEAAP8AcHNidP8BABMAAiEeAAEXDD4AAAEBAACCpP73IUL8j3+PjNNzYnT/AAAAAAAAlpb/AAAAAAAAAAYEAisAAAA=", @@ -77,7 +77,7 @@ ], [ "cHNidP8BAH0CAAAAASWJ53Z5WLoVT5AYzM8N7ephR7tgzRoZS241kKmWVpDWAAAAAAD9////AoCWmAAAAAAAIlEgKWfS0CCpeV2nK1G+Tz/KJbsOV+kcWz56gav6cjKjSUIPwJJ8AAAAABYAFDScXTMCeMMAKmT1l9KwGqPcG9kDAAAAAAABAH0CAAAAAZqLSlB5a5YAmQ9/4R36ALxw79KWBIr8hnGa8PsfqRk3AAAAAAD9////AolcK30AAAAAFgAUz9mLoQJ+pO1L0q4bNIthVqAVA3UA4fUFAAAAACJRINCyJsZZnyc4dN+P5oSrbDAoCBvuiiy+0xoTb1hl9s+k4QAAAAEBH4lcK30AAAAAFgAUz9mLoQJ+pO1L0q4bNIthVqAVA3UiBgKmZlDwi/+k8InrIu3NvnYWZF/2zRgKNkhNS8gQVFlbexi//0SjVAAAgAEAAIAAAACAAQAAAIoCAAAAAQUgC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9QhBwtY4zeqTThSqMKTh8QkCNjPvjphOl45fgqfAaX7cQfUBQAmgN1uIQc0a5lZM1cQfJ00Weneuo0+r0TmY2yFx/hT65C6UujNAAUAWAsIhyEHT6/WX4FpGG/Cv9siM8d+Yw0QvigKJMcWXAmidhF3XCwFAMMkmoIhB/kwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5BQB91lWSIQgLWOM3qk04UqjCk4fEJAjYz746YTpeOX4KnwGl+3EH1GMCNGuZWTNXEHydNFnp3rqNPq9E5mNshcf4U+uQulLozQACT6/WX4FpGG/Cv9siM8d+Yw0QvigKJMcWXAmidhF3XCwC+TCKAZJYwxBJNE+F+J1SKbUxyEWDb5mwhgHxE7zgNvkAIgIDvkrlPTfMB19Asw1tpHKdQK3uuYNTOvFhYZK8dtWyYSoYv/9Eo1QAAIABAACAAAAAgAEAAACNAgAAAA==", - "Output musig2 participants pubkeys aggregate key is not 34 bytes" + "Size of key was not 34 for the type Output MuSig2 Participants Pubkeys" ], [ "cHNidP8BAH0CAAAAASWJ53Z5WLoVT5AYzM8N7ephR7tgzRoZS241kKmWVpDWAAAAAAD9////AoCWmAAAAAAAIlEgKWfS0CCpeV2nK1G+Tz/KJbsOV+kcWz56gav6cjKjSUIPwJJ8AAAAABYAFDScXTMCeMMAKmT1l9KwGqPcG9kDAAAAAAABAH0CAAAAAZqLSlB5a5YAmQ9/4R36ALxw79KWBIr8hnGa8PsfqRk3AAAAAAD9////AolcK30AAAAAFgAUz9mLoQJ+pO1L0q4bNIthVqAVA3UA4fUFAAAAACJRINCyJsZZnyc4dN+P5oSrbDAoCBvuiiy+0xoTb1hl9s+k4QAAAAEBH4lcK30AAAAAFgAUz9mLoQJ+pO1L0q4bNIthVqAVA3UiBgKmZlDwi/+k8InrIu3NvnYWZF/2zRgKNkhNS8gQVFlbexi//0SjVAAAgAEAAIAAAACAAQAAAIoCAAAAAQUgC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9QhBwtY4zeqTThSqMKTh8QkCNjPvjphOl45fgqfAaX7cQfUBQAmgN1uIQc0a5lZM1cQfJ00Weneuo0+r0TmY2yFx/hT65C6UujNAAUAWAsIhyEHT6/WX4FpGG/Cv9siM8d+Yw0QvigKJMcWXAmidhF3XCwFAMMkmoIhB/kwigGSWMMQSTRPhfidUim1MchFg2+ZsIYB8RO84Db5BQB91lWSIggDC1jjN6pNOFKowpOHxCQI2M++OmE6Xjl+Cp8BpftxB9RiNGuZWTNXEHydNFnp3rqNPq9E5mNshcf4U+uQulLozQACT6/WX4FpGG/Cv9siM8d+Yw0QvigKJMcWXAmidhF3XCwC+TCKAZJYwxBJNE+F+J1SKbUxyEWDb5mwhgHxE7zgNvkAIgIDvkrlPTfMB19Asw1tpHKdQK3uuYNTOvFhYZK8dtWyYSoYv/9Eo1QAAIABAACAAAAAgAEAAACNAgAAAA==", From f926c326bb81f211f26c7f7997da5efaac4459a0 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 16 Mar 2026 21:13:18 -0700 Subject: [PATCH 05/32] gui: Store PSBT in std::optional in PSBTOperationsDialog Use std::optional to store the PSBT to avoid having a default constructed PSBT --- src/qt/psbtoperationsdialog.cpp | 18 +++++++++--------- src/qt/psbtoperationsdialog.h | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/qt/psbtoperationsdialog.cpp b/src/qt/psbtoperationsdialog.cpp index 64ce6d2821b..c77c827d930 100644 --- a/src/qt/psbtoperationsdialog.cpp +++ b/src/qt/psbtoperationsdialog.cpp @@ -59,7 +59,7 @@ void PSBTOperationsDialog::openWithPSBT(PartiallySignedTransaction psbtx) bool complete = FinalizePSBT(psbtx); // Make sure all existing signatures are fully combined before checking for completeness. if (m_wallet_model) { size_t n_could_sign; - const auto err{m_wallet_model->wallet().fillPSBT({.sign = false, .bip32_derivs= true}, &n_could_sign, m_transaction_data, complete)}; + const auto err{m_wallet_model->wallet().fillPSBT({.sign = false, .bip32_derivs= true}, &n_could_sign, *m_transaction_data, complete)}; if (err) { showStatus(tr("Failed to load transaction: %1") .arg(QString::fromStdString(PSBTErrorString(*err).translated)), @@ -83,7 +83,7 @@ void PSBTOperationsDialog::signTransaction() WalletModel::UnlockContext ctx(m_wallet_model->requestUnlock()); - const auto err{m_wallet_model->wallet().fillPSBT({.sign = true, .bip32_derivs = true}, &n_signed, m_transaction_data, complete)}; + const auto err{m_wallet_model->wallet().fillPSBT({.sign = true, .bip32_derivs = true}, &n_signed, *m_transaction_data, complete)}; if (err) { showStatus(tr("Failed to sign transaction: %1") @@ -110,7 +110,7 @@ void PSBTOperationsDialog::signTransaction() void PSBTOperationsDialog::broadcastTransaction() { CMutableTransaction mtx; - if (!FinalizeAndExtractPSBT(m_transaction_data, mtx)) { + if (!FinalizeAndExtractPSBT(*m_transaction_data, mtx)) { // This is never expected to fail unless we were given a malformed PSBT // (e.g. with an invalid signature.) showStatus(tr("Unknown error processing transaction."), StatusLevel::ERR); @@ -133,19 +133,19 @@ void PSBTOperationsDialog::broadcastTransaction() void PSBTOperationsDialog::copyToClipboard() { DataStream ssTx{}; - ssTx << m_transaction_data; + ssTx << *m_transaction_data; GUIUtil::setClipboard(EncodeBase64(ssTx.str()).c_str()); showStatus(tr("PSBT copied to clipboard."), StatusLevel::INFO); } void PSBTOperationsDialog::saveTransaction() { DataStream ssTx{}; - ssTx << m_transaction_data; + ssTx << *m_transaction_data; QString selected_filter; QString filename_suggestion = ""; bool first = true; - for (const CTxOut& out : m_transaction_data.tx->vout) { + for (const CTxOut& out : m_transaction_data->tx->vout) { if (!first) { filename_suggestion.append("-"); } @@ -171,8 +171,8 @@ void PSBTOperationsDialog::saveTransaction() { } void PSBTOperationsDialog::updateTransactionDisplay() { - m_ui->transactionDescription->setText(renderTransaction(m_transaction_data)); - showTransactionStatus(m_transaction_data); + m_ui->transactionDescription->setText(renderTransaction(*m_transaction_data)); + showTransactionStatus(*m_transaction_data); } QString PSBTOperationsDialog::renderTransaction(const PartiallySignedTransaction &psbtx) @@ -251,7 +251,7 @@ size_t PSBTOperationsDialog::couldSignInputs(const PartiallySignedTransaction &p size_t n_signed; bool complete; - const auto err{m_wallet_model->wallet().fillPSBT({.sign = false, .bip32_derivs = false}, &n_signed, m_transaction_data, complete)}; + const auto err{m_wallet_model->wallet().fillPSBT({.sign = false, .bip32_derivs = false}, &n_signed, *m_transaction_data, complete)}; if (err) { return 0; diff --git a/src/qt/psbtoperationsdialog.h b/src/qt/psbtoperationsdialog.h index 55c6ec5063c..be2b742560d 100644 --- a/src/qt/psbtoperationsdialog.h +++ b/src/qt/psbtoperationsdialog.h @@ -35,7 +35,7 @@ public Q_SLOTS: private: Ui::PSBTOperationsDialog* m_ui; - PartiallySignedTransaction m_transaction_data; + std::optional m_transaction_data; WalletModel* m_wallet_model; ClientModel* m_client_model; From 7eacc21ff67a78860c76f18cf551c201a4b78f3e Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 17 Mar 2026 11:33:47 -0700 Subject: [PATCH 06/32] psbt: make PSBT structs into classes --- src/external_signer.h | 2 +- src/interfaces/wallet.h | 2 +- src/psbt.h | 9 ++++++--- src/wallet/wallet.h | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/external_signer.h b/src/external_signer.h index 5ba37c0626b..87fbbf0b8c2 100644 --- a/src/external_signer.h +++ b/src/external_signer.h @@ -11,7 +11,7 @@ #include #include -struct PartiallySignedTransaction; +class PartiallySignedTransaction; //! Enables interaction with an external signing device or service, such as //! a hardware wallet. See doc/external-signer.md diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index ab142080fc6..d21163171d5 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -32,7 +32,7 @@ class CFeeRate; class CKey; enum class FeeReason; enum class OutputType; -struct PartiallySignedTransaction; +class PartiallySignedTransaction; struct bilingual_str; namespace common { enum class PSBTError; diff --git a/src/psbt.h b/src/psbt.h index 719d68d9213..f6bf144ab8d 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -261,8 +261,9 @@ static inline void ExpectedKeySize(const std::string& key_name, const std::vecto } /** A structure for PSBTs which contain per-input information */ -struct PSBTInput +class PSBTInput { +public: CTransactionRef non_witness_utxo; CTxOut witness_utxo; CScript redeem_script; @@ -791,8 +792,9 @@ struct PSBTInput }; /** A structure for PSBTs which contains per output information */ -struct PSBTOutput +class PSBTOutput { +public: CScript redeem_script; CScript witness_script; std::map hd_keypaths; @@ -1027,8 +1029,9 @@ struct PSBTOutput }; /** A version of CTransaction with the PSBT format*/ -struct PartiallySignedTransaction +class PartiallySignedTransaction { +public: std::optional tx; // We use a vector of CExtPubKey in the event that there happens to be the same KeyOriginInfos for different CExtPubKeys // Note that this map swaps the key and values from the serialization diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index cbf3efc0376..d066edf8150 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -74,7 +74,7 @@ struct CBlockLocator; struct CExtKey; struct FlatSigningProvider; struct KeyOriginInfo; -struct PartiallySignedTransaction; +class PartiallySignedTransaction; struct SignatureData; using LoadWalletFn = std::function wallet)>; From 990b084f11c038367035ad520482174900cdd29c Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 22 Jul 2024 17:14:07 -0400 Subject: [PATCH 07/32] Have PSBTInput and PSBTOutput know the PSBT's version --- src/psbt.cpp | 4 ++-- src/psbt.h | 30 +++++++++++++++++++++++++----- src/rpc/rawtransaction.cpp | 8 ++++---- src/test/fuzz/deserialize.cpp | 4 ++-- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/psbt.cpp b/src/psbt.cpp index 3086ae7775e..416eae11d1f 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -15,8 +15,8 @@ using common::PSBTError; PartiallySignedTransaction::PartiallySignedTransaction(const CMutableTransaction& tx) : tx(tx) { - inputs.resize(tx.vin.size()); - outputs.resize(tx.vout.size()); + inputs.resize(tx.vin.size(), PSBTInput(GetVersion())); + outputs.resize(tx.vout.size(), PSBTOutput(GetVersion())); } bool PartiallySignedTransaction::IsNull() const diff --git a/src/psbt.h b/src/psbt.h index f6bf144ab8d..4272cd3a807 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -263,6 +263,9 @@ static inline void ExpectedKeySize(const std::string& key_name, const std::vecto /** A structure for PSBTs which contain per-input information */ class PSBTInput { +private: + uint32_t m_psbt_version; + public: CTransactionRef non_witness_utxo; CTxOut witness_utxo; @@ -300,7 +303,12 @@ public: void FillSignatureData(SignatureData& sigdata) const; void FromSignatureData(const SignatureData& sigdata); void Merge(const PSBTInput& input); - PSBTInput() = default; + uint32_t GetVersion() const { return m_psbt_version; } + explicit PSBTInput(uint32_t psbt_version) + : m_psbt_version(psbt_version) + { + assert(m_psbt_version == 0); + } template inline void Serialize(Stream& s) const { @@ -794,6 +802,9 @@ public: /** A structure for PSBTs which contains per output information */ class PSBTOutput { +private: + uint32_t m_psbt_version; + public: CScript redeem_script; CScript witness_script; @@ -809,7 +820,12 @@ public: void FillSignatureData(SignatureData& sigdata) const; void FromSignatureData(const SignatureData& sigdata); void Merge(const PSBTOutput& output); - PSBTOutput() = default; + uint32_t GetVersion() const { return m_psbt_version; } + explicit PSBTOutput(uint32_t psbt_version) + : m_psbt_version(psbt_version) + { + assert(m_psbt_version == 0); + } template inline void Serialize(Stream& s) const { @@ -1031,6 +1047,9 @@ public: /** A version of CTransaction with the PSBT format*/ class PartiallySignedTransaction { +private: + std::optional m_version; + public: std::optional tx; // We use a vector of CExtPubKey in the event that there happens to be the same KeyOriginInfos for different CExtPubKeys @@ -1039,7 +1058,6 @@ public: std::vector inputs; std::vector outputs; std::map, std::vector> unknown; - std::optional m_version; std::set m_proprietary; bool IsNull() const; @@ -1241,10 +1259,12 @@ public: throw std::ios_base::failure("No unsigned transaction was provided"); } + const uint32_t psbt_ver = GetVersion(); + // Read input data unsigned int i = 0; while (!s.empty() && i < tx->vin.size()) { - PSBTInput input; + PSBTInput input(psbt_ver); s >> input; inputs.push_back(input); @@ -1267,7 +1287,7 @@ public: // Read output data i = 0; while (!s.empty() && i < tx->vout.size()) { - PSBTOutput output; + PSBTOutput output(psbt_ver); s >> output; outputs.push_back(output); ++i; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 0f3cebb5dcf..67067110648 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1652,10 +1652,10 @@ static RPCMethod createpsbt() PartiallySignedTransaction psbtx; psbtx.tx = rawTx; for (unsigned int i = 0; i < rawTx.vin.size(); ++i) { - psbtx.inputs.emplace_back(); + psbtx.inputs.emplace_back(0); } for (unsigned int i = 0; i < rawTx.vout.size(); ++i) { - psbtx.outputs.emplace_back(); + psbtx.outputs.emplace_back(0); } // Serialize the PSBT @@ -1720,10 +1720,10 @@ static RPCMethod converttopsbt() PartiallySignedTransaction psbtx; psbtx.tx = tx; for (unsigned int i = 0; i < tx.vin.size(); ++i) { - psbtx.inputs.emplace_back(); + psbtx.inputs.emplace_back(0); } for (unsigned int i = 0; i < tx.vout.size(); ++i) { - psbtx.outputs.emplace_back(); + psbtx.outputs.emplace_back(0); } // Serialize the PSBT diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index 1329f471c70..b3f35baa0fc 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -192,11 +192,11 @@ FUZZ_TARGET_DESERIALIZE(prefilled_transaction_deserialize, { DeserializeFromFuzzingInput(buffer, prefilled_transaction); }) FUZZ_TARGET_DESERIALIZE(psbt_input_deserialize, { - PSBTInput psbt_input; + PSBTInput psbt_input(0); DeserializeFromFuzzingInput(buffer, psbt_input); }) FUZZ_TARGET_DESERIALIZE(psbt_output_deserialize, { - PSBTOutput psbt_output; + PSBTOutput psbt_output(0); DeserializeFromFuzzingInput(buffer, psbt_output); }) FUZZ_TARGET_DESERIALIZE(block_deserialize, { From 9671aa08c2a7eb0af75ad3016585872c5b781f75 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 16 Mar 2026 14:05:39 -0700 Subject: [PATCH 08/32] psbt: add tx input and output fields in PSBTInput and PSBTOutput PSBTInput should be aware of the previous txid, output index, and sequence numbers for inputs, extracting them from the global unsigned tx. PSBTOutput should be aware of the output amount and script, extracting them from the global unsigned tx. This prepares for PSBTv2 where these fields are serialized. --- src/psbt.cpp | 10 ++++++++-- src/psbt.h | 24 ++++++++++++++++++------ src/rpc/rawtransaction.cpp | 18 ++---------------- src/test/fuzz/deserialize.cpp | 4 ++-- 4 files changed, 30 insertions(+), 26 deletions(-) diff --git a/src/psbt.cpp b/src/psbt.cpp index 416eae11d1f..29f16e73c87 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -15,8 +15,14 @@ using common::PSBTError; PartiallySignedTransaction::PartiallySignedTransaction(const CMutableTransaction& tx) : tx(tx) { - inputs.resize(tx.vin.size(), PSBTInput(GetVersion())); - outputs.resize(tx.vout.size(), PSBTOutput(GetVersion())); + inputs.reserve(tx.vin.size()); + for (const CTxIn& input : tx.vin) { + inputs.emplace_back(GetVersion(), input.prevout.hash, input.prevout.n, input.nSequence); + } + outputs.reserve(tx.vout.size()); + for (const CTxOut& output : tx.vout) { + outputs.emplace_back(GetVersion(), output.nValue, output.scriptPubKey); + } } bool PartiallySignedTransaction::IsNull() const diff --git a/src/psbt.h b/src/psbt.h index 4272cd3a807..dcf0d6bd1a5 100644 --- a/src/psbt.h +++ b/src/psbt.h @@ -280,6 +280,10 @@ public: std::map> hash160_preimages; std::map> hash256_preimages; + Txid prev_txid; + uint32_t prev_out; + std::optional sequence; + // Taproot fields std::vector m_tap_key_sig; std::map, std::vector> m_tap_script_sigs; @@ -304,8 +308,11 @@ public: void FromSignatureData(const SignatureData& sigdata); void Merge(const PSBTInput& input); uint32_t GetVersion() const { return m_psbt_version; } - explicit PSBTInput(uint32_t psbt_version) - : m_psbt_version(psbt_version) + explicit PSBTInput(uint32_t psbt_version, const Txid& prev_txid, uint32_t prev_out, std::optional sequence = std::nullopt) + : m_psbt_version(psbt_version), + prev_txid(prev_txid), + prev_out(prev_out), + sequence(sequence) { assert(m_psbt_version == 0); } @@ -816,13 +823,18 @@ public: std::map, std::vector> unknown; std::set m_proprietary; + CAmount amount; + CScript script; + bool IsNull() const; void FillSignatureData(SignatureData& sigdata) const; void FromSignatureData(const SignatureData& sigdata); void Merge(const PSBTOutput& output); uint32_t GetVersion() const { return m_psbt_version; } - explicit PSBTOutput(uint32_t psbt_version) - : m_psbt_version(psbt_version) + explicit PSBTOutput(uint32_t psbt_version, CAmount amount, const CScript& script) + : m_psbt_version(psbt_version), + amount(amount), + script(script) { assert(m_psbt_version == 0); } @@ -1264,7 +1276,7 @@ public: // Read input data unsigned int i = 0; while (!s.empty() && i < tx->vin.size()) { - PSBTInput input(psbt_ver); + PSBTInput input(psbt_ver, tx->vin[i].prevout.hash, tx->vin[i].prevout.n, tx->vin[i].nSequence); s >> input; inputs.push_back(input); @@ -1287,7 +1299,7 @@ public: // Read output data i = 0; while (!s.empty() && i < tx->vout.size()) { - PSBTOutput output(psbt_ver); + PSBTOutput output(psbt_ver, tx->vout[i].nValue, tx->vout[i].scriptPubKey); s >> output; outputs.push_back(output); ++i; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 67067110648..517a02bf39d 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1649,14 +1649,7 @@ static RPCMethod createpsbt() CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf, self.Arg("version")); // Make a blank psbt - PartiallySignedTransaction psbtx; - psbtx.tx = rawTx; - for (unsigned int i = 0; i < rawTx.vin.size(); ++i) { - psbtx.inputs.emplace_back(0); - } - for (unsigned int i = 0; i < rawTx.vout.size(); ++i) { - psbtx.outputs.emplace_back(0); - } + PartiallySignedTransaction psbtx(rawTx); // Serialize the PSBT DataStream ssTx{}; @@ -1717,14 +1710,7 @@ static RPCMethod converttopsbt() } // Make a blank psbt - PartiallySignedTransaction psbtx; - psbtx.tx = tx; - for (unsigned int i = 0; i < tx.vin.size(); ++i) { - psbtx.inputs.emplace_back(0); - } - for (unsigned int i = 0; i < tx.vout.size(); ++i) { - psbtx.outputs.emplace_back(0); - } + PartiallySignedTransaction psbtx(tx); // Serialize the PSBT DataStream ssTx{}; diff --git a/src/test/fuzz/deserialize.cpp b/src/test/fuzz/deserialize.cpp index b3f35baa0fc..15150c4a444 100644 --- a/src/test/fuzz/deserialize.cpp +++ b/src/test/fuzz/deserialize.cpp @@ -192,11 +192,11 @@ FUZZ_TARGET_DESERIALIZE(prefilled_transaction_deserialize, { DeserializeFromFuzzingInput(buffer, prefilled_transaction); }) FUZZ_TARGET_DESERIALIZE(psbt_input_deserialize, { - PSBTInput psbt_input(0); + PSBTInput psbt_input(0, Txid{}, 0); DeserializeFromFuzzingInput(buffer, psbt_input); }) FUZZ_TARGET_DESERIALIZE(psbt_output_deserialize, { - PSBTOutput psbt_output(0); + PSBTOutput psbt_output(0, 0, CScript()); DeserializeFromFuzzingInput(buffer, psbt_output); }) FUZZ_TARGET_DESERIALIZE(block_deserialize, { From c01c7f068c5b67329794bac9354eb112111815ee Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 16 Mar 2026 21:14:46 -0700 Subject: [PATCH 09/32] psbt: Remove default constructor Instead of allowing PSBTs to be default constructor, force usage of the deserialization constructor. CombinePSBTs, DecodeBase64PSBT, and DecodeRawPSBT are all changed to return std::optional or util::result rather than using an output parameter to avoid the need for a default constructor. --- src/external_signer.cpp | 9 ++-- src/psbt.cpp | 28 +++++----- src/psbt.h | 11 ++-- src/qt/test/wallettests.cpp | 5 +- src/qt/walletframe.cpp | 9 ++-- src/rpc/rawtransaction.cpp | 68 +++++++++++------------- src/test/fuzz/base_encode_decode.cpp | 5 +- src/test/fuzz/deserialize.cpp | 16 +++++- src/test/fuzz/psbt.cpp | 23 ++++---- src/test/fuzz/rpc.cpp | 2 +- src/test/fuzz/util.h | 13 +++++ src/wallet/rpc/spend.cpp | 8 +-- src/wallet/test/fuzz/scriptpubkeyman.cpp | 2 +- src/wallet/test/psbt_wallet_tests.cpp | 3 +- 14 files changed, 108 insertions(+), 94 deletions(-) diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 3790f4d36f9..2da9502669a 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -112,14 +112,13 @@ bool ExternalSigner::SignTransaction(PartiallySignedTransaction& psbtx, std::str return false; } - PartiallySignedTransaction signer_psbtx; - std::string signer_psbt_error; - if (!DecodeBase64PSBT(signer_psbtx, signer_result.find_value("psbt").get_str(), signer_psbt_error)) { - error = strprintf("TX decode failed %s", signer_psbt_error); + util::Result signer_psbtx = DecodeBase64PSBT(signer_result.find_value("psbt").get_str()); + if (!signer_psbtx) { + error = strprintf("TX decode failed %s", util::ErrorString(signer_psbtx).original); return false; } - psbtx = signer_psbtx; + psbtx = *signer_psbtx; return true; } diff --git a/src/psbt.cpp b/src/psbt.cpp index 29f16e73c87..1a49d01afcf 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -9,6 +9,7 @@ #include #include