MetacoSA / NBitcoin

Comprehensive Bitcoin library for the .NET framework.
MIT License
1.87k stars 845 forks source link

Add taproot addresses #1016

Closed NicolasDorier closed 3 years ago

NicolasDorier commented 3 years ago

My main issue right now is that I don't know if we should create a new PubKey class or reuse the existing one...

NicolasDorier commented 3 years ago

It's kind of complicated. Because when you are using the taproot script path you NEED the PubKey with both coordinate to properly sign it.

You don't need it for the key path spend though. But on chain, you only get the 32 bytes, without the parity.

I would say in a sense that the 32 bytes could take the place of what was a uint256 hash for p2wsh. For all the rest, we would use PubKey.

NicolasDorier commented 3 years ago

I am thinking of having a type TaprootOutputPubKey which represent this 32 bytes. But this type will be more like KeyId/ScriptId than it looking like PubKey.

I would add Key.SignTaproot() and Key.GetTaprootOutputPubKey(). Those two methods assume the user of taproot is doing single sig operation.

More complicated stuff (interpreter), will be left for later PRs

dangershony commented 3 years ago

More complicated stuff (interpreter), will be left for later PRs

Great, sounds reasonable.

NicolasDorier commented 3 years ago

Ok this is ready. This is a really basic implementation only meant for software to be able to parse and send to Taproot addresses. I decided to create a separated PubKey type called TaprootPubKey, which for now is quite lean.

Note I support this in the net framework, but I do not perform any complicated crypto check whether the taproot pubkey is correct. This mean that legacy .net framework will still be able to parse addresses, and send to taproot, but that I will not support anything else related to taproot for such environment. Also some invalid taproot address would still be identified as valid. (which is harmless)

The crypto behind taproot is implemented by NBitcoin.Secp256k1 which heavily relies on Span which is not available in legacy .net framework. Supporting crypto stuff would mean I need to implement everything using bouncy castle, which I don't want to.

NicolasDorier commented 3 years ago

Merging this for now. Will probably make a breaking change to NBitcoin. The TxDestination class is not really good.