Remove BIP 174's claim that Combine is commutative#2075
Remove BIP 174's claim that Combine is commutative#2075shesek wants to merge 1 commit intobitcoin:masterfrom
Conversation
|
cc: @achow101 |
|
It's supposed to be commutative, although I suppose that is contradictory with "The Combiner must remove any duplicate key-value pairs, in accordance with the specification. It can pick arbitrarily when conflicts occur." Will need to think on this a bit more. |
Thanks for clarifying this and apologies for the confusion. I would consider updating the BIP text to say that explicitly, it is kind of confusing that its in the formula but not mentioned anywhere in the text. One option for making Combine commutative, brought up by @apoelstra in rust-bitcoin/rust-bitcoin#5486 (comment), is to fail it entirely in case of conflicts. This is already mentioned in the BIP, as a may: "For every type that a Combiner understands, it may refuse to combine PSBTs if it detects that there will be inconsistencies or conflicts for that type in the combined PSBT."
Other than implying non-commutativity, this is also contradictory with "The resulting PSBT must contain all of the key-value pairs from each of the PSBTs" (which basically seems impossible to comply with if there are conflicts and should probably be removed?). |
The BIP asserts that
fA(fB(psbt)) == fB(fA(psbt)), however the explanatory text before this doesn't actually say this and even hints that the ordering does matter: "processing [..] A and then B in a sequence". It seems that the BIP text only supports theCombine(fA(psbt), fB(psbt)) == fB(fA(psbt))part, and thatfA(fB(psbt))slipped in by accident?In practice, Bitcoin Core's
combinepsbtisn't commutative and gives precedence to latter PSBTs in the array. Here's a quick example demonstrating this:And here's a related discussion about rust-bitcoin's
Psbt::combine(), which isn't commutative either but documented as "In accordance with BIP 174 this function is commutative": rust-bitcoin/rust-bitcoin#5486