fix: pass import+approve token e2e tests [PoC]#407
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Updates the E2E AppKit/Wagmi network configuration so the import-token E2E flow (Ethereum -> Arbitrum One) includes Arbitrum One in the configured chain list, preventing the re-render loop that leaves token search stuck.
Changes:
- Add
arbitrumto the E2E-only AppKit network list in Wagmi/AppKit setup. - Add a brief comment explaining why mainnet is included in the E2E chain list.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Cherry-picked the lifi registry cache fix from #406 onto this branch — the import test needs both fixes: the E2E chain list fix (this PR) and the cache fix (#406). Without the cache fix every page load refetches the full lifi token registry from li.quest, which alone blows past the test's 30s window. Will rebase once #406 merges. |
95972f7 to
c970455
Compare
| // Derived synchronously so the tokens can never lag behind the fetched lists, | ||
| // which an extra async hook allowed under heavy re-rendering |
There was a problem hiding this comment.
we used SWR because otherwise each caller of this hook would fetch and return their own instances, this was causing performance issues
There was a problem hiding this comment.
Understood. Worth noting that this is only a PoC which tries to get E2Es passing again by fixing some erroring parts of the bridge. If removing token-list SWR got the E2Es passing, then it doesn't necessarily mean we will remove SWR, but that the current implementation is worth going over and optimizing a bit to find out what's wrong with it. Just gives us a starting point of where to poke.
d90204d to
e5b0ab0
Compare
Important: this is just a POC. It is definitely worth breaking this into 3-4 focused, production-ready PRs. It is marked ready-for-review to let E2Es run.
The import token E2E tests were failing on every PR for months. It turned out to be five separate issues stacked on top of each other. Fixing all five (one commit each) got the tests passing.
What was wrong, step by step
The E2E wagmi chain list was missing Arbitrum One. The import test bridges Ethereum -> Arbitrum One, and since the reown migration (feat(bridge): migrate rainbowkit to reown #293) a destination chain missing from the AppKit config breaks the page. Added
arbitrumto the E2E chain list.The lifi token registry was too big to cache. The serialized registry (~2.6MB) is over Next.js' 2MB
unstable_cachelimit, so every cache write silently failed and every request refetched everything from li.quest. Trimmed the cached value to ~170KB (route output verified identical for all 25 chain pairs).The token search store was derived in a second async hook, so it could lag behind the fetched lists. It's a cheap synchronous derivation, so it now runs in
useMemoduring render.The E2E login helper passed an invalid chain pair. It forced
destinationChain=nitro-testnode-l3even when a test explicitly connects to Ethereum. It now only applies the L3 default when no network override is given.SWR was losing the token lists result. CI instrumentation showed the fetch resolving while every subscriber kept getting
data: undefined, isLoading: false— indefinitely, with React rendering normally.useTokenListsnow uses a small module-level store (cache + in-flight dedupe +useSyncExternalStore) instead of SWR, so a subscriber can never miss the result.Result
All 5 import E2E jobs green: https://github.com/OffchainLabs/arbitrum-portal/actions/runs/29520381584