connect: return address we actually connected to.

doc/lightning-connect.7.md

@@ -20,7 +20,7 @@ be of the form *id@host* or *id@host:port*. In this case, the *host* and
 
 If not specified, the *port* defaults to 9735.
 
-If *host* is not specified, the connection will be attempted to an IP
+If *host* is not specified (or doesn't work), the connection will be attempted to an IP
 belonging to *id* obtained through gossip with other already connected
 peers.
 This can fail if your C-lightning node is a fresh install that has not
@@ -40,7 +40,7 @@ RETURN VALUE
 ------------
 
 On success the peer *id* is returned, as well as a hexidecimal *features*
-bitmap.
+bitmap and an *address* object as per lightning-listnodes(7).
 
 ERRORS
 ------

lightningd/connect_control.c

@@ -68,11 +68,13 @@ static struct connect *find_connect(struct lightningd *ld,
 }
 
 static struct command_result *connect_cmd_succeed(struct command *cmd,
-						  const struct peer *peer)
+						  const struct peer *peer,
+						  const struct wireaddr_internal *addr)
 {
 	struct json_stream *response = json_stream_success(cmd);
 	json_add_node_id(response, "id", &peer->id);
 	json_add_hex_talarr(response, "features", peer->their_features);
+	json_add_address_internal(response, "address", addr);
 	return command_success(cmd, response);
 }
 
@@ -143,7 +145,9 @@ static struct command_result *json_connect(struct command *cmd,
 
 		if (peer->uncommitted_channel
 		    || (channel && channel->connected)) {
-			return connect_cmd_succeed(cmd, peer);
+			log_debug(cmd->ld->log, "Already connected via %s",
+				  type_to_string(tmpctx, struct wireaddr_internal, &peer->addr));
+			return connect_cmd_succeed(cmd, peer, &peer->addr);
 		}
 	}
 
@@ -260,14 +264,15 @@ static void connect_failed(struct lightningd *ld, const u8 *msg)
 		delay_then_reconnect(channel, seconds_to_delay, addrhint);
 }
 
-void connect_succeeded(struct lightningd *ld, const struct peer *peer)
+void connect_succeeded(struct lightningd *ld, const struct peer *peer,
+		       const struct wireaddr_internal *addr)
 {
 	struct connect *c;
 
 	/* We can have multiple connect commands: fail them all */
 	while ((c = find_connect(ld, &peer->id)) != NULL) {
 		/* They delete themselves from list */
-		connect_cmd_succeed(c->cmd, peer);
+		connect_cmd_succeed(c->cmd, peer, addr);
 	}
 }
 

lightningd/connect_control.h

@@ -12,7 +12,8 @@ void connectd_activate(struct lightningd *ld);
 
 void delay_then_reconnect(struct channel *channel, u32 seconds_delay,
 			  const struct wireaddr_internal *addrhint TAKES);
-void connect_succeeded(struct lightningd *ld, const struct peer *peer);
+void connect_succeeded(struct lightningd *ld, const struct peer *peer,
+		       const struct wireaddr_internal *addr);
 void gossip_connect_result(struct lightningd *ld, const u8 *msg);
 
 #endif /* LIGHTNING_LIGHTNINGD_CONNECT_CONTROL_H */

lightningd/peer_control.c

@@ -1194,7 +1194,7 @@ void peer_connected(struct lightningd *ld, const u8 *msg,
 	peer_update_features(peer, their_features);
 
 	/* Complete any outstanding connect commands. */
-	connect_succeeded(ld, peer);
+	connect_succeeded(ld, peer, &hook_payload->addr);
 
 	/* Can't be opening, since we wouldn't have sent peer_disconnected. */
 	assert(!peer->uncommitted_channel);

lightningd/test/run-invoice-select-inchan.c

@@ -142,7 +142,8 @@ struct command_result *command_success(struct command *cmd UNNEEDED,
 
 { fprintf(stderr, "command_success called!\n"); abort(); }
 /* Generated stub for connect_succeeded */
-void connect_succeeded(struct lightningd *ld UNNEEDED, const struct peer *peer UNNEEDED)
+void connect_succeeded(struct lightningd *ld UNNEEDED, const struct peer *peer UNNEEDED,
+		       const struct wireaddr_internal *addr UNNEEDED)
 { fprintf(stderr, "connect_succeeded called!\n"); abort(); }
 /* Generated stub for delay_then_reconnect */
 void delay_then_reconnect(struct channel *channel UNNEEDED, u32 seconds_delay UNNEEDED,

tests/test_connection.py

@@ -33,9 +33,12 @@ def test_connect(node_factory):
     # Reconnect should be a noop
     ret = l1.rpc.connect(l2.info['id'], 'localhost', port=l2.port)
     assert ret['id'] == l2.info['id']
+    assert ret['address'] == {'type': 'ipv4', 'address': '127.0.0.1', 'port': l2.port}
 
     ret = l2.rpc.connect(l1.info['id'], host='localhost', port=l1.port)
     assert ret['id'] == l1.info['id']
+    # FIXME: This gives a bogus address (since they connected to us): better to give none!
+    assert 'address' in ret
 
     # Should still only have one peer!
     assert len(l1.rpc.listpeers()) == 1
@@ -62,6 +65,7 @@ def test_connect_standard_addr(node_factory):
     # node@host
     ret = l1.rpc.connect("{}@{}".format(l2.info['id'], 'localhost'), port=l2.port)
     assert ret['id'] == l2.info['id']
+    assert ret['address'] == {'type': 'ipv4', 'address': '127.0.0.1', 'port': l2.port}
 
     # node@host:port
     ret = l1.rpc.connect("{}@localhost:{}".format(l3.info['id'], l3.port))
@@ -127,7 +131,8 @@ def test_connection_moved(node_factory, executor):
     l1.daemon.wait_for_log('connection from')
 
     # Provide correct connection details
-    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+    ret = l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+    assert ret['address'] == {'type': 'ipv4', 'address': '127.0.0.1', 'port': l2.port}
 
     # If we failed to update the connection, this call will error
     fut_hang.result(TIMEOUT)

tests/test_gossip.py

@@ -243,7 +243,14 @@ def test_connect_by_gossip(node_factory, bitcoind):
     l1.daemon.wait_for_logs(['Received node_announcement for node {}'.format(l3.info['id'])])
 
     # Have l1 connect to l3 without explicit host and port.
-    l1.rpc.connect(l3.info['id'])
+    ret = l1.rpc.connect(l3.info['id'])
+    assert ret['address'] == {'type': 'ipv4', 'address': '127.0.0.1', 'port': l3.port}
+
+    # Now give it *wrong* port (after we make sure l2 isn't listening), it should fall back.
+    l1.rpc.disconnect(l3.info['id'])
+    l2.stop()
+    ret = l1.rpc.connect(l3.info['id'], 'localhost', l2.port)
+    assert ret['address'] == {'type': 'ipv4', 'address': '127.0.0.1', 'port': l3.port}
 
 
 @unittest.skipIf(not DEVELOPER, "DEVELOPER=1 needed to speed up gossip propagation, would be too long otherwise")

tests/test_misc.py

@@ -784,7 +784,8 @@ def test_address(node_factory):
     l1.start()
 
     l2 = node_factory.get_node()
-    l2.rpc.connect(l1.info['id'], l1.daemon.opts['bind-addr'])
+    ret = l2.rpc.connect(l1.info['id'], l1.daemon.opts['bind-addr'])
+    assert ret['address'] == {'type': 'local socket', 'socket': l1.daemon.opts['bind-addr']}
 
     # 'addr' with local socket works too.
     l1.stop()

wallet/test/run-wallet.c

@@ -89,7 +89,8 @@ struct command_result *command_success(struct command *cmd UNNEEDED,
 
 { fprintf(stderr, "command_success called!\n"); abort(); }
 /* Generated stub for connect_succeeded */
-void connect_succeeded(struct lightningd *ld UNNEEDED, const struct peer *peer UNNEEDED)
+void connect_succeeded(struct lightningd *ld UNNEEDED, const struct peer *peer UNNEEDED,
+		       const struct wireaddr_internal *addr UNNEEDED)
 { fprintf(stderr, "connect_succeeded called!\n"); abort(); }
 /* Generated stub for create_onionreply */
 struct onionreply *create_onionreply(const tal_t *ctx UNNEEDED,