Skip to content

Commit 55ac5f5

Browse files
ajtownsajtowns
committed
versionbits: Add explicit NEVER_ACTIVE deployments
Previously we used deployments that would timeout prior to Bitcoin's invention, which allowed the deployment to still be activated in unit tests. This switches those deployments to be truly never active.
1 parent dd07e6d commit 55ac5f5

File tree

6 files changed

+48
-41
lines changed

6 files changed

+48
-41
lines changed

‎src/chainparams.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,14 @@ class CMainParams : public CChainParams {
8181
consensus.nRuleChangeActivationThreshold = 1916; // 95% of 2016
8282
consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing
8383
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
84-
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = 1199145601; // January 1, 2008
85-
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = 1230767999; // December 31, 2008
84+
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
85+
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
8686
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay
8787

8888
// Deployment of Taproot (BIPs 340-342)
8989
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
90-
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = 1199145601; // January 1, 2008
91-
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = 1230767999; // December 31, 2008
90+
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
91+
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
9292
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].min_activation_height = 0; // No activation delay
9393

9494
consensus.nMinimumChainWork = uint256S("0x00000000000000000000000000000000000000001533efd8d716a517fe2c5008");
@@ -200,14 +200,14 @@ class CTestNetParams : public CChainParams {
200200
consensus.nRuleChangeActivationThreshold = 1512; // 75% for testchains
201201
consensus.nMinerConfirmationWindow = 2016; // nPowTargetTimespan / nPowTargetSpacing
202202
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
203-
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = 1199145601; // January 1, 2008
204-
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = 1230767999; // December 31, 2008
203+
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
204+
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
205205
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay
206206

207207
// Deployment of Taproot (BIPs 340-342)
208208
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].bit = 2;
209-
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = 1199145601; // January 1, 2008
210-
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = 1230767999; // December 31, 2008
209+
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
210+
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
211211
consensus.vDeployments[Consensus::DEPLOYMENT_TAPROOT].min_activation_height = 0; // No activation delay
212212

213213
consensus.nMinimumChainWork = uint256S("0x0000000000000000000000000000000000000000000001db6ec4ac88cf2272c6");
@@ -337,8 +337,8 @@ class SigNetParams : public CChainParams {
337337
consensus.MinBIP9WarningHeight = 0;
338338
consensus.powLimit = uint256S("00000377ae000000000000000000000000000000000000000000000000000000");
339339
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].bit = 28;
340-
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = 1199145601; // January 1, 2008
341-
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = 1230767999; // December 31, 2008
340+
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nStartTime = Consensus::BIP9Deployment::NEVER_ACTIVE;
341+
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].nTimeout = Consensus::BIP9Deployment::NO_TIMEOUT;
342342
consensus.vDeployments[Consensus::DEPLOYMENT_TESTDUMMY].min_activation_height = 0; // No activation delay
343343

344344
// Activation of Taproot (BIPs 340-342)

‎src/consensus/params.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ struct BIP9Deployment {
4343
* process (which takes at least 3 BIP9 intervals). Only tests that specifically test the
4444
* behaviour during activation cannot use this. */
4545
static constexpr int64_t ALWAYS_ACTIVE = -1;
46+
47+
/** Special value for nStartTime indicating that the deployment is never active.
48+
* This is useful for integrating the code changes for a new feature
49+
* prior to deploying it on some or all networks. */
50+
static constexpr int64_t NEVER_ACTIVE = -2;
4651
};
4752

4853
/**

‎src/rpc/blockchain.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,10 +1225,8 @@ static void BuriedForkDescPushBack(UniValue& softforks, const std::string &name,
12251225
static void BIP9SoftForkDescPushBack(UniValue& softforks, const std::string &name, const Consensus::Params& consensusParams, Consensus::DeploymentPos id) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
12261226
{
12271227
// For BIP9 deployments.
1228-
// Deployments (e.g. testdummy) with timeout value before Jan 1, 2009 are hidden.
1229-
// A timeout value of 0 guarantees a softfork will never be activated.
1230-
// This is used when merging logic to implement a proposed softfork without a specified deployment schedule.
1231-
if (consensusParams.vDeployments[id].nTimeout <= 1230768000) return;
1228+
// Deployments that are never active are hidden.
1229+
if (consensusParams.vDeployments[id].nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) return;
12321230

12331231
UniValue bip9(UniValue::VOBJ);
12341232
const ThresholdState thresholdState = VersionBitsState(::ChainActive().Tip(), consensusParams, id, versionbitscache);

‎src/test/fuzz/versionbits.cpp‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,12 @@ FUZZ_TARGET_INIT(versionbits, initialize)
163163
} else {
164164
if (fuzzed_data_provider.ConsumeBool()) {
165165
start_time = Consensus::BIP9Deployment::ALWAYS_ACTIVE;
166-
timeout = Consensus::BIP9Deployment::NO_TIMEOUT;
167166
always_active_test = true;
168167
} else {
169-
start_time = 1199145601; // January 1, 2008
170-
timeout = 1230767999; // December 31, 2008
168+
start_time = Consensus::BIP9Deployment::NEVER_ACTIVE;
171169
never_active_test = true;
172170
}
171+
timeout = fuzzed_data_provider.ConsumeBool() ? Consensus::BIP9Deployment::NO_TIMEOUT : fuzzed_data_provider.ConsumeIntegral<int64_t>();
173172
}
174173
int min_activation = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * max_periods);
175174

@@ -323,7 +322,7 @@ FUZZ_TARGET_INIT(versionbits, initialize)
323322
assert(exp_state == ThresholdState::ACTIVE || exp_state == ThresholdState::LOCKED_IN);
324323
break;
325324
case ThresholdState::FAILED:
326-
assert(current_block->GetMedianTimePast() >= checker.m_end);
325+
assert(never_active_test || current_block->GetMedianTimePast() >= checker.m_end);
327326
assert(exp_state != ThresholdState::LOCKED_IN && exp_state != ThresholdState::ACTIVE);
328327
break;
329328
default:
@@ -335,26 +334,20 @@ FUZZ_TARGET_INIT(versionbits, initialize)
335334
assert(state == ThresholdState::ACTIVE || state == ThresholdState::FAILED);
336335
}
337336

338-
// "always active" has additional restrictions
339337
if (always_active_test) {
338+
// "always active" has additional restrictions
340339
assert(state == ThresholdState::ACTIVE);
341340
assert(exp_state == ThresholdState::ACTIVE);
342341
assert(since == 0);
342+
} else if (never_active_test) {
343+
// "never active" does too
344+
assert(state == ThresholdState::FAILED);
345+
assert(exp_state == ThresholdState::FAILED);
346+
assert(since == 0);
343347
} else {
344-
// except for always active, the initial state is always DEFINED
348+
// for signalled deployments, the initial state is always DEFINED
345349
assert(since > 0 || state == ThresholdState::DEFINED);
346350
assert(exp_since > 0 || exp_state == ThresholdState::DEFINED);
347351
}
348-
349-
// "never active" does too
350-
if (never_active_test) {
351-
assert(state == ThresholdState::FAILED);
352-
assert(since == period);
353-
if (exp_since == 0) {
354-
assert(exp_state == ThresholdState::DEFINED);
355-
} else {
356-
assert(exp_state == ThresholdState::FAILED);
357-
}
358-
}
359352
}
360353
} // namespace

‎src/test/versionbits_tests.cpp‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,7 @@ class TestAlwaysActiveConditionChecker : public TestConditionChecker
5959
class TestNeverActiveConditionChecker : public TestConditionChecker
6060
{
6161
public:
62-
int64_t BeginTime(const Consensus::Params& params) const override { return 0; }
63-
int64_t EndTime(const Consensus::Params& params) const override { return 1230768000; }
62+
int64_t BeginTime(const Consensus::Params& params) const override { return Consensus::BIP9Deployment::NEVER_ACTIVE; }
6463
};
6564

6665
#define CHECKERS 6
@@ -134,10 +133,7 @@ class VersionBitsTester
134133
BOOST_CHECK_MESSAGE(checker[i].GetStateSinceHeightFor(tip) == height, strprintf("Test %i for StateSinceHeight", num));
135134
BOOST_CHECK_MESSAGE(checker_delayed[i].GetStateSinceHeightFor(tip) == height_delayed, strprintf("Test %i for StateSinceHeight (delayed)", num));
136135
BOOST_CHECK_MESSAGE(checker_always[i].GetStateSinceHeightFor(tip) == 0, strprintf("Test %i for StateSinceHeight (always active)", num));
137-
138-
// never active may go from DEFINED -> FAILED at the first period
139-
const auto never_height = checker_never[i].GetStateSinceHeightFor(tip);
140-
BOOST_CHECK_MESSAGE(never_height == 0 || never_height == checker_never[i].Period(paramsDummy), strprintf("Test %i for StateSinceHeight (never active)", num));
136+
BOOST_CHECK_MESSAGE(checker_never[i].GetStateSinceHeightFor(tip) == 0, strprintf("Test %i for StateSinceHeight (never active)", num));
141137
}
142138
}
143139
num++;
@@ -170,7 +166,7 @@ class VersionBitsTester
170166
BOOST_CHECK_MESSAGE(got == exp, strprintf("Test %i for %s height %d (got %s)", num, StateName(exp), height, StateName(got)));
171167
BOOST_CHECK_MESSAGE(got_delayed == exp_delayed, strprintf("Test %i for %s height %d (got %s; delayed case)", num, StateName(exp_delayed), height, StateName(got_delayed)));
172168
BOOST_CHECK_MESSAGE(got_always == ThresholdState::ACTIVE, strprintf("Test %i for ACTIVE height %d (got %s; always active case)", num, height, StateName(got_always)));
173-
BOOST_CHECK_MESSAGE(got_never == ThresholdState::DEFINED|| got_never == ThresholdState::FAILED, strprintf("Test %i for DEFINED/FAILED height %d (got %s; never active case)", num, height, StateName(got_never)));
169+
BOOST_CHECK_MESSAGE(got_never == ThresholdState::FAILED, strprintf("Test %i for FAILED height %d (got %s; never active case)", num, height, StateName(got_never)));
174170
}
175171
}
176172
num++;
@@ -270,7 +266,7 @@ BOOST_AUTO_TEST_CASE(versionbits_sanity)
270266
// Check min_activation_height is on a retarget boundary
271267
BOOST_CHECK_EQUAL(mainnetParams.vDeployments[i].min_activation_height % mainnetParams.nMinerConfirmationWindow, 0);
272268
// Check min_activation_height is 0 for ALWAYS_ACTIVE and never active deployments
273-
if (mainnetParams.vDeployments[i].nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE || mainnetParams.vDeployments[i].nTimeout <= 1230768000) {
269+
if (mainnetParams.vDeployments[i].nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE || mainnetParams.vDeployments[i].nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) {
274270
BOOST_CHECK_EQUAL(mainnetParams.vDeployments[i].min_activation_height, 0);
275271
}
276272

@@ -304,8 +300,9 @@ static void check_computeblockversion(const Consensus::Params& params, Consensus
304300
// should not be any signalling for first block
305301
BOOST_CHECK_EQUAL(ComputeBlockVersion(nullptr, params), VERSIONBITS_TOP_BITS);
306302

307-
// always active deployments shouldn't need to be tested further
303+
// always/never active deployments shouldn't need to be tested further
308304
if (nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE) return;
305+
if (nStartTime == Consensus::BIP9Deployment::NEVER_ACTIVE) return;
309306

310307
BOOST_REQUIRE(nStartTime < nTimeout);
311308
BOOST_REQUIRE(nStartTime >= 0);
@@ -447,6 +444,15 @@ BOOST_AUTO_TEST_CASE(versionbits_computeblockversion)
447444
}
448445
}
449446

447+
{
448+
// Use regtest/testdummy to ensure we always exercise some
449+
// deployment that's not always/never active
450+
ArgsManager args;
451+
args.ForceSetArg("-vbparams", "testdummy:1199145601:1230767999"); // January 1, 2008 - December 31, 2008
452+
const auto chainParams = CreateChainParams(args, CBaseChainParams::REGTEST);
453+
check_computeblockversion(chainParams->GetConsensus(), Consensus::DEPLOYMENT_TESTDUMMY);
454+
}
455+
450456
{
451457
// Use regtest/testdummy to ensure we always exercise the
452458
// min_activation_height test, even if we're not using that in a

‎src/versionbits.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex*
1818
return ThresholdState::ACTIVE;
1919
}
2020

21+
// Check if this deployment is never active.
22+
if (nTimeStart == Consensus::BIP9Deployment::NEVER_ACTIVE) {
23+
return ThresholdState::FAILED;
24+
}
25+
2126
// A block's state is always the same as that of the first of its period, so it is computed based on a pindexPrev whose height equals a multiple of nPeriod - 1.
2227
if (pindexPrev != nullptr) {
2328
pindexPrev = pindexPrev->GetAncestor(pindexPrev->nHeight - ((pindexPrev->nHeight + 1) % nPeriod));
@@ -129,7 +134,7 @@ BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockI
129134
int AbstractThresholdConditionChecker::GetStateSinceHeightFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const
130135
{
131136
int64_t start_time = BeginTime(params);
132-
if (start_time == Consensus::BIP9Deployment::ALWAYS_ACTIVE) {
137+
if (start_time == Consensus::BIP9Deployment::ALWAYS_ACTIVE || start_time == Consensus::BIP9Deployment::NEVER_ACTIVE) {
133138
return 0;
134139
}
135140

0 commit comments

Comments
 (0)