refactor: Simulate into the future #8110
Assignees
Comments
LHerskind
changed the title
LHerskind
added a commit
that referenced
this issue
Aug 27, 2024
This PR add support for using Aztec slot durations that are not just 1 ethereum slot. To do so it adds more logic to the sequencer such that they can figure out if they are really the sequencer or not. This is mainly in the `canProposeBlock` and `shouldProposeBlock` functions which respectively assert that the proposer can actually propose, and whether he should. The logic in those functions are partly what we had sprinkled over the code before, but also adding some additional checks to match what is in the rollup contract and Leonidas. Since there are now additional restrictions to block production related to WHEN the block is to land on L1, I have added a `commitTimeJump` function into the `l1-publisher`which will jump to the next slot after a block have been published. This functionality toggled with a `TIME_TRAVELER` boolean flag. Note that we will jump INTO the next slot, since simulations in `viem` and `anvil` are limited to run on block values in the bast, meaning that we cannot nicely just simulate as if it was included in the NEXT block which is what we ideally want. See issue #8110 for more information. Since it caused some issues that there was no actual genesis state (just 0), I have inserted a genesis state equal to what we compute as the `lastArchive` for the very first block, fixing #4148. To not mess too much with DEVNET, the extra logic related to the exact timing of when L2 blocks should make it onto L1 can be "toggled" with the `IS_DEV_NET` flag. Namely, if `IS_DEV_NET` is toggled, we can publish outside of the "current" slot, as long as the slots are in order. With the changes in this pr, we should be able to run DEVNET without `automine`, I have tried a minor test but that seemed to work fine when we have `AZTEC_SLOT_DURATION = 36` and internal mining :) Points of interest: - `sequencer.ts` - `canProposeBlock` - `shouldProposeBlock` - `l1-publisher.ts` - Adding `simulate` since viem `write` does not provide meaningful error messages, but `simulate` does. - The `commitTimeJump` won't happen if the `AZTEC_SLOT_DURATION = ETHEREUM_SLOT_DURATION` or `TIME_TRAVELER = false` - `sequencer.test.ts` - Some of the test naming a odd. For example the test `builds a block that contains zero real transactions once flushed` sounds to me like you expect to have an empty block, after the block flushed, e.g., once flushed make it sound like it already happened - `l1-publisher.test.ts` - `does not publish if last archive root is different to expected` is deleted, as that job falls on the sequencer. The sequencer should define whether or not it will send a tx, and publisher deals more with publishing tasks. - Adding a `simulate` that is also mocked to account for `simulate` in the publisher. - `e2e_p2p_network.test.ts` - There is a flag for using a local anvil chain that you are running in another terminal. This is useful for running internal mining etc. --- This PR will be run with `AZTEC_SLOT_DURATION = 12` and `IS_DEV_NET = true`. Note that when using values different from those, there are still some hiccups and kinks, but it should be addressed in a separate PR to not make this explode in size.
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.
|
This was fixed in #8148 |
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
Throughout the sequencer client, we want to check whether "we" are the next sequencer. To figure that out, we can do a lot of compute in TS using view functions that we have on the contract.
However, when we want to check the validity of the transaction itself which we want to do through a simulation we have a problem.
If we are simulating it BEFORE a l1 block within our slot have been published, the simulation will be executed at that point in time, meaning that we are not the sequencer yet and the simulation fails even though it would be valid if included on chain.
Currently that means that we just "waste" the first first block within each L2 slot.
Neither Anvil nor Viem currently support overriding these values, but both reth and geth seems to so real nodes should work.
While it is ugly (I think), the best option is likely to create a view function with the checks where we can then pass in the time for the future etc and check it that way. Essentially moving some of the checks that we usually do in the
canProposeBlockinto the contract itself.The issue is larger the shorter the duration of the L2 slot.
The text was updated successfully, but these errors were encountered: