Disable channel when losing the underlying connection#756
Disable channel when losing the underlying connection#756rustyrussell merged 9 commits intoElementsProject:masterfrom
Conversation
Sends a disable channel_update before issuing the shutdown message, gossipd will also take care to update others and not use for future routes. Signed-off-by: Christian Decker <decker.christian@gmail.com>
Disabling the channel and enqueing the update for broadcast so we don't get forwarding requests from remote peers, and we don't try to ourselves. Signed-off-by: Christian Decker <decker.christian@gmail.com>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We are now too quick in disabling the channel for us to attempt a payment. We need to separate into getroute and sendpay to trigger this now. Signed-off-by: Christian Decker <decker.christian@gmail.com>
850859a to
25bcaeb
Compare
lightningd/peer_control.c
Outdated
| if (peer->ld->config.ignore_fee_limits) | ||
| log_debug(peer->log, "Ignoring fee limits!"); | ||
|
|
||
| peer->direction = pubkey_cmp(&peer->ld->id, &peer->id) > 0; |
There was a problem hiding this comment.
Hoist the channel_direction() macro out of gossipd perhaps?
There was a problem hiding this comment.
It's in routing.h, so just including that should suffice.
| "disable", | ||
| type_to_string(msg, struct short_channel_id, &scid), | ||
| direction); | ||
| goto fail; |
There was a problem hiding this comment.
In this case, what if active was true? Did we just enable a channel w/o a channel update?
I think this test should be moved up above the assignment?
There was a problem hiding this comment.
Can't do that, otherwise we can only ever deactivate a channel that has reached announce_depth, i.e., 6 blocks. The channel is activated locally after funding_depth, i.e., 2 confirmations. So if we happen to lose the connection or similar, which the channel has 2-5 confirmations we cannot deactivate and we're stuck with a non-functioning channel being returned as part of the routes.
If the channel is public, we also have a channel_update which we can update with the disable flag, if the channel is not public, then others don't care anyway.
There was a problem hiding this comment.
Good point.
OK, my q still stands. We're asserting that active == false in this branch (based on the msg), we should actually assert that?
I'll apply this now, but please consider...
Can't push the deactivation further down, addressed the macro inclusion.
We were not updating our own local view of the network, nor were we sending out disabling
channel_updates in case we lose connection. This PR fixes that.Fixes #751
Fixes #635
Fixes #682