QA Report

[code423n4]

Forked files should have changelog

Your version of Pausable.sol does a good job of saying the exact source of the fork as well as most of the changes

/**
 * @notice Base contract which allows children to implement an emergency stop
 * mechanism
 * @dev Forked from https://github.com/OpenZeppelin/openzeppelin-contracts/blob/feb665136c0dae9912e08397c1a21c4af3651ef3/contracts/lifecycle/Pausable.sol
 * Modifications:
 * 1. Added pauser role, switched pause/unpause to be onlyPauser (6/14/2018)
 * 2. Removed whenNotPause/whenPaused from pause/unpause (6/14/2018)
 * 3. Removed whenPaused (6/14/2018)
 * 4. Switches ownable library to use ZeppelinOS (7/12/18)
 * 5. Remove constructor (7/13/18)
 * 6. Reformat, conform to Solidity 0.6 syntax and add error messages (5/13/20)
 * 7. Make public functions external (5/27/20)
 */

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

There are a lot of other files that do not have such changelogs

$ find ./contracts -name "*.sol" | xargs grep -li openzeppelin | xargs grep -iL "forked from"
./contracts/proxy/ERC1967Proxy.sol
./contracts/proxy/Proxy.sol
./contracts/util/SafeERC20.sol
./contracts/util/Context.sol
./contracts/util/ECRecover.sol
./contracts/util/Address.sol
./contracts/util/IERC20.sol
./contracts/test/ERC20.sol
./contracts/test/IERC20Metadata.sol
./contracts/upgradeability/ERC1967Upgrade.sol
./contracts/upgradeability/UUPSUpgradeable.sol
./contracts/upgradeability/draft-IERC1822.sol
./contracts/v1/Ownable.sol

For example, Ownable has one of its functions removed, and all of the files have __gap storage variables added. Having a changelog will make upgrading again easier, and will make security reviews more thorough.

Not all contracts have added a __gap variable

AbstractFiatTokenV1.sol is missing a __gap storage variable. Consider adding one so that storage can be used in this contract in future versions https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v1/AbstractFiatTokenV1.sol

Update other sensitive terms

I see that there has been an attempt to get rid of sensitive terms such as 'blacklist' by using 'blocklist'. The contracts still use some of the others, and I would suggest changing 'whitelist' to 'allowlist', and 'masterMinter' to 'primaryMinter' or 'minterAdmin' https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol

[thurendous]

All of 3 are valid.

No. 1 Forked files should have changelog Thank you for pointing out. No. 2 Not all contracts have added a __gap variable We would add gap in abstract contract. No. 3 Update other sensitive terms Thank you for pointing out. We actually already have plans to change these things.

[thurendous]

AbstractFiatTokenV1.sol is missing a __gap storage variable.

Fixed and thanks.

Final change can be viewed here.

[thurendous]

Update other sensitive terms

Forked files should have changelog

Fixed and thanks.