diff --git a/Bitkit/Utilities/Bip21Utils.swift b/Bitkit/Utilities/Bip21Utils.swift index deb64e230..22e5fa163 100644 --- a/Bitkit/Utilities/Bip21Utils.swift +++ b/Bitkit/Utilities/Bip21Utils.swift @@ -2,8 +2,6 @@ import Foundation /// Utility methods for BIP21 URI handling enum Bip21Utils { - private static let bip21Prefix = "bitcoin:" - static func hardwareInvoice(address: String, amountSats: UInt64, message: String) -> String { var components = URLComponents() components.scheme = "bitcoin" @@ -24,16 +22,4 @@ enum Bip21Utils { components.queryItems = queryItems.isEmpty ? nil : queryItems return components.string ?? "bitcoin:\(address)" } - - /// Checks if a BIP21 URI is duplicated (contains multiple bitcoin: prefixes). - /// Workaround for https://github.com/synonymdev/bitkit-core/issues/63 - /// - Parameter input: The string to check - /// - Returns: true if the input contains duplicated BIP21 URIs, false otherwise - static func isDuplicatedBip21(_ input: String) -> Bool { - let lowercased = input.lowercased() - guard let firstIndex = lowercased.range(of: bip21Prefix)?.upperBound else { - return false - } - return lowercased[firstIndex...].contains(bip21Prefix) - } } diff --git a/Bitkit/Utilities/ClipboardPrompt.swift b/Bitkit/Utilities/ClipboardPrompt.swift index 8d2e7f179..f2d6ddfb6 100644 --- a/Bitkit/Utilities/ClipboardPrompt.swift +++ b/Bitkit/Utilities/ClipboardPrompt.swift @@ -67,7 +67,6 @@ enum ClipboardPromptValidator { } let normalized = uri.removingLightningSchemes() - guard !Bip21Utils.isDuplicatedBip21(normalized) else { return false } // Only recognize these shapes here: fetching remote metadata must wait for the user's OK tap. if requiresNetworkResolution(normalized) { return true diff --git a/Bitkit/Utilities/ShopPaymentRequest.swift b/Bitkit/Utilities/ShopPaymentRequest.swift index e3cf7a7a9..4d01e5544 100644 --- a/Bitkit/Utilities/ShopPaymentRequest.swift +++ b/Bitkit/Utilities/ShopPaymentRequest.swift @@ -8,6 +8,16 @@ enum ScanHandlingScope { } enum ShopPaymentRequest { + static func decodingErrorToast(for error: Error) -> Toast? { + guard error is DecodingError else { return nil } + return Toast( + type: .error, + title: t("other__scan_err_decoding"), + description: t("other__scan__error__generic"), + accessibilityIdentifier: "InvalidAddressToast" + ) + } + static func isSupported(_ data: BitkitCore.Scanner) -> Bool { switch data { case .onChain, .lightning, .lnurlPay: diff --git a/Bitkit/ViewModels/AppViewModel.swift b/Bitkit/ViewModels/AppViewModel.swift index 609a47b49..b23ea107b 100644 --- a/Bitkit/ViewModels/AppViewModel.swift +++ b/Bitkit/ViewModels/AppViewModel.swift @@ -638,15 +638,6 @@ extension AppViewModel { else { throw ScanHandlingError.unsupportedRequest } - if Bip21Utils.isDuplicatedBip21(uri) { - toast( - type: .error, - title: t("other__scan_err_decoding"), - description: t("other__scan__error__generic"), - accessibilityIdentifier: "InvalidAddressToast" - ) - return - } let data = try await decode(invoice: uri) try ensureScannedDataHandlingOwnership(handlingId, claimedContactPaymentContext: claimedContactPaymentContext) guard ShopPaymentRequest.isSupported(data) else { throw ScanHandlingError.unsupportedRequest } @@ -668,17 +659,6 @@ extension AppViewModel { return } - // Workaround for duplicated BIP21 URIs (bitkit-core#63) - if scope == .unrestricted, Bip21Utils.isDuplicatedBip21(uri) { - toast( - type: .error, - title: t("other__scan_err_decoding"), - description: t("other__scan__error__generic"), - accessibilityIdentifier: "InvalidAddressToast" - ) - return - } - if PubkyAuthRequest.isProtocolURL(uri) { guard scope == .unrestricted else { throw ScanHandlingError.pubkyAuthRequest @@ -1179,15 +1159,6 @@ extension AppViewModel { return } - // Workaround for duplicated BIP21 URIs (bitkit-core#63) - if Bip21Utils.isDuplicatedBip21(normalized) { - guard currentSequence == manualEntryValidationSequence else { return } - manualEntryValidationResult = .invalid - isManualEntryInputValid = false - showValidationErrorToast(for: .invalid) - return - } - if PubkyPublicKeyFormat.normalized(normalized) != nil { guard currentSequence == manualEntryValidationSequence else { return } if PaykitFeatureFlags.isUIEnabled { diff --git a/Bitkit/Views/Shop/ShopMain.swift b/Bitkit/Views/Shop/ShopMain.swift index 815ed30b0..b8311ac81 100644 --- a/Bitkit/Views/Shop/ShopMain.swift +++ b/Bitkit/Views/Shop/ShopMain.swift @@ -68,7 +68,16 @@ struct ShopMain: View { sheetViewModel: sheets ) } catch { - app.toast(error) + if let toast = ShopPaymentRequest.decodingErrorToast(for: error) { + app.toast( + type: toast.type, + title: toast.title, + description: toast.description, + accessibilityIdentifier: toast.accessibilityIdentifier + ) + } else { + app.toast(error) + } } } } diff --git a/BitkitTests/Bip21DecodingTests.swift b/BitkitTests/Bip21DecodingTests.swift new file mode 100644 index 000000000..38ef2ec1e --- /dev/null +++ b/BitkitTests/Bip21DecodingTests.swift @@ -0,0 +1,100 @@ +@testable import Bitkit +import BitkitCore +import Combine +import XCTest + +@MainActor +final class Bip21DecodingTests: XCTestCase { + private let address = "bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk" + + private var duplicatedURI: String { + "bitcoin:\(address)?amount=0.0000002&message=Bitkit" + + "bitcoin:\(address)?amount=0.0000003&message=Bitkit" + } + + func testCoreRejectsDuplicateSingletonParameters() async { + for uri in [ + duplicatedURI, + "bitcoin:\(address)?amount=0.000035&AMOUNT=0.00005", + "bitcoin:\(address)?label=first&LABEL=second", + "bitcoin:\(address)?message=first&Message=second", + "bitcoin:\(address)?pop=callback%3a&req-pop=callback%3a", + ] { + do { + _ = try await decode(invoice: uri) + XCTFail("Expected duplicate singleton parameters to fail: \(uri)") + } catch DecodingError.InvalidFormat { + } catch { + XCTFail("Unexpected error: \(error)") + } + } + } + + func testCorePreservesBitcoinTextInQueryMetadata() async throws { + for (key, value) in [ + ("message", "bitcoin:donation"), + ("label", "BITCOIN:donation"), + ("custom", "bitcoin:1BoatSLRHtKNngkdXEeobR76b53LETtpyT"), + ("message", "Why?"), + ] { + let data = try await decode(invoice: "bitcoin:\(address)?amount=0.000035&\(key)=\(value)") + guard case let .onChain(invoice) = data else { + return XCTFail("Expected an on-chain invoice") + } + XCTAssertEqual(invoice.address, address) + XCTAssertEqual(invoice.amountSatoshis, 3500) + XCTAssertEqual(invoice.params?[key], value) + } + } + + func testShopDecodeFailureThrowsWithoutReplacingPreviousInvoice() async { + let app = AppViewModel() + app.scannedOnchainInvoice = OnChainInvoice( + address: address, amountSatoshis: 42, label: nil, message: "Previous invoice", params: [:] + ) + + do { + try await app.handleScannedData(duplicatedURI, scope: .paymentRequests) + XCTFail("Expected Shop decoding to throw instead of returning success") + } catch DecodingError.InvalidFormat { + } catch { + XCTFail("Unexpected error: \(error)") + } + + XCTAssertEqual(app.scannedOnchainInvoice?.amountSatoshis, 42) + XCTAssertEqual(app.scannedOnchainInvoice?.message, "Previous invoice") + XCTAssertNil(app.scannedLightningInvoice) + } + + func testValidBitcoinNoteReachesPaymentStateInEveryPaymentScope() async throws { + for scope: ScanHandlingScope in [.unrestricted, .paymentRequests, .onchainPayments] { + let app = AppViewModel() + try await app.handleScannedData( + "bitcoin:\(address)?amount=0.000035&message=bitcoin:donation", + scope: scope, + alternativeOnchainBalanceSats: 100_000 + ) + XCTAssertEqual(app.scannedOnchainInvoice?.address, address) + XCTAssertEqual(app.scannedOnchainInvoice?.amountSatoshis, 3500) + XCTAssertEqual(app.scannedOnchainInvoice?.message, "bitcoin:donation") + } + } + + func testManualEntryUsesCoreValidation() async { + for (uri, expected): (String, ManualEntryValidationResult) in [ + ("bitcoin:\(address)?amount=0.000035&message=bitcoin:donation", .valid), + (duplicatedURI, .invalid), + ] { + let app = AppViewModel() + let validated = expectation(description: "manual entry validated") + let subscription = app.$manualEntryValidationResult.dropFirst().sink { result in + XCTAssertEqual(result, expected) + validated.fulfill() + } + app.validateManualEntryInput(uri, savingsBalanceSats: 100_000, spendingBalanceSats: 0) + await fulfillment(of: [validated], timeout: 5) + XCTAssertEqual(app.isManualEntryInputValid, expected == .valid) + subscription.cancel() + } + } +} diff --git a/BitkitTests/Bip21UtilsTests.swift b/BitkitTests/Bip21UtilsTests.swift index 9b3916c80..5897c5ebb 100644 --- a/BitkitTests/Bip21UtilsTests.swift +++ b/BitkitTests/Bip21UtilsTests.swift @@ -29,51 +29,4 @@ final class Bip21UtilsTests: XCTestCase { "bitcoin:bcrt1qreceive?amount=0.00021&message=Hardware%20receive" ) } - - // MARK: - isDuplicatedBip21 Tests - - func testIsDuplicatedBip21_ReturnsFalseForSingleValidBIP21URI() { - let input = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=0.001&message=Bitkit" - XCTAssertFalse(Bip21Utils.isDuplicatedBip21(input)) - } - - func testIsDuplicatedBip21_ReturnsTrueWhenBIP21URIIsDuplicated() { - let first = "bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000002&message=Bitkit" - let second = "bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000003&message=Bitkit" - let input = first + second - XCTAssertTrue(Bip21Utils.isDuplicatedBip21(input)) - } - - func testIsDuplicatedBip21_HandlesCaseInsensitiveBitcoinPrefix() { - let first = "BITCOIN:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=0.001" - let second = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=0.002" - let input = first + second - XCTAssertTrue(Bip21Utils.isDuplicatedBip21(input)) - } - - func testIsDuplicatedBip21_ReturnsFalseForNonBitcoinURIs() { - let input = "lnbc500n1p3k9v3pp5kzmj..." - XCTAssertFalse(Bip21Utils.isDuplicatedBip21(input)) - } - - func testIsDuplicatedBip21_ReturnsFalseForEmptyString() { - XCTAssertFalse(Bip21Utils.isDuplicatedBip21("")) - } - - func testIsDuplicatedBip21_HandlesMixedCaseDuplicatedURIs() { - let first = "Bitcoin:bc1qaddr1?amount=0.001" - let second = "BITCOIN:bc1qaddr2?amount=0.002" - let input = first + second - XCTAssertTrue(Bip21Utils.isDuplicatedBip21(input)) - } - - func testIsDuplicatedBip21_ReturnsFalseForPlainBitcoinAddress() { - let input = "bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq" - XCTAssertFalse(Bip21Utils.isDuplicatedBip21(input)) - } - - func testIsDuplicatedBip21_ReturnsFalseForSingleBIP21WithLightningParam() { - let input = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?lightning=lnbc500n1p3k9v3pp5kzmj" - XCTAssertFalse(Bip21Utils.isDuplicatedBip21(input)) - } } diff --git a/BitkitTests/ClipboardPromptValidatorTests.swift b/BitkitTests/ClipboardPromptValidatorTests.swift index 18335a5d5..2d252f45a 100644 --- a/BitkitTests/ClipboardPromptValidatorTests.swift +++ b/BitkitTests/ClipboardPromptValidatorTests.swift @@ -89,13 +89,26 @@ final class ClipboardPromptValidatorTests: XCTestCase { } } - func testDuplicateBip21IsRejectedBeforeCandidateRecognition() async { - let supported = await ClipboardPromptValidator.isSupportedURI( - "bitcoin:bitcoin:invalid?lightning=lnurl1qqqqqq", ownPublicKey: nil, contacts: [] - ) { _ in - XCTFail("Duplicated BIP21 must be rejected before decoding") - return true + func testBip21ValidationUsesDecoderResult() async { + let uri = "bitcoin:mipcBbFg9gMiCh81Kj8tqqdgoZub1ZJRfn?message=bitcoin:donation" + for decoderResult in [false, true] { + var decodeCount = 0 + let supported = await ClipboardPromptValidator.isSupportedURI(uri, ownPublicKey: nil, contacts: []) { + decodeCount += 1 + XCTAssertEqual($0, uri) + return decoderResult + } + XCTAssertEqual(decodeCount, 1) + XCTAssertEqual(supported, decoderResult) } + } + + func testDuplicateBip21IsRejectedByCoreDecoder() async { + let first = "bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000002&message=Bitkit" + let second = "bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000003&message=Bitkit" + let supported = await ClipboardPromptValidator.isSupportedURI( + first + second, ownPublicKey: nil, contacts: [] + ) XCTAssertFalse(supported) } diff --git a/BitkitTests/ShopPaymentRequestTests.swift b/BitkitTests/ShopPaymentRequestTests.swift index d53d1a0fe..faaeb0020 100644 --- a/BitkitTests/ShopPaymentRequestTests.swift +++ b/BitkitTests/ShopPaymentRequestTests.swift @@ -4,6 +4,22 @@ import XCTest @MainActor final class ShopPaymentRequestTests: XCTestCase { + func testDecodingErrorsUseLocalizedScanToast() throws { + for error: DecodingError in [.InvalidFormat, .InvalidAmount, .InvalidAddress, .RequestFailed] { + let toast = try XCTUnwrap(ShopPaymentRequest.decodingErrorToast(for: error)) + XCTAssertEqual(toast.type, .error) + XCTAssertEqual(toast.title, t("other__scan_err_decoding")) + XCTAssertEqual(toast.description, t("other__scan__error__generic")) + XCTAssertEqual(toast.accessibilityIdentifier, "InvalidAddressToast") + } + } + + func testNonDecodingErrorsKeepExistingErrorHandling() { + for error in [CancellationError(), ScanHandlingError.unsupportedRequest, ScanHandlingError.pubkyAuthRequest] as [Error] { + XCTAssertNil(ShopPaymentRequest.decodingErrorToast(for: error)) + } + } + func testLightningInvoiceIsSupported() { XCTAssertTrue(ShopPaymentRequest.isSupported(.lightning(invoice: lightningInvoice))) } diff --git a/changelog.d/next/909.fixed.md b/changelog.d/next/909.fixed.md new file mode 100644 index 000000000..e01113ca0 --- /dev/null +++ b/changelog.d/next/909.fixed.md @@ -0,0 +1 @@ +Fixed payment URI validation to accept Bitcoin links in notes and reject ambiguous payment requests with localized errors instead of reopening a previous payment. diff --git a/journeys/send/README.md b/journeys/send/README.md new file mode 100644 index 000000000..3050ade96 --- /dev/null +++ b/journeys/send/README.md @@ -0,0 +1,9 @@ +# Send + +`bip21-core-validation.xml` is shared with Android, with identical actions and fixtures. It requires a released bitkit-core SDK containing [PR #151](https://github.com/synonymdev/bitkit-core/pull/151); the current 0.5.18 dependency cannot satisfy its duplicate-key checks. Keep the adoption PR in draft until that dependency is bumped and the journey passes. + +Deliver each `Open bitcoin:...` action with `xcrun simctl openurl ""`, replacing XML `&` with `&` in the actual URI. Inspect errors immediately. Use XcodeBuildMCP snapshots for payment state and screenshots for transient error toasts. + +Use an onboarded regtest wallet with at least 100,000 sats in Savings and Quickpay disabled. The fixture address is not a destination to pay; close every payment sheet without confirming payment. + +The native-binding tests also cover labels, proof-of-payment aliases and question marks in notes. App tests cover manual entry and the Shop rejection path; a malformed `payment_intent` from the real third-party Shop cannot be generated through this journey environment. Automatic clipboard prompting additionally has a manual check because arbitrary OS clipboard injection is not listed in the Capabilities table. diff --git a/journeys/send/bip21-core-validation.xml b/journeys/send/bip21-core-validation.xml new file mode 100644 index 000000000..fc5a5f770 --- /dev/null +++ b/journeys/send/bip21-core-validation.xml @@ -0,0 +1,17 @@ + + Preconditions: an onboarded English-language regtest wallet with at least 100,000 sats in Savings, Quickpay disabled, and a bitkit-core release containing PR #151. Do not pay any fixture. Open each bitcoin: link through the platform's external-URI delivery command described in this suite's README. This checks that the native decoder rejects ambiguous payments while accepting bitcoin: text as metadata. + + Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.000035&message=bitcoin:donation + Verify Send shows the recipient bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk, an amount of 3,500 sats, and the note bitcoin:donation without a decoding error + Close Send without paying + Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000002&message=Bitkitbitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000003&message=Bitkit + Verify a decoding error appears and Send does not open with the previous payment + Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.000035&AMOUNT=0.00005 + Verify a decoding error appears and no payment sheet opens + Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?message=first&Message=second + Verify a decoding error appears and no payment sheet opens + Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.000035&custom=bitcoin:1BoatSLRHtKNngkdXEeobR76b53LETtpyT + Verify Send still shows bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk as the recipient and 3,500 sats as the amount + Close Send without paying + +