feat: Implement order validation and payment confirmation modal - #607
feat: Implement order validation and payment confirmation modal#607sundayonah wants to merge 4 commits into
Conversation
- Added `validateOrder` function to confirm user receipt of funds for stuck orders, integrating with the aggregator's validate API. - Introduced `PaymentConfirmationModal` component to prompt users for payment confirmation after a delay if their transaction is stuck. - Updated `TransactionStatus` to manage payment confirmation state and handle user interactions for confirming payments. - Enhanced configuration to include `aggregatorSenderApiKeyId` for secure API interactions.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds authenticated order validation through a server proxy and introduces persistent delayed payment confirmation for stuck transactions. Confirmed payments are validated and settled; declined payments are reindexed and tracked again. ChangesPayment confirmation flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TransactionStatus
participant PaymentConfirmationModal
participant validateOrder
participant POSTValidateRoute
participant SupabaseTransactions
participant SenderValidationService
TransactionStatus->>PaymentConfirmationModal: Open after 120 seconds
PaymentConfirmationModal->>TransactionStatus: Confirm payment
TransactionStatus->>validateOrder: Validate order
validateOrder->>POSTValidateRoute: POST authenticated request
POSTValidateRoute->>SupabaseTransactions: Check wallet ownership
POSTValidateRoute->>SenderValidationService: Validate order
SenderValidationService-->>POSTValidateRoute: Return validation result
POSTValidateRoute-->>validateOrder: Return validation result
validateOrder-->>TransactionStatus: Settle validated transaction
TransactionStatus-->>PaymentConfirmationModal: Close modal
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/pages/TransactionStatus.tsx (1)
1725-1744: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
recipientAddressshows a bank/mobile-money identifier as if it were a destination wallet for off-ramp stuck orders.
recipientWalletAddressis sourced fromFormData.walletAddress, documented elsewhere as onramp-only ("For onramp: stablecoin wallet address"). For off-ramp swaps — the case wherefulfilling/fulfilledmost commonly stalls (PSP delay paying out fiat) —recipientWalletAddressis empty, soString(accountIdentifier || "")(a bank account/mobile-money number) is passed intorecipientAddress, whichPaymentConfirmationModaldocuments and renders as "Destination wallet where funds are going" viatruncateAddress's hex-style formatting. This misrepresents where the funds actually went during the exact moment the user is deciding whether to confirm receipt.Pass a swap-direction-aware label (or omit
recipientAddressfor off-ramp and rely on the token/amount + explorer link) instead of reusing the wallet-address prop for a non-wallet identifier.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/pages/TransactionStatus.tsx` around lines 1725 - 1744, Update the PaymentConfirmationModal invocation in TransactionStatus so off-ramp swaps do not pass accountIdentifier as recipientAddress; omit the prop or provide a direction-appropriate label. Preserve recipientWalletAddress for onramp flows, ensuring the modal’s destination-wallet display never formats bank or mobile-money identifiers as wallet addresses.
🧹 Nitpick comments (3)
app/pages/TransactionStatus.tsx (1)
891-923: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEffect re-runs on every poll tick due to full
orderDetailsobject in deps.
orderDetailsgets a new reference every ~5s fromsetOrderDetails(responseData)in the polling effect, and it's in this effect's dependency array, sowriteStuckPaymentSessionre-serializes to localStorage and the 120s timer is torn down/rebuilt every poll cycle while a transaction is stuck (potentially for a long time). Functionally harmless (the elapsed-time math is ref-based and stays correct), but it's needless localStorage churn and timer thrash.Consider depending only on the specific fields you actually use (
orderDetails?.network,orderDetails?.txHash) instead of the whole object.Also applies to: 954-971
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/pages/TransactionStatus.tsx` around lines 891 - 923, Update the effect containing writeStuckPaymentSession and its related 120-second timer to remove the full orderDetails object from the dependency array. Depend only on the specific values read by the effect, orderDetails?.network and orderDetails?.txHash, while preserving the existing session persistence behavior.app/lib/stuckPaymentSession.ts (2)
3-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSession key and per-order timer prefix are hardcoded independently in three files instead of being centralized.
SESSION_KEY("noblocks_stuck_payment_session") and thestuck_fulfilling_since_prefix live only as private constants instuckPaymentSession.ts, so every other consumer re-types the literal strings (or, in one case, re-implements logic the module already exports) — any future rename silently breaks the others.
app/lib/stuckPaymentSession.ts#L3-L102: exportSESSION_KEYand thestuck_fulfilling_since_prefix (or agetStuckFulfillingSinceKey(orderId)helper) so other modules can import them instead of duplicating the literals.app/lib/session-cleanup.ts#L5-L49: replace the hardcoded"noblocks_stuck_payment_session"(line 11) with a call toclearStuckPaymentSession(), and replace the hardcoded"stuck_fulfilling_since_"prefix scan (34-49) with the exported prefix constant.app/pages/TransactionStatus.tsx#L858-L949: drop the locally hardcodedSTUCK_STORAGE_KEY_PREFIXand call the already-importedclearStuckFulfillingSince(orderId)(871-878) andresetStuckFulfillingSince(orderId)(940-947) instead of re-implementing the samelocalStorageread/write logic inline.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/stuckPaymentSession.ts` around lines 3 - 102, Centralize the stuck-session storage identifiers and timer operations: in app/lib/stuckPaymentSession.ts lines 3-102, export SESSION_KEY and the stuck_fulfilling_since_ prefix (or a key-building helper); in app/lib/session-cleanup.ts lines 5-49, use clearStuckPaymentSession() and the exported timer prefix instead of hardcoded literals; in app/pages/TransactionStatus.tsx lines 858-949, remove STUCK_STORAGE_KEY_PREFIX and replace inline localStorage timer access with clearStuckFulfillingSince(orderId) and resetStuckFulfillingSince(orderId).
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
SESSION_KEYand thestuck_fulfilling_since_prefix are duplicated in other files.Both this file's
SESSION_KEYstring and itsstuck_fulfilling_since_prefix are hard-coded again inapp/lib/session-cleanup.tsandapp/pages/TransactionStatus.tsxinstead of being imported/reused. See the consolidated comment below.Also applies to: 86-102
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/stuckPaymentSession.ts` at line 3, Centralize the shared SESSION_KEY and stuck_fulfilling_since_ prefix in stuckPaymentSession.ts, exporting them for reuse. Update session-cleanup.ts and TransactionStatus.tsx to import these constants instead of duplicating the hard-coded strings, preserving existing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/stuckPaymentSession.ts`:
- Around line 40-58: Update readStuckPaymentSession to reject otherwise-valid
sessions whose savedAt or createdAt timestamp exceeds a reasonable short TTL of
a few hours, using the available timestamp consistently and preserving null
returns for invalid or expired data. Ensure stale sessions are not restored by
the existing navigation flow.
In `@app/pages/TransactionStatus.tsx`:
- Around line 1038-1077: Update handlePaymentNotReceived so
setShowPaymentConfirmation(false) and the successful reindex timer-reset actions
run only after reindexSingleTransaction succeeds; preserve the modal’s open
state when validation or reindexing fails, while retaining the existing success
behavior and retry-related state updates.
---
Outside diff comments:
In `@app/pages/TransactionStatus.tsx`:
- Around line 1725-1744: Update the PaymentConfirmationModal invocation in
TransactionStatus so off-ramp swaps do not pass accountIdentifier as
recipientAddress; omit the prop or provide a direction-appropriate label.
Preserve recipientWalletAddress for onramp flows, ensuring the modal’s
destination-wallet display never formats bank or mobile-money identifiers as
wallet addresses.
---
Nitpick comments:
In `@app/lib/stuckPaymentSession.ts`:
- Around line 3-102: Centralize the stuck-session storage identifiers and timer
operations: in app/lib/stuckPaymentSession.ts lines 3-102, export SESSION_KEY
and the stuck_fulfilling_since_ prefix (or a key-building helper); in
app/lib/session-cleanup.ts lines 5-49, use clearStuckPaymentSession() and the
exported timer prefix instead of hardcoded literals; in
app/pages/TransactionStatus.tsx lines 858-949, remove STUCK_STORAGE_KEY_PREFIX
and replace inline localStorage timer access with
clearStuckFulfillingSince(orderId) and resetStuckFulfillingSince(orderId).
- Line 3: Centralize the shared SESSION_KEY and stuck_fulfilling_since_ prefix
in stuckPaymentSession.ts, exporting them for reuse. Update session-cleanup.ts
and TransactionStatus.tsx to import these constants instead of duplicating the
hard-coded strings, preserving existing behavior.
In `@app/pages/TransactionStatus.tsx`:
- Around line 891-923: Update the effect containing writeStuckPaymentSession and
its related 120-second timer to remove the full orderDetails object from the
dependency array. Depend only on the specific values read by the effect,
orderDetails?.network and orderDetails?.txHash, while preserving the existing
session persistence behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c82dcbfc-286f-4bea-9104-c551eff582cf
📒 Files selected for processing (5)
app/components/MainPageContent.tsxapp/components/PaymentConfirmationModal.tsxapp/lib/session-cleanup.tsapp/lib/stuckPaymentSession.tsapp/pages/TransactionStatus.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/components/PaymentConfirmationModal.tsx (1)
135-163: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winModal's recipient display assumes a wallet address; off-ramp passes a bank/mobile-money identifier instead.
truncateAddress+ monospace formatting is designed for on-chain hashes, but off-ramp orders (the primary target of this feature, per the off-ramp-only auto-reindex logic) feed inaccountIdentifier, producing a truncated bank account number styled as a crypto address.
app/components/PaymentConfirmationModal.tsx#L135-L163: branch the recipient row's formatting on whether the value is a wallet address vs. an account identifier (e.g., skiptruncateAddress/font-monofor non-address values).app/pages/TransactionStatus.tsx#L1732-L1750: pass a type hint (or the institution name) alongsiderecipientAddressso the modal can render bank recipients appropriately instead of as a truncated hash.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/PaymentConfirmationModal.tsx` around lines 135 - 163, Update PaymentConfirmationModal’s recipient row to distinguish wallet addresses from off-ramp account identifiers, applying truncateAddress and font-mono only to wallet values while displaying bank or mobile-money identifiers in full with normal styling. Update the TransactionStatus payment confirmation call site to pass the required recipient type hint or institution name so the modal can select the appropriate rendering; apply these changes in app/components/PaymentConfirmationModal.tsx lines 135-163 and app/pages/TransactionStatus.tsx lines 1732-1750.app/pages/TransactionStatus.tsx (1)
916-945: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winStuck-payment session should not persist recipient PII in
localStorage.
writeStuckPaymentSessionstoresrecipientNameandaccountIdentifierin a script-readable, persistent store. Keep only the fields needed to resume the flow, or move this data to shorter-lived storage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/pages/TransactionStatus.tsx` around lines 916 - 945, Update the writeStuckPaymentSession payload in TransactionStatus so it no longer persists recipientName or accountIdentifier in localStorage-backed session data. Remove these PII fields while preserving the remaining resume-flow fields and existing writeStuckPaymentSession behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/pages/TransactionStatus.tsx`:
- Around line 1068-1071: Add a user-visible toast.error call in
handlePaymentConfirmed’s catch block before rethrowing the payment confirmation
error, while preserving the existing logging and propagation. In
app/pages/TransactionStatus.tsx lines 1068-1071, include clear failure feedback;
in app/components/PaymentConfirmationModal.tsx lines 48-58, no functional change
is required, but update the stale comment to reflect that the parent surfaces
errors via a toast.
---
Outside diff comments:
In `@app/components/PaymentConfirmationModal.tsx`:
- Around line 135-163: Update PaymentConfirmationModal’s recipient row to
distinguish wallet addresses from off-ramp account identifiers, applying
truncateAddress and font-mono only to wallet values while displaying bank or
mobile-money identifiers in full with normal styling. Update the
TransactionStatus payment confirmation call site to pass the required recipient
type hint or institution name so the modal can select the appropriate rendering;
apply these changes in app/components/PaymentConfirmationModal.tsx lines 135-163
and app/pages/TransactionStatus.tsx lines 1732-1750.
In `@app/pages/TransactionStatus.tsx`:
- Around line 916-945: Update the writeStuckPaymentSession payload in
TransactionStatus so it no longer persists recipientName or accountIdentifier in
localStorage-backed session data. Remove these PII fields while preserving the
remaining resume-flow fields and existing writeStuckPaymentSession behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e4c64c7-b86b-431c-a81b-48369162fddf
📒 Files selected for processing (3)
app/components/PaymentConfirmationModal.tsxapp/lib/reindex.tsapp/pages/TransactionStatus.tsx
…nStatus for improved user feedback
2 participants
Description
This PR adds a stuck-transaction prompt so users can manually confirm receipt when a swap stays in Fulfilling or Fulfilled for more than 2 minutes. It reduces support load and lets users settle the transaction without waiting for the provider to update.
Background: Swaps can remain in "fulfilling" or "fulfilled" for longer than expected. The user may have already received funds. This flow asks "Have you received this payment?" and, on confirmation, validates the order on the aggregator and marks the Noblocks transaction as settled.
Changes:
localStoragekeyed byorderId, so the 120s count survives refresh and navigating away. On open/refresh, if already stuck ≥120s, the prompt can show immediately.POST /api/v1/orders/:id/validate(proxy to aggregator sender API) → aggregator validates order → Noblocks updates transaction to settled/completed and dismisses the prompt. Stuck timestamp is cleared fromlocalStorage. No, I haven't dismisses without validating.PaymentConfirmationModalto match Figma (Pending badge, amount + destination address + View, Yes/No actions).config.aggregatorUrlandconfig.aggregatorSenderApiKeyIdfrom@/app/lib/configfor the validate route (falls back toNEXT_PUBLIC_AGGREGATOR_SENDER_API_KEY_IDwhen the server-only key is unset).New/updated surface:
POST /api/v1/orders/[id]/validate(rate-limited, requiresx-wallet-address),validateOrder()inaggregator.ts,TransactionStatus120s + localStorage logic andhandlePaymentConfirmed, redesignedPaymentConfirmationModal.aggregatorSenderApiKeyIdadded toConfigandconfig.ts; envAGGREGATOR_SENDER_API_KEY_ID(UUID) preferred for validate.References
POST /v1/sender/orders/:id/validate(sender API),ValidateOrderinaggregator/controllers/sender/sender.goChecklist
mainBy submitting a PR, I agree to Paycrest's Contributor Code of Conduct and Contribution Guide.
Summary by CodeRabbit
New Features
Improvements
Chores