aljaz/lightning
1
0
Fork 0
mirror of https://github.com/aljazceru/lightning.git synced 2025-12-19 15:14:23 +01:00
Code Issues Packages Projects Releases Wiki Activity

gossipd: fix gossmap race.

Browse Source 
When upgrading a channel from private to public, we would delete the
private channel then add the public one.  However, this is visible in
the gossmap!  In particular, the node may be removed from the gossmap
and then re-added, so it may temporarily vanish!

I was seeing an occasional assertion inside node_factory.line_graph:

```
@pytest.mark.developer
    def test_reconnect_remote_sends_no_sigs(node_factory):
        """We re-announce, even when remote node doesn't send its announcement_signatures on reconnect.
        """
>       l1, l2 = node_factory.line_graph(2, wait_for_announce=True, opts={'may_reconnect': True})

tests/test_connection.py:870: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
contrib/pyln-testing/pyln/testing/utils.py:1467: in line_graph
    self.join_nodes(nodes, fundchannel, fundamount, wait_for_announce, announce_channels)
contrib/pyln-testing/pyln/testing/utils.py:1460: in join_nodes
    wait_for(lambda: 'alias' in only_one(end.rpc.listnodes(n.info['id'])['nodes']))
contrib/pyln-testing/pyln/testing/utils.py:88: in wait_for
    while not success():
contrib/pyln-testing/pyln/testing/utils.py:1460: in <lambda>
    wait_for(lambda: 'alias' in only_one(end.rpc.listnodes(n.info['id'])['nodes']))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

arr = []

    def only_one(arr):
        """Many JSON RPC calls return an array; often we only expect a single entry
        """
>       assert len(arr) == 1
E       AssertionError
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell
2022-06-26 14:08:01 +09:30
parent 2b7915359c
commit 11de721ba9
1 changed files with 62 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

100
gossipd/routing.c
View File

@@ -493,18 +493,23 @@ static void destroy_chan_check(struct chan *chan)
}
#endif
/* We used to make this a tal_add_destructor2, but that costs 40 bytes per
* chan, and we only ever explicitly free it anyway. */
void free_chan(struct routing_state *rstate, struct chan *chan)
static void free_chans_from_node(struct routing_state *rstate, struct chan *chan)
{
remove_chan_from_node(rstate, chan->nodes[0], chan);
remove_chan_from_node(rstate, chan->nodes[1], chan);
uintmap_del(&rstate->chanmap, chan->scid.u64);
#if DEVELOPER
chan->sat.satoshis = (unsigned long)chan; /* Raw: dev-hack */
#endif
}
/* We used to make this a tal_add_destructor2, but that costs 40 bytes per
* chan, and we only ever explicitly free it anyway. */
void free_chan(struct routing_state *rstate, struct chan *chan)
{
free_chans_from_node(rstate, chan);
uintmap_del(&rstate->chanmap, chan->scid.u64);
tal_free(chan);
}
@@ -789,13 +794,45 @@ static void add_channel_announce_to_broadcast(struct routing_state *rstate,
rstate->local_channel_announced |= is_local;
}
static void delete_chan_messages_from_store(struct routing_state *rstate,
struct chan *chan)
{
int update_type, announcment_type;
if (is_chan_public(chan)) {
update_type = WIRE_CHANNEL_UPDATE;
announcment_type = WIRE_CHANNEL_ANNOUNCEMENT;
} else {
update_type = WIRE_GOSSIP_STORE_PRIVATE_UPDATE;
announcment_type = WIRE_GOSSIP_STORE_PRIVATE_CHANNEL;
}
/* If these aren't in the store, these are noops. */
gossip_store_delete(rstate->gs,
&chan->bcast, announcment_type);
gossip_store_delete(rstate->gs,
&chan->half[0].bcast, update_type);
gossip_store_delete(rstate->gs,
&chan->half[1].bcast, update_type);
}
void remove_channel_from_store(struct routing_state *rstate,
struct chan *chan)
{
/* Put in tombstone marker */
gossip_store_mark_channel_deleted(rstate->gs, &chan->scid);
/* Now delete old entries. */
delete_chan_messages_from_store(rstate, chan);
}
bool routing_add_channel_announcement(struct routing_state *rstate,
const u8 *msg TAKES,
struct amount_sat sat,
u32 index,
struct peer *peer)
{
struct chan *chan;
struct chan *oldchan;
secp256k1_ecdsa_signature node_signature_1, node_signature_2;
secp256k1_ecdsa_signature bitcoin_signature_1, bitcoin_signature_2;
u8 *features;
@@ -821,34 +858,35 @@ bool routing_add_channel_announcement(struct routing_state *rstate,
/* The channel may already exist if it was non-public from
* local_add_channel(); normally we don't accept new
* channel_announcements. See handle_channel_announcement. */
chan = get_channel(rstate, &scid);
oldchan = get_channel(rstate, &scid);
/* private updates will exist in the store before the announce: we
* can't index those for broadcast since they would predate it, so we
* add fresh ones. */
if (chan) {
if (oldchan) {
/* If this was in the gossip_store, gossip_store is bad! */
if (index) {
status_broken("gossip_store channel_announce"
" %u replaces %u!",
index, chan->bcast.index);
index, oldchan->bcast.index);
return false;
}
/* Reload any private updates */
if (chan->half[0].bcast.index)
if (oldchan->half[0].bcast.index)
private_updates[0]
= gossip_store_get_private_update(NULL,
rstate->gs,
chan->half[0].bcast.index);
if (chan->half[1].bcast.index)
oldchan->half[0].bcast.index);
if (oldchan->half[1].bcast.index)
private_updates[1]
= gossip_store_get_private_update(NULL,
rstate->gs,
chan->half[1].bcast.index);
oldchan->half[1].bcast.index);
remove_channel_from_store(rstate, chan);
free_chan(rstate, chan);
/* We don't delete it from store until *after* we've put the
* other one in! */
uintmap_del(&rstate->chanmap, oldchan->scid.u64);
}
uc = tal(rstate, struct unupdated_channel);
@@ -875,6 +913,15 @@ bool routing_add_channel_announcement(struct routing_state *rstate,
routing_add_channel_update(rstate, take(private_updates[1]), 0,
peer, false);
/* Now we can finish cleanup of gossip store, so there's no window where
* channel (or nodes) vanish. */
if (oldchan) {
/* Mark private messages deleted, but don't tombstone the channel! */
delete_chan_messages_from_store(rstate, oldchan);
free_chans_from_node(rstate, oldchan);
tal_free(oldchan);
}
return true;
}
@@ -1419,29 +1466,6 @@ static const struct node_id *get_channel_owner(struct routing_state *rstate,
return NULL;
}
void remove_channel_from_store(struct routing_state *rstate,
struct chan *chan)
{
int update_type, announcment_type;
if (is_chan_public(chan)) {
update_type = WIRE_CHANNEL_UPDATE;
announcment_type = WIRE_CHANNEL_ANNOUNCEMENT;
} else {
update_type = WIRE_GOSSIP_STORE_PRIVATE_UPDATE;
announcment_type = WIRE_GOSSIP_STORE_PRIVATE_CHANNEL;
}
gossip_store_mark_channel_deleted(rstate->gs, &chan->scid);
/* If these aren't in the store, these are noops. */
gossip_store_delete(rstate->gs,
&chan->bcast, announcment_type);
gossip_store_delete(rstate->gs,
&chan->half[0].bcast, update_type);
gossip_store_delete(rstate->gs,
&chan->half[1].bcast, update_type);
}
u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES,
struct peer *peer,
struct short_channel_id *unknown_scid,