From 062c725522844219565383eb273d8507f0dbe151 Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 16 Feb 2024 21:48:41 -0800 Subject: [PATCH 1/3] fix comments --- contracts/modules/licensing/LicensingModule.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/modules/licensing/LicensingModule.sol b/contracts/modules/licensing/LicensingModule.sol index 2c2c6542..f37ea79e 100644 --- a/contracts/modules/licensing/LicensingModule.sol +++ b/contracts/modules/licensing/LicensingModule.sol @@ -130,8 +130,8 @@ contract LicensingModule is AccessControlled, ILicensingModule, BaseModule, Reen } /// Adds a policy to an ipId, which can be used to mint licenses. - /// Licnses are permissions for ipIds to be derivatives (children). - /// if policyId is not defined in LicenseRegistry, reverts. + /// Licenses are permissions for ipIds to be derivatives (children). + /// If policyId is not defined in LicenseRegistry, reverts. /// Will revert if ipId already has the same policy /// @param ipId to receive the policy /// @param polId id of the policy data From 81dd9cefda4898a86f5be25ea65e33759d2d293e Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 16 Feb 2024 22:09:45 -0800 Subject: [PATCH 2/3] add contract and unit test adjusments to limit license minting for last node --- contracts/lib/Errors.sol | 1 + .../policies/RoyaltyPolicyLAP.sol | 25 +++++++--- .../modules/royalty/RoyaltyModule.t.sol | 50 +++++++++++++++---- .../modules/royalty/RoyaltyPolicyLAP.t.sol | 46 +++++++++++++++++ 4 files changed, 106 insertions(+), 16 deletions(-) diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index 1687fa0d..28a2c5f5 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -232,6 +232,7 @@ library Errors { error RoyaltyPolicyLAP__NotFullOwnership(); error RoyaltyPolicyLAP__UnlinkableToParents(); error RoyaltyPolicyLAP__TransferFailed(); + error RoyaltyPolicyLAP__LastPositionNotAbleToMintLicense(); error AncestorsVaultLAP__ZeroRoyaltyPolicyLAP(); error AncestorsVaultLAP__AlreadyClaimed(); diff --git a/contracts/modules/royalty-module/policies/RoyaltyPolicyLAP.sol b/contracts/modules/royalty-module/policies/RoyaltyPolicyLAP.sol index 0a59c4ee..73a75926 100644 --- a/contracts/modules/royalty-module/policies/RoyaltyPolicyLAP.sol +++ b/contracts/modules/royalty-module/policies/RoyaltyPolicyLAP.sol @@ -114,13 +114,24 @@ contract RoyaltyPolicyLAP is IRoyaltyPolicyLAP, Governable, ERC1155Holder, Reent if (data.royaltyStack + newLicenseRoyalty > TOTAL_RNFT_SUPPLY) revert Errors.RoyaltyPolicyLAP__AboveRoyaltyStackLimit(); - // If the policy is already initialized, it means that the ipId setup is already done. If not, it means that - // the license for this royalty policy is being minted for the first time parentIpIds are zero given that only - // roots can call _initPolicy() for the first time in the function onLicenseMinting() while derivatives already - // called _initPolicy() when linking to their parents with onLinkToParents() call. - address[] memory rootParents = new address[](0); - bytes[] memory rootParentRoyalties = new bytes[](0); - if (data.splitClone == address(0)) _initPolicy(_ipId, rootParents, rootParentRoyalties, _externalData); + if (data.splitClone == address(0)) { + // If the policy is already initialized, it means that the ipId setup is already done. If not, it means that + // the license for this royalty policy is being minted for the first time parentIpIds are zero given that only + // roots can call _initPolicy() for the first time in the function onLicenseMinting() while derivatives already + // called _initPolicy() when linking to their parents with onLinkToParents() call. + address[] memory rootParents = new address[](0); + bytes[] memory rootParentRoyalties = new bytes[](0); + _initPolicy(_ipId, rootParents, rootParentRoyalties, _externalData); + } else { + InitParams memory params = abi.decode(_externalData, (InitParams)); + // If the policy is already initialized and an ipId has the maximum number of ancestors + // it can not have any derivative and therefore is not allowed to mint any license + if (params.targetAncestors.length == MAX_ANCESTORS) revert Errors.RoyaltyPolicyLAP__LastPositionNotAbleToMintLicense(); + if ( + keccak256(abi.encodePacked(params.targetAncestors, params.targetRoyaltyAmount)) != + royaltyData[_ipId].ancestorsHash + ) revert Errors.RoyaltyPolicyLAP__InvalidAncestorsHash(); + } } /// @notice Executes royalty related logic on linking to parents diff --git a/test/foundry/modules/royalty/RoyaltyModule.t.sol b/test/foundry/modules/royalty/RoyaltyModule.t.sol index 40ebd1be..bc917752 100644 --- a/test/foundry/modules/royalty/RoyaltyModule.t.sol +++ b/test/foundry/modules/royalty/RoyaltyModule.t.sol @@ -205,22 +205,46 @@ contract TestRoyaltyModule is BaseTest { address licensor = address(3); bytes memory licenseData = abi.encode(uint32(15)); + address[] memory parents = new address[](2); + address[] memory targetAncestors1 = new address[](2); + uint32[] memory targetRoyaltyAmount1 = new uint32[](2); + uint32[] memory parentRoyalties1 = new uint32[](2); + bytes[] memory encodedLicenseData = new bytes[](2); + + address[] memory nullParentAncestors1 = new address[](0); + address[] memory nullParentAncestors2 = new address[](0); + uint32[] memory nullParentAncestorsRoyalties1 = new uint32[](0); + uint32[] memory nullParentAncestorsRoyalties2 = new uint32[](0); + + parents[0] = address(7); + parents[1] = address(8); + parentRoyalties1[0] = 7; + parentRoyalties1[1] = 8; + targetAncestors1[0] = address(7); + targetAncestors1[1] = address(8); + targetRoyaltyAmount1[0] = 7; + targetRoyaltyAmount1[1] = 8; + InitParams memory initParams = InitParams({ + targetAncestors: targetAncestors1, + targetRoyaltyAmount: targetRoyaltyAmount1, + parentAncestors1: nullParentAncestors1, + parentAncestors2: nullParentAncestors2, + parentAncestorsRoyalties1: nullParentAncestorsRoyalties1, + parentAncestorsRoyalties2: nullParentAncestorsRoyalties2 + }); + for (uint32 i = 0; i < parentRoyalties1.length; i++) { + encodedLicenseData[i] = abi.encode(parentRoyalties1[i]); + } + bytes memory encodedBytes = abi.encode(initParams); + vm.startPrank(address(licensingModule)); - royaltyModule.onLicenseMinting(licensor, address(royaltyPolicyLAP), licenseData, ""); + royaltyModule.onLicenseMinting(licensor, address(royaltyPolicyLAP), licenseData, encodedBytes); } function test_RoyaltyModule_onLicenseMinting_Root() public { address licensor = address(7); bytes memory licenseData = abi.encode(uint32(15)); - vm.startPrank(address(licensingModule)); - royaltyModule.onLicenseMinting(licensor, address(royaltyPolicyLAP), licenseData, ""); - vm.stopPrank(); - - vm.startPrank(u.admin); - royaltyModule.whitelistRoyaltyPolicy(address(royaltyPolicyLAP2), true); - vm.stopPrank(); - // mint a license of another policy address[] memory nullTargetAncestors = new address[](0); uint32[] memory nullTargetRoyaltyAmount = new uint32[](0); @@ -239,6 +263,14 @@ contract TestRoyaltyModule is BaseTest { }); bytes memory nullBytes = abi.encode(nullInitParams); + vm.startPrank(address(licensingModule)); + royaltyModule.onLicenseMinting(licensor, address(royaltyPolicyLAP), licenseData, nullBytes); + vm.stopPrank(); + + vm.startPrank(u.admin); + royaltyModule.whitelistRoyaltyPolicy(address(royaltyPolicyLAP2), true); + vm.stopPrank(); + vm.startPrank(address(licensingModule)); royaltyModule.onLicenseMinting(licensor, address(royaltyPolicyLAP2), licenseData, nullBytes); } diff --git a/test/foundry/modules/royalty/RoyaltyPolicyLAP.t.sol b/test/foundry/modules/royalty/RoyaltyPolicyLAP.t.sol index 56ea19b1..b4c20ba6 100644 --- a/test/foundry/modules/royalty/RoyaltyPolicyLAP.t.sol +++ b/test/foundry/modules/royalty/RoyaltyPolicyLAP.t.sol @@ -432,6 +432,52 @@ contract TestRoyaltyPolicyLAP is BaseTest { royaltyPolicyLAP.onLicenseMinting(address(100), abi.encode(uint32(1001)), inputBytes); } + function test_RoyaltyPolicyLAP_onLicenseMinting_revert_InvalidAncestors() public { + address[] memory targetAncestors = new address[](0); + uint32[] memory targetRoyaltyAmount = new uint32[](0); + address[] memory parentAncestors1 = new address[](0); + address[] memory parentAncestors2 = new address[](0); + uint32[] memory parentAncestorsRoyalties1 = new uint32[](0); + uint32[] memory parentAncestorsRoyalties2 = new uint32[](0); + InitParams memory initParams = InitParams({ + targetAncestors: targetAncestors, + targetRoyaltyAmount: targetRoyaltyAmount, + parentAncestors1: parentAncestors1, + parentAncestors2: parentAncestors2, + parentAncestorsRoyalties1: parentAncestorsRoyalties1, + parentAncestorsRoyalties2: parentAncestorsRoyalties2 + }); + bytes memory inputBytes = abi.encode(initParams); + + royaltyPolicyLAP.onLicenseMinting(address(100), abi.encode(uint32(0)), inputBytes); + + address[] memory targetAncestors2 = new address[](2); + initParams = InitParams({ + targetAncestors: targetAncestors2, + targetRoyaltyAmount: targetRoyaltyAmount, + parentAncestors1: parentAncestors1, + parentAncestors2: parentAncestors2, + parentAncestorsRoyalties1: parentAncestorsRoyalties1, + parentAncestorsRoyalties2: parentAncestorsRoyalties2 + }); + inputBytes = abi.encode(initParams); + + vm.expectRevert(Errors.RoyaltyPolicyLAP__InvalidAncestorsHash.selector); + royaltyPolicyLAP.onLicenseMinting(address(100), abi.encode(uint32(0)), inputBytes); + } + + function test_RoyaltyPolicyLAP_onLicenseMinting_revert_LastPositionNotAbleToMintLicense() public { + bytes[] memory encodedLicenseData = new bytes[](2); + for (uint32 i = 0; i < parentsIpIds100.length; i++) { + encodedLicenseData[i] = abi.encode(parentsIpIds100[i]); + } + + royaltyPolicyLAP.onLinkToParents(address(100), parentsIpIds100, encodedLicenseData, MAX_ANCESTORS); + + vm.expectRevert(Errors.RoyaltyPolicyLAP__LastPositionNotAbleToMintLicense.selector); + royaltyPolicyLAP.onLicenseMinting(address(100), abi.encode(uint32(0)), MAX_ANCESTORS); + } + function test_RoyaltyPolicyLAP_onLicenseMinting() public { address[] memory targetAncestors = new address[](0); uint32[] memory targetRoyaltyAmount = new uint32[](0); From 0ba59ad2afb6fc860813466e1674bef49c4f3516 Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 16 Feb 2024 22:25:22 -0800 Subject: [PATCH 3/3] add comment and fix --- .../modules/royalty-module/policies/RoyaltyPolicyLAP.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/contracts/modules/royalty-module/policies/RoyaltyPolicyLAP.sol b/contracts/modules/royalty-module/policies/RoyaltyPolicyLAP.sol index 73a75926..16e0ee82 100644 --- a/contracts/modules/royalty-module/policies/RoyaltyPolicyLAP.sol +++ b/contracts/modules/royalty-module/policies/RoyaltyPolicyLAP.sol @@ -126,7 +126,9 @@ contract RoyaltyPolicyLAP is IRoyaltyPolicyLAP, Governable, ERC1155Holder, Reent InitParams memory params = abi.decode(_externalData, (InitParams)); // If the policy is already initialized and an ipId has the maximum number of ancestors // it can not have any derivative and therefore is not allowed to mint any license - if (params.targetAncestors.length == MAX_ANCESTORS) revert Errors.RoyaltyPolicyLAP__LastPositionNotAbleToMintLicense(); + if (params.targetAncestors.length >= MAX_ANCESTORS) revert Errors.RoyaltyPolicyLAP__LastPositionNotAbleToMintLicense(); + // the check below ensures that the ancestors hash is the same as the one stored in the royalty data + // and that the targetAncestors passed in by the user matches the record stored in state on policy initialization if ( keccak256(abi.encodePacked(params.targetAncestors, params.targetRoyaltyAmount)) != royaltyData[_ipId].ancestorsHash