From d0aa6db3b7ce5c686d2054e60d4f3c6345d2ae2f Mon Sep 17 00:00:00 2001 From: mmsqe Date: Mon, 7 Oct 2024 16:13:20 +0800 Subject: [PATCH 1/5] Problem: no header hash from fallback historicalInfo --- CHANGELOG.md | 2 +- x/evm/keeper/state_transition.go | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f593c90c74..208f661084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,7 +69,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (rpc) [#516](https://github.com/crypto-org-chain/ethermint/pull/516) Avoid method eth_chainId crashed due to nil pointer on IsEIP155 check. * (cli) [#524](https://github.com/crypto-org-chain/ethermint/pull/524) Allow tx evm raw run for generate only when offline with evm-denom flag. * (rpc) [#527](https://github.com/crypto-org-chain/ethermint/pull/527) Fix balance consistency between trace-block and state machine. -* (rpc) [#534](https://github.com/crypto-org-chain/ethermint/pull/534) Fix opBlockhash when no block header in abci request. +* (rpc) [#534](https://github.com/crypto-org-chain/ethermint/pull/534), [#540](https://github.com/crypto-org-chain/ethermint/pull/540) Fix opBlockhash when no block header in abci request. * (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. diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index c6d493f56a..3b300fd985 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -28,6 +28,7 @@ import ( "github.com/evmos/ethermint/x/evm/statedb" "github.com/evmos/ethermint/x/evm/types" + cmttypes "github.com/cometbft/cometbft/types" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" ethtypes "github.com/ethereum/go-ethereum/core/types" @@ -109,7 +110,21 @@ func (k Keeper) GetHashFn(ctx sdk.Context) vm.GetHashFunc { return common.BytesToHash(headerHash) } } - return common.BytesToHash(k.GetHeaderHash(ctx, height)) + hash := k.GetHeaderHash(ctx, height) + if !bytes.Equal(hash, []byte{}) { + return common.BytesToHash(hash) + } + histInfo, err := k.stakingKeeper.GetHistoricalInfo(ctx, h) + if err != nil { + k.Logger(ctx).Debug("historical info not found", "height", h, "err", err.Error()) + return common.Hash{} + } + header, err := cmttypes.HeaderFromProto(&histInfo.Header) + if err != nil { + k.Logger(ctx).Error("failed to cast tendermint header from proto", "error", err) + return common.Hash{} + } + return common.BytesToHash(header.Hash()) } } From 678cd1e1adc8e13c0dda0a15cf4577c575f152da Mon Sep 17 00:00:00 2001 From: mmsqe Date: Mon, 7 Oct 2024 18:01:24 +0800 Subject: [PATCH 2/5] test --- tests/integration_tests/test_fee_history.py | 25 ++++++--------- tests/integration_tests/test_upgrade.py | 34 +++++++++++++++++++-- tests/integration_tests/utils.py | 15 +++++++++ 3 files changed, 56 insertions(+), 18 deletions(-) diff --git a/tests/integration_tests/test_fee_history.py b/tests/integration_tests/test_fee_history.py index 22f21d5b9b..3f65fa7fae 100644 --- a/tests/integration_tests/test_fee_history.py +++ b/tests/integration_tests/test_fee_history.py @@ -1,5 +1,4 @@ import hashlib -import json from concurrent.futures import ThreadPoolExecutor, as_completed from pathlib import Path @@ -9,9 +8,9 @@ from .network import setup_custom_ethermint from .utils import ( ADDRS, - approve_proposal, eth_to_bech32, send_transaction, + submit_gov_proposal, w3_wait_for_block, w3_wait_for_new_blocks, ) @@ -167,27 +166,21 @@ def update_feemarket_param(node, tmp_path, new_multiplier=2, new_denominator=200 p["base_fee"] = new_base_fee p["elasticity_multiplier"] = new_multiplier p["base_fee_change_denominator"] = new_denominator - proposal = tmp_path / "proposal.json" + # governance module account as signer data = hashlib.sha256("gov".encode()).digest()[:20] - signer = eth_to_bech32(data) - proposal_src = { - "messages": [ + authority = eth_to_bech32(data) + submit_gov_proposal( + node, + tmp_path, + messages=[ { "@type": "/ethermint.feemarket.v1.MsgUpdateParams", - "authority": signer, + "authority": authority, "params": p, } ], - "deposit": "2aphoton", - "title": "title", - "summary": "summary", - } - proposal.write_text(json.dumps(proposal_src)) - rsp = cli.submit_gov_proposal(proposal, from_="community") - assert rsp["code"] == 0, rsp["raw_log"] - approve_proposal(node, rsp) - print("check params have been updated now") + ) p = cli.get_params("feemarket")["params"] assert p["base_fee"] == new_base_fee assert p["elasticity_multiplier"] == new_multiplier diff --git a/tests/integration_tests/test_upgrade.py b/tests/integration_tests/test_upgrade.py index da02f3b4b2..58a0e7bb69 100644 --- a/tests/integration_tests/test_upgrade.py +++ b/tests/integration_tests/test_upgrade.py @@ -1,4 +1,5 @@ import configparser +import hashlib import json import re import subprocess @@ -14,7 +15,10 @@ CONTRACTS, approve_proposal, deploy_contract, + eth_to_bech32, send_transaction, + submit_gov_proposal, + w3_wait_for_new_blocks, wait_for_block, wait_for_port, ) @@ -84,7 +88,7 @@ def custom_ethermint(tmp_path_factory): ) -def test_cosmovisor_upgrade(custom_ethermint: Ethermint): +def test_cosmovisor_upgrade(custom_ethermint: Ethermint, tmp_path): """ - propose an upgrade and pass it - wait for it to happen @@ -100,7 +104,8 @@ def test_cosmovisor_upgrade(custom_ethermint: Ethermint): old_erc20_balance = contract.caller.balanceOf(ADDRS["validator"]) print("old values", old_height, old_balance, old_base_fee) - target_height = w3.eth.block_number + 10 + height_before = w3.eth.block_number + target_height = height_before + 10 print("upgrade height", target_height) plan_name = "sdk50" @@ -166,3 +171,28 @@ def test_cosmovisor_upgrade(custom_ethermint: Ethermint): ) ) assert p == {"allowed_clients": ["06-solomachine", "07-tendermint", "09-localhost"]} + + p = cli.get_params("evm")["params"] + header_hash_num = "2" + p["header_hash_num"] = header_hash_num + # governance module account as signer + data = hashlib.sha256("gov".encode()).digest()[:20] + authority = eth_to_bech32(data) + submit_gov_proposal( + custom_ethermint, + tmp_path, + messages=[ + { + "@type": "/ethermint.evm.v1.MsgUpdateParams", + "authority": authority, + "params": p, + } + ], + ) + p = cli.get_params("evm")["params"] + assert p["header_hash_num"] == header_hash_num, p + contract, _ = deploy_contract(w3, CONTRACTS["TestBlockTxProperties"]) + w3_wait_for_new_blocks(w3, 1) + res = contract.caller.getBlockHash(height_before).hex() + blk = w3.eth.get_block(height_before) + assert f"0x{res}" == blk.hash.hex(), res diff --git a/tests/integration_tests/utils.py b/tests/integration_tests/utils.py index b450c99eeb..9d5e9a15b2 100644 --- a/tests/integration_tests/utils.py +++ b/tests/integration_tests/utils.py @@ -357,6 +357,21 @@ def cb(attrs): assert proposal["status"] == "PROPOSAL_STATUS_PASSED", proposal +def submit_gov_proposal(ethermint, tmp_path, **kwargs): + proposal = tmp_path / "proposal.json" + proposal_src = { + "title": "title", + "summary": "summary", + "deposit": "2aphoton", + **kwargs, + } + proposal.write_text(json.dumps(proposal_src)) + rsp = ethermint.cosmos_cli().submit_gov_proposal(proposal, from_="community") + assert rsp["code"] == 0, rsp["raw_log"] + approve_proposal(ethermint, rsp) + print("check params have been updated now") + + class ContractAddress(rlp.Serializable): fields = [ ("from", rlp.sedes.Binary()), From 774e16950308afa43dd47694b8d1ba4a78b80256 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 8 Oct 2024 10:36:03 +0800 Subject: [PATCH 3/5] keep headerNum no change --- tests/integration_tests/test_upgrade.py | 2 -- x/evm/keeper/state_transition.go | 14 +++++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/integration_tests/test_upgrade.py b/tests/integration_tests/test_upgrade.py index 58a0e7bb69..76b1063097 100644 --- a/tests/integration_tests/test_upgrade.py +++ b/tests/integration_tests/test_upgrade.py @@ -18,7 +18,6 @@ eth_to_bech32, send_transaction, submit_gov_proposal, - w3_wait_for_new_blocks, wait_for_block, wait_for_port, ) @@ -192,7 +191,6 @@ def test_cosmovisor_upgrade(custom_ethermint: Ethermint, tmp_path): p = cli.get_params("evm")["params"] assert p["header_hash_num"] == header_hash_num, p contract, _ = deploy_contract(w3, CONTRACTS["TestBlockTxProperties"]) - w3_wait_for_new_blocks(w3, 1) res = contract.caller.getBlockHash(height_before).hex() blk = w3.eth.get_block(height_before) assert f"0x{res}" == blk.hash.hex(), res diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index 3b300fd985..d36b023cf0 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -101,19 +101,27 @@ func (k Keeper) GetHashFn(ctx sdk.Context) vm.GetHashFunc { if err != nil { return common.Hash{} } - if ctx.BlockHeight() < h { + latestHeight := ctx.BlockHeight() + if latestHeight < h { return common.Hash{} } - if ctx.BlockHeight() == h { + latest, err := ethermint.SafeUint64(latestHeight) + if err != nil { + return common.Hash{} + } + if latestHeight == h { headerHash := ctx.HeaderHash() if len(headerHash) != 0 { return common.BytesToHash(headerHash) } } hash := k.GetHeaderHash(ctx, height) - if !bytes.Equal(hash, []byte{}) { + if len(hash) > 0 { return common.BytesToHash(hash) } + if height < latest-k.GetParams(ctx).HeaderHashNum { + return common.Hash{} + } histInfo, err := k.stakingKeeper.GetHistoricalInfo(ctx, h) if err != nil { k.Logger(ctx).Debug("historical info not found", "height", h, "err", err.Error()) From a5d2bbb14e7c81021d4949d8e0d347f084663fa2 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 8 Oct 2024 11:13:53 +0800 Subject: [PATCH 4/5] align check --- .../integration_tests/configs/default.jsonnet | 1 - tests/integration_tests/test_block.py | 3 -- tests/integration_tests/test_upgrade.py | 20 +++++++--- x/evm/keeper/state_transition.go | 38 ++++++++++--------- 4 files changed, 35 insertions(+), 27 deletions(-) diff --git a/tests/integration_tests/configs/default.jsonnet b/tests/integration_tests/configs/default.jsonnet index c90f910848..1345ee682a 100644 --- a/tests/integration_tests/configs/default.jsonnet +++ b/tests/integration_tests/configs/default.jsonnet @@ -63,7 +63,6 @@ evm: { params: { evm_denom: 'aphoton', - header_hash_num: 2, }, }, gov: { diff --git a/tests/integration_tests/test_block.py b/tests/integration_tests/test_block.py index f19b026e57..b8c1facc32 100644 --- a/tests/integration_tests/test_block.py +++ b/tests/integration_tests/test_block.py @@ -9,6 +9,3 @@ def test_call(ethermint): res = contract.caller.getBlockHash(height).hex() blk = w3.eth.get_block(height) assert f"0x{res}" == blk.hash.hex(), res - w3_wait_for_new_blocks(w3, 1) - res = contract.caller.getBlockHash(height).hex() - assert f"0x{res}" == "0x" + "0" * 64, res diff --git a/tests/integration_tests/test_upgrade.py b/tests/integration_tests/test_upgrade.py index 76b1063097..c230a16ef7 100644 --- a/tests/integration_tests/test_upgrade.py +++ b/tests/integration_tests/test_upgrade.py @@ -103,8 +103,7 @@ def test_cosmovisor_upgrade(custom_ethermint: Ethermint, tmp_path): old_erc20_balance = contract.caller.balanceOf(ADDRS["validator"]) print("old values", old_height, old_balance, old_base_fee) - height_before = w3.eth.block_number - target_height = height_before + 10 + target_height = w3.eth.block_number + 10 print("upgrade height", target_height) plan_name = "sdk50" @@ -172,7 +171,7 @@ def test_cosmovisor_upgrade(custom_ethermint: Ethermint, tmp_path): assert p == {"allowed_clients": ["06-solomachine", "07-tendermint", "09-localhost"]} p = cli.get_params("evm")["params"] - header_hash_num = "2" + header_hash_num = "20" p["header_hash_num"] = header_hash_num # governance module account as signer data = hashlib.sha256("gov".encode()).digest()[:20] @@ -191,6 +190,15 @@ def test_cosmovisor_upgrade(custom_ethermint: Ethermint, tmp_path): p = cli.get_params("evm")["params"] assert p["header_hash_num"] == header_hash_num, p contract, _ = deploy_contract(w3, CONTRACTS["TestBlockTxProperties"]) - res = contract.caller.getBlockHash(height_before).hex() - blk = w3.eth.get_block(height_before) - assert f"0x{res}" == blk.hash.hex(), res + for h in [target_height - 1, target_height, target_height + 1]: + res = contract.caller.getBlockHash(h).hex() + blk = w3.eth.get_block(h) + assert f"0x{res}" == blk.hash.hex(), res + + height = w3.eth.block_number + for h in [ + height - int(header_hash_num) - 1, # num64 < lower + height + 100, # num64 >= upper + ]: + res = contract.caller.getBlockHash(h).hex() + assert f"0x{res}" == "0x" + "0" * 64, res diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index d36b023cf0..ecb2b3ee21 100644 --- a/x/evm/keeper/state_transition.go +++ b/x/evm/keeper/state_transition.go @@ -91,37 +91,41 @@ func (k *Keeper) NewEVM( return evm } -// GetHashFn implements vm.GetHashFunc for Ethermint. It handles 3 cases: -// 1. The requested height matches the current height from context (and thus same epoch number) -// 2. The requested height is from an previous height from the same chain epoch -// 3. The requested height is from a height greater than the latest one +// GetHashFn implements vm.GetHashFunc for Ethermint. It returns hash for 3 cases: +// 1. The requested height matches current block height from the context. +// 2. The requested height is within the valid range, retrieve the hash from GetHeaderHash for heights after sdk50. +// 3. The requested height is within the valid range, retrieve the hash from GetHistoricalInfo for heights before sdk50. func (k Keeper) GetHashFn(ctx sdk.Context) vm.GetHashFunc { - return func(height uint64) common.Hash { - h, err := ethermint.SafeInt64(height) + return func(num64 uint64) common.Hash { + h, err := ethermint.SafeInt64(num64) if err != nil { return common.Hash{} } - latestHeight := ctx.BlockHeight() - if latestHeight < h { - return common.Hash{} - } - latest, err := ethermint.SafeUint64(latestHeight) + upper, err := ethermint.SafeUint64(ctx.BlockHeight()) if err != nil { return common.Hash{} } - if latestHeight == h { + if upper == num64 { headerHash := ctx.HeaderHash() - if len(headerHash) != 0 { + if len(headerHash) > 0 { return common.BytesToHash(headerHash) } } - hash := k.GetHeaderHash(ctx, height) - if len(hash) > 0 { - return common.BytesToHash(hash) + // Align check with https://github.com/ethereum/go-ethereum/blob/release/1.11/core/vm/instructions.go#L433 + headerNum := k.GetParams(ctx).HeaderHashNum + var lower uint64 + if upper <= headerNum { + lower = 0 + } else { + lower = upper - headerNum } - if height < latest-k.GetParams(ctx).HeaderHashNum { + if num64 < lower || num64 >= upper { return common.Hash{} } + hash := k.GetHeaderHash(ctx, num64) + if len(hash) > 0 { + return common.BytesToHash(hash) + } histInfo, err := k.stakingKeeper.GetHistoricalInfo(ctx, h) if err != nil { k.Logger(ctx).Debug("historical info not found", "height", h, "err", err.Error()) From e9576862254ab025422c1d6e60ef0e512af636b1 Mon Sep 17 00:00:00 2001 From: mmsqe Date: Tue, 8 Oct 2024 11:19:16 +0800 Subject: [PATCH 5/5] more test --- x/evm/keeper/state_transition_test.go | 58 ++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 10 deletions(-) diff --git a/x/evm/keeper/state_transition_test.go b/x/evm/keeper/state_transition_test.go index cad82800be..01c8278c02 100644 --- a/x/evm/keeper/state_transition_test.go +++ b/x/evm/keeper/state_transition_test.go @@ -33,6 +33,7 @@ import ( "github.com/evmos/ethermint/x/evm/keeper" "github.com/evmos/ethermint/x/evm/statedb" "github.com/evmos/ethermint/x/evm/types" + evmtypes "github.com/evmos/ethermint/x/evm/types" feemarkettypes "github.com/evmos/ethermint/x/feemarket/types" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -119,36 +120,73 @@ func (suite *StateTransitionTestSuite) registerHeader(header tmtypes.Header) { } func (suite *StateTransitionTestSuite) TestGetHashFn() { - height := uint64(10) + height := uint64(evmtypes.DefaultHeaderHashNum + 2) header := makeRandHeader(height) hash := header.Hash() testCases := []struct { msg string height uint64 - malleate func(height uint64) + malleate func(int64) expHash common.Hash }{ { - "header not found", + "use cached header hash", height, - func(height uint64) {}, - common.Hash{}, + func(_ int64) { + suite.Ctx = suite.Ctx.WithHeaderHash(hash) + }, + common.BytesToHash(hash), }, { - "header found", - height, - func(height uint64) { - suite.Ctx = suite.Ctx.WithBlockHeight(header.Height).WithHeaderHash(header.Hash()) + "header after sdk50 found", + height - 1, + func(height int64) { + suite.Ctx = suite.Ctx.WithBlockHeight(height).WithHeaderHash(header.Hash()) suite.App.EvmKeeper.SetHeaderHash(suite.Ctx) }, common.BytesToHash(hash), }, + { + "header before sdk50 found", + height - 1, + func(height int64) { + suite.App.StakingKeeper.SetHistoricalInfo(suite.Ctx, height, &stakingtypes.HistoricalInfo{ + Header: *header.ToProto(), + }) + }, + common.BytesToHash(hash), + }, + { + "header in context not found with current height", + height, + func(_ int64) {}, + common.Hash{}, + }, + { + "height greater than current height", + height + 1, + func(_ int64) {}, + common.Hash{}, + }, + { + "height less than header hash num range", + height - evmtypes.DefaultHeaderHashNum - 1, + func(_ int64) {}, + common.Hash{}, + }, + { + "header not found in stores", + height - 1, + func(_ int64) {}, + common.Hash{}, + }, } for _, tc := range testCases { suite.Run(fmt.Sprintf("Case %s", tc.msg), func() { suite.SetupTest() // reset - tc.malleate(tc.height) + tc.malleate(int64(tc.height)) + suite.Ctx = suite.Ctx.WithBlockHeight(header.Height) hash := suite.App.EvmKeeper.GetHashFn(suite.Ctx)(tc.height) suite.Require().Equal(tc.expHash, hash) })