Repository navigation
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>
… CON-430) (sei-protocol#4289) ## Summary Backport the Autobahn EVM-only JSON-RPC surface from `giga-1` onto `main`, one commit per original merge, in apply order: - **CON-427** / giga-1 sei-protocol#4192 — `eth_getTransactionCount`, `eth_blockNumber`, `eth_chainId`, and reorganize `giga/evmonly/rpc` by JSON-RPC namespace - **CON-428** / giga-1 sei-protocol#4194 — `eth_call` (`Executor.Call` + read-only RPC) - **CON-429** / giga-1 sei-protocol#4205 — `eth_getTransactionByHash` - **CON-430** / giga-1 sei-protocol#4208 — `eth_getBlockByNumber` / `eth_getBlockByHash` `giga-1`'s durable execution cursor (sei-protocol#4190 / sei-protocol#4231) is not part of this backport. `EvmCall` and `EvmGasLimit` read main's in-memory `evmOnlyState` the same way giga-1 HEAD reads committed height/time/gas/hash and refuses while a finalized block is pending Commit. ## Test plan - [x] `go test -count=1 ./giga/evmonly/ ./giga/evmonly/rpc/ ./sei-tendermint/internal/proxy/` - [x] `scripts/ramtest.sh ./sei-tendermint/internal/evmonlyapp/ -count=1` - [ ] seidroid review - [ ] CI Coverage / Go Test on this PR
…ntegration tests (sei-protocol#4316) Autobahn had two shapes: with `evm-only` it ran the disk-backed EVM-only executor and served the EVM JSON-RPC, and without it the Cosmos application and Tendermint RPC. The second shape was not a product we intend to keep, and the flag let a node be configured into a combination we do not support. Enabling Autobahn now selects the EVM-only executor and serves the EVM JSON-RPC instead of Tendermint RPC. The `evm-only` config key is gone. Selection happens in `wrapApplication`, the one function every startup path passes through, keyed on the Giga storage manager that `autobahn-config-file` already opens. `mock-app` still replaces the application under Autobahn so consensus can be load-tested without execution. It is ordered ahead of the EVM-only branch. Cosmos-under-Autobahn integration coverage is removed or disabled rather than ported (`AUTOBAHN_EVMONLY` is gone; `make autobahn-evmonly-integration-test` remains as an alias). Autobahn CI rows that need the full `evmrpc` surface or Cosmos gov stay in the matrix as `"disabled": true` with TODOs. ## Autobahn Basic The suite still starts a four-validator cluster plus the `sei-rpc-node` fullnode sidecar. It runs: - `EVMOnlyLoad` — 4000 raw transfers on chain 713715, receipts and balances on every validator, plus a receipt on the fullnode - `LivenessUnderMaxFaults` — after `f` kills, a submitted transfer must finalize - `HaltsBeyondMaxFaults` — after `f+1` kills, a submitted transfer must not land - `Recovery` — skipped until the durable EVM-only execution cursor ([sei-protocol#4231](sei-protocol#4231), on `giga-1`) reaches `main`. Without it a restarted validator reports height 0, replays block 1 onto ahead-of-it FlatKV state, and panics with `nonce too low` The CI startup gate for `AUTOBAHN=true` rows checks `eth_getBalance` and waits for one committed transfer. Docker clusters keep `allow_empty_blocks` off. ## Test plan - [x] `go test ./sei-tendermint/node/ ./sei-tendermint/config/ ./cmd/autobahn-e2e/` - [x] `go vet -tags autobahn_integration ./integration_test/autobahn/` - [ ] Autobahn Basic in CI: load, liveness, halt; Recovery skipped until the cursor lands --------- Co-authored-by: Cursor <cursoragent@cursor.com>
…4351) This PR backports sei-protocol#4231 from `giga-1`. An EVM-only Autobahn node keeps its execution position (the last executed height and app hash) only in memory, so a restarted validator reports height 0 from `Info()`. The Giga router then takes the fresh-genesis path which later panics. Now, the EVM-only application persists an 80-byte cursor (height, app hash, block hash, gas limit) as an `evmonly` named change-set outside `keys.EVMStoreKey`. This durable cursor helps the node to recover upon restarts. This also unblocks the failing recovery integration tests (skipped for now - will be handled in a separate PR) when running Autobahn with EVM only execution. ### Divergences from `giga-1` `main` has moved since sei-protocol#4231's base, so four files conflicted. - All of `main`'s behaviour (the `[giga]` execution config, stale-nonce rejection, the shared `currentExecutionContext`) is kept and cursor is layered on top. - `NewEVMOnlyApplication` keeps `main`'s `execution` argument and now also returns an error. `wrapApplication` returns that error without closing storage, since `prepareApplication` already closes it on error. - The tests open storage with `seidbconfig.AutobahnStorageConfig`, because `evmonly.NewValidatorStorageConfig` does not exist here. - Two doc comments that sei-protocol#4231 misplaced are fixed. - sei-protocol#4241 / sei-protocol#4242 are not ported as they cancel each other out and net effect is zero. ### Additional notes - sei-protocol#4224 (PREVRANDAO from the prior app hash). It also grows the cursor to 112 bytes, so porting it later changes the on-disk cursor format. - A node whose EVM-only state was written by a build without the cursor has blocks but no cursor. It now refuses to start (`EVM-only state is already at height N before InitChain`) instead of panicking on replay, and needs its data wiped and resynced once. <!-- Guidance for AI agents and humans writing this description. Goal: a reader who has not seen the diff should understand, in under a minute, what this PR changes, why, and what could go wrong. Optimise for the reader's cognitive cost, not for completeness. Write plain prose. No headings, no bullet-list recap of the diff, no "Testing performed" section. Two to four short paragraphs is the target. Paragraph 1: the problem or motivation. State the observed behaviour or gap and why it matters. Link the issue if there is one. Paragraph 2: the change. Explain the approach and the key decision(s), not the file list. Name concrete symbols (`Keeper.SetNonce`, `TxMempool`) so readers can jump into the diff. If there is a non-obvious alternative you rejected, say why in one sentence. Paragraph 3 (when relevant): risk and rollout. Consensus, state, or wire-format impact; migration or upgrade-handler needs; feature gating; what a reviewer should scrutinise most closely. Say in one sentence how you validated the change (which tests or manual runs), without pasting output. Omit anything a reader can trivially see in the diff. Do not restate commit messages. Do not include agent/session links or attribution. Keep this description current. Every time you push to this branch, re-read the description against the full diff (`git diff origin/main...HEAD`) and update it so that it still describes the PR as it is now, not as it was first opened. Remove or rewrite paragraphs that review feedback has made stale. Replace this template text entirely; do not leave any of these comments or any placeholder in the final description. --> Co-authored-by: FromTheRain <bdchatham@gmail.com> 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.