Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/jest.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: Jest Tests

on:
pull_request:
branches:
- master

jobs:
jest:
name: Run Jest tests
runs-on: ubuntu-latest

steps:
- name: Check out Git repository
uses: actions/checkout@v3

- name: Set up Node
uses: actions/setup-node@v3
with:
node-version: 20

- name: Install dependencies
run: yarn install --immutable

- name: Run Jest tests
run: yarn test --testPathPattern="rpcParsing" --testTimeout=30000
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why only this test?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want all tests we should work on them in a separate PR as there is a lot of legacy tests that have not been maintained failing and would block this PR from merging and I thought we want to have this fix for rpc handling to resolve.

6 changes: 6 additions & 0 deletions babel.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
presets: [
['@babel/preset-env', { targets: { node: 'current' } }],
'@babel/preset-typescript',
],
}
8 changes: 8 additions & 0 deletions jest.setup.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const nodeFetch = require('node-fetch')
const globalObject = typeof globalThis !== 'undefined' ? globalThis : global
if (!globalObject.fetch) {
globalObject.fetch = nodeFetch.default || nodeFetch
globalObject.Request = nodeFetch.Request
globalObject.Response = nodeFetch.Response
globalObject.Headers = nodeFetch.Headers
}
16 changes: 15 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@
"@apollo/react-components": "^3.1.5",
"@apollo/react-hooks": "^3.1.5",
"@babel/core": "^7.15.8",
"@babel/preset-env": "^7.29.7",
"@babel/preset-flow": "^7.18.6",
"@babel/preset-react": "^7.18.6",
"@babel/preset-typescript": "^7.29.7",
"@commitlint/cli": "^12.1.1",
"@commitlint/config-conventional": "^12.1.1",
"@ethersproject/experimental": "^5.0.1",
Expand Down Expand Up @@ -87,6 +89,7 @@
"@web3-react/core": "^6.1.9",
"ajv": "^6.12.3",
"autoprefixer": "^9",
"babel-jest": "29",
"babel-plugin-import": "^1.13.5",
"babel-plugin-macros": "^3.1.0",
"babel-plugin-react-native-web": "^0.18.10",
Expand Down Expand Up @@ -114,6 +117,8 @@
"graphql": "^15.5.1",
"husky": "^8.0.2",
"jazzicon": "^1.5.0",
"jest": "29",
"jest-environment-jsdom": "^30.4.1",
Comment on lines +120 to +121
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): The Jest core version and jest-environment-jsdom major versions are out of sync.

jest is pinned to 29 while jest-environment-jsdom is ^30.4.1. Jest environments are expected to match Jest’s major version, so this mismatch can cause subtle test failures or runtime issues. Please either use a 29.x jest-environment-jsdom or upgrade Jest to 30.x to keep the majors aligned.

Comment on lines +120 to +121
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Upgrade the Node 12 workflows before adding Jest 29

Adding Jest 29 plus jest-environment-jsdom 30 makes the existing CI jobs in .github/workflows/tests.yaml incompatible with their configured node-version: '12' (both the integration and unit-test jobs install devDependencies before building/testing). The new jest.yml uses Node 20, but these older workflows still run on every push/PR to master, so yarn install/yarn test there will fail until those jobs are upgraded or the Jest packages are pinned to versions that support Node 12.

Useful? React with 👍 / 👎.

"jotai-immer": "^0.2.0",
"jsbi": "^3.1.5",
"lint-staged": "^13.1.0",
Expand All @@ -122,6 +127,7 @@
"luxon": "^1.25.0",
"multicodec": "^2.0.0",
"multihashes": "^3.0.1",
"node-fetch": "2",
"node-vibrant": "^3.1.5",
"numeral": "^2.0.6",
"patch-package": "^6.4.7",
Expand Down Expand Up @@ -165,6 +171,7 @@
"swr": "^0.5.5",
"tailwindcss": "^3.3.2",
"tailwindcss-border-gradient-radius": "^2.0.0",
"ts-jest": "^29.4.11",
"use-count-up": "^2.2.5",
"vercel": "^52.0.0",
"vite": "^4.3.5",
Expand Down Expand Up @@ -250,5 +257,12 @@
"wagmi": "^2.14.13",
"web3": "1.8.2"
},
"packageManager": "yarn@3.6.1"
"packageManager": "yarn@3.6.1",
"jest": {
"testEnvironment": "jsdom",
"setupFiles": [
"./jest.setup.js"
],
"testTimeout": 30000
}
}
26 changes: 20 additions & 6 deletions src/components/NetworkModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useCallback, useMemo, useState } from 'react'
import { t } from '@lingui/macro'
import { useLingui } from '@lingui/react'
import { SupportedChains, useSwitchNetwork } from '@gooddollar/web3sdk-v2'
import { Text, Link } from 'native-base'
import { Text, Link, Spinner, HStack } from 'native-base'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are these file changes why are they in this pr?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was testing if the RPCs were working and noticed a bug specifically on Ethereum, so I pushed a fix. thought it minor enough that it did not matter so much

import { SwitchChainModal } from '@gooddollar/good-design'
import { ChainId } from '@sushiswap/sdk'
import { UnsupportedChainId } from '@gooddollar/web3sdk'
Expand Down Expand Up @@ -39,11 +39,11 @@ const TextWrapper = styled.div`
}
`

const ChainOption = ({ chainId, chain, toggleNetworkModal, switchChain, labels, icons, error }: any) => {
const ChainOption = ({ chainId, chain, switchChain, labels, icons, error, disabled }: any) => {
const onOptionClick = useCallback(() => {
toggleNetworkModal()
if (disabled) return
switchChain(chain)
}, [switchChain, toggleNetworkModal, chain])
}, [disabled, switchChain, chain])

const isUnsupported = error instanceof UnsupportedChainId

Expand All @@ -56,6 +56,7 @@ const ChainOption = ({ chainId, chain, toggleNetworkModal, switchChain, labels,
icon={icons[chain]}
id={String(chain)}
onClick={onOptionClick}
disabled={disabled}
/>
)
}
Expand All @@ -72,6 +73,7 @@ export default function NetworkModal(): JSX.Element | null {
const networkModalOpen = useModalOpen(ApplicationModal.NETWORK)
const toggleNetworkModal = useNetworkModalToggle()
const [toAddNetwork, setToAddNetwork] = useState<SupportedChains | undefined>()
const [switchingChain, setSwitchingChain] = useState(false)

const networkLabel: string | null = error ? null : (NETWORK_LABEL as any)[+(chainId ?? 42220)]
const network = getEnv()
Expand Down Expand Up @@ -102,17 +104,19 @@ export default function NetworkModal(): JSX.Element | null {

const switchChain = useCallback(
async (chain: SupportedChains) => {
setSwitchingChain(true)
try {
await new Promise((resolve) => setTimeout(resolve, 250))
await switchNetwork(chain)
sendData({
event: 'network_switch',
action: 'network_switch_success',
network: ChainId[chain],
})
toggleNetworkModal()
} catch (e: any) {
if (e?.code === 4902) {
setToAddNetwork(chain)
toggleNetworkModal()
return
}

Expand All @@ -124,6 +128,7 @@ export default function NetworkModal(): JSX.Element | null {
network: ChainId[chain],
})
console.warn('Wallet not initialized. Network preference saved.')
toggleNetworkModal()
return
}

Expand All @@ -135,6 +140,8 @@ export default function NetworkModal(): JSX.Element | null {

error: e?.message || 'Unknown error',
})
} finally {
setSwitchingChain(false)
}
},

Expand Down Expand Up @@ -174,6 +181,13 @@ export default function NetworkModal(): JSX.Element | null {
)}
</TextWrapper>

{switchingChain && (
<HStack mt={3} space={2} alignItems="center">
<Spinner size="sm" />
<Text fontSize="sm">{i18n._(t`Switching network...`)}</Text>
</HStack>
)}

<div className="flex flex-col mt-3 space-y-5 overflow-y-auto">
{allowedNetworks.map((chain: SupportedChains) => (
<ChainOption
Expand All @@ -182,9 +196,9 @@ export default function NetworkModal(): JSX.Element | null {
chain={chain}
labels={NETWORK_LABEL}
icons={NETWORK_ICON}
toggleNetworkModal={toggleNetworkModal}
switchChain={switchChain}
error={error}
disabled={switchingChain}
/>
))}
</div>
Expand Down
7 changes: 5 additions & 2 deletions src/components/WalletModal/Option.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export default function Option({
icon,
active = false,
id,
disabled = false,
}: {
clickable?: boolean
size?: number | null
Expand All @@ -81,15 +82,17 @@ export default function Option({
icon: string
active?: boolean
id: string
disabled?: boolean
/** @deprecated */
color?: string
}) {
const content = (
<OptionCardClickable
id={id}
onClick={clickable ? onClick : undefined}
clickable={clickable && !active}
onClick={clickable && !disabled ? onClick : undefined}
clickable={clickable && !active && !disabled}
active={active}
disabled={disabled}
>
<div className="flex items-center">
<IconWrapper size={size}>
Expand Down
55 changes: 52 additions & 3 deletions src/components/Web3Status/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useMemo } from 'react'
import React, { useEffect, useMemo, useState } from 'react'
import useENSName from '../../hooks/useENSName'
import { isTransactionRecent, useAllTransactions } from '../../state/transactions/hooks'
import { TransactionDetails } from '../../state/transactions/reducer'
Expand All @@ -12,6 +12,9 @@ import { Text, HStack } from 'native-base'
import { useNativeBalance } from '@gooddollar/web3sdk-v2'
import { Currency } from '@sushiswap/sdk'
import { useAppKitAccount, useAppKitNetwork } from '@reown/appkit/react'
import { useEthers } from '@usedapp/core'
import { Spinner } from 'native-base'
import { formatUnits } from 'viem'

// we want the latest one to come first, so return negative if a is after b
function newTransactionsFirst(a: TransactionDetails, b: TransactionDetails) {
Expand All @@ -23,11 +26,54 @@ function Web3StatusInner() {
const sendData = useSendAnalyticsData()
const { address } = useAppKitAccount()
const { chainId } = useAppKitNetwork()
const { library } = useEthers() as any

const { ENSName } = useENSName(address ?? undefined)

const allTransactions = useAllTransactions()
const nativeBalance = useNativeBalance()
const [directNativeBalance, setDirectNativeBalance] = useState<string | undefined>()
const [showBalance, setShowBalance] = useState(false)

// added a timed-out fallback for when native-balance does not seem to complete (happening on mainnet)
// it uses a timeout to avoid mis-formatted amounts based on old-chain decimals.
useEffect(() => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the new balance timeout/fallback logic into a dedicated hook so Web3StatusInner is responsible only for rendering and not side‑effect orchestration.

You can keep the new behavior while reducing Web3StatusInner’s responsibilities by extracting the fallback logic into a dedicated hook. That moves the timeout / cancellation / formatting concerns out of the view layer and keeps this component focused on rendering.

1. Extract a useNativeBalanceWithFallback hook

Move the effect, extra state, and merging logic into a hook:

// hooks/useNativeBalanceWithFallback.ts
import { useEffect, useState } from 'react'
import { useNativeBalance } from '@gooddollar/web3sdk-v2'
import { useEthers } from '@usedapp/core'
import { formatUnits } from 'viem'

export function useNativeBalanceWithFallback(address?: string, chainId?: number) {
  const { library } = useEthers() as any
  const nativeBalance = useNativeBalance()
  const [directNativeBalance, setDirectNativeBalance] = useState<string>()
  const [showBalance, setShowBalance] = useState(false)

  useEffect(() => {
    let cancelled = false
    setShowBalance(false)
    setDirectNativeBalance(undefined)

    const timeoutId = window.setTimeout(() => {
      if (!cancelled) setShowBalance(true)
    }, 500)

    async function readBalance() {
      if (!address || !library?.getBalance) return
      try {
        const balance = await library.getBalance(address)
        if (!cancelled) {
          setDirectNativeBalance(formatUnits(balance.toString(), 18))
        }
      } catch {
        if (!cancelled) {
          setDirectNativeBalance(undefined)
        }
      }
    }

    void readBalance()

    return () => {
      cancelled = true
      window.clearTimeout(timeoutId)
    }
  }, [address, library, chainId])

  const balance = nativeBalance ?? directNativeBalance
  const loading = !showBalance || !balance

  return { balance, loading }
}

2. Simplify Web3StatusInner to consume the hook

Web3StatusInner no longer needs the effect/state/spinner logic:

import { Spinner, Text, HStack } from 'native-base'
import { Currency } from '@sushiswap/sdk'
import { useAppKitAccount, useAppKitNetwork } from '@reown/appkit/react'
import { useNativeBalanceWithFallback } from '../../hooks/useNativeBalanceWithFallback'

function Web3StatusInner() {
  const { i18n } = useLingui()
  const sendData = useSendAnalyticsData()
  const { address } = useAppKitAccount()
  const { chainId } = useAppKitNetwork()

  const { balance: displayNativeBalance, loading: balanceLoading } =
    useNativeBalanceWithFallback(address ?? undefined, chainId ?? 1)

  // ...transactions logic unchanged...

  return (
    <HStack space={8} flexDirection="row">
      {address && (
        <div className="flex flex-row gap-4">
          {balanceLoading ? (
            <Spinner size="sm" color="gdPrimary" />
          ) : (
            displayNativeBalance && (
              <Text fontSize="sm" fontFamily="subheading" fontWeight="normal" color="gdPrimary">
                {parseFloat(displayNativeBalance).toFixed(4)}{' '}
                {Currency.getNativeCurrencySymbol(+(chainId ?? 1))}
              </Text>
            )
          )}
          {/* pending tx / account click UI unchanged */}
        </div>
      )}
    </HStack>
  )
}

This keeps all existing behavior (including the timeout, fallback getBalance, and spinner) but:

  • Removes the multi-purpose useEffect from the component.
  • Reduces local state in Web3StatusInner.
  • Encapsulates the timing/cancellation concerns in a single, testable hook.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what are these balance additions?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those additions are a fallback path for the native balance display, plus a small loading-state tweak.

Concretely in Web3StatusInner:

  • const nativeBalance = useNativeBalance() – the original hook-based balance, which sometimes stalls / never resolves on mainnet in your current setup.
  • directNativeBalance state and the useEffect – a direct library.getBalance(address) call via useEthers() that kicks in if the hook is slow or stuck.
  • showBalance + timeout – you show a spinner for ~500ms first. After that:
    • if useNativeBalance() has given a value, you use it
    • otherwise you fall back to the getBalance result

The net behavior:

  • Users no longer get a “forever spinner” when useNativeBalance() doesn’t resolve on mainnet.
  • You avoid rendering a balance immediately on chain change (before decimals / RPC are correct) because you wait for the timeout + fresh read before showing it.

My review comment wasn’t suggesting changing that behavior, just moving this logic into a small hook (e.g. useNativeBalanceWithFallback) so the component stays more focused on rendering and less on side‑effects.

If you prefer to keep it inline here, the current logic is fine functionally; the suggestion is purely for readability/testability.

let cancelled = false
setShowBalance(false)
setDirectNativeBalance(undefined)
const timeoutId = window.setTimeout(() => {
if (!cancelled) {
setShowBalance(true)
}
}, 500)

async function readBalance() {
if (!address || !library?.getBalance) {
return
}

try {
const balance = await library.getBalance(address)
if (!cancelled) {
setDirectNativeBalance(formatUnits(balance.toString(), 18))
}
} catch {
if (!cancelled) {
setDirectNativeBalance(undefined)
}
}
}

void readBalance()

return () => {
cancelled = true
window.clearTimeout(timeoutId)
}
}, [address, library, /*used*/ chainId])

const displayNativeBalance = nativeBalance ?? directNativeBalance
const shouldShowBalance = showBalance && !!displayNativeBalance

const sortedRecentTransactions = useMemo(() => {
const txs = Object.values(allTransactions)
Expand All @@ -46,10 +92,13 @@ function Web3StatusInner() {
<HStack space={8} flexDirection="row">
{address && (
<div className="flex flex-row gap-4">
{nativeBalance && (
{shouldShowBalance ? (
<Text fontSize="sm" fontFamily="subheading" fontWeight="normal" color="gdPrimary">
{parseFloat(nativeBalance).toFixed(4)} {Currency.getNativeCurrencySymbol(+(chainId ?? 1))}
{parseFloat(displayNativeBalance!).toFixed(4)}{' '}
{Currency.getNativeCurrencySymbol(+(chainId ?? 1))}
</Text>
) : (
<Spinner size="sm" color="gdPrimary" />
)}
{hasPendingTransactions ? (
<div className="flex items-center justify-between">
Expand Down
70 changes: 70 additions & 0 deletions src/functions/rpcParsing.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/* eslint-env jest */

const EXTRA_RPCS_SOURCE = `
const privacyStatement = {
onerpc: 'ignored',
}

export const extraRpcs = {
1: {
rpcs: [
"https://eth.llamarpc.com",
{ url: "https://1rpc.io/eth", trackingDetails: privacyStatement.onerpc },
{ url: "wss://ethereum-rpc.publicnode.com" },
],
},
122: { rpcs: [{ url: "https://rpc.fuse.io" }, { url: "https://fuse.drpc.org" }] },
1220: { rpcs: [{ url: "https://should-not-match-1220.example" }] },
42220: { rpcs: ["https://forno.celo.org", { url: "wss://forno.celo.org/ws" }] },
422201: { rpcs: [{ url: "https://should-not-match-422201.example" }] },
11142220: { rpcs: [{ url: "https://should-not-match-11142220.example" }] },
50: { rpcs: ["https://rpc.xinfin.network", { url: "https://erpc.xinfin.network" }] },
500: { rpcs: [{ url: "https://should-not-match-500.example" }] },
5050: { rpcs: [{ url: "https://should-not-match-5050.example" }] },
1750: { rpcs: [{ url: "https://should-not-match-1750.example" }] },
250: { rpcs: [{ url: "https://should-not-match-250.example" }] },
}
`

describe('rpcParsing', () => {
const originalEnv = process.env

beforeEach(() => {
process.env = { ...originalEnv }
})

afterEach(() => {
process.env = originalEnv
jest.restoreAllMocks()
})

it('parses HTTP(S) RPCs from extraRpcs.js', async () => {
jest.spyOn(global, 'fetch').mockResolvedValue({
ok: true,
text: async () => EXTRA_RPCS_SOURCE,
} as Response)

const { fetchRpcsFromChainlist } = await import('./rpcParsing')

await expect(fetchRpcsFromChainlist()).resolves.toEqual({
'1': ['https://eth.llamarpc.com', 'https://1rpc.io/eth'],
'122': ['https://rpc.fuse.io', 'https://fuse.drpc.org'],
'42220': ['https://forno.celo.org'],
'50': ['https://rpc.xinfin.network', 'https://erpc.xinfin.network'],
})
})

it('returns only configured fallback chains', async () => {
process.env.REACT_APP_RPC_FALLBACK_CHAIN_IDS = '122,50'
process.env.REACT_APP_FUSE_RPC = 'https://fuse.example'

const { getFallbackRpcsByChain } = await import('./rpcParsing')

expect(getFallbackRpcsByChain()).toEqual({
'1': [],
'122': ['https://fuse.example'],
'42220': [],
'50': ['https://rpc.xinfin.network'],
})
})
})
Loading
Loading