No msg.sender checks in validator selection

Author: spalladino

l1-contracts/src/core/Rollup.sol

@@ -183,16 +183,17 @@ contract Rollup is IStaking, IValidatorSelection, IRollup, RollupCore {
    *
    * @param _ts - The timestamp to check
    * @param _archive - The archive to check (should be the latest archive)
+   * @param _who - The address to check
    *
    * @return uint256 - The slot at the given timestamp
    * @return uint256 - The block number at the given timestamp
    */
-  function canProposeAtTime(Timestamp _ts, bytes32 _archive)
+  function canProposeAtTime(Timestamp _ts, bytes32 _archive, address _who)
     external
     override(IRollup)
     returns (Slot, uint256)
   {
-    return ExtRollupLib2.canProposeAtTime(_ts, _archive);
+    return ExtRollupLib2.canProposeAtTime(_ts, _archive, _who);
   }
 
   function getTargetCommitteeSize() external view override(IValidatorSelection) returns (uint256) {

l1-contracts/src/core/interfaces/IRollup.sol

@@ -155,7 +155,9 @@ interface IRollup is IRollupCore, IHaveVersion {
     BlockHeaderValidationFlags memory _flags
   ) external;
 
-  function canProposeAtTime(Timestamp _ts, bytes32 _archive) external returns (Slot, uint256);
+  function canProposeAtTime(Timestamp _ts, bytes32 _archive, address _who)
+    external
+    returns (Slot, uint256);
 
   function getTips() external view returns (ChainTips memory);
 

l1-contracts/src/core/libraries/Errors.sol

@@ -102,6 +102,7 @@ library Errors {
   // Sequencer Selection (ValidatorSelection)
   error ValidatorSelection__EpochNotSetup(); // 0x10816cae
   error ValidatorSelection__InvalidProposer(address expected, address actual); // 0xa8843a68
+  error ValidatorSelection__MissingProposerSignature(address proposer, uint256 index);
   error ValidatorSelection__InvalidDeposit(address attester, address proposer); // 0x533169bd
   error ValidatorSelection__InsufficientAttestations(uint256 minimumNeeded, uint256 provided); // 0xaf47297f
   error ValidatorSelection__InvalidCommitteeCommitment(bytes32 reconstructed, bytes32 expected); // 0xca8d5954

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

@@ -76,8 +76,11 @@ library ExtRollupLib2 {
     return StakingLib.trySlash(_attester, _amount);
   }
 
-  function canProposeAtTime(Timestamp _ts, bytes32 _archive) external returns (Slot, uint256) {
-    return ValidatorSelectionLib.canProposeAtTime(_ts, _archive);
+  function canProposeAtTime(Timestamp _ts, bytes32 _archive, address _who)
+    external
+    returns (Slot, uint256)
+  {
+    return ValidatorSelectionLib.canProposeAtTime(_ts, _archive, _who);
   }
 
   function getCommitteeAt(Epoch _epoch) external returns (address[] memory) {

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

@@ -130,15 +130,10 @@ library ValidatorSelectionLib {
       setCachedProposer(_slot, proposer, proposerIndex);
     }
 
-    // If the proposer is who sent the tx, we're good
-    if (proposer == msg.sender) {
-      return;
-    }
-
     // Check if the proposer has signed, if not, fail
     bool hasProposerSignature = _attestations.isSignature(proposerIndex);
     if (!hasProposerSignature) {
-      revert Errors.ValidatorSelection__InvalidProposer(proposer, msg.sender);
+      revert Errors.ValidatorSelection__MissingProposerSignature(proposer, proposerIndex);
     }
 
     // Check if the signature is correct
@@ -358,7 +353,10 @@ library ValidatorSelectionLib {
     }
   }
 
-  function canProposeAtTime(Timestamp _ts, bytes32 _archive) internal returns (Slot, uint256) {
+  function canProposeAtTime(Timestamp _ts, bytes32 _archive, address _who)
+    internal
+    returns (Slot, uint256)
+  {
     Slot slot = _ts.slotFromTimestamp();
     RollupStore storage rollupStore = STFLib.getStorage();
 
@@ -373,9 +371,7 @@ library ValidatorSelectionLib {
     require(tipArchive == _archive, Errors.Rollup__InvalidArchive(tipArchive, _archive));
 
     (address proposer,) = getProposerAt(slot);
-    require(
-      proposer == msg.sender, Errors.ValidatorSelection__InvalidProposer(proposer, msg.sender)
-    );
+    require(proposer == _who, Errors.ValidatorSelection__InvalidProposer(proposer, _who));
 
     return (slot, pendingBlockNumber + 1);
   }

l1-contracts/test/RollupGetters.t.sol

@@ -153,8 +153,7 @@ contract RollupShouldBeGetters is ValidatorSelectionTestBase {
 
     vm.record();
 
-    vm.prank(proposer);
-    rollup.canProposeAtTime(t, log.archive);
+    rollup.canProposeAtTime(t, log.archive, proposer);
 
     (, bytes32[] memory writes) = vm.accesses(address(rollup));
     assertEq(writes.length, 0, "No writes should be done");

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

@@ -307,7 +307,7 @@ contract ValidatorSelectionTest is ValidatorSelectionTestBase {
   function testProposerAttestationNotProvided() public setup(4, 4) progressEpochs(2) {
     _testBlock(
       "mixed_block_1",
-      Errors.ValidatorSelection__InvalidProposer.selector,
+      Errors.ValidatorSelection__MissingProposerSignature.selector,
       3,
       4,
       TestFlags({
@@ -498,17 +498,26 @@ contract ValidatorSelectionTest is ValidatorSelectionTestBase {
     bytes32 digest = ProposeLib.digest(ree.proposePayload);
 
     {
-      uint256 signaturesCollected = 0;
+      uint256 signersIndex = 0;
+      uint256 signaturesCollected = _flags.proposerAttestationNotProvided ? 0 : 1;
       for (uint256 i = 0; i < ree.attestationsCount; i++) {
-        if (
-          (signaturesCollected >= _signatureCount)
-            || (ree.committee[i] == ree.proposer && _flags.proposerAttestationNotProvided)
-        ) {
+        if ((ree.committee[i] == ree.proposer && _flags.proposerAttestationNotProvided)) {
+          // If the proposer is not providing an attestation, we skip it
+          ree.attestations[i] = _createEmptyAttestation(ree.committee[i]);
+        } else if ((ree.committee[i] == ree.proposer)) {
+          // If the proposer is providing an attestation, set it
+          ree.attestations[i] = _createAttestation(ree.committee[i], digest);
+          ree.signers[signersIndex] = ree.committee[i];
+          signersIndex++;
+        } else if ((signaturesCollected >= _signatureCount)) {
+          // No need to create more signatures if we have collected enough
           ree.attestations[i] = _createEmptyAttestation(ree.committee[i]);
         } else {
+          // Create an attestation for the committee member and add them to the signers
           ree.attestations[i] = _createAttestation(ree.committee[i], digest);
-          ree.signers[signaturesCollected] = ree.committee[i];
+          ree.signers[signersIndex] = ree.committee[i];
           signaturesCollected++;
+          signersIndex++;
         }
       }
     }

yarn-project/ethereum/src/contracts/rollup.ts

@@ -504,6 +504,7 @@ export class RollupContract {
     }
     const latestBlock = await this.client.getBlock();
     const timeOfNextL1Slot = latestBlock.timestamp + slotDuration;
+    const who = typeof account === 'string' ? account : account.address;
 
     try {
       const {
@@ -512,7 +513,7 @@ export class RollupContract {
         address: this.address,
         abi: RollupAbi,
         functionName: 'canProposeAtTime',
-        args: [timeOfNextL1Slot, `0x${archive.toString('hex')}`],
+        args: [timeOfNextL1Slot, `0x${archive.toString('hex')}`, who],
         account,
       });