cosmos / cosmos/evm

Unintended process when other modules use CacheContext's commit

Open
#903 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
164
Forks
213
Avg merge
3d 58m
Merged PRs (30d)
12

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:
1. WASM submessage's commitFn() call triggers snapshotmulti.Store.Write(), invalidating the snapshot stack
2. 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:
1. Change snapshotmulti.Store.Write() to a no-op
- Preserve snapshot stack when external modules call commitFn()
2. Add Commit() method to Snapshotter interface
- Explicit control over actual commits
3. 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
- https://github.com/CosmWasm/wasmd/blob/8048c560d5aba2f62544139793a5257f1f1308fb/x/wasm/keeper/msg_dispatcher.go#L126
- Code example modified with the above solution
- https://github.com/xpladev/evm/pull/1

## Question
I'm wondering if my understanding of the snapshot structure is correct. I'm also wondering if there might be a better solution.

Contributor guide

Open the contributing guide

Research direction

Trace snapshotmulti.Store.Write, the Snapshotter interface, StateDB.writeCache, and the wasm msg_dispatcher.go reference cited in the issue. First verify how CacheContext commitFn affects the snapshot stack, then validate that a nested failure can still call RevertToSnapshot after an external commitFn; done means preserving selective reverts while committing the completed Ethereum transaction.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, wasm
Domain
blockchain
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.