Skip to content

fix(receive): handle additional receive liquidity edge cases - #1222

Open
pwltr wants to merge 11 commits into
masterfrom
fix/additional-cjit
Open

fix(receive): handle additional receive liquidity edge cases#1222
pwltr wants to merge 11 commits into
masterfrom
fix/additional-cjit

Conversation

@pwltr

@pwltr pwltr commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #1226

Description

Ports the receive liquidity/CJIT behavior from bitkit-ios synonymdev/bitkit-ios#711.

  • Adds a pure ReceiveLiquidityDecision helper for Lightning invoice vs additional CJIT routing.
  • Prevents Savings and Auto edits from routing into CJIT while still updating the shared receive amount.
  • Prevents normal Lightning invoices when there is no ready channel, zero inbound liquidity, or a fixed amount exceeds inbound liquidity.
  • Adds Spending-only additional CJIT routing for amounts above inbound liquidity, including below-min routing to amount entry and real max validation using Blocktank default LSP balance.
  • Makes Auto fall back to Savings/on-chain QR when Lightning cannot receive the edited amount.
  • Resets Receive sheet session state on close/reopen and defaults fresh sessions to Auto when available, otherwise Savings.
  • Keeps direct Spending additional-CJIT creation from regenerating the normal receive invoice before CJIT confirmation navigation.
  • Polishes Receive details spacing and copy card address text wrapping.

Preview

Screen.Recording.2026-09-03.at.12.35.19.mov

QA Notes

Manual checks:

  • Savings edit above inbound returns to Savings/on-chain QR and does not route to CJIT.
  • Auto edit above inbound falls back to Savings/on-chain QR.
  • Spending edit below CJIT minimum routes to CJIT amount entry.
  • Spending edit with valid CJIT amount creates additional CJIT and opens confirmation.
  • Spending edit above real CJIT maximum routes to amount entry and shows max validation.
  • Receive sheet close/reopen starts fresh on Auto when available, otherwise Savings.
  • Auto details show both on-chain and Lightning copy cards with expected spacing.

Automated checks:

  • ReceiveLiquidityDecisionTest.kt
  • ReceiveInvoiceUtilsTest.kt
  • EditInvoiceVMTest.kt

@pwltr
pwltr force-pushed the fix/additional-cjit branch 2 times, most recently from 53e0156 to 314445d Compare September 3, 2026 10:38
@pwltr
pwltr marked this pull request as ready for review September 3, 2026 10:38
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown

Greptile Summary

This PR centralizes receive-liquidity decisions and updates invoice creation, CJIT limits, receive navigation, session reset behavior, and receive-detail presentation.

  • Adds source-aware Lightning and additional-CJIT routing.
  • Computes and enforces a CJIT maximum from Blocktank channel-size options.
  • Resets receive-sheet state and adjusts Auto, Savings, and Spending navigation.
  • Adds focused unit tests and receive-liquidity documentation.

Confidence Score: 4/5

The PR should not merge until the CJIT maximum is derived from current Blocktank options rather than potentially stale cached limits.

A previously populated Blocktank info object bypasses refresh during maximum calculation, so the receive flow can enforce an outdated channel-size limit and repeat it after a server rejection.

Files Needing Attention: app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt

Important Files Changed

Filename Overview
app/src/main/java/to/bitkit/models/ReceiveLiquidityDecision.kt Adds a pure, tested decision model for ordinary Lightning invoices and source-aware additional-liquidity routing.
app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt Adds channel-size validation and maximum-CJIT calculation, but derives the maximum from potentially stale cached Blocktank options.
app/src/main/java/to/bitkit/repositories/WalletRepo.kt Gates invoice generation and channel-event refreshes on ready-channel inbound capacity.
app/src/main/java/to/bitkit/ui/screens/wallets/receive/EditInvoiceVM.kt Coordinates source-specific liquidity decisions and Blocktank limit lookup for edited receive amounts.
app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveAmountScreen.kt Adds maximum CJIT validation, input clamping, and dedicated maximum-exceeded feedback.
app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveQrScreen.kt Updates receive-tab availability, fallback selection, and details layout according to Lightning receive capability.
app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveSheet.kt Tracks the edit source and routes initial versus additional CJIT flows to their corresponding confirmation screens.
app/src/main/java/to/bitkit/ui/ContentView.kt Keys each receive-sheet instance so remembered navigation and CJIT state reset on close and reopen.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Edit receive amount] --> B{Selected source}
    B -->|Savings or Auto| C[Update shared receive invoice]
    B -->|Spending| D{Amount exceeds inbound liquidity?}
    D -->|No| C
    D -->|Yes| E{Geo-blocked?}
    E -->|Yes| F[Geo-block screen]
    E -->|No| G{Within current CJIT limits?}
    G -->|No| H[CJIT amount entry]
    G -->|Yes| I[Create additional CJIT]
    I --> J[Additional-liquidity confirmation]
    H --> J
Loading

Reviews (1): Last reviewed commit: "fix(receive): handle additional receive ..." | Re-trigger Greptile

Comment thread app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt Outdated
@pwltr
pwltr requested review from jvsena42 and ovitrif September 3, 2026 11:37
ovitrif
ovitrif previously approved these changes Sep 3, 2026

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

QA Notes

Pixel_8_Pro emulator (sdk_gphone16k_arm64), regtest via network Electrum/Blocktank.

  • Savings edit above inbound stayed on the on-chain QR and did not route to CJIT.
  • Auto edit above inbound fell back to Savings/on-chain QR.
  • Spending edit below the CJIT minimum routed to CJIT amount entry.
  • Spending edit with a valid additional-CJIT amount opened confirmation.
  • Spending edit above the real CJIT maximum stayed on amount entry and clamped to max.
  • Receive sheet close/reopen started fresh on Auto.
  • Auto details showed both on-chain and Lightning copy cards.

Approve.

@ovitrif ovitrif added this to the 2.5.0 milestone Sep 4, 2026
@piotr-iohk

Copy link
Copy Markdown
Collaborator

There are conflicts.

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

Added some nits after manual review for covering potential edge cases and defaulting to existing patterns throughout the code.

Been db checking them with AI on the side.

Comment thread app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveCjitErrors.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveQrScreen.kt Outdated
@ovitrif
ovitrif self-requested a review September 4, 2026 11:18
Comment thread app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt Fixed
@pwltr

pwltr commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

All review comments addressed

@piotr-iohk

piotr-iohk commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

QA Android, regtest. Existing channel: inbound 766,164, spending 3,000.

Invoice at 766,164 is fine. Savings edit above inbound stayed on-chain. Auto edit above inbound fell back to Savings QR. Neither entered extra CJIT.

Spending 766,165 opens extra-CJIT amount (MINIMUM 3,000). Continue at 3,000 fails.

UI toast is the raw exception, not the new max copy:

errorDetails=Failed to create CJIT entry: Client error: Bad request: Failed order channel size check for node 03a65053…de027dab. Node capacity is above our capacity limit.

Same Blocktank node-cap rejection as synonymdev/bitkit-ios#711. Client max handling only covers per-channel maxChannelSizeSat, so extra CJIT is still offered and then fails.

Please don’t offer extra CJIT (or map this error) when the node is already at Blocktank’s capacity limit. Don’t toast errorDetails / node id.

Recording:

Screen.Recording.2026-09-04.at.15.37.47.mov

Logs:
logs-android.zip

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

Diffed against the iOS port (#711). ReceiveLiquidityDecision and its 12 tests are a 1:1 match, and the routing, CJIT max search and session reset are equivalent. Android is the better of the two in three places (refreshes Blocktank info before the max check and tests it, hides Auto while a CJIT invoice is shown, routes edit-flow failures to the amount screen).

The two items worth attention are the tab effects, where this PR changed behaviour that the iOS side did not. Three of these are replies on existing threads.

Separately, and not filed as a finding since it is a bitkit-core question rather than a defect in this PR: get_default_lsp_balance returns max_channel_size_sat above threshold 2 (~EUR 495), so client + lsp > max for every amount above that and the "real max" this PR surfaces is effectively capped there regardless of Blocktank's maxChannelSizeSat. calc_default_lsp_balance uses max - client in the same band. Worth confirming that cap is intended.

Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveQrScreen.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveQrScreen.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/ContentView.kt Outdated
Comment thread app/src/main/res/values/strings.xml Outdated

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

Two things regress behaviour that master had, both in code the rebase brought in, so I'd like them fixed before this goes in.

The bigger one is BlocktankRepo.freshMaxChannelSizeSat(). It calls refreshInfo().getOrThrow(), and refreshInfo() wraps the whole cache-then-server sequence in a single runCatching, so a failing info(refresh = true) request returns Result.failure even after the cached value was already written into _blocktankState. Since createCjit() now calls it before the LSP request, one flaky Blocktank /info call aborts CJIT creation entirely. On master, createCjit had no dependency on /info at all. The earlier thread on this was marked fixed, but getOrThrow() is still there on this head.

The second is the AUTO auto-switch effect in ReceiveQrScreen. It lost the initialTab == null guard that master had. A LaunchedEffect body runs on first composition, so it now scrolls to ReceiveTab.AUTO and sets selectedTab = ReceiveTab.AUTO whenever Auto is visible, undoing the effect above it that applied the requested tab. ReceiveSheet passes initialTab = ReceiveTab.TREZOR whenever a hardware wallet is attached or the user is coming back from a hardware invoice edit, so hardware receive lands on Auto instead of Trezor.

The rest are non-blocking. ./gradlew detekt reports two violations on BlocktankRepo.kt on this head (ComplexCondition at line 478 and ImportOrdering), which is why the code-scanning alert is still open; the error-classification substring is broader than intended; and there are a few small rule and cleanup items noted inline.

Comment thread app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/EditInvoiceVM.kt Outdated
Comment thread app/src/main/res/values/strings.xml Outdated
Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveQrScreen.kt Outdated
@ovitrif ovitrif removed this from the 2.5.0 milestone Sep 7, 2026
@piotr-iohk

piotr-iohk commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

HW receive broke after rebasing onto master (Trezor tab). e2e hardware_wallet fails for the same reason (hardware_wallet_2/3/4: Timed out waiting for hardware receive address).

Home Receive, paired Trezor, Spending 0 / no LN:

  • master → Savings
  • this branch → Trezor

New Lightning-unavailable fallback does scrollToItem(0). Tabs are now [Trezor, Savings, Spending], so that lands on Trezor. Master never scrolled.

E2E helper reads the default-tab QR, taps Trezor, and waits for a different address. This branch already opened on Trezor, so the addresses match and it times out.

Home Receive should stay Savings (Auto if LN works). Trezor only when opening from the HW wallet screen.

Recording (master):

Screen.Recording.2026-09-07.at.12.41.16-master.mov

Recording (this branch):

Screen.Recording.2026-09-07.at.12.44.47-1222.mov

@pwltr

pwltr commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

HW receive broke after rebasing onto master (Trezor tab). e2e hardware_wallet fails for the same reason (hardware_wallet_2/3/4: Timed out waiting for hardware receive address).

Addressed. Home Receive no longer defaults to Trezor just because a single hardware wallet is paired; it defaults to Auto when Lightning is available, otherwise Savings.

Trezor is now only used as the initial tab when opening from the hardware wallet receive flow, or when returning from an edit that actually originated on the Trezor tab. I also fixed the Savings fallback to scroll to the actual Savings index instead of hardcoding 0, so [Trezor, Savings, Spending] no longer lands on Trezor.

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

Traced the funds-critical parts and they hold up: createCjit still sends invoiceSat = amountSats with channelSizeSat = amountSats + lspBalance, the Confirm screen's receiveAmountSats is the same action.amountSats used for the invoice, the binary search in maxCjitAmountSats() is over a monotone predicate and is overflow-safe, getInvoiceForTab is called with the per-item tab, and the hardware address is only produced under ReceiveTab.TREZOR. I found no way to show a destination belonging to the wrong tab and no sat/msat or off-by-one error in the limits.

One regression and one nit inline.

Comment thread app/src/main/java/to/bitkit/repositories/WalletRepo.kt Outdated
Comment thread app/src/main/java/to/bitkit/ui/screens/wallets/receive/ReceiveAmountScreen.kt Outdated
@jvsena42

jvsena42 commented Sep 7, 2026

Copy link
Copy Markdown
Member

Regression test — stale channel cache in canCreateLightningInvoice

Two tests that separate the cached source from the live one. Test-only, applies cleanly onto fix/additional-cjit (78fa7d4).

./gradlew :app:testDevDebugUnitTest --tests 'to.bitkit.repositories.WalletRepoTest'

On head: 50 tests, 2 failed (the two new ones; all 48 pre-existing pass).

refreshBip21ForEvent ChannelClosed should clear bolt11 when live channels are gone
  -> org.junit.ComparisonFailure: expected:<[]> but was:<[testInvoice]>

refreshBip21ForEvent ChannelReady should create invoice from live channels
  -> WantedButNotInvoked: lightningRepo.createInvoice(<any>, <any String>, <any int>);
     However, there were exactly 2 interactions with this mock: lightningRepo.ge...

The ChannelClosed one is the deterministic case: stale cache still holds the ready channel, live getChannels() is empty, so the !canCreateLightningInvoice(...) guard at :339 is skipped and the unpayable bolt11 survives in the BIP21/QR.

Both pass with the minimal fix at WalletRepo.kt:755:

val channels = lightningRepo.getChannels() ?: lightningRepo.lightningState.value.channels

⚠️ One thing to know before you land the fix: with it applied, 4 pre-existing tests fail — updateBip21Invoice should create bolt11 when node can receive, refreshBip21ForEvent ChannelReady should update bolt11 and preserve amount, refreshBip21ForEvent ChannelClosed should not clear bolt11 when can still receive, and refreshBip21 should create a fresh invoice after PaymentReceived invalidates the old one. They only stub the cached source, which is precisely why none of them caught this. The ?: fallback doesn't rescue them because Mockito's default for a List<ChannelDetails>? getter is an empty list, not null. Adding whenever(lightningRepo.getChannels()).thenReturn(channels) beside each existing lightningState stub makes the class green at 50/50.

Those 4 stub updates are deliberately not in this diff — they'd be unused stubs on unfixed code, so they belong with the fix commit.

pr1222-regression.diff
diff --git a/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt b/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt
index c141eff37..84eccc614 100644
--- a/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt
+++ b/app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt
@@ -588,6 +588,39 @@ class WalletRepoTest : BaseUnitTest() {
         assertEquals(INVOICE, sut.walletState.value.bolt11)
     }
 
+    @Test
+    fun `refreshBip21ForEvent ChannelClosed should clear bolt11 when live channels are gone`() = test {
+        sut.setBolt11(INVOICE)
+        // stale cache still holds the channel that just closed
+        whenever(lightningRepo.lightningState).thenReturn(MutableStateFlow(LightningState(channels = channels)))
+        whenever(lightningRepo.getChannels()).thenReturn(emptyList())
+
+        sut.refreshBip21ForEvent(
+            Event.ChannelClosed(
+                channelId = "testChannelId",
+                userChannelId = "testUserChannelId",
+                counterpartyNodeId = null,
+                reason = null,
+            )
+        )
+
+        assertEquals("", sut.walletState.value.bolt11)
+    }
+
+    @Test
+    fun `refreshBip21ForEvent ChannelReady should create invoice from live channels`() = test {
+        sut.setBip21AmountSats(SATS)
+        // stale cache does not know about the channel that just became ready
+        whenever(lightningRepo.lightningState).thenReturn(MutableStateFlow(LightningState()))
+        whenever(lightningRepo.getChannels()).thenReturn(channels)
+        whenever(lightningRepo.createInvoice(anyOrNull(), any(), any())).thenReturn(Result.success(INVOICE))
+
+        sut.refreshBip21ForEvent(channelReady)
+
+        verify(lightningRepo).createInvoice(anyOrNull(), any(), any())
+        assertEquals(INVOICE, sut.walletState.value.bolt11)
+    }
+
     @Test
     fun `refreshBip21ForEvent PaymentReceived should refresh address if used`() = test {
         whenever(cacheStore.data).thenReturn(flowOf(AppCacheData(onchainAddress = ADDRESS, bolt11 = INVOICE)))

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.

[Bug]: Receive liquidity flow edge cases

5 participants