Integrate evmonly executor with giga store - #3864
Conversation
|
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 @@
## main #3864 +/- ##
==========================================
- Coverage 61.28% 60.27% -1.02%
==========================================
Files 2155 2055 -100
Lines 188513 176894 -11619
==========================================
- Hits 115526 106616 -8910
+ Misses 62192 60439 -1753
+ Partials 10795 9839 -956
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
a80537b to
66a304c
Compare
1a66532 to
819a564
Compare
PR SummaryMedium Risk Overview A new in-memory Load harness and API tightening: the loadtest wires Reviewed by Cursor Bugbot for commit 83f8025. Bugbot is set up for automated code reviews on this repo. Configure here. |
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Solid, well-tested refactor making the evmonly executor store-only over the giga StateDB interface; the encoder slab arithmetic, storage-clear versioning, commit validation ordering, and error/release paths all check out. One suggestion: MemoryStore's touch-based AccountExists diverges from the account-existence semantics sei-db/state_db/giga/api.go documents.
Findings: 0 blocking | 2 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion]
--workersin the loadtest now rejects every value except its default of1(cmd/evmonly-loadtest/config.go), leaving a CLI flag with a single legal value. Either drop the flag until the harness can order commits across workers, or keep it and have the harness serializeCommitStateChangesitself so the knob stays meaningful. - 1 suggestion(s)/nit(s) flagged inline on specific lines.
| _, codeTouched := latestMemoryStoreValue(s.store.code[addr], s.height) | ||
| firstStorageTouch, storageTouched := s.store.storageTouch[addr] | ||
| s.store.mu.RUnlock() | ||
| if balanceTouched || nonceTouched || codeTouched || storageTouched && firstStorageTouch <= s.height { |
There was a problem hiding this comment.
[suggestion] AccountExists is touch-based rather than value-based: once any balance/nonce/code node or storage touch is recorded for an address, it reports true at every height from that point on. Two consequences diverge from the contract in sei-db/state_db/giga/api.go:
- A block that self-destructs a contract emits
BalanceChange{Balance: 0}, a code delete, and aStorageClear. All three create nodes, andtouchStorageAccountLockedrecords the clear as a storage touch, so the destroyed account reportsAccountExists() == trueforever — not just "in the current block" as the interface doc allows.GetCodeHash(line 451) then returnskeccak256("")where the doc specifies the zero hash for non-existent accounts, i.e. the opposite of EXTCODEHASH /keeper.GetCodeHashbehaviour. TestMemoryStoreTracksZeroValueAndStorageOnlyAccountspins the same shape for an account whose only write isBalance: 0, which a real SC/SS store would not materialize as an existing account.
Nothing consumes this today — gigaSnapshotStateReader only implements GetBalance/GetNonce/GetCode/GetState, and nativeStateDB.Exist derives existence itself — so this is not a correctness bug in the executor path. But MemoryStore is the first and only implementation of gigastore.StateDB and is positioned as the test double for it, so tests written against these semantics will pass here and fail against the production store. Consider resolving existence from the effective values at s.height (non-zero balance, non-zero nonce, non-empty code, or a live storage slot) instead of from touch records, and dropping storageTouch in favour of a height-versioned marker — or, if the touch-based approximation is deliberate, say so in the MemoryStore godoc and note the GetCodeHash deviation.
| } | ||
|
|
||
| func (e *Executor) executePreparedBlock(ctx context.Context, req PreparedBlock) (*BlockResult, error) { | ||
| func (e *Executor) executePreparedBlock(ctx context.Context, req PreparedBlock, source StateReader) (*BlockResult, error) { |
There was a problem hiding this comment.
Is source properly closed after the execution is done? The way how StateSnapshot works require the underlying goroutine to close the reader after usage is done
There was a problem hiding this comment.
Yes. source is only an adapter around the snapshot opened in executePreparedBlockWithStore, and that function registers defer snapshot.Close() immediately after the nil check. Sequential execution is synchronous; on the OCC path, occWorkerPool.Run returns only after errgroup.Wait() has joined every worker. Each worker also releases its nativeStateDB and resets its source before executePreparedBlock returns. Therefore the opening goroutine closes the snapshot only after all executor goroutines have stopped using it, including error paths, and before ExecutePreparedBlock proceeds to the result sink. TestExecutorGigaStoreSnapshotFeedsOCCExecution covers closure on the OCC path.
| @@ -45,6 +45,8 @@ type StateDB interface { | |||
| // | |||
| // Until Close, the underlying resources (e.g. an ephemeral SC snapshot or a | |||
| // pinned SS version) stay alive, even concurrently with later writes/commits. | |||
| // All read methods must be safe for concurrent calls because EVM executor | |||
| // workers may share one snapshot while executing a block. | |||
There was a problem hiding this comment.
We will rename this to StateView in the future PR, just FYI, no action needed for now
819a564 to
83f8025
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 83f8025. Configure here.
| } | ||
| s.mu.RUnlock() | ||
| return &memoryStoreSnapshot{store: s, height: height} | ||
| } |
There was a problem hiding this comment.
Genesis snapshot collides with block 0
Medium Severity
OpenView pins uncommitted genesis at height 0, and CommitStateChanges also accepts block 0. A snapshot opened before that commit then sees the new nodes, because latestMemoryStoreValue treats height-0 versions as visible. That breaks the StateView rule that an open view never observes later writes.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 83f8025. Configure here.
## Describe your changes and provide context This PR builds on the EVM-only Giga store merged in sei-protocol#3864 and is rebased onto that merge commit. - Add a test-only top-level `evm-only-in-memory` node setting that replaces the Cosmos ABCI application before `proxy.New`, so Autobahn and the executor share the same application. - Initialize the EVM-only executor from `InitChain`, accept protected raw Ethereum transactions on chain ID 1337, and execute finalized blocks with a deterministic app hash. - Add a minimal EVM JSON-RPC listener on port 8545 with only `eth_sendRawTransaction`. It validates Ethereum encoding, routes by Autobahn EVM shard ownership, submits through the in-process mempool, and returns the Ethereum transaction hash. - Do not start Tendermint RPC for EVM-only nodes. The Docker harness also disables the unused Cosmos REST, gRPC, and gRPC-web servers. - Keep executor and Autobahn consensus state in memory; Docker disables Autobahn persistence with `--persistent-state-dir=`. - Add `make autobahn-evmonly-integration-test`, which submits 4,000 signed transfers through EVM JSON-RPC, asserts Tendermint RPC is unavailable, and uses internal Prometheus execution counters to wait for every validator. - Add the `autobahn-e2e` cluster manager with `deploy`, `list`, `forward`, and `teardown` subcommands for local Docker or a managed EC2 host. `list` obtains height from internal Prometheus metrics and does not depend on Tendermint RPC. - Keep the runtime deliberately ephemeral and load-test-only. Persistence, restart recovery, state sync, receipts, additional EVM RPC methods, and staking or other custom precompiles are out of scope. ## Testing performed to validate your change - `make autobahn-evmonly-integration-test` (4,000 raw transfers through EVM JSON-RPC; all four validators executed through height 8; 5,010 tx/s observed locally; Tendermint RPC unavailable) - Real `autobahn-e2e` local lifecycle: deploy four nodes, list live status/Prometheus heights, forward node 2 to `127.0.0.1:18545`, call the EVM RPC through the tunnel, confirm Tendermint `/status` resets, then teardown all containers and the network - `go test -count=1 ./cmd/autobahn-e2e` - `go test -count=1 -run "^TestEVMOnlyRPC" ./sei-tendermint/node` - `go test -c -tags autobahn_integration ./integration_test/autobahn` - `go vet ./sei-tendermint/node ./cmd/autobahn-e2e` - `go vet -tags autobahn_integration ./integration_test/autobahn` - golangci-lint v2.8.0 on the changed packages with `--build-tags autobahn_integration` (0 issues) - golangci-lint v2.8.0 `fmt --diff` across the repository - `sh -n docker/localnode/scripts/step4_config_override.sh docker/localnode/scripts/step5_start_sei.sh` - `git diff --check`
2 participants
Summary
CommitStateChangesNamedChangeSetEncoderand preserve storage-prefix clearsMemoryStoreimplementation over the existing immutableStateReaderNamedChangeSetkey/value pairs with contiguous backing allocationsWhy
The evmonly executor now has one persistence model: a giga
Store. The concrete store implementation can vary, but execution no longer has a separate non-giga state path.The first loadtest adapter wrapped the complete native changeset in RLP and decoded it immediately inside
CommitStateChanges. The direct format removes that redundant work while continuing to exercise the real giga interface.Loadtest
Configuration: 400 blocks, 1,000 transactions/block, one ordered block worker, 12 executor workers, zero gas price, and discard result sink. Values are three-run medians in tx/s.
Every run completed 400,000/400,000 transactions successfully with zero execution errors and zero OCC fallbacks.
The pre-MemoryStore comparison is not an equivalent persistence implementation: it executes against
WithStateand discards block state changes, while the Giga run encodes, commits, and retains current and historical state for later snapshots. It is therefore a useful lower bound on commit overhead, not evidence that the Giga interface itself costs 10% in production.In an 800-block snapshot/revert profile,
EncodeMemoryStoreChangeSetandCommitStateChangeseach represented about 0.1% of sampled CPU. Most MemoryStore-specific allocation was the retained versioned storage map. A pointer-free indexed-history experiment did not improve end-to-end throughput and was reverted.Validation
go test ./giga/... ./sei-db/state_db/gigago test -race ./giga/evmonly/...go vet ./giga/evmonly/... ./sei-db/state_db/gigagofmt -s -landgoimports -lgit diff --checkThe full-tree
goimports -l .reports pre-existing untouched generated and test files.