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 extract BIP341 interface #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

[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]

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

• done

which is the tiny-secp256k1 minimal interface needed

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

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

• 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)

[junderw]

I was thinking it should be a member of the first argument a

[motorina0]

• a and the output o are related data structures (o is a after the computations and validations have been applied)
  • if eccLib is not expected to be present on o then it shouldn't be on a either
• the PaymentOpts looks be a better place for eccLib (eccLib is actually an option for the payment)

[junderw]

good point. PaymentOpts sounds better

[motorina0]

good point. PaymentOpts sounds better

Done!

[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]

not needed anymore

[motorina0]

Done!

[junderw]

only include the 1st 2nd and 5th vector. Remember these will be included in bundles people make, so the bare minimum to make sure it handles an edge case, and 1-2 main cases should be enough.

[motorina0]

Done!

[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: #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...

[junderw]

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

It would be much easier to create a new interface called TweakedSigner which has a method called signTweaked(hash: Buffer, tweak: Buffer, extraEntropy?: Buffer): Buffer etc.

Then we can implement that for ECPair and bip32 separately.

[junderw]

Although, another way to deal with it is to just tell the Signer it is to sign itself tweaked... and just automatically do that with signSchnorr when the internal flag is set... (it knows the tweak since the "recommended" tweak is of its own xonlypubkey)

I don't know which way is more useable... but this helper function is not the right path imo.

[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 bitcoinjs/ecpair#6

[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.

[motorina0]

@antonilol: thank you for your inputs. I have made the changes you suggested (except 2 of them, see comments)
@junderw:

• rebase master and solve conflicts
• bump version

[OttoAllmendinger]

was this supposed to be Array.isArray()? This is calling the constructor and always returns a truthy value

[OttoAllmendinger]

nevermind - we have a const Array = further down