Implementation contracts inherit from `openzeppelin-contracts` instead of `openzeppelin-contracts-upgradeable`

[hats-bug-reporter[bot]]

Github username: @nuthan2x Twitter username: nuthan2x Submission hash (on-chain): 0x57d808816e69c11afc16d28b236471eb09cf258fb1d89ade847eb191bb5b918b Severity: low

Description: Description\ The issue doen't affect current system, but it goes against the standard of proxy and openzeppelin contracts implementation Due to the clones proxy pattern, the implemenattion contracts should inherit from openzeppelin-contracts-upgradeable instead of openzeppelin-contracts

Attack Scenario\

abstract contract CatalystVaultCommon is
    Initializable,
    Multicall,
    ReentrancyGuard, // @audit should use reentrancy guard upgradable
    ERC20,
    ICatalystV1Vault
{

• The CatalystVaultCommon is inherited by volatile and amplified vaults, and they are deployed as implementations and used in cloning for proxies.

• But the vault inherits plain ReentrancyGuard and ERC20 contracts instead of ReentrancyGuardUpgradeable and ERC20Upgradeable

• The main difference between these are, instead of using constructor to set STATUS of reentrancy guard, __ReentrancyGuard_init_unchained will be used to set the initial status. And its same in ERC20 where name and symbol, are updated in initializer functions instead of constructor, due to the proxy patterns.

• Look at the logs, where initally the STATUS will be set to 1 while deployment itself.

• But the status is not 1 while cloning a proxy of that implementation.

• But there's no issue, because the status is changed to 1 after a function with non-reentrant is called with clone.

• Also, the name and symbols of ERC20 are updated in initializer functions while cloning the proxy itself here. So its not an issue.

[PASS] testRe() (gas: 196304)
Logs:
  0x0000000000000000000000000000000000000000000000000000000000000001
  true
  0x0000000000000000000000000000000000000000000000000000000000000001
  ................................................
  0x0000000000000000000000000000000000000000000000000000000000000000
  true
  0x0000000000000000000000000000000000000000000000000000000000000001

Attachments

1. Proof of Concept (PoC) File

   1. paste this gist file to test/ folder
   2. run forge t --mt testRe -vvvv

2. Revised Code File (Optional) will post in comments section of github issue

[reednaa]

For this issue we would require a fix that follows the standard.

[nuthan2x]

Modify CatalystVaultCommon.sol as https://gist.github.com/nuthan2x/0989f4ec2bf3bd91cd3e2f25034e3a0e