From 60a9ab95b3c37c9d56f506e26bd874103958a9e8 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Thu, 6 Mar 2025 15:54:20 +0800 Subject: [PATCH 1/3] Problem: incorrect spendable balance when debug trace tx --- .github/workflows/test.yml | 2 +- .golangci.yml | 2 - CHANGELOG.md | 1 + .../hardhat/contracts/SelfDestruct.sol | 19 +++++++ tests/integration_tests/test_tracers.py | 52 +++++++++++++++++++ tests/integration_tests/utils.py | 1 + 6 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 tests/integration_tests/hardhat/contracts/SelfDestruct.sol diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 262e87b7b1..998884584d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -120,7 +120,7 @@ jobs: - name: 'Tar debug files' if: failure() run: tar cfz debug_files.tar.gz -C /tmp/pytest-of-runner . - - uses: actions/upload-artifact@v3 + - uses: actions/upload-artifact@v4 if: failure() with: name: debug-files diff --git a/.golangci.yml b/.golangci.yml index d1aa62dfe1..b0bdbd481c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -21,7 +21,6 @@ linters: - misspell - nakedret - prealloc - - exportloopref - staticcheck - stylecheck - typecheck @@ -30,7 +29,6 @@ linters: - unused - nolintlint - asciicheck - - exportloopref - gofumpt - gomodguard - whitespace diff --git a/CHANGELOG.md b/CHANGELOG.md index 5acd5f4e31..f603a93e1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -80,6 +80,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (ante) [#560](https://github.com/crypto-org-chain/ethermint/pull/560) Check gasWanted only in checkTx mode. * (rpc) [#562](https://github.com/crypto-org-chain/ethermint/pull/562) Fix nil pointer panic with legacy transaction format. * (evm) [#567](https://github.com/crypto-org-chain/ethermint/pull/567) Fix nonce management in batch transaction. +* (rpc) [#574](https://github.com/crypto-org-chain/ethermint/pull/574) Fix incorrect spendable balance when debug trace tx. ### Improvements diff --git a/tests/integration_tests/hardhat/contracts/SelfDestruct.sol b/tests/integration_tests/hardhat/contracts/SelfDestruct.sol new file mode 100644 index 0000000000..b14f72c560 --- /dev/null +++ b/tests/integration_tests/hardhat/contracts/SelfDestruct.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +contract SelfDestruct { + address payable recipient = payable(0x0F0cb39319129BA867227e5Aae1abe9e7dd5f861); + address payable owner; + + constructor() { + owner = payable(msg.sender); + } + + receive() external payable {} + + function execute() public payable { + require(msg.sender == owner, string(abi.encodePacked("Unauthorized caller: ", msg.sender, " Owner: ", owner))); + payable(recipient).transfer(msg.value); + selfdestruct(owner); + } +} diff --git a/tests/integration_tests/test_tracers.py b/tests/integration_tests/test_tracers.py index 968bb7c196..6a4b0c7b15 100644 --- a/tests/integration_tests/test_tracers.py +++ b/tests/integration_tests/test_tracers.py @@ -2,6 +2,7 @@ import json from concurrent.futures import ThreadPoolExecutor, as_completed +import pytest from web3 import Web3 from .expected_constants import ( @@ -24,6 +25,7 @@ send_txs, sign_transaction, w3_wait_for_new_blocks, + wait_for_fn, ) @@ -191,6 +193,56 @@ def test_trace_tx_reverse_transfer(ethermint): print(tx_res) +@pytest.mark.flaky(max_runs=50) +def test_destruct(ethermint): + method = "debug_traceTransaction" + tracer = {"tracer": "callTracer"} + receiver = "0x0F0cb39319129BA867227e5Aae1abe9e7dd5f861" + acc = derive_new_account(11) # ethm13c2n7geavjfsqcan290mq74kajjlxehyzhly4p + w3 = ethermint.w3 + fund_acc(w3, acc, fund=3077735635376769427) + sender = acc.address + raw_transactions = [] + contracts = [] + total = 3 + for _ in range(total): + contract, _ = deploy_contract(w3, CONTRACTS["SelfDestruct"], key=acc.key) + contracts.append(contract) + + nonce = w3.eth.get_transaction_count(sender) + + for i in range(total): + tx = ( + contracts[i] + .functions.execute() + .build_transaction( + { + "from": sender, + "nonce": nonce, + "gas": 167115, + "gasPrice": 5050000000000, + "value": 353434350000000000, + } + ) + ) + raw_transactions.append(sign_transaction(w3, tx, acc.key).rawTransaction) + nonce += 1 + sended_hash_set = send_raw_transactions(w3, raw_transactions) + + def wait_balance(): + return w3.eth.get_balance(receiver) > 0 + + wait_for_fn("wait_balance", wait_balance) + for h in sended_hash_set: + tx_hash = h.hex() + res = w3.provider.make_request( + method, + [tx_hash, tracer], + ) + print(tx_hash, res) + assert "insufficient funds" not in res, res + + def test_tracecall_insufficient_funds(ethermint, geth): method = "debug_traceCall" acc = derive_random_account() diff --git a/tests/integration_tests/utils.py b/tests/integration_tests/utils.py index 1893cdc374..9e2f8f55c5 100644 --- a/tests/integration_tests/utils.py +++ b/tests/integration_tests/utils.py @@ -45,6 +45,7 @@ "Random": "Random.sol", "TestBlockTxProperties": "TestBlockTxProperties.sol", "FeeCollector": "FeeCollector.sol", + "SelfDestruct": "SelfDestruct.sol", } From 6015b604d24c713e5597f3646476999950dbf4f4 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 7 Mar 2025 14:05:15 +0800 Subject: [PATCH 2/3] fix --- x/evm/keeper/state_transition.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index c561f6ba7b..ffddc3b5b5 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -326,6 +326,11 @@ func (k *Keeper) ApplyMessageWithConfig( leftoverGas := msg.GasLimit sender := vm.AccountRef(msg.From) tracer := cfg.GetTracer() + debugFn := func() { + if tracer != nil && cfg.DebugTrace { + stateDB.AddBalance(sender.Address(), new(big.Int).Mul(msg.GasPrice, new(big.Int).SetUint64(leftoverGas))) + } + } if tracer != nil { if cfg.DebugTrace { amount := new(big.Int).Mul(msg.GasPrice, new(big.Int).SetUint64(msg.GasLimit)) @@ -337,9 +342,7 @@ func (k *Keeper) ApplyMessageWithConfig( } tracer.CaptureTxStart(leftoverGas) defer func() { - if cfg.DebugTrace { - stateDB.AddBalance(sender.Address(), new(big.Int).Mul(msg.GasPrice, new(big.Int).SetUint64(leftoverGas))) - } + debugFn() tracer.CaptureTxEnd(leftoverGas) }() } @@ -404,13 +407,6 @@ func (k *Keeper) ApplyMessageWithConfig( vmError = vmErr.Error() } - // The dirty states in `StateDB` is either committed or discarded after return - if commit { - if err := stateDB.Commit(); err != nil { - return nil, errorsmod.Wrap(err, "failed to commit stateDB") - } - } - // calculate a minimum amount of gas to be charged to sender if GasLimit // is considerably higher than GasUsed to stay more aligned with Tendermint gas mechanics // for more info https://github.com/evmos/ethermint/issues/1085 @@ -438,6 +434,16 @@ func (k *Keeper) ApplyMessageWithConfig( // reset leftoverGas, to be used by the tracer leftoverGas = msg.GasLimit - gasUsed + debugFn() + debugFn = func() {} + + // The dirty states in `StateDB` is either committed or discarded after return + if commit { + if err := stateDB.Commit(); err != nil { + return nil, errorsmod.Wrap(err, "failed to commit stateDB") + } + } + return &types.MsgEthereumTxResponse{ GasUsed: gasUsed, VmError: vmError, From e5cd86fabd5fad48816a809cda57f17dc7edbaf1 Mon Sep 17 00:00:00 2001 From: yihuang Date: Wed, 12 Mar 2025 10:53:25 +0800 Subject: [PATCH 3/3] Update tests/integration_tests/test_tracers.py Co-authored-by: mmsqe Signed-off-by: yihuang --- tests/integration_tests/test_tracers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/test_tracers.py b/tests/integration_tests/test_tracers.py index 6a4b0c7b15..f8f4043e6d 100644 --- a/tests/integration_tests/test_tracers.py +++ b/tests/integration_tests/test_tracers.py @@ -193,7 +193,7 @@ def test_trace_tx_reverse_transfer(ethermint): print(tx_res) -@pytest.mark.flaky(max_runs=50) +@pytest.mark.flaky(max_runs=10) def test_destruct(ethermint): method = "debug_traceTransaction" tracer = {"tracer": "callTracer"}