From 35fbbcb3be78bd07c0d4394bb869a2d9616fc5f3 Mon Sep 17 00:00:00 2001 From: Michael De Luca Date: Sun, 12 Dec 2021 05:11:08 -0500 Subject: [PATCH 1/8] chore: Remove TODO on pullFunds (c4 #10) --- contracts/DebtLocker.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/DebtLocker.sol b/contracts/DebtLocker.sol index 068bbb0..ac2c236 100644 --- a/contracts/DebtLocker.sol +++ b/contracts/DebtLocker.sol @@ -75,7 +75,6 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { return _repossessed ? _handleClaimOfRepossessed() : _handleClaim(); } - // TODO: Discuss pros/cons of pause on this function function pullFundsFromLiquidator(address token_, address destination_, uint256 amount_) external override { require(msg.sender == _getPoolDelegate(), "DL:SA:NOT_PD"); From e5f0d2e53ba36a4f93b39c707712dc5aaeff96af Mon Sep 17 00:00:00 2001 From: Michael De Luca Date: Sun, 12 Dec 2021 05:13:19 -0500 Subject: [PATCH 2/8] chore: strict 0.8.7 (c4 #23) --- contracts/DebtLocker.sol | 2 +- contracts/DebtLockerFactory.sol | 2 +- contracts/DebtLockerInitializer.sol | 2 +- contracts/DebtLockerStorage.sol | 2 +- contracts/interfaces/IDebtLocker.sol | 2 +- contracts/interfaces/IDebtLockerFactory.sol | 2 +- contracts/interfaces/Interfaces.sol | 2 +- contracts/test/DebtLocker.t.sol | 2 +- contracts/test/mocks/DebtLockerHarness.sol | 2 +- contracts/test/mocks/ManipulatableDebtLocker.sol | 2 +- contracts/test/mocks/Mocks.sol | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/contracts/DebtLocker.sol b/contracts/DebtLocker.sol index ac2c236..4c5ce28 100644 --- a/contracts/DebtLocker.sol +++ b/contracts/DebtLocker.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; import { IMapleProxyFactory } from "../modules/maple-proxy-factory/contracts/interfaces/IMapleProxyFactory.sol"; diff --git a/contracts/DebtLockerFactory.sol b/contracts/DebtLockerFactory.sol index de66caf..cf9939c 100644 --- a/contracts/DebtLockerFactory.sol +++ b/contracts/DebtLockerFactory.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; import { IMapleProxyFactory, MapleProxyFactory } from "../modules/maple-proxy-factory/contracts/MapleProxyFactory.sol"; diff --git a/contracts/DebtLockerInitializer.sol b/contracts/DebtLockerInitializer.sol index 0106cb9..0a53b72 100644 --- a/contracts/DebtLockerInitializer.sol +++ b/contracts/DebtLockerInitializer.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; import { IMapleGlobalsLike, IMapleLoanLike, IPoolFactoryLike, IPoolLike } from "./interfaces/Interfaces.sol"; diff --git a/contracts/DebtLockerStorage.sol b/contracts/DebtLockerStorage.sol index 33d5614..16f8595 100644 --- a/contracts/DebtLockerStorage.sol +++ b/contracts/DebtLockerStorage.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; /// @title DebtLockerStorage maps the storage layout of a DebtLocker. contract DebtLockerStorage { diff --git a/contracts/interfaces/IDebtLocker.sol b/contracts/interfaces/IDebtLocker.sol index 6431bc1..b391503 100644 --- a/contracts/interfaces/IDebtLocker.sol +++ b/contracts/interfaces/IDebtLocker.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; import { IMapleProxied } from "../../modules/maple-proxy-factory/contracts/interfaces/IMapleProxied.sol"; diff --git a/contracts/interfaces/IDebtLockerFactory.sol b/contracts/interfaces/IDebtLockerFactory.sol index 67b2cbc..b759419 100644 --- a/contracts/interfaces/IDebtLockerFactory.sol +++ b/contracts/interfaces/IDebtLockerFactory.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; import { IMapleProxyFactory } from "../../modules/maple-proxy-factory/contracts/interfaces/IMapleProxyFactory.sol"; diff --git a/contracts/interfaces/Interfaces.sol b/contracts/interfaces/Interfaces.sol index b096246..f3956af 100644 --- a/contracts/interfaces/Interfaces.sol +++ b/contracts/interfaces/Interfaces.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; interface IERC20Like { diff --git a/contracts/test/DebtLocker.t.sol b/contracts/test/DebtLocker.t.sol index b1b2b92..d235741 100644 --- a/contracts/test/DebtLocker.t.sol +++ b/contracts/test/DebtLocker.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; import { TestUtils } from "../../modules/contract-test-utils/contracts/test.sol"; import { MockERC20 } from "../../modules/erc20/src/test/mocks/MockERC20.sol"; diff --git a/contracts/test/mocks/DebtLockerHarness.sol b/contracts/test/mocks/DebtLockerHarness.sol index 8850316..6d71a20 100644 --- a/contracts/test/mocks/DebtLockerHarness.sol +++ b/contracts/test/mocks/DebtLockerHarness.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; import { DebtLocker } from "../../DebtLocker.sol"; diff --git a/contracts/test/mocks/ManipulatableDebtLocker.sol b/contracts/test/mocks/ManipulatableDebtLocker.sol index 8066077..eef9b5a 100644 --- a/contracts/test/mocks/ManipulatableDebtLocker.sol +++ b/contracts/test/mocks/ManipulatableDebtLocker.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; import { IMapleLoanLike } from "../../interfaces/Interfaces.sol"; diff --git a/contracts/test/mocks/Mocks.sol b/contracts/test/mocks/Mocks.sol index 1555842..e2ee2d7 100644 --- a/contracts/test/mocks/Mocks.sol +++ b/contracts/test/mocks/Mocks.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-or-later -pragma solidity ^0.8.7; +pragma solidity 0.8.7; import { IERC20 } from "../../../modules/erc20/src/interfaces/IERC20.sol"; import { ILiquidatorLike } from "../../../modules/liquidations/contracts/interfaces/Interfaces.sol"; From b97df8afb4c4945764fa01a6b208eac94db7797e Mon Sep 17 00:00:00 2001 From: Michael De Luca Date: Sun, 12 Dec 2021 05:15:59 -0500 Subject: [PATCH 3/8] fix: Add named returns where missing (c4 #25) --- contracts/DebtLocker.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/DebtLocker.sol b/contracts/DebtLocker.sol index 4c5ce28..21cb141 100644 --- a/contracts/DebtLocker.sol +++ b/contracts/DebtLocker.sol @@ -251,7 +251,7 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { return oracleAmount > minRatioAmount ? oracleAmount : minRatioAmount; } - function implementation() external view override returns (address) { + function implementation() external view override returns (address implementation_) { return _implementation(); } @@ -279,7 +279,7 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { return _pool; } - function poolDelegate() external override view returns(address) { + function poolDelegate() external override view returns (address poolDelegate_) { return _getPoolDelegate(); } @@ -299,15 +299,15 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { /*** Internal View Functions ***/ /*******************************/ - function _getGlobals() internal view returns (address) { + function _getGlobals() internal view returns (address globals_) { return IPoolFactoryLike(IPoolLike(_pool).superFactory()).globals(); } - function _getPoolDelegate() internal view returns(address) { + function _getPoolDelegate() internal view returns(address poolDelegate_) { return IPoolLike(_pool).poolDelegate(); } - function _isLiquidationActive() internal view returns (bool) { + function _isLiquidationActive() internal view returns (bool isActive_) { return (_liquidator != address(0)) && (IERC20Like(IMapleLoanLike(_loan).collateralAsset()).balanceOf(_liquidator) > 0); } From 4a4c95451cb6ebd52ca68d3786042a27b2f52801 Mon Sep 17 00:00:00 2001 From: Michael De Luca Date: Sun, 12 Dec 2021 05:20:00 -0500 Subject: [PATCH 4/8] fix: `handleClaimOfRepossessed` gas optimization (c4 #66) --- contracts/DebtLocker.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/DebtLocker.sol b/contracts/DebtLocker.sol index 21cb141..5c2b0ca 100644 --- a/contracts/DebtLocker.sol +++ b/contracts/DebtLocker.sol @@ -199,9 +199,11 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { // Funds recovered from liquidation and any unclaimed previous payment amounts uint256 recoveredFunds = IERC20Like(fundsAsset).balanceOf(address(this)) - fundsCaptured; + uint256 totalClaimed = recoveredFunds + fundsCaptured; + // If `recoveredFunds` is greater than `principalToCover`, the remaining amount is treated as interest in the context of the pool. // If `recoveredFunds` is less than `principalToCover`, the difference is registered as a shortfall. - details_[0] = recoveredFunds + fundsCaptured; + details_[0] = totalClaimed; details_[1] = recoveredFunds > principalToCover ? recoveredFunds - principalToCover : 0; details_[2] = fundsCaptured; details_[5] = recoveredFunds > principalToCover ? principalToCover : recoveredFunds; @@ -210,7 +212,7 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { _fundsToCapture = uint256(0); _repossessed = false; - require(ERC20Helper.transfer(fundsAsset, _pool, recoveredFunds + fundsCaptured), "DL:HCOR:TRANSFER"); + require(ERC20Helper.transfer(fundsAsset, _pool, totalClaimed), "DL:HCOR:TRANSFER"); } /**********************/ From 0f8bfc71995a9ca9da16e0aceaf61af673341be6 Mon Sep 17 00:00:00 2001 From: Michael De Luca Date: Sun, 12 Dec 2021 05:22:23 -0500 Subject: [PATCH 5/8] fix: `getExpectedAmount` gas savings (c4 #62) --- contracts/DebtLocker.sol | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/contracts/DebtLocker.sol b/contracts/DebtLocker.sol index 5c2b0ca..0795cfc 100644 --- a/contracts/DebtLocker.sol +++ b/contracts/DebtLocker.sol @@ -236,19 +236,24 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { } function getExpectedAmount(uint256 swapAmount_) external view override whenProtocolNotPaused returns (uint256 returnAmount_) { - address collateralAsset = IMapleLoanLike(_loan).collateralAsset(); - address fundsAsset = IMapleLoanLike(_loan).fundsAsset(); + address loanAddress = _loan; + address collateralAsset = IMapleLoanLike(loanAddress).collateralAsset(); + address fundsAsset = IMapleLoanLike(loanAddress).fundsAsset(); + + address globals = _getGlobals(); + + uint8 collateralAssetDecimals = IERC20Like(collateralAsset).decimals(); uint256 oracleAmount = swapAmount_ - * IMapleGlobalsLike(_getGlobals()).getLatestPrice(collateralAsset) // Convert from `fromAsset` value. - * 10 ** uint256(IERC20Like(fundsAsset).decimals()) // Convert to `toAsset` decimal precision. - * (10_000 - _allowedSlippage) // Multiply by allowed slippage basis points - / IMapleGlobalsLike(_getGlobals()).getLatestPrice(fundsAsset) // Convert to `toAsset` value. - / 10 ** uint256(IERC20Like(collateralAsset).decimals()) // Convert from `fromAsset` decimal precision. - / 10_000; // Divide basis points for slippage - - uint256 minRatioAmount = swapAmount_ * _minRatio / 10 ** IERC20Like(collateralAsset).decimals(); + * IMapleGlobalsLike(globals).getLatestPrice(collateralAsset) // Convert from `fromAsset` value. + * uint256(10) ** uint256(IERC20Like(fundsAsset).decimals()) // Convert to `toAsset` decimal precision. + * (uint256(10_000) - _allowedSlippage) // Multiply by allowed slippage basis points + / IMapleGlobalsLike(globals).getLatestPrice(fundsAsset) // Convert to `toAsset` value. + / uint256(10) ** uint256(collateralAssetDecimals) // Convert from `fromAsset` decimal precision. + / uint256(10_000); // Divide basis points for slippage. + + uint256 minRatioAmount = swapAmount_ * _minRatio / 10 ** collateralAssetDecimals; return oracleAmount > minRatioAmount ? oracleAmount : minRatioAmount; } From 991df09ec7878550f36c3fdd2019824aff312b9f Mon Sep 17 00:00:00 2001 From: Michael De Luca Date: Sun, 12 Dec 2021 05:31:06 -0500 Subject: [PATCH 6/8] fix: Use `!= 0` instead of `> 0` (c4 #24) --- contracts/DebtLocker.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/DebtLocker.sol b/contracts/DebtLocker.sol index 0795cfc..ce1c4c1 100644 --- a/contracts/DebtLocker.sol +++ b/contracts/DebtLocker.sol @@ -315,7 +315,7 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { } function _isLiquidationActive() internal view returns (bool isActive_) { - return (_liquidator != address(0)) && (IERC20Like(IMapleLoanLike(_loan).collateralAsset()).balanceOf(_liquidator) > 0); + return (_liquidator != address(0)) && (IERC20Like(IMapleLoanLike(_loan).collateralAsset()).balanceOf(_liquidator) != uint256(0)); } } From 19d33d5fe806c610bb3176513339feb31c1dedb4 Mon Sep 17 00:00:00 2001 From: Michael De Luca Date: Sun, 12 Dec 2021 05:38:00 -0500 Subject: [PATCH 7/8] fix: Input validation for `setAllowedSlippage` (c4 #45) --- contracts/DebtLocker.sol | 3 ++- contracts/test/DebtLocker.t.sol | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/contracts/DebtLocker.sol b/contracts/DebtLocker.sol index ce1c4c1..f5195e8 100644 --- a/contracts/DebtLocker.sol +++ b/contracts/DebtLocker.sol @@ -82,7 +82,8 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { } function setAllowedSlippage(uint256 allowedSlippage_) external override whenProtocolNotPaused { - require(msg.sender == _getPoolDelegate(), "DL:SAS:NOT_PD"); + require(msg.sender == _getPoolDelegate(), "DL:SAS:NOT_PD"); + require(allowedSlippage_ <= uint256(10000), "DL:SAS:INVALID_SLIPPAGE"); emit AllowedSlippageSet(_allowedSlippage = allowedSlippage_); } diff --git a/contracts/test/DebtLocker.t.sol b/contracts/test/DebtLocker.t.sol index d235741..5208394 100644 --- a/contracts/test/DebtLocker.t.sol +++ b/contracts/test/DebtLocker.t.sol @@ -726,6 +726,19 @@ contract DebtLockerTests is TestUtils { pool.triggerDefault(address(debtLocker)); } + /******************************/ + /*** Input Validation Tests ***/ + /******************************/ + + function test_setAllowedSlippage_invalidSlippage() external { + MapleLoan loan = _createLoan(1_000_000, 30_000); + + DebtLocker debtLocker = DebtLocker(pool.createDebtLocker(address(dlFactory), address(loan))); + + assertTrue(!poolDelegate.try_debtLocker_setAllowedSlippage(address(debtLocker), 10001)); + assertTrue( poolDelegate.try_debtLocker_setAllowedSlippage(address(debtLocker), 10000)); + } + /***********************/ /*** Refinance Tests ***/ /***********************/ From 23354982af7a26fde4fbc5061e82e591e13a47f8 Mon Sep 17 00:00:00 2001 From: Michael De Luca Date: Sun, 12 Dec 2021 05:55:03 -0500 Subject: [PATCH 8/8] refactor: POst-C4 Audit Cleanup - natspec, spacing, and alignment - factory `mapleGlobals_` check now handled by mpf - `success_` -> `success` for non-param variable - reordered `DebtLockerStorage` - take latest dependencies - `acceptNewTerms` gas savings - `claim`, `_handleClaimOfRepossessed`, and `_handleClaim` gas savings - `_isLiquidationActive` gas savings - `triggerDefault` gas savings --- contracts/DebtLocker.sol | 68 ++++++++------- contracts/DebtLockerFactory.sol | 10 +-- contracts/DebtLockerStorage.sol | 6 +- contracts/interfaces/IDebtLocker.sol | 2 +- contracts/interfaces/IDebtLockerFactory.sol | 2 +- contracts/test/DebtLocker.t.sol | 84 +++++++++---------- contracts/test/accounts/PoolDelegate.sol | 6 +- .../test/mocks/ManipulatableDebtLocker.sol | 2 +- contracts/test/mocks/Mocks.sol | 3 +- modules/erc20 | 2 +- modules/erc20-helper | 2 +- modules/liquidations | 2 +- modules/loan | 2 +- modules/maple-proxy-factory | 2 +- 14 files changed, 98 insertions(+), 95 deletions(-) diff --git a/contracts/DebtLocker.sol b/contracts/DebtLocker.sol index f5195e8..0d6dc69 100644 --- a/contracts/DebtLocker.sol +++ b/contracts/DebtLocker.sol @@ -12,7 +12,7 @@ import { IERC20Like, IMapleGlobalsLike, IMapleLoanLike, IPoolLike, IPoolFactoryL import { DebtLockerStorage } from "./DebtLockerStorage.sol"; -/// @title DebtLocker interacts with Loans on behalf of PoolV1 +/// @title DebtLocker interacts with Loans on behalf of PoolV1. contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { /*****************/ @@ -53,32 +53,35 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { function acceptNewTerms(address refinancer_, bytes[] calldata calls_, uint256 amount_) external override whenProtocolNotPaused { require(msg.sender == _getPoolDelegate(), "DL:ANT:NOT_PD"); - IMapleLoanLike loan_ = IMapleLoanLike(_loan); + address loanAddress = _loan; require( - (loan_.claimableFunds() + _fundsToCapture == 0) && - (loan_.principal() == _principalRemainingAtLastClaim), + (IMapleLoanLike(loanAddress).claimableFunds() + _fundsToCapture == uint256(0)) && + (IMapleLoanLike(loanAddress).principal() == _principalRemainingAtLastClaim), "DL:ANT:NEED_TO_CLAIM" ); - require(amount_ == uint256(0) || ERC20Helper.transfer(loan_.fundsAsset(), address(_loan), amount_), "DL:ANT:TRANSFER_FAILED"); + require( + amount_ == uint256(0) || ERC20Helper.transfer(IMapleLoanLike(loanAddress).fundsAsset(), loanAddress, amount_), + "DL:ANT:TRANSFER_FAILED" + ); - loan_.acceptNewTerms(refinancer_, calls_, uint256(0)); + IMapleLoanLike(loanAddress).acceptNewTerms(refinancer_, calls_, uint256(0)); // NOTE: This must be set after accepting the new terms, which affects the loan principal. - _principalRemainingAtLastClaim = loan_.principal(); + _principalRemainingAtLastClaim = IMapleLoanLike(loanAddress).principal(); } function claim() external override whenProtocolNotPaused returns (uint256[7] memory details_) { require(msg.sender == _pool, "DL:C:NOT_POOL"); - return _repossessed ? _handleClaimOfRepossessed() : _handleClaim(); + return _repossessed ? _handleClaimOfRepossessed(msg.sender, _loan) : _handleClaim(msg.sender, _loan); } function pullFundsFromLiquidator(address token_, address destination_, uint256 amount_) external override { require(msg.sender == _getPoolDelegate(), "DL:SA:NOT_PD"); - - Liquidator(_liquidator).pullFunds( token_, destination_, amount_); + + Liquidator(_liquidator).pullFunds(token_, destination_, amount_); } function setAllowedSlippage(uint256 allowedSlippage_) external override whenProtocolNotPaused { @@ -120,9 +123,11 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { function triggerDefault() external override whenProtocolNotPaused { require(msg.sender == _pool, "DL:TD:NOT_POOL"); + address loanAddress = _loan; + require( - (IMapleLoanLike(_loan).claimableFunds() == 0) && - (IMapleLoanLike(_loan).principal() == _principalRemainingAtLastClaim), + (IMapleLoanLike(loanAddress).claimableFunds() == uint256(0)) && + (IMapleLoanLike(loanAddress).principal() == _principalRemainingAtLastClaim), "DL:TD:NEED_TO_CLAIM" ); @@ -132,18 +137,18 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { // accounting is updated properly when principal is updated and there are no claimable funds. // Repossess collateral and funds from Loan. - ( uint256 collateralAssetAmount, ) = IMapleLoanLike(_loan).repossess(address(this)); + ( uint256 collateralAssetAmount, ) = IMapleLoanLike(loanAddress).repossess(address(this)); - address collateralAsset = IMapleLoanLike(_loan).collateralAsset(); - address fundsAsset = IMapleLoanLike(_loan).fundsAsset(); + address collateralAsset = IMapleLoanLike(loanAddress).collateralAsset(); + address fundsAsset = IMapleLoanLike(loanAddress).fundsAsset(); - if (collateralAsset == fundsAsset || collateralAssetAmount == 0) return; + if (collateralAsset == fundsAsset || collateralAssetAmount == uint256(0)) return; // Deploy Liquidator contract and transfer collateral. require( ERC20Helper.transfer( collateralAsset, - _liquidator = address(new Liquidator(address(this), collateralAsset, fundsAsset, address(this), address(this),_getGlobals())), + _liquidator = address(new Liquidator(address(this), collateralAsset, fundsAsset, address(this), address(this), _getGlobals())), collateralAssetAmount ), "DL:TD:TRANSFER" @@ -154,16 +159,16 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { /*** Internal Functions ***/ /**************************/ - function _handleClaim() internal returns (uint256[7] memory details_) { + function _handleClaim(address pool_, address loan_) internal returns (uint256[7] memory details_) { // Get loan state variables needed - uint256 claimableFunds = IMapleLoanLike(_loan).claimableFunds(); + uint256 claimableFunds = IMapleLoanLike(loan_).claimableFunds(); require(claimableFunds > uint256(0), "DL:HC:NOTHING_TO_CLAIM"); // Send funds to pool - IMapleLoanLike(_loan).claimFunds(claimableFunds, _pool); + IMapleLoanLike(loan_).claimFunds(claimableFunds, pool_); - uint256 currentPrincipalRemaining = IMapleLoanLike(_loan).principal(); + uint256 currentPrincipalRemaining = IMapleLoanLike(loan_).principal(); // Determine how much of `claimableFunds` is principal uint256 principalPortion = _principalRemainingAtLastClaim - currentPrincipalRemaining; @@ -186,14 +191,14 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { _fundsToCapture = uint256(0); - require(ERC20Helper.transfer(IMapleLoanLike(_loan).fundsAsset(), _pool, amountOfFundsToCapture), "DL:HC:CAPTURE_FAILED"); + require(ERC20Helper.transfer(IMapleLoanLike(loan_).fundsAsset(), pool_, amountOfFundsToCapture), "DL:HC:CAPTURE_FAILED"); } } - function _handleClaimOfRepossessed() internal returns (uint256[7] memory details_) { + function _handleClaimOfRepossessed(address pool_, address loan_) internal returns (uint256[7] memory details_) { require(!_isLiquidationActive(), "DL:HCOR:LIQ_NOT_FINISHED"); - address fundsAsset = IMapleLoanLike(_loan).fundsAsset(); + address fundsAsset = IMapleLoanLike(loan_).fundsAsset(); uint256 principalToCover = _principalRemainingAtLastClaim; // Principal remaining at time of liquidation uint256 fundsCaptured = _fundsToCapture; @@ -205,15 +210,15 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { // If `recoveredFunds` is greater than `principalToCover`, the remaining amount is treated as interest in the context of the pool. // If `recoveredFunds` is less than `principalToCover`, the difference is registered as a shortfall. details_[0] = totalClaimed; - details_[1] = recoveredFunds > principalToCover ? recoveredFunds - principalToCover : 0; + details_[1] = recoveredFunds > principalToCover ? recoveredFunds - principalToCover : uint256(0); details_[2] = fundsCaptured; details_[5] = recoveredFunds > principalToCover ? principalToCover : recoveredFunds; - details_[6] = principalToCover > recoveredFunds ? principalToCover - recoveredFunds : 0; + details_[6] = principalToCover > recoveredFunds ? principalToCover - recoveredFunds : uint256(0); _fundsToCapture = uint256(0); _repossessed = false; - require(ERC20Helper.transfer(fundsAsset, _pool, totalClaimed), "DL:HCOR:TRANSFER"); + require(ERC20Helper.transfer(fundsAsset, pool_, totalClaimed), "DL:HCOR:TRANSFER"); } /**********************/ @@ -240,8 +245,7 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { address loanAddress = _loan; address collateralAsset = IMapleLoanLike(loanAddress).collateralAsset(); address fundsAsset = IMapleLoanLike(loanAddress).fundsAsset(); - - address globals = _getGlobals(); + address globals = _getGlobals(); uint8 collateralAssetDecimals = IERC20Like(collateralAsset).decimals(); @@ -254,7 +258,7 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { / uint256(10) ** uint256(collateralAssetDecimals) // Convert from `fromAsset` decimal precision. / uint256(10_000); // Divide basis points for slippage. - uint256 minRatioAmount = swapAmount_ * _minRatio / 10 ** collateralAssetDecimals; + uint256 minRatioAmount = (swapAmount_ * _minRatio) / (uint256(10) ** collateralAssetDecimals); return oracleAmount > minRatioAmount ? oracleAmount : minRatioAmount; } @@ -316,7 +320,9 @@ contract DebtLocker is IDebtLocker, DebtLockerStorage, MapleProxied { } function _isLiquidationActive() internal view returns (bool isActive_) { - return (_liquidator != address(0)) && (IERC20Like(IMapleLoanLike(_loan).collateralAsset()).balanceOf(_liquidator) != uint256(0)); + address liquidatorAddress = _liquidator; + + return (liquidatorAddress != address(0)) && (IERC20Like(IMapleLoanLike(_loan).collateralAsset()).balanceOf(liquidatorAddress) != uint256(0)); } } diff --git a/contracts/DebtLockerFactory.sol b/contracts/DebtLockerFactory.sol index cf9939c..fe546fc 100644 --- a/contracts/DebtLockerFactory.sol +++ b/contracts/DebtLockerFactory.sol @@ -10,16 +10,14 @@ contract DebtLockerFactory is IDebtLockerFactory, MapleProxyFactory { uint8 public constant override factoryType = uint8(1); - constructor(address mapleGlobals_) MapleProxyFactory(mapleGlobals_) { - require(mapleGlobals_ != address(0)); - } + constructor(address mapleGlobals_) MapleProxyFactory(mapleGlobals_) {} function newLocker(address loan_) external override returns (address debtLocker_) { bytes memory arguments = abi.encode(loan_, msg.sender); - bool success_; - ( success_, debtLocker_ ) = _newInstance(defaultVersion, arguments); - require(success_, "DLF:NL:FAILED"); + bool success; + ( success, debtLocker_ ) = _newInstance(defaultVersion, arguments); + require(success, "DLF:NL:FAILED"); emit InstanceDeployed(defaultVersion, debtLocker_, arguments); } diff --git a/contracts/DebtLockerStorage.sol b/contracts/DebtLockerStorage.sol index 16f8595..ba842f2 100644 --- a/contracts/DebtLockerStorage.sol +++ b/contracts/DebtLockerStorage.sol @@ -4,12 +4,12 @@ pragma solidity 0.8.7; /// @title DebtLockerStorage maps the storage layout of a DebtLocker. contract DebtLockerStorage { - bool internal _repossessed; - - address internal _loan; address internal _liquidator; + address internal _loan; address internal _pool; + bool internal _repossessed; + uint256 internal _allowedSlippage; uint256 internal _amountRecovered; uint256 internal _fundsToCapture; diff --git a/contracts/interfaces/IDebtLocker.sol b/contracts/interfaces/IDebtLocker.sol index b391503..a6411d1 100644 --- a/contracts/interfaces/IDebtLocker.sol +++ b/contracts/interfaces/IDebtLocker.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.7; import { IMapleProxied } from "../../modules/maple-proxy-factory/contracts/interfaces/IMapleProxied.sol"; -/// @title DebtLocker holds custody of LoanFDT tokens. +/// @title DebtLocker interacts with Loans on behalf of PoolV1. interface IDebtLocker is IMapleProxied { /**************/ diff --git a/contracts/interfaces/IDebtLockerFactory.sol b/contracts/interfaces/IDebtLockerFactory.sol index b759419..50b3cea 100644 --- a/contracts/interfaces/IDebtLockerFactory.sol +++ b/contracts/interfaces/IDebtLockerFactory.sol @@ -12,7 +12,7 @@ interface IDebtLockerFactory is IMapleProxyFactory { function factoryType() external view returns (uint8 factoryType_); /** - * @dev Deploys a new DebtLocker proxy instance. + * @dev Deploys a new DebtLocker proxy instance. * @param loan_ Loan contract that corresponds to DebtLocker. */ function newLocker(address loan_) external returns (address debtLocker_); diff --git a/contracts/test/DebtLocker.t.sol b/contracts/test/DebtLocker.t.sol index 5208394..8d95672 100644 --- a/contracts/test/DebtLocker.t.sol +++ b/contracts/test/DebtLocker.t.sol @@ -8,6 +8,8 @@ import { MapleLoanFactory } from "../../modules/loan/contracts/MapleLoanFact import { MapleLoanInitializer } from "../../modules/loan/contracts/MapleLoanInitializer.sol"; import { Refinancer } from "../../modules/loan/contracts/Refinancer.sol"; +import { ILiquidatorLike } from "../interfaces/Interfaces.sol"; + import { DebtLocker } from "../DebtLocker.sol"; import { DebtLockerFactory } from "../DebtLockerFactory.sol"; import { DebtLockerInitializer } from "../DebtLockerInitializer.sol"; @@ -15,45 +17,43 @@ import { DebtLockerInitializer } from "../DebtLockerInitializer.sol"; import { Governor } from "./accounts/Governor.sol"; import { PoolDelegate } from "./accounts/PoolDelegate.sol"; -import { ILiquidatorLike } from "../interfaces/Interfaces.sol"; - import { DebtLockerHarness } from "./mocks/DebtLockerHarness.sol"; import { ManipulatableDebtLocker } from "./mocks/ManipulatableDebtLocker.sol"; -import { - MockGlobals, - MockLiquidationStrategy, +import { + MockGlobals, + MockLiquidationStrategy, MockLoan, - MockMigrator, - MockPool, - MockPoolFactory + MockMigrator, + MockPool, + MockPoolFactory } from "./mocks/Mocks.sol"; interface Hevm { - // Sets block timestamp to `x` + // Sets block timestamp to `x`. function warp(uint256 x) external view; } contract DebtLockerTests is TestUtils { - DebtLockerFactory internal dlFactory; - Governor internal governor; - MapleLoanFactory internal loanFactory; - MapleLoanInitializer internal loanInitializer; - MockERC20 internal collateralAsset; - MockERC20 internal fundsAsset; - MockGlobals internal globals; - MockPool internal pool; - MockPoolFactory internal poolFactory; - PoolDelegate internal notPoolDelegate; - PoolDelegate internal poolDelegate; + DebtLockerFactory internal dlFactory; + Governor internal governor; + MapleLoanFactory internal loanFactory; + MapleLoanInitializer internal loanInitializer; + MockERC20 internal collateralAsset; + MockERC20 internal fundsAsset; + MockGlobals internal globals; + MockPool internal pool; + MockPoolFactory internal poolFactory; + PoolDelegate internal notPoolDelegate; + PoolDelegate internal poolDelegate; - Hevm hevm = Hevm(address(bytes20(uint160(uint256(keccak256("hevm cheat code")))))); + Hevm internal hevm = Hevm(address(bytes20(uint160(uint256(keccak256("hevm cheat code")))))); uint256 internal constant MAX_TOKEN_AMOUNT = 1e12 * 1e18; - function setUp() public { + function setUp() external { governor = new Governor(); notPoolDelegate = new PoolDelegate(); poolDelegate = new PoolDelegate(); @@ -124,14 +124,14 @@ contract DebtLockerTests is TestUtils { debtLocker_ = DebtLocker(pool.createDebtLocker(address(dlFactory), address(loan_))); - _fundAndDrawdownLoan(address(loan_), address(debtLocker_)); + _fundAndDrawdownLoan(address(loan_), address(debtLocker_)); } /*******************/ /*** Claim Tests ***/ /*******************/ - function test_claim(uint256 principalRequested_, uint256 collateralRequired_) public { + function test_claim(uint256 principalRequested_, uint256 collateralRequired_) external { /**********************************/ /*** Create Loan and DebtLocker ***/ @@ -214,7 +214,7 @@ contract DebtLockerTests is TestUtils { assertEq(details[6], 0); // Zero shortfall since no liquidation } - function test_initialize_invalidCollateralAsset() public { + function test_initialize_invalidCollateralAsset() external { MapleLoan loan = _createLoan(1_000_000, 300_000); assertTrue(globals.isValidCollateralAsset(loan.collateralAsset())); @@ -230,7 +230,7 @@ contract DebtLockerTests is TestUtils { assertTrue(pool.createDebtLocker(address(dlFactory), address(loan)) != address(0)); } - function test_initialize_invalidLiquidityAsset() public { + function test_initialize_invalidLiquidityAsset() external { MapleLoan loan = _createLoan(1_000_000, 300_000); assertTrue(globals.isValidLiquidityAsset(loan.fundsAsset())); @@ -246,7 +246,7 @@ contract DebtLockerTests is TestUtils { assertTrue(pool.createDebtLocker(address(dlFactory), address(loan)) != address(0)); } - function test_claim_liquidation_shortfall(uint256 principalRequested_, uint256 collateralRequired_) public { + function test_claim_liquidation_shortfall(uint256 principalRequested_, uint256 collateralRequired_) external { /**********************************/ /*** Create Loan and DebtLocker ***/ @@ -338,7 +338,7 @@ contract DebtLockerTests is TestUtils { assertEq(details[6], principalToCover - amountRecovered); // Shortfall to be covered by burning BPTs } - function test_claim_liquidation_equalToPrincipal(uint256 principalRequested_) public { + function test_claim_liquidation_equalToPrincipal(uint256 principalRequested_) external { /*************************/ /*** Set up parameters ***/ @@ -419,7 +419,7 @@ contract DebtLockerTests is TestUtils { assertEq(details[6], 0); // Zero shortfall since principalToCover == amountRecovered } - function test_claim_liquidation_greaterThanPrincipal(uint256 principalRequested_, uint256 excessRecovered_) public { + function test_claim_liquidation_greaterThanPrincipal(uint256 principalRequested_, uint256 excessRecovered_) external { /*************************/ /*** Set up parameters ***/ @@ -433,7 +433,7 @@ contract DebtLockerTests is TestUtils { /**********************************/ /*** Create Loan and DebtLocker ***/ /**********************************/ - + ( MapleLoan loan, DebtLocker debtLocker ) = _createFundAndDrawdownLoan(principalRequested_, collateralRequired); /*************************************/ @@ -499,7 +499,7 @@ contract DebtLockerTests is TestUtils { assertEq(details[6], 0); // Zero shortfall since principalToCover == amountRecovered } - function test_liquidation_dos_prevention(uint256 principalRequested_, uint256 collateralRequired_) public { + function test_liquidation_dos_prevention(uint256 principalRequested_, uint256 collateralRequired_) external { /**********************************/ /*** Create Loan and DebtLocker ***/ @@ -544,7 +544,7 @@ contract DebtLockerTests is TestUtils { pool.claim(address(debtLocker)); // Can successfully claim } - + /****************************/ /*** Access Control Tests ***/ /****************************/ @@ -805,7 +805,7 @@ contract DebtLockerTests is TestUtils { /*** Funds Capture Tests ***/ /***************************/ - function test_fundsToCaptureForNextClaim() public { + function test_fundsToCaptureForNextClaim() external { ( MapleLoan loan, DebtLocker debtLocker ) = _createFundAndDrawdownLoan(1_000_000, 30_000); // Make a payment amount with interest and principal @@ -838,7 +838,7 @@ contract DebtLockerTests is TestUtils { assertEq(details[6], 0); } - function test_fundsToCaptureWhileInDefault() public { + function test_fundsToCaptureWhileInDefault() external { ( MapleLoan loan, DebtLocker debtLocker ) = _createFundAndDrawdownLoan(1_000_000, 30_000); // Prepare additional amount to be captured @@ -879,7 +879,7 @@ contract DebtLockerTests is TestUtils { assertEq(details[6], 700_000); // 300k recovered on a 1m loan } - function testFail_fundsToCaptureForNextClaim() public { + function testFail_fundsToCaptureForNextClaim() external { ( MapleLoan loan, DebtLocker debtLocker ) = _createFundAndDrawdownLoan(1_000_000, 30_000); fundsAsset.mint(address(loan), 1_000_000); @@ -905,7 +905,7 @@ contract DebtLockerTests is TestUtils { /*** Internal View Function Tests ***/ /************************************/ - function _registerDebtLockerHarnesss() internal { + function _registerDebtLockerHarness() internal { // Deploying and registering DebtLocker implementation and initializer address implementation = address(new DebtLockerHarness()); address initializer = address(new DebtLockerInitializer()); @@ -914,8 +914,8 @@ contract DebtLockerTests is TestUtils { governor.mapleProxyFactory_setDefaultVersion(address(dlFactory), 2); } - function test_getGlobals() public { - _registerDebtLockerHarnesss(); + function test_getGlobals() external { + _registerDebtLockerHarness(); MapleLoan loan = _createLoan(1_000_000, 30_000); @@ -924,8 +924,8 @@ contract DebtLockerTests is TestUtils { assertEq(debtLocker.getGlobals(), address(globals)); } - function test_getPoolDelegate() public { - _registerDebtLockerHarnesss(); + function test_getPoolDelegate() external { + _registerDebtLockerHarness(); MapleLoan loan = _createLoan(1_000_000, 30_000); @@ -934,8 +934,8 @@ contract DebtLockerTests is TestUtils { assertEq(debtLocker.poolDelegate(), address(poolDelegate)); } - function test_isLiquidationActive() public { - _registerDebtLockerHarnesss(); + function test_isLiquidationActive() external { + _registerDebtLockerHarness(); MapleLoan loan = _createLoan(1_000_000, 30_000); diff --git a/contracts/test/accounts/PoolDelegate.sol b/contracts/test/accounts/PoolDelegate.sol index 4a06abd..b2ee578 100644 --- a/contracts/test/accounts/PoolDelegate.sol +++ b/contracts/test/accounts/PoolDelegate.sol @@ -34,7 +34,7 @@ contract PoolDelegate is ProxyUser { function debtLocker_stopLiquidation(address debtLocker_) external { IDebtLocker(debtLocker_).stopLiquidation(); } - + function debtLocker_upgrade(address debtLocker_, uint256 toVersion_, bytes memory arguments_) external { IDebtLocker(debtLocker_).upgrade(toVersion_, arguments_); } @@ -44,7 +44,7 @@ contract PoolDelegate is ProxyUser { /*********************/ function try_debtLocker_acceptNewTerms( - address debtLocker_, + address debtLocker_, address refinancer_, bytes[] calldata calls_, uint256 amount_ @@ -71,7 +71,7 @@ contract PoolDelegate is ProxyUser { function try_debtLocker_stopLiquidation(address debtLocker_) external returns (bool ok_) { ( ok_, ) = debtLocker_.call(abi.encodeWithSelector(IDebtLocker.stopLiquidation.selector)); } - + function try_debtLocker_upgrade(address debtLocker_, uint256 toVersion_, bytes memory arguments_) external returns (bool ok_) { ( ok_, ) = debtLocker_.call(abi.encodeWithSelector(IMapleProxied.upgrade.selector, toVersion_, arguments_)); } diff --git a/contracts/test/mocks/ManipulatableDebtLocker.sol b/contracts/test/mocks/ManipulatableDebtLocker.sol index eef9b5a..08833d8 100644 --- a/contracts/test/mocks/ManipulatableDebtLocker.sol +++ b/contracts/test/mocks/ManipulatableDebtLocker.sol @@ -9,7 +9,7 @@ contract ManipulatableDebtLocker is DebtLocker { bytes32 constant FACTORY_SLOT = bytes32(0x7a45a402e4cb6e08ebc196f20f66d5d30e67285a2a8aa80503fa409e727a4af1); - constructor(address loan_, address pool_, address factory_) public { + constructor(address loan_, address pool_, address factory_) { _loan = loan_; _pool = pool_; diff --git a/contracts/test/mocks/Mocks.sol b/contracts/test/mocks/Mocks.sol index e2ee2d7..552a37c 100644 --- a/contracts/test/mocks/Mocks.sol +++ b/contracts/test/mocks/Mocks.sol @@ -1,8 +1,7 @@ // SPDX-License-Identifier: AGPL-3.0-or-later pragma solidity 0.8.7; -import { IERC20 } from "../../../modules/erc20/src/interfaces/IERC20.sol"; -import { ILiquidatorLike } from "../../../modules/liquidations/contracts/interfaces/Interfaces.sol"; +import { ILiquidatorLike } from "../../../modules/liquidations/contracts/interfaces/Interfaces.sol"; import { ERC20Helper } from "../../../modules/erc20-helper/src/ERC20Helper.sol"; import { MockERC20 } from "../../../modules/erc20/src/test/mocks/MockERC20.sol"; diff --git a/modules/erc20 b/modules/erc20 index de7fc2c..3e5f4f1 160000 --- a/modules/erc20 +++ b/modules/erc20 @@ -1 +1 @@ -Subproject commit de7fc2cda0f847c4e8fa087bdc5dc04018bdb49a +Subproject commit 3e5f4f1c90d1f2c5520bf929c983323917581ca4 diff --git a/modules/erc20-helper b/modules/erc20-helper index ff79ca1..3214e1d 160000 --- a/modules/erc20-helper +++ b/modules/erc20-helper @@ -1 +1 @@ -Subproject commit ff79ca15440caac524d114e71582055784c249ec +Subproject commit 3214e1da6d72619b7268286bbdd8cb2b3a0fe6b4 diff --git a/modules/liquidations b/modules/liquidations index c85ad8f..7c4481a 160000 --- a/modules/liquidations +++ b/modules/liquidations @@ -1 +1 @@ -Subproject commit c85ad8fba1a2c5e92971a715d5e3410dc2dbf8de +Subproject commit 7c4481a52a7c69d07203c359ee3e44d05a29e3a9 diff --git a/modules/loan b/modules/loan index 9684bce..5ac31e1 160000 --- a/modules/loan +++ b/modules/loan @@ -1 +1 @@ -Subproject commit 9684bcef06481e493d060974b1777a4517c4e792 +Subproject commit 5ac31e1d6aeeea5c303a85c4b000f96425b328e9 diff --git a/modules/maple-proxy-factory b/modules/maple-proxy-factory index ee5f6be..5562c7c 160000 --- a/modules/maple-proxy-factory +++ b/modules/maple-proxy-factory @@ -1 +1 @@ -Subproject commit ee5f6beaf857c1b900cf74bcfdc4250a5189482a +Subproject commit 5562c7cf05d131c7da63934c536170f037a73507