From 4300682947271e377983fb5216bfef557c3a2962 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 ad9969a5fe..38e2fb44d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (rpc) [#536](https://github.com/crypto-org-chain/ethermint/pull/536) Fix validate basic after transaction conversion with raw field. * (cli) [#537](https://github.com/crypto-org-chain/ethermint/pull/537) Fix unsuppored sign mode SIGN_MODE_TEXTUAL for bank transfer. * (cli) [#543](https://github.com/crypto-org-chain/ethermint/pull/543) Fix graceful shutdown. +* (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 1345ee682a..e43a490b55 100644 --- a/tests/integration_tests/configs/default.jsonnet +++ b/tests/integration_tests/configs/default.jsonnet @@ -88,6 +88,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 eedb0f031d..ea1c051641 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} @@ -459,6 +458,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 ecb2b3ee21..3b4906246e 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -314,41 +314,36 @@ func (k *Keeper) ApplyMessageWithConfig( return nil, errorsmod.Wrap(types.ErrCallDisabled, "failed to call contract") } + stateDB := statedb.NewWithParams(ctx, k, cfg.TxConfig, cfg.Params.EvmDenom) + 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, sdkmath.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, sdkmath.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.EvmDenom) - 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) - rules := cfg.Rules contractCreation := msg.To == nil intrinsicGas, err := k.GetEthIntrinsicGas(msg, rules, contractCreation) diff --git a/x/evm/keeper/statedb.go b/x/evm/keeper/statedb.go index e4e6f0d9ae..e718de0148 100644 --- a/x/evm/keeper/statedb.go +++ b/x/evm/keeper/statedb.go @@ -138,20 +138,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 b5f3caf027..ab4f7ac414 100644 --- a/x/evm/statedb/statedb.go +++ b/x/evm/statedb/statedb.go @@ -637,6 +637,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 {