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

Update bip-0341.mediawiki #1348

Closed
wants to merge 2 commits into from
Closed

Update bip-0341.mediawiki #1348

wants to merge 2 commits into from

Conversation

Randy808
Copy link

@Randy808 Randy808 commented Aug 7, 2022

"If the spending conditions do not require a script path, the output key should commit to an unspendable script path instead of having no script path. This can be achieved by computing the output key point as Q = P + int(hash_TapTweak(bytes(P)))G"

Calling 'taproot_tree_helper' calls 'hash_TapLeaf' instead of 'hash_TapTweak'.

@jonasnick
Copy link
Contributor

Not sure what your point is exactly but the commit doesn't make sense. h is supposed to be a hash, not a tweaked public key.

@Randy808
Copy link
Author

Randy808 commented Aug 10, 2022

@jonasnick Yeah I wasn't looking too closely and put taproot_tweak_pubkey assuming it returned the tagged hash and assuming that taproot_tweak_seckey used the h parameter as the tagged hash directly (which I see now doesn't).

I guess the main thing I was trying to get at is why script_tree is a parameter to the taproot_sign_key in the 'Spending using the key path' section when no script is needed to compute the tweak_seckey for a key path signature**?

@Randy808
Copy link
Author

I'll assume script_tree and the subsequent call to taproot_tree_helper is there in the case where a valid internal key is chosen for an output that can also be spent with a script path.

Could an explicit case be added to return empty bytes like in https://github.com/bitcoin/bitcoin/blob/bada9636d7f2efbc620fd89107baa2bf3e64a6b8/test/functional/test_framework/script.py#L822?

@@ -199,6 +199,8 @@ The following function, <code>taproot_output_script</code>, returns a byte array

<source lang="python">
def taproot_tree_helper(script_tree):
if len(scripts) == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

scripts is undefined

@jonasnick
Copy link
Contributor

Thanks, I see what you're saying now. Here's my attempt at a fix: #1350. Feel free to mention there whether this addresses the issue you brought up.

@Randy808
Copy link
Author

@jonasnick
Another thing to look at might be the commented lines below:

def taproot_tweak_pubkey(pubkey, h):
    t = int_from_bytes(tagged_hash("TapTweak", pubkey + h)) # Line A
    if t >= SECP256K1_ORDER:
        raise ValueError
    Q = point_add(lift_x(pubkey), point_mul(G, t))  # Line B
    return 0 if has_even_y(Q) else 1, bytes_from_int(x(Q))

The addition of pubkey and h in 'Line A' implies pubkey and h are of type 'bytes' but the usage of lift_x(pubkey) in 'Line B' implies pubkey is an int (since bip340 reads "The function lift_x(x), where x is a 256-bit unsigned integer" ). Does it make sense to change pubkey to int(pubkey) in 'Line B' (just like how p is converted to an int in the definition of Q under the 'Script validation rules' section)?

There isn't a place to file issues for this repo and since this is a pretty small thing I thought I might just add the comment here

@jonasnick
Copy link
Contributor

Yes that makes sense. Good catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants