BIP-110: Clarify P2A exemption in rule 3 requires an empty witness stack - #2225
Conversation
9085fc1 to
ed0d033
Compare
jonatack
left a comment
There was a problem hiding this comment.
@dathonohm This is less complete than #2218 that you have been obstructing.
|
Looks fine to me |
|
Added test vectors. |
murchandamus
left a comment
There was a problem hiding this comment.
Consensus rules should be clearly stated in the Specification section. The Rationale is not the right place for a singular mention of a consensus rule.
|
Just to be clear: NACK ed0d033. This does not adequately address the issue that not all proposed changes to consensus rules are explicitly and comprehensively described in the Specification. |
|
NACK ed0d033. The description of P2A is imprecise and potentially misleading. |
|
Split "Defined witness versions" into its own subsection. This format should be much harder to misinterpret. |
murchandamus
left a comment
There was a problem hiding this comment.
Thanks, looks good. The first commit is now obsolete, since it only makes changes that are superseded again by the third commit, so you could drop it. Otherwise ACK 3553edb.
Co-Authored-By: jarolrod <jarolrod@tutanota.com>
3553edb to
2dfb201
Compare
|
@murchandamus replaced commit 1 with commit 3; contents are identical to before. |
|
|
||
| * '''Witness v0''' with a 20-byte program (P2WPKH) or a 32-byte program (P2WSH), as defined in [[bip-0141.mediawiki|BIP 141]]. Other v0 program lengths are already invalid under BIP 141 itself. | ||
| * '''Witness v1''' with a 32-byte program (Taproot/P2TR), as defined in [[bip-0341.mediawiki|BIP 341]]. In a script path spend, the only defined Tapleaf version is 0xc0 (Tapscript), as defined in [[bip-0342.mediawiki|BIP 342]]. | ||
| * '''Witness v1''' with the 2-byte program 0x4e73 (P2A), and only when spent with an empty witness stack, as defined in [[bip-0433.mediawiki#specification|BIP 433]]. |
There was a problem hiding this comment.
I'm all for appeasing the author, as long as it's a clear win for documentations spec, integrity, and a win for implementers of this bip.
But the author is stating here, as written, that bip433 states a p2a must be spent with an empty witness, which is incorrect.
All attempts around the 8th rule, point to this being a clear isolatable consensus rule imposed by bip110
There was a problem hiding this comment.
But the author is stating here, as written, that bip433 states a p2a must be spent with an empty witness
That is indeed what it says:
"P2A inputs are considered standard by Bitcoin Core for spending if no witness data is attached."

There was a problem hiding this comment.
"Consensus meaning is unchanged"
There was a problem hiding this comment.
We've been over this; "Consensus meaning is unchanged" just means that P2A has no consensus meaning at all. That isn't relevant to BIP-110.
There was a problem hiding this comment.
P2A output type specifically states that no witness data is attached.
If witness data is attached to the above specification, can it still be considered P2A? @jarolrod
There was a problem hiding this comment.
P2A output type specifically states that no witness data is attached.
The output type is defined independently of the spend. It's a template, just like P2MS or P2PKH. Spending a P2A with witness data is currently consensus valid. The current relay policy, however, restricts it to an empty witness.
If witness data is attached to the above specification, can it still be considered P2A?
Undoubtedly, as BIP 433 is unequivocal in its definition of P2A: it's the output type, not the separate policy restricting its spend. In Bitcoin Core, IsPayToAnchor only looks at the scriptPubKey. It's like asking whether a 6-of-12 P2MS is still considered a P2MS despite it not relaying in Bitcoin Core due to policy. The answer is yes.
Edit: to address @dathonohm 's objection about "P2A has no consensus meaning at all". That's not true. The script pattern had existing consensus meaning before P2A (anyone can spend even with a non-empty witness), just like P2MS's script pattern had existing consensus meaning before BIP11 made it a standard template (and also didn't change any consensus rules).
There was a problem hiding this comment.
@delcin-raj Moreover, this PR itself also characterizes P2A by the output type, not the spending witness status.
There was a problem hiding this comment.
But the author (dathonohm) is stating here, as written, that bip433 states a p2a must be spent with an empty witness
That is indeed what it says:
"P2A inputs are considered standard by Bitcoin Core for spending if no witness data is attached."
@dathonohm Your statement is not equivalent to the text you've quoted in BIP 433. @john-moffett is correct and proposes a minimum viable clarification on this.
|
I second that the phrasing suggested by @john-moffett would be an improvement. I also agree that a version bump would be appropriate, although I could live with something less than a major bump, given the authors’ stance this is just a clarification to an already correct specification. That said, 2dfb201 resolves my main concern that the consensus rules should be documented in the specification. |
|
@murchandamus I decline to update the wording. It is correct as is. I don't think a version bump is required since the spec was only clarified, not changed, and no prior clarifications of the spec are listed in the changelog either. |
|
NACK 2dfb201. The description of P2A continues to be imprecise and potentially misleading. |
|
I don't want to necessarily get in the way of bip maintainers here as this is becoming a giant time and attention sink, but politely, it would be a disservice to bip433 to allow for the misrepresentation of its contents here. |
|
On the BIP-433 attribution — the quoted sentence is one of three in that Specification paragraph, and the two that follow are the ones in dispute:
Read whole, that is a standardness claim scoped explicitly to Bitcoin Core, immediately followed by a statement that consensus is unaffected. @instagibbs has confirmed in #2218 that those following sentences exist precisely to prevent the reading being applied here: "It's meant to precisely guard against a mis-reading of it to think that P2A spends cannot have witness data." So the empty-witness requirement is BIP-110's, not BIP-433's — which is all the line needs to say: "...and only when spent with an empty witness stack. P2A is defined in BIP 433; this witness restriction is imposed by this proposal." That attributes each requirement to the document that imposes it and doesn't require conceding anything about rule numbering. On the version: if the specification was only clarified and not changed, an implementation built from v1.0.0 alone would already pass the test vectors added in 2dfb201. If it would not, the text changed what implementers build. BIP 3 puts an incompatible specification change at MAJOR, and @murchandamus has offered to accept less. Declining any increment leaves implementers to detect this by diffing the file. |
It’s unclear to me why you continue to insist on making determinations about the meaning of another BIP, especially one that is so abundantly clear in its phrasing. @john-moffett’s suggestion is a neutral phrasing that sidesteps this unnecessary conflict and clarifies how BIP 110 is implemented correctly without the need to evaluate the meaning of BIP 433. I have already conceded (charitably, if I may say so) to entertain that the existing phrasing of BIP 110 implies the new proposed phrasing. Anyway, these two PRs are becoming a huge distraction. I’m gonna take another look later this week. |
|
@murchandamus I agree. I think the current phrasing, "Witness v1 with the 2-byte program 0x4e73 (P2A), and only when spent with an empty witness stack, as defined in BIP 433" is not incorrect or misleading in any way. BIP-433 objectively contains text about requiring an empty witness. |
|
NACK 2dfb201 |
|
ACK |
No it does not. BIP 433 states that an input spending a P2A output with an empty witness passes Bitcoin Core’s standardness rules. Your additional interpretation on top of this is exactly that. I suggest you incorporate John Moffett’s suggestion from above, which sidesteps this issue by only pointing at BIP 433 for the definition of the P2A output without making the claim that it says something it does not. |
Agree. |
Agree. |
This is a contradiction. "No it does not" implies that BIP-433 contains no text about requiring an empty witness, and "BIP 433 states that an input spending a P2A output with an empty witness passes Bitcoin Core’s standardness rules." implies that it does. Your first statement, "No it does not." is factually incorrect. If you like, I can change the wording to highlight that this is the relevant section of BIP-433. How about: |
The ambiguity, again, lies in what "required" means here vis-a-vis consensus versus standardness. That's been the running theme of this entire discussion. It does not require an empty witness for consensus, but does currently for standardness. "Require" is more naturally read as a consensus-related word, so @murchandamus 's objection is understandable and not a contradiction.
It's ~30% longer than my suggestion and no clearer. If anything, the unusually elaborate attribution might make a reader wonder why a simpler phrasing was avoided. Also, there is no "standardness rules" phrase or clause in BIP 433. |
|
Let’s try something different. John Moffett’s suggestion is correct, unambiguous, and neutral. Could you please explain what is lost by adopting John Moffett’s phrase? |
|
At this point, maybe a simple post-scriptum by the editors mentioning the discussion on #2218 could suffice. |
|
The sentence proposed by John is correct and captures the essence of what Dathon wanted to add. Since BIP 110 was now Deployed, I would suggest we just add John’s phrasing, move the BIP to Deployed, then move the BIP to Closed as it’s obsolete and does not pertain to Bitcoin. |
|
SGTM |
Co-authored-by: John Moffett <116917595+john-moffett@users.noreply.github.com>
No description provided.