peer_control: getinfo show correct port on discovered IPs

Changelog-Fixed: peer_control: getinfo shows the correct port on discovered IPs

Author: m-schmoock

gossipd/gossipd.c

@@ -350,9 +350,6 @@ static void handle_remote_addr(struct daemon *daemon, const u8 *msg)
 	if (!fromwire_gossipd_remote_addr(msg, &remote_addr))
 		master_badmsg(WIRE_GOSSIPD_REMOTE_ADDR, msg);
 
-	/* Best guess is that we use default port for the selected network */
-	remote_addr.port = chainparams_get_ln_port(chainparams);
-
 	switch (remote_addr.type) {
 	case ADDR_TYPE_IPV4:
 		if (daemon->remote_addr_v4 != NULL &&

lightningd/lightningd.c

@@ -209,6 +209,8 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 
 	ld->remote_addr_v4 = NULL;
 	ld->remote_addr_v6 = NULL;
+	ld->discovered_ip_v4 = NULL;
+	ld->discovered_ip_v6 = NULL;
 	ld->listen = true;
 	ld->autolisten = true;
 	ld->reconnect = true;

lightningd/lightningd.h

@@ -158,6 +158,10 @@ struct lightningd {
 	struct node_id remote_addr_v4_peer;
 	struct node_id remote_addr_v6_peer;
 
+	/* verified discovered IPs to be used for anouncement */
+	struct wireaddr *discovered_ip_v4;
+	struct wireaddr *discovered_ip_v6;
+
 	/* Bearer of all my secrets. */
 	int hsm_fd;
 	struct subd *hsm;

lightningd/peer_control.c

@@ -1274,10 +1274,17 @@ static void update_remote_addr(struct lightningd *ld,
 			       const struct wireaddr *remote_addr,
 			       const struct node_id peer_id)
 {
+	u16 public_port;
+
 	/* failsafe to prevent privacy leakage. */
 	if (ld->always_use_proxy || ld->config.disable_ip_discovery)
 		return;
 
+	/* Peers will have likey reported our dynamic outbound TCP port.
+	 * Best guess is that we use default port for the selected network,
+	 * until we add a commandline switch to override this. */
+	public_port = chainparams_get_ln_port(chainparams);
+
 	switch (remote_addr->type) {
 	case ADDR_TYPE_IPV4:
 		/* init pointers first time */
@@ -1292,10 +1299,14 @@ static void update_remote_addr(struct lightningd *ld,
 			break;
 		}
 		/* tell gossip we have a valid update */
-		if (wireaddr_eq_without_port(ld->remote_addr_v4, remote_addr))
+		if (wireaddr_eq_without_port(ld->remote_addr_v4, remote_addr)) {
+			ld->discovered_ip_v4 = tal_dup(ld, struct wireaddr,
+						       ld->remote_addr_v4);
+			ld->discovered_ip_v4->port = public_port;
 			subd_send_msg(ld->gossip, towire_gossipd_remote_addr(
 							  tmpctx,
-							  ld->remote_addr_v4));
+							  ld->discovered_ip_v4));
+		}
 		/* store latest values */
 		*ld->remote_addr_v4 = *remote_addr;
 		ld->remote_addr_v4_peer = peer_id;
@@ -1311,10 +1322,14 @@ static void update_remote_addr(struct lightningd *ld,
 			*ld->remote_addr_v6 = *remote_addr;
 			break;
 		}
-		if (wireaddr_eq_without_port(ld->remote_addr_v6, remote_addr))
+		if (wireaddr_eq_without_port(ld->remote_addr_v6, remote_addr)) {
+			ld->discovered_ip_v6 = tal_dup(ld, struct wireaddr,
+						       ld->remote_addr_v6);
+			ld->discovered_ip_v6->port = public_port;
 			subd_send_msg(ld->gossip, towire_gossipd_remote_addr(
 							  tmpctx,
-							  ld->remote_addr_v6));
+							  ld->discovered_ip_v6));
+		}
 		*ld->remote_addr_v6 = *remote_addr;
 		ld->remote_addr_v6_peer = peer_id;
 		break;
@@ -2247,22 +2262,22 @@ static struct command_result *json_getinfo(struct command *cmd,
 		for (size_t i = 0; i < count_announceable; i++)
 			json_add_address(response, NULL, cmd->ld->announceable+i);
 
-		/* Currently, IP discovery will only be announced by gossipd, if we
-		 * don't already have usable addresses.
+		/* Currently, IP discovery will only be announced by gossipd,
+		 * if we don't already have usable addresses.
 		 * See `create_node_announcement` in `gossip_generation.c`. */
 		if (count_announceable == 0) {
-			if (cmd->ld->remote_addr_v4 != NULL &&
+			if (cmd->ld->discovered_ip_v4 != NULL &&
 					!wireaddr_arr_contains(
 						cmd->ld->announceable,
-						cmd->ld->remote_addr_v4))
+						cmd->ld->discovered_ip_v4))
 				json_add_address(response, NULL,
-						 cmd->ld->remote_addr_v4);
-			if (cmd->ld->remote_addr_v6 != NULL &&
+						 cmd->ld->discovered_ip_v4);
+			if (cmd->ld->discovered_ip_v6 != NULL &&
 					!wireaddr_arr_contains(
 						cmd->ld->announceable,
-						cmd->ld->remote_addr_v6))
+						cmd->ld->discovered_ip_v6))
 				json_add_address(response, NULL,
-						 cmd->ld->remote_addr_v6);
+						 cmd->ld->discovered_ip_v6);
 		}
 		json_array_end(response);
 

tests/test_connection.py

@@ -83,6 +83,7 @@ def test_remote_addr(node_factory, bitcoind):
     l2.daemon.opts['bind-addr'] = l2.daemon.opts['addr']
     del l2.daemon.opts['addr']
     l2.start()
+    assert len(l2.rpc.getinfo()['address']) == 0
 
     l2.rpc.connect(l1.info['id'], 'localhost', l1.port)
     logmsg = l2.daemon.wait_for_log("Peer says it sees our address as: 127.0.0.1:[0-9]{5}")
@@ -95,6 +96,7 @@ def test_remote_addr(node_factory, bitcoind):
     bitcoind.generate_block(5)
     l1.daemon.wait_for_log(f"Received node_announcement for node {l2.info['id']}")
     assert(len(l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses']) == 0)
+    assert len(l2.rpc.getinfo()['address']) == 0
 
     def_port = default_ln_port(l2.info["network"])
 
@@ -110,6 +112,7 @@ def test_remote_addr(node_factory, bitcoind):
     # Now l1 sees l2 but without announced addresses.
     assert(len(l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses']) == 0)
     assert not l2.daemon.is_in_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port))
+    assert len(l2.rpc.getinfo()['address']) == 0
 
     # connect second node. This will not yet trigger `node_annoucement` update,
     # as we again do not have a channel at the time we connected.
@@ -120,6 +123,7 @@ def test_remote_addr(node_factory, bitcoind):
     l2.fundchannel(l3, wait_for_active=True)
     bitcoind.generate_block(5)
     assert not l2.daemon.is_in_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port))
+    assert len(l2.rpc.getinfo()['address']) == 0
 
     # restart, reconnect and re-check for updated node_annoucement. This time
     # l2 sees that two different peers with channel reported the same `remote_addr`.
@@ -129,11 +133,19 @@ def test_remote_addr(node_factory, bitcoind):
     l2.daemon.wait_for_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port))
     l1.daemon.wait_for_log(f"Received node_announcement for node {l2.info['id']}")
 
+    # check l1 sees the updated node announcement via CLI listnodes
     address = l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses'][0]
     assert address['type'] == "ipv4"
     assert address['address'] == "127.0.0.1"
     assert address['port'] == def_port
 
+    # also check l2 returns the announced address (and port) via CLI getinfo
+    getinfo = l2.rpc.getinfo()
+    assert len(getinfo['address']) == 1
+    assert getinfo['address'][0]['type'] == 'ipv4'
+    assert getinfo['address'][0]['address'] == '127.0.0.1'
+    assert getinfo['address'][0]['port'] == def_port
+
 
 @pytest.mark.developer("needs DEVELOPER=1 for fast gossip and --dev-allow-localhost for local remote_addr")
 def test_remote_addr_disabled(node_factory, bitcoind):