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:

1. QuickAccManager.sol#send()

   https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/wallet/QuickAccManager.sol#L76-L76

} else {
    address signer = SignatureValidator.recoverAddr(hash, sigs.one);
    require(acc.one == signer || acc.two == signer, 'SIG');
    // no need to check whether `scheduled[hash]` is already set here cause of the incrementing nonce
    scheduled[hash] = block.timestamp + acc.timelock;
    emit LogScheduled(hash, accHash, signer, initialNonce, block.timestamp, txns);
}

block.timestamp + acc.timelock will never overflow.

2. Zapper.sol#exchangeV2()

   https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/wallet/Zapper.sol#L114-L118

} else {
    uint[] memory amounts = trade.router.swapExactTokensForTokens(trade.amountIn, trade.amountOutMin, trade.path, address(this), deadline);
    uint lastIdx = trade.path.length - 1;
    lendingPool.deposit(trade.path[lastIdx], amounts[lastIdx], to, aaveRefCode);
}

trade.path.length - 1 will never underflow.

3. SignatureValidatorV2.sol#recoverAddrImpl()

   https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/libs/SignatureValidatorV2.sol#L53-L61

if (mode == SignatureMode.SmartWallet) {
    // 32 bytes for the addr, 1 byte for the type = 33
    require(sig.length > 33, "SignatureValidator: wallet sig len");
    // @TODO: can we pack the addr tigher into 20 bytes? should we?
    IERC1271Wallet wallet = IERC1271Wallet(address(uint160(uint256(sig.readBytes32(sig.length - 33)))));
    sig.trimToSize(sig.length - 33);
    require(ERC1271_MAGICVALUE_BYTES32 == wallet.isValidSignature(hash, sig), "SignatureValidator: invalid wallet sig");
    return address(wallet);
}

sig.length - 33 will never underflow.

4. Identity.sol#execute()

   https://github.com/code-423n4/2021-10-ambire/blob/bc01af4df3f70d1629c4e22a72c19e6a814db70d/contracts/Identity.sol#L95-L95

nonce = currentNonce + 1;

currentNonce + 1 will never overflow.

[Ivshti]

might be a duplicate of https://github.com/code-423n4/2021-10-ambire-findings/issues/22 but it seems more like a superset as it's more detailed

[GalloDaSballo]

Agree with the finding, although unchecked ends up cluttering the source code

Will mark the other one as duplicate