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

Show closed and disabled channels, fix policy order #76

Merged
merged 4 commits into from
Sep 19, 2022

Conversation

rkfg
Copy link
Contributor

@rkfg rkfg commented Sep 1, 2022

This PR fixes a couple of annoyances:

  1. New channels displayed incoming/outgoing policies wrong due to outdated information. Now these policies are fetched on every channel update if any of them is nil. This should only happen when the channel is very new and the policy information isn't well propagated. It won't result in a big gRPC request inefficiency.
  2. Closed (fully resolved) channels are now marked as such explicitly. It's possible to delete these channels when they're closed but it might be confusing so I decided to add a new channel status closed instead and mark all channels that aren't present in the listchannels result as closed. Additionally I added channel status subscription, there was such a function already but its implementation was mostly commented out so I finished it. For now it simply triggers channel update when any channel is reported as fully resolved. To remove the closed channels from the list currently the user should restart the program. Maybe in the future it would be worth it to add a special key bind to fully refresh the channel information.
  3. Disabled channels are marked with red arrows in the status column. ⇊ — disabled to us, ⇈ — disabled from us, ⇅ — disabled in both directions. Characters I picked are debatable of course, let me know if you like them or not and what to use.
    2022-09-03_00-41-36

There's also one more event subscription added, to graph events. Currently it only retrieves updated chan points and updates policies for corresponding channels. The rest of event information isn't used.

PS: there's also a very small alias cleanup that removes \ufe0f character because it breaks the UI. When the cursor is on that line all columns after the alias are shifted by 1 character to the left and some positions aren't redrawn after that. I think there are more such breaking characters but I've only encountered one in this node alias.
PPS: I think fixing the underlying TUI library is a better solution but it might be a much harder task. I tried upgrading to the modern fork of gocui and the UI was in shambles, the columns were not lined up at all. Probably it needs more efforts to fix.
The character I remove simply switches the previous symbol rendering from text-like to emoji-like (colored and shiny), cool thing to have but not at the cost of broken UI.

@rkfg
Copy link
Contributor Author

rkfg commented Sep 2, 2022

Looks like the policy issue isn't solved by this. I opened a new channel and the incoming/outgoing policies were still reversed. They're shown fine if the channel was present before the program was started but for new channels it's a problem. I use Polar for testing, really great LN "simulator", it runs real bitcoind/lnd and other implementations in Docker containers and lets quickly create nodes and channels. I'll see what can be done.

The main issue is that policies aren't returned as a part of the channel data, they're requested separately and not even every time the channel is updated in the UI. The results can be also incomplete if gossip isn't yet propagated fully. And worst of all there's no "local" and "remote" side, just policy1 and policy2 which should be corresponded to local/remote sides using separate node1pub/node2pub parameters. A lot of moving parts and edge cases it seems...

@rkfg
Copy link
Contributor Author

rkfg commented Sep 2, 2022

Okay, a much simpler solution is to remove the ordering parameter and simply swap the policies in case ours isn't first. It should also fix #72 probably showing the opposite fees for some channels. I'm not 100% sure but it seems the policies are ordered by the node ids so if your node id is numerically greater than your peer's then your policy will be the second. For our purposes their numbers don't matter so it should be safe to swap and consider us always first in all cases.

Sometimes only one policy is available. If it takes the slot 1 no swap is required and if it's in the second slot then slot 1 equals nil and since it's not equal to our pubkey a swap happens and we land in slot 1 anyway. I don't think a situation is possible where only the peer's policy is present but not our own. If that's the case then they might be swapped if the peer's policy is in slot 2. They will go back to normal as soon as both policies are learned. I've never seen it so to not complicate the code I decided to leave it as is.

@rkfg rkfg changed the title Show closed channels, fix policy order Show closed and disabled channels, fix policy order Sep 2, 2022
@rkfg
Copy link
Contributor Author

rkfg commented Sep 2, 2022

Added displaying channel status

@rkfg
Copy link
Contributor Author

rkfg commented Sep 4, 2022

Pinging @edouardparis just in case he's not watching his own repo! It happened to me a couple of times when I accidentally unsubscribed from updates.

network/backend/lnd/lnd.go Outdated Show resolved Hide resolved
@rkfg
Copy link
Contributor Author

rkfg commented Sep 13, 2022

Policies renamed, branch rebased.

@edouardparis
Copy link
Owner

Seems good to me, tested with polar this weekend ! Thanks !

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.

None yet

3 participants