devtools/create-gossipstore: cleanups

added sanity check to make sure scid of csv is the same as scid in gossip.
Revised style, mem allocation, and error checks

[ Minor fixups, and updated benchmark script -- RR ]

With data.tar.gz: 456609740 Apr  2 12:33

store_load_msec:35300-42354(37118.2+/-2.7e+03)
vsz_kb:582832
store_rewrite_sec:12.700000-13.430000(12.988+/-0.27)
listnodes_sec:3.000000-3.160000(3.076+/-0.057)
listchannels_sec:30.790000-31.690000(31.03+/-0.34)
routing_sec:0.00
peer_write_all_sec:63.640000-67.860000(66.294+/-1.4)

devtools/Makefile

@@ -43,7 +43,7 @@ devtools/dump-gossipstore: $(DEVTOOLS_OBJS) $(DEVTOOLS_COMMON_OBJS) $(JSMN_OBJS)
 devtools/dump-gossipstore.o: gossipd/gen_gossip_store.h
 
 devtools/create-gossipstore: $(DEVTOOLS_OBJS) $(DEVTOOLS_COMMON_OBJS) $(JSMN_OBJS) $(CCAN_OBJS) $(BITCOIN_OBJS) wire/fromwire.o wire/towire.o devtools/create-gossipstore.o gossipd/gen_gossip_store.o
-devtools/create-gossipstore.o: gossipd/gen_gossip_store.h devtools/create-gossipstore.h
+devtools/create-gossipstore.o: gossipd/gen_gossip_store.h
 
 devtools/onion.c: ccan/config.h
 

devtools/create-gossipstore.c

@@ -1,37 +1,52 @@
+#include <bitcoin/short_channel_id.h>
 #include <ccan/crc/crc.h>
 #include <ccan/err/err.h>
 #include <ccan/opt/opt.h>
 #include <ccan/read_write_all/read_write_all.h>
+#include <common/amount.h>
 #include <common/type_to_string.h>
 #include <common/utils.h>
-#include <devtools/create-gossipstore.h>
 #include <fcntl.h>
 #include <gossipd/gen_gossip_store.h>
 #include <gossipd/gossip_store.h>
 #include <inttypes.h>
+#include <stdio.h>
+#include <stdlib.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <wire/gen_peer_wire.h>
 
 
+struct scidsat {
+	struct short_channel_id scid;
+        struct amount_sat sat;
+} scidsat;
 
-struct scidsat * load_scid_file(FILE * scidfd)
+/* read scid,satoshis csv file and create return an array of scidsat pointers */
+static struct scidsat *load_csv_file(FILE *scidf)
 {
-        int n, ok;
+        int n, r;
-        ok = fscanf(scidfd, "%d\n", &n);
+	char title[15];
-	if (ok == EOF)
-		return NULL;
-	char title[16];
-        ok = fscanf(scidfd, "%s\n", title);
-	if (ok == EOF)
-		return NULL;
-	struct scidsat * scids = calloc(n, sizeof(scidsat));
 	int i = 0;
-        while(fscanf(scidfd, "%s ,%" SCNu64 "\n", scids[i].scid, &scids[i].sat.satoshis) == 2 ) { /* Raw: read from file */
+	struct scidsat *scidsats;
-			i++;
+        /* max characters is 8 (0xffffff) + 8 for tx + 5 (0xffffff) for outputs (0xffff) + 2 (x's) */
+        char str[23];
+
+        if (fscanf(scidf, "%d\n", &n) != 1)
+                err(1, "reading number of entries from csv failed");
+
+	scidsats = tal_arr(NULL, struct scidsat, n);
+        r = fscanf(scidf, "%5s ,%8s\n", title, &title[6]);
+	if (r != 2 || strcmp(title, "scid") != 0 || strcmp(&title[6], "satoshis") != 0)
+		err(1, "reading 'scid ,satoshis' from csv failed");
+
+        while(fscanf(scidf, "%s ,%ld\n", str, &scidsats[i].sat.satoshis) == 2 ) { /* Raw: read from file */
+		if (!short_channel_id_from_str(str, strlen(str), &scidsats[i].scid, 0))
+			err(1, "failed to make scid struct");
+		i++;
 	}
-	return scids;
+	return scidsats;
 }
 
 int main(int argc, char *argv[])
@@ -40,10 +55,9 @@ int main(int argc, char *argv[])
 	beint16_t be_inlen;
 	struct amount_sat sat;
 	bool verbose = false;
-	char *infile = NULL, *outfile = NULL, *scidfile = NULL, *csat = NULL;
+	char *infile = NULL, *outfile = NULL, *csvfile = NULL, *csat = NULL;
-	int infd = 0, outfd;
+	int infd, outfd, scidi = 0, channels = 0, nodes = 0, updates = 0;
-       	FILE * scidfd;
+	struct scidsat *scidsats;
-	struct scidsat * scids = NULL;
 	unsigned max = -1U;
 
 	setup_locale();
@@ -54,10 +68,10 @@ int main(int argc, char *argv[])
 			 "Send output to this file instead of stdout");
 	opt_register_arg("--input|-i", opt_set_charp, NULL, &infile,
 			 "Read input from this file instead of stdin");
-	opt_register_arg("--scidfile", opt_set_charp, NULL, &scidfile,
+	opt_register_arg("--csv", opt_set_charp, NULL, &csvfile,
 			 "Input for 'scid, satshis' csv");
 	opt_register_arg("--sat", opt_set_charp, NULL, &csat,
-			 "default satoshi value if --scidfile flag not present");
+			 "default satoshi value if --csv flag not present");
 	opt_register_arg("--max", opt_set_uintval, opt_show_uintval, &max,
 			 "maximum number of messages to read");
 	opt_register_noarg("--help|-h", opt_usage_and_exit,
@@ -67,20 +81,20 @@ int main(int argc, char *argv[])
 	opt_parse(&argc, argv, opt_log_stderr_exit);
 
 
-        if (scidfile) {
+        if (csvfile && !csat) {
-		scidfd = fopen(scidfile, "r");
+       	        FILE *scidf;
-		if (scidfd < 0)
+		scidf = fopen(csvfile, "r");
-			err(1, "opening %s", scidfile);
+		if (!scidf)
-                scids = load_scid_file(scidfd);
+			err(1, "opening %s", csvfile);
-	        fclose(scidfd);
+                scidsats = load_csv_file(scidf);
-	}
+	        fclose(scidf);
-	else if (csat) {
+	} else if (csat && !csvfile) {
 		if (!parse_amount_sat(&sat, csat, strlen(csat))) {
 		        errx(1, "Invalid satoshi amount %s", csat);
 		}
 	}
 	else {
-		err(1, "must contain either --sat xor --scidfile");
+		err(1, "must contain either --sat xor --csv");
 	}
 
 	if (infile) {
@@ -100,10 +114,6 @@ int main(int argc, char *argv[])
 	if (!write_all(outfd, &version, sizeof(version)))
 		err(1, "Writing version");
 
-	int scidi = 0;
-	int channels = 0;
-	int nodes = 0;
-	int updates = 0;
 	while (read_all(infd, &be_inlen, sizeof(be_inlen))) {
 		u32 msglen = be16_to_cpu(be_inlen);
 		u8 *inmsg = tal_arr(NULL, u8, msglen), *outmsg;
@@ -115,12 +125,35 @@ int main(int argc, char *argv[])
 
 		switch (fromwire_peektype(inmsg)) {
 		case WIRE_CHANNEL_ANNOUNCEMENT:
-			if (scids) {
+			if (csvfile) {
-				sat = scids[scidi].sat;
+				struct short_channel_id scid;
-				scidi += 1;
+				/* We ignore these; we just want scid */
+				secp256k1_ecdsa_signature sig;
+				u8 *features;
+				struct bitcoin_blkid hash;
+				struct node_id id;
+				struct pubkey pubkey;
+
+				if (!fromwire_channel_announcement(inmsg,
+								   inmsg,
+								   &sig,
+								   &sig,
+								   &sig,
+								   &sig,
+								   &features,
+								   &hash,
+								   &scid,
+								   &id,
+								   &id,
+								   &pubkey,
+								   &pubkey))
+					errx(1, "bad channel_announcement");
+			        if (!short_channel_id_eq(&scid, &scidsats[scidi].scid))
+			        	errx(1, "scid of message does not match scid in csv");
+				scidi++;
 			}
-			channels += 1;
 			outmsg = towire_gossip_store_channel_announcement(inmsg, inmsg, sat);
+			channels += 1;
 			break;
 		case WIRE_CHANNEL_UPDATE:
 			outmsg = towire_gossip_store_channel_update(inmsg, inmsg);
@@ -153,6 +186,7 @@ int main(int argc, char *argv[])
 			break;
 	}
 	fprintf(stderr, "channels %d, updates %d, nodes %d\n", channels, updates, nodes);
-	free(scids);
+	if (csvfile)
+                tal_free(scidsats);
 	return 0;
 }

devtools/create-gossipstore.h

@@ -1,13 +0,0 @@
-#ifndef LIGHTNING_DEVTOOLS_CREATE_GOSSIPSTORE_H
-#define LIGHTNING_DEVTOOLS_CREATE_GOSSIPSTORE_H
-#include <stdlib.h>
-#include <stdio.h>
-#include <common/amount.h>
-
-struct scidsat {
-	char scid[17];
-        struct amount_sat sat;
-} scidsat;
-
-struct scidsat * load_scid_file(FILE * scidfd);
-#endif /* LIGHTNING_DEVTOOLS_CREATE_GOSSIPSTORE_H */

tools/bench-gossipd.sh

@@ -82,7 +82,7 @@ if [ -z "$DIR" ]; then
     trap 'rm -rf "$DIR"' 0
 
     DIR="$(mktemp -d)"
-    ./devtools/create-gossipstore 100000 -i "$MCP_DIR"/1M.gossip -o "$DIR"/gossip_store
+    ./devtools/create-gossipstore --csv "$MCP_DIR"/scidSatoshis.csv -i "$MCP_DIR"/1M.gossip -o "$DIR"/gossip_store
 fi
 
 # shellcheck disable=SC2086