-
Notifications
You must be signed in to change notification settings - Fork 5
Description
In the aforementioned function, I suggest replacing this:
if (lockedFundsRatio < 10 ** 18) {
uint256 lockedProfit = vault.lockedProfit();
return lockedProfit - ((lockedFundsRatio * lockedProfit) / 10 ** 18);
}
With this:
if (lockedFundsRatio < 10 ** 18) {
uint256 lockedProfit = vault.lockedProfit();
return lockedProfit * (10 ** 18 - lockedFundsRatio) // 10 ** 18;
}
First of all, because 10 ** 18 - lockedFundsRatio makes it easier to see that the result is never negative.
But more importantly, because it ensures that the practical output is never larger than the theoretical output.
For example, if:
lockedProfit = 100000000000000001lockedFundsRatio = 100000000000000001
Then:
- The theoretical result = 90000000000000000.8
- Your current result = 90000000000000001, which is larger than the theoretical result
- My suggestion's result = 90000000000000000, which is smaller than the theoretical result
The maximum difference here is of course 1 wei, so it is possibly negligible and/or harmless.
In my experience, in order to ensure financial robustness, it is imperative to always have the error between the theoretical result and the practical result in the same direction (i.e., ensure that always practical <= theoretical or always practical >= theoretical).
In the case at hand, it depends on the purpose of function calculateLockedProfit.
It could be that you actually want to ensure practical >= theoretical in this case.
But just in case you want to ensure the opposite, FYI everything above.
Side note, you may as well use scale instead 10 ** 18, since you already have it in your code (set as 1e18).