FIAT DAO veFDT contest - apostle0x01's results

[L-01] Unsafe ERC20 approve operation

Impact

ERC20 operations can be unsafe due to different implementations and vulnerabilities in the standard.

It is therefore recommended to always either use OpenZeppelin's SafeERC20 library or at least to wrap each operation in a require statement.

To circumvent ERC20's approve functions race-condition vulnerability use OpenZeppelin's SafeERC20 library's safe{Increase|Decrease}Allowance functions.

Findings:

contracts/mocks/MockSmartWallet.sol::20 => fdt.approve(ve, amount);
contracts/mocks/MockSmartWallet.sol::25 => fdt.approve(ve, amount);

[L-02] Unspecific Compiler Version Pragma

Impact

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall-back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

Findings:

contracts/VotingEscrow.sol::2 => pragma solidity ^0.8.3;
contracts/features/Blocklist.sol::2 => pragma solidity ^0.8.3;
contracts/interfaces/IBlocklist.sol::2 => pragma solidity ^0.8.3;
contracts/interfaces/IERC20.sol::2 => pragma solidity ^0.8.3;
contracts/interfaces/IVotingEscrow.sol::2 => pragma solidity ^0.8.3;
contracts/mocks/MockERC20.sol::7 => pragma solidity ^0.8.0;
contracts/mocks/MockSmartWallet.sol::2 => pragma solidity ^0.8.3;

[G-01] Don't Initialize Variables with Default Value

Impact

Uninitialized variables are assigned with the types default value.

Explicitly initializing a variable with it's default value costs unnecesary gas.

Findings:

contracts/VotingEscrow.sol::298 => uint256 blockSlope = 0; // dblock/dt
contracts/VotingEscrow.sol::309 => for (uint256 i = 0; i < 255; i++) {
contracts/VotingEscrow.sol::714 => uint256 min = 0;
contracts/VotingEscrow.sol::717 => for (uint256 i = 0; i < 128; i++) {
contracts/VotingEscrow.sol::737 => uint256 min = 0;
contracts/VotingEscrow.sol::739 => for (uint256 i = 0; i < 128; i++) {
contracts/VotingEscrow.sol::793 => uint256 dBlock = 0;
contracts/VotingEscrow.sol::794 => uint256 dTime = 0;
contracts/VotingEscrow.sol::834 => for (uint256 i = 0; i < 255; i++) {
contracts/VotingEscrow.sol::889 => uint256 dTime = 0;

[G-02] Use != 0 instead of > 0 for Unsigned Integer Comparison

Impact

When dealing with unsigned integer types, comparisons with != 0 are cheaper then with > 0.

Findings:

contracts/VotingEscrow.sol::412 => require(_value > 0, "Only non zero amount");
contracts/VotingEscrow.sol::448 => require(_value > 0, "Only non zero amount");
contracts/VotingEscrow.sol::449 => require(locked_.amount > 0, "No lock");
contracts/VotingEscrow.sol::469 => require(locked_.amount > 0, "Delegatee has no lock");
contracts/VotingEscrow.sol::502 => require(locked_.amount > 0, "No lock");
contracts/VotingEscrow.sol::529 => require(locked_.amount > 0, "No lock");
contracts/VotingEscrow.sol::564 => require(locked_.amount > 0, "No lock");
contracts/VotingEscrow.sol::587 => require(toLocked.amount > 0, "Delegatee has no lock");
contracts/VotingEscrow.sol::635 => require(locked_.amount > 0, "No lock");

[G-03] Use Shift Right/Left instead of Division/Multiplication if possible

Impact

A division/multiplication by any number x being a power of 2 can be calculated by shifting log2(x) to the right/left.

While the DIV opcode uses 5 gas, the SHR opcode only uses 3 gas. Furthermore, Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting.

Findings:

contracts/VotingEscrow.sol::719 => uint256 mid = (min + max + 1) / 2;
contracts/VotingEscrow.sol::743 => uint256 mid = (min + max + 1) / 2;