introduce support for alternative addresses for peer connections #7422

maxrantil · 2024-06-21T10:09:05Z

This PR introduces a new configuration parameter, alt_addr, which allows specifying an alternative address for peer connections.

Key Features

Initial Connection: Upon connecting to a peer, if an alt_addr is specified, this address is communicated to the peer.
Reconnection: During reconnection attempts, the alt_addr is used to establish the connection.
Configuration Parameters: Added alt-addr, alt-bind-addr, and alt-announce-addr for managing alternative addresses.
Database Integration: Added fields to the database to store alternative addresses for peers.
New RPC Command: Implemented command for managing alternative addresses in combination with alt-bind-addr for specific peers.

cdecker · 2024-06-21T10:14:18Z

Thanks @maxrantil for the proposal. I read through the description, and I'm not sure I understand the rationale for this change. Is this intended as a way to say "hey, if we get disconnected, this is how you can reach me again", or is the intention something else?

Just thought I'd ask this before looking at the code itself, wanting to start with the right mental model.

maxrantil · 2024-06-21T10:30:32Z

Hi @cdecker , as you might know, this project is part of Summer of Bitcoin and this is the project description:

The lightning network has regimes of public and private data. Public channels and nodes are announced with gossip, while channel activity is private between peers. A nodes connection address is disseminated with the public node announcement message, which is well suited for two peers initiating a connection for the first time. Peers with an established channel or history, may prefer to connect via an alternate address for privacy, reliability, or latency concerns. This project will add a new message to communicate a private alternative connection address, and then use that address when reconnecting to a peer who has provided it.

I think this answers your question, right?

cdecker · 2024-06-21T10:43:53Z

Oh, cool, yeah I hadn't made the connection right away. Thanks for sharing the description, it's much clearer now ☺️

- Aswell as parameters alt-bind-addr and alt-announce-addr Changelog-Added: Introduced the 'alt-addr' config parameter for specifying alternative addresses. Signed-off-by: Max Rantil <rantil@pm.me>

rustyrussell

I apologize that I am only reviewing this now. It needs significant rework to be included, and may not make this release.

I'm aware that this was a Summer of Bitcoin project, so I can make the changes myself.

rustyrussell · 2025-01-22T02:58:33Z

lightningd/lightningd.h

+	/* Alternative address for peer connections not publicly announced */
+	u8 *alt_addr;
+	/* Alternative binding address for peer connections not publicly announced */
+	u8 *alt_bind_addr;
+


This is a very strange type to use! We have a struct wireaddr, which is far more convenient, and has the bonus we parse it immediately.

rustyrussell · 2025-01-22T04:02:22Z

lightningd/options.c

+	char **bind_addrs = tal_strsplit(tmpctx, (char *)ld->alt_bind_addr,
+					 ",", STR_NO_EMPTY);
+	for (size_t i = 0; bind_addrs[i] != NULL; i++)
+		if (strcmp(arg, bind_addrs[i]) == 0) {


This has the problem that equivalent addresses can be different strings.

There are two things here, however: restricted addresses, to which only whitelisted peers can connect to, and the addresses we advertize. This is not clearly documented, and needs to be.

rustyrussell · 2025-01-22T04:27:55Z

channeld/channeld.c

+	/* We need the addrs in my own db too for later whitelist confirmation */
+	u8 *msg = towire_channeld_my_alt_addr(tmpctx, peer->my_alt_addr);
+	wire_sync_write(MASTER_FD, take(msg));
+}


This is a very weird thing for channeld to do. It's more gossipy than channel-related. These days, this kind of thing is handled by connectd.

rustyrussell · 2025-01-22T04:29:38Z

wire/peer_wire.csv

+msgtype,peer_alt_addr,209
+msgdata,peer_alt_addr,addr_len,u8,
+msgdata,peer_alt_addr,addr,byte,addr_len,


This is a spec change! This means:

It needs an associated assignment for a BOLT PR.

Meanwhile it should use 32768+ as that's the experimental range.

We should probably mark it experimental in the mean time.

Also, this file is generated from the spec, rather than hand-edited.

regarding the BOLT PR, after discussions with other devs here we came to the conclusion for a blip-0043 instead of a BOLT.

rustyrussell · 2025-01-22T04:30:15Z

wallet/db.c

@@ -1021,6 +1021,8 @@ static struct migration dbmigrations[] = {
    {SQL("ALTER TABLE channels ADD remote_htlc_minimum_msat BIGINT DEFAULT NULL;"), NULL},
    {SQL("ALTER TABLE channels ADD last_stable_connection BIGINT DEFAULT 0;"), NULL},
    {NULL, migrate_initialize_alias_local},
+    {SQL("ALTER TABLE peers ADD COLUMN peer_alt_addr TEXT DEFAULT '';"), NULL},


I think this should be an array of wireaddr, not a text field.

sounds good to me

rustyrussell · 2025-01-22T04:30:28Z

wallet/db.c

@@ -1021,6 +1021,8 @@ static struct migration dbmigrations[] = {
    {SQL("ALTER TABLE channels ADD remote_htlc_minimum_msat BIGINT DEFAULT NULL;"), NULL},
    {SQL("ALTER TABLE channels ADD last_stable_connection BIGINT DEFAULT 0;"), NULL},
    {NULL, migrate_initialize_alias_local},
+    {SQL("ALTER TABLE peers ADD COLUMN peer_alt_addr TEXT DEFAULT '';"), NULL},
+    {SQL("ALTER TABLE peers ADD COLUMN my_alt_addr TEXT DEFAULT '';"), NULL},


I don't understand why this is in the db?

You might give different alt_addrs to different peers, so you want to confirm, when they connect to you, that you have given that peer the alt_addr it is trying to connect to.

And ofcourse the db is read on node restart, that's why its there.

rustyrussell · 2025-01-22T04:33:28Z

connectd/connectd.h

@@ -289,6 +308,11 @@ struct daemon {
 	/* Our features, as lightningd told us */
 	struct feature_set *our_features;

+	/* check whitelist before accepting incoming alt addr */
+	struct whitelisted_peer_htable *whitelisted_peer_htable;


I think this can simply be a set of node ids (i.e a htable of node_ids).

I think the previous answer does explain why we need more then only the node_id

rustyrussell · 2025-01-22T04:34:37Z

lightningd/channel_control.c

+	/* This is a dev command: fix the api if you need this! */
+	if (more_than_one)
+		return command_fail(cmd, LIGHTNINGD, "More than one channel");
+


No it's not a dev command!

Plus, you can totally whitelist ids which are not peers.

If you say so! I just couldn't understand how to whitelist ids which are not peers.

maxrantil · 2025-01-23T12:10:32Z

I apologize that I am only reviewing this now. It needs significant rework to be included, and may not make this release.

I'm aware that this was a Summer of Bitcoin project, so I can make the changes myself.

Thanks for looking into it @rustyrussell , I understand that there now needs to be a lot of fixes to merge it and if it doesn't make it to the release it will hopefully make it to the next one.

I will try to answer all your questions

maxrantil requested a review from cdecker as a code owner June 21, 2024 10:09

maxrantil marked this pull request as draft June 21, 2024 10:11

rustyrussell added Highlight - Protocol Protocol and spec enhancements / implementation SoB Summer of Bitcoin project labels Jun 23, 2024

maxrantil force-pushed the max/alt-addr branch 4 times, most recently from f6bacf0 to 24018af Compare June 26, 2024 07:25

maxrantil changed the title ~~WIP, add a new message to communicate a private alternative connection address~~ WIP: add a new message to communicate a private alternative connection address Jun 26, 2024

maxrantil force-pushed the max/alt-addr branch 8 times, most recently from d212cd9 to 572a616 Compare June 29, 2024 12:30

maxrantil force-pushed the max/alt-addr branch 5 times, most recently from 821e28d to e461ad2 Compare July 5, 2024 09:14

maxrantil changed the title ~~WIP: add a new message to communicate a private alternative connection address~~ channeld: introduce support for alternative addresses for peer connections Jul 5, 2024

maxrantil force-pushed the max/alt-addr branch 3 times, most recently from aae893b to e6ff7da Compare July 5, 2024 10:02

maxrantil force-pushed the max/alt-addr branch from a6e2fa1 to 131a324 Compare July 15, 2024 19:14

maxrantil force-pushed the max/alt-addr branch 11 times, most recently from 0360fcf to be019d1 Compare September 5, 2024 19:42

maxrantil added 9 commits September 6, 2024 11:12

lightningd: add alt-addr config parameter

8cd3547

- Aswell as parameters alt-bind-addr and alt-announce-addr Changelog-Added: Introduced the 'alt-addr' config parameter for specifying alternative addresses. Signed-off-by: Max Rantil <rantil@pm.me>

channeld: provide alt-addr to peer upon channel establish

183ca77

wallet: handle storing and reveiving alt_addrs from db

5a9c205

connectd: handle alt addr for outbound connection

e43ab91

connectd: validate connections to alt-addr against a whitelist

1be20b5

json-rpc: add rpc to provide bound alt-addr

14b873d

pytest: ... start testing

9b5a55d

lightningd: Update 'listpeers' command to print alt-addrs for a peer

3f8bd91

generated files added to pass the 'make check-gen-updated' test

6be03fc

maxrantil force-pushed the max/alt-addr branch from be019d1 to 6be03fc Compare September 6, 2024 09:12

rustyrussell added this to the v25.02 milestone Jan 22, 2025

rustyrussell reviewed Jan 22, 2025

View reviewed changes

rustyrussell modified the milestones: v25.02, v25.05 Feb 12, 2025

rustyrussell modified the milestones: v25.05, v25.08 May 12, 2025

rustyrussell self-assigned this May 12, 2025

madelinevibes added the Status::Assigned The issue has been given to a team member for resolution. label Jun 26, 2025

introduce support for alternative addresses for peer connections #7422

Are you sure you want to change the base?

introduce support for alternative addresses for peer connections #7422

Uh oh!

Conversation

maxrantil commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Key Features

Uh oh!

cdecker commented Jun 21, 2024

Uh oh!

maxrantil commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cdecker commented Jun 21, 2024

Uh oh!

rustyrussell left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxrantil Jan 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxrantil commented Jan 23, 2025

Uh oh!

Uh oh!

maxrantil commented Jun 21, 2024 •

edited

Loading

maxrantil commented Jun 21, 2024 •

edited

Loading

maxrantil Jan 23, 2025 •

edited

Loading