Collateralization Memoizer #157
Conversation
| } | ||
|
|
||
| // @notice returns true if the cached values are outdated. | ||
| function isOutdated() external override view returns (bool outdated) { |
There was a problem hiding this comment.
I think this function should use early returning liberally to simplify the logic
There was a problem hiding this comment.
This method should split in two, isOutdated returning the time condition only and a second called isExceededDeviationThreshold
There was a problem hiding this comment.
I did not use early returns, since the function is intended to be called offchain only, I kept the code as simple as possible without too much branching
|
|
||
| require(_validityStatus, "CollateralizationMemoizer: CollateralizationOracle reading is invalid"); | ||
|
|
||
| Decimal.D256 memory _thresholdLow = Decimal.from(10_000 - deviationThresholdBasisPoints).div(10_000); |
There was a problem hiding this comment.
I still think a helper method would be cleaner but this looks fine so I won't die on that hill
| int256 protocolEquity, | ||
| bool validityStatus | ||
| ) { | ||
| bool fetchedValidityStatus; |
There was a problem hiding this comment.
this fetchedValidityStatus isn't used
There was a problem hiding this comment.
resolved in 491fff8, it was a bug
| Decimal.D256 memory _thresholdLow = Decimal.from(10_000 - deviationThresholdBasisPoints).div(10_000); | ||
| Decimal.D256 memory _thresholdHigh = Decimal.from(10_000 + deviationThresholdBasisPoints).div(10_000); | ||
|
|
||
| obsolete = paused(); |
There was a problem hiding this comment.
Can we refactor the obsolete logic to use a helper method or at least be more transparent?
Parsing this isn't easy.
There was a problem hiding this comment.
Will merge as is and add a refactor in another PR
This PR adds a new contract to cache calls to the CollateralizationOracle