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>

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

CHANGELOG.md

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

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.

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

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>

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

app/src/main/java/to/bitkit/viewmodels/AppViewModel.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>

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

Activity list showed 222 sats while review screen showed 223 for a 222222 msat payment. The PaymentDetails extension used floor division (amountMsat / 1000). Use ceiling to match all other display paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

ben-kaufman · 2026-04-05T05:18:18Z

@piotr-iohk Fixed — the activity list was using floor division (amountMsat / 1000) in the PaymentDetails.amountSats extension. Changed to ceiling division so activity amounts match the review/confirmation screens. Same fix applied to iOS.

WakeNodeWorker and NotifyPaymentReceivedHandler used floor division for msat amounts in payment notifications. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

claude · 2026-04-05T05:42:21Z

Code review

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

ovitrif · 2026-04-05T19:28:36Z

I'm testing using synonymdev/bitkit-docker#5 by generating a LNURL-PAY invoice via url:

http://localhost:3000/generate/pay?minSendable=32222&maxSendable=32222

During the send flow, I see 33 sat, after it's paid I see 32. It's strange seeing the conversation, descriptions, etc, which supposedly is expected to have the right implementation amendments for this exact issue that I encounter:

bug_sat.mp4

Had me double-check if I'm on the latest commit of this PR's branch, it sure seems so.

EDIT

Looks like the problem might've been on my side.

ovitrif

tAck

Tests

All tests used msat values.

LnUrl-Pay (QuickPay ON & OFF) using synonymdev/bitkit-docker#5
LnUrl-Withdraw (QuickPay ON & OFF) using synonymdev/bitkit-docker#5
Bolt11 (QuickPay ON & OFF) using synonymdev/bitkit-docker#6
regression: Bolt11 zero-amount invoice

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

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

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

claude bot reviewed Apr 3, 2026

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

claude bot reviewed Apr 3, 2026

View reviewed changes

app/src/main/java/to/bitkit/repositories/LightningRepo.kt Outdated Show resolved Hide resolved

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

app/src/main/java/to/bitkit/repositories/LightningRepo.kt Outdated Show resolved Hide resolved

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

app/src/main/java/to/bitkit/ext/Lnurl.kt Outdated Show resolved Hide resolved

claude bot reviewed Apr 3, 2026

View reviewed changes

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

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>

This comment has been minimized.

Sign in to view

fix: use ceiling for received payment notification amounts

2608e65

WakeNodeWorker and NotifyPaymentReceivedHandler used floor division for msat amounts in payment notifications. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

ovitrif mentioned this pull request Apr 5, 2026

fix: preserve lnurl msat precision synonymdev/bitkit-docker#5

Merged

4 tasks

ovitrif approved these changes Apr 5, 2026

View reviewed changes

app/src/main/java/to/bitkit/ext/PaymentDetails.kt Show resolved Hide resolved

ovitrif mentioned this pull request Apr 5, 2026

refactor: Define MSat DDD model for conversions #888

Open

ovitrif merged commit a84e332 into master Apr 5, 2026
21 checks passed

ovitrif deleted the fix/msat-invoice-precision branch April 5, 2026 20:50

ovitrif mentioned this pull request Apr 5, 2026

refactor: add MSat model to encapsulate conversions #889

Open

4 tasks

	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)

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!

Uh oh!

Uh oh!

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!

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!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

piotr-iohk commented Apr 3, 2026

Uh oh!

ben-kaufman commented Apr 5, 2026

Uh oh!

This comment has been minimized.

claude bot commented Apr 5, 2026

Code review

Uh oh!

ovitrif commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

EDIT

Uh oh!

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

Tests

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ovitrif left a comment •

edited

Loading

ovitrif commented Apr 5, 2026 •

edited

Loading

ovitrif left a comment •

edited

Loading