fix(constructor): add parameter bounds & extract constants / wExp #66
Conversation
Rubilmax
requested review from
QGarchery,
MerlinEgalite,
MathisGD and
Jean-Grimal
Rubilmax
changed the title
|
This PR should not be merged before the underlying PR is merged otherwise we'll have file renamings mixed with file changes |
Rubilmax
force-pushed
the
fix/bound-constructor
branch
from
061f89f to
c9a3e20
Compare
| int256 internal constant WEXP_UPPER_BOUND = 93.859467695000404319 ether; | ||
|
|
||
| /// @dev The value of wExp(`WEXP_UPPER_BOUND`). | ||
| int256 internal constant WEXP_UPPER_VALUE = 57716089161558943949701069502944508345128.422502756744429568 ether; |
There was a problem hiding this comment.
this value was changed because the previous value didn't correspond to the bound from the unbounded wExp
There was a problem hiding this comment.
except that the code is correct (just checked)
…-periphery into fix/bound-constructor
| int256 internal constant WEXP_UPPER_BOUND = 93.859467695000404319 ether; | ||
|
|
||
| /// @dev The value of wExp(`WEXP_UPPER_BOUND`). | ||
| int256 internal constant WEXP_UPPER_VALUE = 57716089161558943949701069502944508345128.422502756744429568 ether; |
There was a problem hiding this comment.
except that the code is correct (just checked)
…-periphery into fix/bound-constructor
| // SPDX-License-Identifier: UNLICENSED | ||
| pragma solidity ^0.8.0; | ||
|
|
||
| library ConstantsLib { |
There was a problem hiding this comment.
not sure if we need a constantLib for the IRM. Everything in the same file is great
There was a problem hiding this comment.
Using a file is handy for integrators (ourselves) and having it in an dedicated file avoids having to compile the IRM in case the constants are used by integrators (including ourselves)
There was a problem hiding this comment.
This and if we do follow this suggestion then there should be only 2 constants left in this lib and those would almost never be useful to integrators (who cares about the max curve steepness, you only really care about the curve steepness of the IRM you are interacting with)
There was a problem hiding this comment.
The same arguments could be used for Blue but still we extracted constants to a file because it's shown to be handy for our test suite alone ; having constants defined in IRM (such as the min/max rates) require to deploy the contract to test things using these bounds... For example what we do in ExpLib but thanks to the constants library we don't need to deploy an IRM to test the upper value of wExp
There was a problem hiding this comment.
I'm not saying that we should never extract constants. My point is that min/max rates should really be immutables, and I think that the other constants would almost never be useful
There was a problem hiding this comment.
To me the benefit of having constants defined in a dedicated library and being able to access it outside of the IRM outweighs the loss of conciseness (now the IRM depends on 1 more file)
Whether min/max rates should be immutable is another topic that I'm open to discuss in a dedicated issue
There was a problem hiding this comment.
opened #96
I think that those topics are not so independent, but I'm fine with the current code in any case
…eriphery into fix/bound-constructor
MerlinEgalite
dismissed stale reviews from Jean-Grimal, QGarchery, and themself
The base branch was changed.
…nto fix/bound-constructor
The suggested bounds are purely arbitrary and open to debate