Do not sample entire committee when checking proposer

Author: spalladino

l1-contracts/src/core/libraries/rollup/StakingLib.sol

@@ -353,6 +353,14 @@ library StakingLib {
     );
   }
 
+  function getAttesterFromIndexAtTime(uint256 _index, Timestamp _timestamp)
+    internal
+    view
+    returns (address)
+  {
+    return getStorage().gse.getAttesterFromIndexAtTime(address(this), _index, _timestamp);
+  }
+
   function getAttestersFromIndicesAtTime(Timestamp _timestamp, uint256[] memory _indices)
     internal
     view

l1-contracts/src/core/libraries/rollup/ValidatorSelectionLib.sol

@@ -219,7 +219,6 @@ library ValidatorSelectionLib {
     setCachedProposer(_slot, proposer, stack.proposerIndex);
   }
 
-  // Q: Do we still need to cache the proposer?
   function setCachedProposer(Slot _slot, address _proposer, uint256 _proposerIndex) internal {
     require(
       _proposerIndex <= type(uint96).max,
@@ -230,25 +229,24 @@ library ValidatorSelectionLib {
   }
 
   function getProposerAt(Slot _slot) internal returns (address, uint256) {
-    (address cachedProposer, uint256 proposerIndex) = getCachedProposer(_slot);
+    (address cachedProposer, uint256 cachedProposerIndex) = getCachedProposer(_slot);
     if (cachedProposer != address(0)) {
-      return (cachedProposer, proposerIndex);
+      return (cachedProposer, cachedProposerIndex);
     }
 
-    // @note this is deliberately "bad" for the simple reason of code reduction.
-    //       it does not need to actually return the full committee and then draw from it
-    //       it can just return the proposer directly, but then we duplicate the code
-    //       which we just don't have room for right now...
     Epoch epochNumber = _slot.epochFromSlot();
 
     uint224 sampleSeed = getSampleSeed(epochNumber);
-    address[] memory committee = sampleValidators(epochNumber, sampleSeed);
-    if (committee.length == 0) {
+    (uint32 ts, uint256[] memory indices) = sampleValidatorsIndices(epochNumber, sampleSeed);
+    uint256 committeeSize = indices.length;
+    if (committeeSize == 0) {
       return (address(0), 0);
     }
-
-    uint256 index = computeProposerIndex(epochNumber, _slot, sampleSeed, committee.length);
-    return (committee[index], index);
+    uint256 proposerIndex = computeProposerIndex(epochNumber, _slot, sampleSeed, committeeSize);
+    return (
+      StakingLib.getAttesterFromIndexAtTime(indices[proposerIndex], Timestamp.wrap(ts)),
+      proposerIndex
+    );
   }
 
   /**
@@ -260,24 +258,7 @@ library ValidatorSelectionLib {
    * @return The validators for the given epoch
    */
   function sampleValidators(Epoch _epoch, uint224 _seed) internal returns (address[] memory) {
-    ValidatorSelectionStorage storage store = getStorage();
-    uint32 ts = epochToSampleTime(_epoch);
-    uint256 validatorSetSize = StakingLib.getAttesterCountAtTime(Timestamp.wrap(ts));
-    uint256 targetCommitteeSize = store.targetCommitteeSize;
-
-    require(
-      validatorSetSize >= targetCommitteeSize,
-      Errors.ValidatorSelection__InsufficientCommitteeSize(validatorSetSize, targetCommitteeSize)
-    );
-
-    if (targetCommitteeSize == 0) {
-      return new address[](0);
-    }
-
-    // Sample the larger committee
-    uint256[] memory indices =
-      SampleLib.computeCommittee(targetCommitteeSize, validatorSetSize, _seed);
-
+    (uint32 ts, uint256[] memory indices) = sampleValidatorsIndices(_epoch, _seed);
     return StakingLib.getAttestersFromIndicesAtTime(Timestamp.wrap(ts), indices);
   }
 
@@ -420,6 +401,35 @@ library ValidatorSelectionLib {
     }
   }
 
+  /**
+   * @notice  Samples a validator set for a specific epoch and returns their indices within the set.
+   *
+   * @dev     Only used internally, should never be called for anything but the "next" epoch
+   *          Allowing us to always use `lastSeed`.
+   *
+   * @return  The sample time and the indices of the validators for the given epoch
+   */
+  function sampleValidatorsIndices(Epoch _epoch, uint224 _seed)
+    internal
+    returns (uint32, uint256[] memory)
+  {
+    ValidatorSelectionStorage storage store = getStorage();
+    uint32 ts = epochToSampleTime(_epoch);
+    uint256 validatorSetSize = StakingLib.getAttesterCountAtTime(Timestamp.wrap(ts));
+    uint256 targetCommitteeSize = store.targetCommitteeSize;
+
+    require(
+      validatorSetSize >= targetCommitteeSize,
+      Errors.ValidatorSelection__InsufficientCommitteeSize(validatorSetSize, targetCommitteeSize)
+    );
+
+    if (targetCommitteeSize == 0) {
+      return (ts, new uint256[](0));
+    }
+
+    return (ts, SampleLib.computeCommittee(targetCommitteeSize, validatorSetSize, _seed));
+  }
+
   /**
    * @notice  Computes the nextSeed for an epoch
    *
@@ -457,7 +467,7 @@ library ValidatorSelectionLib {
    * @return The index of the proposer
    */
   function computeProposerIndex(Epoch _epoch, Slot _slot, uint256 _seed, uint256 _size)
-    private
+    internal
     pure
     returns (uint256)
   {

l1-contracts/test/RollupGetters.t.sol

@@ -14,6 +14,7 @@ import {StakingQueueConfig} from "@aztec/core/libraries/compressed-data/StakingQ
 import {ValidatorSelectionTestBase} from "./validator-selection/ValidatorSelectionBase.sol";
 import {IRewardDistributor} from "@aztec/governance/interfaces/IRewardDistributor.sol";
 import {IBoosterCore} from "@aztec/core/reward-boost/RewardBooster.sol";
+import {ValidatorSelectionLib} from "@aztec/core/libraries/rollup/ValidatorSelectionLib.sol";
 
 /**
  * Testing the things that should be getters are not updating state!
@@ -103,6 +104,34 @@ contract RollupShouldBeGetters is ValidatorSelectionTestBase {
     assertEq(writes.length, 0, "No writes should be done");
   }
 
+  // Checks that getProposerAt yields the same result as sampling the entire committee
+  // and then fetching the proposer from it given the proposer index.
+  function test_getProposerFromCommittee(uint16 _slot, bool _setup) external setup(4, 4) {
+    timeCheater.cheat__jumpForwardEpochs(2);
+    Slot s = Slot.wrap(timeCheater.currentSlot()) + Slot.wrap(_slot);
+    Timestamp t = timeCheater.slotToTimestamp(s);
+
+    vm.warp(Timestamp.unwrap(t));
+
+    if (_setup) {
+      rollup.setupEpoch();
+    }
+
+    vm.record();
+
+    address proposer = rollup.getProposerAt(t);
+
+    address[] memory committee = rollup.getCommitteeAt(t);
+    uint256 seed = rollup.getSampleSeedAt(t);
+    Epoch epoch = rollup.getEpochAt(t);
+    uint256 proposerIndex = ValidatorSelectionLib.computeProposerIndex(epoch, s, seed, 4);
+
+    assertEq(proposer, committee[proposerIndex], "proposer should be the same");
+
+    (, bytes32[] memory writes) = vm.accesses(address(rollup));
+    assertEq(writes.length, 0, "No writes should be done");
+  }
+
   function test_validateHeader() external setup(4, 4) {
     // Todo this one is a bit annoying here really. We need a lot of header information.
   }