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

Debugging bad gossip flakes #7046

Closed
Closed
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
6 changes: 3 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ jobs:
BITCOIN_VERSION: "25.1"
ELEMENTS_VERSION: 22.0.2
RUST_PROFILE: release # Has to match the one in the compile step
PYTEST_OPTS: --timeout=1200 --force-flaky
PYTEST_OPTS: --timeout=1200
needs:
- compile
strategy:
Expand Down Expand Up @@ -323,7 +323,7 @@ jobs:
ELEMENTS_VERSION: 22.0.2
RUST_PROFILE: release # Has to match the one in the compile step
CFG: compile-gcc
PYTEST_OPTS: --test-group-random-seed=42 --timeout=1800 --force-flaky
PYTEST_OPTS: --test-group-random-seed=42 --timeout=1800
needs:
- compile
strategy:
Expand Down Expand Up @@ -394,7 +394,7 @@ jobs:
RUST_PROFILE: release
SLOW_MACHINE: 1
TEST_DEBUG: 1
PYTEST_OPTS: --test-group-random-seed=42 --timeout=1800 --force-flaky
PYTEST_OPTS: --test-group-random-seed=42 --timeout=1800
needs:
- compile
strategy:
Expand Down
11 changes: 9 additions & 2 deletions connectd/connectd.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <wire/peer_wire.h>
#include <wire/wire_io.h>
#include <wire/wire_sync.h>

Expand Down Expand Up @@ -1851,8 +1852,11 @@ static void peer_send_msg(struct io_conn *conn,
/* This can happen if peer hung up on us (or wrong counter
* if it reconnected). */
peer = peer_htable_get(daemon->peers, &id);
if (peer && peer->counter == counter)
if (peer && peer->counter == counter) {
status_debug("Sending %s from lightningd",
peer_wire_name(fromwire_peektype(sendmsg)));
inject_peer_msg(peer, take(sendmsg));
}
}

static void dev_connect_memleak(struct daemon *daemon, const u8 *msg)
Expand Down Expand Up @@ -2175,8 +2179,11 @@ static struct io_plan *recv_gossip(struct io_conn *conn,
fromwire_peektype(msg));

peer = peer_htable_get(daemon->peers, &dst);
if (peer)
if (peer) {
status_debug("Sending %s from gossipd",
peer_wire_name(fromwire_peektype(gossip_msg)));
inject_peer_msg(peer, take(gossip_msg));
}

return daemon_conn_read_next(conn, daemon->gossipd);
}
Expand Down
17 changes: 13 additions & 4 deletions connectd/multiplex.c
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,9 @@ static u8 *maybe_from_gossip_store(const tal_t *ctx, struct peer *peer)
msg = tal_free(msg);
goto again;
}
status_peer_debug(&peer->id, "Sending %s from gossip_store (next off = %zu)",
peer_wire_name(fromwire_peektype(msg)),
peer->gs.off);
status_peer_io(LOG_IO_OUT, &peer->id, msg);
return msg;
}
Expand Down Expand Up @@ -581,8 +584,11 @@ void send_custommsg(struct daemon *daemon, const u8 *msg)

/* Races can happen: this might be gone by now. */
peer = peer_htable_get(daemon->peers, &id);
if (peer)
if (peer) {
status_debug("Sending %s from custommsg",
peer_wire_name(fromwire_peektype(custommsg)));
inject_peer_msg(peer, take(custommsg));
}
}

static void handle_ping_in(struct peer *peer, const u8 *msg)
Expand Down Expand Up @@ -693,15 +699,18 @@ static void handle_gossip_timestamp_filter_in(struct peer *peer, const u8 *msg)
*/
/* For us, this means we only sweep the gossip store for messages
* if the first_timestamp is 0 */
if (first_timestamp == 0)
if (first_timestamp == 0) {
peer->gs.off = 1;
else if (first_timestamp == 0xFFFFFFFF)
status_peer_debug(&peer->id, "gossip_store zero timestamp off = %zu", peer->gs.off);
} else if (first_timestamp == 0xFFFFFFFF) {
peer->gs.off = peer->daemon->gossip_store_end;
else {
status_peer_debug(&peer->id, "gossip_store inf timestamp off = %zu", peer->gs.off);
} else {
/* We are actually a bit nicer than the spec, and we include
* "recent" gossip here. */
update_recent_timestamp(peer->daemon);
peer->gs.off = peer->daemon->gossip_store_recent_off;
status_peer_debug(&peer->id, "gossip_store other timestamp off = %zu", peer->gs.off);
}

/* BOLT #7:
Expand Down
2 changes: 2 additions & 0 deletions gossipd/gossmap_manage.c
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,8 @@ const char *gossmap_manage_channel_update(const tal_t *ctx,
if (!gossmap_find_chan(gossmap, &scid)
&& source_peer
&& sigcheck_channel_update(tmpctx, source_peer, &signature, update) == NULL) {
status_debug("gossip update for unknown channel %s",
type_to_string(tmpctx, struct short_channel_id, &scid));
tell_lightningd_peer_update(gm->daemon, source_peer,
scid, fee_base_msat,
fee_proportional_millionths,
Expand Down
2 changes: 1 addition & 1 deletion tests/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,7 @@ def test_reconnect_remote_sends_no_sigs(node_factory):
l2.daemon.wait_for_log('peer_out WIRE_ANNOUNCEMENT_SIGNATURES')

l1msgs = [l.split()[4] for l in l1.daemon.logs[l1needle:] if 'WIRE_ANNOUNCEMENT_SIGNATURES' in l]
assert l1msgs == ['peer_out', 'peer_in']
assert l1msgs == ['Sending', 'peer_out', 'peer_in']

# l2 only sends one.
assert len([l for l in l2.daemon.logs[l2needle:] if 'peer_out WIRE_ANNOUNCEMENT_SIGNATURES' in l]) == 1
Expand Down
Loading