Skip to content

Commit c60fbe5

Browse files
kupermindclaude
andcommitted
fix(Dispenser): close vulnerabilities-list items #8, #9, #12, #25
Behavioral fixes on top of the proxy-based Dispenser, each with a regression test that fails on the pre-fix code (verified by running the new suite against the parent commit): - #12 (High): the public state-mutating calculateStakingIncentives set the one-way mapZeroWeightEpochRefunded flag while deferring the actual refund to the caller, so a standalone external call could permanently mark a zero-weight epoch refunded without any refund executed. The flag and Tokenomics.refundFromStaking are now atomic in the same call; totalReturnAmount accordingly no longer includes zero-weight epoch amounts (callers refunding it would double-count). - #9 (Low): the withheld-covered portion of claimed staking incentives is paid from OLAS already minted under a previous allocation, but the current allocation was still counted as inflation spent. Both the single-claim and batch netting paths now return the withheld-used amount to staking inflation (refundFromStaking directly on the single path; via totalAmounts[2] on the batch path). - #25 (Informative): addNominee now clears mapRemovedNomineeEpochs from a previous nominee lifecycle, so a removed-then-re-added nominee no longer permanently reverts on every claim (Overflow from the stale removal epoch). The Dispenser no longer relies on the upstream Vote Weighting enforcing "remove is final". - #8 (Informative): changeManagers now only swaps the voteWeighting address while staking incentives are paused (StakingIncentivesPaused or AllPaused; new Unpaused error), forcing outstanding claims to be settled before the nominee set is orphaned. Treasury changes stay ungated. New test/DispenserFixes.t.sol (5 tests, all green; all 5 fail on the parent commit): atomic zero-weight refund + no double refund on claim, withheld-reuse inflation credit (single + batch), remove-then-re-add claimability, pause-gated voteWeighting swap. Existing suites unaffected: forge Dispenser 3/3 + proxy 9/9, hardhat 31 passing, full yarn test green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 22f5352 commit c60fbe5

2 files changed

Lines changed: 361 additions & 1 deletion

File tree

contracts/Dispenser.sol

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,9 @@ error AlreadyInitialized();
234234
/// @dev The contract is paused.
235235
error Paused();
236236

237+
/// @dev The contract is unpaused.
238+
error Unpaused();
239+
237240
/// @dev Incentives claim has failed.
238241
/// @param account Account address.
239242
/// @param reward Reward amount.
@@ -709,14 +712,21 @@ contract Dispenser {
709712
if (transferAmounts[i] > 0) {
710713
uint256 withheldAmount = mapChainIdWithheldAmounts[chainIds[i]];
711714
if (withheldAmount > 0) {
715+
uint256 withheldUsed;
712716
if (withheldAmount >= transferAmounts[i]) {
717+
withheldUsed = transferAmounts[i];
713718
withheldAmount -= transferAmounts[i];
714719
transferAmounts[i] = 0;
715720
} else {
721+
withheldUsed = withheldAmount;
716722
transferAmounts[i] -= withheldAmount;
717723
withheldAmount = 0;
718724
}
719725
mapChainIdWithheldAmounts[chainIds[i]] = withheldAmount;
726+
727+
// The withheld-covered portion is paid from OLAS already minted under a previous allocation;
728+
// add it to the return amount so the caller refunds it back to staking inflation
729+
totalAmounts[2] += withheldUsed;
720730
}
721731
}
722732

@@ -761,6 +771,14 @@ contract Dispenser {
761771

762772
// Change Vote Weighting contract address
763773
if (_voteWeighting != address(0)) {
774+
// Swapping the Vote Weighting contract orphans the per-nominee claim cursors recorded under the
775+
// previous nominee set: any staking incentives not yet claimed become unreachable. Require staking
776+
// incentives to be paused first, so all outstanding claims are settled before the swap
777+
Pause currentPause = paused;
778+
if (currentPause != Pause.StakingIncentivesPaused && currentPause != Pause.AllPaused) {
779+
revert Unpaused();
780+
}
781+
764782
voteWeighting = _voteWeighting;
765783
emit VoteWeightingUpdated(_voteWeighting);
766784
}
@@ -833,6 +851,12 @@ contract Dispenser {
833851

834852
mapLastClaimedStakingEpochs[nomineeHash] = ITokenomics(tokenomics).epochCounter();
835853

854+
// Clear a possible removal epoch left from a previous nominee lifecycle: without this, a re-added
855+
// nominee would permanently revert in _checkpointNomineeAndGetClaimedEpochCounters (the claim cursor
856+
// just set above always satisfies firstClaimedEpoch >= epochRemoved). The Dispenser no longer relies
857+
// on the upstream Vote Weighting enforcing "remove is final"
858+
delete mapRemovedNomineeEpochs[nomineeHash];
859+
836860
emit AddNomineeHash(nomineeHash);
837861
}
838862

@@ -937,6 +961,9 @@ contract Dispenser {
937961

938962
/// @dev Calculates staking incentives for a specific staking target.
939963
/// @notice Call this function via staticcall in order not to write in the nominee checkpoint map.
964+
/// A state-mutating call refunds zero-total-weight epochs back to staking inflation atomically with
965+
/// setting their one-way refunded flag, so a standalone external call cannot strand the refund;
966+
/// the returned totalReturnAmount accordingly does not include the zero-weight epoch amounts.
940967
/// @param numClaimedEpochs Specified number of claimed epochs.
941968
/// @param chainId Chain Id.
942969
/// @param stakingTarget Staking target corresponding to the chain Id.
@@ -1016,8 +1043,11 @@ contract Dispenser {
10161043

10171044
// Check if the totalWeightSum is zero, then all staking incentives must be returned back to tokenomics
10181045
if (totalWeightSum == 0) {
1046+
// The one-way refunded flag and the actual refund are performed atomically in the same call:
1047+
// this function is public, and setting the flag while deferring the refund to the caller would
1048+
// let a standalone external call permanently mark the epoch refunded without any refund executed
10191049
mapZeroWeightEpochRefunded[j] = true;
1020-
totalReturnAmount += stakingPoint.stakingIncentive;
1050+
ITokenomics(tokenomics).refundFromStaking(stakingPoint.stakingIncentive);
10211051
continue;
10221052
}
10231053

@@ -1140,16 +1170,24 @@ contract Dispenser {
11401170
// as normalized amounts are returned from another side
11411171
uint256 withheldAmount = mapChainIdWithheldAmounts[chainId];
11421172
if (withheldAmount > 0) {
1173+
uint256 withheldUsed;
11431174
// If withheld amount is enough to cover all the staking incentives, the transfer of OLAS is not needed
11441175
if (withheldAmount >= transferAmount) {
1176+
withheldUsed = transferAmount;
11451177
withheldAmount -= transferAmount;
11461178
transferAmount = 0;
11471179
} else {
11481180
// Otherwise, reduce the transfer of tokens for the OLAS withheld amount
1181+
withheldUsed = withheldAmount;
11491182
transferAmount -= withheldAmount;
11501183
withheldAmount = 0;
11511184
}
11521185
mapChainIdWithheldAmounts[chainId] = withheldAmount;
1186+
1187+
// The withheld-covered portion of the incentives is paid from OLAS that was already minted under
1188+
// a previous allocation; return the current allocation for that portion back to staking inflation
1189+
// so the reused amount is not double-counted as inflation spent
1190+
ITokenomics(tokenomics).refundFromStaking(withheldUsed);
11531191
}
11541192

11551193
// Check if minting is needed as the actual OLAS transfer is required

0 commit comments

Comments
 (0)