feat: implement Wallet Not Ready modal and readiness checks#462
Merged
Benjtalkshow merged 2 commits intoboundlessfi:mainfrom Mar 9, 2026
Merged
feat: implement Wallet Not Ready modal and readiness checks#462Benjtalkshow merged 2 commits intoboundlessfi:mainfrom
Benjtalkshow merged 2 commits intoboundlessfi:mainfrom
Conversation
|
@Agbeleshe is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR adds wallet-readiness checks and UI to prevent actions with unactivated or underfunded wallets: a new useWalletReadiness hook, expanded useWalletProtection with not-ready modal controls, a WalletNotReadyModal component, and wiring of these checks/UX into FundEscrow, FundProject, and Initialize flows. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Component (Init / FundEscrow / FundProject)
participant Protection as useWalletProtection
participant Readiness as useWalletReadiness
participant Modal as WalletNotReadyModal / WalletRequiredModal
participant Wallet as WalletSheet / WalletProvider
participant Network as Backend / Ledger
User->>UI: Click action (Create / Fund)
UI->>Protection: requireWallet(callback, requiredUsdc)
alt No walletAddress
Protection->>Modal: show WalletRequiredModal
Modal->>Wallet: User opens WalletSheet
Wallet->>Protection: onWalletConnected
else Wallet present
Protection->>Readiness: checkReadiness(requiredUsdc)
Readiness-->>Protection: {isReady: false, reasons}
Protection->>Modal: show WalletNotReadyModal(with reasons)
Modal->>Wallet: User clicks "Open wallet"
Wallet->>User: Sync / Add trustline / Top up balances
User->>Modal: Close modal
Modal->>Protection: closeNotReadyModal
Protection->>Readiness: re-check readiness
alt Ready
Protection->>UI: proceed callback
UI->>Network: construct & sign tx
UI->>Network: sendTransaction
Network-->>UI: success / error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/modals/fund-project/index.tsx (1)
307-403:⚠️ Potential issue | 🟠 MajorCallback result not awaited — errors may be silently ignored.
The async callback passed to
requireWallet(lines 307-402) will execute, but sincerequireWalletdoesn't await the callback (seeuse-wallet-protection.tsline 47), any errors thrown inside won't propagate to thewalletValidcheck. This meanswalletValidwill betrueeven if the funding fails.This is a downstream effect of the issue in
use-wallet-protection.ts. Once that hook awaits the callback, this code will work correctly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/modals/fund-project/index.tsx` around lines 307 - 403, The async callback passed to requireWallet is not being awaited so errors inside (e.g., in fundEscrow, signTransaction, sendTransaction) can be swallowed and walletValid will be incorrect; fix by awaiting the callback call here (change the call to await requireWallet(...)) and/or update the requireWallet implementation in the useWalletProtection hook to await and propagate the passed async callback (ensure it does await cb(...) and returns/rethrows errors) so walletValid reflects failures.
🧹 Nitpick comments (6)
hooks/use-wallet-readiness.ts (2)
22-58: Consider memoizingcheckReadinesswithuseCallback.The
checkReadinessfunction is recreated on every render. If consumers pass it as a dependency to effects or memoized values, this could cause unnecessary re-executions. Wrapping it inuseCallbackwith[walletAddress, balances]dependencies would provide a stable reference.♻️ Memoize checkReadiness
+import { useCallback } from 'react'; + export function useWalletReadiness() { const { walletAddress, balances } = useWalletContext(); - const checkReadiness = (requiredUsdc: number = 0): WalletReadinessResult => { + const checkReadiness = useCallback((requiredUsdc: number = 0): WalletReadinessResult => { if (!walletAddress) { return { isReady: false, reasons: [], hasWallet: false }; } // ... rest of function return { isReady: reasons.length === 0, reasons, hasWallet: true }; - }; + }, [walletAddress, balances]); return { checkReadiness }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-readiness.ts` around lines 22 - 58, The checkReadiness function is recreated each render; wrap the const checkReadiness declaration in useCallback to memoize it and stabilize its reference (e.g., const checkReadiness = useCallback((requiredUsdc = 0) => { ... }, [walletAddress, balances])); ensure you include walletAddress and balances in the dependency array so the logic updates when those change.
1-1: Remove unuseduseMemoimport.
useMemois imported but not used in this hook.🧹 Remove unused import
-import { useMemo } from 'react'; +import { useCallback } from 'react';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-readiness.ts` at line 1, Remove the unused import by deleting the imported symbol useMemo from the top of the file (the ES import statement that currently reads "import { useMemo } from 'react';"); ensure only needed React imports remain so there are no unused symbols in hooks/use-wallet-readiness.ts.components/escrow/FundEscrowButton.tsx (2)
77-81: Redundant wallet check — already handled byrequireWallet.The guard at lines 78-81 checks
walletAddressand shows an error, butrequireWallet(line 116) already performs this check and shows the appropriate modal. This guard will never trigger sincehandleFundEscrowproceeds torequireWalletwhich handles the missing wallet case.Consider removing this redundant check, or if you want early termination before
setIsLoading(true), move it before line 103.🧹 Remove redundant check or relocate
const handleFundEscrow = async () => { - if (!walletAddress) { - toast.error('Please connect your wallet first'); - return; - } - if (!contractId) {Or relocate the
requireWalletcall to the top of the function before validation:const handleFundEscrow = async () => { + // Early wallet check before any state changes + const walletReady = await requireWallet(undefined, 0); + if (!walletReady) return; + if (!contractId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/escrow/FundEscrowButton.tsx` around lines 77 - 81, The walletAddress guard in handleFundEscrow is redundant because requireWallet already enforces and shows the wallet modal; remove the initial if (!walletAddress) { toast.error... return; } block from handleFundEscrow so only requireWallet is used, or alternatively call await requireWallet() as the very first action in handleFundEscrow (before setIsLoading or other state changes) to ensure early termination; update any related comments and keep function behavior consistent with requireWallet's modal handling.
23-28: Move type definition after all imports for consistency.The
MultiReleaseEscrowWithBalancetype alias is defined between imports, breaking the import grouping convention. Consider moving it after all imports.🧹 Reorganize imports and type definition
import { toast } from 'sonner'; import { reportError } from '@/lib/error-reporting'; import { useWalletProtection } from '@/hooks/use-wallet-protection'; import WalletRequiredModal from '@/components/wallet/WalletRequiredModal'; import WalletNotReadyModal from '@/components/wallet/WalletNotReadyModal'; import { WalletSheet } from '@/components/wallet/WalletSheet'; - -// Extended type to include balance property that may exist at runtime -// Using intersection type to avoid type conflicts with required balance property -type MultiReleaseEscrowWithBalance = MultiReleaseEscrow & { - balance?: number; -}; import { Loader2, CheckCircle2, DollarSign } from 'lucide-react'; import { Card, CardContent, CardDescription, CardHeader, CardTitle, } from '@/components/ui/card'; + +// Extended type to include balance property that may exist at runtime +// Using intersection type to avoid type conflicts with required balance property +type MultiReleaseEscrowWithBalance = MultiReleaseEscrow & { + balance?: number; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/escrow/FundEscrowButton.tsx` around lines 23 - 28, The type alias MultiReleaseEscrowWithBalance is declared among import statements which breaks the import grouping convention; move the MultiReleaseEscrowWithBalance definition so it appears after all import lines in FundEscrowButton.tsx (i.e., keep imports like Loader2, CheckCircle2, DollarSign together at the top and then declare the MultiReleaseEscrowWithBalance type), ensuring no other imports or code are interleaved with the type.hooks/use-wallet-protection.ts (1)
1-1: Remove unuseduseCallbackimport.
useCallbackis imported but not used in this hook.🧹 Remove unused import
-import { useState, useCallback } from 'react'; +import { useState } from 'react';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-protection.ts` at line 1, Remove the unused import symbol useCallback from the import statement at the top of the file (currently: import { useState, useCallback } from 'react';) so only used symbols remain (e.g., import { useState } from 'react';) to eliminate the unused import warning.components/wallet/WalletNotReadyModal.tsx (1)
11-12: Remove unused imports.
CheckCircle2andImageare imported but never used in this component.🧹 Remove unused imports
-import { XIcon, AlertCircle, CheckCircle2, Wallet, ExternalLink } from 'lucide-react'; -import Image from 'next/image'; +import { XIcon, AlertCircle, Wallet, ExternalLink } from 'lucide-react';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/wallet/WalletNotReadyModal.tsx` around lines 11 - 12, The import list in WalletNotReadyModal includes unused symbols; remove CheckCircle2 and Image from the import statement so only used icons (XIcon, AlertCircle, Wallet, ExternalLink) and any other used modules remain; update the import line that currently references CheckCircle2 and Image to avoid unused-import lint errors and unnecessary bundle size.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/escrow/FundEscrowButton.tsx`:
- Around line 166-174: The code adds a balance property to an escrow object (see
MultiReleaseEscrowWithBalance and the updateEscrow call) but casts it back to
MultiReleaseEscrow, hiding a real type mismatch; update the type definitions so
balance is formally optional (e.g., add balance?: number to MultiReleaseEscrow)
or create an ambient augmentation for the external `@trustless-work/escrow` type
so MultiReleaseEscrow includes balance?: number, then remove the unsafe cast in
FundEscrowButton (and the similar site in ProjectFundEscrow.tsx) so updateEscrow
receives a correctly typed object.
In `@components/wallet/WalletNotReadyModal.tsx`:
- Around line 42-47: The close icon inside WalletNotReadyModal.tsx is not
focusable—wrap the XIcon in a real focusable button element when using
DialogClose (keep DialogClose asChild) so keyboard users can tab to and activate
it; replace the direct XIcon child with a <button type="button"
aria-label="Close" className="..."> wrapper (include the existing
positioning/focus-visible styles) and render XIcon inside that button to
preserve visuals and accessibility.
In `@hooks/use-wallet-protection.ts`:
- Around line 46-48: The callback passed into the requireWallet flow is invoked
without awaiting, causing race conditions and unhandled errors; update the
requireWallet function in hooks/use-wallet-protection.ts to be async (if it
isn't already) and change the invocation to await callback(); ensure any
surrounding logic that depends on the callback completing awaits the returned
promise so errors propagate to callers (e.g., from
FundEscrowButton/Initialize.tsx) or rethrow after optional try/catch so
walletReady is only returned after the callback finishes.
---
Outside diff comments:
In `@components/modals/fund-project/index.tsx`:
- Around line 307-403: The async callback passed to requireWallet is not being
awaited so errors inside (e.g., in fundEscrow, signTransaction, sendTransaction)
can be swallowed and walletValid will be incorrect; fix by awaiting the callback
call here (change the call to await requireWallet(...)) and/or update the
requireWallet implementation in the useWalletProtection hook to await and
propagate the passed async callback (ensure it does await cb(...) and
returns/rethrows errors) so walletValid reflects failures.
---
Nitpick comments:
In `@components/escrow/FundEscrowButton.tsx`:
- Around line 77-81: The walletAddress guard in handleFundEscrow is redundant
because requireWallet already enforces and shows the wallet modal; remove the
initial if (!walletAddress) { toast.error... return; } block from
handleFundEscrow so only requireWallet is used, or alternatively call await
requireWallet() as the very first action in handleFundEscrow (before
setIsLoading or other state changes) to ensure early termination; update any
related comments and keep function behavior consistent with requireWallet's
modal handling.
- Around line 23-28: The type alias MultiReleaseEscrowWithBalance is declared
among import statements which breaks the import grouping convention; move the
MultiReleaseEscrowWithBalance definition so it appears after all import lines in
FundEscrowButton.tsx (i.e., keep imports like Loader2, CheckCircle2, DollarSign
together at the top and then declare the MultiReleaseEscrowWithBalance type),
ensuring no other imports or code are interleaved with the type.
In `@components/wallet/WalletNotReadyModal.tsx`:
- Around line 11-12: The import list in WalletNotReadyModal includes unused
symbols; remove CheckCircle2 and Image from the import statement so only used
icons (XIcon, AlertCircle, Wallet, ExternalLink) and any other used modules
remain; update the import line that currently references CheckCircle2 and Image
to avoid unused-import lint errors and unnecessary bundle size.
In `@hooks/use-wallet-protection.ts`:
- Line 1: Remove the unused import symbol useCallback from the import statement
at the top of the file (currently: import { useState, useCallback } from
'react';) so only used symbols remain (e.g., import { useState } from 'react';)
to eliminate the unused import warning.
In `@hooks/use-wallet-readiness.ts`:
- Around line 22-58: The checkReadiness function is recreated each render; wrap
the const checkReadiness declaration in useCallback to memoize it and stabilize
its reference (e.g., const checkReadiness = useCallback((requiredUsdc = 0) => {
... }, [walletAddress, balances])); ensure you include walletAddress and
balances in the dependency array so the logic updates when those change.
- Line 1: Remove the unused import by deleting the imported symbol useMemo from
the top of the file (the ES import statement that currently reads "import {
useMemo } from 'react';"); ensure only needed React imports remain so there are
no unused symbols in hooks/use-wallet-readiness.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b7cfcbc4-60f6-4662-96c0-710b3ad8c8fb
📒 Files selected for processing (6)
components/escrow/FundEscrowButton.tsxcomponents/modals/fund-project/index.tsxcomponents/wallet/WalletNotReadyModal.tsxfeatures/projects/components/Initialize.tsxhooks/use-wallet-protection.tshooks/use-wallet-readiness.ts
Comment on lines
+166
to
+174
| if (escrow) { | ||
| const escrowWithBalance = escrow as MultiReleaseEscrowWithBalance; | ||
| const currentBalance = escrowWithBalance.balance || 0; | ||
| const updatedEscrow: MultiReleaseEscrowWithBalance = { | ||
| ...escrow, | ||
| balance: currentBalance + totalAmount, | ||
| }; | ||
| updateEscrow(updatedEscrow as MultiReleaseEscrow); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how updateEscrow and escrow are used in the codebase
rg -n "updateEscrow|escrow\.balance" --type=tsx --type=ts -C 3 | head -80Repository: boundlessfi/boundless
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Search for updateEscrow usage and type definitions
echo "=== Searching for updateEscrow and related types ==="
rg "updateEscrow|MultiReleaseEscrow|EscrowContextValue" -g "*.ts" -g "*.tsx" -A 3 -B 1 | head -150Repository: boundlessfi/boundless
Length of output: 9428
🏁 Script executed:
#!/bin/bash
# Read the FundEscrowButton.tsx file to see the complete code and type definitions
cat -n components/escrow/FundEscrowButton.tsx | head -200Repository: boundlessfi/boundless
Length of output: 7598
🏁 Script executed:
#!/bin/bash
# Search for other usages of balance property with escrow and updateEscrow calls
echo "=== Other balance assignments with escrow ==="
rg "\.balance\s*=" --context=2 | grep -A 3 -B 3 "escrow"
echo "=== Other updateEscrow calls ==="
rg "updateEscrow\(" --context=3Repository: boundlessfi/boundless
Length of output: 3531
Type safety mismatch: balance property is used downstream but not formally typed in MultiReleaseEscrow.
The balance property being added and passed to updateEscrow (line 173) is actually checked in lib/utils/hackathon-winner-distribution.ts during escrow validation (escrow.balance || 0), but the MultiReleaseEscrow type doesn't formally include it. The current workaround using as MultiReleaseEscrow cast masks this type gap.
Fix by either:
- Extending
MultiReleaseEscrowto includebalance?: numberin the context type definition - Extending the imported
MultiReleaseEscrowtype from@trustless-work/escrowwith a proper type declaration file if it's an external library limitation
This pattern repeats in ProjectFundEscrow.tsx as well, suggesting a systemic type definition issue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/escrow/FundEscrowButton.tsx` around lines 166 - 174, The code adds
a balance property to an escrow object (see MultiReleaseEscrowWithBalance and
the updateEscrow call) but casts it back to MultiReleaseEscrow, hiding a real
type mismatch; update the type definitions so balance is formally optional
(e.g., add balance?: number to MultiReleaseEscrow) or create an ambient
augmentation for the external `@trustless-work/escrow` type so MultiReleaseEscrow
includes balance?: number, then remove the unsafe cast in FundEscrowButton (and
the similar site in ProjectFundEscrow.tsx) so updateEscrow receives a correctly
typed object.
Comment on lines
+42
to
+47
| <DialogClose | ||
| className='absolute top-2 right-2 rounded-full p-1' | ||
| asChild | ||
| > | ||
| <XIcon className='h-8 w-8 text-white' /> | ||
| </DialogClose> |
There was a problem hiding this comment.
Wrap close icon in a focusable button for accessibility.
The XIcon is rendered directly inside DialogClose without a button wrapper. This may cause keyboard accessibility issues since the icon itself is not a focusable element. Users relying on keyboard navigation may not be able to close the dialog.
♿ Proposed fix for accessibility
<DialogClose
- className='absolute top-2 right-2 rounded-full p-1'
+ className='absolute top-2 right-2 rounded-full p-1 hover:bg-white/10 focus:outline-none focus:ring-2 focus:ring-white/20'
asChild
>
- <XIcon className='h-8 w-8 text-white' />
+ <button type="button" aria-label="Close dialog">
+ <XIcon className='h-8 w-8 text-white' />
+ </button>
</DialogClose>📝 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
| <DialogClose | |
| className='absolute top-2 right-2 rounded-full p-1' | |
| asChild | |
| > | |
| <XIcon className='h-8 w-8 text-white' /> | |
| </DialogClose> | |
| <DialogClose | |
| className='absolute top-2 right-2 rounded-full p-1 hover:bg-white/10 focus:outline-none focus:ring-2 focus:ring-white/20' | |
| asChild | |
| > | |
| <button type="button" aria-label="Close dialog"> | |
| <XIcon className='h-8 w-8 text-white' /> | |
| </button> | |
| </DialogClose> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/wallet/WalletNotReadyModal.tsx` around lines 42 - 47, The close
icon inside WalletNotReadyModal.tsx is not focusable—wrap the XIcon in a real
focusable button element when using DialogClose (keep DialogClose asChild) so
keyboard users can tab to and activate it; replace the direct XIcon child with a
<button type="button" aria-label="Close" className="..."> wrapper (include the
existing positioning/focus-visible styles) and render XIcon inside that button
to preserve visuals and accessibility.
Contributor
Author
|
Hello @0xdevcollins |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
hooks/use-wallet-protection.ts (2)
27-30: Callback type should allow async functions.The
callbackparameter is typed as() => voidbut is awaited on line 57. This type mismatch may cause TypeScript to reject async callbacks or obscure the intent.♻️ Proposed fix for callback type
const requireWallet = async ( - callback?: () => void, + callback?: () => void | Promise<void>, requiredUsdcAmount: number = 0 ) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-protection.ts` around lines 27 - 30, The callback parameter of requireWallet is currently typed as () => void but is awaited inside the function (await callback()), so update the signature of requireWallet to accept asynchronous callbacks (e.g., callback?: () => void | Promise<void> or callback?: () => Promise<void>) to match usage; locate the requireWallet function and change the callback type accordingly so TypeScript recognizes awaiting the callback is valid.
68-74: Consider memoizing close handlers withuseCallback.Both
closeWalletModalandcloseNotReadyModalare returned from the hook and may cause unnecessary re-renders in consumers if their references change on each render.♻️ Proposed memoization
- const closeWalletModal = () => { - setShowWalletModal(false); - }; + const closeWalletModal = useCallback(() => { + setShowWalletModal(false); + }, []); - const closeNotReadyModal = () => { - setShowNotReadyModal(false); - }; + const closeNotReadyModal = useCallback(() => { + setShowNotReadyModal(false); + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-wallet-protection.ts` around lines 68 - 74, The close handlers closeWalletModal and closeNotReadyModal are recreated every render which can force re-renders in consumers; wrap them with React's useCallback and depend only on their setters (setShowWalletModal and setShowNotReadyModal) so their references stay stable when returned from the hook (i.e., replace the plain functions with const closeWalletModal = useCallback(() => setShowWalletModal(false), [setShowWalletModal]) and likewise for closeNotReadyModal).features/projects/components/Initialize.tsx (1)
61-61: Local drawer state creates separate instance from centralized wallet state.The component uses local
isWalletDrawerOpenstate (line 61) for theWalletSheet, whileLandingWalletWrapperuses centralizedisWalletOpenfrom context. This means the project initialization flow has its own drawer instance that won't sync with the global wallet button.If this is intentional (e.g., to allow independent drawer control per flow), consider documenting this design decision. Otherwise, use the centralized state for consistency.
♻️ Alternative: Use centralized wallet state
+import { useWalletContext } from '@/components/providers/wallet-provider'; const Initialize: React.FC<InitializeProps> = ({ onSuccess }) => { + const { isWalletOpen, onOpenWallet, onCloseWallet } = useWalletContext(); // ... - const [isWalletDrawerOpen, setIsWalletDrawerOpen] = useState(false); // In render: <WalletNotReadyModal // ... - onOpenWallet={() => setIsWalletDrawerOpen(true)} + onOpenWallet={onOpenWallet} /> <WalletSheet - open={isWalletDrawerOpen} - onOpenChange={setIsWalletDrawerOpen} + open={isWalletOpen} + onOpenChange={onCloseWallet} />Also applies to: 214-227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@features/projects/components/Initialize.tsx` at line 61, The component currently uses a local state isWalletDrawerOpen via useState (const [isWalletDrawerOpen, setIsWalletDrawerOpen]) which creates an independent drawer instance that won't sync with the global wallet state in LandingWalletWrapper (isWalletOpen); replace the local state with the centralized context/state used by LandingWalletWrapper (consume and use isWalletOpen and its setter from the wallet context or prop) for the WalletSheet and related handlers so the drawer opens/closes in sync, and remove the duplicate local state usage (also update the other occurrences referenced around the 214-227 block to use the same centralized state).components/landing-page/navbar.tsx (1)
357-363:WalletNotReadyModalinUnauthenticatedActionsis redundant.For unauthenticated users,
showNotReadyModalwill never betruebecause the protection flow showsWalletRequiredModalfirst when no wallet is connected. The readiness check only runs after confirming a wallet exists.This doesn't cause bugs but adds unnecessary JSX. Consider removing it for clarity.
🧹 Remove redundant modal
<WalletRequiredModal open={showWalletModal} onOpenChange={closeWalletModal} actionName={ACTIONS.CREATE_PROJECT} onWalletConnected={handleWalletConnected} /> - <WalletNotReadyModal - open={showNotReadyModal} - onOpenChange={closeNotReadyModal} - reasons={notReadyReasons} - actionName={ACTIONS.CREATE_PROJECT} - onOpenWallet={onOpenWallet} - />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/landing-page/navbar.tsx` around lines 357 - 363, In UnauthenticatedActions remove the redundant WalletNotReadyModal JSX: the WalletNotReadyModal component (props: open={showNotReadyModal}, onOpenChange={closeNotReadyModal}, reasons={notReadyReasons}, actionName={ACTIONS.CREATE_PROJECT}, onOpenWallet={onOpenWallet}) is never shown for unauthenticated users because WalletRequiredModal handles the no-wallet case first and the readiness check runs only after a wallet exists; delete this WalletNotReadyModal node from UnauthenticatedActions to eliminate dead JSX while leaving WalletRequiredModal and any WalletNotReadyModal usage in authenticated paths intact.components/escrow/FundEscrowButton.tsx (2)
23-28: Move import statement to the top of the file.The
lucide-reactimport on line 28 is placed after the type alias definition, breaking the convention of grouping all imports at the top. As per coding guidelines, always include all necessary imports at the top of the file.🔧 Suggested fix
Move the import to the imports section (after line 21):
import WalletNotReadyModal from '@/components/wallet/WalletNotReadyModal'; import { WalletSheet } from '@/components/wallet/WalletSheet'; +import { Loader2, CheckCircle2, DollarSign } from 'lucide-react'; // Extended type to include balance property that may exist at runtime // Using intersection type to avoid type conflicts with required balance property type MultiReleaseEscrowWithBalance = MultiReleaseEscrow & { balance?: number; }; -import { Loader2, CheckCircle2, DollarSign } from 'lucide-react';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/escrow/FundEscrowButton.tsx` around lines 23 - 28, Move the lucide-react import to the top of the file so all imports are grouped together; specifically relocate "import { Loader2, CheckCircle2, DollarSign } from 'lucide-react';" above the type alias declaration (so it sits with the other imports) to keep the file-level imports organized and ensure MultiReleaseEscrowWithBalance and its type alias remain below the import block.
192-195: RedundantsetIsLoading(false)call.The
setIsLoading(false)on line 193 is redundant since thefinallyblock (line 205) will always execute, even when returning early. This can be simplified.🔧 Suggested fix
if (!walletReady) { - setIsLoading(false); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/escrow/FundEscrowButton.tsx` around lines 192 - 195, In FundEscrowButton's wallet readiness check (inside the handleFundEscrow handler), remove the redundant setIsLoading(false) right before the early return and rely on the existing try/finally cleanup to clear loading; simply return early when !walletReady and let the finally block in handleFundEscrow handle setIsLoading(false).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/escrow/FundEscrowButton.tsx`:
- Around line 230-232: In FundEscrowButton replace the failure-state spinner:
import XCircle (or AlertCircle) from 'lucide-react' and swap the Loader2 usage
in the error branch to the chosen XCircle/AlertCircle component (preserving the
same className 'h-5 w-5 text-red-600') so the UI uses a semantic error icon
instead of a loading spinner; update the import statement that currently brings
in Loader2 accordingly and remove Loader2 if unused elsewhere.
In `@components/wallet/LandingWalletWrapper.tsx`:
- Around line 26-30: The onCloseWallet handler signature conflicts with
FamilyWalletDrawer.onOpenChange because onOpenChange passes a boolean (isOpen);
update the onCloseWallet function in wallet-provider.tsx to accept a boolean
parameter (e.g., onCloseWallet(isOpen: boolean)) and update the
WalletContextType to reflect the new signature; adjust the implementation to
ignore the arg if unnecessary (call existing close logic) and update any other
callers to accept/forward the boolean so the prop passed to FamilyWalletDrawer
matches onOpenChange.
In `@components/wallet/WalletTrigger.tsx`:
- Around line 24-26: The component currently destructures onOpenWallet from
useWalletContext but uses a local open/setOpen state, causing inconsistent
behavior with LandingWalletWrapper and WalletNotReadyModal; fix by removing the
local useState and switching the drawer control to the centralized context: read
isWalletOpen (instead of open) and replace any setOpen(true)/setOpen(false)
calls with onOpenWallet()/onCloseWallet(), ensuring WalletTrigger uses
isWalletOpen, onOpenWallet, and onCloseWallet from useWalletContext so the modal
and drawer remain in sync.
---
Nitpick comments:
In `@components/escrow/FundEscrowButton.tsx`:
- Around line 23-28: Move the lucide-react import to the top of the file so all
imports are grouped together; specifically relocate "import { Loader2,
CheckCircle2, DollarSign } from 'lucide-react';" above the type alias
declaration (so it sits with the other imports) to keep the file-level imports
organized and ensure MultiReleaseEscrowWithBalance and its type alias remain
below the import block.
- Around line 192-195: In FundEscrowButton's wallet readiness check (inside the
handleFundEscrow handler), remove the redundant setIsLoading(false) right before
the early return and rely on the existing try/finally cleanup to clear loading;
simply return early when !walletReady and let the finally block in
handleFundEscrow handle setIsLoading(false).
In `@components/landing-page/navbar.tsx`:
- Around line 357-363: In UnauthenticatedActions remove the redundant
WalletNotReadyModal JSX: the WalletNotReadyModal component (props:
open={showNotReadyModal}, onOpenChange={closeNotReadyModal},
reasons={notReadyReasons}, actionName={ACTIONS.CREATE_PROJECT},
onOpenWallet={onOpenWallet}) is never shown for unauthenticated users because
WalletRequiredModal handles the no-wallet case first and the readiness check
runs only after a wallet exists; delete this WalletNotReadyModal node from
UnauthenticatedActions to eliminate dead JSX while leaving WalletRequiredModal
and any WalletNotReadyModal usage in authenticated paths intact.
In `@features/projects/components/Initialize.tsx`:
- Line 61: The component currently uses a local state isWalletDrawerOpen via
useState (const [isWalletDrawerOpen, setIsWalletDrawerOpen]) which creates an
independent drawer instance that won't sync with the global wallet state in
LandingWalletWrapper (isWalletOpen); replace the local state with the
centralized context/state used by LandingWalletWrapper (consume and use
isWalletOpen and its setter from the wallet context or prop) for the WalletSheet
and related handlers so the drawer opens/closes in sync, and remove the
duplicate local state usage (also update the other occurrences referenced around
the 214-227 block to use the same centralized state).
In `@hooks/use-wallet-protection.ts`:
- Around line 27-30: The callback parameter of requireWallet is currently typed
as () => void but is awaited inside the function (await callback()), so update
the signature of requireWallet to accept asynchronous callbacks (e.g.,
callback?: () => void | Promise<void> or callback?: () => Promise<void>) to
match usage; locate the requireWallet function and change the callback type
accordingly so TypeScript recognizes awaiting the callback is valid.
- Around line 68-74: The close handlers closeWalletModal and closeNotReadyModal
are recreated every render which can force re-renders in consumers; wrap them
with React's useCallback and depend only on their setters (setShowWalletModal
and setShowNotReadyModal) so their references stay stable when returned from the
hook (i.e., replace the plain functions with const closeWalletModal =
useCallback(() => setShowWalletModal(false), [setShowWalletModal]) and likewise
for closeNotReadyModal).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e8717dd4-e1ce-4290-99d3-cf21abfe0f58
📒 Files selected for processing (10)
components/escrow/FundEscrowButton.tsxcomponents/landing-page/navbar.tsxcomponents/providers/wallet-provider.tsxcomponents/wallet/LandingWalletWrapper.tsxcomponents/wallet/WalletNotReadyModal.tsxcomponents/wallet/WalletTrigger.tsxfeatures/projects/components/Initialize.tsxhooks/use-protected-action.tshooks/use-wallet-protection.tshooks/use-wallet-readiness.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/use-wallet-readiness.ts
Comment on lines
+230
to
+232
| ) : ( | ||
| <Loader2 className='h-5 w-5 text-red-600' /> | ||
| )} |
There was a problem hiding this comment.
Incorrect icon for failure state.
Loader2 is a spinner/loading indicator and doesn't convey failure semantically. Consider using XCircle or AlertCircle from lucide-react to properly indicate an error state.
🔧 Suggested fix
Update the import:
-import { Loader2, CheckCircle2, DollarSign } from 'lucide-react';
+import { Loader2, CheckCircle2, XCircle, DollarSign } from 'lucide-react';Update the icon usage:
) : (
- <Loader2 className='h-5 w-5 text-red-600' />
+ <XCircle className='h-5 w-5 text-red-600' />
)}📝 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
| ) : ( | |
| <Loader2 className='h-5 w-5 text-red-600' /> | |
| )} | |
| import { Loader2, CheckCircle2, XCircle, DollarSign } from 'lucide-react'; | |
| // ... other code ... | |
| ) : ( | |
| <XCircle className='h-5 w-5 text-red-600' /> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/escrow/FundEscrowButton.tsx` around lines 230 - 232, In
FundEscrowButton replace the failure-state spinner: import XCircle (or
AlertCircle) from 'lucide-react' and swap the Loader2 usage in the error branch
to the chosen XCircle/AlertCircle component (preserving the same className 'h-5
w-5 text-red-600') so the UI uses a semantic error icon instead of a loading
spinner; update the import statement that currently brings in Loader2
accordingly and remove Loader2 if unused elsewhere.
Comment on lines
26
to
30
| <FamilyWalletDrawer | ||
| open={open} | ||
| open={isWalletOpen} | ||
| initialView={drawerView} | ||
| onOpenChange={setOpen} | ||
| onOpenChange={onCloseWallet} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the FamilyWalletDrawer onOpenChange prop type
ast-grep --pattern $'interface FamilyWalletDrawerProps {
$$$
onOpenChange: $_
$$$
}'Repository: boundlessfi/boundless
Length of output: 408
🏁 Script executed:
#!/bin/bash
# Find and examine the wallet-provider.tsx file to check onCloseWallet definition
fd -t f "wallet-provider" --type fRepository: boundlessfi/boundless
Length of output: 105
🏁 Script executed:
#!/bin/bash
# Search for onCloseWallet definition in context
rg "onCloseWallet" -A 3 -B 3 --type ts --type tsxRepository: boundlessfi/boundless
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Read the wallet-provider.tsx file
wc -l components/providers/wallet-provider.tsxRepository: boundlessfi/boundless
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Search for onCloseWallet with correct syntax
rg "onCloseWallet" -A 3 -B 3 components/providers/wallet-provider.tsxRepository: boundlessfi/boundless
Length of output: 531
🏁 Script executed:
#!/bin/bash
# Examine FamilyWalletDrawer implementation to see how onOpenChange is used
cat -n components/wallet/FamilyWalletDrawer.tsx | head -100Repository: boundlessfi/boundless
Length of output: 3810
🏁 Script executed:
#!/bin/bash
# Search for calls to onOpenChange in FamilyWalletDrawer
rg "onOpenChange" components/wallet/FamilyWalletDrawer.tsx -A 2 -B 2Repository: boundlessfi/boundless
Length of output: 687
Type mismatch: onCloseWallet signature doesn't match FamilyWalletDrawer.onOpenChange.
The FamilyWalletDrawer component calls onOpenChange(isOpen) with a boolean argument, but onCloseWallet from the wallet context is typed as () => void and doesn't accept parameters. This causes a TypeScript type error when passing onCloseWallet to the onOpenChange prop.
Update onCloseWallet in wallet-provider.tsx to accept the boolean parameter:
- const onCloseWallet = useCallback(() => {
- setIsWalletOpen(false);
+ const onCloseWallet = useCallback((open?: boolean) => {
+ setIsWalletOpen(open ?? false);
}, []);And update the type in WalletContextType:
- onCloseWallet: () => void;
+ onCloseWallet: (open?: boolean) => void;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/wallet/LandingWalletWrapper.tsx` around lines 26 - 30, The
onCloseWallet handler signature conflicts with FamilyWalletDrawer.onOpenChange
because onOpenChange passes a boolean (isOpen); update the onCloseWallet
function in wallet-provider.tsx to accept a boolean parameter (e.g.,
onCloseWallet(isOpen: boolean)) and update the WalletContextType to reflect the
new signature; adjust the implementation to ignore the arg if unnecessary (call
existing close logic) and update any other callers to accept/forward the boolean
so the prop passed to FamilyWalletDrawer matches onOpenChange.
Comment on lines
+24
to
26
| const { walletAddress, hasWalletFromSession, isLoading, onOpenWallet } = | ||
| useWalletContext(); | ||
| const [open, setOpen] = useState(false); |
There was a problem hiding this comment.
onOpenWallet is destructured but unused — wallet readiness flow may not work correctly.
The onOpenWallet callback is extracted from context but never used. The component continues to use local open state (line 26) for drawer visibility, while LandingWalletWrapper and WalletNotReadyModal use the centralized isWalletOpen from context.
This creates a state inconsistency: when WalletNotReadyModal calls onOpenWallet(), it sets the context's isWalletOpen to true, but WalletTrigger's drawer won't respond because it's controlled by local state.
🐛 Proposed fix: Use centralized state or remove unused import
Option 1: Use centralized state (recommended for consistency):
const { walletAddress, hasWalletFromSession, isLoading, onOpenWallet } =
useWalletContext();
- const [open, setOpen] = useState(false);
+ const { isWalletOpen, onOpenWallet, onCloseWallet } = useWalletContext();Then replace open with isWalletOpen, setOpen(true) with onOpenWallet(), and setOpen with onCloseWallet throughout.
Option 2: If local state is intentional, remove unused destructuring:
- const { walletAddress, hasWalletFromSession, isLoading, onOpenWallet } =
+ const { walletAddress, hasWalletFromSession, isLoading } =
useWalletContext();📝 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
| const { walletAddress, hasWalletFromSession, isLoading, onOpenWallet } = | |
| useWalletContext(); | |
| const [open, setOpen] = useState(false); | |
| const { walletAddress, hasWalletFromSession, isLoading } = | |
| useWalletContext(); | |
| const [open, setOpen] = useState(false); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/wallet/WalletTrigger.tsx` around lines 24 - 26, The component
currently destructures onOpenWallet from useWalletContext but uses a local
open/setOpen state, causing inconsistent behavior with LandingWalletWrapper and
WalletNotReadyModal; fix by removing the local useState and switching the drawer
control to the centralized context: read isWalletOpen (instead of open) and
replace any setOpen(true)/setOpen(false) calls with
onOpenWallet()/onCloseWallet(), ensuring WalletTrigger uses isWalletOpen,
onOpenWallet, and onCloseWallet from useWalletContext so the modal and drawer
remain in sync.
Benjtalkshow
approved these changes
Mar 9, 2026
Collaborator
Benjtalkshow
left a comment
Benjtalkshow
left a comment
There was a problem hiding this comment.
LGTM! Though made some fixes on your behalf
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
2 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 PR implements a "Wallet Not Ready" modal and a corresponding readiness hook to improve the user experience when a Stellar wallet is not fully prepared for an action.
closes #449
Key Changes:
New Hook: hooks/use-wallet-readiness.ts - Centralizes logic for checking Stellar account activation, USDC trustlines, and sufficient balances.
New Component: components/wallet/WalletNotReadyModal.tsx - A premium modal that explains exactly why a wallet is not ready and provides steps to fix it.
Enhanced Protection: Updated hooks/use-wallet-protection.ts to automatically trigger the readiness check and display the modal when necessary.
Integrated Flows:
Campaign Creation: Added readiness guard to the project initialization flow.
Escrow Funding: Integrated into the FundEscrowButton for hackathon and milestone funding.
Project Funding: Added to the project funding modal to ensure backers have activated wallets and trustlines.
This implementation reduces user friction by replacing technical errors with guided, actionable steps.
Testing: Verified by attempting to initialize or fund with an unactivated wallet, which now correctly triggers the guided modal.
demo.test.mp4
Summary by CodeRabbit
New Features
Bug Fixes