Fixes around license derivatives

contracts/interfaces/modules/licensing/IUMLPolicyFrameworkManager.sol

@@ -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
@@ -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;

contracts/lib/UMLFrameworkErrors.sol

@@ -21,6 +21,5 @@ library UMLFrameworkErrors {
  error UMLPolicyFrameworkManager__ReciprocalButDifferentPolicyIds();
  error UMLPolicyFrameworkManager__ReciprocalValueMismatch();
  error UMLPolicyFrameworkManager__CommercialValueMismatch();
- error UMLPolicyFrameworkManager__DerivativesValueMismatch();
  error UMLPolicyFrameworkManager__StringArrayMismatch();
 }

contracts/modules/licensing/UMLPolicyFrameworkManager.sol

@@ -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(
@@ -85,23 +84,24 @@ 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;
  }
 
@@ -123,19 +123,15 @@ contract UMLPolicyFrameworkManager is
  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 policy was inherited (so this is a derivative) we don't allow minting
+ if (!policy.derivativesReciprocal && policyWasInherited) {
  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;
  }
@@ -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)),
@@ -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,

test/foundry/modules/licensing/UMLPolicyFramework.derivation.t.sol

@@ -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();
  }
 
  /////////////////////////////////////////////////////////////////
@@ -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);

test/foundry/modules/licensing/UMLPolicyFramework.multi-parent.sol

@@ -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({