Skip to content

Commit

Permalink
Return reused id instead of reverting on registerPolicy (storyprotoco…
Browse files Browse the repository at this point in the history 
…l#88)

* instead of failing on registerPolicy on duplicate, return existing id. Add getter

* fix

---------

Co-authored-by: Raul <[email protected]>
  • Loading branch information
@Ramarti
Ramarti and Raul authored Feb 8, 2024
1 parent f97ce66 commit b6500f3
Show file tree
Hide file tree
Showing 8 changed files with 158 additions and 110 deletions.
3 changes: 3 additions & 0 deletions contracts/interfaces/modules/licensing/ILicensingModule.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ interface ILicensingModule is IModule {
/// @param data The policy data
function registerPolicy(bool isLicenseTransferable, bytes memory data) external returns (uint256 policyId);

/// @notice returns the policy id for the given data, or 0 if not found
function getPolicyId(address framework, bool isLicenseTransferable, bytes memory data) external view returns (uint256 policyId);

/// @notice Adds a policy to an IP policy list
/// @param ipId The id of the IP
/// @param polId The id of the policy
Expand Down
3 changes: 3 additions & 0 deletions contracts/interfaces/modules/licensing/IUMLPolicyFrameworkManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ interface IUMLPolicyFrameworkManager is IPolicyFrameworkManager {
/// @return policy The UMLPolicy struct
function getPolicy(uint256 policyId) external view returns (UMLPolicy memory policy);

/// @notice gets the policy ID for the given policy data, or 0 if not found
function getPolicyId(UMLPolicy calldata umlPolicy) external view returns (uint256 policyId);

/// @notice gets the aggregation data for inherited policies.
function getAggregator(address ipId) external view returns (UMLAggregator memory rights);
}
1 change: 0 additions & 1 deletion contracts/lib/Errors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,6 @@ library Errors {
error LicensingModule__FrameworkNotFound();
error LicensingModule__EmptyLicenseUrl();
error LicensingModule__InvalidPolicyFramework();
error LicensingModule__PolicyAlreadyAdded();
error LicensingModule__ParamVerifierLengthMismatch();
error LicensingModule__PolicyNotFound();
error LicensingModule__NotLicensee();
Expand Down
218 changes: 113 additions & 105 deletions contracts/modules/licensing/LicensingModule.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,32 @@ contract LicensingModule is AccessControlled, ILicensingModule {
emit PolicyFrameworkRegistered(manager, fwManager.name(), licenseUrl);
}

/// @notice Registers a policy into the contract. MUST be called by a registered
/// framework or it will revert. The policy data and its integrity must be
/// verified by the policy framework manager.
/// @param isLicenseTransferable True if the license is transferable
/// @param data The policy data
function registerPolicy(bool isLicenseTransferable, bytes memory data) external returns (uint256 policyId) {
_verifyRegisteredFramework(address(msg.sender));
Licensing.Policy memory pol = Licensing.Policy({
isLicenseTransferable: isLicenseTransferable,
policyFramework: msg.sender,
data: data
});
(uint256 polId, bool newPol) = DataUniqueness.addIdOrGetExisting(
abi.encode(pol),
_hashedPolicies,
_totalPolicies
);

if (newPol) {
_totalPolicies = polId;
_policies[polId] = pol;
emit PolicyRegistered(msg.sender, polId, data);
}
return polId;
}

/// Adds a policy to an ipId, which can be used to mint licenses.
/// Licnses are permissions for ipIds to be derivatives (children).
/// if policyId is not defined in LicenseRegistry, reverts.
Expand Down Expand Up @@ -119,35 +145,6 @@ contract LicensingModule is AccessControlled, ILicensingModule {
}
}

/// @notice Registers a policy into the contract. MUST be called by a registered
/// framework or it will revert. The policy data and its integrity must be
/// verified by the policy framework manager.
/// @param isLicenseTransferable True if the license is transferable
/// @param data The policy data
function registerPolicy(bool isLicenseTransferable, bytes memory data) external returns (uint256 policyId) {
_verifyRegisteredFramework(address(msg.sender));
Licensing.Policy memory pol = Licensing.Policy({
isLicenseTransferable: isLicenseTransferable,
policyFramework: msg.sender,
data: data
});
(uint256 polId, bool newPol) = DataUniqueness.addIdOrGetExisting(
abi.encode(pol),
_hashedPolicies,
_totalPolicies
);

if (!newPol) {
revert Errors.LicensingModule__PolicyAlreadyAdded();
} else {
_totalPolicies = polId;
_policies[polId] = pol;
emit PolicyRegistered(msg.sender, polId, data);
}

return polId;
}

/// Mints license NFTs representing a policy granted by a set of ipIds (licensors). This NFT needs to be burned
/// in order to link a derivative IP with its parents.
/// If this is the first combination of policy and licensors, a new licenseId
Expand Down Expand Up @@ -299,82 +296,6 @@ contract LicensingModule is AccessControlled, ILicensingModule {
LICENSE_REGISTRY.burnLicenses(holder, licenseIds);
}

function _linkIpToParent(
uint256 iteration,
uint256 licenseId,
uint256 policyId,
address licensor,
address childIpId,
address royaltyPolicyAddress,
uint32 royaltyDerivativeRevShare,
uint32 derivativeRevShareSum
)
private
returns (
address nextRoyaltyPolicyAddress,
uint32 nextRoyaltyDerivativeRevShare,
uint32 nextDerivativeRevShareSum
)
{
// TODO: check licensor not part of a branch tagged by disputer
if (licensor == childIpId) {
revert Errors.LicensingModule__ParentIdEqualThanChild();
}
// Verify linking params
Licensing.Policy memory pol = policy(policyId);
IPolicyFrameworkManager.VerifyLinkResponse memory response = IPolicyFrameworkManager(pol.policyFramework)
.verifyLink(licenseId, msg.sender, childIpId, licensor, pol.data);

if (!response.isLinkingAllowed) {
revert Errors.LicensingModule__LinkParentParamFailed();
}

// Compatibility check: If link says no royalty is required for license (licenseIds[i]) but
// another license requires royalty, revert.
if (!response.isRoyaltyRequired && royaltyPolicyAddress != address(0)) {
revert Errors.LicensingModule__IncompatibleLicensorCommercialPolicy();
}

// If link says royalty is required for license (licenseIds[i]) and no royalty policy is set, set it.
// But if the index is NOT 0, this is previous licenses didn't set the royalty policy because they don't
// require royalty payment. So, revert in this case. Similarly, if the new royaltyPolicyAddress is different
// from the previous one (in iteration > 0), revert. We currently restrict all licenses (parents) to have
// the same royalty policy, so the child can inherit it.
if (response.isRoyaltyRequired) {
if (iteration > 0 && royaltyPolicyAddress != response.royaltyPolicy) {
// If iteration > 0 and
// - royaltyPolicyAddress == address(0), revert. Previous licenses didn't set RP.
// - royaltyPolicyAddress != response.royaltyPolicy, revert. Previous licenses set different RP.
// ==> this can be considered as royaltyPolicyAddress != response.royaltyPolicy
revert Errors.LicensingModule__IncompatibleRoyaltyPolicyAddress();
}

// TODO: Unit test.
// If the previous license's derivativeRevShare is different from that of the current license, revert.
// For iteration == 0, this check is skipped as `royaltyDerivativeRevShare` param is at 0.
if (iteration > 0 && royaltyDerivativeRevShare != response.royaltyDerivativeRevShare) {
revert Errors.LicensingModule__IncompatibleRoyaltyPolicyDerivativeRevShare();
}

// TODO: Read max RNFT supply instead of hardcoding the expected max supply
// TODO: Do we need safe check?
// TODO: Test this in unit test.
if (derivativeRevShareSum + response.royaltyDerivativeRevShare > 1000) {
revert Errors.LicensingModule__DerivativeRevShareSumExceedsMaxRNFTSupply();
}

nextRoyaltyPolicyAddress = response.royaltyPolicy;
nextRoyaltyDerivativeRevShare = response.royaltyDerivativeRevShare;
nextDerivativeRevShareSum = derivativeRevShareSum + response.royaltyDerivativeRevShare;
}

// Add the policy of licenseIds[i] to the child. If the policy's already set from previous parents,
// then the addition will be skipped.
_addPolicyIdToIp({ ipId: childIpId, policyId: policyId, isInherited: true, skipIfDuplicate: true });
// Set parent
_ipIdParents[childIpId].add(licensor);
}

/// @notice True if the framework address is registered in LicenseRegistry
function isFrameworkRegistered(address policyFramework) external view returns (bool) {
return _registeredFrameworkManagers[policyFramework];
Expand All @@ -392,6 +313,16 @@ contract LicensingModule is AccessControlled, ILicensingModule {
return pol;
}

/// @notice gets the policy id for the given data, or 0 if not found
function getPolicyId(address framework, bool isLicenseTransferable, bytes memory data) external view returns (uint256 policyId) {
Licensing.Policy memory pol = Licensing.Policy({
isLicenseTransferable: isLicenseTransferable,
policyFramework: framework,
data: data
});
return _hashedPolicies[keccak256(abi.encode(pol))];
}

function policyAggregatorData(address framework, address ipId) external view returns (bytes memory) {
return _ipRights[framework][ipId];
}
Expand Down Expand Up @@ -498,6 +429,83 @@ contract LicensingModule is AccessControlled, ILicensingModule {
return index;
}

function _linkIpToParent(
uint256 iteration,
uint256 licenseId,
uint256 policyId,
address licensor,
address childIpId,
address royaltyPolicyAddress,
uint32 royaltyDerivativeRevShare,
uint32 derivativeRevShareSum
)
private
returns (
address nextRoyaltyPolicyAddress,
uint32 nextRoyaltyDerivativeRevShare,
uint32 nextDerivativeRevShareSum
)
{
// TODO: check licensor not part of a branch tagged by disputer
if (licensor == childIpId) {
revert Errors.LicensingModule__ParentIdEqualThanChild();
}
// Verify linking params
Licensing.Policy memory pol = policy(policyId);
IPolicyFrameworkManager.VerifyLinkResponse memory response = IPolicyFrameworkManager(pol.policyFramework)
.verifyLink(licenseId, msg.sender, childIpId, licensor, pol.data);

if (!response.isLinkingAllowed) {
revert Errors.LicensingModule__LinkParentParamFailed();
}

// Compatibility check: If link says no royalty is required for license (licenseIds[i]) but
// another license requires royalty, revert.
if (!response.isRoyaltyRequired && royaltyPolicyAddress != address(0)) {
revert Errors.LicensingModule__IncompatibleLicensorCommercialPolicy();
}

// If link says royalty is required for license (licenseIds[i]) and no royalty policy is set, set it.
// But if the index is NOT 0, this is previous licenses didn't set the royalty policy because they don't
// require royalty payment. So, revert in this case. Similarly, if the new royaltyPolicyAddress is different
// from the previous one (in iteration > 0), revert. We currently restrict all licenses (parents) to have
// the same royalty policy, so the child can inherit it.
if (response.isRoyaltyRequired) {
if (iteration > 0 && royaltyPolicyAddress != response.royaltyPolicy) {
// If iteration > 0 and
// - royaltyPolicyAddress == address(0), revert. Previous licenses didn't set RP.
// - royaltyPolicyAddress != response.royaltyPolicy, revert. Previous licenses set different RP.
// ==> this can be considered as royaltyPolicyAddress != response.royaltyPolicy
revert Errors.LicensingModule__IncompatibleRoyaltyPolicyAddress();
}

// TODO: Unit test.
// If the previous license's derivativeRevShare is different from that of the current license, revert.
// For iteration == 0, this check is skipped as `royaltyDerivativeRevShare` param is at 0.
if (iteration > 0 && royaltyDerivativeRevShare != response.royaltyDerivativeRevShare) {
revert Errors.LicensingModule__IncompatibleRoyaltyPolicyDerivativeRevShare();
}

// TODO: Read max RNFT supply instead of hardcoding the expected max supply
// TODO: Do we need safe check?
// TODO: Test this in unit test.
if (derivativeRevShareSum + response.royaltyDerivativeRevShare > 1000) {
revert Errors.LicensingModule__DerivativeRevShareSumExceedsMaxRNFTSupply();
}

nextRoyaltyPolicyAddress = response.royaltyPolicy;
nextRoyaltyDerivativeRevShare = response.royaltyDerivativeRevShare;
nextDerivativeRevShareSum = derivativeRevShareSum + response.royaltyDerivativeRevShare;
}

// Add the policy of licenseIds[i] to the child. If the policy's already set from previous parents,
// then the addition will be skipped.
_addPolicyIdToIp({ ipId: childIpId, policyId: policyId, isInherited: true, skipIfDuplicate: true });
// Set parent
_ipIdParents[childIpId].add(licensor);
}


function _verifyCanAddPolicy(uint256 policyId, address ipId, bool isInherited) private {
bool ipIdIsDerivative = _policySetPerIpId(true, ipId).length() > 0;
if (
Expand Down
4 changes: 4 additions & 0 deletions contracts/modules/licensing/UMLPolicyFrameworkManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,10 @@ contract UMLPolicyFrameworkManager is IUMLPolicyFrameworkManager, BasePolicyFram
rights = abi.decode(policyAggregatorData, (UMLAggregator));
}

function getPolicyId(UMLPolicy calldata umlPolicy) external view returns (uint256 policyId) {
return LICENSING_MODULE.getPolicyId(address(this), umlPolicy.transferable, abi.encode(umlPolicy));
}

function getRoyaltyPolicy(uint256 policyId) external view returns (address) {
return getPolicy(policyId).royaltyPolicy;
}
Expand Down
2 changes: 2 additions & 0 deletions test/foundry/mocks/licensing/MockLicensingModule.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ contract MockLicensingModule is ILicensingModule {

function isPolicyIdSetForIp(bool isInherited, address ipId, uint256 policyId) external view returns (bool) {}

function getPolicyId(address framework, bool isLicenseTransferable, bytes memory data) external view returns (uint256 policyId) {}

function policyIdForIpAtIndex(
bool isInherited,
address ipId,
Expand Down
15 changes: 11 additions & 4 deletions test/foundry/modules/licensing/LicensingModule.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,22 @@ contract LicensingModuleTest is Test {
assertEq(polId, 1, "polId not 1");
}

function test_LicensingModule_registerPolicy_revert_policyAlreadyAdded() public {
function test_LicensingModule_registerPolicy_reusesIdForAlreadyAddedPolicy() public {
licensingModule.registerPolicyFrameworkManager(address(module1));
vm.startPrank(address(module1));
licensingModule.registerPolicy(true, _createPolicy());
vm.expectRevert(Errors.LicensingModule__PolicyAlreadyAdded.selector);
licensingModule.registerPolicy(true, _createPolicy());
uint256 polId = licensingModule.registerPolicy(true, _createPolicy());
assertEq(polId, licensingModule.registerPolicy(true, _createPolicy()));
vm.stopPrank();
}

function test_LicensingModule_getPolicyId() public {
licensingModule.registerPolicyFrameworkManager(address(module1));
bytes memory policy = _createPolicy();
vm.prank(address(module1));
uint256 policyId = licensingModule.registerPolicy(true, policy);
assertEq(licensingModule.getPolicyId(address(module1), true, policy), policyId, "policyId not found");
}

function test_LicensingModule_registerPolicy_revert_frameworkNotFound() public {
bytes memory policy = _createPolicy();
vm.expectRevert(Errors.LicensingModule__FrameworkNotFound.selector);
Expand Down
22 changes: 22 additions & 0 deletions test/foundry/modules/licensing/UMLPolicyFramework.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,28 @@ contract UMLPolicyFrameworkTest is TestHelper {
ipId2 = ipAccountRegistry.registerIpAccount(block.chainid, address(nft), 2);
}

function test_UMLPolicyFrameworkManager_getPolicyId() public {
UMLPolicy memory umlPolicy = UMLPolicy({
transferable: true,
attribution: true,
commercialUse: false,
commercialAttribution: false,
commercializers: emptyStringArray,
commercialRevShare: 0,
derivativesAllowed: false,
derivativesAttribution: false,
derivativesApproval: false,
derivativesReciprocal: false,
derivativesRevShare: 0,
territories: emptyStringArray,
distributionChannels: emptyStringArray,
contentRestrictions: emptyStringArray,
royaltyPolicy: address(0)
});
uint256 policyId = umlFramework.registerPolicy(umlPolicy);
assertEq(umlFramework.getPolicyId(umlPolicy), policyId);
}

function test_UMLPolicyFrameworkManager__valuesSetCorrectly() public {
string[] memory territories = new string[](2);
territories[0] = "test1";
Expand Down

0 comments on commit b6500f3

Please sign in to comment.