refactor(bridge): serve native bridge history from the indexer - #481
refactor(bridge): serve native bridge history from the indexer#481douglance wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
7e384fa to
9de23a3
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Pull request overview
This PR migrates native bridge transaction history for Arbitrum One (42161) and Arbitrum Sepolia (421614) to be served only from the indexer, leaving Nova (42170) as the remaining subgraph-backed chain. It also ensures backend/indexer failures surface as errors/warnings instead of silently rendering an empty history.
Changes:
- Introduces a shared
fetchBridgeHistoryhelper +assertOkto reject non-2xx responses (preventing “empty history” on outages). - Reworks bridge history availability/routing via
hasBridgeHistory(indexer-configured chains + Nova subgraph). - Updates unit and Playwright E2E tests to assert source selection and failure visibility, and wires
NEXT_PUBLIC_INDEXER_CHILD_CHAIN_IDSinto CI workflows.
File summaries
| File | Description |
|---|---|
| packages/arb-token-bridge-ui/tests/e2e/playwright/specs/txHistory.spec.ts | Adds E2E coverage asserting indexer sourcing for Arbitrum Sepolia and warning behavior on 502 responses. |
| packages/arb-token-bridge-ui/src/util/withdrawals/fetchWithdrawalsFromSubgraph.ts | Routes withdrawals history through the shared bridge history fetcher and gates by hasBridgeHistory. |
| packages/arb-token-bridge-ui/src/util/txHistory/sources.ts | Replaces “canonical source” selection with hasBridgeHistory (indexer-configured or Nova subgraph). |
| packages/arb-token-bridge-ui/src/util/txHistory/sources.test.ts | Updates tests to cover hasBridgeHistory behavior, including Nova special-casing. |
| packages/arb-token-bridge-ui/src/util/txHistory/fetchBridgeHistory.ts | New shared bridge-history fetcher that asserts response.ok before unwrapping data. |
| packages/arb-token-bridge-ui/src/util/txHistory/fetchBridgeHistory.test.ts | Adds unit tests ensuring error statuses throw (even with JSON “empty data” bodies). |
| packages/arb-token-bridge-ui/src/util/txHistory/bridgeHistoryAvailability.test.ts | Ensures fetchers avoid network calls when no backend serves a chain, and do call when served. |
| packages/arb-token-bridge-ui/src/util/SubgraphUtils.ts | Removes subgraph availability helpers; keeps block-number helper used by history logic. |
| packages/arb-token-bridge-ui/src/util/index.ts | Adds assertOk utility for consistent error handling with truncated error bodies. |
| packages/arb-token-bridge-ui/src/util/deposits/fetchEthDepositsToCustomDestinationFromSubgraph.ts | Routes custom-destination ETH deposits through fetchBridgeHistory and hasBridgeHistory. |
| packages/arb-token-bridge-ui/src/util/deposits/fetchDepositsFromSubgraph.ts | Routes deposits history through fetchBridgeHistory and hasBridgeHistory. |
| packages/arb-token-bridge-ui/src/util/deposits/fetchDeposits.ts | Propagates history failures (via Promise.all) instead of swallowing them and returning empty results. |
| packages/arb-token-bridge-ui/src/util/deposits/tests/fetchDeposits.test.ts | Adds tests asserting failures propagate rather than returning empty lists. |
| packages/arb-token-bridge-ui/src/util/cctp/fetchCCTP.ts | Switches CCTP fetch error handling to shared assertOk. |
| packages/arb-token-bridge-ui/src/components/TransactionHistory/TransactionHistoryTable.tsx | Adds a data-testid for the failed-chain-pairs warning icon used in E2E assertions. |
| packages/arb-token-bridge-ui/src/app/api/chains/[chainId]/block-number.ts | Returns 502 on backend failures (instead of {data: []}-style success), and 0 only when no backend serves the chain. |
| packages/arb-token-bridge-ui/src/app/api/chains/[chainId]/block-number.test.ts | Updates route tests for new 502 behavior and “no backend serves chain” semantics. |
| packages/arb-token-bridge-ui/src/api-utils/ServerSubgraphUtils.ts | Removes Arbitrum One/Sepolia subgraph entries/clients, keeping Nova subgraphs only. |
| packages/arb-token-bridge-ui/src/api-utils/tests/ServerSubgraphUtils.test.ts | Updates tests to assert One/Sepolia subgraph clients now throw while Nova resolves. |
| packages/app/.env.sample | Clarifies required semantics for NEXT_PUBLIC_INDEXER_CHILD_CHAIN_IDS (42161, 421614) and failure modes. |
| .github/workflows/test.yml | Exposes NEXT_PUBLIC_INDEXER_CHILD_CHAIN_IDS to unit-test workflow from secrets. |
| .github/workflows/e2e-tests.yml | Exposes NEXT_PUBLIC_INDEXER_CHILD_CHAIN_IDS to E2E workflow from secrets. |
| .github/workflows/build.yml | Exposes NEXT_PUBLIC_INDEXER_CHILD_CHAIN_IDS to build workflow from secrets. |
Review details
- Files reviewed: 23/23 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
9de23a3 to
f2dd055
Compare
f2dd055 to
8638145
Compare
The indexer has backfilled bridge history for Arbitrum One and Arbitrum
Sepolia, so the `l1-*` / `l2-*` subgraphs behind them are a second source that
can silently disagree. Drop them, leaving Nova — the one chain the indexer has
no history for — as all that remains of the subgraph path.
The flip itself is config: `NEXT_PUBLIC_INDEXER_CHILD_CHAIN_IDS` must list
42161 and 421614 before this merges, and it is now passed to the workflows that
build the app — it is `NEXT_PUBLIC_`, so e2e needs it at build time.
Removing the subgraph makes an existing bug load-bearing. `proxyToIndexer`
answers a failure with `{ data: [], message: 'Indexer unavailable' }` at 502,
and the history fetchers read `.data` without checking `response.ok`, so the
error body's own empty array was consumed as a successful empty history. Guard
before `.json()` so a non-JSON error body reports its status rather than a
parse error.
That guard alone was not enough: `fetchDeposits` caught the throw and logged it
at info, so it never reached `useTransactionHistory`, which is what marks a
chain pair as failed and tells the user their history is incomplete rather than
empty. It could not simply stop catching, because the fetchers used the same
channel to report "no backend is configured for this chain" — every un-indexed
orbit chain would have raised an error icon on every load.
So the two are now separate answers, at both layers. Unavailable is an empty
result: the fetchers return `[]`, and `/api/chains/[chainId]/block-number`
answers `{ data: 0 }` at 200, which sends the caller to a full event-log scan.
A source that was there and failed throws, or answers 502. Reserving 502 for
real failures matters at volume — the eleven chains in `BATCH_FETCH_BLOCKS` ask
for a block number once per withdrawal batch, hundreds of times per load.
Three copies of the status guard and three of the same fetch-and-unwrap would
have been the result, so the unwrap lives in `fetchBridgeHistory` and the guard
in `util`'s `assertOk`, which `fetchCCTP` shares. The routing question was
likewise a hand-written conjunction of two predicates from two modules at three
call sites, next door to a dead `getCanonicalSource` that already modelled it;
it is now `hasBridgeHistory`, and the dead pair is gone.
`txHistory.spec.ts` was the only e2e spec that reached the bridge-history
routes at all — every other spec runs on nitro-testnode chains, which have no
backend and so never call them. But it only asserted that rows render, which
looks identical whichever backend answered, and nothing covered the failure
path this stack introduces.
Two tests. The first reads `meta.source` off `/api/chains/421614/block-number`
and requires `arbitrum-indexer`: Arbitrum Sepolia has no subgraph left, so a
subgraph label would mean the narrowing missed something, and `{ data: 0 }`
with no `meta` would mean the chain never made it into
NEXT_PUBLIC_INDEXER_CHILD_CHAIN_IDS. That makes the config half of the flip
verifiable rather than assumed.
The second stubs `/api/deposits` to the 502 the proxy actually returns —
`{ data: [], message }`, empty array included — and asserts the failed-chain-pair
warning appears and names the chain. That is the contract behind dropping the
subgraph fallback, and it was only covered by unit tests until now. The healthy
test asserts the same warning is absent, so it can't pass by never rendering.
The warning icon had no stable selector, hence the `data-testid`.
8638145 to
083f518
Compare
dewanshparashar
left a comment
There was a problem hiding this comment.
I think the withdrawal failure path is still swallowing the error.
/block-number now returns a 502 when the indexer is down, but fetchLatestIndexedBlockNumber converts any non-2xx response back to 0. fetchWithdrawals also catches errors from the indexed history request and falls back to event logs.
So a failed withdrawal indexer does not reach the failed-chain warning added in this PR. The new e2e test only covers /api/deposits, so this path is not caught.
If the goal is to show failures for both deposits and withdrawals, can we propagate these errors and add a withdrawal test?
Also, the unit test job is currently failing because NEXT_PUBLIC_INDEXER_CHILD_CHAIN_IDS is empty, so the existing Arbitrum One history tests return no data. This needs to be configured or made deterministic in the tests before merge.
Arbitrum One and Arbitrum Sepolia bridge history now comes from the indexer instead of the subgraphs. Nova is the only chain still on one.
With no subgraph behind them, a backend failure has to be visible. The history fetchers were reading the route's error body — whose
datais an empty array — as a successful empty history, so an indexer outage looked like an account with no transactions. They now check the status, and the panel shows its "unable to fetch" warning instead.Stacked on #469. Needs
NEXT_PUBLIC_INDEXER_CHILD_CHAIN_IDSto list42161,421614in Vercel prod + preview and as a repo secret before merge — it's baked at build time and there's no subgraph left to fall back to.To test:
Load More./api/deposits?l2ChainId=42170still reportsmeta.source: l1-arbitrum-nova.txHistory.spec.tsasserts/api/chains/421614/block-numberreportsarbitrum-indexer, and that a stubbed 502 on/api/depositsraises the warning rather than an empty list.FS-2518