Skip to content

Instantly share code, notes, and snippets.

@misirov
Created February 22, 2024 21:38
Show Gist options
  • Save misirov/7b60a697b5c3e0fea9f0e0a6eeb7e3d4 to your computer and use it in GitHub Desktop.
Save misirov/7b60a697b5c3e0fea9f0e0a6eeb7e3d4 to your computer and use it in GitHub Desktop.
Out of Scope: Arcade-Protocol 4nalyzer

Report

Scope:  [
  'LoanCore.sol',
  'RepaymentController.sol',
  'origination/OriginationCalculator.sol',
  'origination/OriginationConfiguration.sol',
  'origination/OriginationController.sol',
  'origination/OriginationControllerMigrate.sol',
  'origination/RefinanceController.sol',
  'libraries/InterestCalculator.sol',
  'libraries/LoanLibrary.sol',
  'libraries/OriginationLibrary.sol'
]

Gas Optimizations

Issue Instances
GAS-1 Using bools for storage incurs overhead 8
GAS-2 Cache array length outside of loop 6
GAS-3 For Operations that will not overflow, you could use unchecked 290
GAS-4 Don't initialize variables with default value 8
GAS-5 Functions guaranteed to revert when called by normal users can be marked payable 3
GAS-6 ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) 8
GAS-7 Using private rather than public for constants, saves gas 18
GAS-8 Use != 0 instead of > 0 for unsigned integer comparison 27

[GAS-1] 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 (8):

File: LoanCore.sol

89:     mapping(bytes32 => bool) private collateralInUse;

93:     mapping(address => mapping(uint160 => bool)) public usedNonces;

519:         mapping(uint160 => bool) storage _usedNonces = usedNonces[msg.sender];

861:         mapping(uint160 => bool) storage _usedNonces = usedNonces[user];
File: origination/OriginationConfiguration.sol

37:     mapping(address => bool) private _allowedVerifiers;

41:     mapping(address => bool) private _allowedCollateral;
File: origination/OriginationController.sol

85:     mapping(address => mapping(address => bool)) private _signerApprovals;
File: origination/OriginationControllerMigrate.sol

47:     bool public paused;

[GAS-2] Cache array length outside of loop

If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).

Instances (6):

File: LoanCore.sol

739:         for (uint256 i = 0; i < codes.length;) {
File: libraries/OriginationLibrary.sol

102:         for (uint i = 0; i < predicates.length;){
File: origination/OriginationConfiguration.sol

145:         for (uint256 i = 0; i < verifiers.length;) {

175:         for (uint256 i = 0; i < tokens.length;) {

205:         for (uint256 i = 0; i < tokens.length;) {
File: origination/OriginationController.sol

471:         for (uint256 i = 0; i < itemPredicates.length;) {

[GAS-3] For Operations that will not overflow, you could use unchecked

Instances (290):

File: LoanCore.sol

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

6: import "@openzeppelin/contracts/token/ERC721/IERC721.sol";

6: import "@openzeppelin/contracts/token/ERC721/IERC721.sol";

6: import "@openzeppelin/contracts/token/ERC721/IERC721.sol";

6: import "@openzeppelin/contracts/token/ERC721/IERC721.sol";

7: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

7: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

7: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

8: import "@openzeppelin/contracts/security/Pausable.sol";

8: import "@openzeppelin/contracts/security/Pausable.sol";

8: import "@openzeppelin/contracts/security/Pausable.sol";

9: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

9: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

9: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

10: import "@openzeppelin/contracts/utils/Counters.sol";

10: import "@openzeppelin/contracts/utils/Counters.sol";

10: import "@openzeppelin/contracts/utils/Counters.sol";

12: import "./interfaces/ILoanCore.sol";

12: import "./interfaces/ILoanCore.sol";

13: import "./interfaces/ICallDelegator.sol";

13: import "./interfaces/ICallDelegator.sol";

14: import "./interfaces/IPromissoryNote.sol";

14: import "./interfaces/IPromissoryNote.sol";

16: import "./PromissoryNote.sol";

17: import "./libraries/InterestCalculator.sol";

17: import "./libraries/InterestCalculator.sol";

18: import "./libraries/Constants.sol";

18: import "./libraries/Constants.sol";

19: import "./vault/OwnableERC721.sol";

19: import "./vault/OwnableERC721.sol";

38: } from "./errors/Lending.sol";

38: } from "./errors/Lending.sol";

179:         if (protocolFee > 0) feesWithdrawable[terms.payableCurrency][address(this)] += protocolFee;

180:         if (affiliateFee > 0) feesWithdrawable[terms.payableCurrency][affiliate] += affiliateFee;

284:             receipt.amount += _amountToLender;

329:         uint256 dueDate = data.startDate + data.terms.durationSecs + Constants.GRACE_PERIOD;

329:         uint256 dueDate = data.startDate + data.terms.durationSecs + Constants.GRACE_PERIOD;

345:             if (protocolFee > 0) _feesWithdrawable[address(this)] += protocolFee;

346:             if (affiliateFee > 0) _feesWithdrawable[affiliate] += affiliateFee;

438:         data.interestAmountPaid += _interestAmount;

444:         if (_amountToOldLender + _amountToLender + _amountToBorrower > _settledAmount)

444:         if (_amountToOldLender + _amountToLender + _amountToBorrower > _settledAmount)

445:             revert LC_CannotSettle(_amountToOldLender + _amountToLender + _amountToBorrower, _settledAmount);

445:             revert LC_CannotSettle(_amountToOldLender + _amountToLender + _amountToBorrower, _settledAmount);

449:             unchecked { feesEarned = _settledAmount - _amountToOldLender - _amountToLender - _amountToBorrower; }

449:             unchecked { feesEarned = _settledAmount - _amountToOldLender - _amountToLender - _amountToBorrower; }

449:             unchecked { feesEarned = _settledAmount - _amountToOldLender - _amountToLender - _amountToBorrower; }

457:             if (protocolFee > 0) _feesWithdrawable[address(this)] += protocolFee;

458:             if (affiliateFee > 0) _feesWithdrawable[affiliate] += affiliateFee;

589:                 i++;

589:                 i++;

648:             if (data.lastAccrualTimestamp > data.startDate + data.terms.durationSecs) {

665:                     uint256(data.lastAccrualTimestamp) - uint256(data.startDate),

694:         unchecked { _feesWithdrawable[msg.sender] -= amount; }

752:                 i++;

752:                 i++;

793:         amountFromPayer = _paymentToPrincipal + _interestAmount;

801:         unchecked { feesEarned = amountFromPayer - _amountToLender; }

808:         if (protocolFee > 0) _feesWithdrawable[address(this)] += protocolFee;

809:         if (affiliateFee > 0) _feesWithdrawable[affiliate] += affiliateFee;

819:         loans[loanId].interestAmountPaid += _interestAmount;

820:         loans[loanId].balance -= _paymentToPrincipal;

846:         affiliateFee = amount * split.splitBps / Constants.BASIS_POINTS_DENOMINATOR;

846:         affiliateFee = amount * split.splitBps / Constants.BASIS_POINTS_DENOMINATOR;

847:         unchecked { protocolFee = amount - affiliateFee; }

868:         if (_nonceUses + 1 == maxUses) {

878:             numNonceUses[user][nonce]++;

878:             numNonceUses[user][nonce]++;
File: RepaymentController.sol

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

7: import "./interfaces/IRepaymentController.sol";

7: import "./interfaces/IRepaymentController.sol";

8: import "./interfaces/IPromissoryNote.sol";

8: import "./interfaces/IPromissoryNote.sol";

9: import "./interfaces/ILoanCore.sol";

9: import "./interfaces/ILoanCore.sol";

10: import "./interfaces/IFeeController.sol";

10: import "./interfaces/IFeeController.sol";

12: import "./libraries/InterestCalculator.sol";

12: import "./libraries/InterestCalculator.sol";

13: import "./libraries/FeeLookups.sol";

13: import "./libraries/FeeLookups.sol";

14: import "./libraries/LoanLibrary.sol";

14: import "./libraries/LoanLibrary.sol";

15: import "./libraries/Constants.sol";

15: import "./libraries/Constants.sol";

22: } from "./errors/Lending.sol";

22: } from "./errors/Lending.sol";

161:         uint256 totalOwed = terms.principal + interest + data.interestAmountPaid;

161:         uint256 totalOwed = terms.principal + interest + data.interestAmountPaid;

163:         uint256 claimFee = (totalOwed * data.feeSnapshot.lenderDefaultFee) / Constants.BASIS_POINTS_DENOMINATOR;

163:         uint256 claimFee = (totalOwed * data.feeSnapshot.lenderDefaultFee) / Constants.BASIS_POINTS_DENOMINATOR;

224:         unchecked { paymentToPrincipal = amount - interestAmount; }

233:         uint256 interestFee = (interestAmount * data.feeSnapshot.lenderInterestFee) / Constants.BASIS_POINTS_DENOMINATOR;

233:         uint256 interestFee = (interestAmount * data.feeSnapshot.lenderInterestFee) / Constants.BASIS_POINTS_DENOMINATOR;

234:         uint256 principalFee = (paymentToPrincipal * data.feeSnapshot.lenderPrincipalFee) / Constants.BASIS_POINTS_DENOMINATOR;

234:         uint256 principalFee = (paymentToPrincipal * data.feeSnapshot.lenderPrincipalFee) / Constants.BASIS_POINTS_DENOMINATOR;

237:         uint256 amountFromBorrower = paymentToPrincipal + interestAmount;

239:         amountToLender = amountFromBorrower - interestFee - principalFee;

239:         amountToLender = amountFromBorrower - interestFee - principalFee;
File: libraries/InterestCalculator.sol

5: import "./Constants.sol";

47:         uint256 timeSinceStart = currentTimestamp - loanStartTime;

53:             uint256 endTimestamp = loanStartTime + loanDuration;

60:             timeSinceLastPayment = endTimestamp - lastAccrualTimestamp;

62:             timeSinceLastPayment = currentTimestamp - lastAccrualTimestamp;

65:         interestAmountDue = balance * timeSinceLastPayment * interestRate

65:         interestAmountDue = balance * timeSinceLastPayment * interestRate

66:             / (Constants.BASIS_POINTS_DENOMINATOR * Constants.SECONDS_IN_YEAR);

66:             / (Constants.BASIS_POINTS_DENOMINATOR * Constants.SECONDS_IN_YEAR);

85:         return (totalInterestAmountPaid * Constants.SECONDS_IN_YEAR * Constants.BASIS_POINTS_DENOMINATOR)

85:         return (totalInterestAmountPaid * Constants.SECONDS_IN_YEAR * Constants.BASIS_POINTS_DENOMINATOR)

86:             / (totalTimeElapsed * loanPrincipal);

86:             / (totalTimeElapsed * loanPrincipal);

124:             totalInterestAmountPaid + interestAmountDue,

125:             currentTimestamp - loanStartTime,
File: libraries/OriginationLibrary.sol

5: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

5: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

5: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

5: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

5: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

6: import "@openzeppelin/contracts/interfaces/IERC1271.sol";

6: import "@openzeppelin/contracts/interfaces/IERC1271.sol";

6: import "@openzeppelin/contracts/interfaces/IERC1271.sol";

8: import "../libraries/LoanLibrary.sol";

8: import "../libraries/LoanLibrary.sol";

10: import "../interfaces/IOriginationController.sol";

10: import "../interfaces/IOriginationController.sol";

114:                 i++;

114:                 i++;
File: origination/OriginationCalculator.sol

5: import "../interfaces/IFeeController.sol";

5: import "../interfaces/IFeeController.sol";

7: import "../libraries/LoanLibrary.sol";

7: import "../libraries/LoanLibrary.sol";

8: import "../libraries/OriginationLibrary.sol";

8: import "../libraries/OriginationLibrary.sol";

9: import "../libraries/Constants.sol";

9: import "../libraries/Constants.sol";

10: import "../libraries/InterestCalculator.sol";

10: import "../libraries/InterestCalculator.sol";

50:                 borrowerOwedForNewLoan = newPrincipalAmount - borrowerFee;

51:                 amounts.amountFromLender = newPrincipalAmount + lenderFee;

55:                 amounts.amountFromLender += interestFee;

63:         uint256 repayAmount = oldBalance + oldInterestAmount;

70:                 amounts.needFromBorrower = repayAmount - borrowerOwedForNewLoan;

76:                     amounts.leftoverPrincipal = amounts.amountFromLender - repayAmount;

81:             amounts.leftoverPrincipal = amounts.amountFromLender - repayAmount;

85:                 amounts.amountToBorrower = borrowerOwedForNewLoan - repayAmount;

92:             amounts.amountToOldLender = repayAmount - interestFee;

105:                     amounts.amountToLender = repayAmount - amounts.amountFromLender;

145:         uint256 interestFee = (interest * oldLoanData.feeSnapshot.lenderInterestFee)

146:             / Constants.BASIS_POINTS_DENOMINATOR;
File: origination/OriginationConfiguration.sol

5: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

5: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

5: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

7: import "../libraries/OriginationLibrary.sol";

7: import "../libraries/OriginationLibrary.sol";

8: import "../libraries/Constants.sol";

8: import "../libraries/Constants.sol";

10: import "../interfaces/IOriginationConfiguration.sol";

10: import "../interfaces/IOriginationConfiguration.sol";

24: } from "../errors/Lending.sol";

24: } from "../errors/Lending.sol";

153:                 i++;

153:                 i++;

183:                 i++;

183:                 i++;

213:                 i++;

213:                 i++;
File: origination/OriginationController.sol

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

6: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

6: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

6: import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";

7: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

7: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

7: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

7: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

7: import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol";

8: import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";

8: import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";

8: import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";

8: import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";

9: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

9: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

9: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

11: import "./OriginationCalculator.sol";

13: import "../interfaces/IOriginationController.sol";

13: import "../interfaces/IOriginationController.sol";

14: import "../interfaces/IOriginationConfiguration.sol";

14: import "../interfaces/IOriginationConfiguration.sol";

15: import "../interfaces/ILoanCore.sol";

15: import "../interfaces/ILoanCore.sol";

16: import "../interfaces/ISignatureVerifier.sol";

16: import "../interfaces/ISignatureVerifier.sol";

17: import "../interfaces/IFeeController.sol";

17: import "../interfaces/IFeeController.sol";

18: import "../interfaces/IExpressBorrow.sol";

18: import "../interfaces/IExpressBorrow.sol";

20: import "../libraries/OriginationLibrary.sol";

20: import "../libraries/OriginationLibrary.sol";

21: import "../libraries/FeeLookups.sol";

21: import "../libraries/FeeLookups.sol";

22: import "../libraries/Constants.sol";

22: import "../libraries/Constants.sol";

24: import "../verifiers/ArcadeItemsVerifier.sol";

24: import "../verifiers/ArcadeItemsVerifier.sol";

38: } from "../errors/Lending.sol";

38: } from "../errors/Lending.sol";

496:                 i++;

496:                 i++;

524:         uint256 amountFromLender = loanTerms.principal + lenderFee;

525:         uint256 amountToBorrower = loanTerms.principal - borrowerFee;

528:         uint256 feesEarned = borrowerFee + lenderFee;

590:             settledAmount += amounts.amountFromLender;

595:             settledAmount += amounts.leftoverPrincipal;

601:             settledAmount += amounts.needFromBorrower;
File: origination/OriginationControllerMigrate.sol

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

6: import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";

6: import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";

6: import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";

6: import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";

6: import "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";

8: import "./OriginationController.sol";

10: import "../interfaces/IMigrationBase.sol";

10: import "../interfaces/IMigrationBase.sol";

12: import "../v3/interfaces/ILoanCoreV3.sol";

12: import "../v3/interfaces/ILoanCoreV3.sol";

12: import "../v3/interfaces/ILoanCoreV3.sol";

13: import "../v3/interfaces/IRepaymentControllerV3.sol";

13: import "../v3/interfaces/IRepaymentControllerV3.sol";

13: import "../v3/interfaces/IRepaymentControllerV3.sol";

14: import "../v3/libraries/LoanLibraryV3.sol";

14: import "../v3/libraries/LoanLibraryV3.sol";

14: import "../v3/libraries/LoanLibraryV3.sol";

28: } from "../errors/Lending.sol";

28: } from "../errors/Lending.sol";

215:         feesEarned = borrowerFee + lenderFee;

276:         repayAmount = oldLoanData.terms.principal + interest;

394:             opData.migrationAmounts.amountFromLender - opData.migrationAmounts.leftoverPrincipal + premiums[0]

394:             opData.migrationAmounts.amountFromLender - opData.migrationAmounts.leftoverPrincipal + premiums[0]

404:         asset.safeTransfer(VAULT, amounts[0] + premiums[0]);
File: origination/RefinanceController.sol

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

5: import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

6: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

6: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

6: import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

8: import "./OriginationCalculator.sol";

10: import "../libraries/OriginationLibrary.sol";

10: import "../libraries/OriginationLibrary.sol";

11: import "../libraries/LoanLibrary.sol";

11: import "../libraries/LoanLibrary.sol";

12: import "../libraries/Constants.sol";

12: import "../libraries/Constants.sol";

14: import "../interfaces/IRefinanceController.sol";

14: import "../interfaces/IRefinanceController.sol";

15: import "../interfaces/IOriginationConfiguration.sol";

15: import "../interfaces/IOriginationConfiguration.sol";

16: import "../interfaces/ILoanCore.sol";

16: import "../interfaces/ILoanCore.sol";

17: import "../interfaces/IFeeController.sol";

17: import "../interfaces/IFeeController.sol";

30: } from "../errors/Lending.sol";

30: } from "../errors/Lending.sol";

44:     uint256 public constant MINIMUM_INTEREST_CHANGE = 1000; // 10%

44:     uint256 public constant MINIMUM_INTEREST_CHANGE = 1000; // 10%

109:         if (block.timestamp < oldLoanData.startDate + 2 days) revert REFI_TooEarly(oldLoanData.startDate + 2 days);

109:         if (block.timestamp < oldLoanData.startDate + 2 days) revert REFI_TooEarly(oldLoanData.startDate + 2 days);

113:             oldLoanData.terms.interestRate * (Constants.BASIS_POINTS_DENOMINATOR - MINIMUM_INTEREST_CHANGE);

113:             oldLoanData.terms.interestRate * (Constants.BASIS_POINTS_DENOMINATOR - MINIMUM_INTEREST_CHANGE);

116:             newTerms.interestRate * Constants.BASIS_POINTS_DENOMINATOR > aprMinimumScaled

120:         uint256 oldDueDate = oldLoanData.startDate + oldLoanData.terms.durationSecs;

121:         uint256 newDueDate = block.timestamp + newTerms.durationSecs;

191:         uint256 newLenderOwes = amounts.amountFromLender + amounts.needFromBorrower;

[GAS-4] Don't initialize variables with default value

Instances (8):

File: LoanCore.sol

574:         for (uint256 i = 0; i < noteCount;) {

739:         for (uint256 i = 0; i < codes.length;) {
File: libraries/OriginationLibrary.sol

102:         for (uint i = 0; i < predicates.length;){
File: origination/OriginationCalculator.sol

46:         uint256 borrowerOwedForNewLoan = 0;
File: origination/OriginationConfiguration.sol

145:         for (uint256 i = 0; i < verifiers.length;) {

175:         for (uint256 i = 0; i < tokens.length;) {

205:         for (uint256 i = 0; i < tokens.length;) {
File: origination/OriginationController.sol

471:         for (uint256 i = 0; i < itemPredicates.length;) {

[GAS-5] Functions guaranteed to revert when called by normal users can be marked payable

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided.

Instances (3):

File: LoanCore.sol

708:     function withdrawProtocolFees(address token, address to) external override nonReentrant onlyRole(FEE_CLAIMER_ROLE) {

763:     function shutdown() external onlyRole(SHUTDOWN_ROLE) {
File: origination/OriginationControllerMigrate.sol

472:     function pause(bool _pause) external override onlyRole(MIGRATION_MANAGER_ROLE) {

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

Saves 5 gas per loop

Instances (8):

File: LoanCore.sol

589:                 i++;

752:                 i++;

878:             numNonceUses[user][nonce]++;
File: libraries/OriginationLibrary.sol

114:                 i++;
File: origination/OriginationConfiguration.sol

153:                 i++;

183:                 i++;

213:                 i++;
File: origination/OriginationController.sol

496:                 i++;

[GAS-7] 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 (18):

File: LoanCore.sol

67:     bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");

68:     bytes32 public constant ORIGINATOR_ROLE = keccak256("ORIGINATOR");

69:     bytes32 public constant REPAYER_ROLE = keccak256("REPAYER");

70:     bytes32 public constant AFFILIATE_MANAGER_ROLE = keccak256("AFFILIATE_MANAGER");

71:     bytes32 public constant FEE_CLAIMER_ROLE = keccak256("FEE_CLAIMER");

72:     bytes32 public constant SHUTDOWN_ROLE = keccak256("SHUTDOWN");
File: libraries/OriginationLibrary.sol

47:     bytes32 public constant _TOKEN_ID_TYPEHASH =

54:     bytes32 public constant _LOAN_TERMS_TYPEHASH =

61:     bytes32 public constant _ITEMS_TYPEHASH =

68:     bytes32 public constant _LOAN_TERMS_WITH_ITEMS_TYPEHASH =

75:     bytes32 public constant _PREDICATE_TYPEHASH =

81:     bytes32 public constant _SIG_PROPERTIES_TYPEHASH =
File: origination/OriginationConfiguration.sol

29:     bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");

30:     bytes32 public constant WHITELIST_MANAGER_ROLE = keccak256("WHITELIST_MANAGER");

32:     uint256 public constant MAX_LENGTH = 50;
File: origination/OriginationController.sol

73:     bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");

74:     bytes32 public constant MIGRATION_MANAGER_ROLE = keccak256("MIGRATION_MANAGER");
File: origination/RefinanceController.sol

44:     uint256 public constant MINIMUM_INTEREST_CHANGE = 1000; // 10%

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

Instances (27):

File: LoanCore.sol

179:         if (protocolFee > 0) feesWithdrawable[terms.payableCurrency][address(this)] += protocolFee;

180:         if (affiliateFee > 0) feesWithdrawable[terms.payableCurrency][affiliate] += affiliateFee;

339:         if (_amountFromLender > 0) {

345:             if (protocolFee > 0) _feesWithdrawable[address(this)] += protocolFee;

346:             if (affiliateFee > 0) _feesWithdrawable[affiliate] += affiliateFee;

457:             if (protocolFee > 0) _feesWithdrawable[address(this)] += protocolFee;

458:             if (affiliateFee > 0) _feesWithdrawable[affiliate] += affiliateFee;

808:         if (protocolFee > 0) _feesWithdrawable[address(this)] += protocolFee;

809:         if (affiliateFee > 0) _feesWithdrawable[affiliate] += affiliateFee;

924:         if (amount > 0) token.safeTransfer(to, amount);

939:         if (amount > 0) token.safeTransferFrom(from, address(this), amount);
File: libraries/OriginationLibrary.sol

269:         if (sig.extraData.length > 0) {
File: origination/OriginationCalculator.sol

47:         if (borrowerFee > 0 || lenderFee > 0 || interestFee > 0) {

47:         if (borrowerFee > 0 || lenderFee > 0 || interestFee > 0) {

47:         if (borrowerFee > 0 || lenderFee > 0 || interestFee > 0) {

103:             if (amounts.needFromBorrower > 0 && repayAmount > amounts.amountFromLender) {
File: origination/OriginationController.sol

170:         if (itemPredicates.length > 0) _runPredicatesCheck(borrowerData.borrower, lender, loanTerms, itemPredicates);

226:         if (itemPredicates.length > 0) _runPredicatesCheck(borrower, lender, loanTerms, itemPredicates);

368:         if (itemPredicates.length > 0) {

538:         if (borrowerData.callbackData.length > 0) {

591:         } else if (amounts.leftoverPrincipal > 0) {

598:         if (amounts.needFromBorrower > 0) {
File: origination/OriginationControllerMigrate.sol

118:             if (amounts.amountToBorrower > 0) {

129:         if (itemPredicates.length > 0) _runPredicatesCheck(msg.sender, lender, newTerms, itemPredicates);

236:             if (amounts.leftoverPrincipal > 0) {

241:         if (amounts.needFromBorrower > 0) {

397:         if (opData.migrationAmounts.amountToBorrower > 0) {

Low Issues

Issue Instances
L-1 abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() 1
L-2 Do not use deprecated library functions 8
L-3 Empty Function Body - Consider commenting why 1
L-4 Initializers could be front-run 2
L-5 Unsafe ERC20 operation(s) 3

[L-1] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

Use abi.encode() instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456) => 0x123456 => abi.encodePacked(0x1,0x23456), but abi.encode(0x123,0x456) => 0x0...1230...456). "Unless there is a compelling reason, abi.encode should be preferred". If there is only one argument to abi.encodePacked() it can often be cast to bytes() or bytes32() instead. If all arguments are strings and or bytes, bytes.concat() should be used instead

Instances (1):

File: libraries/OriginationLibrary.sol

119:         itemsHash = keccak256(abi.encodePacked(itemHashes));

[L-2] Do not use deprecated library functions

Instances (8):

File: LoanCore.sol

126:         _setupRole(ADMIN_ROLE, msg.sender);
File: origination/OriginationConfiguration.sol

46:         _setupRole(ADMIN_ROLE, msg.sender);

49:         _setupRole(WHITELIST_MANAGER_ROLE, msg.sender);
File: origination/OriginationController.sol

109:         _setupRole(ADMIN_ROLE, msg.sender);

112:         _setupRole(MIGRATION_MANAGER_ROLE, msg.sender);

605:         payableCurrency.safeApprove(address(loanCore), settledAmount);
File: origination/OriginationControllerMigrate.sol

426:         payableCurrency.safeApprove(loanCoreV3, repayAmount);
File: origination/RefinanceController.sol

195:         payableCurrency.safeApprove(address(loanCore), newLenderOwes);

[L-3] Empty Function Body - Consider commenting why

Instances (1):

File: origination/OriginationControllerMigrate.sol

53:     ) OriginationController(_originationConfiguration, _loanCore, _feeController) {}

[L-4] Initializers could be front-run

Initializers could be front-run, allowing an attacker to either set their own values, take ownership of the contract, and in the best case forcing a re-deployment

Instances (2):

File: origination/OriginationController.sol

166:         loanId = _initialize(loanTerms, borrowerData, lender);

511:     function _initialize(

[L-5] Unsafe ERC20 operation(s)

Instances (3):

File: origination/OriginationController.sol

544:         IERC721(loanTerms.collateralAddress).transferFrom(borrowerData.borrower, address(loanCore), loanTerms.collateralId);
File: origination/OriginationControllerMigrate.sol

423:         IPromissoryNote(borrowerNoteV3).transferFrom(_borrower, address(this), borrowerNoteId);

451:         IERC721(newTerms.collateralAddress).transferFrom(address(this), address(loanCore), newTerms.collateralId);

Medium Issues

Issue Instances
M-1 Centralization Risk for trusted owners 16

[M-1] Centralization Risk for trusted owners

Impact:

Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds.

Instances (16):

File: LoanCore.sol

55:     AccessControlEnumerable,

167:     ) external override whenNotPaused onlyRole(ORIGINATOR_ROLE) nonReentrant returns (uint256 loanId) {

222:     ) external override onlyRole(REPAYER_ROLE) nonReentrant {

270:     ) external override onlyRole(REPAYER_ROLE) nonReentrant {

321:         onlyRole(REPAYER_ROLE)

374:     ) external override onlyRole(REPAYER_ROLE) nonReentrant {

430:     ) external override whenNotPaused onlyRole(ORIGINATOR_ROLE) nonReentrant returns (uint256 newLoanId) {

507:     ) external override whenNotPaused onlyRole(ORIGINATOR_ROLE) {

708:     function withdrawProtocolFees(address token, address to) external override nonReentrant onlyRole(FEE_CLAIMER_ROLE) {

736:     ) external override whenNotPaused onlyRole(AFFILIATE_MANAGER_ROLE) nonReentrant {

763:     function shutdown() external onlyRole(SHUTDOWN_ROLE) {
File: origination/OriginationConfiguration.sol

26: contract OriginationConfiguration is IOriginationConfiguration, AccessControlEnumerable {

140:     ) external override onlyRole(WHITELIST_MANAGER_ROLE) {

170:     ) external override onlyRole(WHITELIST_MANAGER_ROLE) {

200:     ) external override onlyRole(WHITELIST_MANAGER_ROLE) {
File: origination/OriginationControllerMigrate.sol

472:     function pause(bool _pause) external override onlyRole(MIGRATION_MANAGER_ROLE) {
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment