fix(pilot-app): handle EOA related updates better (#634)

* make sure roles waypoints use the correct from address

* make sure also the endpoint is updated correctly

* chain id update does not affect eoa accounts

* use helpers more

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)
+    })
+  })
 })

packages/modules/src/updateChainId.ts

@@ -1,6 +1,6 @@
 import type { ChainId } from '@zodiac/chains'
 import type { ExecutionRoute, Waypoint } from '@zodiac/schema'
-import { AccountType } from 'ser-kit'
+import { AccountType, type PrefixedAddress } from 'ser-kit'
 import { createDelayWaypoint } from './createDelayWaypoint'
 import { createRolesWaypoint } from './createRolesWaypoint'
 import { createSafeWaypoint } from './createSafeWaypoint'
@@ -20,17 +20,25 @@ 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)),
     ],
   }
 }
 
+const updateInitiator = (
+  initiator: PrefixedAddress | undefined,
+  chainId: ChainId,
+) => {
+  if (initiator == null) {
+    return
+  }
+
+  return updatePrefixedAddress(initiator, { chainId })
+}
+
 const updateWaypoint = (
   { account, connection }: Waypoint,
   chainId: ChainId,

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 {

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)
 }

packages/modules/src/updateProviderType.spec.ts

@@ -11,8 +11,13 @@ 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 { createOwnsConnection } from './createOwnsConnection'
 import { getStartingWaypoint } from './getStartingWaypoint'
 import { getWaypoints } from './getWaypoints'
 import { updateProviderType } from './updateProviderType'
@@ -89,13 +94,61 @@ 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),
+        }),
+      })
+
+      const updatedRoute = updateProviderType(
+        route,
+        ProviderType.InjectedWallet,
+      )
+
+      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),
+      )
+    })
+
+    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),
+          }),
         }),
       })
 
@@ -104,7 +157,12 @@ describe('updateProviderType', () => {
         ProviderType.InjectedWallet,
       )
 
-      expect(getWaypoints(updatedRoute)).toEqual(waypoints)
+      const [endPoint] = getWaypoints(updatedRoute)
+
+      expect(endPoint.connection).toHaveProperty(
+        'from',
+        formatPrefixedAddress(undefined, safeAccount.address),
+      )
     })
   })
 
@@ -156,19 +214,69 @@ 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(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)
 
-      expect(getWaypoints(updatedRoute)).toEqual(waypoints)
+      const [rolesWaypoint] = getWaypoints(updatedRoute)
+
+      expect(rolesWaypoint.connection).toHaveProperty(
+        'from',
+        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),
+      )
     })
   })