Skip to content

Commit

Permalink
fix: wrong route used when creating a new one (#260)
Browse files Browse the repository at this point in the history
* chore: update route schema

* add failing spec

* useZodiacMod operates solely on the safe address

* remove obsolete call to validateAddress

* show placeholder for unnamed routes

* use same names for mods
  • Loading branch information
frontendphil authored Dec 5, 2024
1 parent 460942a commit cda1c65
Show file tree
Hide file tree
Showing 48 changed files with 110 additions and 60 deletions.
1 change: 0 additions & 1 deletion extension/src/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ export { Breadcrumbs } from './Breadcrumbs'
export * from './buttons'
export { Circle } from './Circle'
export { ConfirmationModal, useConfirmationModal } from './ConfirmationModal'
export { ConnectionStack } from './ConnectionStack'
export { CopyToClipboard } from './CopyToClipboard'
export { Divider } from './Divider'
export * from './inputs'
Expand Down
8 changes: 8 additions & 0 deletions extension/src/panel/integrations/zodiac/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export {
MULTISEND,
MULTISEND_CALL_ONLY,
queryRolesV1MultiSend,
queryRolesV2MultiSend,
} from './rolesMultisend'
export type { SupportedModuleType } from './types'
export { useZodiacModules } from './useZodiacModules'
25 changes: 11 additions & 14 deletions extension/src/panel/integrations/zodiac/useZodiacModules.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { getChainId } from '@/chains'
import { useExecutionRoute } from '@/execution-routes'
import { getReadOnlyProvider } from '@/providers'
import { validateAddress } from '@/utils'
import { HexAddress } from '@/types'
import {
ContractAbis,
ContractAddresses,
Expand All @@ -11,7 +10,7 @@ import { selectorsFromBytecode } from '@shazow/whatsabi'
import { Contract, id, Interface, ZeroAddress } from 'ethers'
import detectProxyTarget from 'evm-proxy-detection'
import { useEffect, useState } from 'react'
import { ChainId } from 'ser-kit'
import { ChainId, parsePrefixedAddress, PrefixedAddress } from 'ser-kit'
import { SupportedModuleType } from './types'

const SUPPORTED_MODULES = [
Expand All @@ -27,37 +26,35 @@ interface Module {
}

export const useZodiacModules = (
safeAddress: string,
connectionId?: string
safeAddress: PrefixedAddress
): { loading: boolean; isValidSafe: boolean; modules: Module[] } => {
const [loading, setLoading] = useState(false)
const [error, setError] = useState(false)
const [modules, setModules] = useState<Module[]>([])

const route = useExecutionRoute(connectionId)
const chainId = getChainId(route.avatar)
const chainId = getChainId(safeAddress)
const [, address] = parsePrefixedAddress(safeAddress)

useEffect(() => {
setLoading(true)
setError(false)
fetchModules(safeAddress, chainId)
fetchModules(address, chainId)
.then((modules) => setModules(modules))
.catch((e) => {
console.error(`Could not fetch modules of Safe ${safeAddress}`, e)
console.error(`Could not fetch modules of Safe ${address}`, e)
setError(true)
})
.finally(() => setLoading(false))
}, [chainId, safeAddress])
}, [address, chainId])

if (!validateAddress(safeAddress) || error) {
if (error) {
return { isValidSafe: false, loading, modules: [] }
}

return { loading, isValidSafe: true, modules }
}

async function fetchModules(
safeOrModifierAddress: string,
safeOrModifierAddress: HexAddress,
chainId: ChainId,
previous: Set<string> = new Set()
): Promise<Module[]> {
Expand All @@ -82,7 +79,7 @@ async function fetchModules(
)
const moduleAddresses = (
await contract.getModulesPaginated(ADDRESS_ONE, 100)
)[0] as string[]
)[0] as HexAddress[]

const enabledAndSupportedModules = moduleAddresses.map(
async (moduleAddress) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { Address as BaseAddress, Blockie, Tag } from '@/components'
import { shortenAddress, validateAddress } from '@/utils'
import { Unlink } from 'lucide-react'
import { Address as BaseAddress } from '../Address'
import { Blockie } from '../Blockie'
import { Tag } from '../Tag'

interface Props {
address: string
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MODULE_NAMES } from '../../const'
import { LegacyConnection } from '../../types'
import { MODULE_NAMES } from '@/const'
import { LegacyConnection } from '@/types'
import { Address } from './Address'

interface Props {
Expand Down
9 changes: 2 additions & 7 deletions extension/src/panel/pages/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { RouteObject } from 'react-router-dom'
import { EditRoute } from './edit-route'
import { ListRoutes } from './list-routes'
import { Root } from './Root'
import { routes } from './routes'
import { Transactions } from './transactions'

export const pages: RouteObject[] = [
Expand All @@ -15,11 +14,7 @@ export const pages: RouteObject[] = [
},
{
path: '/routes',
element: <ListRoutes />,
},
{
path: '/routes/:routeId',
element: <EditRoute />,
children: routes,
},
],
},
Expand Down
5 changes: 1 addition & 4 deletions extension/src/panel/pages/legacyConnectionMigrations.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ExecutionRoute, LegacyConnection, ProviderType } from '@/types'
import { MULTISEND, MULTISEND_CALL_ONLY } from '@/zodiac'
import { KnownContracts } from '@gnosis.pm/zodiac'
import { ZeroAddress } from 'ethers'
import {
Expand All @@ -10,10 +11,6 @@ import {
Roles,
Waypoint,
} from 'ser-kit'
import {
MULTISEND,
MULTISEND_CALL_ONLY,
} from '../integrations/zodiac/rolesMultisend'

export function fromLegacyConnection(
connection: LegacyConnection
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,28 @@
import { chromeMock, expectRouteToBe, mockRoute, render } from '@/test-utils'
import { getReadOnlyProvider } from '@/providers'
import {
chromeMock,
expectRouteToBe,
mockRoute,
mockRoutes,
render,
} from '@/test-utils'
import { screen, within } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { describe, expect, it } from 'vitest'
import { describe, expect, it, vi } from 'vitest'
import { EditRoute } from './EditRoute'

vi.mock('@/providers', async (importOriginal) => {
const module = await importOriginal<typeof import('@/providers')>()

return {
...module,

getReadOnlyProvider: vi.fn(module.getReadOnlyProvider),
}
})

const mockGetReadOnlyProvider = vi.mocked(getReadOnlyProvider)

describe('Edit Zodiac route', () => {
it('is possible to rename a route', async () => {
mockRoute({ id: 'route-id' })
Expand Down Expand Up @@ -103,4 +122,29 @@ describe('Edit Zodiac route', () => {
await expectRouteToBe('/routes')
})
})

describe('New route', () => {
it('uses the correct chain to fetch zodiac modules', async () => {
mockRoutes()

await render('/routes/route-id', [
{
path: '/routes/:routeId',
Component: EditRoute,
},
])

await userEvent.click(screen.getByRole('combobox', { name: 'Chain' }))
await userEvent.click(
screen.getByRole('option', { name: 'Arbitrum One' })
)

await userEvent.type(
screen.getByRole('textbox', { name: 'Piloted Safe' }),
'0x5a064eC22bf46dfFAb8a23b52a442FC98bBBD0Fb'
)

expect(mockGetReadOnlyProvider).toHaveBeenCalledWith(42161)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,22 @@ import {
} from '@/components'
import { INITIAL_DEFAULT_ROUTE, useExecutionRoutes } from '@/execution-routes'
import { useDisconnectWalletConnectIfNeeded } from '@/providers'
import { LegacyConnection } from '@/types'
import { HexAddress, LegacyConnection } from '@/types'
import { decodeRoleKey, encodeRoleKey } from '@/utils'
import { KnownContracts } from '@gnosis.pm/zodiac'
import { ZeroAddress } from 'ethers'
import { useState } from 'react'
import {
queryRolesV1MultiSend,
queryRolesV2MultiSend,
} from '../../integrations/zodiac/rolesMultisend'
import { useZodiacModules } from '../../integrations/zodiac/useZodiacModules'
useZodiacModules,
} from '@/zodiac'
import { KnownContracts } from '@gnosis.pm/zodiac'
import { ZeroAddress } from 'ethers'
import { useState } from 'react'
import { formatPrefixedAddress } from 'ser-kit'
import {
asLegacyConnection,
fromLegacyConnection,
} from '../legacyConnectionMigrations'
import { useConfirmClearTransactions } from '../useConfirmClearTransaction'
} from '../../legacyConnectionMigrations'
import { useConfirmClearTransactions } from '../../useConfirmClearTransaction'
import { AvatarInput } from './AvatarInput'
import { ChainSelect } from './ChainSelect'
import { LaunchButton } from './LaunchButton'
Expand Down Expand Up @@ -53,8 +54,12 @@ export const EditRoute = () => {

const decodedRoleKey = roleId && decodeRoleKey(roleId)

const prefixedAvatarAddress = formatPrefixedAddress(
chainId,
avatarAddress as HexAddress
)
// TODO modules is a nested list, but we currently only render the top-level items
const { modules } = useZodiacModules(avatarAddress, routeId)
const { modules } = useZodiacModules(prefixedAvatarAddress)

const [confirmClearTransactions, ConfirmationModal] =
useConfirmClearTransactions()
Expand Down Expand Up @@ -157,7 +162,7 @@ export const EditRoute = () => {
/>

<ZodiacMod
avatarAddress={avatarAddress}
avatarAddress={prefixedAvatarAddress}
pilotAddress={pilotAddress}
value={
selectedModule
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Warning } from '@/components'
import { MODULE_NAMES } from '../../../const'
import { SupportedModuleType } from '../../integrations/zodiac/types'
import { useZodiacModules } from '../../integrations/zodiac/useZodiacModules'
import { MODULE_NAMES } from '@/const'
import { SupportedModuleType, useZodiacModules } from '@/zodiac'
import { PrefixedAddress } from 'ser-kit'
import { ModSelect, NO_MODULE_OPTION } from './ModSelect'
import { useRouteId } from './useRouteId'
import { useSafeDelegates } from './useSafeDelegates'
Expand All @@ -13,7 +13,7 @@ type Value = {
}

type ZodiacModProps = {
avatarAddress: string
avatarAddress: PrefixedAddress
pilotAddress: string

value: Value | null
Expand All @@ -32,7 +32,7 @@ export const ZodiacMod = ({
loading: loadingMods,
isValidSafe,
modules,
} = useZodiacModules(avatarAddress, routeId)
} = useZodiacModules(avatarAddress)

const { safes } = useSafesWithOwner(pilotAddress, routeId)
const { delegates } = useSafeDelegates(avatarAddress, routeId)
Expand Down Expand Up @@ -61,7 +61,7 @@ export const ZodiacMod = ({
...(pilotIsOwner || pilotIsDelegate ? [NO_MODULE_OPTION] : []),
...modules.map((mod) => ({
value: mod.moduleAddress,
label: `${MODULE_NAMES[mod.type]} Mod`,
label: MODULE_NAMES[mod.type],
})),
]}
onChange={async (selected) => {
Expand Down
8 changes: 8 additions & 0 deletions extension/src/panel/pages/routes/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { RouteObject } from 'react-router-dom'
import { EditRoute } from './edit.$routeId'
import { ListRoutes } from './list'

export const routes: RouteObject[] = [
{ path: '', element: <ListRoutes /> },
{ path: ':routeId', element: <EditRoute /> },
]
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import {
ConnectionStack,
SecondaryButton,
SecondaryLinkButton,
Tag,
} from '@/components'
import { SecondaryButton, SecondaryLinkButton, Tag } from '@/components'
import { useExecutionRoute, useRouteConnect } from '@/execution-routes'
import { ExecutionRoute } from '@/types'
import { formatDistanceToNow } from 'date-fns'
import { Cable, PlugZap, Unplug } from 'lucide-react'
import { useState } from 'react'
import { asLegacyConnection } from '../legacyConnectionMigrations'
import { ClearTransactionsModal } from '../useConfirmClearTransaction'
import { ConnectionStack } from '../../ConnectionStack'
import { asLegacyConnection } from '../../legacyConnectionMigrations'
import { ClearTransactionsModal } from '../../useConfirmClearTransaction'

interface RouteProps {
route: ExecutionRoute
Expand Down
5 changes: 3 additions & 2 deletions extension/src/panel/pages/transactions/RouteBubble/index.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Blockie, ConnectionStack } from '@/components'
import { Blockie } from '@/components'
import { useExecutionRoute } from '@/execution-routes'
import { Transition } from '@headlessui/react'
import { AlignJustify, Cog } from 'lucide-react'
import { useState } from 'react'
import { Link } from 'react-router-dom'
import Stick from 'react-stick'
import { ConnectionStack } from '../../ConnectionStack'
import { asLegacyConnection } from '../../legacyConnectionMigrations'

export const RouteBubble = () => {
Expand Down Expand Up @@ -45,7 +46,7 @@ export const RouteBubble = () => {
/>

<p className="overflow-hidden text-ellipsis whitespace-nowrap">
{connection.label}
{connection.label || <span className="italic">Unnamed route</span>}
</p>
</div>

Expand Down
4 changes: 3 additions & 1 deletion extension/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"@/components": ["./src/components/index.ts"],
"@/utils": ["./src/utils/index.ts"],
"@/types": ["./src/types.ts"],
"@/const": ["./src/const.ts"],
"@/chains": ["./src/chains/index.ts"],
"@/execution-routes": ["./src/panel/execution-routes/index.ts"],
"@/state": ["./src/panel/state/index.tsx"],
Expand All @@ -33,7 +34,8 @@
"@/transaction-translation": [
"./src/panel/transactionTranslations/index.ts"
],
"@/safe": ["./src/panel/integrations/safe/index.ts"]
"@/safe": ["./src/panel/integrations/safe/index.ts"],
"@/zodiac": ["./src/panel/integrations/zodiac/index.ts"]
}
},
"include": ["src", "e2e", "test-utils", "./vitest.setup.mts"]
Expand Down

0 comments on commit cda1c65

Please sign in to comment.