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: add button to auto-balance LN channels #859
base: master
Are you sure you want to change the base?
Conversation
I need some help here. After fixing the mentioned bug, I can add tests and finish the remaining work in the PR. |
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.
Trying to auto-balance the channel I got the error
Insufficient balance
, but this should not happen since the source has more than enough funds to pay the target and that can be confirmed by logging the amounts in the console.
I tested this PR and it's working fine for me. I'm not sure why you're getting that error on your end. Can you share more of the LND logs from one of the nodes?
Since it works for me, I left some feedback on the code.
f473d7a
to
01e3f8c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #859 +/- ##
===========================================
- Coverage 100.00% 99.16% -0.84%
===========================================
Files 140 140
Lines 4582 4447 -135
Branches 851 862 +11
===========================================
- Hits 4582 4410 -172
- Misses 0 37 +37 ☔ View full report in Codecov by Sentry. |
@jamaljsr I addressed nearly all issues except the dropdown menu. I can implement tests for the added functionality, but I stil get the following error:
You said it worked on your side, so I ask you to test again. I'll try to figure out why it is not working on my machine. Actually, I'm getting some buggy behavior unrelated to the PR: sometimes I can't click on anything, which I suppose not to be a polar bug but a bug with electron due to my window manager. |
I'll be back to it in a few days. |
Hey @uwla are you planning to continue to work on this PR? I took a look at your latest changes and noticed what is causing the promisesToAwait.push(
createInvoice({ node: target, amount }).then(invoice =>
payInvoice({ node: source, amount, invoice }),
),
); |
Thanks @jamaljsr ! I am back to the PR, working on it today |
I got some problems:
After playing a bit, I found out I am also getting the same behavior when generating and paying invoices manually with the mentioned implementations: Create invoice -> Copy and Close -> Pay invoices. The only which works pretty well is LND to LND. |
Thanks for the details on the issues your facing. I think I've found solutions to your outstanding problems.
I had to dive pretty deep into the weeds to figure this one out. I was able to reproduce the behavior on my end as well. The problem is essentially a configuration issue on the Eclair side. By default, it won't let you make a single payment that is more than 45% of the channel capacity. Check the reference.conf for a more detailed explanation. It would be nice if Eclair logged a clearer error message, but it doesn't. When I changed this to 100%, then Eclair began behaving correctly.
Copy this config value into the default Eclair startup flags in constants.ts.
This error is actually saying that you should not provide the The API Docs state: "The pay RPC command attempts to find a route to the given destination, and send the funds it asks for. If the bolt11 does not contain an amount, amount_msat is required, otherwise if it is specified it must be null." I also saw this error in my testing and remove the |
After testing this a bit more, I feel like immediately balancing the channels when the button is clicked can be a bit unexpected. It would be very cool if it instead displayed a modal with the details of the payments that will be made on each channel. Then when the Submit button in the modal is clicked, the channels would get balanced. What do you think about this idea? |
@jamaljsr I think it is a good idea. So, we show a modal and for each channel:
After all channels, we add a button on the bottom "auto balance all channels", which will autobalance the preview values. Finally, at the very bottom, a CONFIRM button an a CANCEL button. If user confirms, each channel will be updated in batch if needed. Otherwise the modal is hidden and its state reset. What about it now? |
I can create the modal and add some tests for the coverage not to drop. However, I have been facing an weird issue while developing polar locally: after launching the live Electron app, a lot of times I can not click on buttons, and the cursor icon does not alter (it is the same as normal, instead of a click pointer). This has been the most headache thing, because I have to open the devtools and do something like |
After writing the above response, I did a quick google search and found the cause of the issue. It is the |
@jamaljsr I have sync my fork and now the bug is fixed. I'm working on the dropdown menu and the modal. |
Hey @jamaljsr, when creating a 250,000 sats channels, which is the default, I noticed if the implementation is LND, then the source starts with 246530 sats and the target 0 sats. Where are the missing sats? |
01e3f8c
to
cb98bbe
Compare
I have not finished the PR, but I pushed it anyway so you can see how it is going. I added the dropdown and the modal. I still need to: implement business logic, add tests, code styling refactoring. |
Thanks for the update. I will check it out over the next few days. |
I'll add business logic today. The laast PR added UI, but it did not implement any logic |
Sorry, I forgot to answer this question. The balance is decreased by the fee for the commitment transaction and the amount allocated to the anchor output, since Anchor channels are created by default in LND. You can use For example, here's the output for one 250K sat channel I just created.
The balance breakdown is as follows:
|
User can manually balance channels using a slicer. The channels are shown in a modal after clicking a button. NOTE: this is a prototype, not working yet.
cb98bbe
to
4cd78dd
Compare
Thanks @jamaljsr for the updates. My last push had some UI improvements. I will implement business logic tomorrow, by adding a new store action. Things are working pretty well now that I can click buttons and test things with hot-reload, after fixing the react overlay bug. |
With the last push, it is WORKING! At least with LND nodes. It stills does not work well with Clightning and Eclair due to the mentioned bugs, which we were discussing previously. Now, what remains:
@jamaljsr I need your help with (1), since you know what is causing it. I also need your opinion with (4), because I am not familiar with Polar's preferences for code style, where the added functions and interfaces should be placed. |
Did you apply the fixes I mentioned in this prior comment? This resolved the Core Lightning and Eclair issues for me.
I will review your latest changes by tomorrow for sure. |
Solves #831.
NOTE: work in progress, implementation not ready yet.
Done
To do
Help needed here.
Trying to auto-balance the channel I got the error
Insufficient balance
, but this should not happen since the source has more than enough funds to pay the target and that can be confirmed by logging the amounts in the console.I decided to open the PR anyway, so I can get help with the bug.