Adds ERC165 check for external royalty policy registration (#319)

* add ERC165 check for external policy registration

* fix lint

contracts/interfaces/modules/royalty/policies/IExternalRoyaltyPolicy.sol

@@ -1,11 +1,8 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity 0.8.26;
 
+import { IERC165 } from "@openzeppelin/contracts/utils/introspection/IERC165.sol";
+import { IExternalRoyaltyPolicyBase } from "./IExternalRoyaltyPolicyBase.sol";
+
 /// @title IExternalRoyaltyPolicy interface
-interface IExternalRoyaltyPolicy {
-    /// @notice Returns the amount of royalty tokens required to link a child to a given IP asset
-    /// @param ipId The ipId of the IP asset
-    /// @param licensePercent The percentage of the license
-    /// @return The amount of royalty tokens required to link a child to a given IP asset
-    function getPolicyRtsRequiredToLink(address ipId, uint32 licensePercent) external view returns (uint32);
-}
+interface IExternalRoyaltyPolicy is IExternalRoyaltyPolicyBase, IERC165 {}

contracts/interfaces/modules/royalty/policies/IExternalRoyaltyPolicyBase.sol

@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity 0.8.26;
+
+/// @title IExternalRoyaltyPolicyBase interface
+interface IExternalRoyaltyPolicyBase {
+    /// @notice Returns the amount of royalty tokens required to link a child to a given IP asset
+    /// @param ipId The ipId of the IP asset
+    /// @param licensePercent The percentage of the license
+    /// @return The amount of royalty tokens required to link a child to a given IP asset
+    function getPolicyRtsRequiredToLink(address ipId, uint32 licensePercent) external view returns (uint32);
+}

contracts/interfaces/modules/royalty/policies/IRoyaltyPolicy.sol

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity 0.8.26;
 
-import { IExternalRoyaltyPolicy } from "./IExternalRoyaltyPolicy.sol";
+import { IExternalRoyaltyPolicyBase } from "./IExternalRoyaltyPolicyBase.sol";
 
 /// @title RoyaltyPolicy interface
-interface IRoyaltyPolicy is IExternalRoyaltyPolicy {
+interface IRoyaltyPolicy is IExternalRoyaltyPolicyBase {
     /// @notice Executes royalty related logic on minting a license
     /// @dev Enforced to be only callable by RoyaltyModule
     /// @param ipId The ipId whose license is being minted (licensor)

contracts/lib/Errors.sol

@@ -616,6 +616,9 @@ library Errors {
     /// @notice IP is expired.
     error RoyaltyModule__IpExpired();
 
+    /// @notice Invalid external royalty policy.
+    error RoyaltyModule__InvalidExternalRoyaltyPolicy();
+
     ////////////////////////////////////////////////////////////////////////////
     //                            Royalty Policy LAP                          //
     ////////////////////////////////////////////////////////////////////////////

contracts/modules/royalty/RoyaltyModule.sol

@@ -14,7 +14,7 @@ import { BaseModule } from "../BaseModule.sol";
 import { VaultController } from "./policies/VaultController.sol";
 import { IRoyaltyModule } from "../../interfaces/modules/royalty/IRoyaltyModule.sol";
 import { IRoyaltyPolicy } from "../../interfaces/modules/royalty/policies/IRoyaltyPolicy.sol";
-import { IExternalRoyaltyPolicy } from "../../interfaces/modules/royalty/policies/IExternalRoyaltyPolicy.sol";
+import { IExternalRoyaltyPolicyBase } from "../../interfaces/modules/royalty/policies/IExternalRoyaltyPolicyBase.sol";
 import { IGroupIPAssetRegistry } from "../../interfaces/registries/IGroupIPAssetRegistry.sol";
 import { IIpRoyaltyVault } from "../../interfaces/modules/royalty/policies/IIpRoyaltyVault.sol";
 import { IDisputeModule } from "../../interfaces/modules/dispute/IDisputeModule.sol";
@@ -223,12 +223,17 @@ contract RoyaltyModule is IRoyaltyModule, VaultController, ReentrancyGuardUpgrad
             $.isRegisteredExternalRoyaltyPolicy[externalRoyaltyPolicy]
         ) revert Errors.RoyaltyModule__PolicyAlreadyWhitelistedOrRegistered();
 
-        // checks if the IExternalRoyaltyPolicy call does not revert
         // external royalty policies contracts should inherit IExternalRoyaltyPolicy interface
-        if (IExternalRoyaltyPolicy(externalRoyaltyPolicy).getPolicyRtsRequiredToLink(address(0), 0) >= uint32(0)) {
-            $.isRegisteredExternalRoyaltyPolicy[externalRoyaltyPolicy] = true;
-            emit ExternalRoyaltyPolicyRegistered(externalRoyaltyPolicy);
-        }
+        // and implement the getPolicyRtsRequiredToLink() and ERC165 supportsInterface() functions
+        if (
+            !ERC165Checker.supportsInterface(
+                externalRoyaltyPolicy,
+                IExternalRoyaltyPolicyBase.getPolicyRtsRequiredToLink.selector
+            )
+        ) revert Errors.RoyaltyModule__InvalidExternalRoyaltyPolicy();
+
+        $.isRegisteredExternalRoyaltyPolicy[externalRoyaltyPolicy] = true;
+        emit ExternalRoyaltyPolicyRegistered(externalRoyaltyPolicy);
     }
 
     /// @notice Executes royalty related logic on license minting

test/foundry/mocks/policy/MockExternalRoyaltyPolicy1.sol

@@ -1,11 +1,18 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity 0.8.26;
 
+import { IERC165, ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
+
 // solhint-disable-next-line max-line-length
 import { IExternalRoyaltyPolicy } from "../../../../contracts/interfaces/modules/royalty/policies/IExternalRoyaltyPolicy.sol";
 
-contract MockExternalRoyaltyPolicy1 is IExternalRoyaltyPolicy {
+contract MockExternalRoyaltyPolicy1 is ERC165, IExternalRoyaltyPolicy {
     function getPolicyRtsRequiredToLink(address ipId, uint32 licensePercent) external view returns (uint32) {
         return licensePercent * 2;
     }
+
+    /// @notice IERC165 interface support
+    function supportsInterface(bytes4 interfaceId) public view override(ERC165, IERC165) returns (bool) {
+        return interfaceId == this.getPolicyRtsRequiredToLink.selector || super.supportsInterface(interfaceId);
+    }
 }

test/foundry/mocks/policy/MockExternalRoyaltyPolicy2.sol

@@ -1,11 +1,18 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity 0.8.26;
 
+import { IERC165, ERC165 } from "@openzeppelin/contracts/utils/introspection/ERC165.sol";
+
 // solhint-disable-next-line max-line-length
 import { IExternalRoyaltyPolicy } from "../../../../contracts/interfaces/modules/royalty/policies/IExternalRoyaltyPolicy.sol";
 
-contract MockExternalRoyaltyPolicy2 is IExternalRoyaltyPolicy {
+contract MockExternalRoyaltyPolicy2 is ERC165, IExternalRoyaltyPolicy {
     function getPolicyRtsRequiredToLink(address ipId, uint32 licensePercent) external view returns (uint32) {
         return 10 * 10 ** 6;
     }
+
+    /// @notice IERC165 interface support
+    function supportsInterface(bytes4 interfaceId) public view override(ERC165, IERC165) returns (bool) {
+        return interfaceId == this.getPolicyRtsRequiredToLink.selector || super.supportsInterface(interfaceId);
+    }
 }

test/foundry/modules/royalty/RoyaltyModule.t.sol

@@ -380,6 +380,11 @@ contract TestRoyaltyModule is BaseTest {
         royaltyModule.registerExternalRoyaltyPolicy(address(royaltyPolicyLAP));
     }
 
+    function test_RoyaltyModule_registerExternalRoyaltyPolicy_revert_InvalidExternalRoyaltyPolicy() public {
+        vm.expectRevert(Errors.RoyaltyModule__InvalidExternalRoyaltyPolicy.selector);
+        royaltyModule.registerExternalRoyaltyPolicy(address(1));
+    }
+
     function test_RoyaltyModule_registerExternalRoyaltyPolicy() public {
         address externalRoyaltyPolicy = address(new MockExternalRoyaltyPolicy1());
         assertEq(royaltyModule.isRegisteredExternalRoyaltyPolicy(externalRoyaltyPolicy), false);