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: opBlockhash broke after sdk50 #534

Merged
merged 15 commits into from
Oct 4, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +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.

### Improvements

Expand Down
1 change: 1 addition & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,7 @@ func (app *EthermintApp) RegisterAPIRoutes(apiSvr *api.Server, apiConfig config.

// RegisterTxService implements the Application.RegisterTxService method.
func (app *EthermintApp) RegisterTxService(clientCtx client.Context) {
app.EvmKeeper.WithCometClient(clientCtx.Client)
authtx.RegisterTxService(app.BaseApp.GRPCQueryRouter(), clientCtx, app.BaseApp.Simulate, app.interfaceRegistry)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract TestBlockTxProperties {
function getBlockHash(uint256 blockNumber) public view returns (bytes32) {
return blockhash(blockNumber);
}
}
11 changes: 11 additions & 0 deletions tests/integration_tests/test_block.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
from .utils import CONTRACTS, deploy_contract, w3_wait_for_new_blocks


def test_call(ethermint):
w3 = ethermint.w3
contract, res = deploy_contract(w3, CONTRACTS["TestBlockTxProperties"])
height = w3.eth.get_block_number()
w3_wait_for_new_blocks(w3, 1)
res = contract.caller.getBlockHash(height).hex()
blk = w3.eth.get_block(height)
assert f"0x{res}" == blk.hash.hex(), res
1 change: 1 addition & 0 deletions tests/integration_tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
"Calculator": "Calculator.sol",
"Caller": "Caller.sol",
"Random": "Random.sol",
"TestBlockTxProperties": "TestBlockTxProperties.sol",
}


Expand Down
2 changes: 1 addition & 1 deletion x/evm/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (k *Keeper) BeginBlock(ctx sdk.Context) error {
if _, err := k.EVMBlockConfig(ctx, k.ChainID()); err != nil {
return err
}

k.SetHeaderHash(ctx)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

path flow from Begin/EndBlock to a panic call
return nil
}

Expand Down
34 changes: 34 additions & 0 deletions x/evm/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,15 @@
package keeper

import (
"encoding/binary"
"math/big"

errorsmod "cosmossdk.io/errors"
"cosmossdk.io/log"
"cosmossdk.io/store/prefix"
storetypes "cosmossdk.io/store/types"
tmrpcclient "github.com/cometbft/cometbft/rpc/client"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
Expand Down Expand Up @@ -73,6 +77,8 @@ type Keeper struct {
// Legacy subspace
ss paramstypes.Subspace
customContractFns []CustomContractFn

signClient tmrpcclient.SignClient
}

// NewKeeper generates new evm module keeper
Expand Down Expand Up @@ -144,6 +150,14 @@ func (k Keeper) ChainID() *big.Int {
return k.eip155ChainID
}

func (k *Keeper) WithCometClient(c client.CometRPC) {
sc, ok := c.(tmrpcclient.SignClient)
if !ok {
panic("invalid rpc client")
}
k.signClient = sc
}

// ----------------------------------------------------------------------------
// Block Bloom
// Required by Web3 API.
Expand Down Expand Up @@ -305,3 +319,23 @@ func (k Keeper) AddTransientGasUsed(ctx sdk.Context, gasUsed uint64) (uint64, er
k.SetTransientGasUsed(ctx, result)
return result, nil
}

// SetHeaderHash stores the hash of the current block header in the store.
func (k Keeper) SetHeaderHash(ctx sdk.Context) {
store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixHeaderHash)
yihuang marked this conversation as resolved.
Show resolved Hide resolved
key := make([]byte, 8)
mmsqe marked this conversation as resolved.
Show resolved Hide resolved
height, err := ethermint.SafeUint64(ctx.BlockHeight())
if err != nil {
panic(err)
Fixed Show fixed Hide fixed

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}
binary.BigEndian.PutUint64(key, height)
store.Set(key, ctx.HeaderHash())
mmsqe marked this conversation as resolved.
Show resolved Hide resolved
}

// GetHeaderHash retrieves the hash of a block header from the store by height.
func (k Keeper) GetHeaderHash(ctx sdk.Context, height uint64) []byte {
store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixHeaderHash)
mmsqe marked this conversation as resolved.
Show resolved Hide resolved
yihuang marked this conversation as resolved.
Show resolved Hide resolved
key := make([]byte, 8)
binary.BigEndian.PutUint64(key, height)
return store.Get(key)
}
45 changes: 2 additions & 43 deletions x/evm/keeper/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (
"math/big"
"sort"

cmttypes "github.com/cometbft/cometbft/types"

errorsmod "cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -100,51 +98,12 @@ func (k Keeper) GetHashFn(ctx sdk.Context) vm.GetHashFunc {
return func(height uint64) common.Hash {
h, err := ethermint.SafeInt64(height)
if err != nil {
k.Logger(ctx).Error("failed to cast height to int64", "error", err)
return common.Hash{}
}

switch {
case ctx.BlockHeight() == h:
// Case 1: The requested height matches the one from the context so we can retrieve the header
// hash directly from the context.
// Note: The headerHash is only set at begin block, it will be nil in case of a query context
headerHash := ctx.HeaderHash()
if len(headerHash) != 0 {
return common.BytesToHash(headerHash)
}

// only recompute the hash if not set (eg: checkTxState)
contextBlockHeader := ctx.BlockHeader()
header, err := cmttypes.HeaderFromProto(&contextBlockHeader)
if err != nil {
k.Logger(ctx).Error("failed to cast tendermint header from proto", "error", err)
return common.Hash{}
}

headerHash = header.Hash()
return common.BytesToHash(headerHash)

case ctx.BlockHeight() > h:
// Case 2: if the chain is not the current height we need to retrieve the hash from the store for the
// current chain epoch. This only applies if the current height is greater than the requested height.
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())
default:
// Case 3: heights greater than the current one returns an empty hash.
if ctx.BlockHeight() < h {
return common.Hash{}
}
return common.BytesToHash(k.GetHeaderHash(ctx, height))
}
}

Expand Down
117 changes: 66 additions & 51 deletions x/evm/keeper/state_transition_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,20 @@ import (
"math"
"math/big"
"testing"
"time"

sdkmath "cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
cmtcrypto "github.com/cometbft/cometbft/crypto"
"github.com/cometbft/cometbft/crypto/tmhash"
tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
cmtrand "github.com/cometbft/cometbft/libs/rand"
cmtversion "github.com/cometbft/cometbft/proto/tendermint/version"
tmrpctypes "github.com/cometbft/cometbft/rpc/core/types"
tmtypes "github.com/cometbft/cometbft/types"
"github.com/cometbft/cometbft/version"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
Expand All @@ -22,6 +28,7 @@ import (
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/params"
"github.com/evmos/ethermint/app"
"github.com/evmos/ethermint/rpc/backend/mocks"
"github.com/evmos/ethermint/tests"
"github.com/evmos/ethermint/testutil"
utiltx "github.com/evmos/ethermint/testutil/tx"
Expand Down Expand Up @@ -84,86 +91,94 @@ func TestStateTransitionTestSuite(t *testing.T) {
suite.Run(t, new(StateTransitionTestSuite))
}

func makeRandHeader() tmtypes.Header {
chainID := "test"
t := time.Now()
height := cmtrand.Int63()
randBytes := cmtrand.Bytes(tmhash.Size)
randAddress := cmtrand.Bytes(cmtcrypto.AddressSize)
h := tmtypes.Header{
Version: cmtversion.Consensus{Block: version.BlockProtocol, App: 1},
ChainID: chainID,
Height: height,
Time: t,
LastBlockID: tmtypes.BlockID{},
LastCommitHash: randBytes,
DataHash: randBytes,
ValidatorsHash: randBytes,
NextValidatorsHash: randBytes,
ConsensusHash: randBytes,
AppHash: randBytes,
LastResultsHash: randBytes,
EvidenceHash: randBytes,
ProposerAddress: randAddress,
}
return h
}

func (suite *StateTransitionTestSuite) registerHeaderError(height uint64) {
c := mocks.NewClient(suite.T())
suite.App.EvmKeeper.WithCometClient(c)
suite.Ctx = suite.Ctx.WithBlockHeight(10).WithConsensusParams(*testutil.DefaultConsensusParams)
height0 := int64(height)
c.On("Header", suite.Ctx, &height0).Return(nil, errortypes.ErrNotFound)
}

func (suite *StateTransitionTestSuite) registerHeader(header tmtypes.Header) func(uint64) {
return func(height uint64) {
c := mocks.NewClient(suite.T())
suite.App.EvmKeeper.WithCometClient(c)
suite.Ctx = suite.Ctx.WithBlockHeight(10).WithConsensusParams(*testutil.DefaultConsensusParams)
height0 := int64(height)
c.On("Header", suite.Ctx, &height0).Return(&tmrpctypes.ResultHeader{Header: &header}, nil)
}
}

func (suite *StateTransitionTestSuite) TestGetHashFn() {
header := suite.Ctx.BlockHeader()
h, _ := tmtypes.HeaderFromProto(&header)
hash := h.Hash()
header := makeRandHeader()
hash := header.Hash()

testCases := []struct {
msg string
height uint64
malleate func()
malleate func(height uint64)
expHash common.Hash
}{
{
"case 1.1: context hash cached",
uint64(suite.Ctx.BlockHeight()),
func() {
suite.Ctx = suite.Ctx.WithHeaderHash(tmhash.Sum([]byte("header"))).WithConsensusParams(*testutil.DefaultConsensusParams)
},
common.BytesToHash(tmhash.Sum([]byte("header"))),
},
{
"case 1.2: failed to cast Tendermint header",
uint64(suite.Ctx.BlockHeight()),
func() {
header := tmproto.Header{}
header.Height = suite.Ctx.BlockHeight()
suite.Ctx = suite.Ctx.WithBlockHeader(header).WithConsensusParams(*testutil.DefaultConsensusParams)
},
"case 1.1: height equals to current one, header not found",
10,
suite.registerHeaderError,
common.Hash{},
},
{
"case 1.3: hash calculated from Tendermint header",
uint64(suite.Ctx.BlockHeight()),
func() {
suite.Ctx = suite.Ctx.WithBlockHeader(header).WithConsensusParams(*testutil.DefaultConsensusParams)
},
"case 1.2: height equals to current one, header found",
10,
suite.registerHeader(header),
common.BytesToHash(hash),
},
{
"case 2.1: height lower than current one, hist info not found",
1,
func() {
suite.Ctx = suite.Ctx.WithBlockHeight(10).WithConsensusParams(*testutil.DefaultConsensusParams)
},
common.Hash{},
},
{
"case 2.2: height lower than current one, invalid hist info header",
"case 2.1: height lower than current one, header not found",
1,
func() {
suite.App.StakingKeeper.SetHistoricalInfo(suite.Ctx, 1, &stakingtypes.HistoricalInfo{})
suite.Ctx = suite.Ctx.WithBlockHeight(10).WithConsensusParams(*testutil.DefaultConsensusParams)
},
suite.registerHeaderError,
common.Hash{},
},
{
"case 2.3: height lower than current one, calculated from hist info header",
"case 2.2: height lower than current one, header found",
1,
func() {
histInfo := &stakingtypes.HistoricalInfo{
Header: header,
}
suite.App.StakingKeeper.SetHistoricalInfo(suite.Ctx, 1, histInfo)
suite.Ctx = suite.Ctx.WithBlockHeight(10).WithConsensusParams(*testutil.DefaultConsensusParams)
},
suite.registerHeader(header),
common.BytesToHash(hash),
},
{
"case 3: height greater than current one",
200,
func() {},
func(_ uint64) {},
common.Hash{},
},
}

for _, tc := range testCases {
suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
suite.SetupTest() // reset

tc.malleate()

tc.malleate(tc.height)
hash := suite.App.EvmKeeper.GetHashFn(suite.Ctx)(tc.height)
suite.Require().Equal(tc.expHash, hash)
})
Expand Down
8 changes: 5 additions & 3 deletions x/evm/types/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ const (
prefixCode = iota + 1
prefixStorage
prefixParams
prefixHeaderHash
)

// prefix bytes for the EVM object store
Expand All @@ -55,9 +56,10 @@ const (

// KVStore key prefixes
var (
KeyPrefixCode = []byte{prefixCode}
KeyPrefixStorage = []byte{prefixStorage}
KeyPrefixParams = []byte{prefixParams}
KeyPrefixCode = []byte{prefixCode}
KeyPrefixStorage = []byte{prefixStorage}
KeyPrefixParams = []byte{prefixParams}
KeyPrefixHeaderHash = []byte{prefixHeaderHash}
)

// Object Store key prefixes
Expand Down