Omniscia Evergon Labs Audit
RateBasedOpenRewardDistributionFacetStorage Manual Review Findings
RateBasedOpenRewardDistributionFacetStorage Manual Review Findings
RBD-01M: Incompatible Initialization Implementation
| Type | Severity | Location |
|---|---|---|
| Logical Fault |  | RateBasedOpenRewardDistributionFacetStorage.sol:L121, L122 |
Description:
The mechanism via which campaign creations are permitted allows facet initialization functions to be invoked multiple times.
While most initialization mechanisms are compatible with this approach, the RateBasedOpenRewardDistributionFacetStorage::setCampaignRewardsDistribution implementation is not as it will push to an array instead of overwriting it with a single data entry.
Impact:
Multiple invocations of the RateBasedOpenRewardDistributionFacetStorage::setCampaignRewardsDistribution function will result in corrupted campaignRates and rateChangeSnapshots arrays that could cause the overall campaign to malfunction.
Example:
94function setCampaignRewardsDistribution(95 Layout storage l,96 uint256 campaignId,97 bytes calldata campaignRewardsDistributionData98) internal returns (uint256) {99 if (GeneralStorage.layout().campaignsInfo[campaignId].state != 1) {100 revert CampaignNotOnCreationStateForSetting(campaignId);101 }102
103 uint256 campaignRate = abi.decode(campaignRewardsDistributionData, (uint256));104
105 if (campaignRate == 0) {106 revert InvalidZeroCampaignRate(campaignId);107 }108
109 CampaignInfo storage campaignInfoLocal = l.campaignInfoLocal[campaignId];110
111 campaignInfoLocal.campaignRates.push(campaignRate);112 campaignInfoLocal.rateChangeSnapshots.push(block.timestamp);113
114 return campaignRate;115}Recommendation:
We advise the code to either locally overwrite each array with a single entry, or the CampaignCreationSkeletonNID::createCampaign implementation and its non-NexeraID counterpart to ensure each function signature is invoked only once (i.e. by mandating that the function signatures are in a strictly ascending order).
Alleviation (b64b659786cf3c84bea52feb3a69f546ba3601f0):
The Evergon Labs team evaluated this exhibit and opted to prevent duplicate initializations of optional facets by ensuring that each optional facet's function selector is invoked at most once in both campaign creation implementations, alleviating this exhibit in full.
RBD-02M: Inexistent Gradual Catch-Up Mechanism
| Type | Severity | Location |
|---|---|---|
| Logical Fault |  | RateBasedOpenRewardDistributionFacetStorage.sol:L243-L261 |
Description:
The RateBasedOpenRewardDistributionFacetStorage::getReward implementation mandates that all rate changes are properly applied to a particular reward calculation which may result in stakes becoming impossible to unstake and claim rewards due to breaching the block gas limit.
Impact:
A malicious campaign owner can sabotage funds deposited to it by rapidly changing the campaign's rate multiple times, and inactive stakers in honest campaigns with multiple rate changes might become unable to unstake their funds.
Example:
209/**210 * @notice Returns the claimable rewards for a specific position.211 * @dev See `_calculateReward()` for details on reward calculation.212 * @param l A reference to the Layout struct in storage.213 * @param campaignId The unique identifier of the targeted staking campaign.214 * @param nftId The unique identifier of the NFT associated with the position.215 * @return The rewards that can be claimed by the specified position.216 */217function getReward(Layout storage l, uint256 campaignId, uint256 nftId) internal returns (uint256) {218 CampaignInfo storage campaignInfoLocal = l.campaignInfoLocal[campaignId];219 NftInfo storage nftInfoLocal = campaignInfoLocal.nftInfoLocal[nftId];220
221 uint256 lastRewardTimestamp = nftInfoLocal.lastRewardTimestamp;222
223 uint256 i = nftInfoLocal.snapshotStartingIndex;224 uint256 currentIndex = campaignInfoLocal.currentIndex;225 uint256 reward;226
227 // 1st calculation always uses as starting timestamp the NFT's last reward timestamp.228 // Subsequent calculations use the snapshots of `i` and `i+1` as the starting timestamp and ending timestamp respectively.229 // The last calculation always uses as ending timestamp the `block.timestamp`.230
231 // Rate has never been updated while position was active232 if (currentIndex == i) {233 reward = _calculateReward(campaignInfoLocal.campaignRates[i], nftId, block.timestamp - lastRewardTimestamp);234 } else {235 // The first one can be calculated externally to save gas upon the rest of execution.236 reward = _calculateReward(237 campaignInfoLocal.campaignRates[i],238 nftId,239 campaignInfoLocal.rateChangeSnapshots[i + 1] - lastRewardTimestamp240 );241 i++;242
243 while (i <= currentIndex) {244 // if they have no difference, then `block.timestamp` is used as endingTimestamp,245 // as it is the last iteration246 if (currentIndex == i) {247 reward += _calculateReward(248 campaignInfoLocal.campaignRates[i],249 nftId,250 block.timestamp - campaignInfoLocal.rateChangeSnapshots[i]251 );252 break;253 } else {254 reward += _calculateReward(255 campaignInfoLocal.campaignRates[i],256 nftId,257 campaignInfoLocal.rateChangeSnapshots[i + 1] - campaignInfoLocal.rateChangeSnapshots[i]258 );259 i++;260 }261 }262 }263
264 nftInfoLocal.lastRewardTimestamp = block.timestamp;265 return reward;266}Recommendation:
We advise a mechanism to be introduced permitting rewards to be gradually claimed, ensuring that inactive stakes can be properly extracted from the system even if multiple rate changes have occurred.
Alternative alleviations include being able to forfeit rewards, imposing an upper bound on the total number of rate changes permitted for a campaign, and / or calculating rewards for a fixed maximum number of rate changes forfeiting the rest.
Alleviation (b64b659786cf3c84bea52feb3a69f546ba3601f0):
A maximum rate change limitation was introduced to the codebase instead, ensuring that a campaign's rate changes are finite and thus always able to be caught up.