Repository navigation
fix: authorize initial USDT subscriptions once - #1568
ben-kaufman wants to merge 12 commits into
Conversation
 [Critical impact] The change reaches the wallet's USDT payment flow for initial subscriptions. The PR appears safe to merge, with a non-blocking test gap in the automatic payment handoff. Findings
SummaryEligible initial USDT subscription payments now continue from Subscribe without a second payment swipe.
Tests were inspected, not run. No device UI capture was made. Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Swipe Subscribe] --> B{Eligible initial USDT payment?}
B -->|No| C[Existing amount and payment review]
B -->|Yes| D[Open USDT review]
D --> E[Consume one-use authorization]
E --> F[Prepare quote]
F -->|Failed| G[Show error; require explicit retry]
F -->|Ready and authorized| H[Confirm payment]
H --> I[Required warnings and authentication]
I --> J[Check terms, deadlines, and previous attempts]
J --> K[Send payment]
K --> L{Transfer confirmed?}
L -->|No| M[Pending or failed payment view]
L -->|Yes| N[Subscribed]
Reviews (1) · Last reviewed commit: "fix: reuse initial USDT subscription aut..." · 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. An initial USDT subscription payment now goes straight from Subscribe to the USDT review and confirms itself once. It only does that when the terms leave the amount unchanged: USDT with no conversion, or USD with a fixed 1:1 USDT rate. Reviewed 2c486a9, full tier.
What I checked, and 6 candidates I ruled out
Read in full: PaykitUsdtReview.kt, the UsdtReview route in SendSheet.kt, UsdtViewModel.kt (quote, confirm, send, waitForTransfer), ContactPaymentContext.initialUsdtSubscriptionPayment, openPaykitEndpoints, takeInitialUsdtSubscriptionAuthorization, sendPaykitUsdt / authorizeUsdtExecution, PaykitUsdtPaymentRepo.prepare / validateQuote / validateRequestTerms, PaykitRequestPricing.payment
Call sites traced: initialSubscriptionPaymentAutoStartPending (set in resetSendState, consumed by onStartInitialSubscriptionPayment on the Bitcoin path and by the new taker on the USDT path), isInitialSubscriptionPayment = true (accept, failure, in-sheet retry), AutoPayOverlay, TransactionConfetti
CI: green: build (with testDevDebugUnitTest), lint, detekt, all seven e2e-tests-local shards, e2e-status
Not built here: this review was reasoned from the code and from CI.
Ruled out
- Auto-pay runs again after a rotation or recomposition:
LaunchedEffect(Unit)does re-run, but the taker clears the pending flag before it returns, and the second call returns false. The new test asserts that. - Auto-pay skips the PIN or the sanity warnings:
preparePaymentcalls the sameconfirm()as the swipe, sowarnings()andisPinForPaymentsEnabledstill gatesend(). - Auto-pay with terms that differ from the ones checked: the taker recomputes the terms from
activeContactPaymentContextand compares them withpaykitPaymentTermsandpaykitReviewedAmount.validateRequestTermscompares them again at submission. If they differ, it falls back to a manual swipe, never to a different amount. - Rate selection disagrees with
payment(): both look upusdt-arbitrum, thenusdt. The test also covers a 1:1usdtrate shadowed by a 2:1usdt-arbitrumone, which is rejected. - Hardware wallet or amount review paths auto-pay:
shouldAutomaticallyPayexcludeshardwareWalletIdandrequiresAmountReview. The non-eligible route still goes through Amount, and the taker returns false there. - Back is stuck when
UsdtReviewis the start destination: the newpopBackStack()-or-hideSheet()fallback matches the pattern already used atSendSheet.kt:515andPinSheet.kt:52.
Merge confidence: 4/5. The auto-confirm call in preparePayment moves money without a swipe and has no test of its own: the tests cover eligibility and the one-use taker, not the composable that calls confirm().
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Reviewed the full PR diff against its merge base, at 21fd91e. This is the first assessment of pull request 1568.
No new actionable code findings.
An eligible initial USDT subscription quotes and confirms once after Subscribe. Authorization is consumed before confirm and must still match the stored terms, reviewed amount, expiry, and automatic-pay guards. Quote failure, authentication cancellation, and leaving while preparation is in flight do not submit a payment or start another automatic attempt. A quote whose amount or terms differ from the authorized payment is rejected at send. Back closes the sheet when USDT review is the first destination. A confirmed transfer shows Subscribed with the existing USDT confetti, and the subscription progress layout stays up while the payment is busy.
The shared eligibility rules and journeys/subscriptions/review-and-subscribe.xml match bitkit-ios#1006 at ba31357. That comparison covers only those shared contracts.
The open Greptile thread asked for coverage of the automatic confirm path. At this revision, UsdtViewModelTest drives preparePayment through quote completion, authentication cancellation, explicit retry, failed recovery, in-flight cancellation, and a single submission. I inspected those tests and did not re-run them. The CI build on this commit completed testDevDebugUnitTest successfully. Journey files are not executed by that workflow.
Device testing: not performed in this review.
Ready for device testing.
Suggested additional test cases
- Android, dev build with the local USDT stack. Create a subscription that accepts both Bitcoin and USDT, or a USD subscription whose fixed USDT rate is not 1:1, with the first period due. Swipe Subscribe. Expected: the amount screen remains, no USDT payment is sent, and payment still requires an explicit review and swipe.
- Android, payer with payment warnings enabled and a balance that triggers an amount or fee warning. Swipe Subscribe on an eligible USDT-only proposal. When the safety dialog appears, tap Cancel. Expected: no payment is recorded, preparation does not start again on its own, and the unpaid period can still be paid by a later explicit authorization.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan: no findings.
Checked and clean:
- One-shot authorization:
takeInitialUsdtSubscriptionAuthorizationclears the flag before returning. It returns true only when the recomputed terms equalpaykitPaymentTermsand the amount equalspaykitReviewedAmount. Reopening a period from Payments is not marked initial, so it never auto-pays. - Changed amount: a refreshed request with a different amount falls back to a manual swipe, and
validateQuoteandauthorizeUsdtExecutioncheck quote = reviewed = terms again at send. - Dismiss:
hideSheetclears the context,_sendEffecthas no replay, and dismissing during the quote cancels theLaunchedEffectbeforeconfirm(). - Double pay:
validateReplacementthrowsOperationInProgresson an unresolved attempt, andisSubmittingPaymentRequestguards resubmission.
review-and-subscribe.xml is byte-identical to synonymdev/bitkit-ios#1006.
Device gate (partial): not run yet. Pending: subscriptions/review-and-subscribe.xml on a funded local USDT stack, plus timing from the Subscribe swipe to Subscribed (perf-relevant: a new user-facing wait under the progress overlay, with Back blocked) against master. The device result follows in a second review.
|
Outside this PR's scope; no change needed here. The USDT payment sheet ignores the bottom navigation-bar inset. On the device gate, after cancelling the PIN on an initial USDT subscription payment, the "Payment Request" review came back with Swipe To Pay under the gesture handle: 
Cause: It's already fixed in #1564, which adds |
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Approving at 21fd91e. The code scan from my earlier review stands; this review only adds the device result.
Device gate: 21fd91e — journeys: subscriptions/review-and-subscribe.xml 23/23 actions pass on Android (payer) against synonymdev/bitkit-ios#1006 as the creator; action 13 failed once and passed on re-run, see below; perf: USDT subscribe, swipe → Subscribed 80.3 s with one swipe vs master's swipe → Sent 102.4 s with Amount, Continue and a second swipe (automated taps; both wait ~35 s for the private payment list after acceptance); Figma: not compared (routing change, no new layout); l10n: no UI text changed.
Setup: Pixel_9 with on-chain sats and no channel, 60 USDT from the local fixture. The creator was the iOS twin on the iPhone 17 simulator, the two linked as contacts. Proofs reached the creator for every paid subscription ("1 subscriber, 1 payment"; "0 payments" for the unaffordable one).
Covered:
- BTC first period paid without a second swipe (Subscribed 95 s after the swipe).
- USDT first period paid through the fee warning with no amount entry.
- PIN on: swipe, then cancel the PIN; nothing was paid, Back closed the sheet, and reopening the unpaid period and authorizing paid once (balance 57.222109 → 54.546773).
- $100 over balance: "Not enough USDT to cover the amount and fees." with the period still offered and the balance unchanged.
- Overview: one MONTHLY COST total across BTC and USD subscriptions.
Action 13 ("billing period no longer offered"): right after Close on the BTC subscription, the Payments tab still listed the period with Pay for a few seconds. It was gone on re-run ~30 s later. This PR doesn't change the BTC path, so I'm treating it as refresh lag, not a finding.
UsdtViewModelTest and the code trace in my earlier review.
Out of scope, tracked in #1571: after the PIN cancel, the USDT "Payment Request" review puts Swipe To Pay under the gesture handle. It's the same on master, and #1564 fixes it.
CI at 21fd91e: green.
|
Fixed the matching manual-preparation issue found in the iOS review in 1711f13. Review & Subscribe progress now appears only when the initial subscription authorization was actually consumed; manual quote preparation and explicit retries keep the normal payment review. Quote, fee and authentication checks are unchanged. Added delayed-preparation coverage through the real review helper and UsdtViewModel, and extended the cancellation/re-entry test. All three PR commits are signed and verified locally and by GitHub. Re-requesting review. |
![ovi-reviewer[bot]](https://avatars.githubusercontent.com/in/4862686?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 💬 Comment: one LOW, reproduced on device. The automatic, manual and failed-preparation routes paid at most once.
The PR moved to 1711f13 after this run; these results are for 21fd91e.
Findings:
- LOW
app/src/main/java/to/bitkit/ui/sheets/SendSheet.kt:338: tapping the locked amount on the direct USDT review closes the payment sheet.- On the new direct route (
AppViewModel.kt:3564),UsdtReviewis the only entry in the sheet's back stack. - The review's amount header is still tappable while the payment is not busy (
UsdtSendContent.kt:547). ItsonPageChangeisback()(PaykitUsdtReview.kt:116), sopopBackStack()returns false andhideSheet()closes the sheet. - In Test 4, after a cancelled PIN, one tap on $2.00 closed the sheet. Nothing was paid, and the period stayed payable from Payment Requests.
- Before this PR the same review sat on top of the locked Amount screen, and the tap went back there. The amount is not editable on this route, so the tap can do nothing here.
- The code is unchanged at 1711f13.
- On the new direct route (
QA for 21fd91e:
Two Android 15 emulators, one creating and one paying subscriptions, on a local Arbitrum fork with test USDT. Balances were checked on the fork before and after each case.
Test 1: safety warning cancelled on an eligible initial payment ✅ Cancel sent nothing and started nothing again; a later explicit payment paid once.
- The creator proposed $6 monthly, USDT only. The payer held 10 USDT.
- Swipe To Subscribe kept the Review & Subscribe layout while it prepared, then showed the Payment Request review with the over-50% warning. There was no Amount step.
- Cancel left the locked Amount screen. Nothing changed for 40 s, and the balance on the fork stayed at 10 USDT.
- Continue, Swipe To Pay and Yes, Send reached Subscribed. That step fell in a gap between recordings; the second clip starts after it.
- The second clip shows a balance of 3.17 USDT, one −6.83 row in activity and one payment on the subscription. The creator's list shows "1 subscriber, 1 payment".
https://github.com/user-attachments/assets/81bb341e-6fef-4f4d-8ec2-0f774be5b5c0
https://github.com/user-attachments/assets/afb9cd70-4bf2-4dbc-b7da-93199ab69bf9
Test 2: USD subscription with a 2:1 USDT rate ✅ The payer got the manual Amount and review steps, and nothing was paid without a swipe.
- The creator proposed $1 with a fixed rate of 2 USDT per USD. Bitkit's own creator never sends that rate, so a test build of the creator stood in for an external issuer.
- Swipe To Subscribe opened Amount at $2, USDT on Arbitrum One, with Continue.
- Continue opened the $2.00 Payment Request review, which waited at Swipe To Pay with no warning or PIN prompt. I closed it without paying. The balance stayed at 28.17 USDT, and the creator shows 0 payments (creator's list under Test 1).
- Between Continue and the review, Review & Subscribe at $1.00 with a spinning swipe showed for about 5 s. This is the paying overlay on the manual route that the pair PR's review raised, which 1711f13 now limits to automatic payments. I did not retest it.
T2-2x.2x.mp4
Test 3: failed preparation, then an explicit retry ✅ Preparation failed without sending or retrying, and the explicit retry paid once.
- The fork's fee provider was set to issue fee authorizations that had already expired. Then the payer swiped to subscribe to a $2 USDT-only proposal.
- The payment stopped on Amount with "The fee quote expired. Review the payment again." It stayed there for 40 s, and the balance stayed at 28.17 USDT.
- With the provider working again, Continue prepared a new quote and waited at Swipe To Pay. Swipe, the fee warning and the PIN led to Subscribed.
- The balance went from 28.170809 to 25.495473 USDT, which is one $2 payment plus fee. The subscription lists one payment, and the creator shows "1 subscriber, 1 payment" (creator's list under Test 1).
- This manual payment also showed the Review & Subscribe overlay while sending, the same case Test 2 notes.
T3-2x.2x.mp4
Test 4: amount tapped after a cancelled PIN ❌ The tap closed the whole payment sheet.
- PIN for payments was on. The creator proposed $2 monthly, USDT only.
- Swipe To Subscribe, then Yes, Send on the fee warning, opened the PIN screen. Back cancelled it, and the Payment Request review came back with Swipe To Pay.
- Tapping $2.00 closed the sheet and returned to the USDT wallet.
- Nothing was paid: the balance stayed at 28.17 USDT, and the creator shows 0 payments (creator's list under Test 1). The period was still listed under Payment Requests with Pay and Dismiss.
T4-2x.2x.mp4
Not driven:
- Paykit quote terms expiring or being replaced during preparation (this PR's Manual Test). Test 3 expires the USDT fee authorization, not the request's quote terms, so this case rests on
UsdtViewModelTest. - A request accepting both Bitcoin and USDT. Bitkit's creator cannot send one, so that half of @piotr-iohk's first case rests on
AppViewModelSendFlowTest.
Relied on:
- @jvsena42's device run at 21fd91e of
review-and-subscribe.xml(23/23). It covers the first USDT period paid automatically through to Subscribed, PIN cancel with a retry, the over-balance error and the BTC first period. I did not re-drive those. - Test 1 is @piotr-iohk's second suggested case, and Test 2 is the non-1:1 half of the first.
Pair PR synonymdev/bitkit-ios#1006 matches in eligibility: accepted methods must be exactly usdt-arbitrum, and the usdt-arbitrum, then usdt, rate must be 1:1.
Full review of 402aecb...21fd91e, 10 files. This PR moved to 1711f13 during the run. That commit is not tested here; I read its diff only for the two notes above.
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: Follow-up review of f6f2c631 since 21fd91e, inheriting the completed baseline review. This pass covered the two later commits: automatic-payment progress only after a fresh takeInitialUsdtSubscriptionAuthorization result, locked review amounts staying on the payment review while Back still closes the sheet and ordinary send amounts stay editable, submitted-payment reconciliation, and the updated review-and-subscribe.xml steps. Unchanged eligibility, direct USDT routing and one-use authorization, send-time quote checks, and duplicate-submit guards remain covered by that baseline.
No new actionable code findings.
Manual preparation quotes and leaves confirmation to an explicit swipe, so the subscription progress overlay stays on an authorized automatic payment. A locked amount tap on the direct USDT review leaves that review open. Back still closes a sheet whose first destination is USDT review. After a confirmed initial transfer, Subscribed has no back button, and system back closes the sheet. journeys/subscriptions/review-and-subscribe.xml is byte-identical to bitkit-ios#1006 at 72814ca6.
The amount-tap dismissal reported on 21fd91e is addressed at this revision. I inspected UsdtViewModelTest and UsdtPaymentContentTest and did not run them. The build workflow passed on this commit; journey files are not part of that workflow. Device results posted for 21fd91e predate these commits.
Device testing: not performed in this review.
Ready for device testing.
Suggested additional test cases
- Android, dev build with the local USDT stack. Propose a USD subscription whose fixed USDT rate is not 1:1, with the first period due. Swipe Subscribe, then Continue from the amount screen. Expected: while the quote is prepared, the Review and Subscribe progress overlay stays hidden; the payment review waits for an explicit swipe, and nothing is sent until that swipe.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Delta since my approval at 21fd91e (1711f13, f6f2c63): no findings.
- The overlay is now
AutoPayOverlay(isVisible = automaticPayment && state.busy). The flag comes only from the one-shot taker and is cleared by an explicit Continue or swipe. - Warning Cancel, PIN cancel and quote failure all leave
busyfalse. The fee warning andUsdtAuthenticationare never covered. - A manual initial payment and a period reopened from Payments no longer show "Review & Subscribe" before the user authorizes.
amountEditable = falsemakes the locked amount a no-op only on this route. Ordinary USDT sends keep amount navigation.- No payment trigger changed:
preparePaymentstill confirms only ontakeInitialUsdtSubscriptionAuthorization()with a quote.
#1571 still applies: PaykitUsdtReview line 98 still has no navigationBarsPadding().
Device gate (partial): 21fd91e ran the full review-and-subscribe.xml. Not run yet at f6f2c63: the two changed actions in subscriptions/review-and-subscribe.xml (overlay only during the automatic payment; locked amount on the manual route), on Android against synonymdev/bitkit-ios#1006. The device result will follow.
|
Resolved the master conflict in 47030f9, retaining the locked-amount/Back regression alongside master's decimal-entry and enlarged-header tests. Existing authorization, quote and duplicate-payment safeguards are unchanged.
|
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: No new findings. The locked-amount dismissal is fixed and verified in Test 2.
Reaudit: 21fd91e…47030f9, 74 changed files including the master merge; 12 files differ from the current base.
Pair PR: synonymdev/bitkit-ios#1006 fix: authorize initial USDT subscriptions once at 72814ca matches the eligibility rules, manual progress behavior, locked-amount fix and shared journey. I did not drive iOS in this run.
QA for 47030f9:
Tested on two Android 15 / API 35 Pixel 9 emulators using test USDT on a local Arbitrum fork. Creator is left, payer is right; videos play at 2× speed. Tests 1, 2, 3 and 5 used the unmodified head on the payer. Disposable issuer hooks supplied the non-1:1 and mixed-method proposals.
Test 1: Automatic initial payment ✅ One Subscribe swipe reached Subscribed; Back dismissed it.
The fee warning remained actionable, and no payment swipe was required after Subscribe. Subscribed had no Back arrow. The period appeared once in payment history without Pay or Dismiss. Fork balance confirmed one 2 USDT payment plus its fee.
T1.2x.mp4
Test 2: PIN cancellation and manual retry ✅ The locked amount kept review open; explicit retry paid once.
Canceling PIN sent nothing, checked against the fork balance. Tapping the locked amount retained Payment Request; Back closed it. Reopening the unpaid period required Continue and a manual payment swipe. Preparation stayed on normal payment UI, and PIN authorization reached USDT Sent. Payer history and creator showed one payment.
T2.2x.mp4
Test 3: Non-1:1 conversion ✅ A 2:1 rate required manual confirmation without subscription progress.
The $2 proposal opened Amount at 4 USDT. Continue prepared the normal Payment Request and waited at Swipe To Pay. Closing unpaid left the fork balance unchanged and the creator at zero payments.
T3.2x.mp4
Test 4: Conversion quote expiry and recovery ✅ Expired terms prevented sending; explicit reopening paid once.
A disposable payer debug hook injected a per-period conversion quote with 10-second validity and held preparation for 15 seconds; payment guards remained unchanged. After the manual swipe, fee warning and PIN, the app rejected the request. Diagnostics confirmed expired Paykit terms, a valid native USDT quote and matching amounts; the fork balance was unchanged. Reopening loaded the original valid fixed terms and required manual authorization, then showed USDT Sent and one history row without Pay. The fork debit was one 2 USDT payment plus its fee; the creator showed one incoming row pending proof reconciliation. Quote replacement was not separately exercised.
T4.2x.mp4
Test 5: Mixed accepted methods ✅ Accepting Bitcoin and USDT kept the payment manual when only USDT resolved.
The issuer declared both methods while publishing only USDT private endpoints. The payer exposed only USDT at Amount, then prepared normal Payment Request and waited for Swipe To Pay. Closing unpaid left the fork balance unchanged and the creator at zero payments.
T5.2x.mp4
Unchanged cases were not repeated:
- Earlier device report at 21fd91e: BTC first period, insufficient funds and monthly cost overview.
- Earlier review and device report at 21fd91e: safety-warning cancellation and failed fee-authorization recovery.
- Author's component report at f6f2c63: ordinary editable-amount navigation. I did not repeat that control on device; the merged component tests retain it.
Focused authorization, request-pricing and USDT repository/view-model checks passed on the unmodified head. Master integration received source review and these checks; the device cases above cover the changed subscription paths.
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: Follow-up at 19e3a31, since 1f55d03, using the completed baseline review. This pass checked payer subscription details, which now list only belonging history rows with a submitted payment proof, while creator details still use received payments. It also checked the updated repository assertions, the new review-and-subscribe journey sentence, and the same filter on iOS pull request 1006 at 78cfdb7. Inherited coverage is initial USDT eligibility and one-use authorization, quote preparation and payment guards, locked-amount navigation, subscription success, expired-period retention in payment history, payable versus expired status, the remaining author tests and journey actions, and the earlier iOS eligibility comparison at 9450bdb.
No new actionable code findings.
The subscription-detail sent-payment report is addressed at this revision. Expired unpaid periods stay in Payments as Expired, and a passed payment deadline leaves a period with a submitted proof in the subscription detail. The iOS comparison covers this shared listing and journey sentence.
Validation: the new PaykitPaymentRequestRepoSubscriptionTest assertions were inspected. The CI build on this revision completed its unit-test step, and lint and detekt passed. Device testing: not performed in this review. The device pass covers 1f55d03.
Ready for device testing. The targeted check is the updated review-and-subscribe.xml sentence: an expired unpaid period stays in Payments and is absent from the subscription detail, while previously paid periods remain there.
jvsena42
left a comment
jvsena42
left a comment
There was a problem hiding this comment.
Code scan at 19e3a31: the MEDIUM is fixed. subscriptionPayments lists only periods with payment evidence on the payer side and keeps receivedPaymentRequests() for the creator. The repo tests assert that an expired unpaid period stays out after the timer, after a refresh and after a reload. Nothing else in the commit changes behaviour.
Device gate (partial): 19e3a31 — not run yet. Pending: subscriptions/review-and-subscribe.xml, including the new expiry and relaunch action, cross-device with the iOS twin and an issuer that sets PeriodStart deadlines. Device results will follow in a separate review.
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: No new findings. The expired unpaid period shown as sent is fixed: payer subscription details retain only periods with payment evidence; creator proof rows remain available.
QA for 19e3a31:
Two Android 15 / API 35 emulators with local-fork test USDT. Creator is left, payer is right; recordings are edited and play at 2×.
Test 1: Expired unpaid periods excluded from subscription payments ✅ Expired history and the earlier paid period survive relaunch; rollover adds only a payable new period.
I first made a real 2-USDT payment on the unmodified payer build. Fork balances and the queued-proof log establish that paid control. The deadline checks then used a billing-clock fixture: the repository and status view see controlled billing time, while SDK authentication and the subscription-detail selection remain unchanged.
The unpaid October 21 period remains exactly once in Payments as Expired, with no Pay action. Subscription details show only the paid October 20 period. Both checks hold after process restart. At the October 22 rollover, only the new period offers Pay; October 21 stays Expired, and subscription details still contain just the October 20 payment. UI dumps independently check the row counts, dates and payment actions shown in the recordings.
T1.2x.mp4
Test fixture controls — not pushed
The creator supplies a signed five-minute period-start deadline. The payer fixture advances only billing and expiration time; it does not fabricate SDK records, payment proofs or selection results. The initial payment used the unmodified payer build. The patches used for the subsequent deadline checks are:
diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt
index 1e62b60e0..1dfa669d1 100644
--- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt
@@ -924,6 +924,7 @@ class PaykitPaymentRequestRepo @Inject constructor(
conversion = rates.takeIf { it.isNotEmpty() }?.let(PaymentConversion::Fixed),
paymentReference = "bitkit-${UUID.randomUUID()}",
proposalExpiresAt = draft.expiresAt.toString(),
+ paymentDeadline = if (name.startsWith("Deadline")) PaymentDeadline.PeriodStart(300uL) else null,
acceptedPaymentEndpointIdentifiers = endpointIdentifiers,
metadataJson = JsonObject(
mapOf(diff --git a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt
index 1e62b60e0..4b43ec7c9 100644
--- a/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt
+++ b/app/src/main/java/to/bitkit/repositories/PaykitPaymentRequestRepo.kt
@@ -319,7 +319,7 @@ class PaykitPaymentRequestRepo @Inject constructor(
private val paymentProofRepo: PaykitPaymentProofRepo,
private val usdtPayments: PaykitUsdtPaymentRepo,
private val subscriptionNotificationScheduler: PaykitSubscriptionNotificationScheduler,
- private val clock: Clock,
+ clock: Clock, // test hook
@SubscriptionClock private val subscriptionClock: Clock,
private val lightningRepo: LightningRepo,
private val currencyRepo: CurrencyRepo,
@@ -332,6 +332,8 @@ class PaykitPaymentRequestRepo @Inject constructor(
private const val PAYMENT_REQUEST_DISCOVERY_CONCURRENCY = 8
}
+ private val clock = to.bitkit.utils.ReviewBillingClock.wrap(clock) // test hook
+
private val operationMutex = Mutex()
private val refreshMutex = Mutex()
private val targetDiscoveryMutex = Mutex()
diff --git a/app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt b/app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt
index 38288ffc7..e9844cb2f 100644
--- a/app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt
+++ b/app/src/main/java/to/bitkit/ui/screens/paymentrequests/PaymentRequestsScreen.kt
@@ -504,7 +504,7 @@ internal fun paymentRequestStatus(
internal fun paymentRequestLifecycleStatusRes(request: PaykitPaymentRequest, isPending: Boolean): Int {
val canExpire = request.lifecycleState == PaymentRequestLifecycleState.PROPOSED ||
request.lifecycleState == PaymentRequestLifecycleState.ACTIVE_RECURRING
- if (canExpire && request.isExpired(Clock.System.now())) {
+ if (canExpire && request.isExpired(to.bitkit.utils.ReviewBillingClock.date(Clock.System.now())) /* test hook */) {
return R.string.wallet__payment_request_status_expired
}
diff --git a/app/src/main/java/to/bitkit/utils/SubscriptionClockOffset.kt b/app/src/main/java/to/bitkit/utils/SubscriptionClockOffset.kt
index 21812272b..441d589bb 100644
--- a/app/src/main/java/to/bitkit/utils/SubscriptionClockOffset.kt
+++ b/app/src/main/java/to/bitkit/utils/SubscriptionClockOffset.kt
@@ -44,8 +44,8 @@ object SubscriptionClockOffset {
): Instant {
if (!isAvailable) return date
val days = clampedOffsetDays(offsetDays)
- if (days == 0) return date
- return date + days.days
+ if (days == 0) return ReviewBillingClock.date(date) // test hook
+ return ReviewBillingClock.date(date + days.days) // test hook
}
fun subscriptionNow(clock: Clock = Clock.System): Instant = subscriptionDate(clock.now())
diff --git a/app/src/main/java/to/bitkit/utils/ReviewBillingClock.kt b/app/src/main/java/to/bitkit/utils/ReviewBillingClock.kt
new file mode 100644
index 000000000..b9ae6adb9
--- /dev/null
+++ b/app/src/main/java/to/bitkit/utils/ReviewBillingClock.kt
@@ -0,0 +1,18 @@
+package to.bitkit.utils
+
+import to.bitkit.BuildConfig
+import java.io.File
+import kotlin.time.Clock
+import kotlin.time.Duration.Companion.milliseconds
+import kotlin.time.Instant
+
+// test hook: changes only Paykit billing/expiration time; SDK authentication keeps real time.
+internal object ReviewBillingClock {
+ fun date(date: Instant): Instant = if (BuildConfig.DEBUG) date + offset().milliseconds else date
+ fun wrap(base: Clock): Clock = object : Clock {
+ override fun now(): Instant = date(base.now())
+ }
+ private fun offset(): Long = runCatching {
+ File("/data/user/0/${BuildConfig.APPLICATION_ID}/files/review-billing-offset-ms").readText().trim().toLong()
+ }.getOrDefault(0)
+}Test 2: Creator submitted-proof history ✅ The creator retains one +$2.00 proof row after restart.
The October 20 row remains exactly once after process restart, labeled pending verification. The fork independently confirms 2 USDT received. This checks retained proof history, not a change to receipt verification. The creator uses only the proposal-deadline fixture shown above; its receipt selection and detail screen are unchanged.
T2.2x.mp4
Retained coverage for paths this delta leaves unchanged:
- The 13-case device report at 1f55d03 covers initial authorization, cancellation/retry, insufficient funds, conversion guards, signed quote expiry, locked amounts, expiry retention and monthly cost.
- The quote-replacement device check at d5ae1af covers rejection of replaced terms and explicit recovery.
No planned cases remain unrun. The current-head repository expiry/restoration/rollover and presentation regressions also pass on unmodified sources.
Twin synonymdev/bitkit-ios#1006 fix: authorize initial USDT subscriptions once at 78cfdb7 has the equivalent paid-only filter and byte-identical journey. I checked that contract in source; iOS devices were not repeated in this delta pass.
Delta review covers 1f55d03…19e3a31, all four changed files and their history consumers. The changelog received source review only.
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.
Approving the current revision based on the completed code review, full paired 13-case device baseline, and current-head fix/regression retests. The paid-only subscription-history fix is verified, prior findings are addressed, and applicable build/unit/integration/lint checks pass. E2E is non-blocking for this approval.
The observed clock-skew subscription sequence is a separate, unconfirmed follow-up: one payment was verified on both wallets, with no duplicate charge. Controlled skew and base-branch comparison will be investigated separately. This approval does not claim that follow-up is fixed.
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: No new findings. The merge retains expired subscription periods in history, keeps unpaid periods out of subscription payments, and preserves the USDT review's navigation-bar padding. One-time expiry notifications exclude subscription billing periods.
QA for 844d295:
Two Android 15 / API 35 emulators using a locally built, unmodified payer and creator at this head with local-fork test USDT. In paired clips, creator is left and payer is right. Videos play at 2×.
Test 1: Expiry and initial-payment integration regressions ✅ Focused current-head regressions pass.
Executed the repository expiration, refresh/restoration and rollover tests, including the production paid-only subscription selection and retained creator receipts. Also ran the one-time expiry-event tests, initial-USDT routing and one-use authorization tests, and USDT preparation, cancellation and recovery tests. No failures or skips in the selected checks.
Test 2: PIN cancellation and explicit unpaid-period retry ✅ Cancellation sends nothing; reopening requires manual authorization and pays once.
Subscribe retained the preparation layout and reached the fee warning and PIN without another payment swipe. Cancelling PIN returned to review; tapping the locked amount kept it open, and Back closed it. The swipe and PIN controls clear the gesture bar.
From Payments, the period showed Waiting for payment. Pay opened fixed Amount; Continue prepared ordinary payment review without subscription progress. A manual swipe, fee-warning confirmation and PIN reached USDT Sent, then Proof submitted with no Pay action.
Fork balances independently confirmed no debit from cancellation and exactly 2 USDT received from each explicitly authorized retry. The recorded retry increased the recipient from 2 to 4 USDT; the payer debit included its fee.
t2a-2x.2x.mp4
t2b-2x.2x.mp4
Test 3: Editable USDT amounts and enlarged-text review ✅ Ordinary amounts remain editable, and review controls remain visible at 150% text size.
Entered 1.5 USDT and prepared review. At 150% text size, expanded details showed the recipient, fee note and Swipe To Pay above the gesture bar. On another ordinary 1-USDT review, tapping the amount returned to entry and accepted an edit to 12. No payment was submitted in this case.
t3-2x.2x.mp4
Retained coverage for paths this merge leaves unchanged:
- The paid-history device checks at 19e3a31 cover expired unpaid periods through refresh, restart and rollover, and retained paid payer/creator history.
- The 13-case device baseline at 1f55d03 covers eligibility, insufficient funds, warnings, conversion guards, signed quote expiry and the remaining journey actions.
- The quote-replacement check at d5ae1af covers stale-term rejection and explicit recovery.
No planned cases remain unrun. The bitkit-core rc7→rc9 changes leave direct Arbitrum quote, send and deadline execution unchanged.
Twin synonymdev/bitkit-ios#1006 fix: authorize initial USDT subscriptions once at ac7e2fd matches the eligibility and paid-history contract; its journey is byte-identical. This pass compared those contracts in source.
Delta review covers 19e3a31…844d295, including master integration and both conflict resolutions. Other merged master changes and documentation received source review without separate device cases.
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: Re-reviewed the full pull request at 844d295 after the comparison base changed from 3cdc79c to 22a5cb56 and no verified baseline coverage was supplied. The merge base is 22a5cb5.
No new actionable code findings.
Eligible initial USDT subscriptions open the USDT review and can authorize that payment once. Eligibility is USDT with no conversion, or USD with a fixed 1:1 USDT rate, and USDT as the only method. A failed quote, cancelled authentication, or a later entry leaves confirmation to an explicit retry. Terms that still need conversion stay on the amount screen. Expired unpaid periods of an active subscription leave the payable list, remain in payment history without a Pay action, and stay out of subscription details. Paid payer rows and creator receipts remain. One-time proposal expiry toasts still apply only to one-time requests. The journey text and this payment, history, and success behavior match bitkit-ios#1006 at ac7e2fdb.
Validation: inspected the authorization, quote/confirm, history, and presentation tests. This review did not execute them. Build and lint succeeded for this revision. Device testing: not performed in this review. J1 still needs the local USDT fixture and funded identities in the PR setup.
GitHub still has changes requested on d5ae1af and 1f55d03. Those threads are resolved, and later reviews approved the paid-history fix. Dismissing the stale reviews is what unblocks merge. Ready for device testing of J1 on this revision with that USDT fixture.
|
Resolved the current-master conflict in signed merge Final |
|
Resolved the new master conflicts in signed merge Final The funded initial-subscription journey and controlled provider/quote-expiry checks remain open. New-head CI will be monitored; human review is re-requested. |
There was a problem hiding this comment.
Verdict: ✅ Approve
Findings: No new findings.
QA for c8a6466:
Built the unchanged app sources at this head. Device cases used two Android 15 / API 35 emulators and local-fork test USDT. Videos play at 2×; fork balances independently check transfers.
Test 1: Integration regressions ✅ Focused current-head regressions passed.
Executed initial-USDT eligibility, routing and one-use authorization tests; preparation, cancellation and recovery tests; subscription expiry/restoration and presentation tests; and lifecycle review-token and Paykit admission tests. All selected checks passed without skips.
Test 2: Initial USDT payment ✅ One Subscribe swipe reached Subscribed and recorded one payment.
The fee warning remained actionable, with no Amount step or second payment swipe. System Back dismissed Subscribed. Subscription details showed one paid $2 period and the next renewal. The fork recipient increased from zero to exactly 2 USDT; payer debit included the fee. Payment PIN was disabled for this case.
t2-2x.2x.mp4
Test 3: PIN cancellation, resume and manual retry ✅ Cancellation sent nothing; explicit retry paid once.
With payment PIN enabled, cancelling authentication sent nothing, checked against unchanged fork balances. Tapping the locked amount kept review open. Background/resume retained Swipe To Pay without retrying, and Back closed the sheet.
The unpaid period showed Waiting for payment. Pay opened fixed Amount; Continue prepared ordinary review without subscription progress. A manual swipe, fee warning and PIN reached USDT Sent, then Proof submitted with no Pay action. The fork recipient increased from 2 to exactly 4 USDT.
t3a-2x.2x.mp4
t3b-2x.2x.mp4
Test 4: Failed preparation and explicit recovery ✅ Expired fee authorization sent nothing; a manual retry paid once.
The provider issued an expired native USDT fee authorization. Preparation showed “The fee quote expired. Review the payment again.” Diagnostics confirmed QuoteExpired, and both fork balances stayed unchanged. Restoring the provider did not retry automatically.
Explicit Continue prepared ordinary review and waited for Swipe To Pay. The manual swipe, fee warning and PIN reached Subscribed. The fork recipient increased from 4 to exactly 6 USDT. This case expires the native fee authorization; signed Paykit conversion-term expiry is covered by the retained report below.
t4a-2x.2x.mp4
t4b-2x.2x.mp4
Test 5: Editable amount and enlarged review ✅ Ordinary amount editing and review controls worked at 150% text size.
Prepared 1.5 USDT and expanded details. The recipient, fee note, details toggle and Swipe To Pay stayed visible above the gesture bar. After hiding details, tapping the amount returned to entry and accepted an edit to 1.52 USDT. Closed without sending; fork balances stayed unchanged.
t5-2x.2x.mp4
Retained coverage for contracts this delta preserves:
- The 13-case device baseline at 1f55d03 covers the remaining journey branches, including Bitcoin first payment, no initial payment due, insufficient funds, warning cancellation, conversion-policy guards, signed Paykit quote expiry and monthly cost.
- The paid-history checks at 19e3a31 cover unpaid expiry through refresh, restart and rollover, paid-only subscription details and retained creator proof history.
- The quote-replacement check at d5ae1af covers rejection of replaced terms and explicit recovery.
No planned cases remain unrun.
Twin synonymdev/bitkit-ios#1006 fix: authorize initial USDT subscriptions once at ac7e2fd matches the eligibility and paid-history contracts and has byte-identical journey text. This pass compared those contracts in source; iOS devices were not repeated.
Delta review covers 844d295…c8a6466, including both master integrations and their conflict resolutions. Other inherited master features and documentation received source review without separate device cases.
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: Re-reviewed the full PR at c8a6466 after the comparison base changed from 22a5cb5 to 0f44d936 and no verified baseline coverage was supplied. This pass covers the complete diff against merge base 0f44d93, with no inherited coverage.
No new actionable code findings.
Eligible initial USDT subscriptions open review and confirm once, only for USDT with no conversion or USD with a fixed 1:1 USDT rate. The authorization flag is consumed before confirm, so a cancelled PIN, a failed quote, or leaving during preparation does not start another automatic payment. Pay from Payments leaves the initial-subscription flag unset, so that retry stays on Amount, Continue, and an explicit swipe. A locked amount tap stays on review, and Back closes the sheet when review is the only destination. Expired unpaid periods of an active subscription move into payment history with no Pay action and stay out of subscription details, which list only periods with payment evidence. The same eligibility, one-shot confirm, expired-history, and paid-only detail contracts match bitkit-ios#1006 at ac7e2fd.
Device testing: not performed in this review. The changed unit tests were inspected and not executed here. Earlier device results on this PR stay tied to the revisions named in those reviews. Ready for device testing.
Reviewer model: grok-4.7
Reasoning effort: high
piotr-iohk
left a comment
piotr-iohk
left a comment
There was a problem hiding this comment.
Approved at c8a6466d0234ad545da1b1f26a0725d82de9d6ba based on the current full code review and current-head funded device and regression evidence, with linked baseline evidence for unchanged subscription cases. Head/base, CI/E2E and review threads rechecked before approval; no failed or pending checks, unresolved threads or active changes requests.
This branch has not been deployed
5 participants
Fixes #1540
Description
Continues eligible initial USDT payments directly from Subscribe and reuses subscription progress and success views. Eligibility is limited to USDT-only requests with unchanged amounts: USDT without conversion or USD with an explicit fixed 1:1 USDT rate.
Retains quote preparation, fee checks, payment authentication, warnings, deadline validation and duplicate-payment protection. Other conversion terms still require amount/payment review; failed or cancelled preparation never automatically retries. Manual preparation and retries keep the normal Amount → Continue → payment review flow without automatic-payment progress. Tapping a locked amount keeps review open; Back still closes it, and ordinary send amounts remain editable.
Shows Waiting for payment for payable unpaid periods. Expired unpaid periods of active subscriptions stay in Payments and retain their details across refresh and restart, without a Pay action. Paid history and cancellation behavior are preserved. Subscription details list only paid periods, so an expired unpaid request never appears as a sent payment.
Out of Scope
Design
Reuses Review & Subscribe and Subscribed, with existing USDT styling.
Preview
Existing subscription layouts; this removes the extra payment swipe. External device review at
c8a6466d0includes recordings of initial payment, PIN cancellation/manual retry, expired-fee recovery and enlarged review using local-fork test USDT.QA Notes
Setup
Use app revision
c8a6466d0and its pinned dependencies. For the device cases, enable USDT and use two linked test identities with an eligible incoming USDT-only subscription. Follow the local USDT fixture prerequisites; the standard Bitcoin regtest setup does not provide controlled or funded USDT. The required local gateway/RPC/bundler fixture, owned test funds and billing clock were not available for the author’s integration check. A subsequent external review used two Android 15 / API 35 emulators and local-fork test USDT at this exact revision; its completed cases and remaining coverage limits are attributed below. Do not reset an existing wallet.Journeys
review-and-subscribe.xml— initial USDT payment, authentication cancellation and locked-amount navigation, insufficient funds, an unpaid-period retry, expiry/history restoration, and exclusion of unpaid periods from subscription payments using local test funds.Manual Tests
External Device Evidence
Review 5622474755 tested unchanged app sources at
c8a6466d0234ad545da1b1f26a0725d82de9d6baon two Android 15 / API 35 emulators with local-fork test USDT and independent balance checks. The reviewer reported no new findings:QuoteExpiredand unchanged balances. Restoring the provider did not retry; explicit manual authorization recovered with exactly one payment.The reviewer retained earlier device evidence for unchanged contracts: the 13-case baseline at
1f55d03, history checks at19e3a31, and replacement rejection atd5ae1af. These are historical revisions, not complete device reruns atc8a6466d0. The history deadline checks used a billing-clock fixture; replacement used a disposable in-memory terms hook rather than signed replacement terms arriving over the wire.J1 remains unchecked for a full current-head device matrix. In particular, successful automatic initial payment with PIN approval was not exercised by this new review: its automatic success used PIN disabled, while PIN success followed explicit manual retry. Physical-device biometrics and fresh iOS funded/provider execution remain unverified; the reviewer compared iOS contracts in source only. The native fee-expiry case does not replace current-head signed Paykit conversion-term expiry/replacement validation.
Automated Checks
AppViewModelSendFlowTest.kt— initial-only eligibility, conversion/deadline rejection, direct routing and one-use authorization; mixed accepted methods still require selection when only USDT resolves.UsdtViewModelTest.ktcases through the actual review preparation helper: quote-before-authorization, authentication cancellation/manual retry, insufficient balance/expired quote recovery, cancelled preparation and duplicate prevention. Delayed manual preparation stays unauthorised and does not show automatic-payment progress.PaykitPaymentRequestRepoSubscriptionTest.kt— real expiry timer, refresh/restoration, next-period isolation, and the production subscription-detail selection: unpaid history is excluded, paid payer rows and creator receipts are retained. All 48 repository subscription tests pass.PaymentRequestPresentationTest.kt— payable and expired subscription periods, unavailable periods and submitted proofs.just compile,just testatc8a6466d0after integrating master0f44d936— all 4,591 tests passed (zero failures, errors or skips); existing USDT tests cover expiry, insufficient funds, authentication cancellation, retries and duplicate prevention.UsdtPaymentContentTestinstrumented tests before the history-only follow-up, including locked-amount taps, explicit Back and editable amount navigation.just lintatc8a6466d0— complete Detekt and Android Lint reports contain zero findings; master’s removal of the lint baseline is preserved. Full diagnostics were inspected.The master integration preserves lifecycle-scoped payment-review tracking together with the subscription authorization flow, busy/PIN navigation guards, cancellation and explicit retry. It also retains both parents' locked-amount, dot-decimal and unknown-versus-zero/privacy UI tests, and master's shared received artwork. The exact tested tree passed a fresh independent correctness/quality review. No tests were added or removed by the conflict resolution.
Full diagnostics and structured test/lint reports were inspected. Manifest removal, Gradle deprecation and Robolectric cursor warnings trace to files/config byte-identical to actual master. No new lint or formatting findings remain.
compileDevDebugAndroidTestKotlinalso passed; the merged instrumented tests were compiled, not executed in this integration. Earlier instrumented results above remain evidence for their original revision. The subsequent external current-head device checks above are separate from these author-run integration checks; full J1 and the signed-term expiry/replacement matrix remain unchecked for this exact head.Models used
gpt-6-astra(reasoning:xhigh).gpt-6-astra(reasoning:xhigh).gpt-6-astra(reasoning:xhigh).