From ed9e493f3f1c1c2765287e297075d2fb738dcc6a Mon Sep 17 00:00:00 2001 From: Iain Nash Date: Wed, 27 Sep 2023 11:59:51 -0400 Subject: [PATCH] Seperate upgrade gate new contract (#204) * wip * wip * fix tests * chore: run lint & update natspec * update storage layout --------- Co-authored-by: Rohan Kulkarni --- .storage-layout | 20 ++++++------ script/ZoraDeployerBase.sol | 2 +- src/deployment/DeploymentConfig.sol | 3 +- src/factory/ZoraCreator1155FactoryImpl.sol | 3 +- ...anagedUpgradeGate.sol => IUpgradeGate.sol} | 5 ++- src/nft/ZoraCreator1155Impl.sol | 10 +++--- ...ManagedUpgradeGate.sol => UpgradeGate.sol} | 32 ++++++++++++++----- ...StorageV1.sol => UpgradeGateStorageV1.sol} | 2 +- test/factory/ZoraCreator1155Factory.t.sol | 8 +++-- test/mock/MockUpgradeGate.sol | 10 ------ test/mock/MockUpgradeGateBase.sol | 10 ------ test/nft/ZoraCreator1155.t.sol | 12 +++---- ...gedUpgradeGate.t.sol => UpgradeGate.t.sol} | 9 +++--- 13 files changed, 63 insertions(+), 63 deletions(-) rename src/interfaces/{IFactoryManagedUpgradeGate.sol => IUpgradeGate.sol} (91%) rename src/upgrades/{FactoryManagedUpgradeGate.sol => UpgradeGate.sol} (58%) rename src/upgrades/{FactoryManagedUpgradeGateStorageV1.sol => UpgradeGateStorageV1.sol} (75%) delete mode 100644 test/mock/MockUpgradeGate.sol delete mode 100644 test/mock/MockUpgradeGateBase.sol rename test/upgrades/{FactoryManagedUpgradeGate.t.sol => UpgradeGate.t.sol} (87%) diff --git a/.storage-layout b/.storage-layout index c8795d895..c367a5bc0 100644 --- a/.storage-layout +++ b/.storage-layout @@ -40,14 +40,12 @@ ➡ ZoraCreator1155FactoryImpl ======================= -| Name | Type | Slot | Offset | Bytes | Contract | -|------------------|----------------------------------------------|------|--------|-------|-----------------------------------------------------------------------| -| _owner | address | 0 | 0 | 20 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | -| _pendingOwner | address | 1 | 0 | 20 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | -| __gap | uint256[50] | 2 | 0 | 1600 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | -| _initialized | uint8 | 52 | 0 | 1 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | -| _initializing | bool | 52 | 1 | 1 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | -| isAllowedUpgrade | mapping(address => mapping(address => bool)) | 53 | 0 | 32 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | -| __gap | uint256[50] | 54 | 0 | 1600 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | -| __gap | uint256[50] | 104 | 0 | 1600 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | -| __gap | uint256[50] | 154 | 0 | 1600 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | +| Name | Type | Slot | Offset | Bytes | Contract | +|---------------|-------------|------|--------|-------|-----------------------------------------------------------------------| +| _owner | address | 0 | 0 | 20 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | +| _pendingOwner | address | 1 | 0 | 20 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | +| __gap | uint256[50] | 2 | 0 | 1600 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | +| _initialized | uint8 | 52 | 0 | 1 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | +| _initializing | bool | 52 | 1 | 1 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | +| __gap | uint256[50] | 53 | 0 | 1600 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | +| __gap | uint256[50] | 103 | 0 | 1600 | src/factory/ZoraCreator1155FactoryImpl.sol:ZoraCreator1155FactoryImpl | diff --git a/script/ZoraDeployerBase.sol b/script/ZoraDeployerBase.sol index c69ed7771..797222059 100644 --- a/script/ZoraDeployerBase.sol +++ b/script/ZoraDeployerBase.sol @@ -16,7 +16,7 @@ import {ZoraCreator1155PremintExecutor} from "../src/delegation/ZoraCreator1155P import {IMinter1155} from "../src/interfaces/IMinter1155.sol"; /// @notice Deployment drops for base where -abstract contract ZoraDeployerBase is ScriptDeploymentConfig, Script { +abstract contract ZoraDeployerBase is ScriptDeploymentConfig { using stdJson for string; /// @notice File used for demo metadata on verification test mint diff --git a/src/deployment/DeploymentConfig.sol b/src/deployment/DeploymentConfig.sol index 348aae27d..2e9dfb82c 100644 --- a/src/deployment/DeploymentConfig.sol +++ b/src/deployment/DeploymentConfig.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.17; import "forge-std/Test.sol"; import {CommonBase} from "forge-std/Base.sol"; import {Strings} from "@openzeppelin/contracts/utils/Strings.sol"; +import {Script} from "forge-std/Script.sol"; /// @notice Chain configuration for constants set manually during deploy. Does not get written to after deploys. struct ChainConfig { @@ -37,7 +38,7 @@ struct Deployment { address preminter; } -abstract contract DeploymentConfig is CommonBase { +abstract contract DeploymentConfig is Script { using stdJson for string; /// @notice ChainID convenience getter diff --git a/src/factory/ZoraCreator1155FactoryImpl.sol b/src/factory/ZoraCreator1155FactoryImpl.sol index 1f48a75b0..ef00580e6 100644 --- a/src/factory/ZoraCreator1155FactoryImpl.sol +++ b/src/factory/ZoraCreator1155FactoryImpl.sol @@ -10,7 +10,6 @@ import {ICreatorRoyaltiesControl} from "../interfaces/ICreatorRoyaltiesControl.s import {IMinter1155} from "../interfaces/IMinter1155.sol"; import {IContractMetadata} from "../interfaces/IContractMetadata.sol"; import {Ownable2StepUpgradeable} from "../utils/ownable/Ownable2StepUpgradeable.sol"; -import {FactoryManagedUpgradeGate} from "../upgrades/FactoryManagedUpgradeGate.sol"; import {Zora1155} from "../proxies/Zora1155.sol"; import {Create2Upgradeable} from "@zoralabs/openzeppelin-contracts-upgradeable/contracts/utils/Create2Upgradeable.sol"; import {CREATE3} from "solmate/src/utils/CREATE3.sol"; @@ -19,7 +18,7 @@ import {ContractVersionBase} from "../version/ContractVersionBase.sol"; /// @title ZoraCreator1155FactoryImpl /// @notice Factory contract for creating new ZoraCreator1155 contracts -contract ZoraCreator1155FactoryImpl is IZoraCreator1155Factory, ContractVersionBase, FactoryManagedUpgradeGate, UUPSUpgradeable, IContractMetadata { +contract ZoraCreator1155FactoryImpl is IZoraCreator1155Factory, Ownable2StepUpgradeable, ContractVersionBase, UUPSUpgradeable, IContractMetadata { IZoraCreator1155 public immutable implementation; IMinter1155 public immutable merkleMinter; diff --git a/src/interfaces/IFactoryManagedUpgradeGate.sol b/src/interfaces/IUpgradeGate.sol similarity index 91% rename from src/interfaces/IFactoryManagedUpgradeGate.sol rename to src/interfaces/IUpgradeGate.sol index 08cdaee96..4f6a2a734 100644 --- a/src/interfaces/IFactoryManagedUpgradeGate.sol +++ b/src/interfaces/IUpgradeGate.sol @@ -2,7 +2,10 @@ pragma solidity 0.8.17; /// @notice Factory Upgrade Gate Admin Factory Implementation – Allows specific contract upgrades as a safety measure -interface IFactoryManagedUpgradeGate { +interface IUpgradeGate { + /// @notice Event emitted when upgrade gate is emitted + event UpgradeGateSetup(); + /// @notice If an implementation is registered by the Builder DAO as an optional upgrade /// @param baseImpl The base implementation address /// @param upgradeImpl The upgrade implementation address diff --git a/src/nft/ZoraCreator1155Impl.sol b/src/nft/ZoraCreator1155Impl.sol index c03e79c3b..7f49525bb 100644 --- a/src/nft/ZoraCreator1155Impl.sol +++ b/src/nft/ZoraCreator1155Impl.sol @@ -23,7 +23,7 @@ import {ICreatorCommands} from "../interfaces/ICreatorCommands.sol"; import {IMinter1155} from "../interfaces/IMinter1155.sol"; import {IRenderer1155} from "../interfaces/IRenderer1155.sol"; import {ITransferHookReceiver} from "../interfaces/ITransferHookReceiver.sol"; -import {IFactoryManagedUpgradeGate} from "../interfaces/IFactoryManagedUpgradeGate.sol"; +import {IUpgradeGate} from "../interfaces/IUpgradeGate.sol"; import {IZoraCreator1155} from "../interfaces/IZoraCreator1155.sol"; import {LegacyNamingControl} from "../legacy-naming/LegacyNamingControl.sol"; import {PublicMulticall} from "../utils/PublicMulticall.sol"; @@ -67,15 +67,15 @@ contract ZoraCreator1155Impl is /// @notice This user role allows for only withdrawing funds and setting funds withdraw address uint256 public constant PERMISSION_BIT_FUNDS_MANAGER = 2 ** 5; /// @notice Factory contract - IFactoryManagedUpgradeGate internal immutable factory; + IUpgradeGate internal immutable upgradeGate; constructor( uint256, // TODO remove address _mintFeeRecipient, - address _factory, + address _upgradeGate, address _protocolRewards ) ERC1155Rewards(_protocolRewards, _mintFeeRecipient) initializer { - factory = IFactoryManagedUpgradeGate(_factory); + upgradeGate = IUpgradeGate(_upgradeGate); } /// @notice Initializes the contract @@ -771,7 +771,7 @@ contract ZoraCreator1155Impl is /// @dev This function is called in `upgradeTo` & `upgradeToAndCall` /// @param _newImpl The new implementation address function _authorizeUpgrade(address _newImpl) internal view override onlyAdmin(CONTRACT_BASE_ID) { - if (!factory.isRegisteredUpgradePath(_getImplementation(), _newImpl)) { + if (!upgradeGate.isRegisteredUpgradePath(_getImplementation(), _newImpl)) { revert(); } } diff --git a/src/upgrades/FactoryManagedUpgradeGate.sol b/src/upgrades/UpgradeGate.sol similarity index 58% rename from src/upgrades/FactoryManagedUpgradeGate.sol rename to src/upgrades/UpgradeGate.sol index a5b9e5353..292536442 100644 --- a/src/upgrades/FactoryManagedUpgradeGate.sol +++ b/src/upgrades/UpgradeGate.sol @@ -1,25 +1,41 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.17; -import {IFactoryManagedUpgradeGate} from "../interfaces/IFactoryManagedUpgradeGate.sol"; +import {IUpgradeGate} from "../interfaces/IUpgradeGate.sol"; import {Ownable2StepUpgradeable} from "../utils/ownable/Ownable2StepUpgradeable.sol"; -import {FactoryManagedUpgradeGateStorageV1} from "./FactoryManagedUpgradeGateStorageV1.sol"; +import {UpgradeGateStorageV1} from "./UpgradeGateStorageV1.sol"; -/// @title FactoryManagedUpgradeGate +/// @title UpgradeGate /// @notice Contract for managing upgrades and safe upgrade paths for 1155 contracts -abstract contract FactoryManagedUpgradeGate is IFactoryManagedUpgradeGate, Ownable2StepUpgradeable, FactoryManagedUpgradeGateStorageV1 { +contract UpgradeGate is IUpgradeGate, Ownable2StepUpgradeable, UpgradeGateStorageV1 { + /// @notice Constructor for deployment pathway + constructor(address _defaultOwner) initializer { + __Ownable_init(_defaultOwner); + emit UpgradeGateSetup(); + } + + /// @notice The URI of the upgrade grate contract + function contractURI() external pure returns (string memory) { + return "https://github.com/ourzora/zora-1155-contracts/"; + } + + /// @notice The name of the upgrade gate contract + function contractName() external pure returns (string memory) { + return "ZORA 1155 Factory Managed Upgrade Gate"; + } + /// /// - /// CREATOR TOKEN UPGRADES /// + /// CREATOR TOKEN UPGRADES /// /// /// - /// @notice If an implementation is registered by the Builder DAO as an optional upgrade + /// @notice If an implementation is registered as an optional upgrade /// @param baseImpl The base implementation address /// @param upgradeImpl The upgrade implementation address function isRegisteredUpgradePath(address baseImpl, address upgradeImpl) public view returns (bool) { return isAllowedUpgrade[baseImpl][upgradeImpl]; } - /// @notice Called by the Builder DAO to offer implementation upgrades for created DAOs + /// @notice Registers optional upgrades /// @param baseImpls The base implementation addresses /// @param upgradeImpl The upgrade implementation address function registerUpgradePath(address[] memory baseImpls, address upgradeImpl) public onlyOwner { @@ -31,7 +47,7 @@ abstract contract FactoryManagedUpgradeGate is IFactoryManagedUpgradeGate, Ownab } } - /// @notice Called by the Builder DAO to remove an upgrade + /// @notice Removes an upgrade /// @param baseImpl The base implementation address /// @param upgradeImpl The upgrade implementation address function removeUpgradePath(address baseImpl, address upgradeImpl) public onlyOwner { diff --git a/src/upgrades/FactoryManagedUpgradeGateStorageV1.sol b/src/upgrades/UpgradeGateStorageV1.sol similarity index 75% rename from src/upgrades/FactoryManagedUpgradeGateStorageV1.sol rename to src/upgrades/UpgradeGateStorageV1.sol index 636505f4b..e6d6945f5 100644 --- a/src/upgrades/FactoryManagedUpgradeGateStorageV1.sol +++ b/src/upgrades/UpgradeGateStorageV1.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.17; -abstract contract FactoryManagedUpgradeGateStorageV1 { +abstract contract UpgradeGateStorageV1 { mapping(address => mapping(address => bool)) public isAllowedUpgrade; uint256[50] private __gap; diff --git a/test/factory/ZoraCreator1155Factory.t.sol b/test/factory/ZoraCreator1155Factory.t.sol index 03d8c0292..b006d766f 100644 --- a/test/factory/ZoraCreator1155Factory.t.sol +++ b/test/factory/ZoraCreator1155Factory.t.sol @@ -12,6 +12,7 @@ import {IZoraCreator1155Errors} from "../../src/interfaces/IZoraCreator1155Error import {IMinter1155} from "../../src/interfaces/IMinter1155.sol"; import {ICreatorRoyaltiesControl} from "../../src/interfaces/ICreatorRoyaltiesControl.sol"; import {Zora1155} from "../../src/proxies/Zora1155.sol"; +import {UpgradeGate} from "../../src/upgrades/UpgradeGate.sol"; import {MockContractMetadata} from "../mock/MockContractMetadata.sol"; import {ProxyShim} from "../../src/utils/ProxyShim.sol"; @@ -21,16 +22,19 @@ contract ZoraCreator1155FactoryTest is Test { ZoraCreator1155FactoryImpl internal factoryImpl; ZoraCreator1155FactoryImpl internal factory; + UpgradeGate internal upgradeGate; function setUp() external { zora = makeAddr("zora"); mintFeeAmount = 0.000777 ether; + upgradeGate = new UpgradeGate(zora); + address factoryShimAddress = address(new ProxyShim(zora)); Zora1155Factory factoryProxy = new Zora1155Factory(factoryShimAddress, ""); ProtocolRewards protocolRewards = new ProtocolRewards(); - ZoraCreator1155Impl zoraCreator1155Impl = new ZoraCreator1155Impl(mintFeeAmount, zora, address(factoryProxy), address(protocolRewards)); + ZoraCreator1155Impl zoraCreator1155Impl = new ZoraCreator1155Impl(mintFeeAmount, zora, address(upgradeGate), address(protocolRewards)); factoryImpl = new ZoraCreator1155FactoryImpl(zoraCreator1155Impl, IMinter1155(address(1)), IMinter1155(address(2)), IMinter1155(address(3))); factory = ZoraCreator1155FactoryImpl(address(factoryProxy)); @@ -253,7 +257,7 @@ contract ZoraCreator1155FactoryTest is Test { baseImpls[0] = address(factory.implementation()); vm.prank(zora); - factory.registerUpgradePath(baseImpls, address(newZoraCreator)); + upgradeGate.registerUpgradePath(baseImpls, address(newZoraCreator)); vm.prank(creatorProxy.owner()); creatorProxy.upgradeTo(address(newZoraCreator)); diff --git a/test/mock/MockUpgradeGate.sol b/test/mock/MockUpgradeGate.sol deleted file mode 100644 index 9679bdb64..000000000 --- a/test/mock/MockUpgradeGate.sol +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.17; - -import {FactoryManagedUpgradeGate} from "../../src/upgrades/FactoryManagedUpgradeGate.sol"; - -contract MockUpgradeGate is FactoryManagedUpgradeGate { - function initialize(address _initialOwner) external { - _transferOwnership(_initialOwner); - } -} diff --git a/test/mock/MockUpgradeGateBase.sol b/test/mock/MockUpgradeGateBase.sol deleted file mode 100644 index 3782d2e8d..000000000 --- a/test/mock/MockUpgradeGateBase.sol +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.17; - -import {FactoryManagedUpgradeGate} from "../../src/upgrades/FactoryManagedUpgradeGate.sol"; - -contract MockUpgradeGateBase is FactoryManagedUpgradeGate { - function setup(address initialOwner) public initializer { - __Ownable_init(initialOwner); - } -} diff --git a/test/nft/ZoraCreator1155.t.sol b/test/nft/ZoraCreator1155.t.sol index 70e3fc9d9..17a4ec944 100644 --- a/test/nft/ZoraCreator1155.t.sol +++ b/test/nft/ZoraCreator1155.t.sol @@ -9,6 +9,7 @@ import {MathUpgradeable} from "@zoralabs/openzeppelin-contracts-upgradeable/cont import {ZoraCreator1155Impl} from "../../src/nft/ZoraCreator1155Impl.sol"; import {Zora1155} from "../../src/proxies/Zora1155.sol"; import {ZoraCreatorFixedPriceSaleStrategy} from "../../src/minters/fixed-price/ZoraCreatorFixedPriceSaleStrategy.sol"; +import {UpgradeGate} from "../../src/upgrades/UpgradeGate.sol"; import {IZoraCreator1155Errors} from "../../src/interfaces/IZoraCreator1155Errors.sol"; import {IZoraCreator1155} from "../../src/interfaces/IZoraCreator1155.sol"; @@ -20,7 +21,6 @@ import {ICreatorRendererControl} from "../../src/interfaces/ICreatorRendererCont import {SimpleMinter} from "../mock/SimpleMinter.sol"; import {SimpleRenderer} from "../mock/SimpleRenderer.sol"; -import {MockUpgradeGate} from "../mock/MockUpgradeGate.sol"; contract ZoraCreator1155Test is Test { using stdJson for string; @@ -31,7 +31,7 @@ contract ZoraCreator1155Test is Test { SimpleMinter simpleMinter; ZoraCreatorFixedPriceSaleStrategy internal fixedPriceMinter; - MockUpgradeGate internal upgradeGate; + UpgradeGate internal upgradeGate; address payable internal admin; address internal recipient; @@ -55,16 +55,16 @@ contract ZoraCreator1155Test is Test { createReferral = makeAddr("createReferral"); zora = makeAddr("zora"); + admin = payable(vm.addr(0x1)); + recipient = vm.addr(0x2); + protocolRewards = new ProtocolRewards(); - upgradeGate = new MockUpgradeGate(); - upgradeGate.initialize(admin); + upgradeGate = new UpgradeGate(admin); zoraCreator1155Impl = new ZoraCreator1155Impl(0, zora, address(upgradeGate), address(protocolRewards)); target = ZoraCreator1155Impl(address(new Zora1155(address(zoraCreator1155Impl)))); simpleMinter = new SimpleMinter(); fixedPriceMinter = new ZoraCreatorFixedPriceSaleStrategy(); - admin = payable(vm.addr(0x1)); - recipient = vm.addr(0x2); adminRole = target.PERMISSION_BIT_ADMIN(); minterRole = target.PERMISSION_BIT_MINTER(); fundsManagerRole = target.PERMISSION_BIT_FUNDS_MANAGER(); diff --git a/test/upgrades/FactoryManagedUpgradeGate.t.sol b/test/upgrades/UpgradeGate.t.sol similarity index 87% rename from test/upgrades/FactoryManagedUpgradeGate.t.sol rename to test/upgrades/UpgradeGate.t.sol index 24f32314e..b8837bc63 100644 --- a/test/upgrades/FactoryManagedUpgradeGate.t.sol +++ b/test/upgrades/UpgradeGate.t.sol @@ -2,15 +2,14 @@ pragma solidity 0.8.17; import {Test} from "forge-std/Test.sol"; -import {MockUpgradeGateBase} from "../mock/MockUpgradeGateBase.sol"; +import {UpgradeGate} from "../../src/upgrades/UpgradeGate.sol"; -contract FactoryManagedUpgradeGateTest is Test { - MockUpgradeGateBase upgradeGate; +contract UpgradeGateTest is Test { + UpgradeGate upgradeGate; address constant admin = address(0x123); function setUp() public { - upgradeGate = new MockUpgradeGateBase(); - upgradeGate.setup(admin); + upgradeGate = new UpgradeGate(admin); } function test_AdminOnly() public {