Audit ready

contracts/MeritNFT.sol

@@ -20,15 +20,23 @@ contract MeritNFT is ERC721, AccessControlEnumerable, IMeritMintableNFT {
         _;
     }
 
+    /// @notice Constructor
+    /// @param _name The name of the NFT
+    /// @param _symbol Symbol aka ticker
+    /// @param _baseTokenURI Prepends the tokenId for the tokenURI
     constructor(
-        string memory name,
-        string memory symbol,
+        string memory _name,
+        string memory _symbol,
         string memory _baseTokenURI
-    ) ERC721(name, symbol) {
+    ) ERC721(_name, _symbol) {
         baseTokenURI = _baseTokenURI;
         _setupRole(DEFAULT_ADMIN_ROLE, _msgSender());
     }
 
+
+    /// @notice Mints an NFT. Can only be called by an address with the minter role and tokenId must be unique
+    /// @param _tokenId Id of the token
+    /// @param _receiver Address receiving the NFT
     function mint(uint256 _tokenId, address _receiver) external override onlyMinter {
         _mint(_receiver, _tokenId);
     }
@@ -37,6 +45,14 @@ contract MeritNFT is ERC721, AccessControlEnumerable, IMeritMintableNFT {
         return baseTokenURI;
     }
 
+    /// @notice returns the baseURI
+    /// @return The tokenURI
+    function baseURI() external view returns (string memory) {
+        return _baseURI();
+    }
+
+    /// @notice Signals support for a given interface
+    /// @param interfaceId 4bytes signature of the interface
     function supportsInterface(bytes4 interfaceId)
         public
         view

contracts/MeritNFTDropFactory.sol

@@ -10,6 +10,8 @@ error MerkleSetterError();
 
 contract MeritNFTDropFactory {
 
+    error MerkleProofError();
+
     struct MerkleTree {
         bytes32 root;
         string ipfsHash; //to be able to fetch the merkle tree without relying on a centralized UI
@@ -31,6 +33,12 @@ contract MeritNFTDropFactory {
     event MerkleSetterUpdated(address indexed NFT, address indexed newSetter);
     event NFTClaimed(address indexed NFT, uint256 indexed tokenId, address indexed receiver);
 
+    /// @notice deploys an NFT contract
+    /// @param _name Name of the NFT
+    /// @param _symbol Symbol aka ticker
+    /// @param _baseTokenURI Prepends the tokenId for the tokenURI
+    /// @param _merkleRoot root of the merkle drop tree
+    /// @param _merkleIpfsHash IPFS hash of all leafs in the merkle tree
     function deployNFT(
         string memory _name,
         string memory _symbol,
@@ -40,7 +48,6 @@ contract MeritNFTDropFactory {
         bool _immutable
     ) external returns(address) {
         // TODO consider using a transparant proxy to bring down gas cost
-
         MeritNFT NFT = new MeritNFT(
             _name,
             _symbol,
@@ -64,6 +71,10 @@ contract MeritNFTDropFactory {
         return address(NFT);
     }
 
+    /// @notice Updates the NFT drop merkle tree. Can only be called by the merkle setter
+    /// @param _NFT address of the NFT contract
+    /// @param _merkleRoot new merkleRoot
+    /// @param _merkleIpfsHash IPFS hash of all leafs in the merkle tree
     function updateNFTMerkleTree(
         address _NFT,
         bytes32 _merkleRoot,
@@ -77,17 +88,30 @@ contract MeritNFTDropFactory {
         emit MerkleTreeUpdated(_NFT, _merkleRoot, _merkleIpfsHash);
     }
 
+    /// @notice Update the Merkle setter. Can only be called by the current setter
+    /// @param _NFT address of the nft contract
+    /// @param _merkleSetter address of the new merkleSetter
     function setMerkleSetter(address _NFT, address _merkleSetter) onlyMerkleSetter(_NFT) external {
         NFTMerkleSetter[_NFT] = _merkleSetter;
         emit MerkleSetterUpdated(_NFT, _merkleSetter);
     }
 
+    /// @notice Claim an nft using the merkleProof
+    /// @param _NFT address of the nft contract
+    /// @param _tokenId ID of the token to claim
+    /// @param _receiver Receiver of the NFT
+    /// @param _proof merkle proof
     function claim(address _NFT, uint256 _tokenId, address _receiver, bytes32[] calldata _proof) external {
         bytes32 leaf = keccak256(abi.encodePacked(_tokenId, _receiver));
-        // TODO custom error
-        require(MerkleProof.verify(_proof, NFTMerkleTree[_NFT].root, leaf), "MerkleNFTDrop.claim: Proof invalid");
+
+        if(!MerkleProof.verify(_proof, NFTMerkleTree[_NFT].root, leaf)) {
+            revert MerkleProofError();
+        }
+
         // Mint NFT
         MeritNFT NFT = MeritNFT(_NFT);
+
+        // relies on nft contract enforcing the same NFT cannot be minted twice
         NFT.mint(_tokenId, _receiver);
 
         emit NFTClaimed(_NFT, _tokenId, _receiver);

contracts/WhitelistedNFTSale.sol

@@ -51,6 +51,16 @@ contract WhitelistedNFTSale is AccessControlEnumerable {
         _;
     }
 
+
+    /// @notice constructor
+    /// @param _merkleRoot merkle root
+    /// @param _startTime start time of the sale
+    /// @param _endTime end time of the sale
+    /// @param _NFT address of the nft contract
+    /// @param _saleCap maximum of nfts sold during the sale
+    /// @param _capPerUser maximum a single address can buy
+    /// @param _price price per nft
+    /// @param _idOffset offset from which to start the incrementing of token ids
     constructor(
         bytes32 _merkleRoot,
         uint256 _startTime,
@@ -74,15 +84,22 @@ contract WhitelistedNFTSale is AccessControlEnumerable {
         _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
     }
 
-
+    /// @notice Set the merkle root. Can only be called by a merkle root setter
+    /// @param _merkleRoot new merkleRoot
     function setMerkleRoot(bytes32 _merkleRoot) external onlyMerkleRootSetter {
         merkleRoot = _merkleRoot;
     }
 
+    /// @notice Claim funds received from the sale
+    /// @param _receiver address receiving the funds
     function claimFunds(address _receiver) external onlyFundsClaimer() {
         payable(_receiver).transfer(address(this).balance);
     }
 
+    /// @notice Buy an NFT
+    /// @param _amount amount of nfts to buy
+    /// @param _receiver address receiving the bought nfts
+    /// @param _proof merkle proof confirming inclusion in the whitelist
     function buy(uint256 _amount, address _receiver, bytes32[] calldata _proof) external payable {
         if(startTime > block.timestamp) {
             revert SaleNotStartedError();

package.json

@@ -76,7 +76,6 @@
     "lint": "yarn lint:sol && yarn lint:ts && yarn prettier:check",
     "lint:sol": "solhint --config ./.solhint.json --max-warnings 0 \"contracts/**/*.sol\"",
     "lint:ts": "eslint --config ./.eslintrc.yaml --ignore-path ./.eslintignore --ext .js,.ts .",
-    "postinstall": "husky install",
     "postpublish": "pinst --enable",
     "prepublishOnly": "pinst --disable",
     "prettier": "prettier --config ./.prettierrc.yaml --write \"**/*.{js,json,md,sol,ts}\"",
@@ -85,6 +84,8 @@
     "typechain": "cross-env TS_NODE_TRANSPILE_ONLY=true hardhat typechain"
   },
   "dependencies": {
-    "@openzeppelin/contracts": "^4.4.1"
+    "@openzeppelin/contracts": "^4.4.1",
+    "global": "^4.4.0",
+    "solidity-shell": "^0.0.10"
   }
 }

test/MeritNFT.ts

@@ -42,6 +42,9 @@ describe("MeritNFT", function() {
         DEFAULT_ADMIN_ROLE = await NFT.DEFAULT_ADMIN_ROLE();
         MINTER_ROLE = await NFT.MINTER_ROLE();
 
+        console.log("DEFAULT_ADMIN_ROLE", DEFAULT_ADMIN_ROLE);
+        console.log("MINTER_ROLE", MINTER_ROLE);
+
         await timeTraveler.snapshot();
     });