fix(evmonly): restore the durable execution cursor - #4231
Conversation
Reverts #4212, which reverted #4190. sei-chain#4169 is live again and it took down a 40-validator harbor chain today. giga-testnet-2 ran healthily at 64,500 tx/s on a build carrying #4190. The image automation bumped it to e104749 (the revert), that bump rolled all 40 pods, and every restarted validator panicked identically: execute: r.data.GlobalBlock(1): blockStore.ReadBlockByNumber(1): pruned: below retention watermark Without a durable cursor Info() reports height 0, the Giga router takes the fresh-genesis branch, and the node asks for block 1 -- which littdb retention had pruned 294,000 blocks earlier. Deterministic, fleet-wide, unrecoverable by restart. #4212 predicted it: "a restarted EVM-only node again reports height 0 from Info(), so #4169 is reopened by this change." #4210 is present and does not help: it retains a lane's LAST block across prune, not block 1. The revert of the revert applies cleanly and keeps the three commits that landed on top of #4190 -- #4183 checkedSenders, #4194 EvmChainConfig and EvmBaseFee, #4205 EvmCall and lastBlockTime are all intact. #4212's conflicts came from removing #4190 while keeping those three; returning to a state that had both is unambiguous. This is option A from #4169: height is already derivable from storage.SC().GetLatestVersion(), but the app hash is a chained SHA-256 (appHash(N) = H(appHash(N-1) || height || blockHash || gasUsed || changesets)), so it is not a function of state at version N and must be persisted. The cursor rides as an "evmonly" named changeset in the same CommitStateChanges call, outside keys.EVMStoreKey, so it shares the block's version without entering the EVM state hash. Verified: gofmt and go vet clean on the four changed packages; evmonlyapp, giga/evmonly, p2p and node tests pass; TestEVMOnlyApplicationResumesFromStorageAfterRestart and the cursor round-trip and malformed-decode tests pass -- these are #4169's acceptance criteria 1 and 2. Not run: staticcheck, golangci-lint, govulncheck, scripts/ramtest.sh, and the harbor cluster check in #4169 AC 4. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PR SummaryHigh Risk Overview The executor gains optional Tests cover cursor encoding, restart resume (including finalize-without-commit), and repeated Reviewed by Cursor Bugbot for commit 2b30967. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## giga-1 #4231 +/- ##
==========================================
+ Coverage 65.55% 65.59% +0.03%
==========================================
Files 2081 2079 -2
Lines 157460 157583 +123
==========================================
+ Hits 103222 103363 +141
+ Misses 54097 54079 -18
Partials 141 141
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f1e55e0. Configure here.
There was a problem hiding this comment.
Restores the durable EVM-only execution cursor by committing it as a named changeset in the same CommitStateChanges call as the block's state, which makes Info() survive a restart; the crash-consistency story (cursor and state at the same FlatKV version, InitChain refused when the store already holds blocks) holds up and is well covered by the new tests. Remaining notes are non-blocking: lastBlockTime is the one piece of per-block context not restored, and the new BlockChangeSetEncoder hook has an unenforced key-space contract and forces the commit pipeline to settle on every block.
Findings: 0 blocking | 4 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 3 suggestion(s)/nit(s) flagged inline on specific lines.
- 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion] An evmonly store written by the current cursor-less build cannot be resumed by this change either:
loadEVMOnlyCursorfinds noevmonly/cursorkey,Info()reports 0, the router callsInitChain, andseedInitialStateVersionthen fails with "EVM-only state is already at height N before InitChain". That is a clear failure rather than the base branch'spruned: below retention watermarkpanic, but there is no migration and no documented recovery (wipe state / state-sync) ingiga/evmonly/README.mdor the PR body.
…line slack Three review suggestions on the restored cursor. InitLastHeader now seeds lastBlockTime. The cursor carries height, hashes and gas limit but not Time, so a resumed app answered EvmCall -- eth_call through giga/evmonly/rpc -- with TIMESTAMP 0 and PrevRandao keccak(0) while reporting the correct NUMBER, BlockHash and GasLimit. The router already passes the last header on its restart path; the app inherited BaseApplication's no-op. Block production was unaffected, since FinalizeBlock takes time from the header. TestEVMOnlyApplicationInitLastHeaderSeedsBlockTime covers the gap, which neither existing restart test reached. The keys.EVMStoreKey reservation is now enforced where every block encoder's output passes, rather than left as a doc comment the next encoder author has to remember. An EVM-keyed changeset would have been written into account, storage and code state as part of the block, diverging the app hash from the committed state. WithStoreIndependentBlockChangeSetEncoder lets an encoder declare that it reads only the block context and result. encodingReadsTheStore treated any configured encoder as store-reading, which made settleBeforeEncoding permanently true for this app and gave up the receipt-stage slack the pipeline exists for -- worst exactly when commits are slow. encodeCursorChangeSet reads only in-memory cursor state, so it takes the new variant; the store-reading variant keeps the conservative wait and is now opt-in rather than implied. Verified: gofmt and go vet clean; evmonlyapp, giga/evmonly, p2p and node tests pass. Not run: staticcheck, golangci-lint, govulncheck, ramtest.sh. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Reverts #4212, which reverted #4190. #4169 is live again and it took down a 40-validator harbor chain today.
giga-testnet-2ran at 64,500 tx/s on a build carrying #4190. Flux bumped it toe104749(the revert), that bump rolled all 40 pods, and every restarted validator panicked:Without a durable cursor
Info()reports height 0, the router takes the fresh-genesis branch, and the node asks for block 1 — pruned ~294,000 blocks earlier. #4212 predicted it: "a restarted EVM-only node again reports height 0 fromInfo(), so #4169 is reopened by this change."#4210 is present and does not help: it retains a lane's last block across prune, not block 1.
Why the app hash must be persisted (option A from #4169)
Height needs no new storage —
CommitStateChanges(blockNumber, …)stamps the block number as the FlatKV version andGetLatestVersionreads it back. But the app hash is a chained SHA-256,appHash(N) = H(appHash(N-1) ‖ height ‖ blockHash ‖ gasUsed ‖ changesets), so it is not a function of state at version N and recomputing it means replaying from genesis.#4169's Alternative (B) — app hash as the lthash root — is derivable but changes what the hash commits to and needs an ADR. Option A does not touch consensus.
This is a pure revert
The tree is byte-identical to
7f1653924, so nothing here is mine. The three commits that landed on top of #4190 survive: #4183checkedSenders, #4194EvmChainConfig/EvmBaseFee, #4205EvmCall/lastBlockTime. #4212's conflicts came from removing #4190 while keeping those three; returning to a state that had both is unambiguous.Verified
gofmtandgo vetclean on the four changed packages.evmonlyapp,giga/evmonly,p2pandnodetests pass, includingTestEVMOnlyApplicationResumesFromStorageAfterRestart,TestEVMOnlyCursorRoundTripandTestEVMOnlyCursorDecodeRejectsMalformed— #4169's acceptance criteria 1 and 2.Not run:
staticcheck,golangci-lint,govulncheck,scripts/ramtest.sh, and #4169 AC 4's cluster check.The question I cannot answer
#4212 records no functional reason for the revert — its body explains only how the conflicts were resolved. If #4190 had a defect, say so and I will withdraw in favour of fixing it or pursuing Alternative (B). If the revert was a preference, this PR is the counter-argument: the chain cannot survive a restart without a durable
Info(), and the controller rolled all 40 pods twice today for unrelated reasons.#4169 should be reopened either way; it is marked closed while the bug is live.