6.1 Changes to yieldRate cause wrong computation of rewards
The interest computed in the calculateInterest(...) function is based on the currently set yieldRate and is not tied to any time periods. This means that after the yieldRate is increased, the protocol will pay more to users who have not yet triggered the interest calculation for the current period.
function calculateInterest(
Deposit memory userDeposit
) internal view returns (uint256) {
// ...
// @audit When yieldRate goes up,
// the interest for the current timeElapsed goes up as well
return
(userDeposit.amount * yieldRate * timeElapsed) /
(SECONDS_PER_YEAR * ONE_HUNDRED_PERCENT);
}
Users who don't call refresh(...) and don't extend their verification are at an advantage compared to users who do since they benefit from the higher yield for the past period.
Recommendation
Consider revisiting the interest calculation logic to correctly account for changes to the yieldRate.
6.2 The AllowanceModule cannot be changed after it is set
The AllowanceModule is set once during the WLDVault contract's construction. The WLDVault contract does not define the functionality to change the address of the AllowanceModule similar to the setter functions for other key parameters. A lack of a dedicated setter function might cause inconvenience when the Safe module must be updated to a new version. Since the AllowanceModule is not an upgradeable contract, a new version would have to be redeployed at a new address, forcing the redeployment of the WLDVault contract as well.
Recommendation
If the AllowanceModule contract is expected to change in the future, consider implementing a setter function to change the AllowanceModule address after the initial deployment.
6.3 Users must trigger calculateInterest(...) whenever their verified endTime changes to receive the correct rewards
Whenever users call refresh(...) their current verification endTime is attached to their deposit and marked as the end timestamp of interest accrual. Since only the endTime is tracked, users can miss the reward period if they extend the verification time but forget to call refresh(...). Consider the following scenario:
- Alice verifies herself at time
T and calls refresh(...); - Her verification
endTime is set to T+1; - Between
T and T+1, she extends her verification in the AddressBook until T+2 but does not call refresh(...); - Between
T+1 and T+2, she extends the verification to T+3 and calls refresh(...) towards the end of the period at T+2;
Timeline (T, T+1, T+2, T+3):
-T----------------------T+1---------------------T+2----------------------T+3->
| Period T -> T + 1 | Period T+1 -> T+2 | Period T+2 -> T+3 |
| (user is verified) | (user is verified) | (user is verified) |
|call refresh(...) at T | |call refresh(...) at T+2|
+------------------------------------------------------------------------+
| Interest Accrual |
+------------------------------------------------------------------------+
| Earns Interest for |No call to refresh(...)| Last saved endTime=T+1 |
| Period T->T+1 | No interest accrued | New endTime=T+3 |
| Extends Verif to T+2 | Extends Verif to T+3 |Missed yield T+1 -> T+2 |
+-----------------------+-----------------------+------------------------+
Since Alice didn't call refresh(...) after extending the verification endTime to T+2, from the perspective of the WLDVault contract, it looks like she skipped T+2 and went straight from T+1 to T+3. Even though Alice was verified from T+1 to T+2, she won't earn the interest for that period.
Recommendation
Consider documenting this behavior so that users are aware of the need to call refresh(...) to earn the rewards for all time periods when verified.
6.4 Users that refresh interest often, accrue the yield faster due to compounding
Whenever the interest is refreshed for a verified user, the interest accrued for the current period is added to the user's balance.
function refreshInterest(Deposit storage userDeposit) internal {
if (userDeposit.amount == 0) return;
uint256 interest = calculateInterest(userDeposit);
// @audit Interest is added to the user's balance
userDeposit.amount += interest;
userDeposit.lastInterestCalculation = block.timestamp;
}
Subsequently, when interest is refreshed again, the user's balance is used to compute the interest for that period.
function calculateInterest(
Deposit memory userDeposit
) internal view returns (uint256) {
// ...
// @audit
return
(userDeposit.amount * yieldRate * timeElapsed) /
(SECONDS_PER_YEAR * ONE_HUNDRED_PERCENT);
}
This setup leads to a compounding effect, where users who refresh their interest frequently will accrue interest faster than users who refresh less often. This happens because each time the interest is refreshed, the newly accumulated interest is added to the balance, which increases the principal for the following interest calculation. The more frequently interest is compounded, the faster the overall balance grows.
This behavior may lead to an uneven distribution of yields, favoring users who call the refresh function more frequently. The overall impact of this mechanism on the protocol is minimized because of the maxYieldAmount cap that limits the amount of tokens the users can earn. However, some users can reach the cap faster than others in the current setup.
Recommendation
Consider revisiting the yield strategy to make it stable for the users.
6.5 Missing address(0) checks
The AlowanceModule address can be set to address(0) in the WLDVault contract's constructor. In this state, the withdrawals from the yield source would revert since the address(0) does not have the executeAllowanceTransfer(...) function defined. The WLDVault contract would have to be re-deployed to work correctly.
Recommendation
To keep the logic consistent with the rest of the existing code, consider implementing an address(0) check for the AllowanceModule address in the constructor.
