[pietro909]

1. Debounce reloadWallet
2. Only act on VTXOs delegation if delegate is disabled

Summary by CodeRabbit

• Improvements

  • Wallet now consolidates rapid update notifications into a single short-delayed refresh to prevent redundant reloads.
  • Service-worker message handling simplified for more reliable and less error-prone update delivery.

• Changes

  • Wallet startup behavior adjusted: automatic coin delegation at startup removed so startup follows configuration-driven renewal only.

[ghost]

🔍 Arkana PR Review — wallet#464

Reduce requests number by @pietro909

Summary

Two targeted optimizations to reduce redundant network requests on VTXO/UTXO updates.

Changes Reviewed

1. Debounced reloadWallet (good)
Replaces the old pattern (immediate reload + second reload after 5s) with a single debounced call at 1s. When multiple VTXO_UPDATE/UTXO_UPDATE messages arrive in quick succession, only the last one triggers a reload. Clean improvement.

2. Remove delegateVtxos call when delegation is enabled (good)
When config.delegate is true, the SDK's VtxoManager handles delegation automatically via onContractEvent, so the wallet-side call was redundant. Only renewCoins is now called, and only in non-delegate mode.

Notes

• The delegateVtxos import is correctly removed from the import block.
• The reloadTimer variable is scoped inside the useEffect, so cleanup via clearTimeout on the next event is correct. However, there's no cleanup of reloadTimer in the effect's teardown return. If the component unmounts while a timer is pending, the callback would fire on a stale svcWallet reference. Consider adding clearTimeout(reloadTimer) to the cleanup function that already removes the event listener.

Verdict

Clean, focused change. The debounce pattern is a solid improvement. Minor suggestion about cleanup on unmount above — low risk but worth considering.

[coderabbitai]

Walkthrough

Debounce service-worker VTXO/UTXO messages to schedule a single reload after 1000ms, switch service-worker listener management to store/remove the exact handler function, and remove the startup delegateVtxos call so startup only calls renewCoins when delegation is disabled.

Changes

Cohort / File(s)	Summary
Wallet provider src/providers/wallet.tsx	Debounced reload: replace immediate reload + delayed follow-up with a single 1000ms debounced reloadWallet(svcWallet) using reloadTimerRef.
Service-worker listener handling src/providers/wallet.tsx	Track the service-worker message handler in swMessageHandlerRef and remove/add the exact function when registering, removing the previous listeningForServiceWorker flag gating.
Startup delegation flow src/providers/wallet.tsx	Remove call to delegateVtxos(svcWallet) and associated import; retain renewCoins(svcWallet, ...) only when !config.delegate (no startup delegation from this provider).

Sequence Diagram(s)

sequenceDiagram
  participant SW as ServiceWorker
  participant Provider as WalletProvider
  participant Timer as ReloadTimer
  participant Wallet as reloadWallet / renewCoins

  rect rgba(173,216,230,0.5)
  SW->>Provider: postMessage(VTXO_UPDATE / UTXO_UPDATE)
  end

  Provider->>Provider: clear existing reloadTimerRef
  Provider->>Timer: set timeout (1000ms)
  Timer--)Provider: timeout triggers
  Provider->>Wallet: reloadWallet(svcWallet)
  alt delegation disabled
    Provider->>Wallet: renewCoins(svcWallet, ...)
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Use new Service Worker #316: Modifies service-worker message handling and ServiceWorker wallet wiring in src/providers/wallet.tsx (reload/handler logic overlap).
• add vtxo manager #168: Changes vtxo lifecycle/initialization (VtxoManager / renewals) and touches startup vtxo logic in the same file.
• asp.ts: read history and vtxos from database #170: Adjusts reloadWallet usage and interacts with ServiceWorkerWallet handling used by these reload paths.

Suggested reviewers

• bordalix
• louisinger

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Reduce requests number' accurately reflects the main objective of the PR, which focuses on debouncing wallet reloads and avoiding unnecessary delegation calls to reduce API requests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch reduce-requests-number

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/providers/wallet.tsx (1)

312-319: Debounce implementation looks correct, but consider timer cleanup on reinitialization.

The debounce pattern works well for the normal flow. However, if initSvcWorkerWallet is called again (via restartWallet or retry), any pending timer from the previous closure will still fire with a stale svcWallet reference. Consider promoting reloadTimer to a useRef or clearing it at the start of initSvcWorkerWallet.

♻️ Optional: Use a ref to ensure cleanup across reinitializations

+  const reloadTimerRef = useRef<ReturnType<typeof setTimeout>>()
+
   const initSvcWorkerWallet = async ({
     ...
   }) => {
     try {
+      // Clear any pending reload from previous initialization
+      clearTimeout(reloadTimerRef.current)
       ...
-      let reloadTimer: ReturnType<typeof setTimeout> | undefined
       const handleServiceWorkerMessages = (event: MessageEvent) => {
         if (event.data && ['VTXO_UPDATE', 'UTXO_UPDATE'].includes(event.data.type)) {
-          clearTimeout(reloadTimer)
-          reloadTimer = setTimeout(() => reloadWallet(svcWallet), 1000)
+          clearTimeout(reloadTimerRef.current)
+          reloadTimerRef.current = setTimeout(() => reloadWallet(svcWallet), 1000)
         }
       }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/providers/wallet.tsx` around lines 312 - 319, The debounce timer
reloadTimer in handleServiceWorkerMessages can survive across re-initializations
of initSvcWorkerWallet and call reloadWallet with a stale svcWallet; fix by
promoting reloadTimer to a React ref (e.g., useRef<ReturnType<typeof setTimeout>
| undefined>) or ensure you clearTimeout(reloadTimer) at the very start of
initSvcWorkerWallet (and when restartWallet runs) so any pending timer is
cancelled before re-registering the service worker handlers; update references
in handleServiceWorkerMessages and any cleanup logic to use the ref or cleared
variable.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/providers/wallet.tsx`:
- Around line 312-319: The debounce timer reloadTimer in
handleServiceWorkerMessages can survive across re-initializations of
initSvcWorkerWallet and call reloadWallet with a stale svcWallet; fix by
promoting reloadTimer to a React ref (e.g., useRef<ReturnType<typeof setTimeout>
| undefined>) or ensure you clearTimeout(reloadTimer) at the very start of
initSvcWorkerWallet (and when restartWallet runs) so any pending timer is
cancelled before re-registering the service worker handlers; update references
in handleServiceWorkerMessages and any cleanup logic to use the ref or cleared
variable.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8a05f480-0493-4e6a-83ed-627801d24a1e

📥 Commits

Reviewing files that changed from the base of the PR and between 25c38ca and 6ed9e8f.

📒 Files selected for processing (1)

• src/providers/wallet.tsx

[cloudflare-workers-and-pages]

Deploying wallet-mutinynet with    Cloudflare Pages

Latest commit:	5b7da51
Status:	✅  Deploy successful!
Preview URL:	https://b40b3824.arkade-wallet.pages.dev
Branch Preview URL:	https://reduce-requests-number.arkade-wallet.pages.dev

View logs

[cloudflare-workers-and-pages]

Deploying wallet-bitcoin with    Cloudflare Pages

Latest commit:	5b7da51
Status:	✅  Deploy successful!
Preview URL:	https://76e87c34.wallet-bitcoin.pages.dev
Branch Preview URL:	https://reduce-requests-number.wallet-bitcoin.pages.dev

View logs

[ghost]

🔍 Arkana Review — Incremental (commit 75186c6)

Scope: Addresses stale timer and listener accumulation from prior review feedback.

Changes

1. reloadTimerRef — Stores the debounce timeout in a ref, cleared on wallet re-initialization (clearTimeout(reloadTimerRef.current) before setting up new listener). Prevents stale closures from firing reloads on a previous wallet instance.

2. swMessageHandlerRef — Replaces the listeningForServiceWorker boolean guard with a ref to the actual handler function. On re-init, the old handler is explicitly removed before attaching the new one. This prevents listener accumulation across wallet switches — clean fix.

3. Debounce consolidation — The previous dual-reload pattern (reloadWallet + setTimeout(..., 5000)) is replaced by a single 1s debounced reload via clearTimeout/setTimeout. Multiple rapid VTXO_UPDATE/UTXO_UPDATE events now only trigger one reload.

Assessment

All three issues (stale timer, listener accumulation, redundant fetches) are properly addressed:

• Timer cleanup on wallet re-init ✅
• Listener cleanup via ref-tracked handler ✅
• Debounce prevents burst reloads ✅

The delegation guard cleanup (!config.delegate inversion, removing the now-unnecessary delegateVtxos call) is also correct — VtxoManager handles auto-delegation via onContractEvent when delegation is enabled.

LGTM — Clean incremental fix.

[ghost]

🔍 Review — New commits since last review

Commits reviewed: feeaa433..5b7da51c (4 commits)

Changes

1. Debounced reloadWallet — Service worker VTXO_UPDATE/UTXO_UPDATE messages now reset a 1s debounce timer instead of firing immediately + a 5s delayed duplicate. Good fix: rapid round settlements could trigger cascading reloads.

2. Listener accumulation fix — swMessageHandlerRef properly tracks the current handler and removes it before adding a new one on wallet re-init. Previous listeningForServiceWorker boolean flag could leak stale handlers pointing at old svcWallet instances.

3. Removed delegateVtxos on startup — When config.delegate is enabled, the SDK's VtxoManager handles delegation via onContractEvent, so the explicit wallet-side call was redundant (and could race with the SDK's own delegation).

Notes

• The clearTimeout(reloadTimerRef.current) at the top of wallet init is a nice defensive touch — prevents a pending reload from a previous wallet instance from firing against stale state.
• 1s debounce delay (down from 5s) is reasonable given the indexer is local. If indexer cache staleness becomes an issue, this is the knob to tune.
• No security concerns — no key material or transaction signing paths affected.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/providers/wallet.tsx`:
- Around line 331-351: In lockWallet, also clear the debounced reload timer and
unregister the service worker message handler: call
clearTimeout(reloadTimerRef.current) and set reloadTimerRef.current = null,
remove the listener via navigator.serviceWorker.removeEventListener('message',
swMessageHandlerRef.current) and set swMessageHandlerRef.current = null, and
ensure you still clear statusPingInterval as before; this prevents reloadWallet
from being invoked after wallet is cleared and avoids leaving the message
listener registered.
- Around line 368-373: The comment above the renewCoins call is inaccurate about
how delegation works; update the comment near renewCoins(svcWallet,
aspInfo.dust, wallet.thresholdMs) to state that when delegation is enabled the
SDK auto-delegates internally via the delegatorUrl passed during wallet
initialization (see initSvcWorkerWallet and delegatorUrl), so wallet-side
renewCoins is skipped in delegate mode; keep the existing best-effort error
handling (.catch(() => {})) unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 43bab0de-da10-4ba5-9c5c-dbe009945865

📥 Commits

Reviewing files that changed from the base of the PR and between 75186c6 and 5b7da51.

📒 Files selected for processing (1)

• src/providers/wallet.tsx