feat: add initial p2tr output script support #1726
Conversation
This introduces support for creating p2tr output scripts and addresses from one or more pubkeys and/or one or more scripts according to BIPs 340 & 341. Multiple pubkeys are aggregated according to the procedure defined by MuSig whereby all keys are concatenated and hashed, with the resulting hash multiplied against each pubkey to tweak it. These tweaked pub keys are added together to produce the internal pubkey. If only one pubkey is provided, then the internal pubkey is simply set to that. And if no pubkeys are provided, a key with no known secret key is used instead. Any scripts provided are arranged into a Huffman tree according to their weighted probability and then hashed to form a merkle tree. The root hash of this tree is then added to the internal pubkey to tweak it. This tweaked key is finally used for creating the output script which is then encoded using Bech32m. Test cases in this commit include examples from the Bitcoin Optech taproot workshop that derive a Bech32m address from scripts and an internal key. Follow-ups to this commit should include expanding the coverage and number of test cases as well as possibly restructuring existing code or moving logic to other packages/modules. We should also consider support for taproot output descriptors, which are currently under review and developmnet in Bitcoin Core. Ticket: BG-35571
|
|
||
| const { bech32m } = require('bech32'); | ||
|
|
||
| /** Internal key with unknown discrete logarithm for eliminating keypath spends */ |
There was a problem hiding this comment.
This should be noted that this is SHA256(uncompressedDER(GENERATOR_POINT))
| export function trimFirstByte(pubkey: Buffer): Buffer { | ||
| assert(pubkey.length === 33); | ||
| return pubkey.slice(1, 33); | ||
| } |
There was a problem hiding this comment.
Is this really all that's needed? If this is true, then both 02 and 03 versions of pubkeys will collapse onto one pubkey value.
This doesn't seem correct.
There was a problem hiding this comment.
Yes I'm pretty sure this is the case. For taproot purposes, we assume the positive Y coordinate by default. This is covered in more detail in the "Implicit Y coordinates" section of BIP340. My thinking here is ideally we would work with 32 byte keys for the key addition/subtraction natively with the secp256k1 dependency, I think that's possible if it were updated to use the latest libsecp256k1 (would need to update rust-secp256k1 first I believe). For now I'm just manually trimming the first byte where necessary, though.
| // sort keys in ascending order | ||
| pubkeys.sort(); | ||
|
|
||
| const trimmedPubkeys: Buffer[] = []; | ||
| pubkeys.forEach(pubkey => { | ||
| const trimmedPubkey = trimFirstByte(pubkey); | ||
| trimmedPubkeys.push(trimmedPubkey); | ||
| }); |
There was a problem hiding this comment.
Shouldn't these be sorted after trimming?
There was a problem hiding this comment.
Also, shouldn't there be a check (and throw an error) if two pubkeys are the same?
There was a problem hiding this comment.
(for p2ms we allow multiple of the same pubkey since Bitcoin allows this... but for aggregating, I think two of the same pubkey is not mathematically possible? I could be wrong.)
There was a problem hiding this comment.
You're correct about sorting after trimming. I think a sanity check here to prevent duplicate pubkeys makes sense even if it's technically allowable
|
Thanks for the comments @junderw. I actually opened this PR by mistake - I was intending to open it against our BitGo fork (here BitGo#8) for internal review before trying to merge changes upstream once they're in a more complete state. But I appreciate the input/feedback. I was meaning to bump the discussion in #1522 about the best path forward for developing and working on these changes. I also notice now from rereading that issue that there's a tagged hash module. I could use that here. For now I'm going to tweak the approach in my branch to make it a bit cleaner/more reusable. |
This introduces support for creating p2tr output scripts and addresses from one or more pubkeys and/or one or more scripts according to BIPs 340 & 341.
Multiple pubkeys are aggregated according to the procedure defined by MuSig whereby all keys are concatenated and hashed, with the resulting hash multiplied against each pubkey to tweak it. These tweaked pub keys are added together to produce the internal pubkey. If only one pubkey is provided, then the internal pubkey is simply set to that. And if no pubkeys are provided, a key with no known secret key is used instead.
Any scripts provided are arranged into a Huffman tree according to their weighted probability and then hashed to form a merkle tree. The root hash of this tree is then added to the internal pubkey to tweak it. This tweaked key is finally used for creating the output script which is then encoded using Bech32m.
Test cases in this commit include examples from the Bitcoin Optech taproot workshop that derive a Bech32m address from scripts and an internal key.
Follow-ups to this commit should include expanding the coverage and number of test cases as well as possibly restructuring existing code or moving logic to other packages/modules. We should also consider support for taproot output descriptors, which are currently under review and developmnet in Bitcoin Core.
Ticket: BG-35571