Connect to peers alt addr (if provided) when reconnecting. If both nodes

maxrantil · maxrantil · commit b0017419be9a · 2024-06-26T11:55:59.000+02:00
provide alt addrsses only the connection_out will utilize the alt addr.

Signed-off-by: Max Rantil &lt;rantil@pm.me&gt;
diff --git a/channeld/channeld.c b/channeld/channeld.c
@@ -196,11 +196,11 @@ struct peer {
 	bool experimental_upgrade;
 
 	/* Alt address for peer connections not publicly announced */
-	u8 *alt_addr;
+	u8 *our_alt_addr;
 };
 
 static void start_commit_timer(struct peer *peer);
-static void send_peer_alt_address(struct peer *peer);
+static void send_peer_our_alt_address(struct peer *peer);
 
 static void billboard_update(const struct peer *peer)
 {
@@ -542,11 +542,11 @@ static void handle_peer_splice_locked(struct peer *peer, const u8 *msg)
 	check_mutual_splice_locked(peer);
 }
 
-static void send_peer_alt_address(struct peer *peer) {
+static void send_peer_our_alt_address(struct peer *peer) {
 	struct pubkey node_id;
 
 	if (pubkey_from_node_id(&node_id, &peer->id)) {
-		u8 *msg = towire_peer_alt_address(peer, &node_id, peer->alt_addr);
+		u8 *msg = towire_peer_alt_address(peer, &node_id, peer->our_alt_addr);
 		peer_write(peer->pps, take(msg));
 	}
 }
@@ -4179,8 +4179,8 @@ static void peer_in(struct peer *peer, const u8 *msg)
 
 	check_tx_abort(peer, msg);
 
-	if (peer->alt_addr)
-		send_peer_alt_address(peer);
+	if (peer->our_alt_addr)
+		send_peer_our_alt_address(peer);
 
 	/* If we're in STFU mode and aren't waiting for a STFU mode
 	 * specific message, the only valid message was tx_abort */
@@ -5890,7 +5890,7 @@ static void init_channel(struct peer *peer)
 				    &peer->experimental_upgrade,
 				    &peer->splice_state->inflights,
 				    &peer->local_alias,
-				    &peer->alt_addr,
+				    &peer->our_alt_addr,
 				    &peer->id)) {
 		master_badmsg(WIRE_CHANNELD_INIT, msg);
 	}
diff --git a/connectd/connectd.c b/connectd/connectd.c
@@ -506,9 +506,26 @@ static struct io_plan *connection_in(struct io_conn *conn,
 
 	conn_in_arg.daemon = daemon;
 	conn_in_arg.is_websocket = false;
+
 	return conn_in(conn, &conn_in_arg);
 }
 
+void handle_peer_alt_address(struct peer *peer, const u8 *msg)
+{
+	u8 *peer_alt_addr;
+	struct pubkey peer_id;
+	/* u32 *timestamp = NULL; */ /* TODO */
+
+	if (!fromwire_peer_alt_address(peer, msg, &peer_id, &peer_alt_addr/* , timestamp */)) {
+		master_badmsg(WIRE_PEER_ALT_ADDRESS, msg);
+	}
+
+	msg = towire_connectd_alt_address(NULL, &peer_id, peer_alt_addr);
+	daemon_conn_send(peer->daemon->master, take(msg));
+
+	tal_free(peer_alt_addr);
+}
+
 /*~ <hello>I speak web socket</hello>.
  *
  * Actually that's dumb, websocket (aka rfc6455) looks nothing like that. */
@@ -1776,22 +1793,6 @@ static void try_connect_peer(struct daemon *daemon,
 	try_connect_one_addr(connect);
 }
 
-void handle_peer_alt_addr(struct peer *peer, const u8 *msg)
-{
-	u8 *alt_addr;
-	struct pubkey peer_id;
-
-	// u32 *timestamp = NULL;
-	if (!fromwire_peer_alt_address(peer, msg, &peer_id, &alt_addr/* , timestamp */)) {
-		master_badmsg(WIRE_PEER_ALT_ADDRESS, msg);
-	}
-
-	msg = towire_connectd_alt_address(NULL, &peer_id, alt_addr);
-	daemon_conn_send(peer->daemon->master, take(msg));
-
-	tal_free(alt_addr);
-}
-
 /* lightningd tells us to connect to a peer by id, with optional addr hint. */
 static void connect_to_peer(struct daemon *daemon, const u8 *msg)
 {
diff --git a/connectd/connectd.h b/connectd/connectd.h
@@ -61,6 +61,7 @@ struct peer {
 
 	/* The pubkey of the node */
 	struct node_id id;
+
 	/* Counters and keys for symmetric crypto */
 	struct crypto_state cs;
 
@@ -294,6 +295,6 @@ void destroy_peer(struct peer *peer);
 void close_random_connection(struct daemon *daemon);
 
 /* Handles alternative address message from peer. */
-void handle_peer_alt_addr(struct peer *peer, const u8 *msg);
+void handle_peer_alt_address(struct peer *peer, const u8 *msg);
 
 #endif /* LIGHTNING_CONNECTD_CONNECTD_H */
diff --git a/connectd/multiplex.c b/connectd/multiplex.c
@@ -772,8 +772,8 @@ static bool handle_message_locally(struct peer *peer, const u8 *msg)
 	} else if (type == WIRE_ONION_MESSAGE) {
 		handle_onion_message(peer->daemon, peer, msg);
 		return true;
-	} else if (type == WIRE_PEER_ALT_ADDRESS) { // IS THIS THE RIGHT PLACE ?? IT WORKS BUT...
-		handle_peer_alt_addr(peer, msg);
+	} else if (type == WIRE_PEER_ALT_ADDRESS) {
+		handle_peer_alt_address(peer, msg);
 		return true;
 	} else if (handle_custommsg(peer->daemon, peer, msg)) {
 		return true;
diff --git a/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json
@@ -4146,6 +4146,7 @@
                 "netaddr": [
                   "127.0.0.1:40119"
                 ],
+                "alt_addr": "127.0.0.21:7171",
                 "features": "08a0000a8a5961",
                 "log": [
                   {
@@ -21203,6 +21204,7 @@
                           ]
                         }
                       },
+                      "alt_addr": {},
                       "remote_addr": {
                         "type": "string",
                         "description": [
@@ -21247,6 +21249,7 @@
                 "netaddr": [
                   "127.0.0.1:44619"
                 ],
+                "alt_addr": "127.0.0.21:7171",
                 "features": "08a0000a0a69a2"
               }
             ]
@@ -21270,6 +21273,7 @@
                 "netaddr": [
                   "127.0.0.1:48862"
                 ],
+                "alt_addr": "127.0.0.21:7171",
                 "features": "08a0000a0a69a2"
               }
             ]
diff --git a/doc/schemas/lightning-commando.json b/doc/schemas/lightning-commando.json
@@ -136,6 +136,7 @@
             "netaddr": [
               "127.0.0.1:40119"
             ],
+            "alt_addr": "127.0.0.21:7171",
             "features": "08a0000a8a5961",
             "log": [
               {
diff --git a/doc/schemas/lightning-listconfigs.json b/doc/schemas/lightning-listconfigs.json
@@ -1185,7 +1185,7 @@
             ],
             "properties": {
               "values_str": {
-								"added": "v24.05",
+                "added": "v24.05",
                 "type": "array",
                 "items": {
                   "type": "string",
diff --git a/doc/schemas/lightning-listpeers.json b/doc/schemas/lightning-listpeers.json
@@ -279,6 +279,7 @@
                       ]
                     }
                   },
+                  "alt_addr": {},
                   "remote_addr": {
                     "type": "string",
                     "description": [
@@ -323,6 +324,7 @@
             "netaddr": [
               "127.0.0.1:44619"
             ],
+            "alt_addr": "127.0.0.21:7171",
             "features": "08a0000a0a69a2"
           }
         ]
@@ -346,6 +348,7 @@
             "netaddr": [
               "127.0.0.1:48862"
             ],
+            "alt_addr": "127.0.0.21:7171",
             "features": "08a0000a0a69a2"
           }
         ]
diff --git a/lightningd/channel.h b/lightningd/channel.h
@@ -798,4 +798,5 @@ const u8 *channel_update_for_error(const tal_t *ctx,
 
 struct amount_msat htlc_max_possible_send(const struct channel *channel);
 
+
 #endif /* LIGHTNING_LIGHTNINGD_CHANNEL_H */
diff --git a/lightningd/channel_control.c b/lightningd/channel_control.c
@@ -1726,7 +1726,7 @@ bool peer_start_channeld(struct channel *channel,
 				       cast_const2(const struct inflight **,
 						   inflights),
 				       *channel->alias[LOCAL],
-				       ld->alt_addr,
+				       ld->our_alt_addr,
 				       &ld->id);
 
 	/* We don't expect a response: we are triggered by funding_depth_cb. */
diff --git a/lightningd/connect_control.c b/lightningd/connect_control.c
@@ -335,6 +335,9 @@ static void try_connect(const tal_t *ctx,
 {
 	struct delayed_reconnect *d;
 	struct peer *peer;
+	const struct wireaddr_internal *alt_addr;
+
+	alt_addr = wallet_get_peer_alt_addr(ld->wallet, id);
 
 	/* Don't stack, unless this is an instant reconnect */
 	d = delayed_reconnect_map_get(ld->delayed_reconnect_map, id);
@@ -349,7 +352,7 @@ static void try_connect(const tal_t *ctx,
 	d = tal(ctx, struct delayed_reconnect);
 	d->ld = ld;
 	d->id = *id;
-	d->addrhint = tal_dup_or_null(d, struct wireaddr_internal, addrhint);
+	d->addrhint = tal_dup_or_null(d, struct wireaddr_internal, alt_addr ? alt_addr : addrhint);
 	d->dns_fallback = dns_fallback;
 	delayed_reconnect_map_add(ld->delayed_reconnect_map, d);
 	tal_add_destructor(d, destroy_delayed_reconnect);
@@ -364,9 +367,6 @@ static void try_connect(const tal_t *ctx,
 	/* Update any channel billboards */
 	peer = peer_by_id(ld, id);
 	if (peer) {
-		// struct pubkey pubkey;
-		// if (pubkey_from_node_id(&pubkey, id)) {
-		// 	send_peer_alt_address(peer, &pubkey, (const u8 *)"127.21.21.21"); // THIS MIGHT BE THE RIGHT PLACE IN THE END, NEED TO SEND ANOTHER MSG FROM MASTER -> CHANNELD then????
 		struct channel *channel;
 		list_for_each(&peer->channels, channel, list) {
 			if (!channel_state_wants_peercomms(channel->state))
@@ -559,20 +559,20 @@ static void handle_custommsg_in(struct lightningd *ld, const u8 *msg)
 	plugin_hook_call_custommsg(ld, NULL, p);
 }
 
-static void handle_alt_addr_in(struct lightningd *ld, const u8 *msg)
+static void handle_peer_alt_addr_in(struct lightningd *ld, const u8 *msg)
 {
-	struct pubkey node_id;
-	struct node_id id;
-	u8 *alt_addr;
+	struct pubkey peer_node_id;
+	u8 *peer_alt_addr;
 
-	if (!fromwire_connectd_alt_address(tmpctx, msg, &node_id, &alt_addr)) {
+	if (!fromwire_connectd_alt_address(tmpctx, msg, &peer_node_id, &peer_alt_addr)) {
 		log_broken(ld->log, "Malformed peer_alt_addr_msg: %s",
 								tal_hex(tmpctx, msg));
 		return;
 	}
 
-	node_id_from_pubkey(&id, &node_id);
-	wallet_peer_alt_addr(ld->wallet->db, &id, (char *)alt_addr);
+	struct node_id id;
+	node_id_from_pubkey(&id, &peer_node_id);
+	wallet_add_peer_alt_addr(ld->wallet->db, &id, (char *)peer_alt_addr);
 }
 
 static void connectd_start_shutdown_reply(struct subd *connectd,
@@ -666,7 +666,7 @@ static unsigned connectd_msg(struct subd *connectd, const u8 *msg, const int *fd
 		break;
 
 	case WIRE_CONNECTD_ALT_ADDRESS:
-		handle_alt_addr_in(connectd->ld, msg);
+		handle_peer_alt_addr_in(connectd->ld, msg);
 		break;
 	}
 	return 0;
diff --git a/lightningd/lightningd.h b/lightningd/lightningd.h
@@ -175,9 +175,6 @@ struct lightningd {
 	/* Do we want to reconnect to other peers having only unannouced channels with us? */
 	bool reconnect_private;
 
-	/* Alt address for peer connections not publicly announced */
-	u8 *alt_addr;
-
 	/* How many outstanding startup connection attempts? */
 	size_t num_startup_connects;
 
@@ -196,6 +193,9 @@ struct lightningd {
 	struct wireaddr_internal *binding;
 	struct wireaddr *announceable;
 
+	/* Alternative address for peer connections not publicly announced */
+	u8 *our_alt_addr;
+
 	/* Current node announcement (if any) */
 	const u8 *node_announcement;
 
diff --git a/lightningd/options.c b/lightningd/options.c
@@ -444,12 +444,8 @@ static char *opt_add_alt_addr(const char *arg, struct lightningd *ld)
 {
 	assert(arg != NULL);
 
-	ld->alt_addr = tal_free(ld->alt_addr);
-
-	ld->alt_addr = (u8 *)tal_strdup(ld, arg);
-	if (ld->alt_addr == NULL) {
-		return tal_fmt(tmpctx, "Failed to allocate memory for address: %s", arg);
-	}
+	ld->our_alt_addr = tal_free(ld->our_alt_addr);
+	ld->our_alt_addr = (u8 *)tal_strdup(ld, arg);
 
 	return opt_add_addr_withtype(arg, ld, ADDR_LISTEN);
 }
diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c
@@ -2228,6 +2228,7 @@ static void json_add_peer(struct lightningd *ld,
 			  const enum log_level *ll)
 {
 	struct channel *channel;
+	struct wireaddr_internal *peer_alt_addr;
 	u32 num_channels;
 
 	json_object_start(response, NULL);
@@ -2250,6 +2251,12 @@ static void json_add_peer(struct lightningd *ld,
 					fmt_wireaddr(response, p->remote_addr));
 	}
 
+	peer_alt_addr = wallet_get_peer_alt_addr(ld->wallet, &p->id);
+	if (peer_alt_addr) {
+		json_add_string(response, "alt_addr",
+				fmt_wireaddr_internal(tmpctx, peer_alt_addr));
+	}
+
 	/* Note: If !PEER_CONNECTED, peer may use different features on reconnect */
 	json_add_hex_talarr(response, "features", p->their_features);
 
diff --git a/pytest.ini b/pytest.ini
diff --git a/tests/test_connection.py b/tests/test_connection.py
@@ -130,15 +130,6 @@ def test_connect_with_alt_addr(node_factory, bitcoind):
         logging.error(f"Error verifying connection using alt-addr: {e}")
         raise
 
-    # Verify the channel state
-    logging.info("Checking the channel state after reconnection")
-    try:
-        channel_state = l1.rpc.listpeerchannels(l2.info['id'])['channels'][0]['state']
-        assert channel_state == 'CHANNELD_NORMAL', f"Channel state is {channel_state}, expected CHANNELD_NORMAL"
-    except Exception as e:
-        logging.error(f"Channel state not normal: {e}")
-        raise
-
 
 def test_remote_addr(node_factory, bitcoind):
     """Check address discovery (BOLT1 #917) init remote_addr works as designed:
diff --git a/wallet/wallet.c b/wallet/wallet.c
diff --git a/wallet/wallet.h b/wallet/wallet.h

Original file line number	Diff line number	Diff line change
`@@ -4146,6 +4146,7 @@`
`4146`	`4146`	`"netaddr": [`
`4147`	`4147`	`"127.0.0.1:40119"`
`4148`	`4148`	`],`
	`4149`	`+ "alt_addr": "127.0.0.21:7171",`
`4149`	`4150`	`"features": "08a0000a8a5961",`
`4150`	`4151`	`"log": [`
`4151`	`4152`	`{`
`@@ -21203,6 +21204,7 @@`
`21203`	`21204`	`]`
`21204`	`21205`	`}`
`21205`	`21206`	`},`
	`21207`	`+ "alt_addr": {},`
`21206`	`21208`	`"remote_addr": {`
`21207`	`21209`	`"type": "string",`
`21208`	`21210`	`"description": [`
`@@ -21247,6 +21249,7 @@`
`21247`	`21249`	`"netaddr": [`
`21248`	`21250`	`"127.0.0.1:44619"`
`21249`	`21251`	`],`
	`21252`	`+ "alt_addr": "127.0.0.21:7171",`
`21250`	`21253`	`"features": "08a0000a0a69a2"`
`21251`	`21254`	`}`
`21252`	`21255`	`]`
`@@ -21270,6 +21273,7 @@`
`21270`	`21273`	`"netaddr": [`
`21271`	`21274`	`"127.0.0.1:48862"`
`21272`	`21275`	`],`
	`21276`	`+ "alt_addr": "127.0.0.21:7171",`
`21273`	`21277`	`"features": "08a0000a0a69a2"`
`21274`	`21278`	`}`
`21275`	`21279`	`]`
Original file line number	Diff line number	Diff line change
`@@ -136,6 +136,7 @@`
`136`	`136`	`"netaddr": [`
`137`	`137`	`"127.0.0.1:40119"`
`138`	`138`	`],`
	`139`	`+ "alt_addr": "127.0.0.21:7171",`
`139`	`140`	`"features": "08a0000a8a5961",`
`140`	`141`	`"log": [`
`141`	`142`	`{`
Original file line number	Diff line number	Diff line change
`@@ -279,6 +279,7 @@`
`279`	`279`	`]`
`280`	`280`	`}`
`281`	`281`	`},`
	`282`	`+ "alt_addr": {},`
`282`	`283`	`"remote_addr": {`
`283`	`284`	`"type": "string",`
`284`	`285`	`"description": [`
`@@ -323,6 +324,7 @@`
`323`	`324`	`"netaddr": [`
`324`	`325`	`"127.0.0.1:44619"`
`325`	`326`	`],`
	`327`	`+ "alt_addr": "127.0.0.21:7171",`
`326`	`328`	`"features": "08a0000a0a69a2"`
`327`	`329`	`}`
`328`	`330`	`]`
`@@ -346,6 +348,7 @@`
`346`	`348`	`"netaddr": [`
`347`	`349`	`"127.0.0.1:48862"`
`348`	`350`	`],`
	`351`	`+ "alt_addr": "127.0.0.21:7171",`
`349`	`352`	`"features": "08a0000a0a69a2"`
`350`	`353`	`}`
`351`	`354`	`]`
Original file line number	Diff line number	Diff line change
`@@ -798,4 +798,5 @@ const u8 channel_update_for_error(const tal_t ctx,`
`798`	`798`
`799`	`799`	`struct amount_msat htlc_max_possible_send(const struct channel *channel);`
`800`	`800`
	`801`	`+`
`801`	`802`	`#endif /* LIGHTNING_LIGHTNINGD_CHANNEL_H */`