edouardparis / lntop

:zap: LN terminal dashboard :bar_chart:
MIT License
183 stars 24 forks source link

Fix crash on missing policies #78

Closed rkfg closed 2 years ago

rkfg commented 2 years ago

Fixes a crash introduced in #72 if one or both policies are missing for any channel. It happens for new channels usually if policies aren't propagated quick enough. This PR adds checks in display and sort functions so missing policies default to zero values.

rkfg commented 2 years ago

Noticed another very important case when one of the policies is missing: private channels. I have a few of them and they all have only one policy on the node side. Which makes sense since the devices on the other side don't route so they don't need a policy but forwarding to them indeed requires one. Without this patch lntop crashes if you have private channels and enabled any fee columns.

hieblmi commented 2 years ago

@rkfg amazing - I was running into this issue locally. Thanks for the fix @rkfg. @edouardparis can we get this merged?

rkfg commented 2 years ago

Thanks for the tip 😄 Just in case, I have my own dev branch that has all my PRs merged altogether and that I use myself. I cherry-pick from it to separate branches for PRs. Some of my PRs here can't be merged together (need rebasing) and this branch has all sorted out. As soon as review/fix/merge process begins I'll update them to be compatible depending on which PR gets merged first.

Also, the policies will be swapped for some channels if your peer's ID is numerically lower than your own so your policy goes second. It seems to be the rule, at least for lnd. My other PR fixes it by swapping the policies so that yours is always first but I decided not to complicate things and leave it there. My node's ID starts with 021 so it's pretty low and this issue only affected a couple of channels. For those with id starting with 03 it might look completely invalid all the time.

hieblmi commented 2 years ago

Thanks for the effort, will checkout your branch and test it on testnet. Hope that @edouardparis comes back to maintain his repo :-).

rkfg commented 2 years ago

Try Polar as well, it's much faster to develop with it than with unpredictable slow testnet. The only small inconvenience for me is that it automatically generates blocks when you open or close channels from the GUI so it's not possible to debug the process of opening/closing itself (and various state changes). But it lets you open a terminal window for any node and run lncli commands there, then you're free to generate blocks one by one and see what happens in your program.

hieblmi commented 2 years ago

Thanks for the tip, I had started Polar a while back and played around with lncli and channel openings there. Since I am currently working a bit on lnd I prefer to take the time and do the real thing on testnet to test my patches. Also good for reproducibility and test evidence when committing PRs...

edouardparis commented 2 years ago

Yes, testing it right now

edouardparis commented 2 years ago

Thanks for the valuable work