From 5b1d11c096bef9a62ec36b4aff4fb3b3258c206d Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 2 Oct 2026 14:48:53 +0000 Subject: [PATCH 1/2] fix: Avoid early proof submission prover griefing --- l1-contracts/src/core/Rollup.sol | 9 +++ l1-contracts/src/core/interfaces/IRollup.sol | 1 + .../core/libraries/rollup/RewardExtLib.sol | 4 ++ .../src/core/libraries/rollup/RewardLib.sol | 35 +++++++--- l1-contracts/test/MultiProof.t.sol | 69 ++++++++++++++++++ .../libraries/rewardlib/RewardLibWrapper.sol | 19 ++++- .../libraries/rewardlib/handleRewards.t.sol | 70 ++++++++++++++++++- .../libraries/rewardlib/handleRewards.tree | 11 ++- 8 files changed, 207 insertions(+), 11 deletions(-) diff --git a/l1-contracts/src/core/Rollup.sol b/l1-contracts/src/core/Rollup.sol index 50e3a9ee7c8..2aa32c2de6e 100644 --- a/l1-contracts/src/core/Rollup.sol +++ b/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); } diff --git a/l1-contracts/src/core/interfaces/IRollup.sol b/l1-contracts/src/core/interfaces/IRollup.sol index 1c5b7114d8d..97d23caab16 100644 --- a/l1-contracts/src/core/interfaces/IRollup.sol +++ b/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); diff --git a/l1-contracts/src/core/libraries/rollup/RewardExtLib.sol b/l1-contracts/src/core/libraries/rollup/RewardExtLib.sol index 79f60743a3d..58ae975bc37 100644 --- a/l1-contracts/src/core/libraries/rollup/RewardExtLib.sol +++ b/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); } diff --git a/l1-contracts/src/core/libraries/rollup/RewardLib.sol b/l1-contracts/src/core/libraries/rollup/RewardLib.sol index 04f1c325083..a39043ddb1c 100644 --- a/l1-contracts/src/core/libraries/rollup/RewardLib.sol +++ b/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 previous_registration = $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( + previous_registration.shares == 0 || (!previous_registration.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 - previous_registration.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); } /** diff --git a/l1-contracts/test/MultiProof.t.sol b/l1-contracts/test/MultiProof.t.sol index d287712a392..ba6ee3bbb64 100644 --- a/l1-contracts/test/MultiProof.t.sol +++ b/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"); + } } diff --git a/l1-contracts/test/rollup/libraries/rewardlib/RewardLibWrapper.sol b/l1-contracts/test/rollup/libraries/rewardlib/RewardLibWrapper.sol index f49bc5c2b6c..f82865a378a 100644 --- a/l1-contracts/test/rollup/libraries/rewardlib/RewardLibWrapper.sol +++ b/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) { diff --git a/l1-contracts/test/rollup/libraries/rewardlib/handleRewards.t.sol b/l1-contracts/test/rollup/libraries/rewardlib/handleRewards.t.sol index 35f19bb2977..01fb3439104 100644 --- a/l1-contracts/test/rollup/libraries/rewardlib/handleRewards.t.sol +++ b/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() { diff --git a/l1-contracts/test/rollup/libraries/rewardlib/handleRewards.tree b/l1-contracts/test/rollup/libraries/rewardlib/handleRewards.tree index 417044132f5..15d255e8844 100644 --- a/l1-contracts/test/rollup/libraries/rewardlib/handleRewards.tree +++ b/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 From e965b8713f22bd3f0b97a4c3bef20dda69f33c7e Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 2 Oct 2026 17:09:20 +0000 Subject: [PATCH 2/2] rename --- l1-contracts/src/core/libraries/rollup/RewardLib.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/l1-contracts/src/core/libraries/rollup/RewardLib.sol b/l1-contracts/src/core/libraries/rollup/RewardLib.sol index a39043ddb1c..172cc30817f 100644 --- a/l1-contracts/src/core/libraries/rollup/RewardLib.sol +++ b/l1-contracts/src/core/libraries/rollup/RewardLib.sol @@ -218,14 +218,14 @@ library RewardLib { SubEpochRewards storage $sr = $er.subEpoch[length]; address prover = _args.args.proverId; - ProverRegistration memory previous_registration = $sr.registrations[prover]; + 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( - previous_registration.shares == 0 || (!previous_registration.fullEpoch && _fullEpochProof), + 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 @@ -241,7 +241,7 @@ library RewardLib { require(shares > 0, Errors.RewardLib__ZeroShares(prover)); $sr.registrations[prover] = ProverRegistration({shares: shares.toUint248(), fullEpoch: _fullEpochProof}); - $sr.summedShares = $sr.summedShares - previous_registration.shares + shares; + $sr.summedShares = $sr.summedShares - previousRegistration.shares + shares; } if (length > $er.longestProvenLength) {