Shop: duplicated BIP21 payment_intent returns success, opening send sheet with the previous invoice

[jvsena42]

Summary

In AppViewModel.handleScannedData with scope: .paymentRequests, a duplicated BIP21 URI is rejected with a toast and a plain return — it neither throws nor reaches resetSendState(). The Shop caller only guards on catch, so this "failure" is treated as success and the send sheet opens pre-filled with whatever payment state was already in AppViewModel.

Where

Bitkit/ViewModels/AppViewModel.swift:384

if Bip21Utils.isDuplicatedBip21(uri) {
    toast(
        type: .error,
        title: t("other__scan_err_decoding"),
        description: t("other__scan__error__generic"),
        accessibilityIdentifier: "InvalidAddressToast"
    )
    return          // <-- returns success; resetSendState() at :401 is never reached
}

Caller — Bitkit/Views/Shop/ShopMain.swift:57:

do {
    try await app.handleScannedData(paymentUri, scope: .paymentRequests)
    PaymentNavigationHelper.openPaymentSheet(...)   // <-- runs on the malformed-URI path
} catch {
    app.toast(error)
}

Every other rejection in the .paymentRequests branch throws (SamRockSetupRequest guards, the ShopPaymentRequest.isSupported guard). The duplicated-BIP21 check is the only exit that returns instead.

Impact

1. User scans a Lightning invoice or BIP21 URI normally, populating app.scannedLightningInvoice / send state.
2. User opens Shop; the embedded page posts a payment_intent whose paymentUri is a duplicated BIP21.
3. User sees a "decoding error" toast and the send sheet opens, pre-filled with the stale prior invoice, sitting at .confirm.

That is one tap away from paying the wrong recipient, with an error toast on screen implying nothing was accepted.

ShopPaymentRequestTests.testNonPaymentRequestDoesNotClearExistingPaymentState already covers exactly this state-leak concern for the throwing path — this path slips past it.

Suggested fix

Throw instead of returning, so the Shop caller's catch handles it uniformly:

if Bip21Utils.isDuplicatedBip21(uri) {
    throw ShopPaymentRequestError.unsupportedRequest
}

The toast is then emitted by app.toast(error) in the caller, matching the other rejection paths. Non-shop (.unrestricted) scanning is unaffected, since the check lives inside the scope == .paymentRequests branch.

Worth adding a regression test alongside testNonPaymentRequestDoesNotClearExistingPaymentState covering a duplicated-BIP21 payment_intent.

Notes

Found during review of release-2.4.1. Not a cherry-pick regression — present on master (introduced with the shop payment bridge hardening, #668), and byte-identical on both branches.