From adfea5ffe936518a03e37f4ad50a47296084d545 Mon Sep 17 00:00:00 2001 From: Philipp Giese Date: Thu, 23 Jan 2025 16:07:11 +0100 Subject: [PATCH 1/4] make sure roles waypoints use the correct from address --- packages/modules/src/updatePilotAddress.ts | 13 +-- .../modules/src/updateProviderType.spec.ts | 82 ++++++++++++++++--- packages/modules/src/updateProviderType.ts | 39 ++++++++- .../src/creators/createMockRoleWaypoint.ts | 7 +- 4 files changed, 117 insertions(+), 24 deletions(-) diff --git a/packages/modules/src/updatePilotAddress.ts b/packages/modules/src/updatePilotAddress.ts index 6c86fda1e..3692db677 100644 --- a/packages/modules/src/updatePilotAddress.ts +++ b/packages/modules/src/updatePilotAddress.ts @@ -1,6 +1,10 @@ -import { getChainId } from '@zodiac/chains' import type { ExecutionRoute, HexAddress, Waypoint } from '@zodiac/schema' -import { AccountType, ConnectionType, formatPrefixedAddress } from 'ser-kit' +import { + AccountType, + ConnectionType, + formatPrefixedAddress, + splitPrefixedAddress, +} from 'ser-kit' import { getStartingWaypoint } from './getStartingWaypoint' import { getWaypoints } from './getWaypoints' import { updateConnection } from './updateConnection' @@ -11,10 +15,7 @@ export const updatePilotAddress = ( address: HexAddress, ): ExecutionRoute => { const startingPoint = getStartingWaypoint(route.waypoints) - const chainId = - startingPoint.account.type === AccountType.EOA - ? undefined - : getChainId(route.avatar) + const [chainId] = splitPrefixedAddress(startingPoint.account.prefixedAddress) const waypoints = getWaypoints(route) return { diff --git a/packages/modules/src/updateProviderType.spec.ts b/packages/modules/src/updateProviderType.spec.ts index ae528057e..2825fb49b 100644 --- a/packages/modules/src/updateProviderType.spec.ts +++ b/packages/modules/src/updateProviderType.spec.ts @@ -1,7 +1,6 @@ import { Chain, getChainId } from '@zodiac/chains' import { ProviderType } from '@zodiac/schema' import { - createMockEndWaypoint, createMockEoaAccount, createMockExecutionRoute, createMockRoleWaypoint, @@ -11,7 +10,11 @@ import { randomAddress, randomPrefixedAddress, } from '@zodiac/test-utils' -import { AccountType, splitPrefixedAddress } from 'ser-kit' +import { + AccountType, + formatPrefixedAddress, + splitPrefixedAddress, +} from 'ser-kit' import { describe, expect, it } from 'vitest' import { getStartingWaypoint } from './getStartingWaypoint' import { getWaypoints } from './getWaypoints' @@ -89,13 +92,13 @@ describe('updateProviderType', () => { expect(chainId).not.toBeDefined() }) - it('keeps the other waypoints untouched', () => { - const waypoints = [createMockRoleWaypoint(), createMockEndWaypoint()] + it('switches the initiator field to an EOA without chain', () => { + const safeAccount = createMockSafeAccount() const route = createMockExecutionRoute({ + initiator: safeAccount.prefixedAddress, waypoints: createMockWaypoints({ - start: createMockStartingWaypoint(createMockSafeAccount()), - waypoints, + start: createMockStartingWaypoint(safeAccount), }), }) @@ -104,7 +107,35 @@ describe('updateProviderType', () => { ProviderType.InjectedWallet, ) - expect(getWaypoints(updatedRoute)).toEqual(waypoints) + expect(updatedRoute).toHaveProperty( + 'initiator', + formatPrefixedAddress(undefined, safeAccount.address), + ) + }) + + it('switches roles waypoints to EOA without chain', () => { + const safeAccount = createMockSafeAccount() + + const route = createMockExecutionRoute({ + waypoints: createMockWaypoints({ + start: createMockStartingWaypoint(safeAccount), + waypoints: [ + createMockRoleWaypoint({ from: safeAccount.prefixedAddress }), + ], + }), + }) + + const updatedRoute = updateProviderType( + route, + ProviderType.InjectedWallet, + ) + + const [rolesWaypoint] = getWaypoints(updatedRoute) + + expect(rolesWaypoint.connection).toHaveProperty( + 'from', + formatPrefixedAddress(undefined, safeAccount.address), + ) }) }) @@ -156,19 +187,46 @@ describe('updateProviderType', () => { ) }) - it('keeps the other waypoints untouched', () => { - const waypoints = [createMockRoleWaypoint(), createMockEndWaypoint()] + it('switches the initiator field to contain a chain', () => { + const eoaAccount = createMockEoaAccount() const route = createMockExecutionRoute({ + avatar: randomPrefixedAddress({ chainId: Chain.GNO }), + initiator: eoaAccount.prefixedAddress, waypoints: createMockWaypoints({ - start: createMockStartingWaypoint(createMockEoaAccount()), - waypoints, + start: createMockStartingWaypoint(eoaAccount), }), }) const updatedRoute = updateProviderType(route, ProviderType.WalletConnect) - expect(getWaypoints(updatedRoute)).toEqual(waypoints) + expect(updatedRoute).toHaveProperty( + 'initiator', + formatPrefixedAddress(Chain.GNO, eoaAccount.address), + ) + }) + + it('switches roles waypoints to connection to use a from address with chain', () => { + const eoaAccount = createMockEoaAccount() + + const route = createMockExecutionRoute({ + avatar: randomPrefixedAddress({ chainId: Chain.GNO }), + waypoints: createMockWaypoints({ + start: createMockStartingWaypoint(eoaAccount), + waypoints: [ + createMockRoleWaypoint({ from: eoaAccount.prefixedAddress }), + ], + }), + }) + + const updatedRoute = updateProviderType(route, ProviderType.WalletConnect) + + const [rolesWaypoint] = getWaypoints(updatedRoute) + + expect(rolesWaypoint.connection).toHaveProperty( + 'from', + formatPrefixedAddress(Chain.GNO, eoaAccount.address), + ) }) }) diff --git a/packages/modules/src/updateProviderType.ts b/packages/modules/src/updateProviderType.ts index 364922886..fddb4b853 100644 --- a/packages/modules/src/updateProviderType.ts +++ b/packages/modules/src/updateProviderType.ts @@ -1,5 +1,14 @@ import { getChainId } from '@zodiac/chains' -import { ProviderType, type ExecutionRoute } from '@zodiac/schema' +import { + ProviderType, + type ExecutionRoute, + type Waypoint, +} from '@zodiac/schema' +import { + AccountType, + formatPrefixedAddress, + type PrefixedAddress, +} from 'ser-kit' import { createEoaWaypoint } from './createEoaWaypoint' import { createSafeStartingPoint } from './createSafeStartingPoint' import { getStartingWaypoint } from './getStartingWaypoint' @@ -16,14 +25,17 @@ export const updateProviderType = ( switch (providerType) { case ProviderType.InjectedWallet: { + const initiator = formatPrefixedAddress(undefined, address) + return { ...route, + initiator, providerType: ProviderType.InjectedWallet, waypoints: [ createEoaWaypoint({ address, }), - ...waypoints, + ...waypoints.map((waypoint) => updateWaypoint(waypoint, initiator)), ], } } @@ -31,17 +43,38 @@ export const updateProviderType = ( case ProviderType.WalletConnect: { const chainId = getChainId(route.avatar) + const initiator = formatPrefixedAddress(chainId, address) + return { ...route, + initiator, providerType: ProviderType.WalletConnect, waypoints: [ createSafeStartingPoint({ chainId, address, }), - ...waypoints, + ...waypoints.map((waypoint) => updateWaypoint(waypoint, initiator)), ], } } } } + +const updateWaypoint = ( + waypoint: Waypoint, + from: PrefixedAddress, +): Waypoint => { + if (waypoint.account.type === AccountType.ROLES) { + return { + ...waypoint, + + connection: { + ...waypoint.connection, + from, + }, + } + } + + return waypoint +} diff --git a/packages/test-utils/src/creators/createMockRoleWaypoint.ts b/packages/test-utils/src/creators/createMockRoleWaypoint.ts index 25345928e..d90dd3a2d 100644 --- a/packages/test-utils/src/creators/createMockRoleWaypoint.ts +++ b/packages/test-utils/src/creators/createMockRoleWaypoint.ts @@ -5,6 +5,7 @@ import { ConnectionType, formatPrefixedAddress, type ChainId, + type PrefixedAddress, type Waypoint, } from 'ser-kit' import { randomAddress } from './randomHex' @@ -14,7 +15,7 @@ type CreateRoleWaypointOptions = { chainId?: ChainId version?: 1 | 2 roleId?: string - from?: HexAddress + from?: PrefixedAddress } export const createMockRoleWaypoint = ({ @@ -22,7 +23,7 @@ export const createMockRoleWaypoint = ({ version = 2, chainId = Chain.ETH, roleId, - from = randomAddress(), + from, }: CreateRoleWaypointOptions = {}): Waypoint => ({ account: { type: AccountType.ROLES, @@ -33,7 +34,7 @@ export const createMockRoleWaypoint = ({ version, }, connection: { - from: formatPrefixedAddress(chainId, from), + from: from != null ? from : formatPrefixedAddress(chainId, randomAddress()), type: ConnectionType.IS_MEMBER, roles: roleId != null ? [roleId] : [], }, From 3a4607374a0834a406ef2d346c486f47da990674 Mon Sep 17 00:00:00 2001 From: Philipp Giese Date: Thu, 23 Jan 2025 16:14:21 +0100 Subject: [PATCH 2/4] make sure also the endpoint is updated correctly --- .../modules/src/updateProviderType.spec.ts | 50 +++++++++++++++++++ packages/modules/src/updateProviderType.ts | 18 +++---- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/packages/modules/src/updateProviderType.spec.ts b/packages/modules/src/updateProviderType.spec.ts index 2825fb49b..7af2f0f7a 100644 --- a/packages/modules/src/updateProviderType.spec.ts +++ b/packages/modules/src/updateProviderType.spec.ts @@ -1,6 +1,7 @@ import { Chain, getChainId } from '@zodiac/chains' import { ProviderType } from '@zodiac/schema' import { + createMockEndWaypoint, createMockEoaAccount, createMockExecutionRoute, createMockRoleWaypoint, @@ -16,6 +17,7 @@ import { splitPrefixedAddress, } from 'ser-kit' import { describe, expect, it } from 'vitest' +import { createOwnsConnection } from './createOwnsConnection' import { getStartingWaypoint } from './getStartingWaypoint' import { getWaypoints } from './getWaypoints' import { updateProviderType } from './updateProviderType' @@ -137,6 +139,31 @@ describe('updateProviderType', () => { formatPrefixedAddress(undefined, safeAccount.address), ) }) + + it('updates the safe endpoint to use the EOA address when no roles are in between', () => { + const safeAccount = createMockSafeAccount() + + const route = createMockExecutionRoute({ + waypoints: createMockWaypoints({ + start: createMockStartingWaypoint(safeAccount), + end: createMockEndWaypoint({ + connection: createOwnsConnection(safeAccount.prefixedAddress), + }), + }), + }) + + const updatedRoute = updateProviderType( + route, + ProviderType.InjectedWallet, + ) + + const [endPoint] = getWaypoints(updatedRoute) + + expect(endPoint.connection).toHaveProperty( + 'from', + formatPrefixedAddress(undefined, safeAccount.address), + ) + }) }) describe('EOA -> SAFE', () => { @@ -228,6 +255,29 @@ describe('updateProviderType', () => { formatPrefixedAddress(Chain.GNO, eoaAccount.address), ) }) + + it('updates the safe endpoint to use an address with a chain when no roles are in between', () => { + const eoaAccount = createMockEoaAccount() + + const route = createMockExecutionRoute({ + avatar: randomPrefixedAddress({ chainId: Chain.GNO }), + waypoints: createMockWaypoints({ + start: createMockStartingWaypoint(eoaAccount), + end: createMockEndWaypoint({ + connection: createOwnsConnection(eoaAccount.prefixedAddress), + }), + }), + }) + + const updatedRoute = updateProviderType(route, ProviderType.WalletConnect) + + const [endPoint] = getWaypoints(updatedRoute) + + expect(endPoint.connection).toHaveProperty( + 'from', + formatPrefixedAddress(Chain.GNO, eoaAccount.address), + ) + }) }) it.each([ diff --git a/packages/modules/src/updateProviderType.ts b/packages/modules/src/updateProviderType.ts index fddb4b853..f6e959c18 100644 --- a/packages/modules/src/updateProviderType.ts +++ b/packages/modules/src/updateProviderType.ts @@ -6,6 +6,7 @@ import { } from '@zodiac/schema' import { AccountType, + ConnectionType, formatPrefixedAddress, type PrefixedAddress, } from 'ser-kit' @@ -62,19 +63,18 @@ export const updateProviderType = ( } const updateWaypoint = ( - waypoint: Waypoint, + { account, connection }: Waypoint, from: PrefixedAddress, ): Waypoint => { - if (waypoint.account.type === AccountType.ROLES) { - return { - ...waypoint, + if (account.type === AccountType.ROLES) { + return { account, connection: { ...connection, from } } + } - connection: { - ...waypoint.connection, - from, - }, + if (account.type === AccountType.SAFE) { + if (connection.type === ConnectionType.OWNS) { + return { account, connection: { ...connection, from } } } } - return waypoint + return { account, connection } } From d95703b41d7b8b2aa72802514ee3a778b481b970 Mon Sep 17 00:00:00 2001 From: Philipp Giese Date: Thu, 23 Jan 2025 16:30:44 +0100 Subject: [PATCH 3/4] chain id update does not affect eoa accounts --- packages/modules/src/updateChainId.spec.ts | 66 ++++++++++++++----- packages/modules/src/updateChainId.ts | 29 ++++++-- packages/modules/src/updatePrefixedAddress.ts | 8 ++- 3 files changed, 78 insertions(+), 25 deletions(-) diff --git a/packages/modules/src/updateChainId.spec.ts b/packages/modules/src/updateChainId.spec.ts index 87dda154a..70be9259b 100644 --- a/packages/modules/src/updateChainId.spec.ts +++ b/packages/modules/src/updateChainId.spec.ts @@ -57,24 +57,6 @@ describe('updateChainId', () => { describe('Waypoints', () => { describe('Starting point', () => { - it('does not update the starting waypoint of EOA accounts', () => { - const route = createMockExecutionRoute({ - waypoints: createMockWaypoints({ - start: createMockStartingWaypoint(createMockEoaAccount()), - }), - }) - - const updatedRoute = updateChainId(route, Chain.GNO) - - const startingPoint = getStartingWaypoint(updatedRoute.waypoints) - - const [chainId] = splitPrefixedAddress( - startingPoint.account.prefixedAddress, - ) - - expect(chainId).not.toBeDefined() - }) - it('updates the chain property of SAFE starting points', () => { const route = createMockExecutionRoute({ waypoints: createMockWaypoints({ @@ -147,4 +129,52 @@ describe('updateChainId', () => { }) }) }) + + describe('EOA Accounts', () => { + it('does not update the starting waypoint of EOA accounts', () => { + const route = createMockExecutionRoute({ + waypoints: createMockWaypoints({ + start: createMockStartingWaypoint(createMockEoaAccount()), + }), + }) + + const updatedRoute = updateChainId(route, Chain.GNO) + + const startingPoint = getStartingWaypoint(updatedRoute.waypoints) + + const [chainId] = splitPrefixedAddress( + startingPoint.account.prefixedAddress, + ) + + expect(chainId).not.toBeDefined() + }) + + it('does not update the initiator field when it is an EOA account', () => { + const initiator = formatPrefixedAddress(undefined, randomAddress()) + + const route = createMockExecutionRoute({ + initiator, + }) + + const updatedRoute = updateChainId(route, Chain.GNO) + + expect(updatedRoute).toHaveProperty('initiator', initiator) + }) + + it('does not update the from field of a roles waypoint when it is an EOA account', () => { + const from = formatPrefixedAddress(undefined, randomAddress()) + + const route = createMockExecutionRoute({ + waypoints: createMockWaypoints({ + waypoints: [createMockRoleWaypoint({ from })], + }), + }) + + const updatedRoute = updateChainId(route, Chain.GNO) + + const [rolesWaypoint] = getWaypoints(updatedRoute) + + expect(rolesWaypoint.connection).toHaveProperty('from', from) + }) + }) }) diff --git a/packages/modules/src/updateChainId.ts b/packages/modules/src/updateChainId.ts index 6e7a8a4e0..bb059dd83 100644 --- a/packages/modules/src/updateChainId.ts +++ b/packages/modules/src/updateChainId.ts @@ -1,6 +1,11 @@ import type { ChainId } from '@zodiac/chains' import type { ExecutionRoute, Waypoint } from '@zodiac/schema' -import { AccountType } from 'ser-kit' +import { + AccountType, + formatPrefixedAddress, + splitPrefixedAddress, + type PrefixedAddress, +} from 'ser-kit' import { createDelayWaypoint } from './createDelayWaypoint' import { createRolesWaypoint } from './createRolesWaypoint' import { createSafeWaypoint } from './createSafeWaypoint' @@ -20,10 +25,7 @@ export const updateChainId = ( return { ...route, avatar: updatePrefixedAddress(route.avatar, { chainId }), - initiator: - route.initiator == null - ? undefined - : updatePrefixedAddress(route.initiator, { chainId }), + initiator: updateInitiator(route.initiator, chainId), waypoints: [ updateStartingWaypoint(startingPoint, { chainId }), ...waypoints.map((waypoint) => updateWaypoint(waypoint, chainId)), @@ -31,6 +33,23 @@ export const updateChainId = ( } } +const updateInitiator = ( + initiator: PrefixedAddress | undefined, + chainId: ChainId, +) => { + if (initiator == null) { + return + } + + const [currentChainId, address] = splitPrefixedAddress(initiator) + + if (currentChainId == null) { + return initiator + } + + return formatPrefixedAddress(chainId, address) +} + const updateWaypoint = ( { account, connection }: Waypoint, chainId: ChainId, diff --git a/packages/modules/src/updatePrefixedAddress.ts b/packages/modules/src/updatePrefixedAddress.ts index 3e537ec48..df333dfad 100644 --- a/packages/modules/src/updatePrefixedAddress.ts +++ b/packages/modules/src/updatePrefixedAddress.ts @@ -19,11 +19,15 @@ export const updatePrefixedAddress = ( address = parsePrefixedAddress(prefixedAddress), }: UpdatePrefixedAddressOptions, ) => { + const [defaultChainId] = splitPrefixedAddress(prefixedAddress) + + if (defaultChainId == null) { + return formatPrefixedAddress(undefined, address) + } + if (chainId != null) { return formatPrefixedAddress(chainId, address) } - const [defaultChainId] = splitPrefixedAddress(prefixedAddress) - return formatPrefixedAddress(defaultChainId, address) } From cdddd07a996a0d7b1bed6a013fab0fef9c2734f7 Mon Sep 17 00:00:00 2001 From: Philipp Giese Date: Thu, 23 Jan 2025 16:33:33 +0100 Subject: [PATCH 4/4] use helpers more --- packages/modules/src/updateChainId.ts | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/packages/modules/src/updateChainId.ts b/packages/modules/src/updateChainId.ts index bb059dd83..2dee34e30 100644 --- a/packages/modules/src/updateChainId.ts +++ b/packages/modules/src/updateChainId.ts @@ -1,11 +1,6 @@ import type { ChainId } from '@zodiac/chains' import type { ExecutionRoute, Waypoint } from '@zodiac/schema' -import { - AccountType, - formatPrefixedAddress, - splitPrefixedAddress, - type PrefixedAddress, -} from 'ser-kit' +import { AccountType, type PrefixedAddress } from 'ser-kit' import { createDelayWaypoint } from './createDelayWaypoint' import { createRolesWaypoint } from './createRolesWaypoint' import { createSafeWaypoint } from './createSafeWaypoint' @@ -41,13 +36,7 @@ const updateInitiator = ( return } - const [currentChainId, address] = splitPrefixedAddress(initiator) - - if (currentChainId == null) { - return initiator - } - - return formatPrefixedAddress(chainId, address) + return updatePrefixedAddress(initiator, { chainId }) } const updateWaypoint = (