-
Notifications
You must be signed in to change notification settings - Fork 33
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
Sequencer better DA fork handling & minor improvements #1718
base: nightly
Are you sure you want to change the base?
Sequencer better DA fork handling & minor improvements #1718
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
|
||
Ok((last_finalized_block, l1_fee_rate)) | ||
} | ||
|
||
async fn ensure_no_da_forks<Da: DaService>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to implement this for all other nodes? because they also follow the wrong fork, at which point, being at the wrong DA block might lead them to misbehave.
I am not totally sure, but this is something to check for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seemed to me like there is nothing to do for other nodes for this PR specifically:
- light client prover: light client proof execution will fail due to header chain verification (prev_hash will not be equal), hence it wont continue
- batch prover: there won't be new seq comms since sequencer will be stopped so no new batch proofs. we will have to manually rollback to the start of the fork block anyways, hence, it doesn't matter if batch prover is up without producing any proof.
- full node: similar logic with batch prover, no new seq comms, and we will have to rollback manually anyways.
Remaining work seems to me to be blocked by rollback feat for our nodes. Once we have proper rollback functionality, we can detect forks in all types of nodes, rollback to the beginning of the fork, and continue happily ever after from that point on, and we won't need manual intervention at all.
Description
Panic if we are on the wrong side of the DA fork. It is better to not continue with an invalid state and manually recover, which we will have to do until we do a very smart handling of DA forks.
To ensure no forks happened while sequencer restart, we check that the head soft confirmation's l1 height still corresponds to the same l1 hash we stored.
To ensure no forks happened while sequencer is running, in da block monitor, if the queried l1 block height is same as the previously queried l1 block, we ensure that their hashes match as well, if 1 block generated, we ensure that the prev hash matches with the previously queried l1 block, if more than 1 block generated, we re-query the previously queried l1 block and ensure it's hash is still the same.
Also made some minor improvements to sequencer.
Linked Issues
Testing
Describe how these changes were tested. If you've added new features, have you added unit tests?
Docs
Describe where this code is documented. If it changes a documented interface, have the docs been updated?