Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions tests/integration_tests/configs/default.jsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
params: {
no_base_fee: false,
base_fee: '100000000000',
min_gas_multiplier: '0.5',
},
},
},
Expand Down
97 changes: 95 additions & 2 deletions tests/integration_tests/test_tracers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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

Expand Down
33 changes: 14 additions & 19 deletions x/evm/keeper/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,41 +314,36 @@
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")
}

Check warning on line 322 in x/evm/keeper/state_transition.go

View check run for this annotation

Codecov / codecov/patch

x/evm/keeper/state_transition.go#L320-L322

Added lines #L320 - L322 were not covered by tests
}
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)
Expand Down
14 changes: 0 additions & 14 deletions x/evm/keeper/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 4 additions & 0 deletions x/evm/statedb/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,10 @@
s.validRevisions = s.validRevisions[:idx]
}

func (s *StateDB) Error() error {
return s.err

Check warning on line 641 in x/evm/statedb/statedb.go

View check run for this annotation

Codecov / codecov/patch

x/evm/statedb/statedb.go#L640-L641

Added lines #L640 - L641 were not covered by tests
}

// Commit writes the dirty states to keeper
// the StateDB object should be discarded after committed.
func (s *StateDB) Commit() error {
Expand Down
Loading