JPYC contest - kyliek's results

#1 State variable shadowing inside functions

Ownable.owner has supreme power in this contract. There are many functions that have local variables using the same name as owner, hence shadowing this important state variable.

Functions that have this issue are, in FiatTokenV1,

allowance
_approve
_increaseAllowance
_decreaseAllowance
_permit

To mitigate this issue, change local variable name from owner to _owner

#2. NatSpec Comments missing @return

Functions that have this issue are, in FiatTokenV1,

isMinter
balanceOf

#3 Typos

 Blocklist => Blacklist 
 Blocklistable => Blacklistable 
 Blocklister => Blacklister 

#4 Reimplementation of OpenZeppline ERC20, Pausable

ERC20 standard is reimplemented here. I wonder why OZ implementation of ERC20 is not used.

#5 Use AccessControl instead of Ownable

There are many roles in this contract, including minter, masterMinter, owner, Pauser, rescuer and functions require more granular role-based access control. Thus, @openzeppelin/contracts/access/AccessControl.sol could be more suitable and simpler implementation than using @openzeppelin/contracts/access/Ownable.sol.

#6 Not using the latest version of UUPSUpgradeable The latest version of UUPSUpgradeable contract is v4.5 and the contract uses v4.4.1

#7 Inherit @openzeppelin/contracts-upgradeable library It could be less risky in inheriting the upgradeable version of openzeppelin library, particularly initializable, ERC20Upgradeable, 'PausableUpgradeable', 'OwnableUpgradeable'. Upgradability pattern could be complex and prone to attacks.

#8 public function can be external Instances are FiatTokenV1.initialize, FiatTokenV2.initialize