Repository navigation
fix: use one btc/usd denomination toggle - #1654
Conversation
 [Medium impact] The change reaches payment-request and subscription amount entry. The PR appears safe to merge, with a non-blocking accessibility cleanup for the toggle. Findings
SummaryReplaces separate BTC and USD buttons with one filled denomination toggle in payment-request and subscription amount entry.
One non-blocking accessibility issue remains: the swap icon repeats the denomination label.
The fixed BTC label is intentional: Reviews (1) · Last reviewed commit: "fix: use one btc/usd denomination toggle" · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Regtest APKDownload bitkit-dev-debug universal APK (expires in 30 days). |
talosmachina
left a comment
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Replaces the separate BTC and USD denomination buttons with one toggle that shows the current denomination and converts to the next allowed asset. Reviewed 60f0473, light tier.
What I checked, and 4 candidates I ruled out
Read in full: PaymentRequestDenominationToggle and PaymentRequestAmountContent in CreatePaymentRequestScreen.kt, NumberPadActionButton.kt, the allowedAssets caller in CreateSubscriptionScreen.kt, both journey diffs.
Call sites traced: PaymentRequestAmountContent (request screen, subscription amount step, CreatePaymentRequestScreenTest, PaymentRequestAmountLocaleTest).
CI: build, build-local and Greptile green; lint and the e2e shards were still pending at review time.
Not built here: Bitkit Android branches are reviewed from the code and CI, not run on this side.
Ruled out
- Removed
PaymentRequestAssetBTC/PaymentRequestAssetUSDtags still referenced: nothing inapp/src/androidTest,app/src/test,journeys/orbitkit-e2e-testsmain uses them. - BTC-only subscription still switches: with
allowedAssets = [BTC],nextAssetis null, soclickable = falsedrops the click and no swap icon renders, which is what J2 asserts. - Current asset outside
allowedAssetsgets stuck: if a USD draft opens with only BTC allowed,nextAssetresolves to BTC and the one tap converts through the sameconversion.convertpath as before. Then the label reads BTC with no icon. - Double TalkBack read of the label: already raised inline by Greptile, so not repeated here.
Merge confidence: 4/5, no automated test drives the toggle (both compose tests skip it), and lint and e2e were still running.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Re-reviewed the full PR diff against its merge base, at da64f8bb, after an explicit full reassessment. No earlier coverage was inherited.
1 actionable finding — resolve or provide an evidence-backed rebuttal.
The control shows the current asset and converts to the other allowed asset through the existing PaykitAmountConversion path on payment-request and subscription amount entry. An unedited amount round-trips from the converter's saved original; PaykitAmountTest asserts that for 10 USD. The Bitcoin/fiat unit control and accepted-method chips stay in place. Only the denomination toggle was compared on bitkit-ios#1078 at 9f34b932: both sides switch to the other allowed asset, use an orange label, and keep a fixed BTC label with no swap icon when USD is unavailable.
At this revision, leading icons in NumberPadActionButton use a null content description, so the visible caption is the accessible name. The earlier duplicate-label note is in this thread. TalkBack was not run; manual case 1 is still open.
Validation: CI build for this revision succeeded, and that build job runs testDevDebugUnitTest. This review inspected the workflow and PaykitAmountTest and did not re-run them. bitkit-e2e-tests was not readable (GitHub 404). This repo has no remaining PaymentRequestAssetBTC or PaymentRequestAssetUSD references. Device testing: not performed in this review. J1, J2, and manual case 1 were not executed.
Suggested additional test cases
- Android, using the PR's two linked Paykit wallets and a fresh BTC/USD rate. Enter a non-zero USD amount, then use a missing or stale rate and tap the denomination control. The label and amount stay on USD, and the rate-unavailable message is shown.
- Android, same setup. Enter 10 USD, switch to BTC, change the satoshi amount, then switch back to USD. The USD value follows the edited satoshi amount.
Reviewer model: grok-4.7
Reasoning effort: Not exposed
Run metadata records originalModelName and does not include a reasoning setting.
Findings
- [LOW] Quick second tap does not switch the denomination back — inline at
app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt:295.
|
Milestone 3.1.0: this fixes the BTC/USD selector part of #1533, which is on 3.1.0; the layout part stays tracked there. |
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: ⛔️ Request changes at da64f8b — the quick reverse tap is ignored.
The PR moved to 427e35c after this run; these results are for da64f8b.
Findings: One existing LOW finding confirmed with device evidence; no additional findings.
At app/src/main/java/to/bitkit/ui/screens/paymentrequests/CreatePaymentRequestScreen.kt:295, the single control retains its 500 ms debouncer after changing denomination. Two quick taps leave BTC selected instead of restoring USD; a later tap works. I confirmed the existing finding in both amount editors. Allow the immediate reverse conversion.
QA for da64f8b: Independently built the pinned source and drove two Android 16 / API 36 Pixel 7 emulators with linked regtest wallets. Recordings play at 2×; paired recordings show the creator on the left and payer on the right. No payment or subscription was signed.
Test 1: TalkBack denomination label ✅ Each denomination is spoken once per focus in both editors and BTC-only mode.
Actual Android speech, speech-display output and TTS diagnostics confirmed one USD/BTC announcement per focus. The fixed BTC label is announced once without a swap icon.
TalkBack speech and focus:
talkback.2x.mp4
Test 2: Immediate reverse tap ❌ The second tap is discarded in request and subscription amount entry.
Starting from 10 USD, both tap pairs completed within 280 ms and left BTC selected. A later tap restored 10,00 USD.
Request first, subscription second:
quick-toggle.2x.mp4
Test 3: Stale rate ✅ Freshly entered 10 USD stays unchanged and shows the rate-unavailable error.
The device clock was advanced beyond the rate’s ten-minute validity, then restored after the check.
Stale-rate rejection:
stale-rate.2x.mp4
Test 4: Edit converted Bitcoin amount ✅ Editing the converted value updates USD instead of restoring the old source amount.
After converting 10 USD, entering 20,000 sats returned 16,68 USD at the current rate.
Edited amount conversion:
edited-bitcoin.2x.mp4
Test J1: Request and subscription denomination ⏭️ Changed scenarios pass at normal tap intervals; the full journey remains incomplete.
Four request round trips restored 10,00 USD. 150,50 USD survived request and subscription review and delivery. Clearing disabled Continue; trailing 5, reviewed as 5,00 USD.
The USDT-only contact branch and stored-payload comparison remain unverified. The optional Savings/Spending confirmation lacked a usable channel. Direct USDT Send, Receive and contact comma entry were checked separately.
| Amount entry | Request delivery | Subscription delivery |
|---|---|---|
j1-amounts.2x.mp4 |
j1-request-delivery.2x.mp4 |
j1-subscription-delivery.2x.mp4 |
Test J2: BTC-only subscription ✅ Fixed BTC, separate input units and the 5,000-sat proposal work.
With USDT disabled, BTC stayed fixed without an icon. Large-text keyboard checks kept Name and Description reachable. Removing Savings disabled Choose Recipient; the 600-character description was rejected, and shortening it delivered the monthly 5,000-sat Savings-only proposal.
| BTC-only form and validation | Proposal delivery |
|---|---|
j2-btc-only.2x.mp4 |
j2-proposal-delivery.2x.mp4 |
Twin: synonymdev/bitkit-ios#1078 fix: use one btc/usd denomination toggle matches normal-toggle and fixed-BTC behavior by source comparison and its device report. The Android quick-tap debounce differs.
Full review: ae800ae..da64f8b, five files. Both amount editors were driven; remaining shared-button callers were reviewed in source. The automated suite was not rerun here. Figma frames were unavailable for a pixel comparison.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Full reassessment of the complete PR diff against merge base ae800aef, at 427e35c. No earlier coverage baseline was supplied.
No new actionable code findings.
The filled control replaces the separate BTC and USD buttons in payment-request and subscription amount entry. It shows the current denomination, converts through the existing PaykitAmountConversion anchor, and uses a zero debounce so a return tap is accepted after the label updates. Other NumberPadActionButton callers keep the 500 ms debounce. With only Bitcoin allowed, the control stays a filled BTC label, omits the swap icon, and does not change the amount. Leading icons on these action buttons are decorative; the visible caption remains the accessible name. Continue still submits the amount shown for the selected denomination.
Compared with twin bitkit-ios#1078 at 9f34b932: same control id, orange label, conversion anchor, and journey wording. iOS places the control on the trailing edge beside the unit button. Android keeps the existing leading placement, which this PR leaves to #1533.
Non-blocking consistency suggestions
- BTC-only amount entry: Android leaves the filled control at full opacity and ignores taps. iOS marks that control disabled, so the system dims it. Both omit the swap icon and stay on BTC. J2 does not require dimming; match the opacity only if the two apps should look the same in that state.
Validation: Gradle, Compose tests, and device journeys were not run in this review. PaykitAmountTest asserts that repeated BTC/USD toggles restore an unedited 10 USD and that a changed amount is reconverted. CreatePaymentRequestScreenTest and PaymentRequestAmountLocaleTest do not click PaymentRequestDenominationToggle. Journeys paykit-quoted-usdt.xml and subscriptions/create-and-propose.xml now use that id. Appium checkout synonymdev/bitkit-e2e-tests at bcf91fee has no denomination-toggle selector.
Device testing: not performed in this review.
Ready for device testing. Run J1, J2, and manual cases 1–2. Case 2 is the return tap within 500 ms after the label changes.
Earlier findings reconciled at this revision: 2 verified fixed.
There was a problem hiding this comment.
Verdict: ⛔️ Request changes — J1’s funded Android navigation check fails its written expectation.
Findings: No new defects in the debounce delta. The rapid reverse-tap finding is fixed.
QA for 427e35c: Two Android 16 / API 36 Pixel 7 emulators with linked regtest wallets and a comma-decimal locale. Recordings play at 2×. No payment or subscription was signed.
Test 2: Immediate reverse denomination tap ✅ Both editors restore exactly 10,00 USD within 500 ms.
On the unmodified APK, the two real taps completed within 327 ms in Request Payment and 377 ms in subscription amount entry. Each recording shows the intermediate BTC value and restored USD amount; timing comes from the Android input commands’ wall-clock bounds.
| Request Payment | Subscription |
|---|---|
rapid-request.2x.mp4 |
rapid-subscription.2x.mp4 |
Test J1-contact: USDT-only contact ✅ USDT-only entry shows AVAILABLE and matches direct USDT Send typography.
The receiving device’s private-list cache contains only usdt-arbitrum-address. Bitcoin sources are disabled; 0,01, the balance, USDT label and Continue use the same typography as direct USDT Send. Continue remains visible. The unpublished test hook below controls only the peer’s published list.
| USDT-only contact | Direct USDT Send |
|---|---|
usdt-only.2x.mp4 |
direct-usdt.2x.mp4 |
Test J1-payload: Cents and stored precision ✅ Formatting and reopening preserve the SDK amounts 150.5 USD and 5.123456 USDT.
150,50 survives request review, delivery and receiving USDT review/reopening. The controlled six-decimal request keeps 5,123456 in amount entry, including after a quote error; a healthy retry reaches confirmation. SDK records on both wallets retain the same asset and numeric value before and after formatting/reopening. Confirmation summaries retain their existing two-decimal display.
The unpublished hook supplies the six-decimal request shape and logs the real SDK fields. It does not change the receiver’s amount or confirmation decisions. Paired videos align sender and receiver in time.
| USD delivery | USD reopened |
|---|---|
payload-usd.2x.mp4 |
payload-usd-reopen.2x.mp4 |
| Six-decimal delivery | Six-decimal reopened |
|---|---|
payload-six.2x.mp4 |
payload-six-reopen.2x.mp4 |
Test J1-methods: Funded Savings/Spending switch ❌ Confirmation stays open where J1 expects Amount to reopen.
With both Bitcoin sources funded and usable Lightning channels, one tap switches Savings to Spending while keeping confirmation open at 11,827 sats. Opening Amount manually and pressing its Back arrow closes the sheet without returning to confirmation.
reviewFundingSourceChange rebinds valid Bitcoin terms in place. That implementation is byte-identical to the base, so this is an existing mismatch with the journey rather than a regression attributed to the denomination toggle. Could the Android journey expectation be reconciled with the intended behavior before J1 is marked complete?
The test hook opens a real native peer channel; navigation and payment decisions are unchanged.
methods.2x.mp4
The earlier device report proves TalkBack, J2, stale-rate rejection, edited-satoshi conversion and the completed J1 branches at da64f8b. Those paths are untouched by this delta; they were not repeated. Full J1 remains unpassed because of the navigation assertion above.
Twin: synonymdev/bitkit-ios#1078 fix: use one btc/usd denomination toggle at 9f34b93 matches the denomination behavior by source comparison, including immediate reverse taps.
Reaudit: da64f8b to 427e35c, two files. Both denomination editors were driven; the default debounce of other shared-button callers was checked in source. APK builds passed; the automated suite and lint were not rerun. Figma pixel comparison remains unverified because the linked frames were inaccessible.
Unpublished J1 test hook
Debug-only fixture controls publish a USDT-only private list, send a six-decimal request, log SDK amount fields, and open/close a real native peer channel. No receiving conversion or navigation decisions are replaced.
diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 00c57a69a..c4177d5c1 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -14,6 +14,8 @@ import dagger.hilt.components.SingletonComponent
import kotlinx.serialization.Serializable
import kotlinx.serialization.json.Json
import to.bitkit.async.ServiceQueue
+import org.lightningdevkit.ldknode.PeerDetails // test hook
+import to.bitkit.ext.of // test hook
import to.bitkit.models.msatCeilOf
import to.bitkit.repositories.LightningRepo
import to.bitkit.repositories.ProbeOutcome
@@ -63,6 +65,7 @@ private sealed interface DevCommand {
companion object {
fun parse(method: String, arg: String?): DevCommand? = when (method) {
+ QaPeer.METHOD -> QaPeer.parse(arg) // test hook
CreateInvoice.METHOD -> CreateInvoice.parse(arg)
ProbeInvoice.METHOD -> ProbeInvoice.parse(arg)
ProbeNode.METHOD -> ProbeNode.parse(arg)
@@ -74,6 +77,27 @@ private sealed interface DevCommand {
suspend fun execute(deps: DevToolsProvider.Dependencies): DevResult
+ data class QaPeer(val args: Args) : DevCommand { // test hook
+ companion object { // test hook
+ const val METHOD = "qaPeer" // test hook
+ fun parse(arg: String?) = QaPeer(arg.deserialize<Args>()) // test hook
+ } // test hook
+ @Serializable data class Args(val uri: String, val action: String = "open") // test hook
+ override suspend fun execute(deps: DevToolsProvider.Dependencies): DevResult { // test hook
+ val repo = deps.lightningRepo() // test hook
+ val peer = PeerDetails.of(args.uri) // test hook
+ if (args.action == "open") { // test hook
+ val opened = repo.openChannel(peer, 500000u, 250000u).getOrThrow() // test hook
+ Logger.info("QA_PEER opened $opened", context = TAG) // test hook
+ } else if (args.action == "close") { // test hook
+ repo.lightningState.value.channels.filter { it.counterpartyNodeId == peer.nodeId }.forEach { repo.closeChannel(it, force = true).getOrThrow() } // test hook
+ } else { // test hook
+ repo.lightningState.value.channels.forEach { Logger.info("QA_CHANNEL $it", context = TAG) } // test hook
+ } // test hook
+ return DevResult.Ack() // test hook
+ } // test hook
+ } // test hook
+
data class CreateInvoice(val args: Args) : DevCommand {
companion object {
const val METHOD = "createInvoice"
diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt
index 430448a93..1930f9816 100644
--- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt
@@ -66,6 +66,7 @@ import to.bitkit.services.isBitkitPaymentRequest
import to.bitkit.utils.AppError
import to.bitkit.utils.Logger
import to.bitkit.utils.SubscriptionIcon
+import java.io.File
import java.math.BigDecimal
import java.util.UUID
import java.util.concurrent.atomic.AtomicLong
@@ -782,10 +783,12 @@ class PaykitPaymentRequestRepo @Inject constructor(
currencyRepo.currencyState.value.paykitRate,
proposalDate
)
+ val fixtureAmount = if (Env.isDebug && File(Env.bitkitCoreStoragePath(0), "qa-six-decimal").exists()) PaykitAmount.parse(PaykitAsset.USDT, "5.123456") else draft.amount // test hook
+ if (fixtureAmount != draft.amount) Logger.info("QA_FIXTURE proposing exact six-decimal USDT", context = TAG) // test hook
return PaykitPaymentRequestProposalTerms(
- amountValue = draft.amount.value,
- amountAsset = draft.amount.asset.code,
- conversion = rates.takeIf { it.isNotEmpty() }?.let(PaymentConversion::Fixed),
+ amountValue = fixtureAmount.value, // test hook
+ amountAsset = fixtureAmount.asset.code, // test hook
+ conversion = if (fixtureAmount.asset == PaykitAsset.USDT) null else rates.takeIf { it.isNotEmpty() }?.let(PaymentConversion::Fixed), // test hook
paymentDeadline = PaymentDeadline.At(draft.expiresAt.toString()),
paymentReference = "bitkit-${UUID.randomUUID()}",
proposalExpiresAt = draft.expiresAt.toString(),
@@ -918,10 +921,12 @@ class PaykitPaymentRequestRepo @Inject constructor(
iconUri?.let { subscriptionMetadata["icon_uri"] = JsonPrimitive(it) }
val rates = PaykitRequestPricing.rates(draft.amount.asset, endpointIdentifiers, null, startsAt)
val timestamp = Instant.fromEpochSeconds(startsAt.epochSeconds).toString()
+ val fixtureAmount = if (Env.isDebug && File(Env.bitkitCoreStoragePath(0), "qa-six-decimal").exists()) PaykitAmount.parse(PaykitAsset.USDT, "5.123456") else draft.amount // test hook
+ if (fixtureAmount != draft.amount) Logger.info("QA_FIXTURE proposing exact six-decimal USDT", context = TAG) // test hook
return PaykitPaymentRequestProposalTerms(
- amountValue = draft.amount.value,
- amountAsset = draft.amount.asset.code,
- conversion = rates.takeIf { it.isNotEmpty() }?.let(PaymentConversion::Fixed),
+ amountValue = fixtureAmount.value, // test hook
+ amountAsset = fixtureAmount.asset.code, // test hook
+ conversion = if (fixtureAmount.asset == PaykitAsset.USDT) null else rates.takeIf { it.isNotEmpty() }?.let(PaymentConversion::Fixed), // test hook
paymentReference = "bitkit-${UUID.randomUUID()}",
proposalExpiresAt = draft.expiresAt.toString(),
acceptedPaymentEndpointIdentifiers = endpointIdentifiers,
@@ -1906,6 +1911,7 @@ private fun PaymentRequestRecord.parsePaykitPaymentRequest(
}
val requestTerms = terms
?: return PaykitPaymentRequestParseResult.Rejected(PaykitPaymentRequest.ParseFailure.MissingTerms)
+ if (Env.isDebug) Logger.info("QA_PAYLOAD id=$paymentRequestId asset=${requestTerms.amount.asset} value=${requestTerms.amount.value} endpoints=${requestTerms.acceptedPaymentEndpointIdentifiers}", context = "PayloadCheck") // test hook
if (requestTerms.recurrence != null) {
return PaykitPaymentRequestParseResult.Rejected(
if (toPaykitSubscription() != null) {
diff --git a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
index 3670e42bb..39a8788c2 100644
--- a/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PrivatePaykitRepo.kt
@@ -1743,7 +1743,10 @@ class PrivatePaykitRepo @Inject constructor(
}
}
- endpoints
+ if (to.bitkit.env.Env.isDebug && App.currentActivity?.value?.filesDir?.resolve("qa-usdt-only")?.exists() == true) { // test hook
+ Logger.info("QA_ENDPOINTS USDT-only private list", context = TAG) // test hook
+ endpoints.filter { it.methodId == MethodId.UsdtArbitrum } // test hook
+ } else endpoints // test hook
}
}
diff --git a/app/src/main/java/to/bitkit/services/LightningService.kt b/app/src/main/java/to/bitkit/services/LightningService.kt
index eeddf093e..15525ee5b 100644
--- a/app/src/main/java/to/bitkit/services/LightningService.kt
+++ b/app/src/main/java/to/bitkit/services/LightningService.kt
@@ -309,6 +309,7 @@ class LightningService internal constructor(
.map { it.toLdkAddressType() }
val builder = Builder.fromConfig(config).apply {
+ if (Env.isDebug && File(Env.bitkitCoreStoragePath(0), "qa-listen").exists()) setListeningAddresses(listOf("0.0.0.0:9735")) // test hook
setCustomLogger(LdkLogWriter())
configureChainSource(customServerUrl)
configureGossipSource(customRgsServerUrl)Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest
|
Addressed the J1 expectation mismatch from this review in 3d01e7d. Android's base commit Documentation checks (XML, references, |
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: None. The revised Android journey now matches the existing valid-quote confirmation contract.
QA for 3d01e7d: Built the pinned source and drove two Android 16 / API 36 regtest wallets, with an Arbitrum test fixture. The builds used the unpublished input hooks shown below; payment conversion and navigation decisions were unchanged. Videos play at 2×. Paired videos show wallet A on the left and wallet B on the right. No payment or subscription was authorized.
Test J1-methods: Valid quoted Bitcoin switches ✅ Savings and Spending stay on Confirm with their own quoted amounts and refreshed fees.
A fixed 2.50 USD request binds Savings to 2,500 sats with a 141-sat fee, then Spending to 5,000 sats with the direct-channel zero-fee estimate, then Savings again. Payment still requires a swipe. Opening Amount manually and tapping Back closes the sheet.
methods.2x.mp4
Test J1-unavailable: Missing target terms ✅ A native funding-source tap returns to Amount when the target Bitcoin quote is unavailable.
The input hook removes the target Lightning rate after Savings review; the existing app handles the switch.
unavailable.2x.mp4
Test J1-expired: Expired target terms ✅ The switch returns to Amount with the rate-expired message and Continue disabled.
The target's controlled two-second deadline expires before the native source tap. The resulting input is 5,000 sats.
expired.2x.mp4
Test J1-minimum: Lightning minimum ✅ A 250-sat quote below the 1,000-sat LNURL minimum returns to Amount with Continue disabled.
The hook supplies the LNURL minimum and dispatches the existing method-switch event. This branch was event-driven: LNURL confirmation has no visible funding-source button. It does not replace the decision under test.
minimum.2x.mp4
Test J1-initial: Balance-based initial selection ✅ A request above the USDT balance starts on Bitcoin; a covered request starts on USDT.
With 25 USDT available, a 50 USD request starts on Bitcoin and retains manual USDT selection. A 2.50 USD request starts on USDT. These checks concern the requested amount; fee coverage is evaluated later.
initial.2x.mp4
Test J1-rails: Accepted methods and subscription conversions ✅ Independent method choices and exact amount round trips reach the payer correctly.
Both Bitcoin rails can be selected together. Removing all selected rails disables Choose Recipient; restoring Savings enables it. Four USDT/Savings round trips each restore exactly 12,345 sats; after editing, four round trips each restore exactly 10.00 USD.
sub-selections.2x.mp4
sub-btc-rounds.2x.mp4
sub-usd-rounds.2x.mp4
German request chips wrap at 600px without truncating their labels. A Savings-only 11,807-sat proposal reaches the payer with Savings as its sole method. A 12,345-sat request accepting Savings and USDT preserves both fixed quoted amounts and rejects edits. Spending remains disabled in the funding menu; tapping it does nothing.
request-layout.2x.mp4
rails-receive.2x.mp4
btc-request.2x.mp4
btc-disabled-spending.2x.mp4
Test J1-input: Denomination and input boundaries ✅ Repeated toggles restore 10 USD, and review preserves decimal input and trailing zeros.
Four BTC/USD round trips restore 10.00 USD each time. Empty BTC disables Continue; entering 5, reviews as 5,00. Entering 150,50, reviewing and reopening preserves the cents and trailing zero. Removing every receiving method disables Send; restoring USDT enables it.
request-rounds.2x.mp4
request-boundaries.2x.mp4
Test J1-delivery: USD request and subscription ✅ Actual 150.50 USD requests and monthly proposals preserve cents and USDT-only terms across linked wallets.
The sent request, outgoing and incoming cards, details, fixed USDT input and payment review preserve 150,50. A keypad tap cannot edit the requested amount. Reopening the unpaid request shows disabled teal Preparing, then USDT Amount; Back closes the sheet. German request and direct USDT entry both use “Betrag”.
usd-request.2x.mp4
usd-request-review.2x.mp4
usd-unpaid.2x.mp4
german-usdt.2x.mp4
The subscription editor restores exactly 150,50 after BTC/USD toggling and reopening. USDT is selected, description and icon controls are adjacent, and the proposal reaches both wallets at 150.50 USD per month. The payer's review offers only USDT and a teal subscribe swipe. Closed without subscribing.
sub-cents-form.2x.mp4
sub-cents-delivery.2x.mp4
sub-cents-review.2x.mp4
Test J1-direct: USDT comma entry ✅ Direct Send, Receive amount entry and contact payment each preserve 5,5 with a comma decimal key.
All three flows were closed without paying or sharing a deposit request.
direct-send.2x.mp4
direct-receive.2x.mp4
contact-usdt.2x.mp4
Check 1: Delta and retained coverage ✅ The two documentation changes agree with the existing source contract.
Reviewed the 427e35c to 3d01e7d diff, AppViewModel quote binding and manual Amount navigation, and the existing focused quoted-switch tests. Ancestry, XML parsing and whitespace checks passed. No runtime code changed in this delta; the automated suite and lint were not rerun.
Untouched, separately proved cases were retained:
- Earlier device report at da64f8b: TalkBack, BTC-only J2, stale-rate rejection and edited-satoshi conversion.
- Device report at 427e35c: immediate reverse taps, USDT-only contact entry, exact USD cents and six-decimal SDK payloads.
The former aggregate J1 result was incomplete; its remaining branches were driven above. Paid submission/history and broader USDT recovery tests are outside this selector journey. Figma pixel comparison remains unverified because the linked frames returned 403.
Twin: synonymdev/bitkit-ios#1078 fix: use one btc/usd denomination toggle at 9f34b93 matches the denomination behavior by source comparison. The journey now explicitly distinguishes Android's valid quoted-method rebinding from iOS's unchanged-terms requirement.
Unpublished input and native-channel test hooks
The debug provider supplies fixed request amounts/rates/deadlines, changes the active request's test terms, supplies an LNURL minimum and exposes the existing switch event. Its peer helper opens a real regtest channel; the debug listener makes the two test nodes reachable. Actual request/subscription creation and delivery use the normal app and SDK. No SDK result, payment result, receiving conversion or navigation decision is replaced.
diff --git a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
index 00c57a69a..ea9c78fc2 100644
--- a/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
+++ b/app/src/debug/java/to/bitkit/dev/DevToolsProvider.kt
@@ -1,5 +1,27 @@
+@file:OptIn(kotlin.time.ExperimentalTime::class)
+
package to.bitkit.dev
+import androidx.fragment.app.FragmentActivity
+import androidx.lifecycle.ViewModelProvider
+import com.synonym.paykit.ConversionRate
+import com.synonym.paykit.PaymentConversion
+import com.synonym.paykit.PaymentDeadline
+import to.bitkit.App
+import to.bitkit.models.PaykitAmount
+import to.bitkit.models.PaykitAsset
+import to.bitkit.models.PaykitRequestPricing
+import to.bitkit.repositories.Endpoint
+import to.bitkit.repositories.MethodId
+import to.bitkit.repositories.PaykitPaymentRequest
+import to.bitkit.viewmodels.AppViewModel
+import to.bitkit.viewmodels.ContactPaymentContext
+import org.lightningdevkit.ldknode.PeerDetails
+import to.bitkit.ext.of
+import kotlin.time.Clock
+import kotlin.time.Duration.Companion.seconds
+import java.io.File
+import java.util.UUID
import android.content.ContentProvider
import android.content.ContentValues
import android.net.Uri
@@ -18,7 +40,6 @@ import to.bitkit.models.msatCeilOf
import to.bitkit.repositories.LightningRepo
import to.bitkit.repositories.ProbeOutcome
import to.bitkit.utils.Logger
-import kotlin.time.Duration.Companion.seconds
import to.bitkit.repositories.ProbeReadiness as NodeProbeReadiness
private const val TAG = "DevToolsProvider"
@@ -42,6 +63,50 @@ class DevToolsProvider : ContentProvider() {
override fun call(method: String, arg: String?, extras: Bundle?): Bundle {
check(Binder.getCallingUid() == SHELL_UID) { "Only ADB shell callers are allowed" }
+ if (method == "qaUi") { // unpublished test hook: request fixture inputs only
+ val args = Json.decodeFromString<QaUi>(requireNotNull(arg))
+ val activity = App.currentActivity?.value as FragmentActivity
+ activity.runOnUiThread {
+ runCatching {
+ val vm = ViewModelProvider(activity)[AppViewModel::class.java]
+ if (args.action == "state") {
+ val state = vm.sendUiState.value
+ Logger.info("QA_STATE sheet=${vm.currentSheet.value} method=${state.payMethod} usdt=${state.paykitUsesUsdt} amount=${state.amount} usdtAmount=${state.paykitAmount} lightningFee=${state.lightningFeeSats} onchainFee=${state.onchainFeeUi} valid=${state.isAmountInputValid} confirm=${state.shouldConfirmPay}", context = TAG)
+ } else if (args.action == "switch") {
+ vm.setSendEvent(to.bitkit.viewmodels.SendEvent.PaymentMethodSwitch)
+ } else if (args.action == "minimum") {
+ val field = AppViewModel::class.java.getDeclaredField("_sendUiState").apply { isAccessible = true }
+ @Suppress("UNCHECKED_CAST")
+ val flow = field.get(vm) as kotlinx.coroutines.flow.MutableStateFlow<to.bitkit.viewmodels.SendUiState>
+ val lnurl = flow.value.lnurl as to.bitkit.viewmodels.LnurlParams.LnurlPay
+ flow.value = flow.value.copy(lnurl = lnurl.copy(data = lnurl.data.copy(minSendable = 1000000u)))
+ Logger.info("QA_METADATA LNURL minimum=1000 sats", context = TAG)
+ } else if (args.action == "terms") {
+ val field = AppViewModel::class.java.getDeclaredField("activeContactPaymentContext").apply { isAccessible = true }
+ val context = field.get(vm) as ContactPaymentContext
+ val request = requireNotNull(context.incomingPaymentRequest)
+ field.set(vm, context.copy(incomingPaymentRequest = request.copy(pricing = args.pricing())))
+ Logger.info("QA_TERMS ${args.pricing()}", context = TAG)
+ } else {
+ val endpoints = buildList {
+ add(Endpoint(MethodId.P2wpkh, args.address, rawPayload = args.address))
+ if (args.bolt11.isNotEmpty()) add(Endpoint(MethodId.Bolt11, args.bolt11, rawPayload = args.bolt11))
+ if (args.lnurl.isNotEmpty()) add(Endpoint(MethodId.Lnurl, args.lnurl, rawPayload = args.lnurl))
+ if (args.usdt.isNotEmpty()) add(Endpoint(MethodId.UsdtArbitrum, args.usdt, rawPayload = args.usdt))
+ }
+ val request = PaykitPaymentRequest(
+ paymentRequestId = UUID.randomUUID().toString(), counterparty = args.peer,
+ amount = PaykitAmount.parse(PaykitAsset.USD, args.usd),
+ paymentReference = "qa-quoted-request", pricing = args.pricing(),
+ expiresAt = null, acceptedPaymentEndpointIdentifiers = endpoints.map { it.methodId.rawValue },
+ )
+ Logger.info("QA_OPEN amount=${request.amount} pricing=${request.pricing} endpoints=${request.acceptedPaymentEndpointIdentifiers}", context = TAG)
+ vm.openContactPayment(args.address, args.peer, incomingPaymentRequest = request, endpoints = endpoints)
+ }
+ }.onFailure { Logger.error("Failed QA request fixture", it, context = TAG) }
+ }
+ return bundleOf("result" to "queued")
+ }
return runCatching {
val command = requireNotNull(DevCommand.parse(method, arg)) { "Unknown command: '$method'" }
ServiceQueue.LDK.blocking { command.execute(deps) }
@@ -63,6 +128,7 @@ private sealed interface DevCommand {
companion object {
fun parse(method: String, arg: String?): DevCommand? = when (method) {
+ QaPeer.METHOD -> QaPeer.parse(arg) // test hook
CreateInvoice.METHOD -> CreateInvoice.parse(arg)
ProbeInvoice.METHOD -> ProbeInvoice.parse(arg)
ProbeNode.METHOD -> ProbeNode.parse(arg)
@@ -74,6 +140,26 @@ private sealed interface DevCommand {
suspend fun execute(deps: DevToolsProvider.Dependencies): DevResult
+
+ data class QaPeer(val args: Args) : DevCommand {
+ companion object {
+ const val METHOD = "qaPeer"
+ fun parse(arg: String?) = QaPeer(arg.deserialize<Args>())
+ }
+ @Serializable data class Args(val uri: String = "", val action: String = "info")
+ override suspend fun execute(deps: DevToolsProvider.Dependencies): DevResult {
+ val repo = deps.lightningRepo()
+ if (args.action == "open") {
+ val opened = repo.openChannel(PeerDetails.of(args.uri), 500000u, 250000u).getOrThrow()
+ Logger.info("QA_PEER opened $opened", context = TAG)
+ } else if (args.action == "close") {
+ repo.lightningState.value.channels.forEach { repo.closeChannel(it, force = true).getOrThrow() }
+ }
+ Logger.info("QA_NODE ${repo.getNodeId()} channels=${repo.lightningState.value.channels}", context = TAG)
+ return DevResult.Ack()
+ }
+ }
+
data class CreateInvoice(val args: Args) : DevCommand {
companion object {
const val METHOD = "createInvoice"
@@ -295,3 +381,19 @@ private fun ProbeOutcome.toDevResult(paymentIds: Set<String>): DevResult = when
private inline fun <reified T> String?.deserialize(): T =
if (isNullOrBlank()) Json.decodeFromString("{}") else Json.decodeFromString(this)
+
+@Serializable private data class QaUi(
+ val action: String = "open", val address: String = "", val lnurl: String = "",
+ val usdt: String = "", val peer: String = "", val usd: String = "2.50", val bolt11: String = "",
+ val bitcoin: String = "0.00001", val lightning: String = "0.00002",
+ val deadlineSeconds: Long? = null,
+) {
+ fun pricing() = PaykitRequestPricing(
+ PaymentConversion.Fixed(buildList {
+ add(ConversionRate("btc-regtest", bitcoin))
+ if (lightning.isNotEmpty()) add(ConversionRate("btc-lightning", lightning))
+ add(ConversionRate("usdt", "1"))
+ }),
+ deadlineSeconds?.let { PaymentDeadline.At((Clock.System.now() + it.seconds).toString()) },
+ )
+}
diff --git a/app/src/main/java/to/bitkit/services/LightningService.kt b/app/src/main/java/to/bitkit/services/LightningService.kt
index eeddf093e..a87e81a77 100644
--- a/app/src/main/java/to/bitkit/services/LightningService.kt
+++ b/app/src/main/java/to/bitkit/services/LightningService.kt
@@ -309,6 +309,8 @@ class LightningService internal constructor(
.map { it.toLdkAddressType() }
val builder = Builder.fromConfig(config).apply {
+ if (Env.isDebug) setListeningAddresses(listOf("0.0.0.0:9735")) // unpublished test hook: native peer fixture
+
setCustomLogger(LdkLogWriter())
configureChainSource(customServerUrl)
configureGossipSource(customRgsServerUrl)Reviewed by gpt-6.1-sol with reasoning effort high; independent reviewer used the same model and effort.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up of 3d01e7de since 427e35c1, against merge base ae800aef. Inherited baseline: review 5601467975, covering the BTC/USD toggle and conversion failure paths, BTC-only subscription entry, the 500 ms number-pad debounce and decorative icons, the unchanged denomination and BTC-only journey wording, and the previously inspected amount tests. This pass reviewed the quoted Savings/Spending confirmation contract in paykit-quoted-usdt.xml and journeys/README.md, and its parity with bitkit-ios#1078 at 9f34b932.
No new actionable code findings.
The revised journey matches the Android send flow. A Savings/Spending switch that can bind valid quoted Bitcoin terms stays on Confirm, shows that method's quoted satoshi amount, updates the selected rail's fee state, and still requires a swipe. Unavailable or expired terms, or a Lightning amount below its minimum, open Amount. After a valid switch, opening Amount manually and tapping Back closes the sheet, because this Bitcoin-and-USDT request starts on Amount. iOS at the pinned twin stays on confirmation only when the recomputed terms are unchanged and still valid, and the Android text records that difference. The iOS journey and README at 9f34b932 still say Android returns to Amount whenever a quoted request changes method.
Non-blocking consistency suggestions
- Update that Android method-switch sentence in bitkit-ios#1078
journeys/paykit-quoted-usdt.xmlandjourneys/README.mdso a cross-platform run uses the contract this PR now states.
Validation: git diff --check passed and paykit-quoted-usdt.xml parses. The switch was traced through AppViewModel.onPaymentMethodSwitch, bindBitcoinRequestTerms, and SendSheet back handling, and compared with the inspected AppViewModelSendFlowTest cases for staying on confirmation and returning to Amount. Those tests were inspected. This review did not run Gradle or a device journey. The two earlier denomination threads are resolved, and their code is unchanged since 427e35c1.
Device testing: not performed in this review. Recordings linked from the PR at this head are that reviewer's evidence.
Ready for device testing. J1's revised Savings/Spending expectation matches the current send flow and can be run as written.
|
Addressed the companion-documentation suggestion from review 5609799465 in synonymdev/bitkit-ios#1078 with 4c95fd94d6aa51b118e47fba18fb7d210f7d3445. J1 and its README now describe Android’s valid quoted-term rebinding, fallback to Amount and manual Amount/Back behavior, while retaining the iOS exact-valid-terms distinction. XML/reference checks, |
Related to #1533 (denomination selector portion).
Twin: synonymdev/bitkit-ios#1078
This PR replaces the separate BTC and USD denomination buttons with one filled toggle that shows the current denomination in payment-request and subscription amount entry.
Description
Out of Scope
Design
Request payment — enter amount, denomination decision in thread #1008. The control switches BTC/USD denomination; Savings is a separate balance choice.
Preview
Reviewer recordings at
3d01e7de10cacover the revised J1 method-switch, fallback, input and linked-wallet delivery cases with disclosed fixture hooks. Reviewer recordings at427e35c1918ashow rapid reverse taps restoring the original USD amount in both editors on the unmodified APK. Earlier recordings atda64f8bb4effcover TalkBack and request/subscription flows; their rapid-tap failure predates the fix.QA Notes
Setup
Use the two-wallet recipe in
journeys/subscriptions/README.mdfrom baseae800aef, then build this branch withjust e2e network assembleDevDebugand install the dev APK on the prepared devices. Use the repository-pinned dependencies. Two saved contacts must already be linked as identities with Paykit UI enabled. J1 requires USDT enabled and a fresh BTC/USD rate; its amount-entry checks need no funds. J2 requires USDT disabled with no USDT funds or pending transfers. Use the journey's comma-decimal locale for exact-cents checks; close payment reviews without paying. For manual case 1, enable TalkBack in Android Settings → Accessibility on the same prepared device.Journeys
paykit-quoted-usdt.xml— one denomination control in request/subscription amount entry, exact USD restoration across repeated BTC/USD switches, and preserved review amounts. Its funded-method navigation expectation was corrected in3d01e7de10ca: valid Android Savings/Spending terms stay on Confirm; unavailable/expired/below-minimum terms return to Amount. J1 retains its ID; earlier results do not certify the revised full journey.create-and-propose.xml— BTC-only subscriptions show a fixed BTC label with no swap icon, while the separate Bitcoin/fiat input-unit control remains usable.Manual Tests
1. With TalkBack enabled, focus the denomination toggle in Request Payment and subscription amount entry, then switch BTC/USD → the current denomination is announced once per focus; repeat with the fixed BTC label in BTC-only mode — audible screen-reader output not in Capabilities.
2. Request Payment or subscription amount entry with a fresh rate → switch BTC/USD, then tap again within 500 ms after the label changes → both taps take effect and the original amount is restored — controlled sub-500-ms input timing not in Capabilities.
Automated Checks
N/A — no automated test changes; existing amount-conversion coverage is retained.
Evidence
At
60f047373cd5before the accessibility follow-up:just compilepassed;just --set gradle './gradlew --max-workers=1 -Dorg.gradle.parallel=false' testpassed all 4,384 tests across 242 suites; the same one-workerjust lintpassed. Complete Detekt SARIF and Android Lint XML/text reports contain zero findings. Gradle reported a Detekt-plugin ReportingExtension deprecation; the plugin/version/build configuration is unchanged from baseae800aef.The initial independent review of
60f047373cd5found no issues, but missed the duplicate icon/caption accessibility label later identified by Greptile. A fresh review of the two-line correction and its callers found no further issues. Both updated journey XML files parse andgit diff --checkpasses.At
da64f8bb4eff, the two-line accessibility fix retains the visible caption as the button name.just compile,just testandjust lintpassed with--max-workers=1 -Dorg.gradle.parallel=false: all 4,384 tests across 242 suites passed (zero skipped), and complete Detekt SARIF plus Android Lint SARIF/XML/text reports contain zero findings. Test stderr included OpenJDK class-sharing warnings and four cursor-close warnings traced toSharedPubkyProviderTest.kt; that test and its provider are byte-identical to baseae800aef.At
427e35c1918a, the denomination control accepts a rapid return tap without changing other buttons’ 500 ms debounce. One-workerjust compile,just testandjust lintpassed on the final source: 4,384 tests in 242 suites, zero failures/skips, zero findings in complete Detekt SARIF and Android Lint SARIF/XML/text reports. The four cursor-close warnings still trace to the same provider/test files, byte-identical to targetf87b8710. A fresh independent review of this delta and its callers found no issues. Native rapid-tap timing is manual case 2; the separate device evidence below verifies that interaction.At
427e35c1918a, GitHub build, lint/Detekt and all seven E2E jobs passed (E2E run). Green CI does not cover the controlled rapid-tap case or certify the complete linked-wallet journeys.Independent reviewer device evidence at
da64f8bb4eff, on two linked regtest wallets using Android 16 / API 36 Pixel 7 emulators: manual case 1 passed with actual TalkBack speech and TTS diagnostics in both editors and fixed-BTC mode. J1 denomination scenarios passed at normal tap intervals (four exact USD round trips, request/subscription review and delivery, empty input, trailing comma); full J1 remained incomplete. J2 fixed BTC, separate input units, large-text keyboard reachability, description validation, and the 5,000-sat proposal passed. Stale-rate rejection and edited-satoshi conversion also passed. No payment or subscription was signed. The review's requested change and manual case 2 failure concernda64f8bb4eff, before427e35c1918a; that review explicitly does not assess the later fix.Independent reviewer device evidence at
427e35c1918a, using two linked regtest wallets on Android 16 / API 36 Pixel 7 emulators in a comma-decimal locale: manual case 2 passed on the unmodified APK in both request and subscription editors (tap pairs completed within 327 ms and 377 ms), with intermediate BTC and exact 10,00 USD restoration. J1-contact and J1-payload passed with the review's disclosed unpublished fixture hooks: USDT-only contact typography, 150.50 USD delivery/reopening and exact 5.123456 USDT SDK amounts. The funded Savings/Spending case used a real native test channel; switching stayed on Confirm at 11,827 sats, and opening Amount manually then Back closed the sheet. No payment or subscription was signed.The funded-case report exposed a stale J1 expectation inherited from the base. Base commit
6eecb224d7165deliberately keeps valid quoted Bitcoin switches on Confirm, covered byAppViewModelSendFlowTest.ktandfixed-quoted-confirmation.xml; the relevant implementation and tests are unchanged by this PR.3d01e7de10cacorrects J1 and its README to that contract, including method-specific quoted amounts, refreshed fees, invalid-term fallback and manual Amount/Back dismissal. This is documentation only: XML parsing, reference checks andgit diff --checkpassed, with a fresh independent review. No application build or device run was repeated for the documentation delta. The earlier full compile/test/lint results remain tied to427e35c1918a; no new runtime or automated test changes were made.At
3d01e7de10ca, the reviewer approved the revised J1 contract after driving two Android 16 / API 36 regtest wallets with an Arbitrum fixture. The report covers valid Savings/Spending quotes with different amounts and refreshed fees; missing/expired target quotes and below-minimum fallback; balance-based initial selection; accepted methods; exact denomination round trips; comma input; 150.50 USD request/subscription delivery; and direct USDT entry. The minimum branch was event-driven because LNURL confirmation has no visible funding-source control. These builds used the report’s disclosed unpublished input and native-channel hooks; the report states that payment conversion and navigation decisions were unchanged. Earlier separately verified TalkBack/J2 and rapid-tap cases remain attributed to their original revisions. No payment or subscription was authorized. GitHub build, lint/Detekt and all seven E2E jobs also passed at this head (E2E run).Gaps
The author did not drive J1/J2 or manual cases 1–2: this task has no prepared pair of linked Paykit wallets. Reviewer coverage combines the remaining J1 branches at
3d01e7de10cawith retained, unchanged cases at earlier revisions; it is not a complete unmodified-APK journey rerun at this head. Manual case 2 has unmodified-APK evidence at427e35c1918a; TalkBack/J2 evidence remains atda64f8bb4eff, with no later rerun. J1 fixture-assisted coverage and its event-driven minimum check are disclosed above. Historical results remain tied to their tested revisions. Figma pixel comparison remains unverified; the reviewer could not access the linked frames. No payments were made.Models used
gpt-6.1-sol(reasoning:xhigh),gpt-6-astra(reasoning:xhigh)gpt-6.1-sol(reasoning:xhigh),gpt-6-astra(reasoning:xhigh)gpt-6-astra(reasoning:xhigh),gpt-6.1-sol(reasoning:high)