[codex] add evm-only giga executor path#3583
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3583 +/- ##
==========================================
- Coverage 59.16% 58.24% -0.92%
==========================================
Files 2262 2175 -87
Lines 187009 176465 -10544
==========================================
- Hits 110643 102790 -7853
+ Misses 66430 64647 -1783
+ Partials 9936 9028 -908
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
codchen
changed the title
PR SummaryMedium Risk Overview The Reviewed by Cursor Bugbot for commit 5e36ab5. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
codchen
force-pushed
the
codex/sei-v3-evm-only-scaffold
branch
from
44a10bc to
ef04c0e
Compare
codchen
force-pushed
the
codex/sei-v3-evm-only-scaffold
branch
from
ef04c0e to
cb338ff
Compare
codchen
force-pushed
the
codex/sei-v3-evm-only-scaffold
branch
from
06357b4 to
966d056
Compare
|
|
||
| - sequential execution of the ordered block transaction list | ||
| - RLP decoding and sender recovery through go-ethereum signers | ||
| - go-ethereum `core.ApplyMessage` execution against an SDK-free `vm.StateDB` |
There was a problem hiding this comment.
does "SDK" mean Cosmos?
| for _, acct := range s.accounts { | ||
| if acct.SelfDestructed { | ||
| acct.Code = nil | ||
| acct.Storage = map[common.Hash]common.Hash{} |
There was a problem hiding this comment.
do we need to set acct.Created = false?
There was a problem hiding this comment.
Created is used to decided whether SelfDestruct6780 should early return, and I think the behavior of SelfDestruct6780 is to not early return if selfdestruct is called multiple times, so I think we shouldn't set Created to false upon selfdestruct (consistent with geth)
| if gasLimit == 0 { | ||
| gasLimit = math.MaxUint64 | ||
| } |
There was a problem hiding this comment.
is this dangerous? could someone maliciously set gasLimit = 0 to bypass any limits we may have?
There was a problem hiding this comment.
gasLimit here is the block gas limit, so it'd be decided by consensus and not set by tx senders
| @@ -0,0 +1,653 @@ | |||
| package evmonly | |||
There was a problem hiding this comment.
this is a lot of critical state changing code. i think we need a lot of tests here. Could we use AI to generate a ton of unit cases, interleaving ordering of contract creation/deletion, invalid txs (out of gas, invalid state transition, invalid nonce, verify receipt outputs...etc).
codchen
force-pushed
the
codex/sei-v3-evm-only-scaffold
branch
from
966d056 to
ab82ec3
Compare
codchen
force-pushed
the
codex/sei-v3-evm-only-scaffold
branch
from
ab82ec3 to
23fc6d3
Compare
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 5e36ab5. Configure here.
| Value: newValue, | ||
| Delete: newValue == (common.Hash{}), | ||
| }) | ||
| } |
There was a problem hiding this comment.
Self-destruct omits unstaged storage
High Severity
When a contract is self-destructed or recreated via CreateAccount, Finalise clears only the in-memory storage map. ChangeSet compares storage using keys from base.Storage, which is populated lazily via ensureStorage. Slots that still exist in the persistent StateReader but were never read or written in that block never appear in the changeset, so ApplyChangeSet leaves orphaned storage at that address.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5e36ab5. Configure here.
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
A new, self-contained giga/evmonly package adds an SDK-free EVM-only block executor with a go-ethereum-backed vm.StateDB, changeset output, receipts, and extensive tests. It is not wired into the production app path, but contains two genuine EVM-semantics correctness bugs (EXTCODEHASH for existing code-less accounts, and storage clearing on legacy SELFDESTRUCT/SetStorage) plus a notable per-snapshot deep-copy performance cost that should be addressed before this becomes the live giga path.
Findings: 2 blocking | 5 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
- 2 blocking issue(s) flagged inline on specific lines.
Non-blocking
- cursor-review.md was empty — the Cursor second-opinion pass produced no output, so only this review and Codex's findings were merged.
GetStorageRootalways returns the zero hash, so the CREATE/CREATE2 collision check (nonce != 0 || codeHash != EmptyCodeHash || storageRoot != EmptyRootHash) can miss collisions against a pre-existing account that has storage but no code/nonce. Combined with the GetCodeHash issue this narrows collision detection to nonce-only for storage-only accounts. Edge case, but worth documenting alongside the known limitations.- EIP-158/161 empty-account deletion is not implemented in
Finalise(touched-but-empty accounts are not pruned). For the value-based changeset this is mostly benign since no diff is recorded, but it diverges from go-ethereum StateDB semantics and is worth a note in the README limitations. - Consider adding tests that exercise the two correctness gaps above: EXTCODEHASH against a funded code-less EOA (expect EmptyCodeHash, not 0x0), and a legacy SELFDESTRUCT of a pre-existing contract whose persisted storage slots were never read in-block (expect deletes in the changeset).
- 1 suggestion(s)/nit(s) flagged inline on specific lines.
| func (s *nativeStateDB) GetCodeHash(addr common.Address) common.Hash { | ||
| code := s.GetCode(addr) | ||
| if len(code) == 0 { | ||
| return common.Hash{} |
There was a problem hiding this comment.
[blocker] EXTCODEHASH semantics bug: this returns common.Hash{} (0x0) whenever code is empty, conflating "account does not exist" with "account exists but has no code". go-ethereum's StateDB returns types.EmptyCodeHash (keccak256 of empty) for an existing account with no code and only common.Hash{} for a non-existent account. As written, EXTCODEHASH on a funded/active EOA returns 0x0 instead of the empty-code hash, so contracts observe incorrect account existence/code state. Distinguish existence (e.g. via Exist(addr)) and return types.EmptyCodeHash for existing code-less accounts.
| Delete: len(acct.Code) == 0, | ||
| }) | ||
| } | ||
| storageKeys := storageKeyUnion(base.Storage, acct.Storage) |
There was a problem hiding this comment.
[blocker] The changeset only diffs storage keys present in base.Storage/acct.Storage, and base.Storage is populated lazily only for slots read via ensureStorage. When Finalise clears a self-destructed account (legacy SELFDESTRUCT) or SetStorage resets an account, persisted slots that were never read in-block are in neither map, so no delete is emitted for them. Applying the changeset to durable state then leaves stale storage behind. Because the backend is key-addressable with no range reads, the executor cannot enumerate those slots — this needs an explicit account-clear/delete signal in the changeset (or a documented constraint) rather than per-slot diffs.
| return nil | ||
| } | ||
|
|
||
| func (s *nativeStateDB) Snapshot() int { |
There was a problem hiding this comment.
[suggestion] Snapshot() deep-copies the entire accounts, base, logs, accessList, transientStates, and preimages on every call, and the EVM snapshots on every CALL/CREATE/STATICCALL. This is O(touched-state) per snapshot and will be a severe performance cliff for any non-trivial contract execution. Acceptable for the sequential placeholder, but flag it as a known cost — a journal/undo-log approach (as in go-ethereum) will be needed before this is the live giga path.
3 participants
Summary
giga/evmonlyas the final-form EVM-only giga execution path boundarygiga/evmonly:vm.StateDBStateChangeSetoutputMemoryStatebackend for tests and early integrationNotes
Custom precompile behavior is intentionally still open. Registered custom precompile addresses return
ErrCustomPrecompilesOpen, including through geth's precompile map, so calls fail closed instead of silently executing as empty accounts.The current port is sequential. The state boundary and changeset shape are intended to be replaceable with the sei-v3 OCC scheduler/store once that layer is brought over.
Historical
BLOCKHASHlookups beyond the parent block are not wired yet;BlockHashis currently used for receipt/log metadata.Validation