Remove & Rename Errors and Correct Names (storyprotocol#112)

* nit: Remove unused code in library

* fix: Error typo & remove unused errors, variable name change

* fix: more error comment and name correction

* nit: comments

* nit: Error comments

* refactor: ShortStringOps directory

contracts/IPAccountImpl.sol

@@ -37,7 +37,7 @@ contract IPAccountImpl is IPAccountStorage, IIPAccount {
         address licenseRegistry,
         address moduleRegistry
     ) IPAccountStorage(ipAssetRegistry, licenseRegistry, moduleRegistry) {
-        if (accessController == address(0)) revert Errors.IPAccount__InvalidAccessController();
+        if (accessController == address(0)) revert Errors.IPAccount__ZeroAccessController();
         ACCESS_CONTROLLER = accessController;
     }
 

contracts/access/AccessController.sol

@@ -65,7 +65,7 @@ contract AccessController is IAccessController, ProtocolPausableUpgradeable, UUP
     }
 
     /// @notice Sets the addresses of the IP account registry and the module registry
-    /// @dev TODO: figure out how to set these addresses in the constructor to make them immutable
+    /// @dev TODO: Set these addresses in the constructor to make them immutable
     /// @param ipAccountRegistry address of the IP account registry
     /// @param moduleRegistry address of the module registry
     function setAddresses(address ipAccountRegistry, address moduleRegistry) external restricted {
@@ -75,10 +75,9 @@ contract AccessController is IAccessController, ProtocolPausableUpgradeable, UUP
     }
 
     /// @notice Sets a batch of permissions in a single transaction.
-    /// @dev This function allows setting multiple permissions at once. Pausable.
+    /// @dev This function allows setting multiple permissions at once. Pausable via setPermission.
     /// @param permissions An array of `Permission` structs, each representing the permission to be set.
     function setBatchPermissions(AccessPermission.Permission[] memory permissions) external {
-        // TODO: removed pause.
         for (uint256 i = 0; i < permissions.length; ) {
             setPermission(
                 permissions[i].ipAccount,

contracts/interfaces/registries/ILicenseRegistry.sol

@@ -64,8 +64,6 @@ interface ILicenseRegistry {
     /// @return Whether the IP has derivative IPs.
     function hasDerivativeIps(address ipId) external view returns (bool);
 
-    // TODO: getDerivativeIpCount
-
     /// @notice Verifies the minting of a license token.
     /// @param licensorIpId The address of the licensor IP.
     /// @param licenseTemplate The address of the license template where the license terms are defined.

contracts/lib/PILicenseTemplateErrors.sol

@@ -4,27 +4,42 @@ pragma solidity 0.8.23;
 /// @title PILicenseTemplate Errors Library
 /// @notice Library for all PILicenseTemplate related contract errors.
 library PILicenseTemplateErrors {
-    ////////////////////////////////////////////////////////////////////////////
-    //                         PILicenseTemplate                      //
-    ////////////////////////////////////////////////////////////////////////////
+    /// @notice Zero address provided for Access Manager at initialization.
+    error PILicenseTemplate__ZeroAccessManager();
 
+    /// @notice Cannot add commercializers when commercial use is disabled.
     error PILicenseTemplate__CommercialDisabled_CantAddCommercializers();
+
+    /// @notice Provided commercializer does not support IHookModule.
     error PILicenseTemplate__CommercializerCheckerDoesNotSupportHook(address checker);
+
+    /// @notice PIL terms royalty policy is not whitelisted by the Royalty Module.
     error PILicenseTemplate__RoyaltyPolicyNotWhitelisted();
+
+    /// @notice PIL terms currency token is not whitelisted by the Royalty Module.
     error PILicenseTemplate__CurrencyTokenNotWhitelisted();
+
+    /// @notice Royalty policy requires a currency token.
     error PILicenseTemplate__RoyaltyPolicyRequiresCurrencyToken();
+
+    /// @notice Cannot add commercial attribution when commercial use is disabled.
     error PILicenseTemplate__CommercialDisabled_CantAddAttribution();
+
+    /// @notice Cannot add commercial revenue share when commercial use is disabled.
     error PILicenseTemplate__CommercialDisabled_CantAddRevShare();
+
+    /// @notice Cannot add commercial royalty policy when commercial use is disabled.
+    error PILicenseTemplate__CommercialDisabled_CantAddRoyaltyPolicy();
+
+    /// @notice Royalty policy is required when commercial use is enabled.
+    error PILicenseTemplate__CommercialEnabled_RoyaltyPolicyRequired();
+
+    /// @notice Cannot add derivative attribution when derivative use is disabled.
     error PILicenseTemplate__DerivativesDisabled_CantAddAttribution();
+
+    /// @notice Cannot add derivative approval when derivative use is disabled.
     error PILicenseTemplate__DerivativesDisabled_CantAddApproval();
+
+    /// @notice Cannot add derivative reciprocal when derivative use is disabled.
     error PILicenseTemplate__DerivativesDisabled_CantAddReciprocal();
-    error PILicenseTemplate__LicenseTermsNotFound();
-    error PILicenseTemplate__CommercialDisabled_CantAddRoyaltyPolicy();
-    error PILicenseTemplate__CommercialEnabled_RoyaltyPolicyRequired();
-    error PILicenseTemplate__ReciprocalButDifferentPolicyIds();
-    error PILicenseTemplate__ReciprocalValueMismatch();
-    error PILicenseTemplate__CommercialValueMismatch();
-    error PILicenseTemplate__StringArrayMismatch();
-    error PILicenseTemplate__CommercialDisabled_CantAddMintingFee();
-    error PILicenseTemplate__CommercialDisabled_CantAddMintingFeeToken();
 }

contracts/lib/ShortStringOps.sol

@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: BUSL-1.1
+pragma solidity 0.8.23;
+
+import { ShortString, ShortStrings } from "@openzeppelin/contracts/utils/ShortStrings.sol";
+import { Strings } from "@openzeppelin/contracts/utils/Strings.sol";
+
+/// @notice Library for working with Openzeppelin's ShortString data types.
+library ShortStringOps {
+    using ShortStrings for *;
+    using Strings for *;
+
+    /// @dev Convert string to bytes32 using ShortString
+    function stringToBytes32(string memory s) internal pure returns (bytes32) {
+        return ShortString.unwrap(s.toShortString());
+    }
+}

contracts/modules/dispute/DisputeModule.sol

@@ -14,7 +14,7 @@ import { ILicenseRegistry } from "../../interfaces/registries/ILicenseRegistry.s
 import { IDisputeModule } from "../../interfaces/modules/dispute/IDisputeModule.sol";
 import { IArbitrationPolicy } from "../../interfaces/modules/dispute/policies/IArbitrationPolicy.sol";
 import { Errors } from "../../lib/Errors.sol";
-import { ShortStringOps } from "../../utils/ShortStringOps.sol";
+import { ShortStringOps } from "../../lib/ShortStringOps.sol";
 import { ProtocolPausableUpgradeable } from "../../pause/ProtocolPausableUpgradeable.sol";
 
 /// @title Dispute Module
@@ -71,20 +71,20 @@ contract DisputeModule is
     ILicenseRegistry public immutable LICENSE_REGISTRY;
 
     /// Constructor
-    /// @param controller The address of the access controller
-    /// @param assetRegistry The address of the asset registry
+    /// @param accessController The address of the access controller
+    /// @param ipAssetRegistry The address of the asset registry
     /// @param licenseRegistry The address of the license registry
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor(
-        address controller,
-        address assetRegistry,
+        address accessController,
+        address ipAssetRegistry,
         address licenseRegistry
-    ) AccessControlled(controller, assetRegistry) {
+    ) AccessControlled(accessController, ipAssetRegistry) {
         if (licenseRegistry == address(0)) revert Errors.DisputeModule__ZeroLicenseRegistry();
-        if (assetRegistry == address(0)) revert Errors.DisputeModule__ZeroAssetRegistry();
-        if (controller == address(0)) revert Errors.DisputeModule__ZeroController();
+        if (ipAssetRegistry == address(0)) revert Errors.DisputeModule__ZeroIPAssetRegistry();
+        if (accessController == address(0)) revert Errors.DisputeModule__ZeroAccessController();
 
-        IP_ASSET_REGISTRY = IIPAssetRegistry(assetRegistry);
+        IP_ASSET_REGISTRY = IIPAssetRegistry(ipAssetRegistry);
         LICENSE_REGISTRY = ILicenseRegistry(licenseRegistry);
         _disableInitializers();
     }

contracts/modules/licensing/PILicenseTemplate.sol

@@ -12,7 +12,6 @@ import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/
 import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 
 // contracts
-import { Errors } from "../../lib/Errors.sol";
 import { IHookModule } from "../../interfaces/modules/base/IHookModule.sol";
 import { ILicenseRegistry } from "../../interfaces/registries/ILicenseRegistry.sol";
 import { IRoyaltyModule } from "../../interfaces/modules/royalty/IRoyaltyModule.sol";
@@ -68,7 +67,7 @@ contract PILicenseTemplate is
     /// @param metadataURI The URL to the off chain metadata
     function initialize(address accessManager, string memory name, string memory metadataURI) external initializer {
         if (accessManager == address(0)) {
-            revert Errors.PILicenseTemplate__ZeroAccessManager();
+            revert PILicenseTemplateErrors.PILicenseTemplate__ZeroAccessManager();
         }
         __BaseLicenseTemplate_init(name, metadataURI);
         __AccessManaged_init(accessManager);

contracts/registries/IPAccountRegistry.sol

@@ -24,7 +24,7 @@ abstract contract IPAccountRegistry is IIPAccountRegistry {
 
     /// @custom:oz-upgrades-unsafe-allow constructor
     constructor(address erc6551Registry, address ipAccountImpl) {
-        if (ipAccountImpl == address(0)) revert Errors.IPAccountRegistry_InvalidIpAccountImpl();
+        if (ipAccountImpl == address(0)) revert Errors.IPAccountRegistry_ZeroIpAccountImpl();
         IP_ACCOUNT_IMPL = ipAccountImpl;
         IP_ACCOUNT_SALT = bytes32(0);
         ERC6551_PUBLIC_REGISTRY = erc6551Registry;

test/foundry/modules/dispute/DisputeModule.t.sol

@@ -10,7 +10,7 @@ import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/P
 import { Errors } from "contracts/lib/Errors.sol";
 import { IModule } from "contracts/interfaces/modules/base/IModule.sol";
 import { ArbitrationPolicySP } from "contracts/modules/dispute/policies/ArbitrationPolicySP.sol";
-import { ShortStringOps } from "contracts/utils/ShortStringOps.sol";
+import { ShortStringOps } from "contracts/lib/ShortStringOps.sol";
 import { IDisputeModule } from "contracts/interfaces/modules/dispute/IDisputeModule.sol";
 // test
 import { BaseTest } from "test/foundry/utils/BaseTest.t.sol";