Skip to content

Commit

Permalink
Fixes around license derivatives (storyprotocol#112)
Browse files Browse the repository at this point in the history 
---------

Co-authored-by: Leo Chen <[email protected]>
  • Loading branch information
@Ramarti @LeoHChen @kingster-will
2 people authored and kingster-will committed Mar 16, 2024
1 parent 1951ec6 commit 9df71c5
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 78 deletions.
4 changes: 2 additions & 2 deletions contracts/interfaces/modules/licensing/IPolicyFrameworkManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ interface IPolicyFrameworkManager is IERC165 {
/// @notice Verify policy parameters for minting a license.
/// @dev Enforced to be only callable by LicenseRegistry
/// @param caller the address executing the mint
/// @param policyWasInherited true if the policy was inherited (licensorIpId is not original IP owner)
/// @param mintingFromADerivative true if we verify minting a license from a derivative IP ID
/// @param receiver the address receiving the license
/// @param licensorIpId the IP id of the licensor
/// @param mintAmount the amount of licenses to mint
/// @param policyData the encoded framework policy data to verify
/// @return verified True if the link is verified
function verifyMint(
address caller,
bool policyWasInherited,
bool mintingFromADerivative,
address licensorIpId,
address receiver,
uint256 mintAmount,
Expand Down
2 changes: 0 additions & 2 deletions contracts/interfaces/modules/licensing/IUMLPolicyFrameworkManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ struct RegisterUMLPolicyParams {
/// new policies.
/// @dev The assumption is that new policies may be added later, not only when linking an IP to its parent.
/// @param commercial Whether or not there is a policy that allows commercial use
/// @param derivatives Whether or not there is a policy that allows derivatives
/// @param derivativesReciprocal Whether or not there is a policy that requires derivatives to be licensed under the
/// same terms
/// @param lastPolicyId The last policy ID that was added to the IP
Expand All @@ -58,7 +57,6 @@ struct RegisterUMLPolicyParams {
/// @param contentRestrictionsAcc The last hash of the contentRestrictions array
struct UMLAggregator {
bool commercial;
bool derivatives;
bool derivativesReciprocal;
uint256 lastPolicyId;
bytes32 territoriesAcc;
Expand Down
1 change: 0 additions & 1 deletion contracts/lib/UMLFrameworkErrors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,5 @@ library UMLFrameworkErrors {
error UMLPolicyFrameworkManager__ReciprocalButDifferentPolicyIds();
error UMLPolicyFrameworkManager__ReciprocalValueMismatch();
error UMLPolicyFrameworkManager__CommercialValueMismatch();
error UMLPolicyFrameworkManager__DerivativesValueMismatch();
error UMLPolicyFrameworkManager__StringArrayMismatch();
}
42 changes: 17 additions & 25 deletions contracts/modules/licensing/UMLPolicyFrameworkManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ contract UMLPolicyFrameworkManager is
_verifyComercialUse(params.policy, params.royaltyPolicy);
_verifyDerivatives(params.policy);
/// TODO: DO NOT deploy on production networks without hashing string[] values instead of storing them

// No need to emit here, as the LicensingModule will emit the event
return
LICENSING_MODULE.registerPolicy(
Expand Down Expand Up @@ -85,57 +84,54 @@ contract UMLPolicyFrameworkManager is
) external override nonReentrant onlyLicensingModule returns (bool) {
UMLPolicy memory policy = abi.decode(policyData, (UMLPolicy));

// Trying to burn a license to create a derivative, when the license doesn't allow derivatives.
if (!policy.derivativesAllowed) {
return false;
}

// If the policy defines the licensor must approve derivatives, check if the
// derivative is approved by the licensor
if (policy.derivativesApproval) {
return isDerivativeApproved(licenseId, ipId);
if (policy.derivativesApproval && !isDerivativeApproved(licenseId, ipId)) {
return false;
}
// Check if the commercializerChecker allows the link
if (policy.commercializerChecker != address(0)) {
if (!policy.commercializerChecker.supportsInterface(type(IHookModule).interfaceId)) {
revert Errors.PolicyFrameworkManager__CommercializerCheckerDoesNotSupportHook(
policy.commercializerChecker
);
}

// No need to check if the commercializerChecker supports the IHookModule interface, as it was checked
// when the policy was registered.
if (!IHookModule(policy.commercializerChecker).verify(caller, policy.commercializerCheckerData)) {
return false;
}
}

return true;
}

/// @notice Verify policy parameters for minting a license.
/// @dev Enforced to be only callable by LicenseRegistry
/// @param caller the address executing the mint
/// @param policyWasInherited true if the policy was inherited (licensorIpId is not original IP owner)
/// @param mintingFromADerivative true if the license is minting from a derivative IPA
/// @param licensorIpId the IP id of the licensor
/// @param receiver the address receiving the license
/// @param mintAmount the amount of licenses to mint
/// @param policyData the encoded framework policy data to verify
/// @return verified True if the link is verified
function verifyMint(
address caller,
bool policyWasInherited,
bool mintingFromADerivative,
address licensorIpId,
address receiver,
uint256 mintAmount,
bytes memory policyData
) external nonReentrant onlyLicensingModule returns (bool) {
UMLPolicy memory policy = abi.decode(policyData, (UMLPolicy));
// If the policy defines no derivative is allowed, and policy was inherited,
// we don't allow minting
if (!policy.derivativesAllowed && policyWasInherited) {
// If the policy defines no reciprocal derivatives are allowed (no derivatives of derivatives),
// and we are mintingFromADerivative we don't allow minting
if (!policy.derivativesReciprocal && mintingFromADerivative) {
return false;
}

if (policy.commercializerChecker != address(0)) {
if (!policy.commercializerChecker.supportsInterface(type(IHookModule).interfaceId)) {
revert Errors.PolicyFrameworkManager__CommercializerCheckerDoesNotSupportHook(
policy.commercializerChecker
);
}

// No need to check if the commercializerChecker supports the IHookModule interface, as it was checked
// when the policy was registered.
if (!IHookModule(policy.commercializerChecker).verify(caller, policy.commercializerCheckerData)) {
return false;
}
Expand Down Expand Up @@ -186,7 +182,6 @@ contract UMLPolicyFrameworkManager is
// Initialize the aggregator
agg = UMLAggregator({
commercial: newPolicy.commercialUse,
derivatives: newPolicy.derivativesAllowed,
derivativesReciprocal: newPolicy.derivativesReciprocal,
lastPolicyId: policyId,
territoriesAcc: keccak256(abi.encode(newPolicy.territories)),
Expand All @@ -211,9 +206,6 @@ contract UMLPolicyFrameworkManager is
if (agg.commercial != newPolicy.commercialUse) {
revert UMLFrameworkErrors.UMLPolicyFrameworkManager__CommercialValueMismatch();
}
if (agg.derivatives != newPolicy.derivativesAllowed) {
revert UMLFrameworkErrors.UMLPolicyFrameworkManager__DerivativesValueMismatch();
}

bytes32 newHash = _verifHashedParams(
agg.territoriesAcc,
Expand Down
35 changes: 24 additions & 11 deletions test/foundry/modules/licensing/UMLPolicyFramework.derivation.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,26 @@ contract UMLPolicyFrameworkCompatibilityTest is BaseTest {
vm.stopPrank();
}

function test_UMLPolicyFramework_originalWork_bobSetsPoliciesThenCompatibleParent()
/////////////////////////////////////////////////////////////////
////// LICENSES THAT DONT ALLOW DERIVATIVES //////
/////////////////////////////////////////////////////////////////

function test_UMLPolicyFramework_non_derivative_license()
public
withUMLPolicySimple("comm_deriv", true, true, false)
withUMLPolicySimple("comm_non_deriv", true, false, false)
withUMLPolicySimple("non_comm_no_deriv", true, false, false)
{
// TODO: This works if all policies compatible.
// Can bob disable some policies?
// Bob can add different policies on IP1 without compatibility checks.
vm.startPrank(bob);
uint256 licenseId1 = licensingModule.mintLicense(_getUmlPolicyId("non_comm_no_deriv"), ipId1, 2, alice, "");
assertEq(licenseRegistry.balanceOf(alice, licenseId1), 2, "dan doesn't have license1");
vm.stopPrank();

uint256[] memory licenseIds = new uint256[](1);
licenseIds[0] = licenseId1;
vm.expectRevert(Errors.LicensingModule__LinkParentParamFailed.selector);
vm.startPrank(alice);
licensingModule.linkIpToParents(licenseIds, ipId2, "");
vm.stopPrank();
}

/////////////////////////////////////////////////////////////////
Expand All @@ -151,23 +164,23 @@ contract UMLPolicyFrameworkCompatibilityTest is BaseTest {

function test_UMLPolicyFramework_derivative_revert_cantMintDerivativeOfDerivative()
public
withUMLPolicySimple("comm_non_deriv", true, false, false)
withAliceOwningDerivativeIp2("comm_non_deriv")
withUMLPolicySimple("comm_non_recip", true, true, false)
withAliceOwningDerivativeIp2("comm_non_recip")
{
vm.expectRevert(Errors.LicensingModule__MintLicenseParamFailed.selector);
vm.startPrank(dan);
licensingModule.mintLicense(_getUmlPolicyId("comm_non_deriv"), ipId2, 1, dan, "");
licensingModule.mintLicense(_getUmlPolicyId("comm_non_recip"), ipId2, 1, dan, "");

vm.expectRevert(Errors.LicensingModule__MintLicenseParamFailed.selector);
vm.startPrank(alice);
licensingModule.mintLicense(_getUmlPolicyId("comm_non_deriv"), ipId2, 1, alice, "");
licensingModule.mintLicense(_getUmlPolicyId("comm_non_recip"), ipId2, 1, alice, "");
}

function test_UMLPolicyFramework_derivative_revert_AliceCantSetPolicyOnDerivativeOfDerivative()
public
withUMLPolicySimple("comm_non_deriv", true, false, false)
withUMLPolicySimple("comm_non_recip", true, true, false)
withUMLPolicySimple("comm_deriv", true, true, false)
withAliceOwningDerivativeIp2("comm_non_deriv")
withAliceOwningDerivativeIp2("comm_non_recip")
{
vm.expectRevert(Errors.LicensingModule__DerivativesCannotAddPolicy.selector);
vm.prank(alice);
Expand Down
37 changes: 0 additions & 37 deletions test/foundry/modules/licensing/UMLPolicyFramework.multi-parent.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,43 +242,6 @@ contract UMLPolicyFrameworkMultiParentTest is BaseTest {
_testSuccessCompat(inputA, inputB, 2);
}

function test_UMLPolicyFramework_multiParent_revert_NonReciprocalDerivatives() public {
// First we create 2 policies.
_mapUMLPolicySimple({
name: "pol_a",
commercial: true,
derivatives: true,
reciprocal: false,
commercialRevShare: 100
});
RegisterUMLPolicyParams memory inputA = _getMappedUmlParams("pol_a");
_mapUMLPolicySimple({
name: "pol_b",
commercial: true,
derivatives: true,
reciprocal: false,
commercialRevShare: 100
});
RegisterUMLPolicyParams memory inputB = _getMappedUmlParams("pol_b");
// We set some indifferents
inputA.policy.attribution = true;
inputB.policy.attribution = !inputB.policy.attribution;
inputA.transferable = true;
inputB.transferable = !inputA.transferable;

// Derivatives (revert)
inputA.policy.derivativesAllowed = true;
inputB.policy.derivativesAllowed = !inputA.policy.derivativesAllowed;

// TODO: passing in two different royaltyPolicy addresses
// solhint-disable-next-line max-line-length
_testRevertCompat(
inputA,
inputB,
UMLFrameworkErrors.UMLPolicyFrameworkManager__DerivativesValueMismatch.selector
);
}

function test_UMLPolicyFramework_multiParent_NonReciprocalTerritories() public {
// First we create 2 policies.
_mapUMLPolicySimple({
Expand Down

0 comments on commit 9df71c5

Please sign in to comment.