retocrooman
commented
2 years ago 1.Ensure that the ‘to’! =address(0) and ! =_rescuer
There is no problem because only we use rescue ERC20.
2.Imports are already outdated.
We need to check if these are ture. If they are true we will fix them.
3.Use addition/subtraction assignment(+=/-=) for better readability 4.Default value doesn't need to be set
Duplicate of (#2)
1.Ensure that the ‘to’! =address(0) and ! =_rescuer :
https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/v1/Rescuable.sol#L65
2.Imports are already outdated.
Ensure that you refrain from copying and pasting from the OZ contracts directly. This will prevent the contracts from being adequately maintained since OZ updates these files regularly. Consider, importing directly from the OZ-contracts.
See: https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/v1/FiatTokenV1.sol#L34
https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/upgradeability/UUPSUpgradeable.sol#L2
compared to : https://github.com/OpenZeppelin/openzeppelin-contracts/blob/3e74681e779e74391d4116dedfcd77aadfe1579c/contracts/proxy/utils/UUPSUpgradeable.sol#L2
NB. This is to be done for :
(https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v1/Rescuable.sol#L27-L29)
(https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v1/FiatTokenV1.sol#L27-L28)
(https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L27-L28)
NB. All the folders that include the OZ contracts should be imports.
3.Use addition/subtraction assignment(+=/-=) for better readability
Instead of totalSupply=totalSupply+amount. Use totalSupply_+=amount.
Apply to : (https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v1/FiatTokenV1.sol#L144-L145)
(https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L151-L152)
(https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L326)
(https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/v2/FiatTokenV2.sol#L381)
(https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v1/FiatTokenV1.sol#L316)
(https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v1/FiatTokenV1.sol#L371)
4.Default value doesn't need to be set
Variables are already the default, no need to assign them if they will be assigned later. For eg, a declared address is address(0) before assignment and uints are already 0.
See:
(https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v1/FiatTokenV1.sol#L58)
(https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L58)