test: add blue tests #134
test: add blue tests #134
Conversation
Jean-Grimal
requested review from
Rubilmax,
QGarchery,
MerlinEgalite and
MathisGD
|
|
||
| vm.warp(timeElapsed); | ||
|
|
||
| morpho.accrueInterest(marketParams); |
There was a problem hiding this comment.
It would make sense to test that the accrued intereste are in a certain range (approximate it very roughly). This is because forgetting the line fixedRateIrm.setBorrowRate would still make the test pass, which is a bit loose IMO.
Could do the same thing for the adaptive curve IRM, although finding a rough estimate is a bit harder
| uint256 internal constant MIN_TEST_AMOUNT = 100; | ||
| uint256 internal constant MAX_TEST_AMOUNT = 1e28; | ||
| uint256 internal constant MIN_TIME_ELAPSED = 10; | ||
| uint256 internal constant MAX_TIME_ELAPSED = 315360000; |
There was a problem hiding this comment.
where dois this value come from?
There was a problem hiding this comment.
for MAX_TEST_AMOUNT and MIN_TEST_AMOUNT I used the same ones as we use for blue tests
MIN_TIME_ELAPSED is 10sec (approx one block)
MAX_TIME_ELAPSED is 315360000 sec (ten years)
There was a problem hiding this comment.
i think we can just use 10 year instead then
There was a problem hiding this comment.
it's no longer available since solc 0.5.0
There was a problem hiding this comment.
you can write 10 * 365 days though
| "build:blue": "yarn --cwd lib/morpho-blue/ build:forge --out ../../out/", | ||
| "build:hardhat": "hardhat compile", | ||
| "test:forge": "FOUNDRY_PROFILE=test forge test", | ||
| "test:forge": "yarn build:blue && FOUNDRY_PROFILE=test forge test", |
There was a problem hiding this comment.
didn't we agree to only use the "import files technique" from now, which is more robust
| uint256 internal constant MIN_TEST_AMOUNT = 100; | ||
| uint256 internal constant MAX_TEST_AMOUNT = 1e28; | ||
| uint256 internal constant MIN_TIME_ELAPSED = 10; | ||
| uint256 internal constant MAX_TIME_ELAPSED = 315360000; |
There was a problem hiding this comment.
it's no longer available since solc 0.5.0
| uint256 internal constant MIN_TEST_AMOUNT = 100; | ||
| uint256 internal constant MAX_TEST_AMOUNT = 1e28; | ||
| uint256 internal constant MIN_TIME_ELAPSED = 10; | ||
| uint256 internal constant MAX_TIME_ELAPSED = 315360000; |
There was a problem hiding this comment.
you can write 10 * 365 days though
|
|
||
| uint256 internal constant MIN_TEST_AMOUNT = 100; | ||
| uint256 internal constant MAX_TEST_AMOUNT = 1e28; | ||
| uint256 internal constant MIN_TIME_ELAPSED = 10; |
| IAdaptiveCurveIrm internal adaptiveCurveIrm = | ||
| IAdaptiveCurveIrm(deployCode("AdaptiveCurveIrm.sol", abi.encode(address(morpho)))); | ||
| IFixedRateIrm public fixedRateIrm = IFixedRateIrm(deployCode("FixedRateIrm.sol")); |
There was a problem hiding this comment.
Can't we instead have one test that tests with all the IRMs (not sure how to do this) , and all their configs (That is why I was more thinking about doing some formal verification here, maybe with halmos?) ?
There was a problem hiding this comment.
What kind of properties you are thinking of @MathisGD ? For some of them I think fuzzing would be more appropriate because bounds are not "hardcoded". For example, if the fixed rate IRM has a given rate then we could check that, fuzzing the parameters, this is "more or less" the rate that borrowers get over time. This "more or less" is not formal and difficult to put in a specification. But it would not be difficult with fuzzing, just use approxEqRel with some well-chosen percentage
Fixes #129