fix: avoid msat truncation when paying invoices with built-in amounts by ben-kaufman · Pull Request #879 · synonymdev/bitkit-android

ben-kaufman · 2026-04-01T10:31:55Z

Summary

Bump bitkit-core from 0.1.38 to v0.1.56 which rounds up sub-satoshi invoice amounts (ceiling division instead of floor)
Stop overriding the amount for invoices that already have one — pass null so LDK uses the invoice's native msat precision instead of our truncated sats value converted back to msat
Only pass the amount for zero-amount invoices where the user specifies it

Test plan

E2E: pay invoices created with lnd.addInvoice({ valueMsat }) using amounts 222538, 222222, 500500 msat
Verify zero-amount invoice flow still works
Verify LNURL pay flow still works
Verify quickpay flow still works

Depends on synonymdev/bitkit-core#85
Closes #877

🤖 Generated with Claude Code

Bump bitkit-core to v0.1.56 which rounds up sub-satoshi invoice amounts. Additionally, stop overriding the amount for invoices that already have one. Pass null so LDK uses the invoice's native msat precision instead of our truncated sats value converted back to msat. Only pass the amount for zero-amount invoices where the user specifies it. Closes #877

app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

ovitrif

utAck
Will test too

piotr-iohk · 2026-04-02T12:31:48Z

Observations from testing:

1) The fix rounds up to full sats precision for the payment and display amount, so ln invoice with 500500 msats becomes 501 sats payment. In bitkit RN it seems that the displayed number of sats for such invoice still remains 500 - seems that, perhaps, Bitkit RN behind the scenes falls back to msats precision for the payment and just displays the floor sats amount. Just want to make sure we want that behaviour and not follow the Bitkit RN.
See example, for the 500500 invoice for Bitkit RN (left) and Bitkit android native:
Invoice generated with:

$ ./bitcoin-cli getlninvoice --msat 500500

from synonymdev/bitkit-e2e-tests#141.

Screen.Recording.2026-04-02.at.12.58.54.mov

2) There is an issue with lnurl-pay and and lnurl-withdrawal.

For testing on local regtest, more utility commands for bitcoin-cli can be used, added in synonymdev/bitkit-e2e-tests#141.

  LNURL (requires LNURL server, default: http://localhost:30001):
    startlnurlserver [--port N]        Start LNURL server using current e2e Bitcoin/LND
    stoplnurlserver                    Stop LNURL server container
    checklnurl                         Diagnose LNURL health, routing, and adb reverse
    getlnurlpay --msat N               Create LNURL-pay with fixed msat amount
    getlnurlwithdraw --msat N          Create LNURL-withdraw with fixed msat amount

$ ./bitcoin-cli startlnurlserver
$ ./bitcoin-cli checklnurl
$ ./bitcoin-cli getlnurlpay --msat 500500 
$ ./bitcoin-cli getlnurlwithdraw --msat 500500

⚠️ lnurl-pay with msat precision shows 0 amount and cannot be paid. (Works on bitkit RN). The video is for lnurl-pay with 500500 msats $ ./bitcoin-cli getlnurlpay --msat 500500 - Bitkit RN on the left.

Screen.Recording.2026-04-02.at.13.53.59.mov

⚠️ lnurl-withdrawal with msat precision produces error toast. Video for $ ./bitcoin-cli getlnurlwithdraw --msat 500500 (note that lnurl-withdrawal with msat precision also doesn't seem to work properly on Bitkit RN - no error toast there, but the request not working)

Screen.Recording.2026-04-02.at.14.26.12.mov

LNURL protocol uses millisatoshis, but the app was converting to sats and back for callbacks, losing the fractional part. For fixed-amount LNURL-pay (e.g. 500500 msat), ceil(min)=501 > floor(max)=500 caused the UI to show 0/invalid amount. - Add isFixedAmount() helpers that detect sub-sat fixed amounts - Add callbackAmountMsats() to return original msat for fixed amounts - Change fetchLnurlInvoice to accept amountMsats directly - For fixed-amount LNURL-withdraw, use floor division for invoice amount - Fix validateAmount for withdraw: use <= instead of < for max bound - Add unit tests for all new helper functions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

app/src/main/java/to/bitkit/viewmodels/QuickPayViewModel.kt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude · 2026-04-03T06:52:34Z

app/src/main/java/to/bitkit/viewmodels/QuickPayViewModel.kt

                            return@launch
                        }
-                    invoice.bolt11 to data.sats
+                    Triple(invoice.bolt11, data.sats, data.sats)


Bug: LnurlPay quick-pay still overrides invoice amount with truncated sats

The LnurlPay branch passes data.sats (a truncated sat value) as the payment amount, so sendLightning calls sendUsingAmount(bolt11, data.sats * 1000u) — which is exactly the msat truncation this PR is fixing. The Bolt11 branch correctly passes null to let LDK use the invoice's native msat precision. The LNURL-fetched invoice already has the correct msat amount embedded, so amount here should also be null.

Compare with AppViewModel.kt which correctly does:

val paymentAmount = if (decodedInvoice.amountSatoshis > 0uL) null else amount

Suggested change

Triple(invoice.bolt11, data.sats, data.sats)

Triple(invoice.bolt11, null, data.sats)

claude · 2026-04-03T06:52:41Z

CHANGELOG.md

+- Preserve msat precision for LNURL pay and withdraw callbacks #879
+- Avoid msat truncation when paying invoices with built-in amounts #879


CLAUDE.md violation: multiple changelog entries for the same PR

Per CLAUDE.md:

NEVER add multiple changelog lines for the same PR — summarize all changes in a single concise entry

Both lines reference #879. Please consolidate into one entry, e.g.:

Suggested change

- Preserve msat precision for LNURL pay and withdraw callbacks #879

- Avoid msat truncation when paying invoices with built-in amounts #879

- Avoid msat truncation when paying invoices and LNURL callbacks #879

claude · 2026-04-03T06:52:48Z

app/src/main/java/to/bitkit/repositories/LightningRepo.kt

        }.onFailure {
            Logger.error(
-                "fetchLnurlInvoice error, url: $callbackUrl, amount: $amountSats, comment: $comment",
+                "fetchLnurlInvoice error, url: $callbackUrl, amountMsats: $amountMsats, comment: $comment",


CLAUDE.md violation: log parameter values must be wrapped in single quotes

Per CLAUDE.md:

ALWAYS wrap parameter values in log messages with single quotes, e.g. Logger.info("Received event '$eventName'", context = TAG)

Suggested change

"fetchLnurlInvoice error, url: $callbackUrl, amountMsats: $amountMsats, comment: $comment",

"fetchLnurlInvoice error, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",

claude · 2026-04-03T06:52:54Z

app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt

-        if (hasAmount) {
-            Logger.info("Found amount $$minSendable in lnurlPay, proceeding with payment", context = TAG)
+        if (isFixed) {
+            Logger.info("Found fixed amount $displaySats sats in lnurlPay, proceeding with payment", context = TAG)


CLAUDE.md violation: log parameter values must be wrapped in single quotes

Per CLAUDE.md:

ALWAYS wrap parameter values in log messages with single quotes, e.g. Logger.info("Received event '$eventName'", context = TAG)

Suggested change

Logger.info("Found fixed amount $displaySats sats in lnurlPay, proceeding with payment", context = TAG)

Logger.info("Found fixed amount '$displaySats' sats in lnurlPay, proceeding with payment", context = TAG)

- Pass null instead of data.sats for LNURL quick-pay so LDK uses the invoice's native msat precision (same fix as Bolt11 path) - Wrap log parameter values in single quotes per CLAUDE.md - Consolidate duplicate changelog entries into one Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

ben-kaufman · 2026-04-03T06:55:40Z

Thanks for the thorough testing @piotr-iohk!

Issue 1 (display 501 vs 500): This is because amountSatoshis from bitkit-core now uses ceiling division (div_ceil). The payment itself works correctly — we pass sats: null so LDK uses the invoice's native msat precision. The display difference (501 vs RN's 500) is a consequence of the safety-net rounding in bitkit-core. We can change the display to floor if needed — let us know if this should match RN behavior.

Issue 2 (LNURL-pay and LNURL-withdraw): Both fixed in the latest commits. Root cause was msat→sats→msat round-trip losing precision for LNURL callbacks. The fix:

LNURL-pay: fetchLnurlInvoice now accepts amountMsats directly; fixed-amount requests pass the original msat value to the callback instead of roundedSats * 1000
LNURL-withdraw: fixed-amount requests use floor(msats/1000) for the invoice so it never exceeds the server's maxWithdrawable
Added isFixedAmount() helpers that detect the sub-sat edge case where ceil(min) > floor(max)

Ready for re-testing.

claude · 2026-04-03T07:20:01Z

app/src/main/java/to/bitkit/repositories/LightningRepo.kt

        }.onFailure {
            Logger.error(
-                "fetchLnurlInvoice error, url: $callbackUrl, amount: $amountSats, comment: $comment",
+                "fetchLnurlInvoice error, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",


CLAUDE.md violation: Log messages must always start with a verb.

"fetchLnurlInvoice error" starts with a method name, not a verb. This line was modified by this PR (parameter rename + quote-wrapping), which was an opportunity to fix the prefix.

bitkit-android/app/src/main/java/to/bitkit/repositories/LightningRepo.kt

Lines 912 to 916 in 7bfb20b

Logger.error(

"fetchLnurlInvoice error, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",

it,

context = TAG,

)

Suggested change

"fetchLnurlInvoice error, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",

"Failed to fetch LNURL invoice, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

For LNURL-withdraw with sub-sat precision (e.g. 222538 msat), neither floor (222 sats) nor ceiling (223 sats) matches the server's exact amount range. Add receiveMsats/createInvoiceMsats to create invoices with native msat precision, used for fixed-amount LNURL withdrawals. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

piotr-iohk · 2026-04-03T10:22:02Z

Issue 1 (display 501 vs 500): This is because amountSatoshis from bitkit-core now uses ceiling division (div_ceil). The payment itself works correctly — we pass sats: null so LDK uses the invoice's native msat precision. The display difference (501 vs RN's 500) is a consequence of the safety-net rounding in bitkit-core. We can change the display to floor if needed — let us know if this should match RN behavior.

Ok, clear, thanks for explanation. I think that it is fine as it is then.

piotr-iohk · 2026-04-03T12:59:56Z

I found remaining amount-display inconsistencies vs regular BOLT11 when using an msat-precision LNURL server.

Item	LNURL-pay	LNURL-withdraw	BOLT11 (reference)
Test amount	`222_222 msat`	`222_222 msat`	`222_222 msat`
Review/receive screen	222 sats	222 sats	223 sats
Sent/confirmation	223 sats	222 sats	223 sats
Behavior summary	⚠️ Inconsistent across screens (222 -> 223)	⚠️ Consistent internally, but inconsistent vs BOLT11 policy	Consistent (223 -> 223)
Video	Screen.Recording.2026-04-03.at.14.26.27.mov	Screen.Recording.2026-04-03.at.14.53.07.mov	Screen.Recording.2026-04-03.at.14.40.01.mov

Notes

This is with LNURL server creating msat-precision invoices (expected behavior).
For the same msat amount:
- LNURL-pay currently appears to floor on the payment screen but shows rounded-up amount after send.
- LNURL-withdraw appears to floor (222) while BOLT11 flow displays 223.

Expected

LNURL-pay should show a consistent amount across:
- payment/review screen
- sent success/notification
LNURL-withdraw should follow the same amount-display policy as BOLT11 for the same invoice amount.
Ideally both LNURL flows should match the BOLT11 behavior for equivalent msat amounts.

Previously LNURL-pay showed 222 sats on review but 223 after sending, and LNURL-withdraw showed 222 while BOLT11 showed 223 for the same 222222 msat amount. Use ceiling division consistently for display. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

ben-kaufman · 2026-04-03T15:03:26Z

@piotr-iohk Display inconsistency fixed — all LNURL display amounts now use ceiling division (satsCeil) to match BOLT11 behavior. For 222222 msat, all screens consistently show 223 sats now.

app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude · 2026-04-03T15:14:25Z

app/src/main/java/to/bitkit/ext/Lnurl.kt

+ *
+ * Uses floor division so the invoice amount never exceeds `maxWithdrawable` in msats.
+ */
+fun LnurlWithdrawData.fixedWithdrawAmountSat(): ULong = maxWithdrawable / MSATS_PER_SAT


fixedWithdrawAmountSat() is added to production code but has no production call sites — it is only referenced from LnurlExtTest.kt. This violates CLAUDE.md's YAGNI rule:

ALWAYS apply the YAGNI (You Ain't Gonna Need It) principle for new code

Looking at AppViewModel.kt:1530, the isFixed branch in the displayAmount expression calls data.minWithdrawableSat() (ceiling division on minWithdrawable) when data.fixedWithdrawAmountSat() (floor division on maxWithdrawable) was likely the intended call — to ensure the displayed sat amount never exceeds maxWithdrawable for sub-sat fixed amounts. If that's the case, this function should be wired up there rather than left as dead production code.

claude · 2026-04-03T15:14:33Z

app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt

            return
        }

+        val displayAmount = if (isFixed) data.minWithdrawableSat() else minWithdrawable


Both branches of the displayAmount conditional always produce the same value as minWithdrawable:

val minWithdrawable = data.minWithdrawableSat() // line 1527 — ceiling division on minWithdrawable // ... val displayAmount = if (isFixed) data.minWithdrawableSat() else minWithdrawable // ^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^ // same as minWithdrawable same as minWithdrawable

The isFixed branch was likely intended to call data.fixedWithdrawAmountSat() (floor division on maxWithdrawable), which was added in this PR precisely for this purpose. Using floor division in the fixed-amount branch ensures the displayed sat value never exceeds maxWithdrawable for sub-sat fixed withdrawals (e.g. 500 500 msat → displays 500 sat instead of 501 sat).

Suggested fix:

val displayAmount = if (isFixed) data.fixedWithdrawAmountSat() else minWithdrawable

(See also the related dead-code note on fixedWithdrawAmountSat() in Lnurl.kt.)

fixedWithdrawAmountSat() had no production call sites after switching to createInvoiceMsats for fixed-amount withdrawals and satsCeil for display. Remove the function, its test, and simplify the redundant displayAmount conditional. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

app/src/test/java/to/bitkit/ext/LnurlExtTest.kt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude · 2026-04-03T15:27:59Z

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

piotr-iohk · 2026-04-03T16:24:37Z

@piotr-iohk Display inconsistency fixed — all LNURL display amounts now use ceiling division (satsCeil) to match BOLT11 behavior. For 222222 msat, all screens consistently show 223 sats now.

Ok, that works consistent accross BOLT11, LNURL pay and withdraw now. 👍

Unfortunately, while hardening/adapting e2e tests to this behavior, I spotted inconsistency on what's reported on the screens vs. on what is reported on the activity list... activity details show 222. This is common for LNURL-pay, withdraw and BOLT11 for invoices with msats precision...

Screen.Recording.2026-04-03.at.18.19.40.mov

claude bot reviewed Apr 1, 2026

View reviewed changes

app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Outdated Show resolved Hide resolved

fix: remove inline comments per CLAUDE.md guidelines

55dee91

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

This comment has been minimized.

Sign in to view

ovitrif added this to the 2.2.0 milestone Apr 1, 2026

piotr-iohk mentioned this pull request Apr 2, 2026

Fix/msat invoice precision synonymdev/bitkit-e2e-tests#141

Open

3 tasks

Merge branch 'master' into fix/msat-invoice-precision

912fdca

ovitrif previously approved these changes Apr 2, 2026

View reviewed changes

ovitrif self-requested a review April 2, 2026 17:24

Merge branch 'master' into fix/msat-invoice-precision

01ec250

This comment has been minimized.

Sign in to view

ben-kaufman dismissed ovitrif’s stale review via 03403b3 April 3, 2026 06:27

github-advanced-security bot found potential problems Apr 3, 2026

View reviewed changes

ben-kaufman and others added 2 commits April 3, 2026 09:35

chore: add changelog entry and fix detekt line length

5d7614d

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Merge branch 'master' into fix/msat-invoice-precision

388c669

claude bot reviewed Apr 3, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

claude bot reviewed Apr 3, 2026

View reviewed changes

ben-kaufman and others added 2 commits April 3, 2026 10:27

fix: log message should start with verb per CLAUDE.md

74f3972

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

This comment has been minimized.

Sign in to view

github-advanced-security bot found potential problems Apr 3, 2026

View reviewed changes

app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt Fixed Show fixed Hide fixed

fix: remove unused fixedWithdrawAmountSat import

4750afb

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude bot reviewed Apr 3, 2026

View reviewed changes

github-advanced-security bot found potential problems Apr 3, 2026

View reviewed changes

app/src/test/java/to/bitkit/ext/LnurlExtTest.kt Fixed Show fixed Hide fixed

fix: remove needless blank line flagged by detekt

c72e52c

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

	Triple(invoice.bolt11, data.sats, data.sats)
	Triple(invoice.bolt11, null, data.sats)

		- Preserve msat precision for LNURL pay and withdraw callbacks #879
		- Avoid msat truncation when paying invoices with built-in amounts #879

	"fetchLnurlInvoice error, url: $callbackUrl, amountMsats: $amountMsats, comment: $comment",
	"fetchLnurlInvoice error, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",

	Logger.info("Found fixed amount $displaySats sats in lnurlPay, proceeding with payment", context = TAG)
	Logger.info("Found fixed amount '$displaySats' sats in lnurlPay, proceeding with payment", context = TAG)

	Logger.error(
	"fetchLnurlInvoice error, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",
	it,
	context = TAG,
	)

	"fetchLnurlInvoice error, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",
	"Failed to fetch LNURL invoice, url: '$callbackUrl', amountMsats: '$amountMsats', comment: '$comment'",

Conversation

ben-kaufman commented Apr 1, 2026

Summary

Test plan

Uh oh!

Uh oh!

This comment has been minimized.

ovitrif left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

piotr-iohk commented Apr 2, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

ben-kaufman commented Apr 3, 2026

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

piotr-iohk commented Apr 3, 2026

Uh oh!

piotr-iohk commented Apr 3, 2026

Notes

Expected

Uh oh!

ben-kaufman commented Apr 3, 2026

Uh oh!

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

claude bot commented Apr 3, 2026

Code review

Uh oh!

piotr-iohk commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ovitrif left a comment •

edited

Loading