feat: add support for pay to taproot

[motorina0]

Summary

This is the first version for the p2tr payment. It is not complete, but it covers most of the functionality. I'm opening this PR for others to test, review and provide feedback. Main changes listed below:

New payment type: P2TR

It follows the same approach as the other payments (p2wsh, p2pkh, ...)

• the PaymentOpts object takes an extra field: eccLib?: TinySecp256k1Interface
  • this field is required for P2TR (but not for the other payment types)
  • eccLib is required for the tweak operations. The ECC logic has been extracted out of this lib.
• examples of how to use P2TR can be found in the unit tests and integration tests (see tests section)

Payment interface

The Payment interface has 3 new fields:

• internalPubkey - the internal key as defined by BIP341.
• scriptTree - a Merkle tree whose leaves consist of a version number and a script (see BIP341)
• redeemVersion - the version of the leaf being spent (if the script-path spend is used). Default value: 0xc0

Some Payment fields have taproot specific meaning:

• pubkey - the tweaked internalPubkey as defined in BIP341

  • is computed as P + hash(P||m)G for a public key P, and the root m of the Merkle tree

• hash - the hash of the root of the Merkle scriptTree. Empty if not scriptTree is used.
• signature - a Schnorr signature, if key-path spent is used. Empty otherwise.
• redeem.output - the leaf script, for script-path spends

Taproot Helpers

• taproot related logic has been extracted to ts_src/payment/taprootutils.ts
• in the future it might be moved to a bip341 module. See https://github.com/bitcoinjs/bitcoinjs-lib/issues/1780

Address

• add stricter rules for Taproot (data size 32 and public key valid x-only coordinate)
• from|toOutputScript takes in an optional eccLib. If missing, then it skips the p2tr matching falls down to the final Error

PSBT

• it can now add Taproot inputs and outputs
• for the key-path spend the signer has to be tweaked (it does not perform the tweak internally)
• for the script-path spend a custom finalizer (FinalScriptsFunc) is required when a Taproot input is finalized. See test/psbt.utils.ts for such an example
• PsbtOptsOptional has a new field eccLib?: TinySecp256k1Interface;. eccLib is required if the PSBT instance uses taproot ins/outs
• the Signer interface has a new method: signSchnorr?(hash: Buffer): Buffer;. This method is required if a Taproot UTXO is being spent.

Tests

• unit test data
  • p2tr.json: data generated using the bitcoin test framework test/functional/wallet_taproot.py
  • BIP341 test cases: Test vectors for scriptPubKey construction recommended here by Pieter Wuille
• integration tests (test/integration/taproot.spec.ts)
• script-path spend on testnet (code sample and transaction links)

Further Comments

Note for reviewers: all .js files are generated, only the .ts files require review

[motorina0]

~~Note: lint expected to fail, see Summary~~

[junderw]

Thanks for the PR!

Just a quick note before I review:

This effort is duplicated with BitGo's fork, which is adding native JS schnorr and using TransactionBuilder.

https://github.com/BitGo/bitcoinjs-lib/blob/master/ts_src/payments/p2tr.ts

I will most likely pick and choose parts from both efforts and add everyone as a co-author.

[motorina0]

Thanks @junderw! That is fine with me. Do you have a timeline for the official release that will include the p2tr functionality?

[junderw]

timeline? Not really.

Right now:

1. Moving ecpair and bip32 libraries to a modular method of injecting the needed ecc interfaces. (following tiny-secp256k1, so we will ensure that tiny-secp256k1 implements the needed interface)
2. Adding segwit v1 sigHash to Transaction.
3. Adding p2tr Payment.
4. Adding support for Psbt.

4 will be more difficult since right now the extensions needed for taproot have not been decided yet, so it will need to be made as an experimental API (that isn't importable/exportable, similar to how TransactionBuilder used to hold extra data that can't be represented in the transaction serialization) added onto Psbt, which we will mark as experimental, and make it clear that the API will break on a patch/minor version change without warning.

I am thinking of just releasing the new v6 once we're done with 3, then we can just have a few examples of creating a Transaction manually using just the Transaction class and its methods...

Then once the taproot PSBT extensions are in stone, we will need to add support to bip174 for v2 PSBTs (I think taproot will require v2 support), then implement the taproot extensions, then implement into Psbt here, then maybe release v6.1.0.

I hope to get done with up to number 3 by beginning of next week ish.

[junderw]

Also, I think p2tr should also take and injected interface for the tweak operations. (which tiny-secp256k1 will implement)

I want to keep ECC stuff out of bitcoinjs-lib moving forward.

Basically, people will just add tiny-secp256k1 and pass it into ecpair, bip32, and p2ts etc. and it should just work.

Then anyone who implements a native JS version of tiny-secp256k1 with schnorr moving forward (ie. BitGo) they can inject it.

[junderw]

1-3 won't take too long, I just need to find time.

If you are looking for something to help on, a PR adding v2 support for bip174 would help implement 4 faster once taproot extensions are decided.

[junderw]

v6 pushed.

[motorina0]

rebased

[junderw]

I was hoping to keep p2tr a non-breaking change.

So rather than use a Factory to get all payments, I was thinking either:

1. p2tr is a factory
2. p2tr input arguments include an optional eccLib (which is the tiny-secp256k1 minimal interface needed) which will only be validated when the inputs require it... ie. if only address is input, we can only output the contents of the address, so we don't need eccLib... but if the input has a pubkey then we need eccLib to do the tweaking, so if we have pubkey without eccLib the validation logic will throw an Error.

I think 2 is better, since it allows people who don't need to perform calcs with ecc to not need an ecc library.

[motorina0]

p2tr input arguments include an optional eccLib

• [x] done

which is the tiny-secp256k1 minimal interface needed

• [x] done. Only isXOnlyPoint() and xOnlyPointAddTweak() are exposed/required

which will only be validated when the inputs require it...

• [x] done. Cases when the ecc lib is required:
  • check if internalPubkey and pubkey pass isXOnlyPoint()
  • internalPubkey and tweak are provided -> compute pubkey (taproot output key), using xOnlyPointAddTweak()
    • also validates that the tweaked internalPubKey matches the pubkey (taproot output key)

[motorina0]

I'm trying to figure out how this PR can be improved in order to get it merged sooner rather than later. This is what I could think off so far:

1. more code comments (explain code in relation to BIP341)
2. more unit tests (what scenarios?)
3. documentation on how to use p2tr

Any other ideas/suggestions/requests/complaints ?

[junderw]

I wonder if we should offer a tool for sorting leafs in order of weight (likelihood of use) It should not be required to sort in any sort of order, but we should maybe offer some sort of helper function to help out.

I don't think it should be a show stopper, but it would be nice to have.

[motorina0]

... but we should maybe offer some sort of helper function to help out.

It would definitely be nice to have this. I have added this task for it: https://github.com/bitcoinjs/bitcoinjs-lib/issues/1766 It might start some conversations about how the Inputs/Outputs should look like, so it is better to keep it separate from this PR.

[motorina0]

Me again...

I have made the following changes to the Psbt class. It allows to spend from taproot UTXOs and to send to taproot addresses. A testnet transaction created with this library: https://mempool.space/testnet/tx/735321667cf43ded23dc935f355c4b1b1e77d9a47d11c77d384af5094e1ad171

The main changes are listed below. They can be rolled back and included in a separate PR if needed. They are useful for testing the p2tr functionality:

• signInput() creates and serializes the Schnorr signature for the taproot inputs (key spend)
• validateSignaturesOfInput() validates taproot inputs for Key Spend signatures
• only SIGHASH_DEFAULT supported at the moment
• add tweakSigner() as static method
• add unit tests
• the Signer interface has an optional privateKey field (used for tweaking)
• the signInputHD() and signInputHDAsync() logic not implemented yet (waiting for feedback)
• direct dependency to tiny-secp256k1 introduced :(
  • it is awkward to pass an ecc lib from outside <- must be revisited
  • added 'tiny-secp256k1' to package.json deps <- must be revisited

I know that "4. Adding support for Psbt." requirements are more broad, this is just a start...

[motorina0]

... but this helper function is not the right path imo

• I agree...

it knows the tweak since the "recommended" tweak is of its own xonlypubkey

• this would mean adding a dependency in ecpair to 'create-hash' (in order to compute the tweak)
• it would also not work when the tweak of the script tree is used

One option is to change the signSchnorr() of ecpair: From:

• signSchnorr(hash: Buffer): Buffer

To:

• signSchnorr(hash: Buffer, tweakHash?: Buffer): Buffer

This would mean:

• the tweakHash is computed outside the Signer, no tweaking if falsy
• only signSchnorr() can sign tweaked (so far)
• signSchnorr() should also take extraEntropy optional param (missing now in ecpair)

Implementation quick view:

    signSchnorr(hash: Buffer, tweakHash?: Buffer, e?: Buffer): Buffer {
      if (!this.privateKey) throw new Error('Missing private key');
      if (!ecc.signSchnorr)
        throw new Error('signSchnorr not supported by ecc library');
      if (!tweakHash)
        return Buffer.from(ecc.signSchnorr(hash, this.privateKey, e));

      const privateKey =
        this.publicKey[0] === 2
          ? this.privateKey
          : ecc.privateNegate(this.privateKey!);

      const tweakedPrivateKey = ecc.privateAdd(privateKey, tweakHash);
      if (!tweakedPrivateKey) {
        throw new Error('Invalid tweaked private key!');
      }
      return Buffer.from(ecc.signSchnorr(hash, tweakedPrivateKey, e));
    }

[motorina0]

One option is to change the signSchnorr() of ecpair...

I've open a quick PR with these change https://github.com/bitcoinjs/ecpair/pull/6

[motorina0]

This helper function is out of place. It also is returning ECPair which we removed from the library in v6.

What was I thinking? I have removed these changes.

[motorina0]

@junderw what can I do to get this PR merged? Can we create a list of tasks to implement/test/document? I'm willing to put in the effort, but I feel a bit lost...

[junderw]

• Needs integration tests (so we can verify that we can actually make PSBTs and sign them and they are valid to Bitcoin Core)
• ECPair decisions might still affect the Signer interface changes...
• bip174 library should add support for the input keys that store taproot info. (I don't think we need to support PSBTv2 in order to support those new keys.)
• Psbt should also support taproot info. At least holding it. And the finalizing function should be able to access that data (so people could write their own custom finalizer for their script path spends.

Is about what I'm thinking right now.

Some of this could be split off into separate PRs tho.

1. Def want integration tests before merge
2. I would like to nail down the Signer changes before any published release. We could merge to master tho.

[junderw]

I think writing integration tests using Psbt will help uncover any shortcomings.

tbh, I think when you first write an integration, just do it normally like you would with any app you made. (Don't try to over optimize it with the createPayment abstraction I have there currently etc.)

That way you get some experience actually using the p2tr payment and Psbt with taproot, and maybe you'll find some things about the API that are unintuitive etc.

[junderw]

Also, since (I think) we use fromOutputScript etc. within Psbt, it might be a good time to create stricter rules for segwit v1 (taproot) in the address module as well.

This can also be a separate PR. (Was there already one?)

[junderw]

from|toOutputScript should take in an optional eccLib, and if there is no eccLib, skip the p2tr matching and let the logic fall down to the final Error (each function ends with a throw Error) or maybe figure out a good way to throw an error when we only know that they could be trying to en|decode p2tr...

[motorina0]

from|toOutputScript:

• changes here (if OK, will merge into this branch):
  • https://github.com/bitcoincoretech/bitcoinjs-lib/pull/2
• original issue here:
  • https://github.com/bitcoinjs/bitcoinjs-lib/issues/1750

[junderw]

Oh yeah that's good. It's probably better to merge into this PR.

[brandonblack]

With the goal of eventually converging to a single implementation, I have updated the BitGo version of Taproot support to work with v6 here: https://github.com/BitGo/bitcoinjs-lib/tree/6_sync . Not sure if anything we have there is currently of use, but wanted to mention it.

Main differences:

• This PR includes PSBT updates
• BitGo code includes MuSig2 aggregate key creation (but not signing)

[tiero]

• taproot related logic has been extracted to ts_src/payment/taprootutils.ts. There might be a better place for it.

What about naming it as bip341.ts and placing it in the root path like has been done for bip66 file?

(we are trying to merge v6 into LiquidJS, a bitcoinJS fork for Liquid Network, and we found the need to have some utils as well, you can look our work over here, ~~would love to be compatible~~ we are now compatible with your taprootutils in terms of method signature, ofc with different Tag prefixes for the Elements chain)

[motorina0]

@brandonblack

• thank you! There is certainly some useful functionality on that branch.
• I will try to see if there are some low hanging fruits.
• please not the this branch is still WIP, changes to the public APIs are still happening (mainly p2tr)

[motorina0]

@tiero that is a great idea. I have created this issue in order to keep the discussion separate (this tread is quite large already) https://github.com/bitcoinjs/bitcoinjs-lib/issues/1780

[motorina0]

@junderw

• integration tests have been added (in taproot.spec.ts) for:
  • [x] 2 x taproot key-path spend
    • p2tr with just the internal pub key
    • p2tr with a scriptTree (unused)
  • [x] 3 x taproot script-path spend
    • OP_CHECKSIG
    • OP_CHECKSEQUENCEVERIFY
    • OP_CHECKSIGADD (3-of-3)
      • 2-of-3 should be possible with some changes to the taproot finalizer
• more unit tests for psbt and p2tr
• various refactorings

LE: finer grained commits can be found here

[motorina0]

Updates:

• update the PR description and hide old comments (make it easier for reviewers to understand the changes)
• script-path spend example with this lib on testnet (code sample and transaction links)
• open small bug bounty (retweets help)