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

2 stars 1 forks source link

QA Report #186

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

USE A MORE RECENT VERSION OF SOLIDITY

Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value

the blocklist.sol and VotingEscrow all use solidty version 0.8.3

Use Openzeppelin ownable to manage the ownership.

Use https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable.sol to manage the ownership save from passing owner into constructor and use require to check in every critcal function.

We can just use onlyOwner modifier.

Use SafeERC20 instead of IERC20 interface.

safeERC20 take care of transfer return value check so

 require(token.transfer(msg.sender, value), "Transfer failed");

in lines below is no longer needed.

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L676

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L546

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L657

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L767

is no longer needed.