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 <[email protected]>
johnongit · Sep 24, 2022 · 6338758 · 6338758
1 parent 3817a69
commit 6338758
Show file tree

Hide file tree

Showing 9 changed files with 51 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -10,6 +10,10 @@
 #include <unistd.h>
 #include <wire/peer_wire.h>
 
+/* 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
@@ -17,6 +17,8 @@
 #include <wire/peer_wire.h>
 
 #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
@@ -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]);