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

Fedimint sweep #862

Merged
merged 5 commits into from
Feb 6, 2024
Merged

Fedimint sweep #862

merged 5 commits into from
Feb 6, 2024

Conversation

AnthonyRonning
Copy link
Collaborator

This does a few refactors on common components.

Does the swap and warns the user when a channel fee may be required. However one critical part is to use estimate_sweep_federation_fee once the user has selected the amount so that they can see the fee before they continue. Handles a bit different than the lightning->channel open flow does today which is hard for me to replicate.

Relies on MutinyWallet/mutiny-node#995

Copy link

cloudflare-workers-and-pages bot commented Feb 3, 2024

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 835236c
Status: ✅  Deploy successful!
Preview URL: https://11d24d52.mutiny-web.pages.dev
Branch Preview URL: https://fedimint-sweep.mutiny-web.pages.dev

View logs

@AnthonyRonning
Copy link
Collaborator Author

Known problems:

  • putting in a small amount and trying to do fee estimation when a new channel is needed doesn't work properly (amount is too low)
  • i don't yet handle when the user previews the fee and then wants to change the fee, it should ask for another preview and don't allow edits (or switch back to preview button instead of confirm button)
  • I think when a new channel is needed, the final success screen shows the amount that was transfered on the fedimint side (minus fedimint fee) but that amount is not considering the channel open fee. However the fee is accurate I think. Needs to investigate more, but perhaps that's a bug on the mutiny node side.

@futurepaul
Copy link
Collaborator

@TonyGiorgio okay I think I've cleaned this up / addressed your issues, let me know if I'm missing anything

@AnthonyRonning
Copy link
Collaborator Author

Awesome thank you so much. I'll test tonight.

@AnthonyRonning
Copy link
Collaborator Author

Only frontend issue here is not being able to hit the back button on desktop to edit the amount

@AnthonyRonning AnthonyRonning marked this pull request as ready for review February 6, 2024 10:48
@benthecarman
Copy link
Collaborator

2 issues I noticed

  1. No max button, @TonyGiorgio said that @futurepaul removed this from the codebase, imo we should bring it back, if that is true though can be a follow up
  2. After a successful swap it says Swap Initiated and that the balance will be added, however, it should be added already since the invoice was paid
    image

@AnthonyRonning
Copy link
Collaborator Author

AnthonyRonning commented Feb 6, 2024

No max button, @TonyGiorgio said that @futurepaul removed this from the codebase, imo we should bring it back, if that is true though can be a follow up

I do think this should be a follow up.

After a successful swap it says Swap Initiated and that the balance will be added, however, it should be added already since the invoice was paid

This has been fixed

@futurepaul
Copy link
Collaborator

No max button

Ah yeah, that used to be in the amount editor. Will add an issue.

Copy link
Collaborator

@futurepaul futurepaul left a comment

Choose a reason for hiding this comment

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

tony the frontend master

@AnthonyRonning AnthonyRonning merged commit d03c1bf into master Feb 6, 2024
5 checks passed
@AnthonyRonning AnthonyRonning deleted the fedimint-sweep branch February 6, 2024 11:14
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