Fix get modules paginated

.prettierrc.json

@@ -0,0 +1,6 @@
+{
+  "trailingComma": "es5",
+  "tabWidth": 2,
+  "semi": true,
+  "singleQuote": false
+}

.vscode/settings.json

@@ -0,0 +1,9 @@
+{
+    "editor.tabSize": 2,
+    "editor.formatOnSave": true,
+    "typescript.tsdk": "node_modules/typescript/lib",
+    "typescript.enablePromptUseWorkspaceTsdk": true,
+    "editor.codeActionsOnSave": {
+      "source.fixAll.eslint": true
+    },
+  }

contracts/core/Modifier.sol

@@ -18,6 +18,9 @@ abstract contract Modifier is Module, IAvatar {
     /// `module` is invalid.
     error InvalidModule(address module);
 
+    /// `pageSize` is invalid.
+    error InvalidPageSize();
+
     /// `module` is already disabled.
     error AlreadyDisabledModule(address module);
 
@@ -111,31 +114,50 @@ abstract contract Modifier is Module, IAvatar {
     }
 
     /// @dev Returns array of modules.
-    /// @param start Start of the page.
-    /// @param pageSize Maximum number of modules that should be returned.
+    ///      If all entries fit into a single page, the next pointer will be 0x1.
+    ///      If another page is present, next will be the last element of the returned array.
+    /// @param start Start of the page. Has to be a module or start pointer (0x1 address)
+    /// @param pageSize Maximum number of modules that should be returned. Has to be > 0
     /// @return array Array of modules.
     /// @return next Start of the next page.
     function getModulesPaginated(
         address start,
         uint256 pageSize
     ) external view override returns (address[] memory array, address next) {
-        /// Init array with max page size.
+        if (start != SENTINEL_MODULES && !isModuleEnabled(start)) {
+            revert InvalidModule(start);
+        }
+        if (pageSize == 0) {
+            revert InvalidPageSize();
+        }
+
+        // Init array with max page size
         array = new address[](pageSize);
 
-        /// Populate return array.
+        // Populate return array
         uint256 moduleCount = 0;
-        address currentModule = modules[start];
+        next = modules[start];
         while (
-            currentModule != address(0x0) &&
-            currentModule != SENTINEL_MODULES &&
+            next != address(0) &&
+            next != SENTINEL_MODULES &&
             moduleCount < pageSize
         ) {
-            array[moduleCount] = currentModule;
-            currentModule = modules[currentModule];
+            array[moduleCount] = next;
+            next = modules[next];
             moduleCount++;
         }
-        next = currentModule;
-        /// Set correct size of returned array.
+
+        // Because of the argument validation we can assume that
+        // the `currentModule` will always be either a module address
+        // or sentinel address (aka the end). If we haven't reached the end
+        // inside the loop, we need to set the next pointer to the last element
+        // because it skipped over to the next module which is neither included
+        // in the current page nor won't be included in the next one
+        // if you pass it as a start.
+        if (next != SENTINEL_MODULES) {
+            next = array[moduleCount - 1];
+        }
+        // Set correct size of returned array
         // solhint-disable-next-line no-inline-assembly
         assembly {
             mstore(array, moduleCount)

hardhat.config.ts

@@ -1,9 +1,5 @@
-import "@typechain/hardhat";
-import "@nomiclabs/hardhat-ethers";
-import "@nomiclabs/hardhat-etherscan";
-import "@nomiclabs/hardhat-waffle";
-import "solidity-coverage";
-import "hardhat-deploy";
+import "@nomicfoundation/hardhat-toolbox";
+
 import dotenv from "dotenv";
 import type { HttpNetworkUserConfig } from "hardhat/types";
 import yargs from "yargs";

package.json

@@ -23,7 +23,7 @@
   "scripts": {
     "build": "hardhat compile",
     "build:sdk": "rm -rf dist && yarn generate:types && tsc -p tsconfig.cjs.json && tsc -p tsconfig.esm.json",
-    "clean": "rm -rf build typechain-types",
+    "clean": "rm -rf dist build typechain-types",
     "deploy": "hardhat singleton-deployment --network",
     "test:sdk": "hardhat test ./sdk/factory/__tests__/index.spec.ts",
     "test": "hardhat test && yarn test:sdk",
@@ -49,43 +49,40 @@
     "url": "https://github.com/gnosis/zodiac.git"
   },
   "devDependencies": {
-    "@nomiclabs/hardhat-ethers": "^2.2.0",
-    "@nomiclabs/hardhat-etherscan": "^3.1.1",
-    "@nomiclabs/hardhat-waffle": "^2.0.3",
-    "@typechain/ethers-v5": "^10.1.1",
-    "@typechain/hardhat": "^6.1.4",
+    "@nomicfoundation/hardhat-chai-matchers": "^1.0.5",
+    "@nomicfoundation/hardhat-network-helpers": "^1.0.7",
+    "@nomicfoundation/hardhat-toolbox": "^2.0.0",
+    "@nomiclabs/hardhat-ethers": "2.2.3",
+    "@nomiclabs/hardhat-etherscan": "3.1.7",
+    "@typechain/ethers-v5": "^11.1.0",
+    "@typechain/hardhat": "^6.1.5",
     "@types/chai": "^4.3.3",
-    "@types/ethereumjs-abi": "^0.6.3",
     "@types/mocha": "^10.0.0",
     "@types/node": "^18.8.5",
     "@types/yargs": "^17.0.13",
     "@typescript-eslint/eslint-plugin": "^5.40.0",
     "@typescript-eslint/parser": "^5.40.0",
-    "chai": "^4.3.6",
-    "debug": "^4.3.4",
+    "chai": "^4.3.7",
     "dotenv": "^16.0.3",
     "eslint": "^8.25.0",
     "eslint-config-prettier": "^8.5.0",
     "eslint-plugin-import": "^2.26.0",
     "eslint-plugin-no-only-tests": "^3.0.0",
     "eslint-plugin-prettier": "^4.2.1",
-    "ethereum-waffle": "^3.4.4",
-    "ethereumjs-abi": "^0.6.8",
-    "ethereumjs-util": "^7.1.5",
-    "hardhat": "^2.12.0",
+    "hardhat": "^2.14.0",
     "hardhat-change-network": "^0.0.7",
-    "hardhat-deploy": "^0.11.18",
+    "hardhat-deploy": "^0.11.28",
+    "hardhat-gas-reporter": "^1.0.9",
     "husky": "^8.0.1",
-    "prettier": "^2.7.1",
-    "prettier-plugin-solidity": "^1.0.0-beta.24",
-    "rimraf": "^3.0.2",
-    "solc": "^0.8.17",
+    "prettier": "^2.8.8",
+    "prettier-plugin-solidity": "^1.1.3",
+    "rimraf": "^5.0.1",
     "solhint": "^3.3.7",
     "solhint-plugin-prettier": "^0.0.5",
-    "solidity-coverage": "^0.8.2",
+    "solidity-coverage": "^0.8.4",
     "ts-node": "^10.9.1",
-    "typechain": "^8.1.1",
-    "typescript": "^4.8.4",
+    "typechain": "^8.3.0",
+    "typescript": "^5.1.6",
     "yargs": "^17.6.0"
   },
   "dependencies": {

sdk/factory/__tests__/index.spec.ts

@@ -1,6 +1,7 @@
+import { loadFixture } from "@nomicfoundation/hardhat-network-helpers";
 import { expect } from "chai";
 import { Contract } from "ethers";
-import { ethers } from "hardhat";
+import hre from "hardhat";
 
 import {
   ContractAddresses,
@@ -14,27 +15,26 @@ import {
   getModuleFactoryAndMasterCopy,
 } from "../moduleDeployer";
 
-import "@nomiclabs/hardhat-ethers";
 import { KnownContracts } from "../types";
 
 const AddressOne = "0x0000000000000000000000000000000000000001";
 
 describe("Factory JS functions ", () => {
-  //let newModuleAddress: string;
-  let chainId: number;
-  let mockContract: Contract;
-
   const saltNonce = "0x7255";
-  const provider = ethers.provider;
 
-  before(async () => {
-    const Mock = await ethers.getContractFactory("MockContract");
-    mockContract = await Mock.deploy();
-    chainId = await (await provider.getNetwork()).chainId;
-  });
+  async function setup() {
+    const Mock = await hre.ethers.getContractFactory("MockContract");
+    const mock = await Mock.deploy();
+    const chainId = (await hre.ethers.provider.getNetwork()).chainId;
+
+    return { mock, chainId };
+  }
 
   it("should execute transaction and retrieve expected address ", async () => {
-    const [signer] = await ethers.getSigners();
+    const { mock, chainId } = await loadFixture(setup);
+
+    const [signer] = await hre.ethers.getSigners();
+
     const args = {
       values: [
         AddressOne,
@@ -61,7 +61,7 @@ describe("Factory JS functions ", () => {
       await deployAndSetUpModule(
         KnownContracts.REALITY_ETH,
         args,
-        provider,
+        hre.ethers.provider,
         chainId,
         saltNonce
       );
@@ -72,11 +72,11 @@ describe("Factory JS functions ", () => {
     expect(receipt.transactionHash).to.be.a("string");
     expect(receipt.status).to.be.eq(1);
     expect(expectedModuleAddress).to.a("string");
-    //newModuleAddress = expectedModuleAddress;
   });
 
   it("should execute transaction and retrieve expected address when providing the address and ABI directly", async () => {
-    const [signer] = await ethers.getSigners();
+    const { chainId } = await loadFixture(setup);
+    const [signer] = await hre.ethers.getSigners();
     const args = {
       values: [
         AddressOne,
@@ -109,7 +109,7 @@ describe("Factory JS functions ", () => {
         masterCopyAddress,
         abi,
         args,
-        provider,
+        hre.ethers.provider,
         chainId,
         saltNonce
       );
@@ -120,16 +120,17 @@ describe("Factory JS functions ", () => {
     expect(receipt.transactionHash).to.be.a("string");
     expect(receipt.status).to.be.eq(1);
     expect(expectedModuleAddress).to.a("string");
-    //newModuleAddress = expectedModuleAddress;
   });
 
   it("should retrieve module instance", async () => {
+    const { mock } = await loadFixture(setup);
+
     const module = await getModuleInstance(
       KnownContracts.REALITY_ETH,
-      mockContract.address,
-      provider
+      mock.address,
+      hre.ethers.provider
     );
-    await mockContract.givenMethodReturnBool(
+    await mock.givenMethodReturnBool(
       module.interface.getSighash("owner"),
       true
     );
@@ -140,10 +141,12 @@ describe("Factory JS functions ", () => {
   });
 
   it("should retrieve factory and module instance", async () => {
+    const { chainId } = await loadFixture(setup);
+
     const { moduleFactory, moduleMastercopy } =
       await getModuleFactoryAndMasterCopy(
         KnownContracts.REALITY_ETH,
-        provider,
+        hre.ethers.provider,
         chainId
       );
     expect(moduleFactory).to.be.instanceOf(Contract);

sdk/factory/moduleDeployer.ts

@@ -1,13 +1,13 @@
 import { Provider } from "@ethersproject/providers";
 import { ethers, Contract, Signer, BigNumber } from "ethers";
-import { ModuleProxyFactory__factory } from "../types";
-
 import {
   ContractAddresses,
   ContractAbis,
   SupportedNetworks,
   ContractFactories,
 } from "../contracts";
+import { ModuleProxyFactory__factory } from "../types";
+
 import { KnownContracts } from "./types";
 
 type ABI = any[] | readonly any[];
@@ -175,7 +175,7 @@ export const getModuleInstance = <T extends KnownContracts>(
   return ContractFactories[moduleName].connect(
     moduleAddress,
     provider
-  ) as ReturnType<typeof ContractFactories[T]["connect"]>;
+  ) as ReturnType<(typeof ContractFactories)[T]["connect"]>;
 };
 
 export const getModuleFactoryAndMasterCopy = <T extends KnownContracts>(

tasks/deploy-replay.ts

@@ -1,3 +1,4 @@
+import assert from "assert";
 import { task } from "hardhat/config";
 import { HardhatRuntimeEnvironment } from "hardhat/types";
 import {
@@ -27,12 +28,10 @@ export const deploy = async (
       const [deployer] = await hre.ethers.getSigners();
       const signer = hre.ethers.provider.getSigner(deployer.address);
       for (let index = 0; index < contracts.length; index++) {
-        const initData: InitData = MasterCopyInitData[contracts[index]];
-        if (
-          MasterCopyInitData[contracts[index]] &&
-          initData.initCode &&
-          initData.salt
-        ) {
+        const initData: InitData | undefined =
+          MasterCopyInitData[contracts[index]];
+
+        if (initData && initData.initCode && initData.salt) {
           console.log(`    \x1B[4m${contracts[index]}\x1B[0m`);
           try {
             await deployMastercopyWithInitData(

tasks/singleton-deployment.ts

@@ -3,7 +3,7 @@ import { HardhatRuntimeEnvironment } from "hardhat/types";
 import { KnownContracts, MasterCopyInitData } from "../sdk/factory";
 import { deployModuleFactory } from "../sdk/factory/deployModuleFactory";
 
-const FactoryInitCode = MasterCopyInitData[KnownContracts.FACTORY].initCode;
+const FactoryInitCode = MasterCopyInitData[KnownContracts.FACTORY]?.initCode;
 
 export const deploy = async (_: null, hre: HardhatRuntimeEnvironment) => {
   const Factory = await hre.ethers.getContractFactory("ModuleProxyFactory");