Skip to content

fix(add-money): stop truncating Bridge deposit reference to 10 chars#2415

Closed
abalinda wants to merge 4 commits into
mainfrom
fix/deposit-reference-truncation
Closed

fix(add-money): stop truncating Bridge deposit reference to 10 chars#2415
abalinda wants to merge 4 commits into
mainfrom
fix/deposit-reference-truncation

Conversation

@abalinda

@abalinda abalinda commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

On the bank-transfer Add Money screen, the Bridge deposit reference was sliced to its first 10 characters in display, the copy button, the copy-all-details blob, and the confirmation summary (AddMoneyBankDetails.tsx, 4 sites). Users wired funds with the truncated reference, Bridge couldn't reconcile the incoming transfer, and the deposit sat in AWAITING_FUNDS until cancelled/returned — a happy-path money-flow break confirmed via a real user case and a PostHog session recording.

The truncation came in via 44c5beb63 ("limit deposit msg to 10 chars") — a display-overflow patch that also truncated textToCopy. This PR restores the full depositMessage everywhere and solves the original overflow with break-all, matching the correct sibling implementation in BridgeDepositInstructions.tsx.

Follow-up fixes from /code-review high:

  • All four surfaces now read one hoisted depositReference const — the per-surface copy-paste is what let the original bug land on some surfaces only.
  • Share Details is now gated on the reference existing — previously the literal 'Loading...' fallback could be shared as the deposit reference if tapped before onramp data resolved (same unreconcilable-wire outcome as the truncation).

Previous

image image

Now

image

Design notes / accepted trade-offs

  • The on-screen 'Loading...' display fallbacks stay (legit loading UI); only the machine-consumed paths (copy/share) are gated on real data.
  • No wrap-guard added to the "double check" InfoCard line: Bridge references are a fixed 20-char shape, which fits at body-text size; break-all is only needed on the 4xl display.
  • Pre-existing smell, out of scope: AddMoneyBankDetails and BridgeDepositInstructions render the same Bridge deposit instructions independently — that divergence is how this bug shipped. Consolidation is a follow-up.

Risks / breaking changes

  • None cross-repo — backend already returns the full depositMessage; this is display/copy only.
  • Visual: the reference now wraps across lines on narrow screens (previously truncated). break-all keeps it inside the card.
  • Hotfix to main → back-merge debt main→dev after merge.

QA

  • New colocated regression suite __tests__/AddMoneyBankDetails.test.tsx (4 tests): full reference in display, textToCopy, double-check summary, and shareable text; no copy/share while the reference is loading. Red-checked: reintroducing the slice fails the suite.
  • Note: add-money-states.test.tsx stubs this component, so the regression tests live colocated with it instead.
  • Full local gate: prettier ✅ · typecheck ✅ · jest 125/125 suites ✅ · changed files lint clean
  • Visual QA on the local harness (UI_REPO=worktree qa up): full 20-char sandbox reference renders, wraps, copies.

abalinda added 2 commits July 14, 2026 16:23
Users wired funds with the first 10 chars of the deposit reference, so
Bridge could not match the incoming transfer and deposits sat in
AWAITING_FUNDS until cancelled/returned. Display overflow (the original
reason for the truncation) is handled with break-all instead, matching
BridgeDepositInstructions.tsx.
@vercel

vercel Bot commented Jul 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
peanut-wallet Ready Ready Preview, Comment Jul 14, 2026 2:55pm

Request Review

@abalinda

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@abalinda, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 52de5b8b-05fe-447d-8c22-3d9c2c405c7d

📥 Commits

Reviewing files that changed from the base of the PR and between dd4d596 and 92a392a.

📒 Files selected for processing (2)
  • src/components/AddMoney/components/AddMoneyBankDetails.tsx
  • src/components/AddMoney/components/__tests__/AddMoneyBankDetails.test.tsx

Walkthrough

Changes

Deposit reference handling

Layer / File(s) Summary
Propagate the full deposit reference
src/components/AddMoney/components/AddMoneyBankDetails.tsx
Bank details, displayed reference text, clipboard copying, and the confirmation summary now use the complete deposit message.
Verify full-reference behavior
src/components/AddMoney/components/__tests__/AddMoneyBankDetails.test.tsx
Adds regression tests for rendering, copying, confirmation text, and shareable bank-details text.

Estimated code review effort: 2 (Simple) | ~15 minutes

Suggested labels: enhancement

Suggested reviewers: hugo0

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely summarizes the main change: removing Bridge deposit reference truncation.
Description check ✅ Passed The description is directly related to the changeset and accurately describes the truncation fix and related tests.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Code-analysis diff

Painscore total: 6109.87 → 6110.11 (+0.24)
Findings: 0 net (+7 new, -7 resolved)

🆕 New findings (7)

  • critical complexity — src/components/AddMoney/components/AddMoneyBankDetails.tsx — CC 83, MI 58.44, SLOC 149
  • high method-complexity — src/components/AddMoney/components/AddMoneyBankDetails.tsx:50 — AddMoneyBankDetails CC 34 SLOC 64
  • medium react-long-component — src/components/AddMoney/components/AddMoneyBankDetails.tsx:50 — AddMoneyBankDetails is 394 lines — split it
  • medium high-mdd — src/components/AddMoney/components/AddMoneyBankDetails.tsx:50 — AddMoneyBankDetails: MDD 115.3 (uses across many lines from declarations)
  • medium high-mdd — src/components/AddMoney/components/AddMoneyBankDetails.tsx:184 — generateBankDetails: MDD 22.1 (uses across many lines from declarations)
  • medium method-complexity — src/components/AddMoney/components/AddMoneyBankDetails.tsx:184 — CC 21 SLOC 22
  • low high-dlt — src/components/AddMoney/components/AddMoneyBankDetails.tsx:50 — AddMoneyBankDetails: DLT 27 (calls 27 distinct functions — high context load)

✅ Resolved (7)

  • src/components/AddMoney/components/AddMoneyBankDetails.tsx — CC 83, MI 58.4, SLOC 148
  • src/components/AddMoney/components/AddMoneyBankDetails.tsx:50 — AddMoneyBankDetails CC 33 SLOC 63
  • src/components/AddMoney/components/AddMoneyBankDetails.tsx:50 — AddMoneyBankDetails is 388 lines — split it
  • src/components/AddMoney/components/AddMoneyBankDetails.tsx:50 — AddMoneyBankDetails: MDD 112.7 (uses across many lines from declarations)
  • src/components/AddMoney/components/AddMoneyBankDetails.tsx:179 — CC 22 SLOC 22
  • src/components/AddMoney/components/AddMoneyBankDetails.tsx:179 — generateBankDetails: MDD 22.1 (uses across many lines from declarations)
  • src/components/AddMoney/components/AddMoneyBankDetails.tsx:50 — AddMoneyBankDetails: DLT 28 (calls 28 distinct functions — high context load)

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🧪 UI test report — ✅ all green

Suites

  • unit: 1834 ran, 0 failed, 0 skipped, 31.8s

📊 Coverage (unit)

metric %
statements 58.1%
branches 42.1%
functions 46.7%
lines 58.2%
⏱ 10 slowest test cases
time test
4.2s src/components/Card/share-asset/__tests__/shareAssetLayout.test.ts › never places two stickers in heavy overlap (broad seed sweep)
0.6s src/components/Card/share-asset/__tests__/shareAssetLayout.test.ts › every sticker stays within canvas at any count
0.5s src/app/actions/__tests__/api-headers.test.ts › should include Content-Type in updateUserById
0.3s src/app/actions/__tests__/api-headers-extended.test.ts › should not include apiKey in updateUserById body
0.3s src/components/Card/share-asset/__tests__/shareAssetLayout.test.ts › keeps stickers off the username pill (final pass respects the keep-out)
0.1s src/components/Global/GeneralRecipientInput/__tests__/GeneralRecipientInput.test.tsx › should handle valid ETH address with surrounding spaces
0.1s src/components/Global/GeneralRecipientInput/__tests__/GeneralRecipientInput.test.tsx › should handle invalid ETH address (missing 0x prefix)
0.1s src/components/Global/GeneralRecipientInput/__tests__/GeneralRecipientInput.test.tsx › should handle valid 9-digit US account
0.1s src/components/Global/GeneralRecipientInput/__tests__/GeneralRecipientInput.test.tsx › should handle too long for US account
0.1s src/app/(mobile-ui)/qr-pay/__tests__/qr-pay-states.test.tsx › Perk claim in progress shows disabled button + progress
📍 Inline annotations are in the **Unit test report** check above. Coverage artifact: `coverage-unit`. Generated by `.github/workflows/tests.yml`.

…nce presence

Code-review findings: the literal 'Loading...' fallback could be shared
as the deposit reference via ShareButton before onrampData resolved, and
the reference expression was copy-pasted across four surfaces — the same
structure that let the original truncation land on one surface only.
@abalinda abalinda marked this pull request as ready for review July 14, 2026 14:56
@abalinda

Copy link
Copy Markdown
Contributor Author

Closing — diagnosis superseded. The 10-char truncation is intentional (some banks, e.g. Wise, cap reference fields at 10 chars; Bridge matches deposits on the partial reference), so removing it was the wrong direction. The real customer-impacting bugs are (1) the transaction drawer showing the FULL reference while Add Money shows the truncated one (two-codes confusion) and (2) Cancel deposit firing instantly with no confirmation, right next to the support link. Replacement PR follows: uniform 10-char reference everywhere + confirmation step before cancelling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant