Merge pull request #1903 from AmbireTech/fix/bad-pending-state

Fix: add checks for bad pending state

Author: superKalo

src/controllers/accounts/accounts.ts

@@ -179,6 +179,53 @@ export class AccountsController extends EventEmitter implements IAccountsControl
           networkAccountStates.forEach((accountState) => {
             const addr = accountState.accountAddr
             if (!this.accountStates[addr]) this.accountStates[addr] = {}
+
+            // if the block tag is pending & we have a fetched latest state,
+            // we check each nonce against the latest state.
+            // if the pending nonce is lower than the latest state, we override the res
+            // also, we check isDeployed, isSmarterEoa & isEOA
+            // we do this because rogue RPCs sometimes mess up the pending state
+            if (blockTag === 'pending') {
+              const current = this.accountStates[addr][network.chainId.toString()]
+              if (current) {
+                const account = accounts.find((acc) => acc.addr === addr)
+                if (account) {
+                  if (accountState.erc4337Nonce < current.erc4337Nonce) {
+                    // eslint-disable-next-line no-param-reassign
+                    accountState.erc4337Nonce = current.erc4337Nonce
+                  }
+                  if (accountState.nonce < current.nonce) {
+                    // eslint-disable-next-line no-param-reassign
+                    accountState.nonce = current.nonce
+                  }
+                  if (
+                    accountState.eoaNonce !== null &&
+                    current.eoaNonce !== null &&
+                    accountState.eoaNonce < current.eoaNonce
+                  ) {
+                    // eslint-disable-next-line no-param-reassign
+                    accountState.eoaNonce = current.eoaNonce
+                  }
+                  if (!accountState.isDeployed && current.isDeployed) {
+                    // eslint-disable-next-line no-param-reassign
+                    accountState.isDeployed = current.isDeployed
+                  }
+                  if (!accountState.isErc4337Enabled && current.isErc4337Enabled) {
+                    // eslint-disable-next-line no-param-reassign
+                    accountState.isErc4337Enabled = current.isErc4337Enabled
+                  }
+                  if (!accountState.isSmarterEoa && current.isSmarterEoa) {
+                    // eslint-disable-next-line no-param-reassign
+                    accountState.isSmarterEoa = current.isSmarterEoa
+                  }
+                  if (!accountState.isEOA && current.isEOA) {
+                    // eslint-disable-next-line no-param-reassign
+                    accountState.isEOA = current.isEOA
+                  }
+                }
+              }
+            }
+
             this.accountStates[addr][network.chainId.toString()] = accountState
           })
         } catch (err: any) {

src/libs/estimate/estimateBundler.ts

@@ -2,12 +2,10 @@
 /* eslint-disable no-continue */
 /* eslint-disable no-constant-condition */
 
-import { Contract, Interface, toBeHex } from 'ethers'
+import { Interface, toBeHex } from 'ethers'
 
 import AmbireAccount from '../../../contracts/compiled/AmbireAccount.json'
-import entryPointAbi from '../../../contracts/compiled/EntryPoint.json'
 import { EIP7702Auth } from '../../consts/7702'
-import { ERC_4337_ENTRYPOINT } from '../../consts/deploy'
 import { AccountOnchainState } from '../../interfaces/account'
 import { Network } from '../../interfaces/network'
 import { RPCProvider } from '../../interfaces/provider'
@@ -20,8 +18,10 @@ import { AccountOp, getSignableCalls } from '../accountOp/accountOp'
 import { SubmittedAccountOp } from '../accountOp/submittedAccountOp'
 import { AccountOpStatus } from '../accountOp/types'
 import { PaymasterEstimationData } from '../erc7677/types'
+import { DecodedError } from '../errorDecoder/types'
 import { getHumanReadableEstimationError } from '../errorHumanizer'
 import { TokenResult } from '../portfolio'
+import { fetchNonce } from '../userOperation/fetchEntryPointNonce'
 import { UserOperation } from '../userOperation/types'
 import { getSigForCalculations, getUserOperation } from '../userOperation/userOperation'
 import { estimateWithRetries } from './estimateWithRetries'
@@ -119,7 +119,12 @@ async function estimate(
 
   const nonFatalErrors: Error[] = []
   const estimateErrorCallback = (e: Error) => {
-    const decodedError = bundler.decodeBundlerError(e)
+    let decodedError: Error | DecodedError = e
+    try {
+      decodedError = bundler.decodeBundlerError(e)
+    } catch (err) {
+      // failed to decode the error, move on with the original one
+    }
 
     // if the bundler estimation fails, add a nonFatalError so we can react to
     // it on the FE. The BE at a later stage decides if this error is actually fatal
@@ -202,6 +207,7 @@ export async function bundlerEstimate(
   }
 
   const flags: EstimationFlags = {}
+  flags.invalid4337NonceCounter = 0
   while (true) {
     // estimate
     const estimations = await estimate(
@@ -235,14 +241,20 @@ export async function bundlerEstimate(
       estimations.nonFatalErrors.find((err) => err.cause === '4337_INVALID_NONCE')
     ) {
       flags.has4337NonceDiscrepancy = true
+      flags.invalid4337NonceCounter += 1
+
+      // prevent infinite loops in the event of a terrible rpc malfunction
+      if (flags.invalid4337NonceCounter >= 3) {
+        return estimations.nonFatalErrors.find((err) => err.cause === '4337_INVALID_NONCE')!
+      }
 
       // wait a bit to allow the state to sync
       await wait(2000)
-      const ep = new Contract(ERC_4337_ENTRYPOINT, entryPointAbi, provider)
-      const accountNonce = await ep
-        .getNonce(account.addr, 0, { blockTag: 'pending' })
-        .catch(() => null)
-      if (!accountNonce) continue
+      const accountNonce = await fetchNonce(account, provider)
+      if (accountNonce === null || accountNonce === 0n) {
+        // RPC serious malfunction
+        return estimations.nonFatalErrors.find((err) => err.cause === '4337_INVALID_NONCE')!
+      }
 
       if (network.chainId === 1n && BigInt(userOp.nonce) === BigInt(accountNonce)) {
         return estimations.nonFatalErrors.find((err) => err.cause === '4337_INVALID_NONCE')!

src/libs/estimate/interfaces.ts

@@ -24,6 +24,7 @@ export interface EstimationFlags {
   hasNonceDiscrepancy?: boolean
   has4337NonceDiscrepancy?: boolean
   hasInitialGasLimitFailed?: boolean
+  invalid4337NonceCounter?: number
 }
 
 export interface Erc4337GasLimits {

src/libs/relayerCall/relayerCall.test.ts

@@ -39,7 +39,7 @@ describe('Relayer', () => {
       expect(true).toBe(false)
     } catch (e: any) {
       expect(e.message).toBe(
-        'Broadcast failed: INVALID_SIGNATURE. Please try again or contact support'
+        'Signature validation was not successful. Please try again or contact support'
       )
     }
   })

src/libs/userOperation/fetchEntryPointNonce.ts

@@ -0,0 +1,42 @@
+import { Interface } from 'ethers'
+
+import entryPointAbi from '../../../contracts/compiled/EntryPoint.json'
+import { ERC_4337_ENTRYPOINT } from '../../consts/deploy'
+import { Account } from '../../interfaces/account'
+import { RPCProvider } from '../../interfaces/provider'
+
+export async function fetchNonce(account: Account, provider: RPCProvider): Promise<bigint | null> {
+  const epInterface = new Interface(entryPointAbi)
+  const failure = () => {
+    // eslint-disable-next-line no-console
+    console.error('unable to fetch the entry point nonce, estimateBundler')
+    return null
+  }
+  const [accountNonceHexLatest, accountNonceHexPending] = await Promise.all([
+    provider
+      .call({
+        to: ERC_4337_ENTRYPOINT,
+        data: epInterface.encodeFunctionData('getNonce', [account.addr, 0]),
+        blockTag: 'latest'
+      })
+      .catch(failure),
+    provider
+      .call({
+        to: ERC_4337_ENTRYPOINT,
+        data: epInterface.encodeFunctionData('getNonce', [account.addr, 0]),
+        blockTag: 'pending'
+      })
+      .catch(failure)
+  ])
+
+  // if there's an RPC problem and we can't fetch the nonce, we return an error
+  if (accountNonceHexLatest === null && accountNonceHexPending === null) return null
+  if (accountNonceHexLatest === null) return BigInt(accountNonceHexPending as string) // shouldn't happen
+  if (accountNonceHexPending === null) return BigInt(accountNonceHexLatest as string)
+
+  const accountNonceLatest = BigInt(accountNonceHexLatest)
+  const accountNoncePending = BigInt(accountNonceHexPending)
+
+  // always trust latest except the time when pending is higher
+  return accountNoncePending > accountNonceLatest ? accountNoncePending : accountNonceLatest
+}