Resupply sreUSD

Weights can potentially fall outside the expected scale, affecting the PriceWatcher consumers.

The implementation of getCurrentWeight() fetches the reUSD price from the Oracle and adjusts the scale to return a value within 1e6 precision.

153:     function getCurrentWeight() public view returns (uint64) {
154:         uint256 price = IReusdOracle(oracle).price();
155:         uint256 weight = price > 1e18 ? 0 : 1e18 - price;
156:         //our oracle has a floor that matches redemption fee
157:         //e.g. it returns a minimum price of 0.9900 when there is a 1% redemption fee
158:         //at this point a price of 0.99000 has a weight of 0.010000 or 1e16
159:         //reduce precision to 1e6
160:         return uint64(weight / 1e10);
161:     }

The expected behavior here is that the Oracle price is clamped to $1 minus the redemption fee (1%), making the resulting weight fall within the [0, 0.01] range, which is then scaled down to the 1e6 region

However, two dependencies could potentially break this invariant

• The ReusdOracle price() function multiplies the reUSD price (floored to the redemption fee) by the crvUSD price, potentially allowing the resulting value to be actually less than $0.99
• The redemption fee can be updated, breaking the floored price assumption.

Medium. Users of the PriceWatcher contract consume the weight as a rate while assuming the returned scale is 1e6, potentially causing downstream calculations to exceed 100%.

The weight calculation should use priceAsCrvusd(), which returns the floored price without the crvUSD price scaling. Additionally, ensure the redemption fee is not changed, as this might also break the calculations.

Fixed in 4cc34fb.

The public facing variant of previewSyncRewards() doesn't account for fee distribution and will return incorrect results.

The implementation of _syncRewards() has been updated so that _distributeFees() is called before previewSyncRewards() but only if the current cycle has elapsed (block.timestamp <= rewardsCycleData.cycleEnd).

157:     function _syncRewards() internal virtual {
158:         if (block.timestamp <= rewardsCycleData.cycleEnd) return;
159:         _distributeFees();
160:         RewardsCycleData memory _rewardsCycleData = previewSyncRewards();

This is needed so that fees are pulled into the contract and accounted for as rewards for the upcoming cycle.

However, if previewSyncRewards() is called externally, the fee distribution isn't executed and rewards are actually not accounted for in the preview simulation.

Low. previewSyncRewards() returns incorrect results.

The implementation should be refactored so that previewSyncRewards() simulates the effects of fee distribution.

Fixed in 48d2b16.

The new staked reUSD split is not considered in the update logic of FeeDepositController.

The implementation of setSplits() takes all four split arguments but requires only three of them to equal the total BPS.

181:     function setSplits(uint256 _insuranceSplit, uint256 _treasurySplit, uint256 _platformSplit, uint256 _stakedStableSplit) external onlyOwner {
182:         require(_insuranceSplit + _treasurySplit + _platformSplit == BPS, "invalid splits");
183:         splits.insurance = uint40(_insuranceSplit);
184:         splits.treasury = uint40(_treasurySplit);
185:         splits.platform = uint40(_platformSplit);
186:         splits.stakedStable = uint40(_stakedStableSplit);
187:         emit SplitsSet(uint40(_insuranceSplit), uint40(_treasurySplit), uint40(_platformSplit), uint40(_stakedStableSplit));
188:     }

Additionally, during initialization, the stakedStable variable is missing from the check on line 62. However, in this case, the split can still be initialized properly, and would overflow on line 66 if the splits exceed 100%.

62:         require(_insuranceSplit + _treasurySplit <= BPS, "invalid splits");

Low.

Change the condition in setSplits() to account for the new staked split.

-   require(_insuranceSplit + _treasurySplit + _platformSplit == BPS, "invalid splits");
+   require(_insuranceSplit + _treasurySplit + _platformSplit + _stakedStableSplit == BPS, "invalid splits");

The condition in the contract's constructor can be adjusted as well.

-   require(_insuranceSplit + _treasurySplit <= BPS, "invalid splits");
+   require(_insuranceSplit + _treasurySplit + _stakedStableSplit <= BPS, "invalid splits");

Alternatively, the platform split can be removed and interpreted as the difference between 100% and the sum of the other three split, aligning the behavior with the actual split implementation.

Fixed the summation check in 2f9510b.

We think it's preferable to leave the struct as-is for the following reasons:

1. Explicitness.
2. Convenient for users or indexers to read from.

The function setRateCalculator() updates the contract used to calculate the interest rate. When called the param _updateInterest can be set to true to flush interests prior to changing the contract.

The new contract InterestRateCalculatorV2 and PriceWatcher use the pair's lastPairUpdate variable to determine the new interest rate. This variable is only updated when interest are flushed/added on the pair.

When the PriceWatcher is deployed, it will save two new priceData, one at timestamp 0 and one at the deployment timestamp's floor.

If the lastPairUpdate is smaller than the deployment's timestamp floor, the findPairPriceWeight() function called by the InterestRateCalculatorV2 will loop forever looking for a valid priceData all the way to timestamp 0 which will likely result in the transaction reverting out of gas.

Low. The pair might not be able to flush interests anymore until a new interest rate contract is set.

Ensure that the param _updateInterest is set to true inside the deployment script when calling setRateCalculator() (already the case inside LaunchSreUsd) and document this in the natspec of the InterestRateCalculatorV2 contract.

Included in the draft governance proposal: LaunchSreUsd.s.sol#L126.

The initialization of LinearRewardsErc4626 triggers a dummy distribution to bootstrap to state, which could create a conflict if there are actual rewards being distributed.

The constructor of LinearRewardsErc4626 calls _syncRewards()

67:         // initialize rewardsCycleEnd value
68:         // NOTE: normally distribution of rewards should be done prior to _syncRewards but in this case we know there are no users or rewards yet.
69:         _syncRewards();
70: 
71:         // initialize lastRewardsDistribution value
72:         _distributeRewards();

As the comment indicates, is it expected that no users or rewards are present at this point. However, this may not be true if the call to _distributeFees() ends up doing an actual distribution of tokens.

Informational.

Ensure no reward distribution happens as part of the initialization of the staked reUSD vault.

Added an extra sanity check in constructor: c41e8ae.

Inside the function previewDistributeRewards() return early if lastRewardsDistribution == block.timestamp.

This will save gas on deposits and withdraws as the contract currently _distributeRewards() first which will update lastRewardsDistribution but then call the function previewDistributeRewards() again inside totalAssets() to determine the amount of shares to mint/burn.

Gas.

Inside the function previewDistributeRewards() return early if lastRewardsDistribution == block.timestamp.

Updated in b2139c2.