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 = 100000000000000001
lockedFundsRatio = 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).
In the aforementioned function, I suggest replacing this:
With this:
First of all, because
10 ** 18 - lockedFundsRatiomakes 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 = 100000000000000001Then:
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 <= theoreticalor alwayspractical >= theoretical).In the case at hand, it depends on the purpose of function
calculateLockedProfit.It could be that you actually want to ensure
practical >= theoreticalin this case.But just in case you want to ensure the opposite, FYI everything above.
Side note, you may as well use
scaleinstead10 ** 18, since you already have it in your code (set as1e18).