-
Notifications
You must be signed in to change notification settings - Fork 2
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
Refactor rollup into a consumable library #75
Draft
ernestognw
wants to merge
72
commits into
main
Choose a base branch
from
refactor/consumable-library
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n/minimal-rollup into ahead-of-time-prover
…n/minimal-rollup into ahead-of-time-prover
…n/minimal-rollup into ahead-of-time-prover
…l-rollup into ahead-of-time-prover
…l-rollup into ahead-of-time-prover
I think we can still get in a weird situation where a prover exits after the liveness window has passed. This means the actual largest window is livenessWindow + exitDelay + provingDeadline, but that is probably fine
The CheckpointTracker only saves a hash of the last proven checkpoint. I add a parameter so the caller can provide the actual checkpoint to be checked against the hash. I did not update the tests, because I think we should actually update the CheckpointTracker to save the whole checkpoint, but I did not want to edit it until the merge conflicts are resolved.
It represents a time window. The period.deadline remains unchanged (which is the actual deadline after the proving window)
At the moment, a publication on the boundary is not considered part of either period. We need it to fall on one side or the other. I have arbitrarily chosen period.end to be an inclusive bound, so publications on that timestamp are within the period
The first publication of every new period will call `getCurrentFees` before the currentPeriodId is updated, but will pay the fees after it is updated. This means it will pay the wrong fee. In the case the fee goes up, the publication will fail unless the proposer already has enough of a deposit to make up the difference
The function comments should explain the rationale
It is possible for a period to end without the prover getting their stake. This could occur if they forget to include nextPublicationHeaderBytes but also: - the last publication for a while might occur in their proving window, so they cannot wait for another publication, or - they might prove some publications during their window before they realise that there will be no new publications in their window. In either case, we want a mechanism to recognise that the prover has completed their task but has not receive their stake. This PR clears the fees and stake when the funds are distributed so any non-zero values are still to be distributed.
Co-authored-by: Gustavo Gonzalez <[email protected]>
This was referenced Mar 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Over the development of the project, I've felt we've been working on multiple tightly related topics without properly gluing them together into the correct abstractions.
Some code smells like comparing commitment to its hashes in the PublicationFeed, or multiple functions permissioned to addresses that seemed to be similar (e.g. checkpoint tracker, signal service) convinced me to experiment building a modular L1Rollup using language features.
By "language features", I mean that the Solidity compiler should guide the developer in how to get started with a simple minimal rollup and then add functionalities as they see fit.
Rationale
I decided to aim for having a single contract on the L1 that is just a "rollup".
I had the gut that multiple concepts were duplicated across the implementations. For example, the current PublicationFeed only defines the
PublicationHeader
struct and tracks its hashes, where the publications include a commitment that's up to the L2 inbox to understand.It seems to me that there's a general concept of a single commitment of a publication, and whatever the publication means is left to the implementation. However, for developers to build their own publication formats, they must compose all these implementations and understand where/how to insert their logic.
Minimal Implementation
I'm proposing a minimal implementation of a rollup with the following interface:
Extending to other features
So far, the features we've been discussing like delayed inclusion, preconfirmations (i.e. permissioned proposer) or proving markets (i.e. who can prove) can build on top of these interfaces as overrides to a base implementation. It's up to the developer how they want to handle those cases, and if they want to prove with a simple ECDSA that should be ok.
Though, the library should offer the building blocks to deploy the most decentralized version of a rollup since the beginning. Here are some ideas of how the L1Rollup contract can be extended:
L1RollupDepositable
Multiple use cases to design a rollup involve economic incentives. For those, an extension that allows depositing and withdrawing ETH would be a good extension that is compatible with those use cases
L1RollupProverMarket
An extension that implements @ggonzalez94 work on an ahead-of-time auction for provers. Instead of being a manager contract, the proving market is held within the same rollup instance.
Preconfirmations
Teams can use the
L1Rollup
contract to override thepropose
function and set mechanisms to give the proposer rights dynamically.