diff --git a/.husky/pre-push b/.husky/pre-push index f44b56b1..91649208 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -3,4 +3,4 @@ yarn lint:sol:fix # recheck -yarn lint:sol \ No newline at end of file +yarn solhint \ No newline at end of file diff --git a/contracts/governance/Governance.sol b/contracts/governance/Governance.sol index 8c37f8f0..2e42b0eb 100644 --- a/contracts/governance/Governance.sol +++ b/contracts/governance/Governance.sol @@ -10,6 +10,7 @@ import { GovernanceLib } from "../lib/GovernanceLib.sol"; /// @title Governance /// @dev This contract is used for governance of the protocol. +/// TODO: Replace with OZ's 2StepOwnable contract Governance is AccessControl, IGovernance { GovernanceLib.ProtocolState internal state; diff --git a/contracts/interfaces/modules/licensing/ILicensingModule.sol b/contracts/interfaces/modules/licensing/ILicensingModule.sol index a6f7f91d..e33d5a40 100644 --- a/contracts/interfaces/modules/licensing/ILicensingModule.sol +++ b/contracts/interfaces/modules/licensing/ILicensingModule.sol @@ -60,7 +60,11 @@ interface ILicensingModule is IModule { function registerPolicy(bool isLicenseTransferable, bytes memory data) external returns (uint256 policyId); /// @notice returns the policy id for the given data, or 0 if not found - function getPolicyId(address framework, bool isLicenseTransferable, bytes memory data) external view returns (uint256 policyId); + function getPolicyId( + address framework, + bool isLicenseTransferable, + bytes memory data + ) external view returns (uint256 policyId); /// @notice Adds a policy to an IP policy list /// @param ipId The id of the IP diff --git a/contracts/modules/RegistrationModule.sol b/contracts/modules/RegistrationModule.sol index 5fceb3d4..c453bb96 100644 --- a/contracts/modules/RegistrationModule.sol +++ b/contracts/modules/RegistrationModule.sol @@ -63,7 +63,8 @@ contract RegistrationModule is BaseModule, IRegistrationModule { // Check that the caller is authorized to perform the registration. // TODO: Perform additional registration authorization logic, allowing // registrants or root-IP creators to specify their own auth logic. - if (IERC721(tokenContract).ownerOf(tokenId) != msg.sender) { + address owner = IERC721(tokenContract).ownerOf(tokenId); + if (msg.sender != owner && !IERC721(tokenContract).isApprovedForAll(owner, msg.sender)) { revert Errors.RegistrationModule__InvalidOwner(); } @@ -120,7 +121,8 @@ contract RegistrationModule is BaseModule, IRegistrationModule { // Check that the caller is authorized to perform the registration. // TODO: Perform additional registration authorization logic, allowing // registrants or IP creators to specify their own auth logic. - if (IERC721(tokenContract).ownerOf(tokenId) != msg.sender) { + address owner = IERC721(tokenContract).ownerOf(tokenId); + if (msg.sender != owner && !IERC721(tokenContract).isApprovedForAll(owner, msg.sender)) { revert Errors.RegistrationModule__InvalidOwner(); } diff --git a/contracts/modules/licensing/LicensingModule.sol b/contracts/modules/licensing/LicensingModule.sol index 2e1519a1..e5ec5ce7 100644 --- a/contracts/modules/licensing/LicensingModule.sol +++ b/contracts/modules/licensing/LicensingModule.sol @@ -314,7 +314,11 @@ contract LicensingModule is AccessControlled, ILicensingModule { } /// @notice gets the policy id for the given data, or 0 if not found - function getPolicyId(address framework, bool isLicenseTransferable, bytes memory data) external view returns (uint256 policyId) { + function getPolicyId( + address framework, + bool isLicenseTransferable, + bytes memory data + ) external view returns (uint256 policyId) { Licensing.Policy memory pol = Licensing.Policy({ isLicenseTransferable: isLicenseTransferable, policyFramework: framework, @@ -505,7 +509,6 @@ contract LicensingModule is AccessControlled, ILicensingModule { _ipIdParents[childIpId].add(licensor); } - function _verifyCanAddPolicy(uint256 policyId, address ipId, bool isInherited) private { bool ipIdIsDerivative = _policySetPerIpId(true, ipId).length() > 0; if ( diff --git a/contracts/registries/IPAssetRegistry.sol b/contracts/registries/IPAssetRegistry.sol index 4c8ef7c5..eb3a9d62 100644 --- a/contracts/registries/IPAssetRegistry.sol +++ b/contracts/registries/IPAssetRegistry.sol @@ -16,6 +16,8 @@ import { LICENSING_MODULE_KEY } from "contracts/lib/modules/Module.sol"; import { IModuleRegistry } from "../interfaces/registries/IModuleRegistry.sol"; import { ILicensingModule } from "../interfaces/modules/licensing/ILicensingModule.sol"; import { IIPAssetRegistry } from "../interfaces/registries/IIPAssetRegistry.sol"; +import { IRegistrationModule } from "../interfaces/modules/IRegistrationModule.sol"; +import { Governable } from "../governance/Governable.sol"; /// @title IP Asset Registry /// @notice This contract acts as the source of truth for all IP registered in @@ -26,7 +28,7 @@ import { IIPAssetRegistry } from "../interfaces/registries/IIPAssetRegistry.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, Governable { /// @notice Attributes for the IP asset type. struct Record { // Metadata provider for Story Protocol canonicalized metadata. @@ -39,12 +41,12 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry { /// @notice The canonical module registry used by the protocol. IModuleRegistry public immutable MODULE_REGISTRY; + /// @notice The registration module that interacts with IPAssetRegistry. + IRegistrationModule public REGISTRATION_MODULE; + /// @notice Tracks the total number of IP assets in existence. uint256 public totalSupply = 0; - /// @notice Protocol governance administrator of the IP record registry. - address public owner; - /// @notice Checks whether an operator is approved to register on behalf of an IP owner. mapping(address owner => mapping(address operator => bool)) public isApprovedForAll; @@ -54,29 +56,20 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry { /// @notice Tracks the current metadata provider used for IP registrations. IMetadataProviderMigratable internal _metadataProvider; - /// @notice Ensures only protocol governance owner may call a function. - modifier onlyOwner() { - if (msg.sender != owner) { - revert Errors.IPAssetRegistry__Unauthorized(); - } - _; - } - /// @notice Initializes the IP Asset Registry. /// @param erc6551Registry The address of the ERC6551 registry. /// @param accessController The address of the access controller. /// @param ipAccountImpl The address of the IP account implementation. /// @param moduleRegistry The address of the module registry. + /// @param governance The address of the governance contract. /// TODO: Utilize module registry for fetching different modules. constructor( address accessController, address erc6551Registry, address ipAccountImpl, - address moduleRegistry - ) IPAccountRegistry(erc6551Registry, accessController, ipAccountImpl) { - // TODO: Migrate this to a parameterized governance owner address. - // TODO: Replace with OZ's 2StepOwnable - owner = msg.sender; + address moduleRegistry, + address governance + ) IPAccountRegistry(erc6551Registry, accessController, ipAccountImpl) Governable(governance) { MODULE_REGISTRY = IModuleRegistry(moduleRegistry); _metadataProvider = IMetadataProviderMigratable(new MetadataProviderV1(address(this))); } @@ -90,6 +83,12 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry { emit ApprovalForAll(msg.sender, operator, approved); } + /// @notice Sets the registration module that interacts with IPAssetRegistry. + /// @param registrationModule The address of the registration module. + function setRegistrationModule(address registrationModule) external onlyProtocolAdmin { + REGISTRATION_MODULE = IRegistrationModule(registrationModule); + } + /// @notice Registers an NFT as an IP asset. /// @param chainId The chain identifier of where the NFT resides. /// @param tokenContract The address of the NFT. @@ -180,7 +179,7 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry { /// @notice Sets the provider for storage of new IP metadata, while enabling /// existing IP assets to migrate their metadata to the new provider. /// @param newMetadataProvider Address of the new metadata provider contract. - function setMetadataProvider(address newMetadataProvider) external onlyOwner { + function setMetadataProvider(address newMetadataProvider) external onlyProtocolAdmin { _metadataProvider.setUpgradeProvider(newMetadataProvider); _metadataProvider = IMetadataProviderMigratable(newMetadataProvider); } @@ -238,8 +237,10 @@ contract IPAssetRegistry is IIPAssetRegistry, IPAccountRegistry { revert Errors.IPAssetRegistry__AlreadyRegistered(); } - address owner = IERC721(tokenContract).ownerOf(tokenId); - if (msg.sender != owner && !isApprovedForAll[owner][msg.sender]) { + address _owner = IERC721(tokenContract).ownerOf(tokenId); + if ( + msg.sender != _owner && msg.sender != address(REGISTRATION_MODULE) && !isApprovedForAll[_owner][msg.sender] + ) { revert Errors.IPAssetRegistry__RegistrantUnauthorized(); } diff --git a/package.json b/package.json index 9b14af13..9e0aa0b7 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "lint:js:fix": "prettier --log-level warn --ignore-path .gitignore '**/*.{js,ts}' --write && eslint --ignore-path .gitignore . --fix", "lint:sol": "prettier --log-level warn --ignore-path .gitignore '{contracts,test}/**/*.sol' --check && solhint '{contracts,test}/**/*.sol'", "lint:sol:fix": "prettier --log-level warn --ignore-path .gitignore '{contracts,test}/**/*.sol' --write", + "solhint": "solhint '{contracts,test}/**/*.sol'", "test": "npx hardhat test", "prepare": "husky install" }, diff --git a/script/foundry/deployment/Main.s.sol b/script/foundry/deployment/Main.s.sol index 933a2fe6..4982e124 100644 --- a/script/foundry/deployment/Main.s.sol +++ b/script/foundry/deployment/Main.s.sol @@ -164,7 +164,13 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler { contractKey = "IPAssetRegistry"; _predeploy(contractKey); - ipAssetRegistry = new IPAssetRegistry(address(accessController), ERC6551_REGISTRY, address(ipAccountImpl), address(moduleRegistry)); + ipAssetRegistry = new IPAssetRegistry( + address(accessController), + ERC6551_REGISTRY, + address(ipAccountImpl), + address(moduleRegistry), + address(governance) + ); _postdeploy(contractKey, address(ipAssetRegistry)); contractKey = "MetadataProviderV1"; @@ -290,6 +296,7 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler { _postdeploy("IPMetadataProvider", address(ipMetadataProvider)); licenseRegistry.setLicensingModule(address(licensingModule)); + ipAssetRegistry.setRegistrationModule(address(registrationModule)); } function _configureAccessController() private { diff --git a/test/foundry/IPAssetRegistry.t.sol b/test/foundry/IPAssetRegistry.t.sol index ababb84a..8bf5975c 100644 --- a/test/foundry/IPAssetRegistry.t.sol +++ b/test/foundry/IPAssetRegistry.t.sol @@ -69,7 +69,13 @@ contract IPAssetRegistryTest is BaseTest { erc6551Registry = address(new ERC6551Registry()); ipAccountImpl = address(new IPAccountImpl()); ipAccountRegistry = new IPAccountRegistry(erc6551Registry, accessController, ipAccountImpl); - registry = new IPAssetRegistry(accessController, erc6551Registry, ipAccountImpl, address(moduleRegistry)); + registry = new IPAssetRegistry( + accessController, + erc6551Registry, + ipAccountImpl, + address(moduleRegistry), + address(governance) + ); MockERC721 erc721 = new MockERC721("MockERC721"); tokenAddress = address(erc721); tokenId = erc721.mintId(alice, 99); diff --git a/test/foundry/integration/BaseIntegration.sol b/test/foundry/integration/BaseIntegration.sol index 9ec82595..8ced1b80 100644 --- a/test/foundry/integration/BaseIntegration.sol +++ b/test/foundry/integration/BaseIntegration.sol @@ -123,7 +123,8 @@ contract BaseIntegration is Test { address(accessController), address(erc6551Registry), address(ipAccountImpl), - address(moduleRegistry) + address(moduleRegistry), + address(governance) ); licenseRegistry = new LicenseRegistry(); licensingModule = new LicensingModule( @@ -173,6 +174,7 @@ contract BaseIntegration is Test { vm.startPrank(u.admin); accessController.initialize(address(ipAccountRegistry), address(moduleRegistry)); royaltyModule.setLicensingModule(address(licensingModule)); + ipAssetRegistry.setRegistrationModule(address(registrationModule)); moduleRegistry.registerModule(REGISTRATION_MODULE_KEY, address(registrationModule)); moduleRegistry.registerModule(IP_RESOLVER_MODULE_KEY, address(ipResolver)); diff --git a/test/foundry/mocks/licensing/MockLicensingModule.sol b/test/foundry/mocks/licensing/MockLicensingModule.sol index b88315cf..d5209f2a 100644 --- a/test/foundry/mocks/licensing/MockLicensingModule.sol +++ b/test/foundry/mocks/licensing/MockLicensingModule.sol @@ -37,7 +37,11 @@ contract MockLicensingModule is ILicensingModule { function isPolicyIdSetForIp(bool isInherited, address ipId, uint256 policyId) external view returns (bool) {} - function getPolicyId(address framework, bool isLicenseTransferable, bytes memory data) external view returns (uint256 policyId) {} + function getPolicyId( + address framework, + bool isLicenseTransferable, + bytes memory data + ) external view returns (uint256 policyId) {} function policyIdForIpAtIndex( bool isInherited, diff --git a/test/foundry/modules/ModuleBase.t.sol b/test/foundry/modules/ModuleBase.t.sol index 85b00ecc..daa52d0b 100644 --- a/test/foundry/modules/ModuleBase.t.sol +++ b/test/foundry/modules/ModuleBase.t.sol @@ -14,6 +14,8 @@ import { IPAssetRegistry } from "contracts/registries/IPAssetRegistry.sol"; import { IPAccountImpl } from "contracts/IPAccountImpl.sol"; import { Governance } from "contracts/governance/Governance.sol"; import { RoyaltyModule } from "contracts/modules/royalty-module/RoyaltyModule.sol"; +import { IPResolver } from "contracts/resolvers/IPResolver.sol"; +import { RegistrationModule } from "contracts/modules/RegistrationModule.sol"; /// @title Module Base Test Contract /// @notice Base contract for testing standard module functionality. @@ -41,6 +43,10 @@ abstract contract ModuleBaseTest is BaseTest { RoyaltyModule public royaltyModule; + IPResolver public ipResolver; + + RegistrationModule public registrationModule; + /// @notice Initializes the base module for testing. function setUp() public virtual override(BaseTest) { BaseTest.setUp(); @@ -53,7 +59,8 @@ abstract contract ModuleBaseTest is BaseTest { address(accessController), address(new ERC6551Registry()), address(new IPAccountImpl()), - address(moduleRegistry) + address(moduleRegistry), + address(governance) ); royaltyModule = new RoyaltyModule(address(governance)); licenseRegistry = new LicenseRegistry(); @@ -63,10 +70,18 @@ abstract contract ModuleBaseTest is BaseTest { address(licenseRegistry), address(royaltyModule) ); + ipResolver = new IPResolver(address(accessController), address(ipAssetRegistry)); + registrationModule = new RegistrationModule( + address(accessController), + address(ipAssetRegistry), + address(licensingModule), + address(ipResolver) + ); licenseRegistry.setLicensingModule(address(licensingModule)); baseModule = IModule(_deployModule()); accessController.initialize(address(ipAssetRegistry), address(moduleRegistry)); royaltyModule.setLicensingModule(address(licensingModule)); + ipAssetRegistry.setRegistrationModule(address(registrationModule)); } /// @notice Tests that the default resolver constructor runs successfully. diff --git a/test/foundry/modules/licensing/LicensingModule.t.sol b/test/foundry/modules/licensing/LicensingModule.t.sol index 785686fb..b6efcf3a 100644 --- a/test/foundry/modules/licensing/LicensingModule.t.sol +++ b/test/foundry/modules/licensing/LicensingModule.t.sol @@ -19,6 +19,8 @@ import { LicenseRegistry } from "contracts/registries/LicenseRegistry.sol"; import { LicensingModule } from "contracts/modules/licensing/LicensingModule.sol"; import { RoyaltyModule } from "contracts/modules/royalty-module/RoyaltyModule.sol"; import { IPAssetRegistry } from "contracts/registries/IPAssetRegistry.sol"; +import { IPResolver } from "contracts/resolvers/IPResolver.sol"; +import { RegistrationModule } from "contracts/modules/RegistrationModule.sol"; // test // solhint-disable-next-line max-line-length @@ -74,7 +76,8 @@ contract LicensingModuleTest is Test { address(accessController), address(erc6551Registry), address(ipAccountImpl), - address(moduleRegistry) + address(moduleRegistry), + address(governance) ); royaltyModule = new RoyaltyModule(address(governance)); licenseRegistry = new LicenseRegistry(); @@ -105,7 +108,15 @@ contract LicensingModuleTest is Test { "UMLPolicyFrameworkManager", licenseUrl ); + IPResolver ipResolver = new IPResolver(address(accessController), address(ipAssetRegistry)); + RegistrationModule registrationModule = new RegistrationModule( + address(accessController), + address(ipAssetRegistry), + address(licensingModule), + address(ipResolver) + ); + ipAssetRegistry.setRegistrationModule(address(registrationModule)); // Set licensing module in royalty module royaltyModule.setLicensingModule(address(licensingModule)); royaltyModule.whitelistRoyaltyPolicy(address(mockRoyaltyPolicyLS), true); diff --git a/test/foundry/registries/metadata/IPAssetRenderer.t.sol b/test/foundry/registries/metadata/IPAssetRenderer.t.sol index d4ee2d0c..b2593638 100644 --- a/test/foundry/registries/metadata/IPAssetRenderer.t.sol +++ b/test/foundry/registries/metadata/IPAssetRenderer.t.sol @@ -95,7 +95,8 @@ contract IPAssetRendererTest is BaseTest { address(accessController), address(erc6551Registry), address(ipAccountImpl), - address(moduleRegistry) + address(moduleRegistry), + address(governance) ); RoyaltyModule royaltyModule = new RoyaltyModule(address(governance)); licenseRegistry = new LicenseRegistry(); @@ -121,6 +122,7 @@ contract IPAssetRendererTest is BaseTest { ); accessController.initialize(address(ipAccountRegistry), address(moduleRegistry)); royaltyModule.setLicensingModule(address(licensingModule)); + ipAssetRegistry.setRegistrationModule(address(registrationModule)); vm.prank(alice); uint256 tokenId = erc721.mintId(alice, 99); diff --git a/test/foundry/registries/metadata/MetadataProvider.t.sol b/test/foundry/registries/metadata/MetadataProvider.t.sol index 32616e19..eb4b166c 100644 --- a/test/foundry/registries/metadata/MetadataProvider.t.sol +++ b/test/foundry/registries/metadata/MetadataProvider.t.sol @@ -18,6 +18,8 @@ import { MetadataProviderV1 } from "contracts/registries/metadata/MetadataProvid import { IMetadataProvider } from "contracts/interfaces/registries/metadata/IMetadataProvider.sol"; import { Errors } from "contracts/lib/Errors.sol"; import { MockERC721 } from "test/foundry/mocks/MockERC721.sol"; +import { IPResolver } from "contracts/resolvers/IPResolver.sol"; +import { RegistrationModule } from "contracts/modules/RegistrationModule.sol"; /// @title IP Metadata Provider Testing Contract /// @notice Contract for metadata provider settings. @@ -119,19 +121,26 @@ contract MetadataProviderTest is BaseTest { address(accessController), address(ipAccountImpl) ); - vm.prank(bob); registry = new IPAssetRegistry( address(accessController), address(erc6551Registry), address(ipAccountImpl), - address(licensingModule) + address(licensingModule), + address(governance) + ); + IPResolver ipResolver = new IPResolver(address(accessController), address(registry)); + RegistrationModule registrationModule = new RegistrationModule( + address(accessController), + address(registry), + address(licensingModule), + address(ipResolver) ); accessController.initialize(address(ipAccountRegistry), address(moduleRegistry)); + registry.setRegistrationModule(address(registrationModule)); MockERC721 erc721 = new MockERC721("MockERC721"); uint256 tokenId = erc721.mintId(alice, 99); - IPResolver resolver = new IPResolver(address(accessController), address(registry)); vm.prank(alice); - ipId = registry.register(block.chainid, address(erc721), tokenId, address(resolver), true, v1Metadata); + ipId = registry.register(block.chainid, address(erc721), tokenId, address(ipResolver), true, v1Metadata); metadataProvider = MetadataProviderV1(registry.metadataProvider()); upgradedProvider = new MockMetadataProviderV2(address(registry)); diff --git a/test/foundry/resolvers/IPResolver.t.sol b/test/foundry/resolvers/IPResolver.t.sol index 024c39ca..67ebe986 100644 --- a/test/foundry/resolvers/IPResolver.t.sol +++ b/test/foundry/resolvers/IPResolver.t.sol @@ -16,9 +16,6 @@ contract IPResolverTest is ResolverBaseTest { string public constant TEST_KEY = "Key"; string public constant TEST_VALUE = "Value"; - /// @notice The token contract SUT. - IPResolver public ipResolver; - /// @notice Mock IP identifier for resolver testing. address public ipId; diff --git a/test/foundry/utils/DeployHelper.sol b/test/foundry/utils/DeployHelper.sol index 1d4b4eb1..d4973ed1 100644 --- a/test/foundry/utils/DeployHelper.sol +++ b/test/foundry/utils/DeployHelper.sol @@ -116,7 +116,8 @@ contract DeployHelper is Test { address(accessController), address(erc6551Registry), address(ipAccountImpl), - address(moduleRegistry) + address(moduleRegistry), + address(governance) ); licenseRegistry = new LicenseRegistry(); licensingModule = new LicensingModule( @@ -125,7 +126,6 @@ contract DeployHelper is Test { address(royaltyModule), address(licenseRegistry) ); - licenseRegistry.setLicensingModule(address(licensingModule)); ipMetadataProvider = new IPMetadataProvider(address(moduleRegistry)); ipResolver = new IPResolver(address(accessController), address(ipAssetRegistry)); registrationModule = new RegistrationModule( @@ -170,6 +170,8 @@ contract DeployHelper is Test { vm.startPrank(u.admin); accessController.initialize(address(ipAccountRegistry), address(moduleRegistry)); royaltyModule.setLicensingModule(address(licensingModule)); + licenseRegistry.setLicensingModule(address(licensingModule)); + ipAssetRegistry.setRegistrationModule(address(registrationModule)); moduleRegistry.registerModule(REGISTRATION_MODULE_KEY, address(registrationModule)); moduleRegistry.registerModule(IP_RESOLVER_MODULE_KEY, address(ipResolver));