Skip to content

Conversation

blico
Copy link
Contributor

@blico blico commented Sep 25, 2017

An amendment was recently made in the set of specifications to recommend that nodes prune stale entries from their channel graph as it's possible for channel to become completely inactive for a variety of reasons.

What this change includes:

  • Within AuthenticatedGossiper's ChannelUpdate re-broadcast loop added a prune check, which prunes an edge from the graph if it is at least 2 weeks old.
  • Modified the main ChannelUpdate re-broadcast loop to only rebroadcast a ChannelUpdate if it has not been updated for at least 13 days.
  • Created an updateChannel function which creates a new ChannelUpdate, updates the edge Timestamp field, re-signs the channel, and writes the updated ChannelEdgePolicy to disk. This function is used by both processFeeChanUpdate and the re-broadcast loop.

@mention-bot
Copy link

@blico, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Roasbeef, @AndrewSamokhvalov and @cfromknecht to be potential reviewers.

Copy link
Contributor

@jimpo jimpo left a comment

Choose a reason for hiding this comment

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

The refactor of processFeeChanUpdate into updateChannel looks good to me.

With regards to the pruning, however, I believe the idea is to prune channels that we are not a participant in, whereas this only prunes our own outgoing channels. In fact, after this change that should no longer be possible since the edge is updated before the prune interval. I think instead you want to use Graph.ForEachChannel and it probably no longer makes sense to do it at the same time as the rebroadcast .

Not sure whether the resigning channel updates every 13 days is a good idea or not, but it makes sense to me.

Notes:

  • Rebroadcast interval is changed from every 30 minutes to every 13 days.
  • All outgoing channels are automatically every 13 days to prevent pruning.

@Roasbeef
Copy link
Member

Roasbeef commented Oct 5, 2017

Thanks for doing the initial work to implement this @blico!

I've picked this issue up, and implemented the fixes for the review items @jimpo pointed out, and also added some initial functionality. I've left your original commits in place in order to ensure you get credit for the initial work.

Check out https://git.io/vdBkH to see the of commits pushed.

@Roasbeef Roasbeef closed this Oct 5, 2017
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.

4 participants