fix: replace invalid Lightning Network routes. (OK-35333) #6587
Conversation
huhuanming
requested review from
ezailWang,
loatheb,
sidmorizon and
originalix
as code owners
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
WalkthroughThe pull request introduces a significant refactoring of Lightning Network (LNURL) related modals and routing. The changes involve transitioning from send-modal routes to signature-confirmation routes across multiple components. This modification affects how LNURL authentication, payment requests, and withdrawal requests are handled, shifting the routing context from sending transactions to signature confirmation. Changes
Sequence DiagramsequenceDiagram
participant User
participant LNURLModal
participant SignatureConfirmModal
participant Backend
User->>LNURLModal: Initiate LNURL Action
LNURLModal->>SignatureConfirmModal: Request Signature Confirmation
SignatureConfirmModal-->>User: Prompt for Confirmation
User->>SignatureConfirmModal: Approve/Reject
SignatureConfirmModal->>Backend: Process Confirmed Action
Backend-->>User: Return Result
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
huhuanming
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (4)
packages/kit/src/views/LightningNetwork/pages/Send/LnurlWithdrawModal.tsx (1)
Line range hint
171-174: Standardize error handling.Use
messageinstead ofinfofor consistency with other error handling in the file.- info: message, + message, autoToast: true,packages/kit/src/views/LightningNetwork/pages/Send/LnurlAuthModal.tsx (1)
Line range hint
75-75: Fix typo in action check.'reigster' should be 'register'.
- if (lnurlDetails.action === 'reigster') { + if (lnurlDetails.action === 'register') {packages/kit/src/views/LightningNetwork/pages/Send/LnurlPayRequestModal.tsx (2)
Line range hint
13-14: Move lightning utils to shared module.The TODO comment indicates a code organization issue. Let's track this task.
Would you like me to create an issue to track moving the lightning utils to the shared module?
Line range hint
122-122: Remove console.log statements.Replace console.log statements with proper error logging.
- console.log('fetchLnurlPayRequestResult error: ', e); + // TODO: Add proper error logging- console.log('lnurl withdraw error: ', e); + // TODO: Add proper error loggingAlso applies to: 177-177
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
packages/kit-bg/src/providers/ProviderApiWebln.ts(3 hunks)packages/kit/src/views/LightningNetwork/pages/Send/LnurlAuthModal.tsx(2 hunks)packages/kit/src/views/LightningNetwork/pages/Send/LnurlPayRequestModal.tsx(2 hunks)packages/kit/src/views/LightningNetwork/pages/Send/LnurlWithdrawModal.tsx(2 hunks)packages/kit/src/views/Send/router/index.ts(1 hunks)packages/shared/src/routes/send.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/shared/src/routes/send.ts
🔇 Additional comments (3)
packages/kit/src/views/Send/router/index.ts (1)
Line range hint
69-77: Verify unused components before removal.Let's verify if these components are truly unused.
packages/kit-bg/src/providers/ProviderApiWebln.ts (2)
274-277: Good move switching to SignatureConfirmModal for auth!This change better reflects the actual operation - authenticating via signature rather than sending funds.
288-291: Verify payment flow remains intact after route changeWhile moving to SignatureConfirmModal makes sense for consistency, let's ensure the payment flow works correctly.
Run this script to check for any payment-related regressions:
✅ Verification successful
Payment flow verified and intact ✓
The route change maintains all required parameters and properly handles both direct sends and dApp requests.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for payment flow dependencies rg -A 5 "EModalSignatureConfirmRoutes.LnurlPayRequest"Length of output: 3323
Summary by CodeRabbit
Routing Changes
Modal Updates
Type Modifications