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

Keep channel announcements when pruning channels #5839

Merged
merged 5 commits into from Jan 30, 2023

Conversation

endothermicdev
Copy link
Collaborator

BOLT 7 allows pruning channels when one side's channel_update expires. This results in the channel_announcement being deleted. When the offline node sends a new channel_update, the channel_announcement will be unavailable and the channel is not reestablished.

Here instead of being deleted entirely, the channel is zombified: the channel_announcement is maintained along with the still relevant channel_update in case the expired channel_update is ever refreshed. These are marked with a zombie bit in the gossip store so that they are available to gossipd, but ignored by gossmap and other consumers. In the case that all channels of a node become zombies, the node_announcement is tagged as well.

Two valid channel_updates will resurrect a channel (and possibly a node), deleting the zombie tagged gossip entries and replacing them with fresh entries.

Changelog-Fixed: Pruned channels are more reliably restored.

@cdecker
Copy link
Member

cdecker commented Dec 20, 2022

I would have expected the direction that has the timed out channel_update just be immobilized by removing the information from the channel_update. This would be symmetric to the way we may have a partially initialized channel in case we got the announcement and only one update. The channel is functional in one direction, but naturally can't be used in the other direction.

I'm wondering what the added benefit of yet another state is, but then again I haven't reviewed the PR, just an upfront question.

@endothermicdev
Copy link
Collaborator Author

A node:
  - if the `timestamp` of the latest `channel_update` in either direction is
  older than two weeks (1209600 seconds):
    - MAY prune the channel.

The goal was to maintain this previous behavior, wherein the channel is removed from the routing graph (but some information must be maintained in order to restore it.) From a practical perspective, if a node fails to update the other side of the channel, would we ever be interested in trying to route through the updated side? Partially disabling the channel would certainly be simpler, but what is the practical use case? It seems to me it would make for a worse user experience to keep the partial channel in the graph as the odds of successfully routing would be ~0%.

@endothermicdev endothermicdev added this to the v23.02 milestone Dec 20, 2022
@endothermicdev endothermicdev force-pushed the zombie-channels branch 2 times, most recently from 9965a6f to 2ef7574 Compare December 21, 2022 00:40
@rustyrussell rustyrussell self-requested a review January 17, 2023 04:57
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ah, this zombifies every channel. An interesting (and IMHO totally valid!) choice.

Needs minor fix for rebasing on master (peers.channels is deprecated!).

diff --git a/tests/test_gossip.py b/tests/test_gossip.py
index a4803ea8b..7566828cf 100644
--- a/tests/test_gossip.py
+++ b/tests/test_gossip.py
@@ -2256,7 +2257,7 @@ def test_channel_ressurection(node_factory, bitcoind):
     l2.rpc.call('dev-gossip-set-time', [prune_due])
     l3.rpc.call('dev-gossip-set-time', [prune_due])
     # Make sure l1 is recognized as disconnected
-    wait_for(lambda: 'connected' not in only_one(l2.rpc.listpeers()['peers'][0]['channels']))
+    wait_for(lambda: not only_one(l2.rpc.listpeers()['peers'])['connected'])
     # Wait for the channel to be pruned.
     wait_for(lambda: l2.rpc.listchannels()['channels'] == [])
     l3.daemon.wait_for_log("Pruning channel")

Also, first patch test fails: You need to add @pytest.mark.xfail(strict=True) in that first commit, and remove it at the end.

Finally, s/test_channel_ressurection/test_channel_resurrection/ !

This will allow gossipd to store and persist gossip for channels rather
than deleting them entirely when the channels are pruned from the
network.
Though BOLT 7 says a channel may be pruned when one side becomes inactive
and fails to refresh their channel_update, in practice, the
channel_announcement can be difficult to recover if deleted entirely.
Here the channel_announcement is tagged as zombie such that gossip_store
consumers may safely ignore it, but it may be retained should the channel
come back online in the future. Node_announcements and channel_updates may
also be retained in such a fashion until the channel is ready to be
resurrected.

Changelog-Fixed: Pruned channels are more reliably restored.
@rustyrussell
Copy link
Contributor

rustyrussell commented Jan 30, 2023

Ack 5cbd4dd

@rustyrussell rustyrussell merged commit 98bfd23 into ElementsProject:master Jan 30, 2023
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