Problem: state overwrite not work in debug trace API (backport: #545)

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
 

tests/integration_tests/configs/default.jsonnet

@@ -78,6 +78,7 @@
           params: {
             no_base_fee: false,
             base_fee: '100000000000',
+            min_gas_multiplier: '0.5',
           },
         },
       },

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
 

x/evm/keeper/state_transition.go

@@ -355,41 +355,36 @@
 		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)

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))

x/evm/statedb/statedb.go

@@ -590,6 +590,10 @@
 	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 {