Skip to content

Commit

Permalink
feat: normalize routes (#27)
Browse files Browse the repository at this point in the history
This PR normalizes routes before planning, in a way that will fill in
threshold if that number is missing.

(Addresses the always propose bug)

closes #26
  • Loading branch information
cristovaoth authored Jan 8, 2025
1 parent 5924e8f commit 173c892
Show file tree
Hide file tree
Showing 14 changed files with 297 additions and 121 deletions.
Binary file modified bun.lockb
Binary file not shown.
16 changes: 8 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,18 @@
},
"devDependencies": {
"@gnosis-guild/zodiac-core": "^2.0.4",
"@types/bun": "^1.1.12",
"@types/bun": "^1.1.15",
"@types/wait-on": "^5.3.4",
"prettier": "^3.3.3",
"prettier": "^3.4.2",
"tsup": "^8.1.0",
"typescript": "^5.6.3",
"typescript": "^5.7.2",
"wait-on": "8.0.1"
},
"dependencies": {
"@safe-global/api-kit": "^2.5.3",
"@safe-global/protocol-kit": "^5.0.3",
"@safe-global/safe-deployments": "^1.37.13",
"@safe-global/types-kit": "^1.0.0",
"viem": "^2.21.38"
"@safe-global/api-kit": "^2.5.6",
"@safe-global/protocol-kit": "^5.1.1",
"@safe-global/safe-deployments": "^1.37.22",
"@safe-global/types-kit": "^1.0.1",
"viem": "^2.22.4"
}
}
26 changes: 25 additions & 1 deletion src/addresses.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,30 @@
import { describe, expect, it } from 'bun:test'
import { parsePrefixedAddress } from './addresses'
import { parsePrefixedAddress, splitPrefixedAddress } from './addresses'
import { zeroAddress } from 'viem'
import { PrefixedAddress } from './types'

describe('splitPrefixedAddress', () => {
it('correctly splits a prefixed address with a chain shortName', () => {
expect(splitPrefixedAddress(`eth:${zeroAddress}`)).toEqual([1, zeroAddress])
})

it('correctly splits a prefixed address with eoa', () => {
expect(splitPrefixedAddress(`eoa:${zeroAddress}`)).toEqual([
undefined,
zeroAddress,
])
})

it('throws error when prefix chain shortName is unknown', () => {
expect(() =>
splitPrefixedAddress(`abc:${zeroAddress}` as PrefixedAddress)
).toThrow()
})

it('correctly splits a simple address', () => {
expect(splitPrefixedAddress(zeroAddress)).toEqual([undefined, zeroAddress])
})
})

describe('parsePrefixedAddress', () => {
it('returns the address part of a prefixed address', () => {
Expand Down
47 changes: 26 additions & 21 deletions src/addresses.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,46 @@
import { Address, getAddress } from 'viem'
import { Address, getAddress, isAddress, zeroAddress } from 'viem'
import { chains } from './chains'
import type { ChainId, PrefixedAddress } from './types'

export const formatPrefixedAddress = (
chainId: ChainId | undefined,
address: Address
) => {
const chain = chainId && chains.find((chain) => chain.chainId === chainId)
const chain = chains.find((chain) => chain.chainId === chainId)

if (!chain && chainId) {
throw new Error(`Unsupported chain ID: ${chainId}`)
if (chainId && !chain) {
throw new Error(`Unsupported chainId: ${chainId}`)
}

const prefix = chain ? chain.shortName : 'eoa'
return `${prefix}:${getAddress(address)}` as PrefixedAddress
}

export const splitPrefixedAddress = (prefixedAddress: PrefixedAddress) => {
const [prefix, address] = prefixedAddress.split(':')
const chain =
prefix !== 'eoa'
? chains.find(({ shortName }) => shortName === prefix)
: undefined
if (!chain && prefix !== 'eoa') {
throw new Error(`Unknown chain prefix: ${prefix}`)
export const splitPrefixedAddress = (
prefixedAddress: PrefixedAddress | Address
): [ChainId | undefined, Address] => {
if (prefixedAddress.length == zeroAddress.length) {
if (!isAddress(prefixedAddress)) {
throw new Error(`Not an Address: ${prefixedAddress}`)
}
return [undefined, getAddress(prefixedAddress)]
} else {
if (prefixedAddress.indexOf(':') == -1) {
throw new Error(`Unsupported PrefixedAddress format: ${prefixedAddress}`)
}
const [prefix, address] = prefixedAddress.split(':')
const chain = chains.find(({ shortName }) => shortName === prefix)
if (prefix && prefix != 'eoa' && !chain) {
throw new Error(`Unsupported chain shortName: ${prefix}`)
}

return [chain?.chainId, getAddress(address)] as const
}
const checksummedAddress = getAddress(address) as `0x${string}`
return [chain?.chainId, checksummedAddress] as const
}

export const parsePrefixedAddress = (
prefixedAddress: PrefixedAddress | Address
) => {
if (!prefixedAddress.includes(':')) {
return getAddress(prefixedAddress) as `0x${string}`
}

const [, address] = prefixedAddress.split(':')
return getAddress(address) as `0x${string}`
): Address => {
const [, address] = splitPrefixedAddress(prefixedAddress)
return address
}
2 changes: 1 addition & 1 deletion src/encode/execTransaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default function encodeExecTransaction({
args: [
safeTransaction.to,
BigInt(safeTransaction.value),
safeTransaction.data as Hex,
safeTransaction.data,
safeTransaction.operation,
BigInt(safeTransaction.safeTxGas),
BigInt(safeTransaction.baseGas),
Expand Down
8 changes: 4 additions & 4 deletions src/encode/execTransactionWithRole.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ export default function encodeExecTransactionWithRole({
abi: ROLES_V1_ABI,
functionName: 'execTransactionWithRole',
args: [
transaction.to as `0x${string}`,
transaction.to,
BigInt(transaction.value),
transaction.data as `0x${string}`,
transaction.data,
transaction.operation || 0,
Number(BigInt(role)),
true,
Expand All @@ -97,9 +97,9 @@ export default function encodeExecTransactionWithRole({
abi: ROLES_V2_ABI,
functionName: 'execTransactionWithRole',
args: [
transaction.to as `0x${string}`,
transaction.to,
BigInt(transaction.value),
transaction.data as `0x${string}`,
transaction.data,
transaction.operation || 0,
role,
true,
Expand Down
10 changes: 1 addition & 9 deletions src/execute/execute.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import assert from 'assert'
import {
Address,
Chain,
Expand All @@ -7,7 +6,6 @@ import {
defineChain,
getAddress,
hashTypedData,
isAddress,
} from 'viem'
import { type Eip1193Provider } from '@safe-global/protocol-kit'
import SafeApiKit from '@safe-global/api-kit'
Expand Down Expand Up @@ -68,7 +66,7 @@ export const execute = async (
const [relayer] = (await provider.request({
// message can be relayed by any account, request one
method: 'eth_accounts',
})) as string[]
})) as Address[]

const walletClient = getWalletClient({
chain: action.chain,
Expand Down Expand Up @@ -98,12 +96,6 @@ export const execute = async (
break
}
case ExecutionActionType.PROPOSE_TRANSACTION: {
const [relayer] = (await provider.request({
method: 'eth_accounts',
})) as string[]

assert(isAddress(relayer))

const { chain, safe, safeTransaction, proposer } = action
const previousOutput = state[i - 1]

Expand Down
45 changes: 45 additions & 0 deletions src/execute/normalizeRoute.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import assert from 'assert'

import { expect, describe, it } from 'bun:test'

import { privateKeyToAccount } from 'viem/accounts'
import { randomHash, testClient } from '../../test/client'
import { deploySafe } from '../../test/avatar'
import { eoaSafe } from '../../test/routes'
import { AccountType, ChainId } from '../types'
import { normalizeRoute } from './normalizeRoute'

describe('normalizeRoute', () => {
it('queries and patches missing threshold in a SAFE account', async () => {
const signer = privateKeyToAccount(randomHash())
const signer2 = privateKeyToAccount(randomHash())
const signer3 = privateKeyToAccount(randomHash())
const signer4 = privateKeyToAccount(randomHash())

const safe = await deploySafe({
owners: [
signer.address,
signer2.address,
signer3.address,
signer4.address,
],
creationNonce: BigInt(randomHash()),
threshold: 3,
})

let route = eoaSafe({
eoa: signer.address,
safe,
})

assert(route.waypoints[1].account.type == AccountType.SAFE)
;(route.waypoints[1].account as any).threshold = undefined
assert(route.waypoints[1].account.threshold == undefined)

route = await normalizeRoute(route, {
providers: { [testClient.chain.id as ChainId]: testClient },
})
assert(route.waypoints[1].account.type == AccountType.SAFE)
expect(route.waypoints[1].account.threshold).toEqual(3)
})
})
109 changes: 109 additions & 0 deletions src/execute/normalizeRoute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import { encodeFunctionData, getAddress, parseAbi } from 'viem'

import { formatPrefixedAddress, splitPrefixedAddress } from '../addresses'

import {
Account,
AccountType,
ChainId,
Connection,
PrefixedAddress,
Route,
StartingPoint,
Waypoint,
} from '../types'
import { getEip1193Provider, Options } from './options'

export async function normalizeRoute(
route: Route,
options?: Options
): Promise<Route> {
const waypoints = await Promise.all(
route.waypoints.map((w) => normalizeWaypoint(w, options))
)

return {
id: route.id,
initiator: normalizePrefixedAddress(route.initiator),
avatar: normalizePrefixedAddress(route.avatar),
waypoints: waypoints as [StartingPoint, ...Waypoint[]],
}
}

export async function normalizeWaypoint(
waypoint: StartingPoint | Waypoint,
options?: Options
): Promise<StartingPoint | Waypoint> {
waypoint = {
...waypoint,
account: await normalizeAccount(waypoint.account, options),
}

if ('connection' in waypoint) {
waypoint = {
...waypoint,
connection: normalizeConnection(waypoint.connection as Connection),
}
}

return waypoint
}

async function normalizeAccount(
account: Account,
options?: Options
): Promise<Account> {
account = {
...account,
address: getAddress(account.address),
prefixedAddress: normalizePrefixedAddress(account.prefixedAddress),
}

if (
account.type == AccountType.SAFE &&
typeof account.threshold != 'number'
) {
account.threshold = await fetchThreshold(account, options)
}

return account
}

function normalizeConnection(connection: Connection): Connection {
return {
...connection,
from: normalizePrefixedAddress(connection.from),
}
}

function normalizePrefixedAddress(
prefixedAddress: PrefixedAddress
): PrefixedAddress {
const [chainId, address] = splitPrefixedAddress(prefixedAddress)
return formatPrefixedAddress(chainId, address)
}

async function fetchThreshold(
account: Account,
options?: Options
): Promise<number> {
const [chainId, safe] = splitPrefixedAddress(account.prefixedAddress)
const provider = getEip1193Provider({ chainId: chainId as ChainId, options })

return Number(
await provider.request({
method: 'eth_call',
params: [
{
to: safe,
data: encodeFunctionData({
abi: parseAbi(['function getThreshold() view returns (uint256)']),
functionName: 'getThreshold',
args: [],
}),
},
'latest',
],
})
)
}
Loading

0 comments on commit 173c892

Please sign in to comment.