Skip to content

fix: apply fixed btc prices with paykit rc71 - #1437

Merged
ovitrif merged 5 commits into
masterfrom
codex/paykit-rc71
Oct 8, 2026
Merged

ovitrif merged 5 commits into
masterfrom
codex/paykit-rc71

Conversation

@ben-kaufman

@ben-kaufman ben-kaufman commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Twin: synonymdev/bitkit-ios#887

This PR updates Paykit to rc71 and applies fixed BTC prices to incoming one-time requests.

Description

  • Calculate BTC owed from immutable request rates, including explicit same-asset prices. Rail-specific rates take precedence over asset-wide rates.
  • Use the calculated satoshi amount for the existing confirmation, invoice checks and payment validation. Fiat estimates still use Bitkit's market rate; original Paykit terms are unchanged.
  • Reject unsupported or unrepresentable pricing instead of treating the requested denomination as BTC. Both platforms bound combined normalized operand precision to 38 digits before multiplying, preserving exact rounding.
  • End preparation with unavailable-payment feedback when an endpoint is not payable. Keep the request pending for manual retry without blocking other requests.

Out of Scope

  • Non-BTC settlement and conversion-based subscriptions.
  • Dynamic quotes, differing prices across available BTC rails, and Lightning amounts that cannot be paid in whole satoshis.

Design

N/A - existing payment UI is reused.

Preview

Not captured. The added journey verifies the amounts in the existing confirmation sheet.

QA Notes

Journeys

  • J1 fixed-price-bitcoin.xml - reviewer device results cover USD-to-BTC, same-asset rail pricing and local fiat display, with additional funded on-chain checks. Exact source provenance was not independently verified.
  • J2 unpayable-endpoint.xml - verify rejected-destination feedback, manual retry and a later payable request on device.

Manual Tests

  • J1 setup: start and link a Paykit rc71-or-newer issuer that can send conversion.fixed, such as the standalone fixture sender. The basic BTC fixture and Bitkit Request UI cannot issue these terms.
  • J2 setup: prepare a one-time address already recorded as paid by this payer, and an issuer able to bind a new request to it and another to a fresh unused address. The journey sends no additional payment.

These prerequisites are not provided by the base journey environment. Without them, report the corresponding journey as blocked, not failed or passed.

Automated Checks

  • updated PaykitPaymentRequestRepoTest.kt - cover fixed USD/USDT-to-BTC amounts, same-asset prices, rail precedence, endpoint filtering, exact rounding and precision boundaries, invoice/payment matching, invalid rates, overflow and unsupported subscriptions.
  • updated AppViewModelSendFlowTest.kt - cover closing rejected-endpoint preparation, feedback, manual retry and progress to another payable request.
  • ran release-artifact verification: Remote Maven AAR matches the published rc71 metadata; no local SDK override.

The reviewer confirmed 21,000 / 50,000 / 50,001 satoshi recipient outputs and one matching issuer proof per payment. These are focused on-chain results, not a full device pass. Lightning and the corrected used-address presentation still need device testing.

Local validation: all 3,594 unit tests passed on the final edits. Compilation passed;
all 19 Detekt findings match the baseline, with no introduced compiler or formatting diagnostics.

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[High risk] Updates a payment library dependency.

The PR appears safe to merge, with no actionable issues found.

Summary

Updates com.synonym:paykit-android from 0.1.0-rc70 to 0.1.0-rc71.

  • No other dependency declarations or app code change.
  • No actionable issues were found.
  • The PR reports passing unit tests and artifact checks; these were not independently rerun.

Reviews (1) · Last reviewed commit: "chore: update paykit to rc71" · Reviewed by Greptile

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from dc54e6e (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

@ben-kaufman
ben-kaufman requested a review from ovitrif October 7, 2026 20:39
ovi-reviewer[bot]
ovi-reviewer Bot previously requested changes Oct 7, 2026

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ⛔️ Request Changes

Review: diff 1 file.
synonymdev/bitkit-ios#887 pins the same rc71 release, while iOS master still uses rc70 from synonymdev/bitkit-ios#886. Both apps derive BTC payment amounts without retaining conversion terms, so explicit BTC pricing requires the same intake protection on both platforms.

Findings:
1 inline (1 HIGH)

QA:
No tests ran because the description reports no user-visible change and marks journeys and manual tests N/A.


Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

Comment thread gradle/libs.versions.toml
@ben-kaufman ben-kaufman changed the title chore: update paykit to rc71 fix: apply fixed btc prices with paykit rc71 Oct 7, 2026

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: 👍 Approve

Reaudit: diff 8 files.
New findings: 3 inline (3 LOW); the rest is in the review.
synonymdev/bitkit-ios#887 implements the same fixed BTC pricing, rounding limits and conversion-subscription rejection. Its paired journey has the same filename, name and action prose.

QA:
Tests queued.

Note

Retest Suggested J1

@ovi-reviewer retest J1

Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

Comment thread app/src/main/java/to/bitkit/repositories/PaykitBitcoinRequestPricing.kt Outdated
Comment thread app/src/test/java/to/bitkit/repositories/PaykitPaymentRequestRepoTest.kt Outdated
Comment thread journeys/payment-requests/fixed-price-bitcoin.xml Outdated
@ovi-reviewer
ovi-reviewer Bot dismissed their stale review October 7, 2026 21:49

addressed - reaudit confirmed

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: 👍 Approve

Tests for the review.

QA:
Tested on Android 15 emulator.
0 of 1 passed; blocked by qa env setup.

🟠 Test J1
Test J1

Blocked: Not run: the issuer cannot send fixed conversion rates.



Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review

Scope: full PR diff against 484e6dd, reviewed at c36ee53, including pricing, affected payment paths and the rc71 SDK contract.

No new actionable code findings.

The earlier overpayment concern is verified fixed. Targeted comparison with iOS #887 at bfa8db2 found matching pricing and journey contracts; shared platform behavior depends on that companion landing.

Validation: the exact-head CI report records 3,591 passing unit tests, including the four added pricing regressions. Locally, 2,028 isolated Kotlin helper checks passed with data-only SDK types. The local app test run stopped before compilation because its JetBrains 21 toolchain URL returned HTTP 400. The published Maven AAR could not be independently inspected (HTTP 401); rc71 source and FFI contracts were inspected.

Recommended before device testing: provision a linked issuer that can publish fixed conversion terms. The reported J1 attempt could not run because its issuer lacked that capability.

Device testing: not performed in this review.

Suggested additional test cases

  • Android/regtest: with distinct linked identities and funded wallets, issue separate 10 USD requests at btc: 0.000021, then pay through on-chain and Lightning endpoints. Expect 21,000 sats received per request, fees charged separately, and one matching proof per request with the original USD terms retained.
  • Android/regtest: use the same pricing with an LNURL endpoint whose first invoice callback fails before dispatch. Restart, restore the callback and retry the accepted request. Expect the amount to remain 21,000 sats and exactly one payment/proof after recovery.

@piotr-iohk

piotr-iohk commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Tested the on-chain fixed-price flow on Android using the standalone issuer from bitkit-e2e-tests tool PR #269. Paired testing: other platform PR.

Case Issuer terms Checked result
USD 10 USD, btc=0.000021 21,000 sats
BTC rail precedence 0.001 BTC, btc=2, btc-regtest=0.5 50,000 sats
Rounding 1 USD, btc=0.000500005 50,000.5 rounds up to 50,001 sats

For each payable request, obtained an unused address from Receive, checked confirmation/success amounts, verified the exact confirmed recipient output, and verified exactly one issuer proof matching the transaction. Fees were separate. Fiat display used Bitkit's local market valuation.

Also checked USD without a conversion rate (unsupported_asset, no payable request/spend), dismiss/reopen with the amount preserved, and relaunch with paid requests not offered again and activity retained.

Observation reproduced on both platforms: Sending a new request to an already-paid one-time on-chain address leaves the confirmation spinning in preparation. Logs report endpoint_not_payable, but the UI does not stop loading or surface an unavailable-payment error. Rejecting the used endpoint appears intentional; whether these PRs introduced the spinner is not established. No second payment was made.

ANDROID payment request stuck preparing after endpoint_not_payable

Session log archive is prepared; attachment upload is currently blocked. I will add it to this comment once uploaded.

Fixed-price Lightning was intentionally excluded. Installed apps match the user's built AUTs by checksum; exact source provenance was not independently verified. iOS USD/rounding were repeated after log rotation to retain complete logs for those cases. These are focused on-chain results, not a full PR device pass.

Copy link
Copy Markdown
Contributor Author

@piotr-iohk The small code/test/journey cleanups are addressed in f6dfb3a. Both issuer guides now document the shared 38-significant-digit product limit and the extra exact values iOS can accept. This documents existing behavior; it does not change pricing or rounding.

Final validation: 3,591 unit tests and compilation passed; all 19 Detekt findings match existing findings, with no introduced compiler or formatting diagnostics. Both XML copies validate. The fixed-rate issuer/device and funded recovery cases remain unrun; they are not being counted as passes.

@ben-kaufman
ben-kaufman requested a review from piotr-iohk October 8, 2026 13:04
@piotr-iohk

Copy link
Copy Markdown
Collaborator

@ben-kaufman Could you clarify the spinner observation reproduced on both platforms? A request using an already-paid on-chain address returns endpoint_not_payable, but the confirmation keeps loading without showing an error. Should this be addressed in these PRs, or tracked separately as an existing issue?

@ovi-reviewer ovi-reviewer Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: 👍 Approve

Reaudit: diff 4 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-ios#887 carries the same lowercase journey name and action prose, and both issuer guides document Android's 38-significant-digit product limit and iOS's wider exact range.

QA:
Tests running.

Replies:

@piotr-iohk: Tested the on-chain fixed-price flow on Android using the standalone issuer (comment)

@piotr-iohk I have retained your focused on-chain results and the issuer tool in synonymdev/bitkit-e2e-tests#269 for J1. Your report does not independently establish the apps' source commits, so it does not certify this head. The used-address spinner is outside this delta: endpoint preparation and its UI handling are unchanged.


Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

Copy link
Copy Markdown
Contributor Author

@piotr-iohk Fixed here in f8ee077, with the matching change in iOS #887 (cdb692ad).

The retry path already existed in master: endpoint_not_payable was treated as waiting for payment details, so automatic preparation kept retrying with the sheet loading. It now closes preparation and shows the existing unavailable-payment message. The request remains pending for manual retry, and another payable request can proceed. Used-address rejection and transport-error retries are unchanged.

Validation passed all 3,593 unit tests and compilation, with no introduced lint/format findings. The paired unpayable-endpoint.xml device check is still unchecked; please retest with your used-address fixture.

Also corrected the QA notes: your confirmed on-chain outputs and matching proofs are valid focused evidence. My earlier statement that funded conversion checks were outstanding missed your report posted during validation.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at f8ee077 (includes "stop preparing unpayable payment requests"). No HIGH/MEDIUM. Three LOW inline comments, all docs: the parse-reason vocabulary in docs/payment-requests.md, the journey's issuer precondition, and the interop doc's fixture claim (which also carries the "iOS accepts exactly" sentence that synonymdev/bitkit-ios#887 shows to be wrong). Each can be fixed here or moved to a follow-up issue, your choice; please reply on each thread with which. This is a COMMENT rather than an approval only because the device gate could not run here and CI was still pending at posting time; nothing in the code blocks.

Checked and clean:

  • Fixed-price parse path (PaykitPaymentRequestRepo.kt:1781-1799 → PaykitBitcoinRequestPricing.bitcoinPayment): rail > asset-wide > parity precedence, cross-asset unpriced rails dropped, duplicate/empty/zero/sign/exponent/whitespace/>80-char/>38-digit operands rejected, exact BigDecimal multiply with a 38-digit product guard, Lightning whole-sat requirement, ULong.MAX_VALUE/1000 cap on both branches, differing per-rail amounts rejected. Non-conversion requests take the old path unchanged.
  • Approve-time amount equals pay-time amount: SendConfirmScreen.kt:354 renders preparingRequest.amountSats; AppViewModel.kt:4772-4794 refuses to send unless uiState.amount == request.amountSats and the bolt11 msats equal sats×1000 (strict equality, Repo.kt:136-142). The original amountValue ("10" USD) only reaches the SDK when no request is attached (PrivatePaykitRepo.kt:593, :664-677).
  • toPaykitSubscription returns null for any conversion terms (PaykitSubscription.kt:301), so no renewal/auto-pay path can use a priced request.
  • f8ee077: the NotOpened branch (AppViewModel.kt:1325-1329) runs inside finishIncomingPaymentRequestPreparation, which is guarded by isCurrentPaymentRequestPresentation(request, generation, preparation), so finishPaymentRequestPreparation(paymentRequestPreparation.value) at :1472 closes this request's sheet only; the id goes into dismissedPreparingRequestIds so it does not auto-reopen, and the explicit Pay path removes it again (:5989). Covered by the two new AppViewModelSendFlowTest cases.
  • Line-for-line parity with synonymdev/bitkit-ios#887 except that iOS lacks the product-precision guard; both journey files and the action prose are identical. rc70→rc71 adds no storage format; v2.5.0 pins rc51 with no conversion field, so no migration finding.

Device gate: not run — needs an issuer that can send conversion.fixed and an already-paid one-time address (neither is in the Capabilities table; the documented paykit-server fixture cannot). @piotr-iohk drove the two fixed-price-bitcoin.xml cases and a rounding case at c36ee53 with the standalone issuer from synonymdev/bitkit-e2e-tests#269 (#1437 (comment)); the commits since are a constant reference, docs, the journey name and the unpayable-endpoint change, which has no device run yet. unpayable-endpoint.xml: not run.
Skipped — covered by CI at f8ee077: unit tests, detekt, e2e (pending at posting time).
The endpoint_not_payable spinner @piotr-iohk reported is answered by f8ee077; left to that thread.

Comment thread journeys/payment-requests/README.md
Comment thread docs/paykit-issuer-interoperability.md Outdated

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: 👍 Approve

Reaudit: diff 5 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-ios#887 carries the same immediate unavailable-endpoint feedback and identical unpayable-endpoint.xml journey.

QA:
Tests queued.
Test J1 failed at c36ee53 (review).

Replies:

@ben-kaufman: Fixed here in f8ee077, with the matching change in iOS #887. (comment)

@ben-kaufman I verified that rejected endpoints now close preparation, retain manual retry and allow the next payable request. The used-address device check remains pending.

Note

Retest Suggested J1

@ovi-reviewer retest J1

Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@piotr-iohk

Copy link
Copy Markdown
Collaborator

Logs from previous test session:
android-session-logs.zip

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: 👍 Approve

Reaudit: diff 5 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-ios#887 fixes the same loading issue for unpayable endpoints and adds the matching unpayable-endpoint.xml journey.

QA:
Test J2 needs an issuer that can reuse a paid address. Test J1 was validated by ben-kaufman.

Note

Retest Suggested J1

@ovi-reviewer retest J1

Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

Copy link
Copy Markdown
Contributor Author

@piotr-iohk Checked the attached logs: they confirm repeated endpoint_not_payable on the already-paid address, which the preparation-feedback fix addresses. They also contain SharedStateBusy during the 12:16 relaunch; that is separate from fixed-price calculation.

5c3637d now matches the conservative precision guard in iOS #887. All 3,594 unit tests pass, with no introduced lint/format findings. The paired docs and PR prerequisites explicitly require the conversion-capable issuer from bitkit-e2e-tests#269: the earlier J1 run blocked by the basic issuer is not evidence of an app failure. Your later focused funded results remain recorded, and J2 remains pending a fresh used-address retest.

@ben-kaufman
ben-kaufman requested a review from jvsena42 October 8, 2026 13:49

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked at 5c3637d: no findings.

  • PaykitBitcoinRequestPricing.kt: the combined precision bound (amount + rate ≤ 38 after stripTrailingZeros()) now runs before the multiply and matches the iOS rule in synonymdev/bitkit-ios#887, so both platforms reject the same quotes.
  • docs/payment-requests.md lists unsupported_pricing and describes the endpoint_not_payable behaviour.
  • The interop doc no longer claims an iOS-only range and says fixed pricing is covered by the matching test tables, not the JSON fixture.
  • The journey README and both journey descriptions state the rc71 conversion.fixed issuer and the paid-address fixture, and say to report the journey as blocked without them.

Device gate: not run — fixed-price-bitcoin.xml and unpayable-endpoint.xml need a conversion-capable issuer and a paid-address fixture that are not available here. Build, lint and detekt pass at this head; e2e was still pending.

ovi-reviewer[bot]

This comment was marked as duplicate.

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: 👍 Approve

Reaudit: diff 7 files.
No new findings; the rest is in the review.
Pair PR synonymdev/bitkit-ios#887 applies the same precision limit before multiplication and adds matching boundary cases. Both journey files are identical.

QA:
J2 needs a controlled issuer that can reuse a paid address.
J1 was validated by ben-kaufman.

Note

Retest Suggested J1

@ovi-reviewer retest J1

Reviewed by gpt-6.1-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest

@piotr-iohk

piotr-iohk commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

Spinner fix retested on Android at 5c3637db using a locally built network-regtest app and the linked issuer from bitkit-e2e-tests #269.

Passed the unpayable-endpoint.xml checks:

  • A new request bound to a previously paid address ended preparation with “The payment request is no longer available.”; the loading confirmation closed.
  • The request stayed pending. One controlled manual retry again showed unavailable feedback and closed preparation.
  • A subsequent request using a fresh address reached a ready 50,000-sat confirmation while the rejected request remained pending. Dismissed without paying.
  • No automatic reopening observed afterward. Wallet sat balance/activity stayed unchanged; issuer proofs were zero for the retest requests, and both fresh addresses had zero chain/mempool transactions.

The earlier attempt affected by manual taps was dismissed and excluded; this verdict uses a new request observed without intervening taps.

Screenshots below show unavailable feedback and the subsequent ready confirmation. Full retest logs were captured and archived locally for manual attachment. Fixed-price Lightning was not tested. The separate transport/concurrent_update observation is not claimed fixed.

manual-unavailable fresh-ready

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved from our reviewed and tested scope at 5c3637d. Follow-up check of the precision guard and unpayable-endpoint preparation cleanup found no new actionable concern; the paired spinner retest passed. Earlier focused BTC/USD on-chain payments had verified recipient outputs and matching issuer proofs. Fixed-price Lightning remains untested, and the separate transport observation is unresolved. CI completion remains a separate merge check.

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck

@ovitrif
ovitrif merged commit 35ddc1d into master Oct 8, 2026
21 checks passed
@ovitrif
ovitrif deleted the codex/paykit-rc71 branch October 8, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants