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

Design and Implement Engine Driver #5

Closed
Tracked by #9 ...
frisitano opened this issue Dec 27, 2024 · 15 comments · Fixed by #12
Closed
Tracked by #9 ...

Design and Implement Engine Driver #5

frisitano opened this issue Dec 27, 2024 · 15 comments · Fixed by #12
Assignees

Comments

@frisitano
Copy link
Collaborator

frisitano commented Dec 27, 2024

Overview

We should implement a component that is used to drive the execution client via he Engine API.

Engine API reference: https://hackmd.io/@danielrachi/engine_api

@greged93
Copy link
Collaborator

greged93 commented Jan 11, 2025

First layout of the types and methods for the EngineDriver. Let me know what you think.

pub struct ScrollPayloadAttributes {
   /// The payload attributes.
   payload_attributes: PayloadAttributes,
   /// The l1 messages for block building.
   l1_messages: Vec<TxL1Message>,
}

/// Information about a block.
pub struct BlockInfo {
   /// The block number.
   number: u64,
   /// The block hash.
   hash: B256
}

pub struct EngineDriver<E> {
   /// The engine API client.
   client: E,
   /// The unsafe L2 block info.
   unsafe_block_info: BlockInfo,
   /// The safe L2 block info.
   safe_head: BlockInfo,
   /// The finalized L2 block info.
   finalized_head: BlockInfo
}

impl<E, P> EngineDriver<E, P> 
where
    E: EngineApiClient<Types>,
    Types: EngineTypes<PayloadAttributes = ScrollPayloadAttributes>, 
{
    /// Initialize the driver and wait for the Engine server to be ready.
    pub fn init_and_wait_for_engine(client: E, unsafe_head: BlockInfo, safe_head: BlockInfo, finalized_head: BlockInfo) -> Self;
    /// Handles a execution payload:
    ///   - Sends the payload to the EL via `engine_newPayloadVx`.
    ///   - Sets the current fork choice for the EL via `engine_forkchoiceUpdatedVx`. 
    pub fn handle_execution_payload(&self, execution_payload: ExecutionPayload) -> Result<()>;
    /// Handles a payload attributes:
    ///   - Starts a payload building task on the EL via `engine_forkchoiceUpdatedVx`, passing
    ///     the provided payload attributes.
    ///   - Retrieve the payload with `engine_getPayloadVx`.
    ///   - Sends the constructed payload to the EL via `engine_newPayloadVx`.
    ///   - Sets the current fork choice for the EL via `engine_forkchoiceUpdatedVx`.
    pub fn handle_payload_attributes(&self, payload_attributes: ScrollPayloadAttributes) -> Result<()>;
    /// Reorgs the driver by setting the safe and unsafe block info to the finalized info.
    pub fn reorg(&mut self);
    /// Set the finalized L2 block info.
    pub fn set_finalized_block_info(&mut self, finalized: BlockInfo);
    /// Set the safe L2 block info.
    pub fn set_safe_block_info(&mut self, block: BlockInfo);
    /// Set the unsafe L2 block info.
    pub fn set_unsafe_block_info(&mut self, block: BlockInfo);
}

@frisitano
Copy link
Collaborator Author

This looks great. One question I have is about reorgs. If I'm not mistaken the scroll chain should never reorg as we only process L1 messages when the messages have been finalized on L1. There have been cases where batches need to be reverted but I'm not sure if this results in a reorg. Let's follow up with other members of scroll to better understand the potential for reorgs. If reorgs is impossible then we can remove the reorg method. Should we also add methods to set the safe and unsafe head? Putting this together we get:

impl<E, P> EngineDriver<E, P>
where
    E: EngineApiClient<Types>,
    Types: EngineTypes<PayloadAttributes = ScrollPayloadAttributes>,
{
    /// Initialize the driver and wait for the Engine server to be ready.
    pub fn init_and_wait_for_engine(
        client: E,
        unsafe_head: BlockInfo,
        safe_head: BlockInfo,
        finalized_head: BlockInfo,
    ) -> Self;
    /// Handles a execution payload:
    ///   - Sends the payload to the EL via `engine_newPayloadVx`.
    ///   - Sets the current fork choice for the EL via `engine_forkchoiceUpdatedVx`.
    pub fn handle_execution_payload(&self, execution_payload: ExecutionPayload) -> Result<()>;
    /// Handles a payload attributes:
    ///   - Starts a payload building task on the EL via `engine_forkchoiceUpdatedVx`, passing
    ///     the provided payload attributes.
    ///   - Retrieve the payload with `engine_getPayloadVx`.
    ///   - Sends the constructed payload to the EL via `engine_newPayloadVx`.
    ///   - Sets the current fork choice for the EL via `engine_forkchoiceUpdatedVx`.
    pub fn handle_payload_attributes(
        &self,
        payload_attributes: ScrollPayloadAttributes,
    ) -> Result<()>;
    /// Set the finalized L2 block info.
    pub fn set_finalized_block_info(&mut self, block: BlockInfo);
    /// Set the safe L2 block info.
    pub fn set_safe_block_info(&mut self, block: BlockInfo);
    /// Set the unsafe L2 block info.
    pub fn set_unsafe_block_info(&mut self, block: BlockInfo);
}

@greged93
Copy link
Collaborator

Should we also add methods to set the safe and unsafe head?

Yes true, let's add both.

If we agree on the above API I will get started for the implementation

@frisitano
Copy link
Collaborator Author

Looks good to me, feel free to proceed.

@Thegaram
Copy link
Contributor

/// The l2 base fee per gas derived from the l1 parent block.
/// https://github.com/scroll-tech/go-ethereum/blob/ac8164f5a4190ff9e536f296195013ea7a4e3e3d/consensus/misc/eip1559.go#L53
base_fee_per_gas: U256

What's the rationale for moving this from EN to RN? If we want to store L2 base fee coefficients in state in the future, it seems simpler to keep this in EN.

/// The unsafe L2 block info.
unsafe_block_info: BlockInfo,

RN will keep receiving gossiped blocks and feeding these into the EN. Once the provided block is executed, it will be set as unsafe_block_info and we issue the corresponding fork-choice call, is that correct?

pub fn init_and_wait_for_engine(client: E, unsafe_head: BlockInfo, safe_head: BlockInfo, finalized_head: BlockInfo) -> Self;

How will we set these params? Simply get them from L1Watcher and L2Watcher during startup?

pub fn handle_execution_payload(&self, execution_payload: ExecutionPayload) -> Result<()>;
pub fn handle_payload_attributes(&self, payload_attributes: ScrollPayloadAttributes) -> Result<()>;

Would be useful to sync through the whole process of the RN. During normal operations it will keep receiving L2 blocks from gossip, as well as blocks from L1 sync. If everything is good, then the latter chain will be a prefix of the former. Do we submit both of these using handle_execution_payload, but the difference is that we will issue different fork choice for these? I don't see this expressed in the proposed interface.

@Thegaram
Copy link
Contributor

If I'm not mistaken the scroll chain should never reorg as we only process L1 messages when the messages have been finalized on L1. There have been cases where batches need to be reverted but I'm not sure if this results in a reorg.

We can differentiate between 3 cases:

  1. L2-only reorg: L2 unsafe (not-yet-committed) blocks get reorged. We try to avoid this but currently there is no protocol guarantee that there are no L2 reorgs. One example that might cause reorgs is async CCC.

  2. Reorg due to revert batch: True, currently the protocol allows reverting committed (but not yet finalized) batches on L1. This does not necessarily lead to L2 reorgs: In past cases we've submitted the same L2 blocks with different batch encoding. But it could lead to L2 reorgs, e.g. if we accidentally produced an unprovable block.

  3. Reorg along with L1: @frisitano is right that we currently conservatively wait for finalization before relaying deposit messages. However, we want to reduce the latency in the future; this is great for deposits and especially relevant for other features like L1SLOAD. In that case, we optimistically relay information from L1 (knowing that deep reorgs on L1 are rare). If L1 reorgs, L2 must reorg along with it.

/// Reorgs the driver by setting the safe and unsafe block info to the finalized info.
pub fn reorg(&mut self);

Need to consider more how this will work. Realistically, a component (L1Watcher) will keep track of the L1 ledger and the derivation pipeline. If there is an L1 reorg, we can rewind the derivation pipeline to the reorg root and continue from there. So I'm not sure it's necessary to set this to finalized info.

@greged93
Copy link
Collaborator

What's the rationale for moving this from EN to RN? If we want to store L2 base fee coefficients in state in the future, it seems simpler to keep this in EN.

Afaiu, you need to the parent L1 block info in order to derive the L2 base fee right? In the current state, how would you retrieve this L1 block info in the EN?
Also if we move to storing the L2 base fee coefficients in the state, I think it would make sense to adopt something similar to EIP-2935 and update these coefficients during pre-execution.

RN will keep receiving gossiped blocks and feeding these into the EN. Once the provided block is executed, it will be set as unsafe_block_info and we issue the corresponding fork-choice call, is that correct?

Yes this is the path I had in mind.

How will we set these params? Simply get them from L1Watcher and L2Watcher during startup?

Yes at start up the Rollup node should initialize the engine driver with the correct heads. If we decide to adopt a "pipeline" path as Kona did, we could have an "initialize" function for the whole pipeline which sets all the value.

Do we submit both of these using handle_execution_payload, but the difference is that we will issue different fork choice for these?

Afaiu, if we consider the reorgless path, you don't need to resubmit the execution payload for a finalized head, you can just update the head via set_finalized_block_info. The next call to handle_execution_payload will result in issuing a fork choice update with this updated finalized head.

If we reorg, the state of the driver will be reset to the finalized head, and the next call to handle_execution_payload will notify the EN about the reorg.

@greged93
Copy link
Collaborator

Need to consider more how this will work. Realistically, a component (L1Watcher) will keep track of the L1 ledger and the derivation pipeline. If there is an L1 reorg, we can rewind the derivation pipeline to the reorg root and continue from there. So I'm not sure it's necessary to set this to finalized info.

You do need to notify the EngineDriver for a reorg no? Otherwise it won't update its internal safe with regards to the safe and unsafe heads. If you prefer we can modify the reorg the below, which would avoid rewinding all the way to the finalized head during a reorg:

    /// Reorgs the driver by rewinding the safe and unsafe block info to the provided block head.
    pub fn reorg(&mut self, head: BlockInfo);

@Thegaram
Copy link
Contributor

Afaiu, you need to the parent L1 block info in order to derive the L2 base fee right? In the current state, how would you retrieve this L1 block info in the EN?

Oh I get your point, you mean most L1 info should be provided through the RN. I generally agree, but the L1 base fee info is already read from state. This info is relayed by another component (gas-oracle). In the spirit of incremental protocol changes, I'd prefer to keep this mechanism and consider changing it to something else later.

Also if we move to storing the L2 base fee coefficients in the state, I think it would make sense to adopt something similar to EIP-2935 and update these coefficients during pre-execution.

Currently l1BaseFee is updated through a normal EOA. And the coefficients are hardcoded into l2geth. We could change both these either to a system transaction, or some pre-execution update to a system contract, yes. But let's consider that in a later version of the protocol. (This is tricky because anything that's part of the state transition function needs to be proven.)

Afaiu, if we consider the reorgless path, you don't need to resubmit the execution payload for a finalized head, you can just update the head via set_finalized_block_info.

This is tricky, because we don't get full blocks from the derivation pipeline, only a block without execution results (is that PayloadAttributes? not sure...). As a result at this point we don't know that corresponding block hash either. So we'd need to pass this to EN for execution before we can issue a fork choice rule.

You do need to notify the EngineDriver for a reorg no?

This boils down to: Which component should be responsible for maintaining this info? Ultimately safe and finalized will be maintained by the derivation pipeline. Either EngineDriver reads from there, or the pipeline can call reorg (or simply set_finalized_block_info and set_safe_block_info) to push updates to EngineDriver.

@frisitano frisitano mentioned this issue Jan 14, 2025
8 tasks
@frisitano
Copy link
Collaborator Author

Whilst I agree that it is useful to consider our future ambitions to have unsafe block consolidation and a provable derivation pipeline I would encourage that we primarily focus on Milestone 1 ambitions which is supporting L2 p2p sync as described in #9. As such I would say that the current solution is reasonable whilst being mindful that it is likely to change as we integrate components in the stack and certainly as we proceed with our future milestones.

@greged93
Copy link
Collaborator

greged93 commented Jan 14, 2025

Below are three execution flows that should be considered and impact the above design

Optimistic sync

The optimistic sync will ingest L2 blocks received via P2P. This path should have the lowest priority: the RN should favor processing L1 derived blocks.

sequenceDiagram
  participant RN
  participant EN
  RN ->> RN: receive ExecutionPayload
  RN ->> EN: engine_newPayload(ExecutionPayload)
  EN ->> RN: {status: VALID, ..}
  RN ->> EN: engine_forkChoiceUpdate({head_block_hash: ExecutionPayload.block_hash, ..}, None)
  EN ->> RN: {status: VALID, ..}
Loading

The engine driver receives the execution payload, sends it to the EN via engine_newPayload and updates the new head for the EN by issuing a engine_forkChoiceUpdate.

Pessimistic sync

The pessimistic sync will ingest L1 blocks and derive the payload attributes for building L2 blocks. This path has the highest priority for the RN and L1 derived payload should always be prioritized over P2P payload.

In order to avoid redundant execution, the RN node should be able to retrieve past processed execution payloads. This can be achieved by caching execution payloads or fetching blocks via the EN.

Attributes matching

If the L1 derived payload attributes match the retrieved execution payload for the unsafe block at safe head + 1, we can consolidate it by following the below execution path:

sequenceDiagram
  participant RN
  participant EN
  RN ->> EN: engine_forkChoiceUpdate({safe_block_hash: (SafeBlock.number + 1).hash, ..}, None)
  EN ->> RN: {status: VALID, ..}
Loading

Attributes not existing or mismatch (L2 reorg)

If the L1 derived payload attributes do no match the retrieved execution payload, we are faced with a L2 reorg and cannot consolidate. A new block should be constructed from the payload, on top of the safe head:

sequenceDiagram
  participant RN
  participant EN
  RN ->> RN: receive ScrollPayloadAttributes
  RN ->> EN: engine_forkChoiceUpdate(FCU, {no_tx_pool: true, ..attributes})
  EN ->> RN: {status: VALID, payload_id}
  RN ->> EN: engine_getPayload(payload_id)
  EN ->> RN: ExecutionPayload
  RN ->> EN: engine_newPayload(ExecutionPayload)
  EN ->> RN: {status: VALID, ..}
  RN ->> EN: engine_forkChoiceUpdate({safe_block_hash: ExecutionPayload.block_hash, ..}, None)
  EN ->> RN: {status: VALID, ..}
Loading

The block building process is started by calling engine_forkChoiceUpdate with the received attributes, on which we set the no_tx_pool field to true. This ensures our block building task will not include any unwanted transaction. The RN can then fetch the resulting execution payload with engine_getPayload, forward it to the EN with engine_newPayload and instruct the EN on the new FC via engine_forkChoiceUpdate.

Outcome

The above leads to the following updates:

  • Add a transactions and no_tx_pool field on the ScrollPayloadAttributes.
  • Introduce a ExecutionPayloadProvider trait.
  • Add an execution_payload_provider field on the EngineDriver.
pub struct ScrollPayloadAttributes {
   /// The payload attributes.
   payload_attributes: PayloadAttributes,
   /// The l1 messages for block building.
   l1_messages: Vec<TxL1Message>,
   /// An optional array of transaction to be forced included in the block.
   transactions: Option<Vec<Transaction>>,
   /// Indicates whether the payload building job should happen with or without pool transactions.
   no_tx_pool: bool
}

// Implementers of the trait can provide the L2 execution payload for a block id.
pub trait ExecutionPayloadProvider {
   execution_payload_by_block(&self, block_id: BlockId) -> Result<ExecutionPayload>;
}

pub struct EngineDriver<EC, P> {
   /// The engine API client.
   client: EC,
   /// The execution payload provider
   execution_payload_provider: P,
   /// The unsafe L2 block info.
   unsafe_block_info: BlockInfo,
   /// The safe L2 block info.
   safe_head: BlockInfo,
   /// The finalized L2 block info.
   finalized_head: BlockInfo
}

Sources

Opnode:

Magi:

@Thegaram
Copy link
Contributor

The block building process is started by calling engine_forkChoiceUpdate with the received attributes, on which we set the no_tx_pool field to true. This ensures our block building task will not include any unwanted transaction.

Just to make sure I understand, block building and executing payload attributes derived from L1 are almost exactly the same process? The only difference is the in the latter case the EN will not insert more txs into the block from txpool. The rest (execution, computing state/receipt/tx roots) are the same.

@greged93
Copy link
Collaborator

Just to make sure I understand, block building and executing payload attributes derived from L1 are almost exactly the same process

This is what I understood from Optimism yes. You issue a engine_forkChoiceUpdate call for both of them, but use the additional transactions and no_tx_pool field to control whether you are operating as a "block proposer" or simply executing payload attributes

@jonastheis
Copy link

Do we allow RN to run without optimistic sync? In case we do, then Attributes mismatch (L2 reorg) should be more general: Attributes not existing or mismatch (L2 reorg). I think the rest of the flow can stay the same

@frisitano
Copy link
Collaborator Author

Great write-up; this looks good to me. Something to note for future reference is that we may need to persist the payload_id associated with the latest payload build process as part of the state. However, this may only be relevant when we have multiple alternating proposers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants