From 88a4502a51133c23dd9a3106924223c48644564f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 25 Jan 2022 06:34:52 +1030 Subject: [PATCH] channeld: pause before sending initial channel_update. The last change exposed a race: the peer sends funding_locked then immediately sends an update_channel. channeld used to process the funding_locked from the peer, tell gossipd about the new channel, then finally forward the channel_update. We can have the channel_update hit gossipd before we've told it about the channel. It ignores the channel_update for the currently-unknown channel: we get a 'bad gossip' message, but the immediate symptom is a timeout in tests/test_closing.py::test_onchain_multihtlc_their_unilateral: ``` node_factory = bitcoind = @pytest.mark.developer("needs DEVELOPER=1 for dev_ignore_htlcs") @pytest.mark.slow_test def test_onchain_multihtlc_their_unilateral(node_factory, bitcoind): """Node pushes a channel onchain with multiple HTLCs with same payment_hash """ > h, nodes = setup_multihtlc_test(node_factory, bitcoind) tests/test_closing.py:2938: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ tests/test_closing.py:2780: in setup_multihtlc_test nodes = node_factory.line_graph(7, wait_for_announce=True, /usr/local/lib/python3.8/dist-packages/pyln/testing/utils.py:1416: in line_graph self.join_nodes(nodes, fundchannel, fundamount, wait_for_announce, announce_channels) /usr/local/lib/python3.8/dist-packages/pyln/testing/utils.py:1394: in join_nodes nodes[i + 1].wait_channel_active(scids[i]) /usr/local/lib/python3.8/dist-packages/pyln/testing/utils.py:958: in wait_channel_active wait_for(lambda: self.is_channel_active(chanid)) ``` Note that we are usually much faster to send between subds than we are between peers, but during CI this is common, as we're all running on the same machine. Signed-off-by: Rusty Russell --- channeld/channeld.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index a3d9fa113..c0a2d4685 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -419,6 +419,13 @@ static void send_channel_update(struct peer *peer, int disable_flag) wire_sync_write(MASTER_FD, take(msg)); } +/* Tell gossipd and the other side what parameters we expect should + * they route through us */ +static void send_channel_initial_update(struct peer *peer) +{ + send_channel_update(peer, 0); +} + /** * Add a channel locally and send a channel update to the peer * @@ -442,9 +449,12 @@ static void make_channel_local_active(struct peer *peer) annfeatures); wire_sync_write(MASTER_FD, take(msg)); - /* Tell gossipd and the other side what parameters we expect should - * they route through us */ - send_channel_update(peer, 0); + /* Under CI, because blocks come so fast, we often find that the + * peer sends its first channel_update before the above message has + * reached it. */ + notleak(new_reltimer(&peer->timers, peer, + time_from_sec(5), + send_channel_initial_update, peer)); } static void send_announcement_signatures(struct peer *peer)