Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BIP-341: Define c[0] as leaf version plus pubkey parity bit #1329

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

david-bakin
Copy link

Add explicit definition of c[0] as the leaf version and the public
key parity bit.

Prior to this change that interesting fact is only implicit in the code
of taproot_tweak_pubkey and taproot_sign_script. Making it explicit
will help reader understanding of the control block and the script
validation rules.

@luke-jr
Copy link
Member

luke-jr commented Jul 25, 2022

@sipa @jonasnick @ajtowns

Copy link
Contributor

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK for defining the term control byte. The BIP already uses this without a definition.

@@ -66,8 +66,10 @@ The following rules only apply when such an output is being spent. Any other out
* If there are at least two witness elements left, script path spending is used:
** Call the second-to-last stack element ''s'', the script.
** The last stack element is called the control block ''c'', and must have length ''33 + 32m'', for a value of ''m'' that is an integer between 0 and 128<ref>'''Why is the Merkle path length limited to 128?''' The optimally space-efficient Merkle tree can be constructed based on the probabilities of the scripts in the leaves, using the Huffman algorithm. This algorithm will construct branches with lengths approximately equal to ''log<sub>2</sub>(1/probability)'', but to have branches longer than 128 you would need to have scripts with an execution chance below 1 in ''2<sup>128</sup>''. As that is our security bound, scripts that truly have such a low chance can probably be removed entirely.</ref>, inclusive. Fail if it does not have such a length.
** Let ''c[0]'' be the ''control byte'' consisting of:
*** Let ''v = c[0] & 0xfe'' and call it the ''leaf version''<ref>'''What constraints are there on the leaf version?''' First, the leaf version cannot be odd as ''c[0] & 0xfe'' will always be even, and cannot be ''0x50'' as that would result in ambiguity with the annex. In addition, in order to support some forms of static analysis that rely on being able to identify script spends without access to the output being spent, it is recommended to avoid using any leaf versions that would conflict with a valid first byte of either a valid P2WPKH pubkey or a valid P2WSH script (that is, both ''v'' and ''v | 1'' should be an undefined, invalid or disabled opcode or an opcode that is not valid as the first opcode). The values that comply to this rule are the 32 even values between ''0xc0'' and ''0xfe'' and also ''0x66'', ''0x7e'', ''0x80'', ''0x84'', ''0x96'', ''0x98'', ''0xba'', ''0xbc'', ''0xbe''. Note also that this constraint implies that leaf versions should be shared amongst different witness versions, as knowing the witness version requires access to the output being spent.</ref>.
*** Let ''ppar = c[0] & 0x01'' and call it the ''public key parity''<ref>'''What is the public key parity?''' The encoding of whether the ''y''-coordinate of the EC point is even or odd, in the standard way that compressed keys are formed. See also the return value of <code>taproot_tweak_pubkey</code>.</ref>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"public key parity" sounds a bit imprecise and we don't use it anywhere. Instead we mostly refer to it as "parity of the public key's Y coordinate". How about we make the name of the constant more explicit, don't give it an informal name, explain what it's used for exactly and drop the footnote.

Let ''pyparity = c[0] & 0x01'' which indicates the parity of the Y coordinate of point ''Q'' (defined below).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. (Although I didn't add "defined below", ok?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Add explicit definition of _c[0]_ as the _leaf version_ and the public
key _parity bit_.

Prior to this change that interesting fact is only implicit in the code
of `taproot_tweak_pubkey` and `taproot_sign_script`.  Making it explicit
will help reader understanding of the control block and the script
validation rules.
@david-bakin david-bakin force-pushed the bip-341-define-leaf-version-plus-parity branch from c7247e4 to 1fccbd9 Compare July 27, 2022 17:33
- Give a proper (short) descriptive name to the public key parity bit and
  just describe it inline.
@jonasnick
Copy link
Contributor

Looks good, feel free to squash

@murchandamus
Copy link
Contributor

@sipa, @jonasnick, @ajtowns: Am I understanding right that you would like this to be merged?

@@ -66,8 +66,10 @@ The following rules only apply when such an output is being spent. Any other out
* If there are at least two witness elements left, script path spending is used:
** Call the second-to-last stack element ''s'', the script.
** The last stack element is called the control block ''c'', and must have length ''33 + 32m'', for a value of ''m'' that is an integer between 0 and 128<ref>'''Why is the Merkle path length limited to 128?''' The optimally space-efficient Merkle tree can be constructed based on the probabilities of the scripts in the leaves, using the Huffman algorithm. This algorithm will construct branches with lengths approximately equal to ''log<sub>2</sub>(1/probability)'', but to have branches longer than 128 you would need to have scripts with an execution chance below 1 in ''2<sup>128</sup>''. As that is our security bound, scripts that truly have such a low chance can probably be removed entirely.</ref>, inclusive. Fail if it does not have such a length.
** Let ''c[0]'' be the ''control byte'' consisting of:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
** Let ''c[0]'' be the ''control byte'' consisting of:
** Let ''c[0]'' be the ''control byte''.

"Consisting of" doesn't go well with the bullet points that follow.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

Let c[0] be the control byte consisting of fields v and pyparity:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't work because v is not a subfield of the control byte but the entire control byte with the lowest bit masked off.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see what you mean.

Let c[0] be the control byte encoding v and pyparity:

@@ -66,8 +66,10 @@ The following rules only apply when such an output is being spent. Any other out
* If there are at least two witness elements left, script path spending is used:
** Call the second-to-last stack element ''s'', the script.
** The last stack element is called the control block ''c'', and must have length ''33 + 32m'', for a value of ''m'' that is an integer between 0 and 128<ref>'''Why is the Merkle path length limited to 128?''' The optimally space-efficient Merkle tree can be constructed based on the probabilities of the scripts in the leaves, using the Huffman algorithm. This algorithm will construct branches with lengths approximately equal to ''log<sub>2</sub>(1/probability)'', but to have branches longer than 128 you would need to have scripts with an execution chance below 1 in ''2<sup>128</sup>''. As that is our security bound, scripts that truly have such a low chance can probably be removed entirely.</ref>, inclusive. Fail if it does not have such a length.
** Let ''c[0]'' be the ''control byte'' consisting of:
*** Let ''v = c[0] & 0xfe'' and call it the ''leaf version''<ref>'''What constraints are there on the leaf version?''' First, the leaf version cannot be odd as ''c[0] & 0xfe'' will always be even, and cannot be ''0x50'' as that would result in ambiguity with the annex. In addition, in order to support some forms of static analysis that rely on being able to identify script spends without access to the output being spent, it is recommended to avoid using any leaf versions that would conflict with a valid first byte of either a valid P2WPKH pubkey or a valid P2WSH script (that is, both ''v'' and ''v | 1'' should be an undefined, invalid or disabled opcode or an opcode that is not valid as the first opcode). The values that comply to this rule are the 32 even values between ''0xc0'' and ''0xfe'' and also ''0x66'', ''0x7e'', ''0x80'', ''0x84'', ''0x96'', ''0x98'', ''0xba'', ''0xbc'', ''0xbe''. Note also that this constraint implies that leaf versions should be shared amongst different witness versions, as knowing the witness version requires access to the output being spent.</ref>.
*** Let ''pyparity = c[0] & 0x01'', the parity of the Y coordinate of point ''Q''.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q is used here before it's defined, and it doesn't make sense to include it in the definition of pyparity since we only check later whether it's the parity of the Y coordinate of Q.

Also, what is "pyparity" supposed to mean? Why not tweak_parity or just parity?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"pyparity" was suggested above and I think was to suggest "public-key's y-parity".

A naked "parity" seems too generic. "tweak_parity" seems a bit incomplete as a shorthand for "tweaked_public_key's_parity". ??

@murchandamus murchandamus added Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author and removed Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified labels May 8, 2024
@murchandamus
Copy link
Contributor

Hey, is there any update on this, @david-bakin?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author Proposed BIP modification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants