From bb8f9c2f8d3ec0ce74f585197a459c174d742bf7 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 22 Oct 2024 10:56:36 +0800 Subject: [PATCH] Problem: state overwrite not work in debug trace API (#545) * Problem: state overwrite not work in debug trace API * test * fix --- CHANGELOG.md | 1 + .../integration_tests/configs/default.jsonnet | 1 + tests/integration_tests/test_tracers.py | 97 ++++++++++++++++++- x/evm/keeper/state_transition.go | 33 +++---- x/evm/keeper/statedb.go | 14 --- x/evm/statedb/statedb.go | 4 + 6 files changed, 115 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 777d8459ba..d0e57d8ded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (rpc) [#488](https://github.com/crypto-org-chain/ethermint/pull/488) Fix handling of pending transactions related APIs. * (rpc) [#521](https://github.com/crypto-org-chain/ethermint/pull/521) Align hash and miner in subscribe newHeads with eth_getBlockByNumber. * (rpc) [#527](https://github.com/crypto-org-chain/ethermint/pull/527) Fix balance consistency between trace-block and state machine. +* (rpc) [#545](https://github.com/crypto-org-chain/ethermint/pull/545) Fix state overwrite in debug trace APIs. ### Improvements diff --git a/tests/integration_tests/configs/default.jsonnet b/tests/integration_tests/configs/default.jsonnet index 6a7e2a81af..0ab747e47d 100644 --- a/tests/integration_tests/configs/default.jsonnet +++ b/tests/integration_tests/configs/default.jsonnet @@ -78,6 +78,7 @@ params: { no_base_fee: false, base_fee: '100000000000', + min_gas_multiplier: '0.5', }, }, }, diff --git a/tests/integration_tests/test_tracers.py b/tests/integration_tests/test_tracers.py index 80a5b8800b..9b1281491b 100644 --- a/tests/integration_tests/test_tracers.py +++ b/tests/integration_tests/test_tracers.py @@ -113,8 +113,7 @@ def process(w3): assert (res[0] == res[-1] == EXPECTED_CONTRACT_CREATE_TRACER), res -def fund_acc(w3, acc): - fund = 3000000000000000000 +def fund_acc(w3, acc, fund=3000000000000000000): addr = acc.address if w3.eth.get_balance(addr, "latest") == 0: tx = {"to": addr, "value": fund, "gasPrice": w3.eth.gas_price} @@ -445,6 +444,100 @@ def process(w3): assert (res[0] == res[-1] == balance), res +def test_refund_unused_gas_when_contract_tx_reverted(ethermint): + w3 = ethermint.w3 + test_revert, _ = deploy_contract(w3, CONTRACTS["TestRevert"]) + gas = 1000000 + gas_price = 6060000000000 + acc = derive_new_account(10) + fund_acc(w3, acc, fund=10000000000000000000) + p = ethermint.cosmos_cli().get_params("feemarket")["params"] + min_gas_multiplier = float(p["min_gas_multiplier"]) + sender = acc.address.lower() + tx_res = w3.provider.make_request( + "debug_traceCall", + [ + { + "value": "0x0", + "to": test_revert.address, + "from": sender, + "data": "0x9ffb86a5", + "gas": hex(gas), + "gasPrice": hex(gas_price), + }, + "latest", + { + "tracer": "prestateTracer", + "tracerConfig": { + "diffMode": True, + }, + }, + ], + ) + assert "result" in tx_res + tx_res = tx_res["result"] + pre = int(tx_res["pre"][sender]["balance"], 16) + post = int(tx_res["post"][sender]["balance"], 16) + diff = pre - gas * gas_price * min_gas_multiplier - post + assert diff == 0, diff + + pre = w3.eth.get_balance(acc.address) + receipt = send_transaction( + w3, + test_revert.functions.revertWithMsg().build_transaction( + { + "gas": gas, + "gasPrice": gas_price, + } + ), + key=acc.key, + ) + assert receipt["status"] == 0, receipt["status"] + post = w3.eth.get_balance(acc.address) + diff = pre - gas * gas_price * min_gas_multiplier - post + assert diff == 0, diff + + +def test_refund_unused_gas_when_contract_tx_reverted_state_overrides(ethermint): + w3 = ethermint.w3 + test_revert, _ = deploy_contract(w3, CONTRACTS["TestRevert"]) + gas = 21000 + gas_price = 6060000000000 + acc = derive_new_account(10) + fund_acc(w3, acc, fund=10000000000000000000) + sender = acc.address.lower() + balance = 10000000000000000000000 + nonce = 1000 + tx_res = w3.provider.make_request( + "debug_traceCall", + [ + { + "value": "0x1", + "to": test_revert.address, + "from": sender, + "gas": hex(gas), + "gasPrice": hex(gas_price), + }, + "latest", + { + "tracer": "prestateTracer", + "stateOverrides": { + sender: { + "balance": hex(balance), + "nonce": hex(nonce), + } + } + }, + ], + ) + assert "result" in tx_res + tx_res = tx_res["result"] + balance_af = int(tx_res[sender]["balance"], 16) + nonce_af = tx_res[sender]["nonce"] + assert balance_af == balance, balance_af + assert nonce_af == nonce, nonce_af + + def test_debug_tracecall_return_revert_data_when_call_failed(ethermint, geth): expected = "08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001a46756e6374696f6e20686173206265656e207265766572746564000000000000" # noqa: E501 diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index 3ba3bad245..71fb73490e 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -355,41 +355,36 @@ func (k *Keeper) ApplyMessageWithConfig( return nil, errorsmod.Wrap(types.ErrCallDisabled, "failed to call contract") } + stateDB := statedb.NewWithParams(ctx, k, cfg.TxConfig, cfg.Params) + var evm *vm.EVM + if cfg.Overrides != nil { + if err := cfg.Overrides.Apply(stateDB); err != nil { + return nil, errorsmod.Wrap(err, "failed to apply state override") + } + } + evm = k.NewEVM(ctx, msg, cfg, stateDB) // Allow the tracer captures the tx level events, mainly the gas consumption. leftoverGas := msg.GasLimit - senderAddr := sdk.AccAddress(msg.From.Bytes()) + sender := vm.AccountRef(msg.From) tracer := cfg.GetTracer() if tracer != nil { if cfg.DebugTrace { - // msg.GasPrice should have been set to effective gas price amount := new(big.Int).Mul(msg.GasPrice, new(big.Int).SetUint64(msg.GasLimit)) - if err := k.SubBalance(ctx, senderAddr, sdk.NewCoins(sdk.NewCoin(cfg.Params.EvmDenom, sdk.NewIntFromBigInt(amount)))); err != nil { - return nil, errorsmod.Wrap(err, "failed to subtract balance") - } - if err := k.incrNonce(ctx, senderAddr); err != nil { - return nil, errorsmod.Wrap(err, "failed to increment nonce") + stateDB.SubBalance(sender.Address(), amount) + if err := stateDB.Error(); err != nil { + return nil, err } + stateDB.SetNonce(sender.Address(), stateDB.GetNonce(sender.Address())+1) } tracer.CaptureTxStart(leftoverGas) defer func() { if cfg.DebugTrace { - amount := new(big.Int).Mul(msg.GasPrice, new(big.Int).SetUint64(leftoverGas)) - _ = k.AddBalance(ctx, senderAddr, sdk.NewCoins(sdk.NewCoin(cfg.Params.EvmDenom, sdk.NewIntFromBigInt(amount)))) + stateDB.AddBalance(sender.Address(), new(big.Int).Mul(msg.GasPrice, new(big.Int).SetUint64(leftoverGas))) } tracer.CaptureTxEnd(leftoverGas) }() } - stateDB := statedb.NewWithParams(ctx, k, cfg.TxConfig, cfg.Params) - var evm *vm.EVM - if cfg.Overrides != nil { - if err := cfg.Overrides.Apply(stateDB); err != nil { - return nil, errorsmod.Wrap(err, "failed to apply state override") - } - } - evm = k.NewEVM(ctx, msg, cfg, stateDB) - sender := vm.AccountRef(msg.From) - isLondon := cfg.ChainConfig.IsLondon(evm.Context.BlockNumber) contractCreation := msg.To == nil intrinsicGas, err := k.GetEthIntrinsicGas(ctx, msg, cfg.ChainConfig, contractCreation) diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index 04bc5d87e0..d414bd193d 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -133,20 +133,6 @@ func (k *Keeper) SetAccount(ctx sdk.Context, addr common.Address, account stated return nil } -func (k *Keeper) incrNonce(ctx sdk.Context, addr sdk.AccAddress) error { - acct := k.accountKeeper.GetAccount(ctx, addr) - if acct == nil { - acct = k.accountKeeper.NewAccountWithAddress(ctx, addr) - } - - if err := acct.SetSequence(acct.GetSequence() + 1); err != nil { - return err - } - - k.accountKeeper.SetAccount(ctx, acct) - return nil -} - // SetState update contract storage, delete if value is empty. func (k *Keeper) SetState(ctx sdk.Context, addr common.Address, key common.Hash, value []byte) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.AddressStoragePrefix(addr)) diff --git a/x/evm/statedb/statedb.go b/x/evm/statedb/statedb.go index f210523b4f..a5897f801a 100644 --- a/x/evm/statedb/statedb.go +++ b/x/evm/statedb/statedb.go @@ -590,6 +590,10 @@ func (s *StateDB) RevertToSnapshot(revid int) { s.validRevisions = s.validRevisions[:idx] } +func (s *StateDB) Error() error { + return s.err +} + // Commit writes the dirty states to keeper // the StateDB object should be discarded after committed. func (s *StateDB) Commit() error {