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

feat(evmengine): use current finalized block events & remove prev events #514

Draft
wants to merge 6 commits into
base: release/1.2
Choose a base branch
from
Draft
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
18 changes: 18 additions & 0 deletions client/app/upgrades/v1_3_0/constants.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//nolint:revive,stylecheck // versioning
package v_1_3_0

import (
storetypes "cosmossdk.io/store/types"

"github.com/piplabs/story/client/app/upgrades"
)

const (
UpgradeName = "v1.3.0"
)

var Upgrade = upgrades.Upgrade{
UpgradeName: UpgradeName,
CreateUpgradeHandler: CreateUpgradeHandler,
StoreUpgrades: storetypes.StoreUpgrades{},
}
61 changes: 61 additions & 0 deletions client/app/upgrades/v1_3_0/upgrades.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//nolint:revive,stylecheck // versioning
package v_1_3_0

import (
"context"

upgradetypes "cosmossdk.io/x/upgrade/types"

"github.com/cosmos/cosmos-sdk/types/module"
"github.com/ethereum/go-ethereum/common"

"github.com/piplabs/story/client/app/keepers"
"github.com/piplabs/story/lib/errors"
"github.com/piplabs/story/lib/log"
)

func CreateUpgradeHandler(
_ *module.Manager,
_ module.Configurator,
keepers *keepers.Keepers,
) upgradetypes.UpgradeHandler {
return func(ctx context.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
log.Info(ctx, "Start upgrade v1.3.0")

// When this handler is triggered at the beginning of block X, we must process the events from block X-1.
// Otherwise, events from block X-1 will be discarded as the upgrade changes the logic to process events of the
// current block once finalized.

// Get the block hash and events of the previous block.
head, err := keepers.EVMEngKeeper.GetExecutionHead(ctx)
if err != nil {
return nil, errors.Wrap(err, "get execution head")
}
lastBlockHeight := head.GetBlockHeight()
lastBlockHash := common.BytesToHash(head.GetBlockHash())

events, err := keepers.EVMEngKeeper.EvmEvents(ctx, lastBlockHash)
if err != nil {
return nil, errors.Wrap(err, "fetch evm event logs")
}

// Deliver all the payload log events of the block X-1
if err := keepers.EvmStakingKeeper.ProcessStakingEvents(ctx, lastBlockHeight, events); err != nil {
return nil, errors.Wrap(err, "deliver staking-related event logs")
}
if err := keepers.EVMEngKeeper.ProcessUpgradeEvents(ctx, lastBlockHeight, events); err != nil {
return nil, errors.Wrap(err, "deliver upgrade-related event logs")
}
if err := keepers.EVMEngKeeper.ProcessUbiEvents(ctx, lastBlockHeight, events); err != nil {
return nil, errors.Wrap(err, "deliver ubi-related event logs")
}

if err := keepers.EVMEngKeeper.UpdateExecutionHeadWithBlock(ctx, lastBlockHeight, lastBlockHash, head.GetBlockTime()); err != nil {
return nil, errors.Wrap(err, "update execution head")
}

log.Info(ctx, "Upgrade v1.3.0 complete")

return vm, nil
}
}
2 changes: 1 addition & 1 deletion client/proto/story/evmengine/v1/types/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ message MsgExecutionPayload {
option (cosmos.msg.v1.signer) = "authority";
string authority = 1;
bytes execution_payload = 2;
repeated EVMEvent prev_payload_events = 3;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please mark reserve or deprecated, not removing the field for future backward or forward compatibility.
It can cause unexpected issue in deserialization.
https://stackoverflow.com/questions/71782502/what-happened-if-you-delete-a-field-on-protobuf

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 5b55016

repeated EVMEvent prev_payload_events = 3 [deprecated = true];
}

message ExecutionPayloadResponse {}
Expand Down
21 changes: 3 additions & 18 deletions client/x/evmengine/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,23 +137,10 @@ func (k *Keeper) PrepareProposal(ctx sdk.Context, req *abci.RequestPreparePropos
return nil, errors.Wrap(err, "encode")
}

// First, collect all vote extension msgs from the vote provider.
// voteMsgs, err := k.voteProvider.PrepareVotes(ctx, req.LocalLastCommit)
// if err != nil {
// return nil, errors.Wrap(err, "prepare votes")
//}

// Next, collect all prev payload evm event logs.
evmEvents, err := k.evmEvents(ctx, payloadResp.ExecutionPayload.ParentHash)
if err != nil {
return nil, errors.Wrap(err, "prepare evm event logs")
}

// Then construct the execution payload message.
payloadMsg := &types.MsgExecutionPayload{
Authority: authtypes.NewModuleAddress(types.ModuleName).String(),
ExecutionPayload: payloadData,
PrevPayloadEvents: evmEvents,
Authority: authtypes.NewModuleAddress(types.ModuleName).String(),
ExecutionPayload: payloadData,
}

// Combine all the votes messages and the payload message into a single transaction.
Expand All @@ -171,8 +158,6 @@ func (k *Keeper) PrepareProposal(ctx sdk.Context, req *abci.RequestPreparePropos
log.Info(ctx, "Proposing new block",
"height", req.Height,
log.Hex7("execution_block_hash", payloadResp.ExecutionPayload.BlockHash[:]),
// "vote_msgs", len(voteMsgs),
"evm_events", len(evmEvents),
)

return &abci.ResponsePrepareProposal{Txs: [][]byte{tx}}, nil
Expand Down Expand Up @@ -253,7 +238,7 @@ func (k *Keeper) PostFinalize(ctx sdk.Context) error {
// startBuild triggers the building of a new execution payload on top of the current execution head.
// It returns the EngineAPI response which contains a status and payload ID.
func (k *Keeper) startBuild(ctx context.Context, feeRecipient common.Address, withdrawals []*etypes.Withdrawal, appHash common.Hash, timestamp time.Time) (engine.ForkChoiceResponse, error) {
head, err := k.getExecutionHead(ctx)
head, err := k.GetExecutionHead(ctx)
if err != nil {
return engine.ForkChoiceResponse{}, errors.Wrap(err, "latest execution block")
}
Expand Down
13 changes: 7 additions & 6 deletions client/x/evmengine/keeper/abci_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {

// get the genesis block to build on top of
// Get the parent block we will build on top of
head, err := keeper.getExecutionHead(ctx)
head, err := keeper.GetExecutionHead(ctx)
require.NoError(t, err)

req := &abci.RequestPrepareProposal{
Expand Down Expand Up @@ -536,10 +536,6 @@ func assertExecutablePayload(t *testing.T, msg sdk.Msg, ts int64, blockHash comm
require.Equal(t, payload.FeeRecipient, validatorAddr)
require.Empty(t, payload.Withdrawals)
require.Equal(t, payload.Number, height)

// require.Len(t, executionPayload.PrevPayloadEvents, 1)
// evmLog := executionPayload.PrevPayloadEvents[0]
// require.Equal(t, evmLog.Address, zeroAddr.Bytes())
}

func ctxWithAppHash(t *testing.T, appHash common.Hash) (context.Context, *storetypes.KVStoreKey) {
Expand Down Expand Up @@ -599,6 +595,7 @@ type mockEngineAPI struct {
forkchoiceUpdatedV3Func func(context.Context, eengine.ForkchoiceStateV1, *eengine.PayloadAttributes) (eengine.ForkChoiceResponse, error)
getPayloadV3Func func(context.Context, eengine.PayloadID) (*eengine.ExecutionPayloadEnvelope, error)
newPayloadV3Func func(context.Context, eengine.ExecutableData, []common.Hash, *common.Hash) (eengine.PayloadStatusV1, error)
filterLogsFunc func(context.Context, ethereum.FilterQuery) ([]types.Log, error)
// forceInvalidNewPayloadV3 forces the NewPayloadV3 returns an invalid status.
forceInvalidNewPayloadV3 bool
// forceInvalidForkchoiceUpdatedV3 forces the ForkchoiceUpdatedV3 returns an invalid status.
Expand Down Expand Up @@ -683,7 +680,11 @@ func (m mockEngineAPI) maybeSync() (eengine.PayloadStatusV1, bool) {
}
}

func (mockEngineAPI) FilterLogs(context.Context, ethereum.FilterQuery) ([]types.Log, error) {
func (m mockEngineAPI) FilterLogs(ctx context.Context, q ethereum.FilterQuery) ([]types.Log, error) {
if m.filterLogsFunc != nil {
return m.filterLogsFunc(ctx, q)
}

return nil, nil
}

Expand Down
24 changes: 17 additions & 7 deletions client/x/evmengine/keeper/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ func (k *Keeper) InsertGenesisHead(ctx context.Context, executionBlockHash []byt
return nil
}

// getExecutionHead returns the current execution head.
func (k *Keeper) getExecutionHead(ctx context.Context) (*ExecutionHead, error) {
// GetExecutionHead returns the current execution head.
func (k *Keeper) GetExecutionHead(ctx context.Context) (*ExecutionHead, error) {
head, err := k.headTable.Get(ctx, executionHeadID)
if err != nil {
return nil, errors.Wrap(err, "get execution head")
Expand All @@ -44,14 +44,24 @@ func (k *Keeper) getExecutionHead(ctx context.Context) (*ExecutionHead, error) {
return head, nil
}

// updateExecutionHead updates the execution head with the given payload.
func (k *Keeper) updateExecutionHead(ctx context.Context, payload engine.ExecutableData) error {
// UpdateExecutionHead updates the execution head with the given payload.
func (k *Keeper) UpdateExecutionHead(ctx context.Context, payload engine.ExecutableData) error {
return k.updateExecutionHead(ctx, payload.Number, payload.BlockHash, payload.Timestamp)
}

func (k *Keeper) UpdateExecutionHeadWithBlock(ctx context.Context, blockHeight uint64, blockHash common.Hash, blockTime uint64) error {
return k.updateExecutionHead(ctx, blockHeight, blockHash, blockTime)
}

func (k *Keeper) updateExecutionHead(
ctx context.Context, blockHeight uint64, blockHash common.Hash, blockTime uint64,
) error {
head := &ExecutionHead{
Id: executionHeadID,
CreatedHeight: uint64(sdk.UnwrapSDKContext(ctx).BlockHeight()),
BlockHeight: payload.Number,
BlockHash: payload.BlockHash.Bytes(),
BlockTime: payload.Timestamp,
BlockHeight: blockHeight,
BlockHash: blockHash.Bytes(),
BlockTime: blockTime,
}

err := k.headTable.Update(ctx, head)
Expand Down
12 changes: 6 additions & 6 deletions client/x/evmengine/keeper/db_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func TestKeeper_InsertGenesisHead(t *testing.T) {
ctx, keeper := createTestKeeper(t)

// make sure the execution head does not exist
_, err := keeper.getExecutionHead(ctx)
_, err := keeper.GetExecutionHead(ctx)
require.Error(t, err, "execution head should not exist")

// insert genesis head
Expand All @@ -23,7 +23,7 @@ func TestKeeper_InsertGenesisHead(t *testing.T) {
require.NoError(t, err)

// make sure the execution head is set correctly
head, err := keeper.getExecutionHead(ctx)
head, err := keeper.GetExecutionHead(ctx)
require.NoError(t, err)
require.NotNil(t, head, "execution head should exist")
require.Equal(t, dummyBlockHash, head.GetBlockHash(), "block hash should match")
Expand All @@ -39,7 +39,7 @@ func TestKeeper_updateExecutionHead(t *testing.T) {
ctx, keeper := createTestKeeper(t)

// make sure the execution head does not exist
_, err := keeper.getExecutionHead(ctx)
_, err := keeper.GetExecutionHead(ctx)
require.Error(t, err, "execution head should not exist")

// insert genesis head
Expand All @@ -48,21 +48,21 @@ func TestKeeper_updateExecutionHead(t *testing.T) {
require.NoError(t, err)

// make sure the execution head is set correctly
head, err := keeper.getExecutionHead(ctx)
head, err := keeper.GetExecutionHead(ctx)
require.NoError(t, err)
require.NotNil(t, head, "execution head should exist")

// update the execution head
newBlockHash := common.BytesToHash([]byte("new hash"))
err = keeper.updateExecutionHead(ctx, engine.ExecutableData{
err = keeper.UpdateExecutionHead(ctx, engine.ExecutableData{
Number: 100,
BlockHash: newBlockHash,
Timestamp: 0,
})
require.NoError(t, err)

// make sure the execution head is updated correctly
head, err = keeper.getExecutionHead(ctx)
head, err = keeper.GetExecutionHead(ctx)
require.NoError(t, err)
require.NotNil(t, head, "execution head should exist")
require.Equal(t, newBlockHash.Bytes(), head.GetBlockHash(), "block hash should match")
Expand Down
12 changes: 5 additions & 7 deletions client/x/evmengine/keeper/evmmsgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import (
clog "github.com/piplabs/story/lib/log"
)

// evmEvents returns selected EVM log events from the provided block hash.
func (k *Keeper) evmEvents(ctx context.Context, blockHash common.Hash) ([]*types.EVMEvent, error) {
// EvmEvents returns selected EVM log events from the provided block hash.
func (k *Keeper) EvmEvents(ctx context.Context, blockHash common.Hash) ([]*types.EVMEvent, error) {
var logs []ethtypes.Log
err := retryForever(ctx, func(ctx context.Context) (fetched bool, err error) {
logs, err = k.engineCl.FilterLogs(ctx, ethereum.FilterQuery{
Expand Down Expand Up @@ -43,18 +43,16 @@ func (k *Keeper) evmEvents(ctx context.Context, blockHash common.Hash) ([]*types
for _, t := range l.Topics {
topics = append(topics, t.Bytes())
}
events = append(events, &types.EVMEvent{
event := &types.EVMEvent{
Address: l.Address.Bytes(),
Topics: topics,
Data: l.Data,
TxHash: l.TxHash.Bytes(),
})
}

for _, event := range events {
}
if err := event.Verify(); err != nil {
return nil, errors.Wrap(err, "verify event")
}
events = append(events, event)
}

return events, nil
Expand Down
2 changes: 1 addition & 1 deletion client/x/evmengine/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (k *Keeper) parseAndVerifyProposedPayload(ctx context.Context, msg *types.M
}

// Fetch the latest execution head from the local keeper DB.
head, err := k.getExecutionHead(ctx)
head, err := k.GetExecutionHead(ctx)
if err != nil {
return engine.ExecutableData{}, errors.Wrap(err, "latest execution block")
}
Expand Down
16 changes: 8 additions & 8 deletions client/x/evmengine/keeper/keeper_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func TestKeeper_parseAndVerifyProposedPayload(t *testing.T) {
{
name: "fail: payload parent hash is not equal to head hash",
msg: func(c context.Context) *types.MsgExecutionPayload {
execHead, err := keeper.getExecutionHead(c)
execHead, err := keeper.GetExecutionHead(c)
require.NoError(t, err)

payload, err := ethclient.MakePayload(fuzzer, execHead.GetBlockHeight()+1, uint64(now.Unix()), common.Hash{}, common.Address{}, common.Hash{}, &common.Hash{})
Expand All @@ -184,7 +184,7 @@ func TestKeeper_parseAndVerifyProposedPayload(t *testing.T) {
{
name: "fail: invalid payload timestamp",
msg: func(c context.Context) *types.MsgExecutionPayload {
execHead, err := keeper.getExecutionHead(c)
execHead, err := keeper.GetExecutionHead(c)
require.NoError(t, err)
weekAgo := execHead.GetBlockTime() - 604800

Expand All @@ -204,7 +204,7 @@ func TestKeeper_parseAndVerifyProposedPayload(t *testing.T) {
{
name: "fail: invalid payload random",
msg: func(c context.Context) *types.MsgExecutionPayload {
execHead, err := keeper.getExecutionHead(c)
execHead, err := keeper.GetExecutionHead(c)
require.NoError(t, err)

payload, err := ethclient.MakePayload(fuzzer, execHead.GetBlockHeight()+1, uint64(now.Unix()), execHead.Hash(), common.Address{}, common.Hash{}, &common.Hash{})
Expand All @@ -223,7 +223,7 @@ func TestKeeper_parseAndVerifyProposedPayload(t *testing.T) {
{
name: "fail: invalid authority",
msg: func(c context.Context) *types.MsgExecutionPayload {
execHead, err := keeper.getExecutionHead(c)
execHead, err := keeper.GetExecutionHead(c)
require.NoError(t, err)

payload, err := ethclient.MakePayload(fuzzer, execHead.GetBlockHeight()+1, uint64(now.Unix()), execHead.Hash(), common.Address{}, execHead.Hash(), &common.Hash{})
Expand All @@ -242,7 +242,7 @@ func TestKeeper_parseAndVerifyProposedPayload(t *testing.T) {
{
name: "pass: valid payload",
msg: func(c context.Context) *types.MsgExecutionPayload {
execHead, err := keeper.getExecutionHead(c)
execHead, err := keeper.GetExecutionHead(c)
require.NoError(t, err)

payload, err := ethclient.MakePayload(fuzzer, execHead.GetBlockHeight()+1, uint64(now.Unix()), execHead.Hash(), common.Address{}, execHead.Hash(), &common.Hash{})
Expand All @@ -260,10 +260,10 @@ func TestKeeper_parseAndVerifyProposedPayload(t *testing.T) {
{
name: "pass: valid payload when consensus block time is less than execution block time",
setup: func(c context.Context) sdk.Context {
execHead, err := keeper.getExecutionHead(c)
execHead, err := keeper.GetExecutionHead(c)
require.NoError(t, err)
// update execution head with current block time
err = keeper.updateExecutionHead(c, engine.ExecutableData{
err = keeper.UpdateExecutionHead(c, engine.ExecutableData{
Number: execHead.GetBlockHeight(),
BlockHash: common.BytesToHash(execHead.GetBlockHash()),
Timestamp: uint64(now.Unix()),
Expand All @@ -277,7 +277,7 @@ func TestKeeper_parseAndVerifyProposedPayload(t *testing.T) {
return sdkCtx
},
msg: func(c context.Context) *types.MsgExecutionPayload {
execHead, err := keeper.getExecutionHead(c)
execHead, err := keeper.GetExecutionHead(c)
require.NoError(t, err)

payload, err := ethclient.MakePayload(fuzzer, execHead.GetBlockHeight()+1, execHead.GetBlockTime()+1, execHead.Hash(), common.Address{}, execHead.Hash(), &common.Hash{})
Expand Down
Loading