Skip to content

Commit f054f6b

Browse files
ajtownsajtowns
committed
versionbits: simplify state transitions
This removes the DEFINED->FAILED transition and changes the STARTED->FAILED transition to only occur if signalling didn't pass the threshold. This ensures that it is always possible for activation to occur, no matter what settings are chosen, or the speed at which blocks are found.
1 parent 55ac5f5 commit f054f6b

File tree

3 files changed

+29
-32
lines changed

3 files changed

+29
-32
lines changed

‎src/test/fuzz/versionbits.cpp

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -147,19 +147,14 @@ FUZZ_TARGET_INIT(versionbits, initialize)
147147
// pick the timestamp to switch based on a block
148148
// note states will change *after* these blocks because mediantime lags
149149
int start_block = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * (max_periods - 3));
150-
int end_block = fuzzed_data_provider.ConsumeIntegralInRange<int>(start_block, period * (max_periods - 3));
150+
int end_block = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, period * (max_periods - 3));
151151

152152
start_time = block_start_time + start_block * interval;
153153
timeout = block_start_time + end_block * interval;
154154

155-
assert(start_time <= timeout);
156-
157155
// allow for times to not exactly match a block
158156
if (fuzzed_data_provider.ConsumeBool()) start_time += interval / 2;
159157
if (fuzzed_data_provider.ConsumeBool()) timeout += interval / 2;
160-
161-
// this may make timeout too early; if so, don't run the test
162-
if (start_time > timeout) return;
163158
} else {
164159
if (fuzzed_data_provider.ConsumeBool()) {
165160
start_time = Consensus::BIP9Deployment::ALWAYS_ACTIVE;
@@ -297,13 +292,12 @@ FUZZ_TARGET_INIT(versionbits, initialize)
297292
assert(since == 0);
298293
assert(exp_state == ThresholdState::DEFINED);
299294
assert(current_block->GetMedianTimePast() < checker.m_begin);
300-
assert(current_block->GetMedianTimePast() < checker.m_end);
301295
break;
302296
case ThresholdState::STARTED:
303297
assert(current_block->GetMedianTimePast() >= checker.m_begin);
304-
assert(current_block->GetMedianTimePast() < checker.m_end);
305298
if (exp_state == ThresholdState::STARTED) {
306299
assert(blocks_sig < threshold);
300+
assert(current_block->GetMedianTimePast() < checker.m_end);
307301
} else {
308302
assert(exp_state == ThresholdState::DEFINED);
309303
}
@@ -313,7 +307,6 @@ FUZZ_TARGET_INIT(versionbits, initialize)
313307
assert(current_block->nHeight + 1 < min_activation);
314308
} else {
315309
assert(exp_state == ThresholdState::STARTED);
316-
assert(current_block->GetMedianTimePast() < checker.m_end);
317310
assert(blocks_sig >= threshold);
318311
}
319312
break;
@@ -323,7 +316,11 @@ FUZZ_TARGET_INIT(versionbits, initialize)
323316
break;
324317
case ThresholdState::FAILED:
325318
assert(never_active_test || current_block->GetMedianTimePast() >= checker.m_end);
326-
assert(exp_state != ThresholdState::LOCKED_IN && exp_state != ThresholdState::ACTIVE);
319+
if (exp_state == ThresholdState::STARTED) {
320+
assert(blocks_sig < threshold);
321+
} else {
322+
assert(exp_state == ThresholdState::FAILED);
323+
}
327324
break;
328325
default:
329326
assert(false);

‎src/test/versionbits_tests.cpp

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -190,18 +190,20 @@ BOOST_FIXTURE_TEST_SUITE(versionbits_tests, TestingSetup)
190190
BOOST_AUTO_TEST_CASE(versionbits_test)
191191
{
192192
for (int i = 0; i < 64; i++) {
193-
// DEFINED -> FAILED
193+
// DEFINED -> STARTED after timeout reached -> FAILED
194194
VersionBitsTester().TestDefined().TestStateSinceHeight(0)
195195
.Mine(1, TestTime(1), 0x100).TestDefined().TestStateSinceHeight(0)
196196
.Mine(11, TestTime(11), 0x100).TestDefined().TestStateSinceHeight(0)
197197
.Mine(989, TestTime(989), 0x100).TestDefined().TestStateSinceHeight(0)
198-
.Mine(999, TestTime(20000), 0x100).TestDefined().TestStateSinceHeight(0)
199-
.Mine(1000, TestTime(20000), 0x100).TestFailed().TestStateSinceHeight(1000)
200-
.Mine(1999, TestTime(30001), 0x100).TestFailed().TestStateSinceHeight(1000)
201-
.Mine(2000, TestTime(30002), 0x100).TestFailed().TestStateSinceHeight(1000)
202-
.Mine(2001, TestTime(30003), 0x100).TestFailed().TestStateSinceHeight(1000)
203-
.Mine(2999, TestTime(30004), 0x100).TestFailed().TestStateSinceHeight(1000)
204-
.Mine(3000, TestTime(30005), 0x100).TestFailed().TestStateSinceHeight(1000)
198+
.Mine(999, TestTime(20000), 0x100).TestDefined().TestStateSinceHeight(0) // Timeout and start time reached simultaneously
199+
.Mine(1000, TestTime(20000), 0).TestStarted().TestStateSinceHeight(1000) // Hit started, stop signalling
200+
.Mine(1999, TestTime(30001), 0).TestStarted().TestStateSinceHeight(1000)
201+
.Mine(2000, TestTime(30002), 0x100).TestFailed().TestStateSinceHeight(2000) // Hit failed, start signalling again
202+
.Mine(2001, TestTime(30003), 0x100).TestFailed().TestStateSinceHeight(2000)
203+
.Mine(2999, TestTime(30004), 0x100).TestFailed().TestStateSinceHeight(2000)
204+
.Mine(3000, TestTime(30005), 0x100).TestFailed().TestStateSinceHeight(2000)
205+
.Mine(4000, TestTime(30006), 0x100).TestFailed().TestStateSinceHeight(2000)
206+
205207
// DEFINED -> STARTED -> FAILED
206208
.Reset().TestDefined().TestStateSinceHeight(0)
207209
.Mine(1, TestTime(1), 0).TestDefined().TestStateSinceHeight(0)
@@ -212,19 +214,19 @@ BOOST_AUTO_TEST_CASE(versionbits_test)
212214
.Mine(3000, TestTime(20000), 0).TestFailed().TestStateSinceHeight(3000) // 50 old blocks (so 899 out of the past 1000)
213215
.Mine(4000, TestTime(20010), 0x100).TestFailed().TestStateSinceHeight(3000)
214216

215-
// DEFINED -> STARTED -> FAILED while threshold reached
217+
// DEFINED -> STARTED -> LOCKEDIN after timeout reached -> ACTIVE
216218
.Reset().TestDefined().TestStateSinceHeight(0)
217219
.Mine(1, TestTime(1), 0).TestDefined().TestStateSinceHeight(0)
218220
.Mine(1000, TestTime(10000) - 1, 0x101).TestDefined().TestStateSinceHeight(0) // One second more and it would be defined
219221
.Mine(2000, TestTime(10000), 0x101).TestStarted().TestStateSinceHeight(2000) // So that's what happens the next period
220222
.Mine(2999, TestTime(30000), 0x100).TestStarted().TestStateSinceHeight(2000) // 999 new blocks
221-
.Mine(3000, TestTime(30000), 0x100).TestFailed().TestStateSinceHeight(3000) // 1 new block (so 1000 out of the past 1000 are new)
222-
.Mine(3999, TestTime(30001), 0).TestFailed().TestStateSinceHeight(3000)
223-
.Mine(4000, TestTime(30002), 0).TestFailed().TestStateSinceHeight(3000)
224-
.Mine(14333, TestTime(30003), 0).TestFailed().TestStateSinceHeight(3000)
225-
.Mine(24000, TestTime(40000), 0).TestFailed().TestStateSinceHeight(3000)
223+
.Mine(3000, TestTime(30000), 0x100).TestLockedIn().TestStateSinceHeight(3000) // 1 new block (so 1000 out of the past 1000 are new)
224+
.Mine(3999, TestTime(30001), 0).TestLockedIn().TestStateSinceHeight(3000)
225+
.Mine(4000, TestTime(30002), 0).TestActiveDelayed().TestStateSinceHeight(4000, 3000)
226+
.Mine(14333, TestTime(30003), 0).TestActiveDelayed().TestStateSinceHeight(4000, 3000)
227+
.Mine(24000, TestTime(40000), 0).TestActive().TestStateSinceHeight(4000, 15000)
226228

227-
// DEFINED -> STARTED -> LOCKEDIN at the last minute -> ACTIVE
229+
// DEFINED -> STARTED -> LOCKEDIN before timeout -> ACTIVE
228230
.Reset().TestDefined()
229231
.Mine(1, TestTime(1), 0).TestDefined().TestStateSinceHeight(0)
230232
.Mine(1000, TestTime(10000) - 1, 0x101).TestDefined().TestStateSinceHeight(0) // One second more and it would be defined
@@ -247,8 +249,10 @@ BOOST_AUTO_TEST_CASE(versionbits_test)
247249
.Mine(3000, TestTime(10000), 0).TestStarted().TestStateSinceHeight(3000)
248250
.Mine(4000, TestTime(10000), 0).TestStarted().TestStateSinceHeight(3000)
249251
.Mine(5000, TestTime(10000), 0).TestStarted().TestStateSinceHeight(3000)
252+
.Mine(5999, TestTime(20000), 0).TestStarted().TestStateSinceHeight(3000)
250253
.Mine(6000, TestTime(20000), 0).TestFailed().TestStateSinceHeight(6000)
251254
.Mine(7000, TestTime(20000), 0x100).TestFailed().TestStateSinceHeight(6000)
255+
.Mine(24000, TestTime(20000), 0x100).TestFailed().TestStateSinceHeight(6000) // stay in FAILED no matter how much we signal
252256
;
253257
}
254258
}

‎src/versionbits.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,12 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex*
5757

5858
switch (state) {
5959
case ThresholdState::DEFINED: {
60-
if (pindexPrev->GetMedianTimePast() >= nTimeTimeout) {
61-
stateNext = ThresholdState::FAILED;
62-
} else if (pindexPrev->GetMedianTimePast() >= nTimeStart) {
60+
if (pindexPrev->GetMedianTimePast() >= nTimeStart) {
6361
stateNext = ThresholdState::STARTED;
6462
}
6563
break;
6664
}
6765
case ThresholdState::STARTED: {
68-
if (pindexPrev->GetMedianTimePast() >= nTimeTimeout) {
69-
stateNext = ThresholdState::FAILED;
70-
break;
71-
}
7266
// We need to count
7367
const CBlockIndex* pindexCount = pindexPrev;
7468
int count = 0;
@@ -80,6 +74,8 @@ ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex*
8074
}
8175
if (count >= nThreshold) {
8276
stateNext = ThresholdState::LOCKED_IN;
77+
} else if (pindexPrev->GetMedianTimePast() >= nTimeTimeout) {
78+
stateNext = ThresholdState::FAILED;
8379
}
8480
break;
8581
}

0 commit comments

Comments
 (0)