SIZE contest - ajtra's results

Non Critical

1. NC01 - Event is missing indexed fields
2. NC02 - NatSpec incomplete
3. NC03 - Should not use magic number
4. NC04 - Not using the named return variables when a function returns in anyware is confusing
5. NC05 - The struct should be at the begining of the contract

Non Critical

NC01 - Event is missing indexed fields

Description

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (threefields). Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

Lines in the code

ISizeSealed.sol#L97-L122

NC02 - NatSpec incomplete

Description

NatSpec is incomplete in many functions. It's a best practice to complete it to a major understanding.

Lines in the code

Example in the code where is missing the return in NatSpec

ECCMath.sol#L18 ECCMath.sol#L37 ECCMath.sol#L51 ECCMath.sol#L60

NC03 - Use constant instead of magic number

Description

Replace the magic numbers for a constant that describe the meaning of it.

Lines in the code

SizeSealed.sol#L157 SizeSealed.sol#L241 SizeSealed.sol#L249 SizeSealed.sol#L251 SizeSealed.sol#L309

NC04 - Not using the named return variables when a function returns in anyware is confusing

ECCMath.sol#L51 SizeSealed.sol#L451

NC05 - The struct should be at the begining of the contract

Description

To a best understanding the struct definition should be at the begining of the contract.

Lines in the code

SizeSealed.sol#L203-L209

Index

1. I = I + (-) X is cheaper in gas cost than I += (-=) X
2. ABI.ENCODE() is less efficient than ABI.ENCODEPACKED()
3. Use unchecked in for/while loops when it's not possible to overflow
4. Calldata vs Memory
5. Multiplication/division by two should use bit shifting
6. Remove local variables that are used once

Details

1. I = I + (-) X is cheaper in gas cost than I += (-=) X

Description

In the following example (optimizer = 10000) it's possible to demostrate that I = I + X cost less gas than I += X in state variable.

contract Test_Optimization {
    uint256 a = 1;
    function Add () external returns (uint256) {
        a = a + 1;
        return a;
    }
}

contract Test_Without_Optimization {
    uint256 a = 1;
    function Add () external returns (uint256) {
        a += 1;
        return a;
    }
}

• Test_Optimization cost is 26324 gas
• Test_Without_Optimization cost is 26440 gas

With this optimization it's possible to save 116 gas

Lines in the code

SizeSealed.sol#L294 SizeSealed.sol#L373

2. ABI.ENCODE() is less efficient than ABI.ENCODEPACKED()

Description

abi.encode will apply ABI encoding rules. Therefore all elementary types are padded to 32 bytes and dynamic arrays include their length. Therefore it is possible to also decode this data again (with abi.decode) when the type are known.

abi.encodePacked will only use the only use the minimal required memory to encode the data. E.g. an address will only use 20 bytes and for dynamic arrays only the elements will be stored without length. For more info see the Solidity docs for packed mode

For the input of the keccak method it is important that you can ensure that the resulting bytes of the encoding are unique. So if you always encode the same types and arrays always have the same length then there is no problem. But if you switch the parameters that you encode or encode multiple dynamic arrays you might have conflicts.

For example:

abi.encodePacked(address(0x0000000000000000000000000000000000000001), uint(0)) encodes to the same as abi.encodePacked(uint(0x0000000000000000000000000000000000000001000000000000000000000000), address(0))

and

abi.encodePacked(uint, uint) encodes to the same as abi.encodePacked(uint, uint)

Therefore these examples will also generate the same hashes even so they are different inputs.

On the other hand you require less memory and therefore in most cases abi.encodePacked uses less gas than abi.encode.

Lines in the code

SizeSealed.sol#L467 ECCMath.sol#L26 ECCMath.sol#L61

3. Use unchecked in for/while loops when it's not possible to overflow

Description

Use unchecked { i++; } or unchecked{ ++i; } when it's not possible to overflow to save gas.

Lines in the code

SizeSealed.sol#L244 SizeSealed.sol#L302

4. Calldata vs Memory

Description

Use calldata instead of memory in a function parameter when you are only to read the data can save gas by storing it in calldata

Lines in the code

SizeSealed.sol#L217 ECCMath.sol#L25 ECCMath.sol#L37 ECCMath.sol#L51 ECCMath.sol#L60

5. Multiplication/division by two should use bit shifting

Description

<x> * 2 is equivalent to <x> << 1 and <x> / 2 is the same as <x> >> 1. The MUL and DIV opcodes cost 5 gas, whereas SHL and SHR only cost 3 gas. In the code there are some division by 256 that is the same than <x> >> 8.

-	uint256[] memory seenBidMap = new uint256[]((bidIndices.length/256)+1);
+	uint256[] memory seenBidMap = new uint256[]((bidIndices.length >> 8)+1);

SizeSealed.sol#L241

-	uint256 bitmapIndex = bidIndex / 256;
-	uint256 bitmapIndex = bidIndex >> 8;

SizeSealed.sol#L249

6. Remove local variables that are used once

Description

-	uint256 quoteBought = FixedPointMathLib.mulDivDown(baseAmount, a.data.lowestQuote, a.data.lowestBase);
-	uint256 refundedQuote = b.quoteAmount - quoteBought;
+	uint256 refundedQuote = b.quoteAmount - FixedPointMathLib.mulDivDown(baseAmount, a.data.lowestQuote, a.data.lowestBase);

Lines in the code

SizeSealed.sol#L377-L378