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 2 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?
| 2500 was chosen as the tightest value that did not make any non-pathological standard transaction | ||
| invalid[^7]. | ||
|
|
||
| In the presence of 64-byte transactions a block header's Merkle root may be valid for different sets |
There was a problem hiding this comment.
Commit 346f938 ACK, should the extra precision be added at the top of this paragraph too?
| In computing a block's Merkle root, a transaction with exactly 64 bytes of non-witness data can be | ||
| interpreted both as an intermediate node in the tree and as a leaf in the tree. This makes it | ||
| possible to fake inclusion proofs by pretending a 64-byte block transaction is an inner node, as | ||
| well as to pretend the inner nodes on one level of the tree are the actual block transactions. |
There was a problem hiding this comment.
I think it's worth being explicit here that it's not possible for a block to pass validation with a malleated merkle root. Also I think it's worth showing specifically the current approach that this fork replaces (suggestion in italics):
In computing a block's Merkle root, a transaction with exactly 64 bytes of non-witness data can be interpreted both as an intermediate node in the tree and as a leaf in the tree. While it is not possible for a malleated block to pass validation, this does makes it possible for an SPV server to fake inclusion proofs by pretending a 64-byte block transaction is an inner node, as well as to pretend the inner nodes on one level of the tree are the actual block transactions. Presently this must be guarded by obtaining the coinbase tx (e.g. with the header), verifying its inclusion, and that it starts with a null point.
| block. 64-byte transactions can only contain a scriptPubKey that lets anyone spend the funds, or one | ||
| that burns them. 64-byte transactions have also been non-standard since 2019 and unused since 2016. | ||
| It was suggested that the known vulnerabilities could instead be mitigated by committing to the | ||
| Merkle tree depth in the header's version field[^8], to avoid introducing a "seam". The authors |
There was a problem hiding this comment.
This implies the only viable alterative is the above suggestion (which was not mine). I suggested (which is not included anywhere in the discussion here) that mitigation may not be necessary given that:
- A block that is validated could not have been malleated.
- On nodes malleation only affects certain caching optimizations.
- Mitigation of (2) is no more costly than implementing this rule.
- On SPV wallets malleation affects inclusion proofs.
- Mitigation of (4) marginally complicates SPV clients.
- Eliminating (5) may or may not be worth the costs of adding a consensus rule.
| is necessary in the first place. See [this post][64 bytes debate] for an attempt at summarizing the | ||
| arguments for both sides of this debate. Reinterpreting Merkle tree nodes as transactions could be | ||
| used to cause a vulnerable node to fall out of consensus[^9]. There are better ways to mitigate this | ||
| issue on its own, but it is also addressed as a byproduct of invalidating 64-byte transactions. |
There was a problem hiding this comment.
There are better ways to mitigate this issue on its own
This hints at (1,2,3) above, but this isn't very clear. A "vulnerable node" is one that has implemented caching without considering malleation.
So what this says is that, with this rule, SPV wallets don't require the mitigation, which has a marginal runtime benefit, and as a byproduct nodes implement this rule instead of the mitigation. It's also worth pointing out that the caching node mitigations are to ensure:
- first tx in block starts with a null point
- there are no duplicate txs in the block
(1) is an immediate exit on block parse, as these are the first bytes after the header, and (2) is straightforward. And of course neither of these are possible with simple static validation (check) of the block.
All of the above was discussed in detail with examples, etc. on bitcoin dev. To provide more clarity it might be worth including that reference.
There was a problem hiding this comment.
IIRC number (2) above is a different aspect of malleability not mitigated by this proposal, so that node guard would remain in any case.
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.