From a2ad87c8a1ccf9722f7a6516e2cf74df01eb146d Mon Sep 17 00:00:00 2001 From: mmsqe Date: Wed, 9 Oct 2024 10:01:50 +0800 Subject: [PATCH] Problem: no header hash from fallback historicalInfo (#540) * Problem: no header hash from fallback historicalInfo * test * keep headerNum no change * align check * more test --- CHANGELOG.md | 2 +- .../integration_tests/configs/default.jsonnet | 1 - tests/integration_tests/test_block.py | 3 - tests/integration_tests/test_fee_history.py | 25 +++----- tests/integration_tests/test_upgrade.py | 38 +++++++++++- tests/integration_tests/utils.py | 15 +++++ x/evm/keeper/state_transition.go | 47 +++++++++++---- x/evm/keeper/state_transition_test.go | 58 +++++++++++++++---- 8 files changed, 147 insertions(+), 42 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/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_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..c230a16ef7 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,9 @@ CONTRACTS, approve_proposal, deploy_contract, + eth_to_bech32, send_transaction, + submit_gov_proposal, wait_for_block, wait_for_port, ) @@ -84,7 +87,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 @@ -166,3 +169,36 @@ 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 = "20" + 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"]) + 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/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()), diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index c6d493f56a..ecb2b3ee21 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" @@ -90,26 +91,52 @@ 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{} } - if ctx.BlockHeight() < h { + upper, err := ethermint.SafeUint64(ctx.BlockHeight()) + if err != nil { return common.Hash{} } - if ctx.BlockHeight() == h { + if upper == num64 { headerHash := ctx.HeaderHash() - if len(headerHash) != 0 { + if len(headerHash) > 0 { return common.BytesToHash(headerHash) } } - return common.BytesToHash(k.GetHeaderHash(ctx, height)) + // 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 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()) + 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()) } } 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) })