available effective value calc#33
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
context dependent rounding
sisyphusSmiling
left a comment
There was a problem hiding this comment.
Nice work @nialexsan! The utils are much less verbose and rounding operations look good.
| // let availableHealth = healthAfterDeposit == UInt128.max ? UInt128.max : healthAfterDeposit - targetHealth | ||
| // let availableEffectiveValue = (effectiveDebtAfterDeposit == 0 || availableHealth == UInt128.max) | ||
| // ? effectiveCollateralAfterDeposit | ||
| // : DeFiActionsMathUtils.mul(availableHealth, effectiveDebtAfterDeposit) |
There was a problem hiding this comment.
Curious why we no longer have to concern ourselves with these conditions
There was a problem hiding this comment.
for this comment
availableHealth could be UInt128.max only when effectiveDebtAfterDeposit(0, which should set
and the second branch is the same as the one-liner:
| } | ||
|
|
||
| /// Returns the compounded interest index reflecting the passage of time | ||
| /// The result is: newIndex = oldIndex * perSecondRate ^ seconds |
There was a problem hiding this comment.
Is there a reason we can't implement this directly? Something like
let base = DeFiActionsMathUtils.mul(oldIndex, perSecondRate)
// would require pow to take UInt128 exponent value
return DeFiActionsMathUtils.pow(base, to: DeFiActionsMathUtils.toUInt128(elapsedSeconds))Maybe because the bitwise operations are more performant, but we should explicitly state that in a comment since the implementation is not as intuitive as the direct implementation.
There was a problem hiding this comment.
I'll need to look into it again
Co-authored-by: Giovanni Sanchez <108043524+sisyphusSmiling@users.noreply.github.com>
Co-authored-by: Giovanni Sanchez <108043524+sisyphusSmiling@users.noreply.github.com>
align changes with DeFi Actions Math Utilis to UInt128 and 10^24 decimal places
tweaked variance to address rounding errors in tests