Nested Finance contest - ye0lde's results

Nested Finance Gas Optimization Report

Unless otherwise noted, manual auditing and testing were done using Visual Studio Code and Remix. The sponsor-provided test suite was used to verify the findings.

The audit was done from February 10-12, 2022 by ye0lde through code4rena.

• Nested Finance Gas Optimization Report
• Findings
  • G-1 - Function store can be more efficient (NestedRecords.sol)
  • G-2 - Save gas and retain code clarity with the unchecked keyword (NestedFactory.sol)

Findings

G-1 - Function store can be more efficient (NestedRecords.sol)

Impact

Caching the references to records[_nftId] in the store function will decrease gas usage as store is called frequently from NestedFactory.sol

Below are the relevant numbers from the sponsor's test suite before and after the change:

Proof of Concept

The store function is here: https://github.com/code-423n4/2022-02-nested/blob/fe6f9ef7783c3c84798c8ab5fc58085a55cebcfc/contracts/NestedRecords.sol#L111-L132

    function store(
        uint256 _nftId,
        address _token,
        uint256 _amount,
        address _reserve
    ) external onlyFactory {
        uint256 amount = records[_nftId].holdings[_token];
        if (amount != 0) {
            require(records[_nftId].reserve == _reserve, "NRC: RESERVE_MISMATCH");
            updateHoldingAmount(_nftId, _token, amount + _amount);
            return;
        }
        require(records[_nftId].tokens.length < maxHoldingsCount, "NRC: TOO_MANY_TOKENS");
        require(
            _reserve != address(0) && (_reserve == records[_nftId].reserve || records[_nftId].reserve == address(0)),
            "NRC: INVALID_RESERVE"
        );


        records[_nftId].holdings[_token] = _amount;
        records[_nftId].tokens.push(_token);
        records[_nftId].reserve = _reserve;
    }

Recommended Mitigation Steps

I suggest the following changes:

    function store(
        uint256 _nftId,
        address _token,
        uint256 _amount,
        address _reserve
    ) external onlyFactory { 
        NftRecord storage nftRecord = records[_nftId];
        uint256 amount = nftRecord.holdings[_token];
        if (amount != 0) {
            require(nftRecord.reserve == _reserve, "NRC: RESERVE_MISMATCH");
            updateHoldingAmount(_nftId, _token, amount + _amount);
            return;
        }
        require(nftRecord.tokens.length < maxHoldingsCount, "NRC: TOO_MANY_TOKENS");
        require(
            _reserve != address(0) && (_reserve == nftRecord.reserve || nftRecord.reserve == address(0)),
            "NRC: INVALID_RESERVE"
        );

        nftRecord.holdings[_token] = _amount;
        nftRecord.tokens.push(_token);
        nftRecord.reserve = _reserve;
    }

---

G-2 - Save gas and retain code clarity with the unchecked keyword (NestedFactory.sol)

Impact

In a previous Code4rena audit, various "unchecked" optimizations were suggested. Some of which were implemented and some were not because the sponsor's focus was code clarity over optimization.

I believe this suggestion meets the requirements for both optimization and clarity. Below are the relevant numbers from the sponsor's test suite before and after the change:

Proof of Concept

The code that can be unchecked is here: https://github.com/code-423n4/2022-02-nested/blob/fe6f9ef7783c3c84798c8ab5fc58085a55cebcfc/contracts/NestedFactory.sol#L339-L347

The "unchecked" keyword can be applied here since there is a require statement at #337 that ensures the arithmetic operations would not cause an integer underflow or overflow.

uint256 underSpentAmount = _inputTokenAmount - feesAmount - amountSpent;
 if (underSpentAmount != 0) {
     tokenSold.safeTransfer(_fromReserve ? address(reserve) : _msgSender(), underSpentAmount);
 }

  // If input is from the reserve, update the records
  if (_fromReserve) {
      _decreaseHoldingAmount(_nftId, address(tokenSold), _inputTokenAmount - underSpentAmount);
  }

Recommended Mitigation Steps

Add unchecked around #L339-L347 as shown below.

require(amountSpent <= _inputTokenAmount - feesAmount, "NF: OVERSPENT");

unchecked {
    uint256 underSpentAmount = _inputTokenAmount - feesAmount - amountSpent; 
    if (underSpentAmount != 0) {
        tokenSold.safeTransfer(_fromReserve ? address(reserve) : _msgSender(), underSpentAmount);
    }

    // If input is from the reserve, update the records
    if (_fromReserve) {
        _decreaseHoldingAmount(_nftId, address(tokenSold), _inputTokenAmount - underSpentAmount);
    }
}

---