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

Be more explicit about ext_flag values in BIP118 #1335

Closed
wants to merge 1 commit into from

Conversation

instagibbs
Copy link
Member

I find it difficult to reason about as-written, even if implicitly it may be correct.

cc @ajtowns

I find it difficult to reason about as-written, even if implicitly it may be correct.
@@ -82,7 +82,7 @@ We define the following constants using bits 6 and 7 of <code>hash_type</code>:
* <code>SIGHASH_ANYPREVOUT = 0x40</code>
* <code>SIGHASH_ANYPREVOUTANYSCRIPT = 0xc0</code>

As per [[bip-0341.mediawiki|BIP 341]], the parameter ''ext_flag'' is an integer in the range 0-127, used for indicating that extensions are added at the end of the message. The parameter ''key_version'' is an 8-bit unsigned value (an integer in the range 0-255) used for committing to the public key version.
As per [[bip-0341.mediawiki|BIP 341]], the parameter ''ext_flag'' is an integer in the range 0-127, used for indicating that extensions are added at the end of the message, with the value of ''0'' indicating no extension, and ''1'' indicating the presence of ''SigExt118(hash_type,key_version)''. The parameter ''key_version'' is an 8-bit unsigned value (an integer in the range 0-255) used for committing to the public key version.
Copy link
Contributor

Choose a reason for hiding this comment

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

What it's intending to describe is:

  • BIP 341's SigMsg behaviour is covered by SigMsg(h,0) = SigMsg118((h & ~0x40), 0)
  • BIP 342's is covered by SigMsg(h, 1) = SigMsg118((h & ~0x40), 1) and ext = SigExt118(h, 0)
  • BIP 118 introduces new behaviour for SigMsg118((h | 0x40), 1) and SigExt118(h, 1)

But maybe it would be better to change it entirely, and describe it more like:

  • If sig is not 64 or 65 bytes, fail.
  • If sig is 64 bytes:
    • hash_type = 0
    • sig_data = sig
  • If sig is 65 bytes:
    • hash_type = sig[64]. If hash_type == 0, fail.
    • sig_data = sig[0:64]
  • Return Verify(p, hash<sub>TapSigHash</sub>(0x00 || msg_and_ext, sig_data) where Verify is as defined in BIP 340, and where msg_and_ext is defined as:
    • If hash_type & 0x40 = 0, msg = SigMsg(hash_type, 1) per BIP341.
    • If hash_type & 0x40 != 0, msg = SigMsg118(hash_type) as "above".
    • msg_and_ext = msg || Ext118(hash_type) as "above".

Then the definition of SigMsg118 only needs to deal with ANYPREVOUT signatures, and Ext118 can hard code key version 0x01.

Copy link
Contributor

Choose a reason for hiding this comment

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

See #1367

Copy link
Member Author

Choose a reason for hiding this comment

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

will review

@ajtowns
Copy link
Contributor

ajtowns commented Oct 7, 2022

#1367 was merged, so I guess this can be closed?

@instagibbs
Copy link
Member Author

instagibbs commented Oct 7, 2022 via email

@fanquake fanquake closed this Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants