FIAT DAO veFDT contest - ret2basic's results

Unlock liquidity for your DeFi fixed income assets.

General Information

Platform: Code4rena

Start Date: 12/08/2022

Pot Size: $35,000 USDC

Total HM: 10

Participants: 126

Period: 3 days

Judge: Justin Goro

Total Solo HM: 3

Id: 154

League: ETH

FIAT DAO

Findings Distribution

Researcher Performance

Rank: 48/126

Findings: 2

Award: $53.56

🌟 Selected for report: 0

🚀 Solo Findings: 0

FIAT DAO veFDT Contest QA Report

Summary

The following QA issues were found during the code audit:

  1. Avoid naming collision (1 instance)
  2. Unsafe ERC20 Operations (7 instances)
  3. Unspecific Compiler Version Pragma (12 instances)
  4. File is missing NatSpec (1 instance)
  5. NatSpec is incomplete (2 instances)

Total 23 instances of 5 issues.

Avoid naming collision

block is the name of a built-in object. Naming this function "block" may create confusion.

2022-08-fiatdao/contracts/features/Blocklist.sol::23 => function block(address addr) external {

Unsafe ERC20 Operations

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.

2022-08-fiatdao/contracts/VotingEscrow.sol::426 => token.transferFrom(msg.sender, address(this), _value),

2022-08-fiatdao/contracts/VotingEscrow.sol::486 => token.transferFrom(msg.sender, address(this), _value),

2022-08-fiatdao/contracts/VotingEscrow.sol::546 => require(token.transfer(msg.sender, value), "Transfer failed");

2022-08-fiatdao/contracts/VotingEscrow.sol::657 => require(token.transfer(msg.sender, remainingAmount), "Transfer failed");

2022-08-fiatdao/contracts/VotingEscrow.sol::676 => require(token.transfer(penaltyRecipient, amount), "Transfer failed");

2022-08-fiatdao/contracts/mocks/MockSmartWallet.sol::20 => fdt.approve(ve, amount);

2022-08-fiatdao/contracts/mocks/MockSmartWallet.sol::25 => fdt.approve(ve, amount);

Unspecific Compiler Version Pragma

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.

2022-08-fiatdao/contracts/VotingEscrow.sol::2 => pragma solidity ^0.8.3;

2022-08-fiatdao/contracts/features/Blocklist.sol::2 => pragma solidity ^0.8.3;

2022-08-fiatdao/contracts/interfaces/IBlocklist.sol::2 => pragma solidity ^0.8.3;

2022-08-fiatdao/contracts/interfaces/IERC20.sol::2 => pragma solidity ^0.8.3;

2022-08-fiatdao/contracts/interfaces/IERC20Permit.sol::4 => pragma solidity ^0.8.3;

2022-08-fiatdao/contracts/interfaces/IVotingEscrow.sol::2 => pragma solidity ^0.8.3;

2022-08-fiatdao/contracts/libraries/Authorizable.sol::2 => pragma solidity >=0.7.0;

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::4 => pragma solidity ^0.8.3;

2022-08-fiatdao/contracts/libraries/ERC20PermitWithMint.sol::4 => pragma solidity ^0.8.3;

2022-08-fiatdao/contracts/libraries/ReentrancyBlock.sol::2 => pragma solidity ^0.8.3;

2022-08-fiatdao/contracts/mocks/MockERC20.sol::7 => pragma solidity ^0.8.0;

2022-08-fiatdao/contracts/mocks/MockSmartWallet.sol::2 => pragma solidity ^0.8.3;

File is missing NatSpec

Providing NatSpec is a good practice for further development / debugging etc.

File: 2022-08-fiatdao/contracts/interfaces/IERC20.sol

MatSpec is incomplete

These tags are used for contracts and interfaces:

  • @title
  • @author
  • @notice
  • @dev

These tags are used for functions:

  • @author
  • @notice
  • @dev
  • @param
  • @return

If the function has return value, it should be documented in NatSpec.

File: 2022-08-fiatdao/contracts/VotingEscrow.sol
Issue: missing @return

705:    // @dev Uses binarysearch to find the most recent point history preceeding block
706:    // @param _block Find the most recent point history before this block
707:    // @param _maxEpoch Do not search pointHistories past this index
708:    function _findBlockEpoch(uint256 _block, uint256 _maxEpoch)
709:        internal
710:        view
711:        returns (uint256)
712:    {
File: 2022-08-fiatdao/contracts/VotingEscrow.sol
Issue: missing @return

729:    // @dev Uses binarysearch to find the most recent user point history preceeding block
730:    // @param _addr User for which to search
731:    // @param _block Find the most recent point history before this block
732:    function _findUserBlockEpoch(address _addr, uint256 _block)
733:        internal
734:        view
735:        returns (uint256)
736:    {

FIAT DAO veFDT Contest Gas Optimization Report

Summary

The following gas optimization issues were found during the code audit:

  1. Use calldata instead of memory (3 instances)
  2. Use unchecked{} to suppress overflow/underflow check (4 instances)
  3. Using bools for storage incurs overhead (1 instance)
  4. Use != 0 instead of > 0 when comparing uint (4 instances)
  5. Don't initialize variables with default value (10 instances)
  6. Use ++i/--i instead of i++/i-- (4 instances)
  7. Use uint256/int256 instead of other variations (43 instances)
  8. Use abi.encodePacked() instead of abi.encode() (2 instances)
  9. Use private instead of public for constants (4 instances)
  10. Use custom errors instead of revert()/require() strings (46 instances)
  11. Use shift right/left instead of division/multiplication if possible (2 instances)
  12. Use storage instead of memory for structs/arrays saves gas (22 instances)
  13. Empty blocks should be removed (2 instances)
  14. Use a recent version of Solidity (1 instance)
  15. Multiple mappings can be combined into to a single mapping to a struct (4 instances)
  16. State variable only set in the constructor should be declared immutable (2 instances)
  17. internal functions only called once can be inlined (1 instance)
  18. Refactor duplicated require()/revert() to function modifier (9 instances)

Total 164 instances of 18 issues.

Use calldata instead of memory

When a function with a memory array is called externally, the abi.decode() step has to use a for loop to copy each index of the calldata to the memory index. This overhead can be optimized by using calldata directly.

2022-08-fiatdao/contracts/VotingEscrow.sol::685 => function _copyLock(LockedBalance memory _locked)

2022-08-fiatdao/contracts/VotingEscrow.sol::825 => function _supplyAt(Point memory _point, uint256 _t)

2022-08-fiatdao/contracts/interfaces/IERC20.sol::5 => function symbol() external view returns (string memory);

Use unchecked{} to suppress overflow/underflow check

Starting from version 0.8.0, Solidity does overflow/underflow checks by default. It is a good feature to prevent vulnerabilities but it has a significant overhead, especially when used in for loop. When using uint256/int256, it is extremely hard to trigger overflow, so it makes sense to skip these checks. To suppress the overflow/underflow checks, use unchecked {}. For increment situations, just use unchecked {} directly; for decrement situations, add a require() statement before decrementing to prevent underflow.

2022-08-fiatdao/contracts/VotingEscrow.sol::309 => for (uint256 i = 0; i < 255; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::717 => for (uint256 i = 0; i < 128; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::739 => for (uint256 i = 0; i < 128; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::834 => for (uint256 i = 0; i < 255; i++) {

Using bools for storage incurs overhead

Use uint256(1) and uint256(2) for true/false. 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.

2022-08-fiatdao/contracts/libraries/ReentrancyBlock.sol::6 => bool private _entered;

Use != 0 instead of > 0 when comparing uint

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

2022-08-fiatdao/contracts/VotingEscrow.sol::288 => if (epoch > 0) {

2022-08-fiatdao/contracts/VotingEscrow.sol::412 => require(_value > 0, "Only non zero amount");

2022-08-fiatdao/contracts/VotingEscrow.sol::448 => require(_value > 0, "Only non zero amount");

2022-08-fiatdao/contracts/features/Blocklist.sol::42 => return size > 0;

Don't initialize variables with default value

Uninitialized variables are assigned with the types default value. Explicitly initializing a variable with it's default value costs unnecesary gas.

2022-08-fiatdao/contracts/VotingEscrow.sol::298 => uint256 blockSlope = 0; // dblock/dt

2022-08-fiatdao/contracts/VotingEscrow.sol::309 => for (uint256 i = 0; i < 255; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::714 => uint256 min = 0;

2022-08-fiatdao/contracts/VotingEscrow.sol::717 => for (uint256 i = 0; i < 128; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::737 => uint256 min = 0;

2022-08-fiatdao/contracts/VotingEscrow.sol::739 => for (uint256 i = 0; i < 128; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::793 => uint256 dBlock = 0;

2022-08-fiatdao/contracts/VotingEscrow.sol::794 => uint256 dTime = 0;

2022-08-fiatdao/contracts/VotingEscrow.sol::834 => for (uint256 i = 0; i < 255; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::889 => uint256 dTime = 0;

Use ++i/--i instead of i++/i--

Using ++i/--i saves 6 gas per loop.

2022-08-fiatdao/contracts/VotingEscrow.sol::309 => for (uint256 i = 0; i < 255; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::717 => for (uint256 i = 0; i < 128; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::739 => for (uint256 i = 0; i < 128; i++) {

2022-08-fiatdao/contracts/VotingEscrow.sol::834 => for (uint256 i = 0; i < 255; i++) {

Use uint256/int256 instead of other variations

Using smaller data types such as uint8/int8 is more expensive than using uint256/int256. The EVM works with 256bit/32byte words. Every operation is based on these base units. If the data is smaller, further operations are needed to downscale from 256 bits to 8 bits, and this is more expensive than using uint256/int256.

2022-08-fiatdao/contracts/VotingEscrow.sol::60 => mapping(uint256 => int128) public slopeChanges;

2022-08-fiatdao/contracts/VotingEscrow.sol::70 => int128 bias;

2022-08-fiatdao/contracts/VotingEscrow.sol::71 => int128 slope;

2022-08-fiatdao/contracts/VotingEscrow.sol::76 => int128 amount;

2022-08-fiatdao/contracts/VotingEscrow.sol::78 => int128 delegated;

2022-08-fiatdao/contracts/VotingEscrow.sol::109 => bias: int128(0),

2022-08-fiatdao/contracts/VotingEscrow.sol::110 => slope: int128(0),

2022-08-fiatdao/contracts/VotingEscrow.sol::174 => int128 value = locked_.amount;

2022-08-fiatdao/contracts/VotingEscrow.sol::205 => int128 bias,

2022-08-fiatdao/contracts/VotingEscrow.sol::206 => int128 slope,

2022-08-fiatdao/contracts/VotingEscrow.sol::229 => int128 oldSlopeDelta = 0;

2022-08-fiatdao/contracts/VotingEscrow.sol::230 => int128 newSlopeDelta = 0;

2022-08-fiatdao/contracts/VotingEscrow.sol::239 => int128(int256(MAXTIME));

2022-08-fiatdao/contracts/VotingEscrow.sol::242 => int128(int256(_oldLocked.end - block.timestamp));

2022-08-fiatdao/contracts/VotingEscrow.sol::247 => int128(int256(MAXTIME));

2022-08-fiatdao/contracts/VotingEscrow.sol::250 => int128(int256(_newLocked.end - block.timestamp));

2022-08-fiatdao/contracts/VotingEscrow.sol::313 => int128 dSlope = 0;

2022-08-fiatdao/contracts/VotingEscrow.sol::319 => int128 biasDelta =

2022-08-fiatdao/contracts/VotingEscrow.sol::321 => int128(int256((iterativeTime - lastCheckpoint)));

2022-08-fiatdao/contracts/VotingEscrow.sol::418 => locked_.amount += int128(int256(_value));

2022-08-fiatdao/contracts/VotingEscrow.sol::420 => locked_.delegated += int128(int256(_value));

2022-08-fiatdao/contracts/VotingEscrow.sol::460 => newLocked.amount += int128(int256(_value));

2022-08-fiatdao/contracts/VotingEscrow.sol::461 => newLocked.delegated += int128(int256(_value));

2022-08-fiatdao/contracts/VotingEscrow.sol::465 => locked_.amount += int128(int256(_value));

2022-08-fiatdao/contracts/VotingEscrow.sol::472 => newLocked.delegated += int128(int256(_value));

2022-08-fiatdao/contracts/VotingEscrow.sol::533 => uint256 value = uint256(uint128(locked_.amount));

2022-08-fiatdao/contracts/VotingEscrow.sol::537 => newLocked.delegated -= int128(int256(value));

2022-08-fiatdao/contracts/VotingEscrow.sol::567 => int128 value = locked_.amount;

2022-08-fiatdao/contracts/VotingEscrow.sol::598 => int128 value,

2022-08-fiatdao/contracts/VotingEscrow.sol::639 => uint256 value = uint256(uint128(locked_.amount));

2022-08-fiatdao/contracts/VotingEscrow.sol::642 => newLocked.delegated -= int128(int256(value));

2022-08-fiatdao/contracts/VotingEscrow.sol::762 => (lastPoint.slope * int128(int256(block.timestamp - lastPoint.ts)));

2022-08-fiatdao/contracts/VotingEscrow.sol::766 => return uint256(uint128(lastPoint.bias));

2022-08-fiatdao/contracts/VotingEscrow.sol::813 => (upoint.slope * int128(int256(blockTime - upoint.ts)));

2022-08-fiatdao/contracts/VotingEscrow.sol::815 => return uint256(uint128(upoint.bias));

2022-08-fiatdao/contracts/VotingEscrow.sol::836 => int128 dSlope = 0;

2022-08-fiatdao/contracts/VotingEscrow.sol::849 => int128(int256(iterativeTime - lastPoint.ts)));

2022-08-fiatdao/contracts/VotingEscrow.sol::860 => return uint256(uint128(lastPoint.bias));

2022-08-fiatdao/contracts/interfaces/IERC20.sol::10 => function decimals() external view returns (uint8);

2022-08-fiatdao/contracts/interfaces/IERC20Permit.sol::43 => uint8 v,

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::18 => uint8 public override decimals;

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::193 => uint8 v,

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::240 => function _setupDecimals(uint8 decimals_) internal {

Use abi.encodePacked() instead of abi.encode()

abi.encodePacked() is more efficient than abi.encode().

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::60 => abi.encode(

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::204 => abi.encode(

Use private instead of public for constants

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.

2022-08-fiatdao/contracts/VotingEscrow.sol::46 => uint256 public constant WEEK = 7 days;

2022-08-fiatdao/contracts/VotingEscrow.sol::47 => uint256 public constant MAXTIME = 365 days;

2022-08-fiatdao/contracts/VotingEscrow.sol::48 => uint256 public constant MULTIPLIER = 10**18;

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::31 => bytes32 public constant PERMIT_TYPEHASH =

Use custom errors instead of revert()/require() strings

Using require()/revert() strings is expensive. Starting from Soldity v0.8.4, there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors.

Custom errors decrease both deploy and runtime gas costs. Note that runtime gas cost is only relevant when the revert condition is met.

2022-08-fiatdao/contracts/VotingEscrow.sol::116 => require(decimals <= 18, "Exceeds max decimals");

2022-08-fiatdao/contracts/VotingEscrow.sol::140 => require(msg.sender == owner, "Only owner");

2022-08-fiatdao/contracts/VotingEscrow.sol::147 => require(msg.sender == owner, "Only owner");

2022-08-fiatdao/contracts/VotingEscrow.sol::154 => require(msg.sender == owner, "Only owner");

2022-08-fiatdao/contracts/VotingEscrow.sol::162 => require(msg.sender == owner, "Only owner");

2022-08-fiatdao/contracts/VotingEscrow.sol::171 => require(msg.sender == blocklist, "Only Blocklist");

2022-08-fiatdao/contracts/VotingEscrow.sol::412 => require(_value > 0, "Only non zero amount");

2022-08-fiatdao/contracts/VotingEscrow.sol::413 => require(locked_.amount == 0, "Lock exists");

2022-08-fiatdao/contracts/VotingEscrow.sol::414 => require(unlock_time >= locked_.end, "Only increase lock end"); // from using quitLock, user should increaseAmount instead

2022-08-fiatdao/contracts/VotingEscrow.sol::415 => require(unlock_time > block.timestamp, "Only future lock end");

2022-08-fiatdao/contracts/VotingEscrow.sol::416 => require(unlock_time <= block.timestamp + MAXTIME, "Exceeds maxtime");

2022-08-fiatdao/contracts/VotingEscrow.sol::448 => require(_value > 0, "Only non zero amount");

2022-08-fiatdao/contracts/VotingEscrow.sol::449 => require(locked_.amount > 0, "No lock");

2022-08-fiatdao/contracts/VotingEscrow.sol::450 => require(locked_.end > block.timestamp, "Lock expired");

2022-08-fiatdao/contracts/VotingEscrow.sol::469 => require(locked_.amount > 0, "Delegatee has no lock");

2022-08-fiatdao/contracts/VotingEscrow.sol::470 => require(locked_.end > block.timestamp, "Delegatee lock expired");

2022-08-fiatdao/contracts/VotingEscrow.sol::502 => require(locked_.amount > 0, "No lock");

2022-08-fiatdao/contracts/VotingEscrow.sol::503 => require(unlock_time > locked_.end, "Only increase lock end");

2022-08-fiatdao/contracts/VotingEscrow.sol::504 => require(unlock_time <= block.timestamp + MAXTIME, "Exceeds maxtime");

2022-08-fiatdao/contracts/VotingEscrow.sol::511 => require(oldUnlockTime > block.timestamp, "Lock expired");

2022-08-fiatdao/contracts/VotingEscrow.sol::529 => require(locked_.amount > 0, "No lock");

2022-08-fiatdao/contracts/VotingEscrow.sol::530 => require(locked_.end <= block.timestamp, "Lock not expired");

2022-08-fiatdao/contracts/VotingEscrow.sol::531 => require(locked_.delegatee == msg.sender, "Lock delegated");

2022-08-fiatdao/contracts/VotingEscrow.sol::546 => require(token.transfer(msg.sender, value), "Transfer failed");

2022-08-fiatdao/contracts/VotingEscrow.sol::563 => require(!IBlocklist(blocklist).isBlocked(_addr), "Blocked contract");

2022-08-fiatdao/contracts/VotingEscrow.sol::564 => require(locked_.amount > 0, "No lock");

2022-08-fiatdao/contracts/VotingEscrow.sol::565 => require(locked_.delegatee != _addr, "Already delegated");

2022-08-fiatdao/contracts/VotingEscrow.sol::587 => require(toLocked.amount > 0, "Delegatee has no lock");

2022-08-fiatdao/contracts/VotingEscrow.sol::588 => require(toLocked.end > block.timestamp, "Delegatee lock expired");

2022-08-fiatdao/contracts/VotingEscrow.sol::589 => require(toLocked.end >= fromLocked.end, "Only delegate to longer lock");

2022-08-fiatdao/contracts/VotingEscrow.sol::635 => require(locked_.amount > 0, "No lock");

2022-08-fiatdao/contracts/VotingEscrow.sol::636 => require(locked_.end > block.timestamp, "Lock expired");

2022-08-fiatdao/contracts/VotingEscrow.sol::637 => require(locked_.delegatee == msg.sender, "Lock delegated");

2022-08-fiatdao/contracts/VotingEscrow.sol::657 => require(token.transfer(msg.sender, remainingAmount), "Transfer failed");

2022-08-fiatdao/contracts/VotingEscrow.sol::676 => require(token.transfer(penaltyRecipient, amount), "Transfer failed");

2022-08-fiatdao/contracts/VotingEscrow.sol::776 => require(_blockNumber <= block.number, "Only past block number");

2022-08-fiatdao/contracts/VotingEscrow.sol::877 => require(_blockNumber <= block.number, "Only past block number");

2022-08-fiatdao/contracts/features/Blocklist.sol::24 => require(msg.sender == manager, "Only manager");

2022-08-fiatdao/contracts/features/Blocklist.sol::25 => require(_isContract(addr), "Only contracts");

2022-08-fiatdao/contracts/libraries/Authorizable.sol::19 => require(msg.sender == owner, "Sender not owner");

2022-08-fiatdao/contracts/libraries/Authorizable.sol::25 => require(isAuthorized(msg.sender), "Sender not Authorized");

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::104 => require(balance >= amount, "ERC20: insufficient-balance");

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::114 => require(allowed >= amount, "ERC20: insufficient-allowance");

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::216 => require(owner != address(0), "ERC20: invalid-address-0");

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::218 => require(owner == ecrecover(digest, v, r, s), "ERC20: invalid-permit");

2022-08-fiatdao/contracts/libraries/ReentrancyBlock.sol::10 => require(!_entered, "Reentrancy");

Use shift right/left instead of division/multiplication if possible

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.

2022-08-fiatdao/contracts/VotingEscrow.sol::719 => uint256 mid = (min + max + 1) / 2;

2022-08-fiatdao/contracts/VotingEscrow.sol::743 => uint256 mid = (min + max + 1) / 2;

Use storage instead of memory for structs/arrays saves gas

When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct.

2022-08-fiatdao/contracts/VotingEscrow.sol::172 => LockedBalance memory locked_ = locked[_addr];

2022-08-fiatdao/contracts/VotingEscrow.sol::214 => Point memory point = userPointHistory[_addr][uepoch];

2022-08-fiatdao/contracts/VotingEscrow.sol::281 => Point memory lastPoint =

2022-08-fiatdao/contracts/VotingEscrow.sol::296 => Point memory initialLastPoint =

2022-08-fiatdao/contracts/VotingEscrow.sol::410 => LockedBalance memory locked_ = locked[msg.sender];

2022-08-fiatdao/contracts/VotingEscrow.sol::446 => LockedBalance memory locked_ = locked[msg.sender];

2022-08-fiatdao/contracts/VotingEscrow.sol::499 => LockedBalance memory locked_ = locked[msg.sender];

2022-08-fiatdao/contracts/VotingEscrow.sol::512 => LockedBalance memory oldLocked = _copyLock(locked_);

2022-08-fiatdao/contracts/VotingEscrow.sol::527 => LockedBalance memory locked_ = locked[msg.sender];

2022-08-fiatdao/contracts/VotingEscrow.sol::534 => LockedBalance memory newLocked = _copyLock(locked_);

2022-08-fiatdao/contracts/VotingEscrow.sol::561 => LockedBalance memory locked_ = locked[msg.sender];

2022-08-fiatdao/contracts/VotingEscrow.sol::601 => LockedBalance memory newLocked = _copyLock(_locked);

2022-08-fiatdao/contracts/VotingEscrow.sol::633 => LockedBalance memory locked_ = locked[msg.sender];

2022-08-fiatdao/contracts/VotingEscrow.sol::640 => LockedBalance memory newLocked = _copyLock(locked_);

2022-08-fiatdao/contracts/VotingEscrow.sol::759 => Point memory lastPoint = userPointHistory[_owner][epoch];

2022-08-fiatdao/contracts/VotingEscrow.sol::783 => Point memory upoint = userPointHistory[_owner][userEpoch];

2022-08-fiatdao/contracts/VotingEscrow.sol::788 => Point memory point0 = pointHistory[epoch];

2022-08-fiatdao/contracts/VotingEscrow.sol::796 => Point memory point1 = pointHistory[epoch + 1];

2022-08-fiatdao/contracts/VotingEscrow.sol::830 => Point memory lastPoint = _point;

2022-08-fiatdao/contracts/VotingEscrow.sol::866 => Point memory lastPoint = pointHistory[epoch_];

2022-08-fiatdao/contracts/VotingEscrow.sol::882 => Point memory point = pointHistory[targetEpoch];

2022-08-fiatdao/contracts/VotingEscrow.sol::891 => Point memory pointNext = pointHistory[targetEpoch + 1];

Empty blocks should be removed

Empty blocks exist in the code. The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting.

2022-08-fiatdao/contracts/libraries/ERC20Permit.sol::73 => function _extraConstruction() internal virtual {}

2022-08-fiatdao/contracts/mocks/MockERC20.sol::14 => ) ERC20PermitWithMint(name_, symbol_, owner_) {}

Use a recent version of Solidity

New version of Solidity provides bug fix / gas optimization that old versions don't have.

2022-08-fiatdao/contracts/libraries/Authorizable.sol::2 => pragma solidity >=0.7.0;

Multiple mappings can be combined into to a single mapping to a struct

Combining multiple mappings into a single mapping to a struct saves storage slots. 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.

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::58 => mapping(address => Point[1000000000]) public userPointHistory;

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::59 => mapping(address => uint256) public userPointEpoch;

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::60 => mapping(uint256 => int128) public slopeChanges;

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::61 => mapping(address => LockedBalance) public locked;

State variable only set in the constructor should be declared immutable

Declaring such state variables as immutable avoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas).

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::64 => string public name;

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::65 => string public symbol;

internal functions only called once can be inlined

Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.

File: 2022-08-fiatdao/contracts/VotingEscrow.sol

662:    function _calculatePenaltyRate(uint256 end)
663:        internal
664:        view
665:        returns (uint256)
666:    {
667:        // We know that end > block.timestamp because expired locks cannot be quitted
668:        return ((end - block.timestamp) * maxPenalty) / MAXTIME;
669:    }

Refactor duplicated require()/revert() to function modifier

Using function modifiers is cheaper than having multiple require()/revert() statements.

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::140 => require(msg.sender == owner, "Only owner");

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::147 => require(msg.sender == owner, "Only owner");

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::154 => require(msg.sender == owner, "Only owner");

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::162 => require(msg.sender == owner, "Only owner");

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::449 => require(locked_.amount > 0, "No lock");

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::502 => require(locked_.amount > 0, "No lock");

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::529 => require(locked_.amount > 0, "No lock");

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::564 => require(locked_.amount > 0, "No lock");

File: 2022-08-fiatdao/contracts/VotingEscrow.sol::635 => require(locked_.amount > 0, "No lock");

#0 - lacoop6tu

2022-08-26T15:35:50Z

Good one

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter