From 6338758018ee5b199c56fbae8ffb09813d94dd6c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 14 Sep 2022 13:20:31 +0930 Subject: [PATCH] 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 c4f6f52bd..514f32096 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 5d25b1122..e64fd983f 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 3a1488c49..8ced38d36 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 33b3668c3..398200d88 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 f58d49add..4d72209e8 100755 --- a/contrib/pyln-client/pyln/client/gossmap.py +++ b/contrib/pyln-client/pyln/client/gossmap.py @@ -9,13 +9,12 @@ import io 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 @@ class Gossmap(object): 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 28d6f0843..d72de28e0 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 1d5100d0a..7370b9c22 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); - printf("GOSSIP VERSION %u\n", version); + /* 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/%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 29d3bc110..64bc313bb 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 82dfdf7eb..52faac965 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]);