diff --git a/chain33.fork.toml b/chain33.fork.toml index 79c79eece..a855a35a6 100644 --- a/chain33.fork.toml +++ b/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] diff --git a/chain33.para.toml b/chain33.para.toml index 00afc4248..a6c8915cb 100644 --- a/chain33.para.toml +++ b/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 diff --git a/go.mod b/go.mod index 42cf2ce82..4e25a1194 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index ef58f3360..93817c8c5 100644 --- a/go.sum +++ b/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= diff --git a/plugin/dapp/evm/cmd/ci2/chain33.proxyminer.toml b/plugin/dapp/evm/cmd/ci2/chain33.proxyminer.toml index 1316dafbb..316bbd346 100644 --- a/plugin/dapp/evm/cmd/ci2/chain33.proxyminer.toml +++ b/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 diff --git a/plugin/dapp/evm/executor/exec.go b/plugin/dapp/evm/executor/exec.go index 8a57e3721..d51ac1773 100644 --- a/plugin/dapp/evm/executor/exec.go +++ b/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())) // 费用消耗溢出,执行失败 diff --git a/plugin/dapp/evm/executor/snapshot_revert_failure_test.go b/plugin/dapp/evm/executor/snapshot_revert_failure_test.go new file mode 100644 index 000000000..883efb731 --- /dev/null +++ b/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() +} diff --git a/plugin/dapp/evm/executor/transfer_revert_checkkv_test.go b/plugin/dapp/evm/executor/transfer_revert_checkkv_test.go new file mode 100644 index 000000000..bd78f298c --- /dev/null +++ b/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") +} diff --git a/plugin/dapp/evm/executor/transfer_revert_integration_test.go b/plugin/dapp/evm/executor/transfer_revert_integration_test.go new file mode 100644 index 000000000..769e419cb --- /dev/null +++ b/plugin/dapp/evm/executor/transfer_revert_integration_test.go @@ -0,0 +1,186 @@ +// 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 + +// Transaction-level repro of the reverted-transfer mint: A calls B with value, B reverts, then +// the phantom credit B kept in the account cache is spent by a later call in the same tx. The +// committed state is assembled from the receipt KV only, so before ForkEVMTransferRevert the +// transaction mints the moved amount. From the fork on the first transfer is rolled back and +// the later spend has no balance to take. + +import ( + "strings" + "testing" + + "github.com/33cn/chain33/common/address" + "github.com/33cn/chain33/system/address/btc" + ctypes "github.com/33cn/chain33/types" + 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" +) + +func newTransferRevertConfig(t *testing.T, forkHeight int64) *ctypes.Chain33Config { + t.Helper() + cfgStr := strings.Replace(ctypes.GetDefaultCfgstring(), `Title="local"`, `Title="integration-test"`, 1) + if !strings.Contains(cfgStr, "[exec.sub.evm]") { + cfgStr += "\n[exec.sub.evm]\nethMapFromExecutor=\"coins\"\nethMapFromSymbol=\"bty\"\n" + } + cfg := ctypes.NewChain33Config(cfgStr) + require.False(t, cfg.IsLocal()) + cfg.SetDappFork(evmtypes.ExecutorName, evmtypes.ForkEVMTransferRevert, forkHeight) + require.Equal(t, forkHeight, cfg.GetDappFork(evmtypes.ExecutorName, evmtypes.ForkEVMTransferRevert)) + return cfg +} + +func pushUint64(v uint64) []byte { + if v == 0 { + return []byte{0x60, 0x00} + } + var b []byte + for n := v; n > 0; n >>= 8 { + b = append([]byte{byte(n)}, b...) + } + return append([]byte{0x60 + byte(len(b)-1)}, b...) +} + +// buildCall emits CALL(callee, value) with the given calldata size (memory offset 0). +func buildCall(callee vcomm.Address, value, inSize uint64) []byte { + code := pushUint64(0) // retSize + code = append(code, pushUint64(0)...) // retOffset + code = append(code, pushUint64(inSize)...) // inSize + code = append(code, pushUint64(0)...) // inOffset + code = append(code, pushUint64(value)...) // value + code = append(code, 0x73) + code = append(code, callee.Bytes()...) + code = append(code, pushUint64(0xffff)...) + code = append(code, 0xf1) // CALL + code = append(code, 0x50) // POP + return code +} + +// buildCallerCode calls callee twice: first with value (the callee aborts), then with one byte +// of calldata, which makes the callee forward its balance. +func buildCallerCode(callee vcomm.Address, value uint64) []byte { + code := buildCall(callee, value, 0) + code = append(code, buildCall(callee, 0, 1)...) + code = append(code, 0x00) // STOP + return code +} + +// buildForwarderCode aborts when calldata is empty, otherwise CALLs target with value. +func buildForwarderCode(target vcomm.Address, value uint64) []byte { + code := []byte{ + 0x36, // CALLDATASIZE + 0x60, 0x08, // PUSH1 8 + 0x57, // JUMPI + 0xfe, // INVALID + 0x00, 0x00, 0x00, // padding + 0x5b, // JUMPDEST @8 + } + code = append(code, buildCall(target, value, 0)...) + code = append(code, 0x00) // STOP + return code +} + +func makeDeployCodeTx(cfg *ctypes.Chain33Config, roleIdx int, code []byte) *ctypes.Transaction { + execAddr := address.ExecAddress(cfg.ExecName(evmtypes.ExecutorName)) + action := &evmtypes.EVMContractAction{ + Amount: 0, GasLimit: 0, GasPrice: 0, + Code: code, ContractAddr: execAddr, + } + tx := &ctypes.Transaction{ + ChainID: cfg.GetChainID(), + Execer: []byte(cfg.ExecName(evmtypes.ExecutorName)), + Payload: ctypes.Encode(action), + Fee: 1e6, To: execAddr, Nonce: 0, + } + signTx(tx, roleIdx) + return tx +} + +func applyReceiptKV(t *testing.T, exec *EVMExecutor, receipt *ctypes.Receipt) { + t.Helper() + for _, kv := range receipt.GetKV() { + require.NoError(t, exec.GetStateDB().Set(kv.GetKey(), kv.GetValue())) + } +} + +// wrapInit turns runtime code into CREATE init code that returns it. +func wrapInit(runtime []byte) []byte { + code := []byte{0x60, byte(len(runtime))} // PUSH1 len + offsetAt := len(code) + code = append(code, 0x60, 0x00) // PUSH1 codeOffset (patched below) + code = append(code, 0x60, 0x00) // PUSH1 0 (mem offset) + code = append(code, 0x39) // CODECOPY + code = append(code, 0x60, byte(len(runtime))) // PUSH1 return size + code = append(code, 0x60, 0x00) // PUSH1 0 (return offset) + code = append(code, 0xf3) // RETURN + code[offsetAt+1] = byte(len(code)) + return append(code, runtime...) +} + +func deployCode(t *testing.T, cfg *ctypes.Chain33Config, exec *EVMExecutor, code []byte) string { + t.Helper() + tx := makeDeployCodeTx(cfg, roleDeployer, wrapInit(code)) + receipt, err := exec.Exec(tx, 0) + require.NoError(t, err) + require.Equal(t, int32(ctypes.ExecOk), receipt.GetTy()) + applyReceiptKV(t, exec, receipt) + return vcomm.NewContractAddress(*vcomm.StringToAddress(tx.From()), tx.Hash()).String() +} + +func TestRevertedCallValueCannotBeSpent(t *testing.T) { + const ( + topUp = uint64(1000) + moved = uint64(400) + ) + + // Other tests in this package (query_test.go) replace the package-wide EVM address driver + // with the ETH one. The executor globals were initialized with the default BTC driver, so + // restore it or the tx envelope address no longer matches the EVM exec address. + driver, err := address.LoadDriver(btc.NormalAddressID, -1) + require.NoError(t, err) + vcomm.InitEvmAddressDriver(driver) + + for _, tc := range []struct { + name string + forkHeight int64 + minted uint64 + }{ + {"pre-fork mints the reverted value", ctypes.MaxHeight, moved}, + {"post-fork mints nothing", 0, 0}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := newTransferRevertConfig(t, tc.forkHeight) + exec := newTestExecutor(t, cfg, 0) + + target := vcomm.BytesToAddress([]byte{0xd0}) + forwarder := deployCode(t, cfg, exec, buildForwarderCode(target, moved)) + caller := deployCode(t, cfg, exec, buildCallerCode(*vcomm.StringToAddress(forwarder), moved)) + + user := addrFromRole(cfg, roleLegitUser) + fundRole(t, exec, cfg, roleLegitUser, int64(topUp)*2) + + sum := func() int64 { + return balanceOf(exec, user) + balanceOf(exec, caller) + + balanceOf(exec, forwarder) + balanceOf(exec, target.String()) + } + before := sum() + + receipt, err := exec.Exec(makeCallTx(cfg, roleLegitUser, caller, []byte{0x01}, topUp), 0) + require.NoError(t, err) + require.Equal(t, int32(ctypes.ExecOk), receipt.GetTy()) + applyReceiptKV(t, exec, receipt) + + require.Equal(t, before+int64(tc.minted), sum(), "committed total must not grow beyond the minted amount") + require.Equal(t, int64(tc.minted), balanceOf(exec, target.String()), "the forwarder must not spend a phantom credit") + }) + } +} + +func balanceOf(exec *EVMExecutor, addr string) int64 { + return exec.GetMStateDB().CoinsAccount.LoadAccount(addr).Balance +} diff --git a/plugin/dapp/evm/executor/transfer_revert_token_checkkv_test.go b/plugin/dapp/evm/executor/transfer_revert_token_checkkv_test.go new file mode 100644 index 000000000..a5c96cdc8 --- /dev/null +++ b/plugin/dapp/evm/executor/transfer_revert_token_checkkv_test.go @@ -0,0 +1,128 @@ +// 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 + +// TransferToToken goes through the same transferChange revert path as Transfer, so it gets the +// same pair of checks on the production StateDB: a legitimate token transfer that survives a +// reverted one still passes checkKV, and the credit of a reverted transfer cannot be spent. + +import ( + "testing" + + "github.com/33cn/chain33/account" + apimock "github.com/33cn/chain33/client/mocks" + cexecutor "github.com/33cn/chain33/executor" + ctypes "github.com/33cn/chain33/types" + evmstate "github.com/33cn/plugin/plugin/dapp/evm/executor/vm/state" + tokenty "github.com/33cn/plugin/plugin/dapp/token/types" + "github.com/stretchr/testify/require" +) + +const ( + tokenRevertFrom = "14KEKbY3kNFLfQEGJbNweV4whre7NpqzuB" + tokenRevertMiddle = "1PUiGcbsccfxW3zuvHXZBJfznziph5miAo" + tokenRevertSink = "12qyocayNF7Lv6C9qW4avxs2E7U41fKSfv" +) + +type tokenRevertFixture struct { + st *cexecutor.StateDB + mdb *evmstate.MemoryStateDB + tokenDB *account.DB +} + +// newTokenRevertFixture builds a MemoryStateDB over the production StateDB, a created token in +// LocalDB and one funded token account. Funding happens before Begin, as in a real block. +func newTokenRevertFixture(t *testing.T) *tokenRevertFixture { + t.Helper() + cfg, st, localDB := productionStateDBFixture(t) + require.NoError(t, localDB.Set([]byte("LODB-token-create-sto-1-BTY-0"), + ctypes.Encode(&tokenty.LocalToken{Symbol: "BTY", Status: 1}))) + + api := new(apimock.QueueProtocolAPI) + api.On("GetConfig").Return(cfg) + mdb := evmstate.NewMemoryStateDB(st, localDB, nil, 0, api) + + tokenDB, err := account.NewAccountDB(cfg, "token", "BTY", st) + require.NoError(t, err) + acc := tokenDB.LoadAccount(tokenRevertFrom) + acc.Balance = 1000 + tokenDB.SaveAccount(acc) + + return &tokenRevertFixture{st: st, mdb: mdb, tokenDB: tokenDB} +} + +func (f *tokenRevertFixture) balance(t *testing.T, addr string) int64 { + t.Helper() + return f.tokenDB.LoadAccount(addr).Balance +} + +// applyKV commits the receipt KV into the state, completing the checkKV path the way the +// framework does before the transaction is accepted. +func (f *tokenRevertFixture) applyKV(t *testing.T, kvs []*ctypes.KeyValue) { + t.Helper() + for _, kv := range kvs { + require.NoError(t, f.st.Set(kv.GetKey(), kv.GetValue())) + } +} + +func TestTransferToTokenRevertCheckKVPair(t *testing.T) { + t.Run("positive: transfers after a revert pass checkKV", func(t *testing.T) { + f := newTokenRevertFixture(t) + f.st.Begin() + outer := f.mdb.Snapshot() + + ok, err := f.mdb.TransferToToken(tokenRevertFrom, tokenRevertSink, "BTY", 100) + require.NoError(t, err) + require.True(t, ok) + + inner := f.mdb.Snapshot() + ok, err = f.mdb.TransferToToken(tokenRevertFrom, tokenRevertMiddle, "BTY", 400) + require.NoError(t, err) + require.True(t, ok) + f.mdb.RevertToSnapshot(inner) + + // The outer frame keeps executing after the reverted inner transfer. + ok, err = f.mdb.TransferToToken(tokenRevertFrom, tokenRevertSink, "BTY", 50) + require.NoError(t, err) + require.True(t, ok) + + kvs, _ := f.mdb.GetChangedData(outer) + require.NoError(t, checkReceiptCoversSetKeys(f.st.GetSetKeys(), kvs), + "the reverted token transfer must not leave keys the receipt lacks") + f.applyKV(t, kvs) + + require.Equal(t, int64(850), f.balance(t, tokenRevertFrom)) + require.Zero(t, f.balance(t, tokenRevertMiddle), "the reverted transfer must not credit the middle account") + require.Equal(t, int64(150), f.balance(t, tokenRevertSink)) + }) + + t.Run("negative: the reverted credit cannot be spent", func(t *testing.T) { + f := newTokenRevertFixture(t) + f.st.Begin() + outer := f.mdb.Snapshot() + + ok, err := f.mdb.TransferToToken(tokenRevertFrom, tokenRevertSink, "BTY", 100) + require.NoError(t, err) + require.True(t, ok) + + inner := f.mdb.Snapshot() + ok, err = f.mdb.TransferToToken(tokenRevertFrom, tokenRevertMiddle, "BTY", 400) + require.NoError(t, err) + require.True(t, ok) + f.mdb.RevertToSnapshot(inner) + + ok, err = f.mdb.TransferToToken(tokenRevertMiddle, tokenRevertSink, "BTY", 400) + require.False(t, ok, "the phantom credit must not be spendable") + require.ErrorIs(t, err, ctypes.ErrNoBalance) + + kvs, _ := f.mdb.GetChangedData(outer) + require.NoError(t, checkReceiptCoversSetKeys(f.st.GetSetKeys(), kvs)) + f.applyKV(t, kvs) + + require.Equal(t, int64(900), f.balance(t, tokenRevertFrom)) + require.Zero(t, f.balance(t, tokenRevertMiddle)) + require.Equal(t, int64(100), f.balance(t, tokenRevertSink)) + }) +} diff --git a/plugin/dapp/evm/executor/vm/runtime/create_revert_test.go b/plugin/dapp/evm/executor/vm/runtime/create_revert_test.go new file mode 100644 index 000000000..ac6552961 --- /dev/null +++ b/plugin/dapp/evm/executor/vm/runtime/create_revert_test.go @@ -0,0 +1,124 @@ +// 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 runtime + +// A failed nested CREATE used to take its attached value from the caller and keep it at the +// child address whose account was rolled back: Create transferred the value before taking the +// snapshot. From ForkEVMTransferRevert on the snapshot is taken first, so the value returns to +// the caller. + +import ( + "bytes" + "strings" + "testing" + + ctypes "github.com/33cn/chain33/types" + evmtypes "github.com/33cn/plugin/plugin/dapp/evm/types" + "github.com/stretchr/testify/require" +) + +func newCreateRevertEnv(t *testing.T, forkHeight int64) *gapEnv { + t.Helper() + cfgStr := strings.Replace(ctypes.GetDefaultCfgstring(), `Title="local"`, `Title="test"`, 1) + if !strings.Contains(cfgStr, "[exec.sub.evm]") { + cfgStr += "\n[exec.sub.evm]\nethMapFromExecutor=\"coins\"\nethMapFromSymbol=\"bty\"\n" + } + cfg := ctypes.NewChain33Config(cfgStr) + require.False(t, cfg.IsLocal(), "a local config would force every fork to height 0") + cfg.SetDappFork(evmtypes.ExecutorName, evmtypes.ForkEVMTransferRevert, forkHeight) + require.Equal(t, forkHeight, cfg.GetDappFork(evmtypes.ExecutorName, evmtypes.ForkEVMTransferRevert)) + require.Equal(t, forkHeight <= gapTestHeight, + cfg.IsDappFork(gapTestHeight, evmtypes.ExecutorName, evmtypes.ForkEVMTransferRevert)) + return newGapEnvWithCfg(t, cfg, "create-revert") +} + +// buildCreateWithValueCode emits CREATE(value, 0, len(init)) with init appended to the code, +// where init is copied into memory with CODECOPY. init is expected to abort, so the create fails. +func buildCreateWithValueCode(value uint64, init []byte) []byte { + code := []byte{0x60, byte(len(init))} // PUSH1 len(init) + offsetAt := len(code) + code = append(code, 0x60, 0x00) // PUSH1 codeOffset (patched below) + code = append(code, 0x60, 0x00) // PUSH1 0 (mem offset) + code = append(code, 0x39) // CODECOPY + code = append(code, 0x60, byte(len(init))) // PUSH1 size + code = append(code, 0x60, 0x00) // PUSH1 0 (mem offset) + code = append(code, pushUint64(value)...) // PUSH value + code = append(code, 0xf0) // CREATE + code = append(code, 0x00) // STOP + code[offsetAt+1] = byte(len(code)) + return append(code, init...) +} + +func TestFailedNestedCreateReturnsValue(t *testing.T) { + const stash = int64(1000) + const attached = uint64(5) + + for _, tc := range []struct { + name string + forkHeight int64 + wantLeft int64 + }{ + {"pre-fork keeps the old behaviour", ctypes.MaxHeight, stash - int64(attached)}, + {"post-fork returns the value", 0, stash}, + } { + t.Run(tc.name, func(t *testing.T) { + env := newCreateRevertEnv(t, tc.forkHeight) + caller, parent := addr(0x31), addr(0x51) + env.fund(parent, stash) + env.deploy(parent, caller, buildCreateWithValueCode(attached, []byte{0xfe})) + + _, _, _, err := env.evm.Call(AccountRef(caller), parent, nil, 5_000_000, 0) + require.NoError(t, err) + require.Equal(t, tc.wantLeft, env.balance(parent)) + }) + } +} + +// TestSuccessfulCreateWithValueCommitsBalances covers a successful top-level CREATE carrying a +// value. With no snapshot open yet, the pre-fork transfer is dropped by addChange: the balances +// move but the receipt never carries the keys, so chain33's checkKV rejects the transaction (or, +// without ForkExecRollback, commits the value without a receipt entry). From the fork on the +// snapshot opens before the transfer and both keys land in the receipt. +func TestSuccessfulCreateWithValueCommitsBalances(t *testing.T) { + const stash = int64(1000) + const attached = uint64(5) + + for _, tc := range []struct { + name string + forkHeight int64 + wantInReceipt bool + }{ + {"pre-fork drops the balance keys", ctypes.MaxHeight, false}, + {"post-fork commits the balance keys", 0, true}, + } { + t.Run(tc.name, func(t *testing.T) { + env := newCreateRevertEnv(t, tc.forkHeight) + caller, contract := addr(0x61), addr(0x71) + env.fund(caller, stash) + + _, snapshot, _, err := env.evm.Create(AccountRef(caller), contract, []byte{0x00}, 5_000_000, "evm.test", "test", attached) + require.NoError(t, err) + require.Equal(t, stash-int64(attached), env.balance(caller)) + require.Equal(t, int64(attached), env.balance(contract)) + + kvs, _ := env.mdb.GetChangedData(snapshot) + for _, key := range [][]byte{ + env.coins.AccountKey(caller.String()), + env.coins.AccountKey(contract.String()), + } { + require.Equal(t, tc.wantInReceipt, hasKV(kvs, key), "receipt key %x", key) + } + }) + } +} + +func hasKV(kvs []*ctypes.KeyValue, key []byte) bool { + for _, kv := range kvs { + if bytes.Equal(kv.Key, key) { + return true + } + } + return false +} diff --git a/plugin/dapp/evm/executor/vm/runtime/evm.go b/plugin/dapp/evm/executor/vm/runtime/evm.go index 57b112ca8..cb916a6d7 100644 --- a/plugin/dapp/evm/executor/vm/runtime/evm.go +++ b/plugin/dapp/evm/executor/vm/runtime/evm.go @@ -539,8 +539,17 @@ func (evm *EVM) Create(caller ContractRef, contractAddr common.Address, code []b // 向合约地址转账 cfg := evm.StateDB.GetConfig() + // From ForkEVMTransferRevert on, the snapshot is taken before the transfer so that a + // failed create also reverts the attached value (see transferChange.revert). + snapshotBeforeTransfer := cfg.IsDappFork(evm.BlockNumber.Int64(), "evm", evmtypes.ForkEVMTransferRevert) + if snapshotBeforeTransfer { + snapshot = evm.StateDB.Snapshot() + } if cfg.IsDappFork(evm.BlockNumber.Int64(), "evm", evmtypes.ForkEVMFixOverflow) { if value > 0 && !evm.Transfer(evm.StateDB, caller.Address(), contractAddr, value) { + if snapshotBeforeTransfer { + evm.StateDB.RevertToSnapshot(snapshot) + } log.Error("evm create transfer failed", "caller", caller.Address().String(), "contractAddr", contractAddr.String(), "value", value) return nil, -1, gas, model.ErrInsufficientBalance } @@ -563,7 +572,9 @@ func (evm *EVM) Create(caller ContractRef, contractAddr common.Address, code []b contract.SetCallCode(&contractAddr, common.ToHash(code), code) // 创建一个新的账户对象(合约账户) - snapshot = evm.StateDB.Snapshot() + if !snapshotBeforeTransfer { + snapshot = evm.StateDB.Snapshot() + } evm.StateDB.CreateAccount(contractAddr.String(), contract.CallerAddress.String(), execName, alias) if EVMDebugOn == evm.VMConfig.Debug && evm.depth == 0 { diff --git a/plugin/dapp/evm/executor/vm/state/snapshot.go b/plugin/dapp/evm/executor/vm/state/snapshot.go index a84c585b7..8151837a4 100644 --- a/plugin/dapp/evm/executor/vm/state/snapshot.go +++ b/plugin/dapp/evm/executor/vm/state/snapshot.go @@ -5,9 +5,12 @@ package state import ( + "errors" + "fmt" "sort" "github.com/33cn/chain33/common/log/log15" + cexecutor "github.com/33cn/chain33/executor" "github.com/33cn/chain33/types" "github.com/33cn/plugin/plugin/dapp/evm/executor/vm/common" @@ -20,7 +23,7 @@ import ( // 如果合约执行出错,会按生成顺序的倒序,依次调用变更对象的回滚接口进行数据回滚,并同步删除变更对象缓存 // 如果合约执行成功,会按生成顺序的正序,依次调用变更对象的数据和日志变更记录,回传给区块链 type DataChange interface { - revert(mdb *MemoryStateDB) + revert(mdb *MemoryStateDB) error getData(mdb *MemoryStateDB) []*types.KeyValue getLog(mdb *MemoryStateDB) []*types.ReceiptLog } @@ -38,14 +41,27 @@ func (ver *Snapshot) GetID() int { } // 回滚当前版本 -func (ver *Snapshot) revert() bool { +func (ver *Snapshot) revert() error { if ver.entries == nil { - return true + return nil + } + // The interface contract requires undoing changes in reverse creation order. Applying two + // writes to the same slot forward would restore the intermediate value instead of the + // pre-frame one. Legacy behaviour is kept before ForkEVMTransferRevert. + if ver.statedb != nil && ver.statedb.isForkActive(evmtypes.ForkEVMTransferRevert) { + for i := len(ver.entries) - 1; i >= 0; i-- { + if err := ver.entries[i].revert(ver.statedb); err != nil { + return err + } + } + return nil } for _, entry := range ver.entries { - entry.revert(ver.statedb) + if err := entry.revert(ver.statedb); err != nil { + return err + } } - return true + return nil } // 添加变更数据 @@ -144,6 +160,17 @@ type ( amount int64 data []*types.KeyValue logs []*types.ReceiptLog + // preState carries the raw account KV read before the transfer. It is only populated + // when ForkEVMTransferRevert is active; empty keeps the legacy no-op revert. + preState []kvPreState + } + + // kvPreState is the raw value of one account key before a transfer wrote to it. + // existed=false means the key was absent and revert must remove it again. + kvPreState struct { + key []byte + value []byte + existed bool } // 合约生成日志事件 @@ -161,7 +188,8 @@ type ( ) // 在baseChang中定义三个基本操作,子对象中只需要实现必要的操作 -func (ch baseChange) revert(s *MemoryStateDB) { +func (ch baseChange) revert(s *MemoryStateDB) error { + return nil } func (ch baseChange) getData(s *MemoryStateDB) (kvset []*types.KeyValue) { @@ -173,8 +201,9 @@ func (ch baseChange) getLog(s *MemoryStateDB) (logs []*types.ReceiptLog) { } // 创建账户对象的回滚,需要删除缓存中的账户和变更标记 -func (ch createAccountChange) revert(s *MemoryStateDB) { +func (ch createAccountChange) revert(s *MemoryStateDB) error { delete(s.accounts, ch.account) + return nil } // 创建账户对象的数据集 @@ -188,15 +217,16 @@ func (ch createAccountChange) getData(s *MemoryStateDB) (kvset []*types.KeyValue return nil } -func (ch suicideChange) revert(mdb *MemoryStateDB) { +func (ch suicideChange) revert(mdb *MemoryStateDB) error { // 如果已经自杀过了,不处理 if ch.prev { - return + return nil } acc := mdb.accounts[ch.account] if acc != nil { acc.State.Suicided = ch.prev } + return nil } func (ch suicideChange) getData(mdb *MemoryStateDB) []*types.KeyValue { @@ -211,11 +241,12 @@ func (ch suicideChange) getData(mdb *MemoryStateDB) []*types.KeyValue { return nil } -func (ch nonceChange) revert(mdb *MemoryStateDB) { +func (ch nonceChange) revert(mdb *MemoryStateDB) error { acc := mdb.accounts[ch.account] if acc != nil { acc.State.Nonce = ch.prev } + return nil } func (ch nonceChange) getData(mdb *MemoryStateDB) []*types.KeyValue { @@ -227,12 +258,13 @@ func (ch nonceChange) getData(mdb *MemoryStateDB) []*types.KeyValue { return nil } -func (ch codeChange) revert(mdb *MemoryStateDB) { +func (ch codeChange) revert(mdb *MemoryStateDB) error { acc := mdb.accounts[ch.account] if acc != nil { acc.Data.Code = ch.prevcode acc.Data.CodeHash = ch.prevhash } + return nil } func (ch codeChange) getData(mdb *MemoryStateDB) (kvset []*types.KeyValue) { @@ -245,11 +277,12 @@ func (ch codeChange) getData(mdb *MemoryStateDB) (kvset []*types.KeyValue) { return nil } -func (ch abiChange) revert(mdb *MemoryStateDB) { +func (ch abiChange) revert(mdb *MemoryStateDB) error { acc := mdb.accounts[ch.account] if acc != nil { acc.Data.Abi = ch.prevabi } + return nil } func (ch abiChange) getData(mdb *MemoryStateDB) (kvset []*types.KeyValue) { @@ -261,11 +294,12 @@ func (ch abiChange) getData(mdb *MemoryStateDB) (kvset []*types.KeyValue) { return nil } -func (ch storageChange) revert(mdb *MemoryStateDB) { +func (ch storageChange) revert(mdb *MemoryStateDB) error { acc := mdb.accounts[ch.account] if acc != nil { acc.SetState(ch.key, ch.prevalue) } + return nil } func (ch storageChange) getData(mdb *MemoryStateDB) []*types.KeyValue { @@ -289,11 +323,12 @@ func (ch storageChange) getLog(mdb *MemoryStateDB) []*types.ReceiptLog { return nil } -func (ch refundChange) revert(mdb *MemoryStateDB) { +func (ch refundChange) revert(mdb *MemoryStateDB) error { mdb.refund = ch.prev + return nil } -func (ch addLogChange) revert(mdb *MemoryStateDB) { +func (ch addLogChange) revert(mdb *MemoryStateDB) error { logs := mdb.logs[ch.txhash] if len(logs) == 1 { delete(mdb.logs, ch.txhash) @@ -303,6 +338,7 @@ func (ch addLogChange) revert(mdb *MemoryStateDB) { mdb.logSize-- log15.Info("addLogChange::revert", "mdb.logSize", mdb.logSize) + return nil } func (ch addLogChange) getLog(mdb *MemoryStateDB) []*types.ReceiptLog { @@ -314,8 +350,9 @@ func (ch addLogChange) getLog(mdb *MemoryStateDB) []*types.ReceiptLog { return ch.logs } -func (ch addPreimageChange) revert(mdb *MemoryStateDB) { +func (ch addPreimageChange) revert(mdb *MemoryStateDB) error { delete(mdb.preimages, ch.hash) + return nil } func (ch transferChange) getData(mdb *MemoryStateDB) []*types.KeyValue { @@ -324,3 +361,51 @@ func (ch transferChange) getData(mdb *MemoryStateDB) []*types.KeyValue { func (ch transferChange) getLog(mdb *MemoryStateDB) []*types.ReceiptLog { return ch.logs } + +// revert restores the account KV captured before the transfer. Before ForkEVMTransferRevert +// preState is empty, so a reverted transfer keeps its old behaviour and leaves the cache as is. +// +// executor.StateDB, the KV used in production, must undo the write through RevertSet rather +// than Set. Under ForkExecRollback Set records the key in GetSetKeys, and every recorded key +// has to show up in the receipt (chain33's checkKV); a restored write that the surviving +// receipt no longer carries would reject the whole transaction. RevertSet removes the key from +// that list as well. A StateDB without RevertSet (the goleveldb used in some tests) still gets +// the direct writes. +func (ch transferChange) revert(mdb *MemoryStateDB) error { + for _, pre := range ch.preState { + var original []byte + if pre.existed { + original = pre.value + } + if err := mdb.revertAccountKey(pre.key, original); err != nil { + log15.Error("transferChange revert failed", "key", string(pre.key), "err", err) + return err + } + } + return nil +} + +// revertAccountKey undoes one write made by the transfer. original=nil means the key did not +// exist before the transfer and must read as absent afterwards. +func (mdb *MemoryStateDB) revertAccountKey(key, original []byte) error { + if reverter, ok := mdb.StateDB.(interface { + RevertSet(key []byte, originalValue []byte) error + }); ok { + err := reverter.RevertSet(key, original) + if err == nil { + return nil + } + if !errors.Is(err, cexecutor.ErrStateDBRevertWithoutTx) { + return fmt.Errorf("revert state key %x: %w", key, err) + } + // No transaction is open, so there is no key list to repair: write through as the + // legacy path does (the fork that records Set keys is also off). + } + if original == nil { + if deleter, ok := mdb.StateDB.(interface{ Delete([]byte) error }); ok { + return deleter.Delete(key) + } + return mdb.StateDB.Set(key, nil) + } + return mdb.StateDB.Set(key, original) +} diff --git a/plugin/dapp/evm/executor/vm/state/statedb.go b/plugin/dapp/evm/executor/vm/state/statedb.go index cf2775626..2b120d021 100644 --- a/plugin/dapp/evm/executor/vm/state/statedb.go +++ b/plugin/dapp/evm/executor/vm/state/statedb.go @@ -72,6 +72,11 @@ type MemoryStateDB struct { stateDirty map[string]interface{} dataDirty map[string]interface{} api client.QueueProtocolAPI + + // revertErr records the first error a snapshot revert hit in the current transaction. + // RevertToSnapshot cannot return it to the EVM (its callers are already unwinding an + // error), so innerExec checks RevertError before accepting a successful execution. + revertErr error } // NewMemoryStateDB 基于执行器框架的三个DB构建内存状态机对象 @@ -400,7 +405,10 @@ func (mdb *MemoryStateDB) RevertToSnapshot(version int) { // 从最近版本开始回滚 for index := len(mdb.snapshots) - 1; index >= version; index-- { - mdb.snapshots[index].revert() + if err := mdb.snapshots[index].revert(); err != nil && mdb.revertErr == nil { + log15.Error("RevertToSnapshot failed", "snapshot", index, "err", err) + mdb.revertErr = err + } } // 只保留回滚版本之前的版本数据 @@ -414,6 +422,18 @@ func (mdb *MemoryStateDB) RevertToSnapshot(version int) { } +// RevertError returns the first error a snapshot revert recorded in the current transaction, +// or nil when every revert succeeded. innerExec turns a non-nil value into a failed +// transaction: the account cache and the receipt no longer describe the same state. +func (mdb *MemoryStateDB) RevertError() error { + return mdb.revertErr +} + +// ResetRevertError clears the recorded revert error at the start of an execution. +func (mdb *MemoryStateDB) ResetRevertError() { + mdb.revertErr = nil +} + // Snapshot 对当前的数据状态打快照,并生成快照版本号,方便后面回滚数据 func (mdb *MemoryStateDB) Snapshot() int { id := mdb.versionID @@ -524,6 +544,63 @@ const ( Error ) +// isForkActive reports whether fork is active at the current block height. +func (mdb *MemoryStateDB) isForkActive(fork string) bool { + if mdb.api == nil { + return false + } + cfg := mdb.api.GetConfig() + if cfg == nil { + return false + } + return cfg.IsDappFork(mdb.blockHeight, evmtypes.ExecutorName, fork) +} + +// transferPreState captures the raw account KV touched by Transfer, keyed the same way the +// coins account DB stores them. +func (mdb *MemoryStateDB) transferPreState(sender, recipient string) ([]kvPreState, error) { + conf := types.ConfSub(mdb.api.GetConfig(), evmtypes.ExecutorName) + ethMapFromExecutor := conf.GStr("ethMapFromExecutor") + if bytes.Equal(types.GetRealExecName([]byte(ethMapFromExecutor)), []byte("coins")) { + return mdb.capturePreState([][]byte{ + mdb.CoinsAccount.AccountKey(sender), + mdb.CoinsAccount.AccountKey(recipient), + }) + } + // paracross: balances live in the exec account of the mapped executor + return mdb.capturePreState([][]byte{ + mdb.execAccountKey(sender), + mdb.execAccountKey(recipient), + }) +} + +func (mdb *MemoryStateDB) execAccountKey(addr string) []byte { + return mdb.CoinsAccount.GetExecKVSet(mdb.evmPlatformAddr, &types.Account{Addr: addr})[0].Key +} + +// capturePreState reads the raw values behind keys before a transfer overwrites them. Only +// ErrNotFound means the key did not exist: any other error leaves us without a value to +// restore, so the transfer must fail instead of recording a state that was never read. +func (mdb *MemoryStateDB) capturePreState(keys [][]byte) ([]kvPreState, error) { + states := make([]kvPreState, 0, len(keys)) + for _, key := range keys { + pre := kvPreState{key: append([]byte(nil), key...)} + value, err := mdb.StateDB.Get(key) + switch { + case err == nil: + pre.value = append([]byte(nil), value...) + pre.existed = true + case errors.Is(err, types.ErrNotFound): + // The key did not exist yet; revert must make it absent again. + default: + log15.Error("capturePreState failed", "key", string(key), "err", err) + return nil, fmt.Errorf("read state key %x before transfer: %w", key, err) + } + states = append(states, pre) + } + return states, nil +} + // Transfer 借助coins执行器进行转账相关操作 func (mdb *MemoryStateDB) Transfer(sender, recipient string, amount uint64) bool { log15.Debug("transfer from contract to external(contract)", "sender", sender, "recipient", recipient, "amount", amount) @@ -550,6 +627,15 @@ func (mdb *MemoryStateDB) Transfer(sender, recipient string, amount uint64) bool conf := types.ConfSub(mdb.api.GetConfig(), evmtypes.ExecutorName) ethMapFromExecutor := conf.GStr("ethMapFromExecutor") + var preState []kvPreState + if mdb.isForkActive(evmtypes.ForkEVMTransferRevert) { + preState, err = mdb.transferPreState(sender, recipient) + if err != nil { + log15.Error("transfer capture pre-state failed", "sender", sender, "recipient", recipient, + "amount", amount, "err", err) + return false + } + } if bytes.Equal(types.GetRealExecName([]byte(ethMapFromExecutor)), []byte("coins")) { ret, err = mdb.CoinsAccount.Transfer(sender, recipient, value) } else { //paracross @@ -567,6 +653,7 @@ func (mdb *MemoryStateDB) Transfer(sender, recipient string, amount uint64) bool amount: value, data: ret.KV, logs: ret.Logs, + preState: preState, }) } @@ -604,6 +691,19 @@ func (mdb *MemoryStateDB) TransferToToken(from, recipient, symbol string, amount if recipient == execaddress { return false, errors.New("not allow") } + // Token accounts use the same non-exec account keys as tokendb.Transfer. + var preState []kvPreState + if mdb.isForkActive(evmtypes.ForkEVMTransferRevert) { + preState, err = mdb.capturePreState([][]byte{ + tokendb.AccountKey(from), + tokendb.AccountKey(recipient), + }) + if err != nil { + log15.Error("TransferToToken capture pre-state failed", "from", from, "recipient", recipient, + "symbol", symbol, "amount", amount, "err", err) + return false, err + } + } receipt, err := tokendb.Transfer(from, recipient, amount) if err != nil { return false, err @@ -614,6 +714,7 @@ func (mdb *MemoryStateDB) TransferToToken(from, recipient, symbol string, amount amount: amount, data: receipt.GetKV(), logs: receipt.GetLogs(), + preState: preState, }) return true, nil diff --git a/plugin/dapp/evm/executor/vm/state/transfer_revert_test.go b/plugin/dapp/evm/executor/vm/state/transfer_revert_test.go new file mode 100644 index 000000000..cfb1f8529 --- /dev/null +++ b/plugin/dapp/evm/executor/vm/state/transfer_revert_test.go @@ -0,0 +1,269 @@ +// 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 state + +import ( + "bytes" + "errors" + "strings" + "testing" + + "github.com/33cn/chain33/account" + 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" + ctypes "github.com/33cn/chain33/types" + "github.com/33cn/chain33/util" + "github.com/33cn/plugin/plugin/dapp/evm/executor/vm/common" + evmtypes "github.com/33cn/plugin/plugin/dapp/evm/types" + "github.com/stretchr/testify/require" +) + +const transferRevertTestHeight = int64(10) + +// newTransferRevertConfig builds an EVM config whose ForkEVMTransferRevert resolves to +// forkHeight. The config must not be local: Title="local" pins every dapp fork to 0 via +// needSetForkZero, which would make the pre-fork side unreachable. +func newTransferRevertConfig(t *testing.T, forkHeight int64) *ctypes.Chain33Config { + t.Helper() + cfgStr := strings.Replace(ctypes.GetDefaultCfgstring(), `Title="local"`, `Title="evm-transfer-revert-test"`, 1) + if !strings.Contains(cfgStr, "[exec.sub.evm]") { + cfgStr += "\n[exec.sub.evm]\nethMapFromExecutor=\"coins\"\nethMapFromSymbol=\"bty\"\n" + } + cfg := ctypes.NewChain33Config(cfgStr) + require.False(t, cfg.IsLocal()) + cfg.SetDappFork(evmtypes.ExecutorName, evmtypes.ForkEVMTransferRevert, forkHeight) + require.Equal(t, forkHeight, cfg.GetDappFork(evmtypes.ExecutorName, evmtypes.ForkEVMTransferRevert)) + return cfg +} + +// newTransferRevertMDBWith builds a MemoryStateDB over kv, so tests can pick a KV without +// Delete to reach transferChange.revert's fallback branch. +func newTransferRevertMDBWith(t *testing.T, cfg *ctypes.Chain33Config, kv db.KV) *MemoryStateDB { + t.Helper() + api := new(apimock.QueueProtocolAPI) + api.On("GetConfig").Return(cfg) + coinsAccount, err := account.NewAccountDB(cfg, "coins", cfg.GetCoinSymbol(), kv) + require.NoError(t, err) + + mdb := NewMemoryStateDB(kv, nil, coinsAccount, transferRevertTestHeight, api) + mdb.evmPlatformAddr = address.ExecAddress(cfg.ExecName(evmtypes.ExecutorName)) + return mdb +} + +func newTransferRevertMDB(t *testing.T, forkHeight int64) *MemoryStateDB { + t.Helper() + cfg := newTransferRevertConfig(t, forkHeight) + dbDir, stateDB, localDB := util.CreateTestDB() + t.Cleanup(func() { util.CloseTestDB(dbDir, stateDB) }) + + mdb := newTransferRevertMDBWith(t, cfg, stateDB) + mdb.LocalDB = localDB + return mdb +} + +func fundCoinAccount(mdb *MemoryStateDB, addr string, amount int64) { + acc := mdb.CoinsAccount.LoadAccount(addr) + acc.Balance = amount + mdb.CoinsAccount.SaveAccount(acc) +} + +func coinBalance(mdb *MemoryStateDB, addr string) int64 { + return mdb.CoinsAccount.LoadAccount(addr).Balance +} + +// TestTransferRevertRestoresBalances pins both sides of ForkEVMTransferRevert: after the fork a +// reverted transfer leaves both balances untouched, before it the cache keeps the transfer. +func TestTransferRevertRestoresBalances(t *testing.T) { + const ( + sender = "14KEKbY3kNFLfQEGJbNweV4whre7NpqzuB" + recipient = "1PUiGcbsccfxW3zuvHXZBJfznziph5miAo" + initial = int64(1000) + moved = uint64(400) + ) + + t.Run("pre-fork keeps the old behaviour", func(t *testing.T) { + mdb := newTransferRevertMDB(t, ctypes.MaxHeight) + fundCoinAccount(mdb, sender, initial) + + snapshot := mdb.Snapshot() + require.True(t, mdb.Transfer(sender, recipient, moved)) + require.Equal(t, initial-int64(moved), coinBalance(mdb, sender)) + require.Equal(t, int64(moved), coinBalance(mdb, recipient)) + + mdb.RevertToSnapshot(snapshot) + require.Equal(t, initial-int64(moved), coinBalance(mdb, sender), "pre-fork revert must not restore the sender") + require.Equal(t, int64(moved), coinBalance(mdb, recipient), "pre-fork revert must leave the phantom credit") + }) + + t.Run("post-fork restores both balances", func(t *testing.T) { + mdb := newTransferRevertMDB(t, 0) + fundCoinAccount(mdb, sender, initial) + + snapshot := mdb.Snapshot() + require.True(t, mdb.Transfer(sender, recipient, moved)) + require.Equal(t, initial-int64(moved), coinBalance(mdb, sender)) + require.Equal(t, int64(moved), coinBalance(mdb, recipient)) + + mdb.RevertToSnapshot(snapshot) + require.Equal(t, initial, coinBalance(mdb, sender)) + require.Zero(t, coinBalance(mdb, recipient), "recipient key did not exist before the transfer and must be removed") + }) +} + +// TestSnapshotRevertSameSlotReverseOrder pins the reverse-order revert: two SSTOREs to the +// same slot followed by a revert must restore the pre-frame value, not the intermediate one. +func TestSnapshotRevertSameSlotReverseOrder(t *testing.T) { + contract := common.BytesToAddress([]byte{0x42}) + slot := common.BytesToHash([]byte("slot")) + pre := common.BytesToHash([]byte("pre")) + first := common.BytesToHash([]byte("first")) + second := common.BytesToHash([]byte("second")) + + t.Run("pre-fork restores the intermediate value", func(t *testing.T) { + mdb := newTransferRevertMDB(t, ctypes.MaxHeight) + mdb.CreateAccount(contract.String(), "creator", "evm.test", "test") + mdb.SetState(contract.String(), slot, pre) + + snapshot := mdb.Snapshot() + mdb.SetState(contract.String(), slot, first) + mdb.SetState(contract.String(), slot, second) + require.Equal(t, second, mdb.GetState(contract.String(), slot)) + + mdb.RevertToSnapshot(snapshot) + require.Equal(t, first, mdb.GetState(contract.String(), slot), "forward revert restores the intermediate write") + }) + + t.Run("post-fork restores the pre-frame value", func(t *testing.T) { + mdb := newTransferRevertMDB(t, 0) + mdb.CreateAccount(contract.String(), "creator", "evm.test", "test") + mdb.SetState(contract.String(), slot, pre) + + snapshot := mdb.Snapshot() + mdb.SetState(contract.String(), slot, first) + mdb.SetState(contract.String(), slot, second) + require.Equal(t, second, mdb.GetState(contract.String(), slot)) + + mdb.RevertToSnapshot(snapshot) + require.Equal(t, pre, mdb.GetState(contract.String(), slot), "reverse revert restores the pre-frame value") + }) +} + +// TestTransferRevertWithoutDeleteHidesKey covers transferChange.revert's fallback when the KV +// has no transaction open: executor.StateDB exposes RevertSet, but outside Begin there is no +// key list to repair, so revert writes directly. A key that did not exist before the transfer +// is then shadowed with a nil value, which Get reports as absent. The goleveldb used by the +// other tests has Delete and never reaches this branch. +func TestTransferRevertWithoutDeleteHidesKey(t *testing.T) { + const ( + sender = "14KEKbY3kNFLfQEGJbNweV4whre7NpqzuB" + recipient = "1PUiGcbsccfxW3zuvHXZBJfznziph5miAo" + initial = int64(1000) + moved = uint64(400) + ) + + st := cexecutor.NewStateDB(nil, nil, nil, nil).(*cexecutor.StateDB) + _, hasDelete := interface{}(st).(interface{ Delete([]byte) error }) + require.False(t, hasDelete, "executor.StateDB must not expose Delete or the fallback stays untested") + _, hasRevertSet := interface{}(st).(interface{ RevertSet([]byte, []byte) error }) + require.True(t, hasRevertSet, "executor.StateDB should undo writes through RevertSet inside a tx") + + mdb := newTransferRevertMDBWith(t, newTransferRevertConfig(t, 0), st) + fundCoinAccount(mdb, sender, initial) + recipientKey := mdb.CoinsAccount.AccountKey(recipient) + + snapshot := mdb.Snapshot() + require.True(t, mdb.Transfer(sender, recipient, moved)) + _, err := mdb.StateDB.Get(recipientKey) + require.NoError(t, err, "the transfer must have written the recipient key") + + mdb.RevertToSnapshot(snapshot) + _, err = mdb.StateDB.Get(recipientKey) + require.Equal(t, ctypes.ErrNotFound, err, "the nil Set must shadow the key as absent") + require.Equal(t, initial, coinBalance(mdb, sender)) +} + +// faultyKV wraps a KV and fails the read of one key, standing in for a store timeout. +type faultyKV struct { + db.KV + failKey []byte + err error +} + +func (f faultyKV) Get(key []byte) ([]byte, error) { + if bytes.Equal(key, f.failKey) { + return nil, f.err + } + return f.KV.Get(key) +} + +// revertFailingKV is a KV whose RevertSet always fails, so the error path of the producer can +// be observed without a real storage fault. +type revertFailingKV struct { + db.KV + err error +} + +func (f revertFailingKV) RevertSet(key, originalValue []byte) error { + return f.err +} + +// TestRevertErrorIsRecorded pins the propagation of a failed RevertSet: the error reaches the +// MemoryStateDB instead of being swallowed, and the next execution can clear it. +func TestRevertErrorIsRecorded(t *testing.T) { + const ( + sender = "14KEKbY3kNFLfQEGJbNweV4whre7NpqzuB" + recipient = "1PUiGcbsccfxW3zuvHXZBJfznziph5miAo" + initial = int64(1000) + ) + + dbDir, stateDB, _ := util.CreateTestDB() + t.Cleanup(func() { util.CloseTestDB(dbDir, stateDB) }) + revertErr := errors.New("revert failed") + mdb := newTransferRevertMDBWith(t, newTransferRevertConfig(t, 0), revertFailingKV{KV: stateDB, err: revertErr}) + fundCoinAccount(mdb, sender, initial) + + snapshot := mdb.Snapshot() + require.True(t, mdb.Transfer(sender, recipient, 400)) + mdb.RevertToSnapshot(snapshot) + require.ErrorIs(t, mdb.RevertError(), revertErr, "the producer error must reach the caller") + + mdb.ResetRevertError() + require.NoError(t, mdb.RevertError()) +} + +// TestCapturePreStateFailsTransferOnReadError pins the reviewer's Suggestion: only ErrNotFound +// may mean "the key did not exist". Any other read error leaves the pre-transfer value unknown, +// so the transfer must fail instead of recording a state that was never read. +func TestCapturePreStateFailsTransferOnReadError(t *testing.T) { + const ( + sender = "14KEKbY3kNFLfQEGJbNweV4whre7NpqzuB" + recipient = "1PUiGcbsccfxW3zuvHXZBJfznziph5miAo" + initial = int64(1000) + moved = uint64(400) + ) + + cfg := newTransferRevertConfig(t, 0) + dbDir, stateDB, _ := util.CreateTestDB() + t.Cleanup(func() { util.CloseTestDB(dbDir, stateDB) }) + + // The key is built from the same config the coins DB uses. + coinsAccount, err := account.NewAccountDB(cfg, "coins", cfg.GetCoinSymbol(), stateDB) + require.NoError(t, err) + storeErr := errors.New("store read timeout") + mdb := newTransferRevertMDBWith(t, cfg, faultyKV{ + KV: stateDB, + failKey: coinsAccount.AccountKey(recipient), + err: storeErr, + }) + fundCoinAccount(mdb, sender, initial) + + snapshot := mdb.Snapshot() + require.False(t, mdb.Transfer(sender, recipient, moved), "a pre-state read error must fail the transfer") + mdb.RevertToSnapshot(snapshot) + require.Equal(t, initial, coinBalance(mdb, sender)) + require.Zero(t, coinBalance(mdb, recipient)) +} diff --git a/plugin/dapp/evm/types/evm.go b/plugin/dapp/evm/types/evm.go index 69815486e..8b43b653c 100644 --- a/plugin/dapp/evm/types/evm.go +++ b/plugin/dapp/evm/types/evm.go @@ -50,7 +50,11 @@ func InitFork(cfg *types.Chain33Config) { cfg.RegisterDappFork(ExecutorName, ForkEVMFixOverflow, 0) // SELFDESTRUCT payer fix: DELEGATECALL/CALLCODE no longer spend the borrowed code's balance cfg.RegisterDappFork(ExecutorName, ForkEVMSuicidePayer, 0) - + // Revertible value transfers are opt-in per chain. Default to MaxHeight (off) because + // registering 0 would enable the consensus change from genesis on every chain that does + // not configure the key, rewriting historical execution. A chain that wants the fix sets + // a concrete height under [fork.sub.evm]; initForkConfig overrides this default. + cfg.RegisterDappFork(ExecutorName, ForkEVMTransferRevert, types.MaxHeight) } // InitExecutor ... diff --git a/plugin/dapp/evm/types/types.go b/plugin/dapp/evm/types/types.go index efd7d98dd..d831621b8 100644 --- a/plugin/dapp/evm/types/types.go +++ b/plugin/dapp/evm/types/types.go @@ -60,6 +60,12 @@ const ( // ForkEVMSuicidePayer SELFDESTRUCT pays from the executing account (Address()) instead of // CodeAddr, so that reading the balance, debiting it and suiciding all use the same account ForkEVMSuicidePayer = "ForkEVMSuicidePayer" + // ForkEVMTransferRevert makes value transfers revertible: transferChange restores the + // pre-transfer account KV, a failed CREATE rolls back its value, and Snapshot.revert + // unwinds entries in reverse order. Without it a reverted transfer stays in the account + // cache while its receipt KV is dropped, so a later call in the same tx can spend the + // phantom balance. Registered in InitFork at MaxHeight, so chains must opt in explicitly. + ForkEVMTransferRevert = "ForkEVMTransferRevert" ) var (