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

Subscribe to a channel with ExEx notifications manually #10571

Closed
shekhirin opened this issue Aug 27, 2024 · 0 comments · Fixed by #10787
Closed

Subscribe to a channel with ExEx notifications manually #10571

shekhirin opened this issue Aug 27, 2024 · 0 comments · Fixed by #10787
Assignees
Labels
A-exex Execution Extensions C-enhancement New feature or request S-needs-design This issue requires design work to think about how it would best be accomplished

Comments

@shekhirin
Copy link
Collaborator

shekhirin commented Aug 27, 2024

Describe the feature

Problem

Currently, we pass a channel with ExExNotifications right into the ExExContext

let (handle, events, notifications) = ExExHandle::new(id.clone());
exex_handles.push(handle);
// create the launch context for the exex
let context = ExExContext {
head,
config: config_container.config.clone(),
reth_config: config_container.toml_config.clone(),
components: components.clone(),
events,
notifications,
};

This approach has a downside: #10489. We don't know from what block height to start sending notifications to the ExEx, or if the ExEx has an incorrect fork, that's no longer canonical.

Solution

Don't subscribe the ExEx to notifications by default, but instead provide a method for the ExEx to call and get a channel for new notifications.

pseudocode:

pub struct ExExHead {
   pub number: BlockNumber,
   pub hash: BlockHash
}

pub fn subscribe_to_notifications(&self, head: ExExHead) -> Receiver<ExExNotification> { ... }

The logic inside the subscribe_to_notifications should also handle the backfill of missing blocks, according to the provided head:

  1. If the ExExHead is ahead of the node head, wait until notifications catch up with head.number, and then compare the hashes to determine if the ExEx is on the canonical chain:
    1. If it's not, we need to revert (HOW — unclear) ExEx to the first canonical block and then send blocks up to the canonical chain tip.
    2. If it is, then the ExEx is on the canonical chain and we can proceed as usual.
  2. If the ExExHead is at the same block number as the node head, but the hash is mismatching, then do step 1.i. above with reverting up to the first canonical block with hashes matching between the ExEx and the node.
  3. If the ExExHead is behind of the node head:
    1. Check if the ExExHead is canonical. If needed, revert to the first canonical block with hashes matching between the ExEx and the node
    2. Send notifications for blocks from head.number + 1 to the node head
    3. Proceed as usual.

It's not clear how to do reverts properly, because the node may not have the data for the provided ExExHead to construct a ChainReverted variant.

Actual Solution for Now

Same as described above, but only handling cases 1.2 and 3 and not checking if the chain is canonical.

Additional context

No response

@shekhirin shekhirin added C-enhancement New feature or request S-needs-design This issue requires design work to think about how it would best be accomplished A-exex Execution Extensions labels Aug 27, 2024
@shekhirin shekhirin self-assigned this Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exex Execution Extensions C-enhancement New feature or request S-needs-design This issue requires design work to think about how it would best be accomplished
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant