FIAT DAO veFDT contest - 0xNineDec's results

VotingEscrow.transferOwnership() should be a two step process

The mentioned function should perform a two step ownership transfer to prevent an irreversible lost of the ownership in case of a mistakenly input address while calling VotingEscrow.transferOwnership().

VotingEscrow.unlock() should have an interlock or an additional call security measure

If the mentioned function is mistakenly called, the whole logic of the voting, penalties and locking system can immediately start misbehave in an irreversible way.

To prevent having further issues with such an irreversible call, I would consider either making this a two step process or maybe adding an input "magic number" that needs to be given in order to ensure that this function is meant to be called (as a way of interlock). For example:

function unlock(uint256 _interlockNumber) external {
    require(_interlockNumber == 12092018, "Wrong interlock number provided");
    require(msg.sender == owner, "Only owner");
    maxPenalty = 0;
    emit Unlock();
}

Iterator variable size can be smaller

The following lines of VotingEscrow use a uint256 i as an iterator which max evaluated constant value is 255.

• Line 309
• Line 834

Because the mentioned iterator will be at most 255, it can be declared as a uint16 instead of uint256 having type(uint16).max == 65535.

For loop iterator can be unchecked

The following lines of VotingEscrow iterate with a for loop capped at 255.

• Line 309
• Line 834

The iterator can be increased in an unchecked way in order to save gas on each iteration because the max amount of loops is hardcapped.

It can be updated as it follows in the end of each loop:

unchecked {
    ++i;
}