Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions app/src/androidTest/java/to/bitkit/services/Bip21DecodingTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package to.bitkit.services

import androidx.test.ext.junit.runners.AndroidJUnit4
import com.synonym.bitkitcore.Scanner
import com.synonym.bitkitcore.decode
import org.junit.Test
import org.junit.runner.RunWith
import to.bitkit.ext.runSuspendCatching
import to.bitkit.test.BaseAndroidTest
import to.bitkit.test.annotations.CoreServiceIntegration
import to.bitkit.test.annotations.DeviceIntegration
import kotlin.test.assertEquals
import kotlin.test.assertIs
import kotlin.test.assertTrue

@RunWith(AndroidJUnit4::class)
@DeviceIntegration
@CoreServiceIntegration
class Bip21DecodingTest : BaseAndroidTest() {
companion object {
/** Valid regtest recipient from the bitkit-core regression fixture. */
private const val ADDRESS = "bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk"
}

@Test
fun coreRejectsDuplicateSingletonParameters() = test {
val duplicatedUri = "bitcoin:$ADDRESS?amount=0.0000002&message=Bitkit" +
"bitcoin:$ADDRESS?amount=0.0000003&message=Bitkit"
for (uri in listOf(
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",
)) {
assertTrue(runSuspendCatching { decode(uri) }.isFailure, uri)
}
}

@Test
fun corePreservesBitcoinTextInQueryMetadata() = test {
for ((key, value) in listOf(
"message" to "bitcoin:donation",
"label" to "BITCOIN:donation",
"custom" to "bitcoin:1BoatSLRHtKNngkdXEeobR76b53LETtpyT",
"message" to "Why?",
)) {
val decoded = assertIs<Scanner.OnChain>(decode("bitcoin:$ADDRESS?amount=0.000035&$key=$value"))
assertEquals(ADDRESS, decoded.invoice.address)
assertEquals(3500uL, decoded.invoice.amountSatoshis)
assertEquals(value, decoded.invoice.params?.get(key))
}
}
}
16 changes: 0 additions & 16 deletions app/src/main/java/to/bitkit/utils/Bip21Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,6 @@ import to.bitkit.models.SATS_IN_BTC

object Bip21Utils {

private const val BIP21_PREFIX = "bitcoin:"

/**
* Checks if a BIP21 URI is duplicated (contains multiple bitcoin: prefixes).
* Workaround for https://github.com/synonymdev/bitkit-core/issues/63
* @return true if the input contains duplicated BIP21 URIs, false otherwise
*/
fun isDuplicatedBip21(input: String): Boolean {
val lowercased = input.lowercase()
val firstIndex = lowercased.indexOf(BIP21_PREFIX)
if (firstIndex == -1) return false

val secondIndex = lowercased.indexOf(BIP21_PREFIX, firstIndex + BIP21_PREFIX.length)
return secondIndex != -1
}

fun buildBip21Url(
bitcoinAddress: String,
amountSats: ULong? = null,
Expand Down
23 changes: 0 additions & 23 deletions app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ import to.bitkit.ui.utils.localizedPubkyAuthMessage
import to.bitkit.usecases.FormatMoneyValue
import to.bitkit.usecases.RefreshContactPaykitLinkUseCase
import to.bitkit.utils.AppError
import to.bitkit.utils.Bip21Utils
import to.bitkit.utils.Logger
import to.bitkit.utils.NetworkValidationHelper
import to.bitkit.utils.ServiceError
Expand Down Expand Up @@ -2441,16 +2440,6 @@ class AppViewModel @Inject constructor(
}

private suspend fun validateAddressWithFeedback(input: String) = withContext(bgDispatcher) {
// TODO Workaround for https://github.com/synonymdev/bitkit-core/issues/63
if (Bip21Utils.isDuplicatedBip21(input)) {
showAddressValidationError(
titleRes = R.string.other__scan_err_decoding,
descriptionRes = R.string.other__scan__error__generic,
testTag = "DuplicatedBip21Toast",
)
return@withContext
}

val scanResult = runCatching { coreService.decode(input.removeLightningSchemes()) }

if (scanResult.isFailure) {
Expand Down Expand Up @@ -3361,18 +3350,6 @@ class AppViewModel @Inject constructor(

val fromMainScanner = isMainScanner

// TODO Workaround for https://github.com/synonymdev/bitkit-core/issues/63
if (Bip21Utils.isDuplicatedBip21(input)) {
if (clearIncomingPaymentRequestTarget()) return@withContext
toast(
type = Toast.ToastType.ERROR,
title = context.getString(R.string.other__scan_err_decoding),
description = context.getString(R.string.other__scan__error__generic),
testTag = "DuplicatedBip21Toast",
)
return@withContext
}

SamRockSetupRequest.parse(input)?.let {
handleSamRockSetup(it)
return@withContext
Expand Down
44 changes: 0 additions & 44 deletions app/src/test/java/to/bitkit/utils/Bip21UrlBuilderTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.JUnit4
import to.bitkit.utils.Bip21Utils.buildBip21Url
import to.bitkit.utils.Bip21Utils.isDuplicatedBip21

@RunWith(JUnit4::class)
class Bip21UrlBuilderTest {
Expand Down Expand Up @@ -173,49 +172,6 @@ class Bip21UrlBuilderTest {
Assert.assertEquals(expected, buildBip21Url(address, amount, lightningInvoice = invoice))
}

// Tests for isDuplicatedBip21 - Workaround for bitkit-core#63

@Test
fun `isDuplicatedBip21 returns false for single valid BIP21 URI`() {
val input = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=0.001&message=Bitkit"
Assert.assertFalse(isDuplicatedBip21(input))
}

@Test
fun `isDuplicatedBip21 returns true when BIP21 URI is duplicated`() {
val first = "bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000002&message=Bitkit"
val second = "bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000003&message=Bitkit"
val input = first + second
Assert.assertTrue(isDuplicatedBip21(input))
}

@Test
fun `isDuplicatedBip21 handles case-insensitive bitcoin prefix`() {
val first = "BITCOIN:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=0.001"
val second = "bitcoin:bc1qar0srrr7xfkvy5l643lydnw9re59gtzzwf5mdq?amount=0.002"
val input = first + second
Assert.assertTrue(isDuplicatedBip21(input))
}

@Test
fun `isDuplicatedBip21 returns false for non-bitcoin URIs`() {
val input = "lnbc500n1p3k9v3pp5kzmj..."
Assert.assertFalse(isDuplicatedBip21(input))
}

@Test
fun `isDuplicatedBip21 returns false for empty string`() {
Assert.assertFalse(isDuplicatedBip21(""))
}

@Test
fun `isDuplicatedBip21 handles mixed case duplicated URIs`() {
val first = "Bitcoin:bc1qaddr1?amount=0.001"
val second = "BITCOIN:bc1qaddr2?amount=0.002"
val input = first + second
Assert.assertTrue(isDuplicatedBip21(input))
}

// Tests for empty/blank message and label handling - Issue #746

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2196,11 +2196,12 @@ class AppViewModelSendFlowTest : BaseUnitTest() {
}

@Test
fun `duplicated bip21 request target leaves terminal feedback visible`() = test {
fun `core rejected bip21 request target leaves terminal feedback visible`() = test {
sut.setIsAuthenticated(true)
val request = paymentRequest()
val first = "bitcoin:bcrt1qfirst?amount=0.00000001"
val duplicatedBip21 = first + "bitcoin:bcrt1qsecond?amount=0.00000001"
whenever(coreService.decode(duplicatedBip21)).doSuspendableAnswer { throw AppError("Invalid payment URI") }
whenever(context.getString(R.string.wallet__payment_request)).thenReturn("Payment Request")
whenever(context.getString(R.string.wallet__payment_request_waiting_for_details)).thenReturn("Waiting")
whenever(context.getString(R.string.wallet__payment_request_unavailable)).thenReturn(
Expand Down Expand Up @@ -6649,6 +6650,65 @@ class AppViewModelSendFlowTest : BaseUnitTest() {
assertEquals("InsufficientSavingsToast", toastCaptor.lastValue.testTag)
}

@Test
fun `manual bip21 with bitcoin text delegates to core and stays valid`() = test {
val uri = "bitcoin:$REGTEST_ADDRESS?amount=0.000035&message=bitcoin:donation"
stubBip21Scan(uri)

sut.setSendEvent(SendEvent.AddressChange(uri))
advanceUntilIdle()

verify(coreService).decode(uri)
assertTrue(sut.sendUiState.value.isAddressInputValid)
assertTrue(sut.canDecodeClipboard(uri))
}

@Test
fun `manual bip21 rejected by core stays invalid`() = test {
val uri = "bitcoin:$REGTEST_ADDRESS?amount=0.000035&amount=0.00005"
whenever(coreService.decode(uri)).doSuspendableAnswer { throw AppError("Invalid payment URI") }
clearInvocations(toastManager)

sut.setSendEvent(SendEvent.AddressChange(uri))
advanceUntilIdle()

verify(coreService).decode(uri)
assertFalse(sut.sendUiState.value.isAddressInputValid)
verify(toastManager).enqueue(check { assertEquals("InvalidAddressToast", it.testTag) })
}

@Test
fun `scanned bip21 with bitcoin text reaches send state`() = test {
val uri = "bitcoin:$REGTEST_ADDRESS?amount=0.000035&message=bitcoin:donation"
stubBip21Scan(uri)
sut.setIsAuthenticated(true)

sut.onScanResult(uri)
advanceUntilIdle()

verify(coreService).decode(uri)
assertEquals(REGTEST_ADDRESS, sut.sendUiState.value.address)
assertEquals(uri, sut.sendUiState.value.addressInput)
assertEquals(3500uL, sut.sendUiState.value.amount)
}

@Test
fun `scanned concatenated bip21 rejected by core clears previous payment`() = test {
val first = "bitcoin:$REGTEST_ADDRESS?amount=0.000035&message=Bitkit"
val uri = first + "bitcoin:$REGTEST_ADDRESS?amount=0.00005&message=Bitkit"
whenever(coreService.decode(uri)).doSuspendableAnswer { throw AppError("Invalid payment URI") }
setSendState(SendUiState(address = REGTEST_ADDRESS, amount = 3500uL))
sut.setIsAuthenticated(true)

sut.onScanResult(uri)
advanceUntilIdle()

verify(coreService).decode(uri)
assertEquals("", sut.sendUiState.value.address)
assertEquals(0uL, sut.sendUiState.value.amount)
assertNull(sut.currentSheet.value)
}

@Test
fun `manual input of unified own lightning invoice with no savings shows self payment toast`() = test {
val bolt11 = "lnbcrt1ownunifiedmanual"
Expand Down Expand Up @@ -10326,6 +10386,28 @@ class AppViewModelSendFlowTest : BaseUnitTest() {
stubLightningScan(bolt11 = bolt11, amountSats = amountSats, payeeNodeId = payeeNodeId)
}

private suspend fun stubBip21Scan(uri: String) {
balanceState.value = BalanceState(maxSendOnchainSats = 100_000uL)
whenever { coreService.decode(uri) }.thenReturn(
Scanner.OnChain(
OnChainInvoice(
address = REGTEST_ADDRESS,
amountSatoshis = 3500uL,
label = null,
message = "bitcoin:donation",
params = mapOf("message" to "bitcoin:donation"),
)
)
)
whenever(coreService.validateBitcoinAddress(REGTEST_ADDRESS)).thenReturn(
ValidationResult(
address = REGTEST_ADDRESS,
network = NetworkType.REGTEST,
addressType = AddressType.P2WPKH,
)
)
}

private fun nonOnchainPaymentScans() = listOf(
"lnbcrt1hardware" to Scanner.Lightning(lightningInvoice("lnbcrt1hardware", 1_000uL)),
"lnurl1hardware" to Scanner.LnurlPay(
Expand Down
1 change: 1 addition & 0 deletions changelog.d/next/1454.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed payment URI validation to accept Bitcoin links in notes while rejecting ambiguous payment requests through the shared decoder.
11 changes: 11 additions & 0 deletions journeys/send/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Send

`bip21-core-validation.xml` is shared with iOS, 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 `adb shell "am start -a android.intent.action.VIEW -d '<uri>' to.bitkit.dev"`, replacing XML `&amp;` with `&` in the actual URI. Inspect errors immediately. Use `android layout` for payment state and screenshots for transient error toasts, which the semantic dump does not expose.

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 payment-request failure handling. Automatic clipboard prompting additionally has a manual check because arbitrary OS clipboard injection is not listed in the Capabilities table. Do not type long URI fixtures with `adb shell input text`, which can truncate them.

`own-invoice-guard.xml` covers the existing Lightning self-payment guard and is unchanged by this adoption.
17 changes: 17 additions & 0 deletions journeys/send/bip21-core-validation.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<journey name="bip21 core validation">
<description>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.</description>
<actions>
<action>Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.000035&amp;message=bitcoin:donation</action>
<action>Verify Send shows the recipient bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk, an amount of 3,500 sats, and the note bitcoin:donation without a decoding error</action>
<action>Close Send without paying</action>
<action>Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000002&amp;message=Bitkitbitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.0000003&amp;message=Bitkit</action>
<action>Verify a decoding error appears and Send does not open with the previous payment</action>
<action>Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.000035&amp;AMOUNT=0.00005</action>
<action>Verify a decoding error appears and no payment sheet opens</action>
<action>Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?message=first&amp;Message=second</action>
<action>Verify a decoding error appears and no payment sheet opens</action>
<action>Open bitcoin:bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk?amount=0.000035&amp;custom=bitcoin:1BoatSLRHtKNngkdXEeobR76b53LETtpyT</action>
<action>Verify Send still shows bcrt1qr289x0fhg62672e8urudfnxnsr8tcax64xk2vk as the recipient and 3,500 sats as the amount</action>
<action>Close Send without paying</action>
</actions>
</journey>
Loading