-
Notifications
You must be signed in to change notification settings - Fork 142
Description
Since this issue exists in the only wasm module, there is no problem in the chain using general cosmos/evm, but this structure is also used in ibc-go's RecvPacket, and there is possible for it to become a problem when expanded further, so I am leaving it here to discuss it.
Problem
When other modules (wasm, ibc, etc.) call CacheContext() and execute commitFn() within an EVM transaction(ex. precompile contract), the cosmos/evm snapshot stack becomes invalidated if a revert occurs, causing RevertToSnapshot() to fail.
Root Cause:
- cosmos-sdk: Failed transactions can be fully reverted
- cosmos/evm: Must be treated as successful even on failure (to include tx_response.data). Requires commit at block creation time
- Problem: After intermediate commits, the snapshot stack is invalidated, preventing selective revert
Scenario
EVM Contract → WASM Precompile → WASM Submessage → Out of Gas
EVM Transaction
├─ Contract A call (snapshot 0)
│ ├─ Contract B call (snapshot 1)
│ │ ├─ Precompile WASM call (snapshot 2)
│ │ │ └─ WASM submessage dispatch
│ │ │ └─ commitFn() called → ❌ Already committed to parent!
│ │ └─ ❌ Out of gas failure
│ │ └─ RevertToSnapshot(1) attempted
│ │ └─ ❌ Impossible! Stack invalidated by WASM submessage commit
│ └─ Contract A continues execution
└─ Must commit at block creation even if ultimately failed
Issue:
- WASM submessage's commitFn() call triggers snapshotmulti.Store.Write(), invalidating the snapshot stack
- Subsequent RevertToSnapshot() attempts result in panic: snapshot index out of bound
Solution
Use Commit only when EthereumTx is fully processed, and make Write() used by CacheContext a no-op to preserve the existing stack.
Changes:
- Change snapshotmulti.Store.Write() to a no-op
- Preserve snapshot stack when external modules call commitFn()
- Add Commit() method to Snapshotter interface
- Explicit control over actual commits
- Call snapshotter.Commit() in StateDB.writeCache
- Perform actual commit only after EthereumTx is fully processed
Effects:
- External modules' commitFn() calls do not invalidate the snapshot stack
- All changes are committed in batch at transaction end, without intermediate commits
- EVM's RevertToSnapshot() works correctly
Interface Changes:
- Snapshotter interface needs a Commit() method
- Compatibility with existing CacheMultiStore interface is maintained (Write() still exists, operates as no-op)
Reference
- Where to use CacheContext's commitFn in wasm
- Code example modified with the above solution
Question
I'm wondering if my understanding of the snapshot structure is correct. I'm also wondering if there might be a better solution.