[Code review: Contract Bugs]: L1WrappedStakedTONFactory creates a contract that cannot be upgraded

[Zena-park]

Describe the bug

Looking at the composition of the L1 contract, L1WrappedStakedTONFactory appears to be a contract that creates an L1WrappedStakedTON contract. It appears that the L1WrappedStakedTON contract was intended to be configured as a Proxy so that it can be upgraded.

If so, L1WrappedStakedTONFactory should deploy the proxy and set the logic to L1WrappedStakedTON. Currently, the contract created by L1WrappedStakedTONFactory is not a proxy configuration, so it creates a contract that cannot be upgraded.

Impact L1WrappedStakedTON contracts deployed with L1WrappedStakedTONFactory cannot be upgraded.

Exploit Scenario

Recommendation

• Current

  https://github.com/tokamak-network/gem-nft-contract/blob/b31e833a38901bee6287b869d34c523bfe13dea2/src/L1/L1WrappedStakedTONFactory.sol#L4

  https://github.com/tokamak-network/gem-nft-contract/blob/b31e833a38901bee6287b869d34c523bfe13dea2/src/L1/L1WrappedStakedTONFactory.sol#L28-L35

  • Recommended : This code only provides direction, so I will omit modifiers and such and only provide a guide.

  
  /// (1) The contract being created must be a proxy.

import { Proxy } from "Proxy.sol";

/// (2)Add Logic Storage /// The logic address must be received and stored separately.

address public logicAddress ;

function setLogicAddress(address _logic) external onlyOwner { logicAddress = _logic; }

/// (3) Deploy the proxy, upgrade the logic, and initialize it.

Proxy wston = new Proxy(); wston.upgradeTo(logicAddress); wston.initialize(_depositManager, _seigManager , _layer2Address,_wton ); wston.transferOwner(_admin); // Using the Proxy's owner change function

[mehdi-defiesta]

Thanks @Zena-park. I switched from a custom proxy to openzeppelin UUPS. Here is L1WrappedStakedTON contract. I've updated the factory to create a new ERC1967 proxy.

[mehdi-defiesta]

I have assign this issue to a "LOW severity" but you can put it as a "MEDIUM" if you find it right.

[Zena-park]

I don't mind, thank you!

[mehdi-defiesta]

fixed in the following commit

[Zena-park]

I confirmed. Thank you.