Fix node announce self-advertize and advertize both sides of channels

gossipd/gossipd.c

@@ -439,29 +439,33 @@ static void dump_our_gossip(struct daemon *daemon, struct peer *peer)
 		return;
 
 	for (chan = first_chan(me, &i); chan; chan = next_chan(me, &i)) {
-		int dir = half_chan_idx(me, chan);
-
+		/* Don't leak private channels, unless it's with you! */
 		if (!is_chan_public(chan)) {
-			/* Don't leak private channels, unless it's with you! */
-			if (!node_id_eq(&chan->nodes[!dir]->id, &peer->id))
-				continue;
-			/* There's no announce for this, of course! */
-			/* Private channel updates are wrapped in the store. */
-			else {
-				if (!is_halfchan_defined(&chan->half[dir]))
-					continue;
+			int dir = half_chan_idx(me, chan);
+
+			if (node_id_eq(&chan->nodes[!dir]->id, &peer->id)
+			    && is_halfchan_defined(&chan->half[dir])) {
+				/* There's no announce for this, of course! */
+				/* Private channel updates are wrapped in the store. */
 				queue_priv_update(peer, &chan->half[dir].bcast);
-				continue;
 			}
-		} else {
-			/* Send announce */
-			queue_peer_from_store(peer, &chan->bcast);
+			continue;
 		}
 
-		/* Send update if we have one */
-		if (is_halfchan_defined(&chan->half[dir]))
-			queue_peer_from_store(peer, &chan->half[dir].bcast);
+		/* Send channel_announce */
+		queue_peer_from_store(peer, &chan->bcast);
+
+		/* Send both channel_updates (if they exist): both help people
+		 * use our channel, so we care! */
+		for (int dir = 0; dir < 2; dir++) {
+			if (is_halfchan_defined(&chan->half[dir]))
+				queue_peer_from_store(peer, &chan->half[dir].bcast);
+		}
 	}
+
+	/* If we have one, we should send our own node_announcement */
+	if (me->bcast.index)
+		queue_peer_from_store(peer, &me->bcast);
 }
 
 /*~ This is where connectd tells us about a new peer we might want to

tests/test_gossip.py

@@ -1383,20 +1383,22 @@ def test_gossipwith(node_factory):
                          check=True,
                          timeout=TIMEOUT, stdout=subprocess.PIPE).stdout
 
-    num_msgs = 0
+    msgs = set()
     while len(out):
         l, t = struct.unpack('>HH', out[0:4])
+        msg = out[2:2 + l]
         out = out[2 + l:]
 
         # Ignore pings, timestamp_filter
         if t == 265 or t == 18:
             continue
         # channel_announcement node_announcement or channel_update
         assert t == 256 or t == 257 or t == 258
-        num_msgs += 1
+        msgs.add(msg)
 
     # one channel announcement, two channel_updates, two node announcements.
-    assert num_msgs == 7
+    # due to initial blast, we can have duplicates!
+    assert len(msgs) == 5
 
 
 def test_gossip_notices_close(node_factory, bitcoind):
@@ -2293,3 +2295,40 @@ def test_channel_resurrection(node_factory, bitcoind):
     for l in gs.stdout.decode().splitlines():
         if "ZOMBIE" in l:
             assert ("DELETED" in l)
+
+
+def test_dump_own_gossip(node_factory):
+    """We *should* send all self-related gossip unsolicited, if we have any"""
+    l1, l2 = node_factory.line_graph(2, wait_for_announce=True)
+
+    # Make sure l1 has updates in both directions, and node_announcements
+    wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 2)
+    wait_for(lambda: len(l1.rpc.listnodes()['nodes']) == 2)
+
+    # We should get channel_announcement, channel_update, node_announcement.
+    # (Plus random pings, timestamp_filter)
+    out = subprocess.run(['devtools/gossipwith',
+                          '--timeout-after={}'.format(int(math.sqrt(TIMEOUT) + 1)),
+                          '{}@localhost:{}'.format(l1.info['id'], l1.port)],
+                         check=True,
+                         timeout=TIMEOUT, stdout=subprocess.PIPE).stdout
+
+    # In theory, we could do the node_announcement any time after channel_announcement, but we don't.
+    expect = [256,  # channel_announcement
+              258,  # channel_update
+              258,  # channel_update
+              257]  # node_announcement
+
+    while len(out):
+        l, t = struct.unpack('>HH', out[0:4])
+        out = out[2 + l:]
+
+        # Ignore pings, timestamp_filter
+        if t == 265 or t == 18:
+            continue
+
+        assert t == expect[0]
+        expect = expect[1:]
+
+    # We should get exactly what we expected.
+    assert expect == []