FIAT DAO veFDT contest - LeoS's results

Low Risk

[L-01] Floating pragma.

It's a good practice to avoid the use of floating pragma. Code must be compiled with the same version it as been tested the most. It also avoids the use of any nightly builds, which can have unexpected and unknown behaviors.

5 instances

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/features/Blocklist.sol#L2
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L2
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/interfaces/IBlocklist.sol#L2
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/interfaces/IVotingEscrow.sol#L2
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/interfaces/IERC20.sol#L2

Consider replacing ^0.8.3 by 0.8.3.

[L-02] Missing check for address(0) when assigning value to address state variable.

Zero address checking is the best practice to prevent burning fee, unexpected exceptions or the redeployment of the contract.

5 instances:

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L120-L121
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L141
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L148
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L155

Consider adding a modifier at the start of those function to check it.

This is low risk because any risk can only result of an owner's error in calling an external function. This can easily be prevented.

Non Critical

[N-01] Typo.

preceeding -> preceding seems more right.

2 instances:

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L705

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L729

[N-02] Large multiple of ten should use scientific notation for readability and consistency.

There is non consistency in the use of large multiple of ten, it does not help the readability.

2 instances:

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L57-L58

Consider changing Point[1000000000000000000] and Point[1000000000] to Point[1e18] and Point[1e9] or Point[10**18] and Point[10**9] for consistency.

[G-01] Index increment can be left uncheck in for loop

For solidity ^0.8.0 there is an overflow check on each increment operation. This check is not needed in those for loop, since it can't overflow.

4 instances:

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L309
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L717
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L739
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L834

Consider removing i++ and replacing it by unchecked { ++i; } at the end of the loop. Transforming i++ to ++i is also cheaper.

These gas avg changes can be observed:

MockSmartWallet: quitLock: 201886 -> 201849 (-37)
MockSmartWallet: withdraw: 666280 -> 665540 (-740)
VotingEscrow: checkpoint: 1272491  -> 1271247 (-1244)
VotingEscrow: createLock: 414348 -> 414267 (-81)
VotingEscrow: delegate: 650225 -> 649908 (-317)
VotingEscrow: increaseAmount: 448554 -> 448332 (-222)
VotingEscrow: increaseUnlockTime: 143186 -> 143157 (-29)
VotingEscrow: quitLock: 375486 -> 375258 (-228)
VotingEscrow: withdraw: 610141 -> 609619 (-522)
Deployment: VotingEscrow: 4374350 -> 4353169 (-21 181)

[G-02] Use != 0 instead of > 0 for unsigned integers.

A uint can't be below zero, so != 0 is sufficient and is gas more efficient. epoch, _value are uint.

4 instances

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L288
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L412
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L448
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/features/Blocklist.sol#L42

Consider replacing > by !=.

save 3 gas each

[G-03] Unnecessary initialization of variables.

int, uint, bool and address are initialized by default with 0, 0, false and address(0). It is not necessary to initialize these value again.

uint256 i =  0 ; -> uint256 i;

14 instances:

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L229-L230
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L298
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L309
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L313
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L714
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L717
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L737
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L739
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L793-L794
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L834
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L836
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L889

Consider shortening these initializations.

save 3 gas each

[G-04] Expression like x = x + y are cheaper than x += y for states variables.

11 instances:

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L418
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L420
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L460-L461
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L465
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L472
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L603
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L654
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L537
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L612
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L642

Consider replacing += and -=

These gas avg changes can be observed:

Blocklist: block: 198815 -> 198813 (-2)
MockSmartWallet: createLock: 355494 -> 355472 (-22)
MockSmartWallet: delegate: 340388 -> 340384 (-4)
MockSmartWallet: quitLock: 201886 -> 201862 (-24)
MockSmartWallet: withdraw: 666280 -> 666266 (-14)
VotingEscrow: createLock: 414348 -> 414326 (-22)
VotingEscrow: delegate: 650225 -> 650221 (-4)
VotingEscrow: increaseAmount: 448554 -> 448532 (-22)
VotingEscrow: quitLock: 375486 -> 375462 (-24)
VotingEscrow : withdraw: 610141 -> 610127 (-14)
Deployment: VotingEscrow: 4374350 -> 4367298 (-7052)

[G-05] external function for the owner can be marked as payable.

If a function is guaranteed to revert when called by a normal user, this function can be marked as payable to avoid the check to know if a payment is provided.

4 instances:

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L139
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L146
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L153
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L161

Consider adding payable keyword.

Save 21 gas cost each

[G-06] Using storage instead of memory for structs can be cheaper.

A storage structure is pre allocated by the contract, by contrast, a memory one is newly created. Depending on the case both can be used to optimize the gas cost because simply, a storage is cheaper to create but more expensive to read from and to return and a memory on the other hand is more expensive to create but cheaper to read from and to return. We can optimize with trials and errors instead of complex calculations (which will probably work a bit better, but it's not done here).

Following this, we can deduce 8 cases that can be swapped to optimize runtime cost and deployment cost.

8 instances:

• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L172
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L177
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L214
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L446
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L788
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L796
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L882
• https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/VotingEscrow.sol#L891

Consider changing memory to storage in these lines

These gas avg changes can be observed:

 Blocklist: block: 198815 -> 196332 (-2483)
 VotingEscrow: increaseAmount: 448554 -> 446788 (-1766)
 Deployment: VotingEscrow: 4374350 -> 4320884 (-53466)