From 18246ed5f09dd078fa1410b7ec2ba4379cc5e032 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 20 Oct 2020 15:08:54 -0700 Subject: Fix and improve taproot_construct comments --- test/functional/test_framework/script.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index 8e5848d493..26ccab3039 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -824,21 +824,33 @@ def taproot_tree_helper(scripts): h = TaggedHash("TapBranch", left_h + right_h) return (left + right, h) +# A TaprootInfo object has the following fields: +# - scriptPubKey: the scriptPubKey (witness v1 CScript) +# - inner_pubkey: the inner pubkey (32 bytes) +# - negflag: whether the pubkey in the scriptPubKey was negated from inner_pubkey+tweak*G (bool). +# - tweak: the tweak (32 bytes) +# - leaves: a dict of name -> TaprootLeafInfo objects for all known leaves TaprootInfo = namedtuple("TaprootInfo", "scriptPubKey,inner_pubkey,negflag,tweak,leaves") + +# A TaprootLeafInfo object has the following fields: +# - script: the leaf script (CScript or bytes) +# - version: the leaf version (0xc0 for BIP342 tapscript) +# - merklebranch: the merkle branch to use for this leaf (32*N bytes) TaprootLeafInfo = namedtuple("TaprootLeafInfo", "script,version,merklebranch") def taproot_construct(pubkey, scripts=None): """Construct a tree of Taproot spending conditions - pubkey: an ECPubKey object for the internal pubkey + pubkey: a 32-byte xonly pubkey for the internal pubkey (bytes) scripts: a list of items; each item is either: - - a (name, CScript) tuple - - a (name, CScript, leaf version) tuple + - 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) - - a function, which specifies how to compute the hashing partner - in function of the hash of whatever it is combined with + - a list of two items; the first of which is an item itself, and the + second is a function. The function takes as input the Merkle root of the + first item, and produces a (fictitious) partner to hash with. - Returns: script (sPK or redeemScript), tweak, {name:(script, leaf version, negation flag, innerkey, merklepath), ...} + Returns: a TaprootInfo object """ if scripts is None: scripts = [] -- cgit v1.2.3 From cdf900cbf26db05c7edb398ea645f1d23049d810 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 20 Oct 2020 15:11:07 -0700 Subject: Document need_vin_vout_mismatch argument to make_spender --- test/functional/feature_taproot.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py index 0f0fe8a34a..116eb7e3d7 100755 --- a/test/functional/feature_taproot.py +++ b/test/functional/feature_taproot.py @@ -444,6 +444,8 @@ def make_spender(comment, *, tap=None, witv0=False, script=None, pkh=None, p2sh= * standard: whether the (valid version of) spending is expected to be standard * err_msg: a string with an expected error message for failure (or None, if not cared about) * sigops_weight: the pre-taproot sigops weight consumed by a successful spend + * need_vin_vout_mismatch: whether this test requires being tested in a transaction input that has no corresponding + transaction output. """ conf = dict() -- cgit v1.2.3 From 8dbb7de67ce0a71f5fc54289c0ff048ac8dd0acc Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 20 Oct 2020 15:17:52 -0700 Subject: Add comments to VerifyTaprootCommitment --- src/script/interpreter.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 5735e7df66..bb5a7158a5 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -1834,9 +1834,13 @@ static bool ExecuteWitnessScript(const Span& stack_span, const CS static bool VerifyTaprootCommitment(const std::vector& control, const std::vector& program, const CScript& script, uint256& tapleaf_hash) { const int path_len = (control.size() - TAPROOT_CONTROL_BASE_SIZE) / TAPROOT_CONTROL_NODE_SIZE; + //! The inner pubkey (x-only, so no Y coordinate parity). const XOnlyPubKey p{uint256(std::vector(control.begin() + 1, control.begin() + TAPROOT_CONTROL_BASE_SIZE))}; + //! The output pubkey (taken from the scriptPubKey). const XOnlyPubKey q{uint256(program)}; + // Compute the tapleaf hash. tapleaf_hash = (CHashWriter(HASHER_TAPLEAF) << uint8_t(control[0] & TAPROOT_LEAF_MASK) << script).GetSHA256(); + // Compute the Merkle root from the leaf and the provided path. uint256 k = tapleaf_hash; for (int i = 0; i < path_len; ++i) { CHashWriter ss_branch{HASHER_TAPBRANCH}; @@ -1848,7 +1852,9 @@ static bool VerifyTaprootCommitment(const std::vector& control, c } k = ss_branch.GetSHA256(); } + // Compute the tweak from the Merkle root and the inner pubkey. k = (CHashWriter(HASHER_TAPTWEAK) << MakeSpan(p) << k).GetSHA256(); + // Verify that the output pubkey matches the tweaked inner pubkey, after correcting for parity. return q.CheckPayToContract(p, k, control[0] & 1); } -- cgit v1.2.3 From 6040de9a46725826330cd63cdf76e2121a18e728 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 20 Oct 2020 15:26:29 -0700 Subject: Add comments on CPubKey::IsValid --- src/pubkey.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/pubkey.h b/src/pubkey.h index 0f784b86e4..80d0c18540 100644 --- a/src/pubkey.h +++ b/src/pubkey.h @@ -170,6 +170,15 @@ public: /* * Check syntactic correctness. * + * When setting a pubkey (Set()) or deserializing fails (its header bytes + * don't match the length of the data), the size is set to 0. Thus, + * by checking size, one can observe whether Set() or deserialization has + * failed. + * + * This does not check for more than that. In particular, it does not verify + * that the coordinates correspond to a point on the curve (see IsFullyValid() + * for that instead). + * * Note that this is consensus critical as CheckECDSASignature() calls it! */ bool IsValid() const -- cgit v1.2.3 From ea0e78677bdbe3313f594118c500cf7784c56970 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 21 Oct 2020 13:03:38 -0700 Subject: Document additional IsWitnessStandard behavior --- src/policy/policy.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/policy/policy.h b/src/policy/policy.h index 8090dff4c6..466cf029a4 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -103,7 +103,9 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs, /** * Check if the transaction is over standard P2WSH resources limit: * 3600bytes witnessScript size, 80bytes per witness stack element, 100 witness stack elements - * These limits are adequate for multi-signature up to n-of-100 using OP_CHECKSIG, OP_ADD, and OP_EQUAL, + * These limits are adequate for multisignatures up to n-of-100 using OP_CHECKSIG, OP_ADD, and OP_EQUAL. + * + * Also enforce a maximum stack item size limit and no annexes for tapscript spends. */ bool IsWitnessStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs); -- cgit v1.2.3 From f867cbcc268a3bfaeef5510a7e40e6d3c0818b6d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 21 Oct 2020 13:20:14 -0700 Subject: Clean up assets test minimizer LDFLAGS --- src/Makefile.test.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 87166ecb79..f4c726b0b2 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -1105,7 +1105,7 @@ test_fuzz_script_interpreter_SOURCES = test/fuzz/script_interpreter.cpp test_fuzz_script_assets_test_minimizer_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) test_fuzz_script_assets_test_minimizer_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) test_fuzz_script_assets_test_minimizer_LDADD = $(FUZZ_SUITE_LD_COMMON) -test_fuzz_script_assets_test_minimizer_LDFLAGS = $(RELDFLAGS) $(AM_LDFLAGS) $(LIBTOOL_APP_LDFLAGS) +test_fuzz_script_assets_test_minimizer_LDFLAGS = $(FUZZ_SUITE_LDFLAGS_COMMON) test_fuzz_script_assets_test_minimizer_SOURCES = test/fuzz/script_assets_test_minimizer.cpp test_fuzz_script_ops_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) -- cgit v1.2.3 From 84e29c7c0141b52044020ec0c5dfa8a462b7e97f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 21 Oct 2020 13:45:02 -0700 Subject: Mention in validation that IsWitnessStandard tests for P2TR --- src/validation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 71402ef263..5ddee092bf 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -696,7 +696,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs"); } - // Check for non-standard witness in P2WSH + // Check for non-standard witnesses. if (tx.HasWitness() && fRequireStandard && !IsWitnessStandard(tx, m_view)) return state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, "bad-witness-nonstandard"); -- cgit v1.2.3 From 2d8099c713dfd4b546150fd53c2e4f364b9009f4 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 30 Oct 2020 14:56:16 -0700 Subject: Mention units of MAX_STANDARD_ policy constants --- src/policy/policy.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/policy/policy.h b/src/policy/policy.h index 466cf029a4..fc144e924b 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -38,11 +38,11 @@ static const unsigned int DEFAULT_BYTES_PER_SIGOP = 20; static const bool DEFAULT_PERMIT_BAREMULTISIG = true; /** The maximum number of witness stack items in a standard P2WSH script */ static const unsigned int MAX_STANDARD_P2WSH_STACK_ITEMS = 100; -/** The maximum size of each witness stack item in a standard P2WSH script */ +/** The maximum size in bytes of each witness stack item in a standard P2WSH script */ static const unsigned int MAX_STANDARD_P2WSH_STACK_ITEM_SIZE = 80; -/** The maximum size of each witness stack item in a standard BIP 342 script (Taproot, leaf version 0xc0) */ +/** The maximum size in bytes of each witness stack item in a standard BIP 342 script (Taproot, leaf version 0xc0) */ static const unsigned int MAX_STANDARD_TAPSCRIPT_STACK_ITEM_SIZE = 80; -/** The maximum size of a standard witnessScript */ +/** The maximum size in bytes of a standard witnessScript */ static const unsigned int MAX_STANDARD_P2WSH_SCRIPT_SIZE = 3600; /** Min feerate for defining dust. Historically this has been based on the * minRelayTxFee, however changing the dust limit changes which transactions are -- cgit v1.2.3