AbraNFT contest - 0xkatana's results

[G-01] Use != 0 instead of > 0

Using > 0 uses slightly more gas than using != 0. Use != 0 when comparing uint variables to zero, which cannot hold values below zero

Locations where this was found include https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L818 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L1062 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L1106 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPair.sol#L717 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L739

Recommended Mitigation Steps

Replace > 0 with != 0 to save gas

[G-02] Use prefix not postfix in loops

Using a prefix increment (++i) instead of a postfix increment (i++) saves gas for each loop cycle and so can have a big gas impact when the loop executes on a large number of elements.

There are many examples of this https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L627 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L954 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L1009 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L1018 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPair.sol#L494 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPair.sol#L641 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L527 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L674

Recommended Mitigation Steps

Use prefix not postfix to increment in a loop

[G-03] Short require strings save gas

Strings in solidity are handled in 32 byte chunks. A require string longer than 32 bytes uses more gas. Shortening these strings will save gas.

One case of this gas optimization was found https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L398

Recommended Mitigation Steps

Shorten all require strings to less than 32 characters

[G-04] Add payable to functions that won't receive ETH

Identifying a function as payable saves gas. Functions that have the onlyOwner modifier cannot be called by normal users and will not mistakenly receive ETH. These functions can be payable to save gas.

There are many functions that have the onlyOwner modifier and can be payable transferOwnership https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L369 whitelistMasterContract https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L522 setStrategyTargetPercentage https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L1029 setStrategy https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L1048

Recommended Mitigation Steps

Add payable to these functions for gas savings

[G-05] Split up require statements instead of &&

Combining require statement conditions with && logic uses unnecessary gas. It is better to split up each part of the logical statement into a separate require statements

One example is

require(success && (data.length == 0 || abi.decode(data, (bool))), "BoringERC20: Transfer failed");

This can be improved to

require(success);
require(data.length == 0 || abi.decode(data, (bool)), "BoringERC20: Transfer failed");

Several places had require statements with many logical "and"s. Instead, split into two to save gas https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L145 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L161 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/BentoBoxFlat.sol#L1058 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPair.sol#L622 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L655

Recommended Mitigation Steps

Use separate require statements instead of concatenating with &&

[G-06] Redundant zero initialization

Solidity does not recognize null as a value, so uint variables are initialized to zero. Setting a uint variable to zero is redundant and can waste gas.

There are several places where an int is initialized to zero, which looks like

uint256 i = 0;

There are at least two places with redundant zero initialization https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPair.sol#L96 https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L113

Recommended Mitigation Steps

Remove the redundant zero initialization uint256 i;

[G-07] Use simple comparison in ternary logic

The comparison operators >= and <= use more gas than >, <, or ==. Replacing the >= and ≤ operators with a comparison operator that has an opcode in the EVM saves gas

The existing code is https://github.com/code-423n4/2022-04-abranft/blob/main/contracts/NFTPairWithOracle.sol#L607

outNum = inNum >= 0 ? uint256(inNum) : (inNum == USE_VALUE1 ? value1 : value2);

A simple comparison can be used for gas savings by reversing the logic

outNum = inNum < 0 ? (inNum == USE_VALUE1 ? value1 : value2) : uint256(inNum);

Recommended Mitigation Steps

Replace the comparison operator and reverse the logic to save gas using the suggestions above