Skip to content

Generalise Signal Service #107

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: signal-service
Choose a base branch
from
Open

Generalise Signal Service #107

wants to merge 10 commits into from

Conversation

nikeshnazareth
Copy link
Collaborator

@nikeshnazareth nikeshnazareth commented Apr 21, 2025 •
edited
Loading

As noted on #89 (see here), I think it will be simpler to create a multi-rollup signal service right from the start. This is WIP but since I will be unavailable this week I thought I should push the current version for your consideration.

The overall intention is:

  • Avoid duplicate checkpoints. The TaikoAnchor and CheckpointTracker now use the signal service for their checkpoint/l1blocks storage.
  • Remove commitment access control. Anyone can post commitments, which are saved under the source address. Access control can be achieved on the recipient end (they should only use checkpoints saved by a valid address). This makes it possible to support multiple rollups. At this point I have made the change to the CommitmentStore and propagated them to the TaikoAnchor and CheckpointTracker, but not to the rest of the files. The codebase is currently inconsistent
  • Generalise the ETH bridge to handle multiple rollups. This is not implemented. I suspect we will want to record which chain holds which deposits (so a malicious rollup cannot withdraw funds held by another rollup). To keep it symmetric, the L2 side of the bridge will be deployed with infinite ETH held by the L1.
  • Allow the CheckpointTracker genesis to start from an arbitrary publication ID (or perhaps the current publication ID). This makes sense as long as we have a shared publication feed, because a new rollup should not have to process all the historical publications before it was deployed.

@nikeshnazareth
Remove l1BlockHashes mapping. Use the commitment store instead
d163d36 
We are already saving every anchor block to the commitment store.
No need to also save a separate mapping.
@nikeshnazareth
Remove _provenCheckpoint. Use the commitment store instead
b9dd479
@nikeshnazareth
Replace CommitmentStore access control with a source address
5bafacb 
The dependent contracts have not been updated so the codebase is
currently inconsistent
@nikeshnazareth
Update TaikoAnchor to use the unauthenticated CommitmentStore
ce491eb
@nikeshnazareth
Update CheckpointTracker to use the unauthenticated CommitmentStore
43ed7db
@nikeshnazareth
Remove SignalService constructor. It no longer needs an owner
d1a42bd
@nikeshnazareth
Provide temporary mechanism to setAuthorizedCommitter
9830cb5 
This mimics the previous behaviour to minimise changes to the tests
before we generalise it
@nikeshnazareth
Ensure SignalService withdrawals are deducted from the release authority
d4fb508 
This means rollups cannot bridge out funds that were not originally sent to them.
@nikeshnazareth
Use arbitrary release authority. There is no longer an official autho…
f064e39 
…rised committer
@nikeshnazareth
Require _isValidDeposit on every claim.
b7928b2 
The L1 will accept any deposit from any chain (noting that it can
only release funds that were originally sent to that chain).
The L2 will ensure the deposit was intended for its chain, by ensuring
the release authority is the relevant checkpoint tracker.
@github-actions GitHub Actions
Copy link

Changes to gas cost

Generated at commit: 1392cc6a039eb6a4d0d89a205fc07c3d7fed2fe7, compared to commit: a9b795fecea7b1132de8b9f3b58c14b7e3743dd3

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
CheckpointTracker getProvenCheckpoint
proveTransition		 +4,303 ❌
-4,485 ✅		 +87.09%
-4.34%
Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
CheckpointTracker 919,173 (-60,264) getProvenCheckpoint
proveTransition		 9,244 (+4,303)
98,899 (-4,485)		 +87.09%
-4.34%		 9,244 (+4,303)
98,899 (-4,485)		 +87.09%
-4.34%		 9,244 (+4,303)
98,899 (-4,485)		 +87.09%
-4.34%		 9,244 (+4,303)
98,899 (-4,485)		 +87.09%
-4.34%		 2 (0)
1 (0)
ProverManager 2,854,874 (+12) prove 103,391 (-1,795) -1.71% 125,449 (-1,795) -1.41% 127,326 (-1,795) -1.39% 140,007 (-1,795) -1.27% 6 (0)
SignalService 3,991,306 (-200,563)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@nikeshnazareth