FIAT DAO veFDT contest - ReyAdmirado's results

1. use of floating pragma

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

• Blocklist.sol
• VotingEscrow.sol

2. event is missing indexed fields

Each event should use three indexed fields if there are three or more fields

• VotingEscrow.sol#L25

• VotingEscrow.sol#L32

• IERC20.sol#L29

• IERC20.sol#L30

3. constants should be defined rather than using magic numbers

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

• VotingEscrow.sol#L57
• VotingEscrow.sol#L116
• VotingEscrow.sol#L309
• VotingEscrow.sol#L717
• VotingEscrow.sol#L739
• VotingEscrow.sol#L834
• VotingEscrow.sol#L653

4. wrong NatSpec use

• VotingEscrow.sol#L439
• VotingEscrow.sol#L699
• VotingEscrow.sol#L700
• VotingEscrow.sol#L705
• VotingEscrow.sol#L706
• VotingEscrow.sol#L707
• VotingEscrow.sol#L729
• VotingEscrow.sol#L730
• VotingEscrow.sol#L731

5. Outdated compiler version

Using very old versions of Solidity prevents benefits of bug fixes and newer security checks. Using the latest versions might make contracts susceptible to undiscovered compiler bugs

1. state variables only set in the constructor should be declared immutable

• VotingEscrow.sol#L45

• VotingEscrow.sol#L49

• VotingEscrow.sol#L64

• VotingEscrow.sol#L65

• VotingEscrow.sol#L66

• Blocklist.sol#L11

• Blocklist.sol#L12

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

• VotingEscrow.sol#L654

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

• VotingEscrow.sol#L204

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

• VotingEscrow.sol#L313
• VotingEscrow.sol#L319
• VotingEscrow.sol#L719
• VotingEscrow.sol#L743
• VotingEscrow.sol#L836

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

Saves 6 gas per loop

• VotingEscrow.sol#L309
• VotingEscrow.sol#L717
• VotingEscrow.sol#L739
• VotingEscrow.sol#L834

6. it costs more gas to initialize non-constant/non-immutable variables to zero than to let the default of zero be applied

• VotingEscrow.sol#L229
• VotingEscrow.sol#L230
• VotingEscrow.sol#L298
• VotingEscrow.sol#L309
• VotingEscrow.sol#L313
• VotingEscrow.sol#L714
• VotingEscrow.sol#L717
• VotingEscrow.sol#L737
• VotingEscrow.sol#L739
• VotingEscrow.sol#L793
• VotingEscrow.sol#L794
• VotingEscrow.sol#L734
• VotingEscrow.sol#L736
• VotingEscrow.sol#L889

7. ++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

• VotingEscrow.sol#L309
• VotingEscrow.sol#L717
• VotingEscrow.sol#L739
• VotingEscrow.sol#L834

8. use a more recent version of solidity

Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

9. use custom errors rather than revert()/require() strings to save deployment gas

https://blog.soliditylang.org/2021/04/21/custom-errors/

10. using calldata instead of memory for read-only arguments in external functions saves gas

• VotingEscrow.sol#L104
• VotingEscrow.sol#L105
• VotingEscrow.sol#L224
• VotingEscrow.sol#L225
• VotingEscrow.sol#L398
• VotingEscrow.sol#L597
• VotingEscrow.sol#L685
• VotingEscrow.sol#L825

11. using bools for storage incurs overhead

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27 Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past

• Blocklist.sol#L10

12. 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.

• VotingEscrow.sol#L662

• VotingEscrow.sol#L732

• Blocklist.sol#L37

13. usage of uint/int smaller than 32 bytes (256 bits) incurs overhead

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size. https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed

• VotingEscrow.sol#L60
• VotingEscrow.sol#L174
• VotingEscrow.sol#L205
• VotingEscrow.sol#L206
• VotingEscrow.sol#L229
• VotingEscrow.sol#L230
• VotingEscrow.sol#L313
• VotingEscrow.sol#L319
• VotingEscrow.sol#L567
• VotingEscrow.sol#L598
• VotingEscrow.sol#L836

14. using private rather than public for constants, saves gas

If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it’s used, and not adding another entry to the method ID table

• VotingEscrow.sol#L46
• VotingEscrow.sol#L47
• VotingEscrow.sol#L48