Gas Optimizations

[code423n4]

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:

Function	Before (AVG)	After (AVG)
create	701747	701219
processInputAndOutputOrders	871716	871184
processInputOrders	406972	406779
processOutputOrders	449829	449712
store	83927	83684

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:

Function	Before (AVG)	After (AVG)
create	701747	701623
processInputAndOutputOrders	871716	871468
processInputOrders	406972	406784

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);
    }
}

---

[maximebrugel]

"Function store can be more efficient (NestedRecords.sol)" (Confirmed)

701781 to 701247 => -534

"Save gas and retain code clarity with the unchecked keyword (NestedFactory.sol)" (Confirmed)

[harleythedogC4]

My personal judgments:

1. "Function store can be more efficient (NestedRecords.sol)". Valid and small-optimization.
2. "unchecked keyword". Although the sponsor confirms here, this was disputed in other gas optimization reports since it already has been raised in the previous audits. To be fair, it should be invalid here too. Invalid.

[harleythedogC4]

Now, here is the methodology I used for calculating a score for each gas report. I first assigned each submission to be either small-optimization (1 point), medium-optimization (5 points) or large-optimization (10 points), depending on how useful the optimization is. The score of a gas report is the sum of these points, divided by the maximum number of points achieved by a gas report. This maximum number was 10 points, achieved by #67.

The number of points achieved by this report is 1 points. Thus the final score of this gas report is (1/10)*100 = 10.