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 <[email protected]> Changelog-Changed: Protocol: We now require all channel_update messages include htlc_maximum_msat (as per latest BOLTs)
richur · Sep 24, 2022 · 253b255 · 253b255
1 parent daa5269
commit 253b255
Show file tree

Hide file tree

Showing 14 changed files with 80 additions and 172 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,
 				    &timestamp, &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
@@ -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,26 +133,15 @@ 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,
 			 opts->cltv_expiry_delta,
 			 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
@@ -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, &timestamp,
 				     &message_flags, &channel_flags,

diff --git 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, &timestamp,
-				     &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, &timestamp,
 				&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,
 				     &timestamp, &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
@@ -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, &timestamp, &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
@@ -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
@@ -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
@@ -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,