Update to latest chronik and use it to handle mobile cases#619
Merged
chedieck merged 5 commits intoPayButton:masterfrom Feb 20, 2026
Merged
Update to latest chronik and use it to handle mobile cases#619chedieck merged 5 commits intoPayButton:masterfrom
chedieck merged 5 commits intoPayButton:masterfrom
Conversation
Contributor
📝 WalkthroughWalkthroughMigrates transaction sockets from Socket.IO to Chronik WebSocket, widens socket types to accept Changes
Sequence DiagramsequenceDiagram
actor User
participant Document
participant Widget
participant WidgetContainer
participant ChronikWS as Chronik WsEndpoint
participant API as Address API
User->>Document: Switches tab away
Document->>Widget: visibilitychange -> hidden
Widget->>ChronikWS: call pause()
Note over Widget,ChronikWS: Chronik stream paused
User->>Document: Returns to tab
Document->>WidgetContainer: visibilitychange -> visible
WidgetContainer->>API: checkForTransactions(getAddressDetails)
API-->>WidgetContainer: recent transactions
WidgetContainer->>WidgetContainer: handle new txs / maybe open dialog
Widget->>ChronikWS: call resume()
Note over Widget,ChronikWS: Chronik stream resumed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
react/lib/components/Widget/Widget.tsx (1)
95-95:⚠️ Potential issue | 🟡 Minor
WidgetProps.txsSockettype not updated to acceptWsEndpoint.The internal state
internalTxsSocketwas widened toSocket | WsEndpoint | undefined, butWidgetProps.txsSocketon line 95 still declarestxsSocket?: Socket. This means any external consumer passing aWsEndpointvia the prop would receive a TypeScript error. Currently this doesn't affect runtime behavior since the Chronik WS is always managed internally, but the types are now inconsistent with the internal union.♻️ Proposed fix
- txsSocket?: Socket + txsSocket?: Socket | WsEndpoint🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react/lib/components/Widget/Widget.tsx` at line 95, WidgetProps.txsSocket is still typed as Socket while internalTxsSocket was widened to Socket | WsEndpoint | undefined, causing a type mismatch for consumers passing a WsEndpoint; update the WidgetProps interface (the txsSocket prop) to use the same union type (Socket | WsEndpoint | undefined) so external props match internal state (refer to WidgetProps.txsSocket and internalTxsSocket and the WsEndpoint type).
🧹 Nitpick comments (2)
react/lib/components/Widget/Widget.tsx (1)
231-233: MoveisChronikWsEndpointto module scope.Defining a pure utility function inside the component body creates a new function reference on every render. Since it closes over nothing and is referenced in a
useEffect, it should live at module scope alongside other utility functions.♻️ Proposed refactor
Add above the component definition:
// Type guard to check if socket is a Chronik WsEndpoint const isChronikWsEndpoint = (socket: Socket | WsEndpoint | undefined): socket is WsEndpoint => { return socket instanceof WsEndpoint; };Then remove lines 230–233 from inside the component body.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react/lib/components/Widget/Widget.tsx` around lines 231 - 233, Move the pure type-guard isChronikWsEndpoint out of the component body to module scope: define the function at top-level (above the component) using the same signature (socket: Socket | WsEndpoint | undefined): socket is WsEndpoint and remove the inner definition from the component; keep references to WsEndpoint and Socket intact so existing useEffect code that calls isChronikWsEndpoint continues to work without recreating the function on every render.react/lib/util/socket.ts (1)
82-88:onMessage,setDialogOpen, andcheckSuccessInfoappear to be dead code.
onMessageis exported but is not imported or called anywhere in the provided code. ItsshouldTriggerOnSuccesslogic is already handled byWidgetContainer.handlePayment.SetupTxsSocketParams.setDialogOpenandcheckSuccessInfoare declared in the interface but never read insidesetupChronikWebSocket— the callback at line 97–99 only callsparams.setNewTxs.Consider either wiring
onMessageintosetupChronikWebSocketor removing it and the unused params from the interface.#!/bin/bash # Verify onMessage is not referenced elsewhere rg -n --type=ts "onMessage" react/libAlso applies to: 97-101, 104-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react/lib/util/socket.ts` around lines 82 - 88, The exported function onMessage and the interface fields SetupTxsSocketParams.setDialogOpen and checkSuccessInfo are dead — onMessage isn't called anywhere and setupChronikWebSocket's websocket callback only invokes params.setNewTxs; either wire onMessage into setupChronikWebSocket so incoming socket messages route through onMessage (so its shouldTriggerOnSuccess logic runs, e.g., call onMessage(message, params) from the websocket message handler) or remove onMessage and delete setDialogOpen and checkSuccessInfo from the SetupTxsSocketParams interface and any callers so the interface matches actual usage (leave setNewTxs and setTxsSocket intact); update exports accordingly and run the project grep to ensure no remaining references to onMessage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@react/lib/components/Widget/WidgetContainer.tsx`:
- Around line 284-303: checkForTransactions currently fetches recentTxs via
getAddressDetails and calls handleNewTransaction for each, but
handleNewTransaction only processes txs with tx.confirmed === false, so
transactions confirmed while the app was backgrounded are ignored; update the
polling logic and/or handleNewTransaction so confirmed transactions are treated
as successful fallback detections—either modify handleNewTransaction to handle
confirmed=true (mark payment success, update state/UI) or add a separate handler
(e.g., handleConfirmedTransaction) invoked from checkForTransactions for
tx.confirmed === true; ensure checkForTransactions still calls the appropriate
handler for all returned txs and that any success state (success flag) is set
when a confirmed matching tx is found.
- Around line 311-384: The visibility polling useEffect in WidgetContainer
registers handleVisibilityChange and can run in child instances; add the same
isChild guard used elsewhere so child widgets skip this effect: inside the
useEffect (before any listeners or async work) check if isChild === true and
return early, ensuring handleVisibilityChange, checkForTransactions,
getAddressDetails and any handlePayment calls are not registered/executed for
child instances. Update the dependency usage accordingly so the early-return
uses the existing isChild prop/state.
---
Outside diff comments:
In `@react/lib/components/Widget/Widget.tsx`:
- Line 95: WidgetProps.txsSocket is still typed as Socket while
internalTxsSocket was widened to Socket | WsEndpoint | undefined, causing a type
mismatch for consumers passing a WsEndpoint; update the WidgetProps interface
(the txsSocket prop) to use the same union type (Socket | WsEndpoint |
undefined) so external props match internal state (refer to
WidgetProps.txsSocket and internalTxsSocket and the WsEndpoint type).
---
Nitpick comments:
In `@react/lib/components/Widget/Widget.tsx`:
- Around line 231-233: Move the pure type-guard isChronikWsEndpoint out of the
component body to module scope: define the function at top-level (above the
component) using the same signature (socket: Socket | WsEndpoint | undefined):
socket is WsEndpoint and remove the inner definition from the component; keep
references to WsEndpoint and Socket intact so existing useEffect code that calls
isChronikWsEndpoint continues to work without recreating the function on every
render.
In `@react/lib/util/socket.ts`:
- Around line 82-88: The exported function onMessage and the interface fields
SetupTxsSocketParams.setDialogOpen and checkSuccessInfo are dead — onMessage
isn't called anywhere and setupChronikWebSocket's websocket callback only
invokes params.setNewTxs; either wire onMessage into setupChronikWebSocket so
incoming socket messages route through onMessage (so its shouldTriggerOnSuccess
logic runs, e.g., call onMessage(message, params) from the websocket message
handler) or remove onMessage and delete setDialogOpen and checkSuccessInfo from
the SetupTxsSocketParams interface and any callers so the interface matches
actual usage (leave setNewTxs and setTxsSocket intact); update exports
accordingly and run the project grep to ensure no remaining references to
onMessage.
Comment on lines
284
to
303
| const checkForTransactions = useCallback(async (): Promise<boolean> => { | ||
| if (success) { | ||
| // Payment already succeeded, stop checking | ||
| return true; | ||
| } | ||
|
|
||
| try { | ||
| const history = await getAddressDetails(to, apiBaseUrl); | ||
| // Save time by only checking the last few transactions | ||
| const recentTxs = history.slice(0, 5); | ||
|
|
||
| recentTxs.forEach(tx => { | ||
| handleNewTransaction(tx); | ||
| }); | ||
| return true; | ||
| } catch (error) { | ||
| // Failed to fetch history, there is no point in retrying | ||
| return false; | ||
| } | ||
| }, [success, to, apiBaseUrl, handleNewTransaction]); |
Contributor
There was a problem hiding this comment.
Polling misses transactions confirmed during backgrounding.
handleNewTransaction (line 275) only processes tx.confirmed === false. getAddressDetails can return already-confirmed transactions (e.g., if the user was backgrounded for longer than the confirmation time on avalanche-finalized XEC). Those txs are silently skipped, leaving the payment undetected until the next user interaction.
This is consistent with the WebSocket path (which also only handles TX_ADDED_TO_MEMPOOL), but worth explicitly noting since this polling is specifically designed as a fallback for the period when the WS was paused.
🛠️ Suggested fix
recentTxs.forEach(tx => {
- handleNewTransaction(tx);
+ // Also handle confirmed txs to catch payments confirmed during backgrounding
+ if (
+ tx.confirmed === true &&
+ isGreaterThanZero(resolveNumber(tx.amount))
+ ) {
+ handlePayment(tx);
+ } else {
+ handleNewTransaction(tx);
+ }
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@react/lib/components/Widget/WidgetContainer.tsx` around lines 284 - 303,
checkForTransactions currently fetches recentTxs via getAddressDetails and calls
handleNewTransaction for each, but handleNewTransaction only processes txs with
tx.confirmed === false, so transactions confirmed while the app was backgrounded
are ignored; update the polling logic and/or handleNewTransaction so confirmed
transactions are treated as successful fallback detections—either modify
handleNewTransaction to handle confirmed=true (mark payment success, update
state/UI) or add a separate handler (e.g., handleConfirmedTransaction) invoked
from checkForTransactions for tx.confirmed === true; ensure checkForTransactions
still calls the appropriate handler for all returned txs and that any success
state (success flag) is set when a confirmed matching tx is found.
Comment on lines
311
to
384
| useEffect(() => { | ||
| if (typeof document === 'undefined') { | ||
| return; | ||
| } | ||
|
|
||
| let wasHidden = document.hidden; | ||
| let hiddenTimestamp = 0; | ||
| let retryTimeoutId: NodeJS.Timeout | null = null; | ||
|
|
||
| const handleVisibilityChange = async () => { | ||
| // Clear any pending retry timeout | ||
| if (retryTimeoutId) { | ||
| clearTimeout(retryTimeoutId); | ||
| retryTimeoutId = null; | ||
| } | ||
|
|
||
| if (document.hidden) { | ||
| wasHidden = true; | ||
| hiddenTimestamp = Date.now(); | ||
| return; | ||
| } | ||
|
|
||
| // Debounce the event to avoid querying the history for spurious events. | ||
| // This happens specifically when the user clicks on the paybutton, | ||
| // before the app handles the deeplink. | ||
| if (!wasHidden || Date.now() - hiddenTimestamp < 200) { | ||
| wasHidden = false; | ||
| return; | ||
| } | ||
|
|
||
| wasHidden = false; | ||
|
|
||
| if (!to || success) { | ||
| // No destination or payment already succeeded, skip checking | ||
| return; | ||
| } | ||
|
|
||
| if (!disablePaymentId && !thisPaymentId) { | ||
| // Skip if paymentId is required but not yet set. This avoids matching | ||
| // transactions against undefined payment IDs. | ||
| return; | ||
| } | ||
|
|
||
| // Run immediately (attempt 1) | ||
| const checkCompleted = await checkForTransactions(); | ||
|
|
||
| // If check completed successfully but payment hasn't succeeded yet, | ||
| // Schedule a single retry after 2 seconds. This is only there to handle | ||
| // the case where the transaction is discovered after the app has been | ||
| // foregrounded, but before the chronik websocket is resumed. 2 seconds | ||
| // should be plenty for this case which is not expected to happen under | ||
| // normal circumstances. | ||
| // Note that we can't check success at this stage because it is captured | ||
| // and we would use a stale value. So we run the timeout unconditionally | ||
| // and let the useEffect cancel it if success turns true. Worst case it | ||
| // does an API call and then it's a no-op. | ||
| if (checkCompleted && !success) { | ||
| retryTimeoutId = setTimeout(async () => { | ||
| await checkForTransactions(); | ||
| retryTimeoutId = null; | ||
| }, 2000); | ||
| } | ||
| }; | ||
|
|
||
| document.addEventListener('visibilitychange', handleVisibilityChange); | ||
|
|
||
| return () => { | ||
| document.removeEventListener('visibilitychange', handleVisibilityChange); | ||
| if (retryTimeoutId) { | ||
| clearTimeout(retryTimeoutId); | ||
| retryTimeoutId = null; | ||
| } | ||
| }; | ||
| }, [to, thisPaymentId, success, disablePaymentId, checkForTransactions]); |
Contributor
There was a problem hiding this comment.
Missing isChild guard on the visibility polling effect.
Widget.tsx's Chronik WS setup explicitly skips child instances (isChild !== true check). However, this visibility polling effect has no equivalent guard. When WidgetContainer is rendered in a child context, it still registers a visibilitychange listener and calls getAddressDetails + handlePayment, potentially triggering duplicate payment notifications alongside the parent's own handling.
🛡️ Suggested fix
useEffect(() => {
if (typeof document === 'undefined') {
return;
}
+
+ // Skip polling in child context — the parent widget handles payment detection
+ if (isChild === true) {
+ return;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@react/lib/components/Widget/WidgetContainer.tsx` around lines 311 - 384, The
visibility polling useEffect in WidgetContainer registers handleVisibilityChange
and can run in child instances; add the same isChild guard used elsewhere so
child widgets skip this effect: inside the useEffect (before any listeners or
async work) check if isChild === true and return early, ensuring
handleVisibilityChange, checkForTransactions, getAddressDetails and any
handlePayment calls are not registered/executed for child instances. Update the
dependency usage accordingly so the early-return uses the existing isChild
prop/state.
Drop the legacy chronik-client-cashtokens and use the latest chronik-client version. Test Plan: yarn test
…round Use the dedicated pause() and resume() methods of WsEndpoint for this. This prevents the chronik client from failing when moving to background on mobile due to the lack of internet connection. Test Plan: On mobile, show a widget and move the browser to background. Check in the console that chronik no longer lose connection (an error is raised before this patch after a few seconds).
This is in the way for updating the type checking for chronik websocket. There is no change in behavior.
And use a type guard to ensure we are working on the expected type. There is no change in behavior.
Fabcien
force-pushed
the
chronik_latest
branch
from
d0d3e79 to
e92eb8f
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
react/lib/components/Widget/Widget.tsx (1)
230-233: Type guard should be extracted outside the component to avoid re-creation on every render.
isChronikWsEndpointis a pure function with no dependency on component state or props. Defining it inside the component body means it's recreated on every render. While perf impact is negligible, extracting it improves readability and signals that it's stateless.♻️ Suggested extraction
Move above the component definition (e.g., near line 56):
import { WsEndpoint } from 'chronik-client' + +/** Type guard to check if socket is a Chronik WsEndpoint */ +const isChronikWsEndpoint = ( + socket: Socket | WsEndpoint | undefined, +): socket is WsEndpoint => { + return socket instanceof WsEndpoint; +};Then remove Lines 230–233 from inside the component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@react/lib/components/Widget/Widget.tsx` around lines 230 - 233, The type guard isChronikWsEndpoint is a pure function and should be moved out of the component to avoid re-creation on each render; extract the isChronikWsEndpoint function (keeping its signature: (socket: Socket | WsEndpoint | undefined): socket is WsEndpoint) and place it above the Widget component definition (e.g., near other top-level helpers), then remove the in-component definition so the component references the top-level isChronikWsEndpoint instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@react/lib/components/Widget/Widget.tsx`:
- Around line 602-642: The visibility change handler (handleVisibilityChange
used in the useEffect) calls thisTxsSocket.resume() without awaiting its
Promise, so rejections escape the try/catch; update the resume branch to await
the async call (await thisTxsSocket.resume()) inside the existing try/catch and
ensure thisTxsSocket is still narrowed via isChronikWsEndpoint before awaiting;
keep the existing guards, pause() call, and event listener cleanup unchanged.
---
Duplicate comments:
In `@react/lib/components/Widget/WidgetContainer.tsx`:
- Around line 313-316: The visibility-polling useEffect in WidgetContainer.tsx
is missing the same isChild guard used by Widget.tsx's Chronik WS logic, so add
an early return in that effect to skip registering the document.visibilitychange
listener and polling when isChild === true (i.e., mirror the Widget.tsx check of
isChild !== true); update the effect that registers the visibilitychange handler
inside the WidgetContainer component's useEffect to return immediately for child
instances and keep the existing cleanup logic intact so listeners/timers are not
set for children.
- Around line 286-305: The polling fallback in checkForTransactions is skipping
already-confirmed payments because handleNewTransaction currently filters out
tx.confirmed === false; update the logic so polling's recentTxs (from
getAddressDetails and limited by POLL_TX_HISTORY_LOOKBACK) still triggers
processing of confirmed transactions: either add an optional parameter to
handleNewTransaction (e.g., forceProcessConfirmed) and call
handleNewTransaction(tx, { forceProcessConfirmed: true }) from
checkForTransactions, or add a separate handler invoked by checkForTransactions
that mirrors handleNewTransaction but accepts confirmed transactions; ensure the
new call path still deduplicates/validates transactions but does not drop
confirmed ones so backgrounded, now-confirmed payments are detected.
---
Nitpick comments:
In `@react/lib/components/Widget/Widget.tsx`:
- Around line 230-233: The type guard isChronikWsEndpoint is a pure function and
should be moved out of the component to avoid re-creation on each render;
extract the isChronikWsEndpoint function (keeping its signature: (socket: Socket
| WsEndpoint | undefined): socket is WsEndpoint) and place it above the Widget
component definition (e.g., near other top-level helpers), then remove the
in-component definition so the component references the top-level
isChronikWsEndpoint instead.
Now that chronik is properly paused and resumed, the retries are no longer necessary: if the event fired before the reconnect, it will be caught by the first polling round and otherwise by the chronik websocket event. We still keep a single retry for the very unlikely case where the polling is not successful but the event fires before the websocket reconnected. A single 2s delay is used for this case and should not be used in practice, it's only a safety net. The timeout is cleared upon success or visibility change, and worst case it's only an extra api call then a no-op. Test Plan: Check payment on mobile still works as expected.
Fabcien
force-pushed
the
chronik_latest
branch
from
e92eb8f to
947fb62
Compare
Fabcien
added a commit
to Fabcien/paybutton
that referenced
this pull request
Feb 20, 2026
So far only unconfirmed (aka mempool) transactions are parsed. This means that any transaction that is mined before it reaches chronik's mempool will be ignored and the payment missed, which is obviously very bad. The transaction confirmed message is added to the list of managed chronik messages, and the success is checked before processing to avoid triggering the payment success twice. Note that the filter exists in the socket.ts txsListener but this is dead code and is removed in PayButton#619. Test Plan: yarn test Check with a widget test, pay for it, check the success and wait for the block to be mined: the payment animation is not replayed.
Fabcien
added a commit
to Fabcien/paybutton
that referenced
this pull request
Feb 20, 2026
So far only unconfirmed (aka mempool) transactions are parsed. This means that any transaction that is mined before it reaches chronik's mempool will be ignored and the payment missed, which is obviously very bad. The transaction confirmed message is added to the list of managed chronik messages, and the success is checked before processing to avoid triggering the payment success twice. Note that the filter exists in the socket.ts txsListener but this is dead code and is removed in PayButton#619. Test Plan: yarn test Check with a widget test, pay for it, check the success and wait for the block to be mined: the payment animation is not replayed.
chedieck
approved these changes
Feb 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This migrates away from chronik-client-cashtokens to chronik-client latest version and leverage the pause() and resume() features to properly handle disconnecting/reconnecting when the widget goes to background/foreground on mobile.
The allows for simplification of the retry logic added in #617 now that the websocket is functional after it is resumed.
Test Plan:
Check the payment on mobile still works.
Check there is no websocket error anymore when the widget is left in the background for several seconds (without this patch it shows a disconnection error).
Note that if moved to background too quickly after resuming, the console might show a warning because the connection is closed before it was establish. This is only a warning message and has no consequence on the behavior.
This fixes #616 .
Summary by CodeRabbit
New Features
Bug Fixes / Performance
Chores