Skip to content

Instantly share code, notes, and snippets.

@Picodes
Created October 25, 2022 20:04
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save Picodes/8d3a45d6d1362fb9953d631d8c84a29f to your computer and use it in GitHub Desktop.
Save Picodes/8d3a45d6d1362fb9953d631d8c84a29f to your computer and use it in GitHub Desktop.

Report

Gas Optimizations

Issue Instances
[GAS-1] Use != 0 instead of > 0 for unsigned integer comparison 20
[GAS-2] Using bools for storage incurs overhead 4
[GAS-3] Use Custom Errors 66
[GAS-4] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) 5
[GAS-5] Using private rather than public for constants, saves gas 1

[GAS-1] Use != 0 instead of > 0 for unsigned integer comparison

Instances (20):

File: DBR.sol

63:         require(newReplenishmentPriceBps_ > 0, "replenishment price must be over 0");

328:         require(deficit > 0, "No deficit");
File: Fed.sol

133:         if(profit > 0) {
File: Market.sol

75:         require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps < 10000, "Invalid liquidation incentive");

162:         require(_liquidationFactorBps > 0 && _liquidationFactorBps <= 10000, "Invalid liquidation factor");

173:         require(_replenishmentIncentiveBps > 0 && _replenishmentIncentiveBps < 10000, "Invalid replenishment incentive");

184:         require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps + liquidationFeeBps < 10000, "Invalid liquidation incentive");

195:         require(_liquidationFeeBps > 0 && _liquidationFeeBps + liquidationIncentiveBps < 10000, "Invalid liquidation fee");

561:         require(deficit > 0, "No DBR deficit");

592:         require(repaidDebt > 0, "Must repay positive debt");

605:         if(liquidationFeeBps > 0) {
File: Oracle.sol

79:         if(fixedPrices[token] > 0) return fixedPrices[token];

83:             require(price > 0, "Invalid feed price");

96:             uint twoDayLow = todaysLow > yesterdaysLow && yesterdaysLow > 0 ? yesterdaysLow : todaysLow;

97:             if(twoDayLow > 0 && newBorrowingPower > twoDayLow) {

113:         if(fixedPrices[token] > 0) return fixedPrices[token];

117:             require(price > 0, "Invalid feed price");

135:             uint twoDayLow = todaysLow > yesterdaysLow && yesterdaysLow > 0 ? yesterdaysLow : todaysLow;

136:             if(twoDayLow > 0 && newBorrowingPower > twoDayLow) {
File: escrows/INVEscrow.sol

81:         if(invBalance > 0) {

[GAS-2] Using bools for storage incurs overhead

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.

Instances (4):

File: BorrowController.sol

11:     mapping(address => bool) public contractAllowlist;
File: DBR.sol

24:     mapping (address => bool) public minters;

25:     mapping (address => bool) public markets;
File: Market.sol

53:     bool public borrowPaused;

[GAS-3] Use Custom Errors

Source Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost.

Instances (66):

File: BorrowController.sol

18:         require(msg.sender == operator, "Only operator");
File: DBR.sol

45:         require(msg.sender == operator, "ONLY OPERATOR");

63:         require(newReplenishmentPriceBps_ > 0, "replenishment price must be over 0");

71:         require(msg.sender == pendingOperator, "ONLY PENDING OPERATOR");

171:         require(balanceOf(msg.sender) >= amount, "Insufficient balance");

195:         require(balanceOf(from) >= amount, "Insufficient balance");

224:         require(deadline >= block.timestamp, "PERMIT_DEADLINE_EXPIRED");

249:             require(recoveredAddress != address(0) && recoveredAddress == owner, "INVALID_SIGNER");

301:         require(markets[msg.sender], "Only markets can call onBorrow");

303:         require(deficitOf(user) == 0, "DBR Deficit");

314:         require(markets[msg.sender], "Only markets can call onRepay");

326:         require(markets[msg.sender], "Only markets can call onForceReplenish");

328:         require(deficit > 0, "No deficit");

329:         require(deficit >= amount, "Amount > deficit");

350:         require(minters[msg.sender] == true || msg.sender == operator, "ONLY MINTERS OR OPERATOR");

373:         require(balanceOf(from) >= amount, "Insufficient balance");
File: Fed.sol

49:         require(msg.sender == gov, "ONLY GOV");

58:         require(msg.sender == gov, "ONLY GOV");

67:         require(msg.sender == gov, "ONLY GOV");

76:         require(msg.sender == chair, "ONLY CHAIR");

87:         require(msg.sender == chair, "ONLY CHAIR");

88:         require(dbr.markets(address(market)), "UNSUPPORTED MARKET");

89:         require(market.borrowPaused() != true, "CANNOT EXPAND PAUSED MARKETS");

104:         require(msg.sender == chair, "ONLY CHAIR");

105:         require(dbr.markets(address(market)), "UNSUPPORTED MARKET");

107:         require(amount <= supply, "AMOUNT TOO BIG"); // can't burn profits
File: Market.sol

74:         require(_collateralFactorBps < 10000, "Invalid collateral factor");

75:         require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps < 10000, "Invalid liquidation incentive");

76:         require(_replenishmentIncentiveBps < 10000, "Replenishment incentive must be less than 100%");

93:         require(msg.sender == gov, "Only gov can call this function");

150:         require(_collateralFactorBps < 10000, "Invalid collateral factor");

162:         require(_liquidationFactorBps > 0 && _liquidationFactorBps <= 10000, "Invalid liquidation factor");

173:         require(_replenishmentIncentiveBps > 0 && _replenishmentIncentiveBps < 10000, "Invalid replenishment incentive");

184:         require(_liquidationIncentiveBps > 0 && _liquidationIncentiveBps + liquidationFeeBps < 10000, "Invalid liquidation incentive");

195:         require(_liquidationFeeBps > 0 && _liquidationFeeBps + liquidationIncentiveBps < 10000, "Invalid liquidation fee");

204:         require(msg.sender == lender, "Only lender can recall");

214:             require(msg.sender == pauseGuardian || msg.sender == gov, "Only pause guardian or governance can pause");

216:             require(msg.sender == gov, "Only governance can unpause");

236:         require(instance != IEscrow(address(0)), "ERC1167: create2 failed");

390:         require(!borrowPaused, "Borrowing is paused");

392:             require(borrowController.borrowAllowed(msg.sender, borrower, amount), "Denied by borrow controller");

396:         require(credit >= debts[borrower], "Exceeded credit limit");

423:         require(deadline >= block.timestamp, "DEADLINE_EXPIRED");

448:             require(recoveredAddress != address(0) && recoveredAddress == from, "INVALID_SIGNER");

462:         require(limit >= amount, "Insufficient withdrawal limit");

487:         require(deadline >= block.timestamp, "DEADLINE_EXPIRED");

512:             require(recoveredAddress != address(0) && recoveredAddress == from, "INVALID_SIGNER");

533:         require(debt >= amount, "Insufficient debt");

561:         require(deficit > 0, "No DBR deficit");

562:         require(deficit >= amount, "Amount > deficit");

567:         require(collateralValue >= debts[user], "Exceeded collateral value");

592:         require(repaidDebt > 0, "Must repay positive debt");

594:         require(getCreditLimitInternal(user) < debt, "User debt is healthy");

595:         require(repaidDebt <= debt * liquidationFactorBps / 10000, "Exceeded liquidation factor");
File: Oracle.sol

36:         require(msg.sender == operator, "ONLY OPERATOR");

67:         require(msg.sender == pendingOperator, "ONLY PENDING OPERATOR");

83:             require(price > 0, "Invalid feed price");

104:         revert("Price not found");

117:             require(price > 0, "Invalid feed price");

143:         revert("Price not found");
File: escrows/GovTokenEscrow.sol

31:         require(market == address(0), "ALREADY INITIALIZED");

44:         require(msg.sender == market, "ONLY MARKET");
File: escrows/INVEscrow.sol

45:         require(market == address(0), "ALREADY INITIALIZED");

60:         require(msg.sender == market, "ONLY MARKET");
File: escrows/SimpleERC20Escrow.sol

26:         require(market == address(0), "ALREADY INITIALIZED");

37:         require(msg.sender == market, "ONLY MARKET");

[GAS-4] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

Saves 5 gas per loop

Instances (5):

File: DBR.sol

239:                                 nonces[owner]++,

259:         nonces[msg.sender]++;
File: Market.sol

438:                                 nonces[from]++,

502:                                 nonces[from]++,

521:         nonces[msg.sender]++;

[GAS-5] Using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it's used, and not adding another entry to the method ID table

Instances (1):

File: DBR.sol

13:     uint8 public constant decimals = 18;

Non Critical Issues

Issue Instances
[NC-1] Return values of approve() not checked 1

[NC-1] Return values of approve() not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything

Instances (1):

File: escrows/INVEscrow.sol

50:         _token.approve(address(xINV), type(uint).max);

Low Issues

Issue Instances
[L-1] Unsafe ERC20 operation(s) 11

[L-1] Unsafe ERC20 operation(s)

Instances (11):

File: Fed.sol

135:             dola.transfer(gov, profit);
File: Market.sol

205:         dola.transfer(msg.sender, amount);

280:         collateral.transferFrom(msg.sender, address(escrow), amount);

399:         dola.transfer(to, amount);

537:         dola.transferFrom(msg.sender, address(this), amount);

570:         dola.transfer(msg.sender, replenisherReward);

602:         dola.transferFrom(msg.sender, address(this), repaidDebt);
File: escrows/GovTokenEscrow.sol

45:         token.transfer(recipient, amount);
File: escrows/INVEscrow.sol

50:         _token.approve(address(xINV), type(uint).max);

63:         token.transfer(recipient, amount);
File: escrows/SimpleERC20Escrow.sol

38:         token.transfer(recipient, amount);
@francocoin1701
Copy link

AAAAAAAA THESE ARE ALL THE VULNERABILITIES THAT I KNOW

@balook
Copy link

balook commented Oct 26, 2022

AAAAAAAA THESE ARE ALL THE VULNERABILITIES THAT I KNOW

hahahaha

@0xAnon101
Copy link

which generator you used to generate this report, since the file name is c4-report-generator

@peritoflores
Copy link

[GAS-4] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)
is invalid when you try to increase nonce++. Cannot change for ++nonce otherwise signatures will be invalid

@bluenights004
Copy link

These findings were used to worth around 9 to 50 usd per report and now suddenly becomes virtually zero.
Good thing though in the long run to focus more on high and medium risk rewards.

@Picodes
Copy link
Author

Picodes commented Oct 28, 2022

which generator you used to generate this report, since the file name is c4-report-generator

https://github.com/Picodes/code4rena-report-generator

@bluenights004
Copy link

Can we call it "The Picodes' List" a.k.a. Publicly known findings to honor the author?

Much easier to call the list for the reference haha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment