Skip to content
Merged
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
107 changes: 107 additions & 0 deletions react/lib/components/Widget/WidgetContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ import {
shouldTriggerOnSuccess,
isPropsTrue,
DEFAULT_DONATION_RATE,
POLL_TX_HISTORY_LOOKBACK,
POLL_REQUEST_DELAY,
POLL_MAX_RETRY,
} from '../../util';
import { getAddressDetails } from '../../util/api-client';

import Widget, { WidgetProps } from './Widget';

Expand Down Expand Up @@ -157,6 +161,7 @@ export const WidgetContainer: React.FunctionComponent<WidgetContainerProps> =
const [thisPrice, setThisPrice] = useState(0);
const [usdPrice, setUsdPrice] = useState(0);
const [success, setSuccess] = useState(false);
const [retryCount, setRetryCount] = useState(0);
const { enqueueSnackbar } = useSnackbar();

const [shiftCompleted, setShiftCompleted] = useState(false);
Expand Down Expand Up @@ -280,12 +285,114 @@ export const WidgetContainer: React.FunctionComponent<WidgetContainerProps> =
[handlePayment],
);

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, POLL_TX_HISTORY_LOOKBACK);

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]);

useEffect(() => {
thisNewTxs?.map(tx => {
handleNewTransaction(tx);
});
}, [thisNewTxs, handleNewTransaction]);

useEffect(() => {
if (typeof document === 'undefined') {
return;
}

let wasHidden = document.hidden;
let hiddenTimestamp = 0;
Comment on lines +320 to +321
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

wasHidden resets to document.hidden on every effect re-run, risking a spurious check

wasHidden and hiddenTimestamp are local variables initialised at effect setup time. Whenever any dep in [to, thisPaymentId, success, disablePaymentId] changes (e.g., thisPaymentId is set while the device is already backgrounded), the old listener is torn down and a new one is registered with wasHidden = document.hidden (i.e., true) and hiddenTimestamp = 0.

On the next visibilitychange to visible, the debounce check is !wasHidden || Date.now() - hiddenTimestamp < 200. With wasHidden = true and hiddenTimestamp = 0, this evaluates to false || (Date.now() - 0 < 200) which is false, so the check proceeds — even though no hide event occurred while this component was mounted. This triggers an unintended immediate poll.

A simple fix is to initialise hiddenTimestamp to the current timestamp so the 200 ms window still applies, or to not set wasHidden = true during initial setup when the page is already hidden:

🔧 Proposed fix
-      let wasHidden = document.hidden;
-      let hiddenTimestamp = 0;
+      // Only track hides that happen after this listener is registered.
+      let wasHidden = false;
+      let hiddenTimestamp = 0;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let wasHidden = document.hidden;
let hiddenTimestamp = 0;
// Only track hides that happen after this listener is registered.
let wasHidden = false;
let hiddenTimestamp = 0;
🤖 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 333 - 334, The
effect in WidgetContainer that sets up the visibilitychange listener resets
local vars wasHidden and hiddenTimestamp on every re-run (deps: [to,
thisPaymentId, success, disablePaymentId]) causing a spurious poll when the page
is already hidden; fix it by initializing hiddenTimestamp to Date.now() (instead
of 0) and keep wasHidden set from document.hidden so the 200ms debounce still
blocks immediate polling on mount, i.e., in the setup that declares let
wasHidden = document.hidden; let hiddenTimestamp = 0; change hiddenTimestamp to
Date.now() so the visibilitychange handler (used in the effect) treats the
initial hidden state as recently hidden.


const handleVisibilityChange = async () => {
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,
// trigger retries. We might be missing the payment transaction.
if (checkCompleted && !success) {
// Start retries
setRetryCount(1);
}
};

document.addEventListener('visibilitychange', handleVisibilityChange);

return () => {
document.removeEventListener('visibilitychange', handleVisibilityChange);
};
}, [to, thisPaymentId, success, disablePaymentId]);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Stale closure: checkForTransactions missing from dependency arrays of both effects.

Both the visibility effect (line 382) and the retry effect (line 408) call checkForTransactions but omit it from their dependency arrays. When checkForTransactions gets a new identity (e.g., because handleNewTransactionhandlePayment deps like currencyObj or thisPrice change), these effects continue to invoke the stale version that closes over outdated payment parameters. This can cause missed payment matches or false negatives after a price/currency update mid-session.

Proposed fix
     }, [to, thisPaymentId, success, disablePaymentId]);
+    // ↑ add checkForTransactions to deps

     // Retry mechanism: check every second if payment hasn't succeeded yet
     useEffect(() => {
       ...
-    }, [retryCount, success]);
+    }, [retryCount, success, checkForTransactions]);

For the visibility effect, adding checkForTransactions will cause re-registration when payment parameters change, which is the correct behavior. You may want to also remove to and success from that array if they're already captured transitively through checkForTransactions, but keeping them explicit is fine for clarity.

Also applies to: 408-408

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@react/lib/components/Widget/WidgetContainer.tsx` at line 382, The effects
that register visibility and retry behavior are missing checkForTransactions in
their dependency arrays, causing them to keep calling a stale closure when
checkForTransactions' identity changes (which happens when
handleNewTransaction/handlePayment dependencies like currencyObj or thisPrice
change); update both useEffect dependency arrays that currently list [to,
thisPaymentId, success, disablePaymentId] and its retry counterpart to include
checkForTransactions so the effects re-subscribe when the matcher logic changes
(you may also remove redundant items like to/success if they are already
captured transitively via checkForTransactions, but adding checkForTransactions
is required).


// Retry mechanism: check every second if payment hasn't succeeded yet
useEffect(() => {

if (retryCount === 0 || success || retryCount >= POLL_MAX_RETRY) {
// Retry up to 5 times or until the payment succeeds. If the payment tx
// is not found within this time period, something has gone wrong.
return;
}

const intervalId = setInterval(async () => {
if (success) {
// Stop retries upon success
setRetryCount(0);
return;
}

await checkForTransactions();

// Increment retry count for next attempt (regardless of success/error)
setRetryCount(prev => prev + 1);
}, POLL_REQUEST_DELAY);

return () => {
clearInterval(intervalId);
};
}, [retryCount, success]);

return (
<React.Fragment>
<Widget
Expand Down
4 changes: 4 additions & 0 deletions react/lib/util/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,7 @@ export const DEFAULT_MINIMUM_DONATION_AMOUNT: { [key: string]: number } = {
BCH: 0.00001000,
XEC: 10,
};

export const POLL_TX_HISTORY_LOOKBACK = 5 // request last 5 txs
export const POLL_REQUEST_DELAY = 1000 // 1s
export const POLL_MAX_RETRY = 5