Skip to content

Commit

Permalink
refactor(octane/evmengine): simplify event processing
Browse files Browse the repository at this point in the history
  • Loading branch information
corverroos committed Jan 8, 2025
1 parent f40a286 commit f9ac08f
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 26 deletions.
3 changes: 3 additions & 0 deletions lib/feature/feature.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
const (
// FlagEVMStakingModule enables the wip EVM Staking Module feature.
FlagEVMStakingModule Flag = "evm-staking-module"
// FlagSimpleEVMEvents enables the simplified EVM events refactor.
FlagSimpleEVMEvents Flag = "simple-evm-events"
)

// enabledFlags holds all globally enabled feature flags. The reason for having it is that
Expand All @@ -20,6 +22,7 @@ var enabledFlags sync.Map

var allFlags = map[Flag]bool{
FlagEVMStakingModule: true,
FlagSimpleEVMEvents: true,
}

// Flag is a feature flag.
Expand Down
12 changes: 8 additions & 4 deletions octane/evmengine/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/omni-network/omni/lib/cast"
"github.com/omni-network/omni/lib/errors"
"github.com/omni-network/omni/lib/feature"
"github.com/omni-network/omni/lib/log"
"github.com/omni-network/omni/octane/evmengine/types"

Expand Down Expand Up @@ -133,10 +134,13 @@ func (k *Keeper) PrepareProposal(ctx sdk.Context, req *abci.RequestPreparePropos
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")
// Next, collect all prev payload evm event logs (only if simple-evm-events feature not enabled).
var evmEvents []types.EVMEvent
if !feature.FlagSimpleEVMEvents.Enabled(ctx) {
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.
Expand Down
65 changes: 59 additions & 6 deletions octane/evmengine/keeper/abci_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
attesttypes "github.com/omni-network/omni/halo/attest/types"
"github.com/omni-network/omni/lib/errors"
"github.com/omni-network/omni/lib/ethclient"
"github.com/omni-network/omni/lib/feature"
"github.com/omni-network/omni/lib/k1util"
"github.com/omni-network/omni/lib/tutil"
etypes "github.com/omni-network/omni/octane/evmengine/types"
Expand Down Expand Up @@ -202,7 +203,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {

for _, msg := range tx.GetMsgs() {
if _, ok := msg.(*etypes.MsgExecutionPayload); ok {
assertExecutablePayload(t, msg, ts.Unix(), nextBlock.Hash(), frp, uint64(req.Height), 0)
assertExecutablePayload(t, msg, ts.Unix(), nextBlock.Hash(), frp, uint64(req.Height), 0, 1)
}
}
})
Expand Down Expand Up @@ -251,7 +252,7 @@ func TestKeeper_PrepareProposal(t *testing.T) {
// assert that the message is an executable payload
for _, msg := range tx.GetMsgs() {
if _, ok := msg.(*etypes.MsgExecutionPayload); ok {
assertExecutablePayload(t, msg, req.Time.Unix(), headHash, frp, head.GetBlockHeight()+1, 0)
assertExecutablePayload(t, msg, req.Time.Unix(), headHash, frp, head.GetBlockHeight()+1, 0, 1)
}
if msgDelegate, ok := msg.(*stypes.MsgDelegate); ok {
require.Equal(t, msgDelegate.Amount, sdk.NewInt64Coin("stake", 100))
Expand Down Expand Up @@ -329,10 +330,59 @@ func TestKeeper_PrepareProposal(t *testing.T) {
found = true
expected := [][]byte{commitment1, commitment2}
require.EqualValues(t, expected, payload.BlobCommitments)
assertExecutablePayload(t, msg, req.Time.Unix(), headHash, frp, head.GetBlockHeight()+1, len(expected))
assertExecutablePayload(t, msg, req.Time.Unix(), headHash, frp, head.GetBlockHeight()+1, len(expected), 1)
}
require.True(t, found)
})

t.Run("TestSimpleEvmEvents", func(t *testing.T) {
t.Parallel()
// setup dependencies
ctx, storeService := setupCtxStore(t, nil)
ctx = ctx.WithContext(feature.WithFlag(ctx, feature.FlagSimpleEVMEvents))

cdc := getCodec(t)
txConfig := authtx.NewTxConfig(cdc, nil)

mockEngine, err := newMockEngineAPI(0)
require.NoError(t, err)

ap := mockAddressProvider{
address: common.BytesToAddress([]byte("test")),
}
frp := newRandomFeeRecipientProvider()
keeper, err := NewKeeper(cdc, storeService, &mockEngine, txConfig, ap, frp, mockEventProc{})
require.NoError(t, err)
keeper.SetVoteProvider(mockVEProvider{})
populateGenesisHead(ctx, t, keeper)

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

req := &abci.RequestPrepareProposal{
Txs: nil,
Height: int64(2),
Time: time.Now(),
MaxTxBytes: cmttypes.MaxBlockSizeBytes,
}

resp, err := keeper.PrepareProposal(withRandomErrs(t, ctx), req)
tutil.RequireNoError(t, err)
require.NotNil(t, resp)

// decode the txn and get the messages
tx, err := txConfig.TxDecoder()(resp.Txs[0])
require.NoError(t, err)

for _, msg := range tx.GetMsgs() {
if _, ok := msg.(*etypes.MsgExecutionPayload); ok {
assertExecutablePayload(t, msg, req.Time.Unix(), headHash, frp, head.GetBlockHeight()+1, 0, 0)
}
}
})
}

func TestOptimistic(t *testing.T) {
Expand Down Expand Up @@ -402,6 +452,7 @@ func assertExecutablePayload(
frp etypes.FeeRecipientProvider,
height uint64,
blobs int,
events int,
) {
t.Helper()
executionPayload, ok := msg.(*etypes.MsgExecutionPayload)
Expand All @@ -417,9 +468,11 @@ func assertExecutablePayload(
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())
require.Len(t, executionPayload.PrevPayloadEvents, events)
if events > 0 {
evmLog := executionPayload.PrevPayloadEvents[0]
require.Equal(t, evmLog.Address, zeroAddr.Bytes())
}

require.Len(t, executionPayload.BlobCommitments, blobs)
}
Expand Down
27 changes: 22 additions & 5 deletions octane/evmengine/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/omni-network/omni/lib/cast"
"github.com/omni-network/omni/lib/errors"
"github.com/omni-network/omni/lib/ethclient"
"github.com/omni-network/omni/lib/feature"
"github.com/omni-network/omni/lib/log"
"github.com/omni-network/omni/lib/umath"
"github.com/omni-network/omni/octane/evmengine/types"
Expand Down Expand Up @@ -89,13 +90,29 @@ func (s msgServer) ExecutionPayload(ctx context.Context, msg *types.MsgExecution
return nil, err
}

parentHeight, ok := umath.Subtract(payload.Number, 1)
if !ok { // payload.Number == 0
return nil, errors.New("invalid zero payload number")
var events []types.EVMEvent
var eventsHeight uint64
var eventsBlockHash common.Hash

if feature.FlagSimpleEVMEvents.Enabled(ctx) {
eventsHeight = payload.Number
eventsBlockHash = payload.BlockHash
events, err = s.evmEvents(ctx, payload.BlockHash)
if err != nil {
return nil, errors.Wrap(err, "fetch evm event logs")
}
} else {
parentHeight, ok := umath.Subtract(payload.Number, 1)
if !ok { // payload.Number == 0
return nil, errors.New("invalid zero payload number")
}

eventsHeight = parentHeight
eventsBlockHash = payload.ParentHash
events = msg.PrevPayloadEvents
}

// Deliver all the previous payload log events
if err := s.deliverEvents(ctx, parentHeight, payload.ParentHash, msg.PrevPayloadEvents); err != nil {
if err := s.deliverEvents(ctx, eventsHeight, eventsBlockHash, events); err != nil {
return nil, errors.Wrap(err, "deliver event logs")
}

Expand Down
9 changes: 7 additions & 2 deletions octane/evmengine/keeper/msg_server_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/omni-network/omni/lib/errors"
"github.com/omni-network/omni/lib/ethclient"
"github.com/omni-network/omni/lib/feature"
"github.com/omni-network/omni/lib/k1util"
"github.com/omni-network/omni/lib/tutil"
"github.com/omni-network/omni/octane/evmengine/types"
Expand Down Expand Up @@ -91,6 +92,10 @@ func Test_msgServer_ExecutionPayload(t *testing.T) {
events, err := keeper.evmEvents(ctx, block.Hash())
require.NoError(t, err)

if feature.FlagSimpleEVMEvents.Enabled(ctx) {
events = nil
}

resp, err := msgSrv.ExecutionPayload(ctx, &types.MsgExecutionPayload{
Authority: authtypes.NewModuleAddress(types.ModuleName).String(),
ExecutionPayload: payloadData,
Expand All @@ -111,11 +116,11 @@ func Test_msgServer_ExecutionPayload(t *testing.T) {
newPayload(ctx)
assertExecutionPayload(ctx)

// now lets run optimistic flow
// Again, but with simple events
ctx = ctx.WithContext(feature.WithFlag(ctx, feature.FlagSimpleEVMEvents))
ctx = ctx.WithBlockTime(ctx.BlockTime().Add(time.Second))

newPayload(ctx)
keeper.SetBuildOptimistic(true)
assertExecutionPayload(ctx)
}

Expand Down
24 changes: 16 additions & 8 deletions octane/evmengine/keeper/proposal_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"github.com/omni-network/omni/lib/errors"
"github.com/omni-network/omni/lib/feature"
"github.com/omni-network/omni/lib/log"
"github.com/omni-network/omni/octane/evmengine/types"

Expand Down Expand Up @@ -45,15 +46,22 @@ func (s proposalServer) ExecutionPayload(ctx context.Context, msg *types.MsgExec
return nil, err
}

// Collect local view of the evm logs from the previous payload.
evmEvents, err := s.evmEvents(ctx, payload.ParentHash)
if err != nil {
return nil, errors.Wrap(err, "prepare evm event logs")
}
if feature.FlagSimpleEVMEvents.Enabled(ctx) {
// Ensure no events included in payload.
if len(msg.PrevPayloadEvents) > 0 {
return nil, errors.New("prev payload events included in payload")
}
} else {
// Collect local view of the evm logs from the previous payload.
evmEvents, err := s.evmEvents(ctx, payload.ParentHash)
if err != nil {
return nil, errors.Wrap(err, "prepare evm event logs")
}

// Ensure the proposed evm event logs are equal to the local view.
if err := evmEventsEqual(evmEvents, msg.PrevPayloadEvents); err != nil {
return nil, errors.Wrap(err, "verify prev payload events")
// Ensure the proposed evm event logs are equal to the local view.
if err := evmEventsEqual(evmEvents, msg.PrevPayloadEvents); err != nil {
return nil, errors.Wrap(err, "verify prev payload events")
}
}

return &types.ExecutionPayloadResponse{}, nil
Expand Down
5 changes: 5 additions & 0 deletions octane/evmengine/keeper/proposal_server_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/omni-network/omni/lib/ethclient"
"github.com/omni-network/omni/lib/expbackoff"
"github.com/omni-network/omni/lib/feature"
"github.com/omni-network/omni/octane/evmengine/types"

"github.com/ethereum/go-ethereum/beacon/engine"
Expand Down Expand Up @@ -86,6 +87,10 @@ func Test_proposalServer_ExecutionPayload(t *testing.T) {

newPayload(sdkCtx)
assertExecutionPayload(sdkCtx)

// Again, but with simple events
sdkCtx = sdkCtx.WithContext(feature.WithFlag(sdkCtx, feature.FlagSimpleEVMEvents))
assertExecutionPayload(sdkCtx)
}

func fastBackoffForT() {
Expand Down
2 changes: 1 addition & 1 deletion octane/evmengine/types/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ message MsgExecutionPayload {
option (cosmos.msg.v1.signer) = "authority";
string authority = 1;
bytes execution_payload = 2;
repeated EVMEvent prev_payload_events = 3 [(gogoproto.nullable) = false];
repeated EVMEvent prev_payload_events = 3 [(gogoproto.nullable) = false]; // TODO(corver): Deprecate this once simple-evm-events released.
repeated bytes blob_commitments = 4; // Array of blob tx KZGCommitments, 48 bytes each.
}

Expand Down

0 comments on commit f9ac08f

Please sign in to comment.