-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make params constant #103
Make params constant #103
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked analytical values, which can be summed up as being scaled by 4 considering that it's linear in the initial rate at target which was multiplied by 4
@@ -8,24 +8,6 @@ interface IAdaptiveCurveIrm is IIrm { | |||
/// @notice Address of Morpho. | |||
function MORPHO() external view returns (address); | |||
|
|||
/// @notice Curve steepness (scaled by WAD). | |||
/// @dev Verified to be inside the expected range at construction. | |||
function CURVE_STEEPNESS() external view returns (int256); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think that these functions should be accessible outside? Because you can anyway expose, implement and return the direct value from ConstantLib
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessary tbh
int256 targetUtilization, | ||
int256 initialRateAtTarget | ||
) { | ||
constructor(address morpho) { | ||
require(morpho != address(0), ErrorsLib.ZERO_ADDRESS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like suggested by this comment https://github.com/morpho-org/morpho-blue-irm/pull/103/files#r1423108397 it would be good to have those sanity checks added (as an additional check) at deployment level so you always know that the deployed IRM is conforming the previous established ranges.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We added a test for this, I think that's enough
/// @notice Mininimum rate at target per second (scaled by WAD) (0.1% APR). | ||
int256 internal constant MIN_RATE_AT_TARGET = int256(0.001 ether) / 365 days; | ||
/// @notice Adjustment speed per second (scaled by WAD). | ||
int256 public constant ADJUSTMENT_SPEED = int256(50 ether) / 365 days; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that part of the "old" ADJUSTMENT_SPEED
natspec defined in IAdaptiveCurveIrm
has not been moved here
@dev The speed is per second, so the rate moves at a speed of ADJUSTMENT_SPEED * err each second (while being continuously compounded).
Fixes #102 and sets the new initial rate at target