Repository navigation
test(consensus): cut the reward-halving vanishing case to milliseconds - #2949
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
benjamin202410,
liam-lai,
wanwiset25 and
wgr523
October 30, 2026 03:04
gzliudan
force-pushed
the
test-reward-halving-fast
branch
from
October 30, 2026 07:39
865c394 to
72a1ffc
Compare
4 of 25 tasks
anunay-xin
approved these changes
Oct 30, 2026
gzliudan
force-pushed
the
test-reward-halving-fast
branch
2 times, most recently
from
November 1, 2026 10:00
ceb11ec to
592bf0b
Compare
gzliudan
added a commit
to gzliudan/XDPoSChain
that referenced
this pull request
Nov 1, 2026
`go test ./consensus/tests/engine_v2_tests` was the slowest package in the tree at 219s; everything else under consensus finishes in under two seconds. The time went to building fixtures (~72s for ~83k blocks at ~0.9ms each), waiting on real countdown and mine periods (~70s), and fixed sleeps (~40s). A fourth quarter - 24s walking 11M epochs of big.Float reward math - is not touched here; it lands separately as XinFinOrg#2949. - Timing knobs: the fixture now deep-clones the chain config and shortens the V2 periods (countdown 4/4/5s -> 1/1/2s, mine 2/3s -> 1/2s). Round 10 stays strictly above round 0 because TestIsYourTurnConsensusV2CrossConfig needs a larger post-switch period. Assertions read the fixture's own config instead of the package-level one. - Waits: the two pool-hygiene tests slept 5s each for a hygiene pass that is synchronous, and those sleeps are gone. The forensics test keeps its 5s sleep: the goroutine it waits for writes without synchronisation (XinFinOrg#2945), so polling would trade a hidden race for a reported one rather than removing either - XinFinOrg#2946 fixes the write itself. Negative windows shrink to one countdown period. - Reads: at one-second periods the countdown keeps broadcasting while a test waits for a vote, so the reads that follow a negative window - and every read whose message is then used as a vote - skip timeout and sync-info broadcasts, and expectNoVote drains whatever sender is already parked on the unbuffered channel when its window closes. Without that, a stale timeout reached a vote type assertion and panicked. - Parallelism: 99 of the 108 tests call t.Parallel(). Nine stay serial: two count messages across consecutive periods, two read the global skipped-proposed-block counter, two install a global logger, and three keep their skip-path siblings in proposed_block_test.go company. Verified: 219s -> ~67s on a 12-core box (the reward-halving quarter is the rest, see XinFinOrg#2949), three back-to-back runs and two -race runs clean (the -race soak is what surfaced the lastWrite race fixed in the previous commit).
benjamin202410
approved these changes
Nov 2, 2026
gzliudan
added a commit
to gzliudan/XDPoSChain
that referenced
this pull request
Nov 2, 2026
`go test ./consensus/tests/engine_v2_tests` was the slowest package in the tree at 219s; everything else under consensus finishes in under two seconds. The time went to building fixtures (~72s for ~83k blocks at ~0.9ms each), waiting on real countdown and mine periods (~70s), and fixed sleeps (~40s). A fourth quarter - 24s walking 11M epochs of big.Float reward math - is not touched here; it lands separately as XinFinOrg#2949. - Timing knobs: the fixture now deep-clones the chain config and shortens the V2 periods (countdown 4/4/5s -> 1/1/2s, mine 2/3s -> 1/2s). Round 10 stays strictly above round 0 because TestIsYourTurnConsensusV2CrossConfig needs a larger post-switch period. Assertions read the fixture's own config instead of the package-level one. - Waits: the two pool-hygiene tests slept 5s each for a hygiene pass that is synchronous, and those sleeps are gone. The forensics test keeps its 5s sleep: the goroutine it waits for writes without synchronisation (XinFinOrg#2945), so polling would trade a hidden race for a reported one rather than removing either - XinFinOrg#2946 fixes the write itself. Negative windows shrink to one countdown period. - Reads: at one-second periods the countdown keeps broadcasting while a test waits for a vote, so the reads that follow a negative window - and every read whose message is then used as a vote - skip timeout and sync-info broadcasts, and expectNoVote drains whatever sender is already parked on the unbuffered channel when its window closes. Without that, a stale timeout reached a vote type assertion and panicked. - Parallelism: 99 of the 108 tests call t.Parallel(). Nine stay serial: two count messages across consecutive periods, two read the global skipped-proposed-block counter, two install a global logger, and three keep their skip-path siblings in proposed_block_test.go company. Verified: 219s -> ~67s on a 12-core box (the reward-halving quarter is the rest, see XinFinOrg#2949), three back-to-back runs and two -race runs clean (the -race soak is what surfaced the lastWrite race fixed in the previous commit).
gzliudan
added a commit
that referenced
this pull request
Nov 2, 2026
…2944) * fix(core): track the last flushed state block per chain lastWrite was a package-level variable, so every BlockChain in a process shared one flush high-water mark. That is wrong whenever more than one chain is live in the same process - the test fixtures build one chain per test - and it is a plain data race once those chains insert blocks concurrently, which `go test -race` reports at writeBlockWithState. Move it onto BlockChain, matching upstream geth (`bc.lastWrite`). The flush throttle now compares against the high-water mark of the chain it is flushing, which is what the surrounding code already assumed (bc.gcproc, bc.cachedb, bc.triedb are all per-chain). The executed-marker fixtures used to save and restore the package variable. With the field there is nothing to restore - each case builds and drops its own chain - so those save/restore pairs, and the comments explaining them, are gone. The one pin a fixture still needs (lastWrite = TriesInMemory) stays, now on the chain that pins it. * test(consensus): cut engine v2 suite wall time from 219s to 67s `go test ./consensus/tests/engine_v2_tests` was the slowest package in the tree at 219s; everything else under consensus finishes in under two seconds. The time went to building fixtures (~72s for ~83k blocks at ~0.9ms each), waiting on real countdown and mine periods (~70s), and fixed sleeps (~40s). A fourth quarter - 24s walking 11M epochs of big.Float reward math - is not touched here; it lands separately as #2949. - Timing knobs: the fixture now deep-clones the chain config and shortens the V2 periods (countdown 4/4/5s -> 1/1/2s, mine 2/3s -> 1/2s). Round 10 stays strictly above round 0 because TestIsYourTurnConsensusV2CrossConfig needs a larger post-switch period. Assertions read the fixture's own config instead of the package-level one. - Waits: the two pool-hygiene tests slept 5s each for a hygiene pass that is synchronous, and those sleeps are gone. The forensics test keeps its 5s sleep: the goroutine it waits for writes without synchronisation (#2945), so polling would trade a hidden race for a reported one rather than removing either - #2946 fixes the write itself. Negative windows shrink to one countdown period. - Reads: at one-second periods the countdown keeps broadcasting while a test waits for a vote, so the reads that follow a negative window - and every read whose message is then used as a vote - skip timeout and sync-info broadcasts, and expectNoVote drains whatever sender is already parked on the unbuffered channel when its window closes. Without that, a stale timeout reached a vote type assertion and panicked. - Parallelism: 99 of the 108 tests call t.Parallel(). Nine stay serial: two count messages across consecutive periods, two read the global skipped-proposed-block counter, two install a global logger, and three keep their skip-path siblings in proposed_block_test.go company. Verified: 219s -> ~67s on a 12-core box (the reward-halving quarter is the rest, see #2949), three back-to-back runs and two -race runs clean (the -race soak is what surfaced the lastWrite race fixed in the previous commit). * test(consensus): assert the applied config, not the message mix TestTimeoutPeriodAndThreadholdConfigChange read three broadcasts, triggered a parameter update, read five more, and demanded exactly six timeouts and two syncInfos. That split is not a property of the engine: it depends on where the update lands inside a countdown period. It held at the old two-second period and stopped holding once this branch shortened the periods to one and two seconds - the same countdown produced five timeouts and three syncInfos, with no dropped message and no failed signature (zero "authentication needed" lines, zero unknown message types, the same nine timeouts and four threshold events). Read with a deadline instead. Each read must produce a message within messageTimeout (ten seconds, several periods), so a dropped broadcast still fails and names the counters, and the five reads after the update stay bounded by twenty seconds. What the update itself must do is now asserted where it is observable: the engine's own current config has to be the round-900 one (threshold 4, two-second period). The engine keeps its own copy of the chain config, so a GetCurrentConfigFaker() accessor sits next to the other test fakers. That is the assertion which catches an UpdateParams that stops applying the config - stubbing it out drops the old six/two split to five/three, exactly the bracket this rewrite replaces, and the relaxed reads alone would not notice. Verified on this branch with the runner's shape (four cores pinned and saturated, -race, GOMAXPROCS=4): before, two of three runs failed with "expected 6, actual 5"; after, ten of ten pass. Dropping a message still fails: draining the channel and reading past the deadline reports "no message in 10s (timeout=0 syncinfo=0)". * test(core/vm): keep the privacy tests out of the default suite The privacy precompiles are unreachable - RING_SIG_VERIFY and BULLET_PROOF_VERIFY have never been called on mainnet or Apothem (#2895) - and the package is scheduled for deletion, so its 27 crypto cases cost the default suite four seconds and the detector 328s for code nothing reaches. #2912 drops the last CI job that ran them. They are now behind -tags=privacytests, so `go test ./...` reports the package as having no tests, and the cases stay runnable on demand: go test -tags=privacytests ./core/vm/privacy The package still builds, because core/vm imports it for the two precompiles.
gzliudan
force-pushed
the
test-reward-halving-fast
branch
from
November 2, 2026 23:16
592bf0b to
bb66798
Compare
wanwiset25
approved these changes
Nov 4, 2026
The case walked 11,225,088 epochs of big.Float arithmetic to reach the point where the geometric schedule pays nothing: 24.7 s on an idle machine and about 13 min under the race detector, past the 10m timeout the race lanes pass - the engine_v2 2/3 lane died on it (XinFinOrg#2912). The series converges to the halving supply for 0 < epochRewardTotal < halvingSupply - the reward/supply ratio only sets how fast - so the sweep now runs on a scaled-down supply whose whole curve is 3,450 epochs, and asserts the same two properties: the per-epoch reward dies out and what is issued stays below the supply. Mainnet's numbers are checked at the two epochs the walk pinned down (11,225,087 pays 1 wei, 11,225,088 pays nothing), which costs two calls. The -short gate goes away with the cost: the case runs in 10 ms. The scaled curve's break point is asserted too (epoch 3,450), so a constant typo cannot turn the sweep into a no-op, and the log line that reports the break point now prints only once the reward is actually gone.
gzliudan
force-pushed
the
test-reward-halving-fast
branch
from
November 4, 2026 15:30
bb66798 to
4421c5b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TestRewardHalvingVanishingwalked the whole mainnet reward curve to watch it die out: 11,225,088 epochs × 3 roles ×FloatPower≈ 7.7e8big.Floatmultiplications.The series converges to the supply for any parameter pair -
Σ a·q^i = a·H/Twithq = 1 − T/H, andΣ roles a = T, givesH- soT/Honly sets how fast the curve dies, while the two properties the case protects (the reward vanishes, the issuance stays below the supply) are scale-free. ShrinkingT/Hmoves the breakpoint from 11,225,088 epochs to 3,450, and mainnet's own breakpoint is now asserted directly instead of being reached by accumulation.Evidence
go test -run '^TestRewardHalvingVanishing$' ./consensus/tests/engine_v2_tests→ 24.7 s on an idle machine; under-race~13 min, past the-timeout 10mtheengine_v2race lanes pass, so the lane died on it: run 40990118093, job 123850259002 (panic: test timed out after 10m0s, 6m25s into this case).After: the same command → 0.01 s; the same under
-race→ 1.1 s for the package's two reward cases. The package's-shortrun goes 12.1 s → 11.5 s.Three mutations of the new assertions were checked, each restored afterwards:
11225088→11225087)the 10,000 XDC reward shall pay nothing from epoch 11,225,088, got 1the 10,000 XDC reward shall still be paid at epoch 11,225,087, got 0reward should be 0 before epoch 100000, issued 1404281543 of 6000000000 weiMerge Danger
Door: two-way
The change is confined to one test function.
eth/util.RewardHalving,FloatPowerand the schedules they compute are untouched, so a revert restores the old case with no migration path to unwind.Blast Radius: test-only
Mainnet's numbers are no longer replayed epoch by epoch: the scaled-down sweep covers the same invariant, and the mainnet breakpoint (11,225,088) is asserted rather than merely logged as before. This is the case that made the
race (engine_v2 2/3)lane of #2912 time out; the other red lane there is the pre-existingForensics.SetCommittedQCswrite/write race (#2945, fixed by #2946), which this PR does not touch.Types of changes
Impacted Components
Checklist