mirror of
https://github.com/purrgrammer/grimoire.git
synced 2026-04-12 00:17:02 +02:00
fix(wallet): address code review issues and simplify user menu
- Fix memory leak: track retry timeout and clear on disconnect - Add explicit WalletSupport type for support observable - Add comments explaining balance refresh error handling behavior - Add comment about restoreWallet not being awaited intentionally - User menu now uses connectionStatus observable (shows connecting/error states) - Remove wallet name display from user menu (simplifies UI) - Remove unused walletServiceProfile hook and getWalletName function https://claude.ai/code/session_01CnJgjFMvZHZWs2ujAiWAiQ
This commit is contained in:
@@ -118,18 +118,12 @@ export default function UserMenu() {
|
||||
return sats.toString();
|
||||
}
|
||||
|
||||
// Get wallet service profile for display name, using wallet relays as hints
|
||||
const walletServiceProfile = useProfile(
|
||||
nwcConnection?.service,
|
||||
nwcConnection?.relays,
|
||||
);
|
||||
|
||||
// Use wallet hook for real-time balance and methods
|
||||
// Use wallet hook for real-time balance and connection status
|
||||
const {
|
||||
disconnect: disconnectWallet,
|
||||
refreshBalance,
|
||||
balance,
|
||||
wallet,
|
||||
connectionStatus,
|
||||
} = useWallet();
|
||||
|
||||
function openProfile() {
|
||||
@@ -185,26 +179,6 @@ export default function UserMenu() {
|
||||
}
|
||||
}
|
||||
|
||||
function getWalletName(): string {
|
||||
if (!nwcConnection) return "";
|
||||
// Use service pubkey profile name, fallback to alias, then pubkey slice
|
||||
return (
|
||||
getDisplayName(nwcConnection.service, walletServiceProfile) ||
|
||||
nwcConnection.info?.alias ||
|
||||
nwcConnection.service.slice(0, 8)
|
||||
);
|
||||
}
|
||||
|
||||
function openWalletServiceProfile() {
|
||||
if (!nwcConnection?.service) return;
|
||||
addWindow(
|
||||
"profile",
|
||||
{ pubkey: nwcConnection.service },
|
||||
`Profile ${nwcConnection.service.slice(0, 8)}...`,
|
||||
);
|
||||
setShowWalletInfo(false);
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<SettingsDialog open={showSettings} onOpenChange={setShowSettings} />
|
||||
@@ -262,28 +236,23 @@ export default function UserMenu() {
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Wallet Name */}
|
||||
<div className="flex items-center justify-between">
|
||||
<span className="text-sm text-muted-foreground">Wallet:</span>
|
||||
<button
|
||||
onClick={openWalletServiceProfile}
|
||||
className="text-sm font-medium hover:underline cursor-crosshair text-primary"
|
||||
>
|
||||
{getWalletName()}
|
||||
</button>
|
||||
</div>
|
||||
|
||||
{/* Connection Status */}
|
||||
<div className="flex items-center justify-between">
|
||||
<span className="text-sm text-muted-foreground">Status:</span>
|
||||
<div className="flex items-center gap-2">
|
||||
<span
|
||||
className={`size-2 rounded-full ${
|
||||
wallet ? "bg-green-500" : "bg-red-500"
|
||||
connectionStatus === "connected"
|
||||
? "bg-green-500"
|
||||
: connectionStatus === "connecting"
|
||||
? "bg-yellow-500 animate-pulse"
|
||||
: connectionStatus === "error"
|
||||
? "bg-red-500"
|
||||
: "bg-gray-500"
|
||||
}`}
|
||||
/>
|
||||
<span className="text-sm font-medium">
|
||||
{wallet ? "Connected" : "Disconnected"}
|
||||
<span className="text-sm font-medium capitalize">
|
||||
{connectionStatus}
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
@@ -409,16 +378,17 @@ export default function UserMenu() {
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<div className="flex items-center gap-1.5">
|
||||
<span
|
||||
className={`size-1.5 rounded-full ${
|
||||
wallet ? "bg-green-500" : "bg-red-500"
|
||||
}`}
|
||||
/>
|
||||
<span className="text-xs text-muted-foreground">
|
||||
{getWalletName()}
|
||||
</span>
|
||||
</div>
|
||||
<span
|
||||
className={`size-1.5 rounded-full ${
|
||||
connectionStatus === "connected"
|
||||
? "bg-green-500"
|
||||
: connectionStatus === "connecting"
|
||||
? "bg-yellow-500 animate-pulse"
|
||||
: connectionStatus === "error"
|
||||
? "bg-red-500"
|
||||
: "bg-gray-500"
|
||||
}`}
|
||||
/>
|
||||
</DropdownMenuItem>
|
||||
) : (
|
||||
<DropdownMenuItem
|
||||
|
||||
@@ -26,6 +26,7 @@
|
||||
import { useEffect, useMemo, useRef } from "react";
|
||||
import { use$ } from "applesauce-react/hooks";
|
||||
import { useGrimoire } from "@/core/state";
|
||||
import type { WalletSupport } from "applesauce-wallet-connect/helpers";
|
||||
import {
|
||||
wallet$,
|
||||
restoreWallet,
|
||||
@@ -53,8 +54,11 @@ export function useWallet() {
|
||||
const lastError = use$(lastError$);
|
||||
const transactionsState = use$(transactionsState$);
|
||||
|
||||
// Wallet support from library's support$ observable (cached)
|
||||
const support = use$(() => wallet?.support$, [wallet]);
|
||||
// Wallet support from library's support$ observable (cached by library for 60s)
|
||||
const support: WalletSupport | null | undefined = use$(
|
||||
() => wallet?.support$,
|
||||
[wallet],
|
||||
);
|
||||
|
||||
// Wallet methods - combines reactive support$ with cached info fallback
|
||||
// The support$ waits for kind 13194 events which some wallets don't publish
|
||||
@@ -63,6 +67,8 @@ export function useWallet() {
|
||||
}, [support?.methods, state.nwcConnection?.info?.methods]);
|
||||
|
||||
// Restore wallet on mount if connection exists
|
||||
// Note: Not awaited intentionally - wallet is available synchronously from wallet$
|
||||
// before validation completes. Any async errors are handled within restoreWallet.
|
||||
useEffect(() => {
|
||||
if (nwcConnection && !wallet && !restoreAttemptedRef.current) {
|
||||
restoreAttemptedRef.current = true;
|
||||
|
||||
@@ -25,6 +25,7 @@ WalletConnect.pool = pool;
|
||||
|
||||
// Internal state
|
||||
let notificationSubscription: Subscription | null = null;
|
||||
let notificationRetryTimeout: ReturnType<typeof setTimeout> | null = null;
|
||||
|
||||
/**
|
||||
* Connection status for the NWC wallet
|
||||
@@ -75,9 +76,13 @@ function hexToBytes(hex: string): Uint8Array {
|
||||
* Notifications trigger balance refresh for real-time updates.
|
||||
*/
|
||||
function subscribeToNotifications(wallet: WalletConnect) {
|
||||
// Clean up existing subscription
|
||||
// Clean up existing subscription and pending retry
|
||||
notificationSubscription?.unsubscribe();
|
||||
notificationSubscription = null;
|
||||
if (notificationRetryTimeout) {
|
||||
clearTimeout(notificationRetryTimeout);
|
||||
notificationRetryTimeout = null;
|
||||
}
|
||||
|
||||
let retryCount = 0;
|
||||
const maxRetries = 5;
|
||||
@@ -109,7 +114,7 @@ function subscribeToNotifications(wallet: WalletConnect) {
|
||||
const delay = baseDelay * Math.pow(2, retryCount);
|
||||
retryCount++;
|
||||
connectionStatus$.next("connecting");
|
||||
setTimeout(subscribe, delay);
|
||||
notificationRetryTimeout = setTimeout(subscribe, delay);
|
||||
} else {
|
||||
connectionStatus$.next("error");
|
||||
lastError$.next(
|
||||
@@ -124,7 +129,7 @@ function subscribeToNotifications(wallet: WalletConnect) {
|
||||
if (wallet$.value && retryCount < maxRetries) {
|
||||
const delay = baseDelay * Math.pow(2, retryCount);
|
||||
retryCount++;
|
||||
setTimeout(subscribe, delay);
|
||||
notificationRetryTimeout = setTimeout(subscribe, delay);
|
||||
}
|
||||
},
|
||||
});
|
||||
@@ -209,8 +214,13 @@ export async function restoreWallet(
|
||||
* Disconnects and clears the wallet.
|
||||
*/
|
||||
export function clearWallet(): void {
|
||||
// Clean up subscription and pending retry
|
||||
notificationSubscription?.unsubscribe();
|
||||
notificationSubscription = null;
|
||||
if (notificationRetryTimeout) {
|
||||
clearTimeout(notificationRetryTimeout);
|
||||
notificationRetryTimeout = null;
|
||||
}
|
||||
|
||||
wallet$.next(null);
|
||||
balance$.next(undefined);
|
||||
@@ -221,7 +231,12 @@ export function clearWallet(): void {
|
||||
|
||||
/**
|
||||
* Refreshes the balance from the wallet.
|
||||
* Includes retry logic for reliability.
|
||||
* Includes retry logic with exponential backoff for reliability.
|
||||
*
|
||||
* Note: If we're already connected and a balance fetch fails after retries,
|
||||
* we don't set error state. This prevents UI flapping - the notification
|
||||
* subscription is the primary health indicator. A transient balance fetch
|
||||
* failure shouldn't mark an otherwise working connection as errored.
|
||||
*/
|
||||
export async function refreshBalance(): Promise<number | undefined> {
|
||||
const wallet = wallet$.value;
|
||||
@@ -253,6 +268,7 @@ export async function refreshBalance(): Promise<number | undefined> {
|
||||
setTimeout(r, baseDelay * Math.pow(2, attempt)),
|
||||
);
|
||||
} else if (connectionStatus$.value !== "connected") {
|
||||
// Only set error state if not already connected (e.g., during initial validation)
|
||||
connectionStatus$.next("error");
|
||||
lastError$.next(
|
||||
error instanceof Error ? error : new Error("Failed to get balance"),
|
||||
|
||||
Reference in New Issue
Block a user