Repository navigation
fix(rpc): give every slot its own root in the test state builders - #793
Conversation
gloas_state() in ptc.rs and fulu_state() in validator.rs filled block_roots with H256::repeat_byte(slot as u8 + 1). The window is SLOTS_PER_HISTORICAL_ROOT long, so 255u8 + 1 overflowed: 60 ethlambda-rpc tests panicked in debug builds, and under release-fast (overflow checks off) the roots wrapped and repeated after slot 255. Both builders now use test_utils::slot_root, which writes slot + 1 big-endian into a root's last eight bytes, and the PTC-duties test's expected dependent_root uses it too. Closes #786
🤖 Grok Code ReviewI'll review the test helper and its call sites for correctness, especially the historical-root overflow the comment describes. Automated review by Grok (xAI) · grok-4.7 · custom prompt |
🤖 Claude Code ReviewReview: PR 793,
|
🤖 Gemini Code ReviewLooks good to me. Replacing Automated review by Gemini · gemini-4-argon · custom prompt |
🤖 Codex Code ReviewNo findings. This test-only change fixes debug overflow at slot 255 and provides distinct, nonzero roots across the historical window. No production or consensus behavior changes. Tests could not run because rustup attempted to write to a read-only directory. Automated review by OpenAI Codex · gpt-6.1-sol · custom prompt |
🤖 Kimi Code ReviewNo issues found. The new Optional nit: the “never the zero hash” documentation technically excludes only Looks good to merge. Automated review by Kimi (Moonshot AI) · kimi-k3.1-pro · custom prompt |
🤖 DeepSeek Code ReviewThis PR is test-only and fixes a real test-hygiene bug: Minor issue:
Optional style nit:
The replacements in Automated review by DeepSeek · deepseek-v4-pro · custom prompt |
Motivation
Closes #786.
cargo test -p ethlambda-rpc --libin the default (debug) profile fails 60 tests withattempt to add with overflow. Two test state builders fillblock_rootswithH256::repeat_byte(slot as u8 + 1), and the window isSLOTS_PER_HISTORICAL_ROOTlong (8192 on the mainnet preset), so255u8 + 1overflows. CI runs underrelease-fast, whereoverflow-checks = false, so there it wraps silently instead. That still undoes what the builders meant: past slot 255 the "distinct roots per slot" repeat.The issue names
ptc.rs:574, butvalidator.rs'sfulu_state()has the same loop. The failures are theptcandvalidatortests that build a state through either one.Changes
test_utils::slot_root(slot)incrates/net/rpc/src/lib.rswritesslot + 1big-endian into a root's last eight bytes, so every slot gets its own root and none is the zero hash.ptc.rs:gloas_state()fillsblock_rootswith it, and the PTC-duties test's expecteddependent_rootuses it too. That assertion recomputed the old formula, so changing only the builder would have broken it.validator.rs:fulu_state()fillsblock_rootswith it. Its assertions already read the root back throughget_block_root_at_slot, so they need no change.Test code only; no production code changes.
Verification
cargo test -p ethlambda-rpc --lib(debug): 335 passed, 0 failed (60 failed before this change)cargo test -p ethlambda-rpc --lib --profile release-fast(what CI runs): 335 passed, 0 failedmake fmt,make lintNot in this PR
The issue also suggests running unit tests with overflow checks in CI.
release-fastsetsoverflow-checks = falseon purpose (see the comment inCargo.toml), so arithmetic matchesrelease. Changing that is a separate decision and is left out here.