Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
14 changes: 14 additions & 0 deletions packages/protocol-kit/hardhat/deploy/deploy-contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 3 additions & 2 deletions packages/protocol-kit/src/Safe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ import {
SafeConfigProps,
SigningMethod,
SigningMethodType,
SwapOwnerTxParams
SwapOwnerTxParams,
SafeModulesPaginated
} from './types'
import {
EthSafeSignature,
Expand Down Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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]
}
}

Expand Down
21 changes: 10 additions & 11 deletions packages/protocol-kit/src/managers/moduleManager.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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> {
Expand Down
5 changes: 5 additions & 0 deletions packages/protocol-kit/src/types/safeProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,8 @@ export type SafeProviderTransaction = {
maxFeePerGas?: number | string
maxPriorityFeePerGas?: number | string
}

export type SafeModulesPaginated = {
modules: string[]
next: string
}
2 changes: 1 addition & 1 deletion packages/protocol-kit/src/utils/address.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
142 changes: 110 additions & 32 deletions packages/protocol-kit/tests/e2e/moduleManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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,
Expand Down Expand Up @@ -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({
Expand All @@ -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 () => {
Expand Down
12 changes: 12 additions & 0 deletions packages/protocol-kit/tests/e2e/utils/setupContracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
Loading