BIP 54: clarify 64-byte transactions item description and rationale#2159
BIP 54: clarify 64-byte transactions item description and rationale#2159darosior wants to merge 4 commits into
Conversation
As Eric points out on the mailing list: 1. the rationale section should mention and address the "seam" objection directly rather than burying it in a footnote; 2. the full node consensus split issue should not be used as sole rationale for invalidating 64-byte txs (but it's fair to point out it's fixed as a nice to have byproduct). ML thread: https://gnusha.org/pi/bitcoindev/43996cb3-9133-4627-8944-5fe08427be68n@googlegroups.com/T/#md66e252f0748f4ef7569d5e15d42631e12b66c0b,
jonatack
left a comment
There was a problem hiding this comment.
LGTM. Perhaps @JeremyRubin or @evoskuil could take a look?
|
Alright, i went further into rewriting the rationale. I removed the description of the source of malleability (inner node / leaf node) since it is already discussed in the motivation. I focused on discussing arguments in favour of the invalidation, and objections / alternatives. I relegated the discussion of the caching consensus bug to a footnote, since really it is largely orthogonal to this proposal. |
|
Since this rule is about reducing the complexity of SPV verification, the motivation should detail what that is. Presently its only described as a marginal reduction in complexity that requires a mitigation. Reading this fresh my first question would be, what specifically is the complexity/mitigation that this new rule will eliminate. I provided that high level description in the resolved comments above. How else can people weigh the tradeoff being proposed: new consensus rule or existing mitigation. I'm not taking a position on that question, but that is the question and should be presented clearly. |
|
I think really the issue is that SPV verifiers need to know a mitigation is necessary in the first place. Invalidating 64-byte transactions remove this footgun at the protocol level, so that applications do not have to care / know about it. This is what is stated in the motivation:
|
|
That's fine, but it's an opinion on a tradeoff. The actual mitigation that is being replaced should be described with sufficient detail for people to make an informed decision. One could also raise the real possibility that a specific size transaction being invalid is also a footgun, as transaction creation (and validation) must avoid it. |
|
I know of at least three mitigations: request an additional Merkle proof for the coinbase transaction and compare depths, reject proofs with inner nodes that deserialize to a Bitcoin transaction, and use modified Merkle proofs (request the preimage of the second SHA256, instead of the result of the double-SHA256). Each have their own tradeoffs, but they all have one thing in common: they are hacks to get around the root cause that the existence of 64-byte transactions make regular Merkle proofs insecure and implementers have to know about it. |
|
Also, this is not an opinion on a tradeoff. This is a fact: invalidating 64-byte transactions removes the need to know that a mitigation is necessary, and the cost of implementing it. |
There is no harm in presenting all three, and any others. I chose a simple one that requires no change to existing server behaviors (which is therefore presumably already in use).
This is an opinion. The objective here is to lay out the option, not to impose a preference or to lead people to it. Others may believe that the issue here is the imposition of a new consensus rule, with its own costs, in order to simplify SPV implementation. |
|
Re-reading, I'll admit to being initially confused if the word "mitigation," as used in this BIP, is referring to the same thing throughout (fixes proposed by the BIP), and where not, propose using a different term (or vice versa). |
| possible to trick an SPV verifier into accepting an inclusion proof for a transaction that is not | ||
| part of a block, by pretending a 64-byte block transaction is actually an inner node[^9]. Invalidating | ||
| 64-byte transactions addresses this vulnerability without requiring users of SPV verifiers to deploy | ||
| one of the available mitigations, or even to know one is necessary in the first place. |
There was a problem hiding this comment.
Might be helpful to link to a footnote or URL that describes the "available mitigations" (if I'm not confused, I may well be) and consider using a different term here than in the test vectors and acknowledgements sections, if they are referring to different things as they seem to be.
| reduction in complexity to SPV verifiers. Others have suggested that the known vulnerabilities could | ||
| instead be mitigated by committing to the Merkle tree depth in the header's version field[^8]. The | ||
| authors believe it is preferable to address the root cause by invalidating 64-byte transactions, | ||
| fixing the vulnerability without developers of SPV verifiers having to implement the mitigation or |
There was a problem hiding this comment.
A recent ML post pointed that not repeating across the BIP how the witness-stripped serialized size is considered for 64-byte transactions invalidation was confusing to some people. This makes the text a bit heavier, but in this PR we change two places to spell it out explicitly, in addition to the preexisting specifications section. Another modification was requested in the ML post, which i believe is incorrect and have responded to there.
A response on that same ML thread requested the rationale section to be clearer as to the motivation for invalidating 64-byte transactions. In particular, the point that it introduces a "seam" that may be surprising should be discussed and addressed in the main section instead of a footnote. Also, it should be clarified that full node consensus failure cannot be the main motivation for invalidating 64-byte transactions since it is better addressed alternatively. Both are done in the second commit of this PR.