feat: Support altering slot_duration
#7850
Assignees
Milestone
Comments
This was referenced Aug 8, 2024
LHerskind
added a commit
that referenced
this issue
Aug 9, 2024
Fixes 7656. Following an earlier PR (#7594), we added slots into the mix. This PR tackles actually using those slots for something. When adding these constraints, several fixes were needed on the sequencing clients and its components. So why are we introducing slots? There is really two things that we are going to use the slots for: - i) We are going to use it to deal with time within the L2 - Previously a sequencer had flexibility on how the defined the timestamp, now it is instead defined as `GENESIS + slot * slot_duration` depending solely on the slot. - ii) We will be using it as the unit of time in which a specific proposer have rights to proposer a block and where the block will be accepted on chain. - This means that it is a timeliness requirement for block inclusion on L1 - Since the proposer can only use the "current" slot as the value for his block, he no flexibility on the timestamp of the L2 block. The first term is mainly of interest to applications, but the second term is really important for how our sequencer selection works. If you do not propose within your block, you are tough out of luck, the block will not be accepted on L1. This means that it is also very important for us to figure out a proper slot duration, since a short slot duration can lead to plenty of empty slots, while too long lead to loss in throughput. Introducing such a strict timeliness requirement does impact our current tests quite a lot, since they are all created without that in mind (similar was the sequencer client). Since we in our E2E are the only producer of L1 blocks as well, we have full control over those. However, using automine on Anvil as the underlying chain makes the behaviour of sequencing with slots quite odd, as things that would normally be within the same block (multiple tx) will now progress time at every tx. This meant that the tests broke horribly, as we with one tx was publishing the data for our block, and then when we wanted to publish the block afterwards the time was progressed, so we might no longer be the proposer. This was handled by adding a combination call that is doing `publishAndProcess` of a block, making sure that we don't progress time unnecessarily. Secondly, when we are using on-demand mining (automine), we won't have that we get to the "next" slot after a block have been included, so to not mess to much with timing, we have specified the `SLOT_DURATION` to the same as the underlying chain slot duration. Since we are using anvil, this is by default 1s. However, we also change the block interval to 12s to behave closer to ethereum. Having just a single l1 slot for every l2 slot means that the proposer is rapidly changing as l1 blocks are coming in. Something that WILL NOT work out the box for a real chain, but is accepted as a first step here in our setup to get started. Since we do not want to break everything just ahead of dev net, we have introduced a flag for turning off the most strict checks and rotation of the sequencers. By setting `IS_DEV_NET = true` in the `constants.nr` the rollup will accept blocks from anyone in the validator set (if not empty), without checking if it is specifically their turn. You can more specifically look at this in the `Rollup` contract. Since it is now required to provide attestations from the L2 network if not `IS_DEV_NET = true` the sequencer client will `collectAttestations` before publishing the block. Currently it is a war crime of a function that is just collecting its own signature and providing that as the attestations. This mean that the current setup ONLY works for validator set size 1. This is to be addressed with the extensions that @Maddiaa0 is making with #7681. When body and headers arrive around the same time (in same l1 block) the archive seemed to run into issues as it would sometime only have loaded the header and not the body, and then fail as it tries to combine the two into an l2 block. If @alexghr, @spypsy, @spalladino or @PhilWindle is able to take a look to the changes made to it (mainly in e321449) that would be great. Notable changes in this PR: - `publishAndProcess` - Publishing the block body AND processing the pending chain is now done in the same TX to not cause issues with timeliness - Block time is set to 12 seconds - When deploying the L1 contracts, we are setting the block time of the underlying Anvil chain to 12 seconds. (Still in automine). - Introduces `IS_DEV_NET` flag in `constants.nr` - When `IS_DEV_NET = true` - Skips some of the stricter timeliness requirements since they are not needed for centralised sequencers. - `IS_DEV_NET = false` - Run with all checks on - Will need signatures from the committee (ordered correctly) to propose a block - Made changes to the archiver as it sometimes broke when block body and header was in the same l1 block. Future work: - Collect attestations properly - E2E should survive running with interval mining and `SLOT_DURATION = n * ETHEREUM_SLOT_DURATION` for `n>1`. --- We will have the test pass with `IS_DEV_NET=false` and then we will do another run where we set it to `true` such that we do not break things for people unnecessarily. --- Issues spawned by this pr: - #7849 - #7821 - #7850 - #7837
LHerskind
added a commit
that referenced
this issue
Aug 29, 2024
Doing some cleanup after #7850, looking to address the issue of #8153 and #8110. - #8153 - Addressed by including a "watcher" as part of the setup, which will push us to the next slot if there is already a block proposed for the current one. - #8110 - Updates the logic in the contract such that we can deal with "simulating" in the future, and use this from the sequencer. Gets rid of the `time_traveler` from the l1-publisher, now lives in the watcher which is used in tests. Issues related to slot duration is still to be addressed, so the name of this branch got slightly funky. Taking over the extra check added in #8204 since i) they are related and ii) the pain of going through CI made me do it.
codygunton
pushed a commit
that referenced
this issue
Aug 30, 2024
Doing some cleanup after #7850, looking to address the issue of #8153 and #8110. - #8153 - Addressed by including a "watcher" as part of the setup, which will push us to the next slot if there is already a block proposed for the current one. - #8110 - Updates the logic in the contract such that we can deal with "simulating" in the future, and use this from the sequencer. Gets rid of the `time_traveler` from the l1-publisher, now lives in the watcher which is used in tests. Issues related to slot duration is still to be addressed, so the name of this branch got slightly funky. Taking over the extra check added in #8204 since i) they are related and ii) the pain of going through CI made me do it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It should be possible to alter the
SLOT_DURATIONto be a>1multiple of the underlying l1 slot duration without it bricking the absolute everything out of the testing setup.The text was updated successfully, but these errors were encountered: