From b4455d517c0e3020fd78d68bdbf1d083b3930cd4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 8 Apr 2019 16:04:06 +0930 Subject: [PATCH] common/node_id: new type. Node ids are pubkeys, but we only use them as pubkeys for routing and checking gossip messages. So we're packing and unpacking them constantly, and wasting some space and time. This introduces a new type, explicitly the SEC1 compressed encoding (33 bytes). We ensure its validity when we load from the db, or get it from JSON. We still use 'struct pubkey' for peer messages, which checks validity. Results from 5 runs, min-max(mean +/- stddev): store_load_msec,vsz_kb,store_rewrite_sec,listnodes_sec,listchannels_sec,routing_sec,peer_write_all_sec 39475-39572(39518+/-36),2880732,41.150000-41.390000(41.298+/-0.085),2.260000-2.550000(2.336+/-0.11),44.390000-65.150000(58.648+/-7.5),32.740000-33.020000(32.89+/-0.093),44.130000-45.090000(44.566+/-0.32) Signed-off-by: Rusty Russell --- common/Makefile | 1 + common/json_helpers.c | 8 +++++ common/json_helpers.h | 5 +++ common/node_id.c | 50 +++++++++++++++++++++++++++ common/node_id.h | 43 +++++++++++++++++++++++ common/test/run-json.c | 3 ++ common/type_to_string.h | 1 + devtools/print_wire.c | 1 + devtools/print_wire.h | 1 + lightningd/Makefile | 1 + lightningd/json.c | 25 ++++++++++++++ lightningd/json.h | 13 +++++++ lightningd/test/run-jsonrpc.c | 4 +++ plugins/Makefile | 1 + wallet/db.c | 65 +++++++++++++++++++++++++++++++++++ wallet/db.h | 9 +++++ wallet/test/run-db.c | 3 ++ wallet/test/run-wallet.c | 3 ++ wire/fromwire.c | 6 ++++ wire/towire.c | 6 ++++ wire/wire.h | 3 ++ 21 files changed, 252 insertions(+) create mode 100644 common/node_id.c create mode 100644 common/node_id.h diff --git a/common/Makefile b/common/Makefile index cb4b4133c..5bb8d4658 100644 --- a/common/Makefile +++ b/common/Makefile @@ -33,6 +33,7 @@ COMMON_SRC_NOGEN := \ common/keyset.c \ common/memleak.c \ common/msg_queue.c \ + common/node_id.c \ common/param.c \ common/peer_billboard.c \ common/peer_failed.c \ diff --git a/common/json_helpers.c b/common/json_helpers.c index d9c20527c..7d2001788 100644 --- a/common/json_helpers.c +++ b/common/json_helpers.c @@ -2,6 +2,7 @@ #include #include #include +#include #include bool json_to_bitcoin_amount(const char *buffer, const jsmntok_t *tok, @@ -32,6 +33,13 @@ bool json_to_bitcoin_amount(const char *buffer, const jsmntok_t *tok, return true; } +bool json_to_node_id(const char *buffer, const jsmntok_t *tok, + struct node_id *id) +{ + return node_id_from_hexstr(buffer + tok->start, + tok->end - tok->start, id); +} + bool json_to_pubkey(const char *buffer, const jsmntok_t *tok, struct pubkey *pubkey) { diff --git a/common/json_helpers.h b/common/json_helpers.h index 5683bcdde..de64d8668 100644 --- a/common/json_helpers.h +++ b/common/json_helpers.h @@ -7,12 +7,17 @@ struct amount_msat; struct amount_sat; struct pubkey; +struct node_id; struct short_channel_id; /* Extract a pubkey from this */ bool json_to_pubkey(const char *buffer, const jsmntok_t *tok, struct pubkey *pubkey); +/* Extract node_id from this: makes sure *id is valid! */ +bool json_to_node_id(const char *buffer, const jsmntok_t *tok, + struct node_id *id); + /* Extract satoshis from this (may be a string, or a decimal number literal) */ bool json_to_bitcoin_amount(const char *buffer, const jsmntok_t *tok, uint64_t *satoshi); diff --git a/common/node_id.c b/common/node_id.c new file mode 100644 index 000000000..4633c40ef --- /dev/null +++ b/common/node_id.c @@ -0,0 +1,50 @@ +#include +#include +#include +#include +#include +#include + +/* Convert from pubkey to compressed pubkey. */ +void node_id_from_pubkey(struct node_id *id, const struct pubkey *key) +{ + size_t outlen = ARRAY_SIZE(id->k); + if (!secp256k1_ec_pubkey_serialize(secp256k1_ctx, id->k, &outlen, + &key->pubkey, + SECP256K1_EC_COMPRESSED)) + abort(); +} + +WARN_UNUSED_RESULT +bool pubkey_from_node_id(struct pubkey *key, const struct node_id *id) +{ + return secp256k1_ec_pubkey_parse(secp256k1_ctx, &key->pubkey, + memcheck(id->k, sizeof(id->k)), + sizeof(id->k)); +} + +/* It's valid if we can convert to a real pubkey. */ +bool node_id_valid(const struct node_id *id) +{ + struct pubkey key; + return pubkey_from_node_id(&key, id); +} + +/* Convert to hex string of SEC1 encoding */ +char *node_id_to_hexstr(const tal_t *ctx, const struct node_id *id) +{ + return tal_hexstr(ctx, id->k, sizeof(id->k)); +} +REGISTER_TYPE_TO_STRING(node_id, node_id_to_hexstr); + +/* Convert from hex string of SEC1 encoding */ +bool node_id_from_hexstr(const char *str, size_t slen, struct node_id *id) +{ + return hex_decode(str, slen, id->k, sizeof(id->k)) + && node_id_valid(id); +} + +int node_id_cmp(const struct node_id *a, const struct node_id *b) +{ + return memcmp(a->k, b->k, sizeof(a->k)); +} diff --git a/common/node_id.h b/common/node_id.h new file mode 100644 index 000000000..15144927c --- /dev/null +++ b/common/node_id.h @@ -0,0 +1,43 @@ +/* Encapsulation for pubkeys used as node ids: more compact, more dangerous. */ +#ifndef LIGHTNING_COMMON_NODE_ID_H +#define LIGHTNING_COMMON_NODE_ID_H +#include "config.h" +#include + +struct node_id { + u8 k[PUBKEY_CMPR_LEN]; +}; + +static inline bool node_id_eq(const struct node_id *a, + const struct node_id *b) +{ + return memcmp(a->k, b->k, sizeof(a->k)) == 0; +} + +/* Is this actually a valid pubkey? Relatively expensive. */ +bool node_id_valid(const struct node_id *id); + +/* Convert from pubkey to compressed pubkey. */ +void node_id_from_pubkey(struct node_id *id, const struct pubkey *key); + +/* Returns false if not a valid pubkey: relatively expensive */ +WARN_UNUSED_RESULT +bool pubkey_from_node_id(struct pubkey *key, const struct node_id *id); + +/* Convert to hex string of SEC1 encoding. */ +char *node_id_to_hexstr(const tal_t *ctx, const struct node_id *id); + +/* Convert from hex string of SEC1 encoding: checks validity! */ +bool node_id_from_hexstr(const char *str, size_t slen, struct node_id *id); + +/* Compare the keys `a` and `b`. Return <0 if `a`<`b`, 0 if equal and >0 otherwise */ +int node_id_cmp(const struct node_id *a, const struct node_id *b); + +/* If the two nodes[] are id1 and id2, which index would id1 be? */ +static inline int node_id_idx(const struct node_id *id1, + const struct node_id *id2) +{ + return node_id_cmp(id1, id2) > 0; +} + +#endif /* LIGHTNING_COMMON_NODE_ID_H */ diff --git a/common/test/run-json.c b/common/test/run-json.c index 42a1e79d4..7829c6541 100644 --- a/common/test/run-json.c +++ b/common/test/run-json.c @@ -5,6 +5,9 @@ #include /* AUTOGENERATED MOCKS START */ +/* Generated stub for node_id_from_hexstr */ +bool node_id_from_hexstr(const char *str UNNEEDED, size_t slen UNNEEDED, struct node_id *id UNNEEDED) +{ fprintf(stderr, "node_id_from_hexstr called!\n"); abort(); } /* Generated stub for parse_amount_msat */ bool parse_amount_msat(struct amount_msat *msat UNNEEDED, const char *s UNNEEDED, size_t slen UNNEEDED) { fprintf(stderr, "parse_amount_msat called!\n"); abort(); } diff --git a/common/type_to_string.h b/common/type_to_string.h index 7c749f54c..e35d87b38 100644 --- a/common/type_to_string.h +++ b/common/type_to_string.h @@ -8,6 +8,7 @@ /* This must match the type_to_string_ cases. */ union printable_types { const struct pubkey *pubkey; + const struct node_id *node_id; const struct bitcoin_txid *bitcoin_txid; const struct bitcoin_blkid *bitcoin_blkid; const struct sha256 *sha256; diff --git a/devtools/print_wire.c b/devtools/print_wire.c index 9ec377b50..3587a2a78 100644 --- a/devtools/print_wire.c +++ b/devtools/print_wire.c @@ -175,6 +175,7 @@ void printwire_u8_array(const char *fieldname, const u8 **cursor, size_t *plen, PRINTWIRE_STRUCT_TYPE_TO_STRING(bitcoin_blkid); PRINTWIRE_STRUCT_TYPE_TO_STRING(bitcoin_txid); PRINTWIRE_STRUCT_TYPE_TO_STRING(channel_id); +PRINTWIRE_STRUCT_TYPE_TO_STRING(node_id); PRINTWIRE_STRUCT_TYPE_TO_STRING(preimage); PRINTWIRE_STRUCT_TYPE_TO_STRING(pubkey); PRINTWIRE_STRUCT_TYPE_TO_STRING(sha256); diff --git a/devtools/print_wire.h b/devtools/print_wire.h index ffa386c52..25342e492 100644 --- a/devtools/print_wire.h +++ b/devtools/print_wire.h @@ -18,6 +18,7 @@ void printwire_amount_sat(const char *fieldname, const struct amount_sat *sat); void printwire_amount_msat(const char *fieldname, const struct amount_msat *msat); void printwire_preimage(const char *fieldname, const struct preimage *preimage); void printwire_pubkey(const char *fieldname, const struct pubkey *pubkey); +void printwire_node_id(const char *fieldname, const struct node_id *id); void printwire_secp256k1_ecdsa_signature(const char *fieldname, const secp256k1_ecdsa_signature *); void printwire_sha256(const char *fieldname, const struct sha256 *sha256); void printwire_secret(const char *fieldname, const struct secret *secret); diff --git a/lightningd/Makefile b/lightningd/Makefile index 16b9bed6a..be8e43e9e 100644 --- a/lightningd/Makefile +++ b/lightningd/Makefile @@ -40,6 +40,7 @@ LIGHTNINGD_COMMON_OBJS := \ common/json_tok.o \ common/memleak.o \ common/msg_queue.o \ + common/node_id.o \ common/param.o \ common/permute_tx.o \ common/pseudorand.o \ diff --git a/lightningd/json.c b/lightningd/json.c index 34f71e24e..a701d8804 100644 --- a/lightningd/json.c +++ b/lightningd/json.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -50,6 +51,13 @@ json_add_route(struct json_stream *r, char const *n, json_array_end(r); } +void json_add_node_id(struct json_stream *response, + const char *fieldname, + const struct node_id *id) +{ + json_add_hex(response, fieldname, id->k, sizeof(id->k)); +} + void json_add_pubkey(struct json_stream *response, const char *fieldname, const struct pubkey *key) @@ -69,6 +77,23 @@ void json_add_txid(struct json_stream *result, const char *fieldname, json_add_string(result, fieldname, hex); } +struct command_result *param_node_id(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct node_id **id) +{ + *id = tal(cmd, struct node_id); + if (json_to_node_id(buffer, tok, *id)) + return NULL; + + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "'%s' should be a pubkey, not '%.*s'", + name, json_tok_full_len(tok), + json_tok_full(buffer, tok)); +} + + struct command_result *param_pubkey(struct command *cmd, const char *name, const char *buffer, const jsmntok_t *tok, struct pubkey **pubkey) diff --git a/lightningd/json.h b/lightningd/json.h index 69f54083c..80338fc91 100644 --- a/lightningd/json.h +++ b/lightningd/json.h @@ -22,6 +22,7 @@ struct command; struct json_escaped; struct json_stream; struct pubkey; +struct node_id; struct route_hop; struct sha256; struct short_channel_id; @@ -39,6 +40,11 @@ void json_add_pubkey(struct json_stream *response, const char *fieldname, const struct pubkey *key); +/* '"fieldname" : "0289abcdef..."' or "0289abcdef..." if fieldname is NULL */ +void json_add_node_id(struct json_stream *response, + const char *fieldname, + const struct node_id *id); + /* '"fieldname" : ' or "" if fieldname is NULL */ void json_add_txid(struct json_stream *result, const char *fieldname, const struct bitcoin_txid *txid); @@ -47,6 +53,13 @@ struct command_result *param_pubkey(struct command *cmd, const char *name, const char *buffer, const jsmntok_t *tok, struct pubkey **pubkey); +/* Makes sure *id is valid. */ +struct command_result *param_node_id(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct node_id **id); + struct command_result *param_short_channel_id(struct command *cmd, const char *name, const char *buffer, diff --git a/lightningd/test/run-jsonrpc.c b/lightningd/test/run-jsonrpc.c index 6ebe3e8e4..90778ae74 100644 --- a/lightningd/test/run-jsonrpc.c +++ b/lightningd/test/run-jsonrpc.c @@ -21,6 +21,10 @@ const char *feerate_name(enum feerate feerate UNNEEDED) /* Generated stub for fmt_wireaddr_without_port */ char *fmt_wireaddr_without_port(const tal_t *ctx UNNEEDED, const struct wireaddr *a UNNEEDED) { fprintf(stderr, "fmt_wireaddr_without_port called!\n"); abort(); } +/* Generated stub for json_to_node_id */ +bool json_to_node_id(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, + struct node_id *id UNNEEDED) +{ fprintf(stderr, "json_to_node_id called!\n"); abort(); } /* Generated stub for json_to_pubkey */ bool json_to_pubkey(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct pubkey *pubkey UNNEEDED) diff --git a/plugins/Makefile b/plugins/Makefile index f5a248d5a..7855017d8 100644 --- a/plugins/Makefile +++ b/plugins/Makefile @@ -26,6 +26,7 @@ PLUGIN_COMMON_OBJS := \ common/json_helpers.o \ common/json_tok.o \ common/memleak.o \ + common/node_id.o \ common/param.o \ common/pseudorand.o \ common/type_to_string.o \ diff --git a/wallet/db.c b/wallet/db.c index 98cae3b06..768878b63 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -951,6 +952,20 @@ bool sqlite3_bind_pubkey(sqlite3_stmt *stmt, int col, const struct pubkey *pk) return err == SQLITE_OK; } +bool sqlite3_column_node_id(sqlite3_stmt *stmt, int col, struct node_id *dest) +{ + assert(sqlite3_column_bytes(stmt, col) == sizeof(dest->k)); + memcpy(dest->k, sqlite3_column_blob(stmt, col), sizeof(dest->k)); + return node_id_valid(dest); +} + +bool sqlite3_bind_node_id(sqlite3_stmt *stmt, int col, const struct node_id *id) +{ + assert(node_id_valid(id)); + int err = sqlite3_bind_blob(stmt, col, id->k, sizeof(id->k), SQLITE_TRANSIENT); + return err == SQLITE_OK; +} + bool sqlite3_bind_pubkey_array(sqlite3_stmt *stmt, int col, const struct pubkey *pks) { @@ -997,6 +1012,56 @@ struct pubkey *sqlite3_column_pubkey_array(const tal_t *ctx, return ret; } +bool sqlite3_bind_node_id_array(sqlite3_stmt *stmt, int col, + const struct node_id *ids) +{ + size_t n; + u8 *arr; + + if (!ids) { + int err = sqlite3_bind_null(stmt, col); + return err == SQLITE_OK; + } + + /* Copy into contiguous array: ARM will add padding to struct node_id! */ + n = tal_count(ids); + arr = tal_arr(NULL, u8, n * sizeof(ids[0].k)); + for (size_t i = 0; i < n; ++i) { + assert(node_id_valid(&ids[i])); + memcpy(arr + sizeof(ids[i].k) * i, + ids[i].k, + sizeof(ids[i].k)); + } + int err = sqlite3_bind_blob(stmt, col, arr, tal_count(arr), SQLITE_TRANSIENT); + + tal_free(arr); + return err == SQLITE_OK; +} + +struct node_id *sqlite3_column_node_id_array(const tal_t *ctx, + sqlite3_stmt *stmt, int col) +{ + size_t n; + struct node_id *ret; + const u8 *arr; + + if (sqlite3_column_type(stmt, col) == SQLITE_NULL) + return NULL; + + n = sqlite3_column_bytes(stmt, col) / sizeof(ret->k); + assert(n * sizeof(ret->k) == (size_t)sqlite3_column_bytes(stmt, col)); + ret = tal_arr(ctx, struct node_id, n); + arr = sqlite3_column_blob(stmt, col); + + for (size_t i = 0; i < n; i++) { + memcpy(ret[i].k, arr + i * sizeof(ret[i].k), sizeof(ret[i].k)); + if (!node_id_valid(&ret[i])) + return tal_free(ret); + } + + return ret; +} + bool sqlite3_column_preimage(sqlite3_stmt *stmt, int col, struct preimage *dest) { assert(sqlite3_column_bytes(stmt, col) == sizeof(struct preimage)); diff --git a/wallet/db.h b/wallet/db.h index 78907884c..aa3567f18 100644 --- a/wallet/db.h +++ b/wallet/db.h @@ -15,6 +15,7 @@ struct lightningd; struct log; +struct node_id; struct db { char *filename; @@ -176,11 +177,19 @@ bool sqlite3_column_signature(sqlite3_stmt *stmt, int col, secp256k1_ecdsa_signa bool sqlite3_column_pubkey(sqlite3_stmt *stmt, int col, struct pubkey *dest); bool sqlite3_bind_pubkey(sqlite3_stmt *stmt, int col, const struct pubkey *pk); +bool sqlite3_column_node_id(sqlite3_stmt *stmt, int col, struct node_id *dest); +bool sqlite3_bind_node_id(sqlite3_stmt *stmt, int col, const struct node_id *id); + bool sqlite3_bind_pubkey_array(sqlite3_stmt *stmt, int col, const struct pubkey *pks); struct pubkey *sqlite3_column_pubkey_array(const tal_t *ctx, sqlite3_stmt *stmt, int col); +bool sqlite3_bind_node_id_array(sqlite3_stmt *stmt, int col, + const struct node_id *ids); +struct node_id *sqlite3_column_node_id_array(const tal_t *ctx, + sqlite3_stmt *stmt, int col); + bool sqlite3_column_preimage(sqlite3_stmt *stmt, int col, struct preimage *dest); bool sqlite3_bind_preimage(sqlite3_stmt *stmt, int col, const struct preimage *p); diff --git a/wallet/test/run-db.c b/wallet/test/run-db.c index 25cfbf393..e7feec566 100644 --- a/wallet/test/run-db.c +++ b/wallet/test/run-db.c @@ -22,6 +22,9 @@ static void db_log_(struct log *log UNUSED, enum log_level level UNUSED, const c struct json_escaped *json_escaped_string_(const tal_t *ctx UNNEEDED, const void *bytes UNNEEDED, size_t len UNNEEDED) { fprintf(stderr, "json_escaped_string_ called!\n"); abort(); } +/* Generated stub for node_id_valid */ +bool node_id_valid(const struct node_id *id UNNEEDED) +{ fprintf(stderr, "node_id_valid called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ static char *db_err; diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index c3765fa4f..bce3b3151 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -319,6 +319,9 @@ void log_add(struct log *log UNNEEDED, const char *fmt UNNEEDED, ...) void log_io(struct log *log UNNEEDED, enum log_level dir UNNEEDED, const char *comment UNNEEDED, const void *data UNNEEDED, size_t len UNNEEDED) { fprintf(stderr, "log_io called!\n"); abort(); } +/* Generated stub for node_id_valid */ +bool node_id_valid(const struct node_id *id UNNEEDED) +{ fprintf(stderr, "node_id_valid called!\n"); abort(); } /* Generated stub for notify_connect */ void notify_connect(struct lightningd *ld UNNEEDED, struct pubkey *nodeid UNNEEDED, struct wireaddr_internal *addr UNNEEDED) diff --git a/wire/fromwire.c b/wire/fromwire.c index 3c993cb8d..a872ac8b3 100644 --- a/wire/fromwire.c +++ b/wire/fromwire.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -128,6 +129,11 @@ void fromwire_pubkey(const u8 **cursor, size_t *max, struct pubkey *pubkey) fromwire_fail(cursor, max); } +void fromwire_node_id(const u8 **cursor, size_t *max, struct node_id *id) +{ + fromwire(cursor, max, &id->k, sizeof(id->k)); +} + void fromwire_secret(const u8 **cursor, size_t *max, struct secret *secret) { fromwire(cursor, max, secret->data, sizeof(secret->data)); diff --git a/wire/towire.c b/wire/towire.c index f6d21e949..03ae563cf 100644 --- a/wire/towire.c +++ b/wire/towire.c @@ -10,6 +10,7 @@ #include #include #include +#include #include void towire(u8 **pptr, const void *data, size_t len) @@ -82,6 +83,11 @@ void towire_pubkey(u8 **pptr, const struct pubkey *pubkey) towire(pptr, output, outputlen); } +void towire_node_id(u8 **pptr, const struct node_id *id) +{ + towire(pptr, id->k, sizeof(id->k)); +} + void towire_secret(u8 **pptr, const struct secret *secret) { towire(pptr, secret->data, sizeof(secret->data)); diff --git a/wire/wire.h b/wire/wire.h index 69a8a209d..986732dc1 100644 --- a/wire/wire.h +++ b/wire/wire.h @@ -24,6 +24,7 @@ STRUCTEQ_DEF(channel_id, 0, id); struct bitcoin_blkid; struct bitcoin_signature; struct bitcoin_txid; +struct node_id; struct preimage; struct ripemd160; struct siphash_seed; @@ -40,6 +41,7 @@ const void *fromwire_fail(const u8 **cursor, size_t *max); void towire(u8 **pptr, const void *data, size_t len); void towire_pubkey(u8 **pptr, const struct pubkey *pubkey); +void towire_node_id(u8 **pptr, const struct node_id *id); void towire_privkey(u8 **pptr, const struct privkey *privkey); void towire_secret(u8 **pptr, const struct secret *secret); void towire_secp256k1_ecdsa_signature(u8 **pptr, @@ -88,6 +90,7 @@ u64 fromwire_var_int(const u8 **cursor, size_t *max); void fromwire_secret(const u8 **cursor, size_t *max, struct secret *secret); void fromwire_privkey(const u8 **cursor, size_t *max, struct privkey *privkey); void fromwire_pubkey(const u8 **cursor, size_t *max, struct pubkey *pubkey); +void fromwire_node_id(const u8 **cursor, size_t *max, struct node_id *id); void fromwire_secp256k1_ecdsa_signature(const u8 **cursor, size_t *max, secp256k1_ecdsa_signature *signature); void fromwire_secp256k1_ecdsa_recoverable_signature(const u8 **cursor,