SIZE contest - ReyAdmirado's results

QA

1. typo in comments

runnning --> running

• SizeSealed.sol#L112

Finalises --> Finalizes

• SizeSealed.sol#L211

finalised --> finalized

• SizeSealed.sol#L412

futher --> further

• SizeSealed.sol#L431

2. event is missing indexed fields

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 (three fields). 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.

• ISizeSealed.sol#L97
• ISizeSealed.sol#L103
• ISizeSealed.sol#L118
• ISizeSealed.sol#L122

3. constants should be defined rather than using magic numbers

Even assembly can benefit from using readable constants instead of hex/numeric literals

• SizeSealed.sol#L72
• SizeSealed.sol#L157
• SizeSealed.sol#L249
• SizeSealed.sol#L251
• SizeSealed.sol#L309

4. Code Structure Deviates From Best-Practice

The best-practice layout for a contract should follow the following order: state variables, events, modifiers, constructor and functions. Function ordering helps readers identify which functions they can call and find constructor and fallback functions easier. Functions should be grouped according to their visibility and ordered as: constructor, receive function (if exists), fallback function (if exists), external, public, internal, private.

• SizeSealed.sol#L203
• SizeSealed.sol#L217

5. inconsistent use of named return variables

there is an inconsistent use of named return variables in the contract some functions return named variables, others return explicit values. consider adopting a consistent approach. this would improve both the explicitness and readability of the code, and it may also help reduce regressions during future code refactors.

most functions do not use named return

• SizeSealed.sol#L454
• SizeSealed.sol#L474
• SizeSealed.sol#L478

Gas

1. state variables should be cached in stack variables rather than re-reading them from storage

The instances below point to the second+ access of a state variable within a function. Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.

because the if (a.params.merkleRoot != bytes32(0)) check will usually pass should cache a.params.merkleRoot before if statement to save gas

• SizeSealed.sol#L132

2. Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if statement

require(a <= b); x = b - a => require(a <= b); unchecked { x = b - a } if(a <= b); x = b - a => if(a <= b); unchecked { x = b - a } this will stop the check for overflow and underflow so it will save gas

• SizeSealed.sol#L290
• SizeSealed.sol#L319

3. <x> += <y> costs more gas than <x> = <x> + <y> for state variables

Using the addition operator instead of plus-equals saves gas

• SizeSealed.sol#L373

4. not using the named return variables when a function returns, wastes deployment gas

• SizeSealed.sol#L454

5. can make the variable outside the loop to save gas

• SizeSealed.sol#L245
• SizeSealed.sol#L249
• SizeSealed.sol#L250
• SizeSealed.sol#L251

6. ++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-loop and while-loops

In Solidity 0.8+, there’s a default overflow check on unsigned integers. It’s possible to uncheck this in for-loops and save some gas at each iteration, but at the cost of some code readability, as this uncheck cannot be made inline.

• SizeSealed.sol#L244
• SizeSealed.sol#L302

7. internal functions only called once can be inlined to save gas

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

decryptMessage

• ECCMath.sol#L51

tokensAvailableAtTime

• CommonTokenMath.sol#L47

8. abi.encode() is less efficient than abi.encodepacked()

• ECCMath.sol#L26