Repository navigation
Upgrade branta sdk to v3.1.2 - #622
kylemccullen wants to merge 5 commits into
Conversation
|
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 ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughSendForm is refactored to use the Branta v2 SDK (dependency bumped to 3.1.3). It instantiates a module-level BrantaService (PrivacyMode.Strict) with env-based baseUrl, replaces legacy client imports/types with the SDK's ChangesBranta v2 SDK Migration
Sequence Diagram(s)sequenceDiagram
participant SendForm
participant BrantaService
participant BrantaAPI
SendForm->>BrantaService: getPaymentsByQrCode(rawScanData) / getPayments(typed)
BrantaService->>BrantaAPI: HTTP request to Branta endpoints
BrantaAPI-->>BrantaService: return Payment[] (SDK Payment objects)
BrantaService-->>SendForm: return first Payment with platformLogoUrl/platformLogoLightUrl/verifyUrl
SendForm->>SendForm: set brantaPayment and brantaVerifyUrl (HTTPS-only)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 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: 1
🤖 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 `@src/screens/Wallet/Send/Form.tsx`:
- Around line 286-302: Add a lightweight local pre-check for Branta/ZK scans
inside the useEffect that reads rawScanData before setting loading or calling
brantaClient.getPaymentsByQrCode: implement an isBrantaQr(rawScanData) helper
(or inline check) that only returns true for the known Branta/ZK QR
patterns/markers you expect, and if it returns false immediately call
setBrantaPayment(null), setBrantaVerifyUrl(undefined), setBrantaLoading(false)
and return; only when isBrantaQr(rawScanData) is true should you
setBrantaLoading(true) and invoke brantaClient.getPaymentsByQrCode(rawScanData).
Reference symbols: rawScanData, useEffect block,
brantaClient.getPaymentsByQrCode, setBrantaPayment, setBrantaVerifyUrl,
setBrantaLoading, and the new isBrantaQr helper.
🪄 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: aeac6f3a-4278-4dfe-9161-d1551f9d2d3e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.jsonsrc/screens/Wallet/Send/Form.tsx
| // fetch branta payment info for ZK QR-scanned addresses (SDK strict mode gates non-ZK) | ||
| useEffect(() => { | ||
| if (!rawScanData) { | ||
| setBrantaPayment(null) | ||
| setBrantaVerifyUrl(undefined) | ||
| setBrantaLoading(false) | ||
| return | ||
| } | ||
|
|
||
| setBrantaPayment(null) | ||
| setBrantaVerifyUrl(undefined) | ||
| setBrantaLoading(true) | ||
| let cancelled = false | ||
|
|
||
| let isValidZKCode = false | ||
| try { | ||
| const url = new URL(rawScanData.trim()) | ||
| isValidZKCode = url.searchParams.has('branta_id') && url.searchParams.has('branta_secret') | ||
| } catch { | ||
| // Invalid URL, not a ZK code | ||
| } | ||
|
|
||
| if (!isValidZKCode) { | ||
| setBrantaLoading(false) | ||
| return | ||
| } | ||
|
|
||
| setBrantaLoading(true) | ||
| brantaClient | ||
| .getPaymentsByQRCode(rawScanData) | ||
| .then((payments: BrantaPayment[]) => { | ||
| .getPaymentsByQrCode(rawScanData) | ||
| .then(({ payments, verifyUrl }) => { |
There was a problem hiding this comment.
Restore a local Branta/ZK pre-check before calling the SDK.
Line 301 now sends every non-empty scanned payload to getPaymentsByQrCode(rawScanData). That means plain invoices, BIP21 URIs, and addresses scanned in this screen get disclosed to Branta even when they are not Branta QR codes and no Branta UI is ultimately shown. Please reintroduce the lightweight local gate that existed before this migration and only call the SDK for Branta/ZK scans.
🤖 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 `@src/screens/Wallet/Send/Form.tsx` around lines 286 - 302, Add a lightweight
local pre-check for Branta/ZK scans inside the useEffect that reads rawScanData
before setting loading or calling brantaClient.getPaymentsByQrCode: implement an
isBrantaQr(rawScanData) helper (or inline check) that only returns true for the
known Branta/ZK QR patterns/markers you expect, and if it returns false
immediately call setBrantaPayment(null), setBrantaVerifyUrl(undefined),
setBrantaLoading(false) and return; only when isBrantaQr(rawScanData) is true
should you setBrantaLoading(true) and invoke
brantaClient.getPaymentsByQrCode(rawScanData). Reference symbols: rawScanData,
useEffect block, brantaClient.getPaymentsByQrCode, setBrantaPayment,
setBrantaVerifyUrl, setBrantaLoading, and the new isBrantaQr helper.
There was a problem hiding this comment.
Setting privacy: 'strict' in the BrantaClientOptions prevents this behavior within the SDK itself. When strict mode is enabled, the SDK will not transmit plain-text addresses, BIP21 URIs, or standard invoice formats to Branta servers—it performs local validation first and only makes network requests for data that matches the Branta/ZK format.
There was a problem hiding this comment.
@kylemccullen, thanks for the clarification! That makes sense — since the client is initialized with privacy: PrivacyMode.Strict, the SDK itself performs the local Branta/ZK format check before any network request, making the removed isValidZKCode pre-check redundant. The concern is no longer valid. I'll note this behavior for future reviews.
(ˆ · ˆ) 🐇
✏️ Learnings added
Learnt from: kylemccullen
Repo: arkade-os/wallet PR: 622
File: src/screens/Wallet/Send/Form.tsx:286-302
Timestamp: 2026-05-19T11:53:58.967Z
Learning: In `src/screens/Wallet/Send/Form.tsx` (React/TypeScript), the `BrantaService` client from `branta-ops/branta` v3.x is initialized with `privacy: PrivacyMode.Strict`. When `PrivacyMode.Strict` is set in `BrantaClientOptions`, the SDK performs local validation before any network request and will NOT transmit plain-text addresses, BIP21 URIs, or standard Lightning invoice formats to Branta servers — only data matching the Branta/ZK QR format triggers a network call. Therefore, no additional local pre-check (e.g. `isValidZKCode`) is needed in the component before calling `brantaClient.getPaymentsByQrCode(rawScanData)`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| const brantaClient = new V2BrantaClient({ | ||
| const brantaClient = new BrantaService({ | ||
| baseUrl: BrantaServerBaseUrl.Production, | ||
| privacy: PrivacyMode.Strict, |
There was a problem hiding this comment.
Note - this SDK version enables Zero-Knowledge for lightning payments (in addition to onchain zero-knowledge)
|
|
||
| setBrantaPayment(null) | ||
| setBrantaVerifyUrl(undefined) | ||
| setBrantaLoading(true) |
There was a problem hiding this comment.
i dont see any code that sets this false again? what if something fails?
There was a problem hiding this comment.
I believe a fail case is handled by the finally block
|
Superseded by #673, which re-creates this branch with |
Summary by CodeRabbit
Dependencies
Improvements