From 99adbadc7e5bc90ff82ef5748802ffb4a046058e Mon Sep 17 00:00:00 2001 From: Raul Date: Thu, 11 Apr 2024 23:07:36 -0300 Subject: [PATCH 1/4] upgradeable ip asset registry --- contracts/registries/IPAccountRegistry.sol | 4 ++ contracts/registries/IPAssetRegistry.sol | 41 +++++++++++++++++-- script/foundry/utils/DeployHelper.sol | 14 ++++++- .../utils/upgrades/ERC7201Helper.s.sol | 2 +- .../licensing/LicensingIntegration.t.sol | 13 +++++- 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/contracts/registries/IPAccountRegistry.sol b/contracts/registries/IPAccountRegistry.sol index d9680986..9d64c5ec 100644 --- a/contracts/registries/IPAccountRegistry.sol +++ b/contracts/registries/IPAccountRegistry.sol @@ -11,14 +11,18 @@ import { Errors } from "../lib/Errors.sol"; /// It leverages a public ERC6551 registry to deploy IPAccount contracts. abstract contract IPAccountRegistry is IIPAccountRegistry { /// @notice Returns the IPAccount implementation address + /// @custom:oz-upgrades-unsafe-allow state-variable-immutable address public immutable IP_ACCOUNT_IMPL; /// @notice Returns the IPAccount salt + /// @custom:oz-upgrades-unsafe-allow state-variable-immutable bytes32 public immutable IP_ACCOUNT_SALT; /// @notice Returns the public ERC6551 registry address + /// @custom:oz-upgrades-unsafe-allow state-variable-immutable address public immutable ERC6551_PUBLIC_REGISTRY; + /// @custom:oz-upgrades-unsafe-allow constructor constructor(address erc6551Registry, address ipAccountImpl) { if (ipAccountImpl == address(0)) revert Errors.IPAccountRegistry_InvalidIpAccountImpl(); IP_ACCOUNT_IMPL = ipAccountImpl; diff --git a/contracts/registries/IPAssetRegistry.sol b/contracts/registries/IPAssetRegistry.sol index 807dca89..a9ce6a64 100644 --- a/contracts/registries/IPAssetRegistry.sol +++ b/contracts/registries/IPAssetRegistry.sol @@ -5,6 +5,9 @@ import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol"; import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; import { Strings } from "@openzeppelin/contracts/utils/Strings.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 { IIPAssetRegistry } from "../interfaces/registries/IIPAssetRegistry.sol"; @@ -21,16 +24,32 @@ import { IPAccountStorageOps } from "../lib/IPAccountStorageOps.sol"; /// attribution and an IP account for protocol authorization. /// IMPORTANT: The IP account address, besides being used for protocol /// auth, is also the canonical IP identifier for the IP NFT. -contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry { +contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry, AccessManagedUpgradeable, UUPSUpgradeable { using ERC165Checker for address; using Strings for *; using IPAccountStorageOps for IIPAccount; + /// @dev Storage structure for the IPAssetRegistry /// @notice Tracks the total number of IP assets in existence. - uint256 public totalSupply = 0; + /// @custom:storage-location erc7201:story-protocol.IPAssetRegistry + struct IPAssetRegistryStorage { + uint256 totalSupply; + } + + // keccak256(abi.encode(uint256(keccak256("story-protocol.IPAssetRegistry")) - 1)) & ~bytes32(uint256(0xff)); + bytes32 private constant IPAssetRegistryStorageLocation = + 0x987c61809af5a42943abd137c7acff8426aab6f7a1f5c967a03d1d718ba5cf00; + /// @custom:oz-upgrades-unsafe-allow constructor constructor(address erc6551Registry, address ipAccountImpl) IPAccountRegistry(erc6551Registry, ipAccountImpl) {} + /// @notice Initializes the IPAssetRegistry contract. + /// @param accessManager The address of the access manager. + function initialize(address accessManager) public initializer { + __AccessManaged_init(accessManager); + __UUPSUpgradeable_init(); + } + /// @notice Registers an NFT as an IP asset. /// @dev The IP required metadata name and URI are derived from the NFT's metadata. /// @param tokenContract The address of the NFT. @@ -69,7 +88,7 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry { ipAccount.setString("URI", uri); ipAccount.setUint256("REGISTRATION_DATE", registrationDate); - totalSupply++; + _getIPAssetRegistryStorage().totalSupply++; emit IPRegistered(id, block.chainid, tokenContract, tokenId, name, uri, registrationDate); } @@ -95,4 +114,20 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry { if (id != ipAccount(chainId, tokenContract, tokenId)) return false; return bytes(IIPAccount(payable(id)).getString("NAME")).length != 0; } + + /// @notice Gets the total number of IP assets registered in the protocol. + function totalSupply() external view returns (uint256) { + return _getIPAssetRegistryStorage().totalSupply; + } + + /// @dev Hook to authorize the upgrade according to UUPSUpgradeable + /// @param newImplementation The address of the new implementation + function _authorizeUpgrade(address newImplementation) internal override restricted {} + + /// @dev Returns the storage struct of IPAssetRegistry. + function _getIPAssetRegistryStorage() private pure returns (IPAssetRegistryStorage storage $) { + assembly { + $.slot := IPAssetRegistryStorageLocation + } + } } diff --git a/script/foundry/utils/DeployHelper.sol b/script/foundry/utils/DeployHelper.sol index 704eb8da..77979a61 100644 --- a/script/foundry/utils/DeployHelper.sol +++ b/script/foundry/utils/DeployHelper.sol @@ -192,7 +192,19 @@ contract DeployHelper is Script, BroadcastManager, JsonDeploymentHandler, Storag contractKey = "IPAssetRegistry"; _predeploy(contractKey); - ipAssetRegistry = new IPAssetRegistry(address(erc6551Registry), address(ipAccountImpl)); + impl = address( + new IPAssetRegistry( + address(erc6551Registry), + address(ipAccountImpl) + ) + ); + ipAssetRegistry = IPAssetRegistry( + TestProxyHelper.deployUUPSProxy( + impl, + abi.encodeCall(IPAssetRegistry.initialize, address(protocolAccessManager)) + ) + ); + impl = address(0); // Make sure we don't deploy wrong impl _postdeploy(contractKey, address(ipAssetRegistry)); IPAccountRegistry ipAccountRegistry = IPAccountRegistry(address(ipAssetRegistry)); diff --git a/script/foundry/utils/upgrades/ERC7201Helper.s.sol b/script/foundry/utils/upgrades/ERC7201Helper.s.sol index 170f8137..f30036b1 100644 --- a/script/foundry/utils/upgrades/ERC7201Helper.s.sol +++ b/script/foundry/utils/upgrades/ERC7201Helper.s.sol @@ -12,7 +12,7 @@ import { console2 } from "forge-std/console2.sol"; contract ERC7201HelperScript is Script { string constant NAMESPACE = "story-protocol"; - string constant CONTRACT_NAME = "IpRoyaltyVault"; + string constant CONTRACT_NAME = "IPAssetRegistry"; function run() external { bytes memory erc7201Key = abi.encodePacked(NAMESPACE, ".", CONTRACT_NAME); diff --git a/test/foundry/integration/flows/licensing/LicensingIntegration.t.sol b/test/foundry/integration/flows/licensing/LicensingIntegration.t.sol index e29153f3..8d44231b 100644 --- a/test/foundry/integration/flows/licensing/LicensingIntegration.t.sol +++ b/test/foundry/integration/flows/licensing/LicensingIntegration.t.sol @@ -92,7 +92,18 @@ contract e2e is Test { erc6551Registry = new ERC6551Registry(); ipAccountImpl = new IPAccountImpl(address(accessController)); - ipAssetRegistry = new IPAssetRegistry(address(erc6551Registry), address(ipAccountImpl)); + impl = address( + new IPAssetRegistry( + address(erc6551Registry), + address(ipAccountImpl) + ) + ); + ipAssetRegistry = IPAssetRegistry( + TestProxyHelper.deployUUPSProxy( + impl, + abi.encodeCall(IPAssetRegistry.initialize, address(protocolAccessManager)) + ) + ); impl = address(new LicenseRegistry()); licenseRegistry = LicenseRegistry( From abb834e74e0489388736694f7eb5c7a2434977c7 Mon Sep 17 00:00:00 2001 From: Raul Date: Thu, 11 Apr 2024 23:15:51 -0300 Subject: [PATCH 2/4] deploy script setTargetFunctionRole --- script/foundry/utils/DeployHelper.sol | 1 + .../integration/flows/licensing/LicensingIntegration.t.sol | 7 +------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/script/foundry/utils/DeployHelper.sol b/script/foundry/utils/DeployHelper.sol index 77979a61..8620b9b8 100644 --- a/script/foundry/utils/DeployHelper.sol +++ b/script/foundry/utils/DeployHelper.sol @@ -432,6 +432,7 @@ contract DeployHelper is Script, BroadcastManager, JsonDeploymentHandler, Storag protocolAccessManager.setTargetFunctionRole(address(royaltyPolicyLAP), selectors, ProtocolAdmin.UPGRADER_ROLE); protocolAccessManager.setTargetFunctionRole(address(licenseRegistry), selectors, ProtocolAdmin.UPGRADER_ROLE); protocolAccessManager.setTargetFunctionRole(address(moduleRegistry), selectors, ProtocolAdmin.UPGRADER_ROLE); + protocolAccessManager.setTargetFunctionRole(address(ipAssetRegistry), selectors, ProtocolAdmin.UPGRADER_ROLE); ///////// Role Granting ///////// protocolAccessManager.grantRole(ProtocolAdmin.UPGRADER_ROLE, multisig, upgraderExecDelay); diff --git a/test/foundry/integration/flows/licensing/LicensingIntegration.t.sol b/test/foundry/integration/flows/licensing/LicensingIntegration.t.sol index 8d44231b..5c761090 100644 --- a/test/foundry/integration/flows/licensing/LicensingIntegration.t.sol +++ b/test/foundry/integration/flows/licensing/LicensingIntegration.t.sol @@ -92,12 +92,7 @@ contract e2e is Test { erc6551Registry = new ERC6551Registry(); ipAccountImpl = new IPAccountImpl(address(accessController)); - impl = address( - new IPAssetRegistry( - address(erc6551Registry), - address(ipAccountImpl) - ) - ); + impl = address(new IPAssetRegistry(address(erc6551Registry), address(ipAccountImpl))); ipAssetRegistry = IPAssetRegistry( TestProxyHelper.deployUUPSProxy( impl, From 20b78ec00f008d28da050a62f0125f02f48fc05d Mon Sep 17 00:00:00 2001 From: Raul Date: Fri, 12 Apr 2024 20:24:23 -0300 Subject: [PATCH 3/4] fixes --- contracts/LicenseToken.sol | 3 +++ contracts/access/AccessController.sol | 3 +++ contracts/lib/Errors.sol | 15 +++++++++++++++ contracts/modules/dispute/DisputeModule.sol | 5 +++-- .../dispute/policies/ArbitrationPolicySP.sol | 5 +++-- contracts/modules/licensing/LicensingModule.sol | 3 +++ contracts/modules/royalty/RoyaltyModule.sol | 4 +++- contracts/registries/IPAssetRegistry.sol | 7 ++++++- contracts/registries/LicenseRegistry.sol | 3 +++ contracts/registries/ModuleRegistry.sol | 3 +++ 10 files changed, 45 insertions(+), 6 deletions(-) diff --git a/contracts/LicenseToken.sol b/contracts/LicenseToken.sol index ae5554e7..598c45cd 100644 --- a/contracts/LicenseToken.sol +++ b/contracts/LicenseToken.sol @@ -51,6 +51,9 @@ contract LicenseToken is ILicenseToken, ERC721EnumerableUpgradeable, AccessManag /// @dev Initializes the LicenseToken contract function initialize(address accessManager, string memory imageUrl) public initializer { __ERC721_init("Programmable IP License Token", "PILicenseToken"); + if (accessManager == address(0)) { + revert Errors.LicenseToken__ZeroAccessManager(); + } __AccessManaged_init(accessManager); __UUPSUpgradeable_init(); _getLicenseTokenStorage().imageUrl = imageUrl; diff --git a/contracts/access/AccessController.sol b/contracts/access/AccessController.sol index dbcaa14d..5ba0b29e 100644 --- a/contracts/access/AccessController.sol +++ b/contracts/access/AccessController.sol @@ -57,6 +57,9 @@ contract AccessController is IAccessController, AccessManagedUpgradeable, UUPSUp /// @notice initializer for this implementation contract /// @param accessManager The address of the protocol admin roles contract function initialize(address accessManager) external initializer { + if (accessManager == address(0)) { + revert Errors.AccessController__ZeroAccessManager(); + } __AccessManaged_init(accessManager); } diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index dd8efdf5..65b021f3 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -29,6 +29,9 @@ library Errors { // IPAssetRegistry // //////////////////////////////////////////////////////////////////////////// + /// @notice zero address provided for access manager in initializer. + error IPAssetRegistry__ZeroAccessManager(); + /// @notice The IP asset has already been registered. error IPAssetRegistry__AlreadyRegistered(); @@ -110,6 +113,7 @@ library Errors { // LicenseRegistry // //////////////////////////////////////////////////////////////////////////// + error LicenseRegistry__ZeroAccessManager(); error LicenseRegistry__CallerNotLicensingModule(); error LicenseRegistry__ZeroLicensingModule(); error LicensingModule__CallerNotLicenseRegistry(); @@ -140,6 +144,7 @@ library Errors { error LicenseToken__CallerNotLicensingModule(); error LicenseToken__ZeroLicensingModule(); error LicenseToken__ZeroDisputeModule(); + error LicenseToken__ZeroAccessManager(); error LicenseToken__RevokedLicense(uint256 tokenId); error LicenseToken__NotTransferable(); error LicenseToken__LicenseTokenExpired(uint256 tokenId, uint256 expiredAt, uint256 currentTimestamp); @@ -152,6 +157,7 @@ library Errors { // LicensingModule // //////////////////////////////////////////////////////////////////////////// + error LicensingModule__ZeroAccessManager(); error LicensingModule__IpAlreadyLinked(); error LicensingModule__PolicyAlreadySetForIpId(); error LicensingModule__FrameworkNotFound(); @@ -222,6 +228,7 @@ library Errors { // Dispute Module // //////////////////////////////////////////////////////////////////////////// + error DisputeModule__ZeroAccessManager(); error DisputeModule__ZeroArbitrationPolicy(); error DisputeModule__ZeroArbitrationRelayer(); error DisputeModule__ZeroDisputeTag(); @@ -242,6 +249,11 @@ library Errors { error DisputeModule__ZeroController(); error DisputeModule__ZeroAccessManager(); + //////////////////////////////////////////////////////////////////////////// + // ArbitrationPolicy SP // + //////////////////////////////////////////////////////////////////////////// + + error ArbitrationPolicySP__ZeroAccessManager(); error ArbitrationPolicySP__ZeroDisputeModule(); error ArbitrationPolicySP__ZeroPaymentToken(); error ArbitrationPolicySP__NotDisputeModule(); @@ -251,6 +263,7 @@ library Errors { // Royalty Module // //////////////////////////////////////////////////////////////////////////// + error RoyaltyModule__ZeroAccessManager(); error RoyaltyModule__ZeroRoyaltyPolicy(); error RoyaltyModule__NotWhitelistedRoyaltyPolicy(); error RoyaltyModule__ZeroRoyaltyToken(); @@ -294,6 +307,7 @@ library Errors { // ModuleRegistry // //////////////////////////////////////////////////////////////////////////// + error ModuleRegistry__ZeroAccessManager(); error ModuleRegistry__ModuleAddressZeroAddress(); error ModuleRegistry__ModuleAddressNotContract(); error ModuleRegistry__ModuleAlreadyRegistered(); @@ -311,6 +325,7 @@ library Errors { // AccessController // //////////////////////////////////////////////////////////////////////////// + error AccessController__ZeroAccessManager(); error AccessController__IPAccountIsZeroAddress(); error AccessController__IPAccountIsNotValid(address ipAccount); error AccessController__SignerIsZeroAddress(); diff --git a/contracts/modules/dispute/DisputeModule.sol b/contracts/modules/dispute/DisputeModule.sol index f4bc1ddb..70433cf8 100644 --- a/contracts/modules/dispute/DisputeModule.sol +++ b/contracts/modules/dispute/DisputeModule.sol @@ -93,8 +93,9 @@ contract DisputeModule is /// @notice Initializer for this implementation contract /// @param accessManager The address of the protocol admin roles contract function initialize(address accessManager) external initializer { - if (accessManager == address(0)) revert Errors.DisputeModule__ZeroAccessManager(); - + if (accessManager == address(0)) { + revert Errors.DisputeModule__ZeroAccessManager(); + } __AccessManaged_init(accessManager); __ReentrancyGuard_init(); __UUPSUpgradeable_init(); diff --git a/contracts/modules/dispute/policies/ArbitrationPolicySP.sol b/contracts/modules/dispute/policies/ArbitrationPolicySP.sol index 666f4c8f..d5c93074 100644 --- a/contracts/modules/dispute/policies/ArbitrationPolicySP.sol +++ b/contracts/modules/dispute/policies/ArbitrationPolicySP.sol @@ -52,8 +52,9 @@ contract ArbitrationPolicySP is IArbitrationPolicy, AccessManagedUpgradeable, UU /// @notice initializer for this implementation contract /// @param accessManager The address of the protocol admin roles contract function initialize(address accessManager) public initializer { - if (accessManager == address(0)) revert Errors.ArbitrationPolicySP__ZeroAccessManager(); - + if (accessManager == address(0)) { + revert Errors.ArbitrationPolicySP__ZeroAccessManager(); + } __AccessManaged_init(accessManager); __UUPSUpgradeable_init(); } diff --git a/contracts/modules/licensing/LicensingModule.sol b/contracts/modules/licensing/LicensingModule.sol index 503f0288..293c0a15 100644 --- a/contracts/modules/licensing/LicensingModule.sol +++ b/contracts/modules/licensing/LicensingModule.sol @@ -99,6 +99,9 @@ contract LicensingModule is function initialize(address accessManager) public initializer { __ReentrancyGuard_init(); __UUPSUpgradeable_init(); + if (accessManager == address(0)) { + revert Errors.LicensingModule__ZeroAccessManager(); + } __AccessManaged_init(accessManager); } diff --git a/contracts/modules/royalty/RoyaltyModule.sol b/contracts/modules/royalty/RoyaltyModule.sol index 294feccf..df328641 100644 --- a/contracts/modules/royalty/RoyaltyModule.sol +++ b/contracts/modules/royalty/RoyaltyModule.sol @@ -58,7 +58,9 @@ contract RoyaltyModule is /// @notice Initializer for this implementation contract /// @param accessManager The address of the protocol admin roles contract function initialize(address accessManager) external initializer { - if (accessManager == address(0)) revert Errors.RoyaltyModule__ZeroAccessManager(); + if (accessManager == address(0)) { + revert Errors.RoyaltyModule__ZeroAccessManager(); + } __AccessManaged_init(accessManager); __ReentrancyGuard_init(); __UUPSUpgradeable_init(); diff --git a/contracts/registries/IPAssetRegistry.sol b/contracts/registries/IPAssetRegistry.sol index a9ce6a64..f69836dd 100644 --- a/contracts/registries/IPAssetRegistry.sol +++ b/contracts/registries/IPAssetRegistry.sol @@ -41,11 +41,16 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry, AccessManagedUp 0x987c61809af5a42943abd137c7acff8426aab6f7a1f5c967a03d1d718ba5cf00; /// @custom:oz-upgrades-unsafe-allow constructor - constructor(address erc6551Registry, address ipAccountImpl) IPAccountRegistry(erc6551Registry, ipAccountImpl) {} + constructor(address erc6551Registry, address ipAccountImpl) IPAccountRegistry(erc6551Registry, ipAccountImpl) { + _disableInitializers(); + } /// @notice Initializes the IPAssetRegistry contract. /// @param accessManager The address of the access manager. function initialize(address accessManager) public initializer { + if (accessManager == address(0)) { + revert Errors.IPAssetRegistry__ZeroAccessManager(); + } __AccessManaged_init(accessManager); __UUPSUpgradeable_init(); } diff --git a/contracts/registries/LicenseRegistry.sol b/contracts/registries/LicenseRegistry.sol index 2701570f..f56d9ad8 100644 --- a/contracts/registries/LicenseRegistry.sol +++ b/contracts/registries/LicenseRegistry.sol @@ -80,6 +80,9 @@ contract LicenseRegistry is ILicenseRegistry, AccessManagedUpgradeable, UUPSUpgr /// @notice initializer for this implementation contract /// @param accessManager The address of the protocol admin roles contract function initialize(address accessManager) public initializer { + if (accessManager == address(0)) { + revert Errors.LicenseRegistry__ZeroAccessManager(); + } __AccessManaged_init(accessManager); __UUPSUpgradeable_init(); } diff --git a/contracts/registries/ModuleRegistry.sol b/contracts/registries/ModuleRegistry.sol index ecdb67cf..ec20cd6b 100644 --- a/contracts/registries/ModuleRegistry.sol +++ b/contracts/registries/ModuleRegistry.sol @@ -41,6 +41,9 @@ contract ModuleRegistry is IModuleRegistry, AccessManagedUpgradeable, UUPSUpgrad /// @notice initializer for this implementation contract /// @param accessManager The address of the governance. function initialize(address accessManager) public initializer { + if (accessManager == address(0)) { + revert Errors.ModuleRegistry__ZeroAccessManager(); + } __AccessManaged_init(accessManager); __UUPSUpgradeable_init(); From 178e3e3e75217e784909ae4472a5c3734e6f5539 Mon Sep 17 00:00:00 2001 From: Raul Date: Fri, 12 Apr 2024 20:36:08 -0300 Subject: [PATCH 4/4] fix duplicates --- contracts/lib/Errors.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index 65b021f3..1d321121 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -228,7 +228,6 @@ library Errors { // Dispute Module // //////////////////////////////////////////////////////////////////////////// - error DisputeModule__ZeroAccessManager(); error DisputeModule__ZeroArbitrationPolicy(); error DisputeModule__ZeroArbitrationRelayer(); error DisputeModule__ZeroDisputeTag(); @@ -253,7 +252,6 @@ library Errors { // ArbitrationPolicy SP // //////////////////////////////////////////////////////////////////////////// - error ArbitrationPolicySP__ZeroAccessManager(); error ArbitrationPolicySP__ZeroDisputeModule(); error ArbitrationPolicySP__ZeroPaymentToken(); error ArbitrationPolicySP__NotDisputeModule(); @@ -263,7 +261,6 @@ library Errors { // Royalty Module // //////////////////////////////////////////////////////////////////////////// - error RoyaltyModule__ZeroAccessManager(); error RoyaltyModule__ZeroRoyaltyPolicy(); error RoyaltyModule__NotWhitelistedRoyaltyPolicy(); error RoyaltyModule__ZeroRoyaltyToken();