feat: introduce a new nonce config mod for enqueue, override or concrete

gnosisguild · Jan 8, 2025 · 05abc10 · 05abc10
1 parent 173c892
commit 05abc10
Show file tree

Hide file tree

Showing 4 changed files with 163 additions and 21 deletions.
diff --git a/src/execute/options.ts b/src/execute/options.ts
@@ -1,8 +1,9 @@
-import { createPublicClient, http } from 'viem'
+import { Address, createPublicClient, http } from 'viem'
 
 import { Eip1193Provider } from '@safe-global/protocol-kit'
 
 import { chains, defaultRpc } from '../chains'
+import { formatPrefixedAddress } from '../addresses'
 
 import { ChainId, PrefixedAddress } from '../types'
 import { SafeTransactionProperties } from './types'
@@ -47,3 +48,28 @@ export function getEip1193Provider({
     return urlOrProvider
   }
 }
+
+export function nonceConfig({
+  chainId,
+  safe,
+  options,
+}: {
+  chainId: ChainId
+  safe: Address
+  options?: Options
+}): 'enqueue' | 'override' | number {
+  const key1 = formatPrefixedAddress(chainId, safe)
+  const key2 = key1.toLocaleLowerCase() as PrefixedAddress
+
+  const properties =
+    options &&
+    options.safeTransactionProperties &&
+    (options.safeTransactionProperties[key1] ||
+      options.safeTransactionProperties[key2])
+
+  if (typeof properties?.nonce == 'undefined') {
+    return 'enqueue'
+  }
+
+  return properties.nonce
+}
diff --git a/src/execute/plan.test.ts b/src/execute/plan.test.ts
@@ -40,6 +40,8 @@ import { planExecution } from './plan'
 import { execute } from './execute'
 import encodeExecTransaction from '../encode/execTransaction'
 
+type NonceConfig = number | 'enqueue' | 'override'
+
 const withPrefix = (address: Address) =>
   formatPrefixedAddress(testClient.chain.id, address)
 
@@ -81,6 +83,7 @@ describe('plan', () => {
             [formatPrefixedAddress(chainId, safe)]: {
               proposeOnly: false,
               onchainSignature: false,
+              nonce: 'override' as NonceConfig,
             },
           },
         }
@@ -138,6 +141,7 @@ describe('plan', () => {
             [formatPrefixedAddress(chainId, safe)]: {
               proposeOnly: false,
               onchainSignature: false,
+              nonce: 'override' as NonceConfig,
             },
           },
         }
@@ -200,7 +204,10 @@ describe('plan', () => {
       const plan = await planExecution([transaction], route, {
         providers: { [testClient.chain.id]: testClient as Eip1193Provider },
         safeTransactionProperties: {
-          [withPrefix(safe)]: { proposeOnly: true },
+          [withPrefix(safe)]: {
+            proposeOnly: true,
+            nonce: 'override' as NonceConfig,
+          },
         },
       })
 
@@ -239,6 +246,11 @@ describe('plan', () => {
       const chainId = testClient.chain.id
       const plan = await planExecution([transaction], route, {
         providers: { [chainId]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe)]: {
+            nonce: 'override' as NonceConfig,
+          },
+        },
       })
 
       expect(plan).toHaveLength(2)
@@ -285,6 +297,10 @@ describe('plan', () => {
       // plan a transfer of 1 eth into receiver
       const plan = await planExecution([transaction], route, {
         providers: { [chainId]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe1)]: { nonce: 'override' as NonceConfig },
+          [withPrefix(safe2)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(3)
@@ -341,6 +357,10 @@ describe('plan', () => {
       // plan a transfer of 1 eth into receiver
       const plan = await planExecution([transaction], route, {
         providers: { [chainId]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe1)]: { nonce: 'override' as NonceConfig },
+          [withPrefix(safe2)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(3)
@@ -444,6 +464,10 @@ describe('plan', () => {
       // plan a transfer of 1 eth into receiver
       const plan = await planExecution([transaction], route, {
         providers: { [chainId]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe1)]: { nonce: 'override' as NonceConfig },
+          [withPrefix(safe2)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(2)
@@ -525,6 +549,10 @@ describe('plan', () => {
         route,
         {
           providers: { [chainId]: testClient as Eip1193Provider },
+          safeTransactionProperties: {
+            [withPrefix(safe1)]: { nonce: 'override' as NonceConfig },
+            [withPrefix(safe2)]: { nonce: 'override' as NonceConfig },
+          },
         }
       )
 
@@ -599,6 +627,9 @@ describe('plan', () => {
 
       const plan = await planExecution([transaction], route, {
         providers: { [testClient.chain.id]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(1)
@@ -665,6 +696,9 @@ describe('plan', () => {
 
       const plan = await planExecution([transaction], route, {
         providers: { [testClient.chain.id]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(1)
@@ -740,6 +774,9 @@ describe('plan', () => {
 
       const plan = await planExecution([transaction], route, {
         providers: { [testClient.chain.id]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(2)
@@ -832,6 +869,9 @@ describe('plan', () => {
 
       const plan = await planExecution([transaction], route, {
         providers: { [testClient.chain.id]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(2)
@@ -933,6 +973,10 @@ describe('plan', () => {
 
       const plan = await planExecution([transaction], route, {
         providers: { [testClient.chain.id]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe1)]: { nonce: 'override' as NonceConfig },
+          [withPrefix(safe2)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(await testClient.getBalance({ address: safe2 })).toEqual(
@@ -1015,6 +1059,10 @@ describe('plan', () => {
 
       const plan = await planExecution([transaction], route, {
         providers: { [testClient.chain.id]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe1)]: { nonce: 'override' as NonceConfig },
+          [withPrefix(safe2)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(2)
@@ -1113,6 +1161,9 @@ describe('plan', () => {
 
       const plan = await planExecution([transaction], route, {
         providers: { [testClient.chain.id]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(2)
@@ -1156,7 +1207,7 @@ describe('plan', () => {
       ).toEqual(parseEther('0.123'))
     })
 
-    it.only('plans and executes independently', async () => {
+    it('plans and executes independently', async () => {
       const owner = privateKeyToAccount(randomHash())
       const eoa = privateKeyToAccount(randomHash())
       const receiver = privateKeyToAccount(randomHash())
@@ -1211,6 +1262,9 @@ describe('plan', () => {
 
       const plan = await planExecution([transaction], route, {
         providers: { [testClient.chain.id]: testClient as Eip1193Provider },
+        safeTransactionProperties: {
+          [withPrefix(safe)]: { nonce: 'override' as NonceConfig },
+        },
       })
 
       expect(plan).toHaveLength(2)

diff --git a/src/execute/safeTransaction.ts b/src/execute/safeTransaction.ts
@@ -1,14 +1,16 @@
+import assert from 'assert'
 import {
   Address,
   encodeFunctionData,
   getAddress,
   parseAbi,
   zeroAddress,
 } from 'viem'
+import SafeApiKit from '@safe-global/api-kit'
 import { OperationType } from '@safe-global/types-kit'
 
 import { formatPrefixedAddress } from '../addresses'
-import { getEip1193Provider, Options } from './options'
+import { getEip1193Provider, nonceConfig, Options } from './options'
 
 import {
   ChainId,
@@ -28,18 +30,61 @@ export async function prepareSafeTransaction({
   transaction: MetaTransactionRequest
   options?: Options
 }): Promise<SafeTransactionRequest> {
-  const provider = getEip1193Provider({ chainId, options })
-
   const key1 = formatPrefixedAddress(chainId, safe)
   const key2 = key1.toLowerCase() as PrefixedAddress
 
   const defaults =
     options?.safeTransactionProperties?.[key1] ||
     options?.safeTransactionProperties?.[key2]
 
+  return {
+    to: transaction.to,
+    value: transaction.value,
+    data: transaction.data,
+    operation: transaction.operation ?? OperationType.Call,
+    safeTxGas: BigInt(defaults?.safeTxGas || 0),
+    baseGas: BigInt(defaults?.baseGas || 0),
+    gasPrice: BigInt(defaults?.gasPrice || 0),
+    gasToken: getAddress(defaults?.gasToken || zeroAddress),
+    refundReceiver: getAddress(defaults?.refundReceiver || zeroAddress),
+    nonce: await nonce({ chainId, safe, options }),
+  }
+}
+
+async function nonce({
+  chainId,
+  safe,
+  options,
+}: {
+  chainId: ChainId
+  safe: Address
+  options?: Options
+}): Promise<number> {
+  const config = nonceConfig({ chainId, safe, options })
+  if (config == 'enqueue') {
+    return fetchQueueNonce({ chainId, safe })
+  } else if (config == 'override') {
+    return fetchOnChainNonce({ chainId, safe, options })
+  } else {
+    const nonce = config
+    assert(typeof nonce == 'number')
+    return nonce
+  }
+}
+
+async function fetchOnChainNonce({
+  chainId,
+  safe,
+  options,
+}: {
+  chainId: ChainId
+  safe: Address
+  options?: Options
+}): Promise<number> {
+  const provider = getEip1193Provider({ chainId, options })
   const avatarAbi = parseAbi(['function nonce() view returns (uint256)'])
 
-  const nonce = BigInt(
+  const nonce = Number(
     (await provider.request({
       method: 'eth_call',
       params: [
@@ -56,18 +101,30 @@ export async function prepareSafeTransaction({
     })) as string
   )
 
-  return {
-    to: transaction.to,
-    value: transaction.value,
-    data: transaction.data,
-    operation: transaction.operation ?? OperationType.Call,
-    safeTxGas: BigInt(defaults?.safeTxGas || 0),
-    baseGas: BigInt(defaults?.baseGas || 0),
-    gasPrice: BigInt(defaults?.gasPrice || 0),
-    gasToken: getAddress(defaults?.gasToken || zeroAddress) as `0x${string}`,
-    refundReceiver: getAddress(
-      defaults?.refundReceiver || zeroAddress
-    ) as `0x${string}`,
-    nonce: Number(defaults?.nonce || nonce),
+  return nonce
+}
+
+async function fetchQueueNonce({
+  chainId,
+  safe,
+}: {
+  chainId: ChainId
+  safe: Address
+}): Promise<number> {
+  const apiKit = initApiKit(chainId)
+
+  const nonce = await apiKit.getNextNonce(safe)
+
+  return nonce
+}
+
+// TODO: remove this once https://github.com/safe-global/safe-core-sdk/issues/514 is closed
+const initApiKit = (chainId: ChainId): SafeApiKit => {
+  // @ts-expect-error SafeApiKit is only available as a CJS module. That doesn't play super nice with us being ESM.
+  if (SafeApiKit.default) {
+    // @ts-expect-error See above
+    return new SafeApiKit.default({ chainId: BigInt(chainId) })
   }
+
+  return new SafeApiKit({ chainId: BigInt(chainId) })
 }
diff --git a/src/execute/types.ts b/src/execute/types.ts
@@ -89,7 +89,7 @@ export type ExecutionPlan = [ExecutionAction, ...ExecutionAction[]]
 export type ExecutionState = `0x${string}`[]
 
 export interface SafeTransactionProperties
-  extends SafeTransactionOptionalProps {
+  extends Omit<SafeTransactionOptionalProps, 'nonce'> {
   /**
    * If a Safe transaction is executable, only approve/propose the transaction,
    * but don't execute it. Anyone will be able to trigger execution.
@@ -100,4 +100,9 @@ export interface SafeTransactionProperties
    * on-chain
    **/
   onchainSignature?: boolean
+  /**
+   * Defines the method used to derive the safeTransaction's nonce. Enqueue gets it
+   * from the txService. Override gets it from onchain. A concrete nonce can be provided
+   */
+  nonce?: 'enqueue' | 'override' | number
 }