AbraNFT contest - z3s's results

Non Critical

[N01] Typos in comments:

change excesss to excess:

NFTPair.sol
  90,56:     // Track assets we own. Used to allow skimming the excesss.

NFTPairWithOracle.sol
  107,56:     // Track assets we own. Used to allow skimming the excesss.

change calculateIntest to calculateInterest:

NFTPair.sol
  114,9:     // `calculateIntest`.

NFTPairWithOracle.sol
  131,9:     // `calculateIntest`.

change initialised to initialized:

NFTPair.sol
  168,39:     /// @notice Subsequent clones are initialised via `init`.

NFTPairWithOracle.sol
  185,39:     /// @notice Subsequent clones are initialised via `init`.

change transfered to transferred:

NFTPair.sol
  233,56:     /// @param skim True if the token has already been transfered
  320,49:     /// @param skim True if the funds have been transfered to the contract
  351,71:     /// @param skimCollateral True if the collateral has already been transfered
  394,54:     /// @param skimFunds True if the funds have been transfered to the contract

NFTPairWithOracle.sol
  253,56:     /// @param skim True if the token has already been transfered
  355,49:     /// @param skim True if the funds have been transfered to the contract
  386,71:     /// @param skimCollateral True if the collateral has already been transfered
  428,54:     /// @param skimFunds True if the funds have been transfered to the contract

change commited to committed:

NFTPair.sol
  389,44:     /// @notice Take collateral from a pre-commited borrower and lend against it

NFTPairWithOracle.sol
  423,44:     /// @notice Take collateral from a pre-commited borrower and lend against it

change inquality to inequality:

NFTPair.sol
  434,22:     /// of the above inquality) fits in 128 bits, then the function is

NFTPairWithOracle.sol
  467,22:     /// of the above inquality) fits in 128 bits, then the function is

Gas Optimizations

[G01] In updateLoanParams() change memory to storage:

gas savings:

• 28328 in NFTPair Deployments
• 901 in updateLoanParams()

Recommended Mitigation Steps:

NFTPair.sol:182
    - TokenLoan memory loan = tokenLoan[tokenId];
    + TokenLoan storage loan = tokenLoan[tokenId];

NFTPair.sol:187
    - TokenLoanParams memory cur = tokenLoanParams[tokenId];
    + TokenLoanParams storage cur = tokenLoanParams[tokenId];

Do same in NFTPairWithOracle.sol.

[G02] In removeCollateral() change memory to storage then cache load.status:

gas savings:

• 14068 in NFTPair Deployments
• 284 in removeCollateral()

Recommended Mitigation Steps:

change:

function removeCollateral(uint256 tokenId, address to) public {
    TokenLoan memory loan = tokenLoan[tokenId];
    if (loan.status == LOAN_REQUESTED) {
        // We are withdrawing collateral that is not in use:
        require(msg.sender == loan.borrower, "NFTPair: not the borrower");
    } else if (loan.status == LOAN_OUTSTANDING) {
        // We are seizing collateral as the lender. The loan has to be
        // expired and not paid off:
        require(to == loan.lender, "NFTPair: not the lender");
        require(
            // Addition is safe: both summands are smaller than 256 bits
            uint256(loan.startTime) + tokenLoanParams[tokenId].duration <= block.timestamp,
            "NFTPair: not expired"
        );
    }
    // If there somehow is collateral but no accompanying loan, then anyone
    // can claim it by first requesting a loan with `skim` set to true, and
    // then withdrawing. So we might as well allow it here..
    delete tokenLoan[tokenId];
    collateral.transferFrom(address(this), to, tokenId);
    emit LogRemoveCollateral(tokenId, to);
}

to:

function removeCollateral(uint256 tokenId, address to) public {
    TokenLoan storage loan = tokenLoan[tokenId];
    uint8 loanStatus = loan.status;
    if (loanStatus == LOAN_REQUESTED) {
        // We are withdrawing collateral that is not in use:
        require(msg.sender == loan.borrower, "NFTPair: not the borrower");
    } else if (loanStatus == LOAN_OUTSTANDING) {
        // We are seizing collateral as the lender. The loan has to be
        // expired and not paid off:
        require(to == loan.lender, "NFTPair: not the lender");
        require(
            // Addition is safe: both summands are smaller than 256 bits
            uint256(loan.startTime) + tokenLoanParams[tokenId].duration <= block.timestamp,
            "NFTPair: not expired"
        );
    }
    // If there somehow is collateral but no accompanying loan, then anyone
    // can claim it by first requesting a loan with `skim` set to true, and
    // then withdrawing. So we might as well allow it here..
    delete tokenLoan[tokenId];
    collateral.transferFrom(address(this), to, tokenId);
    emit LogRemoveCollateral(tokenId, to);
}

Do same in NFTPairWithOracle.sol.

[G03] In _lend() save SLOAD by caching asset:

gas savings:

• 1320 in NFTPair Deployments
• 205 in lend()
• 210 in requestAndBorrow()
• 204 in takeCollateralAndLend()

Recommended Mitigation Steps:

+ IERC20 assetCache = asset;

change assets with assetCache

Do same in NFTPairWithOracle.sol.

[G04] In repay() save SLOAD by caching asset:

gas savings:

• 420 in NFTPair Deployments
• 304 in repay()

Recommended Mitigation Steps:

+ IERC20 assetCache = asset;

change assets with assetCache

Do same in NFTPairWithOracle.sol.