-
Notifications
You must be signed in to change notification settings - Fork 5.9k
443: Fix some mistakes, and add paragraph on fees #1876
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
Conversation
|
ACK on the fees explaination in e4e2b7c, I didn't review the other changes. The |
murchandamus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| The following values of the other parameters have special meanings: | ||
| * If the <code><taptree></code> is -1, it is replaced with the Merkle root of the current input's tapscript tree. If the taptree is the empty buffer, then the taptweak is skipped. | ||
| * If the <code><pk></code> is 0, it is replaced with the NUMS x-only pubkey <code>0x50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0</code> defined in [[bip-0340.mediawiki|BIP-340]]. If the <code><pk></code> is -1, it is replaced with the taproot internal key of the current input. | ||
| * If the <code><pk></code> is 0, it is replaced with the NUMS x-only pubkey <code>0x50929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0</code> defined in [[bip-0341.mediawiki|BIP-341]]. If the <code><pk></code> is -1, it is replaced with the taproot internal key of the current input. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With this change, are you referring to line 157 in BIP-341?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct.
jonatack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
This fixes some mistakes in the python pseudocode (that described an older version that is not aligned to the draft implementation in core), and fixes some minor other details.
Commit e4e2b7c also adds a paragraph to clarify that fees are expected to be paid exogenously. While not specific to CCV and rather common to most/all covenant proposals, it was suggested in the comments of the previous PR to have an explainer (cc @Sjors).