JPYC contest - peritoflores's results

QA (Low and NC) report to JPYC by PeritoFlores

Transfer Ownership pattern incomplete implementation

In function _transferOwnership#Ownable.sol

function _transferOwnership(address newOwner) internal virtual { @audit using transfer ownership
    address oldOwner = _owner;
    _owner = newOwner;
    emit OwnershipTransferred(oldOwner, newOwner);
}

It seems that you tried to implement two-step ownership transfer to make it safer. It is a good idea to add the function acceptOwnership() to complete the pattern.

Rescuer uninialitized at FiatTokenV2

In the function initialize()#FiatTokenV2.sol rescuer in never initialized so it is zero at deployment time.

Consider calling updateRescuer() inside initialize()

Lack emit event after rescueERC20()

In rescuer.sol the function rescueERC20() lacks an event emit.

function rescueERC20(
    IERC20 tokenContract,
    address to,
    uint256 amount
) external onlyRescuer {
    tokenContract.safeTransfer(to, amount);  @audit  emit event  
}

Consider adding it

Typo error

At EIP3009.sol#L227 there is a typo error

`"FEIP3009: authorization is not yet valid"  `

It says "FEIP" instead of "EIP"

Consider using inheritance when using upgradeable contracts

When using upgradeable contracts it is recommended that the new version inherits from previous. One of the reason for that is that you avoid storage collision. A minimum error in the order of parameters on the new version could break the protocol.

Maybe you can consider for the next version something like

contract FiatTokenV3 is FiatTokenV2

Gas Optimization for JPYC by PeritoFlores

Unnecessary initialization of some variables

The following variables are initialized to zero. Consider removing .

https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/v1/Pausable.sol#L49

https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/v1/FiatTokenV1.sol#L58

https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/v2/FiatTokenV2.sol#L58

Use "delete" keyword instead to assigning to zero

minterAllowed[minter] = 0;              FiatTokenV2.sol#L361

Replace for

delete(minterAllowed[minter]);

Same issue at FiatTokenV1#L350