Taproot support

[AdamISZ]

Do you plan to support BIPs 340/341/342 ? Additionally, are you open to having PRs on that? (Not that I plan to, right now, just figuring out future paths both for @JoinMarket-Org and possibly some other projects).

[dgpv]

I would like to add support, but I cannot give any timeline when I will be able to dedicate the time for that. Definitely open for PRs (but since bitcointx code is fully type-annotated, any code in PRs should be, too)

[dgpv]

Done some translating of XOnlyPubKey and shnorrsig-related code from Core in taproot branch (not tested yet)

[dgpv]

Is there a bitcoin library (in any language) that has convenient and robust way for construction of taproot script trees? I want to find a good abstraction for this to use in python-bitcointx.

Test framework in Core has taproot_construct, which takes

     scripts: a list of items; each item is either:                              
             - a (name, CScript or bytes, leaf version) tuple                   
             - a (name, CScript or bytes) tuple (defaulting to leaf version 0xc0)
             - another list of items (with the same structure)

But while this is simple, and adequate for test framework, this might not be good abstraction for the library. I've looked at rust-bitcoin, but quick grep did not find me anything related to constructing taproot script trees. In a library API one would want to detect inconsistencies (no duplicate names for scripts?) and for the converstion to merkle tree to be quite deterministic, maybe for each input tree to correspond to only one output merkle tree... Maybe allow to set default leaf version for the sub-tree instead of the whole tree... Is there any library out there that have this figured out ?

Also mypy does not support recursive types at the moment, so no way to statically typecheck this recursive structure that test framework uses.

It might be OK to have it the same way as taproot_construct() in test framework at first, but if there's better abstraction found later, that would mean the library will have to support backward compatibility with the old structure

[dgpv]

Almost there.

Some code still not tested, though - TaprootScriptTree in particular, but it seems to be a fine API for constructing and handling script trees to me. CScript got an optional name attribute to allow lookup within TaprootScriptTree.

[AdamISZ]

Good to hear. I have not found time to look into this at all yet, but I can certainly help with review/test.

[dgpv]

There's a problem related to libsecp ABI being unversioned and scnorrsig being an experimental module.

In commit bitcoin-core/secp256k1@b6c0b72 the signature of secp256k1_schnorrsig_sign() was changed, and since python-bitcointx works with libsecp via directly resolving the symbols and hardconding function signatures, supporting "generic" libsecp (when using experimental modules) becomes burdensome ("generic" as in not pinned to specific commit, but whatever is available).

I currently look for the presence of secp256k1_schnorrsig_sign_custom that was introduced by that commit mentioned above, and treat any libsecp instances without this symbol as not supporting schnorrsig. But even that may change, as the module is still experimental in libsecp256k1.

Without looking into C headers as CFFI does, the only reliable solution I see is waiting for the libsecp256k1 ABI stabilization (probably, with versioning, AFAIK versioning the ABI of libsecp is mulled over now)

[dgpv]

Another option might be bundling a specific version (of specific commit) of libsecp into the python package, but then this means that the installation of the package will require a working compiler to build the library. So far python-bitcointx was able to avoid that, because the ABI of the library was stable enough.

[dgpv]

Another obstacle is that currently, the bitcoinconsensus library does not support SCRIPT_VERIFY_TAPROOT flag, so testing the script building (with TaprootScriptTree) is not convenient. I think I will wait until SCRIPT_VERIFY_TAPROOT is supported by libbitcoinconsensus, before continuing developing the tests.

[dgpv]

I've got TaprootScriptTree code working - building correct merkle paths with different tree configurations. But to do that, I've made an ad-hoc patch to libbitcoinconsensus to add bitcoinconsensus_verify_script_taproot (https://gist.github.com/dgpv/7bf8787ecb093e12c862287f4b86c3f4), and that allowed me to test the taproot scripts without the need of bitcoind. The patch is too ad-hoc and I'm sure there's a lot of things that it does not address, so I don't plan to submit it as PR to core. Rather, I will wait for the new BIP341 test vectors that are being prepared at bitcoin/bips#1225 and use them for tests. I will disable the tests that use my ad-hoc-patched libbitcoinconsensus until we've got a support for taproot in libbitcoinconsensus from Core.

[dgpv]

Updated the https://github.com/Simplexum/python-bitcointx/tree/taproot branch with the fixes for taproot to make it work. (the code that uses my ad-hoc patch to libbitcoinconsensus is disabled there, but I ran them on my machine and they pass)

[dgpv]

I've made a new issue to discuss the libsecp ABI problem: #59

[dgpv]

Added test cases from bitcoin/bips#1225 to https://github.com/Simplexum/python-bitcointx/tree/taproot

P2TRCoinAddress.from_pubkey() and P2TRCoinAddress.from_xonly_pubkey() now takes internal pubkey, for output pubkey there's P2TRCoinAddress.from_output_pubkey() and P2TRCoinAddress.from_xonly_output_pubkey()

I wonder if I should make P2TRCoinAddress.from_pubkey() and P2TRCoinAddress.from_output_pubkey() take xonly-pubkeys also (right now they only accept CPubKeys)

I plan to add a function to explicitly enable schnorr signatures (for those who complied libsecp with experimental schnorrsig module) and then make a PR to review.

[dgpv]

PR is ready for review: #60