code-423n4 / 2022-08-fiatdao-findings

2 stars 1 forks source link

QA Report #241

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Typos


The same typos (binarysearch and preceeding) occur in both lines below:

VotingEscrow.sol: L705

VotingEscrow.sol: L729

    // @dev Uses binarysearch to find the most recent point history preceeding block

Change binarysearch to binary search and preceeding to precedingin each case



Unclear comment

The readability of the comment below could be improved


VotingEscrow.sol: L270

            // newLocked.end can ONLY by in the FUTURE unless everything expired: than zeros

Suggestion:

            // newLocked.end can ONLY be in the FUTURE unless everything expired: then zeros


Use of exponentiation

Communicating large multiples of ten should be consistent. For readability, scientific notation is preferable to using exponentiation.


Below, exponentiation (10**18) and scientific notation (1e9) both are used:

VotingEscrow.sol: L48-57

    uint256 public constant MULTIPLIER = 10**18;
    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;

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

VotingEscrow.sol: L653

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

Recommendation: Change 10**18 to 1e18 in each case



Update sensitive term

Terms incorporating "black," "white," "slave" or "master" are potentially problematic. Substituting more neutral terminology is becoming common practice.

VotingEscrow.sol: L20

///            3) Whitelisting of SmartWallets outside the VotingEscrow

Suggestion: Change Whitelisting to Allowlisting