[vipwzw]

What

When an EVM sub-call with value reverts, its transfer is dropped from the receipt but stays in the coins account cache. A later transfer in the same transaction can then spend the refunded balance and mint native coin.

#1348 (merged, released in 1.76.4) fixed this behind ForkEVMTransferRevert by undoing each transfer by hand: it captures the account KV before every transfer and writes it back through chain33's StateDB.RevertSet. This PR keeps the fork but replaces the mechanism behind it with automatic rollback, so a new write path cannot bring the bug back.

From ForkEVMTransferRevert on, MemoryStateDB wraps the executor StateDB in journalKV, a write buffer with nested rollback points that line up with Snapshot versions. It works like chain33's txcache, but with one level per EVM call frame instead of one per transaction:

• Reads check the buffer first, then the StateDB. Writes go only to the buffer, and each write records the key's previous value.
• RevertToSnapshot unwinds the buffer to that snapshot's rollback point.
• GetChangedData puts the surviving writes into the receipt, sorted by key, and writes them back to the StateDB so later transactions in the block can read them.

CoinsAccount is a copy bound to the buffer, so the executor's shared object is not modified. Coins and token transfers, SELFDESTRUCT payouts, paracross ExecTransfer and contract account loads all go through the buffer and are undone with no per-path revert code. Nothing reaches the executor StateDB while the EVM runs, so checkKV holds by construction, and RevertSet is no longer used.

After the fork, Snapshot.revert also unwinds entries in reverse order, and Create takes its snapshot before the value transfer, as in #1348.

Flow

How one transaction runs through MemoryStateDB after this PR:

flowchart TD
    nExec["Exec: NewMemoryStateDB"] --> nFork{"ForkEVMTransferRevert active?"}
    nFork -- "no" --> nLegacy["Legacy path, unchanged<br/>writes go straight to the executor StateDB<br/>transferChange carries the receipt KV"]
    nFork -- "yes" --> nWrap["Wrap the StateDB in journalKV<br/>bind a CoinsAccount copy to it"]
    nWrap --> nFrame["CALL / CREATE: Snapshot(v)<br/>journalKV records rollback point v"]
    nFrame --> nWrite["Transfer, token transfer, SELFDESTRUCT<br/>journalKV.Set buffers the write<br/>and logs the previous value"]
    nWrite --> nFailed{"Frame fails?"}
    nFailed -- "yes" --> nRevert["RevertToSnapshot(v)<br/>unwind the log back to v"]
    nRevert --> nMore{"More frames?"}
    nFailed -- "no" --> nMore
    nMore -- "yes" --> nFrame
    nMore -- "no" --> nDone{"Transaction succeeded?"}
    nDone -- "yes" --> nFlush["GetChangedData<br/>surviving writes go into the receipt KV<br/>and back to the executor StateDB"]
    nDone -- "no" --> nDrop["Buffer discarded<br/>executor StateDB untouched"]
    nFlush --> nCheck["checkKV passes:<br/>only receipt keys were written"]

Loading

The mint from the report, before and after the fork:

sequenceDiagram
    participant A as Attacker
    participant P as Contract P
    participant Q as Contract Q
    participant O as Payout address
    A->>P: CALL with 100 BTY (committed)
    P->>Q: CALL with 100 BTY (committed)
    Q->>P: refund 100 BTY
    Note over P: P reverts any plain value call
    Note over Q,P: Before the fork: the receipt drops the refund,<br/>but the cache still credits P with 100
    Note over Q,P: After the fork: journalKV unwinds the refund,<br/>P = 0 and Q = 100 in both buffer and receipt
    P->>O: CALL with 100 BTY
    Note over P,O: Before the fork: succeeds, 100 BTY minted<br/>After the fork: P has 0, the call fails

Loading

Fork

Below the fork the code is the same as before #1348: writes go straight to the StateDB, transferChange carries the receipt KV, Snapshot.revert runs forward, and Create transfers before snapshotting. Historical blocks replay unchanged.

The fork keeps the registration shipped in 1.76.4: MaxHeight in InitFork, -1 in chain33.fork.toml, chain33.para.toml and chain33.proxyminer.toml. Until a chain configures an activation height, the vulnerability stays live there. The height is set at release time.

Removed from #1348: transferChange.preState, capturePreState, revertAccountKey, isForkActive, RevertError/ResetRevertError with the guard in innerExec, snapshot_revert_failure_test.go, and the tests of those internals. A buffered revert cannot fail, so the error plumbing has nothing left to carry.

Tests

The #1348 tests that pin both sides of the fork are kept and pass against the buffer:

• TestRevertedCallValueTxSucceedsUnderCheckKV / TestRevertedCallValueMintTxCreatesNoValue: the production executor.StateDB under ForkExecRollback, checked with chain33's checkKV rule.
• TestTransferToTokenRevertCheckKVPair: the token precompile on the same harness.
• TestRevertedCallValueCannotBeSpent: a transaction-level repro through Exec. Before the fork it mints; after the fork it does not.
• TestFailedNestedCreateReturnsValue, TestSuccessfulCreateWithValueCommitsBalances, TestTransferRevertRestoresBalances, TestSnapshotRevertSameSlotReverseOrder: before/after pairs.

New:

• TestRevertedValueTransferMints: the report's two-contract refund trap. Coin supply is conserved.
• TestRevertedDoubleSStoreLeaks: the parent frame cannot read a value written by a reverted frame.
• TestJournalKVRevertedTransfer: a reverted transfer never reaches the StateDB, and every key written to the StateDB appears in the receipt.
• TestJournalKVNestedRevert.

go test ./plugin/dapp/evm/... ./plugin/dapp/evmxgo/... passes.

🤖 Generated with Claude Code

[vipwzw]

Closing: the reverted-transfer mint is already fixed by #1348. Nested rollback will be added to chain33 itself instead.