FIAT DAO veFDT contest - Bnke0x0's results

Low Risk

[L-01] Missing checks for address(0x0) when assigning values to address state variables:-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 15):

   manager = _manager;

2. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 16):

   ve = _ve;

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 118):

   name = _name;

4. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 119):

   symbol = _symbol;

5. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 120):

   owner = _owner;

6. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 121):

   penaltyRecipient = _penaltyRecipient;

7. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 141):

   owner = _addr;

8. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 148):

   blocklist = _addr;

9. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 155):

   penaltyRecipient = _addr;

Non-Critical Issues

[N-01] Adding a return statement when the function defines a named return variable, is redundant:-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 34):

   return _blocklist[addr];

2. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 42):

   return size > 0;

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 193):

   return locked[_addr].end;

[N-02] Event is missing indexed fields (Each event should use three indexed fields if there are three or more fields):-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 25-31):

   event Deposit( address indexed provider, uint256 value, uint256 locktime, LockAction indexed action, uint256 ts );

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 32-37):

   event Withdraw( address indexed provider, uint256 value, LockAction indexed action, uint256 ts );

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 41):

   event CollectPenalty(uint256 amount, address recipient);

4. File: 2022-08-fiatdao/contracts/interfaces/IERC20.sol (line 29):

   event Transfer(address indexed from, address indexed to, uint256 value);

5. File: 2022-08-fiatdao/contracts/interfaces/IERC20.sol (line 30-34):

   event Approval( address indexed owner, address indexed spender, uint256 value );

[N-03] constants should be defined rather than using magic numbers:-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 48):

   uint256 public constant MULTIPLIER = 10**18;

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 51):

   uint256 public maxPenalty = 10**18;

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 57-58):

   Point[1000000000000000000] public pointHistory; // 1e9 * userPointHistory-length, so sufficient for 1e9 users mapping(address => Point[1000000000]) public userPointHistory;

4. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 66):

   uint256 public decimals = 18;

5. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 116):

   require(decimals <= 18, "Exceeds max decimals");

6. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 309):

   for (uint256 i = 0; i < 255; i++) {

7. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 653):

   uint256 penaltyAmount = (value * penaltyRate) / 10**18;

8. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 717):

   for (uint256 i = 0; i < 128; i++) {

9. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 739):

   for (uint256 i = 0; i < 128; i++) {

[N-04] public functions not called by the contract should be declared external instead (Contracts are allowed to override their parents’ functions and change the visibility from external to public.):-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 33):

   function isBlocked(address addr) public view returns (bool) {

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 754):

   function balanceOf(address _owner) public view override returns (uint256) {

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 770-775):

   function balanceOfAt(address _owner, uint256 _blockNumber) public view override returns (uint256) {

4. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 864):

   function totalSupply() public view override returns (uint256) {

5. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 871-876):

   function totalSupplyAt(uint256 _blockNumber) public view override returns (uint256) {

[N-05] Numeric values having to do with time should use time units for readability (There are units for seconds, minutes, hours, days, and weeks):-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 46):

   uint256 public constant WEEK = 7 days;

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 47):

   uint256 public constant MAXTIME = 365 days;

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 48):

   uint256 public constant MULTIPLIER = 10**18;

[N-06] Non-library/interface files should use fixed compiler versions, not floating ones:-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 2):

   pragma solidity ^0.8.3;

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 2):

   pragma solidity ^0.8.3;

3. File: 2022-08-fiatdao/contracts/interfaces/IBlocklist.sol (line 2):

   pragma solidity ^0.8.3;

4. File: 2022-08-fiatdao/contracts/interfaces/IVotingEscrow.sol (line 2):

   pragma solidity ^0.8.3;

5. File: 2022-08-fiatdao/contracts/interfaces/IERC20.sol (line 2):

   pragma solidity ^0.8.3;

[G-01] State variables only set in the constructor should be declared immutable (Avoids a Gsset (20000 gas)):-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 11):

   address public manager;

2. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 12):

   address public ve;

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 45):

   IERC20 public token;

4. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 49-53):

   address public owner; address public penaltyRecipient; // receives collected penalty payments uint256 public maxPenalty = 10**18; // penalty for quitters with MAXTIME remaining lock uint256 public penaltyAccumulated; // accumulated and unwithdrawn penalty payments address public blocklist;

5. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 64-65):

   string public name; string public symbol;

[G-02] State variables can be packed into fewer storage slots (If variables occupying the same slot are both written the same

function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables are also cheaper):-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 11):

   address public manager;

2. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 12):

   address public ve;

[G-03] x = x + y is cheaper than x += y:-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 537):

   newLocked.delegated -= int128(int256(value));

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 612):

   newLocked.delegated -= value;

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol(line 642):

   newLocked.delegated -= int128(int256(value));

4. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 418):

   locked_.amount += int128(int256(_value));

5. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 420):

   locked_.delegated += int128(int256(_value));

6. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 460-461):

   newLocked.amount += int128(int256(_value)); newLocked.delegated += int128(int256(_value));

7. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 465):

   locked_.amount += int128(int256(_value));

8. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 472):

   newLocked.delegated += int128(int256(_value));

9. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 603):

   newLocked.delegated += value;

10. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 654):

    penaltyAccumulated += penaltyAmount;

[G-04] require()/revert() strings longer than 32 bytes cost extra gas:-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 34):

   return _blocklist[addr];

2. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 42):

   return size > 0;

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 193):

   return locked[_addr].end;

[G-05] Using > 0 costs more gas than != 0 when used on a uint in a require() statement:-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 412):

   require(_value > 0, "Only non zero amount");

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 448):

   require(_value > 0, "Only non zero amount");

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 449):

   require(locked_.amount > 0, "No lock");

4. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 469):

   require(locked_.amount > 0, "Delegatee has no lock");

5. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 502):

   require(locked_.amount > 0, "No lock");

6. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 529):

   require(locked_.amount > 0, "No lock");

7. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 564):

   require(locked_.amount > 0, "No lock");

8. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 587):

   require(toLocked.amount > 0, "Delegatee has no lock");

9. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 635):

   require(locked_.amount > 0, "No lock");

[G-06] It costs more gas to initialize variables to zero than to let the default of zero be applied:-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 163):

   maxPenalty = 0;

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 229-230):

   int128 oldSlopeDelta = 0; int128 newSlopeDelta = 0;

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 298):

   uint256 blockSlope = 0;

4. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 309):

   for (uint256 i = 0; i < 255; i++) {

5. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 313):

   int128 dSlope = 0;

6. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 326):

   lastPoint.bias = 0;

7. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 330):

   lastPoint.slope = 0;

8. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 717):

   for (uint256 i = 0; i < 128; i++) {

9. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 739):

   for (uint256 i = 0; i < 128; i++) {

10. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 834):

    for (uint256 i = 0; i < 255; i++) {

[G-07] ++i costs less gas than i++, especially when it’s used in forloops (--i/i-- too):-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 309):

   for (uint256 i = 0; i < 255; i++) {

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 717):

   for (uint256 i = 0; i < 128; i++) {

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 739):

   for (uint256 i = 0; i < 128; i++) {

4. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 834):

   for (uint256 i = 0; i < 255; i++) {

[G-08] Usage of uints/ints 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):-

1. File: 2022-08-fiatdao/contracts/interfaces/IERC20.sol (line 10):

   function decimals() external view returns (uint8);

[G-09] Duplicated require()/revert() checks should be refactored to a modifier or function:-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 162):

   require(msg.sender == owner, "Only owner");

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 637):

   require(locked_.delegatee == msg.sender, "Lock delegated");

[G-10] require() or revert() statements that check input arguments should be at the top of the function (Checks that involve constants should come before checks that involve state variables):-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 565):

   require(locked_.delegatee != _addr, "Already delegated");

[G-11] Use custom errors rather than revert()/require() strings to save deployment gas:-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 24-25):

   require(msg.sender == manager, "Only manager"); require(_isContract(addr), "Only contracts");

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 116):

   require(decimals <= 18, "Exceeds max decimals");

3. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 125-128):

   require( !IBlocklist(blocklist).isBlocked(msg.sender), "Blocked contract" );

4. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 140):

   require(msg.sender == owner, "Only owner");

5. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 147):

   require(msg.sender == owner, "Only owner");

6. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 154):

   require(msg.sender == owner, "Only owner");

7. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 162):

   require(msg.sender == owner, "Only owner");

8. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 171):

   require(msg.sender == blocklist, "Only Blocklist");

9. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 412-416):

   require(_value > 0, "Only non zero amount"); require(locked_.amount == 0, "Lock exists"); require(unlock_time >= locked_.end, "Only increase lock end"); // from using quitLock, user should increaseAmount instead require(unlock_time > block.timestamp, "Only future lock end"); require(unlock_time <= block.timestamp + MAXTIME, "Exceeds maxtime");

10. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 425-428):

    require( token.transferFrom(msg.sender, address(this), _value), "Transfer failed" );

11. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 448-450):

    require(_value > 0, "Only non zero amount"); require(locked_.amount > 0, "No lock"); require(locked_.end > block.timestamp, "Lock expired");

12. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 469-670):

    require(locked_.amount > 0, "Delegatee has no lock"); require(locked_.end > block.timestamp, "Delegatee lock expired");

13. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 485-488):

    require( token.transferFrom(msg.sender, address(this), _value), "Transfer failed" );

14. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 502-504):

    require(locked_.amount > 0, "No lock"); require(unlock_time > locked_.end, "Only increase lock end"); require(unlock_time <= block.timestamp + MAXTIME, "Exceeds maxtime");

15. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 511):

    require(oldUnlockTime > block.timestamp, "Lock expired");

16. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 529-531):

    require(locked_.amount > 0, "No lock"); require(locked_.end <= block.timestamp, "Lock not expired"); require(locked_.delegatee == msg.sender, "Lock delegated");

17. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 546):

    require(token.transfer(msg.sender, value), "Transfer failed");

18. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 563-565):

    require(!IBlocklist(blocklist).isBlocked(_addr), "Blocked contract"); require(locked_.amount > 0, "No lock"); require(locked_.delegatee != _addr, "Already delegated");

19. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 587-689):

    require(toLocked.amount > 0, "Delegatee has no lock"); require(toLocked.end > block.timestamp, "Delegatee lock expired"); require(toLocked.end >= fromLocked.end, "Only delegate to longer lock");

20. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 635-637):

    require(locked_.amount > 0, "No lock"); require(locked_.end > block.timestamp, "Lock expired"); require(locked_.delegatee == msg.sender, "Lock delegated");

21. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 657):

    require(token.transfer(msg.sender, remainingAmount), "Transfer failed");

22. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 676):

    require(token.transfer(penaltyRecipient, amount), "Transfer failed");

23. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 776):

    require(_blockNumber <= block.number, "Only past block number");

24. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 877):

    require(_blockNumber <= block.number, "Only past block number");

[G-12] Use a more recent version of solidity (Use a solidity version of at least 0.8.15 to have external calls skip

contract existence checks if the external call has a return value):-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 2):

   pragma solidity ^0.8.3;

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 2):

   pragma solidity ^0.8.3;

3. File: 2022-08-fiatdao/contracts/interfaces/IBlocklist.sol (line 2):

   pragma solidity ^0.8.3;

4. File: 2022-08-fiatdao/contracts/interfaces/IVotingEscrow.sol (line 2):

   pragma solidity ^0.8.3;

5. File: 2022-08-fiatdao/contracts/interfaces/IERC20.sol (line 2):

   pragma solidity ^0.8.3;

[G-12] Use calldata instead of memory for function parameters (Use calldata instead of memory for function parameters. Having function arguments use calldata instead of memory can save gas.):-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 100-106):

   constructor( address _owner, address _penaltyRecipient, address _token, string memory _name, string memory _symbol ) {

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 222-226):

   function _checkpoint( address _addr, LockedBalance memory _oldLocked, LockedBalance memory _newLocked )

3. File: 2022-08-fiatdao/contracts/interfaces/IBlocklist.sol (line 595-600):

   function _delegate( address addr, LockedBalance memory _locked, int128 value, LockAction action )

4. File: 2022-08-fiatdao/contracts/interfaces/IVotingEscrow.sol (line 685):

   function _copyLock(LockedBalance memory _locked)

5. File: 2022-08-fiatdao/contracts/interfaces/IERC20.sol (line 825):

   function _supplyAt(Point memory _point, uint256 _t)

[G-13] Amounts should be checked for 0 before calling a transfer (Checking non-zero transfer values can avoid an expensive external call and save gas.

While this is done at some places, it’s not consistently done in the solution.

I suggest adding a non-zero-value check here:):-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 425-428):

   require( token.transferFrom(msg.sender, address(this), _value), "Transfer failed" );

2. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 485-488):

   require( token.transferFrom(msg.sender, address(this), _value), "Transfer failed" );

3. File: 2022-08-fiatdao/contracts/interfaces/IBlocklist.sol (line 546):

   require(token.transfer(msg.sender, value), "Transfer failed");

4. File: 2022-08-fiatdao/contracts/interfaces/IVotingEscrow.sol (line 657):

   require(token.transfer(msg.sender, remainingAmount), "Transfer failed");

5. File: 2022-08-fiatdao/contracts/interfaces/IERC20.sol (line 676):

   require(token.transfer(penaltyRecipient, amount), "Transfer failed");

[G-14] Multiple address mappings can be combined into a single (Saves a storage slot for the mapping. Depending on the circumstances

and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot).

While this is done at some places, it’s not consistently done in the solution.

I suggest adding a non-zero-value check here:):-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 58-61):

   mapping(address => Point[1000000000]) public userPointHistory; mapping(address => uint256) public userPointEpoch; mapping(uint256 => int128) public slopeChanges; mapping(address => LockedBalance) public locked;

[G-15] Using bools for storage incurs overhead:-

1. File: 2022-08-fiatdao/contracts/features/Blocklist.sol (line 10):

   mapping(address => bool) private _blocklist;

[G-16] Using private rather than public for constants, saves gas (If needed, the value can be read from the verified contract source

code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table):-

1. File: 2022-08-fiatdao/contracts/VotingEscrow.sol (line 46-48):

   uint256 public constant WEEK = 7 days; uint256 public constant MAXTIME = 365 days; uint256 public constant MULTIPLIER = 10**18;