From 66dcba099d8b35c6b5c53f4f3ea64f50506f7ec1 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 19 Oct 2018 11:47:49 +1030 Subject: [PATCH] gossipd: hand raw pubkeys in getnodes and getchannels entries. We spend quite a bit of time in libsecp256k1 moving them to and from DER encoding. With a bit of care, we can transfer the raw bytes from gossipd and manually decode them so a malformed one can't make us abort(). Before: real 0m0.629000-0.695000(0.64985+/-0.019)s After: real 0m0.359000-0.433000(0.37645+/-0.023)s At this point, the main issues are 11% of time spent in ccan/io's backend_wake (I tried using a hash table there, but that actually makes the small-number-of-fds case slower), and 65% of gossipd's time is in marshalling the response (all those tal_resize add up!). Signed-off-by: Rusty Russell --- gossipd/gossipd.c | 11 ++++++++--- lightningd/gossip_control.c | 26 ++++++++++++++++++++++---- lightningd/gossip_msg.c | 12 ++++++------ lightningd/gossip_msg.h | 6 ++++-- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 98bfe8031..b1bae2d0a 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -1370,6 +1370,11 @@ static struct io_plan *getroute_req(struct io_conn *conn, struct daemon *daemon, return daemon_conn_read_next(conn, &daemon->master); } +#define raw_pubkey(arr, id) \ + do { BUILD_ASSERT(sizeof(arr) == sizeof(*id)); \ + memcpy(arr, id, sizeof(*id)); \ + } while(0) + static void append_half_channel(struct gossip_getchannels_entry **entries, const struct chan *chan, int idx) @@ -1382,8 +1387,8 @@ static void append_half_channel(struct gossip_getchannels_entry **entries, e = tal_arr_expand(entries); - e->source = chan->nodes[idx]->id; - e->destination = chan->nodes[!idx]->id; + raw_pubkey(e->source, &chan->nodes[idx]->id); + raw_pubkey(e->destination, &chan->nodes[!idx]->id); e->satoshis = chan->satoshis; e->channel_flags = c->channel_flags; e->message_flags = c->message_flags; @@ -1442,7 +1447,7 @@ static void append_node(const struct gossip_getnodes_entry ***entries, *tal_arr_expand(entries) = e = tal(*entries, struct gossip_getnodes_entry); - e->nodeid = n->id; + raw_pubkey(e->nodeid, &n->id); e->last_timestamp = n->last_timestamp; if (e->last_timestamp < 0) return; diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index d4e8f375f..667d4bdce 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -177,6 +177,24 @@ void gossipd_notify_spend(struct lightningd *ld, subd_send_msg(ld->gossip, msg); } +/* Gossipd shouldn't give us bad pubkeys, but don't abort if they do */ +static void json_add_raw_pubkey(struct json_result *response, + const char *fieldname, + const u8 raw_pubkey[sizeof(struct pubkey)]) +{ + secp256k1_pubkey pubkey; + u8 der[PUBKEY_DER_LEN]; + size_t outlen = PUBKEY_DER_LEN; + + memcpy(&pubkey, raw_pubkey, sizeof(pubkey)); + if (!secp256k1_ec_pubkey_serialize(secp256k1_ctx, der, &outlen, + &pubkey, + SECP256K1_EC_COMPRESSED)) + json_add_string(response, fieldname, "INVALID PUBKEY"); + else + json_add_hex(response, fieldname, der, sizeof(der)); +} + static void json_getnodes_reply(struct subd *gossip UNUSED, const u8 *reply, const int *fds UNUSED, struct command *cmd) @@ -198,7 +216,7 @@ static void json_getnodes_reply(struct subd *gossip UNUSED, const u8 *reply, struct json_escaped *esc; json_object_start(response, NULL); - json_add_pubkey(response, "nodeid", &nodes[i]->nodeid); + json_add_raw_pubkey(response, "nodeid", nodes[i]->nodeid); if (nodes[i]->last_timestamp < 0) { json_object_end(response); continue; @@ -349,9 +367,9 @@ static void json_listchannels_reply(struct subd *gossip UNUSED, const u8 *reply, json_array_start(response, "channels"); for (i = 0; i < tal_count(entries); i++) { json_object_start(response, NULL); - json_add_pubkey(response, "source", &entries[i].source); - json_add_pubkey(response, "destination", - &entries[i].destination); + json_add_raw_pubkey(response, "source", entries[i].source); + json_add_raw_pubkey(response, "destination", + entries[i].destination); json_add_string(response, "short_channel_id", type_to_string(reply, struct short_channel_id, &entries[i].short_channel_id)); diff --git a/lightningd/gossip_msg.c b/lightningd/gossip_msg.c index deeb02434..20d84af9b 100644 --- a/lightningd/gossip_msg.c +++ b/lightningd/gossip_msg.c @@ -12,7 +12,7 @@ struct gossip_getnodes_entry *fromwire_gossip_getnodes_entry(const tal_t *ctx, u16 flen; entry = tal(ctx, struct gossip_getnodes_entry); - fromwire_pubkey(pptr, max, &entry->nodeid); + fromwire(pptr, max, entry->nodeid, sizeof(entry->nodeid)); entry->last_timestamp = fromwire_u64(pptr, max); if (entry->last_timestamp < 0) { @@ -43,7 +43,7 @@ struct gossip_getnodes_entry *fromwire_gossip_getnodes_entry(const tal_t *ctx, void towire_gossip_getnodes_entry(u8 **pptr, const struct gossip_getnodes_entry *entry) { - towire_pubkey(pptr, &entry->nodeid); + towire(pptr, entry->nodeid, sizeof(entry->nodeid)); towire_u64(pptr, entry->last_timestamp); if (entry->last_timestamp < 0) @@ -97,8 +97,8 @@ void fromwire_gossip_getchannels_entry(const u8 **pptr, size_t *max, struct gossip_getchannels_entry *entry) { fromwire_short_channel_id(pptr, max, &entry->short_channel_id); - fromwire_pubkey(pptr, max, &entry->source); - fromwire_pubkey(pptr, max, &entry->destination); + fromwire(pptr, max, entry->source, sizeof(entry->source)); + fromwire(pptr, max, entry->destination, sizeof(entry->destination)); entry->satoshis = fromwire_u64(pptr, max); entry->message_flags = fromwire_u8(pptr, max); entry->channel_flags = fromwire_u8(pptr, max); @@ -114,8 +114,8 @@ void towire_gossip_getchannels_entry(u8 **pptr, const struct gossip_getchannels_entry *entry) { towire_short_channel_id(pptr, &entry->short_channel_id); - towire_pubkey(pptr, &entry->source); - towire_pubkey(pptr, &entry->destination); + towire(pptr, entry->source, sizeof(entry->source)); + towire(pptr, entry->destination, sizeof(entry->destination)); towire_u64(pptr, entry->satoshis); towire_u8(pptr, entry->message_flags); towire_u8(pptr, entry->channel_flags); diff --git a/lightningd/gossip_msg.h b/lightningd/gossip_msg.h index ad8f2c391..a9de59961 100644 --- a/lightningd/gossip_msg.h +++ b/lightningd/gossip_msg.h @@ -12,7 +12,8 @@ struct peer_features { }; struct gossip_getnodes_entry { - struct pubkey nodeid; + /* This is raw to optimize marshaling: be careful! */ + u8 nodeid[sizeof(struct pubkey)]; s64 last_timestamp; /* -1 means never: following fields ignored */ u8 *globalfeatures; struct wireaddr *addresses; @@ -21,7 +22,8 @@ struct gossip_getnodes_entry { }; struct gossip_getchannels_entry { - struct pubkey source, destination; + /* These are raw to optimize marshaling: be careful! */ + u8 source[sizeof(struct pubkey)], destination[sizeof(struct pubkey)]; u64 satoshis; struct short_channel_id short_channel_id; u8 message_flags;