Skip to content

Commit

Permalink
Sherlock 70: user can drawDebt that is below dust amount (#598)
Browse files Browse the repository at this point in the history
* ensure borrower debt exceeds dust amount regardless of loan count

* Revert "ensure borrower debt exceeds dust amount regardless of loan count"

This reverts commit c0a64f9.

* re-applied changes lost when rebasing
  • Loading branch information
EdNoepel authored Feb 9, 2023
1 parent e51b2c1 commit 947a125
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 36 deletions.
13 changes: 9 additions & 4 deletions src/libraries/helpers/RevertsHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,23 @@ import { Maths } from '../internal/Maths.sol';
}
}

/**
* @notice Called when borrower debt changes, ensuring minimum debt rules are honored.
* @param loans_ Loans heap, used to determine loan count.
* @param poolDebt_ Total pool debt, used to calculate average debt.
* @param borrowerDebt_ New debt for the borrower, assuming the current transaction succeeds.
* @param quoteDust_ Smallest amount of quote token when can be transferred, determined by token scale.
*/
function _revertOnMinDebt(
LoansState storage loans_,
uint256 poolDebt_,
uint256 borrowerDebt_,
uint256 quoteDust_
) view {
if (borrowerDebt_ != 0) {
if (borrowerDebt_ < quoteDust_) revert DustAmountNotExceeded();
uint256 loansCount = Loans.noOfLoans(loans_);
if (loansCount >= 10) {
if (loansCount >= 10)
if (borrowerDebt_ < _minDebtAmount(poolDebt_, loansCount)) revert AmountLTMinDebt();
} else {
if (borrowerDebt_ < quoteDust_) revert DustAmountNotExceeded();
}
}
}
80 changes: 75 additions & 5 deletions tests/forge/ERC721Pool/ERC721DSTestPlus.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { ERC20 } from '@openzeppelin/contracts/token/ERC20/ERC20.sol';
import '@openzeppelin/contracts/token/ERC721/ERC721.sol';
import '@openzeppelin/contracts/utils/structs/EnumerableSet.sol';

import { DSTestPlus } from '../utils/DSTestPlus.sol';
import { NFTCollateralToken, Token } from '../utils/Tokens.sol';
import { DSTestPlus } from '../utils/DSTestPlus.sol';
import { NFTCollateralToken, Token, TokenWithNDecimals } from '../utils/Tokens.sol';

import { ERC721Pool } from 'src/ERC721Pool.sol';
import { ERC721PoolFactory } from 'src/ERC721PoolFactory.sol';
Expand All @@ -25,7 +25,7 @@ abstract contract ERC721DSTestPlus is DSTestPlus, IERC721PoolEvents {
using EnumerableSet for EnumerableSet.UintSet;

NFTCollateralToken internal _collateral;
Token internal _quote;
TokenWithNDecimals internal _quote;
ERC20 internal _ajnaToken;

mapping(uint256 => uint256) NFTidToIndex;
Expand Down Expand Up @@ -563,6 +563,17 @@ abstract contract ERC721DSTestPlus is DSTestPlus, IERC721PoolEvents {
ERC721Pool(address(_pool)).drawDebt(from, amount, indexLimit, emptyArray);
}

function _assertBorrowDustRevert(
address from,
uint256 amount,
uint256 indexLimit
) internal {
changePrank(from);
vm.expectRevert(IPoolErrors.DustAmountNotExceeded.selector);
uint256[] memory emptyArray;
ERC721Pool(address(_pool)).drawDebt(from, amount, indexLimit, emptyArray);
}

function _assertBorrowMinDebtRevert(
address from,
uint256 amount,
Expand Down Expand Up @@ -635,7 +646,7 @@ abstract contract ERC721HelperContract is ERC721DSTestPlus {

_collateral = new NFTCollateralToken();
vm.makePersistent(address(_collateral));
_quote = new Token("Quote", "Q");
_quote = new TokenWithNDecimals("Quote", "Q", 18);
vm.makePersistent(address(_quote));
_ajnaToken = ERC20(_ajna);
vm.makePersistent(_ajna);
Expand Down Expand Up @@ -702,6 +713,65 @@ abstract contract ERC721HelperContract is ERC721DSTestPlus {
}
}

abstract contract ERC721NDecimalsHelperContract is ERC721DSTestPlus {
using EnumerableSet for EnumerableSet.AddressSet;
ERC721PoolFactory internal _poolFactory;

constructor(uint8 decimals) {
vm.createSelectFork(vm.envString("ETH_RPC_URL"));

_collateral = new NFTCollateralToken();
vm.makePersistent(address(_collateral));
_quote = new TokenWithNDecimals("Quote", "Q", decimals);
vm.makePersistent(address(_quote));
_ajnaToken = ERC20(_ajna);
vm.makePersistent(_ajna);
_poolUtils = new PoolInfoUtils();
vm.makePersistent(address(_poolUtils));
_poolFactory = new ERC721PoolFactory(_ajna);
vm.makePersistent(address(_poolFactory));

_startTime = block.timestamp;
uint256[] memory tokenIds;
address contractAddress = _poolFactory.deployPool(address(_collateral), address(_quote), tokenIds, 0.05 * 10**18);
vm.makePersistent(contractAddress);
_pool = ERC721Pool(contractAddress);
}

function _mintAndApproveQuoteTokens(address operator_, uint256 mintAmount_) internal {
deal(address(_quote), operator_, mintAmount_);
vm.prank(operator_);
_quote.approve(address(_pool), type(uint256).max);
}

function _mintAndApproveCollateralTokens(address operator_, uint256 mintAmount_) internal {
_collateral.mint(operator_, mintAmount_);
vm.prank(operator_);
_collateral.setApprovalForAll(address(_pool), true);
}

/**
* @dev Creates debt for an anonymous non-player borrower not otherwise involved in the test.
**/
function _anonBorrowerDrawsDebt(uint256 loanAmount) internal {
// _anonBorrowerCount += 1;

address borrower = makeAddr(string(abi.encodePacked("anonBorrower", borrowers.length())));
vm.stopPrank();
_mintAndApproveCollateralTokens(borrower, 1);
uint256[] memory tokenIdsToAdd = new uint256[](1);
tokenIdsToAdd[0] = _collateral.totalSupply();

_drawDebtNoLupCheck({
from: borrower,
borrower: borrower,
amountToBorrow: loanAmount,
limitIndex: 7_777,
tokenIds: tokenIdsToAdd
});
}
}

abstract contract ERC721FuzzyHelperContract is ERC721DSTestPlus {

uint256 public constant LARGEST_AMOUNT = type(uint256).max / 10**27;
Expand All @@ -710,7 +780,7 @@ abstract contract ERC721FuzzyHelperContract is ERC721DSTestPlus {

constructor() {
_collateral = new NFTCollateralToken();
_quote = new Token("Quote", "Q");
_quote = new TokenWithNDecimals("Quote", "Q", 18);
_ajnaToken = ERC20(_ajna);
_poolUtils = new PoolInfoUtils();
_poolFactory = new ERC721PoolFactory(_ajna);
Expand Down
81 changes: 58 additions & 23 deletions tests/forge/ERC721Pool/ERC721PoolBorrow.t.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.14;

import { ERC721HelperContract, ERC721FuzzyHelperContract } from './ERC721DSTestPlus.sol';
import { ERC721HelperContract, ERC721FuzzyHelperContract, ERC721NDecimalsHelperContract } from './ERC721DSTestPlus.sol';

import 'src/ERC721Pool.sol';

Expand Down Expand Up @@ -536,31 +536,19 @@ contract ERC721SubsetPoolBorrowTest is ERC721PoolBorrowTest {
}
}

contract ERC721CollectionPoolBorrowTest is ERC721PoolBorrowTest {
uint internal _anonBorrowerCount = 0;
contract ERC721CollectionPoolBorrowTest is ERC721NDecimalsHelperContract(18) {
address internal _borrower;
address internal _lender;

function createPool() external override returns (ERC721Pool) {
return _deployCollectionPool();
}
function setUp() external {
_borrower = makeAddr("borrower");
_lender = makeAddr("lender");

/**
* @dev Creates debt for an anonymous non-player borrower not otherwise involved in the test.
**/
function _anonBorrowerDrawsDebt(uint256 loanAmount) internal {
_anonBorrowerCount += 1;
address borrower = makeAddr(string(abi.encodePacked("anonBorrower", _anonBorrowerCount)));
vm.stopPrank();
_mintAndApproveCollateralTokens(borrower, 1);
uint256[] memory tokenIdsToAdd = new uint256[](1);
tokenIdsToAdd[0] = _collateral.totalSupply();
_mintAndApproveQuoteTokens(_lender, 200_000 * 1e18);
_mintAndApproveCollateralTokens(_borrower, 52);

_drawDebtNoLupCheck({
from: borrower,
borrower: borrower,
amountToBorrow: loanAmount,
limitIndex: 7_777,
tokenIds: tokenIdsToAdd
});
vm.prank(_borrower);
_quote.approve(address(_pool), 200_000 * 1e18);
}

function testMinBorrowAmountCheck() external tearDown {
Expand Down Expand Up @@ -626,9 +614,56 @@ contract ERC721CollectionPoolBorrowTest is ERC721PoolBorrowTest {
amount: 900 * 1e18
});
}
}

contract ERC721ScaledQuoteTokenBorrowTest is ERC721NDecimalsHelperContract(4) {
address internal _borrower;
address internal _lender;

function setUp() external {
_borrower = makeAddr("borrower");
_lender = makeAddr("lender");

_mintAndApproveQuoteTokens(_lender, 20_000 * 1e4);
_mintAndApproveCollateralTokens(_borrower, 5);
}

function testMinDebtBelowDustLimitCheck() external tearDown {
// add initial quote to the pool
changePrank(_lender);
_pool.addQuoteToken(20_000 * 1e18, 2550);

// borrower pledges a single NFT
uint256[] memory tokenIdsToAdd = new uint256[](1);
tokenIdsToAdd[0] = 5;
_pledgeCollateral({
from: _borrower,
borrower: _borrower,
tokenIds: tokenIdsToAdd
});

// should revert if borrower tries to draw debt below dust limit
_assertBorrowDustRevert({
from: _borrower,
amount: 0.00005 * 1e18,
indexLimit: 2550
});

// 10 borrowers draw debt at the dust limit
for (uint i=0; i<10; ++i) {
_anonBorrowerDrawsDebt(0.0001 * 1e18);
}

// should still revert if borrower tries to draw debt below dust limit
_assertBorrowDustRevert({
from: _borrower,
amount: 0.000075 * 1e18,
indexLimit: 2550
});
}
}


contract ERC721PoolBorrowFuzzyTest is ERC721FuzzyHelperContract {

address internal _borrower;
Expand Down
18 changes: 14 additions & 4 deletions tests/forge/ERC721Pool/ERC721PoolFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity 0.8.14;

import { ERC721HelperContract } from './ERC721DSTestPlus.sol';
import { NFTCollateralToken, Token } from '../utils/Tokens.sol';
import { NFTCollateralToken, TokenWithNDecimals } from '../utils/Tokens.sol';

import { ERC721Pool } from 'src/ERC721Pool.sol';
import { ERC721PoolFactory } from 'src/ERC721PoolFactory.sol';
Expand All @@ -23,7 +23,7 @@ contract ERC721PoolFactoryTest is ERC721HelperContract {
function setUp() external {
_startTime = block.timestamp;
_collateral = new NFTCollateralToken();
_quote = new Token("Quote", "Q");
_quote = new TokenWithNDecimals("Quote", "Q", 18);

// deploy factory
_factory = new ERC721PoolFactory(_ajna);
Expand Down Expand Up @@ -153,7 +153,12 @@ contract ERC721PoolFactoryTest is ERC721HelperContract {

function testDeployERC721PoolWithMinRate() external {
uint256[] memory tokenIds = new uint256[](0);
_factory.deployPool(address(new NFTCollateralToken()), address(new Token("Quote", "Q1")), tokenIds, 0.01 * 10**18);
_factory.deployPool(
address(new NFTCollateralToken()),
address(new TokenWithNDecimals("Quote", "Q1", 18)),
tokenIds,
0.01 * 10**18
);

// check tracking of deployed pools
assertEq(_factory.getDeployedPoolsList().length, 4);
Expand All @@ -162,7 +167,12 @@ contract ERC721PoolFactoryTest is ERC721HelperContract {

function testDeployERC721PoolWithMaxRate() external {
uint256[] memory tokenIds = new uint256[](0);
_factory.deployPool(address(new NFTCollateralToken()), address(new Token("Quote", "Q1")), tokenIds, 0.1 * 10**18);
_factory.deployPool(
address(new NFTCollateralToken()),
address(new TokenWithNDecimals("Quote", "Q1", 18)),
tokenIds,
0.1 * 10**18
);

// check tracking of deployed pools
assertEq(_factory.getDeployedPoolsList().length, 4);
Expand Down

0 comments on commit 947a125

Please sign in to comment.