Adding unchecked directive can save gas

[code423n4]

Handle

WatchPug

Vulnerability details

For the arithmetic operations that will never over/underflow, using the unchecked directive (Solidity v0.8 has default overflow/underflow checks) can save some gas from the unnecessary internal over/underflow checks.

For example:

https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/NestedFactory.sol#L285-L309

function _submitInOrders(
    uint256 _nftId,
    IERC20 _inputToken,
    uint256 _inputTokenAmount,
    Order[] calldata _orders,
    bool _reserved,
    bool _fromReserve
) private returns (uint256 feesAmount, IERC20 tokenSold) {
    _inputToken = _transferInputTokens(_nftId, _inputToken, _inputTokenAmount, _fromReserve);
    uint256 amountSpent;
    for (uint256 i = 0; i < _orders.length; i++) {
        amountSpent += _submitOrder(address(_inputToken), _orders[i].token, _nftId, _orders[i], _reserved);
    }
    feesAmount = _calculateFees(_msgSender(), amountSpent);
    assert(amountSpent <= _inputTokenAmount - feesAmount); // overspent

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

    _handleUnderSpending(_inputTokenAmount - feesAmount, amountSpent, _inputToken);

    tokenSold = _inputToken;
}

_inputTokenAmount - feesAmount at L306 will never underflow.

https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/FeeSplitter.sol#L166-L174

function updateShareholder(uint256 _accountIndex, uint256 _weight) external onlyOwner {
    require(_accountIndex + 1 <= shareholders.length, "FeeSplitter: INVALID_ACCOUNT_INDEX");
    uint256 _totalWeights = totalWeights;
    _totalWeights -= shareholders[_accountIndex].weight;
    shareholders[_accountIndex].weight = _weight;
    _totalWeights += _weight;
    require(_totalWeights > 0, "FeeSplitter: TOTAL_WEIGHTS_ZERO");
    totalWeights = _totalWeights;
}

_accountIndex + 1 will never overflow.

https://github.com/code-423n4/2021-11-nested/blob/f646002b692ca5fa3631acfff87dda897541cf41/contracts/NestedBuybacker.sol#L107-L113

function trigger() internal {
    uint256 balance = NST.balanceOf(address(this));
    uint256 toBurn = (balance * burnPercentage) / 1000;
    uint256 toSendToReserve = balance - toBurn;
    _burnNST(toBurn);
    NST.safeTransfer(nstReserve, toSendToReserve);
}

balance - toBurn will never underflow.

[maximebrugel]

We will use unchecked blocks when the code remains readable (since it can't be used as an expression).

• _inputTokenAmount - feesAmount at L306 will never underflow.
  Acknowledge : Won't be easy to read in this function, its better to keep _inputTokenAmount - feesAmount in the assert.
• _accountIndex + 1
  Duplicated : This will change in index + 1 can be simplified #207
• balance - toBurn
  Acknowledge : No need to add this kind of optimization for an owner function