connect: return address we actually connected to.

Otherwise, we might find an address other than the one given and
the user might think that address worked.

Fixes: #4185
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `connect` returns `address` it actually connected to

doc/lightning-connect.7

@@ -22,7 +22,7 @@ be of the form \fIid@host\fR or \fIid@host:port\fR\. In this case, the \fIhost\f
 If not specified, the \fIport\fR defaults to 9735\.
 
 
-If \fIhost\fR is not specified, the connection will be attempted to an IP
+If \fIhost\fR is not specified (or doesn't work), the connection will be attempted to an IP
 belonging to \fIid\fR obtained through gossip with other already connected
 peers\.
 This can fail if your C-lightning node is a fresh install that has not
@@ -43,7 +43,7 @@ another node\. Once the peer is connected a channel can be opened with
 .SH RETURN VALUE
 
 On success the peer \fIid\fR is returned, as well as a hexidecimal \fIfeatures\fR
-bitmap\.
+bitmap and an \fIaddress\fR object as per \fBlightning-listnodes\fR(7)\.
 
 .SH ERRORS
 
@@ -94,4 +94,4 @@ Felix \fI<fixone@gmail.com\fR> is the original author of this manpage\.
 
 Main web site: \fIhttps://github.com/ElementsProject/lightning\fR
 
-\" SHA256STAMP:5f99f87ec4d94984b36da81b3f3b6fd21100b58baea493f5d0c3f50ffdcbf301
+\" SHA256STAMP:336c35e791dc6439115e25a2f58ffb9dc68989c96d51b6c39cb3de6d40328ae5

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/db_postgres_sqlgen.c

@@ -1888,4 +1888,4 @@ struct db_query db_postgres_queries[] = {
 
 #endif /* LIGHTNINGD_WALLET_GEN_DB_POSTGRES */
 
-// SHA256STAMP:fa582dba4c41760ea1760e8c98a53ca4a450ab8236d68bc8749eda0efb8c59af
+// SHA256STAMP:8b9d33d20380d74e6f143f65bb05ab95a05403cba5b5d6afe19edc91c0a185cb

wallet/db_sqlite3_sqlgen.c

@@ -1888,4 +1888,4 @@ struct db_query db_sqlite3_queries[] = {
 
 #endif /* LIGHTNINGD_WALLET_GEN_DB_SQLITE3 */
 
-// SHA256STAMP:fa582dba4c41760ea1760e8c98a53ca4a450ab8236d68bc8749eda0efb8c59af
+// SHA256STAMP:8b9d33d20380d74e6f143f65bb05ab95a05403cba5b5d6afe19edc91c0a185cb

wallet/statements_gettextgen.po

@@ -1238,11 +1238,11 @@ msgstr ""
 msgid "not a valid SQL statement"
 msgstr ""
 
-#: wallet/test/run-wallet.c:1441
+#: wallet/test/run-wallet.c:1442
 msgid "SELECT COUNT(1) FROM channel_funding_inflights WHERE channel_id = ?;"
 msgstr ""
 
-#: wallet/test/run-wallet.c:1639
+#: wallet/test/run-wallet.c:1640
 msgid "INSERT INTO channels (id) VALUES (1);"
 msgstr ""
-#  SHA256STAMP:249962d1ad354071c65dcbdbabf051fde833e7c9b36f78b2db093d3209e419d5
+#  SHA256STAMP:708d261517851b29a6f7a8102b1d7388b0aef6fb95714cb17efab3079e44d700

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,