feat: add balancec-watcher updater [pr 2 - connection logic]#7640
feat: add balancec-watcher updater [pr 2 - connection logic]#7640limitofzero wants to merge 21 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds an SSE-driven balances watcher mode to the ChangesSSE Balances Watcher Mode
Sequence Diagram(s)sequenceDiagram
rect rgba(100, 149, 237, 0.5)
note over CommonPriorityBalancesAndAllowancesUpdater: Feature flag active
CommonPriorityBalancesAndAllowancesUpdater->>BalancesWatcherUpdater: render(account, chainId)
end
BalancesWatcherUpdater->>useEnabledTokensListsUrls: get enabled list URLs
BalancesWatcherUpdater->>useCustomTokensForChain: get custom token addresses
BalancesWatcherUpdater->>useBalancesWatcherSession: start(account, chainId, listUrls, customTokens)
useBalancesWatcherSession->>createBalancesWatcherSession: POST session
createBalancesWatcherSession-->>useBalancesWatcherSession: sessionId
useBalancesWatcherSession->>subscribeToBalancesEvents: open SSE stream
subscribeToBalancesEvents-->>useBalancesWatcherSession: balance diff event
useBalancesWatcherSession->>balancesAtom: merge update (BigInt, normalized addresses)
BalancesWatcherUpdater->>NativeTokenBalanceUpdater: render(account, chainId)
NativeTokenBalanceUpdater->>balancesAtom: write native token balance
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying swap-dev with
|
| Latest commit: |
67445bd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://42ec8792.swap-dev-5u6.pages.dev |
| Branch Preview URL: | https://feat-balance-watcher-updater.swap-dev-5u6.pages.dev |
…-watcher-updater-2 # Conflicts: # libs/balances-and-allowances/src/balancesWatcher/subscribeToBalancesEvents.test.ts
Deploying explorer-dev with
|
| Latest commit: |
67445bd
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7a7a373f.explorer-dev-dxz.pages.dev |
| Branch Preview URL: | https://feat-balance-watcher-updater.explorer-dev-dxz.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/modules/balancesAndAllowances/updaters/CommonPriorityBalancesAndAllowancesUpdater.tsx (1)
25-61: ⚡ Quick winAvoid running legacy updater hooks when watcher mode is enabled.
usePriorityTokenAddresses, timer effects, anduseOrdersFilledEventsTriggerare executed before theisBwEnabledreturn, so watcher mode still initializes legacy multicall-path logic. Split the watcher and legacy paths into separate components and branch immediately after reading the flag.Proposed refactor
export function CommonPriorityBalancesAndAllowancesUpdater(): ReactNode { const sourceChainId = useSourceChainId().chainId const { account } = useWalletInfo() const balancesContext = useBalancesContext() const balancesAccount = balancesContext.account || account const { isBwEnabled } = useFeatureFlags() - - const priorityTokenAddresses = usePriorityTokenAddresses() - const priorityTokenAddressesAsArray = useMemo(() => { - return Array.from(priorityTokenAddresses.values()) - }, [priorityTokenAddresses]) - const priorityTokenCount = priorityTokenAddressesAsArray.length - const [skipFirstPriorityUpdate, setSkipFirstPriorityUpdate] = useState(true) - useEffect(() => { - setSkipFirstPriorityUpdate(true) - }, [sourceChainId]) - useEffect(() => { - if (!account || !priorityTokenCount) return - const timeout = setTimeout(() => { - setSkipFirstPriorityUpdate(false) - }, PRIORITY_TOKENS_REFRESH_INTERVAL) - return () => clearTimeout(timeout) - }, [account, priorityTokenCount]) - const refreshTrigger = useOrdersFilledEventsTrigger() if (isBwEnabled) { return <BalancesWatcherUpdater account={balancesAccount} chainId={sourceChainId} /> } + + return <LegacyPriorityBalancesUpdater account={account} balancesAccount={balancesAccount} sourceChainId={sourceChainId} /> +} + +function LegacyPriorityBalancesUpdater({ + account, + balancesAccount, + sourceChainId, +}: { + account: string | undefined + balancesAccount: string | undefined + sourceChainId: number +}): ReactNode { + const priorityTokenAddresses = usePriorityTokenAddresses() + const priorityTokenAddressesAsArray = useMemo(() => Array.from(priorityTokenAddresses.values()), [priorityTokenAddresses]) + const priorityTokenCount = priorityTokenAddressesAsArray.length + const [skipFirstPriorityUpdate, setSkipFirstPriorityUpdate] = useState(true) + const refreshTrigger = useOrdersFilledEventsTrigger() + // existing effects and return JSX remain here🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cowswap-frontend/src/modules/balancesAndAllowances/updaters/CommonPriorityBalancesAndAllowancesUpdater.tsx` around lines 25 - 61, The legacy updater hooks including usePriorityTokenAddresses, the timer effects with PRIORITY_TOKENS_REFRESH_INTERVAL, and useOrdersFilledEventsTrigger are being executed unconditionally before checking the isBwEnabled flag. Move the check for isBwEnabled to the top of the component and return the BalancesWatcherUpdater immediately when watcher mode is enabled, then wrap all the legacy hook calls and effects (usePriorityTokenAddresses, the two useEffect blocks, and useOrdersFilledEventsTrigger) in a conditional that only executes when isBwEnabled is false. This ensures legacy multicall-path logic is not initialized when watcher mode is active.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/balances-and-allowances/src/hooks/useBalancesWatcherSession.test.tsx`:
- Around line 126-135: The test is using toLowerCase() to normalize the TOKEN_A
address, which violates the repo's address-handling guidelines. Replace
TOKEN_A.toLowerCase() with getAddressKey(TOKEN_A) in two places: in the
renderSession call where customTokens array is passed with the lowercased token,
and in the mockCreateSession assertion where customTokens is expected in the
body. Import getAddressKey from `@cowprotocol/cow-sdk` if not already imported,
and ensure both the input passed to renderSession and the expected value in the
assertion use getAddressKey(TOKEN_A) for consistent address normalization.
---
Nitpick comments:
In
`@apps/cowswap-frontend/src/modules/balancesAndAllowances/updaters/CommonPriorityBalancesAndAllowancesUpdater.tsx`:
- Around line 25-61: The legacy updater hooks including
usePriorityTokenAddresses, the timer effects with
PRIORITY_TOKENS_REFRESH_INTERVAL, and useOrdersFilledEventsTrigger are being
executed unconditionally before checking the isBwEnabled flag. Move the check
for isBwEnabled to the top of the component and return the
BalancesWatcherUpdater immediately when watcher mode is enabled, then wrap all
the legacy hook calls and effects (usePriorityTokenAddresses, the two useEffect
blocks, and useOrdersFilledEventsTrigger) in a conditional that only executes
when isBwEnabled is false. This ensures legacy multicall-path logic is not
initialized when watcher mode is active.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7a9bbe55-4a04-4a33-8564-4fc21c765483
📒 Files selected for processing (10)
apps/cowswap-frontend/src/modules/balancesAndAllowances/updaters/CommonPriorityBalancesAndAllowancesUpdater.tsxlibs/balances-and-allowances/src/hooks/useBalancesWatcherSession.test.tsxlibs/balances-and-allowances/src/hooks/useBalancesWatcherSession.tslibs/balances-and-allowances/src/hooks/useCustomTokensForChain.test.tsxlibs/balances-and-allowances/src/hooks/useCustomTokensForChain.tslibs/balances-and-allowances/src/hooks/useEnabledTokensListsUrls.tslibs/balances-and-allowances/src/index.tslibs/balances-and-allowances/src/updaters/BalancesWatcherUpdater.tsxlibs/balances-and-allowances/src/updaters/NativeTokenBalanceUpdater.test.tsxlibs/balances-and-allowances/src/updaters/NativeTokenBalanceUpdater.tsx
…ol/cowswap into feat/balance-watcher-updater-2
| const tokensListsUrls = useEnabledTokensListsUrls() | ||
| const customTokens = useCustomTokensForChain(chainId) | ||
|
|
||
| useBalancesWatcherSession({ account, chainId, tokensListsUrls, customTokens }) |
There was a problem hiding this comment.
This might be a good opportunity to implement this using only atoms instead of creating new updaters, as we discussed in the past we'd like to decouple business logic and app state from UI / React.
There was a problem hiding this comment.
not really get your point, is still stores data in atoms but we need to run hooks somewhere for fetching data
There was a problem hiding this comment.
Not really, you would do something like this to have the whole flow run without hooks:
export const balancesAtom= atom<...>({})
balancesAtom.onMount = () => {
return observe((get, set) => {
const { chainId, account } = get(walletInfoAtom)
const tokensListsUrls = get(...)
const customTokens = get(...)
let cancelled = false
let subscription: BalancesSubscription | undefined
let isFirstEvent = true
createBalancesWatcherSession({
chainId,
owner: account,
body: { tokensListsUrls, customTokens },
})
.then(() => {
if (cancelled) return
subscription = subscribeToBalancesEvents({
chainId,
owner: account,
onBalances: (balances) => {
if (cancelled) return
set(balancesAtom, (state) => writeBalancesUpdate(state, balances, chainId, isFirstEvent))
isFirstEvent = false
},
onError: (error, terminal) => {
if (cancelled) return
// Non-terminal errors mean EventSource is reconnecting — the
// transport recovers on its own. Only surface terminal errors.
if (!terminal) return
set(balancesAtom, (state) => ({ ...state, error: error.message, isLoading: false }))
},
})
})
.catch((error: unknown) => {
if (cancelled) return
const message = error instanceof Error ? error.message : String(error)
set(balancesAtom, (state) => ({ ...state, error: message, isLoading: false }))
})
return () => {
cancelled = true
subscription?.close()
}
}, jotaiStore)
}
| */ | ||
| export function NativeTokenBalanceUpdater({ account, chainId }: NativeTokenBalanceUpdaterProps): null { | ||
| const updateTokenBalance = useUpdateTokenBalance() | ||
| const { data: nativeTokenBalance } = useNativeTokenBalance(account, chainId) |
Danziger
left a comment
There was a problem hiding this comment.
Approving as the comments are not blockers.
fairlighteth
left a comment
There was a problem hiding this comment.
⚠️ AI Review (Codex GPT-5, worked 5m): watcher failures can leave swaps stuck on balance loading
Finding: Handle watcher errors before the first snapshot as a completed balance load failure
- Location:
libs/balances-and-allowances/src/hooks/useBalancesWatcherSession.ts:75andapps/cowswap-frontend/src/modules/tradeFormValidation/hooks/useTradeFormValidationContext.ts:111 - When
isBwEnabledis true, the PR bypasses the RPC updater and relies on the watcher path for EVM chains. - If
createBalancesWatcherSessionrejects or the SSE stream terminates before the firstbalance_update, the hook setserrorandisLoading: false, but leaveshasFirstLoad: false. - The validation context then computes
isBalancesLoading: !hasFirstLoad || isBalancesLoading, so the UI stays inBalancesLoadinginstead of surfacingBalancesNotLoaded. This matches the current Cypress failures on the PR head where swap flows never find#do-trade-button.
Suggested fix
- Make the error path stop the first-load gate, or make validation treat a balance error as not loading. For example, terminal watcher/create-session failures should transition to a state that renders
BalancesNotLoadedrather than permanentBalancesLoading. - Add coverage for both
createSessionrejection and terminal SSE error before the first snapshot, asserting the downstream validation sees a not-loaded/error state rather than loading forever.
Review scope and related context
Already covered by existing review threads, so not repeated here:
- The updater-vs-atom architecture feedback on
BalancesWatcherUpdater/NativeTokenBalanceUpdater. - The
toLowerCase()test normalization nit, which is already resolved withgetAddressKey.
🤖 Prompt for AI agents
Verify this finding against current PR head. Keep the fix minimal and validate it with focused tests.
Context:
- `CommonPriorityBalancesAndAllowancesUpdater` switches EVM chains to `BalancesWatcherUpdater` when `isBwEnabled` is true.
- `useBalancesWatcherSession` sets `error` and `isLoading: false` on create-session rejection or terminal SSE error, but leaves `hasFirstLoad: false`.
- `useTradeFormValidationContext` computes `isBalancesLoading` as `!hasFirstLoad || isBalancesLoading`, so pre-snapshot watcher failures remain stuck as loading.
- Expected behavior: pre-snapshot watcher failures should render a balance-load error/not-loaded state instead of permanent loading.
Generated using the pr-review skill from the CoW Protocol skills repo.
🧪 Scoped browser QA passed: watcher session + visible balance rows; swap execution not coveredAI usage: Codex drove the Playwright run and drafted this summary from the listed scenarios and QA artifacts. Outcome
Run details
How to retest
Artifacts
Chromium token-selector balances:
Firefox token-selector balances:
Not checked / follow-up
|


Summary
Wires the cowswap-frontend to the new balances-watcher SSE service, behind the LaunchDarkly flag isBwEnabled.
When the flag is on, balances are pushed in real time over an EventSource and the existing multicall pipeline (priority tokens + full token-list polling) is bypassed. When off, nothing changes — the multicall path stays as is.
Note on the native token (ETH / xDAI / MATIC / BNB): the watcher service does not emit native balances, so we keep polling them via a single eth_getBalance RPC call (wagmi useBalance), refetched every 11 seconds.
This means the native balance can lag up to 11s behind the on-chain state — that's an explicit trade-off because I reused current implementation, but if it's crucial - I can rework it.
Out of scope (planned for follow-up):
To Test
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores