Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion protocol-contracts/staking/contracts/OperatorRewarder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,13 @@ contract OperatorRewarder {

function _unpaidFee(uint256 totalAssetsPlusPaidRewards) internal view returns (uint256) {
uint256 totalAssetsPlusPaidRewardsDelta = totalAssetsPlusPaidRewards - _lastClaimTotalAssetsPlusPaidRewards;
return (totalAssetsPlusPaidRewardsDelta * feeBasisPoints()) / 10_000;

// Use ceiling instead of floor to avoid extra rewarding delegators when computing allocations.
// Although floor is the correct behavior for calculating the fee to pay to the beneficiary,
// delegator allocations would be slightly higher than expected as it is based on total
// accumulated rewards subtracted by this unpaid fee amount, reversing the rounding direction.
// This could prevent the last depositor (or the beneficiary) from claiming their rewards.
return totalAssetsPlusPaidRewardsDelta.mulDiv(feeBasisPoints(), 10_000, Math.Rounding.Ceil);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not what OZ suggested for the fix. You should keep original rounding down behaviour, because this is the correct behaviour for the _claimFee function.
On the other hand, you should create a new function variant called _unpaidFeeRoundUp or _unpaidFeeCeil for eg, which contain this formula with the rounding up, and which would be used to compute the historicalReward.

}

/// @dev Compute total allocation based on number of shares and total shares. Must take paid rewards into account after.
Expand Down