Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: peer_control getinfo to show correct port on discovered IPs #5585

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions gossipd/gossip_generation.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,18 @@ static u8 *create_node_announcement(const tal_t *ctx, struct daemon *daemon,
for (i = 0; i < count_announceable; i++)
tal_arr_expand(&was, daemon->announceable[i]);

/* Add IP discovery `remote_addr` v4 and v6 of our self. */
/* Add discovered IPs v4/v6 verified by peer `remote_addr` feature. */
/* Only do that if we don't have addresses announced. */
if (count_announceable == 0) {
if (daemon->remote_addr_v4 != NULL &&
!wireaddr_arr_contains(was, daemon->remote_addr_v4))
tal_arr_expand(&was, *daemon->remote_addr_v4);
if (daemon->remote_addr_v6 != NULL &&
!wireaddr_arr_contains(was, daemon->remote_addr_v6))
tal_arr_expand(&was, *daemon->remote_addr_v6);
if (daemon->discovered_ip_v4 != NULL &&
!wireaddr_arr_contains(was, daemon->discovered_ip_v4))
tal_arr_expand(&was, *daemon->discovered_ip_v4);
if (daemon->discovered_ip_v6 != NULL &&
!wireaddr_arr_contains(was, daemon->discovered_ip_v6))
tal_arr_expand(&was, *daemon->discovered_ip_v6);
}

/* Sort by address type again, as we added dynamic remote_addr v4/v6. */
/* Sort by address type again, as we added dynamic discovered_ip v4/v6. */
/* BOLT #7:
*
* The origin node:
Expand Down
49 changes: 23 additions & 26 deletions gossipd/gossipd.c
Original file line number Diff line number Diff line change
Expand Up @@ -341,36 +341,33 @@ static void handle_local_private_channel(struct daemon *daemon, const u8 *msg)
}
}

/* lightningd tells us it has dicovered and verified new `remote_addr`.
/* lightningd tells us it has discovered and verified new `remote_addr`.
* We can use this to update our node announcement. */
static void handle_remote_addr(struct daemon *daemon, const u8 *msg)
static void handle_discovered_ip(struct daemon *daemon, const u8 *msg)
{
struct wireaddr remote_addr;
struct wireaddr discovered_ip;

if (!fromwire_gossipd_remote_addr(msg, &remote_addr))
master_badmsg(WIRE_GOSSIPD_REMOTE_ADDR, msg);
if (!fromwire_gossipd_discovered_ip(msg, &discovered_ip))
master_badmsg(WIRE_GOSSIPD_DISCOVERED_IP, msg);

/* Best guess is that we use default port for the selected network */
remote_addr.port = chainparams_get_ln_port(chainparams);

switch (remote_addr.type) {
switch (discovered_ip.type) {
case ADDR_TYPE_IPV4:
if (daemon->remote_addr_v4 != NULL &&
wireaddr_eq_without_port(daemon->remote_addr_v4,
&remote_addr))
if (daemon->discovered_ip_v4 != NULL &&
wireaddr_eq_without_port(daemon->discovered_ip_v4,
&discovered_ip))
break;
tal_free(daemon->remote_addr_v4);
daemon->remote_addr_v4 = tal_dup(daemon, struct wireaddr,
&remote_addr);
tal_free(daemon->discovered_ip_v4);
daemon->discovered_ip_v4 = tal_dup(daemon, struct wireaddr,
&discovered_ip);
goto update_node_annoucement;
case ADDR_TYPE_IPV6:
if (daemon->remote_addr_v6 != NULL &&
wireaddr_eq_without_port(daemon->remote_addr_v6,
&remote_addr))
if (daemon->discovered_ip_v6 != NULL &&
wireaddr_eq_without_port(daemon->discovered_ip_v6,
&discovered_ip))
break;
tal_free(daemon->remote_addr_v6);
daemon->remote_addr_v6 = tal_dup(daemon, struct wireaddr,
&remote_addr);
tal_free(daemon->discovered_ip_v6);
daemon->discovered_ip_v6 = tal_dup(daemon, struct wireaddr,
&discovered_ip);
goto update_node_annoucement;

/* ignore all other cases */
Expand All @@ -384,7 +381,7 @@ static void handle_remote_addr(struct daemon *daemon, const u8 *msg)

update_node_annoucement:
status_debug("Update our node_announcement for discovered address: %s",
fmt_wireaddr(tmpctx, &remote_addr));
fmt_wireaddr(tmpctx, &discovered_ip));
maybe_send_own_node_announce(daemon, false);
}

Expand Down Expand Up @@ -1041,8 +1038,8 @@ static struct io_plan *recv_req(struct io_conn *conn,
handle_local_private_channel(daemon, msg);
goto done;

case WIRE_GOSSIPD_REMOTE_ADDR:
handle_remote_addr(daemon, msg);
case WIRE_GOSSIPD_DISCOVERED_IP:
handle_discovered_ip(daemon, msg);
goto done;
#if DEVELOPER
case WIRE_GOSSIPD_DEV_SET_MAX_SCIDS_ENCODE_SIZE:
Expand Down Expand Up @@ -1100,8 +1097,8 @@ int main(int argc, char *argv[])
daemon->node_announce_regen_timer = NULL;
daemon->current_blockheight = 0; /* i.e. unknown */
daemon->rates = NULL;
daemon->remote_addr_v4 = NULL;
daemon->remote_addr_v6 = NULL;
daemon->discovered_ip_v4 = NULL;
daemon->discovered_ip_v6 = NULL;
list_head_init(&daemon->deferred_updates);

/* Tell the ecdh() function how to talk to hsmd */
Expand Down
4 changes: 2 additions & 2 deletions gossipd/gossipd.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ struct daemon {
struct wireaddr *announceable;

/* verified remote_addr as reported by recent peers */
struct wireaddr *remote_addr_v4;
struct wireaddr *remote_addr_v6;
struct wireaddr *discovered_ip_v4;
struct wireaddr *discovered_ip_v6;

/* Timer until we can send an updated node_announcement */
struct oneshot *node_announce_timer;
Expand Down
6 changes: 3 additions & 3 deletions gossipd/gossipd_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,6 @@ msgdata,gossipd_local_private_channel,features,u8,len
msgtype,gossipd_used_local_channel_update,3052
msgdata,gossipd_used_local_channel_update,scid,short_channel_id,

# Tell gossipd we have discovered a new remote_addr
msgtype,gossipd_remote_addr,3009
msgdata,gossipd_remote_addr,remote_addr,wireaddr,
# Tell gossipd we have verified a new public IP by the remote_addr feature
msgtype,gossipd_discovered_ip,3009
msgdata,gossipd_discovered_ip,discovered_ip,wireaddr,
2 changes: 1 addition & 1 deletion lightningd/gossip_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
case WIRE_GOSSIPD_ADDGOSSIP_REPLY:
case WIRE_GOSSIPD_NEW_BLOCKHEIGHT_REPLY:
case WIRE_GOSSIPD_GET_ADDRS_REPLY:
case WIRE_GOSSIPD_REMOTE_ADDR:
case WIRE_GOSSIPD_DISCOVERED_IP:
break;

case WIRE_GOSSIPD_GET_TXOUT:
Expand Down
2 changes: 2 additions & 0 deletions lightningd/lightningd.c
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ static struct lightningd *new_lightningd(const tal_t *ctx)

ld->remote_addr_v4 = NULL;
ld->remote_addr_v6 = NULL;
ld->discovered_ip_v4 = NULL;
ld->discovered_ip_v6 = NULL;
ld->listen = true;
ld->autolisten = true;
ld->reconnect = true;
Expand Down
4 changes: 4 additions & 0 deletions lightningd/lightningd.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ struct lightningd {
struct node_id remote_addr_v4_peer;
struct node_id remote_addr_v6_peer;

/* verified discovered IPs to be used for anouncement */
struct wireaddr *discovered_ip_v4;
struct wireaddr *discovered_ip_v6;

/* Bearer of all my secrets. */
int hsm_fd;
struct subd *hsm;
Expand Down
43 changes: 29 additions & 14 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -1274,10 +1274,17 @@ static void update_remote_addr(struct lightningd *ld,
const struct wireaddr *remote_addr,
const struct node_id peer_id)
{
u16 public_port;

/* failsafe to prevent privacy leakage. */
if (ld->always_use_proxy || ld->config.disable_ip_discovery)
return;

/* Peers will have likey reported our dynamic outbound TCP port.
* Best guess is that we use default port for the selected network,
* until we add a commandline switch to override this. */
public_port = chainparams_get_ln_port(chainparams);

switch (remote_addr->type) {
case ADDR_TYPE_IPV4:
/* init pointers first time */
Expand All @@ -1292,10 +1299,14 @@ static void update_remote_addr(struct lightningd *ld,
break;
}
/* tell gossip we have a valid update */
if (wireaddr_eq_without_port(ld->remote_addr_v4, remote_addr))
subd_send_msg(ld->gossip, towire_gossipd_remote_addr(
if (wireaddr_eq_without_port(ld->remote_addr_v4, remote_addr)) {
ld->discovered_ip_v4 = tal_dup(ld, struct wireaddr,
ld->remote_addr_v4);
ld->discovered_ip_v4->port = public_port;
subd_send_msg(ld->gossip, towire_gossipd_discovered_ip(
tmpctx,
ld->remote_addr_v4));
ld->discovered_ip_v4));
}
/* store latest values */
*ld->remote_addr_v4 = *remote_addr;
ld->remote_addr_v4_peer = peer_id;
Expand All @@ -1311,10 +1322,14 @@ static void update_remote_addr(struct lightningd *ld,
*ld->remote_addr_v6 = *remote_addr;
break;
}
if (wireaddr_eq_without_port(ld->remote_addr_v6, remote_addr))
subd_send_msg(ld->gossip, towire_gossipd_remote_addr(
if (wireaddr_eq_without_port(ld->remote_addr_v6, remote_addr)) {
ld->discovered_ip_v6 = tal_dup(ld, struct wireaddr,
ld->remote_addr_v6);
ld->discovered_ip_v6->port = public_port;
subd_send_msg(ld->gossip, towire_gossipd_discovered_ip(
tmpctx,
ld->remote_addr_v6));
ld->discovered_ip_v6));
}
*ld->remote_addr_v6 = *remote_addr;
ld->remote_addr_v6_peer = peer_id;
break;
Expand Down Expand Up @@ -2247,22 +2262,22 @@ static struct command_result *json_getinfo(struct command *cmd,
for (size_t i = 0; i < count_announceable; i++)
json_add_address(response, NULL, cmd->ld->announceable+i);

/* Currently, IP discovery will only be announced by gossipd, if we
* don't already have usable addresses.
/* Currently, IP discovery will only be announced by gossipd,
* if we don't already have usable addresses.
* See `create_node_announcement` in `gossip_generation.c`. */
if (count_announceable == 0) {
if (cmd->ld->remote_addr_v4 != NULL &&
if (cmd->ld->discovered_ip_v4 != NULL &&
!wireaddr_arr_contains(
cmd->ld->announceable,
cmd->ld->remote_addr_v4))
cmd->ld->discovered_ip_v4))
json_add_address(response, NULL,
cmd->ld->remote_addr_v4);
if (cmd->ld->remote_addr_v6 != NULL &&
cmd->ld->discovered_ip_v4);
if (cmd->ld->discovered_ip_v6 != NULL &&
!wireaddr_arr_contains(
cmd->ld->announceable,
cmd->ld->remote_addr_v6))
cmd->ld->discovered_ip_v6))
json_add_address(response, NULL,
cmd->ld->remote_addr_v6);
cmd->ld->discovered_ip_v6);
}
json_array_end(response);

Expand Down
6 changes: 3 additions & 3 deletions lightningd/test/run-invoice-select-inchan.c
Original file line number Diff line number Diff line change
Expand Up @@ -733,9 +733,9 @@ u8 *towire_errorfmt(const tal_t *ctx UNNEEDED,
const struct channel_id *channel UNNEEDED,
const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "towire_errorfmt called!\n"); abort(); }
/* Generated stub for towire_gossipd_remote_addr */
u8 *towire_gossipd_remote_addr(const tal_t *ctx UNNEEDED, const struct wireaddr *remote_addr UNNEEDED)
{ fprintf(stderr, "towire_gossipd_remote_addr called!\n"); abort(); }
/* Generated stub for towire_gossipd_discovered_ip */
u8 *towire_gossipd_discovered_ip(const tal_t *ctx UNNEEDED, const struct wireaddr *discovered_ip UNNEEDED)
{ fprintf(stderr, "towire_gossipd_discovered_ip called!\n"); abort(); }
/* Generated stub for towire_hsmd_sign_bolt12 */
u8 *towire_hsmd_sign_bolt12(const tal_t *ctx UNNEEDED, const wirestring *messagename UNNEEDED, const wirestring *fieldname UNNEEDED, const struct sha256 *merkleroot UNNEEDED, const u8 *publictweak UNNEEDED)
{ fprintf(stderr, "towire_hsmd_sign_bolt12 called!\n"); abort(); }
Expand Down
12 changes: 12 additions & 0 deletions tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ def test_remote_addr(node_factory, bitcoind):
l2.daemon.opts['bind-addr'] = l2.daemon.opts['addr']
del l2.daemon.opts['addr']
l2.start()
assert len(l2.rpc.getinfo()['address']) == 0

l2.rpc.connect(l1.info['id'], 'localhost', l1.port)
logmsg = l2.daemon.wait_for_log("Peer says it sees our address as: 127.0.0.1:[0-9]{5}")
Expand All @@ -95,6 +96,7 @@ def test_remote_addr(node_factory, bitcoind):
bitcoind.generate_block(5)
l1.daemon.wait_for_log(f"Received node_announcement for node {l2.info['id']}")
assert(len(l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses']) == 0)
assert len(l2.rpc.getinfo()['address']) == 0

def_port = default_ln_port(l2.info["network"])

Expand All @@ -110,6 +112,7 @@ def test_remote_addr(node_factory, bitcoind):
# Now l1 sees l2 but without announced addresses.
assert(len(l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses']) == 0)
assert not l2.daemon.is_in_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port))
assert len(l2.rpc.getinfo()['address']) == 0

# connect second node. This will not yet trigger `node_annoucement` update,
# as we again do not have a channel at the time we connected.
Expand All @@ -120,6 +123,7 @@ def test_remote_addr(node_factory, bitcoind):
l2.fundchannel(l3, wait_for_active=True)
bitcoind.generate_block(5)
assert not l2.daemon.is_in_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port))
assert len(l2.rpc.getinfo()['address']) == 0

# restart, reconnect and re-check for updated node_annoucement. This time
# l2 sees that two different peers with channel reported the same `remote_addr`.
Expand All @@ -129,11 +133,19 @@ def test_remote_addr(node_factory, bitcoind):
l2.daemon.wait_for_log("Update our node_announcement for discovered address: 127.0.0.1:{}".format(def_port))
l1.daemon.wait_for_log(f"Received node_announcement for node {l2.info['id']}")

# check l1 sees the updated node announcement via CLI listnodes
address = l1.rpc.listnodes(l2.info['id'])['nodes'][0]['addresses'][0]
assert address['type'] == "ipv4"
assert address['address'] == "127.0.0.1"
assert address['port'] == def_port

# also check l2 returns the announced address (and port) via CLI getinfo
getinfo = l2.rpc.getinfo()
assert len(getinfo['address']) == 1
assert getinfo['address'][0]['type'] == 'ipv4'
assert getinfo['address'][0]['address'] == '127.0.0.1'
assert getinfo['address'][0]['port'] == def_port


@pytest.mark.developer("needs DEVELOPER=1 for fast gossip and --dev-allow-localhost for local remote_addr")
def test_remote_addr_disabled(node_factory, bitcoind):
Expand Down
11 changes: 3 additions & 8 deletions wallet/test/run-wallet.c
Original file line number Diff line number Diff line change
Expand Up @@ -611,11 +611,6 @@ struct command_result *param_short_channel_id(struct command *cmd UNNEEDED,
const jsmntok_t *tok UNNEEDED,
struct short_channel_id **scid UNNEEDED)
{ fprintf(stderr, "param_short_channel_id called!\n"); abort(); }
/* Generated stub for param_string */
struct command_result *param_string(struct command *cmd UNNEEDED, const char *name UNNEEDED,
const char * buffer UNNEEDED, const jsmntok_t *tok UNNEEDED,
const char **str UNNEEDED)
{ fprintf(stderr, "param_string called!\n"); abort(); }
/* Generated stub for parse_onionpacket */
struct onionpacket *parse_onionpacket(const tal_t *ctx UNNEEDED,
const u8 *src UNNEEDED,
Expand Down Expand Up @@ -774,9 +769,9 @@ u8 *towire_final_incorrect_cltv_expiry(const tal_t *ctx UNNEEDED, u32 cltv_expir
/* Generated stub for towire_final_incorrect_htlc_amount */
u8 *towire_final_incorrect_htlc_amount(const tal_t *ctx UNNEEDED, struct amount_msat incoming_htlc_amt UNNEEDED)
{ fprintf(stderr, "towire_final_incorrect_htlc_amount called!\n"); abort(); }
/* Generated stub for towire_gossipd_remote_addr */
u8 *towire_gossipd_remote_addr(const tal_t *ctx UNNEEDED, const struct wireaddr *remote_addr UNNEEDED)
{ fprintf(stderr, "towire_gossipd_remote_addr called!\n"); abort(); }
/* Generated stub for towire_gossipd_discovered_ip */
u8 *towire_gossipd_discovered_ip(const tal_t *ctx UNNEEDED, const struct wireaddr *discovered_ip UNNEEDED)
{ fprintf(stderr, "towire_gossipd_discovered_ip called!\n"); abort(); }
/* Generated stub for towire_hsmd_get_output_scriptpubkey */
u8 *towire_hsmd_get_output_scriptpubkey(const tal_t *ctx UNNEEDED, u64 channel_id UNNEEDED, const struct node_id *peer_id UNNEEDED, const struct pubkey *commitment_point UNNEEDED)
{ fprintf(stderr, "towire_hsmd_get_output_scriptpubkey called!\n"); abort(); }
Expand Down