Merge branch 'main' into release-v0.1.0_franchise_license_config_script

.github/workflows/test.yml

@@ -6,6 +6,7 @@ env:
   FOUNDRY_PROFILE: ci
 
 jobs:
+
   foundry-test:
     strategy:
       fail-fast: true
@@ -16,15 +17,16 @@ jobs:
       - uses: actions/checkout@v3
         with:
           submodules: recursive
+          fetch-depth: 0
 
       - name: List files in the repository
         run: |
-          ls ${{ github.workspace }}
-
-      - uses: chill-viking/npm-ci@latest
-        name: Install NPM Dependencies
+          ls -R ${{ github.workspace }}
+  
+      - name: Run install
+        uses: borales/actions-yarn@v4
         with:
-          working_directory: ${{ github.workspace }}
+          cmd: install # will run `yarn install` command
 
       - name: Install Foundry
         uses: foundry-rs/foundry-toolchain@v1
@@ -34,7 +36,7 @@ jobs:
       - name: Run Forge build
         run: |
           forge --version
-          forge build --sizes
+          forge build --force --sizes
         id: build
 
       - name: Run Forge tests
@@ -67,4 +69,4 @@ jobs:
       - name: Test
         uses: ambersun1234/[email protected]
         with:
-          network: hardhat
+          network: hardhat

.prettierrc

@@ -2,5 +2,6 @@
     "trailingComma": "es5",
     "tabWidth": 4,
     "semi": false,
-    "singleQuote": false
+    "singleQuote": false,
+    "bracketSpacing": true
 }

.solhint.json

@@ -3,18 +3,18 @@
   "plugins": ["prettier"],
   "rules": {
     "code-complexity": ["error", 8],
-    "compiler-version": ["error", ">=0.5.8"],
+    "compiler-version": ["error", ">=0.8.0"],
     "const-name-snakecase": "off",
     "constructor-syntax": "error",
     "func-visibility": ["error", { "ignoreConstructors": true }],
     "max-line-length": ["error", 120],
     "not-rely-on-time": "off",
-    "prettier/prettier": [
-      "error",
-      {
-        "endOfLine": "auto"
-      }
-    ],
-    "reason-string": ["warn", { "maxLength": 64 }]
+    "reason-string": ["warn", { "maxLength": 64 }],
+    "no-unused-import": "error",
+    "no-unused-vars": "error",
+    "no-inline-assembly": "off",
+    "avoid-low-level-calls": "off",
+    "no-global-import": "error",
+    "prettier/prettier": "error"
   }
 }

GUIDELINES.md

@@ -18,6 +18,13 @@ The test suite coverage must be kept as close to 100% as possible, enforced in p
 
 Test should use Foundry, unless for some reason js or hardhat are needed (for example, upgrades).
 
+The test function names will follow
+
+```
+- test_contextCamel_descriptionOfTheTestCamel
+- context = method name, contract or functionality.
+```
+
 In some cases unit tests may be insufficient and complementary techniques should be used:
 
 1. Property-based tests (aka. fuzzing) for math-heavy code.
@@ -40,7 +47,7 @@ Modularity should be pursued, but not at the cost of the above priorities.
 
 For contributors, project guidelines and processes must be documented publicly.
 
-Every method and contract must have Natspec
+Every method and contract must have Natspec, using the `///` flavour always.
 
 For users, features must be abundantly documented. Documentation should include answers to common questions, solutions to common problems, and recommendations for critical decisions that the user may face.
 
@@ -77,7 +84,7 @@ Pull requests are squash-merged to keep the `main` branch history clean. The tit
 
 We welcome conventional commits, with prefixes the title with "fix:" or "feat:".
 
-Work in progress pull requests should be submitted as Drafts and should not be prefixed with "WIP:".
+Work in progress pull requests should be submitted as Drafts and should **not** be prefixed with "WIP:".
 
 Branch names don't matter, and commit messages within a pull request mostly don't matter either, although they can help the review process.
 
@@ -120,7 +127,9 @@ In addition to the official Solidity Style Guide we have a number of other conve
   interface IERC777 {
   ```
 
-* Group contracts by functionality within folders if possible. 
+* Group contracts by functionality within folders if possible.
+
+* Interfaces should go inside the `interface` folder, mirroring the folder structure of the implementations
 
 * Folder names must be lowercase, hyphen separated.
 
@@ -134,4 +143,24 @@ In addition to the official Solidity Style Guide we have a number of other conve
   ExampleContract.sol
   ```
 
+* Acronyms should be
+  * Uppercase all if in contract name (`UUPSUpgradeable`, `IPAsset`)
+  * Camelcase in properties and function names (`ipAssetId`), except if they are defined otherwise in external contracts or interfaces (`tokenURI`)
+
 * Unchecked arithmetic blocks should contain comments explaining why overflow is guaranteed not to happen. If the reason is immediately apparent from the line above the unchecked block, the comment may be omitted.
+
+* Interfaces should contain methods an events. Structs showing in an interface should be grouped in a library
+
+* Function parameter names will have the **suffix** `_`
+
+*  Naming conventions
+  - Contract: CamelCase (adjectiveNoun)
+  - Struct (noun)
+  - Event (past-tense)
+  - Function Name (verb noun)
+    - local variable (noun / compound noun)
+        - Booleans (use `isXXX`)
+        - `isValid`
+        - `valid`
+    - Modifier (prepositionNoun)
+        - `onlyOwner`

contracts/FranchiseRegistry.sol

@@ -1,17 +1,14 @@
 // SPDX-License-Identifier: BUSL-1.1
 pragma solidity ^0.8.13;
 
-import { IPAsset } from "./IPAsset.sol";
+import { Errors } from "contracts/lib/Errors.sol";
+import { AccessControl } from "contracts/lib/AccessControl.sol";
 import { IPAssetRegistryFactory } from "./ip-assets/IPAssetRegistryFactory.sol";
 import { AccessControlledUpgradeable } from "./access-control/AccessControlledUpgradeable.sol";
-import { UPGRADER_ROLE } from "./access-control/ProtocolRoles.sol";
-import { ZeroAddress, Unauthorized } from "./errors/General.sol";
-import { IVersioned } from "./utils/IVersioned.sol";
-import { IIPAssetRegistry } from "./ip-assets/IIPAssetRegistry.sol";
-import { LibIPAssetId } from "./ip-assets/LibIPAssetId.sol";
+import { IVersioned } from "contracts/interfaces/utils/IVersioned.sol";
+import { IIPAssetRegistry } from "contracts/interfaces/ip-assets/IIPAssetRegistry.sol";
 import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
 import { ERC721Upgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC721/ERC721Upgradeable.sol";
-import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol";
 
 contract FranchiseRegistry is
     UUPSUpgradeable,
@@ -27,8 +24,8 @@ contract FranchiseRegistry is
         string symbol,
         string tokenURI
     );
-    error AlreadyRegistered();
 
+    /// TODO: Add franchise interface and place this in separate lib
     struct FranchiseCreationParams {
         string name;
         string symbol;
@@ -46,82 +43,97 @@ contract FranchiseRegistry is
 
     IPAssetRegistryFactory public immutable FACTORY;
     // keccak256(bytes.concat(bytes32(uint256(keccak256("story-protocol.franchise-registry.storage")) - 1)))
-    bytes32 private constant _STORAGE_LOCATION = 0x5648324915b730d22cca7279385130ad43fd4829d795fb20e9ab398bfe537e8f;
+    bytes32 private constant _STORAGE_LOCATION =
+        0x5648324915b730d22cca7279385130ad43fd4829d795fb20e9ab398bfe537e8f;
     uint256 public constant PROTOCOL_ROOT_ID = 0;
     address public constant PROTOCOL_ROOT_ADDRESS = address(0);
     string private constant _VERSION = "0.1.0";
 
-    constructor(address _factory) {
-        if (_factory == address(0)) revert ZeroAddress();
-        FACTORY = IPAssetRegistryFactory(_factory);
+    constructor(address factory_) {
+        if (factory_ == address(0)) revert Errors.ZeroAddress();
+        FACTORY = IPAssetRegistryFactory(factory_);
         _disableInitializers();
     }
 
-    function initialize(address accessControl) public initializer {
-        __UUPSUpgradeable_init();
-        __AccessControlledUpgradeable_init(accessControl);
-        __ERC721_init("Story Protocol", "SP");
-    }
-
-    function _getFranchiseStorage() private pure returns (FranchiseStorage storage $) {
-        assembly {
-            $.slot := _STORAGE_LOCATION
-        }
-    }
-
-    function version() external pure override returns (string memory) {
-        return _VERSION;
-    }
-
-    function registerFranchise(FranchiseCreationParams calldata params) external returns (uint256, address) {
+    function registerFranchise(
+        FranchiseCreationParams calldata params_
+    ) external returns (uint256, address) {
         FranchiseStorage storage $ = _getFranchiseStorage();
         uint256 nextId = ++$.franchiseIds;
-        address ipAssetRegistry = FACTORY.createFranchiseIPAssets(
+        address ipAssetRegistry = FACTORY.createFranchiseIpAssets(
             nextId,
-            params.name,
-            params.symbol,
-            params.description
+            params_.name,
+            params_.symbol,
+            params_.description
         );
         $.ipAssetRegistries[nextId] = ipAssetRegistry;
-        $.tokenURIs[nextId] = params.tokenURI;
+        $.tokenURIs[nextId] = params_.tokenURI;
         _safeMint(msg.sender, nextId);
         // TODO: set licensing restrictions per franchise, maybe grant commercial root license to the franchise NFT
-
-        emit FranchiseRegistered(msg.sender, nextId, ipAssetRegistry, params.name, params.symbol, params.tokenURI);
-
-        return (nextId, ipAssetRegistry);
-    }
 
-    function ipAssetRegistryForId(
-        uint256 franchiseId
-    ) public view returns (address) {
-        FranchiseStorage storage $ = _getFranchiseStorage();
-        return $.ipAssetRegistries[franchiseId];
+        emit FranchiseRegistered(
+            msg.sender,
+            nextId,
+            ipAssetRegistry,
+            params_.name,
+            params_.symbol,
+            params_.tokenURI
+        );
+
+        return (nextId, ipAssetRegistry);
     }
 
-
-    /**
-     * @notice checks if an address is a valid SP IPAssetRegistry.
-     * @param ipAssetRegistry the address to check
-     * @return true if it's a valid SP IPAssetRegistry, false otherwise
-     */
-    function isIpAssetRegistry(address ipAssetRegistry) external view returns(bool) {
-        try IIPAssetRegistry(ipAssetRegistry).franchiseId() returns (uint256 franchiseId) {
-            return ipAssetRegistryForId(franchiseId) == ipAssetRegistry;
+    /// @notice checks if an address is a valid SP IPAssetRegistry.
+    /// @param ipAssetRegistry_ the address to check
+    /// @return true if it's a valid SP IPAssetRegistry, false otherwise
+    function isIpAssetRegistry(
+        address ipAssetRegistry_
+    ) external view returns (bool) {
+        try IIPAssetRegistry(ipAssetRegistry_).franchiseId() returns (
+            uint256 franchiseId
+        ) {
+            return ipAssetRegistryForId(franchiseId) == ipAssetRegistry_;
         } catch {
             return false;
         }
     }
 
+    function version() external pure override returns (string memory) {
+        return _VERSION;
+    }
+
+    function initialize(address accessControl_) public initializer {
+        __UUPSUpgradeable_init();
+        __AccessControlledUpgradeable_init(accessControl_);
+        __ERC721_init("Story Protocol", "SP");
+    }
+
+    function ipAssetRegistryForId(
+        uint256 franchiseId_
+    ) public view returns (address) {
+        FranchiseStorage storage $ = _getFranchiseStorage();
+        return $.ipAssetRegistries[franchiseId_];
+    }
 
-    function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
-        _requireMinted(tokenId);
+    function tokenURI(
+        uint256 tokenId_
+    ) public view virtual override returns (string memory) {
+        _requireMinted(tokenId_);
         FranchiseStorage storage $ = _getFranchiseStorage();
-        return $.tokenURIs[tokenId];
-    } 
+        return $.tokenURIs[tokenId_];
+    }
 
     function _authorizeUpgrade(
-        address newImplementation
-    ) internal virtual override onlyRole(UPGRADER_ROLE) {}
-
+        address newImplementation_
+    ) internal virtual override onlyRole(AccessControl.UPGRADER_ROLE) {}
+
+    function _getFranchiseStorage()
+        private
+        pure
+        returns (FranchiseStorage storage $)
+    {
+        assembly {
+            $.slot := _STORAGE_LOCATION
+        }
+    }
 }