diff options
author | MarcoFalke <falke.marco@gmail.com> | 2021-11-15 10:56:16 +0100 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2021-11-16 08:20:33 +0100 |
commit | fa3e0da06b491b8c0fa2dbae37682a9112c9deb8 (patch) | |
tree | 3b4433c45304ee8e430858cef37655a91b729c97 | |
parent | 15d109802ab93b0af9647858c9d8adcd8a2db84a (diff) |
policy: Treat taproot as always active
-rw-r--r-- | src/bench/ccoins_caching.cpp | 2 | ||||
-rw-r--r-- | src/interfaces/chain.h | 3 | ||||
-rw-r--r-- | src/node/interfaces.cpp | 6 | ||||
-rw-r--r-- | src/policy/policy.cpp | 11 | ||||
-rw-r--r-- | src/policy/policy.h | 3 | ||||
-rw-r--r-- | src/test/fuzz/coins_view.cpp | 3 | ||||
-rw-r--r-- | src/test/fuzz/transaction.cpp | 3 | ||||
-rw-r--r-- | src/test/script_p2sh_tests.cpp | 6 | ||||
-rw-r--r-- | src/test/transaction_tests.cpp | 2 | ||||
-rw-r--r-- | src/validation.cpp | 3 | ||||
-rw-r--r-- | src/wallet/rpcdump.cpp | 12 | ||||
-rwxr-xr-x | test/functional/feature_taproot.py | 31 | ||||
-rwxr-xr-x | test/functional/wallet_taproot.py | 15 |
13 files changed, 42 insertions, 58 deletions
diff --git a/src/bench/ccoins_caching.cpp b/src/bench/ccoins_caching.cpp index d5275b0b76..dae3a47cd7 100644 --- a/src/bench/ccoins_caching.cpp +++ b/src/bench/ccoins_caching.cpp @@ -45,7 +45,7 @@ static void CCoinsCaching(benchmark::Bench& bench) // Benchmark. const CTransaction tx_1(t1); bench.run([&] { - bool success = AreInputsStandard(tx_1, coins, false); + bool success{AreInputsStandard(tx_1, coins)}; assert(success); }); ECC_Stop(); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index d4ceb517dd..38004ce95d 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -287,9 +287,6 @@ public: //! to be prepared to handle this by ignoring notifications about unknown //! removed transactions and already added new transactions. virtual void requestMempoolTransactions(Notifications& notifications) = 0; - - //! Check if Taproot has activated - virtual bool isTaprootActive() = 0; }; //! Interface to let node manage chain clients (wallets, or maybe tools for diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 9b4ba8eb97..a80209dcab 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -713,12 +713,6 @@ public: notifications.transactionAddedToMempool(entry.GetSharedTx(), 0 /* mempool_sequence */); } } - bool isTaprootActive() override - { - LOCK(::cs_main); - const CBlockIndex* tip = Assert(m_node.chainman)->ActiveChain().Tip(); - return DeploymentActiveAfter(tip, Params().GetConsensus(), Consensus::DEPLOYMENT_TAPROOT); - } NodeContext& m_node; }; } // namespace diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index fced397e51..2c30b20d5b 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -161,13 +161,13 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR * * Note that only the non-witness portion of the transaction is checked here. */ -bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs, bool taproot_active) +bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs) { - if (tx.IsCoinBase()) + if (tx.IsCoinBase()) { return true; // Coinbases don't use vin normally + } - for (unsigned int i = 0; i < tx.vin.size(); i++) - { + for (unsigned int i = 0; i < tx.vin.size(); i++) { const CTxOut& prev = mapInputs.AccessCoin(tx.vin[i].prevout).out; std::vector<std::vector<unsigned char> > vSolutions; @@ -189,9 +189,6 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs, if (subscript.GetSigOpCount(true) > MAX_P2SH_SIGOPS) { return false; } - } else if (whichType == TxoutType::WITNESS_V1_TAPROOT) { - // Don't allow Taproot spends unless Taproot is active. - if (!taproot_active) return false; } } diff --git a/src/policy/policy.h b/src/policy/policy.h index f2a3f35546..f6ac6500f6 100644 --- a/src/policy/policy.h +++ b/src/policy/policy.h @@ -105,10 +105,9 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR /** * Check for standard transaction types * @param[in] mapInputs Map of previous transactions that have outputs we're spending -* @param[in] taproot_active Whether or taproot consensus rules are active (used to decide whether spends of them are permitted) * @return True if all inputs (scriptSigs) use only standard transaction forms */ -bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs, bool taproot_active); +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 diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 325a9a170e..2f33598348 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -221,8 +221,7 @@ FUZZ_TARGET_INIT(coins_view, initialize_coins_view) assert(expected_code_path); }, [&] { - (void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache, false); - (void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache, true); + (void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache); }, [&] { TxValidationState state; diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index a21e5cea0c..ba6c500543 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -98,8 +98,7 @@ FUZZ_TARGET_INIT(transaction, initialize_transaction) CCoinsView coins_view; const CCoinsViewCache coins_view_cache(&coins_view); - (void)AreInputsStandard(tx, coins_view_cache, false); - (void)AreInputsStandard(tx, coins_view_cache, true); + (void)AreInputsStandard(tx, coins_view_cache); (void)IsWitnessStandard(tx, coins_view_cache); UniValue u(UniValue::VOBJ); diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp index d8a44a65dd..17b3359624 100644 --- a/src/test/script_p2sh_tests.cpp +++ b/src/test/script_p2sh_tests.cpp @@ -343,7 +343,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) txTo.vin[3].scriptSig << OP_11 << OP_11 << std::vector<unsigned char>(oneAndTwo.begin(), oneAndTwo.end()); txTo.vin[4].scriptSig << std::vector<unsigned char>(fifteenSigops.begin(), fifteenSigops.end()); - BOOST_CHECK(::AreInputsStandard(CTransaction(txTo), coins, false)); + BOOST_CHECK(::AreInputsStandard(CTransaction(txTo), coins)); // 22 P2SH sigops for all inputs (1 for vin[0], 6 for vin[3], 15 for vin[4] BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txTo), coins), 22U); @@ -356,7 +356,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) txToNonStd1.vin[0].prevout.hash = txFrom.GetHash(); txToNonStd1.vin[0].scriptSig << std::vector<unsigned char>(sixteenSigops.begin(), sixteenSigops.end()); - BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd1), coins, false)); + BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd1), coins)); BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd1), coins), 16U); CMutableTransaction txToNonStd2; @@ -368,7 +368,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard) txToNonStd2.vin[0].prevout.hash = txFrom.GetHash(); txToNonStd2.vin[0].scriptSig << std::vector<unsigned char>(twentySigops.begin(), twentySigops.end()); - BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd2), coins, false)); + BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd2), coins)); BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), coins), 20U); } diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 252a85c282..e05781a6f0 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -406,7 +406,7 @@ BOOST_AUTO_TEST_CASE(test_Get) t1.vout[0].nValue = 90*CENT; t1.vout[0].scriptPubKey << OP_1; - BOOST_CHECK(AreInputsStandard(CTransaction(t1), coins, false)); + BOOST_CHECK(AreInputsStandard(CTransaction(t1), coins)); } static void CreateCreditAndSpend(const FillableSigningProvider& keystore, const CScript& outscript, CTransactionRef& output, CMutableTransaction& input, bool success = true) diff --git a/src/validation.cpp b/src/validation.cpp index f163130a18..881b0abc74 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -723,8 +723,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } // Check for non-standard pay-to-script-hash in inputs - const bool taproot_active = DeploymentActiveAfter(m_active_chainstate.m_chain.Tip(), args.m_chainparams.GetConsensus(), Consensus::DEPLOYMENT_TAPROOT); - if (fRequireStandard && !AreInputsStandard(tx, m_view, taproot_active)) { + if (fRequireStandard && !AreInputsStandard(tx, m_view)) { return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs"); } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 403c978680..e0c5eef96b 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1548,18 +1548,6 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c } } - // Taproot descriptors cannot be imported if Taproot is not yet active. - // Check if this is a Taproot descriptor - CTxDestination dest; - ExtractDestination(scripts[0], dest); - if (std::holds_alternative<WitnessV1Taproot>(dest)) { - // Check if Taproot is active - if (!wallet.chain().isTaprootActive()) { - // Taproot is not active, raise an error - throw JSONRPCError(RPC_WALLET_ERROR, "Cannot import tr() descriptor when Taproot is not active"); - } - } - // If private keys are enabled, check some things. if (!wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { if (keys.keys.empty()) { diff --git a/test/functional/feature_taproot.py b/test/functional/feature_taproot.py index 5f90e49de1..932742ad00 100755 --- a/test/functional/feature_taproot.py +++ b/test/functional/feature_taproot.py @@ -89,6 +89,7 @@ from test_framework.address import ( hash160, ) from collections import OrderedDict, namedtuple +from enum import Enum from io import BytesIO import json import hashlib @@ -423,7 +424,7 @@ def spend(tx, idx, utxos, **kwargs): # Each spender is a tuple of: # - A scriptPubKey which is to be spent from (CScript) # - A comment describing the test (string) -# - Whether the spending (on itself) is expected to be standard (bool) +# - Whether the spending (on itself) is expected to be standard (Enum.Standard) # - A tx-signing lambda returning (scriptsig, witness_stack), taking as inputs: # - A transaction to sign (CTransaction) # - An input position (int) @@ -435,8 +436,14 @@ def spend(tx, idx, utxos, **kwargs): # - Whether this test demands being placed in a txin with no corresponding txout (for testing SIGHASH_SINGLE behavior) Spender = namedtuple("Spender", "script,comment,is_standard,sat_function,err_msg,sigops_weight,no_fail,need_vin_vout_mismatch") +# The full node versions that treat the tx standard. +# ALL means any version +# V23 means the major version 23.0 and any later version +# NONE means no version +Standard = Enum('Standard', 'ALL V23 NONE') -def make_spender(comment, *, tap=None, witv0=False, script=None, pkh=None, p2sh=False, spk_mutate_pre_p2sh=None, failure=None, standard=True, err_msg=None, sigops_weight=0, need_vin_vout_mismatch=False, **kwargs): + +def make_spender(comment, *, tap=None, witv0=False, script=None, pkh=None, p2sh=False, spk_mutate_pre_p2sh=None, failure=None, standard=Standard.ALL, err_msg=None, sigops_weight=0, need_vin_vout_mismatch=False, **kwargs): """Helper for constructing Spender objects using the context signing framework. * tap: a TaprootInfo object (see taproot_construct), for Taproot spends (cannot be combined with pkh, witv0, or script) @@ -446,13 +453,18 @@ def make_spender(comment, *, tap=None, witv0=False, script=None, pkh=None, p2sh= * p2sh: whether the output is P2SH wrapper (this is supported even for Taproot, where it makes the output unencumbered) * spk_mutate_pre_psh: a callable to be applied to the script (before potentially P2SH-wrapping it) * failure: a dict of entries to override in the context when intentionally failing to spend (if None, no_fail will be set) - * standard: whether the (valid version of) spending is expected to be standard + * standard: whether the (valid version of) spending is expected to be standard (True is mapped to Standard.ALL, False is mapped to Standard.NONE) * 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. """ + if standard == True: + standard = Standard.ALL + elif standard == False: + standard = Standard.NONE + conf = dict() # Compute scriptPubKey and set useful defaults based on the inputs. @@ -1137,12 +1149,12 @@ def spenders_taproot_inactive(): tap = taproot_construct(pub, scripts) # Test that keypath spending is valid & non-standard, regardless of validity. - add_spender(spenders, "inactive/keypath_valid", key=sec, tap=tap, standard=False) + add_spender(spenders, "inactive/keypath_valid", key=sec, tap=tap, standard=Standard.V23) add_spender(spenders, "inactive/keypath_invalidsig", key=sec, tap=tap, standard=False, sighash=bitflipper(default_sighash)) add_spender(spenders, "inactive/keypath_empty", key=sec, tap=tap, standard=False, witness=[]) # Same for scriptpath spending (and features like annex, leaf versions, or OP_SUCCESS don't change this) - add_spender(spenders, "inactive/scriptpath_valid", key=sec, tap=tap, leaf="pk", standard=False, inputs=[getter("sign")]) + add_spender(spenders, "inactive/scriptpath_valid", key=sec, tap=tap, leaf="pk", standard=Standard.V23, inputs=[getter("sign")]) add_spender(spenders, "inactive/scriptpath_invalidsig", key=sec, tap=tap, leaf="pk", standard=False, inputs=[getter("sign")], sighash=bitflipper(default_sighash)) add_spender(spenders, "inactive/scriptpath_invalidcb", key=sec, tap=tap, leaf="pk", standard=False, inputs=[getter("sign")], controlblock=bitflipper(default_controlblock)) add_spender(spenders, "inactive/scriptpath_valid_unkleaf", key=sec, tap=tap, leaf="future_leaf", standard=False, inputs=[getter("sign")]) @@ -1172,7 +1184,7 @@ def dump_json_test(tx, input_utxos, idx, success, failure): # The "final" field indicates that a spend should be always valid, even with more validation flags enabled # than the listed ones. Use standardness as a proxy for this (which gives a conservative underestimate). - if spender.is_standard: + if spender.is_standard == Standard.ALL: fields.append(("final", True)) def dump_witness(wit): @@ -1438,8 +1450,13 @@ class TaprootTest(BitcoinTestFramework): for i in range(len(input_utxos)): tx.vin[i].scriptSig = input_data[i][i != fail_input][0] tx.wit.vtxinwit[i].scriptWitness.stack = input_data[i][i != fail_input][1] + taproot_spend_policy = Standard.V23 if node.version is None else Standard.ALL # Submit to mempool to check standardness - is_standard_tx = fail_input is None and all(utxo.spender.is_standard for utxo in input_utxos) and tx.nVersion >= 1 and tx.nVersion <= 2 + is_standard_tx = ( + fail_input is None # Must be valid to be standard + and (all(utxo.spender.is_standard == Standard.ALL or utxo.spender.is_standard == taproot_spend_policy for utxo in input_utxos)) # All inputs must be standard + and tx.nVersion >= 1 # The tx version must be standard + and tx.nVersion <= 2) tx.rehash() msg = ','.join(utxo.spender.comment + ("*" if n == fail_input else "") for n, utxo in enumerate(input_utxos)) if is_standard_tx: diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py index b22c171374..d286b91491 100755 --- a/test/functional/wallet_taproot.py +++ b/test/functional/wallet_taproot.py @@ -237,20 +237,15 @@ class WalletTaprootTest(BitcoinTestFramework): assert_equal(len(rederive), 1) assert_equal(rederive[0], addr_g) - # tr descriptors cannot be imported when Taproot is not active + # tr descriptors can be imported regardless of Taproot status result = self.privs_tr_enabled.importdescriptors([{"desc": desc, "timestamp": "now"}]) assert(result[0]["success"]) result = self.pubs_tr_enabled.importdescriptors([{"desc": desc_pub, "timestamp": "now"}]) assert(result[0]["success"]) - if desc.startswith("tr"): - result = self.privs_tr_disabled.importdescriptors([{"desc": desc, "timestamp": "now"}]) - assert(not result[0]["success"]) - assert_equal(result[0]["error"]["code"], -4) - assert_equal(result[0]["error"]["message"], "Cannot import tr() descriptor when Taproot is not active") - result = self.pubs_tr_disabled.importdescriptors([{"desc": desc_pub, "timestamp": "now"}]) - assert(not result[0]["success"]) - assert_equal(result[0]["error"]["code"], -4) - assert_equal(result[0]["error"]["message"], "Cannot import tr() descriptor when Taproot is not active") + result = self.privs_tr_disabled.importdescriptors([{"desc": desc, "timestamp": "now"}]) + assert result[0]["success"] + result = self.pubs_tr_disabled.importdescriptors([{"desc": desc_pub, "timestamp": "now"}]) + assert result[0]["success"] def do_test_sendtoaddress(self, comment, pattern, privmap, treefn, keys_pay, keys_change): self.log.info("Testing %s through sendtoaddress" % comment) |