Skip to content

Commit

Permalink
gossmap: don't spew to stderr, include counter for callers.
Browse files Browse the repository at this point in the history
Fixes: #4722
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: don't drop complaints about silly channels to stderr.
  • Loading branch information
rustyrussell committed Aug 23, 2021
1 parent c970195 commit cf1dd77
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 42 deletions.
38 changes: 21 additions & 17 deletions common/gossmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ static struct gossmap_chan *add_channel(struct gossmap *map,
* * [`u32`:`fee_proportional_millionths`]
* * [`u64`:`htlc_maximum_msat`] (option_channel_htlc_max)
*/
static void update_channel(struct gossmap *map, size_t cupdate_off)
static bool update_channel(struct gossmap *map, size_t cupdate_off)
{
/* Note that first two bytes are message type */
const size_t scid_off = cupdate_off + 2 + (64 + 32);
Expand All @@ -473,6 +473,7 @@ static void update_channel(struct gossmap *map, size_t cupdate_off)
struct gossmap_chan *chan;
struct half_chan hc;
u8 chanflags;
bool dumb_values;

scid.u64 = map_be64(map, scid_off);
chan = gossmap_find_chan(map, &scid);
Expand Down Expand Up @@ -502,19 +503,18 @@ static void update_channel(struct gossmap *map, size_t cupdate_off)
if (hc.base_fee != map_be32(map, fee_base_off)
|| hc.proportional_fee != map_be32(map, fee_prop_off)
|| hc.delay != map_be16(map, cltv_expiry_delta_off)) {
warnx("channel_update %s ignored: fee %u/%u cltv %u too large",
type_to_string(tmpctx, struct short_channel_id, &scid),
map_be32(map, fee_base_off),
map_be32(map, fee_prop_off),
map_be16(map, cltv_expiry_delta_off));
dumb_values = true;
hc.htlc_max = 0;
hc.enabled = false;
}
} else
dumb_values = false;

/* Preserve this */
hc.nodeidx = chan->half[chanflags & 1].nodeidx;
chan->half[chanflags & 1] = hc;
chan->cupdate_off[chanflags & 1] = cupdate_off;

return !dumb_values;
}

static void remove_channel_by_deletemsg(struct gossmap *map, size_t del_off)
Expand Down Expand Up @@ -580,13 +580,14 @@ static void reopen_store(struct gossmap *map, size_t ended_off)

close(map->fd);
map->fd = fd;
gossmap_refresh(map);
gossmap_refresh(map, NULL);
}

static bool map_catchup(struct gossmap *map)
static bool map_catchup(struct gossmap *map, size_t *num_rejected)
{
size_t reclen;
bool changed = false;
size_t num_bad = 0;

for (; map->map_end + sizeof(struct gossip_hdr) < map->map_size;
map->map_end += reclen) {
Expand All @@ -612,9 +613,9 @@ static bool map_catchup(struct gossmap *map)
else if (type == WIRE_GOSSIP_STORE_PRIVATE_CHANNEL)
add_channel(map, off + 2 + 8 + 2, true);
else if (type == WIRE_CHANNEL_UPDATE)
update_channel(map, off);
num_bad += !update_channel(map, off);
else if (type == WIRE_GOSSIP_STORE_PRIVATE_UPDATE)
update_channel(map, off + 2 + 2);
num_bad += !update_channel(map, off + 2 + 2);
else if (type == WIRE_GOSSIP_STORE_DELETE_CHAN)
remove_channel_by_deletemsg(map, off);
else if (type == WIRE_NODE_ANNOUNCEMENT)
Expand All @@ -627,10 +628,12 @@ static bool map_catchup(struct gossmap *map)
changed = true;
}

if (num_rejected)
*num_rejected = num_bad;
return changed;
}

static bool load_gossip_store(struct gossmap *map)
static bool load_gossip_store(struct gossmap *map, size_t *num_rejected)
{
map->fd = open(map->fname, O_RDONLY);
if (map->fd < 0)
Expand Down Expand Up @@ -665,7 +668,7 @@ static bool load_gossip_store(struct gossmap *map)
map->freed_nodes = init_node_arr(map->node_arr, 0);

map->map_end = 1;
map_catchup(map);
map_catchup(map, num_rejected);
return true;
}

Expand Down Expand Up @@ -910,7 +913,7 @@ void gossmap_remove_localmods(struct gossmap *map,
map->local = NULL;
}

bool gossmap_refresh(struct gossmap *map)
bool gossmap_refresh(struct gossmap *map, size_t *num_rejected)
{
off_t len;

Expand All @@ -928,14 +931,15 @@ bool gossmap_refresh(struct gossmap *map)
map->mmap = mmap(NULL, map->map_size, PROT_READ, MAP_SHARED, map->fd, 0);
if (map->mmap == MAP_FAILED)
map->mmap = NULL;
return map_catchup(map);
return map_catchup(map, num_rejected);
}

struct gossmap *gossmap_load(const tal_t *ctx, const char *filename)
struct gossmap *gossmap_load(const tal_t *ctx, const char *filename,
size_t *num_channel_updates_rejected)
{
map = tal(ctx, struct gossmap);
map->fname = tal_strdup(map, filename);
if (load_gossip_store(map))
if (load_gossip_store(map, num_channel_updates_rejected))
tal_add_destructor(map, destroy_map);
else
map = tal_free(map);
Expand Down
7 changes: 5 additions & 2 deletions common/gossmap.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,14 @@ struct gossmap_chan {
} half[2];
};

struct gossmap *gossmap_load(const tal_t *ctx, const char *filename);
/* If num_channel_updates_rejected is not NULL, indicates how many channels we
* marked inactive because their values were too high to be represented. */
struct gossmap *gossmap_load(const tal_t *ctx, const char *filename,
size_t *num_channel_updates_rejected);

/* Call this before using to ensure it's up-to-date. Returns true if something
* was updated. Note: this can scramble node and chan indexes! */
bool gossmap_refresh(struct gossmap *map);
bool gossmap_refresh(struct gossmap *map, size_t *num_channel_updates_rejected);

/* Local modifications. */
struct gossmap_localmods *gossmap_localmods_new(const tal_t *ctx);
Expand Down
4 changes: 2 additions & 2 deletions common/test/run-gossmap_local.c
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ int main(int argc, char *argv[])
fd = mkstemp(gossfile);
assert(write_all(fd, canned_map, sizeof(canned_map)));

map = gossmap_load(tmpctx, gossfile);
map = gossmap_load(tmpctx, gossfile, NULL);
assert(map);

/* There is a public channel 2<->3 (103x1x0), and private
Expand Down Expand Up @@ -503,6 +503,6 @@ int main(int argc, char *argv[])

/* Now we can refresh. */
assert(write(fd, "", 1) == 1);
gossmap_refresh(map);
gossmap_refresh(map, NULL);
common_shutdown();
}
6 changes: 3 additions & 3 deletions common/test/run-route-specific.c
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ int main(void)
assert(write(store_fd, &gossip_version, sizeof(gossip_version))
== sizeof(gossip_version));

gossmap = gossmap_load(tmpctx, gossipfilename);
gossmap = gossmap_load(tmpctx, gossipfilename, NULL);

/* [{'active': True, 'short_id': '6990:2:1/1', 'fee_per_kw': 10, 'delay': 5, 'message_flags': 0, 'channel_flags': 1, 'destination': '0230ad0e74ea03976b28fda587bb75bdd357a1938af4424156a18265167f5e40ae', 'source': '02ea622d5c8d6143f15ed3ce1d501dd0d3d09d3b1c83a44d0034949f8a9ab60f06', 'last_update': 1504064344}, */
add_connection(store_fd, &c, &b, "6990x2x1",
Expand All @@ -232,7 +232,7 @@ int main(void)
AMOUNT_MSAT(0), AMOUNT_MSAT(1000),
0, 10, 5, false);

assert(gossmap_refresh(gossmap));
assert(gossmap_refresh(gossmap, NULL));

a_node = gossmap_find_node(gossmap, &a);
b_node = gossmap_find_node(gossmap, &b);
Expand Down Expand Up @@ -278,7 +278,7 @@ int main(void)
add_connection(store_fd, &a, &d, "6991x2x1",
AMOUNT_MSAT(100), AMOUNT_MSAT(499968), /* exact repr in fp16! */
0, 0, 5);
assert(gossmap_refresh(gossmap));
assert(gossmap_refresh(gossmap, NULL));

a_node = gossmap_find_node(gossmap, &a);
b_node = gossmap_find_node(gossmap, &b);
Expand Down
12 changes: 6 additions & 6 deletions common/test/run-route.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,18 +190,18 @@ int main(void)
store_fd = mkstemp(gossipfilename);
assert(write(store_fd, &gossip_version, sizeof(gossip_version))
== sizeof(gossip_version));
gossmap = gossmap_load(tmpctx, gossipfilename);
gossmap = gossmap_load(tmpctx, gossipfilename, NULL);

memset(&tmp, 'a', sizeof(tmp));
node_id_from_privkey(&tmp, &a);
memset(&tmp, 'b', sizeof(tmp));
node_id_from_privkey(&tmp, &b);

assert(!gossmap_refresh(gossmap));
assert(!gossmap_refresh(gossmap, NULL));

/* A<->B */
add_connection(store_fd, &a, &b, 1, 1, 1);
assert(gossmap_refresh(gossmap));
assert(gossmap_refresh(gossmap, NULL));

a_node = gossmap_find_node(gossmap, &a);
b_node = gossmap_find_node(gossmap, &b);
Expand All @@ -219,7 +219,7 @@ int main(void)
memset(&tmp, 'c', sizeof(tmp));
node_id_from_privkey(&tmp, &c);
add_connection(store_fd, &b, &c, 1, 1, 1);
assert(gossmap_refresh(gossmap));
assert(gossmap_refresh(gossmap, NULL));

/* These can theoretically change after refresh! */
a_node = gossmap_find_node(gossmap, &a);
Expand All @@ -243,7 +243,7 @@ int main(void)

add_connection(store_fd, &a, &d, 0, 2, 1);
add_connection(store_fd, &d, &c, 0, 2, 1);
assert(gossmap_refresh(gossmap));
assert(gossmap_refresh(gossmap, NULL));

/* These can theoretically change after refresh! */
a_node = gossmap_find_node(gossmap, &a);
Expand Down Expand Up @@ -284,7 +284,7 @@ int main(void)

/* Make B->C inactive, force it back via D */
update_connection(store_fd, &b, &c, 1, 1, 1, true);
assert(gossmap_refresh(gossmap));
assert(gossmap_refresh(gossmap, NULL));

/* These can theoretically change after refresh! */
a_node = gossmap_find_node(gossmap, &a);
Expand Down
8 changes: 5 additions & 3 deletions devtools/route.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ int main(int argc, char *argv[])
struct gossmap *map;
struct node_id dstid;
bool clean_topology = false;
size_t num_channel_updates_rejected;

opt_register_noarg("--clean-topology", opt_set_bool, &clean_topology,
"Clean up topology before run");
Expand All @@ -83,13 +84,14 @@ int main(int argc, char *argv[])
opt_usage_exit_fail("Expect 3 arguments");

tstart = time_mono();
map = gossmap_load(NULL, argv[1]);
map = gossmap_load(NULL, argv[1], &num_channel_updates_rejected);
if (!map)
err(1, "Loading gossip store %s", argv[1]);
tstop = time_mono();

printf("# Time to load: %"PRIu64" msec\n",
time_to_msec(timemono_between(tstop, tstart)));
printf("# Time to load: %"PRIu64" msec (%zu ignored)\n",
time_to_msec(timemono_between(tstop, tstart)),
num_channel_updates_rejected);

if (clean_topology)
clean_topo(map, false);
Expand Down
2 changes: 1 addition & 1 deletion devtools/topology.c
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ int main(int argc, char *argv[])
opt_usage_exit_fail("Expect 3 arguments");

tstart = time_mono();
map = gossmap_load(NULL, argv[1]);
map = gossmap_load(NULL, argv[1], NULL);
if (!map)
err(1, "Loading gossip store %s", argv[1]);
tstop = time_mono();
Expand Down
10 changes: 8 additions & 2 deletions plugins/fetchinvoice.c
Original file line number Diff line number Diff line change
Expand Up @@ -462,20 +462,26 @@ static struct command_result *sendonionmsg_done(struct command *cmd,

static void init_gossmap(struct plugin *plugin)
{
size_t num_cupdates_rejected;
global_gossmap
= notleak_with_children(gossmap_load(NULL,
GOSSIP_STORE_FILENAME));
GOSSIP_STORE_FILENAME,
&num_cupdates_rejected));
if (!global_gossmap)
plugin_err(plugin, "Could not load gossmap %s: %s",
GOSSIP_STORE_FILENAME, strerror(errno));
if (num_cupdates_rejected)
plugin_log(plugin, LOG_DBG,
"gossmap ignored %zu channel updates",
num_cupdates_rejected);
}

static struct gossmap *get_gossmap(struct plugin *plugin)
{
if (!global_gossmap)
init_gossmap(plugin);
else
gossmap_refresh(global_gossmap);
gossmap_refresh(global_gossmap, NULL);
return global_gossmap;
}

Expand Down
10 changes: 8 additions & 2 deletions plugins/libplugin-pay.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,26 @@ static struct gossmap *global_gossmap;

static void init_gossmap(struct plugin *plugin)
{
size_t num_channel_updates_rejected;
global_gossmap
= notleak_with_children(gossmap_load(NULL,
GOSSIP_STORE_FILENAME));
GOSSIP_STORE_FILENAME,
&num_channel_updates_rejected));
if (!global_gossmap)
plugin_err(plugin, "Could not load gossmap %s: %s",
GOSSIP_STORE_FILENAME, strerror(errno));
if (num_channel_updates_rejected)
plugin_log(plugin, LOG_DBG,
"gossmap ignored %zu channel updates",
num_channel_updates_rejected);
}

struct gossmap *get_gossmap(struct plugin *plugin)
{
if (!global_gossmap)
init_gossmap(plugin);
else
gossmap_refresh(global_gossmap);
gossmap_refresh(global_gossmap, NULL);
return global_gossmap;
}

Expand Down
4 changes: 2 additions & 2 deletions plugins/test/run-route-overlong.c
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ int main(void)
assert(write(store_fd, &gossip_version, sizeof(gossip_version))
== sizeof(gossip_version));

gossmap = gossmap_load(tmpctx, gossipfilename);
gossmap = gossmap_load(tmpctx, gossipfilename, NULL);

for (size_t i = 0; i < NUM_NODES; i++) {
struct privkey tmp;
Expand Down Expand Up @@ -384,7 +384,7 @@ int main(void)
1 << i);
}

assert(gossmap_refresh(gossmap));
assert(gossmap_refresh(gossmap, NULL));
for (size_t i = ROUTING_MAX_HOPS; i > 2; i--) {
struct gossmap_node *dst, *src;
struct route_hop *r;
Expand Down
11 changes: 9 additions & 2 deletions plugins/topology.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ static struct plugin *plugin;
/* We load this on demand, since we can start before gossipd. */
static struct gossmap *get_gossmap(void)
{
gossmap_refresh(global_gossmap);
gossmap_refresh(global_gossmap, NULL);
return global_gossmap;
}

Expand Down Expand Up @@ -687,17 +687,24 @@ static struct command_result *json_listincoming(struct command *cmd,
static const char *init(struct plugin *p,
const char *buf UNUSED, const jsmntok_t *config UNUSED)
{
size_t num_cupdates_rejected;

plugin = p;
rpc_scan(p, "getinfo",
take(json_out_obj(NULL, NULL, NULL)),
"{id:%}", JSON_SCAN(json_to_node_id, &local_id));

global_gossmap = notleak_with_children(gossmap_load(NULL,
GOSSIP_STORE_FILENAME));
GOSSIP_STORE_FILENAME,
&num_cupdates_rejected));
if (!global_gossmap)
plugin_err(plugin, "Could not load gossmap %s: %s",
GOSSIP_STORE_FILENAME, strerror(errno));

if (num_cupdates_rejected)
plugin_log(plugin, LOG_DBG,
"gossmap ignored %zu channel updates",
num_cupdates_rejected);
return NULL;
}

Expand Down

0 comments on commit cf1dd77

Please sign in to comment.