Skip to content

fix: align receive liquidity - #1248

Open
pwltr wants to merge 2 commits into
masterfrom
fix/receive-liquidity-parity
Open

fix: align receive liquidity#1248
pwltr wants to merge 2 commits into
masterfrom
fix/receive-liquidity-parity

Conversation

@pwltr

@pwltr pwltr commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up from synonymdev/bitkit-ios#711 (comment)

This PR:

  1. Aligns Receive Lightning invoice eligibility with the shared receive-liquidity spec by using ready-channel inbound capacity for normal invoice creation, Auto availability, and Spending CJIT routing.
  2. Clears receive-session CJIT invoice/details state when editing or starting a fresh Receive session so immutable CJIT invoices are not reused after amount changes, including CJIT QR edits that can return to a normal BOLT11 invoice when the new amount fits inbound liquidity.
  3. Validates CJIT quotes before confirmation UI and maps node-wide Blocktank capacity failures to a separate user-facing error instead of the per-channel maximum message.
  4. Updates the platform-agnostic receive-liquidity spec and focused unit coverage for the receive decision matrix, CJIT quote validation, CJIT session state, and Blocktank error classification.

Design

N/A — no design available.

Preview

Screen.Recording.2026-09-10.at.19.48.49.mov

QA Notes

Manual Tests

  • 1. Receive → Spending CJIT QR → Edit Invoice with an amount that fits ready inbound liquidity → QR: returns to Spending, shows the newly generated normal LN invoice, and does not show the previous CJIT invoice.
  • 2. Receive → Spending CJIT QR → Edit Invoice with an amount that still needs additional liquidity → Confirm → QR: shows a fresh Spending-only CJIT invoice and does not expose the old CJIT invoice.
  • 3. Receive → create or view CJIT QR → close sheet → reopen Receive: starts from fresh Receive state with no CJIT invoice/details preserved.
  • 4. regression: ready channel with inbound > 0 but peer not yet usable → Receive: normal LN invoice/Auto availability follows ready inbound capacity.

Automated Checks

  • Unit tests added: CjitQuoteValidatorTest.kt covers invalid fee/net quote cases and a valid quote.
  • Unit tests added: CjitEntryDetailsTest.kt covers confirmation-entry mapping and quote rejection before confirmation UI.
  • Unit tests updated: ReceiveInvoiceEditStateTest.kt covers clearing stale CJIT invoice state on edit and fresh CJIT creation.
  • Unit tests updated: ReceiveLiquidityDecisionTest.kt and WalletRepoTest.kt cover ready-channel receive eligibility and ready inbound liquidity.
  • Unit tests updated: BlocktankRepoTest.kt covers node capacity error classification separately from per-channel max-size errors.
  • Local checks:
    • ./gradlew testDevDebugUnitTest --tests to.bitkit.models.ReceiveLiquidityDecisionTest --tests to.bitkit.models.CjitQuoteValidatorTest --tests to.bitkit.ui.screens.wallets.receive.ReceiveInvoiceEditStateTest --tests to.bitkit.ui.screens.wallets.receive.CjitEntryDetailsTest --tests to.bitkit.repositories.WalletRepoTest --tests to.bitkit.repositories.BlocktankRepoTest
    • ./gradlew compileDevDebugKotlin

@pwltr
pwltr force-pushed the fix/receive-liquidity-parity branch from 884ef2e to f604acd Compare September 10, 2026 17:36
@pwltr
pwltr marked this pull request as ready for review September 10, 2026 17:46
@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge, with the receive-liquidity, CJIT lifecycle, validation, and error-presentation changes remaining internally consistent.

Summary

  • Uses ready channels consistently for normal Lightning invoice eligibility, Auto availability, and additional-liquidity decisions.
  • Clears old CJIT invoice and quote details when editing or beginning a fresh receive session.
  • Rejects CJIT quotes that cannot produce a valid positive net receive amount.
  • Adds dedicated presentation for node-wide capacity failures and focused tests for the updated decision paths.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Open Receive] --> B[Clear receive-session CJIT state]
    B --> C{Ready inbound capacity covers amount?}
    C -->|Yes| D[Create normal Lightning invoice]
    D --> E[Show Auto or Spending QR]
    C -->|No| F{Spending receive needs CJIT?}
    F -->|No| G[Show Savings/on-chain request]
    F -->|Yes| H[Request CJIT quote]
    H --> I{Quote valid?}
    I -->|No| J[Show quote or capacity error]
    I -->|Yes| K[Show CJIT confirmation]
    K --> L[Confirm fresh CJIT invoice]
    L --> M[Show Spending-only CJIT QR]
    M -->|Edit| N[Clear immutable CJIT state]
    N --> C
Loading

Reviews (1) · Last reviewed commit: "fix: align receive liquidity with spec"

@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.

Funds-focused review. No sats are at risk anywhere in this PR, and three of the four iOS-parity gaps I'd recorded for Android are closed:

  • validateCjitEntry twin — closed. CjitQuoteValidator (models/CjitQuoteValidator.kt:17-24) rejects feeSat >= invoiceSat before subtracting, then channelSizeSat < invoiceSat - feeSat, all in ULong — the guard makes the subtraction underflow-proof. Placement is what matters and it's right: it runs inside BlocktankRepo.createCjit (:283-287) right after the only coreService.blocktank.createCjit call in the app, and both UI callers go through BlocktankViewModel.createCjit → repo. CjitEntryDetails.from (ReceiveConfirmScreen.kt:208-223) is the only production constructor of the confirm model, so the old signed-Long path at :83 can no longer render a negative "you will receive".
  • Stale CJIT after editing, ready-channel half — closed, by a different mechanism than iOS: clearing eagerly rather than replacing the route on completion. That eagerness is the problem inline.
  • Stale channel cache in canCreateLightningInvoice — closed, not reintroduced. currentChannels() (WalletRepo.kt:771-773) reads live getChannels() when the node is running; this PR only swaps isUsableisChannelReady (:776), and calculateRemoteBalance() already filtered on isChannelReady, so the liquidity number is consistent.

Also checked clean: the hardware address is only emitted under ReceiveTab.TREZOR and CJIT only under SPENDING (ReceiveInvoiceUtils.kt:40-52), with AUTO hidden while a CJIT invoice exists — no wrong-destination display. All new arithmetic is ULong behind the underflow guard. Session reset per presentation is guaranteed by key(receiveSheetPresentationKey(sheet)) plus LaunchedEffect(startRoute) { clear() }.

Still open at LOW — zero-inbound first-channel half. With no channels, Edit on the initial CJIT QR → Continue returns None via (inboundCapacitySats ?: 0uL) == 0uL (ReceiveLiquidityDecision.kt:45), so the CJIT is cleared, the typed amount only reaches the bip21, and ReceiveQrScreen.kt:211-217 flips to Savings. "Receive on Spending" then opens ReceiveAmountScreen empty — it uses a fresh hiltViewModel() AmountInputViewModel whose only setSats calls are the max clamp and the min button. Same ~3-line fix as I suggested on iOS: prefill from walletState.bip21AmountSats in a LaunchedEffect, since both the None and ChooseAmount branches already persist the typed amount there. (Not in the diff, so noting it here.)

Requesting changes for one regression — inline. It's in core receive rather than Paykit, so it isn't gated.

Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveSheet.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/EditInvoiceScreen.kt Outdated
@pwltr
pwltr requested a review from jvsena42 September 11, 2026 13:19

@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.

Finding A is fixed79dd4c3bf drops the start-of-edit clear (beginReceiveEdit() is gone from :155-164 and deleted) and clears on completion in updateInvoice (:365-368). Lifting my REQUEST_CHANGES.

I walked every path in EditInvoiceScreen.kt:116-154 to check the stale-invoice bug this PR originally fixed stays closed:

Path Edit applied? CJIT
None yes cleared via updateInvoice → the "edit down with a ready channel" bug stays closed
ChooseAmount yes cleared, then Amount
CreateCjit success on Confirm onCjitConfirmed sets the new invoice
CreateCjit failure / GeoBlocked no old, still-valid CJIT kept — correct
Trezor on-chain edit on-chain only kept — correct
Back from Edit without applying no kept — the bug is fixed

Per-presentation scoping still holds: cjitSessionState is still remembered inside ReceiveSheet (:81) under key(receiveSheetPresentationKey(sheet)), with a fresh UUID per Sheet.Receive, and no CJIT state moved into WalletRepo/WalletViewModel. The only new repo writes are bip21AmountSats/bip21, which every sheet open resets via refreshReceiveState(), so nothing leaks into the next presentation.

B is addressedupdateOnchainBip21Amount keeps the Savings QR on the approved amount. C is addressedgetOrThrow() now sits inside runSuspendCatching (EditInvoiceScreen.kt:133-135). The zero-inbound prefill is still open, but I'd marked it optional.

Three LOW notes inline, none blocking.

private set

fun onCjitCreated(entry: CjitEntryDetails) {
cjitInvoice = null

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.

Low, and a both-platforms follow-up rather than something to fix here alone: Confirm→Back after an Edit-driven CJIT still loses the CJIT the user already showed, and now leaves Savings at the declined amount.

A CJIT QR at 20k is shown to a payer → Edit → 100k → Continue → CreateCjitnavigateReceiveConfirm (:384-390) runs updateOnchainBip21Amount(100k), then onCjitCreated nulls cjitInvoice here — both before the user confirms. They see the fee, decline, Back → Back. The QR comes back with cjitInvoice == null: ReceiveRoute.QR is a data object, entryDetails now holds the declined quote, and cjitEntries is only read by ChannelDetailScreen. bip21AmountSats is 100k, above ready inbound, so Spending shows CJIT onboarding and the tab flips to Savings, which encodes 100k. The Amount-screen path (onCjitCreated at :268, reached when creation fails) nulls it the same way.

It's a regression against Android master, where neither call touched cjitInvoice — it was only replaced at Confirm (master :287, :302). The two halves came from different commits: the null from f604acdf2, hidden behind A until 79dd4c3bf removed A; and the Savings-at-100k half from 79dd4c3bf, which followed my own suggestion on :384 to set the bip21 amount before navigating. My last walkthrough also cited this null as the guard keeping the CreateCjit path clean — true for the success case, but I didn't walk the decline.

Why this isn't blocking: iOS does exactly the same thing. For CreateCjit, ReceiveEdit.swift:244-246 calls finishWithRoute(.cjitConfirm)replaceEditedQrRoute, which removes the stale CJIT QR at creation, not at Confirm — and testReplacingEditedQrRouteCanNavigateToFreshCjitFlow (ReceiveEditTests.swift:40-50) pins that. iOS also sets invoiceAmountSats before branching (:177), so its Savings shows the new amount after a decline too. A was blocking partly because Android diverged from iOS; blocking an alignment PR in order to make it diverge would be backwards. It's also narrower than A — CreateCjit needs ready inbound that's non-zero but below the new amount (ReceiveLiquidityDecision.kt:43-45), so zero-channel first-CJIT users can't reach it — and no sats are at risk, since the old invoice stays payable.

If you want it fixed, do it on both platforms: replace at Confirm, not at creation. Drop cjitInvoice = null here (onCjitConfirmed overwrites it anyway) and move updateOnchainBip21Amount into the ConfirmIncreaseInbound onContinue (:306). Two caveats: onContinue isn't a suspend function, so call onCjitConfirmed and navigate synchronously and launch the bip21 update separately; and ConfirmIncreaseInbound is also reached from the Amount screen, so gate or accept that.

Related, introduced by 79dd4c3bf: wrapping navigateReceiveConfirm in scope.launch means isCreatingCjit = false (EditInvoiceScreen.kt:147) runs before navigation, so a second tap during the DataStore write re-runs createCjit and leaves an orphan LSP quote, overwriting entryDetails. navigateTo is single-top so there's no duplicate screen, and no sats are at risk. Making the navigation synchronous again — which the fix above does — removes it.


fun setBip21AmountSats(amount: ULong?) = _walletState.update { it.copy(bip21AmountSats = amount) }

suspend fun updateOnchainBip21Amount(amountSats: ULong?): Result<Unit> = withContext(bgDispatcher) {

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.

Low, latent: this moves bip21AmountSats but leaves walletState.bolt11 at the old amount.

updateBip21Invoice clears bolt11 when the amount can't be covered (:755-756); this rebuilds bip21 without lightning= but never clears bolt11. So after a CreateCjit edit the wallet holds bolt11 at 20k and bip21AmountSats at 100k.

It's masked today: CreateCjit only fires when the amount exceeds ready inbound, so canCreateLightningInvoice is false and getInvoiceForTab hides the stale bolt11, and a ChannelReady regenerates it. It would surface only if ready inbound grew without a ChannelReady (say, after an outbound payment) while the QR sits with cjitInvoice == null — i.e. after the Confirm→Back above — at which point Spending would show a payable 20k bolt11 under a 100k Savings request.

A setBolt11("") in here matches updateBip21Invoice's cannot-cover branch, and the existing test's never().createInvoice assert still holds.

}

@Test
fun `receive CJIT session keeps invoice when edit is cancelled`() {

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.

These two tests would still pass with the A fix reverted, so the regression isn't actually pinned.

The fix lives in composable lambda wiring (ReceiveSheet.kt:155-164, :365-368) that this unit test can't reach. keeps invoice when edit is cancelled never simulates an edit — it's create → confirm → assert, effectively the same as exposes confirmed fresh invoice (:84-93). clears stale invoice when edit is applied calls clear() directly. Re-adding beginReceiveEdit() and its two call sites would leave both green.

Not blocking — the state class is thin and there's no Compose harness for ReceiveSheet — but worth knowing so nobody reads these as coverage for the Edit→Back behaviour.

@jvsena42
jvsena42 dismissed their stale review September 11, 2026 16:44

Finding A is fixed in 79dd4c3 and nothing blocking remains. Remaining notes are non-blocking — see the latest review.

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.

2 participants