fix(receipt): copy/display crypto withdrawal destinations verbatim (Tron/Solana) - #2507
Merged
Merged
Conversation
…corrupted them
The receipt's account-details row ran the destination through formatIban,
which treats any string whose first two chars are letters as an IBAN and
uppercases + space-chunks it. Every Tron address starts with 'T…' and
base58 is case-sensitive, so:
- COPY: the clipboard got an uppercased, space-chunked (invalid) address
for EVERY Tron/Solana destination.
- DISPLAY: maskAccountIdentifier's 'plain' fallback runs the same formatIban
on any identifier matching /^[a-zA-Z]{2}\d/, so Tron addresses whose 3rd
char is a digit (~13%, e.g. 'TN9R…') were also mangled on screen.
Keyed to the ACTUAL wire vocabulary the BE serializes for a CRYPTO_WITHDRAW:
recipientAccount.type is 'address' (history mapGenericIntent, sender viewer),
'evm-address', or 'peanut-wallet' — never the Prisma 'WALLET_EXTERNAL' enum,
which accountTypeToApi maps to 'evm-address' before it ever reaches the FE.
A closed 3-value isCryptoAddressType allowlist (every other type stays a bank
rail → keeps formatIban) now gates both the copy helper and the display mask;
the row labels crypto destinations 'Address' instead of 'Account Number'.
Tests feed the real wire values ('address'/'evm-address') and cover both copy
and display — each fails on the pre-fix code. (Caught in adversarial review:
the first pass keyed off 'WALLET_*', a type the wire never carries, so the fix
was a no-op and its tests were green on a fabricated value.)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughCrypto wallet identifiers are now recognized by their wire types, kept unmasked and copied verbatim. Bank-account identifiers retain rail-specific labels, IBAN formatting, and masking. The transaction receipt uses the new shared copy-value helper. ChangesCrypto account handling
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
abalinda
marked this pull request as draft
July 24, 2026 13:40
Contributor
Code-analysis diffPainscore total: 6263.81 → 6263.22 (-0.59) 🆕 New findings (9)
✅ Resolved (8)
📈 Painscore deltas (top movers)
|
Contributor
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/TransactionDetails/__tests__/transaction-details.utils.test.ts (1)
34-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover
peanut-walletin the downstream helper tests.The predicate test includes
peanut-wallet, butgetAccountCopyValue,maskAccountIdentifier, andgetBankAccountLabelonly exerciseaddressandevm-address. Addpeanut-walletcases so every allowlisted wire type is protected across copy, display, and labeling paths.🤖 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/components/TransactionDetails/__tests__/transaction-details.utils.test.ts` around lines 34 - 80, Extend the tests for getAccountCopyValue, maskAccountIdentifier, and getBankAccountLabel to include the allowlisted peanut-wallet wire type, covering copy, display, and labeling behavior alongside address and evm-address. Reuse the existing crypto destination expectations and ensure peanut-wallet is labeled “Address.”
🤖 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.
Nitpick comments:
In
`@src/components/TransactionDetails/__tests__/transaction-details.utils.test.ts`:
- Around line 34-80: Extend the tests for getAccountCopyValue,
maskAccountIdentifier, and getBankAccountLabel to include the allowlisted
peanut-wallet wire type, covering copy, display, and labeling behavior alongside
address and evm-address. Reuse the existing crypto destination expectations and
ensure peanut-wallet is labeled “Address.”
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b46a622e-3993-4b78-ba7c-7d0714a5b180
📒 Files selected for processing (4)
src/components/TransactionDetails/TransactionDetailsReceipt.tsxsrc/components/TransactionDetails/__tests__/transaction-details.utils.test.tssrc/components/TransactionDetails/transaction-details.utils.tssrc/utils/account-mask.utils.ts
abalinda
marked this pull request as ready for review
July 24, 2026 14:25
kushagrasarathe
approved these changes
Jul 24, 2026
kushagrasarathe
left a comment
kushagrasarathe
left a comment
Contributor
There was a problem hiding this comment.
Reviewed for code quality, DRY, and bug-risk. Approving — ship as-is (LOW risk).
Traced both the copy path (getAccountCopyValue) and the display path (maskAccountIdentifier) end-to-end against formatIban.
Quality/DRY — clean. isCryptoAddressType is defined once in account-mask.utils and reused by both the copy helper and the mask short-circuit — no duplication. This is a root-cause fix in the right place: it gates both corruption paths, not just the copy symptom the ticket named. Tests feed the real wire values ('address'/'evm-address') and each fails on pre-fix code.
Two small notes (non-blocking):
- Closed allowlist is coupled to the BE vocabulary.
CRYPTO_ADDRESS_TYPES = {address, evm-address, peanut-wallet}. Any future crypto wiretypethe BE introduces (e.g.'tron-address') falls through toformatIban→ this exact corruption class silently returns. The comment frames this as the deliberate safe-default (new rails keep IBAN formatting), which is the right call — just flagging the coupling. peanut-walletnow renders verbatim + labels as "Address" where it previously hit the mask/label default. Behavior change for the user's-own-wallet row; low risk since it's an EVM address and verbatim is correct.
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.
Summary
QA of the Tron/Solana withdraw hotfix (#2490) found the withdrawal receipt corrupts the destination address. The account-details row runs the identifier through
formatIban, which treats any string whose first two chars are letters as an IBAN and uppercases + space-chunks it. Every Tron address starts withT…and base58 is case-sensitive, so:maskAccountIdentifier'splainfallback runs the sameformatIbanon any identifier matching/^[a-zA-Z]{2}\d/, so Tron addresses whose 3rd char is a digit (~13%, e.g.TN9R…) were also mangled on screen. (The commit that shipped fix(withdraw): show non-EVM (Tron/Solana) token selection in the selector button #2490 claimed "display was already safe" — it wasn't.)Fix
Keyed to the actual wire vocabulary the BE serializes for a
CRYPTO_WITHDRAW:recipientAccount.typeis'address'(historymapGenericIntent, sender viewer),'evm-address', or'peanut-wallet'— never the PrismaWALLET_EXTERNALenum, whichaccountTypeToApimaps to'evm-address'before it reaches the FE.A closed 3-value
isCryptoAddressTypeallowlist (every other type stays a bank rail → keepsformatIban) now gates both the copy helper (getAccountCopyValue) and the display mask (maskAccountIdentifiershort-circuit → verbatim). The row labels crypto destinations "Address" instead of "Account Number".Risks / QA
iban/us/clabe/cbu/cvu/pix/BANK_*/rawpayment_rail) are outside the closed allowlist, so theirformatIbancopy + per-rail masking are untouched.'address'/'evm-address') and cover both copy and display — each fails on the pre-fix code (verified:formatIbancorrupts both sample Tron addresses; the digit-3rd-char one also on display). Full jest suite green.hotfix/non-evm-withdraw-false-failed(the BE was flipping these same withdrawals to Failed). Independent deploys.Summary by CodeRabbit