Skip to content
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

frontend: handle zero amount invoices when sending with lightning #2571

Merged

Conversation

thisconnect
Copy link
Collaborator

@thisconnect thisconnect commented Feb 22, 2024

Added a new step edit-invoice to allow specifying a custom amount if no amount was given or amount is 0.

Other change:

  • back button on confirm screen goes now back to the lighting view was going back to the QR scanner

@thisconnect thisconnect force-pushed the frontend-ln-handle-zeroamount branch from 007d584 to 5d15765 Compare February 22, 2024 12:22
@thisconnect thisconnect marked this pull request as ready for review February 22, 2024 12:23
@thisconnect
Copy link
Collaborator Author

I sometimes got this error, but at least once it went through

Screen Shot 2024-02-22 at 12 56 12

in phoenix: "1 incoming payment recently rejected"

reason: "The fee was 2976 sat which is more than 50% of the amount"

I have 600k inbound liquidity
but only 2k outbound

@dangeross
Copy link
Contributor

dangeross commented Feb 22, 2024

It seems as custom description / memo is not supported, added TODO
(if that is even possible to set).

You normally set the description / memo when creating the invoice (receiver side), not when paying the invoice

@thisconnect thisconnect force-pushed the frontend-ln-handle-zeroamount branch from 5d15765 to b38b9cf Compare February 22, 2024 16:04
@thisconnect
Copy link
Collaborator Author

You normally set the description / memo when creating the invoice (receiver side), not when paying the invoice

Thanks, I expected that it's not possible. I removed the TODO and updated the commit message, thanks @dangeross

@thisconnect
Copy link
Collaborator Author

@Beerosagos PTAL

value={customAmount ? `${customAmount}` : ''}
autoFocus
/>
<Input
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This input should be removed, I think

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept as readonly input so that the user can read the memo/description

await postSendPayment({
bolt11: paymentDetails.invoice.bolt11,
// amountMsat is optional, if amount is missing the UI shows edit-invoice step for the user to enter a custom amountMsat which is passed here
amountMsat: customAmount
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK amountMsat unit is milli satoshi. The amount should be multiplied * 1000

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oups 😇

@Beerosagos
Copy link
Collaborator

I sometimes got this error, but at least once it went through

Screen Shot 2024-02-22 at 12 56 12

in phoenix: "1 incoming payment recently rejected"

reason: "The fee was 2976 sat which is more than 50% of the amount"

I have 600k inbound liquidity but only 2k outbound

From my tests, the reason of this error is the missing conversion to milli sats: Setting 1000 in the input seems to successfully send a 1sat transaction

@thisconnect thisconnect force-pushed the frontend-ln-handle-zeroamount branch from b38b9cf to 5bfc632 Compare February 27, 2024 08:28
@thisconnect
Copy link
Collaborator Author

rebased

@thisconnect thisconnect force-pushed the frontend-ln-handle-zeroamount branch 2 times, most recently from 2cc56c1 to 8b2f595 Compare February 27, 2024 09:05
@thisconnect
Copy link
Collaborator Author

@Beerosagos PTAL

Added a new step edit-invoice to allow specifying a custom amount
if no amount was given or amount is 0.

Other change:
- back button on confirm screen goes now back to the lighting view
  was going back to the QR scanner
- changed Sats to Sat so it is consitent with the rest of the app
@thisconnect thisconnect force-pushed the frontend-ln-handle-zeroamount branch from 8b2f595 to e7a8224 Compare February 27, 2024 09:38
Copy link
Collaborator

@Beerosagos Beerosagos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

partially tested ACK

@thisconnect thisconnect merged commit 2d7a237 into BitBoxSwiss:staging-ln Feb 27, 2024
5 checks passed
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.

3 participants