Address matterlabs audit observations

.cspell.json

@@ -0,0 +1,39 @@
+{
+    "version": "0.2",
+    "language": "en",
+    "files": [
+        "**/*.{sol,ts,md}"
+    ],
+    "ignorePaths": [
+        "node_modules",
+        "lib",
+        "out",
+        "cache",
+        "broadcast",
+        "artifacts-zk",
+        "deployments-zk",
+        "cache_hardhat-zk",
+        "zkout"
+    ],
+    "ignoreWords": [
+        "NODL",
+        "Nodle",
+        "depin",
+        "contentsign",
+        "matterlabs",
+        "zksync",
+        "zksolc",
+        "Parachain",
+        "subql",
+        "codegen",
+        "Datasource",
+        "ipfs",
+        "keccak",
+        "IERC",
+        "Mintable",
+        "BOOTLOADER",
+        "devcontainer",
+        "gasleft",
+        "chainid"
+    ]
+}

.github/workflows/checks.yml

@@ -23,7 +23,15 @@ jobs:
         uses: actions/checkout@v4
         with:
           submodules: recursive
+
+      - name: Install cSpell
+        run: npm install -g cspell
+
+      - name: Spell Check
+        run: cspell --config .cspell.json
+
       - name: Lint
         run: forge fmt --check
+
       - name: Run tests
         run: forge test --zksync

README.md

@@ -99,7 +99,7 @@ cast send -i $NFT "grantRole(bytes32,address)" $ROLE 0x68e3981280792A19cC03B5A77
 In the following example `N_VOTER1_ADDR` is the public address of the bridge oracle whose role is going to be a voter for funds coming from
 the parachain side. Similarly `N_VOTER2_ADDR` and `N_VOTER3_ADDR` are addresses of the other two voter oracles. 
 The closer oracle does not need special permissions and thus need not to be mentioned.
-NOTE: `i` flag in the command will make the tool prompt you for the private key of the deployer. So remember to have that handy but you don't need to define it in yout environment.
+NOTE: `i` flag in the command will make the tool prompt you for the private key of the deployer. So remember to have that handy but you don't need to define it in your environment.
 ```shell
 N_VOTER1_ADDR="0x18AB6B4310d89e9cc5521D33D5f24Fb6bc6a215E" \
 N_VOTER2_ADDR="0x571C969688991C6A35420C62d44666c47eB3F752" \
@@ -110,7 +110,7 @@ forge script script/DeployNodlMigration.sol --zksync --rpc-url https://sepolia.e
 Afterwards the user you onboarded should be able to mint NFTs as usual via the `safeMint(ownerAddress, metadataUri)` function.
 
 ### Deploying MigrationNFT contract
-The `MigrationNFT` contract allows the minting of a reward soulbound NFT when users bridge enough tokens through `NODLMigration`. Users can "level up" depending on the amount of tokens they bring in, with each levels being a sorted list of bridged amounts.
+The `MigrationNFT` contract allows the minting of a reward SoulBound NFT when users bridge enough tokens through `NODLMigration`. Users can "level up" depending on the amount of tokens they bring in, with each levels being a sorted list of bridged amounts.
 
 You will need to set the following environment variables:
 - `N_MIGRATION`: address of the `NODLMigration` contract
@@ -136,7 +136,7 @@ forge script script/DeployMigrationNFT.s.sol --zksync --rpc-url https://sepolia.
 ## Scripts
 
 ### Checking on bridging proposals
-Given a tracker id (`proposal`) and the bridge address you may run the script available in `./script/CheckBridge.s.sol`. The script will output proposal details and outline expecations as to the proposal's execution timeline. Here is a simple example:
+Given a tracker id (`proposal`) and the bridge address you may run the script available in `./script/CheckBridge.s.sol`. The script will output proposal details and outline expectations as to the proposal's execution timeline. Here is a simple example:
 ```shell
 N_PROPOSAL_ID=c43005c880cad7b699122b403607187a78251b9850d387521ffb123c473e3392 \
 N_BRIDGE=0x5de7fe085ee66Fb48447e75AA8fb0598a080AEe0 \
@@ -174,4 +174,4 @@ Verification on Etherscan is best done via the Solidity Json Input method as it
   2. Look for the input data variable named `_input`
   3. Copy paste its value and **strip the `0x prefix** as Etherscan will throw an error otherwise
 
-Use all these artefacts on the contract verification page on Etherscan for your given contract (open your contract on Etherscan, select `Contract` and the link starting with `Verify`). When prompted, enter the compiler versions, the license (we use BSD-3 Clause Clear). Then on the next page, enter your normalized JSON input file, and the contract constructor inputs.
+Use all these artifacts on the contract verification page on Etherscan for your given contract (open your contract on Etherscan, select `Contract` and the link starting with `Verify`). When prompted, enter the compiler versions, the license (we use BSD-3 Clause Clear). Then on the next page, enter your normalized JSON input file, and the contract constructor inputs.

hardhat.config.ts

@@ -23,7 +23,7 @@ const config: HardhatUserConfig = {
             zksync: true,
             verifyURL: "https://zksync2-mainnet-explorer.zksync.io/contract_verification",
         },
-        dockerizedNode: {
+        localDockerNode: {
             url: "http://localhost:3050",
             ethNetwork: "http://localhost:8545",
             zksync: true,

src/Grants.sol

@@ -82,8 +82,6 @@ contract Grants {
     ) external {
         validateVestingSchedule(to, period, periodCount, perPeriodAmount);
 
-        token.safeTransferFrom(msg.sender, address(this), perPeriodAmount * periodCount);
-
         VestingSchedule memory schedule = VestingSchedule(cancelAuthority, start, period, periodCount, perPeriodAmount);
 
         uint256 page = currentPage[to];
@@ -93,6 +91,8 @@ contract Grants {
         }
         vestingSchedules[to][page].push(schedule);
 
+        token.safeTransferFrom(msg.sender, address(this), perPeriodAmount * periodCount);
+
         emit VestingScheduleAdded(to, schedule);
     }
 

src/Rewards.sol

@@ -26,7 +26,7 @@ contract Rewards is AccessControl, EIP712 {
     /**
      * @dev The hash of the reward type structure.
      * It is calculated using the keccak256 hash function.
-     * The structure consists of the recipient's address, the amount of the reward, and the sequence number for that receipent.
+     * The structure consists of the recipient's address, the amount of the reward, and the sequence number for that recipient.
      */
     bytes32 public constant REWARD_TYPE_HASH = keccak256("Reward(address recipient,uint256 amount,uint256 sequence)");
     /**
@@ -117,7 +117,7 @@ contract Rewards is AccessControl, EIP712 {
      */
     error ZeroPeriod();
     /**
-     * @dev Error indicating that scheduling the reward quota renewal has failed most likley due to the period being too long.
+     * @dev Error indicating that scheduling the reward quota renewal has failed most likely due to the period being too long.
      */
     error TooLongPeriod();
     /**
@@ -323,12 +323,12 @@ contract Rewards is AccessControl, EIP712 {
     }
 
     /**
-     * @dev Internal check to ensure the `sequence` value is expected for `receipent`.
-     * @param receipent The address of the receipent to check.
+     * @dev Internal check to ensure the `sequence` value is expected for `recipient`.
+     * @param recipient The address of the recipient to check.
      * @param sequence The sequence value.
      */
-    function _mustBeExpectedSequence(address receipent, uint256 sequence) internal view {
-        if (sequences[receipent] != sequence) {
+    function _mustBeExpectedSequence(address recipient, uint256 sequence) internal view {
+        if (sequences[recipient] != sequence) {
             revert InvalidRecipientSequence();
         }
     }
@@ -409,10 +409,10 @@ contract Rewards is AccessControl, EIP712 {
      * @return The digest of the BatchReward struct.
      */
     function digestBatchReward(BatchReward memory batch) public view returns (bytes32) {
-        bytes32 receipentsHash = keccak256(abi.encodePacked(batch.recipients));
+        bytes32 recipientsHash = keccak256(abi.encodePacked(batch.recipients));
         bytes32 amountsHash = keccak256(abi.encodePacked(batch.amounts));
         return
-            _hashTypedDataV4(keccak256(abi.encode(BATCH_REWARD_TYPE_HASH, receipentsHash, amountsHash, batch.sequence)));
+            _hashTypedDataV4(keccak256(abi.encode(BATCH_REWARD_TYPE_HASH, recipientsHash, amountsHash, batch.sequence)));
     }
 
     /**

src/bridge/BridgeBase.sol

@@ -24,8 +24,8 @@ abstract contract BridgeBase {
     /// @notice Maximum number of oracles allowed.
     uint8 public constant MAX_ORACLES = 10;
 
-    /// @notice Mapping of oracles to proposals to track which oracle has voted on which proposal.
-    mapping(address => mapping(bytes32 => bool)) public voted;
+    /// @notice Mapping of proposals to oracles votes on them.
+    mapping(bytes32 => mapping(address => bool)) public voted;
 
     /// @notice Emitted when the first vote a proposal has been cast.
     event VoteStarted(bytes32 indexed proposal, address oracle, address indexed user, uint256 amount);
@@ -88,11 +88,8 @@ abstract contract BridgeBase {
     /// @param user The user address associated with the vote.
     /// @param amount The amount of tokens being bridged.
     function _createVote(bytes32 proposal, address oracle, address user, uint256 amount) internal virtual {
-        _mustNotHaveVotedYet(proposal, oracle);
+        _processVote(proposal, oracle);
 
-        voted[oracle][proposal] = true;
-        _incTotalVotes(proposal);
-        _updateLastVote(proposal, block.number);
         emit VoteStarted(proposal, oracle, user, amount);
     }
 
@@ -102,10 +99,16 @@ abstract contract BridgeBase {
     function _recordVote(bytes32 proposal, address oracle) internal virtual {
         _mustNotHaveVotedYet(proposal, oracle);
 
-        voted[oracle][proposal] = true;
+        _processVote(proposal, oracle);
+
+        emit Voted(proposal, oracle);
+    }
+
+    /// @notice Processes a vote for a proposal.
+    function _processVote(bytes32 proposal, address oracle) internal virtual {
+        voted[proposal][oracle] = true;
         _incTotalVotes(proposal);
         _updateLastVote(proposal, block.number);
-        emit Voted(proposal, oracle);
     }
 
     /// @notice Executes a proposal after all conditions are met.
@@ -201,7 +204,7 @@ abstract contract BridgeBase {
     }
 
     function _mustNotHaveVotedYet(bytes32 proposal, address oracle) internal view {
-        if (voted[oracle][proposal]) {
+        if (voted[proposal][oracle]) {
             revert AlreadyVoted(proposal, oracle);
         }
     }

src/bridge/MigrationNFT.sol

@@ -1,5 +1,25 @@
 // SPDX-License-Identifier: BSD-3-Clause-Clear
-
+/*
+ * NOTE: For anyone considering reusing this contract, we recommend reading through the 
+ * matter-labs audit results carefully before proceeding. The Nodle team will not address 
+ * this issue because we may not use this contract in the near future.
+ *
+ * NFT Minting Can Be Blocked
+ * Severity: Medium
+ * Status: Reported
+ *
+ * Users cannot mint NFTs once the `individualHolders` variable exceeds the `maxHolders` 
+ * value, which is enforced by _mustAlreadyBeHolderOrEnoughHoldersRemaining. To mint a 
+ * level-1 NFT, users will need to bridge a relatively small amount of NODL tokens compared 
+ * to other levels. Given that the `individualHolders` variable is shared among all NFT 
+ * levels, malicious actors could exploit this by bridging small amounts of NODL tokens to 
+ * multiple addresses until reaching the `maxHolders` limit and minting level-1 NFTs for 
+ * them. As a result, it will disrupt the minting process for legitimate users.
+ *
+ * Recommendation:
+ * We recommend revising the NFT minting process to allow unlimited NFT minting for lower 
+ * levels.
+ */
 pragma solidity 0.8.23;
 
 import {ERC721} from "openzeppelin-contracts/contracts/token/ERC721/ERC721.sol";
@@ -37,7 +57,7 @@ contract MigrationNFT is ERC721 {
     error NoLevelUp();
     error ProposalDoesNotExist();
     error NotExecuted();
-    error SoulboundIsNotTransferrable();
+    error SoulBoundIsNotTransferrable();
 
     /**
      * @notice Construct a new MigrationNFT contract
@@ -97,7 +117,7 @@ contract MigrationNFT is ERC721 {
 
         _mustBeAnExistingProposal(target);
         _mustBeExecuted(executed);
-        bool alreadyHolder = _mustAlreadyBeHolderOrEnougHoldersRemaining(target);
+        bool alreadyHolder = _mustAlreadyBeHolderOrEnoughHoldersRemaining(target);
 
         (uint256[] memory levelsToMint, uint256 nbLevelsToMint) = _computeLevelUps(target, amount);
 
@@ -157,7 +177,7 @@ contract MigrationNFT is ERC721 {
         }
     }
 
-    function _mustAlreadyBeHolderOrEnougHoldersRemaining(address target) internal view returns (bool alreadyHolder) {
+    function _mustAlreadyBeHolderOrEnoughHoldersRemaining(address target) internal view returns (bool alreadyHolder) {
         alreadyHolder = balanceOf(target) > 0;
         if (!alreadyHolder && individualHolders == maxHolders) {
             revert TooManyHolders();
@@ -167,8 +187,8 @@ contract MigrationNFT is ERC721 {
     function _update(address to, uint256 tokenId, address auth) internal override(ERC721) returns (address) {
         address from = _ownerOf(tokenId);
         if (from != address(0) && to != address(0)) {
-            // only burn or mint is allowed for a soulbound token
-            revert SoulboundIsNotTransferrable();
+            // only burn or mint is allowed for a SoulBound token
+            revert SoulBoundIsNotTransferrable();
         }
 
         return super._update(to, tokenId, auth);

src/bridge/NODLMigration.sol

@@ -17,7 +17,7 @@ contract NODLMigration is BridgeBase {
         bool executed;
     }
 
-    // We track votes in a seperate mapping to avoid having to write helper functions to
+    // We track votes in a separate mapping to avoid having to write helper functions to
     // expose the votes for each proposal.
     mapping(bytes32 => Proposal) public proposals;
 

subquery/README.md

@@ -28,7 +28,7 @@ The simplest way to run your project is by running `yarn dev` or `npm run-script
 
 1.  `yarn codegen` - Generates types from the GraphQL schema definition and contract ABIs and saves them in the `/src/types` directory. This must be done after each change to the `schema.graphql` file or the contract ABIs
 2.  `yarn build` - Builds and packages the SubQuery project into the `/dist` directory
-3.  `docker-compose pull && docker-compose up` - Runs a Docker container with an indexer, PostgeSQL DB, and a query service. This requires [Docker to be installed](https://docs.docker.com/engine/install) and running locally. The configuration for this container is set from your `docker-compose.yml`
+3.  `docker-compose pull && docker-compose up` - Runs a Docker container with an indexer, PostgreSQL DB, and a query service. This requires [Docker to be installed](https://docs.docker.com/engine/install) and running locally. The configuration for this container is set from your `docker-compose.yml`
 
 You can observe the three services start, and once all are running (it may take a few minutes on your first start), please open your browser and head to [http://localhost:3000](http://localhost:3000) - you should see a GraphQL playground showing with the schemas ready to query. [Read the docs for more information](https://academy.subquery.network/run_publish/run.html) or [explore the possible service configuration for running SubQuery](https://academy.subquery.network/run_publish/references.html).
 

subquery/src/utils/utils.ts

@@ -40,10 +40,10 @@ export const fetchMetadata = async (
     return null;
   }
 
-  const strppedCid = cid.replace("ipfs://", "");
+  const strippedCid = cid.replace("ipfs://", "");
 
   const gateway = gateways[0];
-  const url = `https://${gateway}/ipfs/${strppedCid}`;
+  const url = `https://${gateway}/ipfs/${strippedCid}`;
 
   try {
     const res = await fetch(url);
@@ -55,7 +55,7 @@ export const fetchMetadata = async (
     if (err instanceof SyntaxError && toMatch.includes(err.message)) {
       return null;
     }
-    
+
     return fetchMetadata(cid, gateways.slice(1));
   }
 };

test/Grants.t.sol

@@ -418,7 +418,7 @@ contract GrantsTest is Test {
         assertEq(grants.getGrantsCount(bob), 2);
     }
 
-    function test_cancelAuthorityCouldbeADifferentAccount() public {
+    function test_cancelAuthorityCouldBeDifferentFromGranter() public {
         vm.startPrank(alice);
         token.approve(address(grants), 1000);
         grants.addVestingSchedule(bob, block.timestamp + 1 days, 2 days, 4, 100, charlie);
@@ -509,7 +509,7 @@ contract GrantsTest is Test {
         emit log("Test Renounce Grants Limited Page Range Passed!");
     }
 
-    function test_nonCancellabelSchedule() public {
+    function test_nonCancellableSchedule() public {
         vm.startPrank(alice);
         token.approve(address(grants), 400);
         grants.addVestingSchedule(bob, block.timestamp + 1 days, 2 days, 4, 100, address(0));

test/Rewards.t.sol

@@ -113,7 +113,7 @@ contract RewardsTest is Test {
         rewards.mintReward(reward, signature);
     }
 
-    function test_mintRewardInvalidsequence() public {
+    function test_mintRewardInvalidSequence() public {
         // Prepare the reward and signature
         Rewards.Reward memory reward = Rewards.Reward(recipient, 100, 1); // Invalid sequence
         bytes memory signature = createSignature(reward, oraclePrivateKey);
@@ -183,7 +183,7 @@ contract RewardsTest is Test {
         gasAfter = gasleft();
         console.log("Gas used per recipient in a solo:  %d", gasBefore - gasAfter);
         uint256 ratio = (gasBefore - gasAfter) / gasUsedPerRecipient;
-        console.log("Batch efficieny >= %dX", ratio);
+        console.log("Batch efficiency >= %dX", ratio);
 
         assertTrue(ratio >= 1, "Batch efficiency must be at least 1X to be worth it.");
     }
@@ -306,9 +306,9 @@ contract RewardsTest is Test {
         amounts[0] = 100;
         amounts[1] = 200;
 
-        bytes32 receipentsHash = keccak256(abi.encodePacked(recipients));
+        bytes32 recipientsHash = keccak256(abi.encodePacked(recipients));
         bytes32 amountsHash = keccak256(abi.encodePacked(amounts));
-        bytes32 structHash = keccak256(abi.encode(rewards.BATCH_REWARD_TYPE_HASH(), receipentsHash, amountsHash, 0));
+        bytes32 structHash = keccak256(abi.encode(rewards.BATCH_REWARD_TYPE_HASH(), recipientsHash, amountsHash, 0));
         bytes32 digest = MessageHashUtils.toTypedDataHash(domainSeparator, structHash);
         (uint8 v, bytes32 r, bytes32 s) = vm.sign(oraclePrivateKey, digest);
         bytes memory signature = abi.encodePacked(r, s, v);

test/bridge/MigrationNFT.t.sol

@@ -198,11 +198,11 @@ contract MigrationNFTTest is Test {
         assertEq(migrationNFT.nextTokenId(), maxHolders + 1); // we minted `maxHolders` NFTs in the FOR loop + 1 after
     }
 
-    function test_isSoulbound() public {
+    function test_isSoulBound() public {
         vm.bridgeTokens(migration, oracles[0], 0x0, vm.addr(42), levels[0]);
         migrationNFT.safeMint(0x0);
 
-        vm.expectRevert(MigrationNFT.SoulboundIsNotTransferrable.selector);
+        vm.expectRevert(MigrationNFT.SoulBoundIsNotTransferrable.selector);
         migrationNFT.transferFrom(vm.addr(42), vm.addr(43), 0);
     }
 }