feat(rollup): seed snapshots by Maddiaa0 · Pull Request #13577 · AztecProtocol/aztec-packages

Maddiaa0 · 2025-04-15T15:07:38Z

Overview

Snapshot the seeds across epochs.

How it currently works:

If we are in the first epoch, we use a genesis seed = uint256.max
If the seed for the current epoch has already been calculated, we use the current seed
Each time we enter a new epoch, we use the latest seed
If nothing exists, we use the latest sample seed

In cases where we are looking at the committee for a given block from the future, we may end up calculated a different committee, as we use the latest sample seed, which may have changed in the time since.

This PR builds ontop of validator snapshots ( where the validator set cannot change in size during an epoch ) to make sure that even when we are querying from the future, and the latest seed has changed, we do not end up with a different calculated committee.

How it should work:
Seeds are no longer stored for each epoch in EpochData, instead they are snapshotted.

If an epoch has not been set up before, it's is either genesis, where it is given the seed of uint224 max, or it will use the last seen seed for a given epoch.
Whenever an epoch is setup, it will calculate if it needs to create a new seed snapshot for the next epoch to use.
If the epoch is not setup, it will use the most recent seed snapshot. ( same behaviour, but not we can query into the past and have a consistent latest snapshot )

fixes: #13559

Maddiaa0 · 2025-04-15T15:07:58Z

feat(rollup): seed snapshots #13577 : 2 dependent PRs (#13700 , #13910 ) 👈 (View in Graphite)
feat(contracts): static + snapshotted validator set #13046 : 1 other dependent PR (#13649 )
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

LHerskind

Think we can get rid of a bunch of the branching which should simplify a bunch of the tests. Also think we can get rid of the cheatcode for deposits (but would save that for a separate pr).

LHerskind · 2025-04-28T16:00:38Z

l1-contracts/src/core/RollupCore.sol

    onlyOwner
  {
    CheatLib.cheat__InitialiseValidatorSet(_args);
-    setupEpoch();


This one was leftover from last? 👀

LHerskind · 2025-04-28T16:15:36Z

l1-contracts/src/core/libraries/validator-selection/ValidatorSelectionLib.sol

+
+    // Set the sample seed for the next epoch if required
+    // function handles the case where it is already set
+    setSampleSeedForEpoch(_epochNumber + Epoch.wrap(1));


It might be slightly nicer separation if this is moved upwards, e.g., then you have things altering seeds and then things using seeds. Since it should alter in the future don't seems like it should cause an issue.

LHerskind · 2025-04-28T16:27:48Z

l1-contracts/src/core/libraries/validator-selection/ValidatorSelectionLib.sol

   */
-  function setupEpoch(StakingStorage storage _stakingStore) internal {
-    Epoch epochNumber = Timestamp.wrap(block.timestamp).epochFromTimestamp();
+  function setupEpoch(StakingStorage storage _stakingStore, Epoch _epochNumber) internal {


Just a note for my sanity, we use the case where _epochNumber != currentEpoch when we mught wanna look at a historical and not setup epoch as the sampleValidators are using it.

LHerskind · 2025-04-28T16:31:10Z

l1-contracts/src/core/libraries/validator-selection/ValidatorSelectionLib.sol

+    uint224 sampleSeed = getSampleSeed(_epochNumber);
+
+    // If no sample seed is set, we are in a genesis state, we set the sample seed to max and push it into the store
+    if (sampleSeed == 0) {


Should we just set it up at genesis instead? As part of the construction? I might be forgetting something, but would that not at least get rid of this special case in the function that we will continously call? 🤷

LHerskind · 2025-04-28T16:36:50Z

l1-contracts/src/core/libraries/validator-selection/ValidatorSelectionLib.sol


-    // Emulate a sampling of the validators
-    uint256 sampleSeed = getSampleSeed(_epochNumber);
+    // Check if the latest checkpoint is for the next epoch


If we set the initial using the constructor as well it might simplify us slightly here as well as we won't have the case where the last checkpoint does not exists so one less branch to deal with.

LHerskind · 2025-04-28T16:40:50Z

l1-contracts/test/validator-selection/ValidatorSelectionBase.sol

+  mapping(address => bool) internal _seenCommittee;
+
+  /**
+   * @notice  Set up the contracts needed for the tests with time aligned to the provided block name


The comment seems copied over, not taking a name.

LHerskind · 2025-04-28T16:46:10Z

l1-contracts/test/validator-selection/ValidatorSelectionBase.sol

+    testERC20.approve(address(rollup), TestConstants.AZTEC_MINIMUM_STAKE * _validatorCount);
+
+    if (_validatorCount > 0) {
+      rollup.cheat__InitialiseValidatorSet(initialValidators);


I think we could get rid of the cheat after this pr, but should be done fully separately, no need to grow this one as there is a bunch of node and test stuff that would change

LHerskind · 2025-04-28T16:48:32Z

l1-contracts/test/validator-selection/setupEpoch.tree

@@ -0,0 +1,19 @@
+SetupEpochTest
+├── when the rollup is in genesis state


Think we could get rid of a whole bunch of these by using the constructor to set some of these values 👀

l1-contracts/test/validator-selection/ValidatorSelection.t.sol

LHerskind · 2025-04-28T16:53:03Z

l1-contracts/test/validator-selection/ValidatorSelection.t.sol

-  }
-
-  function testInitialCommitteeMatch() public setup(4) {
+  function testInitialCommitteeMatch() public setup(4) progressEpoch {


If we apply the progressEpoch to each one, whould it not make sense to apply it to the setup so there is one less thing to think about?

Its also used in the setupEpoch test without progressEpoch applied. I know its a bit annoying in this file.

LHerskind · 2025-04-29T10:49:36Z

l1-contracts/src/core/libraries/validator-selection/ValidatorSelectionLib.sol

  function setupEpoch(StakingStorage storage _stakingStore, Epoch _epochNumber) internal {
    ValidatorSelectionStorage storage store = getStorage();

+    //################ Seeds ################


Following the seed snapshots pr #13577 there was a change that mean that `getCurrentProposer` can end up running the `setupEpoch` (reducing number of different flows etc). However, that meant that when we were running our benchmark test to get some gas numbers, we never end up including the gas spent to setup the epoch. To address, we are now explicitly calling `setupEpoch` such that we get some neat measurements for it, also making it clear when changes are made that impact the sampling. A nice side effect, is that it more simply allow us to do the proper amortized cost for sampling as the propose is for 360 tx, but sampling is for 11520 (32 * 360). This new setup makes it more simple to see the direct impact from the sampling on tx costs etc.

Maddiaa0 mentioned this pull request Apr 15, 2025

feat(contracts): static + snapshotted validator set #13046

Merged

Maddiaa0 force-pushed the md/seed-snapshots branch from 0ad7f97 to e9bc7f3 Compare April 15, 2025 15:51

Maddiaa0 marked this pull request as ready for review April 15, 2025 18:19

Maddiaa0 linked an issue Apr 15, 2025 that may be closed by this pull request

feat(sol): snapshotting sample seeds #13559

Closed

Maddiaa0 mentioned this pull request Apr 17, 2025

feat(rollup)!: do not accept blocks from committee's under specified size #13649

Closed

Base automatically changed from md/val-snapshot to master April 18, 2025 22:13

Maddiaa0 requested a review from charlielye as a code owner April 18, 2025 22:13

Maddiaa0 force-pushed the md/seed-snapshots branch from 7f613a9 to 49ab911 Compare April 21, 2025 20:57

Maddiaa0 mentioned this pull request Apr 21, 2025

feat(sol): remove committee storage #13700

Merged

charlielye removed their request for review April 22, 2025 14:42

Maddiaa0 added 8 commits April 23, 2025 08:31

feat(rollup): seed snapshots

53abe87

chore: document

5a6fe8a

chore: add setupEpoch test suite

6bbadd4

fmt

9724ef2

fmt: function order

b212389

Merge branch 'master' into md/seed-snapshots

de29295

post merge fix

40967d9

Merge branch 'master' into md/seed-snapshots

4321429

Maddiaa0 force-pushed the md/seed-snapshots branch from 49ab911 to 4321429 Compare April 28, 2025 09:52

fmt

1ecec4f

Maddiaa0 force-pushed the md/seed-snapshots branch from 93afb33 to 1ecec4f Compare April 28, 2025 10:21

Maddiaa0 requested a review from LHerskind April 28, 2025 10:23

update getseed

08d1132

LHerskind reviewed Apr 28, 2025

View reviewed changes

chore: genesis state changes

091c43e

Maddiaa0 mentioned this pull request Apr 29, 2025

chore(rollup): add function to trigger seed snapshot for next epoch #13910

Merged

LHerskind approved these changes Apr 29, 2025

View reviewed changes

LHerskind added this pull request to the merge queue Apr 29, 2025

Merged via the queue into master with commit a6a33e1 Apr 29, 2025
8 checks passed

LHerskind deleted the md/seed-snapshots branch April 29, 2025 11:46

AztecBot mentioned this pull request Apr 29, 2025

chore(master): release 0.87.0 #13800

Merged

LHerskind mentioned this pull request May 1, 2025

chore: run setupEpoch separately #13984

Merged

		@@ -0,0 +1,19 @@
		SetupEpochTest
		├── when the rollup is in genesis state

Conversation

Maddiaa0 commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Uh oh!

Maddiaa0 commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LHerskind left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Maddiaa0 commented Apr 15, 2025 •

edited

Loading

Maddiaa0 commented Apr 15, 2025 •

edited

Loading