diff --git a/lightningd/opening_control.c b/lightningd/opening_control.c index 9dddddf584d7..23fd552d35bd 100644 --- a/lightningd/opening_control.c +++ b/lightningd/opening_control.c @@ -1351,6 +1351,7 @@ static struct channel *stub_chan(struct command *cmd, 0, &nodeid, &wint, + NULL, false); } diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index ecbdd2fd19ef..c08b2e3253ec 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -76,13 +76,6 @@ static void destroy_peer(struct peer *peer) peer_dbid_map_del(peer->ld->peers_by_dbid, peer); } -static void peer_update_features(struct peer *peer, - const u8 *their_features TAKES) -{ - tal_free(peer->their_features); - peer->their_features = tal_dup_talarr(peer, u8, their_features); -} - void peer_set_dbid(struct peer *peer, u64 dbid) { assert(!peer->dbid); @@ -94,6 +87,7 @@ void peer_set_dbid(struct peer *peer, u64 dbid) struct peer *new_peer(struct lightningd *ld, u64 dbid, const struct node_id *id, const struct wireaddr_internal *addr, + const u8 *their_features, bool connected_incoming) { /* We are owned by our channels, and freed manually by destroy_channel */ @@ -106,12 +100,16 @@ struct peer *new_peer(struct lightningd *ld, u64 dbid, peer->addr = *addr; peer->connected_incoming = connected_incoming; peer->remote_addr = NULL; - peer->their_features = NULL; list_head_init(&peer->channels); peer->direction = node_id_idx(&peer->ld->id, &peer->id); peer->connected = PEER_DISCONNECTED; peer->last_connect_attempt.ts.tv_sec = peer->last_connect_attempt.ts.tv_nsec = 0; + if (their_features) + peer->their_features = tal_dup_talarr(peer, u8, their_features); + else + peer->their_features = NULL; + #if DEVELOPER peer->ignore_htlcs = false; #endif @@ -1403,7 +1401,11 @@ void peer_connected(struct lightningd *ld, const u8 *msg) peer = peer_by_id(ld, &id); if (!peer) peer = new_peer(ld, 0, &id, &hook_payload->addr, - hook_payload->incoming); + their_features, hook_payload->incoming); + else { + tal_free(peer->their_features); + peer->their_features = tal_dup_talarr(peer, u8, their_features); + } /* We track this, because messages can race between connectd and us. * For example, we could tell it to attach a subd, but it's actually @@ -1423,7 +1425,6 @@ void peer_connected(struct lightningd *ld, const u8 *msg) if (peer->remote_addr) tal_free(peer->remote_addr); peer->remote_addr = NULL; - peer_update_features(peer, their_features); hook_payload->peer_id = id; /* If there's a connect command, use its id as basis for hook id */ @@ -1942,9 +1943,6 @@ static void json_add_peer(struct lightningd *ld, num_channels++; json_add_num(response, "num_channels", num_channels); - /* If it's not connected, features are unreliable: we don't - * store them in the database, and they would only reflect - * their features *last* time they connected. */ if (p->connected == PEER_CONNECTED) { json_array_start(response, "netaddr"); json_add_string(response, NULL, @@ -1956,8 +1954,11 @@ static void json_add_peer(struct lightningd *ld, if (p->remote_addr) json_add_string(response, "remote_addr", fmt_wireaddr(response, p->remote_addr)); - json_add_hex_talarr(response, "features", p->their_features); } + + /* Note: If !PEER_CONNECTED, peer may use different features on reconnect */ + json_add_hex_talarr(response, "features", p->their_features); + if (deprecated_apis) { json_array_start(response, "channels"); json_add_uncommitted_channel(response, p->uncommitted_channel, NULL); diff --git a/lightningd/peer_control.h b/lightningd/peer_control.h index 5f3044c38470..0f23ef410d5d 100644 --- a/lightningd/peer_control.h +++ b/lightningd/peer_control.h @@ -73,6 +73,7 @@ struct peer *find_peer_by_dbid(struct lightningd *ld, u64 dbid); struct peer *new_peer(struct lightningd *ld, u64 dbid, const struct node_id *id, const struct wireaddr_internal *addr, + const u8 *their_features, bool connected_incoming); /* Last one out deletes peer. Also removes from db. */ diff --git a/tests/test_connection.py b/tests/test_connection.py index c64f5513c35d..f6d019ca90ef 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -4393,6 +4393,30 @@ def test_peer_disconnected_reflected_in_channel_state(node_factory): wait_for(lambda: only_one(l1.rpc.listpeerchannels(l2.info['id'])['channels'])['peer_connected'] is False) +def test_peer_disconnected_has_featurebits(node_factory): + """ + Make sure that if a node is restarted, it still remembers feature + bits from a peer it has a channel with but isn't connected to + """ + l1, l2 = node_factory.line_graph(2) + + expected_features = expected_peer_features() + + l1_features = only_one(l2.rpc.listpeers()['peers'])['features'] + l2_features = only_one(l1.rpc.listpeers()['peers'])['features'] + assert l1_features == expected_features + assert l2_features == expected_features + + l1.stop() + l2.stop() + + # Ensure we persisted feature bits and return them even when disconnected + l1.start() + + wait_for(lambda: only_one(l1.rpc.listpeers(l2.info['id'])['peers'])['connected'] is False) + wait_for(lambda: only_one(l1.rpc.listpeers()['peers'])['features'] == expected_features) + + @pytest.mark.developer("needs dev-no-reconnect") def test_reconnect_no_additional_transient_failure(node_factory, bitcoind): l1, l2 = node_factory.line_graph(2, opts=[{'may_reconnect': True}, diff --git a/wallet/db.c b/wallet/db.c index bf89d9fbff0d..12623164b459 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -949,6 +949,7 @@ static struct migration dbmigrations[] = { {NULL, migrate_invalid_last_tx_psbts}, {SQL("ALTER TABLE channels ADD channel_type BLOB DEFAULT NULL;"), NULL}, {NULL, migrate_fill_in_channel_type}, + {SQL("ALTER TABLE peers ADD feature_bits BLOB DEFAULT NULL;"), NULL}, }; /** diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index 09ee1d3c1e88..e63f8b786119 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -1172,7 +1172,7 @@ static bool test_wallet_outputs(struct lightningd *ld, const tal_t *ctx) /* Add another utxo that's CSV-locked for 5 blocks */ assert(parse_wireaddr_internal(tmpctx, "localhost:1234", 0, false, &addr) == NULL); - channel.peer = new_peer(ld, 0, &id, &addr, false); + channel.peer = new_peer(ld, 0, &id, &addr, NULL, false); channel.dbid = 1; channel.type = channel_type_anchor_outputs(tmpctx); memset(&u.outpoint, 3, sizeof(u.outpoint)); @@ -1502,7 +1502,7 @@ static bool test_channel_crud(struct lightningd *ld, const tal_t *ctx) c1.first_blocknum = 1; assert(parse_wireaddr_internal(tmpctx, "localhost:1234", 0, false, &addr) == NULL); c1.final_key_idx = 1337; - p = new_peer(ld, 0, &id, &addr, false); + p = new_peer(ld, 0, &id, &addr, NULL, false); c1.peer = p; c1.dbid = wallet_get_channel_dbid(w); c1.state = CHANNELD_NORMAL; @@ -1665,7 +1665,7 @@ static bool test_channel_inflight_crud(struct lightningd *ld, const tal_t *ctx) assert(parse_wireaddr_internal(tmpctx, "localhost:1234", 0, false, &addr) == NULL); /* new channel! */ - p = new_peer(ld, 0, &id, &addr, false); + p = new_peer(ld, 0, &id, &addr, NULL, false); funding_sats = AMOUNT_SAT(4444444); our_sats = AMOUNT_SAT(3333333); diff --git a/wallet/wallet.c b/wallet/wallet.c index 1939b67e618b..a231ea2cfaf1 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -859,7 +859,7 @@ static struct peer *wallet_peer_load(struct wallet *w, const u64 dbid) struct db_stmt *stmt; stmt = db_prepare_v2( - w->db, SQL("SELECT id, node_id, address FROM peers WHERE id=?;")); + w->db, SQL("SELECT id, node_id, address, feature_bits FROM peers WHERE id=?;")); db_bind_u64(stmt, 0, dbid); db_query_prepared(stmt); @@ -869,6 +869,7 @@ static struct peer *wallet_peer_load(struct wallet *w, const u64 dbid) if (db_col_is_null(stmt, "node_id")) { db_col_ignore(stmt, "address"); db_col_ignore(stmt, "id"); + db_col_ignore(stmt, "feature_bits"); goto done; } @@ -886,7 +887,7 @@ static struct peer *wallet_peer_load(struct wallet *w, const u64 dbid) } /* FIXME: save incoming in db! */ - peer = new_peer(w->ld, db_col_u64(stmt, "id"), &id, &addr, false); + peer = new_peer(w->ld, db_col_u64(stmt, "id"), &id, &addr, db_col_arr(stmt, stmt, "feature_bits", u8), false); done: tal_free(stmt); @@ -2271,21 +2272,23 @@ static void wallet_peer_save(struct wallet *w, struct peer *peer) peer_set_dbid(peer, db_col_u64(stmt, "id")); tal_free(stmt); - /* Since we're at it update the wireaddr */ + /* Since we're at it update the wireaddr, feature bits */ stmt = db_prepare_v2( - w->db, SQL("UPDATE peers SET address = ? WHERE id = ?")); + w->db, SQL("UPDATE peers SET address = ?, feature_bits = ? WHERE id = ?")); db_bind_text(stmt, 0, addr); - db_bind_u64(stmt, 1, peer->dbid); + db_bind_talarr(stmt, 1, peer->their_features); + db_bind_u64(stmt, 2, peer->dbid); db_exec_prepared_v2(take(stmt)); } else { /* Unknown peer, create it from scratch */ tal_free(stmt); stmt = db_prepare_v2(w->db, - SQL("INSERT INTO peers (node_id, address) VALUES (?, ?);") + SQL("INSERT INTO peers (node_id, address, feature_bits) VALUES (?, ?, ?);") ); db_bind_node_id(stmt, 0, &peer->id); db_bind_text(stmt, 1,addr); + db_bind_talarr(stmt, 2, peer->their_features); db_exec_prepared_v2(stmt); peer_set_dbid(peer, db_last_insert_id_v2(take(stmt))); }