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

op-node: sequencer does not check op-conductor on node startup #11121

Closed
protolambda opened this issue Jul 9, 2024 · 3 comments
Closed

op-node: sequencer does not check op-conductor on node startup #11121

protolambda opened this issue Jul 9, 2024 · 3 comments
Labels
A-op-conductor Area: op-conductor A-op-node Area: op-node

Comments

@protolambda
Copy link
Contributor

When the sequencer is first initialized, it takes the sequencing enabled & active status from the regular op-node config and state-persistence, but doesn't additionally check the leader-status with the conductor like it does upon sequencer start/stop.
Is this intentional? Should we make it check with the conductor, before allowing the sequencer to continue?

As part of the events-refactor of the sequencer code-path I noticed this, and I am wondering if it should change or not.
See #10991

@zhwrd
Copy link
Contributor

zhwrd commented Jul 9, 2024

we have a config check to ensure config persistence is disabled when conductor is enabled
https://github.com/ethereum-optimism/optimism/blob/develop/op-node/node/config.go#L166-L173

@0x00101010
Copy link
Collaborator

Great observation! This situation could happen, when starting sequencer, leadership transfer could happen and conductor could become a non-leader.

However, we don't need to do the check here because

  1. there isn't really a way to guarantee that leadership does not transfer between the time we call startSequencer and it is actually started
  2. and if the leadership transfer happened during that time, it is fine as sequencer won't be able to gossip its block out, and will shortly be turned off by a functioning conductor

@protolambda protolambda added A-op-node Area: op-node A-op-conductor Area: op-conductor labels Feb 4, 2025
@protolambda
Copy link
Contributor Author

Closing, let me know if we missed anything still.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-op-conductor Area: op-conductor A-op-node Area: op-node
Projects
None yet
Development

No branches or pull requests

3 participants