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 <[email protected]> Changelog-Added: JSON-RPC: `connect` returns `address` it actually connected to
ElementsProject · Mar 16, 2021 · e3f2977 · e3f2977
1 parent 72ad260
commit e3f2977
Show file tree

Hide file tree

Showing 13 changed files with 42 additions and 74 deletions.
diff --git a/doc/lightning-connect.7 b/doc/lightning-connect.7
diff --git a/doc/lightning-connect.7.md b/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
 ------

diff --git a/lightningd/connect_control.c b/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);
 	}
 }
 

diff --git a/lightningd/connect_control.h b/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 */
diff --git a/lightningd/peer_control.c b/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);

diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c
@@ -59,10 +59,6 @@ const char *channel_change_state_reason_str(enum state_change reason UNNEEDED)
 /* Generated stub for channel_cleanup_commands */
 void channel_cleanup_commands(struct channel *channel UNNEEDED, const char *why UNNEEDED)
 { fprintf(stderr, "channel_cleanup_commands called!\n"); abort(); }
-/* Generated stub for channel_close_conn */
-void channel_close_conn(struct channel *channel UNNEEDED,
-			const char *why UNNEEDED)
-{ fprintf(stderr, "channel_close_conn called!\n"); abort(); }
 /* Generated stub for channel_fail_forget */
 void channel_fail_forget(struct channel *channel UNNEEDED, const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "channel_fail_forget called!\n"); abort(); }
@@ -142,7 +138,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,
@@ -333,10 +330,6 @@ void json_add_txid(struct json_stream *result UNNEEDED, const char *fieldname UN
 void json_add_uncommitted_channel(struct json_stream *response UNNEEDED,
 				  const struct uncommitted_channel *uc UNNEEDED)
 { fprintf(stderr, "json_add_uncommitted_channel called!\n"); abort(); }
-/* Generated stub for json_add_unsaved_channel */
-void json_add_unsaved_channel(struct json_stream *response UNNEEDED,
-			      const struct channel *channel UNNEEDED)
-{ fprintf(stderr, "json_add_unsaved_channel called!\n"); abort(); }
 /* Generated stub for json_array_end */
 void json_array_end(struct json_stream *js UNNEEDED)
 { fprintf(stderr, "json_array_end called!\n"); abort(); }
@@ -548,12 +541,6 @@ void peer_memleak_done(struct command *cmd UNNEEDED, struct subd *leaker UNNEEDE
 /* Generated stub for peer_normal_channel */
 struct channel *peer_normal_channel(struct peer *peer UNNEEDED)
 { fprintf(stderr, "peer_normal_channel called!\n"); abort(); }
-/* Generated stub for peer_restart_dualopend */
-void peer_restart_dualopend(struct peer *peer UNNEEDED,
-			    struct per_peer_state *pps UNNEEDED,
-			    struct channel *channel UNNEEDED,
-			    const u8 *send_msg UNNEEDED)
-{ fprintf(stderr, "peer_restart_dualopend called!\n"); abort(); }
 /* Generated stub for peer_start_channeld */
 void peer_start_channeld(struct channel *channel UNNEEDED,
 			 struct per_peer_state *pps UNNEEDED,
@@ -566,11 +553,6 @@ void peer_start_closingd(struct channel *channel UNNEEDED,
 			 bool reconnected UNNEEDED,
 			 const u8 *channel_reestablish UNNEEDED)
 { fprintf(stderr, "peer_start_closingd called!\n"); abort(); }
-/* Generated stub for peer_start_dualopend */
-void peer_start_dualopend(struct peer *peer UNNEEDED,
-			  struct per_peer_state *pps UNNEEDED,
-			  const u8 *send_msg UNNEEDED)
-{ fprintf(stderr, "peer_start_dualopend called!\n"); abort(); }
 /* Generated stub for peer_start_openingd */
 void peer_start_openingd(struct peer *peer UNNEEDED,
 			 struct per_peer_state *pps UNNEEDED,

diff --git a/tests/test_connection.py b/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)

diff --git a/tests/test_gossip.py b/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")

diff --git a/tests/test_misc.py b/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()

diff --git a/wallet/db_postgres_sqlgen.c b/wallet/db_postgres_sqlgen.c
diff --git a/wallet/db_sqlite3_sqlgen.c b/wallet/db_sqlite3_sqlgen.c
diff --git a/wallet/statements_gettextgen.po b/wallet/statements_gettextgen.po
diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
@@ -43,27 +43,13 @@ void bitcoind_getutxout_(struct bitcoind *bitcoind UNNEEDED,
 				    void *arg) UNNEEDED,
 			 void *arg UNNEEDED)
 { fprintf(stderr, "bitcoind_getutxout_ called!\n"); abort(); }
-/* Generated stub for blinding_hash_e_and_ss */
-void blinding_hash_e_and_ss(const struct pubkey *e UNNEEDED,
-			    const struct secret *ss UNNEEDED,
-			    struct sha256 *sha UNNEEDED)
-{ fprintf(stderr, "blinding_hash_e_and_ss called!\n"); abort(); }
-/* Generated stub for blinding_next_pubkey */
-bool blinding_next_pubkey(const struct pubkey *pk UNNEEDED,
-			  const struct sha256 *h UNNEEDED,
-			  struct pubkey *next UNNEEDED)
-{ fprintf(stderr, "blinding_next_pubkey called!\n"); abort(); }
 /* Generated stub for broadcast_tx */
 void broadcast_tx(struct chain_topology *topo UNNEEDED,
 		  struct channel *channel UNNEEDED, const struct bitcoin_tx *tx UNNEEDED,
 		  void (*failed)(struct channel *channel UNNEEDED,
 				 bool success UNNEEDED,
 				 const char *err))
 { fprintf(stderr, "broadcast_tx called!\n"); abort(); }
-/* Generated stub for channel_close_conn */
-void channel_close_conn(struct channel *channel UNNEEDED,
-			const char *why UNNEEDED)
-{ fprintf(stderr, "channel_close_conn called!\n"); abort(); }
 /* Generated stub for channel_tell_depth */
 bool channel_tell_depth(struct lightningd *ld UNNEEDED,
 				 struct channel *channel UNNEEDED,
@@ -89,7 +75,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,
@@ -368,10 +355,6 @@ void json_add_u64(struct json_stream *result UNNEEDED, const char *fieldname UNN
 void json_add_uncommitted_channel(struct json_stream *response UNNEEDED,
 				  const struct uncommitted_channel *uc UNNEEDED)
 { fprintf(stderr, "json_add_uncommitted_channel called!\n"); abort(); }
-/* Generated stub for json_add_unsaved_channel */
-void json_add_unsaved_channel(struct json_stream *response UNNEEDED,
-			      const struct channel *channel UNNEEDED)
-{ fprintf(stderr, "json_add_unsaved_channel called!\n"); abort(); }
 /* Generated stub for json_array_end */
 void json_array_end(struct json_stream *js UNNEEDED)
 { fprintf(stderr, "json_array_end called!\n"); abort(); }
@@ -630,12 +613,6 @@ struct subd *peer_get_owning_subd(struct peer *peer UNNEEDED)
 /* Generated stub for peer_memleak_done */
 void peer_memleak_done(struct command *cmd UNNEEDED, struct subd *leaker UNNEEDED)
 { fprintf(stderr, "peer_memleak_done called!\n"); abort(); }
-/* Generated stub for peer_restart_dualopend */
-void peer_restart_dualopend(struct peer *peer UNNEEDED,
-			    struct per_peer_state *pps UNNEEDED,
-			    struct channel *channel UNNEEDED,
-			    const u8 *send_msg UNNEEDED)
-{ fprintf(stderr, "peer_restart_dualopend called!\n"); abort(); }
 /* Generated stub for peer_start_channeld */
 void peer_start_channeld(struct channel *channel UNNEEDED,
 			 struct per_peer_state *pps UNNEEDED,
@@ -648,11 +625,6 @@ void peer_start_closingd(struct channel *channel UNNEEDED,
 			 bool reconnected UNNEEDED,
 			 const u8 *channel_reestablish UNNEEDED)
 { fprintf(stderr, "peer_start_closingd called!\n"); abort(); }
-/* Generated stub for peer_start_dualopend */
-void peer_start_dualopend(struct peer *peer UNNEEDED,
-			  struct per_peer_state *pps UNNEEDED,
-			  const u8 *send_msg UNNEEDED)
-{ fprintf(stderr, "peer_start_dualopend called!\n"); abort(); }
 /* Generated stub for peer_start_openingd */
 void peer_start_openingd(struct peer *peer UNNEEDED,
 			 struct per_peer_state *pps UNNEEDED,
@@ -701,11 +673,6 @@ void subd_req_(const tal_t *ctx UNNEEDED,
 /* Generated stub for subd_send_msg */
 void subd_send_msg(struct subd *sd UNNEEDED, const u8 *msg_out UNNEEDED)
 { fprintf(stderr, "subd_send_msg called!\n"); abort(); }
-/* Generated stub for subkey_from_hmac */
-void subkey_from_hmac(const char *prefix UNNEEDED,
-		      const struct secret *base UNNEEDED,
-		      struct secret *key UNNEEDED)
-{ fprintf(stderr, "subkey_from_hmac called!\n"); abort(); }
 /* Generated stub for topology_add_sync_waiter_ */
 void topology_add_sync_waiter_(const tal_t *ctx UNNEEDED,
 			       struct chain_topology *topo UNNEEDED,