FIAT DAO veFDT contest - delfin454000's results

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

---

---

State variables should not be initialized to their default values

Initializing uint variables to their default value of 0 is unnecessary and costs gas

---

VotingEscrow.sol: L298

        uint256 blockSlope = 0; // dblock/dt

Change to:

        uint256 blockSlope; // dblock/dt

Similarly for the additional instances below

---

min is initialized to zero in both the following lines:

VotingEscrow.sol: L714

VotingEscrow.sol: L737

        uint256 min = 0;

---

VotingEscrow.sol: L793

        uint256 dBlock = 0;

---

dTime is initialized to zero in both the following lines:

VotingEscrow.sol: L794

VotingEscrow.sol: L889

        uint256 dTime = 0;

---

---

Use ++i instead of i++ to increase count in a for loop

Since use of i++ (or equivalent counter) costs more gas, it is better to use ++i in the four for loops referenced below:

VotingEscrow.sol: L309-347

VotingEscrow.sol: L717-725

VotingEscrow.sol: L739-748

VotingEscrow.sol: L834-855

---

---

Avoid use of default "checked" behavior in a for loop

Underflow/overflow checks are made every time ++i (or i++ or equivalent counter) is called. Such checks are unnecessary since i is already limited. Therefore, use unchecked {++i}/unchecked {i++} instead

---

VotingEscrow.sol: L717-725

        for (uint256 i = 0; i < 128; i++) {
            if (min >= max) break;
            uint256 mid = (min + max + 1) / 2;
            if (pointHistory[mid].blk <= _block) {
                min = mid;
            } else {
                max = mid - 1;
            }
        }

Suggestion:

        for (uint256 i = 0; i < 128;) {
            if (min >= max) break;
            uint256 mid = (min + max + 1) / 2;
            if (pointHistory[mid].blk <= _block) {
                min = mid;
            } else {
                max = mid - 1;
            }

            unchecked {
              ++i;
          }
        }

---

Similarly for the three for loops referenced below:

VotingEscrow.sol: L309-347

VotingEscrow.sol: L739-748

VotingEscrow.sol: L834-855

---

---