From 319ea22b80d52af585518975e38d1ad00eaab366 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 18 May 2022 10:20:16 +0930 Subject: [PATCH 1/5] gossipd: don't ever use zlib compression on gossip. This was eliminated this morning in the latest spec. We still accept them, we just don't produce them any more. Changelog-Removed: Protocol: We no longer create gossip messages which use zlib encoding (we still understand them, for now!) Signed-off-by: Rusty Russell --- gossipd/queries.c | 62 +---------- gossipd/test/run-extended-info.c | 175 +++++-------------------------- tests/test_gossip.py | 4 +- 3 files changed, 30 insertions(+), 211 deletions(-) diff --git a/gossipd/queries.c b/gossipd/queries.c index de7d02fd6fed..9cb8241b058c 100644 --- a/gossipd/queries.c +++ b/gossipd/queries.c @@ -52,52 +52,6 @@ static void encoding_add_query_flag(u8 **encoded, bigsize_t flag) towire_bigsize(encoded, flag); } -/* Greg Maxwell asked me privately about using zlib for communicating a set, - * and suggested that we'd be better off using Golomb-Rice coding a-la BIP - * 158. However, naively using Rice encoding isn't a win: we have to get - * more complex and use separate streams. The upside is that it's between - * 2 and 5 times smaller (assuming optimal Rice encoding + gzip). We can add - * that later. */ -static u8 *zencode(const tal_t *ctx, const u8 *scids, size_t len) -{ - u8 *z; - int err; - unsigned long compressed_len = len; - -#ifdef ZLIB_EVEN_IF_EXPANDS - /* Needed for test vectors */ - compressed_len = 128 * 1024; -#endif - /* Prefer to fail if zlib makes it larger */ - z = tal_arr(ctx, u8, compressed_len); - err = compress2(z, &compressed_len, scids, len, Z_DEFAULT_COMPRESSION); - if (err == Z_OK) { - tal_resize(&z, compressed_len); - return z; - } - return NULL; -} - -/* Try compressing *encoded: fails if result would be longer. - * @off is offset to place result in *encoded. - */ -static bool encoding_end_zlib(u8 **encoded, size_t off) -{ - u8 *z; - size_t len = tal_count(*encoded); - - z = zencode(tmpctx, *encoded, len); - if (!z) - return false; - - /* Successful: copy over and trim */ - tal_resize(encoded, off + tal_count(z)); - memcpy(*encoded + off, z, tal_count(z)); - - tal_free(z); - return true; -} - static void encoding_end_no_compress(u8 **encoded, size_t off) { size_t len = tal_count(*encoded); @@ -110,12 +64,8 @@ static void encoding_end_no_compress(u8 **encoded, size_t off) * Prepends encoding type to @encoding. */ static bool encoding_end_prepend_type(u8 **encoded, size_t max_bytes) { - if (encoding_end_zlib(encoded, 1)) - **encoded = ARR_ZLIB; - else { - encoding_end_no_compress(encoded, 1); - **encoded = ARR_UNCOMPRESSED; - } + encoding_end_no_compress(encoded, 1); + **encoded = ARR_UNCOMPRESSED; #if DEVELOPER if (tal_count(*encoded) > dev_max_encoding_bytes) @@ -127,12 +77,8 @@ static bool encoding_end_prepend_type(u8 **encoded, size_t max_bytes) /* Try compressing, leaving type external */ static bool encoding_end_external_type(u8 **encoded, u8 *type, size_t max_bytes) { - if (encoding_end_zlib(encoded, 0)) - *type = ARR_ZLIB; - else { - encoding_end_no_compress(encoded, 0); - *type = ARR_UNCOMPRESSED; - } + encoding_end_no_compress(encoded, 0); + *type = ARR_UNCOMPRESSED; return tal_count(*encoded) <= max_bytes; } diff --git a/gossipd/test/run-extended-info.c b/gossipd/test/run-extended-info.c index 83debe2643c2..845525a26f67 100644 --- a/gossipd/test/run-extended-info.c +++ b/gossipd/test/run-extended-info.c @@ -1,7 +1,5 @@ #include "config.h" -#define ZLIB_EVEN_IF_EXPANDS 1 - #include "../queries.c" #include #include @@ -112,7 +110,8 @@ void status_fmt(enum log_level level UNNEEDED, { } -static const char *test_vectors[] = { +/* These we can reproduce */ +static const char *test_vectors_nozlib[] = { "{\n" " \"msg\" : {\n" " \"type\" : \"QueryChannelRange\",\n" @@ -137,34 +136,6 @@ static const char *test_vectors[] = { " \"msg\" : {\n" " \"type\" : \"ReplyChannelRange\",\n" " \"chainHash\" : \"0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206\",\n" - " \"firstBlockNum\" : 756230,\n" - " \"numberOfBlocks\" : 1500,\n" - " \"complete\" : 1,\n" - " \"shortChannelIds\" : {\n" - " \"encoding\" : \"UNCOMPRESSED\",\n" - " \"array\" : [ \"0x0x142\", \"0x0x15465\", \"0x69x42692\" ]\n" - " }\n" - " },\n" - " \"hex\" : \"01080f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206000b8a06000005dc01001900000000000000008e0000000000003c69000000000045a6c4\"\n" - "}\n", - "{\n" - " \"msg\" : {\n" - " \"type\" : \"ReplyChannelRange\",\n" - " \"chainHash\" : \"0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206\",\n" - " \"firstBlockNum\" : 1600,\n" - " \"numberOfBlocks\" : 110,\n" - " \"complete\" : 1,\n" - " \"shortChannelIds\" : {\n" - " \"encoding\" : \"COMPRESSED_ZLIB\",\n" - " \"array\" : [ \"0x0x142\", \"0x0x15465\", \"0x4x3318\" ]\n" - " }\n" - " },\n" - " \"hex\" : \"01080f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206000006400000006e01001601789c636000833e08659309a65878be010010a9023a\"\n" - "}\n", - "{\n" - " \"msg\" : {\n" - " \"type\" : \"ReplyChannelRange\",\n" - " \"chainHash\" : \"0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206\",\n" " \"firstBlockNum\" : 122334,\n" " \"numberOfBlocks\" : 1500,\n" " \"complete\" : 1,\n" @@ -202,45 +173,6 @@ static const char *test_vectors[] = { "}\n", "{\n" " \"msg\" : {\n" - " \"type\" : \"ReplyChannelRange\",\n" - " \"chainHash\" : \"0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206\",\n" - " \"firstBlockNum\" : 122334,\n" - " \"numberOfBlocks\" : 1500,\n" - " \"complete\" : 1,\n" - " \"shortChannelIds\" : {\n" - " \"encoding\" : \"COMPRESSED_ZLIB\",\n" - " \"array\" : [ \"0x0x12355\", \"0x7x30934\", \"0x70x57793\" ]\n" - " },\n" - " \"timestamps\" : {\n" - " \"encoding\" : \"COMPRESSED_ZLIB\",\n" - " \"timestamps\" : [ {\n" - " \"timestamp1\" : 164545,\n" - " \"timestamp2\" : 948165\n" - " }, {\n" - " \"timestamp1\" : 489645,\n" - " \"timestamp2\" : 4786864\n" - " }, {\n" - " \"timestamp1\" : 46456,\n" - " \"timestamp2\" : 9788415\n" - " } ]\n" - " },\n" - " \"checksums\" : {\n" - " \"checksums\" : [ {\n" - " \"checksum1\" : 1111,\n" - " \"checksum2\" : 2222\n" - " }, {\n" - " \"checksum1\" : 3333,\n" - " \"checksum2\" : 4444\n" - " }, {\n" - " \"checksum1\" : 5555,\n" - " \"checksum2\" : 6666\n" - " } ]\n" - " }\n" - " },\n" - " \"hex\" : \"01080f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e22060001ddde000005dc01001801789c63600001036730c55e710d4cbb3d3c080017c303b1012201789c63606a3ac8c0577e9481bd622d8327d7060686ad150c53a3ff0300554707db031800000457000008ae00000d050000115c000015b300001a0a\"\n" - "}\n", - "{\n" - " \"msg\" : {\n" " \"type\" : \"QueryShortChannelIds\",\n" " \"chainHash\" : \"0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206\",\n" " \"shortChannelIds\" : {\n" @@ -251,48 +183,6 @@ static const char *test_vectors[] = { " },\n" " \"hex\" : \"01050f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206001900000000000000008e0000000000003c69000000000045a6c4\"\n" "}\n", - "{\n" - " \"msg\" : {\n" - " \"type\" : \"QueryShortChannelIds\",\n" - " \"chainHash\" : \"0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206\",\n" - " \"shortChannelIds\" : {\n" - " \"encoding\" : \"COMPRESSED_ZLIB\",\n" - " \"array\" : [ \"0x0x4564\", \"0x2x47550\", \"0x69x42692\" ]\n" - " },\n" - " \"extensions\" : [ ]\n" - " },\n" - " \"hex\" : \"01050f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206001801789c63600001c12b608a69e73e30edbaec0800203b040e\"\n" - "}\n", - "{\n" - " \"msg\" : {\n" - " \"type\" : \"QueryShortChannelIds\",\n" - " \"chainHash\" : \"0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206\",\n" - " \"shortChannelIds\" : {\n" - " \"encoding\" : \"UNCOMPRESSED\",\n" - " \"array\" : [ \"0x0x12232\", \"0x0x15556\", \"0x69x42692\" ]\n" - " },\n" - " \"extensions\" : [ {\n" - " \"encoding\" : \"COMPRESSED_ZLIB\",\n" - " \"array\" : [ 1, 2, 4 ]\n" - " } ]\n" - " },\n" - " \"hex\" : \"01050f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e22060019000000000000002fc80000000000003cc4000000000045a6c4010c01789c6364620100000e0008\"\n" - "}\n", - "{\n" - " \"msg\" : {\n" - " \"type\" : \"QueryShortChannelIds\",\n" - " \"chainHash\" : \"0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206\",\n" - " \"shortChannelIds\" : {\n" - " \"encoding\" : \"COMPRESSED_ZLIB\",\n" - " \"array\" : [ \"0x0x14200\", \"0x0x46645\", \"0x69x42692\" ]\n" - " },\n" - " \"extensions\" : [ {\n" - " \"encoding\" : \"COMPRESSED_ZLIB\",\n" - " \"array\" : [ 1, 2, 4 ]\n" - " } ]\n" - " },\n" - " \"hex\" : \"01050f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206001801789c63600001f30a30c5b0cd144cb92e3b020017c6034a010c01789c6364620100000e0008\"\n" - "}\n", }; static void get_chainhash(const char *test_vector, @@ -323,14 +213,9 @@ static u8 *get_scid_array(const tal_t *ctx, assert(json_to_short_channel_id(test_vector, t, &scid)); encoding_add_short_channel_id(&encoded, &scid); } - if (json_tok_streq(test_vector, encoding, "UNCOMPRESSED")) { - encoding_end_no_compress(&encoded, 1); - encoded[0] = ARR_UNCOMPRESSED; - } else { - assert(json_tok_streq(test_vector, encoding, "COMPRESSED_ZLIB")); - assert(encoding_end_zlib(&encoded, 1)); - encoded[0] = ARR_ZLIB; - } + assert(json_tok_streq(test_vector, encoding, "UNCOMPRESSED")); + encoding_end_no_compress(&encoded, 1); + encoded[0] = ARR_UNCOMPRESSED; return encoded; } @@ -404,17 +289,10 @@ static u8 *test_reply_channel_range(const char *test_vector, const jsmntok_t *ob &ts.timestamp_node_id_2)); encoding_add_timestamps(&tlvs->timestamps_tlv->encoded_timestamps, &ts); } - if (json_tok_streq(test_vector, encodingtok, "UNCOMPRESSED")) { - encoding_end_no_compress(&tlvs->timestamps_tlv->encoded_timestamps, - 0); - tlvs->timestamps_tlv->encoding_type = ARR_UNCOMPRESSED; - } else { - assert(json_tok_streq(test_vector, encodingtok, - "COMPRESSED_ZLIB")); - assert(encoding_end_zlib(&tlvs->timestamps_tlv->encoded_timestamps, - 0)); - tlvs->timestamps_tlv->encoding_type = ARR_ZLIB; - } + assert(json_tok_streq(test_vector, encodingtok, "UNCOMPRESSED")); + encoding_end_no_compress(&tlvs->timestamps_tlv->encoded_timestamps, + 0); + tlvs->timestamps_tlv->encoding_type = ARR_UNCOMPRESSED; } opt = json_get_member(test_vector, obj, "checksums"); @@ -464,14 +342,9 @@ get_query_flags_array(const tal_t *ctx, assert(json_to_u64(test_vector, t, &f)); encoding_add_query_flag(&tlv->encoded_query_flags, f); } - if (json_tok_streq(test_vector, encoding, "UNCOMPRESSED")) { - encoding_end_no_compress(&tlv->encoded_query_flags, 0); - tlv->encoding_type = ARR_UNCOMPRESSED; - } else { - assert(json_tok_streq(test_vector, encoding, "COMPRESSED_ZLIB")); - assert(encoding_end_zlib(&tlv->encoded_query_flags, 0)); - tlv->encoding_type = ARR_ZLIB; - } + assert(json_tok_streq(test_vector, encoding, "UNCOMPRESSED")); + encoding_end_no_compress(&tlv->encoded_query_flags, 0); + tlv->encoding_type = ARR_UNCOMPRESSED; return tlv; } @@ -509,7 +382,7 @@ int main(int argc, char *argv[]) common_setup(argv[0]); - for (size_t i = 0; i < ARRAY_SIZE(test_vectors); i++) { + for (size_t i = 0; i < ARRAY_SIZE(test_vectors_nozlib); i++) { jsmn_parser parser; u8 *m; const char *hex_m; @@ -517,22 +390,22 @@ int main(int argc, char *argv[]) jsmn_init(&parser); assert(jsmn_parse(&parser, - test_vectors[i], strlen(test_vectors[i]), + test_vectors_nozlib[i], strlen(test_vectors_nozlib[i]), toks, tal_count(toks)) > 0); - msg = json_get_member(test_vectors[i], toks, "msg"); - hex = json_get_member(test_vectors[i], toks, "hex"); - type = json_get_member(test_vectors[i], msg, "type"); - if (json_tok_streq(test_vectors[i], type, "QueryChannelRange")) - m = test_query_channel_range(test_vectors[i], msg); - else if (json_tok_streq(test_vectors[i], type, "ReplyChannelRange")) - m = test_reply_channel_range(test_vectors[i], msg); - else if (json_tok_streq(test_vectors[i], type, "QueryShortChannelIds")) - m = test_query_short_channel_ids(test_vectors[i], msg); + msg = json_get_member(test_vectors_nozlib[i], toks, "msg"); + hex = json_get_member(test_vectors_nozlib[i], toks, "hex"); + type = json_get_member(test_vectors_nozlib[i], msg, "type"); + if (json_tok_streq(test_vectors_nozlib[i], type, "QueryChannelRange")) + m = test_query_channel_range(test_vectors_nozlib[i], msg); + else if (json_tok_streq(test_vectors_nozlib[i], type, "ReplyChannelRange")) + m = test_reply_channel_range(test_vectors_nozlib[i], msg); + else if (json_tok_streq(test_vectors_nozlib[i], type, "QueryShortChannelIds")) + m = test_query_short_channel_ids(test_vectors_nozlib[i], msg); else abort(); hex_m = tal_hex(m, m); - assert(json_tok_streq(test_vectors[i], hex, hex_m)); + assert(json_tok_streq(test_vectors_nozlib[i], hex, hex_m)); tal_free(m); } tal_free(toks); diff --git a/tests/test_gossip.py b/tests/test_gossip.py index e5d76d6affac..0b1cd7619bc2 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -872,7 +872,7 @@ def test_gossip_query_channel_range(node_factory, bitcoind, chainparams): filters=['0109', '0012']) assert len(msgs) == 2 - # This should actually be large enough for zlib to kick in! + # This used to be large enough for zlib to kick in, but no longer! scid34, _ = l3.fundchannel(l4, 10**5) mine_funding_to_announce(bitcoind, [l1, l2, l3, l4]) l2.daemon.wait_for_log('Received node_announcement for node ' + l4.info['id']) @@ -886,7 +886,7 @@ def test_gossip_query_channel_range(node_factory, bitcoind, chainparams): genesis_blockhash, 0, 65535, filters=['0109', '0012']) - encoded = subprocess.run(['devtools/mkencoded', '--scids', '01', scid12, scid23, scid34], + encoded = subprocess.run(['devtools/mkencoded', '--scids', '00', scid12, scid23, scid34], check=True, timeout=TIMEOUT, stdout=subprocess.PIPE).stdout.strip().decode() From 43bd21686b48ed96205c40fdf00b433c454212ca Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 18 May 2022 10:21:16 +0930 Subject: [PATCH 2/5] gossipd: neaten code now we don't have to prepend prefix after. We can simply start with a '0' where encoding is prefixed, so simplify internal interface. Signed-off-by: Rusty Russell --- gossipd/queries.c | 62 ++++++++++++-------------------- gossipd/test/run-extended-info.c | 11 ++---- 2 files changed, 26 insertions(+), 47 deletions(-) diff --git a/gossipd/queries.c b/gossipd/queries.c index 9cb8241b058c..a1ec4dcffe17 100644 --- a/gossipd/queries.c +++ b/gossipd/queries.c @@ -27,9 +27,15 @@ static u32 dev_max_encoding_bytes = -1U; * simple compression scheme: the first byte indicates the encoding, the * rest contains the data. */ -static u8 *encoding_start(const tal_t *ctx) +static u8 *encoding_start(const tal_t *ctx, bool prepend_encoding) { - return tal_arr(ctx, u8, 0); + u8 *ret; + if (prepend_encoding) { + ret = tal_arr(ctx, u8, 1); + ret[0] = ARR_UNCOMPRESSED; + } else + ret = tal_arr(ctx, u8, 0); + return ret; } /* Marshal a single short_channel_id */ @@ -52,35 +58,13 @@ static void encoding_add_query_flag(u8 **encoded, bigsize_t flag) towire_bigsize(encoded, flag); } -static void encoding_end_no_compress(u8 **encoded, size_t off) -{ - size_t len = tal_count(*encoded); - - tal_resize(encoded, off + len); - memmove(*encoded + off, *encoded, len); -} - -/* Once we've assembled it, try compressing. - * Prepends encoding type to @encoding. */ -static bool encoding_end_prepend_type(u8 **encoded, size_t max_bytes) +static bool encoding_end(const u8 *encoded, size_t max_bytes) { - encoding_end_no_compress(encoded, 1); - **encoded = ARR_UNCOMPRESSED; - #if DEVELOPER - if (tal_count(*encoded) > dev_max_encoding_bytes) + if (tal_count(encoded) > dev_max_encoding_bytes) return false; #endif - return tal_count(*encoded) <= max_bytes; -} - -/* Try compressing, leaving type external */ -static bool encoding_end_external_type(u8 **encoded, u8 *type, size_t max_bytes) -{ - encoding_end_no_compress(encoded, 0); - *type = ARR_UNCOMPRESSED; - - return tal_count(*encoded) <= max_bytes; + return tal_count(encoded) <= max_bytes; } /* Query this peer for these short-channel-ids. */ @@ -126,7 +110,7 @@ bool query_short_channel_ids(struct daemon *daemon, if (peer->scid_query_outstanding) return false; - encoded = encoding_start(tmpctx); + encoded = encoding_start(tmpctx, true); for (size_t i = 0; i < tal_count(scids); i++) { /* BOLT #7: * @@ -139,7 +123,7 @@ bool query_short_channel_ids(struct daemon *daemon, encoding_add_short_channel_id(&encoded, &scids[i]); } - if (!encoding_end_prepend_type(&encoded, max_encoded_bytes)) { + if (!encoding_end(encoded, max_encoded_bytes)) { status_broken("query_short_channel_ids: %zu is too many", tal_count(scids)); return false; @@ -150,15 +134,15 @@ bool query_short_channel_ids(struct daemon *daemon, tlvs = tlv_query_short_channel_ids_tlvs_new(tmpctx); tlvq = tlvs->query_flags = tal(tlvs, struct tlv_query_short_channel_ids_tlvs_query_flags); - tlvq->encoded_query_flags = encoding_start(tlvq); + tlvq->encoding_type = ARR_UNCOMPRESSED; + tlvq->encoded_query_flags = encoding_start(tlvq, false); for (size_t i = 0; i < tal_count(query_flags); i++) encoding_add_query_flag(&tlvq->encoded_query_flags, query_flags[i]); max_encoded_bytes -= tal_bytelen(encoded); - if (!encoding_end_external_type(&tlvq->encoded_query_flags, - &tlvq->encoding_type, - max_encoded_bytes)) { + if (!encoding_end(tlvq->encoded_query_flags, + max_encoded_bytes)) { status_broken("query_short_channel_ids:" " %zu query_flags is too many", tal_count(query_flags)); @@ -305,24 +289,24 @@ static void send_reply_channel_range(struct peer *peer, * - MUST limit `number_of_blocks` to the maximum number of blocks * whose results could fit in `encoded_short_ids` */ - u8 *encoded_scids = encoding_start(tmpctx); - u8 *encoded_timestamps = encoding_start(tmpctx); + u8 *encoded_scids = encoding_start(tmpctx, true); + u8 *encoded_timestamps = encoding_start(tmpctx, false); struct tlv_reply_channel_range_tlvs *tlvs = tlv_reply_channel_range_tlvs_new(tmpctx); /* Encode them all */ for (size_t i = 0; i < num_scids; i++) encoding_add_short_channel_id(&encoded_scids, &scids[i]); - encoding_end_prepend_type(&encoded_scids, tal_bytelen(encoded_scids)); + encoding_end(encoded_scids, tal_bytelen(encoded_scids)); if (tstamps) { for (size_t i = 0; i < num_scids; i++) encoding_add_timestamps(&encoded_timestamps, &tstamps[i]); tlvs->timestamps_tlv = tal(tlvs, struct tlv_reply_channel_range_tlvs_timestamps_tlv); - encoding_end_external_type(&encoded_timestamps, - &tlvs->timestamps_tlv->encoding_type, - tal_bytelen(encoded_timestamps)); + tlvs->timestamps_tlv->encoding_type = ARR_UNCOMPRESSED; + encoding_end(encoded_timestamps, + tal_bytelen(encoded_timestamps)); tlvs->timestamps_tlv->encoded_timestamps = tal_steal(tlvs, encoded_timestamps); } diff --git a/gossipd/test/run-extended-info.c b/gossipd/test/run-extended-info.c index 845525a26f67..a89dabd99a8a 100644 --- a/gossipd/test/run-extended-info.c +++ b/gossipd/test/run-extended-info.c @@ -206,7 +206,7 @@ static u8 *get_scid_array(const tal_t *ctx, scids = json_get_member(test_vector, obj, "shortChannelIds"); arr = json_get_member(test_vector, scids, "array"); - encoded = encoding_start(ctx); + encoded = encoding_start(ctx, true); encoding = json_get_member(test_vector, scids, "encoding"); json_for_each_arr(i, t, arr) { struct short_channel_id scid; @@ -214,8 +214,6 @@ static u8 *get_scid_array(const tal_t *ctx, encoding_add_short_channel_id(&encoded, &scid); } assert(json_tok_streq(test_vector, encoding, "UNCOMPRESSED")); - encoding_end_no_compress(&encoded, 1); - encoded[0] = ARR_UNCOMPRESSED; return encoded; } @@ -274,7 +272,7 @@ static u8 *test_reply_channel_range(const char *test_vector, const jsmntok_t *ob = tal(tlvs, struct tlv_reply_channel_range_tlvs_timestamps_tlv); tlvs->timestamps_tlv->encoded_timestamps - = encoding_start(tlvs->timestamps_tlv); + = encoding_start(tlvs->timestamps_tlv, false); encodingtok = json_get_member(test_vector, opt, "encoding"); tstok = json_get_member(test_vector, opt, "timestamps"); json_for_each_arr(i, t, tstok) { @@ -290,8 +288,6 @@ static u8 *test_reply_channel_range(const char *test_vector, const jsmntok_t *ob encoding_add_timestamps(&tlvs->timestamps_tlv->encoded_timestamps, &ts); } assert(json_tok_streq(test_vector, encodingtok, "UNCOMPRESSED")); - encoding_end_no_compress(&tlvs->timestamps_tlv->encoded_timestamps, - 0); tlvs->timestamps_tlv->encoding_type = ARR_UNCOMPRESSED; } @@ -335,7 +331,7 @@ get_query_flags_array(const tal_t *ctx, arr = json_get_member(test_vector, opt, "array"); - tlv->encoded_query_flags = encoding_start(tlv); + tlv->encoded_query_flags = encoding_start(tlv, false); encoding = json_get_member(test_vector, opt, "encoding"); json_for_each_arr(i, t, arr) { bigsize_t f; @@ -343,7 +339,6 @@ get_query_flags_array(const tal_t *ctx, encoding_add_query_flag(&tlv->encoded_query_flags, f); } assert(json_tok_streq(test_vector, encoding, "UNCOMPRESSED")); - encoding_end_no_compress(&tlv->encoded_query_flags, 0); tlv->encoding_type = ARR_UNCOMPRESSED; return tlv; From 53ca4136b4adeb07917000ff9940796d2ea7d17f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 18 May 2022 10:28:58 +0930 Subject: [PATCH 3/5] Makefile: update to BOLTs without zlib. This contains a typo fix and a clarification on channel_type, but also removes ZLIB. Signed-off-by: Rusty Russell --- Makefile | 2 +- channeld/commit_tx.c | 2 +- common/decode_array.c | 6 +++--- common/decode_array.h | 4 ++-- common/initial_commit_tx.c | 2 +- devtools/mkencoded.c | 2 +- devtools/print_wire.c | 4 ++-- gossipd/queries.c | 6 ++---- openingd/openingd.c | 4 ++-- 9 files changed, 15 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index e69e854356ff..70975595b8bc 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../bolts/ -DEFAULT_BOLTVERSION := e60d594abf436e768116684080997a8d4f960263 +DEFAULT_BOLTVERSION := c1b94dfad12d9e29268cbd0d3b22686ea5709cbc # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/channeld/commit_tx.c b/channeld/commit_tx.c index 2fdf0df1ac62..b1f0ae51f8a6 100644 --- a/channeld/commit_tx.c +++ b/channeld/commit_tx.c @@ -379,7 +379,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx, /* BOLT #3: * * 9. Sort the outputs into [BIP 69+CLTV - * order](#transaction-input-and-output-ordering) + * order](#transaction-output-ordering) */ permute_outputs(tx, cltvs, (const void **)*htlcmap); diff --git a/common/decode_array.c b/common/decode_array.c index 7296d072a687..157eea48d68c 100644 --- a/common/decode_array.c +++ b/common/decode_array.c @@ -42,7 +42,7 @@ struct short_channel_id *decode_short_ids(const tal_t *ctx, const u8 *encoded) */ type = fromwire_u8(&encoded, &max); switch (type) { - case ARR_ZLIB: + case ARR_ZLIB_DEPRECATED: encoded = unzlib(tmpctx, encoded, max); if (!encoded) return NULL; @@ -85,7 +85,7 @@ bigsize_t *decode_scid_query_flags(const tal_t *ctx, * - MAY close the connection. */ switch (qf->encoding_type) { - case ARR_ZLIB: + case ARR_ZLIB_DEPRECATED: encoded = unzlib(tmpctx, encoded, max); if (!encoded) return NULL; @@ -119,7 +119,7 @@ decode_channel_update_timestamps(const tal_t *ctx, /* FIXME: BOLT #7 should have a requirements like it does for * query_short_channel_ids_tlvs! */ switch (timestamps_tlv->encoding_type) { - case ARR_ZLIB: + case ARR_ZLIB_DEPRECATED: encoded = unzlib(tmpctx, encoded, max); if (!encoded) return NULL; diff --git a/common/decode_array.h b/common/decode_array.h index c1f09c4bf7e0..697e8ddee19f 100644 --- a/common/decode_array.h +++ b/common/decode_array.h @@ -11,11 +11,11 @@ struct tlv_reply_channel_range_tlvs_timestamps_tlv; * * Encoding types: * * `0`: uncompressed array of `short_channel_id` types, in ascending order. - * * `1`: array of `short_channel_id` types, in ascending order, compressed with zlib deflate[1](#reference-1) + * * `1`: Previously used for zlib compression, this encoding MUST NOT be used. */ enum arr_encode_types { ARR_UNCOMPRESSED = 0, - ARR_ZLIB = 1 + ARR_ZLIB_DEPRECATED = 1 }; struct short_channel_id *decode_short_ids(const tal_t *ctx, const u8 *encoded); diff --git a/common/initial_commit_tx.c b/common/initial_commit_tx.c index 44203f7f4bd6..1c319ceb1dfe 100644 --- a/common/initial_commit_tx.c +++ b/common/initial_commit_tx.c @@ -291,7 +291,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, /* BOLT #3: * * 9. Sort the outputs into [BIP 69+CLTV - * order](#transaction-input-and-output-ordering) + * order](#transaction-output-ordering) */ permute_outputs(tx, NULL, output_order); diff --git a/devtools/mkencoded.c b/devtools/mkencoded.c index f76d2ab817ed..76a2bc20a4e0 100644 --- a/devtools/mkencoded.c +++ b/devtools/mkencoded.c @@ -42,7 +42,7 @@ int main(int argc, char *argv[]) if (encoding == ARR_UNCOMPRESSED) printf("%02x%s\n", encoding, tal_hex(NULL, data)); - else if (encoding == ARR_ZLIB) { + else if (encoding == ARR_ZLIB_DEPRECATED) { /* https://www.zlib.net/zlib_tech.html: * the only expansion is an overhead of five bytes per 16 KB * block (about 0.03%), plus a one-time overhead of six bytes diff --git a/devtools/print_wire.c b/devtools/print_wire.c index f8b4acc1feaa..742b72de6e32 100644 --- a/devtools/print_wire.c +++ b/devtools/print_wire.c @@ -189,7 +189,7 @@ static bool printwire_encoded_short_ids(const u8 **cursor, size_t *plen, size_t case ARR_UNCOMPRESSED: printf(" (UNCOMPRESSED)"); break; - case ARR_ZLIB: + case ARR_ZLIB_DEPRECATED: printf(" (ZLIB)"); break; default: @@ -202,7 +202,7 @@ static bool printwire_encoded_short_ids(const u8 **cursor, size_t *plen, size_t /* If it was unknown, that's different from corrupt */ if (len == 0 || arr[0] == ARR_UNCOMPRESSED - || arr[0] == ARR_ZLIB) { + || arr[0] == ARR_ZLIB_DEPRECATED) { printf(" **CORRUPT**"); return true; } else { diff --git a/gossipd/queries.c b/gossipd/queries.c index a1ec4dcffe17..cb5a61d115ba 100644 --- a/gossipd/queries.c +++ b/gossipd/queries.c @@ -23,9 +23,8 @@ static u32 dev_max_encoding_bytes = -1U; /* BOLT #7: * * There are several messages which contain a long array of - * `short_channel_id`s (called `encoded_short_ids`) so we utilize a - * simple compression scheme: the first byte indicates the encoding, the - * rest contains the data. + * `short_channel_id`s (called `encoded_short_ids`) so we include an encoding + * byte which allows for different encoding schemes to be defined in the future */ static u8 *encoding_start(const tal_t *ctx, bool prepend_encoding) { @@ -117,7 +116,6 @@ bool query_short_channel_ids(struct daemon *daemon, * Encoding types: * * `0`: uncompressed array of `short_channel_id` types, in * ascending order. - * * `1`: array of `short_channel_id` types, in ascending order */ assert(i == 0 || scids[i].u64 > scids[i-1].u64); encoding_add_short_channel_id(&encoded, &scids[i]); diff --git a/openingd/openingd.c b/openingd/openingd.c index 5a83ae1c1c9e..b929a998f9b0 100644 --- a/openingd/openingd.c +++ b/openingd/openingd.c @@ -1000,8 +1000,8 @@ static u8 *fundee_channel(struct state *state, const u8 *open_channel_msg) accept_tlvs->upfront_shutdown_script = state->upfront_shutdown_script[LOCAL]; /* BOLT #2: - * - if it sets `channel_type`: - * - MUST set it to the `channel_type` from `open_channel` + * - if `option_channel_type` was negotiated: + * - MUST set `channel_type` to the `channel_type` from `open_channel` */ accept_tlvs->channel_type = state->channel_type->features; From b685837cf3c104a273102892bc4940dcfc0b5d6f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 18 May 2022 10:29:04 +0930 Subject: [PATCH 4/5] Makefile: update bolts to include remote_pubkey change. Only affects comments. Signed-off-by: Rusty Russell --- Makefile | 2 +- channeld/commit_tx.c | 2 +- common/initial_commit_tx.c | 2 +- onchaind/onchaind.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 70975595b8bc..c6345fc4ee2d 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../bolts/ -DEFAULT_BOLTVERSION := c1b94dfad12d9e29268cbd0d3b22686ea5709cbc +DEFAULT_BOLTVERSION := c4a0369e705ad43babee50dd0466f162567e6427 # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/channeld/commit_tx.c b/channeld/commit_tx.c index b1f0ae51f8a6..54116c42944e 100644 --- a/channeld/commit_tx.c +++ b/channeld/commit_tx.c @@ -298,7 +298,7 @@ struct bitcoin_tx *commit_tx(const tal_t *ctx, * If `option_anchors` applies to the commitment * transaction, the `to_remote` output is encumbered by a one * block csv lock. - * OP_CHECKSIGVERIFY 1 OP_CHECKSEQUENCEVERIFY + * OP_CHECKSIGVERIFY 1 OP_CHECKSEQUENCEVERIFY * *... * Otherwise, this output is a simple P2WPKH to `remotepubkey`. diff --git a/common/initial_commit_tx.c b/common/initial_commit_tx.c index 1c319ceb1dfe..97afedb15059 100644 --- a/common/initial_commit_tx.c +++ b/common/initial_commit_tx.c @@ -236,7 +236,7 @@ struct bitcoin_tx *initial_commit_tx(const tal_t *ctx, * If `option_anchors` applies to the commitment * transaction, the `to_remote` output is encumbered by a one * block csv lock. - * OP_CHECKSIGVERIFY 1 OP_CHECKSEQUENCEVERIFY + * OP_CHECKSIGVERIFY 1 OP_CHECKSEQUENCEVERIFY * *... * Otherwise, this output is a simple P2WPKH to `remotepubkey`. diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c index 7d10e36a4b5c..6859d327d4e4 100644 --- a/onchaind/onchaind.c +++ b/onchaind/onchaind.c @@ -2522,7 +2522,7 @@ static u8 *scriptpubkey_to_remote(const tal_t *ctx, * If `option_anchors` applies to the commitment * transaction, the `to_remote` output is encumbered by a one * block csv lock. - * OP_CHECKSIGVERIFY 1 OP_CHECKSEQUENCEVERIFY + * OP_CHECKSIGVERIFY 1 OP_CHECKSEQUENCEVERIFY * *... * Otherwise, this output is a simple P2WPKH to `remotepubkey`. From 46d4a271cd190d8d5c7acc81c1dcf344cb2092bb Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 18 May 2022 10:52:41 +0930 Subject: [PATCH 5/5] Makefile: update to include fix for remote_addr generation. Now it's formatted properly, we don't need the patch. But we need to explicitly marshal/unmarshal into a byte stream, which involves some code rearrangement. Signed-off-by: Rusty Russell --- Makefile | 2 +- connectd/peer_exchange_initmsg.c | 46 ++++++++++++++---------- wire/extracted_peer_01_remote_addr.patch | 13 ------- wire/peer_wire.csv | 2 +- 4 files changed, 29 insertions(+), 34 deletions(-) delete mode 100644 wire/extracted_peer_01_remote_addr.patch diff --git a/Makefile b/Makefile index c6345fc4ee2d..ba8e5d8504f5 100644 --- a/Makefile +++ b/Makefile @@ -21,7 +21,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../bolts/ -DEFAULT_BOLTVERSION := c4a0369e705ad43babee50dd0466f162567e6427 +DEFAULT_BOLTVERSION := 105c2e5e9f17c68e8c19dc4ca548600a0b8f66f0 # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/connectd/peer_exchange_initmsg.c b/connectd/peer_exchange_initmsg.c index eea73188a52c..f43b66164825 100644 --- a/connectd/peer_exchange_initmsg.c +++ b/connectd/peer_exchange_initmsg.c @@ -91,29 +91,37 @@ static struct io_plan *peer_init_received(struct io_conn *conn, /* fetch optional tlv `remote_addr` */ remote_addr = NULL; - /* BOLT-remote-address #1: + /* BOLT #1: * The receiving node: * ... - * - MAY use the `remote_addr` to update its `node_annoucement` + * - MAY use the `remote_addr` to update its `node_announcement` */ if (tlvs->remote_addr) { - switch (tlvs->remote_addr->type) { - case ADDR_TYPE_IPV4: - case ADDR_TYPE_IPV6: + const u8 *cursor = tlvs->remote_addr; + size_t len = tal_bytelen(tlvs->remote_addr); + + remote_addr = tal(peer, struct wireaddr); + if (fromwire_wireaddr(&cursor, &len, remote_addr)) { + switch (remote_addr->type) { + case ADDR_TYPE_IPV4: + case ADDR_TYPE_IPV6: #if DEVELOPER /* ignore private addresses (non-DEVELOPER builds) */ - if (address_routable(tlvs->remote_addr, true)) + if (!address_routable(remote_addr, true)) #else - if (address_routable(tlvs->remote_addr, false)) + if (!address_routable(remote_addr, false)) #endif /* DEVELOPER */ - remote_addr = tal_steal(peer, tlvs->remote_addr); - break; - /* We are only interested in IP addresses */ - case ADDR_TYPE_TOR_V2_REMOVED: - case ADDR_TYPE_TOR_V3: - case ADDR_TYPE_DNS: - case ADDR_TYPE_WEBSOCKET: - break; - } + remote_addr = tal_free(remote_addr); + break; + /* We are only interested in IP addresses */ + case ADDR_TYPE_TOR_V2_REMOVED: + case ADDR_TYPE_TOR_V3: + case ADDR_TYPE_DNS: + case ADDR_TYPE_WEBSOCKET: + remote_addr = tal_free(remote_addr); + break; + } + } else + remote_addr = tal_free(remote_addr); } /* The globalfeatures field is now unused, but there was a @@ -217,7 +225,7 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn, /* set optional tlv `remote_addr` on incoming IP connections */ tlvs->remote_addr = NULL; - /* BOLT-remote-address #1: + /* BOLT #1: * The sending node: * ... * - SHOULD set `remote_addr` to reflect the remote IP address (and port) of an @@ -229,8 +237,8 @@ struct io_plan *peer_exchange_initmsg(struct io_conn *conn, switch (addr->u.wireaddr.type) { case ADDR_TYPE_IPV4: case ADDR_TYPE_IPV6: - tlvs->remote_addr = tal(tlvs, struct wireaddr); - *tlvs->remote_addr = addr->u.wireaddr; + tlvs->remote_addr = tal_arr(tlvs, u8, 0); + towire_wireaddr(&tlvs->remote_addr, &addr->u.wireaddr); break; /* Only report IP addresses back for now */ case ADDR_TYPE_TOR_V2_REMOVED: diff --git a/wire/extracted_peer_01_remote_addr.patch b/wire/extracted_peer_01_remote_addr.patch deleted file mode 100644 index 5e9aeb46348e..000000000000 --- a/wire/extracted_peer_01_remote_addr.patch +++ /dev/null @@ -1,13 +0,0 @@ -diff --git a/wire/peer_wire.csv b/wire/peer_wire.csv -index a028ddc66..4043c6350 100644 ---- a/wire/peer_wire.csv -+++ b/wire/peer_wire.csv -@@ -6,6 +6,8 @@ msgdata,init,features,byte,flen - msgdata,init,tlvs,init_tlvs, - tlvtype,init_tlvs,networks,1 - tlvdata,init_tlvs,networks,chains,chain_hash,... -+tlvtype,init_tlvs,remote_addr,3 -+tlvdata,init_tlvs,remote_addr,remote_addr,wireaddr, - msgtype,error,17 - msgdata,error,channel_id,channel_id, - msgdata,error,len,u16, diff --git a/wire/peer_wire.csv b/wire/peer_wire.csv index a028ddc66d92..497d43b52fe8 100644 --- a/wire/peer_wire.csv +++ b/wire/peer_wire.csv @@ -7,7 +7,7 @@ msgdata,init,tlvs,init_tlvs, tlvtype,init_tlvs,networks,1 tlvdata,init_tlvs,networks,chains,chain_hash,... tlvtype,init_tlvs,remote_addr,3 -tlvdata,init_tlvs,remote_addr,remote_addr,wireaddr, +tlvdata,init_tlvs,remote_addr,data,byte,... msgtype,error,17 msgdata,error,channel_id,channel_id, msgdata,error,len,u16,