Follow-up to #35665, which fixed the same combiner defect for PSBT_GLOBAL_XPUB. thomasbuilds
and winterrdog asked for this one as its own PR when I reported it there.
m_tap_scripts maps a leaf script to a set of control blocks, but is serialized as one record
per control block, keyed by the control block (SerializeToVector(s, PSBT_IN_TAP_LEAF_SCRIPT, std::span{control_block})). PSBTInput::Merge unions it by the map key, so two PSBTs that map
the same control block to different leaf scripts merge into an input that serializes the 0x15
key twice. Duplicate keys make a PSBT invalid, so it is the same combinepsbt then
decodepsbt failure as the xpub case, at the input level. Present since #22558 (v24.0).
Both decode on their own, and differ only in the leaf script the control block maps to, OP_1
against OP_1 OP_1:
$ A=cHNidP8BADwCAAAAAaqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAIhXAUJKbdMGgSVS3i0tgNel6XgeKWg8o7JbVR7/ums6AOsACUcAAAA==
$ B=cHNidP8BADwCAAAAAaqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAIhXAUJKbdMGgSVS3i0tgNel6XgeKWg8o7JbVR7/ums6AOsADUVHAAAA=
$ bitcoin-cli -regtest decodepsbt "$(bitcoin-cli -regtest combinepsbt "[\"$A\",\"$B\"]")"
error code: -22
error message:
TX decode failed Duplicate Key, input key "15c050929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0" already provided: unspecified iostream_category error
winterrdog reproduced it on the #35665 thread with another pair.
Merge the records rather than the map entries, keeping the leaf script already there. BIP 174
lets the combiner "pick arbitrarily when conflicts occur", and unknown and proprietary records
already resolve that way. Refusing to combine is the BIP's other option, but that would fail
combinepsbt on input it accepts today.
Merging by map key drops records as well. std::map::insert leaves existing keys alone, so
when both PSBTs carry the same leaf script with different control blocks, the incoming set was
dropped. Those keys do not conflict, so merging per record keeps them.
The control blocks already present are collected once per merge rather than searched for per
incoming record, which would be quadratic in the size of the two PSBTs combinepsbt takes from
the caller.
Since this is the second field with this shape I checked the rest. m_xpubs (#35665) and
m_tap_scripts are the only two whose record key comes from the value, so two map entries can
serialize the same key. partial_sigs is keyed by CKeyID and serialized under the pubkey,
but the pubkey determines the CKeyID, so those records stay distinct. The others key the
record by the map key, m_proprietary included, and PSBTOutput has no such field.
The test fails on master on both counts, and covers the merges that do not conflict as well.
I found this with a local assertion in the psbt fuzz target that a combined PSBT must roundtrip. That assertion can go in a follow-up.
Tested:
./build/bin/test_bitcoin --run_test=psbt_tests
./build/bin/test_bitcoin --run_test=psbt_wallet_tests
./build/test/functional/test_runner.py rpc_psbt.py rpc_rawtransaction.py wallet_taproot.py wallet_signer.py feature_taproot.py wallet_basic.py
