From: Matt Corallo <git@bluematt.me>
Date: Tue, 9 Nov 2021 21:22:47 +0000 (+0000)
Subject: Correct Channel type serialization logic
X-Git-Tag: v0.0.104~36^2
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=a44587d9aabe45b5ad419996ec7dbe3e5ddd724b;p=rust-lightning

Correct Channel type serialization logic

Currently, we write out the Channel's `ChannelTypeFeatures` as an
odd type, implying clients which don't understand the
`ChannelTypeFeatures` field can simply ignore it. This is obviously
nonsense if the channel type is some future version - the client
needs to fail to deserialize as it doesn't understand the channel's
type.

We adapt the serialization logic here to only write out the
`ChannelTypeFeatures` field if it is something other than
only-static-remote-key, and simply consider that "default" (as it
is the only supported type today). Then, we write out the channel
type as an even TLV, implying clients which do not understand it
must fail to read the `Channel`.

Note that we do not need to bother reserving the TLV type no longer
written as it never appeared in a release (merged post-0.0.103).
---

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index e39478bd7..01d073549 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -5254,6 +5254,13 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			htlc.write(writer)?;
 		}
 
+		// If the channel type is something other than only-static-remote-key, then we need to have
+		// older clients fail to deserialize this channel at all. If the type is
+		// only-static-remote-key, we simply consider it "default" and don't write the channel type
+		// out at all.
+		let chan_type = if self.channel_type != ChannelTypeFeatures::only_static_remote_key() {
+			Some(&self.channel_type) } else { None };
+
 		write_tlv_fields!(writer, {
 			(0, self.announcement_sigs, option),
 			// minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
@@ -5263,12 +5270,12 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 			// and new versions map the default values to None and allow the TLV entries here to
 			// override that.
 			(1, self.minimum_depth, option),
+			(2, chan_type, option),
 			(3, self.counterparty_selected_channel_reserve_satoshis, option),
 			(5, self.config, required),
 			(7, self.shutdown_scriptpubkey, option),
 			(9, self.target_closing_feerate_sats_per_kw, option),
 			(11, self.monitor_pending_finalized_fulfills, vec_type),
-			(13, self.channel_type, required),
 		});
 
 		Ok(())
@@ -5509,12 +5516,12 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
 			(1, minimum_depth, option),
+			(2, channel_type, option),
 			(3, counterparty_selected_channel_reserve_satoshis, option),
 			(5, config, option), // Note that if none is provided we will *not* overwrite the existing one.
 			(7, shutdown_scriptpubkey, option),
 			(9, target_closing_feerate_sats_per_kw, option),
 			(11, monitor_pending_finalized_fulfills, vec_type),
-			(13, channel_type, option),
 		});
 
 		let chan_features = channel_type.as_ref().unwrap();