the PR description says joinpsbts now unions m_proprietary "matching the combinepsbt behavior from #34893," which is a set insert() i.e. first PSBT wins if the same key shows up twice or more. right now, each global key in the test is set on psbt1_obj or psbt2_obj, but never both at once. that confirms nothing gets dropped when the 2 PSBTs do not overlap -- but it does not tell us what happens if both PSBTs used the same proprietary key with different values (e.g. psbt1_obj sets it to 0x11... and psbt2_obj sets it to 0x22...). that collision case, and which value should survive, is not covered by any existing test.
this can happen in practice: joinpsbts is meant for batching independently-created PSBTs, and two PSBTs from the same wallet or software could easily carry the same proprietary key (e.g. a session ID or some internal bookkeeping field) with different values each time
would it make sense to add a small case like this, right after the existing xpub/proprietary check ?
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index 54cabc5844..cb1a494ebf 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -1109,6 +1109,19 @@ class PSBTTest(BitcoinTestFramework):
assert_equal(joined_globals.g.map[xpub_key2], xpub_value)
assert_equal(joined_globals.g.map[global_prop_key], global_prop_value)
+ # same proprietary key in both PSBTs, different values: first PSBT SHOULD win
+ collide_key = bytes([PSBT_GLOBAL_PROPRIETARY]) + b"\x02\x03\x04\x00"
+ psbt_first_obj = PSBT.from_base64(psbt1)
+ psbt_first_obj.g.map[collide_key] = b"\x11\x11\x11\x11"
+
+ psbt_second_obj = PSBT.from_base64(psbt2)
+ psbt_second_obj.g.map[collide_key] = b"\x22\x22\x22\x22"
+
+ joined_collision = PSBT.from_base64(
+ self.nodes[0].joinpsbts([psbt_first_obj.to_base64(), psbt_second_obj.to_base64()])
+ )
+ assert_equal(joined_collision.g.map[collide_key], b"\x11\x11\x11\x11") # first PSBT's value wins
+
# Newly created PSBT needs UTXOs and updating
addr = self.nodes[1].getnewaddress("", "p2sh-segwit")
utxo = self.create_outpoints(self.nodes[0], outputs=[{addr: 7}])[0]
at core, this proposed test just asks: when 2 PSBTs disagree on the same key, which one wins?
this is just a nice-to-have, not a blocker. what's your take on this ?
