FIAT DAO veFDT contest - Rolezn's results

(1) Missing Checks for Address(0x0)

Severity: Low

Proof Of Concept

function transferOwnership(address _addr) external {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L139

function updateBlocklist(address _addr) external {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L146

function updatePenaltyRecipient(address _addr) external {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L153

function forceUndelegate(address _addr) external override {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L170

function delegate(address _addr)
    external
    override
    nonReentrant
    checkBlocklist
{

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L555

function block(address addr) external {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/features/Blocklist.sol#L23

Recommended Mitigation Steps

Consider adding zero-address checks in the mentioned codebase.

(2) Use Safetransfer Instead Of Transfer

Severity: Low

It is good to add a require() statement that checks the return value of token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of transfers and affect token accounting in contract.

Reference: This similar medium-severity (https://consensys.net/diligence/audits/2021/01/fei-protocol/#unchecked-return-value-for-iweth-transfer-call) finding from Consensys Diligence Audit of Fei Protocol.

Proof Of Concept

token.transferFrom(msg.sender, address(this), _value),

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L421

token.transferFrom(msg.sender, address(this), _value),

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L479

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L540

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L649

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L676

Recommended Mitigation Steps

Consider using safeTransfer/safeTransferFrom or require() consistently.

(3) Transferownership Should Be Two Step

Severity: Low

The owner is the authorized user in the solidity contracts. Usually, an owner can be updated with transferOwnership function. However, the process is only completed with single transaction. If the address is updated incorrectly, an owner functionality will be lost forever.

Proof Of Concept

function transferOwnership(address _addr) external {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L139

Recommended Mitigation Steps

Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.

(4) Users can call public funcions that create something

Severity: Low

Users can potentially call functions that create potentially malicicous locks

Proof Of Concept

function createLock(uint256 _value, uint256 _unlockTime)

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L403

Recommended Mitigation Steps

Consider adding a mapping of who can call these create functions to avoid malicious use

(5) Update OpenZeppelin Packages to ^4.7.2

Severity: Low

According to package.json, the OZ packages are currently set to ^4.4.2

Proof Of Concept

Package.json#L42

Recommended Mitigation Steps

Update the versions of @openzeppelin/contracts to be the latest in package.json. I also recommend double checking the versions of other dependencies as a precaution, as they may include important bug fixes.

(6) Avoid Floating Pragmas: The Version Should Be Locked

Severity: Non-Critical

Proof Of Concept

Found usage of floating pragmas ^0.8.3 of Solidity https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L2

Found usage of floating pragmas ^0.8.3 of Solidity https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/features/Blocklist.sol#L2

(7) Constants Should Be Defined Rather Than Using Magic Numbers

Severity: Non-Critical

Proof Of Concept

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L293

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L715

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L739

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L832

(8) Event Is Missing Indexed Fields

Severity: Non-Critical

Each event should use three indexed fields if there are three or more fields. In addition, each event should have at least one indexed fields to allow easier filtering of logs.

Proof Of Concept

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L25

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L32

event CollectPenalty(uint256 amount, address recipient);

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L41

(9) Implementation contract may not be initialized

OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5

Severity: Non-Critical

Proof Of Concept

contract VotingEscrow is IVotingEscrow, ReentrancyGuard 

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L23

contract Blocklist 

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/features/Blocklist.sol#L9

(10) Use a more recent version of Solidity

Severity: Non-Critical

Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>) Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>) Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

Proof Of Concept

Found old version 0.8.3 of Solidity https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L2

Found old version 0.8.3 of Solidity https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/features/Blocklist.sol#L2

Recommended Mitigation Steps

Consider updating to a more recent solidity version.

(11) Large multiples of ten should use scientific notation

Severity: Non-Critical

Use (e.g. 1e6) rather than decimal literals (e.g. 100000), for better code readability.

Proof Of Concept

Point[1000000000000000000] public pointHistory; // 1e9 * userPointHistory-length, so sufficient for 1e9 users

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

mapping(address => Point[1000000000]) public userPointHistory;

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

(12) Use of Block.Timestamp

Severity: Non-Critical

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

Proof Of Concept

    ts: block.timestamp,

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L111

    if (_oldLocked.end > block.timestamp && _oldLocked.delegated > 0) {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L236

    int128(int256(_oldLocked.end - block.timestamp));

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L242

    if (_newLocked.end > block.timestamp && _newLocked.delegated > 0) {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L244

    int128(int256(_newLocked.end - block.timestamp));

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L250

    userNewPoint.ts = block.timestamp;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L262

    ts: block.timestamp,

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L285

    if (block.timestamp > lastPoint.ts) {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L299

    (block.timestamp - lastPoint.ts);

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L302

    if (iterativeTime > block.timestamp) {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L314

    iterativeTime = block.timestamp;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L315

    if (iterativeTime == block.timestamp) {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L341

    if (_oldLocked.end > block.timestamp) {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L378

    if (_newLocked.end > block.timestamp) {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L386

    require(unlock_time > block.timestamp, "Only future lock end");

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L415

    require(unlock_time <= block.timestamp + MAXTIME, "Exceeds maxtime");

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L416

    block.timestamp

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L434

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L450

    require(locked_.end > block.timestamp, "Delegatee lock expired");

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L470

    block.timestamp

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L479

    emit Deposit(msg.sender, _value, unlockTime, action, block.timestamp);

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L489

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L511

    require(locked_.end <= block.timestamp, "Lock not expired");

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L530

    emit Withdraw(msg.sender, value, LockAction.WITHDRAW, block.timestamp);

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L547

    require(toLocked.end > block.timestamp, "Delegatee lock expired");

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L588

    emit Withdraw(msg.sender, value, LockAction.QUIT, block.timestamp);

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L658

    return ((end - block.timestamp) * maxPenalty) / MAXTIME;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L668

    (lastPoint.slope * int128(int256(block.timestamp - lastPoint.ts)));

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L762

    dTime = block.timestamp - point0.ts;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L801

    return _supplyAt(lastPoint, block.timestamp);

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L867

    ((_blockNumber - point.blk) * (block.timestamp - point.ts)) /

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L899

Recommended Mitigation Steps

Block timestamps should not be used for entropy or generating random numbers—i.e., they should not be the deciding factor (either directly or through some derivation) for winning a game or changing an important state.

Time-sensitive logic is sometimes required; e.g., for unlocking contracts (time-locking), completing an ICO after a few weeks, or enforcing expiry dates. It is sometimes recommended to use block.number and an average block time to estimate times; with a 10 second block time, 1 week equates to approximately, 60480 blocks. Thus, specifying a block number at which to change a contract state can be more secure, as miners are unable to easily manipulate the block number.

(13) Only A Billion Checkpoints Available

Severity: Non-Critical

A user can only have a billion checkpoints which, if the user is a DAO, may cause issues down the line, especially if the last checkpoint involved delegating and can thereafter not be undone

Proof Of Concept

mapping(address => Point[1000000000]) public userPointHistory;

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

(14) Missing NATSPEC documentation

Severity: Non-Critical

Proof Of Concept

function _isContract(address addr) internal view returns (bool) { https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/features/Blocklist.sol

(15) Incomplete NATSPEC documentation

Severity: Non-Critical

Proof Of Concept

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol

(1) ++i Costs Less Gas Than i++, Especially When It’s Used In For-loops (--i/i-- Too)

Severity: Gas Optimizations

Saves 6 gas per loop

Proof Of Concept

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L293

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L715

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L739

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L832

Recommended Mitigation Steps

For example, use ++i instead of i++

(2) It Costs More Gas To Initialize Variables To Zero Than To Let The Default Of Zero Be Applied

Severity: Gas Optimizations

Proof Of Concept

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L293

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L715

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L739

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L832

(3) Using > 0 Costs More Gas Than != 0 When Used On A Uint In A Require() Statement

Severity: Gas Optimizations

This change saves 6 gas per instance

Proof Of Concept

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L410

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L447

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L448

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L464

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L447

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L448

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L464

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L447

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L448

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L464

uint256 unlock_time = _floorToWeek(_unlockTime);                 require(locked_.amount > 0, "No lock");

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L500

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L528

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L563

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L581

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L563

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L581

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L634

(4) Multiple Address Mappings Can Be Combined Into A Single Mapping Of An Address To A Struct, Where Appropriate

Severity: Gas Optimizations

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.

Proof Of Concept

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

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

(5) Multiplication/division By Two Should Use Bit Shifting

Severity: Gas Optimizations

<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

Proof Of Concept

uint256 mid = (min + max + 1) / 2;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L717

uint256 mid = (min + max + 1) / 2;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L743

(6) <x> += <y> Costs More Gas Than <x> = <x> + <y> For State Variables

Severity: Gas Optimizations

Proof Of Concept

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L414

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L416

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L457

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L458

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L461

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L535

newLocked.delegated += value;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L603

newLocked.delegated -= value;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L612

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L640

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L647

(7) Using Private Rather Than Public For Constants, Saves Gas

Severity: Gas Optimizations

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

Proof Of Concept

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L46-L48

Recommended Mitigation Steps

Set variable to private.

(8) Public Functions To External

Severity: Gas Optimizations

The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.

Proof Of Concept

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L754

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L864

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/features/Blocklist.sol#L33

(10) Help The Optimizer By Saving A Storage Variable’s Reference Instead Of Repeatedly Fetching It

Severity: Gas Optimizations

To help the optimizer, declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array. The effect can be quite significant. As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex] and use it.

Proof Of Concept

fromLocked = locked[delegatee];

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L179

lastPoint = pointHistory[epoch];

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L280

dSlope = slopeChanges[iterativeTime];

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L299

locked_ = locked[delegatee];

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L463

fromLocked = locked[delegatee];

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L574

if (pointHistory[mid].blk <= _block) {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L718

Point memory point0 = pointHistory[epoch];

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L786

dSlope = slopeChanges[iterativeTime];

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L839

Point memory point = pointHistory[targetEpoch];

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L882

return _blocklist[addr];

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/features/Blocklist.sol#L34

(11) ++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- And While-loops

Severity: Gas Optimizations

The unchecked keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas PER LOOP

Proof Of Concept

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L293

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L715

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L739

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

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L832

(12) Use assembly to check for address(0)

Severity: Gas Optimizations

The following functions are missing zero address check. Save 6 gas per instance if using assembly to check for address(0)

e.g. assembly { if iszero(_addr) { mstore(0x00, "AddressZero") revert(0x00, 0x20) } }

Proof Of Concept

function transferOwnership(address _addr) external {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L139

function updateBlocklist(address _addr) external {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L146

function updatePenaltyRecipient(address _addr) external {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L153

function forceUndelegate(address _addr) external override {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L170

function delegate(address _addr)
    external
    override
    nonReentrant
    checkBlocklist
{

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L555

function block(address addr) external {

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/features/Blocklist.sol#L23

(13) Using 10**X for constants isn't gas efficient

Severity: Gas Optimizations

In Solidity, a constant expression in a variable will compute the expression everytime the variable is called. It's not the result of the expression that is stored, but the expression itself.

As Solidity supports the scientific notation, constants of form 10**X can be rewritten as 1eX to save the gas cost from the calculation with the exponentiation operator **.

Proof Of Concept

uint256 public constant MULTIPLIER = 10**18;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L48

uint256 public maxPenalty = 10**18; // penalty for quitters with MAXTIME remaining lock

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L51

    uint256 penaltyAmount = (value * penaltyRate) / 10**18; // quitlock_penalty is in 18 decimals precision

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/VotingEscrow.sol#L653

Recommended Mitigation Steps

Replace 10**X with 1eX

(14) Using Bools For Storage Incurs Overhead

Severity: Gas Optimizations

Booleans are more expensive than uint256 or any type that takes up a full word because each write operation emits an extra SLOAD to first read the slot's contents, replace the bits taken up by the boolean, and then write back. This is the compiler's defense against contract upgrades and pointer aliasing, and it cannot be disabled.

Proof Of Concept

mapping(address => bool) private _blocklist;

https://github.com/code-423n4/2022-08-fiatdao/tree/main/contracts/features/Blocklist.sol#L10