From fd3dd95cd4cd7848b497aa5412aff0b916651269 Mon Sep 17 00:00:00 2001 From: Raul Date: Sun, 14 Apr 2024 22:50:05 -0300 Subject: [PATCH] upgradeable coremetadatamodule --- .../modules/metadata/CoreMetadataModule.sol | 32 ++++- script/foundry/utils/DeployHelper.sol | 9 +- .../mocks/module/MockAccessControllerV2.sol | 3 +- test/foundry/upgrades/Upgrades.t.sol | 122 +++++++++++++++--- 4 files changed, 141 insertions(+), 25 deletions(-) diff --git a/contracts/modules/metadata/CoreMetadataModule.sol b/contracts/modules/metadata/CoreMetadataModule.sol index 498eefda5..c210a5987 100644 --- a/contracts/modules/metadata/CoreMetadataModule.sol +++ b/contracts/modules/metadata/CoreMetadataModule.sol @@ -3,6 +3,9 @@ pragma solidity ^0.8.23; import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol"; +import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; +// solhint-disable-next-line max-line-length +import { AccessManagedUpgradeable } from "@openzeppelin/contracts-upgradeable/access/manager/AccessManagedUpgradeable.sol"; import { IIPAccount } from "../../interfaces/IIPAccount.sol"; import { AccessControlled } from "../../access/AccessControlled.sol"; import { BaseModule } from "../BaseModule.sol"; @@ -17,11 +20,15 @@ import { ICoreMetadataModule } from "../../interfaces/modules/metadata/ICoreMeta /// It implements the ICoreMetadataModule interface. /// The metadata can be set and updated by the owner of the IP asset. /// The metadata can be frozen to prevent further changes. -contract CoreMetadataModule is BaseModule, AccessControlled, ICoreMetadataModule { +contract CoreMetadataModule is + BaseModule, + AccessControlled, + ICoreMetadataModule, + AccessManagedUpgradeable, + UUPSUpgradeable +{ using IPAccountStorageOps for IIPAccount; - string public override name = CORE_METADATA_MODULE_KEY; - /// @notice Modifier to ensure that metadata can only be changed when mutable. modifier onlyMutable(address ipId) { if (IIPAccount(payable(ipId)).getBool("IMMUTABLE")) { @@ -33,10 +40,23 @@ contract CoreMetadataModule is BaseModule, AccessControlled, ICoreMetadataModule /// @notice Creates a new CoreMetadataModule instance. /// @param accessController The address of the AccessController contract. /// @param ipAccountRegistry The address of the IPAccountRegistry contract. + /// @custom:oz-upgrades-unsafe-allow constructor constructor( address accessController, address ipAccountRegistry - ) AccessControlled(accessController, ipAccountRegistry) {} + ) AccessControlled(accessController, ipAccountRegistry) { + _disableInitializers(); + } + + /// @notice Initializes the CoreMetadataModule contract. + function initialize(address accessManager) public initializer { + __AccessManaged_init(accessManager); + __UUPSUpgradeable_init(); + } + + function name() external pure override returns (string memory) { + return CORE_METADATA_MODULE_KEY; + } /// @notice Update the nftTokenURI for an IP asset, /// by retrieve the latest TokenURI from IP NFT to which the IP Asset bound. @@ -113,4 +133,8 @@ contract CoreMetadataModule is BaseModule, AccessControlled, ICoreMetadataModule IIPAccount(payable(ipId)).setBytes32("METADATA_HASH", metadataHash); emit MetadataURISet(ipId, metadataURI, metadataHash); } + + /// @dev Hook to authorize the upgrade according to UUPSUpgradeable + /// @param newImplementation The address of the new implementation + function _authorizeUpgrade(address newImplementation) internal override restricted {} } diff --git a/script/foundry/utils/DeployHelper.sol b/script/foundry/utils/DeployHelper.sol index 3b46b0378..b3461fac7 100644 --- a/script/foundry/utils/DeployHelper.sol +++ b/script/foundry/utils/DeployHelper.sol @@ -359,7 +359,13 @@ contract DeployHelper is Script, BroadcastManager, JsonDeploymentHandler, Storag _postdeploy("IpRoyaltyVaultBeacon", address(ipRoyaltyVaultBeacon)); _predeploy("CoreMetadataModule"); - coreMetadataModule = new CoreMetadataModule(address(accessController), address(ipAssetRegistry)); + impl = address(new CoreMetadataModule(address(accessController), address(ipAssetRegistry))); + coreMetadataModule = CoreMetadataModule( + TestProxyHelper.deployUUPSProxy( + impl, + abi.encodeCall(CoreMetadataModule.initialize, address(protocolAccessManager)) + ) + ); _postdeploy("CoreMetadataModule", address(coreMetadataModule)); _predeploy("CoreMetadataViewModule"); @@ -456,6 +462,7 @@ contract DeployHelper is Script, BroadcastManager, JsonDeploymentHandler, Storag protocolAccessManager.setTargetFunctionRole(address(licenseRegistry), selectors, ProtocolAdmin.UPGRADER_ROLE); protocolAccessManager.setTargetFunctionRole(address(moduleRegistry), selectors, ProtocolAdmin.UPGRADER_ROLE); protocolAccessManager.setTargetFunctionRole(address(ipAssetRegistry), selectors, ProtocolAdmin.UPGRADER_ROLE); + protocolAccessManager.setTargetFunctionRole(address(coreMetadataModule), selectors, ProtocolAdmin.UPGRADER_ROLE); // Royalty and Upgrade Beacon // Owner of the beacon is the RoyaltyPolicyLAP diff --git a/test/foundry/mocks/module/MockAccessControllerV2.sol b/test/foundry/mocks/module/MockAccessControllerV2.sol index e5ac7ee7a..ac55250e4 100644 --- a/test/foundry/mocks/module/MockAccessControllerV2.sol +++ b/test/foundry/mocks/module/MockAccessControllerV2.sol @@ -11,7 +11,8 @@ contract MockAccessControllerV2 is AccessController { } // keccak256(abi.encode(uint256(keccak256("story-protocol.AccessControllerV2")) - 1)) & ~bytes32(uint256(0xff)); - bytes32 private constant AccessControllerV2StorageLocation = 0xf328f2cdee4ae4df23921504bfa43e3156fb4d18b23549ca0a43fd1e64947a00; + bytes32 private constant AccessControllerV2StorageLocation = + 0xf328f2cdee4ae4df23921504bfa43e3156fb4d18b23549ca0a43fd1e64947a00; function initialize() public reinitializer(2) { _getAccessControllerV2Storage().newState = "initialized"; diff --git a/test/foundry/upgrades/Upgrades.t.sol b/test/foundry/upgrades/Upgrades.t.sol index 5243c3a7b..08a75779c 100644 --- a/test/foundry/upgrades/Upgrades.t.sol +++ b/test/foundry/upgrades/Upgrades.t.sol @@ -12,7 +12,6 @@ import { MockIpRoyaltyVaultV2 } from "../mocks/module/MockIpRoyaltyVaultV2.sol"; import { MockAccessControllerV2 } from "../mocks/module/MockAccessControllerV2.sol"; contract UpgradesTest is BaseTest { - uint32 execDelay = 600; function setUp() public override { @@ -45,7 +44,6 @@ contract UpgradesTest is BaseTest { } function test_upgradeAccessController() public { - (bool immediate, uint32 delay) = protocolAccessManager.canCall( u.bob, address(accessController), @@ -54,7 +52,6 @@ contract UpgradesTest is BaseTest { assertFalse(immediate); assertEq(delay, execDelay); - address newAccessController = address(new MockAccessControllerV2()); vm.prank(u.bob); (bytes32 operationId, uint32 nonce) = protocolAccessManager.schedule( @@ -75,7 +72,10 @@ contract UpgradesTest is BaseTest { function test_deploymentSetup() public { // Deployer doesn't have the roles - (bool isMember, uint32 executionDelay) = protocolAccessManager.hasRole(ProtocolAdmin.PROTOCOL_ADMIN_ROLE, deployer); + (bool isMember, uint32 executionDelay) = protocolAccessManager.hasRole( + ProtocolAdmin.PROTOCOL_ADMIN_ROLE, + deployer + ); assertFalse(isMember); assertEq(executionDelay, 0); (isMember, executionDelay) = protocolAccessManager.hasRole(ProtocolAdmin.UPGRADER_ROLE, deployer); @@ -84,14 +84,17 @@ contract UpgradesTest is BaseTest { (isMember, executionDelay) = protocolAccessManager.hasRole(ProtocolAdmin.PAUSE_ADMIN_ROLE, deployer); assertFalse(isMember); assertEq(executionDelay, 0); - + (isMember, executionDelay) = protocolAccessManager.hasRole(ProtocolAdmin.PROTOCOL_ADMIN_ROLE, multisig); assertTrue(isMember); assertEq(executionDelay, 0); (isMember, executionDelay) = protocolAccessManager.hasRole(ProtocolAdmin.PAUSE_ADMIN_ROLE, multisig); assertTrue(isMember); assertEq(executionDelay, 0); - (isMember, executionDelay) = protocolAccessManager.hasRole(ProtocolAdmin.PAUSE_ADMIN_ROLE, address(protocolPauser)); + (isMember, executionDelay) = protocolAccessManager.hasRole( + ProtocolAdmin.PAUSE_ADMIN_ROLE, + address(protocolPauser) + ); assertTrue(isMember); assertEq(executionDelay, 0); (isMember, executionDelay) = protocolAccessManager.hasRole(ProtocolAdmin.UPGRADER_ROLE, multisig); @@ -99,7 +102,7 @@ contract UpgradesTest is BaseTest { assertEq(executionDelay, execDelay); // Target function role wiring - + (bool immediate, uint32 delay) = protocolAccessManager.canCall( multisig, address(royaltyPolicyLAP), @@ -107,8 +110,14 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, 600); - assertEq(protocolAccessManager.getTargetFunctionRole(address(royaltyPolicyLAP), RoyaltyPolicyLAP.upgradeVaults.selector), ProtocolAdmin.UPGRADER_ROLE); - + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(royaltyPolicyLAP), + RoyaltyPolicyLAP.upgradeVaults.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); + (immediate, delay) = protocolAccessManager.canCall( multisig, address(accessController), @@ -116,7 +125,13 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(accessController), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(accessController), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); (immediate, delay) = protocolAccessManager.canCall( multisig, @@ -125,7 +140,13 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(licenseToken), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(licenseToken), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); (immediate, delay) = protocolAccessManager.canCall( multisig, @@ -134,7 +155,13 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(disputeModule), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(disputeModule), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); (immediate, delay) = protocolAccessManager.canCall( multisig, @@ -143,7 +170,13 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(arbitrationPolicySP), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(arbitrationPolicySP), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); (immediate, delay) = protocolAccessManager.canCall( multisig, @@ -152,7 +185,13 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(licensingModule), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(licensingModule), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); (immediate, delay) = protocolAccessManager.canCall( multisig, @@ -161,7 +200,13 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(royaltyModule), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(royaltyModule), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); (immediate, delay) = protocolAccessManager.canCall( multisig, @@ -170,7 +215,13 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(licenseRegistry), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(licenseRegistry), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); (immediate, delay) = protocolAccessManager.canCall( multisig, @@ -179,7 +230,13 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(moduleRegistry), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(moduleRegistry), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); (immediate, delay) = protocolAccessManager.canCall( multisig, @@ -188,7 +245,13 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(ipAssetRegistry), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(ipAssetRegistry), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); (immediate, delay) = protocolAccessManager.canCall( multisig, @@ -197,6 +260,27 @@ contract UpgradesTest is BaseTest { ); assertFalse(immediate); assertEq(delay, execDelay); - assertEq(protocolAccessManager.getTargetFunctionRole(address(royaltyPolicyLAP), UUPSUpgradeable.upgradeToAndCall.selector), ProtocolAdmin.UPGRADER_ROLE); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(royaltyPolicyLAP), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); + + (immediate, delay) = protocolAccessManager.canCall( + multisig, + address(coreMetadataModule), + UUPSUpgradeable.upgradeToAndCall.selector + ); + assertFalse(immediate); + assertEq(delay, execDelay); + assertEq( + protocolAccessManager.getTargetFunctionRole( + address(coreMetadataModule), + UUPSUpgradeable.upgradeToAndCall.selector + ), + ProtocolAdmin.UPGRADER_ROLE + ); } }