Skip to content

Commit c5ec036

Browse files
author
MarcoFalke
committed
Merge #20165: Only relay Taproot spends if next block has it active
3d0556d Increase feature_taproot inactive test coverage (Pieter Wuille) 525cbd4 Only relay Taproot spends if next block has it active (Pieter Wuille) Pull request description: There should be no change to mempool transaction behavior for witness v1 transactions as long as no activation is defined. Until that point, we should treat the consensus rules as under debate, and for soft-fork safety, that means spends should be treated as non-standard. It's possible to go further: don't relay them unless the consensus rules are actually active for the next block. This extends non-relay to the period where a deployment is defined, started, locked in, or failed. I see no downsides to this, and the code change is very simple. ACKs for top commit: Sjors: utACK 3d0556d MarcoFalke: review ACK 3d0556d 🏓 jnewbery: utACK 3d0556d Tree-SHA512: ca625a2981716b4b44e8f3722718fd25fd04e25bf3ca1684924b8974fca49f7c1d438fdd9dcdfbc091a442002e20d441d42c41a0e2096e74a61068da6c60267a
2 parents 867dbeb + 3d0556d commit c5ec036

File tree

9 files changed

+28
-16
lines changed

9 files changed

+28
-16
lines changed

src/bench/ccoins_caching.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ static void CCoinsCaching(benchmark::Bench& bench)
4545
// Benchmark.
4646
const CTransaction tx_1(t1);
4747
bench.run([&] {
48-
bool success = AreInputsStandard(tx_1, coins);
48+
bool success = AreInputsStandard(tx_1, coins, false);
4949
assert(success);
5050
});
5151
ECC_Stop();

src/policy/policy.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR
155155
*
156156
* Note that only the non-witness portion of the transaction is checked here.
157157
*/
158-
bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
158+
bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs, bool taproot_active)
159159
{
160160
if (tx.IsCoinBase())
161161
return true; // Coinbases don't use vin normally
@@ -183,6 +183,9 @@ bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs)
183183
if (subscript.GetSigOpCount(true) > MAX_P2SH_SIGOPS) {
184184
return false;
185185
}
186+
} else if (whichType == TxoutType::WITNESS_V1_TAPROOT) {
187+
// Don't allow Taproot spends unless Taproot is active.
188+
if (!taproot_active) return false;
186189
}
187190
}
188191

src/policy/policy.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,11 @@ bool IsStandard(const CScript& scriptPubKey, TxoutType& whichType);
9595
bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeRate& dust_relay_fee, std::string& reason);
9696
/**
9797
* Check for standard transaction types
98-
* @param[in] mapInputs Map of previous transactions that have outputs we're spending
98+
* @param[in] mapInputs Map of previous transactions that have outputs we're spending
99+
* @param[in] taproot_active Whether or taproot consensus rules are active (used to decide whether spends of them are permitted)
99100
* @return True if all inputs (scriptSigs) use only standard transaction forms
100101
*/
101-
bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs);
102+
bool AreInputsStandard(const CTransaction& tx, const CCoinsViewCache& mapInputs, bool taproot_active);
102103
/**
103104
* Check if the transaction is over standard P2WSH resources limit:
104105
* 3600bytes witnessScript size, 80bytes per witness stack element, 100 witness stack elements

src/test/fuzz/coins_view.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ void test_one_input(const std::vector<uint8_t>& buffer)
229229
break;
230230
}
231231
case 1: {
232-
(void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache);
232+
(void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache, false);
233+
(void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache, true);
233234
break;
234235
}
235236
case 2: {

src/test/fuzz/transaction.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,8 @@ void test_one_input(const std::vector<uint8_t>& buffer)
9595

9696
CCoinsView coins_view;
9797
const CCoinsViewCache coins_view_cache(&coins_view);
98-
(void)AreInputsStandard(tx, coins_view_cache);
98+
(void)AreInputsStandard(tx, coins_view_cache, false);
99+
(void)AreInputsStandard(tx, coins_view_cache, true);
99100
(void)IsWitnessStandard(tx, coins_view_cache);
100101

101102
UniValue u(UniValue::VOBJ);

src/test/script_p2sh_tests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
343343
txTo.vin[3].scriptSig << OP_11 << OP_11 << std::vector<unsigned char>(oneAndTwo.begin(), oneAndTwo.end());
344344
txTo.vin[4].scriptSig << std::vector<unsigned char>(fifteenSigops.begin(), fifteenSigops.end());
345345

346-
BOOST_CHECK(::AreInputsStandard(CTransaction(txTo), coins));
346+
BOOST_CHECK(::AreInputsStandard(CTransaction(txTo), coins, false));
347347
// 22 P2SH sigops for all inputs (1 for vin[0], 6 for vin[3], 15 for vin[4]
348348
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txTo), coins), 22U);
349349

@@ -356,7 +356,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
356356
txToNonStd1.vin[0].prevout.hash = txFrom.GetHash();
357357
txToNonStd1.vin[0].scriptSig << std::vector<unsigned char>(sixteenSigops.begin(), sixteenSigops.end());
358358

359-
BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd1), coins));
359+
BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd1), coins, false));
360360
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd1), coins), 16U);
361361

362362
CMutableTransaction txToNonStd2;
@@ -368,7 +368,7 @@ BOOST_AUTO_TEST_CASE(AreInputsStandard)
368368
txToNonStd2.vin[0].prevout.hash = txFrom.GetHash();
369369
txToNonStd2.vin[0].scriptSig << std::vector<unsigned char>(twentySigops.begin(), twentySigops.end());
370370

371-
BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd2), coins));
371+
BOOST_CHECK(!::AreInputsStandard(CTransaction(txToNonStd2), coins, false));
372372
BOOST_CHECK_EQUAL(GetP2SHSigOpCount(CTransaction(txToNonStd2), coins), 20U);
373373
}
374374

src/test/transaction_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ BOOST_AUTO_TEST_CASE(test_Get)
305305
t1.vout[0].nValue = 90*CENT;
306306
t1.vout[0].scriptPubKey << OP_1;
307307

308-
BOOST_CHECK(AreInputsStandard(CTransaction(t1), coins));
308+
BOOST_CHECK(AreInputsStandard(CTransaction(t1), coins, false));
309309
}
310310

311311
static void CreateCreditAndSpend(const FillableSigningProvider& keystore, const CScript& outscript, CTransactionRef& output, CMutableTransaction& input, bool success = true)

src/validation.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -690,7 +690,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
690690
}
691691

692692
// Check for non-standard pay-to-script-hash in inputs
693-
if (fRequireStandard && !AreInputsStandard(tx, m_view)) {
693+
const auto& params = args.m_chainparams.GetConsensus();
694+
auto taproot_state = VersionBitsState(::ChainActive().Tip(), params, Consensus::DEPLOYMENT_TAPROOT, versionbitscache);
695+
if (fRequireStandard && !AreInputsStandard(tx, m_view, taproot_state == ThresholdState::ACTIVE)) {
694696
return state.Invalid(TxValidationResult::TX_INPUTS_NOT_STANDARD, "bad-txns-nonstandard-inputs");
695697
}
696698

test/functional/feature_taproot.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,13 +1129,13 @@ def spenders_taproot_inactive():
11291129
]
11301130
tap = taproot_construct(pub, scripts)
11311131

1132-
# Test that keypath spending is valid & standard if compliant, but valid and nonstandard otherwise.
1133-
add_spender(spenders, "inactive/keypath_valid", key=sec, tap=tap)
1132+
# Test that keypath spending is valid & non-standard, regardless of validity.
1133+
add_spender(spenders, "inactive/keypath_valid", key=sec, tap=tap, standard=False)
11341134
add_spender(spenders, "inactive/keypath_invalidsig", key=sec, tap=tap, standard=False, sighash=bitflipper(default_sighash))
11351135
add_spender(spenders, "inactive/keypath_empty", key=sec, tap=tap, standard=False, witness=[])
11361136

1137-
# Same for scriptpath spending (but using future features like annex, leaf versions, or OP_SUCCESS is nonstandard).
1138-
add_spender(spenders, "inactive/scriptpath_valid", key=sec, tap=tap, leaf="pk", inputs=[getter("sign")])
1137+
# Same for scriptpath spending (and features like annex, leaf versions, or OP_SUCCESS don't change this)
1138+
add_spender(spenders, "inactive/scriptpath_valid", key=sec, tap=tap, leaf="pk", standard=False, inputs=[getter("sign")])
11391139
add_spender(spenders, "inactive/scriptpath_invalidsig", key=sec, tap=tap, leaf="pk", standard=False, inputs=[getter("sign")], sighash=bitflipper(default_sighash))
11401140
add_spender(spenders, "inactive/scriptpath_invalidcb", key=sec, tap=tap, leaf="pk", standard=False, inputs=[getter("sign")], controlblock=bitflipper(default_controlblock))
11411141
add_spender(spenders, "inactive/scriptpath_valid_unkleaf", key=sec, tap=tap, leaf="future_leaf", standard=False, inputs=[getter("sign")])
@@ -1451,7 +1451,11 @@ def run_test(self):
14511451

14521452
# Pre-taproot activation tests.
14531453
self.log.info("Pre-activation tests...")
1454-
self.test_spenders(self.nodes[0], spenders_taproot_inactive(), input_counts=[1, 2, 2, 2, 2, 3])
1454+
# Run each test twice; once in isolation, and once combined with others. Testing in isolation
1455+
# means that the standardness is verified in every test (as combined transactions are only standard
1456+
# when all their inputs are standard).
1457+
self.test_spenders(self.nodes[0], spenders_taproot_inactive(), input_counts=[1])
1458+
self.test_spenders(self.nodes[0], spenders_taproot_inactive(), input_counts=[2, 3])
14551459

14561460

14571461
if __name__ == '__main__':

0 commit comments

Comments
 (0)