From 13ee11a32a05905b79d7aa7841e38c44eb97cfcd Mon Sep 17 00:00:00 2001 From: BoynChen Date: Sun, 4 Dec 2022 01:16:51 +0800 Subject: [PATCH 1/2] feat: add comments --- .gitignore | 1 + contracts/RMRK/LightmInit.sol | 18 ++++++++++-------- contracts/RMRK/LightmUniversalFactory.sol | 20 +++++++++++++++----- contracts/RMRK/access/OwnableLock.sol | 18 ++++++++++++++++-- hardhat.config.ts | 2 +- scripts/deploy_diamond_equippable.ts | 11 +++-------- scripts/deploy_universal_factory.ts | 1 + 7 files changed, 47 insertions(+), 24 deletions(-) diff --git a/.gitignore b/.gitignore index 22df8c18..16007585 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ node_modules .env +pnpm-lock.yaml coverage coverage.json typechain diff --git a/contracts/RMRK/LightmInit.sol b/contracts/RMRK/LightmInit.sol index 31d08269..895d3471 100644 --- a/contracts/RMRK/LightmInit.sol +++ b/contracts/RMRK/LightmInit.sol @@ -11,10 +11,11 @@ import {ILightmEquippable} from "./interfaces/ILightmEquippable.sol"; import {IRMRKCollectionMetadata} from "./interfaces/IRMRKCollectionMetadata.sol"; import {ERC721Storage, MultiAssetStorage, EquippableStorage, CollectionMetadataStorage, LightmImplStorage} from "./internalFunctionSet/Storage.sol"; -// It is expected that this contract is customized if you want to deploy your diamond -// with data from a deployment script. Use the init function to initialize state variables -// of your diamond. Add parameters to the init funciton if you need to. - +/** + * @dev It is expected that this contract is customized if you want to deploy your diamond + * with data from a deployment script. Use the init function to initialize state variables + * of your diamond. Add parameters to the init funciton if you need to. + */ contract LightmInit { struct InitStruct { string name; @@ -23,8 +24,10 @@ contract LightmInit { string collectionMetadataURI; } - // You can add parameters to this function in order to pass in - // data to set your own state variables + /** + * @dev You can add parameters to this function in order to pass in + * You can add parameters to this function in order to pass in + */ function init(InitStruct calldata _initStruct, address _owner) external { // adding ERC165 data LibDiamond.DiamondStorage storage ds = LibDiamond.diamondStorage(); @@ -55,8 +58,7 @@ contract LightmInit { s._name = _initStruct.name; s._symbol = _initStruct.symbol; - MultiAssetStorage.State storage mrs = MultiAssetStorage - .getState(); + MultiAssetStorage.State storage mrs = MultiAssetStorage.getState(); mrs._fallbackURI = _initStruct.fallbackURI; CollectionMetadataStorage.State storage cms = CollectionMetadataStorage diff --git a/contracts/RMRK/LightmUniversalFactory.sol b/contracts/RMRK/LightmUniversalFactory.sol index e3066d42..e993b44b 100644 --- a/contracts/RMRK/LightmUniversalFactory.sol +++ b/contracts/RMRK/LightmUniversalFactory.sol @@ -17,8 +17,12 @@ import {LightmImpl} from "../implementations/LightmImplementer.sol"; import "./interfaces/ILightmUniversalFactory.sol"; -import "hardhat/console.sol"; - +/** + * @title LightmUniversalFactory + * @author Lightm + * @notice Factory on-chain that indexed all facet contract address + * and can be used for deploying RMRK NFT collection + */ contract LightmUniversalFactory is ILightmUniversalFactory { string private constant VERSION = "0.1.0-alpha"; @@ -98,9 +102,15 @@ contract LightmUniversalFactory is ILightmUniversalFactory { return _implContractAddress; } - function deployCollection(LightmInit.InitStruct calldata initStruct) - external - { + /** + * @notice Used to deploy new collection. + * @dev It will deploy diamond contract only. + * @dev It will loads all facet cuts by `diamondCut` method after deployed. + * @param initStruct metadata of NFT + */ + function deployCollection( + LightmInit.InitStruct calldata initStruct + ) external { Diamond instance = new Diamond(address(this), _diamondCutFacetAddress); address instanceAddress = address(instance); diff --git a/contracts/RMRK/access/OwnableLock.sol b/contracts/RMRK/access/OwnableLock.sol index 7395b78e..03fb0830 100644 --- a/contracts/RMRK/access/OwnableLock.sol +++ b/contracts/RMRK/access/OwnableLock.sol @@ -9,20 +9,34 @@ Minimal ownable lock */ error RMRKLocked(); +/** + * @title OwnableLock + * @notice A minimal ownable lock smart contract. + */ contract OwnableLock is Ownable { - bool private lock; + /** + * @dev Reverts if the lock flag is set to true. + */ modifier notLocked() { if (getLock()) revert RMRKLocked(); _; } + /** + * @notice Locks the operation. + * @dev Once locked, functions using `notLocked` modifier cannot be executed. + */ function setLock() external onlyOwner { lock = true; } - function getLock() public view returns(bool) { + /** + * @notice Used to retrieve the status of a lockable smart contract. + * @return bool A boolean value signifying whether the smart contract has been locked + */ + function getLock() public view returns (bool) { return lock; } } diff --git a/hardhat.config.ts b/hardhat.config.ts index f060273c..d708142c 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -73,7 +73,7 @@ const config: HardhatUserConfig = { accounts: process.env.PRIVATE_KEY !== undefined ? [process.env.PRIVATE_KEY] : [], }, localhost: { - url: 'http://localhost:8545', + url: 'http://127.0.0.1:8545', accounts: process.env.PRIVATE_KEY !== undefined ? [process.env.PRIVATE_KEY] : [], }, sepolia: { diff --git a/scripts/deploy_diamond_equippable.ts b/scripts/deploy_diamond_equippable.ts index 8ab7c0a7..fc6d5bb5 100644 --- a/scripts/deploy_diamond_equippable.ts +++ b/scripts/deploy_diamond_equippable.ts @@ -22,7 +22,7 @@ async function isHardhatChain() { return false; } -export async function deployCreate2Deployer() { +export async function deployCreate2Deployer(): Promise { // if is in local chain, send some value to transaction signer address if (await isHardhatChain()) { const publicAccount = new ethers.Wallet(PUBLIC_ACCOUNT_PRIVATE_KEY, ethers.provider); @@ -64,9 +64,7 @@ export async function oneTimeDeploy( noNormalDeploy = false, ) { const create2Deployer = await ethers.getContractAt('Create2Deployer', create2DeployerAddress); - const RMRKMultiAssetRenderUtils = await ethers.getContractFactory( - 'RMRKMultiAssetRenderUtils', - ); + const RMRKMultiAssetRenderUtils = await ethers.getContractFactory('RMRKMultiAssetRenderUtils'); const LightmValidatorLib = await ethers.getContractFactory('LightmValidatorLib'); // ---------- Normal deployment @@ -89,10 +87,7 @@ export async function oneTimeDeploy( const rmrkMultiAssetRenderUtilsAddress: string = await create2Deployer[ 'computeAddress(bytes32,bytes32)' - ]( - rmrkMultiAssetRenderUtilsHash, - ethers.utils.keccak256(RMRKMultiAssetRenderUtils.bytecode), - ); + ](rmrkMultiAssetRenderUtilsHash, ethers.utils.keccak256(RMRKMultiAssetRenderUtils.bytecode)); const rmrkMultiAssetRenderUtils = await ethers.getContractAt( 'LightmValidatorLib', rmrkMultiAssetRenderUtilsAddress, diff --git a/scripts/deploy_universal_factory.ts b/scripts/deploy_universal_factory.ts index 0c6f4750..a523ec5a 100644 --- a/scripts/deploy_universal_factory.ts +++ b/scripts/deploy_universal_factory.ts @@ -1,3 +1,4 @@ +/* eslint-disable node/no-missing-import */ import { ethers } from 'hardhat'; import { LightmInit__factory } from '../typechain-types'; import { ILightmUniversalFactory } from '../typechain-types/contracts/RMRK/LightmUniversalFactory'; From 9114b0c91666817e8c64d980d4bd368ccca341ea Mon Sep 17 00:00:00 2001 From: BoynChen Date: Sun, 4 Dec 2022 14:25:56 +0800 Subject: [PATCH 2/2] feat: add spec for MultiAssetFacet --- contracts/RMRK/Diamond.sol | 13 +- contracts/RMRK/LightmUniversalFactory.sol | 3 +- contracts/RMRK/RMRKMultiAssetFacet.sol | 302 ++++++++++++++-------- 3 files changed, 203 insertions(+), 115 deletions(-) diff --git a/contracts/RMRK/Diamond.sol b/contracts/RMRK/Diamond.sol index af4c010d..e66b7cb9 100644 --- a/contracts/RMRK/Diamond.sol +++ b/contracts/RMRK/Diamond.sol @@ -11,6 +11,13 @@ pragma solidity ^0.8.0; import {LibDiamond} from "./library/LibDiamond.sol"; import {IDiamondCut} from "./interfaces/IDiamondCut.sol"; +/** + * @title Diamond + * @author Lightm + * @notice Entry for external call, all method must be call by Diamond contract + * @dev You should deploy diamond cut facet first before deploy Diamond + * @dev function in diamondCut will automatically register in constructor + */ contract Diamond { constructor(address _contractOwner, address _diamondCutFacet) payable { LibDiamond.setContractOwner(_contractOwner); @@ -27,8 +34,10 @@ contract Diamond { LibDiamond.diamondCut(cut, address(0), ""); } - // Find facet for function that is called and execute the - // function if a facet is found and return any value. + /** + * @dev Find facet for function that is called and execute the + * @dev function if a facet is found and return any value. + */ fallback() external payable { LibDiamond.DiamondStorage storage ds; bytes32 position = LibDiamond.DIAMOND_STORAGE_POSITION; diff --git a/contracts/RMRK/LightmUniversalFactory.sol b/contracts/RMRK/LightmUniversalFactory.sol index e993b44b..a926fcfb 100644 --- a/contracts/RMRK/LightmUniversalFactory.sol +++ b/contracts/RMRK/LightmUniversalFactory.sol @@ -106,7 +106,8 @@ contract LightmUniversalFactory is ILightmUniversalFactory { * @notice Used to deploy new collection. * @dev It will deploy diamond contract only. * @dev It will loads all facet cuts by `diamondCut` method after deployed. - * @param initStruct metadata of NFT + * @dev Diamond contract can use all DiamondCut methods because it has loaded when Diamond contract init. + * @param initStruct metadata of NFT like name,symbol */ function deployCollection( LightmInit.InitStruct calldata initStruct diff --git a/contracts/RMRK/RMRKMultiAssetFacet.sol b/contracts/RMRK/RMRKMultiAssetFacet.sol index 51395f56..184ec774 100644 --- a/contracts/RMRK/RMRKMultiAssetFacet.sol +++ b/contracts/RMRK/RMRKMultiAssetFacet.sol @@ -26,6 +26,12 @@ import "./library/RMRKMultiAssetRenderUtils.sol"; `mint` logic in your own implementer. */ +/** + * @title RMRKMultiAssetFacet + * @author Lightm + * @notice Smart contract of the RMRK Multi asset module. + * @dev This contract has been reoriganized into Diamond facet, we storage it by AppStorage + */ contract RMRKMultiAssetFacet is IERC721, IERC721Metadata, @@ -44,12 +50,9 @@ contract RMRKMultiAssetFacet is s._symbol = symbol_; } - function supportsInterface(bytes4 interfaceId) - public - view - virtual - returns (bool) - { + function supportsInterface( + bytes4 interfaceId + ) public view virtual returns (bool) { return interfaceId == type(IERC165).interfaceId || interfaceId == type(IERC721).interfaceId || @@ -73,38 +76,29 @@ contract RMRKMultiAssetFacet is return getState()._symbol; } - function tokenURI(uint256 tokenId) - public - view - virtual - override - returns (string memory) - { + /** + * @dev See {IERC721Metadata-tokenURI}. + */ + function tokenURI( + uint256 tokenId + ) public view virtual override returns (string memory) { return _tokenURI(tokenId); } // ------------------------ Ownership ------------------------ - function ownerOf(uint256 tokenId) - public - view - virtual - override - returns (address) - { + function ownerOf( + uint256 tokenId + ) public view virtual override returns (address) { return _ownerOf(tokenId); } /** * @dev See {IERC721-balanceOf}. */ - function balanceOf(address owner) - public - view - virtual - override - returns (uint256) - { + function balanceOf( + address owner + ) public view virtual override returns (uint256) { return _balanceOf(owner); } @@ -124,37 +118,29 @@ contract RMRKMultiAssetFacet is /** * @dev See {IERC721-getApproved}. */ - function getApproved(uint256 tokenId) - public - view - virtual - override - returns (address) - { + function getApproved( + uint256 tokenId + ) public view virtual override returns (address) { return _getApproved(tokenId); } /** * @dev See {IERC721-setApprovalForAll}. */ - function setApprovalForAll(address operator, bool approved) - public - virtual - override - { + function setApprovalForAll( + address operator, + bool approved + ) public virtual override { _setApprovalForAll(_msgSender(), operator, approved); } /** * @dev See {IERC721-isApprovedForAll}. */ - function isApprovedForAll(address owner, address operator) - public - view - virtual - override - returns (bool) - { + function isApprovedForAll( + address owner, + address operator + ) public view virtual override returns (bool) { return _isApprovedForAll(owner, operator); } @@ -197,38 +183,96 @@ contract RMRKMultiAssetFacet is // ------------------------ RESOURCES ------------------------ - function acceptAsset(uint256 tokenId, uint64 assetId) - external - virtual - onlyApprovedForAssetsOrOwner(tokenId) - { + /** + * @notice Accepts an asset at from the pending array of given token. + * @dev Migrates the asset from the token's pending asset array to the token's active asset array. + * @dev Active assets cannot be removed by anyone, but can be replaced by a new asset. + * @dev Requirements: + * + * - The caller must own the token or be approved to manage the token's assets + * - `tokenId` must exist. + * - `assetId` must be in pending asset array. + * @dev Emits an {AssetAccepted} event. + * @param tokenId ID of the token for which to accept the pending asset + * @param assetId Index of the asset in the pending array to accept + */ + function acceptAsset( + uint256 tokenId, + uint64 assetId + ) external virtual onlyApprovedForAssetsOrOwner(tokenId) { _acceptAsset(tokenId, assetId); } - function rejectAsset(uint256 tokenId, uint64 assetId) - external - virtual - onlyApprovedForAssetsOrOwner(tokenId) - { + /** + * @notice Rejects an asset from the pending array of given token. + * @dev Removes the asset from the token's pending asset array. + * @dev Requirements: + * + * - The caller must own the token or be approved to manage the token's assets + * - `tokenId` must exist. + * - `assetId` must be in pending array. + * @dev Emits a {AssetRejected} event. + * @param tokenId ID of the token that the asset is being rejected from + * @param assetId Id of the asset in the pending array to be rejected + */ + function rejectAsset( + uint256 tokenId, + uint64 assetId + ) external virtual onlyApprovedForAssetsOrOwner(tokenId) { _rejectAsset(tokenId, assetId); } - function rejectAllAssets(uint256 tokenId) - external - virtual - onlyApprovedForAssetsOrOwner(tokenId) - { + /** + * @notice Rejects all assets from the pending array of a given token. + * @dev Effecitvely deletes the pending array. + * @dev Requirements: + * + * - The caller must own the token or be approved to manage the token's assets + * - `tokenId` must exist. + * @dev Emits a {AssetRejected} event with assetId = 0. + * @param tokenId ID of the token of which to clear the pending array. + */ + function rejectAllAssets( + uint256 tokenId + ) external virtual onlyApprovedForAssetsOrOwner(tokenId) { _rejectAllAssets(tokenId); } - function setPriority(uint256 tokenId, uint16[] memory priorities) - external - virtual - onlyApprovedForAssetsOrOwner(tokenId) - { + /** + * @notice Sets a new priority array for a given token. + * @dev The priority array is a non-sequential list of `uint16`s, where the lowest value is considered highest + * priority. + * @dev Value `0` of a priority is a special case equivalent to unitialized. + * @dev Requirements: + * + * - The caller must own the token or be approved to manage the token's assets + * - `tokenId` must exist. + * - The length of `priorities` must be equal the length of the active assets array. + * @dev Emits a {AssetPrioritySet} event. + * @param tokenId ID of the token to set the priorities for + * @param priorities An array of priorities of active assets. The succesion of items in the priorities array + * matches that of the succesion of items in the active array + */ + function setPriority( + uint256 tokenId, + uint16[] memory priorities + ) external virtual onlyApprovedForAssetsOrOwner(tokenId) { _setPriority(tokenId, priorities); } + /** + * @notice Used to grant permission to the user to manage token's assets. + * @dev This differs from transfer approvals, as approvals are not cleared when the approved party accepts or + * rejects an asset, or sets asset priorities. This approval is cleared on token transfer. + * @dev Only a single account can be approved at a time, so approving the `0x0` address clears previous approvals. + * @dev Requirements: + * + * - The caller must own the token or be an approved operator. + * - `tokenId` must exist. + * @dev Emits an {ApprovalForAssets} event. + * @param to Address of the account to grant the approval to + * @param tokenId ID of the token for which the approval to manage the assets is granted + */ function approveForAssets(address to, uint256 tokenId) external virtual { address owner = ownerOf(tokenId); if (to == owner) revert RMRKApprovalForAssetsToCurrentOwner(); @@ -241,76 +285,110 @@ contract RMRKMultiAssetFacet is _approveForAssets(to, tokenId); } - function setApprovalForAllForAssets(address operator, bool approved) - external - virtual - { + /** + * @notice Used to add or remove an operator of assets for the caller. + * @dev Operators can call {acceptAsset}, {rejectAsset}, {rejectAllAssets} or {setPriority} for any token + * owned by the caller. + * @dev Requirements: + * + * - The `operator` cannot be the caller. + * @dev Emits an {ApprovalForAllForAssets} event. + * @param operator Address of the account to which the operator role is granted or revoked from + * @param approved The boolean value indicating whether the operator role is being granted (`true`) or revoked + * (`false`) + */ + function setApprovalForAllForAssets( + address operator, + bool approved + ) external virtual { address owner = _msgSender(); if (owner == operator) revert RMRKApproveForAssetsToCaller(); _setApprovalForAllForAssets(owner, operator, approved); } - function getAssetMetadata(uint64 assetId) - public - view - virtual - returns (string memory) - { + /** + * @notice Used to fetch the asset metadata of the specified token's for given asset. + * @dev Assets are stored by reference mapping `_assets[assetId]`. + * @dev Can be overriden to implement enumerate, fallback or other custom logic. + * @param assetId Asset Id, must be in the pending or active assets array + * @return string Metadata of the asset + */ + function getAssetMetadata( + uint64 assetId + ) public view virtual returns (string memory) { return _getAssetMetadata(assetId); } - function getActiveAssets(uint256 tokenId) - public - view - virtual - returns (uint64[] memory) - { + /** + * @notice Used to retrieve the active asset IDs of a given token. + * @dev Assets metadata is stored by reference mapping `_asset[assetId]`. + * @param tokenId ID of the token to query + * @return uint64[] Array of active asset IDs + */ + function getActiveAssets( + uint256 tokenId + ) public view virtual returns (uint64[] memory) { return _getActiveAssets(tokenId); } - function getPendingAssets(uint256 tokenId) - public - view - virtual - returns (uint64[] memory) - { + /** + * @notice Returns pending asset IDs for a given token + * @dev Pending assets metadata is stored by reference mapping _pendingAsset[assetId] + * @param tokenId the token ID to query + * @return uint64[] pending asset IDs + */ + function getPendingAssets( + uint256 tokenId + ) public view virtual returns (uint64[] memory) { return _getPendingAssets(tokenId); } - function getActiveAssetPriorities(uint256 tokenId) - public - view - virtual - returns (uint16[] memory) - { + /** + * @notice Used to retrieve active asset priorities of a given token. + * @dev Asset priorities are a non-sequential array of uint16 values with an array size equal to active asset + * priorites. + * @param tokenId ID of the token to query + * @return uint16[] Array of active asset priorities + */ + function getActiveAssetPriorities( + uint256 tokenId + ) public view virtual returns (uint16[] memory) { return _getActiveAssetPriorities(tokenId); } - function getAssetOverwrites(uint256 tokenId, uint64 assetId) - public - view - virtual - returns (uint64) - { + function getAssetOverwrites( + uint256 tokenId, + uint64 assetId + ) public view virtual returns (uint64) { return _getAssetOverwrites(tokenId, assetId); } - function getApprovedForAssets(uint256 tokenId) - public - view - virtual - returns (address) - { + /** + * @notice Used to retrieve the address of the account approved to manage assets of a given token. + * @dev Requirements: + * + * - `tokenId` must exist. + * @param tokenId ID of the token for which to retrieve the approved address + * @return address Address of the account that is approved to manage the specified token's assets + */ + function getApprovedForAssets( + uint256 tokenId + ) public view virtual returns (address) { return _getApprovedForAssets(tokenId); } - function isApprovedForAllForAssets(address owner, address operator) - public - view - virtual - returns (bool) - { + /** + * @notice Used to check whether the address has been granted the operator role by a given address or not. + * @dev See {setApprovalForAllForAssets}. + * @param owner Address of the account that we are checking for whether it has granted the operator role + * @param operator Address of the account that we are checking whether it has the operator role or not + * @return bool The boolean value indicating wehter the account we are checking has been granted the operator role + */ + function isApprovedForAllForAssets( + address owner, + address operator + ) public view virtual returns (bool) { return _isApprovedForAllForAssets(owner, operator); } }