Skip to content

Conversation

@roconnor-blockstream
Copy link
Contributor

According to BIP-341, 'p' is called the taproot internal key, not inner key.

@fanquake fanquake requested a review from sipa February 21, 2021 09:26
@michaelfolkson
Copy link

ACK 56325c9be7a6ea3717efdf2ec5f244a659f26452

Only three comments in interpreter.cpp that refer to "inner key", all of them replaced in this PR. No reference to "inner key" in BIP 341

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

Note that there are also some occurences of "inner pubkey" in the functional taproot test and a test framework script helper module that could be tackled in this PR:

$ git grep "inner pubkey"
src/script/interpreter.cpp:    //! The inner pubkey (x-only, so no Y coordinate parity).
src/script/interpreter.cpp:    // Compute the tweak from the Merkle root and the inner pubkey.
src/script/interpreter.cpp:    // Verify that the output pubkey matches the tweaked inner pubkey, after correcting for parity.
test/functional/feature_taproot.py:    # The inner pubkey for a taproot script path spend (32 bytes).
test/functional/feature_taproot.py:    # The negation flag of the inner pubkey for a taproot script path spend.
test/functional/feature_taproot.py:    # Test that bitflips in the inner pubkey invalidate it.
test/functional/test_framework/script.py:# - inner_pubkey: the inner pubkey (32 bytes)

According to BIP-341, 'p' is called the taproot *internal* key, not inner key.
Copy link
Contributor

@benthecarman benthecarman left a comment

Choose a reason for hiding this comment

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

ACK 6a0a6e7

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

ACK 6a0a6e7

@maflcko maflcko changed the title Correction for VerifyTaprootCommitment comments doc: Correction for VerifyTaprootCommitment comments Mar 2, 2021
@maflcko maflcko added the Docs label Mar 2, 2021
@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 5, 2021

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@sipa
Copy link
Member

sipa commented Mar 5, 2021

ACK 6a0a6e7

I guess I'll need to make this change in #21365 as well.

@fanquake fanquake merged commit fbf5d16 into bitcoin:master Mar 5, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 5, 2021
…ents

6a0a6e7 Correction for VerifyTaprootCommitment comments (Russell O'Connor)

Pull request description:

  According to BIP-341, 'p' is called the taproot *internal* key, not inner key.

ACKs for top commit:
  sipa:
    ACK 6a0a6e7
  benthecarman:
    ACK 6a0a6e7
  theStack:
    ACK 6a0a6e7

Tree-SHA512: 94f553476a8404bff4b2d5724a1a54c5f530b987a616cd00a3800095f245c06e3c7a9066c729976f32069a56029406859a70ba523151d333dc1ed874f242bce8
@roconnor-blockstream roconnor-blockstream deleted the patch-1 branch March 5, 2021 13:58
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants