From 333ceeae0c95d36a8c19792acdcee240b8af9ab5 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 12:16:34 +0930 Subject: [PATCH 01/13] BOLT: update to fix gossip pruning quote. Which I disagreed with, and has been fixed. Signed-off-by: Rusty Russell --- Makefile | 2 +- common/gossip_constants.h | 4 ++-- gossipd/gossipd.c | 4 ++-- gossipd/routing.c | 6 ++---- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index f746cdd2d396..ae3b544a94da 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../bolts/ -DEFAULT_BOLTVERSION := 341ec844f13c0c0abc4fe849059fbb98173f9766 +DEFAULT_BOLTVERSION := 48fed66e26b80031d898c6492434fa9926237d64 # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/common/gossip_constants.h b/common/gossip_constants.h index 436db62a4ab6..345f9126b231 100644 --- a/common/gossip_constants.h +++ b/common/gossip_constants.h @@ -62,8 +62,8 @@ /* BOLT #7: * * A node: - * - if a channel's latest `channel_update`s `timestamp` is older than two weeks - * (1209600 seconds): + * - if the `timestamp` of the latest `channel_update` in + * either direction is older than two weeks (1209600 seconds): * - MAY prune the channel. * - MAY ignore the channel. */ diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 1926c440ec5b..26be95bd6090 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -603,8 +603,8 @@ static struct io_plan *connectd_req(struct io_conn *conn, /* BOLT #7: * * A node: - * - if a channel's latest `channel_update`s `timestamp` is older than two weeks - * (1209600 seconds): + * - if the `timestamp` of the latest `channel_update` in + * either direction is older than two weeks (1209600 seconds): * - MAY prune the channel. * - MAY ignore the channel. */ diff --git a/gossipd/routing.c b/gossipd/routing.c index d9dcdf72b61f..8e569dec8a89 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1926,12 +1926,10 @@ void route_prune(struct routing_state *rstate) continue; /* BOLT #7: - * - if a channel's latest `channel_update`s `timestamp` is - * older than two weeks (1209600 seconds): + * - if the `timestamp` of the latest `channel_update` in + * either direction is older than two weeks (1209600 seconds): * - MAY prune the channel. */ - /* FIXME: I disagree with the above quote: it used to say "oldest", which is what we - use here: */ /* This is a fancy way of saying "both ends must refresh!" */ if (!is_halfchan_defined(&chan->half[0]) || chan->half[0].bcast.timestamp < highwater From 40654c0e0bee7122079b1b3994051e293d7abe5c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:28 +0930 Subject: [PATCH 02/13] gossipd: don't try to upgrade ancient gossip_store. If they really upgrade directly from 0.9.2, it will simply delete the store and re-fetch it. We still update from v9 (which could be v0.11), since it's a noop. Signed-off-by: Rusty Russell --- gossipd/gossip_store.c | 66 ++---------------------------- gossipd/gossip_store_wire.csv | 9 ----- tests/test_gossip.py | 75 +---------------------------------- 3 files changed, 4 insertions(+), 146 deletions(-) diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index 3eed656529ad..873be7f8818a 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -101,26 +101,10 @@ static bool append_msg(int fd, const u8 *msg, u32 timestamp, return true; } -#ifdef COMPAT_V082 -static u8 *mk_private_channelmsg(const tal_t *ctx, - struct routing_state *rstate, - const struct short_channel_id *scid, - const struct node_id *remote_node_id, - struct amount_sat sat, - const u8 *features) -{ - const u8 *ann = private_channel_announcement(tmpctx, scid, - &rstate->local_id, - remote_node_id, - features); - - return towire_gossip_store_private_channel(ctx, sat, ann); -} - -/* The upgrade from version 7 is trivial */ +/* The upgrade from version 9 is a noop: we added the spam flag. */ static bool can_upgrade(u8 oldversion) { - return oldversion == 7 || oldversion == 8 || oldversion == 9; + return oldversion == 9; } static bool upgrade_field(u8 oldversion, @@ -128,52 +112,8 @@ static bool upgrade_field(u8 oldversion, u8 **msg) { assert(can_upgrade(oldversion)); - - if (fromwire_peektype(*msg) == WIRE_GOSSIPD_LOCAL_ADD_CHANNEL_OBS - && oldversion == 7) { - /* Append two 0 bytes, for (empty) feature bits */ - tal_resizez(msg, tal_bytelen(*msg) + 2); - } - - /* We turn these (v8) into a WIRE_GOSSIP_STORE_PRIVATE_CHANNEL */ - if (fromwire_peektype(*msg) == WIRE_GOSSIPD_LOCAL_ADD_CHANNEL_OBS) { - struct short_channel_id scid; - struct node_id remote_node_id; - struct amount_sat satoshis; - u8 *features; - u8 *storemsg; - - if (!fromwire_gossipd_local_add_channel_obs(tmpctx, *msg, - &scid, - &remote_node_id, - &satoshis, - &features)) - return false; - - storemsg = mk_private_channelmsg(tal_parent(*msg), - rstate, - &scid, - &remote_node_id, - satoshis, - features); - tal_free(*msg); - *msg = storemsg; - } return true; } -#else -static bool can_upgrade(u8 oldversion) -{ - return false; -} - -static bool upgrade_field(u8 oldversion, - struct routing_state *rstate, - u8 **msg) -{ - abort(); -} -#endif /* !COMPAT_V082 */ /* Read gossip store entries, copy non-deleted ones. This code is written * as simply and robustly as possible! */ @@ -771,7 +711,7 @@ u32 gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) } if (checksum != crc32c(be32_to_cpu(hdr.timestamp), msg, msglen)) { - bad = "Checksum verification failed"; + bad = tal_fmt(tmpctx, "Checksum verification failed: should be %08x", crc32c(be32_to_cpu(hdr.timestamp), msg, msglen)); goto badmsg; } diff --git a/gossipd/gossip_store_wire.csv b/gossipd/gossip_store_wire.csv index 1b90e38e9073..6781757dcec5 100644 --- a/gossipd/gossip_store_wire.csv +++ b/gossipd/gossip_store_wire.csv @@ -23,12 +23,3 @@ msgdata,gossip_store_delete_chan,scid,short_channel_id, msgtype,gossip_store_ended,4105 msgdata,gossip_store_ended,equivalent_offset,u64, - -# FIXME: Here for COMPAT with v0.9.0 and before only. -msgtype,gossipd_local_add_channel_obs,3503 -msgdata,gossipd_local_add_channel_obs,short_channel_id,short_channel_id, -msgdata,gossipd_local_add_channel_obs,remote_node_id,node_id, -msgdata,gossipd_local_add_channel_obs,satoshis,amount_sat, -msgdata,gossipd_local_add_channel_obs,flen,u16, -msgdata,gossipd_local_add_channel_obs,features,u8,flen - diff --git a/tests/test_gossip.py b/tests/test_gossip.py index fec573363712..1f0d35b3095c 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -5,7 +5,7 @@ from pyln.client import RpcError, Millisatoshi from utils import ( DEVELOPER, wait_for, TIMEOUT, only_one, sync_blockheight, - expected_node_features, COMPAT, + expected_node_features, mine_funding_to_announce, default_ln_port ) @@ -2001,79 +2001,6 @@ def test_torport_onions(node_factory): assert l2.daemon.is_in_log('x2y4zvh4fn5q3eouuh7nxnc7zeawrqoutljrup2xjtiyxgx3emgkemad.onion:45321,127.0.0.1:{}'.format(l2.port)) -@unittest.skipIf(not COMPAT, "needs COMPAT to convert obsolete gossip_store") -def test_gossip_store_upgrade_v7_v8(node_factory): - """Version 8 added feature bits to local channel announcements""" - - # We get BROKEN logs because gossipd talks about non-existent channels to - # lightningd ("**BROKEN** lightningd: Local update for bad scid 103x1x1"). - l1 = node_factory.get_node(start=False, - allow_broken_log=True) - - # A channel announcement with no channel_update. - with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), 'wb') as f: - f.write(bytearray.fromhex("07000000428ce4d2d8000000000daf00" - "00670000010001022d223620a359a47f" - "f7f7ac447c85c46c923da53389221a00" - "54c11c1e3ca31d5900000000000f4240" - "000d8000000000000000000000000000" - "00008e3af3badf000000001006008a01" - "02005a9911d425effd461f803a380f05" - "e72d3332eb6e9a7c6c58405ae61eacde" - "4e2da18240ffb3d5c595f85e4f78b594" - "c59e4d01c0470edd4f5afe645026515e" - "fe06226e46111a0b59caaf126043eb5b" - "bf28c34f3a5e332a1fc7b2b73cf18891" - "0f00006700000100015eaa5eb0010100" - "06000000000000000000000001000000" - "0a000000003b0233800000008e074a6e" - "0f000000001006008a0102463de636b2" - "f46ccd6c23259787fc39dc4fdb983510" - "1651879325b18cf1bb26330127e51ce8" - "7a111b05ef92fe00a9a089979dc49178" - "200f49139a541e7078cdc506226e4611" - "1a0b59caaf126043eb5bbf28c34f3a5e" - "332a1fc7b2b73cf188910f0000670000" - "0100015eaa5eb0010000060000000000" - "000000000000010000000a000000003b" - "023380")) - - l1.start() - - assert l1.rpc.listchannels()['channels'] == [ - {'source': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', - 'destination': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', - 'short_channel_id': '103x1x1', - 'public': False, - 'amount_msat': Millisatoshi(1000000000), - 'message_flags': 1, - 'channel_flags': 0, - 'active': False, - 'last_update': 1588223664, - 'base_fee_millisatoshi': 1, - 'fee_per_millionth': 10, - 'delay': 6, - 'htlc_minimum_msat': Millisatoshi(0), - 'htlc_maximum_msat': Millisatoshi(990000000), - # This store was created on an experimental branch (OPT_ONION_MESSAGES) - 'features': '80000000000000000000000000'}, - {'source': '0266e4598d1d3c415f572a8488830b60f7e744ed9235eb0b1ba93283b315c03518', - 'destination': '022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59', - 'short_channel_id': '103x1x1', - 'public': False, - 'amount_msat': Millisatoshi(1000000000), - 'message_flags': 1, - 'channel_flags': 1, - 'active': False, - 'last_update': 1588223664, - 'base_fee_millisatoshi': 1, - 'fee_per_millionth': 10, - 'delay': 6, - 'htlc_minimum_msat': Millisatoshi(0), - 'htlc_maximum_msat': Millisatoshi(990000000), - 'features': '80000000000000000000000000'}] - - @pytest.mark.developer("devtools are for devs anyway") def test_routetool(node_factory): """Test that route tool can see unpublished channels""" From d1cd57c9f4ebbad802c51da1512eedd2cdd317a4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:31 +0930 Subject: [PATCH 03/13] gossipd: actually validate gossip_store checksums at startup. We rewrite the file to compact it, but as a side effect we recalculate the checksums! Signed-off-by: Rusty Russell --- gossipd/gossip_store.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index 873be7f8818a..29d3bc11032a 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -175,6 +175,16 @@ static u32 gossip_store_compact_offline(struct routing_state *rstate) continue; } + /* Check checksum (upgrade would overwrite, so do it now) */ + if (be32_to_cpu(hdr.crc) + != crc32c(be32_to_cpu(hdr.timestamp), msg, msglen)) { + status_broken("gossip_store_compact_offline: checksum verification failed? %08x should be %08x", + be32_to_cpu(hdr.crc), + crc32c(be32_to_cpu(hdr.timestamp), msg + sizeof(hdr), msglen)); + tal_free(msg); + goto close_and_delete; + } + if (oldversion != version) { if (!upgrade_field(oldversion, rstate, &msg)) { tal_free(msg); From ae81c1d8475b98ec2fb864a73fd6e78c4972a9b4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:31 +0930 Subject: [PATCH 04/13] gossmap: make API more robust against future changes. Many changes to gossmap (including the pending ones!) don't actually concern readers, as long as they obey certain rules: 1. Ignore unknown messages. 2. Treat all 16 upper bits of length as flags, ignore unknown ones. So now we split the version byte into MAJOR and MINOR, and you can ignore MINOR changes. We don't expose the internal version (for creating the map) programmatically: you should really hardcode what major version you understand! Signed-off-by: Rusty Russell --- common/gossip_store.h | 23 ++++++++++++++++++---- common/gossmap.c | 3 ++- common/test/run-route-specific.c | 2 +- common/test/run-route.c | 2 +- contrib/pyln-client/pyln/client/gossmap.py | 9 ++++----- devtools/create-gossipstore.c | 2 +- devtools/dump-gossipstore.c | 20 +++++++++++++++---- gossipd/gossip_store.c | 10 ++++++---- plugins/test/run-route-overlong.c | 2 +- 9 files changed, 51 insertions(+), 22 deletions(-) diff --git a/common/gossip_store.h b/common/gossip_store.h index c4f6f52bd01f..514f32096f54 100644 --- a/common/gossip_store.h +++ b/common/gossip_store.h @@ -11,7 +11,18 @@ struct gossip_rcvd_filter; /** * gossip_store -- On-disk storage related information */ -#define GOSSIP_STORE_VERSION 10 + +/* First byte of file is the version. + * + * Top three bits mean incompatible change. + * As of this writing, major == 0, minor == 10. + */ +#define GOSSIP_STORE_MAJOR_VERSION_MASK 0xE0 +#define GOSSIP_STORE_MINOR_VERSION_MASK 0x1F + +/* Extract version from first byte */ +#define GOSSIP_STORE_MAJOR_VERSION(verbyte) (((u8)(verbyte)) >> 5) +#define GOSSIP_STORE_MINOR_VERSION(verbyte) ((verbyte) & GOSSIP_STORE_MINOR_VERSION_MASK) /** * Bit of length we use to mark a deleted record. @@ -26,12 +37,16 @@ struct gossip_rcvd_filter; /** * Bit of length used to define a rate-limited record (do not rebroadcast) */ - #define GOSSIP_STORE_LEN_RATELIMIT_BIT 0x20000000U +#define GOSSIP_STORE_LEN_RATELIMIT_BIT 0x20000000U + +/** + * Full flags mask + */ +#define GOSSIP_STORE_FLAGS_MASK 0xFFFF0000U /* Mask for extracting just the length part of len field */ #define GOSSIP_STORE_LEN_MASK \ - (~(GOSSIP_STORE_LEN_PUSH_BIT | GOSSIP_STORE_LEN_DELETED_BIT | \ - GOSSIP_STORE_LEN_RATELIMIT_BIT)) + (~(GOSSIP_STORE_FLAGS_MASK)) /** * gossip_hdr -- On-disk format header. diff --git a/common/gossmap.c b/common/gossmap.c index 5d25b1122cac..e64fd983f3a3 100644 --- a/common/gossmap.c +++ b/common/gossmap.c @@ -672,7 +672,8 @@ static bool load_gossip_store(struct gossmap *map, size_t *num_rejected) if (map->mmap == MAP_FAILED) map->mmap = NULL; - if (map_u8(map, 0) != GOSSIP_STORE_VERSION) { + /* We only support major version 0 */ + if (GOSSIP_STORE_MAJOR_VERSION(map_u8(map, 0)) != 0) { close(map->fd); if (map->mmap) munmap(map->mmap, map->map_size); diff --git a/common/test/run-route-specific.c b/common/test/run-route-specific.c index 3a1488c49377..8ced38d36cfc 100644 --- a/common/test/run-route-specific.c +++ b/common/test/run-route-specific.c @@ -179,7 +179,7 @@ int main(int argc, char *argv[]) int store_fd; struct gossmap *gossmap; const double riskfactor = 1.0; - char gossip_version = GOSSIP_STORE_VERSION; + char gossip_version = 10; char *gossipfilename; common_setup(argv[0]); diff --git a/common/test/run-route.c b/common/test/run-route.c index 33b3668c3760..398200d88bc5 100644 --- a/common/test/run-route.c +++ b/common/test/run-route.c @@ -176,7 +176,7 @@ int main(int argc, char *argv[]) int store_fd; struct gossmap *gossmap; const double riskfactor = 1.0; - char gossip_version = GOSSIP_STORE_VERSION; + char gossip_version = 10; char *gossipfilename; chainparams = chainparams_for_network("regtest"); diff --git a/contrib/pyln-client/pyln/client/gossmap.py b/contrib/pyln-client/pyln/client/gossmap.py index f58d49add0e7..4d72209e8e0c 100755 --- a/contrib/pyln-client/pyln/client/gossmap.py +++ b/contrib/pyln-client/pyln/client/gossmap.py @@ -9,13 +9,12 @@ import struct # These duplicate constants in lightning/common/gossip_store.h -GOSSIP_STORE_VERSIONS = [0x09, 0x0a] +GOSSIP_STORE_MAJOR_VERSION = (0 << 5) +GOSSIP_STORE_MAJOR_VERSION_MASK = 0xE0 GOSSIP_STORE_LEN_DELETED_BIT = 0x80000000 GOSSIP_STORE_LEN_PUSH_BIT = 0x40000000 GOSSIP_STORE_LEN_RATELIMIT_BIT = 0x20000000 -GOSSIP_STORE_LEN_MASK = (~(GOSSIP_STORE_LEN_PUSH_BIT - | GOSSIP_STORE_LEN_DELETED_BIT - | GOSSIP_STORE_LEN_RATELIMIT_BIT)) +GOSSIP_STORE_LEN_MASK = (0x0000FFFF) # These duplicate constants in lightning/gossipd/gossip_store_wiregen.h WIRE_GOSSIP_STORE_PRIVATE_CHANNEL = 4104 @@ -174,7 +173,7 @@ def __init__(self, store_filename: str = "gossip_store"): self.channels: Dict[ShortChannelId, GossmapChannel] = {} self._last_scid: Optional[str] = None version = self.store_file.read(1)[0] - if version not in GOSSIP_STORE_VERSIONS: + if (version & GOSSIP_STORE_MAJOR_VERSION_MASK) != GOSSIP_STORE_MAJOR_VERSION: raise ValueError("Invalid gossip store version {}".format(version)) self.bytes_read = 1 self.refresh() diff --git a/devtools/create-gossipstore.c b/devtools/create-gossipstore.c index 28d6f0843e98..d72de28e0efe 100644 --- a/devtools/create-gossipstore.c +++ b/devtools/create-gossipstore.c @@ -155,7 +155,7 @@ int main(int argc, char *argv[]) } else outfd = STDOUT_FILENO; - version = GOSSIP_STORE_VERSION; + version = ((0 << 5) | 10); if (!write_all(outfd, &version, sizeof(version))) err(1, "Writing version"); diff --git a/devtools/dump-gossipstore.c b/devtools/dump-gossipstore.c index 1d5100d0a052..7370b9c22721 100644 --- a/devtools/dump-gossipstore.c +++ b/devtools/dump-gossipstore.c @@ -10,6 +10,10 @@ #include #include +/* Current versions we support */ +#define GSTORE_MAJOR 0 +#define GSTORE_MINOR 10 + int main(int argc, char *argv[]) { int fd; @@ -43,11 +47,19 @@ int main(int argc, char *argv[]) if (read(fd, &version, sizeof(version)) != sizeof(version)) errx(1, "Empty file"); - if (version != GOSSIP_STORE_VERSION) - warnx("UNSUPPORTED GOSSIP VERSION %u (expected %u)", - version, GOSSIP_STORE_VERSION); + if (GOSSIP_STORE_MAJOR_VERSION(version) != GSTORE_MAJOR) + errx(1, "Unsupported major gossip_version %u (expected %u)", + GOSSIP_STORE_MAJOR_VERSION(version), GSTORE_MAJOR); + + /* Unsupported minor just means we might not understand all fields, + * or all flags. */ + if (GOSSIP_STORE_MINOR_VERSION(version) != GSTORE_MINOR) + warnx("UNKNOWN GOSSIP minor VERSION %u (expected %u)", + GOSSIP_STORE_MINOR_VERSION(version), GSTORE_MINOR); - printf("GOSSIP VERSION %u\n", version); + printf("GOSSIP VERSION %u/%u\n", + GOSSIP_STORE_MINOR_VERSION(version), + GOSSIP_STORE_MAJOR_VERSION(version)); off = 1; while (read(fd, &hdr, sizeof(hdr)) == sizeof(hdr)) { diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index 29d3bc11032a..64bc313bb74d 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -17,6 +17,8 @@ #include #define GOSSIP_STORE_TEMP_FILENAME "gossip_store.tmp" +/* We write it as major version 0, minor version 10 */ +#define GOSSIP_STORE_VER ((0 << 5) | 10) struct gossip_store { /* This is false when we're loading */ @@ -123,7 +125,7 @@ static u32 gossip_store_compact_offline(struct routing_state *rstate) int old_fd, new_fd; u64 oldlen, newlen; struct gossip_hdr hdr; - u8 oldversion, version = GOSSIP_STORE_VERSION; + u8 oldversion, version = GOSSIP_STORE_VER; struct stat st; old_fd = open(GOSSIP_STORE_FILENAME, O_RDWR); @@ -267,11 +269,11 @@ struct gossip_store *gossip_store_new(struct routing_state *rstate, if (read(gs->fd, &gs->version, sizeof(gs->version)) == sizeof(gs->version)) { /* Version match? All good */ - if (gs->version == GOSSIP_STORE_VERSION) + if (gs->version == GOSSIP_STORE_VER) return gs; status_unusual("Gossip store version %u not %u: removing", - gs->version, GOSSIP_STORE_VERSION); + gs->version, GOSSIP_STORE_VER); if (ftruncate(gs->fd, 0) != 0) status_failed(STATUS_FAIL_INTERNAL_ERROR, "Truncating store: %s", strerror(errno)); @@ -282,7 +284,7 @@ struct gossip_store *gossip_store_new(struct routing_state *rstate, strerror(errno)); } /* Empty file, write version byte */ - gs->version = GOSSIP_STORE_VERSION; + gs->version = GOSSIP_STORE_VER; if (write(gs->fd, &gs->version, sizeof(gs->version)) != sizeof(gs->version)) status_failed(STATUS_FAIL_INTERNAL_ERROR, diff --git a/plugins/test/run-route-overlong.c b/plugins/test/run-route-overlong.c index 82dfdf7eb7d5..52faac965a6a 100644 --- a/plugins/test/run-route-overlong.c +++ b/plugins/test/run-route-overlong.c @@ -335,7 +335,7 @@ int main(int argc, char *argv[]) int store_fd; struct payment *p; struct payment_modifier **mods; - char gossip_version = GOSSIP_STORE_VERSION; + char gossip_version = 10; char *gossipfilename; common_setup(argv[0]); From 2214ef2a1e25bfa92dca1c01d25035a9627d4acd Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:31 +0930 Subject: [PATCH 05/13] gossipd: bump gossip_store to indicate all channel_update have htlc_max. And in the next patch, gossipd will no longer put new ones in. Signed-off-by: Rusty Russell --- common/gossip_store.h | 2 +- gossipd/gossip_store.c | 30 ++++++++++++++++++----- tests/test_gossip.py | 54 ++++++++++++++++++++++++++++++++---------- 3 files changed, 66 insertions(+), 20 deletions(-) diff --git a/common/gossip_store.h b/common/gossip_store.h index 514f32096f54..e74f7cba4718 100644 --- a/common/gossip_store.h +++ b/common/gossip_store.h @@ -15,7 +15,7 @@ struct gossip_rcvd_filter; /* First byte of file is the version. * * Top three bits mean incompatible change. - * As of this writing, major == 0, minor == 10. + * As of this writing, major == 0, minor == 11. */ #define GOSSIP_STORE_MAJOR_VERSION_MASK 0xE0 #define GOSSIP_STORE_MINOR_VERSION_MASK 0x1F diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index 64bc313bb74d..5a3e9cd27206 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -17,8 +17,8 @@ #include #define GOSSIP_STORE_TEMP_FILENAME "gossip_store.tmp" -/* We write it as major version 0, minor version 10 */ -#define GOSSIP_STORE_VER ((0 << 5) | 10) +/* We write it as major version 0, minor version 11 */ +#define GOSSIP_STORE_VER ((0 << 5) | 11) struct gossip_store { /* This is false when we're loading */ @@ -103,10 +103,12 @@ static bool append_msg(int fd, const u8 *msg, u32 timestamp, return true; } -/* The upgrade from version 9 is a noop: we added the spam flag. */ +/* v9 added the GOSSIP_STORE_LEN_RATELIMIT_BIT. + * v10 removed any remaining non-htlc-max channel_update. + */ static bool can_upgrade(u8 oldversion) { - return oldversion == 9; + return oldversion == 9 || oldversion == 10; } static bool upgrade_field(u8 oldversion, @@ -114,6 +116,15 @@ static bool upgrade_field(u8 oldversion, u8 **msg) { assert(can_upgrade(oldversion)); + + if (oldversion == 10) { + /* Remove old channel_update with no htlc_maximum_msat */ + if (fromwire_peektype(*msg) == WIRE_CHANNEL_UPDATE + && tal_bytelen(*msg) == 130) { + *msg = tal_free(*msg); + } + } + return true; } @@ -182,7 +193,7 @@ static u32 gossip_store_compact_offline(struct routing_state *rstate) != crc32c(be32_to_cpu(hdr.timestamp), msg, msglen)) { status_broken("gossip_store_compact_offline: checksum verification failed? %08x should be %08x", be32_to_cpu(hdr.crc), - crc32c(be32_to_cpu(hdr.timestamp), msg + sizeof(hdr), msglen)); + crc32c(be32_to_cpu(hdr.timestamp), msg, msglen)); tal_free(msg); goto close_and_delete; } @@ -193,6 +204,12 @@ static u32 gossip_store_compact_offline(struct routing_state *rstate) goto close_and_delete; } + /* It can tell us to delete record entirely. */ + if (msg == NULL) { + deleted++; + continue; + } + /* Recalc msglen and header */ msglen = tal_bytelen(msg); hdr.len = cpu_to_be32(msglen); @@ -723,7 +740,8 @@ u32 gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) } if (checksum != crc32c(be32_to_cpu(hdr.timestamp), msg, msglen)) { - bad = tal_fmt(tmpctx, "Checksum verification failed: should be %08x", crc32c(be32_to_cpu(hdr.timestamp), msg, msglen)); + bad = tal_fmt(tmpctx, "Checksum verification failed: %08x should be %08x", + checksum, crc32c(be32_to_cpu(hdr.timestamp), msg, msglen)); goto badmsg; } diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 1f0d35b3095c..a4bacc9ff243 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -1141,11 +1141,11 @@ def test_gossip_store_load(node_factory): "00000000" # timestamp "1005" # WIRE_GOSSIP_STORE_CHANNEL_AMOUNT "0000000001000000" - "00000082" # len - "fd421aeb" # csum + "0000008a" # len + "0c6aca0e" # csum "5b8d9b44" # timestamp "0102" # WIRE_CHANNEL_UPDATE - "1ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440000009000000000000003e8000003e800000001" + "1ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440100009000000000000003e8000003e8000000010000000000FFFFFF" "00000095" # len "f036515e" # csum "5aab817c" # timestamp @@ -1154,10 +1154,36 @@ def test_gossip_store_load(node_factory): l1.start() # May preceed the Started msg waited for in 'start'. - wait_for(lambda: l1.daemon.is_in_log(r'gossip_store: Read 1/1/1/0 cannounce/cupdate/nannounce/cdelete from store \(0 deleted\) in 770 bytes')) + wait_for(lambda: l1.daemon.is_in_log(r'gossip_store: Read 1/1/1/0 cannounce/cupdate/nannounce/cdelete from store \(0 deleted\) in 778 bytes')) assert not l1.daemon.is_in_log('gossip_store.*truncating') +def test_gossip_store_v10_upgrade(node_factory): + """We remove a channel_update without an htlc_maximum_msat""" + l1 = node_factory.get_node(start=False) + with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), 'wb') as f: + f.write(bytearray.fromhex("0a" # GOSSIP_STORE_VERSION + "000001b0" # len + "fea676e8" # csum + "5b8d9b44" # timestamp + "0100" # WIRE_CHANNEL_ANNOUNCEMENT + "bb8d7b6998cca3c2b3ce12a6bd73a8872c808bb48de2a30c5ad9cdf835905d1e27505755087e675fb517bbac6beb227629b694ea68f49d357458327138978ebfd7adfde1c69d0d2f497154256f6d5567a5cf2317c589e0046c0cc2b3e986cf9b6d3b44742bd57bce32d72cd1180a7f657795976130b20508b239976d3d4cdc4d0d6e6fbb9ab6471f664a662972e406f519eab8bce87a8c0365646df5acbc04c91540b4c7c518cec680a4a6af14dae1aca0fd5525220f7f0e96fcd2adef3c803ac9427fe71034b55a50536638820ef21903d09ccddd38396675b598587fa886ca711415c813fc6d69f46552b9a0a539c18f265debd0e2e286980a118ba349c216000043497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b50001021bf3de4e84e3d52f9a3e36fbdcd2c4e8dbf203b9ce4fc07c2f03be6c21d0c67503f113414ebdc6c1fb0f33c99cd5a1d09dd79e7fdf2468cf1fe1af6674361695d203801fd8ab98032f11cc9e4916dd940417082727077609d5c7f8cc6e9a3ad25dd102517164b97ab46cee3826160841a36c46a2b7b9c74da37bdc070ed41ba172033a" + "0000000a" # len + "99dc98b4" # csum + "00000000" # timestamp + "1005" # WIRE_GOSSIP_STORE_CHANNEL_AMOUNT + "0000000001000000" + "00000082" # len + "fd421aeb" # csum + "5b8d9b44" # timestamp + "0102" # WIRE_CHANNEL_UPDATE + "1ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440000009000000000000003e8000003e800000001")) + + l1.start() + # May preceed the Started msg waited for in 'start'. + wait_for(lambda: l1.daemon.is_in_log(r'gossip_store: Unupdated channel_announcement at 1.')) + + def test_gossip_store_load_announce_before_update(node_factory): """Make sure we can read canned gossip store with node_announce before update. This happens when a channel_update gets replaced, leaving node_announce before it""" l1 = node_factory.get_node(start=False) @@ -1173,25 +1199,27 @@ def test_gossip_store_load_announce_before_update(node_factory): "00000000" # timestamp "1005" # WIRE_GOSSIP_STORE_CHANNEL_AMOUNT "0000000001000000" - "80000082" # len (DELETED) - "fd421aeb" # csum + "8000008a" # len (DELETED) + "ca01ed56" # csum "5b8d9b44" # timestamp "0102" # WIRE_CHANNEL_UPDATE - "1ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440000009000000000000003e8000003e800000001" + # Note - msgflags set and htlc_max added by hand, so signature doesn't match (gossipd ignores) + "1ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440100009000000000000003e8000003e8000000010000000000FFFFFF" "00000095" # len "f036515e" # csum "5aab817c" # timestamp "0101" # WIRE_NODE_ANNOUNCEMENT "cf5d870bc7ecabcb7cd16898ef66891e5f0c6c5851bd85b670f03d325bc44d7544d367cd852e18ec03f7f4ff369b06860a3b12b07b29f36fb318ca11348bf8ec00005aab817c03f113414ebdc6c1fb0f33c99cd5a1d09dd79e7fdf2468cf1fe1af6674361695d23974b250757a7a6c6549544300000000000000000000000000000000000000000000000007010566933e2607" - "00000082" # len - "fd421aeb" # csum + "0000008a" # len + "0c6aca0e" # csum "5b8d9b44" # timestamp "0102" # WIRE_CHANNEL_UPDATE - "1ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440000009000000000000003e8000003e800000001")) + # Note - msgflags set and htlc_max added by hand, so signature doesn't match (gossipd ignores) + "1ea7c2eadf8a29eb8690511a519b5656e29aa0a853771c4e38e65c5abf43d907295a915e69e451f4c7a0c3dc13dd943cfbe3ae88c0b96667cd7d58955dbfedcf43497fd7f826957108f4a30fd9cec3aeba79972084e90ead01ea33090000000013a63c0000b500015b8d9b440100009000000000000003e8000003e8000000010000000000FFFFFF")) l1.start() # May preceed the Started msg waited for in 'start'. - wait_for(lambda: l1.daemon.is_in_log(r'gossip_store: Read 1/1/1/0 cannounce/cupdate/nannounce/cdelete from store \(0 deleted\) in 770 bytes')) + wait_for(lambda: l1.daemon.is_in_log(r'gossip_store: Read 1/1/1/0 cannounce/cupdate/nannounce/cdelete from store \(0 deleted\) in 778 bytes')) assert not l1.daemon.is_in_log('gossip_store.*truncating') # Extra sanity check if we can. @@ -1670,7 +1698,7 @@ def test_gossip_store_load_no_channel_update(node_factory): # A channel announcement with no channel_update. with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), 'wb') as f: - f.write(bytearray.fromhex("0a" # GOSSIP_STORE_VERSION + f.write(bytearray.fromhex("0b" # GOSSIP_STORE_VERSION "000001b0" # len "fea676e8" # csum "5b8d9b44" # timestamp @@ -1697,7 +1725,7 @@ def test_gossip_store_load_no_channel_update(node_factory): l1.rpc.call('dev-compact-gossip-store') with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), "rb") as f: - assert bytearray(f.read()) == bytearray.fromhex("0a") + assert bytearray(f.read()) == bytearray.fromhex("0b") @pytest.mark.developer("gossip without DEVELOPER=1 is slow") From 26ff43b2bb75b9005bff16684f15d9908c030463 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:31 +0930 Subject: [PATCH 06/13] BOLT: update to version which requires option_channel_htlc_max. We will now simply reject old-style ones as invalid. Turns out the only trace we could find is a channel between two nodes unconnected to the rest of the network. Signed-off-by: Rusty Russell Changelog-Changed: Protocol: We now require all channel_update messages include htlc_maximum_msat (as per latest BOLTs) --- Makefile | 2 +- common/gossip_constants.h | 12 +++--- common/gossmap.c | 14 ++----- common/test/run-route-specific.c | 24 +++++------ common/test/run-route.c | 24 +++++------ devtools/create-gossipstore.c | 6 +-- devtools/mkgossip.c | 36 ++++------------ gossipd/gossip_generation.c | 17 ++++---- gossipd/routing.c | 16 ++----- hsmd/libhsmd.c | 4 +- plugins/test/run-route-overlong.c | 24 +++++------ tests/test_gossip.py | 2 +- wire/peer_wire.csv | 2 +- wire/test/run-peer-wire.c | 69 ++----------------------------- 14 files changed, 80 insertions(+), 172 deletions(-) diff --git a/Makefile b/Makefile index ae3b544a94da..29e3fb8a582c 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../bolts/ -DEFAULT_BOLTVERSION := 48fed66e26b80031d898c6492434fa9926237d64 +DEFAULT_BOLTVERSION := 6fee63fc342736a2eb7f6e856ae0d85807cc50ec # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/common/gossip_constants.h b/common/gossip_constants.h index 345f9126b231..c407f302381e 100644 --- a/common/gossip_constants.h +++ b/common/gossip_constants.h @@ -22,13 +22,15 @@ /* BOLT #7: * - * The `message_flags` bitfield is used to indicate the presence of optional - * fields in the `channel_update` message: - * | Bit Position | Name | Field | - * | ------------- | ------------------------- | ----------------...- | - * | 0 | `option_channel_htlc_max` | `htlc_maximum_msat` | + * The `message_flags` bitfield is used to provide additional details about the message: + * | Bit Position | Name | + * | ------------- | ---------------| + * | 0 | `must_be_one` | + * | 1 | `dont_forward` | */ +/* FIXME: This is the old name */ #define ROUTING_OPT_HTLC_MAX_MSAT (1 << 0) +#define ROUTING_OPT_DONT_FORWARD (1 << 1) /* BOLT #7: * diff --git a/common/gossmap.c b/common/gossmap.c index e64fd983f3a3..0849fcfc62b9 100644 --- a/common/gossmap.c +++ b/common/gossmap.c @@ -482,7 +482,7 @@ static struct gossmap_chan *add_channel(struct gossmap *map, * * [`u64`:`htlc_minimum_msat`] * * [`u32`:`fee_base_msat`] * * [`u32`:`fee_proportional_millionths`] - * * [`u64`:`htlc_maximum_msat`] (option_channel_htlc_max) + * * [`u64`:`htlc_maximum_msat`] */ static bool update_channel(struct gossmap *map, size_t cupdate_off) { @@ -509,15 +509,7 @@ static bool update_channel(struct gossmap *map, size_t cupdate_off) /* We round this *down*, since too-low min is more conservative */ hc.htlc_min = u64_to_fp16(map_be64(map, htlc_minimum_off), false); - /* I checked my node: 60189 of 62358 channel_update have - * htlc_maximum_msat, so we don't bother setting the rest to the - * channel size (which we don't even read from the gossip_store, let - * alone give up precious bytes to remember) */ - if (map_u8(map, message_flags_off) & 1) - hc.htlc_max - = u64_to_fp16(map_be64(map, htlc_maximum_off), true); - else - hc.htlc_max = 0xFFFF; + hc.htlc_max = u64_to_fp16(map_be64(map, htlc_maximum_off), true); chanflags = map_u8(map, channel_flags_off); hc.enabled = !(chanflags & 2); @@ -1225,7 +1217,7 @@ u8 *gossmap_chan_get_features(const tal_t *ctx, * * [`u64`:`htlc_minimum_msat`] * * [`u32`:`fee_base_msat`] * * [`u32`:`fee_proportional_millionths`] - * * [`u64`:`htlc_maximum_msat`] (option_channel_htlc_max) + * * [`u64`:`htlc_maximum_msat`] */ void gossmap_chan_get_update_details(const struct gossmap *map, const struct gossmap_chan *chan, diff --git a/common/test/run-route-specific.c b/common/test/run-route-specific.c index 8ced38d36cfc..eddf83ba4d06 100644 --- a/common/test/run-route-specific.c +++ b/common/test/run-route-specific.c @@ -79,18 +79,18 @@ static void update_connection(int store_fd, if (!short_channel_id_from_str(shortid, strlen(shortid), &scid)) abort(); - msg = towire_channel_update_option_channel_htlc_max(tmpctx, - &dummy_sig, - &chainparams->genesis_blockhash, - &scid, 0, - ROUTING_OPT_HTLC_MAX_MSAT, - node_id_idx(from, to) - + (disable ? ROUTING_FLAGS_DISABLED : 0), - delay, - min, - base_fee, - proportional_fee, - max); + msg = towire_channel_update(tmpctx, + &dummy_sig, + &chainparams->genesis_blockhash, + &scid, 0, + ROUTING_OPT_HTLC_MAX_MSAT, + node_id_idx(from, to) + + (disable ? ROUTING_FLAGS_DISABLED : 0), + delay, + min, + base_fee, + proportional_fee, + max); write_to_store(store_fd, msg); } diff --git a/common/test/run-route.c b/common/test/run-route.c index 398200d88bc5..e41b141264dc 100644 --- a/common/test/run-route.c +++ b/common/test/run-route.c @@ -70,18 +70,18 @@ static void update_connection(int store_fd, memcpy(&scid, from, sizeof(scid) / 2); memcpy((char *)&scid + sizeof(scid) / 2, to, sizeof(scid) / 2); - msg = towire_channel_update_option_channel_htlc_max(tmpctx, - &dummy_sig, - &chainparams->genesis_blockhash, - &scid, 0, - ROUTING_OPT_HTLC_MAX_MSAT, - node_id_idx(from, to) - + (disable ? ROUTING_FLAGS_DISABLED : 0), - delay, - AMOUNT_MSAT(0), - base_fee, - proportional_fee, - AMOUNT_MSAT(100000 * 1000)); + msg = towire_channel_update(tmpctx, + &dummy_sig, + &chainparams->genesis_blockhash, + &scid, 0, + ROUTING_OPT_HTLC_MAX_MSAT, + node_id_idx(from, to) + + (disable ? ROUTING_FLAGS_DISABLED : 0), + delay, + AMOUNT_MSAT(0), + base_fee, + proportional_fee, + AMOUNT_MSAT(100000 * 1000)); write_to_store(store_fd, msg); } diff --git a/devtools/create-gossipstore.c b/devtools/create-gossipstore.c index d72de28e0efe..45b61b09f587 100644 --- a/devtools/create-gossipstore.c +++ b/devtools/create-gossipstore.c @@ -64,12 +64,12 @@ static u32 get_update_timestamp(const u8 *msg, struct short_channel_id *scid) u8 u8_ignore; u16 u16_ignore; u32 u32_ignore; - struct amount_msat msat; + struct amount_msat msat_ignore; if (fromwire_channel_update(msg, &sig, &chain_hash, scid, ×tamp, &u8_ignore, &u8_ignore, - &u16_ignore, &msat, &u32_ignore, - &u32_ignore)) + &u16_ignore, &msat_ignore, &u32_ignore, + &u32_ignore, &msat_ignore)) return timestamp; errx(1, "Invalid channel_update"); } diff --git a/devtools/mkgossip.c b/devtools/mkgossip.c index 791bd797146b..74fca27b6749 100644 --- a/devtools/mkgossip.c +++ b/devtools/mkgossip.c @@ -23,10 +23,9 @@ static bool verbose = false; struct update_opts { u32 timestamp; u32 cltv_expiry_delta; - struct amount_msat min; + struct amount_msat min, max; struct amount_msat feebase; u32 fee_proportional_millionths; - struct amount_msat *max; u8 *addresses; }; @@ -52,14 +51,9 @@ static int parse_options(char *argv[], struct update_opts *opts, if (!opts->fee_proportional_millionths) errx(1, "Bad %s.fee_proportional_millionths", desc); - if (streq(argv[argnum], "")) - opts->max = NULL; - else { - opts->max = tal(NULL, struct amount_msat); - if (!parse_amount_msat(opts->max, - argv[argnum], strlen(argv[argnum]))) - errx(1, "Bad %s.max", desc); - } + if (!parse_amount_msat(&opts->max, + argv[argnum], strlen(argv[argnum]))) + errx(1, "Bad %s.max", desc); argnum++; opts->addresses = tal_hexdata(NULL, argv[argnum], strlen(argv[argnum])); if (!opts->addresses) @@ -139,8 +133,7 @@ static void print_update(const struct bitcoin_blkid *chainhash, u8 *cupdate; memset(&sig, 0, sizeof(sig)); - if (opts->max) { - cupdate = towire_channel_update_option_channel_htlc_max + cupdate = towire_channel_update (NULL, &sig, chainhash, scid, opts->timestamp, ROUTING_OPT_HTLC_MAX_MSAT, is_lesser_key ? 0 : ROUTING_FLAGS_DIRECTION, @@ -148,17 +141,7 @@ static void print_update(const struct bitcoin_blkid *chainhash, opts->min, opts->feebase.millisatoshis, /* Raw: devtools code */ opts->fee_proportional_millionths, - *opts->max); - } else { - cupdate = towire_channel_update - (NULL, &sig, chainhash, scid, opts->timestamp, - 0, - is_lesser_key ? 0 : ROUTING_FLAGS_DIRECTION, - opts->cltv_expiry_delta, - opts->min, - opts->feebase.millisatoshis, /* Raw: devtools code */ - opts->fee_proportional_millionths); - } + opts->max); sha256_double(&hash, cupdate + channel_update_offset, tal_count(cupdate) - channel_update_offset); sign_hash(privkey, &hash, &sig); @@ -169,7 +152,7 @@ static void print_update(const struct bitcoin_blkid *chainhash, printf(" short_channel_id=%s\n", short_channel_id_to_str(NULL, scid)); printf(" timestamp=%u\n", opts->timestamp); printf(" message_flags=%u\n", - opts->max ? ROUTING_OPT_HTLC_MAX_MSAT : 0); + ROUTING_OPT_HTLC_MAX_MSAT); printf(" channel_flags=%u\n", is_lesser_key ? 0 : ROUTING_FLAGS_DIRECTION); printf(" cltv_expiry_delta=%u\n", @@ -180,9 +163,8 @@ static void print_update(const struct bitcoin_blkid *chainhash, opts->feebase.millisatoshis); /* Raw: devtools code */ printf(" fee_proportional_millionths=%u\n", opts->fee_proportional_millionths); - if (opts->max) - printf(" htlc_maximum_msat=%"PRIu64"\n", - opts->max->millisatoshis); /* Raw: devtools code */ + printf(" htlc_maximum_msat=%"PRIu64"\n", + opts->max.millisatoshis); /* Raw: devtools code */ printf("# crc32c checksum: %08x\n", crc32_of_update(cupdate)); } diff --git a/gossipd/gossip_generation.c b/gossipd/gossip_generation.c index e5ab5eeab727..1ac944110169 100644 --- a/gossipd/gossip_generation.c +++ b/gossipd/gossip_generation.c @@ -539,17 +539,18 @@ static u8 *create_unsigned_update(const tal_t *ctx, /* BOLT #7: * - * The `message_flags` bitfield is used to indicate the presence of - * optional fields in the `channel_update` message: + * The `message_flags` bitfield is used to provide additional + * details about the message: * - *| Bit Position | Name | Field | - *... - *| 0 | `option_channel_htlc_max` | `htlc_maximum_msat` | + * | Bit Position | Name | + * | ------------- | ---------------| + * | 0 | `must_be_one` | + * | 1 | `dont_forward` | */ - message_flags = 0 | ROUTING_OPT_HTLC_MAX_MSAT; + message_flags = ROUTING_OPT_HTLC_MAX_MSAT; /* We create an update with a dummy signature and timestamp. */ - return towire_channel_update_option_channel_htlc_max(ctx, + return towire_channel_update(ctx, &dummy_sig, /* sig set later */ &chainparams->genesis_blockhash, scid, @@ -730,7 +731,7 @@ void refresh_local_channel(struct daemon *daemon, if (!prev) return; - if (!fromwire_channel_update_option_channel_htlc_max(prev, + if (!fromwire_channel_update(prev, &signature, &chain_hash, &short_channel_id, ×tamp, &message_flags, &channel_flags, diff --git a/gossipd/routing.c b/gossipd/routing.c index 8e569dec8a89..8e8a913ad4e6 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1298,16 +1298,7 @@ bool routing_add_channel_update(struct routing_state *rstate, if (taken(update)) tal_steal(tmpctx, update); - if (!fromwire_channel_update(update, &signature, &chain_hash, - &short_channel_id, ×tamp, - &message_flags, &channel_flags, - &expiry, &htlc_minimum, &fee_base_msat, - &fee_proportional_millionths)) - return false; - /* If it's flagged as containing the optional field, reparse for - * the optional field */ - if ((message_flags & ROUTING_OPT_HTLC_MAX_MSAT) && - !fromwire_channel_update_option_channel_htlc_max( + if (!fromwire_channel_update( update, &signature, &chain_hash, &short_channel_id, ×tamp, &message_flags, &channel_flags, @@ -1557,7 +1548,7 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, u32 timestamp; u8 message_flags, channel_flags; u16 expiry; - struct amount_msat htlc_minimum; + struct amount_msat htlc_minimum, htlc_maximum; u32 fee_base_msat; u32 fee_proportional_millionths; struct bitcoin_blkid chain_hash; @@ -1571,7 +1562,8 @@ u8 *handle_channel_update(struct routing_state *rstate, const u8 *update TAKES, ×tamp, &message_flags, &channel_flags, &expiry, &htlc_minimum, &fee_base_msat, - &fee_proportional_millionths)) { + &fee_proportional_millionths, + &htlc_maximum)) { warn = towire_warningfmt(rstate, NULL, "Malformed channel_update %s", tal_hex(tmpctx, serialized)); diff --git a/hsmd/libhsmd.c b/hsmd/libhsmd.c index ad2cb627f3e1..aedb6021fb61 100644 --- a/hsmd/libhsmd.c +++ b/hsmd/libhsmd.c @@ -972,7 +972,7 @@ static u8 *handle_channel_update_sig(struct hsmd_client *c, const u8 *msg_in) if (!fromwire_hsmd_cupdate_sig_req(tmpctx, msg_in, &cu)) return hsmd_status_malformed_request(c, msg_in); - if (!fromwire_channel_update_option_channel_htlc_max(cu, &sig, + if (!fromwire_channel_update(cu, &sig, &chain_hash, &scid, ×tamp, &message_flags, &channel_flags, &cltv_expiry_delta, &htlc_minimum, &fee_base_msat, @@ -989,7 +989,7 @@ static u8 *handle_channel_update_sig(struct hsmd_client *c, const u8 *msg_in) sign_hash(&node_pkey, &hash, &sig); - cu = towire_channel_update_option_channel_htlc_max(tmpctx, &sig, &chain_hash, + cu = towire_channel_update(tmpctx, &sig, &chain_hash, &scid, timestamp, message_flags, channel_flags, cltv_expiry_delta, htlc_minimum, fee_base_msat, fee_proportional_mill, diff --git a/plugins/test/run-route-overlong.c b/plugins/test/run-route-overlong.c index 52faac965a6a..870356fa3294 100644 --- a/plugins/test/run-route-overlong.c +++ b/plugins/test/run-route-overlong.c @@ -260,18 +260,18 @@ static void update_connection(int store_fd, /* So valgrind doesn't complain */ memset(&dummy_sig, 0, sizeof(dummy_sig)); - msg = towire_channel_update_option_channel_htlc_max(tmpctx, - &dummy_sig, - &chainparams->genesis_blockhash, - scid, 0, - ROUTING_OPT_HTLC_MAX_MSAT, - node_id_idx(from, to) - + (disable ? ROUTING_FLAGS_DISABLED : 0), - delay, - min, - base_fee, - proportional_fee, - max); + msg = towire_channel_update(tmpctx, + &dummy_sig, + &chainparams->genesis_blockhash, + scid, 0, + ROUTING_OPT_HTLC_MAX_MSAT, + node_id_idx(from, to) + + (disable ? ROUTING_FLAGS_DISABLED : 0), + delay, + min, + base_fee, + proportional_fee, + max); write_to_store(store_fd, msg); } diff --git a/tests/test_gossip.py b/tests/test_gossip.py index a4bacc9ff243..959baac91b6f 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -1160,7 +1160,7 @@ def test_gossip_store_load(node_factory): def test_gossip_store_v10_upgrade(node_factory): """We remove a channel_update without an htlc_maximum_msat""" - l1 = node_factory.get_node(start=False) + l1 = node_factory.get_node(start=False, allow_broken_log=True) with open(os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, 'gossip_store'), 'wb') as f: f.write(bytearray.fromhex("0a" # GOSSIP_STORE_VERSION "000001b0" # len diff --git a/wire/peer_wire.csv b/wire/peer_wire.csv index 54f48115666a..d9c7dcb590b1 100644 --- a/wire/peer_wire.csv +++ b/wire/peer_wire.csv @@ -291,7 +291,7 @@ msgdata,channel_update,cltv_expiry_delta,u16, msgdata,channel_update,htlc_minimum_msat,u64, msgdata,channel_update,fee_base_msat,u32, msgdata,channel_update,fee_proportional_millionths,u32, -msgdata,channel_update,htlc_maximum_msat,u64,,option_channel_htlc_max +msgdata,channel_update,htlc_maximum_msat,u64, msgtype,query_short_channel_ids,261,gossip_queries msgdata,query_short_channel_ids,chain_hash,chain_hash, msgdata,query_short_channel_ids,len,u16, diff --git a/wire/test/run-peer-wire.c b/wire/test/run-peer-wire.c index d9df608b812b..c6e5393cc897 100644 --- a/wire/test/run-peer-wire.c +++ b/wire/test/run-peer-wire.c @@ -133,18 +133,6 @@ struct msg_revoke_and_ack { struct pubkey next_per_commitment_point; }; struct msg_channel_update { - secp256k1_ecdsa_signature signature; - u32 timestamp; - u8 message_flags; - u8 channel_flags; - u16 expiry; - struct amount_msat htlc_minimum_msat; - u32 fee_base_msat; - u32 fee_proportional_millionths; - struct bitcoin_blkid chain_hash; - struct short_channel_id short_channel_id; -}; -struct msg_channel_update_opt_htlc_max { secp256k1_ecdsa_signature signature; u32 timestamp; u8 message_flags; @@ -406,25 +394,9 @@ static struct msg_node_announcement *fromwire_struct_node_announcement(const tal } static void *towire_struct_channel_update(const tal_t *ctx, - const struct msg_channel_update *s) + const struct msg_channel_update *s) { return towire_channel_update(ctx, - &s->signature, - &s->chain_hash, - &s->short_channel_id, - s->timestamp, - s->message_flags, - s->channel_flags, - s->expiry, - s->htlc_minimum_msat, - s->fee_base_msat, - s->fee_proportional_millionths); -} - -static void *towire_struct_channel_update_opt_htlc_max(const tal_t *ctx, - const struct msg_channel_update_opt_htlc_max *s) -{ - return towire_channel_update_option_channel_htlc_max(ctx, &s->signature, &s->chain_hash, &s->short_channel_id, @@ -437,31 +409,13 @@ static void *towire_struct_channel_update_opt_htlc_max(const tal_t *ctx, s->fee_proportional_millionths, s->htlc_maximum_msat); } -static struct msg_channel_update *fromwire_struct_channel_update(const tal_t *ctx, const void *p) + +static struct msg_channel_update +*fromwire_struct_channel_update(const tal_t *ctx, const void *p) { struct msg_channel_update *s = tal(ctx, struct msg_channel_update); if (fromwire_channel_update(p, - &s->signature, - &s->chain_hash, - &s->short_channel_id, - &s->timestamp, - &s->message_flags, - &s->channel_flags, - &s->expiry, - &s->htlc_minimum_msat, - &s->fee_base_msat, - &s->fee_proportional_millionths)) - return s; - return tal_free(s); -} - -static struct msg_channel_update_opt_htlc_max -*fromwire_struct_channel_update_opt_htlc_max(const tal_t *ctx, const void *p) -{ - struct msg_channel_update_opt_htlc_max *s = tal(ctx, struct msg_channel_update_opt_htlc_max); - - if (fromwire_channel_update_option_channel_htlc_max(p, &s->signature, &s->chain_hash, &s->short_channel_id, @@ -938,13 +892,6 @@ static bool channel_update_eq(const struct msg_channel_update *a, short_channel_id_eq(&a->short_channel_id, &b->short_channel_id); } -static bool channel_update_opt_htlc_max_eq(const struct msg_channel_update_opt_htlc_max *a, - const struct msg_channel_update_opt_htlc_max *b) -{ - return eq_upto(a, b, short_channel_id) && - short_channel_id_eq(&a->short_channel_id, &b->short_channel_id); -} - static bool accept_channel_eq(const struct msg_accept_channel *a, const struct msg_accept_channel *b) { @@ -1024,7 +971,6 @@ int main(int argc, char *argv[]) struct msg_revoke_and_ack raa, *raa2; struct msg_open_channel oc, *oc2; struct msg_channel_update cu, *cu2; - struct msg_channel_update_opt_htlc_max cu_opt_htlc_max, *cu_opt_htlc_max2; struct msg_accept_channel ac, *ac2; struct msg_update_add_htlc uah, *uah2; struct msg_node_announcement na, *na2; @@ -1185,13 +1131,6 @@ int main(int argc, char *argv[]) assert(channel_update_eq(&cu, cu2)); test_corruption(&cu, cu2, channel_update); - memset(&cu_opt_htlc_max, 2, sizeof(cu_opt_htlc_max)); - - msg = towire_struct_channel_update_opt_htlc_max(ctx, &cu_opt_htlc_max); - cu_opt_htlc_max2 = fromwire_struct_channel_update_opt_htlc_max(ctx, msg); - assert(channel_update_opt_htlc_max_eq(&cu_opt_htlc_max, cu_opt_htlc_max2)); - test_corruption(&cu_opt_htlc_max, cu_opt_htlc_max2, channel_update_opt_htlc_max); - memset(&ac, 2, sizeof(ac)); set_pubkey(&ac.funding_pubkey); set_pubkey(&ac.revocation_basepoint); From 5553b85480eb60aef4166c3e22c567ce7d5109ae Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:31 +0930 Subject: [PATCH 07/13] common: assume htlc_maximum_msat, don't check bit any more. Signed-off-by: Rusty Russell --- common/gossmap.c | 10 +++------- common/gossmap.h | 1 - contrib/pyln-testing/pyln/testing/gossip.py | 6 +----- gossipd/routing.c | 19 ++++--------------- plugins/topology.c | 6 ++---- 5 files changed, 10 insertions(+), 32 deletions(-) diff --git a/common/gossmap.c b/common/gossmap.c index 0849fcfc62b9..b5d326938d99 100644 --- a/common/gossmap.c +++ b/common/gossmap.c @@ -1228,7 +1228,6 @@ void gossmap_chan_get_update_details(const struct gossmap *map, u32 *fee_base_msat, u32 *fee_proportional_millionths, struct amount_msat *htlc_minimum_msat, - /* iff message_flags & 1 */ struct amount_msat *htlc_maximum_msat) { /* Note that first two bytes are message type */ @@ -1241,18 +1240,15 @@ void gossmap_chan_get_update_details(const struct gossmap *map, const size_t fee_base_off = htlc_minimum_off + 8; const size_t fee_prop_off = fee_base_off + 4; const size_t htlc_maximum_off = fee_prop_off + 4; - u8 mflags; assert(gossmap_chan_set(chan, dir)); if (timestamp) *timestamp = map_be32(map, timestamp_off); - /* We need this (below), even if they don't want it */ - mflags = map_u8(map, message_flags_off); - if (message_flags) - *message_flags = mflags; if (channel_flags) *channel_flags = map_u8(map, channel_flags_off); + if (message_flags) + *message_flags = map_u8(map, message_flags_off); if (fee_base_msat) *fee_base_msat = map_be32(map, fee_base_off); if (fee_proportional_millionths) @@ -1260,7 +1256,7 @@ void gossmap_chan_get_update_details(const struct gossmap *map, if (htlc_minimum_msat) *htlc_minimum_msat = amount_msat(map_be64(map, htlc_minimum_off)); - if (htlc_maximum_msat && (mflags & 1)) + if (htlc_maximum_msat) *htlc_maximum_msat = amount_msat(map_be64(map, htlc_maximum_off)); } diff --git a/common/gossmap.h b/common/gossmap.h index 38cee0425cc3..6b08ef684a87 100644 --- a/common/gossmap.h +++ b/common/gossmap.h @@ -161,7 +161,6 @@ void gossmap_chan_get_update_details(const struct gossmap *map, u32 *fee_base_msat, u32 *fee_proportional_millionths, struct amount_msat *htlc_minimum_msat, - /* iff message_flags & 1 */ struct amount_msat *htlc_maximum_msat); /* Given a struct node, get the nth channel, and tell us if we're half[0/1]. diff --git a/contrib/pyln-testing/pyln/testing/gossip.py b/contrib/pyln-testing/pyln/testing/gossip.py index 36976e462220..eb7ce99f4480 100644 --- a/contrib/pyln-testing/pyln/testing/gossip.py +++ b/contrib/pyln-testing/pyln/testing/gossip.py @@ -251,11 +251,7 @@ def from_bytes(cls, b: io.BytesIO) -> "ChannelUpdate": (cu.htlc_minimum_msat,) = struct.unpack("!Q", b.read(8)) (cu.fee_base_msat,) = struct.unpack("!I", b.read(4)) (cu.fee_proportional_millionths,) = struct.unpack("!I", b.read(4)) - t = b.read(8) - if len(t) == 8: - (cu.htlc_maximum_msat,) = struct.unpack("!Q", t) - else: - cu.htlc_maximum_msat = None + (cu.htlc_maximum_msat,) = struct.unpack("!Q", b.read(8)) return cu diff --git a/gossipd/routing.c b/gossipd/routing.c index 8e8a913ad4e6..08aef559ebf9 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -1322,21 +1322,10 @@ bool routing_add_channel_update(struct routing_state *rstate, sat = uc->sat; } - if (message_flags & ROUTING_OPT_HTLC_MAX_MSAT) { - /* Reject update if the `htlc_maximum_msat` is greater - * than the total available channel satoshis */ - if (amount_msat_greater_sat(htlc_maximum, sat)) - return false; - } else { - /* If not indicated, set htlc_max_msat to channel capacity */ - if (!amount_sat_to_msat(&htlc_maximum, sat)) { - status_peer_broken(peer ? &peer->id : NULL, - "Channel capacity %s overflows!", - type_to_string(tmpctx, struct amount_sat, - &sat)); - return false; - } - } + /* Reject update if the `htlc_maximum_msat` is greater + * than the total available channel satoshis */ + if (amount_msat_greater_sat(htlc_maximum, sat)) + return false; /* Check timestamp is sane (unless from store). */ if (!index && !timestamp_reasonable(rstate, timestamp)) { diff --git a/plugins/topology.c b/plugins/topology.c index d20e65a8bcc9..116ba81e6846 100644 --- a/plugins/topology.c +++ b/plugins/topology.c @@ -280,10 +280,8 @@ static void json_add_halfchan(struct json_stream *response, json_add_num(response, "delay", c->half[dir].delay); json_add_amount_msat_only(response, "htlc_minimum_msat", htlc_minimum_msat); - - if (message_flags & 1) - json_add_amount_msat_only(response, "htlc_maximum_msat", - htlc_maximum_msat); + json_add_amount_msat_only(response, "htlc_maximum_msat", + htlc_maximum_msat); json_add_hex_talarr(response, "features", chanfeatures); json_object_end(response); } From a06a568d16d7eac21340ebc447fbaa3c3e58c37c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:32 +0930 Subject: [PATCH 08/13] gossipd: set no_forward bit on channel_update for private channels. Signed-off-by: Rusty Russell Changelog-Added: Protocol: We now set the `dont_forward` bit on private channel_update's message_flags (as per latest BOLTs). --- channeld/channeld.c | 18 ++++++++++-------- channeld/channeld_wire.csv | 1 + gossipd/gossip_generation.c | 15 +++++++++++---- gossipd/gossipd_wire.csv | 1 + gossipd/test/run-check_node_announcement.c | 2 +- gossipd/test/run-crc32_of_update.c | 2 +- lightningd/gossip_control.c | 8 +++++--- 7 files changed, 30 insertions(+), 17 deletions(-) diff --git a/channeld/channeld.c b/channeld/channeld.c index 5e20d71e773d..51a39c6fe99f 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -369,14 +369,16 @@ static void send_channel_update(struct peer *peer, int disable_flag) assert(peer->short_channel_ids[LOCAL].u64); msg = towire_channeld_local_channel_update(NULL, - &peer->short_channel_ids[LOCAL], - disable_flag - == ROUTING_FLAGS_DISABLED, - peer->cltv_delta, - peer->htlc_minimum_msat, - peer->fee_base, - peer->fee_per_satoshi, - peer->htlc_maximum_msat); + &peer->short_channel_ids[LOCAL], + disable_flag + == ROUTING_FLAGS_DISABLED, + peer->cltv_delta, + peer->htlc_minimum_msat, + peer->fee_base, + peer->fee_per_satoshi, + peer->htlc_maximum_msat, + peer->channel_flags + & CHANNEL_FLAGS_ANNOUNCE_CHANNEL); wire_sync_write(MASTER_FD, take(msg)); } diff --git a/channeld/channeld_wire.csv b/channeld/channeld_wire.csv index 78abd73416ae..8827fa7b8cd1 100644 --- a/channeld/channeld_wire.csv +++ b/channeld/channeld_wire.csv @@ -256,6 +256,7 @@ msgdata,channeld_local_channel_update,htlc_minimum_msat,amount_msat, msgdata,channeld_local_channel_update,fee_base_msat,u32, msgdata,channeld_local_channel_update,fee_proportional_millionths,u32, msgdata,channeld_local_channel_update,htlc_maximum_msat,amount_msat, +msgdata,channeld_local_channel_update,public,bool, # Channeld: tell gossipd about our channel_announcement msgtype,channeld_local_channel_announcement,1014 diff --git a/gossipd/gossip_generation.c b/gossipd/gossip_generation.c index 1ac944110169..44c4afa8bc93 100644 --- a/gossipd/gossip_generation.c +++ b/gossipd/gossip_generation.c @@ -513,7 +513,8 @@ static u8 *create_unsigned_update(const tal_t *ctx, struct amount_msat htlc_minimum, struct amount_msat htlc_maximum, u32 fee_base_msat, - u32 fee_proportional_millionths) + u32 fee_proportional_millionths, + bool public) { secp256k1_ecdsa_signature dummy_sig; u8 message_flags, channel_flags; @@ -548,6 +549,8 @@ static u8 *create_unsigned_update(const tal_t *ctx, * | 1 | `dont_forward` | */ message_flags = ROUTING_OPT_HTLC_MAX_MSAT; + if (!public) + message_flags |= ROUTING_OPT_DONT_FORWARD; /* We create an update with a dummy signature and timestamp. */ return towire_channel_update(ctx, @@ -771,7 +774,8 @@ void refresh_local_channel(struct daemon *daemon, false, cltv_expiry_delta, htlc_minimum, htlc_maximum, fee_base_msat, - fee_proportional_millionths); + fee_proportional_millionths, + !(message_flags & ROUTING_OPT_DONT_FORWARD)); sign_timestamp_and_apply_update(daemon, chan, direction, take(update)); } @@ -788,6 +792,7 @@ void handle_local_channel_update(struct daemon *daemon, const u8 *msg) int direction; u8 *unsigned_update; const struct half_chan *hc; + bool public; if (!fromwire_gossipd_local_channel_update(msg, &id, @@ -797,7 +802,8 @@ void handle_local_channel_update(struct daemon *daemon, const u8 *msg) &htlc_minimum, &fee_base_msat, &fee_proportional_millionths, - &htlc_maximum)) { + &htlc_maximum, + &public)) { master_badmsg(WIRE_GOSSIPD_LOCAL_CHANNEL_UPDATE, msg); } @@ -822,7 +828,8 @@ void handle_local_channel_update(struct daemon *daemon, const u8 *msg) disable, cltv_expiry_delta, htlc_minimum, htlc_maximum, fee_base_msat, - fee_proportional_millionths); + fee_proportional_millionths, + public); hc = &chan->half[direction]; diff --git a/gossipd/gossipd_wire.csv b/gossipd/gossipd_wire.csv index c058a1dfb9f9..35594b63364d 100644 --- a/gossipd/gossipd_wire.csv +++ b/gossipd/gossipd_wire.csv @@ -103,6 +103,7 @@ msgdata,gossipd_local_channel_update,htlc_minimum_msat,amount_msat, msgdata,gossipd_local_channel_update,fee_base_msat,u32, msgdata,gossipd_local_channel_update,fee_proportional_millionths,u32, msgdata,gossipd_local_channel_update,htlc_maximum_msat,amount_msat, +msgdata,gossipd_local_channel_update,public,bool, # Send this channel_announcement msgtype,gossipd_local_channel_announcement,3006 diff --git a/gossipd/test/run-check_node_announcement.c b/gossipd/test/run-check_node_announcement.c index b7245bee3975..a1119f77549a 100644 --- a/gossipd/test/run-check_node_announcement.c +++ b/gossipd/test/run-check_node_announcement.c @@ -34,7 +34,7 @@ void ecdh(const struct pubkey *point UNNEEDED, struct secret *ss UNNEEDED) struct peer *find_peer(struct daemon *daemon UNNEEDED, const struct node_id *id UNNEEDED) { fprintf(stderr, "find_peer called!\n"); abort(); } /* Generated stub for fromwire_gossipd_local_channel_update */ -bool fromwire_gossipd_local_channel_update(const void *p UNNEEDED, struct node_id *id UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, bool *disable UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, struct amount_msat *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED, struct amount_msat *htlc_maximum_msat UNNEEDED) +bool fromwire_gossipd_local_channel_update(const void *p UNNEEDED, struct node_id *id UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, bool *disable UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, struct amount_msat *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED, struct amount_msat *htlc_maximum_msat UNNEEDED, bool *public UNNEEDED) { fprintf(stderr, "fromwire_gossipd_local_channel_update called!\n"); abort(); } /* Generated stub for fromwire_gossipd_used_local_channel_update */ bool fromwire_gossipd_used_local_channel_update(const void *p UNNEEDED, struct short_channel_id *scid UNNEEDED) diff --git a/gossipd/test/run-crc32_of_update.c b/gossipd/test/run-crc32_of_update.c index 0420958edd28..2a8c197f64b7 100644 --- a/gossipd/test/run-crc32_of_update.c +++ b/gossipd/test/run-crc32_of_update.c @@ -55,7 +55,7 @@ struct peer *find_peer(struct daemon *daemon UNNEEDED, const struct node_id *id bool fromwire_gossipd_dev_set_max_scids_encode_size(const void *p UNNEEDED, u32 *max UNNEEDED) { fprintf(stderr, "fromwire_gossipd_dev_set_max_scids_encode_size called!\n"); abort(); } /* Generated stub for fromwire_gossipd_local_channel_update */ -bool fromwire_gossipd_local_channel_update(const void *p UNNEEDED, struct node_id *id UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, bool *disable UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, struct amount_msat *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED, struct amount_msat *htlc_maximum_msat UNNEEDED) +bool fromwire_gossipd_local_channel_update(const void *p UNNEEDED, struct node_id *id UNNEEDED, struct short_channel_id *short_channel_id UNNEEDED, bool *disable UNNEEDED, u16 *cltv_expiry_delta UNNEEDED, struct amount_msat *htlc_minimum_msat UNNEEDED, u32 *fee_base_msat UNNEEDED, u32 *fee_proportional_millionths UNNEEDED, struct amount_msat *htlc_maximum_msat UNNEEDED, bool *public UNNEEDED) { fprintf(stderr, "fromwire_gossipd_local_channel_update called!\n"); abort(); } /* Generated stub for fromwire_gossipd_used_local_channel_update */ bool fromwire_gossipd_used_local_channel_update(const void *p UNNEEDED, struct short_channel_id *scid UNNEEDED) diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 5f3c933cdb89..23b5eb8ee17c 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -286,7 +286,7 @@ void tell_gossipd_local_channel_update(struct lightningd *ld, const u8 *msg) { struct short_channel_id scid; - bool disable; + bool disable, public; u16 cltv_expiry_delta; struct amount_msat htlc_minimum_msat; u32 fee_base_msat, fee_proportional_millionths; @@ -297,7 +297,7 @@ void tell_gossipd_local_channel_update(struct lightningd *ld, &htlc_minimum_msat, &fee_base_msat, &fee_proportional_millionths, - &htlc_maximum_msat)) { + &htlc_maximum_msat, &public)) { channel_internal_error(channel, "bad channeld_local_channel_update %s", tal_hex(channel, msg)); @@ -317,7 +317,9 @@ void tell_gossipd_local_channel_update(struct lightningd *ld, cltv_expiry_delta, htlc_minimum_msat, fee_base_msat, - fee_proportional_millionths, htlc_maximum_msat))); + fee_proportional_millionths, + htlc_maximum_msat, + public))); } void tell_gossipd_local_channel_announce(struct lightningd *ld, From e8894677eb6bd355baca2855fe1231bb33b76561 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:32 +0930 Subject: [PATCH 09/13] gossipd: batch outpoints spent, add block height. It's a bit more optimal, and tells gossipd exactly what height the spend occurred at (with multiple blocks, it's not always the current height!). It will need that next patch. Signed-off-by: Rusty Russell --- gossipd/gossipd.c | 27 ++++++++++++++++----------- gossipd/gossipd_wire.csv | 8 +++++--- lightningd/chaintopology.c | 11 +++-------- lightningd/gossip_control.c | 14 +++++++++----- lightningd/gossip_control.h | 5 +++-- 5 files changed, 36 insertions(+), 29 deletions(-) diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 26be95bd6090..395f303b4cc9 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -927,22 +927,27 @@ static void handle_new_lease_rates(struct daemon *daemon, const u8 *msg) /*~ This is where lightningd tells us that a channel's funding transaction has * been spent. */ -static void handle_outpoint_spent(struct daemon *daemon, const u8 *msg) +static void handle_outpoints_spent(struct daemon *daemon, const u8 *msg) { - struct short_channel_id scid; - struct chan *chan; + struct short_channel_id *scids; + u32 blockheight; struct routing_state *rstate = daemon->rstate; - if (!fromwire_gossipd_outpoint_spent(msg, &scid)) - master_badmsg(WIRE_GOSSIPD_OUTPOINT_SPENT, msg); - chan = get_channel(rstate, &scid); - if (chan) { + if (!fromwire_gossipd_outpoints_spent(msg, msg, &blockheight, &scids)) + master_badmsg(WIRE_GOSSIPD_OUTPOINTS_SPENT, msg); + + for (size_t i = 0; i < tal_count(scids); i++) { + struct chan *chan = get_channel(rstate, &scids[i]); + + if (!chan) + continue; + status_debug( "Deleting channel %s due to the funding outpoint being " "spent", - type_to_string(msg, struct short_channel_id, &scid)); + type_to_string(msg, struct short_channel_id, &scids[i])); /* Suppress any now-obsolete updates/announcements */ - add_to_txout_failures(rstate, &scid); + add_to_txout_failures(rstate, &scids[i]); remove_channel_from_store(rstate, chan); /* Freeing is sufficient since everything else is allocated off * of the channel and this takes care of unregistering @@ -999,8 +1004,8 @@ static struct io_plan *recv_req(struct io_conn *conn, handle_txout_reply(daemon, msg); goto done; - case WIRE_GOSSIPD_OUTPOINT_SPENT: - handle_outpoint_spent(daemon, msg); + case WIRE_GOSSIPD_OUTPOINTS_SPENT: + handle_outpoints_spent(daemon, msg); goto done; case WIRE_GOSSIPD_LOCAL_CHANNEL_CLOSE: diff --git a/gossipd/gossipd_wire.csv b/gossipd/gossipd_wire.csv index 35594b63364d..2c266584e1b0 100644 --- a/gossipd/gossipd_wire.csv +++ b/gossipd/gossipd_wire.csv @@ -42,9 +42,11 @@ msgdata,gossipd_get_txout_reply,satoshis,amount_sat, msgdata,gossipd_get_txout_reply,len,u16, msgdata,gossipd_get_txout_reply,outscript,u8,len -# master -> gossipd: a potential funding outpoint was spent, please forget the eventual channel -msgtype,gossipd_outpoint_spent,3024 -msgdata,gossipd_outpoint_spent,short_channel_id,short_channel_id, +# master -> gossipd: these potential funding outpoints were spent, please forget any channels +msgtype,gossipd_outpoints_spent,3024 +msgdata,gossipd_outpoints_spent,blockheight,u32, +msgdata,gossipd_outpoints_spent,len,u32, +msgdata,gossipd_outpoints_spent,short_channel_id,short_channel_id,len # master -> gossipd: do you have a memleak? msgtype,gossipd_dev_memleak,3033 diff --git a/lightningd/chaintopology.c b/lightningd/chaintopology.c index d521eae4c2e6..103752c46c8b 100644 --- a/lightningd/chaintopology.c +++ b/lightningd/chaintopology.c @@ -682,11 +682,7 @@ static void topo_update_spends(struct chain_topology *topo, struct block *b) * tell gossipd about them. */ spent_scids = wallet_utxoset_get_spent(tmpctx, topo->ld->wallet, b->height); - - for (size_t i=0; ibitcoind->ld, &spent_scids[i]); - } - tal_free(spent_scids); + gossipd_notify_spends(topo->bitcoind->ld, b->height, spent_scids); } static void topo_add_utxos(struct chain_topology *topo, struct block *b) @@ -792,12 +788,11 @@ static void remove_tip(struct chain_topology *topo) /* This may have unconfirmed txs: reconfirm as we add blocks. */ watch_for_utxo_reconfirmation(topo, topo->ld->wallet); block_map_del(&topo->block_map, b); - tal_free(b); /* These no longer exist, so gossipd drops any reference to them just * as if they were spent. */ - for (size_t i = 0; i < tal_count(removed_scids); i++) - gossipd_notify_spend(topo->bitcoind->ld, &removed_scids[i]); + gossipd_notify_spends(topo->bitcoind->ld, b->height, removed_scids); + tal_free(b); } static void get_new_block(struct bitcoind *bitcoind, diff --git a/lightningd/gossip_control.c b/lightningd/gossip_control.c index 23b5eb8ee17c..b4665cddb151 100644 --- a/lightningd/gossip_control.c +++ b/lightningd/gossip_control.c @@ -153,7 +153,7 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds) /* These are messages we send, not them. */ case WIRE_GOSSIPD_INIT: case WIRE_GOSSIPD_GET_TXOUT_REPLY: - case WIRE_GOSSIPD_OUTPOINT_SPENT: + case WIRE_GOSSIPD_OUTPOINTS_SPENT: case WIRE_GOSSIPD_NEW_LEASE_RATES: case WIRE_GOSSIPD_DEV_SET_MAX_SCIDS_ENCODE_SIZE: case WIRE_GOSSIPD_LOCAL_CHANNEL_CLOSE: @@ -273,11 +273,15 @@ void gossip_init(struct lightningd *ld, int connectd_fd) assert(ret == ld->gossip); } -void gossipd_notify_spend(struct lightningd *ld, - const struct short_channel_id *scid) +/* We save these so we always tell gossipd about new blockheight first. */ +void gossipd_notify_spends(struct lightningd *ld, + u32 blockheight, + const struct short_channel_id *scids) { - u8 *msg = towire_gossipd_outpoint_spent(tmpctx, scid); - subd_send_msg(ld->gossip, msg); + subd_send_msg(ld->gossip, + take(towire_gossipd_outpoints_spent(NULL, + blockheight, + scids))); } /* We unwrap, add the peer id, and send to gossipd. */ diff --git a/lightningd/gossip_control.h b/lightningd/gossip_control.h index c1d587902158..82f2643869ac 100644 --- a/lightningd/gossip_control.h +++ b/lightningd/gossip_control.h @@ -10,8 +10,9 @@ struct lightningd; void gossip_init(struct lightningd *ld, int connectd_fd); -void gossipd_notify_spend(struct lightningd *ld, - const struct short_channel_id *scid); +void gossipd_notify_spends(struct lightningd *ld, + u32 blockheight, + const struct short_channel_id *scids); void gossip_notify_new_block(struct lightningd *ld, u32 blockheight); From e434b068d7f185599918b3640df5b6b819ce6a2c Mon Sep 17 00:00:00 2001 From: Vincenzo Palazzo Date: Tue, 20 Sep 2022 20:39:42 +0100 Subject: [PATCH 10/13] lnprototest: update gossip test including 12 blocks delay Signed-off-by: Vincenzo Palazzo --- external/lnprototest | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/external/lnprototest b/external/lnprototest index 8efd45555ef3..265bac0d5809 160000 --- a/external/lnprototest +++ b/external/lnprototest @@ -1 +1 @@ -Subproject commit 8efd45555ef3d1223f3b9f6a8c009419c9510747 +Subproject commit 265bac0d5809e196c842f05b488b291a22119be1 From 5ac1a9ff761012f5f1af7f0016e36236068cd745 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:32 +0930 Subject: [PATCH 11/13] gossipd: don't close non-local channels immediately, add 12 block delay. This adds a new "chan_dying" message to the gossip_store, but since we already changed the minor version in this PR, we don't bump it again. Signed-off-by: Rusty Russell Changelog-Added: Protocol: We now delay forgetting funding-spent channels for 12 blocks (as per latest BOLTs, to support splicing in future). --- Makefile | 2 +- devtools/dump-gossipstore.c | 6 ++ gossipd/gossip_store.c | 11 ++++ gossipd/gossip_store_wire.csv | 4 ++ gossipd/gossipd.c | 19 ++----- gossipd/gossipd.h | 1 + gossipd/routing.c | 100 ++++++++++++++++++++++++++++++++-- gossipd/routing.h | 35 +++++++++--- tests/test_connection.py | 6 +- tests/test_db.py | 2 +- tests/test_gossip.py | 51 +++++++++++++++-- tests/test_invoices.py | 2 +- 12 files changed, 204 insertions(+), 35 deletions(-) diff --git a/Makefile b/Makefile index 29e3fb8a582c..6e58c15ca626 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../bolts/ -DEFAULT_BOLTVERSION := 6fee63fc342736a2eb7f6e856ae0d85807cc50ec +DEFAULT_BOLTVERSION := 47d325c6ac50587d9974651a7b2a692f85c9a068 # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/devtools/dump-gossipstore.c b/devtools/dump-gossipstore.c index 7370b9c22721..6d4d8523c40f 100644 --- a/devtools/dump-gossipstore.c +++ b/devtools/dump-gossipstore.c @@ -68,6 +68,7 @@ int main(int argc, char *argv[]) u32 msglen = be32_to_cpu(hdr.len); u8 *msg, *inner; bool deleted, push, ratelimit; + u32 blockheight; deleted = (msglen & GOSSIP_STORE_LEN_DELETED_BIT); push = (msglen & GOSSIP_STORE_LEN_PUSH_BIT); @@ -121,6 +122,11 @@ int main(int argc, char *argv[]) printf("delete channel: %s\n", type_to_string(tmpctx, struct short_channel_id, &scid)); + } else if (fromwire_gossip_store_chan_dying(msg, &scid, &blockheight)) { + printf("dying channel: %s (deadline %u)\n", + type_to_string(tmpctx, struct short_channel_id, + &scid), + blockheight); } else { warnx("Unknown message %u", fromwire_peektype(msg)); diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index 5a3e9cd27206..e1f781b455ee 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -802,6 +802,17 @@ u32 gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) chan_ann = tal_steal(gs, msg); chan_ann_off = gs->len; break; + case WIRE_GOSSIP_STORE_CHAN_DYING: { + struct short_channel_id scid; + u32 deadline; + + if (!fromwire_gossip_store_chan_dying(msg, &scid, &deadline)) { + bad = "Bad gossip_store_chan_dying"; + goto badmsg; + } + remember_chan_dying(rstate, &scid, deadline, gs->len); + break; + } case WIRE_GOSSIP_STORE_PRIVATE_UPDATE: if (!fromwire_gossip_store_private_update(tmpctx, msg, &msg)) { bad = "invalid gossip_store_private_update"; diff --git a/gossipd/gossip_store_wire.csv b/gossipd/gossip_store_wire.csv index 6781757dcec5..55e62c3bd61f 100644 --- a/gossipd/gossip_store_wire.csv +++ b/gossipd/gossip_store_wire.csv @@ -23,3 +23,7 @@ msgdata,gossip_store_delete_chan,scid,short_channel_id, msgtype,gossip_store_ended,4105 msgdata,gossip_store_ended,equivalent_offset,u64, + +msgtype,gossip_store_chan_dying,4106 +msgdata,gossip_store_chan_dying,scid,short_channel_id, +msgdata,gossip_store_chan_dying,blockheight,u32, diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c index 395f303b4cc9..23b47385d1c2 100644 --- a/gossipd/gossipd.c +++ b/gossipd/gossipd.c @@ -797,6 +797,8 @@ static void new_blockheight(struct daemon *daemon, const u8 *msg) i--; } + routing_expire_channels(daemon->rstate, daemon->current_blockheight); + daemon_conn_send(daemon->master, take(towire_gossipd_new_blockheight_reply(NULL))); } @@ -931,28 +933,19 @@ static void handle_outpoints_spent(struct daemon *daemon, const u8 *msg) { struct short_channel_id *scids; u32 blockheight; - struct routing_state *rstate = daemon->rstate; if (!fromwire_gossipd_outpoints_spent(msg, msg, &blockheight, &scids)) master_badmsg(WIRE_GOSSIPD_OUTPOINTS_SPENT, msg); for (size_t i = 0; i < tal_count(scids); i++) { - struct chan *chan = get_channel(rstate, &scids[i]); + struct chan *chan = get_channel(daemon->rstate, &scids[i]); if (!chan) continue; - status_debug( - "Deleting channel %s due to the funding outpoint being " - "spent", - type_to_string(msg, struct short_channel_id, &scids[i])); - /* Suppress any now-obsolete updates/announcements */ - add_to_txout_failures(rstate, &scids[i]); - remove_channel_from_store(rstate, chan); - /* Freeing is sufficient since everything else is allocated off - * of the channel and this takes care of unregistering - * the channel */ - free_chan(rstate, chan); + /* We have a current_blockheight, but it's not necessarily + * updated first. */ + routing_channel_spent(daemon->rstate, blockheight, chan); } } diff --git a/gossipd/gossipd.h b/gossipd/gossipd.h index 2129375f7e8b..650d87a5c66f 100644 --- a/gossipd/gossipd.h +++ b/gossipd/gossipd.h @@ -16,6 +16,7 @@ struct channel_update_timestamps; struct broadcastable; struct lease_rates; struct seeker; +struct dying_channel; /*~ The core daemon structure: */ struct daemon { diff --git a/gossipd/routing.c b/gossipd/routing.c index 08aef559ebf9..efffaed0e79e 100644 --- a/gossipd/routing.c +++ b/gossipd/routing.c @@ -33,6 +33,16 @@ struct pending_node_announce { struct peer *peer_softref; }; +/* As per the below BOLT #7 quote, we delay forgetting a channel until 12 + * blocks after we see it close. This gives time for splicing (or even other + * opens) to replace the channel, and broadcast it after 6 blocks. */ +struct dying_channel { + struct short_channel_id scid; + u32 deadline_blockheight; + /* Where the dying_channel marker is in the store. */ + struct broadcastable marker; +}; + /* We consider a reasonable gossip rate to be 2 per day, with burst of * 4 per day. So we use a granularity of one hour. */ #define TOKENS_PER_MSG 12 @@ -249,8 +259,8 @@ static void txout_failure_age(struct routing_state *rstate) txout_failure_age, rstate); } -void add_to_txout_failures(struct routing_state *rstate, - const struct short_channel_id *scid) +static void add_to_txout_failures(struct routing_state *rstate, + const struct short_channel_id *scid) { if (uintmap_add(&rstate->txout_failures, scid->u64, true) && ++rstate->num_txout_failures == 10000) { @@ -288,6 +298,7 @@ struct routing_state *new_routing_state(const tal_t *ctx, rstate->gs = gossip_store_new(rstate, peers); rstate->local_channel_announced = false; rstate->last_timestamp = 0; + rstate->dying_channels = tal_arr(rstate, struct dying_channel, 0); pending_cannouncement_map_init(&rstate->pending_cannouncements); @@ -852,8 +863,8 @@ static void delete_chan_messages_from_store(struct routing_state *rstate, &chan->half[1].bcast, update_type); } -void remove_channel_from_store(struct routing_state *rstate, - struct chan *chan) +static void remove_channel_from_store(struct routing_state *rstate, + struct chan *chan) { /* Put in tombstone marker */ gossip_store_mark_channel_deleted(rstate->gs, &chan->scid); @@ -2087,3 +2098,84 @@ void remove_all_gossip(struct routing_state *rstate) /* Freeing unupdated chanmaps should empty this */ assert(pending_node_map_first(rstate->pending_node_map, &pnait) == NULL); } + +static void channel_spent(struct routing_state *rstate, + struct chan *chan STEALS) +{ + status_debug("Deleting channel %s due to the funding outpoint being " + "spent", + type_to_string(tmpctx, struct short_channel_id, + &chan->scid)); + /* Suppress any now-obsolete updates/announcements */ + add_to_txout_failures(rstate, &chan->scid); + remove_channel_from_store(rstate, chan); + /* Freeing is sufficient since everything else is allocated off + * of the channel and this takes care of unregistering + * the channel */ + free_chan(rstate, chan); +} + +void routing_expire_channels(struct routing_state *rstate, u32 blockheight) +{ + struct chan *chan; + + for (size_t i = 0; i < tal_count(rstate->dying_channels); i++) { + struct dying_channel *d = rstate->dying_channels + i; + + if (blockheight < d->deadline_blockheight) + continue; + chan = get_channel(rstate, &d->scid); + if (chan) + channel_spent(rstate, chan); + /* Delete dying marker itself */ + gossip_store_delete(rstate->gs, + &d->marker, WIRE_GOSSIP_STORE_CHAN_DYING); + tal_arr_remove(&rstate->dying_channels, i); + i--; + } +} + +void remember_chan_dying(struct routing_state *rstate, + const struct short_channel_id *scid, + u32 deadline_blockheight, + u64 index) +{ + struct dying_channel d; + d.scid = *scid; + d.deadline_blockheight = deadline_blockheight; + d.marker.index = index; + tal_arr_expand(&rstate->dying_channels, d); +} + +void routing_channel_spent(struct routing_state *rstate, + u32 current_blockheight, + struct chan *chan) +{ + u64 index; + u32 deadline; + u8 *msg; + + /* FIXME: We should note that delay is not necessary (or even + * sensible) for local channels! */ + if (local_direction(rstate, chan, NULL)) { + channel_spent(rstate, chan); + return; + } + + /* BOLT #7: + * - once its funding output has been spent OR reorganized out: + * - SHOULD forget a channel after a 12-block delay. + */ + deadline = current_blockheight + 12; + + /* Save to gossip_store in case we restart */ + msg = towire_gossip_store_chan_dying(tmpctx, &chan->scid, deadline); + index = gossip_store_add(rstate->gs, msg, 0, false, false, NULL); + + /* Remember locally so we can kill it in 12 blocks */ + status_debug("channel %s closing soon due" + " to the funding outpoint being spent", + type_to_string(msg, struct short_channel_id, &chan->scid)); + remember_chan_dying(rstate, &chan->scid, deadline, index); +} + diff --git a/gossipd/routing.h b/gossipd/routing.h index e9a2bc0f6ae6..ea7feabe75cc 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -230,6 +230,9 @@ struct routing_state { /* Highest timestamp of gossip we accepted (before now) */ u32 last_timestamp; + /* Channels which are closed, but we're waiting 12 blocks */ + struct dying_channel *dying_channels; + #if DEVELOPER /* Override local time for gossip messages */ struct timeabs *gossip_time; @@ -399,21 +402,37 @@ bool routing_add_private_channel(struct routing_state *rstate, */ struct timeabs gossip_time_now(const struct routing_state *rstate); +/** + * Add to rstate->dying_channels + * + * Exposed here for when we load the gossip_store. + */ +void remember_chan_dying(struct routing_state *rstate, + const struct short_channel_id *scid, + u32 deadline_blockheight, + u64 index); + +/** + * When a channel's funding has been spent. + */ +void routing_channel_spent(struct routing_state *rstate, + u32 current_blockheight, + struct chan *chan); + +/** + * Clean up any dying channels. + * + * This finally deletes channel past their deadline. + */ +void routing_expire_channels(struct routing_state *rstate, u32 blockheight); + /* Would we ratelimit a channel_update with this timestamp? */ bool would_ratelimit_cupdate(struct routing_state *rstate, const struct half_chan *hc, u32 timestamp); -/* Remove channel from store: announcement and any updates. */ -void remove_channel_from_store(struct routing_state *rstate, - struct chan *chan); - /* Returns an error string if there are unfinalized entries after load */ const char *unfinalized_entries(const tal_t *ctx, struct routing_state *rstate); void remove_all_gossip(struct routing_state *rstate); - -/* This scid is dead to us. */ -void add_to_txout_failures(struct routing_state *rstate, - const struct short_channel_id *scid); #endif /* LIGHTNING_GOSSIPD_ROUTING_H */ diff --git a/tests/test_connection.py b/tests/test_connection.py index 3ed032d4a81e..38884edd5082 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -4085,7 +4085,7 @@ def test_multichan(node_factory, executor, bitcoind): l2.rpc.close(l3.info['id']) l2.rpc.close(scid23b) - bitcoind.generate_block(1, wait_for_mempool=1) + bitcoind.generate_block(13, wait_for_mempool=1) sync_blockheight(bitcoind, [l1, l2, l3]) # Gossip works as expected. @@ -4130,14 +4130,14 @@ def test_multichan(node_factory, executor, bitcoind): "state": "RCVD_REMOVE_ACK_REVOCATION"}, {"short_channel_id": scid12, "id": 2, - "expiry": 123, + "expiry": 135, "direction": "out", "amount_msat": Millisatoshi(100001001), "payment_hash": inv3['payment_hash'], "state": "RCVD_REMOVE_ACK_REVOCATION"}, {"short_channel_id": scid12, "id": 3, - "expiry": 123, + "expiry": 135, "direction": "out", "amount_msat": Millisatoshi(100001001), "payment_hash": inv4['payment_hash'], diff --git a/tests/test_db.py b/tests/test_db.py index 677a5d290dfb..414bc066c89e 100644 --- a/tests/test_db.py +++ b/tests/test_db.py @@ -96,7 +96,7 @@ def test_block_backfill(node_factory, bitcoind, chainparams): # Now close the channel and make sure `l3` cleans up correctly: txid = l1.rpc.close(l2.info['id'])['txid'] - bitcoind.generate_block(1, wait_for_mempool=txid) + bitcoind.generate_block(13, wait_for_mempool=txid) wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 0) diff --git a/tests/test_gossip.py b/tests/test_gossip.py index 959baac91b6f..a1cccf215630 100644 --- a/tests/test_gossip.py +++ b/tests/test_gossip.py @@ -559,8 +559,9 @@ def non_public(node): # channel from their network view l1.rpc.dev_fail(l2.info['id']) - # We need to wait for the unilateral close to hit the mempool - bitcoind.generate_block(1, wait_for_mempool=1) + # We need to wait for the unilateral close to hit the mempool, + # and 12 blocks for nodes to actually forget it. + bitcoind.generate_block(13, wait_for_mempool=1) wait_for(lambda: active(l1) == [scid23, scid23]) wait_for(lambda: active(l2) == [scid23, scid23]) @@ -1391,7 +1392,7 @@ def test_gossip_notices_close(node_factory, bitcoind): txid = l2.rpc.close(l3.info['id'])['txid'] wait_for(lambda: only_one(l2.rpc.listpeers(l3.info['id'])['peers'])['channels'][0]['state'] == 'CLOSINGD_COMPLETE') - bitcoind.generate_block(1, txid) + bitcoind.generate_block(13, txid) wait_for(lambda: l1.rpc.listchannels()['channels'] == []) wait_for(lambda: l1.rpc.listnodes()['nodes'] == []) @@ -2097,7 +2098,7 @@ def test_topology_leak(node_factory, bitcoind): # Close and wait for gossip to catchup. txid = l2.rpc.close(l3.info['id'])['txid'] - bitcoind.generate_block(1, txid) + bitcoind.generate_block(13, txid) wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 2) @@ -2122,3 +2123,45 @@ def test_parms_listforwards(node_factory): assert len(forwards_new) == 0 assert len(forwards_dep) == 0 + + +@pytest.mark.developer("gossip without DEVELOPER=1 is slow") +def test_close_12_block_delay(node_factory, bitcoind): + l1, l2, l3, l4 = node_factory.line_graph(4, wait_for_announce=True) + + # Close l1-l2 + txid = l1.rpc.close(l2.info['id'])['txid'] + bitcoind.generate_block(1, txid) + + # But l4 doesn't believe it immediately. + l4.daemon.wait_for_log("channel .* closing soon due to the funding outpoint being spent") + + # Close l2-l3 one block later. + txid = l2.rpc.close(l3.info['id'])['txid'] + bitcoind.generate_block(1, txid) + l4.daemon.wait_for_log("channel .* closing soon due to the funding outpoint being spent") + + # BOLT #7: + # - once its funding output has been spent OR reorganized out: + # - SHOULD forget a channel after a 12-block delay. + + # That implies 12 blocks *after* spending, i.e. 13 blocks deep! + + # 12 blocks deep, l4 still sees it + bitcoind.generate_block(10) + sync_blockheight(bitcoind, [l4]) + assert len(l4.rpc.listchannels(source=l1.info['id'])['channels']) == 1 + + # 13 blocks deep does it. + bitcoind.generate_block(1) + wait_for(lambda: l4.rpc.listchannels(source=l1.info['id'])['channels'] == []) + + # Other channel still visible. + assert len(l4.rpc.listchannels(source=l2.info['id'])['channels']) == 1 + + # Restart: it remembers channel is dying. + l4.restart() + + # One more block, it's forgotten too. + bitcoind.generate_block(1) + wait_for(lambda: l4.rpc.listchannels(source=l2.info['id'])['channels'] == []) diff --git a/tests/test_invoices.py b/tests/test_invoices.py index dbb9ab7a4da7..195864d44aa1 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -363,7 +363,7 @@ def test_invoice_routeboost_private(node_factory, bitcoind): # It will use an explicit exposeprivatechannels even if it thinks its a dead-end l0.rpc.close(l1.info['id']) l0.wait_for_channel_onchain(l1.info['id']) - bitcoind.generate_block(1) + bitcoind.generate_block(13) wait_for(lambda: l2.rpc.listchannels(scid_dummy)['channels'] == []) inv = l2.rpc.invoice(amount_msat=123456, label="inv7", description="?", exposeprivatechannels=scid) From 859e3e2f263928280b9483f837d80cd4eca927df Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 24 Sep 2022 09:07:37 +0930 Subject: [PATCH 12/13] BOLT: update to clarify HTLC tx amount calculation. Simple quote update. Signed-off-by: Rusty Russell --- Makefile | 2 +- common/htlc_tx.c | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 6e58c15ca626..54913ec021bc 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ CCANDIR := ccan # Where we keep the BOLT RFCs BOLTDIR := ../bolts/ -DEFAULT_BOLTVERSION := 47d325c6ac50587d9974651a7b2a692f85c9a068 +DEFAULT_BOLTVERSION := f32c6ddb5f11b431c9bb4f501cdec604172a90de # Can be overridden on cmdline. BOLTVERSION := $(DEFAULT_BOLTVERSION) diff --git a/common/htlc_tx.c b/common/htlc_tx.c index e80ea1375813..5be4b47f2ba2 100644 --- a/common/htlc_tx.c +++ b/common/htlc_tx.c @@ -52,8 +52,9 @@ static struct bitcoin_tx *htlc_tx(const tal_t *ctx, /* BOLT #3: * * txout count: 1 - * * `txout[0]` amount: the HTLC amount minus fees - * (see [Fee Calculation](#fee-calculation)) + * * `txout[0]` amount: the HTLC `amount_msat` divided by 1000 + * (rounding down) minus fees in satoshis (see + * [Fee Calculation](#fee-calculation)) * * `txout[0]` script: version-0 P2WSH with witness script as shown * below */ From 101b00834407f30d636f5ee164d73cccde9d13bc Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 24 Sep 2022 09:07:38 +0930 Subject: [PATCH 13/13] doc/GOSSIP_STORE.md: document the gossip_store file format. Mainly for @vincenzopalazzo who is thinking of writing a Rust version! Signed-off-by: Rusty Russell --- doc/GOSSIP_STORE.md | 146 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 doc/GOSSIP_STORE.md diff --git a/doc/GOSSIP_STORE.md b/doc/GOSSIP_STORE.md new file mode 100644 index 000000000000..54a6728eb757 --- /dev/null +++ b/doc/GOSSIP_STORE.md @@ -0,0 +1,146 @@ +# gossip_store: Direct Access To Lightning Gossip + +Hi! + +The lightning_gossipd dameon stores the gossip messages, along with +some internal data, in a file called the "gossip_store". Various +plugins and daemons access this (in a read-only manner), and the +format is documented here. + +## The File Header + +``` +u8 version; +``` + +The gossmap header consists of one byte. The top 3 bits are the major +version: if these are not all zero, you need to re-read this (updated) +document to see what changed. The lower 5 bits are the minor version, +which won't worry you: currently they will be 11. + +After the file header comes a number of records. + +## The Record Header + +``` +be16 flags; +be16 len; +be32 crc; +be32 timestamp; +``` + +Each record consists of a header and a message. The header is +big-endian, containing flags, the length (of the following body), the +crc32c (of the following message, starting with the timestamp field in +the header) and a timestamp extracted from certain messages (zero +where not relevant, but ignore it in those cases). + +The flags currently defined are: + +``` +#define DELETED 0x8000 +#define PUSH 0x4000 +#define RATELIMIT 0x2000 +``` + +Deleted fields should be ignored: on restart, they will be removed as +the gossip_store is rewritten. + +The push flag indicates gossip which is generated locally: this is +important for gossip timestamp filtering, where peers request gossip +and we always send our own gossip messages even if the timestamp +wasn't within their request. + +The ratelimit flag indicates that this gossip message came too fast: +we record it, but don't relay it to peers. + +Other flags should be ignored. + +## The Message + +Each messages consists of a 16-bit big-endian "type" field (for +efficiency, an implementation may read this along with the header), +and optional data. Some messages are defined by the BOLT 7 gossip +protocol, others are for internal use. Unknown message types should be +skipped over. + +### BOLT 7 Messages + +These are the messages which gossipd has validated, and ensured are in +order. + +* `channel_announcement` (256): a complete, validated channel announcement. This will always come before any `channel_update` which refers to it, or `node_announcement` which refers to a node. +* `channel_update` (258): a complete, validated channel update. Note that you can see multiple of these (old ones will be deleted as they are replaced though). +* `node_announcement` (257): a complete, validated node announcement. Note that you can also see multiple of these (old ones will be deleted as they are replaced). + +### Internal Gossip Daemon Messages + +These messages contain additional data, which may be useful. + +* `gossip_store_channel_amount` (4101) + * `satoshis`: u64 + +This always immediately follows `channel_announcement` messages, and +contains the actual capacity of the channel. + +* `gossip_store_private_channel` (4104) + * `amount_sat`: u64 + * `len`: u16 + * `announcement`: u8[len] + +This contains information about a private (could be made public +later!) channel, with announcement in the same format as a normal +`channel_announcement` with invalid signatures. + +* `gossip_store_private_update` (4102) + * `len`: u16 + * `update`: u8[len] + +This contains a private `channel_update` (i.e. for a channel described +by `gossip_store_private_channel`. + +* `gossip_store_delete_chan` (4103) + * `scid`: u64 + +This is added when a channel is deleted. You won't often see this if +you're reading the file once (as the channel record header will have +been marked `deleted` first), but useful if you are polling the file +for updates. + +* `gossip_store_ended` (4105) + * `equivalent_offset`: u64 + +This is only ever added as the final entry in the gossip_store. It +means the file has been deleted (usually because lightningd has been +restarted), and you should re-open it. As an optimization, the +`equivalent_offset` in the new file reflects the point at which the +new gossip_store is equivalent to this one (with deleted records +removed). However, if lightningd has been restarted multiple times it +is possible that this offset is not valid, so it's really only useful +if you're actively monitoring the file. + +* `gossip_store_chan_dying` (4106) + * `scid`: u64 + * `blockheight`: u32 + +This is placed in the gossip_store file when a funding transaction is +spent. `blockheight` is set to 12 blocks beyond the block containing +the spend: at this point, gossipd will delete the channel. + +## Using the Gossip Store File + +- Always check the major version number! We will increment it if the format + changes in a way that breaks readers. +- Ignore unknown flags in the header. +- Ignore message types you don't know. +- You don't need to check the messages, as they have been validated. +- It is possible to see a partially-written record at the end. Ignore it. + +If you are keeping the file open to watch for changes: + +- The file is append-only, so you can simply try reading more records + using inotify (or equivalent) or simply checking every few seconds. +- If you see a `gossip_store_ended` message, reopen the file. + +Happy hacking! +Rusty.