From 9fc2dc5694496b20c1a69e769a035a52cc8e4506 Mon Sep 17 00:00:00 2001 From: zsystm <124245155+zsystm@users.noreply.github.com> Date: Tue, 1 Oct 2024 18:53:49 +0900 Subject: [PATCH] test(evmengine/keeper): add test cases for msg server (#100) changes of mockEngineAPI - forceInvalidNewPayloadV3 and forceInvalidForkchoiceUpdatedV3 are added to simulate failed apis changes of engineMock - Add storeKey to make engineMock's methods dependent on sdk.Context for better testability NewBlock - Because of above changes, we need to rlp encode and decode block data. But if we create a block with nil withdrawals and nil withdrawalHash, rlp doesn't work well (couldn't figure out the root cause) - To avoid rlp error, pass non-nil withdrawals so withdrawalHash can be set as non-nil emptyHash value. --- .../x/evmengine/keeper/abci_internal_test.go | 43 +++- .../evmengine/keeper/keeper_internal_test.go | 12 +- .../keeper/msg_server_internal_test.go | 237 ++++++++++++++---- .../keeper/proposal_server_internal_test.go | 8 +- .../keeper/upgrades_internal_test.go | 6 +- client/x/evmstaking/keeper/keeper_test.go | 2 +- lib/ethclient/enginemock.go | 116 +++++++-- 7 files changed, 333 insertions(+), 91 deletions(-) diff --git a/client/x/evmengine/keeper/abci_internal_test.go b/client/x/evmengine/keeper/abci_internal_test.go index 0fd30ee5..21e00c26 100644 --- a/client/x/evmengine/keeper/abci_internal_test.go +++ b/client/x/evmengine/keeper/abci_internal_test.go @@ -124,7 +124,7 @@ func TestKeeper_PrepareProposal(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - ctx, storeService := setupCtxStore(t, nil) + ctx, storeKey, storeService := setupCtxStore(t, nil) cdc := getCodec(t) txConfig := authtx.NewTxConfig(cdc, nil) @@ -138,7 +138,7 @@ func TestKeeper_PrepareProposal(t *testing.T) { } var err error - tt.mockEngine.EngineClient, err = ethclient.NewEngineMock() + tt.mockEngine.EngineClient, err = ethclient.NewEngineMock(storeKey) require.NoError(t, err) k, err := NewKeeper(cdc, storeService, &tt.mockEngine, &tt.mockClient, txConfig, ak, esk, uk) @@ -160,11 +160,11 @@ func TestKeeper_PrepareProposal(t *testing.T) { t.Run("TestBuildNonOptimistic", func(t *testing.T) { t.Parallel() // setup dependencies - ctx, storeService := setupCtxStore(t, nil) + ctx, storeKey, storeService := setupCtxStore(t, nil) cdc := getCodec(t) txConfig := authtx.NewTxConfig(cdc, nil) - mockEngine, err := newMockEngineAPI(0) + mockEngine, err := newMockEngineAPI(storeKey, 0) require.NoError(t, err) ctrl := gomock.NewController(t) @@ -250,14 +250,14 @@ func assertExecutablePayload(t *testing.T, msg sdk.Msg, ts int64, blockHash comm // require.Equal(t, evmLog.Address, zeroAddr.Bytes()) } -func ctxWithAppHash(t *testing.T, appHash common.Hash) context.Context { +func ctxWithAppHash(t *testing.T, appHash common.Hash) (context.Context, *storetypes.KVStoreKey) { t.Helper() - ctx, _ := setupCtxStore(t, &cmtproto.Header{AppHash: appHash.Bytes()}) + ctx, storeKey, _ := setupCtxStore(t, &cmtproto.Header{AppHash: appHash.Bytes()}) - return ctx + return ctx, storeKey } -func setupCtxStore(t *testing.T, header *cmtproto.Header) (sdk.Context, store.KVStoreService) { +func setupCtxStore(t *testing.T, header *cmtproto.Header) (sdk.Context, *storetypes.KVStoreKey, store.KVStoreService) { t.Helper() key := storetypes.NewKVStoreKey("test") storeService := runtime.NewKVStoreService(key) @@ -267,7 +267,7 @@ func setupCtxStore(t *testing.T, header *cmtproto.Header) (sdk.Context, store.KV } ctx := testCtx.Ctx.WithBlockHeader(*header) - return ctx, storeService + return ctx, key, storeService } func getCodec(t *testing.T) codec.Codec { @@ -304,11 +304,15 @@ type mockEngineAPI struct { headerByTypeFunc func(context.Context, ethclient.HeadType) (*types.Header, error) forkchoiceUpdatedV3Func func(context.Context, eengine.ForkchoiceStateV1, *eengine.PayloadAttributes) (eengine.ForkChoiceResponse, error) newPayloadV3Func func(context.Context, eengine.ExecutableData, []common.Hash, *common.Hash) (eengine.PayloadStatusV1, error) + // forceInvalidNewPayloadV3 forces the NewPayloadV3 returns an invalid status. + forceInvalidNewPayloadV3 bool + // forceInvalidForkchoiceUpdatedV3 forces the ForkchoiceUpdatedV3 returns an invalid status. + forceInvalidForkchoiceUpdatedV3 bool } // newMockEngineAPI returns a new mock engine API with a fuzzer and a mock engine client. -func newMockEngineAPI(syncings int) (mockEngineAPI, error) { - me, err := ethclient.NewEngineMock() +func newMockEngineAPI(key *storetypes.KVStoreKey, syncings int) (mockEngineAPI, error) { + me, err := ethclient.NewEngineMock(key) if err != nil { return mockEngineAPI{}, err } @@ -395,6 +399,13 @@ func (m *mockEngineAPI) NewPayloadV2(ctx context.Context, params eengine.Executa //nolint:nonamedreturns // Required for defer func (m *mockEngineAPI) NewPayloadV3(ctx context.Context, params eengine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash) (resp eengine.PayloadStatusV1, err error) { + if m.forceInvalidNewPayloadV3 { + m.forceInvalidNewPayloadV3 = false + return eengine.PayloadStatusV1{ + Status: eengine.INVALID, + }, nil + } + if status, ok := m.maybeSync(); ok { defer func() { resp.Status = status.Status @@ -410,6 +421,14 @@ func (m *mockEngineAPI) NewPayloadV3(ctx context.Context, params eengine.Executa //nolint:nonamedreturns // Required for defer func (m *mockEngineAPI) ForkchoiceUpdatedV3(ctx context.Context, update eengine.ForkchoiceStateV1, payloadAttributes *eengine.PayloadAttributes) (resp eengine.ForkChoiceResponse, err error) { + if m.forceInvalidForkchoiceUpdatedV3 { + m.forceInvalidForkchoiceUpdatedV3 = false + return eengine.ForkChoiceResponse{ + PayloadStatus: eengine.PayloadStatusV1{ + Status: eengine.INVALID, + }, + }, nil + } if status, ok := m.maybeSync(); ok { defer func() { resp.PayloadStatus.Status = status.Status @@ -448,7 +467,7 @@ func (m *mockEngineAPI) nextBlock( header.ParentBeaconRoot = beaconRoot // Convert header to block - block := types.NewBlock(&header, nil, nil, trie.NewStackTrie(nil)) + block := types.NewBlock(&header, &types.Body{Withdrawals: make([]*types.Withdrawal, 0)}, nil, trie.NewStackTrie(nil)) // Convert block to payload env := eengine.BlockToExecutableData(block, big.NewInt(0), nil) diff --git a/client/x/evmengine/keeper/keeper_internal_test.go b/client/x/evmengine/keeper/keeper_internal_test.go index ccda9907..40fd34a9 100644 --- a/client/x/evmengine/keeper/keeper_internal_test.go +++ b/client/x/evmengine/keeper/keeper_internal_test.go @@ -40,8 +40,6 @@ func createTestKeeper(t *testing.T) (context.Context, *Keeper) { cdc := getCodec(t) txConfig := authtx.NewTxConfig(cdc, nil) - mockEngine, err := newMockEngineAPI(0) - require.NoError(t, err) cmtAPI := newMockCometAPI(t, nil) header := cmtproto.Header{Height: 1} @@ -52,7 +50,9 @@ func createTestKeeper(t *testing.T) (context.Context, *Keeper) { esk := moduletestutil.NewMockEvmStakingKeeper(ctrl) uk := moduletestutil.NewMockUpgradeKeeper(ctrl) - ctx, storeService := setupCtxStore(t, &header) + ctx, storeKey, storeService := setupCtxStore(t, &header) + mockEngine, err := newMockEngineAPI(storeKey, 0) + require.NoError(t, err) keeper, err := NewKeeper(cdc, storeService, &mockEngine, mockClient, txConfig, ak, esk, uk) require.NoError(t, err) @@ -66,8 +66,6 @@ func createKeeper(t *testing.T, args args) (sdk.Context, *mockCometAPI, *Keeper) cdc := getCodec(t) txConfig := authtx.NewTxConfig(cdc, nil) - mockEngine, err := newMockEngineAPI(0) - require.NoError(t, err) cmtAPI := newMockCometAPI(t, args.validatorsFunc) header := args.header(args.height, cmtAPI.validatorSet.Validators[args.current].Address) @@ -81,7 +79,9 @@ func createKeeper(t *testing.T, args args) (sdk.Context, *mockCometAPI, *Keeper) esk := moduletestutil.NewMockEvmStakingKeeper(ctrl) uk := moduletestutil.NewMockUpgradeKeeper(ctrl) - ctx, storeService := setupCtxStore(t, &header) + ctx, storeKey, storeService := setupCtxStore(t, &header) + mockEngine, err := newMockEngineAPI(storeKey, 0) + require.NoError(t, err) keeper, err := NewKeeper(cdc, storeService, &mockEngine, mockClient, txConfig, ak, esk, uk) require.NoError(t, err) keeper.SetCometAPI(cmtAPI) diff --git a/client/x/evmengine/keeper/msg_server_internal_test.go b/client/x/evmengine/keeper/msg_server_internal_test.go index ab0206f4..e2378bca 100644 --- a/client/x/evmengine/keeper/msg_server_internal_test.go +++ b/client/x/evmengine/keeper/msg_server_internal_test.go @@ -18,6 +18,7 @@ import ( moduletestutil "github.com/piplabs/story/client/x/evmengine/testutil" "github.com/piplabs/story/client/x/evmengine/types" + "github.com/piplabs/story/contracts/bindings" "github.com/piplabs/story/lib/errors" "github.com/piplabs/story/lib/ethclient" "github.com/piplabs/story/lib/ethclient/mock" @@ -34,19 +35,12 @@ func Test_msgServer_ExecutionPayload(t *testing.T) { cdc := getCodec(t) txConfig := authtx.NewTxConfig(cdc, nil) - mockEngine, err := newMockEngineAPI(2) - require.NoError(t, err) - ctrl := gomock.NewController(t) mockClient := mock.NewMockClient(ctrl) ak := moduletestutil.NewMockAccountKeeper(ctrl) esk := moduletestutil.NewMockEvmStakingKeeper(ctrl) uk := moduletestutil.NewMockUpgradeKeeper(ctrl) - // Expected call for PeekEligibleWithdrawals - esk.EXPECT().DequeueEligibleWithdrawals(gomock.Any()).Return(nil, nil).AnyTimes() - esk.EXPECT().ProcessStakingEvents(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - cmtAPI := newMockCometAPI(t, nil) // set the header and proposer so we have the correct next proposer header := cmtproto.Header{Height: 1, AppHash: tutil.RandomHash().Bytes()} @@ -54,10 +48,11 @@ func Test_msgServer_ExecutionPayload(t *testing.T) { nxtAddr, err := k1util.PubKeyToAddress(cmtAPI.validatorSet.Validators[1].PubKey) require.NoError(t, err) - ctx, storeService := setupCtxStore(t, &header) + ctx, storeKey, storeService := setupCtxStore(t, &header) ctx = ctx.WithExecMode(sdk.ExecModeFinalize) - evmLogProc := mockLogProvider{deliverErr: errors.New("test error")} + mockEngine, err := newMockEngineAPI(storeKey, 2) + require.NoError(t, err) keeper, err := NewKeeper(cdc, storeService, &mockEngine, mockClient, txConfig, ak, esk, uk) require.NoError(t, err) keeper.SetCometAPI(cmtAPI) @@ -65,54 +60,210 @@ func Test_msgServer_ExecutionPayload(t *testing.T) { populateGenesisHead(ctx, t, keeper) msgSrv := NewMsgServerImpl(keeper) - - var payloadData []byte - var payloadID engine.PayloadID - var latestHeight uint64 - var block *etypes.Block - newPayload := func(ctx context.Context) { + createValidPayload := func(c context.Context) (*etypes.Block, engine.PayloadID, []byte) { // get latest block to build on top - latestBlock, err := mockEngine.HeaderByType(ctx, ethclient.HeadLatest) + latestBlock, err := mockEngine.HeaderByType(c, ethclient.HeadLatest) require.NoError(t, err) latestHeight := latestBlock.Number.Uint64() - sdkCtx := sdk.UnwrapSDKContext(ctx) + sdkCtx := sdk.UnwrapSDKContext(c) appHash := common.BytesToHash(sdkCtx.BlockHeader().AppHash) - b, execPayload := mockEngine.nextBlock(t, latestHeight+1, uint64(time.Now().Unix()), latestBlock.Hash(), keeper.validatorAddr, &appHash) - block = b - - payloadID, err = ethclient.MockPayloadID(execPayload, &appHash) + block, execPayload := mockEngine.nextBlock(t, latestHeight+1, uint64(time.Now().Unix()), latestBlock.Hash(), keeper.validatorAddr, &appHash) + payloadID, err := ethclient.MockPayloadID(execPayload, &appHash) require.NoError(t, err) // Create execution payload message - payloadData, err = json.Marshal(execPayload) + payloadData, err := json.Marshal(execPayload) require.NoError(t, err) - } - assertExecutionPayload := func(ctx context.Context) { - events, err := evmLogProc.Prepare(ctx, block.Hash()) + return block, payloadID, payloadData + } + createRandomEvents := func(c context.Context, blkHash common.Hash) []*types.EVMEvent { + events, err := evmLogProc.Prepare(c, blkHash) require.NoError(t, err) - resp, err := msgSrv.ExecutionPayload(ctx, &types.MsgExecutionPayload{ - Authority: authtypes.NewModuleAddress(types.ModuleName).String(), - ExecutionPayload: payloadData, - PrevPayloadEvents: events, - }) - require.NoError(t, err) - require.NotNil(t, resp) + return events + } - gotPayload, err := mockEngine.GetPayloadV3(ctx, payloadID) - require.NoError(t, err) - // make sure height is increasing in engine, blocks being built - require.Equal(t, gotPayload.ExecutionPayload.Number, latestHeight+1) - require.Equal(t, gotPayload.ExecutionPayload.BlockHash, block.Hash()) - require.Equal(t, gotPayload.ExecutionPayload.FeeRecipient, keeper.validatorAddr) - require.Empty(t, gotPayload.ExecutionPayload.Withdrawals) + tcs := []struct { + name string + setup func(c context.Context) sdk.Context + createPayload func(c context.Context) (*etypes.Block, engine.PayloadID, []byte) + createPrevPayloadEvents func(c context.Context, blkHash common.Hash) []*types.EVMEvent + expectedError string + postCheck func(c context.Context, block *etypes.Block, payloadID engine.PayloadID) + }{ + { + name: "pass: valid payload", + setup: func(c context.Context) sdk.Context { + esk.EXPECT().DequeueEligibleWithdrawals(c).Return(nil, nil) + esk.EXPECT().ProcessStakingEvents(c, gomock.Any(), gomock.Any()).Return(nil) + + return sdk.UnwrapSDKContext(c) + }, + createPayload: createValidPayload, + createPrevPayloadEvents: createRandomEvents, + postCheck: func(c context.Context, block *etypes.Block, payloadID engine.PayloadID) { + gotPayload, err := mockEngine.GetPayloadV3(c, payloadID) + require.NoError(t, err) + require.Equal(t, gotPayload.ExecutionPayload.Number, block.Header().Number.Uint64()) + require.Equal(t, gotPayload.ExecutionPayload.BlockHash, block.Hash()) + require.Equal(t, gotPayload.ExecutionPayload.FeeRecipient, keeper.validatorAddr) + require.Empty(t, gotPayload.ExecutionPayload.Withdrawals) + }, + }, + { + name: "fail: sdk exec mode is not finalize", + setup: func(c context.Context) sdk.Context { + return sdk.UnwrapSDKContext(c).WithExecMode(sdk.ExecModeCheck) + }, + expectedError: "only allowed in finalize mode", + }, + { + name: "fail: no execution head", + setup: func(c context.Context) sdk.Context { + head, err := keeper.headTable.Get(c, executionHeadID) + require.NoError(t, err) + require.NoError(t, keeper.headTable.Delete(c, head)) + + return sdk.UnwrapSDKContext(c) + }, + createPayload: createValidPayload, + expectedError: "not found", + }, + { + name: "fail: invalid payload - wrong payload number", + createPayload: func(ctx context.Context) (*etypes.Block, engine.PayloadID, []byte) { + latestBlock, err := mockEngine.HeaderByType(ctx, ethclient.HeadLatest) + require.NoError(t, err) + latestHeight := latestBlock.Number.Uint64() + wrongNextHeight := latestHeight + 2 + + sdkCtx := sdk.UnwrapSDKContext(ctx) + appHash := common.BytesToHash(sdkCtx.BlockHeader().AppHash) + + block, execPayload := mockEngine.nextBlock(t, wrongNextHeight, uint64(time.Now().Unix()), latestBlock.Hash(), keeper.validatorAddr, &appHash) + payloadID, err := ethclient.MockPayloadID(execPayload, &appHash) + require.NoError(t, err) + + // Create execution payload message + payloadData, err := json.Marshal(execPayload) + require.NoError(t, err) + + return block, payloadID, payloadData + }, + createPrevPayloadEvents: createRandomEvents, + expectedError: "invalid proposed payload number", + }, + { + name: "fail: DequeueEligibleWithdrawals error", + setup: func(ctx context.Context) sdk.Context { + esk.EXPECT().DequeueEligibleWithdrawals(ctx).Return(nil, errors.New("failed to dequeue")) + + return sdk.UnwrapSDKContext(ctx) + }, + createPayload: createValidPayload, + expectedError: "error on withdrawals dequeue", + }, + { + name: "fail: NewPayloadV3 returns status invalid", + setup: func(ctx context.Context) sdk.Context { + esk.EXPECT().DequeueEligibleWithdrawals(ctx).Return(nil, nil) + mockEngine.forceInvalidNewPayloadV3 = true + + return sdk.UnwrapSDKContext(ctx) + }, + createPayload: createValidPayload, + createPrevPayloadEvents: createRandomEvents, + expectedError: "payload invalid", + }, + { + name: "fail: ForkchoiceUpdatedV3 returns status invalid", + setup: func(ctx context.Context) sdk.Context { + esk.EXPECT().DequeueEligibleWithdrawals(ctx).Return(nil, nil) + mockEngine.forceInvalidForkchoiceUpdatedV3 = true + + return sdk.UnwrapSDKContext(ctx) + }, + createPayload: createValidPayload, + createPrevPayloadEvents: createRandomEvents, + expectedError: "payload invalid", + }, + { + name: "fail: ProcessStakingEvents error", + setup: func(ctx context.Context) sdk.Context { + esk.EXPECT().DequeueEligibleWithdrawals(ctx).Return(nil, nil) + esk.EXPECT().ProcessStakingEvents(ctx, gomock.Any(), gomock.Any()).Return(errors.New("failed to process staking events")) + + return sdk.UnwrapSDKContext(ctx) + }, + createPayload: createValidPayload, + createPrevPayloadEvents: createRandomEvents, + expectedError: "deliver staking-related event logs", + }, + { + name: "fail: ProcessUpgradeEvents error", + setup: func(ctx context.Context) sdk.Context { + esk.EXPECT().DequeueEligibleWithdrawals(ctx).Return(nil, nil) + esk.EXPECT().ProcessStakingEvents(ctx, gomock.Any(), gomock.Any()).Return(nil) + + return sdk.UnwrapSDKContext(ctx) + }, + createPayload: createValidPayload, + createPrevPayloadEvents: func(_ context.Context, _ common.Hash) []*types.EVMEvent { + // crate invalid upgrade event to trigger ProcessUpgradeEvents failure + upgradeAbi, err := bindings.UpgradeEntrypointMetaData.GetAbi() + require.NoError(t, err, "failed to load ABI") + data, err := upgradeAbi.Events["SoftwareUpgrade"].Inputs.NonIndexed().Pack("test-upgrade", int64(0), "test-info") + require.NoError(t, err) + + return []*types.EVMEvent{{ + Address: nil, // nil address + Topics: [][]byte{types.SoftwareUpgradeEvent.ID.Bytes()}, + Data: data, + }} + }, + expectedError: "deliver upgrade-related event logs", + }, } - newPayload(ctx) - assertExecutionPayload(ctx) + for _, tc := range tcs { + //nolint:tparallel // currently, we can't run the tests in parallel due to the shared mockEngine. don't know how to fix it yet, just disable parallel for now. + t.Run(tc.name, func(t *testing.T) { + // t.Parallel() + var payloadData []byte + var payloadID engine.PayloadID + var block *etypes.Block + var events []*types.EVMEvent + + cachedCtx, _ := ctx.CacheContext() + if tc.setup != nil { + cachedCtx = tc.setup(cachedCtx) + } + if tc.createPayload != nil { + block, payloadID, payloadData = tc.createPayload(cachedCtx) + } + if tc.createPrevPayloadEvents != nil { + events = tc.createPrevPayloadEvents(cachedCtx, block.Hash()) + } + + resp, err := msgSrv.ExecutionPayload(cachedCtx, &types.MsgExecutionPayload{ + Authority: authtypes.NewModuleAddress(types.ModuleName).String(), + ExecutionPayload: payloadData, + PrevPayloadEvents: events, + }) + if tc.expectedError != "" { + require.ErrorContains(t, err, tc.expectedError) + } else { + require.NoError(t, err) + require.NotNil(t, resp) + if tc.postCheck != nil { + tc.postCheck(cachedCtx, block, payloadID) + } + } + }) + } // now lets run optimistic flow // ctx = ctx.WithBlockTime(ctx.BlockTime().Add(time.Second)) @@ -236,9 +387,9 @@ func Test_pushPayload(t *testing.T) { t.Parallel() appHash := tutil.RandomHash() - ctx := ctxWithAppHash(t, appHash) + ctx, storeKey := ctxWithAppHash(t, appHash) - mockEngine, err := newMockEngineAPI(0) + mockEngine, err := newMockEngineAPI(storeKey, 0) require.NoError(t, err) mockEngine.newPayloadV3Func = tt.args.newPayloadV3Func diff --git a/client/x/evmengine/keeper/proposal_server_internal_test.go b/client/x/evmengine/keeper/proposal_server_internal_test.go index cf093770..96c9d5ec 100644 --- a/client/x/evmengine/keeper/proposal_server_internal_test.go +++ b/client/x/evmengine/keeper/proposal_server_internal_test.go @@ -29,9 +29,6 @@ func Test_proposalServer_ExecutionPayload(t *testing.T) { cdc := getCodec(t) txConfig := authtx.NewTxConfig(cdc, nil) - mockEngine, err := newMockEngineAPI(0) - require.NoError(t, err) - ctrl := gomock.NewController(t) mockClient := mock.NewMockClient(ctrl) ak := moduletestutil.NewMockAccountKeeper(ctrl) @@ -40,9 +37,10 @@ func Test_proposalServer_ExecutionPayload(t *testing.T) { esk.EXPECT().PeekEligibleWithdrawals(gomock.Any()).Return(nil, nil).AnyTimes() - sdkCtx, storeService := setupCtxStore(t, &cmtproto.Header{AppHash: tutil.RandomHash().Bytes()}) + sdkCtx, storeKey, storeService := setupCtxStore(t, &cmtproto.Header{AppHash: tutil.RandomHash().Bytes()}) sdkCtx = sdkCtx.WithExecMode(sdk.ExecModeFinalize) - + mockEngine, err := newMockEngineAPI(storeKey, 0) + require.NoError(t, err) keeper, err := NewKeeper(cdc, storeService, &mockEngine, mockClient, txConfig, ak, esk, uk) require.NoError(t, err) populateGenesisHead(sdkCtx, t, keeper) diff --git a/client/x/evmengine/keeper/upgrades_internal_test.go b/client/x/evmengine/keeper/upgrades_internal_test.go index 9f7a7b6a..722c69a1 100644 --- a/client/x/evmengine/keeper/upgrades_internal_test.go +++ b/client/x/evmengine/keeper/upgrades_internal_test.go @@ -254,8 +254,6 @@ func setupTestEnvironment(t *testing.T) (*Keeper, sdk.Context, *gomock.Controlle t.Helper() cdc := getCodec(t) txConfig := authtx.NewTxConfig(cdc, nil) - mockEngine, err := newMockEngineAPI(0) - require.NoError(t, err) cmtAPI := newMockCometAPI(t, nil) header := cmtproto.Header{Height: 1, AppHash: tutil.RandomHash().Bytes(), ProposerAddress: cmtAPI.validatorSet.Validators[0].Address} @@ -265,7 +263,9 @@ func setupTestEnvironment(t *testing.T) (*Keeper, sdk.Context, *gomock.Controlle esk := moduletestutil.NewMockEvmStakingKeeper(ctrl) uk := moduletestutil.NewMockUpgradeKeeper(ctrl) - ctx, storeService := setupCtxStore(t, &header) + ctx, storeKey, storeService := setupCtxStore(t, &header) + mockEngine, err := newMockEngineAPI(storeKey, 0) + require.NoError(t, err) keeper, err := NewKeeper(cdc, storeService, &mockEngine, mockClient, txConfig, ak, esk, uk) require.NoError(t, err) diff --git a/client/x/evmstaking/keeper/keeper_test.go b/client/x/evmstaking/keeper/keeper_test.go index aee98e5b..cfbefe7b 100644 --- a/client/x/evmstaking/keeper/keeper_test.go +++ b/client/x/evmstaking/keeper/keeper_test.go @@ -127,7 +127,7 @@ func (s *TestSuite) SetupTest() { s.Require().NoError(s.StakingKeeper.SetParams(s.Ctx, stypes.DefaultParams())) // emvstaking keeper - ethCl, err := ethclient.NewEngineMock() + ethCl, err := ethclient.NewEngineMock(evmstakingKey) s.Require().NoError(err) evmstakingKeeper := keeper.NewKeeper( marshaler, diff --git a/lib/ethclient/enginemock.go b/lib/ethclient/enginemock.go index fe4251b2..960a961d 100644 --- a/lib/ethclient/enginemock.go +++ b/lib/ethclient/enginemock.go @@ -1,6 +1,7 @@ package ethclient import ( + "bytes" "context" "crypto/sha256" "math/big" @@ -9,7 +10,10 @@ import ( "testing" "time" + storetypes "cosmossdk.io/store/types" + "github.com/cometbft/cometbft/crypto" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/accounts/abi" "github.com/ethereum/go-ethereum/accounts/abi/bind" @@ -17,6 +21,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/params" + "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/trie" fuzz "github.com/google/gofuzz" @@ -26,6 +31,11 @@ import ( "github.com/piplabs/story/lib/log" ) +const ( + // headKey is the key to store the head block. + headKey = "head" +) + type payloadArgs struct { params engine.ExecutableData beaconRoot *common.Hash @@ -42,8 +52,15 @@ type engineMock struct { fuzzer *fuzz.Fuzzer randomErrs float64 - mu sync.Mutex - head *types.Block + mu sync.Mutex + // storeKey is added to make engineMock dependent on sdk.Context for better testability. + // By using storeKey, engineMock's methods can interact with the sdk.Context's store, + // allowing for independent tests that do not interfere with each other’s store state. + storeKey *storetypes.KVStoreKey + // headKey is the key to store the head block. + headKey []byte + genesisBlock *types.Block + // consider the following maps also dependent on sdk.Context if needed. pendingLogs map[common.Address][]types.Log logs map[common.Hash][]types.Log payloads map[engine.PayloadID]payloadArgs @@ -122,18 +139,20 @@ func MockGenesisBlock() (*types.Block, error) { // NewEngineMock returns a new mock engine API client. // Note only some methods are implemented, it will panic if you call an unimplemented method. -func NewEngineMock(opts ...func(mock *engineMock)) (EngineClient, error) { +func NewEngineMock(key *storetypes.KVStoreKey, opts ...func(mock *engineMock)) (EngineClient, error) { genesisBlock, err := MockGenesisBlock() if err != nil { return nil, err } m := &engineMock{ - fuzzer: NewFuzzer(int64(genesisBlock.Time())), - head: genesisBlock, - pendingLogs: make(map[common.Address][]types.Log), - payloads: make(map[engine.PayloadID]payloadArgs), - logs: make(map[common.Hash][]types.Log), + fuzzer: NewFuzzer(int64(genesisBlock.Time())), + storeKey: key, + headKey: []byte(headKey), + genesisBlock: genesisBlock, + pendingLogs: make(map[common.Address][]types.Log), + payloads: make(map[engine.PayloadID]payloadArgs), + logs: make(map[common.Hash][]types.Log), } for _, opt := range opts { opt(m) @@ -154,6 +173,39 @@ func (m *engineMock) maybeErr(ctx context.Context) error { return nil } +// getHeadBlock returns the head block from the store. +func (m *engineMock) getHeadBlock(ctx context.Context) (*types.Block, error) { + sdkCtx := sdk.UnwrapSDKContext(ctx) + headBz := sdkCtx.KVStore(m.storeKey).Get(m.headKey) + if headBz == nil { + // Set genesis block as head + if err := m.setHeadBlock(ctx, m.genesisBlock); err != nil { + return nil, err + } + + return m.genesisBlock, nil + } + var headBlock types.Block + if err := rlp.DecodeBytes(headBz, &headBlock); err != nil { + return nil, errors.Wrap(err, "decode head") + } + + return &headBlock, nil +} + +// setHeadBlock sets the head block in the store. +func (m *engineMock) setHeadBlock(ctx context.Context, head *types.Block) error { + buf := new(bytes.Buffer) + if err := head.EncodeRLP(buf); err != nil { + return errors.Wrap(err, "encode head") + } + headBz := buf.Bytes() + sdkCtx := sdk.UnwrapSDKContext(ctx) + sdkCtx.KVStore(m.storeKey).Set(m.headKey, headBz) + + return nil +} + func (m *engineMock) FilterLogs(_ context.Context, q ethereum.FilterQuery) ([]types.Log, error) { m.mu.Lock() defer m.mu.Unlock() @@ -195,7 +247,12 @@ func (m *engineMock) BlockNumber(ctx context.Context) (uint64, error) { m.mu.Lock() defer m.mu.Unlock() - return m.head.NumberU64(), nil + headBlock, err := m.getHeadBlock(ctx) + if err != nil { + return 0, err + } + + return headBlock.NumberU64(), nil } func (m *engineMock) HeaderByNumber(ctx context.Context, height *big.Int) (*types.Header, error) { @@ -228,11 +285,15 @@ func (m *engineMock) HeaderByHash(ctx context.Context, hash common.Hash) (*types m.mu.Lock() defer m.mu.Unlock() - if hash != m.head.Hash() { + head, err := m.getHeadBlock(ctx) + if err != nil { + return nil, err + } + if hash != head.Hash() { return nil, errors.New("only head hash supported") // Only support latest block } - return m.head.Header(), nil + return head.Header(), nil } func (m *engineMock) BlockByNumber(ctx context.Context, number *big.Int) (*types.Block, error) { @@ -243,15 +304,19 @@ func (m *engineMock) BlockByNumber(ctx context.Context, number *big.Int) (*types m.mu.Lock() defer m.mu.Unlock() + head, err := m.getHeadBlock(ctx) + if err != nil { + return nil, err + } if number == nil { - return m.head, nil + return head, nil } - if number.Cmp(m.head.Number()) != 0 { + if number.Cmp(head.Number()) != 0 { return nil, errors.New("block not found") // Only support latest block } - return m.head, nil + return head, nil } func (m *engineMock) NewPayloadV3(ctx context.Context, params engine.ExecutableData, _ []common.Hash, beaconRoot *common.Hash) (engine.PayloadStatusV1, error) { @@ -262,6 +327,8 @@ func (m *engineMock) NewPayloadV3(ctx context.Context, params engine.ExecutableD m.mu.Lock() defer m.mu.Unlock() + // if Withdrawals is nil, cannot rlp encode and decode properly. + params.Withdrawals = make([]*types.Withdrawal, 0) args := payloadArgs{ params: params, beaconRoot: beaconRoot, @@ -300,9 +367,14 @@ func (m *engineMock) ForkchoiceUpdatedV3(ctx context.Context, update engine.Fork }, } + head, err := m.getHeadBlock(ctx) + if err != nil { + return engine.ForkChoiceResponse{}, err + } + // Maybe update head //nolint: nestif // this is a mock it's fine - if m.head.Hash() != update.HeadBlockHash { + if head.Hash() != update.HeadBlockHash { var found bool for _, args := range m.payloads { block, err := engine.ExecutableDataToBlock(args.params, nil, args.beaconRoot) @@ -314,11 +386,13 @@ func (m *engineMock) ForkchoiceUpdatedV3(ctx context.Context, update engine.Fork continue } - if err := verifyChild(m.head, block); err != nil { + if err := verifyChild(head, block); err != nil { return engine.ForkChoiceResponse{}, err } - m.head = block + if err := m.setHeadBlock(ctx, block); err != nil { + return engine.ForkChoiceResponse{}, err + } found = true id, err := MockPayloadID(args.params, args.beaconRoot) @@ -331,13 +405,13 @@ func (m *engineMock) ForkchoiceUpdatedV3(ctx context.Context, update engine.Fork } if !found { return engine.ForkChoiceResponse{}, errors.New("forkchoice block not found", - log.Hex7("forkchoice", m.head.Hash().Bytes())) + log.Hex7("forkchoice", head.Hash().Bytes())) } } // If we have payload attributes, make a new payload if attrs != nil { - payload, err := MakePayload(m.fuzzer, m.head.NumberU64()+1, + payload, err := MakePayload(m.fuzzer, head.NumberU64()+1, attrs.Timestamp, update.HeadBlockHash, attrs.SuggestedFeeRecipient, attrs.Random, attrs.BeaconRoot) if err != nil { return engine.ForkChoiceResponse{}, err @@ -356,7 +430,7 @@ func (m *engineMock) ForkchoiceUpdatedV3(ctx context.Context, update engine.Fork } log.Debug(ctx, "Engine mock forkchoice updated", - "height", m.head.NumberU64(), + "height", head.NumberU64(), log.Hex7("forkchoice", update.HeadBlockHash.Bytes()), ) @@ -410,7 +484,7 @@ func MakePayload(fuzzer *fuzz.Fuzzer, height uint64, timestamp uint64, parentHas header.ParentBeaconRoot = beaconRoot // Convert header to block - block := types.NewBlock(&header, nil, nil, trie.NewStackTrie(nil)) + block := types.NewBlock(&header, &types.Body{Withdrawals: make([]*types.Withdrawal, 0)}, nil, trie.NewStackTrie(nil)) // Convert block to payload env := engine.BlockToExecutableData(block, big.NewInt(0), nil)