Repository navigation
Make Autobahn always run the EVM-only executor, disable/remove some integration tests - #4316
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
There was a problem hiding this comment.
This PR drops the evm-only flag so that Autobahn always uses the EVM-only executor, with mock-app checked first, and it deletes the Cosmos-under-Autobahn integration suite. The main change looks sound. The one real risk is that mock-app under Autobahn now binds the EVM-only RPC port, which can collide with the Cosmos app's own EVM HTTP server outside the Docker scripts.
Findings: 0 blocking | 3 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] The integration test cleanup also deletes doc comments on helpers it keeps. Examples:
assertAutobahnEnabledloses its note that seid logs go to a file inside the container (notdocker logs),setupCluster,countLaunchCompleteandfindRepoRootlose their docs, and a leftover comment abovevar clusterSizeis split fromlistRunningNodes. None of this is needed for the behaviour change, and it removes context that explained non-obvious steps. Keep the comments on the surviving helpers. - [suggestion] The PR says it selects the Autobahn shape at one choke point, but there are now two separate conditions.
wrapApplicationchecks whether Giga storage is present, whileOnStartpicks the EVM-only RPC fromAutobahnConfigFile != "". They agree today only becauseprepareApplicationopens storage exactly when the config file is set. Using the same condition in both places, or recording the choice once, would stop them from drifting apart. - 1 suggestion(s)/nit(s) flagged inline on specific lines.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4316 +/- ##
==========================================
- Coverage 67.49% 66.38% -1.11%
==========================================
Files 2181 2060 -121
Lines 167798 155772 -12026
==========================================
- Hits 113251 103416 -9835
+ Misses 54537 52346 -2191
Partials 10 10
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 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1d8717c. Configure here.
| for i := 0; i < maxFaults; i++ { | ||
| killNode(t, clusterSize-1-i) | ||
| } | ||
| t.Logf("height after: %d", waitForHeightAbove(t, before, livenessTimeout)) |
There was a problem hiding this comment.
Liveness test accepts in-flight blocks
Medium Severity
testLivenessUnderMaxFaults treats the first height increase after killNode as proof the remaining committee is live. In-flight blocks keep draining through runExecute after a validator dies, so height can advance once even when quorum is gone and no new block is agreed. A halted cluster can still pass this subtest.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1d8717c. Configure here.
Autobahn had two shapes: with `evm-only` set it ran the disk-backed EVM-only executor and served the EVM JSON-RPC, and without it the Cosmos application and Tendermint RPC. Nothing ran or tested the second shape, and the flag let a node be configured into a combination we do not support. Enabling Autobahn now selects the EVM-only executor and the EVM JSON-RPC. The `evm-only` config key is gone, and the selection happens in `wrapApplication`, the one function every startup path passes through, keyed on the Giga storage manager that `autobahn-config-file` already opens. EVM-only is not reachable without Autobahn. `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 and serves the same EVM JSON-RPC through its in-memory nonce and balance state. The Cosmos-under-Autobahn integration tests are deleted rather than ported, along with the `AUTOBAHN_EVMONLY` environment variable that selected between the two shapes in the docker cluster and the e2e deployer. Co-authored-by: Cursor <cursoragent@cursor.com>
Every matrix job runs TestStartup before its suites, and TestStartup reads `seid q tendermint-validator-set` and `seid status`. Autobahn serves the EVM JSON-RPC only, so those queries answer nothing and every AUTOBAHN=true row dies at the gate before reaching a test. TestAutobahnStartup checks the same two properties where Autobahn does answer: the EVM RPC serves a balance query, and the executor's next_block metric advances between two samples. The Autobahn Cosmos Modules row is removed rather than re-gated: it drives wasm, gov, oracle and authz against an application Autobahn no longer runs. TestAutobahnBankModule went with it, as that row was its only caller. Both Autobahn upgrade rows are marked `"disabled": true` with a TODO to re-enable, so the matrix keeps the intent instead of losing it. The matrix builder filters on that flag and logs what it skipped. Also fix the staticcheck ST1005 failure on the capitalized error string in the Autobahn RPC startup path. Co-authored-by: Cursor <cursoragent@cursor.com>
The Autobahn startup gate watches height, and with allow_empty_blocks false an idle mempool never seals, so the gate cannot pass. Docker clusters now pass --allow-empty-blocks; the generator still defaults off. Co-authored-by: Cursor <cursoragent@cursor.com>
The four remaining AUTOBAHN=true rows reach their suites now that the startup gate is Autobahn-shaped, and fail there: they call eth_estimateGas, eth_getLogs, debug_trace* and sei_*, none of which the EVM-only RPC serves. Each is marked disabled with the reason and a TODO to re-enable. EVM GIGA Mixed (Determinism) is removed. It pairs one GIGA+OCC validator against three V2 validators to catch an AppHash divergence between the two executors, which the matrix no longer needs to carry; make giga-mixed-integration-test still runs it. Co-authored-by: Cursor <cursoragent@cursor.com>
The EVM-only cut had dropped the rpc-node sidecar and the BFT kill/recover subtests; both are Autobahn-native coverage, now observed via EVM RPC and execution-height metrics instead of Tendermint RPC. Co-authored-by: Cursor <cursoragent@cursor.com>
Fault tests and the CI startup gate submit a 713715 transfer and wait for a receipt. Docker clusters keep allow_empty_blocks off. Co-authored-by: Cursor <cursoragent@cursor.com>
1d8717c to
fc6e5c5
Compare
evmOnlyApplication on main keeps committedHeight in memory, so a restarted validator replays block 1 onto ahead-of-it FlatKV state and panics. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@seidroid re-review please |
|
Claude finished @wen-coding's task in 1m 7s —— View job Re-review of #4316 at
|
Nothing passed it after Docker clusters went back to allow_empty_blocks=false. Co-authored-by: Cursor <cursoragent@cursor.com>
PR SummaryHigh Risk Overview CI and local cluster wiring follows that model: matrix jobs skip entries with Autobahn integration tests are narrowed to EVM-only load and fault-tolerance (height from validator metrics, progress via raw EVM txs); Cosmos/Tendermint RPC coverage and the old dual-mode Reviewed by Cursor Bugbot for commit ad88fcd. Bugbot is set up for automated code reviews on this repo. Configure here. |
manav2401
left a comment
There was a problem hiding this comment.
LGTM - except a small nit from AI.
|
@seidroid review |
There was a problem hiding this comment.
Making Autobahn always select the EVM-only executor, keyed in wrapApplication with mock-app checked first, looks correct, and the rewritten Autobahn integration suite and new startup gate fit that shape. Two issues are non-blocking but still open: the mock-app RPC port clash from the last review, and the "EVM GIGA Mixed (Determinism)" CI row, which is removed without being mentioned in the PR description.
Findings: 0 blocking | 4 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] The Autobahn shape is chosen by two separate conditions.
wrapApplicationchecks whether Giga storage is present, whileOnStartchecksAutobahnConfigFile != "". They agree today only becauseprepareApplicationopens storage exactly when the config file is set. Use one condition in both places, or record the decision once, so the executor and the RPC server can't drift apart. - [suggestion] The integration test cleanup also deletes doc comments on helpers that stay:
findRepoRoot, theTestMaingodoc, and the notes insidesetupCluster. None of this is needed for the behaviour change, and those comments explained non-obvious steps. Please keep them. - 2 suggestion(s)/nit(s) flagged inline on specific lines.
| // Start the RPC server before the P2P server | ||
| // so we can eg. receive txs for the first block | ||
| if n.config.EVMOnly { | ||
| if n.config.AutobahnConfigFile != "" { |
There was a problem hiding this comment.
[suggestion] Still open from the last review. With mock-app set, Autobahn now always starts the EVM-only RPC on 8545; before this PR it served Tendermint RPC. MockApp.InitChain still forwards to the wrapped Cosmos app, so app.Initialized() fires. If API or gRPC is enabled, RegisterLocalServices (app/app.go:2596) also starts the Cosmos EVM HTTP/WS servers, and the port clash makes evmHTTPServer.Start() panic. Only the sed overrides in step4_config_override.sh prevent this today. Suppress the Cosmos EVM servers (or reject the config) wherever Autobahn is enabled, instead of relying on the Docker scripts. Codex raised this too.
Start EVM-only RPC from Giga storage like wrapApplication, skip Cosmos evmrpc under Autobahn so mock-app cannot bind 8545 twice, and wait for seid to exit after pkill. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@seidroid review please |
|
Claude finished @wen-coding's task in 40s —— View job Re-review of #4316 at
|
If storage later becomes optional, missing Giga must fail rather than fall through to Cosmos or Tendermint RPC. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@seidroid review please |
|
Claude finished @wen-coding's task in 34s —— View job Re-review of #4316 at
|
Autobahn always runs the EVM-only executor since #4316, so the AUTOBAHN_EVMONLY switch the AWS topology passed through is dropped.
* main: (21 commits) Backport evmonly parse, app-hash and changeset perf fixes from giga-1 (#4345) Remove the oracle module behind a v6.8 upgrade (#4319) fix(seidb): report only the current migration boundary on the snapshot gauge (#4327) fix(flatkv): keep 10 old checkpoints instead of mirroring memIAVL's count (#4322) Backport Autobahn execute-loop and produced-tx metrics from giga-1 (#4330) Add giga.storage.receipts to toggle the Autobahn receipt store (#4333) optimize gather phase (#4326) Regenerate the Unreleased changelog as a plain PR list (#4336) Bump sei-protocol/go-ethereum to v1.15.7-sei-21 (#4332) Fail dynamic-gas precompile out-of-gas as an EVM out-of-gas call (#4318) Add dashboard and topology option for Autobahn e2e (#4167) Add Giga fetch/serve and BlockDB prune metrics (#4329) Add eth_getLogs to the EVM-only Giga RPC (#4308) Generate v6.8 precompiles (#4320) Add [giga] app.toml section and honor it on the Autobahn node (#4323) feat(evmonly): add eth_estimateGas via existing libraries (#4325) Use Pebble batch directly in SS (#4300) Fix pruning issue in SS causing huge disk spike (#4321) Make Autobahn always run the EVM-only executor, disable/remove some integration tests (#4316) reduce seal lock contention (#4314) ...
…ei-protocol#4353) This is part 1 of re-enabling all Autobahn tests in EVM-only mode. This PR enables some of them and others will be enabled once precompiles and RPC methods are available in EVM only. Since sei-protocol#4316, Autobahn runs the EVM-only executor, and most Autobahn integration coverage was disabled or skipped. Even plain-EVM suites could not run: the shared hardhat setup looked up each test account's Sei address, a Cosmos query the EVM-only chain cannot answer, so every suite failed before its first test, and `SetCodeTxTest.js` signed for the old chain ID. The Autobahn Basic `Recovery` test was skipped until the durable execution cursor reached `main`, which it now has (sei-protocol#4351). The setup now skips the Sei address lookup on the EVM-only chain and behaves as before on every other chain, and `SetCodeTxTest.js` reads the chain ID from the node. `SeiSoloTest.js` is skipped on the EVM-only chain, because it claims Sei-account and CosmWasm balances that chain does not have; it still runs on the non-Autobahn row. With that, the "Autobahn EVM Interoperability (Misc Tests)" row passes and is re-enabled, and `Recovery` is un-skipped. `TestAutobahnStartup` also reads the sender's nonce before signing, so it can run more than once on the same cluster. The other Autobahn rows stay disabled because what they test is not on the EVM-only chain yet. The compat and RPC fixture suites need RPC methods it does not serve (`debug_trace*`, reading state at past blocks, `eth_getStorageAt`, `eth_getBlockReceipts`, filters, `web3_clientVersion`, WebSocket), the precompile suite needs Sei precompiles, and the upgrade suites need a governance-driven upgrade. The Giga suite also has test-side fixes to make: it assumes a new account starts with a zero balance, and one test hangs. Those rows will be re-enabled in follow-ups as the missing methods land. Tested locally on an EVM-only Autobahn cluster: - [x] `evm_interoperability_misc_tests.sh` passes end to end (`SeiSoloTest.js` 3 pending, `SetCodeTxTest.js` 1 passing, `TransientStorageTest.js` 19 passing) - [x] `EVMCompatabilityTest.js` gets past setup (59 passing; the rest need the missing RPC methods) - [x] `TestAutobahnStartup` passes twice in a row on the same cluster - [x] Autobahn Basic `Recovery` passes (`make autobahn-integration-test`) - [x] Non-Autobahn rows unchanged (covered by CI)


Autobahn had two shapes: with
evm-onlyit 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-onlyconfig key is gone. Selection happens inwrapApplication, the one function every startup path passes through, keyed on the Giga storage manager thatautobahn-config-filealready opens.mock-appstill 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_EVMONLYis gone;make autobahn-evmonly-integration-testremains as an alias). Autobahn CI rows that need the fullevmrpcsurface or Cosmos gov stay in the matrix as"disabled": truewith TODOs.Autobahn Basic
The suite still starts a four-validator cluster plus the
sei-rpc-nodefullnode sidecar. It runs:EVMOnlyLoad— 4000 raw transfers on chain 713715, receipts and balances on every validator, plus a receipt on the fullnodeLivenessUnderMaxFaults— afterfkills, a submitted transfer must finalizeHaltsBeyondMaxFaults— afterf+1kills, a submitted transfer must not landRecovery— skipped until the durable EVM-only execution cursor (#4231, ongiga-1) reachesmain. Without it a restarted validator reports height 0, replays block 1 onto ahead-of-it FlatKV state, and panics withnonce too lowThe CI startup gate for
AUTOBAHN=truerows checkseth_getBalanceand waits for one committed transfer. Docker clusters keepallow_empty_blocksoff.Test plan
go test ./sei-tendermint/node/ ./sei-tendermint/config/ ./cmd/autobahn-e2e/go vet -tags autobahn_integration ./integration_test/autobahn/