MetacoSA / NBitcoin

Comprehensive Bitcoin library for the .NET framework.
MIT License
1.88k stars 848 forks source link

[WIP] Taproot scripts validation #1019

Closed lontivero closed 3 years ago

lontivero commented 3 years ago

This is just a work-in-progress PR to get early feedback. I will try to make small commits to make a commit-by-commit review easier.

dangershony commented 3 years ago

Great job starting on this. I assume you are porting form core, in that case perhaps posting the links to where in core the code is ported form will be useful? (it will make it easier to help with review)

lontivero commented 3 years ago

Everything is in this PR: Merge #19953: Implement BIP 340-342 validation (Schnorr/taproot/tapscript).

The changes contained in this PR up to now can be found in Implement Tapscript script validation rules (BIP 342) It would have sound more logical to start with Implement Taproot validation (BIP 341) but I suspected that could overlap with what Nicolas is working on.

Anyway, my approach for this is: I read the BIP to understand what has to be done then check Bitcoin Core (interpreter.cpp mostly) code to see how that translates to real-world code and whether it is small enough to be ported at the moment or not; go back to read the BIP again.

lontivero commented 3 years ago

Yes, that's true.

NicolasDorier commented 3 years ago

You need to add the test vectors probably updating script_tests.json and tx_valid.json, tx_invalid.json should be enough to test all the paths. (you can pick those on Bitcoin core repo)

onvej-sl commented 3 years ago

I think ExtractScriptPubKeyParameters2 erroneously returns Version = 0x51 (instead of Version = 0x01) for taproot script pubkeys.

This is because the first byte of a taproot script pubkey is 0x51 (rather than 0x01), which is the opcode for OP_1. This is something that it's not written very clearly in BIP141:

A scriptPubKey (or redeemScript as defined in BIP16/P2SH) that consists of a 1-byte push opcode (for 0 to 16) followed by a data push between 2 and 40 bytes gets a new special meaning. The value of the first push is called the "version byte". The following byte vector pushed is called the "witness program".

Nevertheless, it's obvious from how it is implemented in bitcoin core.

NicolasDorier commented 3 years ago

@onvej-sl this pass.

[Fact]
[Trait("UnitTest", "UnitTest")]
public void CanExtractTaprootScript()
{
    var a = BitcoinAddress.Create("bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqzk5jj0", Network.Main);
    var res = PayToWitTemplate.Instance.ExtractScriptPubKeyParameters2(a.ScriptPubKey);
    Assert.Equal(OpcodeType.OP_1, res.Version);
}
onvej-sl commented 3 years ago

@onvej-sl this pass.

The test confirms what I say, since it asserts that the version of the script pub key is OpcodeType.OP_1 = 0x51.

The version byte is compared with 0x01 here. So either

NicolasDorier commented 3 years ago

I'll take over this. All the taproot signing stuff for key spend is possible, now I just need the script evaluator.

NicolasDorier commented 3 years ago

supersede by https://github.com/MetacoSA/NBitcoin/pull/1026

I did not ported everything yet, just key spend. Will do the rest in a later version of NBitcoin.

NicolasDorier commented 3 years ago

This complete the work: https://github.com/MetacoSA/NBitcoin/pull/1037