-
Notifications
You must be signed in to change notification settings - Fork 92
Description
Problem to Solve
Currently the Octane MsgExecutionPayload
contains both the new payload/block being proposed/added, as well as the previous block event logs.
type MsgExecutionPayload struct {
ExecutionPayload []byte
PrevPayloadEvents []*EVMEvent
}
@chmllr indicated that this could actually be simplified. EVMEvents are deterministic given a finalized block. They can simply be queried after ForkChoiceUpdated
in FinalizeBlock
and provided to the EVMEventProcessors
.
This reduces the size of consensus blocks. It also simplifies the code.
This also avoids the 1 block lag, with execution event logs being applied in the same consensus block.
Note that querying EVM EventLogs is generally only possible for finalized blocks (post ForkchoiceUpdate
). So this would mean that verifying event logs during ProcessProposal
wouldn't be possible. But since it is deterministic and event logs cannot be modified or blocked or omitted, they all need to be processed in anycase. Failing if they are not valid for some reason. So this ins't a problem I think.
Note this would require a
network upgrade
.
Proposed Solution
- Remove
PrevPayloadEvents
fromMsgExecutionPayload
. - Don't generate them during
PrepareProposal
and don't verify them duringProcessProposal
. - During
FinalizeBlock
inEVMEngine
msgServer.ExecutionPayload
, after callingForkchoiceUpdatedV3
- Fetch the newly finalized block evm events via
s.evmEvents
and pass those tos.deliverEvents
- Ensure events are processing in correct order
Also take the opportunity to improve the EVMEventProcessor
interface:
type EvmEventProcessor interface {
// Name returns the name of the processor.
Name() string
// FilterParams returns the addresses and topics to filter events for.
FilterParams() ([]common.Address, [][]common.Hash)
// Deliver processes the EVM log event.
Deliver(ctx context.Context, blockHash common.Hash, log *EVMEvent) error
}
Note that on the network upgrade height itself, we should process both legacy previous block events as well as current block events. Otherwise we will miss events for
upgrade height-1
.