Merge pull request #2022 from oasisprotocol/lw/better-pending

Don't detect pending transactions if no nonce in 20 latest transactions

.changelog/2022.bugfix.md

@@ -0,0 +1 @@
+Don't detect pending transactions if no nonce in 20 latest transactions

src/app/state/account/saga.ts

@@ -15,9 +15,9 @@ import { getAccountBalanceWithFallback } from '../../lib/getAccountBalanceWithFa
 import { walletActions } from '../wallet'
 import { selectSelectedNetwork } from '../network/selectors'
 import { Transaction } from '../transaction/types'
+import { TRANSACTIONS_LIMIT } from '../../../config'
 
 const ACCOUNT_REFETCHING_INTERVAL = process.env.REACT_APP_E2E_TEST ? 5 * 1000 : 10 * 1000
-const TRANSACTIONS_LIMIT = 20
 
 export function* fetchAccount(action: PayloadAction<string>) {
   const address = action.payload

src/app/state/account/selectors.test.ts

@@ -0,0 +1,171 @@
+import { TRANSACTIONS_LIMIT } from '../../../config'
+import { RootState } from '../../../types'
+import { Transaction, TransactionStatus } from '../transaction/types'
+import { hasAccountUnknownPendingTransactions } from './selectors'
+
+const currentAddress = 'oasis1qz78ap0456g2rk7j6rmtvasc9v2kjhz2s58qgj90'
+
+describe('hasAccountUnknownPendingTransactions', () => {
+  it('empty account', () => {
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 0n.toString(),
+          transactions: [] as Transaction[],
+        },
+      } as RootState),
+    ).toBe(false)
+  })
+
+  it('making first transaction', () => {
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 1n.toString(),
+          transactions: [] as Transaction[],
+        },
+      } as RootState),
+    ).toBe(true)
+
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 1n.toString(),
+          transactions: [
+            {
+              from: 'oasis1qp2frld759c6u92pxs768nd2ctnrduhqp5f6f273',
+              to: currentAddress, // Received
+              status: TransactionStatus.Successful,
+              nonce: 0n.toString(),
+            },
+          ] as Transaction[],
+        },
+      } as RootState),
+    ).toBe(true)
+
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 1n.toString(),
+          transactions: [
+            {
+              from: currentAddress, // Sent
+              to: 'oasis1qp2frld759c6u92pxs768nd2ctnrduhqp5f6f273',
+              status: TransactionStatus.Successful,
+              nonce: 0n.toString(),
+            },
+          ] as Transaction[],
+        },
+      } as RootState),
+    ).toBe(false)
+  })
+
+  it('failed transaction also updates account nonce in consensus', () => {
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 1n.toString(),
+          transactions: [
+            {
+              from: currentAddress,
+              to: 'oasis1qp2frld759c6u92pxs768nd2ctnrduhqp5f6f273',
+              status: TransactionStatus.Failed,
+              nonce: 0n.toString(),
+            },
+          ] as Transaction[],
+        },
+      } as RootState),
+    ).toBe(false)
+  })
+
+  it('multiple pending transactions', () => {
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 5n.toString(),
+          transactions: [
+            {
+              from: currentAddress, // Sent
+              to: 'oasis1qp2frld759c6u92pxs768nd2ctnrduhqp5f6f273',
+              status: TransactionStatus.Successful,
+              nonce: 0n.toString(),
+            },
+          ] as Transaction[],
+        },
+      } as RootState),
+    ).toBe(true)
+  })
+
+  it('paratime transaction updates a different nonce within the paratime', () => {
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 1n.toString(),
+          transactions: [
+            {
+              from: currentAddress, // Sent on Emerald
+              to: 'oasis1qq235lqj77855qcemcr5w2qm372s4amqcc4v3ztc', // 0xC3ecf872F643C6238Aa20673798eed6F7dA199e9
+              status: TransactionStatus.Successful,
+              runtimeName: 'Emerald',
+              runtimeId: '000000000000000000000000000000000000000000000000e2eaa99fc008f87f',
+              round: 11129138,
+              type: 'consensus.Deposit',
+              nonce: 0n.toString(), // Misleading Emerald nonce
+            },
+          ] as Transaction[],
+        },
+      } as RootState),
+    ).toBe(true)
+  })
+
+  it('multiple pages of transactions', () => {
+    const onePageOfReceivedTxs = new Array(TRANSACTIONS_LIMIT).fill(null).map(() => ({
+      from: 'oasis1qp2frld759c6u92pxs768nd2ctnrduhqp5f6f273',
+      to: currentAddress, // Received
+      status: TransactionStatus.Successful,
+      nonce: 999n.toString(),
+    })) as Transaction[]
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 101n.toString(),
+          transactions: onePageOfReceivedTxs,
+        },
+      } as RootState),
+    ).toBe(false) // Can't determine within first page. Assume false.
+
+    const onePageIncludingSentTx = [...onePageOfReceivedTxs]
+    onePageIncludingSentTx[4] = {
+      from: currentAddress, // Sent
+      to: 'oasis1qp2frld759c6u92pxs768nd2ctnrduhqp5f6f273',
+      status: TransactionStatus.Successful,
+      nonce: 100n.toString(),
+    } as Transaction
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 101n.toString(),
+          transactions: onePageIncludingSentTx,
+        },
+      } as RootState),
+    ).toBe(false)
+    expect(
+      hasAccountUnknownPendingTransactions({
+        account: {
+          address: currentAddress,
+          nonce: 102n.toString(),
+          transactions: onePageIncludingSentTx,
+        },
+      } as RootState),
+    ).toBe(true)
+  })
+})

src/app/state/account/selectors.ts

@@ -3,6 +3,7 @@ import { createSelector } from '@reduxjs/toolkit'
 import { RootState } from 'types'
 import { initialState } from '.'
 import { selectSelectedNetwork } from '../network/selectors'
+import { TRANSACTIONS_LIMIT } from '../../../config'
 
 const selectSlice = (state: RootState) => state.account || initialState
 
@@ -25,32 +26,21 @@ export const selectPendingTransactionForAccount = createSelector(
 export const hasAccountUnknownPendingTransactions = createSelector(
   [selectAccountNonce, selectTransactions, selectAccountAddress],
   (accountNonce, transactions, accountAddress) => {
-    // TODO: This logic fails, in case transaction are not from the initial page of account's transactions
-    const filteredTransactions = transactions.filter(({ from }) => from && from === accountAddress)
-
-    if (filteredTransactions.length === 0 && BigInt(accountNonce) === 0n) return false
-
-    const maxNonceFromTxs = filteredTransactions.reduce(
-      (acc, { nonce }) => {
-        if (!nonce) {
-          return acc
-        }
-
-        const bigNonce = BigInt(nonce)
-
-        if (!acc) {
-          return bigNonce
-        }
-
-        return bigNonce > acc ? bigNonce : acc
-      },
-      undefined as bigint | undefined,
-    )
-
-    if (maxNonceFromTxs === undefined) {
+    const noncesFromTxs = transactions
+      .filter(tx => !tx.runtimeId)
+      .filter(tx => tx.from !== undefined)
+      .filter(tx => tx.from === accountAddress)
+      .filter(tx => tx.nonce !== undefined)
+      .map(tx => BigInt(tx.nonce!))
+
+    if (noncesFromTxs.length <= 0) {
+      // TODO: last transaction that affected nonce is not in the initial page of account's transactions
+      if (transactions.length >= TRANSACTIONS_LIMIT) return false
+      // Waiting for first transactions
       return BigInt(accountNonce) > 0n
     }
 
-    return BigInt(maxNonceFromTxs) + BigInt(1) < BigInt(accountNonce)
+    const maxNonceFromTxs = noncesFromTxs.reduce((acc, nonce) => (nonce > acc ? nonce : acc))
+    return BigInt(accountNonce) > maxNonceFromTxs + 1n
   },
 )

src/config.ts

@@ -1,5 +1,7 @@
 import { NetworkType } from 'app/state/network/types'
 
+export const TRANSACTIONS_LIMIT = 20
+
 export const consensusDecimals = 9
 
 // Moved outside backend.ts to avoid circular dependency

src/vendors/__tests__/__snapshots__/oasisscan.test.ts.snap

@@ -45,7 +45,7 @@ exports[`oasisscan parse transaction list 1`] = `
     "from": "oasis1qqnk4au603zs94k0d0n7c0hkx8t4p6r87s60axru",
     "hash": "25b84ca4582f6e3140c384ad60b98415d2c3079d1fc5f2221e45da7b13c70817",
     "level": undefined,
-    "nonce": undefined,
+    "nonce": "4",
     "round": 997775,
     "runtimeId": "000000000000000000000000000000000000000000000000e2eaa99fc008f87f",
     "runtimeName": "Emerald",

src/vendors/oasisscan.ts

@@ -230,7 +230,8 @@ export function parseTransactionsList(
         runtimeName: t.runtimeName,
         runtimeId: t.runtimeId,
         round: t.round,
-        nonce: undefined,
+        nonce:
+          t.ctx?.nonce ?? t.etx?.nonce != null ? BigInt(t.ctx?.nonce ?? t.etx?.nonce).toString() : undefined,
       }
       return parsed
     } else {