-
Notifications
You must be signed in to change notification settings - Fork 1
Fix/lnurl minSendable edge case when msats are not divisable by 1000 #276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an edge case in LNURL amount conversion where millisatoshi values not divisible by 1000 could cause validation errors. The fix introduces proper ceiling/floor rounding when converting from millisatoshis to satoshis, ensuring minimum bounds are met and maximum bounds are not exceeded.
Key Changes
- Introduced
LnurlAmountConversionutility enum withsatsCeilandsatsFloormethods for proper msat→sats conversion - Updated LNURL pay and withdraw handlers to use the new conversion methods instead of integer division
- Added comprehensive unit tests covering rounding edge cases
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
Bitkit/Utilities/LnurlAmountConversion.swift |
New utility enum providing ceiling and floor conversion methods from millisatoshis to satoshis |
BitkitTests/LnurlAmountConversionTests.swift |
Unit tests verifying correct rounding behavior for both exact and non-divisible msat values |
Bitkit/ViewModels/AppViewModel.swift |
Updated handleLnurlPayInvoice and handleLnurlWithdraw methods to use new conversion utilities instead of integer division |
pwltr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
ovitrif
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utAck
Description
Linked Issues/Tasks
Screenshot / Video
Insert relevant screenshot / recording