fix(protocol kit): Get modules paginated incorrect interface (#787)

packages/protocol-kit/hardhat/deploy/deploy-contracts.ts

@@ -188,6 +188,20 @@ const deploy: DeployFunction = async (hre: HardhatRuntimeEnvironment): Promise<v
     log: true,
     deterministicDeployment: true
   })
+
+  await deploy('StateChannelModule', {
+    from: deployer,
+    args: [],
+    log: true,
+    deterministicDeployment: true
+  })
+
+  await deploy('WhitelistModule', {
+    from: deployer,
+    args: [],
+    log: true,
+    deterministicDeployment: true
+  })
 }
 
 export default deploy

packages/protocol-kit/src/Safe.ts

@@ -37,7 +37,8 @@ import {
   SafeConfigProps,
   SigningMethod,
   SigningMethodType,
-  SwapOwnerTxParams
+  SwapOwnerTxParams,
+  SafeModulesPaginated
 } from './types'
 import {
   EthSafeSignature,
@@ -406,7 +407,7 @@ class Safe {
    * @param pageSize - The size of the page. It will be the max length of the returning array. Must be greater then 0.
    * @returns The list of addresses of all the enabled Safe modules
    */
-  async getModulesPaginated(start: string, pageSize: number = 10): Promise<string[]> {
+  async getModulesPaginated(start: string, pageSize: number = 10): Promise<SafeModulesPaginated> {
     return this.#moduleManager.getModulesPaginated(start, pageSize)
   }
 

packages/protocol-kit/src/contracts/Safe/v1.0.0/SafeContract_v1_0_0.ts

@@ -1,7 +1,7 @@
 import SafeBaseContract from '@safe-global/protocol-kit/contracts/Safe/SafeBaseContract'
 import SafeProvider from '@safe-global/protocol-kit/SafeProvider'
 import { toTxResult } from '@safe-global/protocol-kit/contracts/utils'
-import { sameString } from '@safe-global/protocol-kit/utils'
+import { sameString, isSentinelAddress } from '@safe-global/protocol-kit/utils'
 import {
   SafeVersion,
   SafeContract_v1_0_0_Abi,
@@ -247,15 +247,25 @@ class SafeContract_v1_0_0
     return toTxResult(txResponse, options)
   }
 
-  async getModulesPaginated(start: string, pageSize: bigint): Promise<string[]> {
+  async getModulesPaginated([start, pageSize]: [string, bigint]): Promise<[string[], string]> {
     if (pageSize <= 0) throw new Error('Invalid page size for fetching paginated modules')
 
+    const size = Number(pageSize)
     const [array] = await this.getModules()
-    if (start === SENTINEL_ADDRESS) {
-      return array.slice(0, Number(pageSize))
+
+    if (isSentinelAddress(start)) {
+      const next = pageSize < array.length ? array[size] : SENTINEL_ADDRESS
+      return [array.slice(0, size), next]
     } else {
       const moduleIndex = array.findIndex((module: string) => sameString(module, start))
-      return moduleIndex === -1 ? [] : array.slice(moduleIndex + 1, Number(pageSize))
+      if (moduleIndex === -1) {
+        return [[], SENTINEL_ADDRESS]
+      }
+
+      const nextElementIndex = moduleIndex + 1
+      const nextPageAddress =
+        nextElementIndex + size < array.length ? array[nextElementIndex + size] : SENTINEL_ADDRESS
+      return [array.slice(moduleIndex + 1, nextElementIndex + size), nextPageAddress]
     }
   }
 

packages/protocol-kit/src/managers/moduleManager.ts

@@ -1,6 +1,9 @@
 import { isRestrictedAddress, sameString } from '@safe-global/protocol-kit/utils/address'
 import { SENTINEL_ADDRESS } from '@safe-global/protocol-kit/utils/constants'
-import { SafeContractImplementationType } from '@safe-global/protocol-kit/types'
+import {
+  SafeContractImplementationType,
+  SafeModulesPaginated
+} from '@safe-global/protocol-kit/types'
 import SafeProvider from '../SafeProvider'
 
 class ModuleManager {
@@ -46,17 +49,13 @@ class ModuleManager {
     return [...modules]
   }
 
-  //TODO: Implement getModulesPaginated in the new code
-  async getModulesPaginated(start: string, pageSize: number): Promise<string[]> {
-    console.log('getModulesPaginated', start, pageSize)
-    return []
-    // if (!this.#safeContract) {
-    //   throw new Error('Safe is not deployed')
-    // }
-
-    // const [modules] = await this.#safeContract.getModulesPaginated(start, pageSize)
+  async getModulesPaginated(start: string, pageSize: number): Promise<SafeModulesPaginated> {
+    if (!this.#safeContract) {
+      throw new Error('Safe is not deployed')
+    }
 
-    // return [...modules]
+    const [modules, next] = await this.#safeContract.getModulesPaginated([start, BigInt(pageSize)])
+    return { modules: modules as string[], next }
   }
 
   async isModuleEnabled(moduleAddress: string): Promise<boolean> {

packages/protocol-kit/src/types/safeProvider.ts

@@ -29,3 +29,8 @@ export type SafeProviderTransaction = {
   maxFeePerGas?: number | string
   maxPriorityFeePerGas?: number | string
 }
+
+export type SafeModulesPaginated = {
+  modules: string[]
+  next: string
+}

packages/protocol-kit/src/utils/address.ts

@@ -8,7 +8,7 @@ export function isZeroAddress(address: string): boolean {
   return sameString(address, ZERO_ADDRESS)
 }
 
-function isSentinelAddress(address: string): boolean {
+export function isSentinelAddress(address: string): boolean {
   return sameString(address, SENTINEL_ADDRESS)
 }
 

packages/protocol-kit/tests/e2e/moduleManager.test.ts

@@ -11,11 +11,14 @@ import { getContractNetworks } from './utils/setupContractNetworks'
 import {
   getDailyLimitModule,
   getSafeWithOwners,
-  getSocialRecoveryModule
+  getSocialRecoveryModule,
+  getStateChannelModule,
+  getWhiteListModule
 } from './utils/setupContracts'
 import { getEip1193Provider } from './utils/setupProvider'
 import { getAccounts } from './utils/setupTestNetwork'
 import { waitSafeTxReceipt } from './utils/transactions'
+import semverSatisfies from 'semver/functions/satisfies'
 
 chai.use(chaiAsPromised)
 
@@ -39,6 +42,8 @@ describe('Safe modules manager', () => {
     return {
       dailyLimitModule: await getDailyLimitModule(),
       socialRecoveryModule: await getSocialRecoveryModule(),
+      stateChannelModule: await getStateChannelModule(),
+      whiteListModule: await getWhiteListModule(),
       safe: await getSafeWithOwners([accounts[0].address]),
       accounts,
       contractNetworks,
@@ -86,8 +91,7 @@ describe('Safe modules manager', () => {
     })
   })
 
-  //TODO: Fix getModulesPaginated tests
-  describe.skip('getModulesPaginated', async () => {
+  describe('getModulesPaginated', async () => {
     it('should fail if the Safe is not deployed', async () => {
       const { predictedSafe, contractNetworks, provider } = await setupTests()
       const safeSdk = await Safe.create({
@@ -108,62 +112,136 @@ describe('Safe modules manager', () => {
         safeAddress,
         contractNetworks
       })
-      chai.expect((await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)).length).to.be.eq(0)
+
+      const emptyModuleList = await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)
       const tx = await safeSdk.createEnableModuleTx(await dailyLimitModule.getAddress())
       const txResponse = await safeSdk.executeTransaction(tx)
       await waitSafeTxReceipt(txResponse)
-      chai.expect((await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)).length).to.be.eq(1)
+      const moduleList = await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)
+
+      chai.expect(emptyModuleList.modules.length).to.be.eq(0)
+      chai.expect(emptyModuleList.next).to.be.eq(SENTINEL_ADDRESS)
+      chai.expect(moduleList.modules.length).to.be.eq(1)
+      chai.expect(emptyModuleList.next).to.be.eq(SENTINEL_ADDRESS)
     })
 
     it('should constraint returned modules by pageSize', async () => {
-      const { safe, dailyLimitModule, contractNetworks, socialRecoveryModule, provider } =
-        await setupTests()
+      const {
+        safe,
+        dailyLimitModule,
+        contractNetworks,
+        socialRecoveryModule,
+        stateChannelModule,
+        whiteListModule,
+        provider
+      } = await setupTests()
       const safeAddress = await safe.getAddress()
       const dailyLimitsAddress = await dailyLimitModule.getAddress()
       const socialRecoveryAddress = await socialRecoveryModule.getAddress()
+      const stateChannelAddress = await stateChannelModule.getAddress()
+      const whiteListAddress = await whiteListModule.getAddress()
       const safeSdk = await Safe.create({
         provider,
         safeAddress,
         contractNetworks
       })
+      const currentPageNext = semverSatisfies(await safeSdk.getContractVersion(), '>=1.4.1')
+
+      chai
+        .expect((await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)).modules.length)
+        .to.be.eq(0)
+
+      const moduleDeployment = [
+        dailyLimitsAddress,
+        socialRecoveryAddress,
+        stateChannelAddress,
+        whiteListAddress
+      ].map(async (moduleAddress) => {
+        const txModule = await safeSdk.createEnableModuleTx(moduleAddress)
+        const moduleResponse = await safeSdk.executeTransaction(txModule)
+        await waitSafeTxReceipt(moduleResponse)
+      })
+
+      await Promise.all(moduleDeployment)
+
+      const modules1 = await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)
+      const modules2 = await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 1)
+      const modules3 = await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 2)
 
-      chai.expect((await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)).length).to.be.eq(0)
-      const txDailyLimits = await safeSdk.createEnableModuleTx(dailyLimitsAddress)
-      const dailyLimitsResponse = await safeSdk.executeTransaction(txDailyLimits)
-      await waitSafeTxReceipt(dailyLimitsResponse)
-      const txSocialRecovery = await safeSdk.createEnableModuleTx(socialRecoveryAddress)
-      const soecialRecoveryResponse = await safeSdk.executeTransaction(txSocialRecovery)
-      await waitSafeTxReceipt(soecialRecoveryResponse)
-
-      chai.expect((await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)).length).to.be.eq(2)
-      chai.expect((await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 1)).length).to.be.eq(1)
-      chai.expect((await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 1)).length).to.be.eq(1)
+      chai.expect(modules1.modules.length).to.be.eq(4)
+      chai
+        .expect(modules1.modules)
+        .to.deep.eq([
+          whiteListAddress,
+          stateChannelAddress,
+          socialRecoveryAddress,
+          dailyLimitsAddress
+        ])
+      chai.expect(modules1.next).to.be.eq(SENTINEL_ADDRESS)
+
+      chai.expect(modules2.modules.length).to.be.eq(1)
+      chai.expect(modules2.modules).to.deep.eq([whiteListAddress])
+      chai.expect(modules2.next).to.be.eq(currentPageNext ? whiteListAddress : stateChannelAddress)
+
+      chai.expect(modules3.modules.length).to.be.eq(2)
+      chai.expect(modules3.modules).to.deep.eq([whiteListAddress, stateChannelAddress])
+      chai
+        .expect(modules3.next)
+        .to.be.eq(currentPageNext ? stateChannelAddress : socialRecoveryAddress)
     })
 
     it('should offset the returned modules', async () => {
-      const { safe, dailyLimitModule, contractNetworks, socialRecoveryModule, provider } =
-        await setupTests()
+      const {
+        safe,
+        dailyLimitModule,
+        contractNetworks,
+        socialRecoveryModule,
+        stateChannelModule,
+        whiteListModule,
+        provider
+      } = await setupTests()
       const safeAddress = await safe.getAddress()
-      const dailyLimitsAddress = await await dailyLimitModule.getAddress()
-      const socialRecoveryAddress = await await socialRecoveryModule.getAddress()
+      const dailyLimitsAddress = await dailyLimitModule.getAddress()
+      const socialRecoveryAddress = await socialRecoveryModule.getAddress()
+      const stateChannelAddress = await stateChannelModule.getAddress()
+      const whiteListAddress = await whiteListModule.getAddress()
       const safeSdk = await Safe.create({
         provider,
         safeAddress,
         contractNetworks
       })
+      const currentPageNext = semverSatisfies(await safeSdk.getContractVersion(), '>=1.4.1')
+
+      const moduleDeployment = [
+        dailyLimitsAddress,
+        socialRecoveryAddress,
+        stateChannelAddress,
+        whiteListAddress
+      ].map(async (moduleAddress) => {
+        const txModule = await safeSdk.createEnableModuleTx(moduleAddress)
+        const moduleResponse = await safeSdk.executeTransaction(txModule)
+        await waitSafeTxReceipt(moduleResponse)
+      })
 
-      const txDailyLimits = await safeSdk.createEnableModuleTx(dailyLimitsAddress)
-      const dailyLimitsResponse = await safeSdk.executeTransaction(txDailyLimits)
-      await waitSafeTxReceipt(dailyLimitsResponse)
-      const txSocialRecovery = await safeSdk.createEnableModuleTx(socialRecoveryAddress)
-      const soecialRecoveryResponse = await safeSdk.executeTransaction(txSocialRecovery)
-      await waitSafeTxReceipt(soecialRecoveryResponse)
+      await Promise.all(moduleDeployment)
 
-      const [firstModule, secondModule] = await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)
+      const {
+        modules: [firstModule, secondModule, thirdModule, fourthModule]
+      } = await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)
 
-      chai.expect((await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)).length).to.be.eq(2)
-      chai.expect((await safeSdk.getModulesPaginated(firstModule, 10)).length).to.be.eq(1)
-      chai.expect((await safeSdk.getModulesPaginated(secondModule, 10)).length).to.be.eq(0)
+      const modules1 = await safeSdk.getModulesPaginated(firstModule, 10)
+      const modules2 = await safeSdk.getModulesPaginated(firstModule, 2)
+      const modules3 = await safeSdk.getModulesPaginated(firstModule, 3)
+
+      chai
+        .expect((await safeSdk.getModulesPaginated(SENTINEL_ADDRESS, 10)).modules.length)
+        .to.be.eq(4)
+      chai.expect(modules1.modules).to.deep.eq([secondModule, thirdModule, fourthModule])
+      chai.expect(modules1.next).to.be.eq(SENTINEL_ADDRESS)
+      chai.expect(modules2.modules).to.deep.eq([secondModule, thirdModule])
+      chai.expect(modules2.next).to.be.eq(currentPageNext ? thirdModule : fourthModule)
+      chai.expect(modules3.modules).to.deep.eq([secondModule, thirdModule, fourthModule])
+      chai.expect(modules3.next).to.be.eq(SENTINEL_ADDRESS)
     })
 
     it('should fail if pageSize is invalid', async () => {

packages/protocol-kit/tests/e2e/utils/setupContracts.ts

@@ -175,6 +175,18 @@ export const getSocialRecoveryModule = async (): Promise<Contract> => {
   return SocialRecoveryModule.attach(SocialRecoveryModuleDeployment.address)
 }
 
+export const getStateChannelModule = async (): Promise<Contract> => {
+  const StateChannelModuleDeployment = await deployments.get('StateChannelModule')
+  const StateChannelModule = await ethers.getContractFactory('StateChannelModule')
+  return StateChannelModule.attach(StateChannelModuleDeployment.address)
+}
+
+export const getWhiteListModule = async (): Promise<Contract> => {
+  const WhiteListModuleDeployment = await deployments.get('WhitelistModule')
+  const WhiteListModule = await ethers.getContractFactory('WhitelistModule')
+  return WhiteListModule.attach(WhiteListModuleDeployment.address)
+}
+
 export const getERC20Mintable = async (): Promise<Contract> => {
   const ERC20MintableDeployment = await deployments.get('ERC20Mintable')
   const ERC20Mintable = await ethers.getContractFactory('ERC20Mintable')