From f2ab963cd5c7cbd6f225a60a0b6f9a0379165d6e Mon Sep 17 00:00:00 2001 From: Raul Date: Wed, 14 Feb 2024 18:06:32 -0800 Subject: [PATCH 01/14] integrated dispute module with licensing --- .../modules/dispute/IDisputeModule.sol | 21 ++++++++++ .../registries/ILicenseRegistry.sol | 5 +++ contracts/lib/Errors.sol | 6 +++ .../modules/dispute-module/DisputeModule.sol | 39 +++++++++++++++++++ .../modules/licensing/LicensingModule.sol | 20 +++++++++- contracts/registries/LicenseRegistry.sol | 18 ++++++++- script/foundry/deployment/Main.s.sol | 25 ++++++------ .../foundry/utils/JsonDeploymentHandler.s.sol | 2 +- .../mocks/module/MockDisputeModule.sol | 24 ++++++++++++ .../mocks/registry/MockLicenseRegistry.sol | 4 ++ .../modules/dispute/DisputeModule.t.sol | 22 +++++++++++ .../modules/licensing/LicensingModule.t.sol | 11 ++++-- test/foundry/utils/DeployHelper.t.sol | 5 ++- test/foundry/utils/LicensingHelper.t.sol | 32 +++++++++------ 14 files changed, 202 insertions(+), 32 deletions(-) diff --git a/contracts/interfaces/modules/dispute/IDisputeModule.sol b/contracts/interfaces/modules/dispute/IDisputeModule.sol index 6ceb1c49a..87950d391 100644 --- a/contracts/interfaces/modules/dispute/IDisputeModule.sol +++ b/contracts/interfaces/modules/dispute/IDisputeModule.sol @@ -155,4 +155,25 @@ interface IDisputeModule { bytes32 targetTag, // The target tag of the dispute bytes32 currentTag // The current tag of the dispute ); + + /// @notice returns true if the ipId is tagged with the tag (meaning the dispute went through) + /// @param _ipId The ipId + /// @param _tag The tag + function isIpTaggedWith(address _ipId, bytes32 _tag) external view returns (bool); + + /// @notice returns true if the ipId is tagged with any tag (meaning at least one dispute went through) + /// @param _ipId The ipId + function isIpTagged(address _ipId) external view returns (bool); + + /// @notice returns the tags for a given ipId (note: this method could be expensive, use in frontends only) + /// @param _ipId The ipId + function ipTags(address _ipId) external view returns (bytes32[] memory); + + /// @notice returns the total tags for a given ipId + /// @param _ipId The ipId + function totalTagsForIp(address _ipId) external view returns (uint256); + + /// @notice returns the tag at a given index for a given ipId. No guarantees on ordering + /// @param _ipId The ipId + function tagForIpAt(address _ipId, uint256 _index) external view returns (bytes32); } diff --git a/contracts/interfaces/registries/ILicenseRegistry.sol b/contracts/interfaces/registries/ILicenseRegistry.sol index 3fdad0434..b760da179 100644 --- a/contracts/interfaces/registries/ILicenseRegistry.sol +++ b/contracts/interfaces/registries/ILicenseRegistry.sol @@ -65,4 +65,9 @@ interface ILicenseRegistry is IERC1155 { /// @notice License data (licensor, policy...) for the license id function license(uint256 licenseId) external view returns (Licensing.License memory); + + /// @notice Returns true if the license has been revoked (licensor tagged after a dispute in + /// the dispute module). If the tag is removed, the license is not revoked anymore. + /// @param licenseId The id of the license + function isLicenseRevoked(uint256 licenseId) external view returns (bool); } diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index bd1b9f7d3..c74d609fb 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -116,6 +116,8 @@ library Errors { error LicensingModule__CallerNotLicenseRegistry(); /// @notice emitted when trying to transfer a license that is not transferable (by policy) error LicenseRegistry__NotTransferable(); + /// @notice emitted on constructor if dispute module is not set + error LicenseRegistry__ZeroDisputeModule(); //////////////////////////////////////////////////////////////////////////// // LicensingModule // @@ -149,6 +151,10 @@ library Errors { error LicensingModule__DerivativeRevShareSumExceedsMaxRNFTSupply(); error LicensingModule__MismatchBetweenCommercialRevenueShareAndMinRoyalty(); error LicensingModule__MismatchBetweenRoyaltyPolicy(); + /// @notice emitted when trying to interact with an IP that has been disputed in the DisputeModule + error LicensingModule__DisputedIpId(); + /// @notice emitted when linking a license from a licensor that has been disputed in the DisputeModule + error LicensingModule__LinkingRevokedLicense(); //////////////////////////////////////////////////////////////////////////// // LicensingModuleAware // diff --git a/contracts/modules/dispute-module/DisputeModule.sol b/contracts/modules/dispute-module/DisputeModule.sol index e612ae267..192bf0396 100644 --- a/contracts/modules/dispute-module/DisputeModule.sol +++ b/contracts/modules/dispute-module/DisputeModule.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.23; import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; +import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import { DISPUTE_MODULE_KEY } from "../../lib/modules/Module.sol"; import { BaseModule } from "../../modules/BaseModule.sol"; @@ -17,6 +18,7 @@ import { ShortStringOps } from "../../utils/ShortStringOps.sol"; /// @notice The Story Protocol dispute module acts as an enforcement layer for /// that allows to raise disputes and resolve them through arbitration. contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuard, AccessControlled { + using EnumerableSet for EnumerableSet.Bytes32Set; /// @notice tag to represent the dispute is in dispute state waiting for judgement bytes32 public constant IN_DISPUTE = bytes32("IN_DISPUTE"); @@ -54,6 +56,8 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar /// @notice Arbitration policy for a given ipId mapping(address ipId => address arbitrationPolicy) public arbitrationPolicies; + mapping(address ipId => EnumerableSet.Bytes32Set) private _taggedIpIds; + /// @notice Initializes the registration module contract /// @param _controller The access controller used for IP authorization /// @param _assetRegistry The address of the IP asset registry @@ -189,6 +193,8 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar if (_decision) { disputes[_disputeId].currentTag = dispute.targetTag; + // We ignore the result of add(), we don't care if the tag is already there + _taggedIpIds[dispute.targetIpId].add(dispute.targetTag); } else { disputes[_disputeId].currentTag = bytes32(0); } @@ -222,11 +228,44 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar if (dispute.currentTag == IN_DISPUTE) revert Errors.DisputeModule__NotAbleToResolve(); if (msg.sender != dispute.disputeInitiator) revert Errors.DisputeModule__NotDisputeInitiator(); + // We ignore the result of remove() + _taggedIpIds[dispute.targetIpId].remove(dispute.currentTag); disputes[_disputeId].currentTag = bytes32(0); emit DisputeResolved(_disputeId); } + /// @notice returns true if the ipId is tagged with the tag (meaning the dispute went through) + /// @param _ipId The ipId + /// @param _tag The tag + function isIpTaggedWith(address _ipId, bytes32 _tag) external view returns (bool) { + return _taggedIpIds[_ipId].contains(_tag); + } + + /// @notice returns true if the ipId is tagged with any tag (meaning at least one dispute went through) + /// @param _ipId The ipId + function isIpTagged(address _ipId) external view returns (bool) { + return _taggedIpIds[_ipId].length() > 0; + } + + /// @notice returns the tags for a given ipId (note: this method could be expensive, use in frontends only) + /// @param _ipId The ipId + function ipTags(address _ipId) external view returns (bytes32[] memory) { + return _taggedIpIds[_ipId].values(); + } + + /// @notice returns the total tags for a given ipId + /// @param _ipId The ipId + function totalTagsForIp(address _ipId) external view returns (uint256) { + return _taggedIpIds[_ipId].length(); + } + + /// @notice returns the tag at a given index for a given ipId. No guarantees on ordering + /// @param _ipId The ipId + function tagForIpAt(address _ipId, uint256 _index) external view returns (bytes32) { + return _taggedIpIds[_ipId].at(_index); + } + /// @notice Gets the protocol-wide module identifier for this module /// @return The dispute module key function name() public pure override returns (string memory) { diff --git a/contracts/modules/licensing/LicensingModule.sol b/contracts/modules/licensing/LicensingModule.sol index b0485371c..95b3df7a7 100644 --- a/contracts/modules/licensing/LicensingModule.sol +++ b/contracts/modules/licensing/LicensingModule.sol @@ -13,6 +13,7 @@ import { IPolicyFrameworkManager } from "../../interfaces/modules/licensing/IPol import { ILicenseRegistry } from "../../interfaces/registries/ILicenseRegistry.sol"; import { ILicensingModule } from "../../interfaces/modules/licensing/ILicensingModule.sol"; import { IIPAccountRegistry } from "../../interfaces/registries/IIPAccountRegistry.sol"; +import { IDisputeModule } from "../../interfaces/modules/dispute/IDisputeModule.sol"; import { Errors } from "../../lib/Errors.sol"; import { DataUniqueness } from "../../lib/DataUniqueness.sol"; import { Licensing } from "../../lib/Licensing.sol"; @@ -40,6 +41,7 @@ contract LicensingModule is AccessControlled, ILicensingModule, BaseModule, Reen RoyaltyModule public immutable ROYALTY_MODULE; ILicenseRegistry public immutable LICENSE_REGISTRY; + IDisputeModule public immutable DISPUTE_MODULE; string public constant override name = LICENSING_MODULE_KEY; mapping(address framework => bool registered) private _registeredFrameworkManagers; @@ -63,10 +65,12 @@ contract LicensingModule is AccessControlled, ILicensingModule, BaseModule, Reen address accessController, address ipAccountRegistry, address royaltyModule, - address registry + address registry, + address disputeModule ) AccessControlled(accessController, ipAccountRegistry) { ROYALTY_MODULE = RoyaltyModule(royaltyModule); LICENSE_REGISTRY = ILicenseRegistry(registry); + DISPUTE_MODULE = IDisputeModule(disputeModule); } function licenseRegistry() external view returns (address) { @@ -130,6 +134,7 @@ contract LicensingModule is AccessControlled, ILicensingModule, BaseModule, Reen if (!isPolicyDefined(polId)) { revert Errors.LicensingModule__PolicyNotFound(); } + _verifyIpNotDisputed(ipId); indexOnIpId = _addPolicyIdToIp({ ipId: ipId, policyId: polId, isInherited: false, skipIfDuplicate: false }); @@ -171,10 +176,10 @@ contract LicensingModule is AccessControlled, ILicensingModule, BaseModule, Reen uint256 amount, // mint amount address receiver ) external nonReentrant returns (uint256 licenseId) { - // TODO: check if licensor has been tagged by disputer if (!IP_ACCOUNT_REGISTRY.isIpAccount(licensorIp)) { revert Errors.LicensingModule__LicensorNotRegistered(); } + _verifyIpNotDisputed(licensorIp); bool isInherited = _policySetups[licensorIp][policyId].isInherited; Licensing.Policy memory pol = policy(policyId); @@ -262,6 +267,7 @@ contract LicensingModule is AccessControlled, ILicensingModule, BaseModule, Reen address childIpId, uint32 minRoyalty ) external nonReentrant verifyPermission(childIpId) { + _verifyIpNotDisputed(childIpId); address holder = IIPAccount(payable(childIpId)).owner(); address[] memory licensors = new address[](licenseIds.length); // If royalty policy address is address(0), this means no royalty policy to set. @@ -271,6 +277,9 @@ contract LicensingModule is AccessControlled, ILicensingModule, BaseModule, Reen for (uint256 i = 0; i < licenseIds.length; i++) { uint256 licenseId = licenseIds[i]; + if (LICENSE_REGISTRY.isLicenseRevoked(licenseId)) { + revert Errors.LicensingModule__LinkingRevokedLicense(); + } if (!LICENSE_REGISTRY.isLicensee(licenseId, holder)) { revert Errors.LicensingModule__NotLicensee(); } @@ -556,4 +565,11 @@ contract LicensingModule is AccessControlled, ILicensingModule, BaseModule, Reen function _policySetPerIpId(bool isInherited, address ipId) private view returns (EnumerableSet.UintSet storage) { return _policiesPerIpId[keccak256(abi.encode(isInherited, ipId))]; } + + function _verifyIpNotDisputed(address ipId) private view { + // TODO: in beta, any tag means revocation, for mainnet we need more context + if (DISPUTE_MODULE.isIpTagged(ipId)) { + revert Errors.LicensingModule__DisputedIpId(); + } + } } diff --git a/contracts/registries/LicenseRegistry.sol b/contracts/registries/LicenseRegistry.sol index d551855bc..2ba4ed234 100644 --- a/contracts/registries/LicenseRegistry.sol +++ b/contracts/registries/LicenseRegistry.sol @@ -6,6 +6,7 @@ import { ERC1155 } from "@openzeppelin/contracts/token/ERC1155/ERC1155.sol"; import { IPolicyFrameworkManager } from "../interfaces/modules/licensing/IPolicyFrameworkManager.sol"; import { ILicenseRegistry } from "../interfaces/registries/ILicenseRegistry.sol"; import { ILicensingModule } from "../interfaces/modules/licensing/ILicensingModule.sol"; +import { IDisputeModule } from "../interfaces/modules/dispute/IDisputeModule.sol"; import { Errors } from "../lib/Errors.sol"; import { Licensing } from "../lib/Licensing.sol"; import { DataUniqueness } from "../lib/DataUniqueness.sol"; @@ -15,6 +16,7 @@ import { DataUniqueness } from "../lib/DataUniqueness.sol"; contract LicenseRegistry is ERC1155, ILicenseRegistry { // TODO: deploy with CREATE2 to make this immutable ILicensingModule private _licensingModule; + IDisputeModule public immutable DISPUTE_MODULE; mapping(bytes32 licenseHash => uint256 ids) private _hashedLicenses; mapping(uint256 licenseIds => Licensing.License licenseData) private _licenses; @@ -31,7 +33,12 @@ contract LicenseRegistry is ERC1155, ILicenseRegistry { _; } - constructor() ERC1155("") {} + constructor(address disputeModule) ERC1155("") { + if (disputeModule == address(0)) { + revert Errors.LicenseRegistry__ZeroDisputeModule(); + } + DISPUTE_MODULE = IDisputeModule(disputeModule); + } function setLicensingModule(address newLicensingModule) external { if (newLicensingModule == address(0)) { @@ -113,6 +120,15 @@ contract LicenseRegistry is ERC1155, ILicenseRegistry { return _licenses[licenseId].policyId; } + /// @notice Returns true if the license has been revoked (licensor tagged after a dispute in + /// the dispute module). If the tag is removed, the license is not revoked anymore. + /// @param licenseId The id of the license + function isLicenseRevoked(uint256 licenseId) external view returns (bool) { + // For beta, any tag means revocation, for mainnet we need more context. + // TODO: signal metadata update when tag changes. + return DISPUTE_MODULE.isIpTagged(_licenses[licenseId].licensorIpId); + } + /// @notice ERC1155 OpenSea metadata JSON representation of the LNFT parameters function uri(uint256 id) public view virtual override returns (string memory) { Licensing.License memory licenseData = _licenses[id]; diff --git a/script/foundry/deployment/Main.s.sol b/script/foundry/deployment/Main.s.sol index e7ff24957..e5a4c5456 100644 --- a/script/foundry/deployment/Main.s.sol +++ b/script/foundry/deployment/Main.s.sol @@ -195,9 +195,18 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler { royaltyModule = new RoyaltyModule(address(governance)); _postdeploy(contractKey, address(royaltyModule)); + contractKey = "DisputeModule"; + _predeploy(contractKey); + disputeModule = new DisputeModule( + address(accessController), + address(ipAssetRegistry), + address(governance) + ); + _postdeploy(contractKey, address(disputeModule)); + contractKey = "LicenseRegistry"; _predeploy(contractKey); - licenseRegistry = new LicenseRegistry(); + licenseRegistry = new LicenseRegistry(address(disputeModule)); _postdeploy(contractKey, address(licenseRegistry)); contractKey = "LicensingModule"; @@ -206,7 +215,8 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler { address(accessController), address(ipAccountRegistry), address(royaltyModule), - address(licenseRegistry) + address(licenseRegistry), + address(disputeModule) ); _postdeploy(contractKey, address(licensingModule)); @@ -229,15 +239,6 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler { ); _postdeploy(contractKey, address(registrationModule)); - contractKey = "DisputeModule"; - _predeploy(contractKey); - disputeModule = new DisputeModule( - address(accessController), - address(ipAssetRegistry), - address(governance) - ); - _postdeploy(contractKey, address(disputeModule)); - contractKey = "ArbitrationPolicySP"; _predeploy(contractKey); arbitrationPolicySP = new ArbitrationPolicySP( @@ -278,7 +279,7 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler { _executeInteractions(); } - function _predeploy(string memory contractKey) private view { + function _predeploy(string memory contractKey) private pure { console2.log(string.concat("Deploying ", contractKey, "...")); } diff --git a/script/foundry/utils/JsonDeploymentHandler.s.sol b/script/foundry/utils/JsonDeploymentHandler.s.sol index ee640c220..36f4d7014 100644 --- a/script/foundry/utils/JsonDeploymentHandler.s.sol +++ b/script/foundry/utils/JsonDeploymentHandler.s.sol @@ -21,7 +21,7 @@ contract JsonDeploymentHandler is Script { key = _key; } - function _readAddress(string memory readPath) internal returns (address) { + function _readAddress(string memory readPath) internal view returns (address) { try vm.parseJsonAddress(readJson, readPath) returns (address addr) { return addr; } catch { diff --git a/test/foundry/mocks/module/MockDisputeModule.sol b/test/foundry/mocks/module/MockDisputeModule.sol index ee8de7a02..805506f2c 100644 --- a/test/foundry/mocks/module/MockDisputeModule.sol +++ b/test/foundry/mocks/module/MockDisputeModule.sol @@ -120,4 +120,28 @@ contract MockDisputeModule is BaseModule, IDisputeModule { function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) { return interfaceId == type(IDisputeModule).interfaceId || super.supportsInterface(interfaceId); } + + // These methods are not really used in the mock. They are just here to satisfy the interface. + + function isIpTaggedWith(address, bytes32) external pure returns (bool) { + return false; + } + + function isIpTagged(address) external pure returns (bool) { + return false; + } + + function ipTags(address) external pure returns (bytes32[] memory) { + bytes32[] memory tags; + return tags; + } + + function totalTagsForIp(address) external pure returns (uint256) { + return 0; + } + + function tagForIpAt(address, uint256) external pure returns (bytes32) { + return bytes32(0); + } + } diff --git a/test/foundry/mocks/registry/MockLicenseRegistry.sol b/test/foundry/mocks/registry/MockLicenseRegistry.sol index 890d86b64..ab6ab7aba 100644 --- a/test/foundry/mocks/registry/MockLicenseRegistry.sol +++ b/test/foundry/mocks/registry/MockLicenseRegistry.sol @@ -78,6 +78,10 @@ contract MockLicenseRegistry is ERC1155, ILicenseRegistry { return _licenses[licenseId].policyId; } + function isLicenseRevoked(uint256 licenseId) external pure returns (bool) { + return false; + } + function uri(uint256 id) public pure override returns (string memory) { // return uint256 id as string value return string(abi.encodePacked("uri_", id)); diff --git a/test/foundry/modules/dispute/DisputeModule.t.sol b/test/foundry/modules/dispute/DisputeModule.t.sol index 9b94d4aa4..a5ce9e9d0 100644 --- a/test/foundry/modules/dispute/DisputeModule.t.sol +++ b/test/foundry/modules/dispute/DisputeModule.t.sol @@ -391,6 +391,13 @@ contract DisputeModuleTest is BaseTest { assertEq(arbitrationPolicySPUSDCBalanceBefore - arbitrationPolicySPUSDCBalanceAfter, ARBITRATION_PRICE); assertEq(currentTagBefore, bytes32("IN_DISPUTE")); assertEq(currentTagAfter, bytes32("PLAGIARISM")); + assertTrue(disputeModule.isIpTaggedWith(ipAddr, bytes32("PLAGIARISM"))); + assertTrue(disputeModule.isIpTagged(ipAddr)); + bytes32[] memory ipTags = new bytes32[](1); + ipTags[0] = bytes32("PLAGIARISM"); + assertEq(keccak256(abi.encode(disputeModule.ipTags(ipAddr))), keccak256(abi.encode(ipTags))); + assertEq(disputeModule.totalTagsForIp(ipAddr), 1); + assertEq(disputeModule.tagForIpAt(ipAddr, 0), bytes32("PLAGIARISM")); } function test_DisputeModule_PolicySP_setDisputeJudgement_False() public { @@ -419,6 +426,11 @@ contract DisputeModuleTest is BaseTest { assertEq(arbitrationPolicySPUSDCBalanceBefore - arbitrationPolicySPUSDCBalanceAfter, 0); assertEq(currentTagBefore, bytes32("IN_DISPUTE")); assertEq(currentTagAfter, bytes32(0)); + assertFalse(disputeModule.isIpTaggedWith(ipAddr, bytes32("PLAGIARISM"))); + assertFalse(disputeModule.isIpTagged(ipAddr)); + bytes32[] memory ipTags = new bytes32[](0); + assertEq(keccak256(abi.encode(disputeModule.ipTags(ipAddr))), keccak256(abi.encode(ipTags))); + assertEq(disputeModule.totalTagsForIp(ipAddr), 0); } function test_DisputeModule_PolicySP_cancelDispute_revert_NotDisputeInitiator() public { @@ -457,6 +469,11 @@ contract DisputeModuleTest is BaseTest { assertEq(currentTagBeforeCancel, bytes32("IN_DISPUTE")); assertEq(currentTagAfterCancel, bytes32(0)); + assertFalse(disputeModule.isIpTaggedWith(ipAddr, bytes32("PLAGIARISM"))); + assertFalse(disputeModule.isIpTagged(ipAddr)); + bytes32[] memory ipTags = new bytes32[](0); + assertEq(keccak256(abi.encode(disputeModule.ipTags(ipAddr))), keccak256(abi.encode(ipTags))); + assertEq(disputeModule.totalTagsForIp(ipAddr), 0); } function test_DisputeModule_resolveDispute_revert_NotDisputeInitiator() public { @@ -501,6 +518,11 @@ contract DisputeModuleTest is BaseTest { assertEq(currentTagBeforeResolve, bytes32("PLAGIARISM")); assertEq(currentTagAfterResolve, bytes32(0)); + assertFalse(disputeModule.isIpTaggedWith(ipAddr, bytes32("PLAGIARISM"))); + assertFalse(disputeModule.isIpTagged(ipAddr)); + bytes32[] memory ipTags = new bytes32[](0); + assertEq(keccak256(abi.encode(disputeModule.ipTags(ipAddr))), keccak256(abi.encode(ipTags))); + assertEq(disputeModule.totalTagsForIp(ipAddr), 0); } function test_DisputeModule_name() public { diff --git a/test/foundry/modules/licensing/LicensingModule.t.sol b/test/foundry/modules/licensing/LicensingModule.t.sol index ad8b4e784..bf4020542 100644 --- a/test/foundry/modules/licensing/LicensingModule.t.sol +++ b/test/foundry/modules/licensing/LicensingModule.t.sol @@ -29,6 +29,7 @@ import { MockPolicyFrameworkManager, MockPolicyFrameworkConfig, MockPolicy } fro import { MockAccessController } from "test/foundry/mocks/access/MockAccessController.sol"; import { MockERC721 } from "test/foundry/mocks/token/MockERC721.sol"; import { MockRoyaltyPolicyLS } from "test/foundry/mocks/policy/MockRoyaltyPolicyLS.sol"; +import { MockDisputeModule } from "test/foundry/mocks/module/MockDisputeModule.sol"; contract LicensingModuleTest is Test { using Strings for *; @@ -58,6 +59,8 @@ contract LicensingModuleTest is Test { MockRoyaltyPolicyLS internal mockRoyaltyPolicyLS; + MockDisputeModule internal disputeModule; + string public licenseUrl = "https://example.com/license"; address public ipId1; address public ipId2; @@ -83,12 +86,14 @@ contract LicensingModuleTest is Test { address(governance) ); royaltyModule = new RoyaltyModule(address(governance)); - licenseRegistry = new LicenseRegistry(); + disputeModule = new MockDisputeModule(); + licenseRegistry = new LicenseRegistry(address(disputeModule)); licensingModule = new LicensingModule( address(accessController), address(ipAssetRegistry), address(royaltyModule), - address(licenseRegistry) + address(licenseRegistry), + address(disputeModule) ); mockRoyaltyPolicyLS = new MockRoyaltyPolicyLS(address(royaltyModule)); @@ -553,7 +558,7 @@ contract LicensingModuleTest is Test { /* solhint-disable */ string - memory expectedJson = "eyJuYW1lIjogIlN0b3J5IFByb3RvY29sIExpY2Vuc2UgTkZUIiwgImRlc2NyaXB0aW9uIjogIkxpY2Vuc2UgYWdyZWVtZW50IHN0YXRpbmcgdGhlIHRlcm1zIG9mIGEgU3RvcnkgUHJvdG9jb2wgSVBBc3NldCIsICJhdHRyaWJ1dGVzIjogW3sidHJhaXRfdHlwZSI6ICJBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIlRyYW5zZmVyYWJsZSIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkNvbW1lcmljYWwgVXNlIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiY29tbWVyY2lhbEF0dHJpYnV0aW9uIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiY29tbWVyY2lhbFJldlNoYXJlIiwgInZhbHVlIjogMH0seyJ0cmFpdF90eXBlIjogImNvbW1lcmNpYWxpemVyQ2hlY2siLCAidmFsdWUiOiAiMHgxMGY3YWJkMDEyNmE5MDkzNWYzZjkwMDJmYTc5NzY3YWZjMGUzYzBkIn0sIHsidHJhaXRfdHlwZSI6ICJkZXJpdmF0aXZlc0FsbG93ZWQiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJkZXJpdmF0aXZlc0F0dHJpYnV0aW9uIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiZGVyaXZhdGl2ZXNBcHByb3ZhbCIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogImRlcml2YXRpdmVzUmVjaXByb2NhbCIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogImRlcml2YXRpdmVzUmV2U2hhcmUiLCAidmFsdWUiOiAwfSx7InRyYWl0X3R5cGUiOiAidGVycml0b3JpZXMiLCAidmFsdWUiOiBbInRlcnJpdG9yeTEiXX0sIHsidHJhaXRfdHlwZSI6ICJkaXN0cmlidXRpb25DaGFubmVscyIsICJ2YWx1ZSI6IFsiZGlzdHJpYnV0aW9uQ2hhbm5lbDEiXX1dfQ=="; + memory expectedJson = "eyJuYW1lIjogIlN0b3J5IFByb3RvY29sIExpY2Vuc2UgTkZUIiwgImRlc2NyaXB0aW9uIjogIkxpY2Vuc2UgYWdyZWVtZW50IHN0YXRpbmcgdGhlIHRlcm1zIG9mIGEgU3RvcnkgUHJvdG9jb2wgSVBBc3NldCIsICJhdHRyaWJ1dGVzIjogW3sidHJhaXRfdHlwZSI6ICJBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIlRyYW5zZmVyYWJsZSIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkNvbW1lcmljYWwgVXNlIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiY29tbWVyY2lhbEF0dHJpYnV0aW9uIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiY29tbWVyY2lhbFJldlNoYXJlIiwgInZhbHVlIjogMH0seyJ0cmFpdF90eXBlIjogImNvbW1lcmNpYWxpemVyQ2hlY2siLCAidmFsdWUiOiAiMHgyMTA1MDNjMzE4ODU1MjU5OTgzMjk4YmE1ODA1NWEzOGQ1ZmY2M2UwIn0sIHsidHJhaXRfdHlwZSI6ICJkZXJpdmF0aXZlc0FsbG93ZWQiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJkZXJpdmF0aXZlc0F0dHJpYnV0aW9uIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiZGVyaXZhdGl2ZXNBcHByb3ZhbCIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogImRlcml2YXRpdmVzUmVjaXByb2NhbCIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogImRlcml2YXRpdmVzUmV2U2hhcmUiLCAidmFsdWUiOiAwfSx7InRyYWl0X3R5cGUiOiAidGVycml0b3JpZXMiLCAidmFsdWUiOiBbInRlcnJpdG9yeTEiXX0sIHsidHJhaXRfdHlwZSI6ICJkaXN0cmlidXRpb25DaGFubmVscyIsICJ2YWx1ZSI6IFsiZGlzdHJpYnV0aW9uQ2hhbm5lbDEiXX1dfQ=="; /* solhint-enable */ string memory expectedUri = string(abi.encodePacked("data:application/json;base64,", expectedJson)); diff --git a/test/foundry/utils/DeployHelper.t.sol b/test/foundry/utils/DeployHelper.t.sol index cd835ae07..27bfc079a 100644 --- a/test/foundry/utils/DeployHelper.t.sol +++ b/test/foundry/utils/DeployHelper.t.sol @@ -255,7 +255,7 @@ contract DeployHelper { console2.log("DeployHelper: Using REAL IPAssetRegistry"); if (d.licenseRegistry) { - licenseRegistry = new LicenseRegistry(); + licenseRegistry = new LicenseRegistry(getDisputeModule()); console2.log("DeployHelper: Using REAL LicenseRegistry"); } } @@ -280,7 +280,8 @@ contract DeployHelper { getAccessController(), address(ipAccountRegistry), getRoyaltyModule(), - getLicenseRegistry() + getLicenseRegistry(), + getDisputeModule() ); console2.log("DeployHelper: Using REAL LicensingModule"); } diff --git a/test/foundry/utils/LicensingHelper.t.sol b/test/foundry/utils/LicensingHelper.t.sol index 98565a3bc..b94b6a5ba 100644 --- a/test/foundry/utils/LicensingHelper.t.sol +++ b/test/foundry/utils/LicensingHelper.t.sol @@ -84,17 +84,7 @@ contract LicensingHelper { //////////////////////////////////////////////////////////////////////////*/ modifier withLFM_UML() { - BasePolicyFrameworkManager _pfm = BasePolicyFrameworkManager( - new UMLPolicyFrameworkManager( - address(accessController), - address(ipAccountRegistry), - address(licensingModule), - "uml", - "license Url" - ) - ); - licensingModule.registerPolicyFrameworkManager(address(_pfm)); - pfm["uml"] = address(_pfm); + _deployLFM_UML(); _; } @@ -354,6 +344,12 @@ contract LicensingHelper { return policyIds[pName]; } + function _registerUMLPolicyFromMapping(string memory name) internal returns (uint256) { + string memory pName = string(abi.encodePacked("uml_", name)); + policyIds[pName] = UMLPolicyFrameworkManager(pfm["uml"]).registerPolicy(policies[pName]); + return policyIds[pName]; + } + function _getMappedUmlPolicy(string memory name) internal view returns (UMLPolicy storage) { string memory pName = string(abi.encodePacked("uml_", name)); return policies[pName]; @@ -382,4 +378,18 @@ contract LicensingHelper { ) ); } + + function _deployLFM_UML() internal { + BasePolicyFrameworkManager _pfm = BasePolicyFrameworkManager( + new UMLPolicyFrameworkManager( + address(accessController), + address(ipAccountRegistry), + address(licensingModule), + "uml", + "license Url" + ) + ); + licensingModule.registerPolicyFrameworkManager(address(_pfm)); + pfm["uml"] = address(_pfm); + } } From 5444fa52519ab5d30519f018d729facd2c206ea1 Mon Sep 17 00:00:00 2001 From: Raul Date: Thu, 15 Feb 2024 10:11:58 -0800 Subject: [PATCH 02/14] wip integration tests --- .../integration/flows/disputes/Disputes.t.sol | 72 +++++++++++++++++++ .../integration/flows/disputes/README.md | 15 ++++ 2 files changed, 87 insertions(+) create mode 100644 test/foundry/integration/flows/disputes/Disputes.t.sol create mode 100644 test/foundry/integration/flows/disputes/README.md diff --git a/test/foundry/integration/flows/disputes/Disputes.t.sol b/test/foundry/integration/flows/disputes/Disputes.t.sol new file mode 100644 index 000000000..bbc371503 --- /dev/null +++ b/test/foundry/integration/flows/disputes/Disputes.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.23; + +// external +import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +// contract +import { IIPAccount } from "contracts/interfaces/IIPAccount.sol"; +import { IP } from "contracts/lib/IP.sol"; +import { Errors } from "contracts/lib/Errors.sol"; + +// test +import { BaseIntegration } from "test/foundry/integration/BaseIntegration.t.sol"; +import { MintPaymentPolicyFrameworkManager } from "test/foundry/mocks/licensing/MintPaymentPolicyFrameworkManager.sol"; +import { UMLPolicyGenericParams, UMLPolicyCommercialParams, UMLPolicyDerivativeParams } from "test/foundry/utils/LicensingHelper.t.sol"; + +contract DisputeIntegrationTest is BaseIntegration { + using EnumerableSet for EnumerableSet.UintSet; + using Strings for *; + + mapping(uint256 tokenId => address ipAccount) internal ipAcct; + uint256 policyId; + + function setUp() public override { + super.setUp(); + + // Register UML Framework + _deployLFM_UML(); + + // Register a License + _mapUMLPolicySimple({ + name: "non-commercial-remix", + commercial: false, + derivatives: true, + reciprocal: true, + commercialRevShare: 0, + derivativesRevShare: 0 + }); + policyId = _registerUMLPolicyFromMapping("non-commercial-remix"); + /* + // Register an original work with both policies set + mockNFT.mintId(u.alice, 1); + vm.startPrank(u.alice); + ipAcct[1] = registerIpAccount(mockNFT, 1, u.alice); + licensingModule.addPolicyToIp(ipAcct[1], _getUmlPolicyId("non-commercial-remix")); + */ + } + + function test_Disputes_revert_cannotMintFromDisputedIp() public { + _disputeIp(ipAcct[1]); + vm.startPrank(u.alice); + vm.expectRevert(Errors.LicensingModule__DisputedIpId.selector); + licensingModule.mintLicense(policyId, ipAcct[1], 1, u.bob); + } + + function test_Disputes_revert_cannotLinkDisputedIp() public { + + } + + function _disputeIp(address ipAddr) public { + vm.startPrank(u.bob); + IERC20(USDC).approve(address(arbitrationPolicySP), ARBITRATION_PRICE); + uint256 disputeId = disputeModule.raiseDispute(ipAddr, string("urlExample"), "PLAGIARISM", ""); + vm.stopPrank(); + + vm.prank(u.admin); + disputeModule.setDisputeJudgement(disputeId, true, ""); + } + +} \ No newline at end of file diff --git a/test/foundry/integration/flows/disputes/README.md b/test/foundry/integration/flows/disputes/README.md new file mode 100644 index 000000000..e2a258cc3 --- /dev/null +++ b/test/foundry/integration/flows/disputes/README.md @@ -0,0 +1,15 @@ +# DISPUTED CASES + +# Plagiarism +Bob owns IP1 +Bob sets P1 in IP1 (he can, since he clains IP1 is original) +Alice owns IP2 +Alice mints L1 from IP1-P1 +Alice links IP2 to IP1 with L1 +Dan finds out IP1 plagiarizes his work, IP0 +Dan raises dispute against IP1 for plagiarism +Dispute passes, IP1 is labeled as plagiarism +Bob cannot set policies in IP1 +Bob cannot mint licenses from IP1 +Alice cannot set policies in IP2 +Alice cannot mint licenses from IP2 From 5306b41a373d7dd4cefbbfa40966758870cd5f69 Mon Sep 17 00:00:00 2001 From: Jongwon Park Date: Thu, 15 Feb 2024 14:33:35 -0600 Subject: [PATCH 03/14] fix: Auth-based module setter on LicenseRegistry, remove in interface --- .../interfaces/registries/ILicenseRegistry.sol | 4 ---- contracts/registries/LicenseRegistry.sol | 15 +++++++++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/contracts/interfaces/registries/ILicenseRegistry.sol b/contracts/interfaces/registries/ILicenseRegistry.sol index b760da179..13b4e90d0 100644 --- a/contracts/interfaces/registries/ILicenseRegistry.sol +++ b/contracts/interfaces/registries/ILicenseRegistry.sol @@ -22,10 +22,6 @@ interface ILicenseRegistry is IERC1155 { Licensing.License licenseData ); - /// @notice Set the licensing module - /// @param newLicensingModule The address of the licensing module - function setLicensingModule(address newLicensingModule) external; - /// @notice Returns the address of the licensing module function licensingModule() external view returns (address); diff --git a/contracts/registries/LicenseRegistry.sol b/contracts/registries/LicenseRegistry.sol index 2ba4ed234..d614c2819 100644 --- a/contracts/registries/LicenseRegistry.sol +++ b/contracts/registries/LicenseRegistry.sol @@ -7,16 +7,17 @@ import { IPolicyFrameworkManager } from "../interfaces/modules/licensing/IPolicy import { ILicenseRegistry } from "../interfaces/registries/ILicenseRegistry.sol"; import { ILicensingModule } from "../interfaces/modules/licensing/ILicensingModule.sol"; import { IDisputeModule } from "../interfaces/modules/dispute/IDisputeModule.sol"; +import { Governable } from "../governance/Governable.sol"; import { Errors } from "../lib/Errors.sol"; import { Licensing } from "../lib/Licensing.sol"; import { DataUniqueness } from "../lib/DataUniqueness.sol"; /// @title LicenseRegistry aka LNFT /// @notice Registry of License NFTs, which represent licenses granted by IP ID licensors to create derivative IPs. -contract LicenseRegistry is ERC1155, ILicenseRegistry { +contract LicenseRegistry is ILicenseRegistry, ERC1155, Governable { // TODO: deploy with CREATE2 to make this immutable ILicensingModule private _licensingModule; - IDisputeModule public immutable DISPUTE_MODULE; + IDisputeModule public DISPUTE_MODULE; mapping(bytes32 licenseHash => uint256 ids) private _hashedLicenses; mapping(uint256 licenseIds => Licensing.License licenseData) private _licenses; @@ -33,14 +34,16 @@ contract LicenseRegistry is ERC1155, ILicenseRegistry { _; } - constructor(address disputeModule) ERC1155("") { - if (disputeModule == address(0)) { + constructor(address governance) ERC1155("") Governable(governance) {} + + function setDisputeModule(address newDisputeModule) external onlyProtocolAdmin { + if (newDisputeModule == address(0)) { revert Errors.LicenseRegistry__ZeroDisputeModule(); } - DISPUTE_MODULE = IDisputeModule(disputeModule); + DISPUTE_MODULE = IDisputeModule(newDisputeModule); } - function setLicensingModule(address newLicensingModule) external { + function setLicensingModule(address newLicensingModule) external onlyProtocolAdmin { if (newLicensingModule == address(0)) { revert Errors.LicenseRegistry__ZeroLicensingModule(); } From 30310975d9bcc57e402e8715e5262ab287e50a5a Mon Sep 17 00:00:00 2001 From: Jongwon Park Date: Thu, 15 Feb 2024 14:35:18 -0600 Subject: [PATCH 04/14] fix: Reorder DisputeModule deployment; Use new constructor & module setter for LicenseRegistry --- script/foundry/deployment/Main.s.sol | 3 ++- .../modules/licensing/LicensingModule.t.sol | 3 ++- test/foundry/utils/BaseTest.t.sol | 5 ++++- test/foundry/utils/DeployHelper.t.sol | 14 +++++++------- test/foundry/utils/LicensingHelper.t.sol | 2 +- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/script/foundry/deployment/Main.s.sol b/script/foundry/deployment/Main.s.sol index e5a4c5456..efcb89f5f 100644 --- a/script/foundry/deployment/Main.s.sol +++ b/script/foundry/deployment/Main.s.sol @@ -206,7 +206,7 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler { contractKey = "LicenseRegistry"; _predeploy(contractKey); - licenseRegistry = new LicenseRegistry(address(disputeModule)); + licenseRegistry = new LicenseRegistry(address(governance)); _postdeploy(contractKey, address(licenseRegistry)); contractKey = "LicensingModule"; @@ -301,6 +301,7 @@ contract Main is Script, BroadcastManager, JsonDeploymentHandler { ipMetadataProvider = IPMetadataProvider(ipAssetRegistry.metadataProvider()); _postdeploy("IPMetadataProvider", address(ipMetadataProvider)); + licenseRegistry.setDisputeModule(address(disputeModule)); licenseRegistry.setLicensingModule(address(licensingModule)); ipAssetRegistry.setRegistrationModule(address(registrationModule)); } diff --git a/test/foundry/modules/licensing/LicensingModule.t.sol b/test/foundry/modules/licensing/LicensingModule.t.sol index bf4020542..146a2b7ba 100644 --- a/test/foundry/modules/licensing/LicensingModule.t.sol +++ b/test/foundry/modules/licensing/LicensingModule.t.sol @@ -87,7 +87,7 @@ contract LicensingModuleTest is Test { ); royaltyModule = new RoyaltyModule(address(governance)); disputeModule = new MockDisputeModule(); - licenseRegistry = new LicenseRegistry(address(disputeModule)); + licenseRegistry = new LicenseRegistry(address(governance)); licensingModule = new LicensingModule( address(accessController), address(ipAssetRegistry), @@ -97,6 +97,7 @@ contract LicensingModuleTest is Test { ); mockRoyaltyPolicyLS = new MockRoyaltyPolicyLS(address(royaltyModule)); + licenseRegistry.setDisputeModule(address(disputeModule)); licenseRegistry.setLicensingModule(address(licensingModule)); // Setup Framework Managers (don't register PFM here, do in each test case) module1 = new MockPolicyFrameworkManager( diff --git a/test/foundry/utils/BaseTest.t.sol b/test/foundry/utils/BaseTest.t.sol index 609fc72d5..4d3548b12 100644 --- a/test/foundry/utils/BaseTest.t.sol +++ b/test/foundry/utils/BaseTest.t.sol @@ -11,6 +11,7 @@ import { AccessController } from "../../../contracts/AccessController.sol"; // solhint-disable-next-line max-line-length import { IP_RESOLVER_MODULE_KEY, REGISTRATION_MODULE_KEY, DISPUTE_MODULE_KEY, TAGGING_MODULE_KEY, ROYALTY_MODULE_KEY, LICENSING_MODULE_KEY } from "../../../contracts/lib/modules/Module.sol"; import { AccessPermission } from "../../../contracts/lib/AccessPermission.sol"; +import { LicenseRegistry } from "../../../contracts/registries/LicenseRegistry.sol"; // test import { DeployHelper } from "./DeployHelper.t.sol"; @@ -183,7 +184,9 @@ contract BaseTest is Test, DeployHelper, LicensingHelper { require(address(licenseRegistry) != address(0), "licenseRegistry not set"); vm.startPrank(u.admin); - licenseRegistry.setLicensingModule(getLicensingModule()); + // Need to cast to LicenseRegistry to set dispute and licensing module, as interface doesn't expose those fns. + LicenseRegistry(address(licenseRegistry)).setDisputeModule(getDisputeModule()); + LicenseRegistry(address(licenseRegistry)).setLicensingModule(getLicensingModule()); vm.stopPrank(); } diff --git a/test/foundry/utils/DeployHelper.t.sol b/test/foundry/utils/DeployHelper.t.sol index 27bfc079a..bb7008a18 100644 --- a/test/foundry/utils/DeployHelper.t.sol +++ b/test/foundry/utils/DeployHelper.t.sol @@ -255,7 +255,7 @@ contract DeployHelper { console2.log("DeployHelper: Using REAL IPAssetRegistry"); if (d.licenseRegistry) { - licenseRegistry = new LicenseRegistry(getDisputeModule()); + licenseRegistry = new LicenseRegistry(getGovernance()); console2.log("DeployHelper: Using REAL LicenseRegistry"); } } @@ -274,6 +274,12 @@ contract DeployHelper { console2.log("DeployHelper: Using REAL RoyaltyModule"); postDeployConditions.royaltyModule_configure = true; } + if (d.disputeModule) { + require(address(ipAssetRegistry) != address(0), "DeployHelper Module: IPAssetRegistry required"); + disputeModule = new DisputeModule(getAccessController(), address(ipAssetRegistry), getGovernance()); + console2.log("DeployHelper: Using REAL DisputeModule"); + postDeployConditions.disputeModule_configure = true; + } if (d.licensingModule) { require(address(ipAccountRegistry) != address(0), "DeployHelper Module: IPAccountRegistry required"); licensingModule = new LicensingModule( @@ -299,12 +305,6 @@ contract DeployHelper { ); console2.log("DeployHelper: Using REAL RegistrationModule"); } - if (d.disputeModule) { - require(address(ipAssetRegistry) != address(0), "DeployHelper Module: IPAssetRegistry required"); - disputeModule = new DisputeModule(getAccessController(), address(ipAssetRegistry), getGovernance()); - console2.log("DeployHelper: Using REAL DisputeModule"); - postDeployConditions.disputeModule_configure = true; - } } function _deployPolicyConditionally(DeployPolicyCondition memory d) public { diff --git a/test/foundry/utils/LicensingHelper.t.sol b/test/foundry/utils/LicensingHelper.t.sol index b94b6a5ba..ade409bc5 100644 --- a/test/foundry/utils/LicensingHelper.t.sol +++ b/test/foundry/utils/LicensingHelper.t.sol @@ -305,7 +305,7 @@ contract LicensingHelper { territories: emptyStringArray, distributionChannels: emptyStringArray, contentRestrictions: emptyStringArray, - royaltyPolicy: address(royaltyPolicy) + royaltyPolicy: commercial ? address(royaltyPolicy) : address(0) }); } From e0a7d1ca22302d56f7d28942a8fabe5a10e48313 Mon Sep 17 00:00:00 2001 From: Jongwon Park Date: Thu, 15 Feb 2024 14:35:44 -0600 Subject: [PATCH 05/14] test: Basic Dispute + Licensing integration test --- .../integration/flows/disputes/Disputes.t.sol | 78 +++++++++++++++---- 1 file changed, 64 insertions(+), 14 deletions(-) diff --git a/test/foundry/integration/flows/disputes/Disputes.t.sol b/test/foundry/integration/flows/disputes/Disputes.t.sol index bbc371503..6827eb846 100644 --- a/test/foundry/integration/flows/disputes/Disputes.t.sol +++ b/test/foundry/integration/flows/disputes/Disputes.t.sol @@ -16,7 +16,7 @@ import { BaseIntegration } from "test/foundry/integration/BaseIntegration.t.sol" import { MintPaymentPolicyFrameworkManager } from "test/foundry/mocks/licensing/MintPaymentPolicyFrameworkManager.sol"; import { UMLPolicyGenericParams, UMLPolicyCommercialParams, UMLPolicyDerivativeParams } from "test/foundry/utils/LicensingHelper.t.sol"; -contract DisputeIntegrationTest is BaseIntegration { +contract Flows_Integration_Disputes is BaseIntegration { using EnumerableSet for EnumerableSet.UintSet; using Strings for *; @@ -39,34 +39,84 @@ contract DisputeIntegrationTest is BaseIntegration { derivativesRevShare: 0 }); policyId = _registerUMLPolicyFromMapping("non-commercial-remix"); - /* + // Register an original work with both policies set mockNFT.mintId(u.alice, 1); - vm.startPrank(u.alice); + mockNFT.mintId(u.bob, 2); + mockNFT.mintId(u.carl, 3); + ipAcct[1] = registerIpAccount(mockNFT, 1, u.alice); + ipAcct[2] = registerIpAccount(mockNFT, 2, u.bob); + ipAcct[3] = registerIpAccount(mockNFT, 3, u.carl); + + vm.startPrank(u.alice); licensingModule.addPolicyToIp(ipAcct[1], _getUmlPolicyId("non-commercial-remix")); - */ + vm.stopPrank(); } - function test_Disputes_revert_cannotMintFromDisputedIp() public { - _disputeIp(ipAcct[1]); - vm.startPrank(u.alice); + function test_Integration_Disputes_revert_cannotMintFromDisputedIp() public { + assertEq(licenseRegistry.balanceOf(u.carl, policyId), 0); + vm.prank(u.carl); + licensingModule.mintLicense(policyId, ipAcct[1], 1, u.carl); + assertEq(licenseRegistry.balanceOf(u.carl, policyId), 1); + + _disputeIp(u.bob, ipAcct[1]); + + vm.prank(u.carl); vm.expectRevert(Errors.LicensingModule__DisputedIpId.selector); - licensingModule.mintLicense(policyId, ipAcct[1], 1, u.bob); + licensingModule.mintLicense(policyId, ipAcct[1], 1, u.carl); } - function test_Disputes_revert_cannotLinkDisputedIp() public { + function test_Integration_Disputes_revert_cannotLinkDisputedIp() public { + assertEq(licenseRegistry.balanceOf(u.carl, policyId), 0); + vm.prank(u.carl); + uint256 licenseId = licensingModule.mintLicense(policyId, ipAcct[1], 1, u.carl); + assertEq(licenseRegistry.balanceOf(u.carl, policyId), 1); + + _disputeIp(u.bob, ipAcct[1]); + + uint256[] memory licenseIds = new uint256[](1); + licenseIds[0] = licenseId; + vm.prank(u.carl); + vm.expectRevert(Errors.LicensingModule__LinkingRevokedLicense.selector); + licensingModule.linkIpToParents(licenseIds, ipAcct[3], 0); // 0, non-commercial } - function _disputeIp(address ipAddr) public { - vm.startPrank(u.bob); + // TODO: check if IPAccount is transferable even if disputed + + function test_Integration_Disputes_transferLicenseAfterIpDispute() public { + assertEq(licenseRegistry.balanceOf(u.carl, policyId), 0); + vm.prank(u.carl); + uint256 licenseId = licensingModule.mintLicense(policyId, ipAcct[1], 1, u.carl); + assertEq(licenseRegistry.balanceOf(u.carl, policyId), 1); + + _disputeIp(u.bob, ipAcct[1]); + + // eEen if the IP asset is disputed, license owners can still transfer license NFTs + vm.prank(u.carl); + licenseRegistry.safeTransferFrom(u.carl, u.bob, licenseId, 1, ""); + } + + function test_Integration_Disputes_mintLicenseAfterDisputeIsResolved() public { + uint256 disputeId = _disputeIp(u.bob, ipAcct[1]); + + vm.prank(u.bob); + disputeModule.resolveDispute(disputeId); + + assertEq(licenseRegistry.balanceOf(u.carl, policyId), 0); + vm.prank(u.carl); + licensingModule.mintLicense(policyId, ipAcct[1], 1, u.carl); + assertEq(licenseRegistry.balanceOf(u.carl, policyId), 1); + } + + function _disputeIp(address disputeInitiator, address ipAddrToDispute) internal returns (uint256 disputeId) { + vm.startPrank(disputeInitiator); IERC20(USDC).approve(address(arbitrationPolicySP), ARBITRATION_PRICE); - uint256 disputeId = disputeModule.raiseDispute(ipAddr, string("urlExample"), "PLAGIARISM", ""); + disputeId = disputeModule.raiseDispute(ipAddrToDispute, string("urlExample"), "PLAGIARISM", ""); vm.stopPrank(); - vm.prank(u.admin); + vm.prank(u.admin); // admin is a judge disputeModule.setDisputeJudgement(disputeId, true, ""); } - } \ No newline at end of file From d509800e9cdd373ddee988fabbcd3068fecac363 Mon Sep 17 00:00:00 2001 From: Jongwon Park Date: Thu, 15 Feb 2024 14:40:10 -0600 Subject: [PATCH 06/14] style: solhint --- .../interfaces/modules/dispute/IDisputeModule.sol | 8 ++++---- test/foundry/integration/flows/disputes/Disputes.t.sol | 10 +++------- test/foundry/mocks/module/MockDisputeModule.sol | 9 ++++----- test/foundry/mocks/registry/MockLicenseRegistry.sol | 2 +- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/contracts/interfaces/modules/dispute/IDisputeModule.sol b/contracts/interfaces/modules/dispute/IDisputeModule.sol index 87950d391..b95850d2b 100644 --- a/contracts/interfaces/modules/dispute/IDisputeModule.sol +++ b/contracts/interfaces/modules/dispute/IDisputeModule.sol @@ -160,19 +160,19 @@ interface IDisputeModule { /// @param _ipId The ipId /// @param _tag The tag function isIpTaggedWith(address _ipId, bytes32 _tag) external view returns (bool); - + /// @notice returns true if the ipId is tagged with any tag (meaning at least one dispute went through) /// @param _ipId The ipId function isIpTagged(address _ipId) external view returns (bool); - + /// @notice returns the tags for a given ipId (note: this method could be expensive, use in frontends only) /// @param _ipId The ipId function ipTags(address _ipId) external view returns (bytes32[] memory); - + /// @notice returns the total tags for a given ipId /// @param _ipId The ipId function totalTagsForIp(address _ipId) external view returns (uint256); - + /// @notice returns the tag at a given index for a given ipId. No guarantees on ordering /// @param _ipId The ipId function tagForIpAt(address _ipId, uint256 _index) external view returns (bytes32); diff --git a/test/foundry/integration/flows/disputes/Disputes.t.sol b/test/foundry/integration/flows/disputes/Disputes.t.sol index 6827eb846..04182f729 100644 --- a/test/foundry/integration/flows/disputes/Disputes.t.sol +++ b/test/foundry/integration/flows/disputes/Disputes.t.sol @@ -7,14 +7,10 @@ import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; // contract -import { IIPAccount } from "contracts/interfaces/IIPAccount.sol"; -import { IP } from "contracts/lib/IP.sol"; import { Errors } from "contracts/lib/Errors.sol"; // test import { BaseIntegration } from "test/foundry/integration/BaseIntegration.t.sol"; -import { MintPaymentPolicyFrameworkManager } from "test/foundry/mocks/licensing/MintPaymentPolicyFrameworkManager.sol"; -import { UMLPolicyGenericParams, UMLPolicyCommercialParams, UMLPolicyDerivativeParams } from "test/foundry/utils/LicensingHelper.t.sol"; contract Flows_Integration_Disputes is BaseIntegration { using EnumerableSet for EnumerableSet.UintSet; @@ -25,10 +21,10 @@ contract Flows_Integration_Disputes is BaseIntegration { function setUp() public override { super.setUp(); - + // Register UML Framework _deployLFM_UML(); - + // Register a License _mapUMLPolicySimple({ name: "non-commercial-remix", @@ -119,4 +115,4 @@ contract Flows_Integration_Disputes is BaseIntegration { vm.prank(u.admin); // admin is a judge disputeModule.setDisputeJudgement(disputeId, true, ""); } -} \ No newline at end of file +} diff --git a/test/foundry/mocks/module/MockDisputeModule.sol b/test/foundry/mocks/module/MockDisputeModule.sol index 805506f2c..32b586791 100644 --- a/test/foundry/mocks/module/MockDisputeModule.sol +++ b/test/foundry/mocks/module/MockDisputeModule.sol @@ -126,22 +126,21 @@ contract MockDisputeModule is BaseModule, IDisputeModule { function isIpTaggedWith(address, bytes32) external pure returns (bool) { return false; } - + function isIpTagged(address) external pure returns (bool) { return false; } - + function ipTags(address) external pure returns (bytes32[] memory) { bytes32[] memory tags; return tags; } - + function totalTagsForIp(address) external pure returns (uint256) { return 0; } - + function tagForIpAt(address, uint256) external pure returns (bytes32) { return bytes32(0); } - } diff --git a/test/foundry/mocks/registry/MockLicenseRegistry.sol b/test/foundry/mocks/registry/MockLicenseRegistry.sol index ab6ab7aba..c1344b16e 100644 --- a/test/foundry/mocks/registry/MockLicenseRegistry.sol +++ b/test/foundry/mocks/registry/MockLicenseRegistry.sol @@ -78,7 +78,7 @@ contract MockLicenseRegistry is ERC1155, ILicenseRegistry { return _licenses[licenseId].policyId; } - function isLicenseRevoked(uint256 licenseId) external pure returns (bool) { + function isLicenseRevoked(uint256) external pure returns (bool) { return false; } From 7610de7a21726a94355cd6f39b9e19d1925b364d Mon Sep 17 00:00:00 2001 From: Jongwon Park Date: Thu, 15 Feb 2024 18:42:02 -0600 Subject: [PATCH 07/14] feat: update metadata in LicenseRegistry and UML PFM & block transfer of revoked IPA licenses --- contracts/lib/Errors.sol | 1 + contracts/lib/Licensing.sol | 5 +- .../licensing/UMLPolicyFrameworkManager.sol | 118 ++++++++++-------- contracts/registries/LicenseRegistry.sol | 60 ++++++++- 4 files changed, 126 insertions(+), 58 deletions(-) diff --git a/contracts/lib/Errors.sol b/contracts/lib/Errors.sol index c74d609fb..0c5c27c25 100644 --- a/contracts/lib/Errors.sol +++ b/contracts/lib/Errors.sol @@ -114,6 +114,7 @@ library Errors { error LicenseRegistry__CallerNotLicensingModule(); error LicenseRegistry__ZeroLicensingModule(); error LicensingModule__CallerNotLicenseRegistry(); + error LicenseRegistry__RevokedLicense(); /// @notice emitted when trying to transfer a license that is not transferable (by policy) error LicenseRegistry__NotTransferable(); /// @notice emitted on constructor if dispute module is not set diff --git a/contracts/lib/Licensing.sol b/contracts/lib/Licensing.sol index 30bcdca89..9b6b756fa 100644 --- a/contracts/lib/Licensing.sol +++ b/contracts/lib/Licensing.sol @@ -16,9 +16,10 @@ library Licensing { } /// @notice Data that define a License Agreement NFT - /// @param policyId Id of the policy this license is based on, which will be set in the derivative - /// IP when the license is burnt + /// @param policyId Id of the policy this license is based on, which will be set in the derivative IP when the + /// license is burnt for linking /// @param licensorIpId Id of the IP this license is for + /// @param transferable Whether or not the license is transferable struct License { uint256 policyId; address licensorIpId; diff --git a/contracts/modules/licensing/UMLPolicyFrameworkManager.sol b/contracts/modules/licensing/UMLPolicyFrameworkManager.sol index 15d009ddf..85a54a96c 100644 --- a/contracts/modules/licensing/UMLPolicyFrameworkManager.sol +++ b/contracts/modules/licensing/UMLPolicyFrameworkManager.sol @@ -3,7 +3,6 @@ pragma solidity ^0.8.23; // external -import { Base64 } from "@openzeppelin/contracts/utils/Base64.sol"; import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol"; import { ReentrancyGuard } from "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; @@ -258,86 +257,97 @@ contract UMLPolicyFrameworkManager is UMLPolicy memory policy = abi.decode(policyData, (UMLPolicy)); /* solhint-disable */ - // Follows the OpenSea standard for JSON metadata - - // base json + // Follows the OpenSea standard for JSON metadata. + // **Attributions** string memory json = string( - '{"name": "Story Protocol License NFT", "description": "License agreement stating the terms of a Story Protocol IPAsset", "attributes": [' - ); - - // Attributions - json = string( abi.encodePacked( - json, '{"trait_type": "Attribution", "value": "', policy.attribution ? "true" : "false", '"},', '{"trait_type": "Transferable", "value": "', policy.transferable ? "true" : "false", '"},', + _policyCommercialTraitsToJson(policy), + _policyDerivativeTraitsToJson(policy) + ) + ); + + json = string(abi.encodePacked(json, '{"trait_type": "Territories", "value": [')); + uint256 count = policy.territories.length; + for (uint256 i = 0; i < count; ++i) { + json = string(abi.encodePacked(json, '"', policy.territories[i], '"')); + if (i != count - 1) { // skip comma for last element in the array + json = string(abi.encodePacked(json, ",")); + } + } + json = string(abi.encodePacked(json, ']},')); // close the trait_type: "Territories" array + + json = string(abi.encodePacked(json, '{"trait_type": "Distribution Channels", "value": [')); + count = policy.distributionChannels.length; + for (uint256 i = 0; i < count; ++i) { + json = string(abi.encodePacked(json, '"', policy.distributionChannels[i], '"')); + if (i != count - 1) { // skip comma for last element in the array + json = string(abi.encodePacked(json, ",")); + } + } + json = string(abi.encodePacked(json, "]},")); // close the trait_type: "Distribution Channels" array + + // NOTE: (above) last trait added by PFM should have a comma at the end. + + /* solhint-enable */ + + return json; + } + + /// @notice Encodes the commercial traits of UML policy into a JSON string for OpenSea + function _policyCommercialTraitsToJson(UMLPolicy memory policy) internal pure returns (string memory) { + // NOTE: TOTAL_RNFT_SUPPLY = 1000 in trait with max_value. For numbers, don't add any display_type, so that + // they will show up in the "Ranking" section of the OpenSea UI. + return string( + abi.encodePacked( + '{"trait_type": "Attribution", "value": "', + policy.attribution ? "true" : "false", + '"},', + // Skip transferable, it's already added in the common attributes by the LicenseRegistry. + // Should be managed by the LicenseRegistry, not the PFM. '{"trait_type": "Commerical Use", "value": "', policy.commercialUse ? "true" : "false", '"},', - '{"trait_type": "commercialAttribution", "value": "', + '{"trait_type": "Commercial Attribution", "value": "', policy.commercialAttribution ? "true" : "false", '"},', - '{"trait_type": "commercialRevShare", "value": ', - Strings.toString(policy.commercialRevShare), - "}," - ) - ); - json = string( - abi.encodePacked( - json, - '{"trait_type": "commercializerCheck", "value": "', - policy.commercializerChecker.toHexString() + '{"trait_type": "Commercial Revenue Share", "max_value": 1000, "value": ', + policy.commercialRevShare.toString(), + "},", + '{"trait_type": "Commercializer Check", "value": "', + policy.commercializerChecker.toHexString(), + // Skip on commercializerCheckerData as it's bytes as irrelevant for the user metadata + '"},' ) ); - // TODO: add commercializersData? - json = string( + } + + /// @notice Encodes the derivative traits of UML policy into a JSON string for OpenSea + function _policyDerivativeTraitsToJson(UMLPolicy memory policy) internal pure returns (string memory) { + return string( abi.encodePacked( - json, - '"}, {"trait_type": "derivativesAllowed", "value": "', + '{"trait_type": "Derivatives Allowed", "value": "', policy.derivativesAllowed ? "true" : "false", '"},', - '{"trait_type": "derivativesAttribution", "value": "', + '{"trait_type": "Derivatives Attribution", "value": "', policy.derivativesAttribution ? "true" : "false", '"},', - '{"trait_type": "derivativesApproval", "value": "', + '{"trait_type": "Derivatives Approval", "value": "', policy.derivativesApproval ? "true" : "false", '"},', - '{"trait_type": "derivativesReciprocal", "value": "', + '{"trait_type": "Derivatives Reciprocal", "value": "', policy.derivativesReciprocal ? "true" : "false", '"},', - '{"trait_type": "derivativesRevShare", "value": ', - Strings.toString(policy.derivativesRevShare), + '{"trait_type": "Derivatives Revenue Share", "max_value": 1000, "value": ', + policy.derivativesRevShare.toString(), "}," - '{"trait_type": "territories", "value": [' ) ); - - uint256 territoryCount = policy.territories.length; - for (uint256 i = 0; i < territoryCount; ++i) { - json = string(abi.encodePacked(json, '"', policy.territories[i], '"')); - if (i != territoryCount - 1) { - json = string(abi.encodePacked(json, ",")); - } - } - - json = string(abi.encodePacked(json, ']}, {"trait_type": "distributionChannels", "value": [')); - - uint256 distributionChannelCount = policy.distributionChannels.length; - for (uint256 i = 0; i < distributionChannelCount; ++i) { - json = string(abi.encodePacked(json, '"', policy.distributionChannels[i], '"')); - if (i != distributionChannelCount - 1) { - json = string(abi.encodePacked(json, ",")); - } - } - - json = string(abi.encodePacked(json, "]}]}")); - /* solhint-enable */ - - return string(abi.encodePacked("data:application/json;base64,", Base64.encode(bytes(json)))); } /// Checks the configuration of commercial use and throws if the policy is not compliant diff --git a/contracts/registries/LicenseRegistry.sol b/contracts/registries/LicenseRegistry.sol index d614c2819..49e0faabf 100644 --- a/contracts/registries/LicenseRegistry.sol +++ b/contracts/registries/LicenseRegistry.sol @@ -2,6 +2,8 @@ pragma solidity ^0.8.23; import { ERC1155 } from "@openzeppelin/contracts/token/ERC1155/ERC1155.sol"; +import { Base64 } from "@openzeppelin/contracts/utils/Base64.sol"; +import { Strings } from "@openzeppelin/contracts/utils/Strings.sol"; import { IPolicyFrameworkManager } from "../interfaces/modules/licensing/IPolicyFrameworkManager.sol"; import { ILicenseRegistry } from "../interfaces/registries/ILicenseRegistry.sol"; @@ -15,6 +17,8 @@ import { DataUniqueness } from "../lib/DataUniqueness.sol"; /// @title LicenseRegistry aka LNFT /// @notice Registry of License NFTs, which represent licenses granted by IP ID licensors to create derivative IPs. contract LicenseRegistry is ILicenseRegistry, ERC1155, Governable { + using Strings for *; + // TODO: deploy with CREATE2 to make this immutable ILicensingModule private _licensingModule; IDisputeModule public DISPUTE_MODULE; @@ -126,17 +130,66 @@ contract LicenseRegistry is ILicenseRegistry, ERC1155, Governable { /// @notice Returns true if the license has been revoked (licensor tagged after a dispute in /// the dispute module). If the tag is removed, the license is not revoked anymore. /// @param licenseId The id of the license - function isLicenseRevoked(uint256 licenseId) external view returns (bool) { + function isLicenseRevoked(uint256 licenseId) public view returns (bool) { // For beta, any tag means revocation, for mainnet we need more context. // TODO: signal metadata update when tag changes. return DISPUTE_MODULE.isIpTagged(_licenses[licenseId].licensorIpId); } /// @notice ERC1155 OpenSea metadata JSON representation of the LNFT parameters + /// @dev Expect PFM.policyToJson to return {'trait_type: 'value'},{'trait_type': 'value'},...,{...} + /// (last attribute must not have a comma at the end) function uri(uint256 id) public view virtual override returns (string memory) { Licensing.License memory licenseData = _licenses[id]; Licensing.Policy memory pol = _licensingModule.policy(licenseData.policyId); - return IPolicyFrameworkManager(pol.policyFramework).policyToJson(pol.data); + + /* solhint-disable */ + // Follows the OpenSea standard for JSON metadata + + // base json, open the attributes array + string memory json = string( + abi.encodePacked( + "{", + '"name": "Story Protocol License NFT",', + '"description": "License agreement stating the terms of a Story Protocol IPAsset",', + '"external_url": "https://storyprotocol.xyz",', + // solhint-disable-next-line max-length + '"image": "https://images.ctfassets.net/5ei3wx54t1dp/1WXOHnPLROsGiBsI46zECe/4f38a95c58d3b0329af3085b36d720c8/Story_Protocol_Icon.png",', + '"attributes": [' + ) + ); + + // append the policy specific attributes (last attribute added by PFM should have a comma at the end) + // TODO: Safeguard mechanism to make sure the attributes added by PFM do NOT overlap with the common traits + // defined above. Currently, we add the common license attributes after adding the PFM attributes to override. + // But OpenSea might take the value of the first duplicate. + json = string(abi.encodePacked(json, IPolicyFrameworkManager(pol.policyFramework).policyToJson(pol.data))); + + // append the common license attributes + json = string( + abi.encodePacked( + json, + '{"trait_type": "Licensor", "value": "', + licenseData.licensorIpId.toHexString(), + '"},', + '{"trait_type": "Policy Framework", "value": "', + pol.policyFramework.toHexString(), + '"},', + '{"trait_type": "Transferable", "value": "', + licenseData.transferable ? "true" : "false", + '"},', + '{"trait_type": "Revoked", "value": "', + isLicenseRevoked(id) ? "true" : "false", + '"}' + ) + ); + + // close the attributes array and the json metadata object + json = string(abi.encodePacked(json, "]}")); + + /* solhint-enable */ + + return string(abi.encodePacked("data:application/json;base64,", Base64.encode(bytes(json)))); } /// @dev Pre-hook for ERC1155's _update() called on transfers. @@ -152,6 +205,9 @@ contract LicenseRegistry is ILicenseRegistry, ERC1155, Governable { for (uint256 i = 0; i < ids.length; i++) { Licensing.License memory lic = _licenses[ids[i]]; // TODO: Hook for verify transfer params + if (isLicenseRevoked(ids[i])) { + revert Errors.LicenseRegistry__RevokedLicense(); + } if (!lic.transferable) { // True if from == licensor if (from != lic.licensorIpId) { From 6a2a16570425597e2edbfe36e712c726b8d1958c Mon Sep 17 00:00:00 2001 From: Jongwon Park Date: Thu, 15 Feb 2024 18:42:30 -0600 Subject: [PATCH 08/14] test: metadata test and revert test --- .../integration/flows/disputes/Disputes.t.sol | 3 +- .../modules/licensing/LicensingModule.t.sol | 172 ++++++++++-------- 2 files changed, 98 insertions(+), 77 deletions(-) diff --git a/test/foundry/integration/flows/disputes/Disputes.t.sol b/test/foundry/integration/flows/disputes/Disputes.t.sol index 04182f729..74b53a0af 100644 --- a/test/foundry/integration/flows/disputes/Disputes.t.sol +++ b/test/foundry/integration/flows/disputes/Disputes.t.sol @@ -89,8 +89,9 @@ contract Flows_Integration_Disputes is BaseIntegration { _disputeIp(u.bob, ipAcct[1]); - // eEen if the IP asset is disputed, license owners can still transfer license NFTs + // If the IP asset is disputed, license owners won't be able to transfer license NFTs vm.prank(u.carl); + vm.expectRevert(Errors.LicenseRegistry__RevokedLicense.selector); licenseRegistry.safeTransferFrom(u.carl, u.bob, licenseId, 1, ""); } diff --git a/test/foundry/modules/licensing/LicensingModule.t.sol b/test/foundry/modules/licensing/LicensingModule.t.sol index 146a2b7ba..d22c70416 100644 --- a/test/foundry/modules/licensing/LicensingModule.t.sol +++ b/test/foundry/modules/licensing/LicensingModule.t.sol @@ -64,7 +64,7 @@ contract LicensingModuleTest is Test { string public licenseUrl = "https://example.com/license"; address public ipId1; address public ipId2; - address public ipOwner = vm.addr(1); + address public ipOwner = address(0x100); // use static address, otherwise uri check fails because licensor changes address public licenseHolder = address(0x101); function setUp() public { @@ -447,13 +447,16 @@ contract LicensingModuleTest is Test { function test_LicensingModule_licenseUri() public { licensingModule.registerPolicyFrameworkManager(address(umlManager)); + MockTokenGatedHook tokenGatedHook = new MockTokenGatedHook(); + gatedNftFoo.mintId(address(this), 1); + UMLPolicy memory policyData = UMLPolicy({ transferable: true, attribution: true, commercialUse: true, commercialAttribution: true, - commercializerChecker: address(0), - commercializerCheckerData: "", + commercializerChecker: address(tokenGatedHook), + commercializerCheckerData: abi.encode(address(gatedNftFoo)), commercialRevShare: 0, derivativesAllowed: true, derivativesAttribution: true, @@ -466,11 +469,6 @@ contract LicensingModuleTest is Test { royaltyPolicy: address(mockRoyaltyPolicyLS) }); - gatedNftFoo.mintId(address(this), 1); - - MockTokenGatedHook tokenGatedHook = new MockTokenGatedHook(); - policyData.commercializerChecker = address(tokenGatedHook); - policyData.commercializerCheckerData = abi.encode(address(gatedNftFoo)); policyData.territories[0] = "territory1"; policyData.distributionChannels[0] = "distributionChannel1"; @@ -479,7 +477,9 @@ contract LicensingModuleTest is Test { vm.prank(ipOwner); licensingModule.addPolicyToIp(ipId1, policyId); - uint256 licenseId = licensingModule.mintLicense(policyId, ipId1, 1, licenseHolder); + // Set the licensor to `0xbeef` for uri testing. Must call LicenseRegistry directly to do so. + vm.prank(address(licensingModule)); + uint256 licenseId = licenseRegistry.mintLicense(policyId, address(0xbeef), true, 1, licenseHolder); string memory actualUri = licenseRegistry.uri(licenseId); @@ -488,78 +488,98 @@ contract LicensingModuleTest is Test { // DEV : Since the raw string below produces stack too deep error, we use the encoded output of the string below. // The string below is left here for reference. /* - string memory expectedJson = string(abi.encodePacked('{', - '"name":"Story Protocol License NFT",' - '"description":"License agreement stating the terms of a Story Protocol IPAsset",', - '"attributes":[', - '{', - '"trait_type":"Attribution",' - '"value":"true"', - '},', - '{', - '"trait_type":"Transferable",', - '"value":"true"', - '},', - '{', - '"trait_type":"Commerical Use",', - '"value":"true"', - '},', - '{', - '"trait_type":"commercialAttribution",', - '"value":"true"', - '},', - '{', - '"trait_type":"commercialRevShare",', - '"value":0', - '},', - '{', - '"trait_type":"commercializers",', - '"value":[', - '"commercializer1",', - '"commercializer2"', - ']', - '},', - '{', - '"trait_type":"derivativesAllowed",', - '"value":"true"', - '},', - '{', - '"trait_type":"derivativesAttribution",', - '"value":"true"', - '},', - '{', - '"trait_type":"derivativesApproval",', - '"value":"true"', - '},', - '{', - '"trait_type":"derivativesReciprocal",', - '"value":"true"', - '},', - '{', - '"trait_type":"derivativesRevShare",', - '"value":0', - '},', - '{', - '"trait_type":"territories",', - '"value":[', - '"territory1",', - ']', - '},' - '{', - '"trait_type":"distributionChannels"', - '"value":[', - '"distributionChannel1",', - ']', - '}', - ']', - '}' - )); + expectedJson = { + "name": "Story Protocol License NFT", + "description": "License agreement stating the terms of a Story Protocol IPAsset", + "external_url": "https://storyprotocol.xyz", + "image": "https://images.ctfassets.net/5ei3wx54t1dp/1WXOHnPLROsGiBsI46zECe/4f38a95c58d3b0329af3085b36d720c8/Story_Protocol_Icon.png", + "attributes": [ + { + "trait_type": "Attribution", + "value": "true" + }, + { + "trait_type": "Transferable", + "value": "true" + }, + { + "trait_type": "Attribution", + "value": "true" + }, + { + "trait_type": "Commerical Use", + "value": "true" + }, + { + "trait_type": "Commercial Attribution", + "value": "true" + }, + { + "trait_type": "Commercial Revenue Share", + "max_value": 1000, + "value": 0 + }, + { + "trait_type": "Commercializer Check", + "value": "0x210503c318855259983298ba58055a38d5ff63e0" + }, + { + "trait_type": "Derivatives Allowed", + "value": "true" + }, + { + "trait_type": "Derivatives Attribution", + "value": "true" + }, + { + "trait_type": "Derivatives Approval", + "value": "true" + }, + { + "trait_type": "Derivatives Reciprocal", + "value": "true" + }, + { + "trait_type": "Derivatives Revenue Share", + "max_value": 1000, + "value": 0 + }, + { + "trait_type": "Territories", + "value": [ + "territory1" + ] + }, + { + "trait_type": "Distribution Channels", + "value": [ + "distributionChannel1" + ] + }, + { + "trait_type": "Licensor", + "value": "0x000000000000000000000000000000000000beef" + }, + { + "trait_type": "Policy Framework", + "value": "0xf1e1d77c54e9c28cc1da2dbc377b4a85765c2542" + }, + { + "trait_type": "Transferable", + "value": "true" + }, + { + "trait_type": "Revoked", + "value": "false" + } + ] + }; */ /* solhint-enable */ /* solhint-disable */ string - memory expectedJson = "eyJuYW1lIjogIlN0b3J5IFByb3RvY29sIExpY2Vuc2UgTkZUIiwgImRlc2NyaXB0aW9uIjogIkxpY2Vuc2UgYWdyZWVtZW50IHN0YXRpbmcgdGhlIHRlcm1zIG9mIGEgU3RvcnkgUHJvdG9jb2wgSVBBc3NldCIsICJhdHRyaWJ1dGVzIjogW3sidHJhaXRfdHlwZSI6ICJBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIlRyYW5zZmVyYWJsZSIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkNvbW1lcmljYWwgVXNlIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiY29tbWVyY2lhbEF0dHJpYnV0aW9uIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiY29tbWVyY2lhbFJldlNoYXJlIiwgInZhbHVlIjogMH0seyJ0cmFpdF90eXBlIjogImNvbW1lcmNpYWxpemVyQ2hlY2siLCAidmFsdWUiOiAiMHgyMTA1MDNjMzE4ODU1MjU5OTgzMjk4YmE1ODA1NWEzOGQ1ZmY2M2UwIn0sIHsidHJhaXRfdHlwZSI6ICJkZXJpdmF0aXZlc0FsbG93ZWQiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJkZXJpdmF0aXZlc0F0dHJpYnV0aW9uIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiZGVyaXZhdGl2ZXNBcHByb3ZhbCIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogImRlcml2YXRpdmVzUmVjaXByb2NhbCIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogImRlcml2YXRpdmVzUmV2U2hhcmUiLCAidmFsdWUiOiAwfSx7InRyYWl0X3R5cGUiOiAidGVycml0b3JpZXMiLCAidmFsdWUiOiBbInRlcnJpdG9yeTEiXX0sIHsidHJhaXRfdHlwZSI6ICJkaXN0cmlidXRpb25DaGFubmVscyIsICJ2YWx1ZSI6IFsiZGlzdHJpYnV0aW9uQ2hhbm5lbDEiXX1dfQ=="; + memory expectedJson = "eyJuYW1lIjogIlN0b3J5IFByb3RvY29sIExpY2Vuc2UgTkZUIiwiZGVzY3JpcHRpb24iOiAiTGljZW5zZSBhZ3JlZW1lbnQgc3RhdGluZyB0aGUgdGVybXMgb2YgYSBTdG9yeSBQcm90b2NvbCBJUEFzc2V0IiwiZXh0ZXJuYWxfdXJsIjogImh0dHBzOi8vc3Rvcnlwcm90b2NvbC54eXoiLCJpbWFnZSI6ICJodHRwczovL2ltYWdlcy5jdGZhc3NldHMubmV0LzVlaTN3eDU0dDFkcC8xV1hPSG5QTFJPc0dpQnNJNDZ6RUNlLzRmMzhhOTVjNThkM2IwMzI5YWYzMDg1YjM2ZDcyMGM4L1N0b3J5X1Byb3RvY29sX0ljb24ucG5nIiwiYXR0cmlidXRlcyI6IFt7InRyYWl0X3R5cGUiOiAiQXR0cmlidXRpb24iLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJUcmFuc2ZlcmFibGUiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkNvbW1lcmljYWwgVXNlIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiQ29tbWVyY2lhbCBBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkNvbW1lcmNpYWwgUmV2ZW51ZSBTaGFyZSIsICJtYXhfdmFsdWUiOiAxMDAwLCAidmFsdWUiOiAwfSx7InRyYWl0X3R5cGUiOiAiQ29tbWVyY2lhbGl6ZXIgQ2hlY2siLCAidmFsdWUiOiAiMHgyMTA1MDNjMzE4ODU1MjU5OTgzMjk4YmE1ODA1NWEzOGQ1ZmY2M2UwIn0seyJ0cmFpdF90eXBlIjogIkRlcml2YXRpdmVzIEFsbG93ZWQiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJEZXJpdmF0aXZlcyBBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkRlcml2YXRpdmVzIEFwcHJvdmFsIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiRGVyaXZhdGl2ZXMgUmVjaXByb2NhbCIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkRlcml2YXRpdmVzIFJldmVudWUgU2hhcmUiLCAibWF4X3ZhbHVlIjogMTAwMCwgInZhbHVlIjogMH0seyJ0cmFpdF90eXBlIjogIlRlcnJpdG9yaWVzIiwgInZhbHVlIjogWyJ0ZXJyaXRvcnkxIl19LHsidHJhaXRfdHlwZSI6ICJEaXN0cmlidXRpb24gQ2hhbm5lbHMiLCAidmFsdWUiOiBbImRpc3RyaWJ1dGlvbkNoYW5uZWwxIl19LHsidHJhaXRfdHlwZSI6ICJMaWNlbnNvciIsICJ2YWx1ZSI6ICIweDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMGJlZWYifSx7InRyYWl0X3R5cGUiOiAiUG9saWN5IEZyYW1ld29yayIsICJ2YWx1ZSI6ICIweGYxZTFkNzdjNTRlOWMyOGNjMWRhMmRiYzM3N2I0YTg1NzY1YzI1NDIifSx7InRyYWl0X3R5cGUiOiAiVHJhbnNmZXJhYmxlIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiUmV2b2tlZCIsICJ2YWx1ZSI6ICJmYWxzZSJ9XX0="; /* solhint-enable */ string memory expectedUri = string(abi.encodePacked("data:application/json;base64,", expectedJson)); From 68cc7c58afd6ed47366b44115d1529afa14202dc Mon Sep 17 00:00:00 2001 From: Jongwon Park Date: Thu, 15 Feb 2024 18:44:35 -0600 Subject: [PATCH 09/14] style: solhint --- contracts/lib/Licensing.sol | 2 +- .../licensing/UMLPolicyFrameworkManager.sol | 100 ++++++++++-------- contracts/registries/LicenseRegistry.sol | 2 +- 3 files changed, 57 insertions(+), 47 deletions(-) diff --git a/contracts/lib/Licensing.sol b/contracts/lib/Licensing.sol index 9b6b756fa..c572637f1 100644 --- a/contracts/lib/Licensing.sol +++ b/contracts/lib/Licensing.sol @@ -16,7 +16,7 @@ library Licensing { } /// @notice Data that define a License Agreement NFT - /// @param policyId Id of the policy this license is based on, which will be set in the derivative IP when the + /// @param policyId Id of the policy this license is based on, which will be set in the derivative IP when the /// license is burnt for linking /// @param licensorIpId Id of the IP this license is for /// @param transferable Whether or not the license is transferable diff --git a/contracts/modules/licensing/UMLPolicyFrameworkManager.sol b/contracts/modules/licensing/UMLPolicyFrameworkManager.sol index 85a54a96c..2e6e3e61f 100644 --- a/contracts/modules/licensing/UMLPolicyFrameworkManager.sol +++ b/contracts/modules/licensing/UMLPolicyFrameworkManager.sol @@ -276,17 +276,19 @@ contract UMLPolicyFrameworkManager is uint256 count = policy.territories.length; for (uint256 i = 0; i < count; ++i) { json = string(abi.encodePacked(json, '"', policy.territories[i], '"')); - if (i != count - 1) { // skip comma for last element in the array + if (i != count - 1) { + // skip comma for last element in the array json = string(abi.encodePacked(json, ",")); } } - json = string(abi.encodePacked(json, ']},')); // close the trait_type: "Territories" array + json = string(abi.encodePacked(json, "]},")); // close the trait_type: "Territories" array json = string(abi.encodePacked(json, '{"trait_type": "Distribution Channels", "value": [')); count = policy.distributionChannels.length; for (uint256 i = 0; i < count; ++i) { json = string(abi.encodePacked(json, '"', policy.distributionChannels[i], '"')); - if (i != count - 1) { // skip comma for last element in the array + if (i != count - 1) { + // skip comma for last element in the array json = string(abi.encodePacked(json, ",")); } } @@ -301,53 +303,61 @@ contract UMLPolicyFrameworkManager is /// @notice Encodes the commercial traits of UML policy into a JSON string for OpenSea function _policyCommercialTraitsToJson(UMLPolicy memory policy) internal pure returns (string memory) { - // NOTE: TOTAL_RNFT_SUPPLY = 1000 in trait with max_value. For numbers, don't add any display_type, so that + /* solhint-disable */ + // NOTE: TOTAL_RNFT_SUPPLY = 1000 in trait with max_value. For numbers, don't add any display_type, so that // they will show up in the "Ranking" section of the OpenSea UI. - return string( - abi.encodePacked( - '{"trait_type": "Attribution", "value": "', - policy.attribution ? "true" : "false", - '"},', - // Skip transferable, it's already added in the common attributes by the LicenseRegistry. - // Should be managed by the LicenseRegistry, not the PFM. - '{"trait_type": "Commerical Use", "value": "', - policy.commercialUse ? "true" : "false", - '"},', - '{"trait_type": "Commercial Attribution", "value": "', - policy.commercialAttribution ? "true" : "false", - '"},', - '{"trait_type": "Commercial Revenue Share", "max_value": 1000, "value": ', - policy.commercialRevShare.toString(), - "},", - '{"trait_type": "Commercializer Check", "value": "', - policy.commercializerChecker.toHexString(), - // Skip on commercializerCheckerData as it's bytes as irrelevant for the user metadata - '"},' - ) - ); + return + string( + abi.encodePacked( + '{"trait_type": "Attribution", "value": "', + policy.attribution ? "true" : "false", + '"},', + // Skip transferable, it's already added in the common attributes by the LicenseRegistry. + // Should be managed by the LicenseRegistry, not the PFM. + '{"trait_type": "Commerical Use", "value": "', + policy.commercialUse ? "true" : "false", + '"},', + '{"trait_type": "Commercial Attribution", "value": "', + policy.commercialAttribution ? "true" : "false", + '"},', + '{"trait_type": "Commercial Revenue Share", "max_value": 1000, "value": ', + policy.commercialRevShare.toString(), + "},", + '{"trait_type": "Commercializer Check", "value": "', + policy.commercializerChecker.toHexString(), + // Skip on commercializerCheckerData as it's bytes as irrelevant for the user metadata + '"},' + ) + ); + /* solhint-enable */ } /// @notice Encodes the derivative traits of UML policy into a JSON string for OpenSea function _policyDerivativeTraitsToJson(UMLPolicy memory policy) internal pure returns (string memory) { - return string( - abi.encodePacked( - '{"trait_type": "Derivatives Allowed", "value": "', - policy.derivativesAllowed ? "true" : "false", - '"},', - '{"trait_type": "Derivatives Attribution", "value": "', - policy.derivativesAttribution ? "true" : "false", - '"},', - '{"trait_type": "Derivatives Approval", "value": "', - policy.derivativesApproval ? "true" : "false", - '"},', - '{"trait_type": "Derivatives Reciprocal", "value": "', - policy.derivativesReciprocal ? "true" : "false", - '"},', - '{"trait_type": "Derivatives Revenue Share", "max_value": 1000, "value": ', - policy.derivativesRevShare.toString(), - "}," - ) - ); + /* solhint-disable */ + // NOTE: TOTAL_RNFT_SUPPLY = 1000 in trait with max_value. For numbers, don't add any display_type, so that + // they will show up in the "Ranking" section of the OpenSea UI. + return + string( + abi.encodePacked( + '{"trait_type": "Derivatives Allowed", "value": "', + policy.derivativesAllowed ? "true" : "false", + '"},', + '{"trait_type": "Derivatives Attribution", "value": "', + policy.derivativesAttribution ? "true" : "false", + '"},', + '{"trait_type": "Derivatives Approval", "value": "', + policy.derivativesApproval ? "true" : "false", + '"},', + '{"trait_type": "Derivatives Reciprocal", "value": "', + policy.derivativesReciprocal ? "true" : "false", + '"},', + '{"trait_type": "Derivatives Revenue Share", "max_value": 1000, "value": ', + policy.derivativesRevShare.toString(), + "}," + ) + ); + /* solhint-enable */ } /// Checks the configuration of commercial use and throws if the policy is not compliant diff --git a/contracts/registries/LicenseRegistry.sol b/contracts/registries/LicenseRegistry.sol index 49e0faabf..5c047face 100644 --- a/contracts/registries/LicenseRegistry.sol +++ b/contracts/registries/LicenseRegistry.sol @@ -160,7 +160,7 @@ contract LicenseRegistry is ILicenseRegistry, ERC1155, Governable { ); // append the policy specific attributes (last attribute added by PFM should have a comma at the end) - // TODO: Safeguard mechanism to make sure the attributes added by PFM do NOT overlap with the common traits + // TODO: Safeguard mechanism to make sure the attributes added by PFM do NOT overlap with the common traits // defined above. Currently, we add the common license attributes after adding the PFM attributes to override. // But OpenSea might take the value of the first duplicate. json = string(abi.encodePacked(json, IPolicyFrameworkManager(pol.policyFramework).policyToJson(pol.data))); From 2bd6b7f120a3a8fac478411efa4638df95fc7513 Mon Sep 17 00:00:00 2001 From: Jongwon Park Date: Thu, 15 Feb 2024 21:10:11 -0600 Subject: [PATCH 10/14] fix: metadata external url --- contracts/registries/LicenseRegistry.sol | 8 ++++++-- test/foundry/modules/licensing/LicensingModule.t.sol | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/contracts/registries/LicenseRegistry.sol b/contracts/registries/LicenseRegistry.sol index 5c047face..6f9db8ca0 100644 --- a/contracts/registries/LicenseRegistry.sol +++ b/contracts/registries/LicenseRegistry.sol @@ -143,6 +143,8 @@ contract LicenseRegistry is ILicenseRegistry, ERC1155, Governable { Licensing.License memory licenseData = _licenses[id]; Licensing.Policy memory pol = _licensingModule.policy(licenseData.policyId); + string memory licensorIpIdHex = licenseData.licensorIpId.toHexString(); + /* solhint-disable */ // Follows the OpenSea standard for JSON metadata @@ -152,7 +154,9 @@ contract LicenseRegistry is ILicenseRegistry, ERC1155, Governable { "{", '"name": "Story Protocol License NFT",', '"description": "License agreement stating the terms of a Story Protocol IPAsset",', - '"external_url": "https://storyprotocol.xyz",', + '"external_url": "https://protocol.storyprotocol.xyz/ipa/', + licensorIpIdHex, + '",', // solhint-disable-next-line max-length '"image": "https://images.ctfassets.net/5ei3wx54t1dp/1WXOHnPLROsGiBsI46zECe/4f38a95c58d3b0329af3085b36d720c8/Story_Protocol_Icon.png",', '"attributes": [' @@ -170,7 +174,7 @@ contract LicenseRegistry is ILicenseRegistry, ERC1155, Governable { abi.encodePacked( json, '{"trait_type": "Licensor", "value": "', - licenseData.licensorIpId.toHexString(), + licensorIpIdHex, '"},', '{"trait_type": "Policy Framework", "value": "', pol.policyFramework.toHexString(), diff --git a/test/foundry/modules/licensing/LicensingModule.t.sol b/test/foundry/modules/licensing/LicensingModule.t.sol index d22c70416..50699d68b 100644 --- a/test/foundry/modules/licensing/LicensingModule.t.sol +++ b/test/foundry/modules/licensing/LicensingModule.t.sol @@ -491,7 +491,7 @@ contract LicensingModuleTest is Test { expectedJson = { "name": "Story Protocol License NFT", "description": "License agreement stating the terms of a Story Protocol IPAsset", - "external_url": "https://storyprotocol.xyz", + "external_url": "https://protocol.storyprotocol.xyz/ipa/0x000000000000000000000000000000000000beef", "image": "https://images.ctfassets.net/5ei3wx54t1dp/1WXOHnPLROsGiBsI46zECe/4f38a95c58d3b0329af3085b36d720c8/Story_Protocol_Icon.png", "attributes": [ { @@ -579,7 +579,7 @@ contract LicensingModuleTest is Test { /* solhint-disable */ string - memory expectedJson = "eyJuYW1lIjogIlN0b3J5IFByb3RvY29sIExpY2Vuc2UgTkZUIiwiZGVzY3JpcHRpb24iOiAiTGljZW5zZSBhZ3JlZW1lbnQgc3RhdGluZyB0aGUgdGVybXMgb2YgYSBTdG9yeSBQcm90b2NvbCBJUEFzc2V0IiwiZXh0ZXJuYWxfdXJsIjogImh0dHBzOi8vc3Rvcnlwcm90b2NvbC54eXoiLCJpbWFnZSI6ICJodHRwczovL2ltYWdlcy5jdGZhc3NldHMubmV0LzVlaTN3eDU0dDFkcC8xV1hPSG5QTFJPc0dpQnNJNDZ6RUNlLzRmMzhhOTVjNThkM2IwMzI5YWYzMDg1YjM2ZDcyMGM4L1N0b3J5X1Byb3RvY29sX0ljb24ucG5nIiwiYXR0cmlidXRlcyI6IFt7InRyYWl0X3R5cGUiOiAiQXR0cmlidXRpb24iLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJUcmFuc2ZlcmFibGUiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkNvbW1lcmljYWwgVXNlIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiQ29tbWVyY2lhbCBBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkNvbW1lcmNpYWwgUmV2ZW51ZSBTaGFyZSIsICJtYXhfdmFsdWUiOiAxMDAwLCAidmFsdWUiOiAwfSx7InRyYWl0X3R5cGUiOiAiQ29tbWVyY2lhbGl6ZXIgQ2hlY2siLCAidmFsdWUiOiAiMHgyMTA1MDNjMzE4ODU1MjU5OTgzMjk4YmE1ODA1NWEzOGQ1ZmY2M2UwIn0seyJ0cmFpdF90eXBlIjogIkRlcml2YXRpdmVzIEFsbG93ZWQiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJEZXJpdmF0aXZlcyBBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkRlcml2YXRpdmVzIEFwcHJvdmFsIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiRGVyaXZhdGl2ZXMgUmVjaXByb2NhbCIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkRlcml2YXRpdmVzIFJldmVudWUgU2hhcmUiLCAibWF4X3ZhbHVlIjogMTAwMCwgInZhbHVlIjogMH0seyJ0cmFpdF90eXBlIjogIlRlcnJpdG9yaWVzIiwgInZhbHVlIjogWyJ0ZXJyaXRvcnkxIl19LHsidHJhaXRfdHlwZSI6ICJEaXN0cmlidXRpb24gQ2hhbm5lbHMiLCAidmFsdWUiOiBbImRpc3RyaWJ1dGlvbkNoYW5uZWwxIl19LHsidHJhaXRfdHlwZSI6ICJMaWNlbnNvciIsICJ2YWx1ZSI6ICIweDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMGJlZWYifSx7InRyYWl0X3R5cGUiOiAiUG9saWN5IEZyYW1ld29yayIsICJ2YWx1ZSI6ICIweGYxZTFkNzdjNTRlOWMyOGNjMWRhMmRiYzM3N2I0YTg1NzY1YzI1NDIifSx7InRyYWl0X3R5cGUiOiAiVHJhbnNmZXJhYmxlIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiUmV2b2tlZCIsICJ2YWx1ZSI6ICJmYWxzZSJ9XX0="; + memory expectedJson = "eyJuYW1lIjogIlN0b3J5IFByb3RvY29sIExpY2Vuc2UgTkZUIiwiZGVzY3JpcHRpb24iOiAiTGljZW5zZSBhZ3JlZW1lbnQgc3RhdGluZyB0aGUgdGVybXMgb2YgYSBTdG9yeSBQcm90b2NvbCBJUEFzc2V0IiwiZXh0ZXJuYWxfdXJsIjogImh0dHBzOi8vcHJvdG9jb2wuc3Rvcnlwcm90b2NvbC54eXovaXBhLzB4MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwYmVlZiIsImltYWdlIjogImh0dHBzOi8vaW1hZ2VzLmN0ZmFzc2V0cy5uZXQvNWVpM3d4NTR0MWRwLzFXWE9IblBMUk9zR2lCc0k0NnpFQ2UvNGYzOGE5NWM1OGQzYjAzMjlhZjMwODViMzZkNzIwYzgvU3RvcnlfUHJvdG9jb2xfSWNvbi5wbmciLCJhdHRyaWJ1dGVzIjogW3sidHJhaXRfdHlwZSI6ICJBdHRyaWJ1dGlvbiIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIlRyYW5zZmVyYWJsZSIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkF0dHJpYnV0aW9uIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiQ29tbWVyaWNhbCBVc2UiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJDb21tZXJjaWFsIEF0dHJpYnV0aW9uIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiQ29tbWVyY2lhbCBSZXZlbnVlIFNoYXJlIiwgIm1heF92YWx1ZSI6IDEwMDAsICJ2YWx1ZSI6IDB9LHsidHJhaXRfdHlwZSI6ICJDb21tZXJjaWFsaXplciBDaGVjayIsICJ2YWx1ZSI6ICIweDIxMDUwM2MzMTg4NTUyNTk5ODMyOThiYTU4MDU1YTM4ZDVmZjYzZTAifSx7InRyYWl0X3R5cGUiOiAiRGVyaXZhdGl2ZXMgQWxsb3dlZCIsICJ2YWx1ZSI6ICJ0cnVlIn0seyJ0cmFpdF90eXBlIjogIkRlcml2YXRpdmVzIEF0dHJpYnV0aW9uIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiRGVyaXZhdGl2ZXMgQXBwcm92YWwiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJEZXJpdmF0aXZlcyBSZWNpcHJvY2FsIiwgInZhbHVlIjogInRydWUifSx7InRyYWl0X3R5cGUiOiAiRGVyaXZhdGl2ZXMgUmV2ZW51ZSBTaGFyZSIsICJtYXhfdmFsdWUiOiAxMDAwLCAidmFsdWUiOiAwfSx7InRyYWl0X3R5cGUiOiAiVGVycml0b3JpZXMiLCAidmFsdWUiOiBbInRlcnJpdG9yeTEiXX0seyJ0cmFpdF90eXBlIjogIkRpc3RyaWJ1dGlvbiBDaGFubmVscyIsICJ2YWx1ZSI6IFsiZGlzdHJpYnV0aW9uQ2hhbm5lbDEiXX0seyJ0cmFpdF90eXBlIjogIkxpY2Vuc29yIiwgInZhbHVlIjogIjB4MDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwYmVlZiJ9LHsidHJhaXRfdHlwZSI6ICJQb2xpY3kgRnJhbWV3b3JrIiwgInZhbHVlIjogIjB4ZjFlMWQ3N2M1NGU5YzI4Y2MxZGEyZGJjMzc3YjRhODU3NjVjMjU0MiJ9LHsidHJhaXRfdHlwZSI6ICJUcmFuc2ZlcmFibGUiLCAidmFsdWUiOiAidHJ1ZSJ9LHsidHJhaXRfdHlwZSI6ICJSZXZva2VkIiwgInZhbHVlIjogImZhbHNlIn1dfQ=="; /* solhint-enable */ string memory expectedUri = string(abi.encodePacked("data:application/json;base64,", expectedJson)); From 2d696698248c2fa98cebe773462178d7d39f120c Mon Sep 17 00:00:00 2001 From: Jongwon Park Date: Thu, 15 Feb 2024 21:23:29 -0600 Subject: [PATCH 11/14] nit: comment --- contracts/modules/dispute-module/DisputeModule.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contracts/modules/dispute-module/DisputeModule.sol b/contracts/modules/dispute-module/DisputeModule.sol index 192bf0396..67ae874fb 100644 --- a/contracts/modules/dispute-module/DisputeModule.sol +++ b/contracts/modules/dispute-module/DisputeModule.sol @@ -228,7 +228,8 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar if (dispute.currentTag == IN_DISPUTE) revert Errors.DisputeModule__NotAbleToResolve(); if (msg.sender != dispute.disputeInitiator) revert Errors.DisputeModule__NotDisputeInitiator(); - // We ignore the result of remove() + // Ignore the result of remove(), resolveDispute can only be called once when there's a dispute tag. + // Once resolveDispute is called, the tag will be removed and calling this fn again will throw an error. _taggedIpIds[dispute.targetIpId].remove(dispute.currentTag); disputes[_disputeId].currentTag = bytes32(0); From add40cd96037778c8bdcd9492d553a6fdbf52344 Mon Sep 17 00:00:00 2001 From: Raul Date: Fri, 16 Feb 2024 15:07:23 -0800 Subject: [PATCH 12/14] using counter to check if tagged --- .../modules/dispute/IDisputeModule.sol | 16 -------- .../modules/dispute-module/DisputeModule.sol | 38 ++++--------------- .../mocks/module/MockDisputeModule.sol | 16 -------- .../modules/dispute/DisputeModule.t.sol | 18 --------- 4 files changed, 7 insertions(+), 81 deletions(-) diff --git a/contracts/interfaces/modules/dispute/IDisputeModule.sol b/contracts/interfaces/modules/dispute/IDisputeModule.sol index b95850d2b..6137f35e2 100644 --- a/contracts/interfaces/modules/dispute/IDisputeModule.sol +++ b/contracts/interfaces/modules/dispute/IDisputeModule.sol @@ -156,24 +156,8 @@ interface IDisputeModule { bytes32 currentTag // The current tag of the dispute ); - /// @notice returns true if the ipId is tagged with the tag (meaning the dispute went through) - /// @param _ipId The ipId - /// @param _tag The tag - function isIpTaggedWith(address _ipId, bytes32 _tag) external view returns (bool); - /// @notice returns true if the ipId is tagged with any tag (meaning at least one dispute went through) /// @param _ipId The ipId function isIpTagged(address _ipId) external view returns (bool); - /// @notice returns the tags for a given ipId (note: this method could be expensive, use in frontends only) - /// @param _ipId The ipId - function ipTags(address _ipId) external view returns (bytes32[] memory); - - /// @notice returns the total tags for a given ipId - /// @param _ipId The ipId - function totalTagsForIp(address _ipId) external view returns (uint256); - - /// @notice returns the tag at a given index for a given ipId. No guarantees on ordering - /// @param _ipId The ipId - function tagForIpAt(address _ipId, uint256 _index) external view returns (bytes32); } diff --git a/contracts/modules/dispute-module/DisputeModule.sol b/contracts/modules/dispute-module/DisputeModule.sol index 67ae874fb..525ebdca5 100644 --- a/contracts/modules/dispute-module/DisputeModule.sol +++ b/contracts/modules/dispute-module/DisputeModule.sol @@ -56,7 +56,10 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar /// @notice Arbitration policy for a given ipId mapping(address ipId => address arbitrationPolicy) public arbitrationPolicies; - mapping(address ipId => EnumerableSet.Bytes32Set) private _taggedIpIds; + /// @notice counter of successful disputes per ipId + /// @dev BETA feature, for mainnet tag semantics must influence effect in other modules. For now + /// any successful dispute will affect the IP protocol wide + mapping(address ipId => uint256) private successfulDisputesPerIp; /// @notice Initializes the registration module contract /// @param _controller The access controller used for IP authorization @@ -193,8 +196,7 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar if (_decision) { disputes[_disputeId].currentTag = dispute.targetTag; - // We ignore the result of add(), we don't care if the tag is already there - _taggedIpIds[dispute.targetIpId].add(dispute.targetTag); + successfulDisputesPerIp[dispute.targetIpId]++; } else { disputes[_disputeId].currentTag = bytes32(0); } @@ -228,43 +230,17 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar if (dispute.currentTag == IN_DISPUTE) revert Errors.DisputeModule__NotAbleToResolve(); if (msg.sender != dispute.disputeInitiator) revert Errors.DisputeModule__NotDisputeInitiator(); - // Ignore the result of remove(), resolveDispute can only be called once when there's a dispute tag. - // Once resolveDispute is called, the tag will be removed and calling this fn again will throw an error. - _taggedIpIds[dispute.targetIpId].remove(dispute.currentTag); + successfulDisputesPerIp[dispute.targetIpId]--; disputes[_disputeId].currentTag = bytes32(0); emit DisputeResolved(_disputeId); } - /// @notice returns true if the ipId is tagged with the tag (meaning the dispute went through) - /// @param _ipId The ipId - /// @param _tag The tag - function isIpTaggedWith(address _ipId, bytes32 _tag) external view returns (bool) { - return _taggedIpIds[_ipId].contains(_tag); - } /// @notice returns true if the ipId is tagged with any tag (meaning at least one dispute went through) /// @param _ipId The ipId function isIpTagged(address _ipId) external view returns (bool) { - return _taggedIpIds[_ipId].length() > 0; - } - - /// @notice returns the tags for a given ipId (note: this method could be expensive, use in frontends only) - /// @param _ipId The ipId - function ipTags(address _ipId) external view returns (bytes32[] memory) { - return _taggedIpIds[_ipId].values(); - } - - /// @notice returns the total tags for a given ipId - /// @param _ipId The ipId - function totalTagsForIp(address _ipId) external view returns (uint256) { - return _taggedIpIds[_ipId].length(); - } - - /// @notice returns the tag at a given index for a given ipId. No guarantees on ordering - /// @param _ipId The ipId - function tagForIpAt(address _ipId, uint256 _index) external view returns (bytes32) { - return _taggedIpIds[_ipId].at(_index); + return successfulDisputesPerIp[_ipId] > 0; } /// @notice Gets the protocol-wide module identifier for this module diff --git a/test/foundry/mocks/module/MockDisputeModule.sol b/test/foundry/mocks/module/MockDisputeModule.sol index 32b586791..3bb5499c7 100644 --- a/test/foundry/mocks/module/MockDisputeModule.sol +++ b/test/foundry/mocks/module/MockDisputeModule.sol @@ -123,24 +123,8 @@ contract MockDisputeModule is BaseModule, IDisputeModule { // These methods are not really used in the mock. They are just here to satisfy the interface. - function isIpTaggedWith(address, bytes32) external pure returns (bool) { - return false; - } - function isIpTagged(address) external pure returns (bool) { return false; } - function ipTags(address) external pure returns (bytes32[] memory) { - bytes32[] memory tags; - return tags; - } - - function totalTagsForIp(address) external pure returns (uint256) { - return 0; - } - - function tagForIpAt(address, uint256) external pure returns (bytes32) { - return bytes32(0); - } } diff --git a/test/foundry/modules/dispute/DisputeModule.t.sol b/test/foundry/modules/dispute/DisputeModule.t.sol index a5ce9e9d0..f3ed5b92a 100644 --- a/test/foundry/modules/dispute/DisputeModule.t.sol +++ b/test/foundry/modules/dispute/DisputeModule.t.sol @@ -391,13 +391,7 @@ contract DisputeModuleTest is BaseTest { assertEq(arbitrationPolicySPUSDCBalanceBefore - arbitrationPolicySPUSDCBalanceAfter, ARBITRATION_PRICE); assertEq(currentTagBefore, bytes32("IN_DISPUTE")); assertEq(currentTagAfter, bytes32("PLAGIARISM")); - assertTrue(disputeModule.isIpTaggedWith(ipAddr, bytes32("PLAGIARISM"))); assertTrue(disputeModule.isIpTagged(ipAddr)); - bytes32[] memory ipTags = new bytes32[](1); - ipTags[0] = bytes32("PLAGIARISM"); - assertEq(keccak256(abi.encode(disputeModule.ipTags(ipAddr))), keccak256(abi.encode(ipTags))); - assertEq(disputeModule.totalTagsForIp(ipAddr), 1); - assertEq(disputeModule.tagForIpAt(ipAddr, 0), bytes32("PLAGIARISM")); } function test_DisputeModule_PolicySP_setDisputeJudgement_False() public { @@ -426,11 +420,7 @@ contract DisputeModuleTest is BaseTest { assertEq(arbitrationPolicySPUSDCBalanceBefore - arbitrationPolicySPUSDCBalanceAfter, 0); assertEq(currentTagBefore, bytes32("IN_DISPUTE")); assertEq(currentTagAfter, bytes32(0)); - assertFalse(disputeModule.isIpTaggedWith(ipAddr, bytes32("PLAGIARISM"))); assertFalse(disputeModule.isIpTagged(ipAddr)); - bytes32[] memory ipTags = new bytes32[](0); - assertEq(keccak256(abi.encode(disputeModule.ipTags(ipAddr))), keccak256(abi.encode(ipTags))); - assertEq(disputeModule.totalTagsForIp(ipAddr), 0); } function test_DisputeModule_PolicySP_cancelDispute_revert_NotDisputeInitiator() public { @@ -469,11 +459,7 @@ contract DisputeModuleTest is BaseTest { assertEq(currentTagBeforeCancel, bytes32("IN_DISPUTE")); assertEq(currentTagAfterCancel, bytes32(0)); - assertFalse(disputeModule.isIpTaggedWith(ipAddr, bytes32("PLAGIARISM"))); assertFalse(disputeModule.isIpTagged(ipAddr)); - bytes32[] memory ipTags = new bytes32[](0); - assertEq(keccak256(abi.encode(disputeModule.ipTags(ipAddr))), keccak256(abi.encode(ipTags))); - assertEq(disputeModule.totalTagsForIp(ipAddr), 0); } function test_DisputeModule_resolveDispute_revert_NotDisputeInitiator() public { @@ -518,11 +504,7 @@ contract DisputeModuleTest is BaseTest { assertEq(currentTagBeforeResolve, bytes32("PLAGIARISM")); assertEq(currentTagAfterResolve, bytes32(0)); - assertFalse(disputeModule.isIpTaggedWith(ipAddr, bytes32("PLAGIARISM"))); assertFalse(disputeModule.isIpTagged(ipAddr)); - bytes32[] memory ipTags = new bytes32[](0); - assertEq(keccak256(abi.encode(disputeModule.ipTags(ipAddr))), keccak256(abi.encode(ipTags))); - assertEq(disputeModule.totalTagsForIp(ipAddr), 0); } function test_DisputeModule_name() public { From 3bcab65bc895445aa199ce745bd31aa044eda8c1 Mon Sep 17 00:00:00 2001 From: Raul Date: Fri, 16 Feb 2024 15:38:57 -0800 Subject: [PATCH 13/14] added check to not decrement dispute counter on resolved disputes --- contracts/modules/dispute-module/DisputeModule.sol | 3 +-- test/foundry/modules/dispute/DisputeModule.t.sol | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/contracts/modules/dispute-module/DisputeModule.sol b/contracts/modules/dispute-module/DisputeModule.sol index 525ebdca5..22d0c63be 100644 --- a/contracts/modules/dispute-module/DisputeModule.sol +++ b/contracts/modules/dispute-module/DisputeModule.sol @@ -226,8 +226,7 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar /// @param _disputeId The dispute id function resolveDispute(uint256 _disputeId) external { Dispute memory dispute = disputes[_disputeId]; - - if (dispute.currentTag == IN_DISPUTE) revert Errors.DisputeModule__NotAbleToResolve(); + if (dispute.currentTag == IN_DISPUTE || dispute.currentTag == bytes32(0)) revert Errors.DisputeModule__NotAbleToResolve(); if (msg.sender != dispute.disputeInitiator) revert Errors.DisputeModule__NotDisputeInitiator(); successfulDisputesPerIp[dispute.targetIpId]--; diff --git a/test/foundry/modules/dispute/DisputeModule.t.sol b/test/foundry/modules/dispute/DisputeModule.t.sol index f3ed5b92a..33a8707d4 100644 --- a/test/foundry/modules/dispute/DisputeModule.t.sol +++ b/test/foundry/modules/dispute/DisputeModule.t.sol @@ -477,6 +477,11 @@ contract DisputeModuleTest is BaseTest { vm.startPrank(ipAccount1); vm.expectRevert(Errors.DisputeModule__NotAbleToResolve.selector); disputeModule.resolveDispute(1); + + // Also if the dispute is already resolved or non-existent + vm.startPrank(ipAccount1); + vm.expectRevert(Errors.DisputeModule__NotAbleToResolve.selector); + disputeModule.resolveDispute(2); } function test_DisputeModule_resolveDispute() public { From fa53a6ae1f2028ae6a5e0270614646dfa1be4df2 Mon Sep 17 00:00:00 2001 From: Raul Date: Fri, 16 Feb 2024 15:58:07 -0800 Subject: [PATCH 14/14] fix test --- contracts/modules/dispute-module/DisputeModule.sol | 3 ++- test/foundry/modules/dispute/DisputeModule.t.sol | 9 +++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/contracts/modules/dispute-module/DisputeModule.sol b/contracts/modules/dispute-module/DisputeModule.sol index 22d0c63be..d8fbf242f 100644 --- a/contracts/modules/dispute-module/DisputeModule.sol +++ b/contracts/modules/dispute-module/DisputeModule.sol @@ -226,8 +226,9 @@ contract DisputeModule is IDisputeModule, BaseModule, Governable, ReentrancyGuar /// @param _disputeId The dispute id function resolveDispute(uint256 _disputeId) external { Dispute memory dispute = disputes[_disputeId]; - if (dispute.currentTag == IN_DISPUTE || dispute.currentTag == bytes32(0)) revert Errors.DisputeModule__NotAbleToResolve(); + if (msg.sender != dispute.disputeInitiator) revert Errors.DisputeModule__NotDisputeInitiator(); + if (dispute.currentTag == IN_DISPUTE || dispute.currentTag == bytes32(0)) revert Errors.DisputeModule__NotAbleToResolve(); successfulDisputesPerIp[dispute.targetIpId]--; disputes[_disputeId].currentTag = bytes32(0); diff --git a/test/foundry/modules/dispute/DisputeModule.t.sol b/test/foundry/modules/dispute/DisputeModule.t.sol index 33a8707d4..7414ed1b1 100644 --- a/test/foundry/modules/dispute/DisputeModule.t.sol +++ b/test/foundry/modules/dispute/DisputeModule.t.sol @@ -478,10 +478,6 @@ contract DisputeModuleTest is BaseTest { vm.expectRevert(Errors.DisputeModule__NotAbleToResolve.selector); disputeModule.resolveDispute(1); - // Also if the dispute is already resolved or non-existent - vm.startPrank(ipAccount1); - vm.expectRevert(Errors.DisputeModule__NotAbleToResolve.selector); - disputeModule.resolveDispute(2); } function test_DisputeModule_resolveDispute() public { @@ -510,6 +506,11 @@ contract DisputeModuleTest is BaseTest { assertEq(currentTagBeforeResolve, bytes32("PLAGIARISM")); assertEq(currentTagAfterResolve, bytes32(0)); assertFalse(disputeModule.isIpTagged(ipAddr)); + + // Cant resolve again + vm.expectRevert(Errors.DisputeModule__NotAbleToResolve.selector); + disputeModule.resolveDispute(1); + vm.stopPrank(); } function test_DisputeModule_name() public {