From 0c32f54031cf484e750fd3c485b4ee88e703e51e Mon Sep 17 00:00:00 2001 From: Raul Date: Sun, 14 Apr 2024 22:50:05 -0300 Subject: [PATCH 1/6] upgradeable coremetadatamodule --- .../modules/metadata/CoreMetadataModule.sol | 32 ++++++++++++++++--- script/foundry/utils/DeployHelper.sol | 11 +++---- test/foundry/upgrades/Upgrades.t.sol | 19 +++++++++-- 3 files changed, 49 insertions(+), 13 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 1aebeac68..33a63e8c3 100644 --- a/script/foundry/utils/DeployHelper.sol +++ b/script/foundry/utils/DeployHelper.sol @@ -500,13 +500,11 @@ contract DeployHelper is Script, BroadcastManager, JsonDeploymentHandler, Storag _postdeploy("IpRoyaltyVaultBeacon", address(ipRoyaltyVaultBeacon)); _predeploy("CoreMetadataModule"); + impl = address(new CoreMetadataModule(address(accessController), address(ipAssetRegistry))); coreMetadataModule = CoreMetadataModule( - create3Deployer.deploy( - _getSalt(type(CoreMetadataModule).name), - abi.encodePacked( - type(CoreMetadataModule).creationCode, - abi.encode(address(accessController), address(ipAssetRegistry)) - ) + TestProxyHelper.deployUUPSProxy( + impl, + abi.encodeCall(CoreMetadataModule.initialize, address(protocolAccessManager)) ) ); _postdeploy("CoreMetadataModule", address(coreMetadataModule)); @@ -608,6 +606,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/upgrades/Upgrades.t.sol b/test/foundry/upgrades/Upgrades.t.sol index 84170e7dd..08a75779c 100644 --- a/test/foundry/upgrades/Upgrades.t.sol +++ b/test/foundry/upgrades/Upgrades.t.sol @@ -52,9 +52,7 @@ contract UpgradesTest is BaseTest { assertFalse(immediate); assertEq(delay, execDelay); - address newAccessController = address( - new MockAccessControllerV2(address(ipAccountRegistry), address(moduleRegistry)) - ); + address newAccessController = address(new MockAccessControllerV2()); vm.prank(u.bob); (bytes32 operationId, uint32 nonce) = protocolAccessManager.schedule( address(accessController), @@ -269,5 +267,20 @@ contract UpgradesTest is BaseTest { ), 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 + ); } } From f379a8c0bf71e1c0cfc5c8a2a34d8942782f1828 Mon Sep 17 00:00:00 2001 From: Raul Date: Mon, 15 Apr 2024 12:07:52 -0300 Subject: [PATCH 2/6] missing check --- contracts/lib/Errors.sol | 5 +++++ contracts/modules/metadata/CoreMetadataModule.sol | 3 +++ 2 files changed, 8 insertions(+) diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index 11d87bd32..9d2f97cf4 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -23,6 +23,11 @@ library Errors { /// @notice Provided calldata is invalid. error IPAccount__InvalidCalldata(); + //////////////////////////////////////////////////////////////////////////// + // CoreMetadataModule // + //////////////////////////////////////////////////////////////////////////// + error CoreMetadataModule__ZeroAccessManager(); + //////////////////////////////////////////////////////////////////////////// // IP Account Storage // //////////////////////////////////////////////////////////////////////////// diff --git a/contracts/modules/metadata/CoreMetadataModule.sol b/contracts/modules/metadata/CoreMetadataModule.sol index c210a5987..18cdfa43b 100644 --- a/contracts/modules/metadata/CoreMetadataModule.sol +++ b/contracts/modules/metadata/CoreMetadataModule.sol @@ -50,6 +50,9 @@ contract CoreMetadataModule is /// @notice Initializes the CoreMetadataModule contract. function initialize(address accessManager) public initializer { + if (accessManager == address(0)) { + revert Errors.CoreMetadataModule__ZeroAccessManager(); + } __AccessManaged_init(accessManager); __UUPSUpgradeable_init(); } From 773967a7e34bbb419018f6df848bfb7a3021473d Mon Sep 17 00:00:00 2001 From: Raul Date: Fri, 26 Apr 2024 17:56:28 -0300 Subject: [PATCH 3/6] fix --- test/foundry/upgrades/Upgrades.t.sol | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/foundry/upgrades/Upgrades.t.sol b/test/foundry/upgrades/Upgrades.t.sol index 08a75779c..42ae765d6 100644 --- a/test/foundry/upgrades/Upgrades.t.sol +++ b/test/foundry/upgrades/Upgrades.t.sol @@ -52,7 +52,11 @@ contract UpgradesTest is BaseTest { assertFalse(immediate); assertEq(delay, execDelay); - address newAccessController = address(new MockAccessControllerV2()); + address newAccessController = address( + new MockAccessControllerV2( + address(ipAccountRegistry), + address(moduleRegistry) + )); vm.prank(u.bob); (bytes32 operationId, uint32 nonce) = protocolAccessManager.schedule( address(accessController), From 9310b8f999761233d0919a8617d458486b5dd88f Mon Sep 17 00:00:00 2001 From: Raul Date: Mon, 20 May 2024 19:54:22 -0300 Subject: [PATCH 4/6] fix deployer --- test/foundry/upgrades/Upgrades.t.sol | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/foundry/upgrades/Upgrades.t.sol b/test/foundry/upgrades/Upgrades.t.sol index 42ae765d6..4aeada7bf 100644 --- a/test/foundry/upgrades/Upgrades.t.sol +++ b/test/foundry/upgrades/Upgrades.t.sol @@ -53,10 +53,8 @@ contract UpgradesTest is BaseTest { assertEq(delay, execDelay); address newAccessController = address( - new MockAccessControllerV2( - address(ipAccountRegistry), - address(moduleRegistry) - )); + new MockAccessControllerV2(address(ipAccountRegistry), address(moduleRegistry)) + ); vm.prank(u.bob); (bytes32 operationId, uint32 nonce) = protocolAccessManager.schedule( address(accessController), From a692a589464505ecc70f7508c4fd042eb15b8a27 Mon Sep 17 00:00:00 2001 From: Raul Date: Tue, 21 May 2024 01:37:04 -0300 Subject: [PATCH 5/6] coremetadata module deployment fix --- script/foundry/utils/DeployHelper.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/script/foundry/utils/DeployHelper.sol b/script/foundry/utils/DeployHelper.sol index 33a63e8c3..c3be104d7 100644 --- a/script/foundry/utils/DeployHelper.sol +++ b/script/foundry/utils/DeployHelper.sol @@ -503,6 +503,8 @@ contract DeployHelper is Script, BroadcastManager, JsonDeploymentHandler, Storag impl = address(new CoreMetadataModule(address(accessController), address(ipAssetRegistry))); coreMetadataModule = CoreMetadataModule( TestProxyHelper.deployUUPSProxy( + create3Deployer, + _getSalt(type(CoreMetadataModule).name), impl, abi.encodeCall(CoreMetadataModule.initialize, address(protocolAccessManager)) ) From e06aa20292c40439d2624ec1893a707ba9c8f620 Mon Sep 17 00:00:00 2001 From: Raul Date: Tue, 21 May 2024 21:02:11 -0300 Subject: [PATCH 6/6] add required verifiers --- script/foundry/utils/DeployHelper.sol | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/script/foundry/utils/DeployHelper.sol b/script/foundry/utils/DeployHelper.sol index c3be104d7..aeefdbd81 100644 --- a/script/foundry/utils/DeployHelper.sol +++ b/script/foundry/utils/DeployHelper.sol @@ -509,6 +509,11 @@ contract DeployHelper is Script, BroadcastManager, JsonDeploymentHandler, Storag abi.encodeCall(CoreMetadataModule.initialize, address(protocolAccessManager)) ) ); + require( + _getDeployedAddress(type(CoreMetadataModule).name) == address(coreMetadataModule), + "Deploy: Core Metadata Module Address Mismatch" + ); + require(_loadProxyImpl(address(coreMetadataModule)) == impl, "CoreMetadataModule Proxy Implementation Mismatch"); _postdeploy("CoreMetadataModule", address(coreMetadataModule)); _predeploy("CoreMetadataViewModule");