Comment licensing

contracts/lib/modules/Licensing.sol

@@ -2,104 +2,147 @@
 // See https://github.com/storyprotocol/protocol-contracts/blob/main/StoryProtocol-AlphaTestingAgreement-17942166.3.pdf
 pragma solidity ^0.8.19;
 
-import { FixedSet } from "contracts/utils/FixedSet.sol";
 import { BitMask } from "contracts/lib/BitMask.sol";
 import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";
 import { ShortStrings, ShortString } from "@openzeppelin/contracts/utils/ShortStrings.sol";
 
 /// @title Licensing Module Library
-/// Structs needed by the Licensing Modules and registries
 library Licensing {
     using ShortStrings for *;
+    /// @notice Enum for the status of a license.
     enum LicenseStatus {
+        /// No status has been set.
         Unset,
+        /// The license is active. It can be linked with IPAs.
         Active,
+        /// The license is pending licensor approval. It can't be linked with IPAs until approved
+        /// by the licensor using the LicensingModule (activateLicense)
         PendingLicensorApproval,
+        /// The license has been revoked. It can't be linked with IPAs or sublicensed.
+        /// Sub-licenses will be considered non active.
         Revoked
     }
 
+    /// @notice Defines the types to encode/decode the parameters of a licensing framework.
     enum ParameterType {
+        /// Boolean value
         Bool,
+        /// Number value (18 decimals)
         Number,
+        /// Address value
         Address,
+        /// String value
         String,
+        /// Array of ShortString values
         ShortStringArray,
         // uint256 bitmask representing indexes in choices array. ParamDefinition will have the available choices array.
         MultipleChoice
     }
 
+    /// @notice Defines the configuration of the licensor for an IP org.
     enum LicensorConfig {
+        /// Null value.
         Unset,
+        /// Licensor is the IP org owner, for all licenses.
         IpOrgOwnerAlways,
+        /// Licensor will be:
+        /// - If parentLicense is provided, the licensee of the parent license.
+        /// - If parentLicense is not provided, the Owner of the linked IPA.
+        /// - If no parentLicense and no linked IPA, the IP org owner.
         Source
     }
 
+    /// @notice Data struct representing a license agreement minted according to an IP Org Licensing Framework
     struct LicenseData {
-        /// License status. // TODO: IPA status should follow
+        /// License status.
         LicenseStatus status;
+        /// If true, other licenses can set this one as parent license, false will prevent it.
         bool derivativesAllowed;
+        /// If true, sublicenses can be created by anyone, and the terms of this license will be
+        /// applied to the sublicenses. If false, sublicenses can only be created by the licensor.
         bool isReciprocal;
+        /// If true, sublicenses will be minted with LicenseStatus.PendingLicensorApproval.
         bool derivativeNeedsApproval;
+        /// Address with permission to revoke the license.
         address revoker;
         /// address granting the license
         address licensor;
         /// address of the ip org that produced the terms
         address ipOrg;
+        /// The id of the license framework used to mint this license
         ShortString frameworkId;
-        /// If the licensee is bound to an IPA, this is the IPA id. 0 otherwise
+        /// If the licensee is linked to an IPA, this is the IPA id. 0 otherwise
         uint256 ipaId;
-        /// The id of the parent license. 0 if this this is tied to the first IPA of an IPOrg
+        /// The id of the parent license, if any. 0 otherwise
         uint256 parentLicenseId;
     }
 
+    /// @notice Parameters to mint a license
     struct LicenseCreation {
+        /// Array of (tag, value) pairs for the parameters, corresponding to the tags in the
+        /// licensing framework.
         ParamValue[] params;
+        /// Parent license id, if any. 0 otherwise.
         uint256 parentLicenseId;
+        /// Linked IPA id, if any. 0 otherwise.
         uint256 ipaId;
     }
 
+    /// @notice Corresponds to a parameter (licensing term) of a licensing framework.
     struct ParamDefinition {
-        /// The parameter id
+        /// The parameter id, used to identify the parameter in the license agreement text.
         ShortString tag;
-        /// The type of the parameter, used to decode the value
+        /// The type of the parameter, used to encode/decode the value
         ParameterType paramType;
         /// Encoded according to paramType, might be empty.
         bytes defaultValue;
         /// If MultipleChoice, String[] of the available choices. Empty bytes otherwise.
         bytes availableChoices;
     }
 
+    /// @notice Corresponds to a value for parameter (licensing term) of a licensing framework.
     struct ParamValue {
+        /// The parameter id, used to identify the parameter in the license agreement text.
         ShortString tag;
+        /// Encoded according to paramType, might be empty.
         bytes value;
     }
 
-    struct FrameworkStorage {
-        string textUrl;
-        FixedSet.ShortStringSet paramTags;
-        ParamDefinition[] paramDefs;
-    }
-
+    /// @notice Parameters to set a licensing framework in LicensingFrameworkRepo
     struct SetFramework {
+        /// The id of the licensing framework.
         string id;
+        /// The URL to the license agreement text.
         string textUrl;
+        /// Array of parameter definitions, corresponding to the parameters in
+        /// the license agreement text.
         ParamDefinition[] paramDefs;
     }
 
+    /// @notice Parameters to set a licensing framework in an IP org
     struct LicensingConfig {
+        /// The id of the licensing framework.
         string frameworkId;
+        /// Array of (tag, value) pairs for the parameters, corresponding to the tags in the
+        /// licensing framework.
         ParamValue[] params;
+        /// Enum with the rules to determine the licensor for this IP org's licenses
         LicensorConfig licensor;
     }
 
-    uint256 internal constant MAX_PARAM_TAGS = 150;
+    /// @notice Maximum number of parameters allowed in a licensing framework.
+    uint256 public constant MAX_PARAM_TAGS = 255;
 
     /// Input for IpOrg legal terms configuration in LicensingModule (for now, the only option)
     bytes32 public constant LICENSING_FRAMEWORK_CONFIG = keccak256("LICENSING_FRAMEWORK_CONFIG");
+    /// Create license action in LicensingModule
     bytes32 public constant CREATE_LICENSE = keccak256("CREATE_LICENSE");
+    /// Activate license action in LicensingModule
     bytes32 public constant ACTIVATE_LICENSE = keccak256("ACTIVATE_LICENSE");
+    /// Revoke license action in LicensingModule
     bytes32 public constant LINK_LNFT_TO_IPA = keccak256("LINK_LNFT_TO_IPA");
 
+    /// @notice Returns the string representation of a license status.
     function _statusToString(LicenseStatus status_) internal pure returns (string memory) {
         if (status_ == LicenseStatus.Unset) {
             return "Unset";
@@ -113,6 +156,11 @@ library Licensing {
         return "Unknown";
     }
 
+    /// @notice decodes value from a MultipleChoice parameter into the corresponding ShortString[]
+    /// with the choices
+    /// @param value the encoded value
+    /// @param availableChoices the encoded available choices for the parameter, defined in parameter
+    /// definition
     function _decodeMultipleChoice(
         bytes memory value,
         bytes memory availableChoices
@@ -127,11 +175,21 @@ library Licensing {
         return result;
     }
 
+    /// @notice Sets the indexes for the chosen options in a MultipleChoice parameter,
+    /// and encodes it into bytes
+    /// @param choiceIndexes_ the indexes of the chosen options
+    /// @return value the encoded value
     function _encodeMultipleChoice(uint8[] memory choiceIndexes_) internal pure returns (bytes memory value) {
         uint256 mask = BitMask._convertToMask(choiceIndexes_);
         return abi.encode(mask);
     }
 
+    /// @notice Validates the value for a parameter, according to its type, meaning it will
+    /// decode the value and try to revert if it's not valid.
+    /// @dev WARNING: Some validations are not possible to do on chain (like string validation),
+    /// so they should be done off chain. Also, Boolean decoded as a Number will be valid.
+    /// @param paramDef_ the parameter definition
+    /// @param value_ the encoded value
     function _validateParamValue(ParamDefinition memory paramDef_, bytes memory value_) internal pure returns (bool) {
         // An empty value signals the parameter is untagged, to trigger default values in the
         // license agreement text, but that's valid
@@ -169,6 +227,10 @@ library Licensing {
         return true;
     }
 
+    /// @notice converts a ShortString[] to a string representation of a JSON array. Used for LicenseRegistry
+    /// metadata rendering.
+    /// @param ss the ShortString[]
+    /// @return the string representation of a JSON array
     function _shortStringArrayToJsonArray(ShortString[] memory ss) internal pure returns (string memory) {
         string memory result = "[";
         uint256 len = ss.length;
@@ -182,6 +244,11 @@ library Licensing {
         return string(abi.encodePacked(result, "]"));
     }
 
+    /// @notice decodes value from a parameter into a string representation that can be used
+    /// in LicenseRegistry metadata rendering.
+    /// @param paramDef_ the parameter definition
+    /// @param value_ the encoded value
+    /// @return the string representation of the value
     function _getDecodedParamString(
         Licensing.ParamDefinition memory paramDef_,
         bytes memory value_

contracts/modules/licensing/LicenseRegistry.sol

@@ -308,16 +308,16 @@ contract LicenseRegistry is ERC721 {
                 paramDef.paramType != Licensing.ParameterType.MultipleChoice &&
                 paramDef.paramType != Licensing.ParameterType.ShortStringArray
             ) {
-                value = string(abi.encodePacked("\"", value, "\"}")); // solhint-disable-line
+                value = string(abi.encodePacked('"', value, '"}')); // solhint-disable-line
             } else {
                 value = string(abi.encodePacked(value, "}"));
             }
             paramAttributes = string(
                 abi.encodePacked(
                     paramAttributes,
-                    "{\"trait_type\": \"", // solhint-disable-line
+                    '{"trait_type": "', // solhint-disable-line
                     params[i].tag.toString(),
-                    "\", \"value\": ", // solhint-disable-line
+                    '", "value": ', // solhint-disable-line
                     value
                 )
             );

contracts/modules/licensing/LicensingFrameworkRepo.sol

@@ -10,21 +10,45 @@ import { AccessControlled } from "contracts/access-control/AccessControlled.sol"
 import { AccessControl } from "contracts/lib/AccessControl.sol";
 import { FixedSet } from "contracts/utils/FixedSet.sol";
 
+/// @title Licensing Framework Repo
+/// @notice Stores licensing frameworks and their parameters. License Modules
+/// can use this repo to fetch the parameters of a framework in order encode and decode
+/// them when creating Licenses
 contract LicensingFrameworkRepo is AccessControlled, Multicall {
     using FixedSet for FixedSet.ShortStringSet;
     using ShortStrings for *;
 
+    /// @notice Struct for storing the parameters of a licensing framework
+    struct FrameworkStorage {
+        /// @notice URL to the legal document of the framework
+        string textUrl;
+        /// @notice The tags of the parameters of the framework.
+        FixedSet.ShortStringSet paramTags;
+        /// @notice The definitions of the parameters of the framework
+        Licensing.ParamDefinition[] paramDefs;
+    }
+
+    /// Emits when a new licensing framework is added
     event FrameworkAdded(string frameworkId, string textUrl);
 
+    /// emits when a new parameter is added to a framework
     event ParamDefinitionAdded(string frameworkId, ShortString tag, Licensing.ParamDefinition definition);
 
-    mapping(string => Licensing.FrameworkStorage) private _frameworks;
+    /// frameworkId => FrameworkStorage
+    mapping(string => FrameworkStorage) private _frameworks;
+    /// Hash of (frameworkId, tag) => ParamDefinition
     mapping(bytes32 => Licensing.ParamDefinition) private _frameworkDefs;
 
+    /// @notice Constructor for the repo
+    /// @param accessControl_ the address of the access control singleton contract
     constructor(address accessControl_) AccessControlled(accessControl_) {}
 
+    /// @notice Adds a new licensing framework to the repo
+    /// @dev this is an admin only function, and can only be called by the
+    /// licensing manager role
+    /// @param input_ the input parameters for the framework
     function addFramework(Licensing.SetFramework calldata input_) external onlyRole(AccessControl.LICENSING_MANAGER) {
-        Licensing.FrameworkStorage storage framework = _frameworks[input_.id];
+        FrameworkStorage storage framework = _frameworks[input_.id];
         if (framework.paramTags.length() > 0) {
             revert Errors.LicensingFrameworkRepo_FrameworkAlreadyAdded();
         }
@@ -39,8 +63,11 @@ contract LicensingFrameworkRepo is AccessControlled, Multicall {
         emit FrameworkAdded(input_.id, input_.textUrl);
     }
 
+    /// @notice Adds a new parameter to a licensing framework
+    /// @param frameworkId_ the ID of the framework
+    /// @param paramDef_ the definition of the parameter
     function _addParameter(string calldata frameworkId_, Licensing.ParamDefinition calldata paramDef_) internal {
-        Licensing.FrameworkStorage storage framework = _frameworks[frameworkId_];
+        FrameworkStorage storage framework = _frameworks[frameworkId_];
         ShortString tag = paramDef_.tag;
         if (framework.paramTags.contains(tag)) {
             revert Errors.LicensingFrameworkRepo_DuplicateParamType();
@@ -51,29 +78,48 @@ contract LicensingFrameworkRepo is AccessControlled, Multicall {
         emit ParamDefinitionAdded(frameworkId_, tag, paramDef_);
     }
 
+    /// Gets the URL to the legal document of a licensing framework
     function getLicenseTextUrl(string calldata frameworkId_) external view returns (string memory) {
         return _frameworks[frameworkId_].textUrl;
     }
 
+    /// Gets the definition of a parameter of a licensing framework at a
+    /// given index
+    /// @param frameworkId_ the ID of the framework
+    /// @param index the index of the parameter
+    /// @return the definition of the parameter
     function getParamDefinitionAt(
         string calldata frameworkId_,
         uint256 index
     ) external view returns (Licensing.ParamDefinition memory) {
-        Licensing.FrameworkStorage storage framework = _frameworks[frameworkId_];
+        FrameworkStorage storage framework = _frameworks[frameworkId_];
         return framework.paramDefs[index];
     }
 
+    /// Gets the amount of parameters of a licensing framework
+    /// @param frameworkId_ the ID of the framework
+    /// @return the amount of parameters
     function getTotalParameters(string calldata frameworkId_) external view returns (uint256) {
         return _frameworks[frameworkId_].paramDefs.length;
     }
 
+    /// Gets the definition of a parameter of a licensing framework for a
+    /// given tag
+    /// @param frameworkId_ the ID of the framework
+    /// @param tag_ the tag of the parameter
+    /// @return the definition of the parameter
     function getParamDefinition(
         string calldata frameworkId_,
         ShortString tag_
     ) external view returns (Licensing.ParamDefinition memory) {
         return _frameworkDefs[keccak256(abi.encode(frameworkId_, tag_))];
     }
 
+    /// Gets all the parameter definitions of a licensing framework
+    /// @dev Warning: this function can be expensive if the framework has
+    /// many parameters
+    /// @param frameworkId_ the ID of the framework
+    /// @return the definitions of the parameters
     function getParameterDefs(string calldata frameworkId_) external view returns (Licensing.ParamDefinition[] memory) {
         return _frameworks[frameworkId_].paramDefs;
     }