fix: Avoid early proof submission prover griefing

l1-contracts/src/core/Rollup.sol

@@ -571,6 +571,15 @@ contract Rollup is IStaking, IValidatorSelection, IRollup, RollupCore {
     return RewardExtLib.getHasSubmitted(_epoch, _length, _prover);
   }
 
+  function getHasSubmittedFullEpoch(Epoch _epoch, uint256 _length, address _prover)
+    external
+    view
+    override(IRollup)
+    returns (bool)
+  {
+    return RewardExtLib.getHasSubmittedFullEpoch(_epoch, _length, _prover);
+  }
+
   function getHasClaimed(address _prover, Epoch _epoch) external view override(IRollup) returns (bool) {
     return RewardExtLib.getHasClaimed(_prover, _epoch);
   }

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

@@ -273,6 +273,7 @@ interface IRollup is IRollupCore, IHaveVersion {
   function getCollectiveProverRewardsForEpoch(Epoch _epoch) external view returns (uint256);
   function getSpecificProverRewardsForEpoch(Epoch _epoch, address _prover) external view returns (uint256);
   function getHasSubmitted(Epoch _epoch, uint256 _length, address _prover) external view returns (bool);
+  function getHasSubmittedFullEpoch(Epoch _epoch, uint256 _length, address _prover) external view returns (bool);
   function getHasClaimed(address _prover, Epoch _epoch) external view returns (bool);
 
   function getProofSubmissionEpochs() external view returns (uint256);

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

@@ -124,6 +124,10 @@ library RewardExtLib {
     return RewardLib.getHasSubmitted(_epoch, _length, _prover);
   }
 
+  function getHasSubmittedFullEpoch(Epoch _epoch, uint256 _length, address _prover) external view returns (bool) {
+    return RewardLib.getHasSubmittedFullEpoch(_epoch, _length, _prover);
+  }
+
   function getHasClaimed(address _prover, Epoch _epoch) external view returns (bool) {
     return RewardLib.getHasClaimed(_prover, _epoch);
   }

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

@@ -43,9 +43,15 @@ library BpsLib {
   }
 }
 
+struct ProverRegistration {
+  uint248 shares;
+  bool fullEpoch; // Set to true when the registration was submitted after the epoch is closed. Used for activity score
+    // bumping
+}
+
 struct SubEpochRewards {
   uint256 summedShares;
-  mapping(address prover => uint256 shares) shares;
+  mapping(address prover => ProverRegistration) registrations;
 }
 
 struct EpochRewards {
@@ -182,7 +188,7 @@ library RewardLib {
 
       EpochRewards storage e = rewardStorage.epochRewards[_epochs[i]];
       SubEpochRewards storage se = e.subEpoch[e.longestProvenLength];
-      uint256 shares = se.shares[_prover];
+      uint256 shares = se.registrations[_prover].shares;
       if (shares > 0) {
         accumulatedRewards += (shares * e.rewards / se.summedShares);
       }
@@ -212,7 +218,16 @@ library RewardLib {
       SubEpochRewards storage $sr = $er.subEpoch[length];
       address prover = _args.args.proverId;
 
-      require($sr.shares[prover] == 0, Errors.Rollup__ProverHaveAlreadySubmitted(prover, _endEpoch));
+      ProverRegistration memory previousRegistration = $sr.registrations[prover];
+      // There is a point in updating in 2 cases:
+      // 1. This is the first time we are submitting, so shares are at 0
+      // 2. A prover is submitting the same proof a second time after an epoch closed so they would get their activity
+      // score bump. If the second check wasn't there, it would theoretically be possible (but unlikely) to block a
+      // prover from increasing their activity score by submitting a proof for the epoch while it is ongoing
+      require(
+        previousRegistration.shares == 0 || (!previousRegistration.fullEpoch && _fullEpochProof),
+        Errors.Rollup__ProverHaveAlreadySubmitted(prover, _endEpoch)
+      );
       // The prover is only marked active if they have provided a full epoch proof
       uint256 shares = _fullEpochProof
         ? rewardStorage.config.booster.updateAndGetShares(prover)
@@ -225,8 +240,8 @@ library RewardLib {
       // back if a misbehaving booster ever crosses this layer.
       require(shares > 0, Errors.RewardLib__ZeroShares(prover));
 
-      $sr.shares[prover] = shares;
-      $sr.summedShares += shares;
+      $sr.registrations[prover] = ProverRegistration({shares: shares.toUint248(), fullEpoch: _fullEpochProof});
+      $sr.summedShares = $sr.summedShares - previousRegistration.shares + shares;
     }
 
     if (length > $er.longestProvenLength) {
@@ -349,7 +364,11 @@ library RewardLib {
   }
 
   function getHasSubmitted(Epoch _epoch, uint256 _length, address _prover) internal view returns (bool) {
-    return getStorage().epochRewards[_epoch].subEpoch[_length].shares[_prover] > 0;
+    return getStorage().epochRewards[_epoch].subEpoch[_length].registrations[_prover].shares > 0;
+  }
+
+  function getHasSubmittedFullEpoch(Epoch _epoch, uint256 _length, address _prover) internal view returns (bool) {
+    return getStorage().epochRewards[_epoch].subEpoch[_length].registrations[_prover].fullEpoch;
   }
 
   function getHasClaimed(address _prover, Epoch _epoch) internal view returns (bool) {
@@ -376,11 +395,11 @@ library RewardLib {
 
     // Only if prover has shares will he get a reward. Also avoid a 0-div
     // in case of no shares at all.
-    if (se.shares[_prover] == 0) {
+    if (se.registrations[_prover].shares == 0) {
       return 0;
     }
 
-    return (se.shares[_prover] * er.rewards / se.summedShares);
+    return (se.registrations[_prover].shares * er.rewards / se.summedShares);
   }
 
   /**

l1-contracts/test/MultiProof.t.sol

@@ -408,4 +408,73 @@ contract MultiProofTest is RollupBase {
 
     assertEq(activityScoreBefore, activityScoreAfter, "Proving an open epoch should not increase activity score");
   }
+
+  function testFullEpochProofUpgradesOpenEpochRegistration() public setUpFor("mixed_checkpoint_1") {
+    address alice = address(bytes20("alice"));
+    Epoch epoch = Epoch.wrap(1);
+
+    deal(address(testERC20), address(feeJuicePortal), 15e6 * 1e18);
+
+    // Start in epoch 1, booster can't boost in epoch 0.
+    warpToL2Slot(EPOCH_DURATION);
+    _proposeCheckpoint("mixed_checkpoint_1", EPOCH_DURATION, 15e6);
+
+    // The proof lands while epoch 1 is still running. It covers every checkpoint the epoch will have, but the
+    // contract cannot know that yet. The proverId is alice (but the actual prover could be anyone at all)
+    _proveCheckpoints("mixed_checkpoint_", 1, 1, alice);
+    assertTrue(rollup.getHasSubmitted(epoch, 1, alice), "open-epoch proof not registered");
+    assertFalse(rollup.getHasSubmittedFullEpoch(epoch, 1, alice), "open-epoch proof registered as full epoch");
+
+    // Close epoch 1 without proposing anything else in it. The proof Alice submitted is now a full proof.
+    warpToL2Slot(2 * EPOCH_DURATION);
+    uint256 activityScoreBefore = rewardBooster.getActivityScore(alice).value;
+
+    // Alice submits the proof again
+    _proveCheckpoints("mixed_checkpoint_", 1, 1, alice);
+
+    assertGt(
+      rewardBooster.getActivityScore(alice).value, activityScoreBefore, "full-finished resubmission did not bump score"
+    );
+    assertTrue(rollup.getHasSubmittedFullEpoch(epoch, 1, alice), "registration not upgraded to full epoch");
+
+    // Alice is the only registration, so she must be owed the whole pool. Counting her shares twice in summedShares
+    // would halve this.
+    uint256 pool = rollup.getCollectiveProverRewardsForEpoch(epoch);
+    assertGt(pool, 0, "empty prover pool");
+    assertEq(rollup.getSpecificProverRewardsForEpoch(epoch, alice), pool, "alice not owed the whole pool");
+
+    // The registration can be upgraded only once.
+    _proveCheckpointsFail(
+      "mixed_checkpoint_",
+      1,
+      1,
+      alice,
+      abi.encodeWithSelector(Errors.Rollup__ProverHaveAlreadySubmitted.selector, alice, epoch)
+    );
+  }
+
+  function testNonFullResubmissionOfOpenEpochProofReverts() public setUpFor("mixed_checkpoint_1") {
+    address alice = address(bytes20("alice"));
+    Epoch epoch = Epoch.wrap(1);
+
+    deal(address(testERC20), address(feeJuicePortal), 30e6 * 1e18);
+
+    warpToL2Slot(EPOCH_DURATION);
+    _proposeCheckpoint("mixed_checkpoint_1", EPOCH_DURATION, 15e6);
+    _proveCheckpoints("mixed_checkpoint_", 1, 1, alice);
+
+    // A second checkpoint lands in epoch 1, so the earlier proof does not cover the whole epoch.
+    _proposeCheckpoint("mixed_checkpoint_2", EPOCH_DURATION + 1, 15e6);
+    warpToL2Slot(2 * EPOCH_DURATION);
+
+    // Resubmitting it after the epoch closed would re-read alice's shares at the decayed score, so it must revert.
+    _proveCheckpointsFail(
+      "mixed_checkpoint_",
+      1,
+      1,
+      alice,
+      abi.encodeWithSelector(Errors.Rollup__ProverHaveAlreadySubmitted.selector, alice, epoch)
+    );
+    assertFalse(rollup.getHasSubmittedFullEpoch(epoch, 1, alice), "partial registration marked full epoch");
+  }
 }

l1-contracts/test/rollup/libraries/rewardlib/RewardLibWrapper.sol

@@ -178,6 +178,15 @@ contract RewardLibWrapper {
     RewardLib.handleRewardsAndFees(_args, _endEpoch, _rollupConfig(), true, new address[](0), registryRewardOverrides);
   }
 
+  function handleRewardsAndFees(SubmitEpochRootProofArgs calldata _args, Epoch _endEpoch, bool _fullEpochProof)
+    external
+  {
+    RegistryRewardOverride[MAX_REGISTRY_REWARD_OVERRIDES] memory registryRewardOverrides;
+    RewardLib.handleRewardsAndFees(
+      _args, _endEpoch, _rollupConfig(), _fullEpochProof, new address[](0), registryRewardOverrides
+    );
+  }
+
   function handleRewardsAndFees(
     SubmitEpochRootProofArgs calldata _args,
     Epoch _endEpoch,
@@ -209,6 +218,14 @@ contract RewardLibWrapper {
     return RewardLib.getHasSubmitted(_epoch, _length, _prover);
   }
 
+  function getHasSubmittedFullEpoch(Epoch _epoch, uint256 _length, address _prover) external view returns (bool) {
+    return RewardLib.getHasSubmittedFullEpoch(_epoch, _length, _prover);
+  }
+
+  function getActivityScore(address _prover) external view returns (uint256) {
+    return booster.getActivityScore(_prover).value;
+  }
+
   function getCurrentEpoch() external view returns (Epoch) {
     return currentEpoch;
   }
@@ -218,7 +235,7 @@ contract RewardLibWrapper {
   }
 
   function getProverShares(Epoch _epoch, uint256 _length, address _prover) external view returns (uint256) {
-    return RewardLib.getStorage().epochRewards[_epoch].subEpoch[_length].shares[_prover];
+    return RewardLib.getStorage().epochRewards[_epoch].subEpoch[_length].registrations[_prover].shares;
   }
 
   function getSummedShares(Epoch _epoch, uint256 _length) external view returns (uint256) {

l1-contracts/test/rollup/libraries/rewardlib/handleRewards.t.sol

@@ -8,12 +8,80 @@ import {Errors} from "@aztec/core/libraries/Errors.sol";
 import {Math} from "@oz/utils/math/Math.sol";
 
 contract HandleRewardsTest is RewardLibBase {
-  function test_GivenProverHasAlreadySubmitted() external prepare(400e18, 7000) {
+  modifier givenProverHasAlreadySubmitted() {
+    _;
+  }
+
+  function test_GivenTheExistingRegistrationIsFullEpoch()
+    external
+    prepare(400e18, 7000)
+    givenProverHasAlreadySubmitted
+  {
     // it reverts with {Rollup__ProverHaveAlreadySubmitted}
     wrapper.handleRewardsAndFees(args, Epoch.wrap(0));
 
     vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__ProverHaveAlreadySubmitted.selector, prover, Epoch.wrap(0)));
     wrapper.handleRewardsAndFees(args, Epoch.wrap(0));
+
+    vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__ProverHaveAlreadySubmitted.selector, prover, Epoch.wrap(0)));
+    wrapper.handleRewardsAndFees(args, Epoch.wrap(0), false);
+  }
+
+  modifier givenTheExistingRegistrationIsNotFullEpoch() {
+    wrapper.handleRewardsAndFees(args, Epoch.wrap(0), false);
+    assertTrue(wrapper.getHasSubmitted(Epoch.wrap(0), 1, prover));
+    assertFalse(wrapper.getHasSubmittedFullEpoch(Epoch.wrap(0), 1, prover));
+    _;
+  }
+
+  function test_WhenTheProofIsNotFullEpoch()
+    external
+    prepare(400e18, 7000)
+    givenProverHasAlreadySubmitted
+    givenTheExistingRegistrationIsNotFullEpoch
+  {
+    // it reverts with {Rollup__ProverHaveAlreadySubmitted}
+    wrapper.setCurrentEpoch(Epoch.wrap(1));
+
+    vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__ProverHaveAlreadySubmitted.selector, prover, Epoch.wrap(0)));
+    wrapper.handleRewardsAndFees(args, Epoch.wrap(0), false);
+  }
+
+  function test_WhenTheProofIsFullEpoch()
+    external
+    prepare(400e18, 7000)
+    givenProverHasAlreadySubmitted
+    givenTheExistingRegistrationIsNotFullEpoch
+  {
+    // it marks the registration as full epoch
+    // it bumps the prover activity score
+    // it replaces the prover shares in summed shares
+    // it leaves longestProvenLength and the prover rewards unchanged
+
+    // A second prover at the same length shows that only the upgraded prover's contribution to summedShares moves.
+    address otherProver = makeAddr("prover2");
+    args.args.proverId = otherProver;
+    wrapper.handleRewardsAndFees(args, Epoch.wrap(0), false);
+    uint256 otherShares = wrapper.getProverShares(Epoch.wrap(0), 1, otherProver);
+
+    uint256 rewardsBefore = wrapper.getCollectiveProverRewardsForEpoch(Epoch.wrap(0));
+    uint256 scoreBefore = wrapper.getActivityScore(prover);
+
+    // The booster bumps at most once per epoch and starts at epoch 0, so move on to the epoch where the full-finished
+    // proof of epoch 0 lands.
+    wrapper.setCurrentEpoch(Epoch.wrap(1));
+    args.args.proverId = prover;
+    wrapper.handleRewardsAndFees(args, Epoch.wrap(0), true);
+
+    uint256 shares = wrapper.getProverShares(Epoch.wrap(0), 1, prover);
+    assertTrue(wrapper.getHasSubmittedFullEpoch(Epoch.wrap(0), 1, prover), "registration not marked full epoch");
+    assertGt(wrapper.getActivityScore(prover), scoreBefore, "activity score not bumped");
+    assertEq(wrapper.getSummedShares(Epoch.wrap(0), 1), shares + otherShares, "summed shares counts prover twice");
+    assertEq(wrapper.getLongestProvenLength(Epoch.wrap(0)), 1, "longest proven length");
+    assertEq(wrapper.getCollectiveProverRewardsForEpoch(Epoch.wrap(0)), rewardsBefore, "prover rewards changed");
+
+    vm.expectRevert(abi.encodeWithSelector(Errors.Rollup__ProverHaveAlreadySubmitted.selector, prover, Epoch.wrap(0)));
+    wrapper.handleRewardsAndFees(args, Epoch.wrap(0), true);
   }
 
   modifier givenProverHasNotSubmitted() {

l1-contracts/test/rollup/libraries/rewardlib/handleRewards.tree

@@ -1,6 +1,15 @@
 HandleRewards
 ├── given prover has already submitted
-│   └── it reverts with {Rollup__ProverHaveAlreadySubmitted}
+│   ├── given the existing registration is full epoch
+│   │   └── it reverts with {Rollup__ProverHaveAlreadySubmitted}
+│   └── given the existing registration is not full epoch
+│       ├── when the proof is not full epoch
+│       │   └── it reverts with {Rollup__ProverHaveAlreadySubmitted}
+│       └── when the proof is full epoch
+│           ├── it marks the registration as full epoch
+│           ├── it bumps the prover activity score
+│           ├── it replaces the prover shares in summed shares
+│           └── it leaves longestProvenLength and the prover rewards unchanged
 └── given prover has not submitted
     ├── when length LE longest proven
     │   ├── it store the prover shares