Channel UI feature: Real-time updates of channels status

[kelvinator07]

Closes #603 and #418

Description

Opening/Closing a lightning channel outside of Polar doesn't reflect on the UI. A click on the Sync button is required to update the UI.

The feature aims to update the UI automatically every time it detects a channel is opened, pending, or closed between lightning nodes. This is implemented by subscribing to each lightning node on startup via a listener that listens for channel events that occur outside of the Polar app and updates the UI accordingly.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (06a7210) to head (9243629).
Report is 33 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##            master      #837    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          140       140            
  Lines         4428      4581   +153     
  Branches       862       852    -10     
==========================================
+ Hits          4428      4581   +153     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jamaljsr]

Thanks for working on this feature. I've wanted this for a pretty long time.

Regarding the functionality, it mostly works as expected. I just have these suggestions:

1. I tested by opening a Terminal on alice and running lncli openchannel .... It didn't display the channel until I mined a few blocks. I know LND sends channel events as soon as the channel opening txn is broadcast. I think it should show pending channels in the UI when this happens.
2. I noticed the syncChart is called many times when I have a few LND nodes in the network. This function is pretty heavy since is makes a bunch of RPC calls for each node. It would be better to only run syncChart once, maybe it can be throttled by time.
3. Right now there's only listeners for channel open events. It would be nice to also listen for channel closes and sent/received payments. I think it would be good to have at least two because it'll force you to create better abstractions for the listeners.

I also left some specific feedback on the code below. I hope this is helpful.

[kelvinator07]

@jamaljsr Thanks for the feedback. Would update and revert.

[jamaljsr]

Thanks for the additional updates. This is looking better. I didn't get a change to review all of the code in the free time I had this morning, but here is some feedback on what I've gone through so far. I will continue reviewing the rest and doing more testing later this week.

[jamaljsr]

I was able to get through the rest of the code. Here's some additional feedback.

[kelvinator07]

@jamaljsr This PR has been updated based on your last comments and is ready to be reviewed.

[jamaljsr]

Thanks for the updates. This is getting very close to done.

I left a bit more feedback on your latest changes. It's mostly small stuff to tidy up the code.

The only other things to address are:

• rebase this branch on master
• fix unit tests and fill the remaining code coverage

Once these are done, I think this will be pretty much ready to merge.

[kelvinator07]

Thanks for the updates. This is getting very close to done.

I left a bit more feedback on your latest changes. It's mostly small stuff to tidy up the code.

The only other things to address are:

• rebase this branch on master
• fix unit tests and fill the remaining code coverage

Once these are done, I think this will be pretty much ready to merge.

Thanks for the feedback. Would wrap this up by the end of this week.

[kelvinator07]

Thanks for the updates. This is getting very close to done.

I left a bit more feedback on your latest changes. It's mostly small stuff to tidy up the code.

The only other things to address are:

• rebase this branch on master
• fix unit tests and fill the remaining code coverage

Once these are done, I think this will be pretty much ready to merge.

@jamaljsr PR has been updated based on your last comments and is ready to be reviewed. Thanks

[jamaljsr]

Thanks for continuing to push this forward. I have a few more comments.

Also, it looks like the tests are failing due to the WebSocket. Can this be mocked?

[jamaljsr]

@kelvinator07 I have pushed a commit to your branch that adds the additional unit tests needed to get the code coverage back to 100%. There were a couple of places where code paths in the app were completely unreachable. I had to modify the implementation to eliminate those branches. This is one of the benefits of maintaining high coverage, it forces you to eliminate code that can never be executed.

I also did another round of testing for each implementation. The real time events are working great.

I am going to rebase this branch on master and once the CI completes, I will merge this PR. Thank you so much for completing this new feature. I'm looking forward to your next contribution 🎉

[jamaljsr]

Oh, I also forgot to mention that in #879 I upgraded Polar to use the latest Core Lightning implementation and switched to using the embedded REST API which now supports WebSocket streaming 🙌. So the I am going to remove the polling and implement receiving events via sockets in that PR.

[kelvinator07]

Oh, I also forgot to mention that in #879 I upgraded Polar to use the latest Core Lightning implementation and switched to using the embedded REST API which now supports WebSocket streaming 🙌. So the I am going to remove the polling and implement receiving events via sockets in that PR.

Awesome! I'll check out the PR.