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

feat: Pay lightning invoice with USDP #1706

Merged
merged 4 commits into from
Dec 8, 2023
Merged

feat: Pay lightning invoice with USDP #1706

merged 4 commits into from
Dec 8, 2023

Conversation

holzeis
Copy link
Contributor

@holzeis holzeis commented Dec 8, 2023

Not shown on the video, but ..

  • the error message is not jumping on the latest version
  • enter amount includes a hint that entering 0 will send the maximum amount (as well as the confirmation)

send-usdp

fixes #1683
starts some rework on #1684

@holzeis holzeis self-assigned this Dec 8, 2023
@holzeis
Copy link
Contributor Author

holzeis commented Dec 8, 2023

A caveat I've noticed during testing is that subtracting the fee and a potential negative pnl, we might end up buying a little bit less sats back than what we use to pay the invoice.

I guess we'll have to iterate over that a bit to find the best UX there.

@holzeis
Copy link
Contributor Author

holzeis commented Dec 8, 2023

image

This was linked to issues Dec 8, 2023
@holzeis holzeis removed a link to an issue Dec 8, 2023
2 tasks
Copy link
Contributor

@luckysori luckysori left a comment

Choose a reason for hiding this comment

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

Concept ACK, nice work!

final tradeValuesChangeNotifier = context.watch<TradeValuesChangeNotifier>();

final tradeValues = tradeValuesChangeNotifier.fromDirection(Direction.long);
tradeValues.updateLeverage(Leverage(1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to update the leverage?

Copy link
Contributor Author

@holzeis holzeis Dec 8, 2023

Choose a reason for hiding this comment

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

Because the default leverage is 2 and if I wouldn't set it to 1 we would get an incorrect sats amount from the quantity.

Copy link
Contributor

@bonomat bonomat left a comment

Choose a reason for hiding this comment

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

Can't say much about the flutter code :)

Why did you go for composing it through flutter and not through rust?

@holzeis
Copy link
Contributor Author

holzeis commented Dec 8, 2023

Why did you go for composing it through flutter and not through rust?

I think it would be cleaner having one function in rust to pay an invoice using the usdp balance.

I guess I did it through flutter because I was faster that way. Happy to refactor in a follow up PR.

@bonomat
Copy link
Contributor

bonomat commented Dec 8, 2023

Why did you go for composing it through flutter and not through rust?

I think it would be cleaner having one function in rust to pay an invoice using the usdp balance.

I guess I did it through flutter because I was faster that way. Happy to refactor in a follow up PR.

No worries, let's go!

@@ -27,45 +33,110 @@ void showConfirmPaymentModal(BuildContext context, Destination destination, Amou
builder: (BuildContext context) {
return SingleChildScrollView(
child: SizedBox(
height: 320,
height: 420,
Copy link
Contributor

Choose a reason for hiding this comment

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

of course 420, it's a multiple of 42!

Amount amt = destination.amount;
if (destination.amount.sats == 0) {
if (payWithUsdp) {
// if the destination does not specify an amount and we ar paying with the usdp balance we
Copy link
Contributor

Choose a reason for hiding this comment

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

🙃

Suggested change
// if the destination does not specify an amount and we ar paying with the usdp balance we
// if the destination does not specify an amount and we are paying with the usdp balance we

@@ -105,7 +105,7 @@ class WalletScreen extends StatelessWidget {
size: 14,
),
SizedBox(
width: 10,
width: 7,
Copy link
Contributor

Choose a reason for hiding this comment

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

man, today you have the eastereggs hidden!

3 7 changed in this file = 3 * 7 = 21 = 10101

Copy link
Contributor

@bonomat bonomat left a comment

Choose a reason for hiding this comment

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

Ack.

@holzeis holzeis added this pull request to the merge queue Dec 8, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 8, 2023
@SXBT69
Copy link
Contributor

SXBT69 commented Dec 8, 2023

goose bumps

@holzeis holzeis added this pull request to the merge queue Dec 8, 2023
Merged via the queue into main with commit 957d961 Dec 8, 2023
9 checks passed
@holzeis holzeis deleted the feat/send-usdp branch December 8, 2023 12:12
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.

Pay lightning invoice with usdp
4 participants