fix(checkout): decrypt-failure hardening (consolidates #43 + #45, closes #44) - #46
Open
gitchadd wants to merge 5 commits into
Open
fix(checkout): decrypt-failure hardening (consolidates #43 + #45, closes #44)#46gitchadd wants to merge 5 commits into
gitchadd wants to merge 5 commits into
Conversation
added 2 commits
June 18, 2026 14:46
resolveIdentity() reads localStorage directly, which throws on corrupt identity
JSON or when storage is blocked (sandboxed / partitioned iframe). The poll-loop
and resume decrypt sites swallow throws in their own catch{}, so a throw left the
accepted screen stuck on "Decrypting..." with no recovery. Extract the shared
decrypt into decryptAndDispatch() wrapped in try/catch so any failure (Result-Err
or thrown) falls back to the existing "Session changed" sentinel.
Adds a regression test mocking the store to throw. verify green: 143 tests,
typecheck + build clean. Ported from the archived feat/upi-intent branch
(2f8ad58); the underlying issue exists independently on main's decrypt path.
…ared sentinel Consolidates the decrypt-failure hardening (supersedes PRs #43 + #45) and applies the dual-review follow-ups: - Shared DECRYPT_FAILED_SENTINEL constant in order-machine, used by the helper and by Checkout, replacing the cross-file "Session changed" magic string. - Checkout shows "Couldn't load payment details. Please contact support." for the failure sentinel (any currency) instead of rendering it as a copyable/QR-able VPA. - INR QR gated via showInrQr() on a real "@" VPA (restores the dropped guard) + a known amount; replaces the third-party api.qrserver.com <img> with a lazy client-side qrcode.react QR, closing the VPA+amount data leak. - Extracts pure paymentAddressView()/showInrQr() helpers + a unit test, giving the decrypt-failure branch real coverage (Checkout had no render harness). Closes #44. verify green: typecheck + build + 150 tests; no api.qrserver.com in the runtime bundle; sentinel centralized to one constant.
This was referenced Jun 18, 2026
added 2 commits
June 18, 2026 17:40
Per the e2e design critique: render the decrypt-failure message in the existing dangerSoft error box (matching state.error) with a warning icon, instead of bare red text. Pure visual consistency on the error branch; no logic/flow change.
When the counterparty VPA can't be decrypted (addrView === "error") the user has no payment details and can't have paid, so disable + dim the "I've paid" button; the remaining actions are Contact support (per the message) and Cancel order.
Collaborator
Author
Final agentic review (full PR, 4 commits)Two strictly read-only agents (bug-hunter, compliance) over Verdict: clean, shippable. No production bugs, compliance PASS.
One pre-existing finding (NOT introduced; non-blocking)During the transient "Decrypting…" state ( Deploy gate: merge needs explicit per-merge approval. This review is the agentic step. |
… failure) Generalize the guard via the unit-tested payableAddressShown() helper: "I've paid" is actionable only once a payable address (or compound fields) is on screen, so it is disabled during both the transient "Decrypting..." state and a decrypt failure — the user can't mark-paid before any VPA is shown. Closes the pre-existing decrypting-state gap flagged in the final review.
Collaborator
Author
|
Final-review finding closed. The one (pre-existing) finding — "I've paid" enabled during the transient "Decrypting…" state — is now fixed in |
gitchadd
added a commit
that referenced
this pull request
Jun 18, 2026
gitchadd
added a commit
that referenced
this pull request
Jun 18, 2026
gitchadd
added a commit
that referenced
this pull request
Jun 18, 2026
gitchadd
added a commit
that referenced
this pull request
Jun 18, 2026
Collaborator
Author
E2E screenshots — PR #46 (decrypt-failure hardening)Captured from a real-browser run against the demo built on this branch (retina). "I've paid" button state called out per screen. 1. Checkout
2. Accepted — client-side QR (replaces
|
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
1 participant
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.
Consolidates the two coupled decrypt-failure PRs (supersedes #43 + #45) into one branch — the dual-agent review found a merge-ordering hazard between them, and the top follow-up (a shared sentinel constant) spans both files, so they're better as one PR. Applies all three review follow-ups.
What it does
decryptAndDispatchwraps the decrypt in try/catch so a thrownresolveIdentity()/localStorage error no longer strands the accepted screen on "Decrypting…".api.qrserver.com<img>is replaced by a lazy client-sideqrcode.reactQR (counterparty VPA + amount + order id no longer sent to a third party).Review follow-ups applied
DECRYPT_FAILED_SENTINELconstant inorder-machine.ts, used by the helper andCheckout.tsx— kills the cross-file"Session changed"magic string (verified: no stray literal outside the constant)..includes("@")VPA guard on the INR QR (viashowInrQr()) — closes the malformed-Result-Ok edge the port had dropped.paymentAddressView()/showInrQr()helpers + a unit test (test/checkoutAddressView.test.ts) covering the decrypt-failure branch and the@-guard —Checkoutpreviously had no render harness.Verification
npm run verifygreen: typecheck, build, 150 tests (142 base + decrypt throw-path + 7 new helper tests).api.qrserver.comin the runtimedist/*.js|*.cjs(the.maphit is only this PR's own comment);QRCodeSVGpresent.Closes #44. Deploy gate: human review + explicit per-merge approval. Not for auto-merge.