Conversation
WalkthroughThe PR replaces the SDK-exported Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Deploying wallet-mutinynet with
|
| Latest commit: |
83523b9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://314f17d5.arkade-wallet.pages.dev |
| Branch Preview URL: | https://fix-branta.arkade-wallet.pages.dev |
Deploying tmp-boltz-upstream-mainnet-arkade-wallet with
|
| Latest commit: |
83523b9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ae7ebe48.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
| Branch Preview URL: | https://fix-branta.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
Deploying wallet-bitcoin with
|
| Latest commit: |
83523b9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f56d8038.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://fix-branta.wallet-bitcoin.pages.dev |
There was a problem hiding this comment.
Review: fix branta lint error
Verdict: APPROVE ✅
Single-file change in src/screens/Wallet/Send/Form.tsx. Replaces the SDK's Payment type (which apparently doesn't match what getPaymentsByQRCode actually returns) with a locally-derived BrantaPayment type inferred from V2BrantaClient['addPayment'].
What changed
- Removed
Paymentimport from@branta-ops/branta - Added local
BrantaPaymenttype usingPartial<Awaited<ReturnType<...>>>+platform_logo_url - Updated state and callback typing to use
BrantaPayment
Analysis
- Not protocol-critical. No VTXO, signing, forfeit, round, or exit path changes. This is purely Branta payment verification UI.
- Type safety is adequate.
Partial<...>makes all fields optional, which is fine — the template already guards access:verify_url?.startsWith(...)(line 801), conditional render onplatform_logo_url(line 810).brantaPayment.platform(line 799) could renderundefinedbut React handles that gracefully (renders nothing). - No cross-repo impact.
BrantaPaymentis file-local, not exported. - TODO is clear.
// TODO: Replace when SDK is accurate— intent to revert to SDK type once upstream fixes their export. - Runtime behavior unchanged. This is a type-level-only change; no logic modified.
Clean fix. Ship it.
Reviewed by Arkana 🔍
There was a problem hiding this comment.
Review: fix branta lint error
Verdict: APPROVE ✅
Single-file change in src/screens/Wallet/Send/Form.tsx. Replaces the SDK's Payment type (which apparently doesn't match what getPaymentsByQRCode actually returns) with a locally-derived BrantaPayment type inferred from V2BrantaClient['addPayment'].
What changed
- Removed
Paymentimport from@branta-ops/branta - Added local
BrantaPaymenttype usingPartial<Awaited<ReturnType<...>>>+platform_logo_url - Updated state and callback typing to use
BrantaPayment
Analysis
- Not protocol-critical. No VTXO, signing, forfeit, round, or exit path changes. This is purely Branta payment verification UI.
- Type safety is adequate.
Partial<...>makes all fields optional, which is fine — the template already guards access:verify_url?.startsWith(...)(line 801), conditional render onplatform_logo_url(line 810).brantaPayment.platform(line 799) could renderundefinedbut React handles that gracefully (renders nothing). - No cross-repo impact.
BrantaPaymentis file-local, not exported. - TODO is clear.
// TODO: Replace when SDK is accurate— intent to revert to SDK type once upstream fixes their export. - Runtime behavior unchanged. This is a type-level-only change; no logic modified.
Clean fix. Ship it.
Reviewed by Arkana 🔍
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/screens/Wallet/Send/Form.tsx (1)
53-58: OuterPartial<>makes theplatform_logo_urlextension optional too.The intersection
& { platform_logo_url: string }is inside thePartial<>, soplatform_logo_urlends up optional alongside every SDK field. That's actually consistent with how the code uses it (the?.startsWith(...)andbrantaPayment.platform_logo_url ?guards at lines 317 and 817), but it conflicts with the stated intent of "extended with a requiredplatform_logo_urlfield". If the property is truly meant to be optional, consider making that explicit; if it should be required, lift the extension outsidePartial<>.♻️ Optional refactor (if you want the field to read as explicitly optional)
// TODO: Replace when SDK is accurate -type BrantaPayment = Partial< - Awaited<ReturnType<V2BrantaClient['addPayment']>>['payment'] & { - platform_logo_url: string - } -> +type BrantaPayment = Partial<Awaited<ReturnType<V2BrantaClient['addPayment']>>['payment']> & { + platform_logo_url?: string +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/Wallet/Send/Form.tsx` around lines 53 - 58, The BrantaPayment type currently wraps the SDK fields and the added platform_logo_url inside Partial, making platform_logo_url optional unintentionally; update the type in Form.tsx so that the platform_logo_url intent is explicit: either move the intersection outside Partial to require platform_logo_url (e.g., Partial<...> & { platform_logo_url: string }) or keep it optional by declaring platform_logo_url?: string outside/inside as appropriate, and update usages if needed; target the BrantaPayment type definition near the top of the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/screens/Wallet/Send/Form.tsx`:
- Around line 53-58: The BrantaPayment type currently wraps the SDK fields and
the added platform_logo_url inside Partial, making platform_logo_url optional
unintentionally; update the type in Form.tsx so that the platform_logo_url
intent is explicit: either move the intersection outside Partial to require
platform_logo_url (e.g., Partial<...> & { platform_logo_url: string }) or keep
it optional by declaring platform_logo_url?: string outside/inside as
appropriate, and update usages if needed; target the BrantaPayment type
definition near the top of the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 27c1b423-d1a0-46f6-b1a1-04869bbfa548
📒 Files selected for processing (1)
src/screens/Wallet/Send/Form.tsx
* remove ionic/normalize and ion-button * rebbit fixes * feat(wallet): prefix fiat amounts with currency symbols (#535) Amounts previously rendered with a trailing ISO code (`100.00 USD`, `50.00 EUR`) now use the common Unicode currency symbol as a prefix: `$100.00`, `€50.00`, `£25.00`, `¥1,000`. Currencies without a widely-used single-char symbol (CHF, CNY) keep the trailing-code form, so CNY does not collide with JPY's `¥`. The new formatters live alongside the existing generic ones: - `prettyFiatAmount(amount, currency)` and `prettyFiatHide(value, currency)` in `src/lib/format.ts` take the `Fiats` enum as a typed parameter. - The generic `prettyAmount` / `prettyHide` are intentionally left untouched so asset tickers (e.g. a stablecoin named `USD`) that flow through those helpers keep their trailing-code format. - Decimals are derived from the currency via `fiatDecimalsFor` in `src/lib/fiat.ts` (JPY = 0, others = 2) — shared between `FiatContext` and `prettyFiatAmount` so call sites no longer need to pass `fiatDecimals()`. Touched displays: hero balance (`Balance.tsx`), transaction list rows (`TransactionsList.tsx`), amount picker (`Keyboard.tsx`), transaction details (`Details.tsx`), Send/Receive/Notes success screens, and Send form available caption. Amount input labels (`InputAmount.tsx`) also use the symbol when available. Side fix: `Details.tsx` previously called `prettyAmount` without passing `fiatDecimals()`, hardcoding 2 decimals. JPY now renders as `¥174` / `¥0` in the transaction details instead of `174.00 JPY` / `0.00 JPY`. Hero alignment: the eye-toggle button wrapper now omits an empty unit `<Text>` when the symbol is inlined into the balance, and the button sits inside a `alignItems: center` flex container next to the unit text — keeping the icon centered with the smaller ISO code in the CHF case and snug against the balance in the `$7.30` case. Tests: - New coverage for `prettyFiatAmount` and `prettyFiatHide` in `src/test/lib/format.test.ts` (symbol + trailing-code + JPY 0-decimals). - Existing `prettyAmount` / `prettyHide` tests unchanged — the generic helpers still behave the same. - UI assertions updated to match the new rendered text: `src/test/screens/wallet/index.test.tsx` → `$0.00` `src/test/e2e/init.test.ts` → `$0.00` `src/test/e2e/keyboard.test.ts` → symbol-prefix regex Co-authored-by: Sahil Chaturvedi <sahilc0@users.noreply.github.com> Co-authored-by: João Bordalo <bordalix@users.noreply.github.com> * Upgrade ts-sdk 0.4.16 - boltz-swap 0.3.17 (#539) * reverse logs in csv export (#546) * reverse logs in csv export * fix * avoid mutating the React state array * Extra tests (#548) * close copy sheet after copy * lint * test swaps in receive * Prevent swap after receival (#547) * Prevent swap after receival * fix * more removals * increase viewport size in tests * more removals * more removals * fixes * fixes * fixes * fix * remove ion-refresher * Merge branch 'master' into remove_ion-button * fix swap page (#549) * fix swap page * lint * add tests * Upgrade ts-sdk 0.4.17 - boltz-swap 0.3.18 (#551) * fix(receive): add spacing between button row and share button (#552) The vertical gap between the Add amount/Copy buttons and the Share button was too tight. Changed FlexCol gap from 0 to 0.75rem. Co-authored-by: Sahil Chaturvedi <sahilc0@users.noreply.github.com> * Revert "Upgrade ts-sdk 0.4.17 - boltz-swap 0.3.18 (#551)" (#553) This reverts commit 1752cc8. * Upgrade ts-sdk 0.4.17 - boltz-swap 0.3.18 (#551) * Revert "Upgrade ts-sdk 0.4.17 - boltz-swap 0.3.18 (#551)" (#553) This reverts commit 1752cc8. * Upgrade ts-sdk 0.4.18 - boltz-swap 0.3.19 (#554) * add git commit to chatwoot custom attributes (#563) * add lnurl tests (#564) * lint * add lnurl tests Co-authored-by: Copilot <copilot@github.com> --------- Co-authored-by: Copilot <copilot@github.com> * fix(ui): top-align tx history rows when assets are present (#537) * fix(ui): top-align tx history rows when assets are present When a transaction has assets, the right column (sats + fiat + asset info) is taller than the left column (icon + label + date). The outer FlexRow used alignItems 'center' by default, pushing the sats amount above the label text. Switch to alignItems 'start' so both columns begin at the same vertical position. Also adds explicit 'between' to the outer FlexRow and removes a dead alignItems on the non-flex wrapper div. * show only 2 coins in the rigth side of a tx in txs list (#550) --------- Co-authored-by: João Bordalo <bordalix@users.noreply.github.com> * final removal Co-authored-by: Copilot <copilot@github.com> * Upgrade ts-sdk 0.4.19 - boltz-swap 0.3.20 (#565) * lint * fix box-sizing Co-authored-by: Copilot <copilot@github.com> * .label styles Co-authored-by: Copilot <copilot@github.com> * fix branta lint error (#568) * works on refresher Co-authored-by: Copilot <copilot@github.com> * fix unit tests Co-authored-by: Copilot <copilot@github.com> * remove ionic/react * fix label alignment * DRY: AssetCard Co-authored-by: Copilot <copilot@github.com> * fix inputAmount --------- Co-authored-by: Sahil Chaturvedi <sahilchaturvedi@protonmail.com> Co-authored-by: Sahil Chaturvedi <sahilc0@users.noreply.github.com> Co-authored-by: pietro909 <2094604+pietro909@users.noreply.github.com> Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Andrew Camilleri <evilkukka@gmail.com>
Summary by CodeRabbit