From 224a2b1c90f2de85b3825974bccc5436b25ad207 Mon Sep 17 00:00:00 2001 From: Spablob Date: Thu, 4 Apr 2024 22:46:40 +0300 Subject: [PATCH 1/9] Move LAPRoyaltyData struct to interface --- .../royalty/policies/IRoyaltyPolicyLAP.sol | 14 ++++++++++++++ .../royalty/policies/RoyaltyPolicyLAP.sol | 18 ++---------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol b/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol index 5ab55359..cd1adf01 100644 --- a/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol +++ b/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol @@ -19,6 +19,20 @@ interface IRoyaltyPolicyLAP is IRoyaltyPolicy { uint32[] targetRoyaltyAmount ); + /// @notice The state data of the LAP royalty policy + /// @param isUnlinkableToParents Indicates if the ipId is unlinkable to new parents + /// @param ipRoyaltyVault The ip royalty vault address + /// @param royaltyStack The royalty stack of a given ipId is the sum of the royalties to be paid to each ancestors + /// @param ancestorsAddresses The ancestors addresses array + /// @param ancestorsRoyalties The ancestors royalties array + struct LAPRoyaltyData { + bool isUnlinkableToParents; + address ipRoyaltyVault; + uint32 royaltyStack; + address[] ancestorsAddresses; + uint32[] ancestorsRoyalties; + } + /// @notice Returns the percentage scale - represents 100% of royalty tokens for an ip function TOTAL_RT_SUPPLY() external view returns (uint32); diff --git a/contracts/modules/royalty/policies/RoyaltyPolicyLAP.sol b/contracts/modules/royalty/policies/RoyaltyPolicyLAP.sol index ef323338..32240c1f 100644 --- a/contracts/modules/royalty/policies/RoyaltyPolicyLAP.sol +++ b/contracts/modules/royalty/policies/RoyaltyPolicyLAP.sol @@ -18,20 +18,6 @@ import { Errors } from "../../../lib/Errors.sol"; contract RoyaltyPolicyLAP is IRoyaltyPolicyLAP, GovernableUpgradeable, ReentrancyGuardUpgradeable, UUPSUpgradeable { using SafeERC20 for IERC20; - /// @notice The state data of the LAP royalty policy - /// @param isUnlinkableToParents Indicates if the ipId is unlinkable to new parents - /// @param ipRoyaltyVault The ip royalty vault address - /// @param royaltyStack The royalty stack of a given ipId is the sum of the royalties to be paid to each ancestors - /// @param ancestorsAddresses The ancestors addresses array - /// @param ancestorsRoyalties The ancestors royalties array - struct LAPRoyaltyData { - bool isUnlinkableToParents; - address ipRoyaltyVault; - uint32 royaltyStack; - address[] ancestorsAddresses; - uint32[] ancestorsRoyalties; - } - /// @dev Storage structure for the RoyaltyPolicyLAP /// @param ipRoyaltyVaultBeacon The ip royalty vault beacon address /// @param snapshotInterval The minimum timestamp interval between snapshots @@ -118,7 +104,7 @@ contract RoyaltyPolicyLAP is IRoyaltyPolicyLAP, GovernableUpgradeable, Reentranc address ipId, bytes calldata licenseData, bytes calldata externalData - ) external onlyRoyaltyModule { + ) external onlyRoyaltyModule nonReentrant { uint32 newLicenseRoyalty = abi.decode(licenseData, (uint32)); RoyaltyPolicyLAPStorage storage $ = _getRoyaltyPolicyLAPStorage(); @@ -153,7 +139,7 @@ contract RoyaltyPolicyLAP is IRoyaltyPolicyLAP, GovernableUpgradeable, Reentranc address[] calldata parentIpIds, bytes[] memory licenseData, bytes calldata externalData - ) external onlyRoyaltyModule { + ) external onlyRoyaltyModule nonReentrant { RoyaltyPolicyLAPStorage storage $ = _getRoyaltyPolicyLAPStorage(); if ($.royaltyData[ipId].isUnlinkableToParents) revert Errors.RoyaltyPolicyLAP__UnlinkableToParents(); From 7993cdee48d04d8d68dad04ffdd284306e1b6515 Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 5 Apr 2024 09:28:59 +0300 Subject: [PATCH 2/9] variable name fix --- contracts/modules/dispute/DisputeModule.sol | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/modules/dispute/DisputeModule.sol b/contracts/modules/dispute/DisputeModule.sol index 3ae0eec7..e99c4080 100644 --- a/contracts/modules/dispute/DisputeModule.sol +++ b/contracts/modules/dispute/DisputeModule.sol @@ -64,11 +64,11 @@ contract DisputeModule is IIPAssetRegistry public immutable IP_ASSET_REGISTRY; /// Constructor - /// @param _controller The address of the access controller - /// @param _assetRegistry The address of the asset registry + /// @param controller The address of the access controller + /// @param assetRegistry The address of the asset registry /// @custom:oz-upgrades-unsafe-allow constructor - constructor(address _controller, address _assetRegistry) AccessControlled(_controller, _assetRegistry) { - IP_ASSET_REGISTRY = IIPAssetRegistry(_assetRegistry); + constructor(address controller, address assetRegistry) AccessControlled(controller, assetRegistry) { + IP_ASSET_REGISTRY = IIPAssetRegistry(assetRegistry); _disableInitializers(); } @@ -169,9 +169,9 @@ contract DisputeModule is address arbitrationPolicy = $.arbitrationPolicies[targetIpId]; if (!$.isWhitelistedArbitrationPolicy[arbitrationPolicy]) arbitrationPolicy = $.baseArbitrationPolicy; - uint256 disputeId_ = ++$.disputeCounter; + uint256 disputeId = ++$.disputeCounter; - $.disputes[disputeId_] = Dispute({ + $.disputes[disputeId] = Dispute({ targetIpId: targetIpId, disputeInitiator: msg.sender, arbitrationPolicy: arbitrationPolicy, @@ -183,7 +183,7 @@ contract DisputeModule is IArbitrationPolicy(arbitrationPolicy).onRaiseDispute(msg.sender, data); emit DisputeRaised( - disputeId_, + disputeId, targetIpId, msg.sender, arbitrationPolicy, @@ -192,7 +192,7 @@ contract DisputeModule is data ); - return disputeId_; + return disputeId; } /// @notice Sets the dispute judgement on a given dispute. Only whitelisted arbitration relayers can call to judge. From 809f9f36b9860dbad0687843f4c091edfb8ad491 Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 5 Apr 2024 10:37:48 +0300 Subject: [PATCH 3/9] add pay royalty payment restrictions to tagged ip assets --- .../modules/royalty/IRoyaltyModule.sol | 9 ++ contracts/modules/royalty/RoyaltyModule.sol | 20 +++- .../modules/royalty/RoyaltyModule.t.sol | 111 +++++++++++++++++- 3 files changed, 132 insertions(+), 8 deletions(-) diff --git a/contracts/interfaces/modules/royalty/IRoyaltyModule.sol b/contracts/interfaces/modules/royalty/IRoyaltyModule.sol index 1cee0596..81a014cd 100644 --- a/contracts/interfaces/modules/royalty/IRoyaltyModule.sol +++ b/contracts/interfaces/modules/royalty/IRoyaltyModule.sol @@ -30,9 +30,18 @@ interface IRoyaltyModule is IModule { /// @param amount The amount paid event LicenseMintingFeePaid(address receiverIpId, address payerAddress, address token, uint256 amount); + /// @notice Sets the license registry + /// @dev Enforced to be only callable by the protocol admin + /// @param licensing The address of the license registry + /// @param dispute The address of the dispute module + function setLicensingAndDisputeModules(address licensing, address dispute) external; + /// @notice Returns the licensing module address function licensingModule() external view returns (address); + /// @notice Returns the dispute module address + function disputeModule() external view returns (address); + /// @notice Indicates if a royalty policy is whitelisted /// @param royaltyPolicy The address of the royalty policy /// @return isWhitelisted True if the royalty policy is whitelisted diff --git a/contracts/modules/royalty/RoyaltyModule.sol b/contracts/modules/royalty/RoyaltyModule.sol index 9a34d305..01cffefd 100644 --- a/contracts/modules/royalty/RoyaltyModule.sol +++ b/contracts/modules/royalty/RoyaltyModule.sol @@ -10,6 +10,7 @@ import { BaseModule } from "../BaseModule.sol"; import { GovernableUpgradeable } from "../../governance/GovernableUpgradeable.sol"; import { IRoyaltyModule } from "../../interfaces/modules/royalty/IRoyaltyModule.sol"; import { IRoyaltyPolicy } from "../../interfaces/modules/royalty/policies/IRoyaltyPolicy.sol"; +import { IDisputeModule } from "../../interfaces/modules/dispute/IDisputeModule.sol"; import { Errors } from "../../lib/Errors.sol"; import { ROYALTY_MODULE_KEY } from "../../lib/modules/Module.sol"; import { BaseModule } from "../BaseModule.sol"; @@ -27,12 +28,14 @@ contract RoyaltyModule is using ERC165Checker for address; /// @dev Storage structure for the RoyaltyModule + /// @param disputeModule The address of the dispute module /// @param licensingModule The address of the licensing module /// @param isWhitelistedRoyaltyPolicy Indicates if a royalty policy is whitelisted /// @param isWhitelistedRoyaltyToken Indicates if a royalty token is whitelisted /// @param royaltyPolicies Indicates the royalty policy for a given IP asset /// @custom:storage-location erc7201:story-protocol.RoyaltyModule struct RoyaltyModuleStorage { + address disputeModule; address licensingModule; mapping(address royaltyPolicy => bool isWhitelisted) isWhitelistedRoyaltyPolicy; mapping(address token => bool) isWhitelistedRoyaltyToken; @@ -69,9 +72,13 @@ contract RoyaltyModule is /// @notice Sets the license registry /// @dev Enforced to be only callable by the protocol admin /// @param licensing The address of the license registry - function setLicensingModule(address licensing) external onlyProtocolAdmin { + /// @param dispute The address of the dispute module + function setLicensingAndDisputeModules(address licensing, address dispute) external onlyProtocolAdmin { if (licensing == address(0)) revert Errors.RoyaltyModule__ZeroLicensingModule(); + if (dispute == address(0)) revert Errors.RoyaltyModule__ZeroDisputeModule(); + _getRoyaltyModuleStorage().licensingModule = licensing; + _getRoyaltyModuleStorage().disputeModule = dispute; } /// @notice Whitelist a royalty policy @@ -175,6 +182,10 @@ contract RoyaltyModule is RoyaltyModuleStorage storage $ = _getRoyaltyModuleStorage(); if (!$.isWhitelistedRoyaltyToken[token]) revert Errors.RoyaltyModule__NotWhitelistedRoyaltyToken(); + IDisputeModule dispute = IDisputeModule($.disputeModule); + if (dispute.isIpTagged(receiverIpId)) revert Errors.RoyaltyModule__IpIsTagged(); + if (dispute.isIpTagged(payerIpId)) revert Errors.RoyaltyModule__IpIsTagged(); + address payerRoyaltyPolicy = $.royaltyPolicies[payerIpId]; // if the payer does not have a royalty policy set, then the payer is not a derivative ip and does not pay // royalties while the receiver ip can have a zero royalty policy since that could mean it is an ip a root @@ -202,7 +213,7 @@ contract RoyaltyModule is ) external onlyLicensingModule { RoyaltyModuleStorage storage $ = _getRoyaltyModuleStorage(); if (!$.isWhitelistedRoyaltyToken[token]) revert Errors.RoyaltyModule__NotWhitelistedRoyaltyToken(); - + if (IDisputeModule($.disputeModule).isIpTagged(receiverIpId)) revert Errors.RoyaltyModule__IpIsTagged(); if (licenseRoyaltyPolicy == address(0)) revert Errors.RoyaltyModule__NoRoyaltyPolicySet(); if (!$.isWhitelistedRoyaltyPolicy[licenseRoyaltyPolicy]) revert Errors.RoyaltyModule__NotWhitelistedRoyaltyPolicy(); @@ -217,6 +228,11 @@ contract RoyaltyModule is return _getRoyaltyModuleStorage().licensingModule; } + /// @notice Returns the dispute module address + function disputeModule() external view returns (address) { + return _getRoyaltyModuleStorage().disputeModule; + } + /// @notice Indicates if a royalty policy is whitelisted /// @param royaltyPolicy The address of the royalty policy /// @return isWhitelisted True if the royalty policy is whitelisted diff --git a/test/foundry/modules/royalty/RoyaltyModule.t.sol b/test/foundry/modules/royalty/RoyaltyModule.t.sol index fc3b4553..1edc2b4a 100644 --- a/test/foundry/modules/royalty/RoyaltyModule.t.sol +++ b/test/foundry/modules/royalty/RoyaltyModule.t.sol @@ -1,10 +1,13 @@ // SPDX-License-Identifier: BUSL-1.1 pragma solidity 0.8.23; +import { ERC6551AccountLib } from "erc6551/lib/ERC6551AccountLib.sol"; + // contracts import { Errors } from "../../../../contracts/lib/Errors.sol"; import { RoyaltyModule } from "../../../../contracts/modules/royalty/RoyaltyModule.sol"; import { RoyaltyPolicyLAP } from "../../../../contracts/modules/royalty/policies/RoyaltyPolicyLAP.sol"; +import { PILPolicy } from "contracts/modules/licensing/PILPolicyFrameworkManager.sol"; import { TestProxyHelper } from "test/foundry/utils/TestProxyHelper.sol"; // tests @@ -19,6 +22,9 @@ contract TestRoyaltyModule is BaseTest { address internal ipAccount1 = address(0x111000aaa); address internal ipAccount2 = address(0x111000bbb); + address internal ipAddr; + address internal arbitrationRelayer; + struct InitParams { address[] targetAncestors; uint32[] targetRoyaltyAmount; @@ -39,9 +45,9 @@ contract TestRoyaltyModule is BaseTest { function setUp() public override { super.setUp(); buildDeployModuleCondition( - DeployModuleCondition({ disputeModule: false, royaltyModule: true, licensingModule: false }) + DeployModuleCondition({ disputeModule: true, royaltyModule: true, licensingModule: false }) ); - buildDeployPolicyCondition(DeployPolicyCondition({ arbitrationPolicySP: false, royaltyPolicyLAP: true })); + buildDeployPolicyCondition(DeployPolicyCondition({ arbitrationPolicySP: true, royaltyPolicyLAP: true })); deployConditionally(); postDeploymentSetup(); @@ -52,6 +58,8 @@ contract TestRoyaltyModule is BaseTest { TestProxyHelper.deployUUPSProxy(impl, abi.encodeCall(RoyaltyPolicyLAP.initialize, (getGovernance()))) ); + arbitrationRelayer = u.admin; + vm.startPrank(u.admin); // whitelist royalty policy royaltyModule.whitelistRoyaltyPolicy(address(royaltyPolicyLAP), true); @@ -64,6 +72,51 @@ contract TestRoyaltyModule is BaseTest { // split made to avoid stack too deep error _setupTree(); vm.stopPrank(); + + USDC.mint(ipAccount1, 1000 * 10 ** 6); + + _setPILPolicyFrameworkManager(); + _addPILPolicy( + "cheap_flexible", + true, + address(royaltyPolicyLAP), + PILPolicy({ + attribution: false, + commercialUse: true, + commercialAttribution: true, + commercializerChecker: address(0), + commercializerCheckerData: "", + commercialRevShare: 10, + derivativesAllowed: true, + derivativesAttribution: true, + derivativesApproval: false, + derivativesReciprocal: false, + territories: new string[](0), + distributionChannels: new string[](0), + contentRestrictions: new string[](0) + }) + ); + + mockNFT.mintId(u.alice, 0); + + address expectedAddr = ERC6551AccountLib.computeAddress( + address(erc6551Registry), + address(ipAccountImpl), + ipAccountRegistry.IP_ACCOUNT_SALT(), + block.chainid, + address(mockNFT), + 0 + ); + vm.label(expectedAddr, "IPAccount0"); + + vm.startPrank(u.alice); + ipAddr = ipAssetRegistry.register(address(mockNFT), 0); + licensingModule.addPolicyToIp(ipAddr, policyIds["pil_cheap_flexible"]); + + // set arbitration policy + vm.startPrank(ipAddr); + disputeModule.setArbitrationPolicy(ipAddr, address(arbitrationPolicySP)); + vm.stopPrank(); } function _setupTree() internal { @@ -119,24 +172,35 @@ contract TestRoyaltyModule is BaseTest { royaltyModule.onLinkToParents(address(3), address(royaltyPolicyLAP), parents, encodedLicenseData, encodedBytes); } - function test_RoyaltyModule_setLicensingModule_revert_ZeroLicensingModule() public { + function test_RoyaltyModule_setLicensingAndDisputeModules_revert_ZeroLicensingModule() public { address impl = address(new RoyaltyModule()); RoyaltyModule testRoyaltyModule = RoyaltyModule( TestProxyHelper.deployUUPSProxy(impl, abi.encodeCall(RoyaltyModule.initialize, (address(getGovernance())))) ); vm.expectRevert(Errors.RoyaltyModule__ZeroLicensingModule.selector); vm.prank(u.admin); - testRoyaltyModule.setLicensingModule(address(0)); + testRoyaltyModule.setLicensingAndDisputeModules(address(0), address(1)); + } + + function test_RoyaltyModule_setLicensingAndDisputeModules_revert_ZeroDisputeModule() public { + address impl = address(new RoyaltyModule()); + RoyaltyModule testRoyaltyModule = RoyaltyModule( + TestProxyHelper.deployUUPSProxy(impl, abi.encodeCall(RoyaltyModule.initialize, (address(getGovernance())))) + ); + vm.expectRevert(Errors.RoyaltyModule__ZeroDisputeModule.selector); + vm.prank(u.admin); + testRoyaltyModule.setLicensingAndDisputeModules(address(1), address(0)); } - function test_RoyaltyModule_setLicensingModule() public { + function test_RoyaltyModule_setLicensingAndDisputeModules() public { vm.startPrank(u.admin); address impl = address(new RoyaltyModule()); RoyaltyModule testRoyaltyModule = RoyaltyModule( TestProxyHelper.deployUUPSProxy(impl, abi.encodeCall(RoyaltyModule.initialize, (address(getGovernance())))) ); - testRoyaltyModule.setLicensingModule(address(licensingModule)); + testRoyaltyModule.setLicensingAndDisputeModules(address(licensingModule), address(disputeModule)); assertEq(testRoyaltyModule.licensingModule(), address(licensingModule)); + assertEq(testRoyaltyModule.disputeModule(), address(disputeModule)); } function test_RoyaltyModule_whitelistRoyaltyPolicy_revert_ZeroRoyaltyToken() public { @@ -449,6 +513,24 @@ contract TestRoyaltyModule is BaseTest { royaltyModule.payRoyaltyOnBehalf(receiverIpId, payerIpId, address(1), royaltyAmount); } + function test_RoyaltyModule_payRoyaltyOnBehalf_revert_IpIsTagged() public { + // raise dispute + vm.startPrank(ipAccount1); + USDC.approve(address(arbitrationPolicySP), ARBITRATION_PRICE); + disputeModule.raiseDispute(ipAddr, string("urlExample"), "PLAGIARISM", ""); + vm.stopPrank(); + + // set dispute judgement + vm.startPrank(arbitrationRelayer); + disputeModule.setDisputeJudgement(1, true, ""); + + vm.expectRevert(Errors.RoyaltyModule__IpIsTagged.selector); + royaltyModule.payRoyaltyOnBehalf(ipAddr, ipAccount1, address(USDC), 100); + + vm.expectRevert(Errors.RoyaltyModule__IpIsTagged.selector); + royaltyModule.payRoyaltyOnBehalf(ipAccount1, ipAddr, address(USDC), 100); + } + function test_RoyaltyModule_payRoyaltyOnBehalf_revert_NotWhitelistedRoyaltyPolicy() public { uint256 royaltyAmount = 100 * 10 ** 6; address receiverIpId = address(7); @@ -487,6 +569,23 @@ contract TestRoyaltyModule is BaseTest { assertEq(ipRoyaltyVaultUSDCBalAfter - ipRoyaltyVaultUSDCBalBefore, royaltyAmount); } + function test_RoyaltyModule_payLicenseMintingFee_revert_IpIsTagged() public { + // raise dispute + vm.startPrank(ipAccount1); + USDC.approve(address(arbitrationPolicySP), ARBITRATION_PRICE); + disputeModule.raiseDispute(ipAddr, string("urlExample"), "PLAGIARISM", ""); + vm.stopPrank(); + + // set dispute judgement + vm.startPrank(arbitrationRelayer); + disputeModule.setDisputeJudgement(1, true, ""); + + vm.startPrank(address(licensingModule)); + + vm.expectRevert(Errors.RoyaltyModule__IpIsTagged.selector); + royaltyModule.payLicenseMintingFee(ipAddr, ipAccount1, address(royaltyPolicyLAP), address(USDC), 100); + } + function test_RoyaltyModule_payLicenseMintingFee() public { uint256 royaltyAmount = 100 * 10 ** 6; address receiverIpId = address(7); From 6cdac10b380b2c1a753811c1ec3f0490f69898ef Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 5 Apr 2024 10:38:35 +0300 Subject: [PATCH 4/9] update error.sol --- contracts/lib/Errors.sol | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index 3f6d9f1d..20a23369 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -223,6 +223,8 @@ library Errors { error RoyaltyModule__ZeroLicensingModule(); error RoyaltyModule__CanOnlyMintSelectedPolicy(); error RoyaltyModule__NoParentsOnLinking(); + error RoyaltyModule__ZeroDisputeModule(); + error RoyaltyModule__IpIsTagged(); error RoyaltyPolicyLAP__ZeroRoyaltyModule(); error RoyaltyPolicyLAP__ZeroLiquidSplitFactory(); @@ -246,6 +248,8 @@ library Errors { error IpRoyaltyVault__SnapshotIntervalTooShort(); error IpRoyaltyVault__AlreadyClaimed(); error IpRoyaltyVault__ClaimerNotAnAncestor(); + error IpRoyaltyVault__IpTagged(); + error IpRoyaltyVault__ZeroDisputeModule(); //////////////////////////////////////////////////////////////////////////// // ModuleRegistry // From 25bd672187bb765d6104fd05673525cdc9af1a92 Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 5 Apr 2024 10:55:02 +0300 Subject: [PATCH 5/9] minor test fixes --- script/foundry/deployment/Main.s.sol | 2 +- test/foundry/mocks/module/MockRoyaltyModule.sol | 9 ++++++++- test/foundry/utils/BaseTest.t.sol | 2 +- test/foundry/utils/DeployHelper.t.sol | 4 +++- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/script/foundry/deployment/Main.s.sol b/script/foundry/deployment/Main.s.sol index e06a704b..86f237d0 100644 --- a/script/foundry/deployment/Main.s.sol +++ b/script/foundry/deployment/Main.s.sol @@ -345,7 +345,7 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler, StorageLayoutC } function _configureRoyaltyRelated() private { - royaltyModule.setLicensingModule(address(licensingModule)); + royaltyModule.setLicensingAndDisputeModules(address(licensingModule), address(disputeModule)); // whitelist royaltyModule.whitelistRoyaltyPolicy(address(royaltyPolicyLAP), true); royaltyModule.whitelistRoyaltyToken(address(erc20), true); diff --git a/test/foundry/mocks/module/MockRoyaltyModule.sol b/test/foundry/mocks/module/MockRoyaltyModule.sol index 98db8ef6..f9f006c1 100644 --- a/test/foundry/mocks/module/MockRoyaltyModule.sol +++ b/test/foundry/mocks/module/MockRoyaltyModule.sol @@ -11,6 +11,8 @@ contract MockRoyaltyModule is BaseModule, IRoyaltyModule { address public LICENSING_MODULE; + address public DISPUTE_MODULE; + mapping(address royaltyPolicy => bool allowed) public isWhitelistedRoyaltyPolicy; mapping(address token => bool) public isWhitelistedRoyaltyToken; @@ -19,14 +21,19 @@ contract MockRoyaltyModule is BaseModule, IRoyaltyModule { constructor() {} - function setLicensingModule(address _licensingModule) external { + function setLicensingAndDisputeModules(address _licensingModule, address _disputeModule) external { LICENSING_MODULE = _licensingModule; + DISPUTE_MODULE = _disputeModule; } function licensingModule() external view override returns (address) { return LICENSING_MODULE; } + function disputeModule() external view override returns (address) { + return DISPUTE_MODULE; + } + function whitelistRoyaltyPolicy(address _royaltyPolicy, bool _allowed) external { isWhitelistedRoyaltyPolicy[_royaltyPolicy] = _allowed; } diff --git a/test/foundry/utils/BaseTest.t.sol b/test/foundry/utils/BaseTest.t.sol index d47d4fec..50f43aa1 100644 --- a/test/foundry/utils/BaseTest.t.sol +++ b/test/foundry/utils/BaseTest.t.sol @@ -157,7 +157,7 @@ contract BaseTest is Test, DeployHelper, LicensingHelper { require(address(royaltyModule) != address(0), "royaltyModule not set"); vm.startPrank(u.admin); - RoyaltyModule(address(royaltyModule)).setLicensingModule(getLicensingModule()); + RoyaltyModule(address(royaltyModule)).setLicensingAndDisputeModules(getLicensingModule(), getDisputeModule()); royaltyModule.whitelistRoyaltyToken(address(erc20), true); if (address(royaltyPolicyLAP) != address(0)) { royaltyModule.whitelistRoyaltyPolicy(address(royaltyPolicyLAP), true); diff --git a/test/foundry/utils/DeployHelper.t.sol b/test/foundry/utils/DeployHelper.t.sol index d9ae105e..1006a2bf 100644 --- a/test/foundry/utils/DeployHelper.t.sol +++ b/test/foundry/utils/DeployHelper.t.sol @@ -299,7 +299,9 @@ contract DeployHelper is Test { // deploy ip royalty vault implementation and beacon vm.startPrank(governanceAdmin); - address ipRoyaltyVaultImplementation = address(new IpRoyaltyVault(address(royaltyPolicyLAP))); + address ipRoyaltyVaultImplementation = address( + new IpRoyaltyVault(address(royaltyPolicyLAP), address(disputeModule)) + ); address ipRoyaltyVaultBeacon = address( new UpgradeableBeacon(ipRoyaltyVaultImplementation, getGovernance()) ); From fb2bc1c2bf29e407dd45cee9211763503b46ed4b Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 5 Apr 2024 11:10:13 +0300 Subject: [PATCH 6/9] add restrictions to claiming unclaimed royalties for tagged ip assets --- .../modules/royalty/policies/IpRoyaltyVault.sol | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/contracts/modules/royalty/policies/IpRoyaltyVault.sol b/contracts/modules/royalty/policies/IpRoyaltyVault.sol index cae75c2b..06d675ef 100644 --- a/contracts/modules/royalty/policies/IpRoyaltyVault.sol +++ b/contracts/modules/royalty/policies/IpRoyaltyVault.sol @@ -11,6 +11,7 @@ import { SafeERC20Upgradeable } from "@openzeppelin/contracts-upgradeable-v4/tok import { IERC20Upgradeable } from "@openzeppelin/contracts-upgradeable-v4/token/ERC20/IERC20Upgradeable.sol"; import { IRoyaltyPolicyLAP } from "../../../interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol"; +import { IDisputeModule } from "../../../interfaces/modules/dispute/IDisputeModule.sol"; import { IIpRoyaltyVault } from "../../../interfaces/modules/royalty/policies/IIpRoyaltyVault.sol"; import { ArrayUtils } from "../../../lib/ArrayUtils.sol"; import { Errors } from "../../../lib/Errors.sol"; @@ -25,6 +26,9 @@ contract IpRoyaltyVault is IIpRoyaltyVault, ERC20SnapshotUpgradeable, Reentrancy /// @custom:oz-upgrades-unsafe-allow state-variable-immutable IRoyaltyPolicyLAP public immutable ROYALTY_POLICY_LAP; + /// @notice Dispute module address + IDisputeModule public immutable DISPUTE_MODULE; + /// @notice Ip id to whom this royalty vault belongs to address public ipId; @@ -58,10 +62,15 @@ contract IpRoyaltyVault is IIpRoyaltyVault, ERC20SnapshotUpgradeable, Reentrancy /// @notice Constructor /// @param royaltyPolicyLAP The address of the royalty policy LAP + /// @param disputeModule The address of the dispute module /// @custom:oz-upgrades-unsafe-allow constructor - constructor(address royaltyPolicyLAP) { + constructor(address royaltyPolicyLAP, address disputeModule) { if (royaltyPolicyLAP == address(0)) revert Errors.IpRoyaltyVault__ZeroRoyaltyPolicyLAP(); + if (disputeModule == address(0)) revert Errors.IpRoyaltyVault__ZeroDisputeModule(); + ROYALTY_POLICY_LAP = IRoyaltyPolicyLAP(royaltyPolicyLAP); + DISPUTE_MODULE = IDisputeModule(disputeModule); + _disableInitializers(); } @@ -186,6 +195,7 @@ contract IpRoyaltyVault is IIpRoyaltyVault, ERC20SnapshotUpgradeable, Reentrancy ipId ); + if (DISPUTE_MODULE.isIpTagged(ipId)) revert Errors.IpRoyaltyVault__IpTagged(); if (isClaimedByAncestor[ancestorIpId]) revert Errors.IpRoyaltyVault__AlreadyClaimed(); // check if the address being claimed to is an ancestor @@ -216,6 +226,9 @@ contract IpRoyaltyVault is IIpRoyaltyVault, ERC20SnapshotUpgradeable, Reentrancy /// @param token The revenue token to claim /// @return The amount of revenue token claimable function _claimableRevenue(address account, uint256 snapshotId, address token) internal view returns (uint256) { + // if the ip is tagged, then the unclaimed royalties are lost + if (DISPUTE_MODULE.isIpTagged(ipId)) return 0; + uint256 balance = balanceOfAt(account, snapshotId); uint256 totalSupply = totalSupplyAt(snapshotId) - unclaimedAtSnapshot[snapshotId]; uint256 claimableToken = claimableAtSnapshot[snapshotId][token]; From 213e2b7ad4314abc59b273e79d364c384a80499f Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 5 Apr 2024 14:51:31 +0300 Subject: [PATCH 7/9] naming fixes --- .../modules/royalty/IRoyaltyModule.sol | 10 +++++--- .../royalty/policies/IRoyaltyPolicyLAP.sol | 2 +- contracts/modules/royalty/RoyaltyModule.sol | 19 ++++++++------ script/foundry/deployment/Main.s.sol | 3 ++- .../mocks/module/MockRoyaltyModule.sol | 7 ++++-- .../modules/royalty/RoyaltyModule.t.sol | 25 +++++++++++-------- test/foundry/utils/BaseTest.t.sol | 3 ++- 7 files changed, 43 insertions(+), 26 deletions(-) diff --git a/contracts/interfaces/modules/royalty/IRoyaltyModule.sol b/contracts/interfaces/modules/royalty/IRoyaltyModule.sol index 81a014cd..99ba838a 100644 --- a/contracts/interfaces/modules/royalty/IRoyaltyModule.sol +++ b/contracts/interfaces/modules/royalty/IRoyaltyModule.sol @@ -30,11 +30,15 @@ interface IRoyaltyModule is IModule { /// @param amount The amount paid event LicenseMintingFeePaid(address receiverIpId, address payerAddress, address token, uint256 amount); - /// @notice Sets the license registry + /// @notice Sets the licensing module + /// @dev Enforced to be only callable by the protocol admin + /// @param licensing The address of the license module + function setLicensingModule(address licensing) external; + + /// @notice Sets the dispute module /// @dev Enforced to be only callable by the protocol admin - /// @param licensing The address of the license registry /// @param dispute The address of the dispute module - function setLicensingAndDisputeModules(address licensing, address dispute) external; + function setDisputeModule(address dispute) external; /// @notice Returns the licensing module address function licensingModule() external view returns (address); diff --git a/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol b/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol index cd1adf01..d69a08a6 100644 --- a/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol +++ b/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol @@ -24,7 +24,7 @@ interface IRoyaltyPolicyLAP is IRoyaltyPolicy { /// @param ipRoyaltyVault The ip royalty vault address /// @param royaltyStack The royalty stack of a given ipId is the sum of the royalties to be paid to each ancestors /// @param ancestorsAddresses The ancestors addresses array - /// @param ancestorsRoyalties The ancestors royalties array + /// @param ancestorsRoyalties Contains royalty token amounts for each ancestor in the same index as ancestorsAddresses struct LAPRoyaltyData { bool isUnlinkableToParents; address ipRoyaltyVault; diff --git a/contracts/modules/royalty/RoyaltyModule.sol b/contracts/modules/royalty/RoyaltyModule.sol index 01cffefd..d2548499 100644 --- a/contracts/modules/royalty/RoyaltyModule.sol +++ b/contracts/modules/royalty/RoyaltyModule.sol @@ -69,15 +69,19 @@ contract RoyaltyModule is _; } - /// @notice Sets the license registry + /// @notice Sets the licensing module /// @dev Enforced to be only callable by the protocol admin - /// @param licensing The address of the license registry - /// @param dispute The address of the dispute module - function setLicensingAndDisputeModules(address licensing, address dispute) external onlyProtocolAdmin { + /// @param licensing The address of the license module + function setLicensingModule(address licensing) external onlyProtocolAdmin { if (licensing == address(0)) revert Errors.RoyaltyModule__ZeroLicensingModule(); - if (dispute == address(0)) revert Errors.RoyaltyModule__ZeroDisputeModule(); - _getRoyaltyModuleStorage().licensingModule = licensing; + } + + /// @notice Sets the dispute module + /// @dev Enforced to be only callable by the protocol admin + /// @param dispute The address of the dispute module + function setDisputeModule(address dispute) external onlyProtocolAdmin { + if (dispute == address(0)) revert Errors.RoyaltyModule__ZeroDisputeModule(); _getRoyaltyModuleStorage().disputeModule = dispute; } @@ -183,8 +187,7 @@ contract RoyaltyModule is if (!$.isWhitelistedRoyaltyToken[token]) revert Errors.RoyaltyModule__NotWhitelistedRoyaltyToken(); IDisputeModule dispute = IDisputeModule($.disputeModule); - if (dispute.isIpTagged(receiverIpId)) revert Errors.RoyaltyModule__IpIsTagged(); - if (dispute.isIpTagged(payerIpId)) revert Errors.RoyaltyModule__IpIsTagged(); + if (dispute.isIpTagged(receiverIpId) || dispute.isIpTagged(payerIpId)) revert Errors.RoyaltyModule__IpIsTagged(); address payerRoyaltyPolicy = $.royaltyPolicies[payerIpId]; // if the payer does not have a royalty policy set, then the payer is not a derivative ip and does not pay diff --git a/script/foundry/deployment/Main.s.sol b/script/foundry/deployment/Main.s.sol index 86f237d0..4e073f72 100644 --- a/script/foundry/deployment/Main.s.sol +++ b/script/foundry/deployment/Main.s.sol @@ -345,7 +345,8 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler, StorageLayoutC } function _configureRoyaltyRelated() private { - royaltyModule.setLicensingAndDisputeModules(address(licensingModule), address(disputeModule)); + royaltyModule.setLicensingModule(address(licensingModule)); + royaltyModule.setDisputeModule(address(disputeModule)); // whitelist royaltyModule.whitelistRoyaltyPolicy(address(royaltyPolicyLAP), true); royaltyModule.whitelistRoyaltyToken(address(erc20), true); diff --git a/test/foundry/mocks/module/MockRoyaltyModule.sol b/test/foundry/mocks/module/MockRoyaltyModule.sol index f9f006c1..ef8d3474 100644 --- a/test/foundry/mocks/module/MockRoyaltyModule.sol +++ b/test/foundry/mocks/module/MockRoyaltyModule.sol @@ -21,11 +21,14 @@ contract MockRoyaltyModule is BaseModule, IRoyaltyModule { constructor() {} - function setLicensingAndDisputeModules(address _licensingModule, address _disputeModule) external { - LICENSING_MODULE = _licensingModule; + function setDisputeModule(address _disputeModule) external { DISPUTE_MODULE = _disputeModule; } + function setLicensingModule(address _licensingModule) external { + LICENSING_MODULE = _licensingModule; + } + function licensingModule() external view override returns (address) { return LICENSING_MODULE; } diff --git a/test/foundry/modules/royalty/RoyaltyModule.t.sol b/test/foundry/modules/royalty/RoyaltyModule.t.sol index 1edc2b4a..b16627a8 100644 --- a/test/foundry/modules/royalty/RoyaltyModule.t.sol +++ b/test/foundry/modules/royalty/RoyaltyModule.t.sol @@ -172,35 +172,40 @@ contract TestRoyaltyModule is BaseTest { royaltyModule.onLinkToParents(address(3), address(royaltyPolicyLAP), parents, encodedLicenseData, encodedBytes); } - function test_RoyaltyModule_setLicensingAndDisputeModules_revert_ZeroLicensingModule() public { + function test_RoyaltyModule_setDisputeModule_revert_ZeroDisputeModule() public { address impl = address(new RoyaltyModule()); RoyaltyModule testRoyaltyModule = RoyaltyModule( TestProxyHelper.deployUUPSProxy(impl, abi.encodeCall(RoyaltyModule.initialize, (address(getGovernance())))) ); - vm.expectRevert(Errors.RoyaltyModule__ZeroLicensingModule.selector); + vm.expectRevert(Errors.RoyaltyModule__ZeroDisputeModule.selector); vm.prank(u.admin); - testRoyaltyModule.setLicensingAndDisputeModules(address(0), address(1)); + testRoyaltyModule.setDisputeModule(address(0)); } - function test_RoyaltyModule_setLicensingAndDisputeModules_revert_ZeroDisputeModule() public { + function test_RoyaltyModule_setDisputeModule() public { + vm.startPrank(u.admin); address impl = address(new RoyaltyModule()); RoyaltyModule testRoyaltyModule = RoyaltyModule( TestProxyHelper.deployUUPSProxy(impl, abi.encodeCall(RoyaltyModule.initialize, (address(getGovernance())))) ); - vm.expectRevert(Errors.RoyaltyModule__ZeroDisputeModule.selector); - vm.prank(u.admin); - testRoyaltyModule.setLicensingAndDisputeModules(address(1), address(0)); + testRoyaltyModule.setDisputeModule(address(disputeModule)); + assertEq(testRoyaltyModule.disputeModule(), address(disputeModule)); } - function test_RoyaltyModule_setLicensingAndDisputeModules() public { + function test_RoyaltyModule_setLicensingModule_revert_ZeroLicensingModule() public { + vm.startPrank(u.admin); + vm.expectRevert(Errors.RoyaltyModule__ZeroLicensingModule.selector); + royaltyModule.setLicensingModule(address(0)); + } + + function test_RoyaltyModule_setLicensingModule() public { vm.startPrank(u.admin); address impl = address(new RoyaltyModule()); RoyaltyModule testRoyaltyModule = RoyaltyModule( TestProxyHelper.deployUUPSProxy(impl, abi.encodeCall(RoyaltyModule.initialize, (address(getGovernance())))) ); - testRoyaltyModule.setLicensingAndDisputeModules(address(licensingModule), address(disputeModule)); + testRoyaltyModule.setLicensingModule(address(licensingModule)); assertEq(testRoyaltyModule.licensingModule(), address(licensingModule)); - assertEq(testRoyaltyModule.disputeModule(), address(disputeModule)); } function test_RoyaltyModule_whitelistRoyaltyPolicy_revert_ZeroRoyaltyToken() public { diff --git a/test/foundry/utils/BaseTest.t.sol b/test/foundry/utils/BaseTest.t.sol index 50f43aa1..a7ffeea7 100644 --- a/test/foundry/utils/BaseTest.t.sol +++ b/test/foundry/utils/BaseTest.t.sol @@ -157,7 +157,8 @@ contract BaseTest is Test, DeployHelper, LicensingHelper { require(address(royaltyModule) != address(0), "royaltyModule not set"); vm.startPrank(u.admin); - RoyaltyModule(address(royaltyModule)).setLicensingAndDisputeModules(getLicensingModule(), getDisputeModule()); + RoyaltyModule(address(royaltyModule)).setLicensingModule(getLicensingModule()); + RoyaltyModule(address(royaltyModule)).setDisputeModule(getDisputeModule()); royaltyModule.whitelistRoyaltyToken(address(erc20), true); if (address(royaltyPolicyLAP) != address(0)) { royaltyModule.whitelistRoyaltyPolicy(address(royaltyPolicyLAP), true); From e1e5a359ee63cdb5f80162c73a43eb3db092367b Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 5 Apr 2024 14:52:24 +0300 Subject: [PATCH 8/9] format fixes --- contracts/modules/royalty/RoyaltyModule.sol | 3 ++- test/foundry/modules/royalty/IpRoyaltyVault.t.sol | 3 +-- test/foundry/modules/royalty/RoyaltyModule.t.sol | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/modules/royalty/RoyaltyModule.sol b/contracts/modules/royalty/RoyaltyModule.sol index d2548499..257c73d3 100644 --- a/contracts/modules/royalty/RoyaltyModule.sol +++ b/contracts/modules/royalty/RoyaltyModule.sol @@ -187,7 +187,8 @@ contract RoyaltyModule is if (!$.isWhitelistedRoyaltyToken[token]) revert Errors.RoyaltyModule__NotWhitelistedRoyaltyToken(); IDisputeModule dispute = IDisputeModule($.disputeModule); - if (dispute.isIpTagged(receiverIpId) || dispute.isIpTagged(payerIpId)) revert Errors.RoyaltyModule__IpIsTagged(); + if (dispute.isIpTagged(receiverIpId) || dispute.isIpTagged(payerIpId)) + revert Errors.RoyaltyModule__IpIsTagged(); address payerRoyaltyPolicy = $.royaltyPolicies[payerIpId]; // if the payer does not have a royalty policy set, then the payer is not a derivative ip and does not pay diff --git a/test/foundry/modules/royalty/IpRoyaltyVault.t.sol b/test/foundry/modules/royalty/IpRoyaltyVault.t.sol index fe5b9e10..8aa39290 100644 --- a/test/foundry/modules/royalty/IpRoyaltyVault.t.sol +++ b/test/foundry/modules/royalty/IpRoyaltyVault.t.sol @@ -11,7 +11,6 @@ import { Errors } from "../../../../contracts/lib/Errors.sol"; import { BaseTest } from "../../utils/BaseTest.t.sol"; contract TestIpRoyaltyVault is BaseTest { - IpRoyaltyVault ipRoyaltyVault; function setUp() public override { @@ -257,7 +256,7 @@ contract TestIpRoyaltyVault is BaseTest { uint256 usdcClaimVaultBefore = ipRoyaltyVault.claimVaultAmount(address(USDC)); uint256 expectedAmount = royaltyAmount - (royaltyAmount * royaltyStack2) / royaltyPolicyLAP.TOTAL_RT_SUPPLY(); - + vm.expectEmit(true, true, true, true, address(ipRoyaltyVault)); emit IIpRoyaltyVault.RevenueTokenClaimed(address(2), address(USDC), expectedAmount); diff --git a/test/foundry/modules/royalty/RoyaltyModule.t.sol b/test/foundry/modules/royalty/RoyaltyModule.t.sol index b16627a8..464ba966 100644 --- a/test/foundry/modules/royalty/RoyaltyModule.t.sol +++ b/test/foundry/modules/royalty/RoyaltyModule.t.sol @@ -24,7 +24,7 @@ contract TestRoyaltyModule is BaseTest { address internal ipAccount2 = address(0x111000bbb); address internal ipAddr; address internal arbitrationRelayer; - + struct InitParams { address[] targetAncestors; uint32[] targetRoyaltyAmount; From 93b3746dd2c57b569463bfbff854a3780b782c46 Mon Sep 17 00:00:00 2001 From: Spablob Date: Fri, 5 Apr 2024 14:54:28 +0300 Subject: [PATCH 9/9] solhint fix --- .../interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol b/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol index d69a08a6..2353c628 100644 --- a/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol +++ b/contracts/interfaces/modules/royalty/policies/IRoyaltyPolicyLAP.sol @@ -24,7 +24,7 @@ interface IRoyaltyPolicyLAP is IRoyaltyPolicy { /// @param ipRoyaltyVault The ip royalty vault address /// @param royaltyStack The royalty stack of a given ipId is the sum of the royalties to be paid to each ancestors /// @param ancestorsAddresses The ancestors addresses array - /// @param ancestorsRoyalties Contains royalty token amounts for each ancestor in the same index as ancestorsAddresses + /// @param ancestorsRoyalties Contains royalty token amounts for each ancestor on same index as ancestorsAddresses struct LAPRoyaltyData { bool isUnlinkableToParents; address ipRoyaltyVault;