fix(evm): make reverted value transfers revertible

chain33.fork.toml

@@ -36,6 +36,8 @@ ForkEthAddressFormat=0
 ForkTxChainIDStrict=-1
 # 创世动作(Genesis)执行高度限制：启用后框架统一拒绝 height != 0 的 Exec_Genesis，防止执行器漏写 height 校验被创世重入；-1 表示关闭
 ForkGenesisOnlyAtHeightZero=0
+# eth签名交易的chain33信封校验启用高度；-1 表示关闭
+ForkEthSignEnvelope=-1
 
 [fork.sub.none]
 ForkUseTimeDelay=0
@@ -99,6 +101,8 @@ ForkEVMFixOverflow=0
 # SELFDESTRUCT pays from the executing account: DELEGATECALL/CALLCODE no longer
 # spend the borrowed code contract balance
 ForkEVMSuicidePayer=0
+# Revertible value transfers; -1 keeps the fix off (resolved to MaxHeight)
+ForkEVMTransferRevert=-1
 
 
 [fork.sub.evmxgo]

chain33.para.toml

@@ -344,6 +344,8 @@ ForkEthAddressFormat=0
 ForkTxChainIDStrict=-1
 # 创世动作(Genesis)执行高度限制：启用后框架统一拒绝 height != 0 的 Exec_Genesis，防止执行器漏写 height 校验被创世重入；-1 表示关闭
 ForkGenesisOnlyAtHeightZero=0
+# eth签名交易的chain33信封校验启用高度；-1 表示关闭
+ForkEthSignEnvelope=-1
 
 [fork.sub.none]
 ForkUseTimeDelay=0
@@ -435,6 +437,9 @@ ForkEVMFixOverflow=0
 # SELFDESTRUCT pays from the executing account: DELEGATECALL/CALLCODE no longer
 # spend the borrowed code contract balance
 ForkEVMSuicidePayer=0
+# Revertible value transfers; -1 keeps the fix off (resolved to MaxHeight)
+ForkEVMTransferRevert=-1
+
 [fork.sub.blackwhite]
 Enable=0
 ForkBlackWhiteV2=0

go.mod

@@ -7,7 +7,7 @@ toolchain go1.22.12
 replace github.com/ava-labs/avalanchego => github.com/33cn/avalanchego v1.10.10-0.20240529041529-ada691598153
 
 require (
-	github.com/33cn/chain33 v1.72.7-0.20261016082959-e1bde75e25d0
+	github.com/33cn/chain33 v1.73.0
 	github.com/BurntSushi/toml v1.3.2
 	github.com/NebulousLabs/Sia v1.3.7
 	github.com/bitly/go-simplejson v0.5.0

go.sum

@@ -46,10 +46,8 @@ dmitri.shuralyov.com/html/belt v0.0.0-20180602232347-f7d459c86be0/go.mod h1:JLBr
 dmitri.shuralyov.com/service/change v0.0.0-20181023043359-a85b471d5412/go.mod h1:a1inKt/atXimZ4Mv927x+r7UpyzRUf4emIoiiSC2TN4=
 dmitri.shuralyov.com/state v0.0.0-20180228185332-28bcc343414c/go.mod h1:0PRwlb0D6DFvNNtx+9ybjezNCa8XF0xaYcETyp6rHWU=
 git.apache.org/thrift.git v0.0.0-20180902110319-2566ecd5d999/go.mod h1:fPE2ZNJGynbRyZ4dJvy6G277gSllfV2HJqblrnkyeyg=
-github.com/33cn/chain33 v1.72.0 h1:Nk1CYLSYd5rfvhw1szE2Jt0cJx3A8nZCZZvIEIfTarw=
-github.com/33cn/chain33 v1.72.0/go.mod h1:QCdEczXYJI43WTeFuzEpEhNZ/SfEnLOdFaw6r6LvPz8=
-github.com/33cn/chain33 v1.72.7-0.20261016082959-e1bde75e25d0 h1:r9UFvcR9Sg9g3UBte5QX//e4e45F9DFxYhm4MnaaRlg=
-github.com/33cn/chain33 v1.72.7-0.20261016082959-e1bde75e25d0/go.mod h1:6C5lKl8ejjoXtyhetFXz13t+JV4UQvp9H4t9eLOTNXs=
+github.com/33cn/chain33 v1.73.0 h1:Pp+rKZnw84oeRue4FoOMbHmIKzgk/qNjQKDWhhB5i0U=
+github.com/33cn/chain33 v1.73.0/go.mod h1:6C5lKl8ejjoXtyhetFXz13t+JV4UQvp9H4t9eLOTNXs=
 github.com/AndreasBriese/bbloom v0.0.0-20190825152654-46b345b51c96 h1:cTp8I5+VIoKjsnZuH8vjyaysT/ses3EvZeaV/1UkF2M=
 github.com/AndreasBriese/bbloom v0.0.0-20190825152654-46b345b51c96/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8=
 github.com/Azure/azure-sdk-for-go v41.3.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
@@ -247,6 +245,10 @@ github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792 h1:R8vQdOQdZ9Y3
 github.com/btcsuite/websocket v0.0.0-20150119174127-31079b680792/go.mod h1:ghJtEyQwv5/p4Mg4C0fgbePVuGr935/5ddU9Z3TmDRY=
 github.com/btcsuite/winsvc v1.0.0/go.mod h1:jsenWakMcC0zFBFurPLEAyrnc/teJEM1O46fmI40EZs=
 github.com/buger/jsonparser v0.0.0-20181115193947-bf1c66bbce23/go.mod h1:bbYlZJ7hK1yFx9hf58LP0zeX7UjIGs20ufpu3evjr+s=
+github.com/bysomeone/chain33 v1.72.1-0.20261103124951-0ba08b76ebaf h1:rY3/3AOPt21SQT88dUe8y/D2KqZkA0QvwyoeAM0Xpck=
+github.com/bysomeone/chain33 v1.72.1-0.20261103124951-0ba08b76ebaf/go.mod h1:6C5lKl8ejjoXtyhetFXz13t+JV4UQvp9H4t9eLOTNXs=
+github.com/bysomeone/chain33 v1.72.1-0.20261103141406-9f5c055ab050 h1:puInZbRE5coLWDn5e04xwtInUXFtY5ODuLhO1Ae2CfQ=
+github.com/bysomeone/chain33 v1.72.1-0.20261103141406-9f5c055ab050/go.mod h1:6C5lKl8ejjoXtyhetFXz13t+JV4UQvp9H4t9eLOTNXs=
 github.com/c-bata/go-prompt v0.2.2 h1:uyKRz6Z6DUyj49QVijyM339UJV9yhbr70gESwbNU3e0=
 github.com/c-bata/go-prompt v0.2.2/go.mod h1:VzqtzE2ksDBcdln8G7mk2RX9QyGjH+OVqOCSiVIqS34=
 github.com/cactus/go-statsd-client/statsd v0.0.0-20191106001114-12b4e2b38748/go.mod h1:l/bIBLeOl9eX+wxJAzxS4TveKRtAqlyDpHjhkfO0MEI=
@@ -1358,8 +1360,6 @@ github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8
 github.com/supranational/blst v0.3.11 h1:LyU6FolezeWAhvQk0k6O/d49jqgO52MSDDfYgbeoEm4=
 github.com/supranational/blst v0.3.11/go.mod h1:jZJtfjgudtNl4en1tzwPIV3KjUnQUvG3/j+w+fVonLw=
 github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7/go.mod h1:q4W45IWZaF22tdD+VEXcAWRA037jwmWEB5VWYORlTpc=
-github.com/syndtr/goleveldb v1.0.1-0.20220614013038-64ee5596c38a h1:1ur3QoCqvE5fl+nylMaIr9PVV1w343YRDtsy+Rwu7XI=
-github.com/syndtr/goleveldb v1.0.1-0.20220614013038-64ee5596c38a/go.mod h1:RRCYJbIwD5jmqPI9XoAFR0OcDxqUctll6zUj/+B4S48=
 github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d h1:vfofYNRScrDdvS342BElfbETmL1Aiz3i2t0zfRj16Hs=
 github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d/go.mod h1:RRCYJbIwD5jmqPI9XoAFR0OcDxqUctll6zUj/+B4S48=
 github.com/tarm/serial v0.0.0-20180830185346-98f6abe2eb07/go.mod h1:kDXzergiv9cbyO7IOYJZWg1U88JhDg3PB6klq9Hg2pA=

plugin/dapp/evm/cmd/ci2/chain33.proxyminer.toml

@@ -605,6 +605,8 @@ ForkEVMFixOverflow=0
 # SELFDESTRUCT pays from the executing account: DELEGATECALL/CALLCODE no longer
 # spend the borrowed code contract balance
 ForkEVMSuicidePayer=0
+# Revertible value transfers; -1 keeps the fix off (resolved to MaxHeight)
+ForkEVMTransferRevert=-1
 
 [fork.sub.evmxgo]
 Enable=0

plugin/dapp/evm/executor/exec.go

@@ -58,6 +58,7 @@ func (evm *EVMExecutor) Exec(tx *types.Transaction, index int) (*types.Receipt,
 func (evm *EVMExecutor) innerExec(msg *common.Message, txHash []byte, sigType int32, index int, txFee uint64, readOnly bool) (receipt *types.Receipt, err error) {
 
 	cfg := evm.GetAPI().GetConfig()
+	evm.mStateDB.ResetRevertError()
 	// 获取当前区块的上下文信息构造EVM上下文
 	context := evm.NewEVMContext(msg, txHash)
 	execAddr := evm.getEvmExecAddress()
@@ -191,6 +192,14 @@ func (evm *EVMExecutor) innerExec(msg *common.Message, txHash []byte, sigType in
 		return receipt, vmerr
 	}
 
+	// A snapshot revert that failed leaves the account cache and the receipt describing
+	// different state, which chain33's checkKV can no longer validate. Fail the transaction
+	// (fee-only receipt, state rolled back by the framework) instead of committing it.
+	if rerr := evm.mStateDB.RevertError(); rerr != nil {
+		log.Error("innerExec snapshot revert failed", "error info", rerr)
+		return receipt, rerr
+	}
+
 	// 计算消耗了多少费用（实际消耗的费用）
 	usedFee, overflow := common.SafeMul(usedGas, uint64(msg.GasPrice()))
 	// 费用消耗溢出，执行失败

plugin/dapp/evm/executor/snapshot_revert_failure_test.go

@@ -0,0 +1,88 @@
+// Copyright Fuzamei Corp. 2018 All Rights Reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package executor
+
+// Executor-level test for the guard in innerExec that turns a failed snapshot revert into a
+// failed transaction. A failed revert leaves the account cache and the receipt describing
+// different state, so the transaction must not be committed.
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/33cn/chain33/common/db"
+	ctypes "github.com/33cn/chain33/types"
+	vcomm "github.com/33cn/plugin/plugin/dapp/evm/executor/vm/common"
+	"github.com/stretchr/testify/require"
+)
+
+// failingRevertKV wraps the production StateDB and fails every RevertSet, standing in for a
+// store fault during the undo of a reverted value transfer.
+type failingRevertKV struct {
+	db.KV
+	err error
+}
+
+func (f failingRevertKV) RevertSet(key, originalValue []byte) error {
+	return f.err
+}
+
+// TestInnerExecFailsWhenSnapshotRevertFails drives the same CALL{value} revert shape as the
+// transfer-revert integration tests, but with a KV whose RevertSet always fails. The inner call
+// is popped by the caller contract, so the outer frame reports success; only the RevertError
+// guard can fail the transaction. The next execution must still succeed: ResetRevertError clears
+// the recorded error at the start of every innerExec call.
+func TestInnerExecFailsWhenSnapshotRevertFails(t *testing.T) {
+	const (
+		topUp = uint64(1000)
+		moved = uint64(400)
+	)
+
+	cfg, st, localDB := productionStateDBFixture(t)
+	revertErr := errors.New("injected RevertSet failure")
+
+	// Wrap the production StateDB so the snapshot revert of the value transfer fails. SetStateDB
+	// also points the coins account at the wrapper, and CheckInit rebuilds the MemoryStateDB.
+	exec := newProductionExec(t, cfg, st, localDB)
+	exec.SetStateDB(failingRevertKV{KV: st, err: revertErr})
+	exec.CheckInit()
+
+	target := vcomm.BytesToAddress([]byte{0xd0})
+	forwarder := deployCode(t, cfg, exec, buildForwarderCode(target, moved))
+	caller := deployCode(t, cfg, exec, buildCallerCode(*vcomm.StringToAddress(forwarder), moved))
+	noop := deployCode(t, cfg, exec, []byte{0x00})
+
+	user := addrFromRole(cfg, roleLegitUser)
+	fundRole(t, exec, cfg, roleLegitUser, int64(topUp)*2)
+	initial := balanceOf(exec, user)
+
+	// First transaction: the forwarder aborts on the value call, the reverted transfer fails to
+	// restore its account keys, and the outer contract continues to success.
+	st.Begin()
+	receipt, err := exec.Exec(makeCallTx(cfg, roleLegitUser, caller, []byte{0x01}, topUp), 0)
+	require.ErrorIs(t, err, revertErr, "innerExec must fail the transaction on a failed snapshot revert")
+	require.Empty(t, receipt.GetKV(), "the failed transaction must carry no state to commit")
+	require.ErrorIs(t, exec.GetMStateDB().RevertError(), revertErr,
+		"the guard must observe the recorded revert error")
+	st.Rollback()
+
+	require.Equal(t, initial, balanceOf(exec, user), "no balance may move out of the user")
+	require.Zero(t, balanceOf(exec, forwarder), "the reverted credit must not be committed")
+	require.Zero(t, balanceOf(exec, target.String()), "the phantom credit must not be committed")
+
+	// Second execution on the same MemoryStateDB (Exec would recreate it through CheckInit).
+	// Nothing reverts here, so a stale error would fail at the guard; ResetRevertError at the
+	// start of innerExec must clear it before execution starts.
+	tx := makeCallTx(cfg, roleLegitUser, noop, nil, 0)
+	msg, err := exec.GetMessage(tx, 0, nil)
+	require.NoError(t, err)
+
+	st.Begin()
+	receipt, err = exec.innerExec(msg, tx.Hash(), tx.GetSignature().GetTy(), 0, msg.GasLimit(), false)
+	require.NoError(t, err, "a failed revert must not poison the next execution")
+	require.Equal(t, int32(ctypes.ExecOk), receipt.GetTy())
+	require.NoError(t, exec.GetMStateDB().RevertError(), "ResetRevertError must clear the recorded error")
+	st.Rollback()
+}

plugin/dapp/evm/executor/transfer_revert_checkkv_test.go

@@ -0,0 +1,194 @@
+// Copyright Fuzamei Corp. 2018 All Rights Reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package executor
+
+// Framework-level tests for reverted value transfers on the production StateDB shape:
+// a concrete executor.StateDB with no Delete, ForkExecRollback open (Begin records every Set
+// key in GetSetKeys) and the receipt/checkKV path that decides whether a transaction is
+// accepted. The two shapes are a pair: a legitimate transaction that survives a reverted call
+// must commit, and the mint shape must not create value.
+
+import (
+	"testing"
+
+	apimock "github.com/33cn/chain33/client/mocks"
+	"github.com/33cn/chain33/common/address"
+	"github.com/33cn/chain33/common/db"
+	cexecutor "github.com/33cn/chain33/executor"
+	"github.com/33cn/chain33/queue"
+	"github.com/33cn/chain33/system/address/btc"
+	ctypes "github.com/33cn/chain33/types"
+	"github.com/33cn/chain33/util"
+	vcomm "github.com/33cn/plugin/plugin/dapp/evm/executor/vm/common"
+	evmtypes "github.com/33cn/plugin/plugin/dapp/evm/types"
+	"github.com/stretchr/testify/require"
+)
+
+// newFrameworkStateDB returns the production StateDB backed by a queue that answers every
+// store lookup with ErrNotFound, so reads of absent keys do not block on a store module.
+func newFrameworkStateDB(t *testing.T, cfg *ctypes.Chain33Config, localDB db.KVDB) *cexecutor.StateDB {
+	t.Helper()
+	q := queue.New("channel")
+	q.SetConfig(cfg)
+
+	cli := q.Client()
+	cli.Sub("store")
+	done := make(chan struct{})
+	go func() {
+		for {
+			select {
+			case msg := <-cli.Recv():
+				if msg == nil {
+					return
+				}
+				msg.Reply(cli.NewMessage("", ctypes.EventStoreGetReply, &ctypes.StoreReplyValue{Values: [][]byte{nil}}))
+			case <-done:
+				return
+			}
+		}
+	}()
+	t.Cleanup(func() {
+		close(done)
+		q.Close()
+	})
+
+	return cexecutor.NewStateDB(cli, nil, localDB, nil).(*cexecutor.StateDB)
+}
+
+// checkReceiptCoversSetKeys mirrors chain33 executor's checkKV: every key recorded by
+// StateDB.Set must also be carried by the receipt.
+func checkReceiptCoversSetKeys(memset []string, kvs []*ctypes.KeyValue) error {
+	keys := make(map[string]struct{}, len(kvs))
+	for _, kv := range kvs {
+		keys[string(kv.GetKey())] = struct{}{}
+	}
+	for _, key := range memset {
+		if _, ok := keys[key]; !ok {
+			return ctypes.ErrNotAllowMemSetKey
+		}
+	}
+	return nil
+}
+
+// newProductionExec builds an EVM executor over the production StateDB with the transfer-revert
+// fork and ForkExecRollback both open at height 0.
+func newProductionExec(t *testing.T, cfg *ctypes.Chain33Config, st *cexecutor.StateDB, localDB db.KVDB) *EVMExecutor {
+	t.Helper()
+	api := new(apimock.QueueProtocolAPI)
+	api.On("GetConfig").Return(cfg)
+	if !evmInitOnce {
+		Init(cfg.ExecName(evmtypes.ExecutorName), cfg, nil)
+		evmInitOnce = true
+	}
+	exec := NewEVMExecutor()
+	exec.SetAPI(api)
+	exec.SetLocalDB(localDB)
+	exec.SetStateDB(st)
+	exec.SetEnv(0, 0, 0)
+	exec.CheckInit()
+	return exec
+}
+
+func restoreBtcAddressDriver(t *testing.T) {
+	t.Helper()
+	// query_test.go replaces the package-wide EVM address driver with the ETH one; the executor
+	// globals were initialized with the BTC driver, so restore it for the tx envelope.
+	driver, err := address.LoadDriver(btc.NormalAddressID, -1)
+	require.NoError(t, err)
+	vcomm.InitEvmAddressDriver(driver)
+}
+
+// buildForwardAndStoreCode calls callee with value (the callee aborts) and then writes slot 0,
+// so the outer frame continues and changes state after the inner value transfer is reverted.
+func buildForwardAndStoreCode(callee vcomm.Address, value uint64) []byte {
+	code := buildCall(callee, value, 0)
+	code = append(code, 0x60, 0x01) // PUSH1 1
+	code = append(code, 0x60, 0x00) // PUSH1 0
+	code = append(code, 0x55)       // SSTORE
+	code = append(code, 0x00)       // STOP
+	return code
+}
+
+func productionStateDBFixture(t *testing.T) (*ctypes.Chain33Config, *cexecutor.StateDB, db.KVDB) {
+	t.Helper()
+	restoreBtcAddressDriver(t)
+	cfg := newTransferRevertConfig(t, 0)
+	// checkKV only sees Set keys when the framework begins a per-tx transaction, which it does
+	// under ForkExecRollback.
+	cfg.SetFork("ForkExecRollback", 0)
+	require.True(t, cfg.IsFork(0, "ForkExecRollback"))
+
+	dbDir, rawDB, localDB := util.CreateTestDB()
+	t.Cleanup(func() { util.CloseTestDB(dbDir, rawDB) })
+	return cfg, newFrameworkStateDB(t, cfg, localDB), localDB
+}
+
+// TestRevertedCallValueTxSucceedsUnderCheckKV is the reviewer's positive case: the outer call
+// succeeds, an inner CALL{value} reverts, execution continues and writes state. Post-fork the
+// transaction must be ExecOk, pass checkKV, and commit exactly the post-revert balances.
+func TestRevertedCallValueTxSucceedsUnderCheckKV(t *testing.T) {
+	const (
+		topUp = uint64(1000)
+		moved = uint64(400)
+	)
+
+	cfg, st, localDB := productionStateDBFixture(t)
+	exec := newProductionExec(t, cfg, st, localDB)
+
+	target := vcomm.BytesToAddress([]byte{0xd0})
+	forwarder := deployCode(t, cfg, exec, buildForwarderCode(target, moved))
+	caller := deployCode(t, cfg, exec, buildForwardAndStoreCode(*vcomm.StringToAddress(forwarder), moved))
+	fundRole(t, exec, cfg, roleLegitUser, int64(topUp)*2)
+	user := addrFromRole(cfg, roleLegitUser)
+	initial := balanceOf(exec, user)
+
+	// Model executor.begin(): StartTx plus the in-tx mode that records Set keys.
+	st.Begin()
+	receipt, err := exec.Exec(makeCallTx(cfg, roleLegitUser, caller, []byte{0x01}, topUp), 0)
+	require.NoError(t, err)
+	require.Equal(t, int32(ctypes.ExecOk), receipt.GetTy())
+	require.NoError(t, checkReceiptCoversSetKeys(st.GetSetKeys(), receipt.GetKV()),
+		"a reverted inner value transfer must not leave keys the receipt lacks")
+
+	applyReceiptKV(t, exec, receipt)
+	require.Equal(t, initial-int64(topUp), balanceOf(exec, user))
+	require.Equal(t, int64(topUp), balanceOf(exec, caller), "the outer transfer survives")
+	require.Zero(t, balanceOf(exec, forwarder), "the reverted transfer must not credit the callee")
+	require.Zero(t, balanceOf(exec, target.String()))
+	require.Equal(t, vcomm.BytesToHash([]byte{0x01}), exec.GetMStateDB().GetState(caller, vcomm.BytesToHash([]byte{0x00})),
+		"execution must continue past the reverted call and commit its state write")
+}
+
+// TestRevertedCallValueMintTxCreatesNoValue keeps the attack shape under the same production
+// StateDB: the phantom credit from a reverted transfer is spent in a later call. The transaction
+// now succeeds (it no longer trips checkKV) but must not mint the reverted amount.
+func TestRevertedCallValueMintTxCreatesNoValue(t *testing.T) {
+	const (
+		topUp = uint64(1000)
+		moved = uint64(400)
+	)
+
+	cfg, st, localDB := productionStateDBFixture(t)
+	exec := newProductionExec(t, cfg, st, localDB)
+
+	target := vcomm.BytesToAddress([]byte{0xd0})
+	forwarder := deployCode(t, cfg, exec, buildForwarderCode(target, moved))
+	caller := deployCode(t, cfg, exec, buildCallerCode(*vcomm.StringToAddress(forwarder), moved))
+	fundRole(t, exec, cfg, roleLegitUser, int64(topUp)*2)
+	user := addrFromRole(cfg, roleLegitUser)
+	initial := balanceOf(exec, user)
+
+	st.Begin()
+	receipt, err := exec.Exec(makeCallTx(cfg, roleLegitUser, caller, []byte{0x01}, topUp), 0)
+	require.NoError(t, err)
+	require.Equal(t, int32(ctypes.ExecOk), receipt.GetTy())
+	require.NoError(t, checkReceiptCoversSetKeys(st.GetSetKeys(), receipt.GetKV()))
+
+	applyReceiptKV(t, exec, receipt)
+	require.Equal(t, initial-int64(topUp), balanceOf(exec, user))
+	require.Equal(t, int64(topUp), balanceOf(exec, caller))
+	require.Zero(t, balanceOf(exec, forwarder), "the reverted credit must not be spendable")
+	require.Zero(t, balanceOf(exec, target.String()), "the mint must not reach the target")
+}