From ce3045dd6bad3379a97d9569fddac7f32ea425b0 Mon Sep 17 00:00:00 2001 From: Philipp Giese Date: Tue, 21 Jan 2025 15:03:13 +0100 Subject: [PATCH] migrate dry run functionality --- .../app/app/routes/edit-route.$data.spec.ts | 29 ++++ .../app/app/routes/edit-route.$data.tsx | 40 ++++-- .../app/app/utils/dryRun/JsonRpcError.ts | 28 ++++ .../app/app/utils/dryRun/decodeError.ts | 85 ++++++++++++ deployables/app/app/utils/dryRun/dryRun.ts | 126 ++++++++++++++++++ .../app/utils/dryRun/handleRolesV1Error.ts | 29 ++++ .../app/utils/dryRun/handleRolesV2Error.ts | 28 ++++ deployables/app/app/utils/dryRun/index.ts | 1 + .../utils/dryRun/isSmartContractAddress.ts | 9 ++ .../app/app/utils/dryRun/maybeGetRoleId.ts | 10 ++ .../app/app/utils/dryRun/wrapRequest.ts | 91 +++++++++++++ deployables/app/app/utils/index.ts | 1 + deployables/app/package.json | 1 + packages/modules/src/getRolesWaypoint.ts | 8 ++ packages/modules/src/index.ts | 1 + packages/test-utils/src/renderFramework.tsx | 18 ++- pnpm-lock.yaml | 3 + 17 files changed, 491 insertions(+), 17 deletions(-) create mode 100644 deployables/app/app/utils/dryRun/JsonRpcError.ts create mode 100644 deployables/app/app/utils/dryRun/decodeError.ts create mode 100644 deployables/app/app/utils/dryRun/dryRun.ts create mode 100644 deployables/app/app/utils/dryRun/handleRolesV1Error.ts create mode 100644 deployables/app/app/utils/dryRun/handleRolesV2Error.ts create mode 100644 deployables/app/app/utils/dryRun/index.ts create mode 100644 deployables/app/app/utils/dryRun/isSmartContractAddress.ts create mode 100644 deployables/app/app/utils/dryRun/maybeGetRoleId.ts create mode 100644 deployables/app/app/utils/dryRun/wrapRequest.ts create mode 100644 packages/modules/src/getRolesWaypoint.ts diff --git a/deployables/app/app/routes/edit-route.$data.spec.ts b/deployables/app/app/routes/edit-route.$data.spec.ts index 3c38b70b..47455fcd 100644 --- a/deployables/app/app/routes/edit-route.$data.spec.ts +++ b/deployables/app/app/routes/edit-route.$data.spec.ts @@ -1,4 +1,5 @@ import { render } from '@/test-utils' +import { dryRun } from '@/utils' import { screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { Chain, CHAIN_NAME } from '@zodiac/chains' @@ -66,6 +67,17 @@ const mockQueryRolesV2MultiSend = vi.mocked(queryRolesV2MultiSend) const mockPostMessage = vi.spyOn(window, 'postMessage') +vi.mock('@/utils', async (importOriginal) => { + const module = await importOriginal() + + return { + ...module, + dryRun: vi.fn(), + } +}) + +const mockDryRun = vi.mocked(dryRun) + describe('Edit route', () => { describe('Label', () => { it('shows the name of a route', async () => { @@ -596,5 +608,22 @@ describe('Edit route', () => { screen.getByRole('button', { name: 'Test route' }), ).toBeInTheDocument() }) + + it('shows errors returned by dry run', async () => { + const route = createMockExecutionRoute() + + await render(`/edit-route/${btoa(JSON.stringify(route))}`) + + mockDryRun.mockResolvedValue({ + error: true, + message: 'Something went wrong', + }) + + await userEvent.click(screen.getByRole('button', { name: 'Test route' })) + + expect( + await screen.findByRole('alert', { name: 'Dry run failed' }), + ).toHaveAccessibleDescription('Something went wrong') + }) }) }) diff --git a/deployables/app/app/routes/edit-route.$data.tsx b/deployables/app/app/routes/edit-route.$data.tsx index edc33374..a185da9c 100644 --- a/deployables/app/app/routes/edit-route.$data.tsx +++ b/deployables/app/app/routes/edit-route.$data.tsx @@ -5,7 +5,8 @@ import { WalletProvider, ZodiacMod, } from '@/components' -import { editRoute, jsonRpcProvider, parseRouteData } from '@/utils' +import { dryRun, editRoute, jsonRpcProvider, parseRouteData } from '@/utils' +import { invariant } from '@epic-web/invariant' import { Chain, getChainId, verifyChainId, ZERO_ADDRESS } from '@zodiac/chains' import { formData, @@ -38,6 +39,7 @@ import { type Waypoints, } from '@zodiac/schema' import { + Error, PilotType, PrimaryButton, SecondaryButton, @@ -97,9 +99,8 @@ export const clientAction = async ({ const intent = getOptionalString(data, 'intent') - console.log({ intent }) - switch (intent) { + case Intent.DryRun: case Intent.Save: { let route = parseRouteData(params.data) @@ -111,12 +112,18 @@ export const clientAction = async ({ route = updateLabel(route, getString(data, 'label')) - window.postMessage( - { type: CompanionAppMessageType.SAVE_ROUTE, data: route }, - '*', - ) + if (intent === Intent.Save) { + window.postMessage( + { type: CompanionAppMessageType.SAVE_ROUTE, data: route }, + '*', + ) + + return editRoute(route) + } + + const chainId = getChainId(route.avatar) - return editRoute(route) + return dryRun(jsonRpcProvider(chainId), route) } case Intent.UpdateChain: { const route = parseRouteData(params.data) @@ -154,9 +161,7 @@ export const clientAction = async ({ return editRoute(updatePilotAddress(route, ZERO_ADDRESS)) } - case Intent.DryRun: { - return null - } + default: return serverAction() } @@ -164,6 +169,7 @@ export const clientAction = async ({ const EditRoute = ({ loaderData: { chainId, label, avatar, providerType, waypoints }, + actionData, }: Route.ComponentProps) => { const submit = useSubmit() @@ -261,8 +267,8 @@ const EditRoute = ({ }} /> -
-
+
+
The Pilot extension must be open to save.
@@ -284,6 +290,12 @@ const EditRoute = ({
+ + {actionData != null && actionData.error === true && ( +
+ {actionData.message} +
+ )} @@ -317,7 +329,7 @@ const getMultisend = (route: ExecutionRoute, module: ZodiacModule) => { return queryRolesV2MultiSend(chainId, module.moduleAddress) } - throw new Error(`Cannot get multisend for module type "${module.type}"`) + invariant(false, `Cannot get multisend for module type "${module.type}"`) } const verifyProviderType = (value: number): ProviderType => diff --git a/deployables/app/app/utils/dryRun/JsonRpcError.ts b/deployables/app/app/utils/dryRun/JsonRpcError.ts new file mode 100644 index 00000000..a701ae1e --- /dev/null +++ b/deployables/app/app/utils/dryRun/JsonRpcError.ts @@ -0,0 +1,28 @@ +import { z } from 'zod' + +export interface JsonRpcError extends Error { + data: { + code: number + message?: string + data?: string + originalError?: JsonRpcError['data'] + } +} + +const jsonRpcErrorSchema = z.object({ + data: z.object({ + code: z.number(), + message: z.string().optional(), + data: z.string().optional(), + }), +}) + +export const isJsonRpcError = (error: unknown): error is JsonRpcError => { + try { + jsonRpcErrorSchema.parse(error) + + return true + } catch { + return false + } +} diff --git a/deployables/app/app/utils/dryRun/decodeError.ts b/deployables/app/app/utils/dryRun/decodeError.ts new file mode 100644 index 00000000..e2cda658 --- /dev/null +++ b/deployables/app/app/utils/dryRun/decodeError.ts @@ -0,0 +1,85 @@ +import { ContractFactories, KnownContracts } from '@gnosis.pm/zodiac' +import { AbiCoder } from 'ethers' +import type { JsonRpcError } from './JsonRpcError' + +const RolesV1Interface = + ContractFactories[KnownContracts.ROLES_V1].createInterface() +const RolesV1PermissionsInterface = + ContractFactories[KnownContracts.PERMISSIONS].createInterface() +const RolesV2Interface = + ContractFactories[KnownContracts.ROLES_V2].createInterface() + +export function getRevertData(error: JsonRpcError) { + // The errors thrown when a transaction is reverted use different formats, depending on: + // - wallet (MetaMask vs. WalletConnect) + // - RPC provider (Infura vs. Alchemy vs. Tenderly) + // - client library (ethers vs. directly using the EIP-1193 provider) + + // first, drill through potential error wrappings down to the original error + while (typeof error === 'object' && (error as any).error) { + error = (error as any).error + } + + // Here we try to extract the revert reason in any of the possible formats + const message = + typeof error.data === 'string' + ? error.data + : error.data?.originalError?.data || + error.data?.data || + error.data?.originalError?.message || + error.data?.message || + error.message + + const prefix = 'Reverted 0x' + return message.startsWith(prefix) + ? message.substring(prefix.length - 2) + : message +} + +export function decodeGenericError(error: JsonRpcError) { + const revertData = getRevertData(error) + + // Solidity `revert "reason string"` will revert with the data encoded as selector of `Error(string)` followed by the ABI encoded string param + if (revertData.startsWith('0x08c379a0')) { + try { + const [reason] = AbiCoder.defaultAbiCoder().decode( + ['string'], + '0x' + revertData.slice(10), // skip over selector + ) + return reason as string + } catch { + return revertData + } + } + + return revertData +} + +export function decodeRolesV1Error(error: JsonRpcError) { + const revertData = getRevertData(error) + if (revertData.startsWith('0x')) { + try { + return ( + RolesV1Interface.parseError(revertData) || + RolesV1PermissionsInterface.parseError(revertData) + ) + } catch { + // ignore + } + } + return null +} + +export function decodeRolesV2Error(error: JsonRpcError) { + const revertData = getRevertData(error) + + if (revertData.startsWith('0x')) { + try { + return RolesV2Interface.parseError(revertData) + } catch { + // ignore + } + } + + return null +} diff --git a/deployables/app/app/utils/dryRun/dryRun.ts b/deployables/app/app/utils/dryRun/dryRun.ts new file mode 100644 index 00000000..3c160254 --- /dev/null +++ b/deployables/app/app/utils/dryRun/dryRun.ts @@ -0,0 +1,126 @@ +import { ZERO_ADDRESS } from '@zodiac/chains' +import { getRolesWaypoint } from '@zodiac/modules' +import type { ExecutionRoute } from '@zodiac/schema' +import type { JsonRpcProvider } from 'ethers' +import { AccountType, parsePrefixedAddress } from 'ser-kit' +import { decodeGenericError } from './decodeError' +import { handleRolesV1Error } from './handleRolesV1Error' +import { handleRolesV2Error } from './handleRolesV2Error' +import { isSmartContractAddress } from './isSmartContractAddress' +import { isJsonRpcError } from './JsonRpcError' +import { maybeGetRoleId } from './maybeGetRoleId' +import { wrapRequest } from './wrapRequest' + +type Error = { error: true; message: string } +type Success = { error: false } + +type Result = Error | Success + +export async function dryRun( + provider: JsonRpcProvider, + route: ExecutionRoute, +): Promise { + if ( + route.initiator == null || + parsePrefixedAddress(route.initiator) === ZERO_ADDRESS + ) { + return { error: true, message: 'This route is not connected to a wallet.' } + } + + if (parsePrefixedAddress(route.avatar) === ZERO_ADDRESS) { + return { error: true, message: 'This route has no target.' } + } + + const rolesWaypoint = getRolesWaypoint(route) + + if ( + rolesWaypoint != null && + !(await isSmartContractAddress(rolesWaypoint.account.address, provider)) + ) { + return { error: true, message: 'Module address is not a smart contract.' } + } + + if ( + !(await isSmartContractAddress( + parsePrefixedAddress(route.avatar), + provider, + )) + ) { + return { error: true, message: 'Avatar is not a smart contract.' } + } + + const request = wrapRequest({ + request: { + to: ZERO_ADDRESS, + data: '0x00000000', + from: parsePrefixedAddress(route.avatar), + }, + route, + revertOnError: false, + }) + + // TODO enable this once we can query role members from ser + // if (!request.from && connection.roleId) { + // // If pilotAddress is not yet determined, we will use a random member of the specified role + // request.from = await getRoleMember(connection) + // } + + try { + await provider.estimateGas(request) + + return { error: false } + } catch (error) { + if (!isJsonRpcError(error)) { + return { error: true, message: 'Unknown dry run error.' } + } + + if ( + rolesWaypoint == null || + rolesWaypoint.account.type !== AccountType.ROLES + ) { + return { error: true, message: decodeGenericError(error) } + } + // For the Roles mod, we actually expect the dry run to fail with TargetAddressNotAllowed() + // In case we see any other error, we try to help the user identify the problem. + + const { account } = rolesWaypoint + + switch (account.version) { + case 1: { + const message = handleRolesV1Error( + error, + maybeGetRoleId(rolesWaypoint) ?? '0', + ) + + if (message == null) { + return { error: false } + } + + return { + error: true, + message, + } + } + case 2: { + const message = handleRolesV2Error( + error, + maybeGetRoleId(rolesWaypoint) ?? '0', + ) + + if (message == null) { + return { error: false } + } + + return { + error: true, + message, + } + } + default: { + console.warn('Unexpected dry run error', error) + + return { error: true, message: 'Unexpected dry run error' } + } + } + } +} diff --git a/deployables/app/app/utils/dryRun/handleRolesV1Error.ts b/deployables/app/app/utils/dryRun/handleRolesV1Error.ts new file mode 100644 index 00000000..463b8176 --- /dev/null +++ b/deployables/app/app/utils/dryRun/handleRolesV1Error.ts @@ -0,0 +1,29 @@ +import { decodeGenericError, decodeRolesV1Error } from './decodeError' +import type { JsonRpcError } from './JsonRpcError' + +export const handleRolesV1Error = (e: JsonRpcError, roleId: string) => { + const rolesError = decodeRolesV1Error(e) + if (rolesError) { + switch (rolesError.signature) { + case 'UnacceptableMultiSendOffset()': + // we're calling to the zero address, so if this error happens it means our call was handled as a multi-send which happens + // if the Role mod's multiSend address has not been initialized + return "The Roles mod is not configured to accept multi-send calls. Use the contract's `setMultiSend` function to set the multi-send address." + + case 'TargetAddressNotAllowed()': + // this is the expected error for a working Roles mod setup + return null + + case 'NoMembership()': + return `The Pilot account is not a member of role #${roleId}.` + + default: + } + } + + const decoded = decodeGenericError(e) + + console.warn('Unexpected Roles v1 error', e, decoded) + + return decoded || 'Unexpected error' +} diff --git a/deployables/app/app/utils/dryRun/handleRolesV2Error.ts b/deployables/app/app/utils/dryRun/handleRolesV2Error.ts new file mode 100644 index 00000000..da7a0ee0 --- /dev/null +++ b/deployables/app/app/utils/dryRun/handleRolesV2Error.ts @@ -0,0 +1,28 @@ +import { decodeRoleKey } from '@zodiac/modules' +import { decodeGenericError, decodeRolesV2Error } from './decodeError' +import type { JsonRpcError } from './JsonRpcError' + +export const handleRolesV2Error = (e: JsonRpcError, roleKey: string) => { + const rolesError = decodeRolesV2Error(e) + if (rolesError) { + switch (rolesError.signature) { + case 'NotAuthorized(address)': + return 'The Pilot account must be enabled as a module of the modifier.' + + case 'ConditionViolation(uint8,bytes32)': + // this is the expected error for a working Roles mod setup + return null + + case 'NoMembership()': + return `The Pilot account is not a member of role ${decodeRoleKey( + roleKey, + )}.` + + default: + } + } + + const decoded = decodeGenericError(e) + console.warn('Unexpected Roles v2 error', e, decoded) + return decoded || 'Unexpected error' +} diff --git a/deployables/app/app/utils/dryRun/index.ts b/deployables/app/app/utils/dryRun/index.ts new file mode 100644 index 00000000..92330a02 --- /dev/null +++ b/deployables/app/app/utils/dryRun/index.ts @@ -0,0 +1 @@ +export { dryRun } from './dryRun' diff --git a/deployables/app/app/utils/dryRun/isSmartContractAddress.ts b/deployables/app/app/utils/dryRun/isSmartContractAddress.ts new file mode 100644 index 00000000..9dd708ab --- /dev/null +++ b/deployables/app/app/utils/dryRun/isSmartContractAddress.ts @@ -0,0 +1,9 @@ +import type { JsonRpcProvider } from 'ethers' + +export async function isSmartContractAddress( + address: string, + provider: JsonRpcProvider, +): Promise { + const code = await provider.getCode(address) + return code !== '0x' +} diff --git a/deployables/app/app/utils/dryRun/maybeGetRoleId.ts b/deployables/app/app/utils/dryRun/maybeGetRoleId.ts new file mode 100644 index 00000000..d8be5787 --- /dev/null +++ b/deployables/app/app/utils/dryRun/maybeGetRoleId.ts @@ -0,0 +1,10 @@ +import type { Waypoint } from '@zodiac/schema' +import { ConnectionType } from 'ser-kit' + +export const maybeGetRoleId = ({ connection }: Waypoint): string | null => { + if (connection.type !== ConnectionType.IS_MEMBER) { + return null + } + + return connection.roles.at(0) ?? null +} diff --git a/deployables/app/app/utils/dryRun/wrapRequest.ts b/deployables/app/app/utils/dryRun/wrapRequest.ts new file mode 100644 index 00000000..339fa64a --- /dev/null +++ b/deployables/app/app/utils/dryRun/wrapRequest.ts @@ -0,0 +1,91 @@ +import { invariant } from '@epic-web/invariant' +import { ContractFactories } from '@gnosis.pm/zodiac' +import { ZERO_ADDRESS } from '@zodiac/chains' +import { getRolesWaypoint, SupportedZodiacModuleType } from '@zodiac/modules' +import type { ExecutionRoute, HexAddress } from '@zodiac/schema' +import { toQuantity } from 'ethers' +import { + AccountType, + parsePrefixedAddress, + type MetaTransactionRequest, +} from 'ser-kit' +import { maybeGetRoleId } from './maybeGetRoleId' + +type TransactionData = { + to?: HexAddress + value?: string + data?: HexAddress + from?: HexAddress +} + +const RolesV1Interface = + ContractFactories[SupportedZodiacModuleType.ROLES_V1].createInterface() +const RolesV2Interface = + ContractFactories[SupportedZodiacModuleType.ROLES_V2].createInterface() +const DelayInterface = + ContractFactories[SupportedZodiacModuleType.DELAY].createInterface() + +type WrapRequestOptions = { + request: MetaTransactionRequest | TransactionData + route: ExecutionRoute + revertOnError?: boolean +} + +export function wrapRequest({ + request, + route, + revertOnError = true, +}: WrapRequestOptions): TransactionData { + const rolesWaypoint = getRolesWaypoint(route) + + invariant( + rolesWaypoint != null, + 'No wrapping should be applied for direct execution', + ) + + let data: string + switch (rolesWaypoint.account.type) { + case AccountType.ROLES: { + if (rolesWaypoint.account.version === 1) { + data = RolesV1Interface.encodeFunctionData('execTransactionWithRole', [ + request.to || '', + request.value || 0, + request.data || '0x', + ('operation' in request && request.operation) || 0, + maybeGetRoleId(rolesWaypoint) ?? 0, + revertOnError, + ]) + } else { + data = RolesV2Interface.encodeFunctionData('execTransactionWithRole', [ + request.to || '', + request.value || 0, + request.data || '0x', + ('operation' in request && request.operation) || 0, + maybeGetRoleId(rolesWaypoint) ?? ZERO_ADDRESS, + revertOnError, + ]) + } + break + } + + case AccountType.DELAY: + data = DelayInterface.encodeFunctionData('execTransactionFromModule', [ + request.to || '', + request.value || 0, + request.data || '0x', + ('operation' in request && request.operation) || 0, + ]) + break + default: + throw new Error(`Unsupported module type: ${rolesWaypoint.account.type}`) + } + + invariant(route.initiator != null, 'No pilot address specified for the route') + + return { + from: parsePrefixedAddress(route.initiator), + to: rolesWaypoint.account.address, + data: data as HexAddress, + value: toQuantity(0n), + } +} diff --git a/deployables/app/app/utils/index.ts b/deployables/app/app/utils/index.ts index 20cdf55d..76c9638c 100644 --- a/deployables/app/app/utils/index.ts +++ b/deployables/app/app/utils/index.ts @@ -1,3 +1,4 @@ +export * from './dryRun' export { editRoute } from './editRoute' export { jsonRpcProvider } from './jsonRpcProvider' export { parseRouteData } from './parseRouteData' diff --git a/deployables/app/package.json b/deployables/app/package.json index de8626a4..dd697c79 100644 --- a/deployables/app/package.json +++ b/deployables/app/package.json @@ -14,6 +14,7 @@ }, "dependencies": { "@epic-web/invariant": "^1.0.0", + "@gnosis.pm/zodiac": "^4.0.3", "@react-router/node": "^7.1.1", "@react-router/serve": "^7.1.1", "@sentry/react": "^8.50.0", diff --git a/packages/modules/src/getRolesWaypoint.ts b/packages/modules/src/getRolesWaypoint.ts new file mode 100644 index 00000000..2ff259e2 --- /dev/null +++ b/packages/modules/src/getRolesWaypoint.ts @@ -0,0 +1,8 @@ +import type { ExecutionRoute, Waypoint } from '@zodiac/schema' +import { AccountType } from 'ser-kit' +import { getWaypoints } from './getWaypoints' + +export const getRolesWaypoint = (route: ExecutionRoute): Waypoint | undefined => + getWaypoints(route).find( + (waypoint) => waypoint.account.type === AccountType.ROLES, + ) diff --git a/packages/modules/src/index.ts b/packages/modules/src/index.ts index c3b6353d..8d026340 100644 --- a/packages/modules/src/index.ts +++ b/packages/modules/src/index.ts @@ -2,6 +2,7 @@ export { createBlankRoute } from './createBlankRoute' export { fetchZodiacModules } from './fetchZodiacModules' export { getPilotAddress } from './getPilotAddress' export { getRolesVersion } from './getRolesVersion' +export { getRolesWaypoint } from './getRolesWaypoint' export { queryRolesV1MultiSend } from './queryRolesV1MultiSend' export { MULTISEND, diff --git a/packages/test-utils/src/renderFramework.tsx b/packages/test-utils/src/renderFramework.tsx index 56cedc76..b0296979 100644 --- a/packages/test-utils/src/renderFramework.tsx +++ b/packages/test-utils/src/renderFramework.tsx @@ -2,7 +2,12 @@ import { type RouteConfig } from '@react-router/dev/routes' import { render, type RenderResult } from '@testing-library/react' import type { ComponentType } from 'react' import type { ActionFunctionArgs } from 'react-router' -import { createRoutesStub, useLoaderData, useParams } from 'react-router' +import { + createRoutesStub, + useActionData, + useLoaderData, + useParams, +} from 'react-router' import type { CreateActionData, CreateComponentProps, @@ -114,10 +119,17 @@ export async function createRenderFramework( ? undefined : () => { const loaderData = useLoaderData() + const actionData = useActionData() const params = useParams() - // @ts-expect-error we're not yet suppliying all required props - return + return ( + // @ts-expect-error we're not yet suppliying all required props + + ) }, } }), diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ecaf111e..3fafe5f9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -79,6 +79,9 @@ importers: '@epic-web/invariant': specifier: ^1.0.0 version: 1.0.0 + '@gnosis.pm/zodiac': + specifier: ^4.0.3 + version: 4.0.3(patch_hash=qw56tepajizvmumojx6tatibqy)(bufferutil@4.0.9)(utf-8-validate@5.0.10) '@react-router/node': specifier: ^7.1.1 version: 7.1.3(react-router@7.1.3(react-dom@19.0.0(react@19.0.0))(react@19.0.0))(typescript@5.7.3)