Skip to content

Commit

Permalink
fix: fix and optimise fetching the recovery state (#2807)
Browse files Browse the repository at this point in the history
* fix: fix and optimise fetching the recovery state

* fix: review issues

* fix: use existing utils function
  • Loading branch information
schmanu committed Nov 14, 2023
1 parent 542391d commit 3ee9b08
Show file tree
Hide file tree
Showing 2 changed files with 115 additions and 75 deletions.
129 changes: 72 additions & 57 deletions src/services/recovery/__tests__/recovery-state.test.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import { faker } from '@faker-js/faker'
import { BigNumber } from 'ethers'
import { BigNumber, ethers } from 'ethers'
import { JsonRpcProvider } from '@ethersproject/providers'
import type { Delay, TransactionAddedEvent } from '@gnosis.pm/zodiac/dist/cjs/types/Delay'
import type { TransactionReceipt } from '@ethersproject/abstract-provider'

import {
getRecoveryState,
_getQueuedTransactionsAdded,
_getRecoveryQueueItem,
_getSafeCreationReceipt,
} from '../recovery-state'
import { getRecoveryState, _getRecoveryQueueItem, _getSafeCreationReceipt } from '../recovery-state'
import { useWeb3ReadOnly } from '@/hooks/wallets/web3'
import { cloneDeep } from 'lodash'

jest.mock('@/hooks/wallets/web3')

Expand All @@ -22,43 +18,6 @@ describe('recovery-state', () => {
_getSafeCreationReceipt.cache.clear?.()
})

describe('getQueuedTransactionsAdded', () => {
it('should filter queued transactions with queueNonce >= current txNonce', () => {
const transactionsAdded = [
{
args: {
queueNonce: BigNumber.from(1),
},
} as unknown,
{
args: {
queueNonce: BigNumber.from(2),
},
} as unknown,
{
args: {
queueNonce: BigNumber.from(3),
},
} as unknown,
] as Array<TransactionAddedEvent>

const txNonce = BigNumber.from(2)

expect(_getQueuedTransactionsAdded(transactionsAdded, txNonce)).toStrictEqual([
{
args: {
queueNonce: BigNumber.from(2),
},
} as unknown,
{
args: {
queueNonce: BigNumber.from(3),
},
},
])
})
})

describe('getRecoveryQueueItem', () => {
it('should return a recovery queue item', async () => {
const transactionAdded = {
Expand Down Expand Up @@ -191,9 +150,9 @@ describe('recovery-state', () => {
const safeAddress = faker.finance.ethereumAddress()
const transactionService = faker.internet.url({ appendSlash: false })
const transactionHash = `0x${faker.string.hexadecimal()}`
const blockHash = faker.string.alphanumeric()
const blockNumber = faker.number.int()
const provider = {
getTransactionReceipt: () => Promise.resolve({ blockHash } as TransactionReceipt),
getTransactionReceipt: () => Promise.resolve({ blockNumber } as TransactionReceipt),
} as unknown as JsonRpcProvider

global.fetch = jest.fn().mockImplementation((_url: string) => {
Expand All @@ -208,14 +167,8 @@ describe('recovery-state', () => {
const txExpiration = BigNumber.from(0)
const txCooldown = BigNumber.from(69420)
const txNonce = BigNumber.from(2)
const queueNonce = BigNumber.from(3)
const queueNonce = BigNumber.from(4)
const transactionsAdded = [
{
getBlock: () => Promise.resolve({ timestamp: 69 }),
args: {
queueNonce: BigNumber.from(1),
},
} as unknown,
{
getBlock: () => Promise.resolve({ timestamp: 420 }),
args: {
Expand All @@ -231,9 +184,13 @@ describe('recovery-state', () => {
] as Array<TransactionAddedEvent>

const queryFilterMock = jest.fn()
const defaultTransactionAddedFilter = {
address: faker.finance.ethereumAddress(),
topics: [ethers.utils.id('TransactionAdded(uint256,bytes32,address,uint256,bytes,uint8)')],
}
const delayModifier = {
filters: {
TransactionAdded: () => ({}),
TransactionAdded: () => cloneDeep(defaultTransactionAddedFilter),
},
address: faker.finance.ethereumAddress(),
getModulesPaginated: () => Promise.resolve([modules]),
Expand All @@ -260,20 +217,78 @@ describe('recovery-state', () => {
queueNonce,
queue: [
{
...transactionsAdded[1],
...transactionsAdded[0],
timestamp: 420,
validFrom: BigNumber.from(420).add(txCooldown),
expiresAt: null,
},
{
...transactionsAdded[2],
...transactionsAdded[1],
timestamp: 69420,
validFrom: BigNumber.from(69420).add(txCooldown),
expiresAt: null,
},
],
})
expect(queryFilterMock).toHaveBeenCalledWith(delayModifier.filters.TransactionAdded(), blockHash)
expect(queryFilterMock).toHaveBeenCalledWith(
{
...defaultTransactionAddedFilter,
topics: [
...defaultTransactionAddedFilter.topics,
[ethers.utils.hexZeroPad('0x2', 32), ethers.utils.hexZeroPad('0x3', 32)],
],
},
blockNumber,
'latest',
)
})

it('should not query data if the queueNonce equals the txNonce', async () => {
const safeAddress = faker.finance.ethereumAddress()
const transactionService = faker.internet.url({ appendSlash: true })
const provider = {} as unknown as JsonRpcProvider

const modules = [faker.finance.ethereumAddress()]
const txExpiration = BigNumber.from(0)
const txCooldown = BigNumber.from(69420)
const txNonce = BigNumber.from(2)
const queueNonce = BigNumber.from(2)

const queryFilterMock = jest.fn()
const defaultTransactionAddedFilter = {
address: faker.finance.ethereumAddress(),
topics: [ethers.utils.id('TransactionAdded(uint256,bytes32,address,uint256,bytes,uint8)')],
}
const delayModifier = {
filters: {
TransactionAdded: () => cloneDeep(defaultTransactionAddedFilter),
},
address: faker.finance.ethereumAddress(),
getModulesPaginated: () => Promise.resolve([modules]),
txExpiration: () => Promise.resolve(txExpiration),
txCooldown: () => Promise.resolve(txCooldown),
txNonce: () => Promise.resolve(txNonce),
queueNonce: () => Promise.resolve(queueNonce),
queryFilter: queryFilterMock.mockRejectedValue('Not required'),
}

const recoveryState = await getRecoveryState({
delayModifier: delayModifier as unknown as Delay,
safeAddress,
transactionService,
provider,
})

expect(recoveryState).toStrictEqual({
address: delayModifier.address,
modules,
txExpiration,
txCooldown,
txNonce,
queueNonce,
queue: [],
})
expect(queryFilterMock).not.toHaveBeenCalled()
})
})
})
61 changes: 43 additions & 18 deletions src/services/recovery/recovery-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,11 @@ import type { JsonRpcProvider } from '@ethersproject/providers'
import type { TransactionReceipt } from '@ethersproject/abstract-provider'

import type { RecoveryQueueItem, RecoveryState } from '@/store/recoverySlice'
import { hexZeroPad } from 'ethers/lib/utils'
import { trimTrailingSlash } from '@/utils/url'

const MAX_PAGE_SIZE = 100

export const _getQueuedTransactionsAdded = (
transactionsAdded: Array<TransactionAddedEvent>,
txNonce: BigNumber,
): Array<TransactionAddedEvent> => {
// Only queued transactions with queueNonce >= current txNonce
return transactionsAdded.filter(({ args }) => args.queueNonce.gte(txNonce))
}

export const _getRecoveryQueueItem = async (
transactionAdded: TransactionAddedEvent,
txCooldown: BigNumber,
Expand Down Expand Up @@ -48,7 +42,7 @@ export const _getSafeCreationReceipt = memoize(
safeAddress: string
provider: JsonRpcProvider
}): Promise<TransactionReceipt> => {
const url = `${transactionService}/v1/${safeAddress}/creation/`
const url = `${trimTrailingSlash(transactionService)}/api/v1/safes/${safeAddress}/creation/`

const { transactionHash } = await fetch(url).then((res) => {
if (res.ok && res.status === 200) {
Expand All @@ -63,30 +57,61 @@ export const _getSafeCreationReceipt = memoize(
({ transactionService, safeAddress }) => transactionService + safeAddress,
)

const queryAddedTransactions = async (
delayModifier: Delay,
queueNonce: BigNumber,
txNonce: BigNumber,
transactionService: string,
provider: JsonRpcProvider,
safeAddress: string,
) => {
if (queueNonce.eq(txNonce)) {
// There are no queued txs
return []
}

const transactionAddedFilter = delayModifier.filters.TransactionAdded()

if (transactionAddedFilter.topics) {
// We filter for the valid nonces while fetching the event logs.
// The nonce has to be one between the current queueNonce and the txNonce.
const diff = queueNonce.sub(txNonce).toNumber()
const queueNonceFilter = Array.from({ length: diff }, (_, idx) => hexZeroPad(txNonce.add(idx).toHexString(), 32))
transactionAddedFilter.topics[1] = queueNonceFilter
}

const { blockNumber } = await _getSafeCreationReceipt({ transactionService, provider, safeAddress })

return await delayModifier.queryFilter(transactionAddedFilter, blockNumber, 'latest')
}

export const getRecoveryState = async ({
delayModifier,
...rest
transactionService,
safeAddress,
provider,
}: {
delayModifier: Delay
transactionService: string
safeAddress: string
provider: JsonRpcProvider
}): Promise<RecoveryState[number]> => {
const { blockHash } = await _getSafeCreationReceipt(rest)

const transactionAddedFilter = delayModifier.filters.TransactionAdded()

const [[modules], txExpiration, txCooldown, txNonce, queueNonce, transactionsAdded] = await Promise.all([
const [[modules], txExpiration, txCooldown, txNonce, queueNonce] = await Promise.all([
delayModifier.getModulesPaginated(SENTINEL_ADDRESS, MAX_PAGE_SIZE),
delayModifier.txExpiration(),
delayModifier.txCooldown(),
delayModifier.txNonce(),
delayModifier.queueNonce(),
// TODO: Improve log retrieval
delayModifier.queryFilter(transactionAddedFilter, blockHash),
])

const queuedTransactionsAdded = _getQueuedTransactionsAdded(transactionsAdded, txNonce)
const queuedTransactionsAdded = await queryAddedTransactions(
delayModifier,
queueNonce,
txNonce,
transactionService,
provider,
safeAddress,
)

const queue = await Promise.all(
queuedTransactionsAdded.map((transactionAdded) =>
Expand Down

0 comments on commit 3ee9b08

Please sign in to comment.