From a0fa8f4bef176c61bee49a2052c3959f6dd0b67a Mon Sep 17 00:00:00 2001 From: MathisGD Date: Wed, 28 Feb 2024 10:47:27 +0100 Subject: [PATCH 1/6] fix: bound rate --- src/fixed-rate-irm/FixedRateIrm.sol | 8 ++++++++ test/forge/FixedRateIrmTest.sol | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/fixed-rate-irm/FixedRateIrm.sol b/src/fixed-rate-irm/FixedRateIrm.sol index 1832bd7..1a42f4f 100644 --- a/src/fixed-rate-irm/FixedRateIrm.sol +++ b/src/fixed-rate-irm/FixedRateIrm.sol @@ -15,6 +15,8 @@ string constant RATE_NOT_SET = "rate not set"; string constant RATE_SET = "rate set"; /// @dev Thrown when trying to set the rate at zero. string constant RATE_ZERO = "rate zero"; +/// @dev Thrown when trying to set a rate that is too high. +string constant RATE_TOO_HIGH = "rate too high"; /// @title FixedRateIrm /// @author Morpho Labs @@ -27,6 +29,11 @@ contract FixedRateIrm is IFixedRateIrm { /// @notice Emitted when a borrow rate is set. event SetBorrowRate(Id indexed id, uint256 newBorrowRate); + /* CONSTANTS */ + + /// @notice Max settable borrow rate (800%). + uint256 public constant MAX_BORROW_RATE = 8.0 ether / uint256(365 days); + /* STORAGE */ /// @notice Borrow rates. @@ -40,6 +47,7 @@ contract FixedRateIrm is IFixedRateIrm { function setBorrowRate(Id id, uint256 newBorrowRate) external { require(borrowRateStored[id] == 0, RATE_SET); require(newBorrowRate != 0, RATE_ZERO); + require(newBorrowRate <= MAX_BORROW_RATE, RATE_TOO_HIGH); borrowRateStored[id] = newBorrowRate; diff --git a/test/forge/FixedRateIrmTest.sol b/test/forge/FixedRateIrmTest.sol index 730360a..ede0e0e 100644 --- a/test/forge/FixedRateIrmTest.sol +++ b/test/forge/FixedRateIrmTest.sol @@ -18,6 +18,7 @@ contract FixedRateIrmTest is Test { function testSetBorrowRate(Id id, uint256 newBorrowRate) external { vm.assume(newBorrowRate != 0); + vm.assume(newBorrowRate <= fixedRateIrm.MAX_BORROW_RATE()); fixedRateIrm.setBorrowRate(id, newBorrowRate); assertEq(fixedRateIrm.borrowRateStored(id), newBorrowRate); @@ -25,6 +26,7 @@ contract FixedRateIrmTest is Test { function testSetBorrowRateEvent(Id id, uint256 newBorrowRate) external { vm.assume(newBorrowRate != 0); + vm.assume(newBorrowRate <= fixedRateIrm.MAX_BORROW_RATE()); vm.expectEmit(true, true, true, true, address(fixedRateIrm)); emit SetBorrowRate(id, newBorrowRate); @@ -33,7 +35,9 @@ contract FixedRateIrmTest is Test { function testSetBorrowRateAlreadySet(Id id, uint256 newBorrowRate1, uint256 newBorrowRate2) external { vm.assume(newBorrowRate1 != 0); + vm.assume(newBorrowRate1 <= fixedRateIrm.MAX_BORROW_RATE()); vm.assume(newBorrowRate2 != 0); + vm.assume(newBorrowRate2 <= fixedRateIrm.MAX_BORROW_RATE()); fixedRateIrm.setBorrowRate(id, newBorrowRate1); vm.expectRevert(bytes(RATE_SET)); fixedRateIrm.setBorrowRate(id, newBorrowRate2); @@ -44,8 +48,15 @@ contract FixedRateIrmTest is Test { fixedRateIrm.setBorrowRate(id, 0); } + function testSetBorrowRateRateTooHigh(Id id, uint256 newBorrowRate) external { + vm.assume(newBorrowRate > fixedRateIrm.MAX_BORROW_RATE()); + vm.expectRevert(bytes(RATE_TOO_HIGH)); + fixedRateIrm.setBorrowRate(id, newBorrowRate); + } + function testBorrowRate(MarketParams memory marketParams, Market memory market, uint256 newBorrowRate) external { vm.assume(newBorrowRate != 0); + vm.assume(newBorrowRate <= fixedRateIrm.MAX_BORROW_RATE()); fixedRateIrm.setBorrowRate(marketParams.id(), newBorrowRate); assertEq(fixedRateIrm.borrowRate(marketParams, market), newBorrowRate); } @@ -59,6 +70,7 @@ contract FixedRateIrmTest is Test { external { vm.assume(newBorrowRate != 0); + vm.assume(newBorrowRate <= fixedRateIrm.MAX_BORROW_RATE()); fixedRateIrm.setBorrowRate(marketParams.id(), newBorrowRate); assertEq(fixedRateIrm.borrowRateView(marketParams, market), newBorrowRate); } From 1af2ddb37810bbc96d6b3448de55c11c7dae0d95 Mon Sep 17 00:00:00 2001 From: MathisGD Date: Wed, 28 Feb 2024 11:09:59 +0100 Subject: [PATCH 2/6] test: minor improvement --- test/forge/FixedRateIrmTest.sol | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/forge/FixedRateIrmTest.sol b/test/forge/FixedRateIrmTest.sol index ede0e0e..106f2ee 100644 --- a/test/forge/FixedRateIrmTest.sol +++ b/test/forge/FixedRateIrmTest.sol @@ -34,10 +34,9 @@ contract FixedRateIrmTest is Test { } function testSetBorrowRateAlreadySet(Id id, uint256 newBorrowRate1, uint256 newBorrowRate2) external { - vm.assume(newBorrowRate1 != 0); - vm.assume(newBorrowRate1 <= fixedRateIrm.MAX_BORROW_RATE()); - vm.assume(newBorrowRate2 != 0); - vm.assume(newBorrowRate2 <= fixedRateIrm.MAX_BORROW_RATE()); + newBorrowRate1 = bound(newBorrowRate1, 1, fixedRateIrm.MAX_BORROW_RATE()); + newBorrowRate2 = bound(newBorrowRate2, 1, fixedRateIrm.MAX_BORROW_RATE()); + fixedRateIrm.setBorrowRate(id, newBorrowRate1); vm.expectRevert(bytes(RATE_SET)); fixedRateIrm.setBorrowRate(id, newBorrowRate2); From 995e08f57daad164e75b0c421c30be4dbdb0b66f Mon Sep 17 00:00:00 2001 From: MathisGD Date: Wed, 28 Feb 2024 15:22:38 +0100 Subject: [PATCH 3/6] test: improve fuzzing efficiency --- test/forge/FixedRateIrmTest.sol | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/test/forge/FixedRateIrmTest.sol b/test/forge/FixedRateIrmTest.sol index 106f2ee..cd35552 100644 --- a/test/forge/FixedRateIrmTest.sol +++ b/test/forge/FixedRateIrmTest.sol @@ -17,16 +17,14 @@ contract FixedRateIrmTest is Test { } function testSetBorrowRate(Id id, uint256 newBorrowRate) external { - vm.assume(newBorrowRate != 0); - vm.assume(newBorrowRate <= fixedRateIrm.MAX_BORROW_RATE()); + newBorrowRate = bound(newBorrowRate, 1, fixedRateIrm.MAX_BORROW_RATE()); fixedRateIrm.setBorrowRate(id, newBorrowRate); assertEq(fixedRateIrm.borrowRateStored(id), newBorrowRate); } function testSetBorrowRateEvent(Id id, uint256 newBorrowRate) external { - vm.assume(newBorrowRate != 0); - vm.assume(newBorrowRate <= fixedRateIrm.MAX_BORROW_RATE()); + newBorrowRate = bound(newBorrowRate, 1, fixedRateIrm.MAX_BORROW_RATE()); vm.expectEmit(true, true, true, true, address(fixedRateIrm)); emit SetBorrowRate(id, newBorrowRate); @@ -48,14 +46,13 @@ contract FixedRateIrmTest is Test { } function testSetBorrowRateRateTooHigh(Id id, uint256 newBorrowRate) external { - vm.assume(newBorrowRate > fixedRateIrm.MAX_BORROW_RATE()); + newBorrowRate = bound(newBorrowRate, fixedRateIrm.MAX_BORROW_RATE() + 1, type(uint256).max); vm.expectRevert(bytes(RATE_TOO_HIGH)); fixedRateIrm.setBorrowRate(id, newBorrowRate); } function testBorrowRate(MarketParams memory marketParams, Market memory market, uint256 newBorrowRate) external { - vm.assume(newBorrowRate != 0); - vm.assume(newBorrowRate <= fixedRateIrm.MAX_BORROW_RATE()); + newBorrowRate = bound(newBorrowRate, 1, fixedRateIrm.MAX_BORROW_RATE()); fixedRateIrm.setBorrowRate(marketParams.id(), newBorrowRate); assertEq(fixedRateIrm.borrowRate(marketParams, market), newBorrowRate); } @@ -68,8 +65,7 @@ contract FixedRateIrmTest is Test { function testBorrowRateView(MarketParams memory marketParams, Market memory market, uint256 newBorrowRate) external { - vm.assume(newBorrowRate != 0); - vm.assume(newBorrowRate <= fixedRateIrm.MAX_BORROW_RATE()); + newBorrowRate = bound(newBorrowRate, 1, fixedRateIrm.MAX_BORROW_RATE()); fixedRateIrm.setBorrowRate(marketParams.id(), newBorrowRate); assertEq(fixedRateIrm.borrowRateView(marketParams, market), newBorrowRate); } From cb9c58e87d8c91a6fcbd1c26d9ba904866602679 Mon Sep 17 00:00:00 2001 From: MathisGD <74971347+MathisGD@users.noreply.github.com> Date: Thu, 29 Feb 2024 22:03:44 +0100 Subject: [PATCH 4/6] test: better test name Co-authored-by: Merlin Egalite <44097430+MerlinEgalite@users.noreply.github.com> Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com> --- test/forge/FixedRateIrmTest.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/forge/FixedRateIrmTest.sol b/test/forge/FixedRateIrmTest.sol index cd35552..610896e 100644 --- a/test/forge/FixedRateIrmTest.sol +++ b/test/forge/FixedRateIrmTest.sol @@ -45,7 +45,7 @@ contract FixedRateIrmTest is Test { fixedRateIrm.setBorrowRate(id, 0); } - function testSetBorrowRateRateTooHigh(Id id, uint256 newBorrowRate) external { + function testSetBorrowRateTooHigh(Id id, uint256 newBorrowRate) external { newBorrowRate = bound(newBorrowRate, fixedRateIrm.MAX_BORROW_RATE() + 1, type(uint256).max); vm.expectRevert(bytes(RATE_TOO_HIGH)); fixedRateIrm.setBorrowRate(id, newBorrowRate); From 038844e0a841074bf4f3b87fe42807bc3ac5ed40 Mon Sep 17 00:00:00 2001 From: MathisGD <74971347+MathisGD@users.noreply.github.com> Date: Fri, 1 Mar 2024 12:06:21 +0100 Subject: [PATCH 5/6] docs: document rate too low issue Signed-off-by: MathisGD <74971347+MathisGD@users.noreply.github.com> --- src/fixed-rate-irm/FixedRateIrm.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fixed-rate-irm/FixedRateIrm.sol b/src/fixed-rate-irm/FixedRateIrm.sol index 1a42f4f..24b5dee 100644 --- a/src/fixed-rate-irm/FixedRateIrm.sol +++ b/src/fixed-rate-irm/FixedRateIrm.sol @@ -44,6 +44,7 @@ contract FixedRateIrm is IFixedRateIrm { /// @notice Sets the borrow rate for a market. /// @dev A rate can be set by anybody, but only once. /// @dev `borrowRate` reverts on rate not set, so the rate needs to be set before the market creation. + /// @dev As interest are rounded down in Morpho, for markets with a low total borrow, setting a rate too low could prevent interest from accruing if interactions are frequent. function setBorrowRate(Id id, uint256 newBorrowRate) external { require(borrowRateStored[id] == 0, RATE_SET); require(newBorrowRate != 0, RATE_ZERO); From 3e435cd27f4b2b55bbfed3a970cb909646e249eb Mon Sep 17 00:00:00 2001 From: MathisGD Date: Fri, 1 Mar 2024 12:07:08 +0100 Subject: [PATCH 6/6] chore: fmt --- src/fixed-rate-irm/FixedRateIrm.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/fixed-rate-irm/FixedRateIrm.sol b/src/fixed-rate-irm/FixedRateIrm.sol index 24b5dee..5b92d70 100644 --- a/src/fixed-rate-irm/FixedRateIrm.sol +++ b/src/fixed-rate-irm/FixedRateIrm.sol @@ -44,7 +44,8 @@ contract FixedRateIrm is IFixedRateIrm { /// @notice Sets the borrow rate for a market. /// @dev A rate can be set by anybody, but only once. /// @dev `borrowRate` reverts on rate not set, so the rate needs to be set before the market creation. - /// @dev As interest are rounded down in Morpho, for markets with a low total borrow, setting a rate too low could prevent interest from accruing if interactions are frequent. + /// @dev As interest are rounded down in Morpho, for markets with a low total borrow, setting a rate too low could + /// prevent interest from accruing if interactions are frequent. function setBorrowRate(Id id, uint256 newBorrowRate) external { require(borrowRateStored[id] == 0, RATE_SET); require(newBorrowRate != 0, RATE_ZERO);