code-423n4 / 2022-06-putty-findings

5 stars 0 forks source link

Gas Optimizations #320

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

1. !=0 instead of >0 for UINT

0 is less efficient than != 0 for unsigned integers (with proof) != 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas) Proof: While it may seem that > 0 is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer at 10k AND you’re in a require statement, this will save gas. You can see this tweet for more proofs: https://twitter.com/gzeon/status/1485428085885640706

Instances

PuttyV2.sol #L293 PuttyV2.sol #L598 PuttyV2.sol #L599

PuttyV2.sol:293:        require(order.baseAsset.code.length > 0, "baseAsset is not contract");
PuttyV2.sol:598:            require(token.code.length > 0, "ERC20: Token is not contract");
PuttyV2.sol:599:            require(tokenAmount > 0, "ERC20: Amount too small");

Reference:

https://twitter.com/gzeon/status/1485428085885640706

Remediation:

I suggest changing > 0 with != 0. Also, please enable the Optimizer.



2. ++i costs less gas than i++, especially when it’s used in for-loops (--i/i-- too)

Saves 6 gas PER LOOP

Instances:

PuttyV2.sol #L556 PuttyV2.sol #L594 PuttyV2.sol #L611 PuttyV2.sol #L627 PuttyV2.sol #L637 PuttyV2.sol #L647 PuttyV2.sol #L658 PuttyV2.sol #L670 PuttyV2.sol #L728 PuttyV2.sol #L742

PuttyV2.sol:556:        for (uint256 i = 0; i < orders.length; i++) {
PuttyV2.sol:594:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:611:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:627:        for (uint256 i = 0; i < floorTokens.length; i++) {
PuttyV2.sol:637:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:647:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:658:        for (uint256 i = 0; i < floorTokens.length; i++) {
PuttyV2.sol:670:        for (uint256 i = 0; i < whitelist.length; i++) {
PuttyV2.sol:728:        for (uint256 i = 0; i < arr.length; i++) {
PuttyV2.sol:742:        for (uint256 i = 0; i < arr.length; i++) {

Reference:

https://code4rena.com/reports/2022-05-cally#g-19-i-costs-less-gas-than-i-especially-when-its-used-in-for-loops---ii---too



3. Variables: No need to explicitly initialize variables with default values

If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address…). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

We can use uint number; instead of uint number = 0;

Instance Includes:

PuttyV2.sol #L497

PuttyV2.sol: 497:    uint256 feeAmount = 0;

Recommendation:

I suggest removing explicit initializations for default values.



4. An array’s length should be cached to save gas in for-loops

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack. Caching the array length in the stack saves around 3 gas per iteration.

Instances:

PuttyV2.sol #L556 PuttyV2.sol #L594 PuttyV2.sol #L611 PuttyV2.sol #L627 PuttyV2.sol #L637 PuttyV2.sol #L647 PuttyV2.sol #L658 PuttyV2.sol #L670 PuttyV2.sol #L728 PuttyV2.sol #L742

PuttyV2.sol:556:        for (uint256 i = 0; i < orders.length; i++) {
PuttyV2.sol:594:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:611:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:627:        for (uint256 i = 0; i < floorTokens.length; i++) {
PuttyV2.sol:637:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:647:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:658:        for (uint256 i = 0; i < floorTokens.length; i++) {
PuttyV2.sol:670:        for (uint256 i = 0; i < whitelist.length; i++) {
PuttyV2.sol:728:        for (uint256 i = 0; i < arr.length; i++) {
PuttyV2.sol:742:        for (uint256 i = 0; i < arr.length; i++) {

Remediation:

Here, I suggest storing the array’s length in a variable before the for-loop, and use it instead.



5. ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas PER LOOP

Instances:

PuttyV2.sol #L556 PuttyV2.sol #L594 PuttyV2.sol #L611 PuttyV2.sol #L627 PuttyV2.sol #L637 PuttyV2.sol #L647 PuttyV2.sol #L658 PuttyV2.sol #L670 PuttyV2.sol #L728 PuttyV2.sol #L742

PuttyV2.sol:556:        for (uint256 i = 0; i < orders.length; i++) {
PuttyV2.sol:594:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:611:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:627:        for (uint256 i = 0; i < floorTokens.length; i++) {
PuttyV2.sol:637:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:647:        for (uint256 i = 0; i < assets.length; i++) {
PuttyV2.sol:658:        for (uint256 i = 0; i < floorTokens.length; i++) {
PuttyV2.sol:670:        for (uint256 i = 0; i < whitelist.length; i++) {
PuttyV2.sol:728:        for (uint256 i = 0; i < arr.length; i++) {
PuttyV2.sol:742:        for (uint256 i = 0; i < arr.length; i++) {

Reference:

https://code4rena.com/reports/2022-05-cally#g-11-ii-should-be-uncheckediuncheckedi-when-it-is-not-possible-for-them-to-overflow-as-is-the-case-when-used-in-for--and-while-loops