Support serializing TLV fields which may or may not be present

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index bbb55fc22e7f81a583fc0b1e7707dce0e93cfc14..27403353de5b422f71c1e732924c102e2971b80d 100644 (file)
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -988,7 +988,7 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
                self.lockdown_from_offchain.write(writer)?;
                self.holder_tx_signed.write(writer)?;
 
-               write_tlv_fields!(writer, {});
+               write_tlv_fields!(writer, {}, {});
 
                Ok(())
        }

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 7da38b7aed656cac727c6f29ebfbfb0bb9a798bb..88e853b498e210ca8310628806ab253c861218c6 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -4585,7 +4585,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
                self.channel_update_status.write(writer)?;
 
-               write_tlv_fields!(writer, {});
+               write_tlv_fields!(writer, {}, {});
 
                Ok(())
        }

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 4919a225aacc2133c1623d245c5bc83c4be6c05f..e06213c78e8d35cc35a8dc21adf9ffa3278b137c 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -4514,7 +4514,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
                        session_priv.write(writer)?;
                }
 
-               write_tlv_fields!(writer, {});
+               write_tlv_fields!(writer, {}, {});
 
                Ok(())
        }

diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
index 3c1a549ada269adb56220aa3c021be3b9ec2b202..94136a9ee65cb16fb66d292ea0f5e4bcf714b9b8 100644 (file)
--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -1285,20 +1285,17 @@ impl Writeable for OnionHopData {
                                        (2, HighZeroBytesDroppedVarInt(self.amt_to_forward)),
                                        (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)),
                                        (6, short_channel_id)
-                               });
+                               }, { });
                        },
-                       OnionHopDataFormat::FinalNode { payment_data: Some(ref final_data) } => {
-                               if final_data.total_msat > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); }
-                               encode_varint_length_prefixed_tlv!(w, {
-                                       (2, HighZeroBytesDroppedVarInt(self.amt_to_forward)),
-                                       (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)),
-                                       (8, final_data)
-                               });
-                       },
-                       OnionHopDataFormat::FinalNode { payment_data: None } => {
+                       OnionHopDataFormat::FinalNode { ref payment_data } => {
+                               if let Some(final_data) = payment_data {
+                                       if final_data.total_msat > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); }
+                               }
                                encode_varint_length_prefixed_tlv!(w, {
                                        (2, HighZeroBytesDroppedVarInt(self.amt_to_forward)),
                                        (4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value))
+                               }, {
+                                       (8, payment_data)
                                });
                        },
                }

diff --git a/lightning/src/ln/onchaintx.rs b/lightning/src/ln/onchaintx.rs
index f18f2d406bc688d71550cae400e20197a43566fb..16bb646ed1837c61297d3aa17693875f0ea1c14f 100644 (file)
--- a/lightning/src/ln/onchaintx.rs
+++ b/lightning/src/ln/onchaintx.rs
@@ -361,7 +361,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
                }
                self.latest_height.write(writer)?;
 
-               write_tlv_fields!(writer, {});
+               write_tlv_fields!(writer, {}, {});
                Ok(())
        }
 }

diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs
index 1e81dd6572a1b2bebe806eb46f5425a7b15fe9a9..87f990a6f0677ee1f2a12735d0d8176ecf3af4d9 100644 (file)
--- a/lightning/src/util/ser_macros.rs
+++ b/lightning/src/util/ser_macros.rs
@@ -8,21 +8,51 @@
 // licenses.
 
 macro_rules! encode_tlv {
-       ($stream: expr, {$(($type: expr, $field: expr)),*}) => { {
+       ($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { {
                #[allow(unused_imports)]
                use util::ser::{BigSize, LengthCalculatingWriter};
+               // Fields must be serialized in order, so we have to potentially switch between optional
+               // fields and normal fields while serializing. Thus, we end up having to loop over the type
+               // counts.
+               // Sadly, while LLVM does appear smart enough to make `max_field` a constant, it appears to
+               // refuse to unroll the loop. If we have enough entries that this is slow we can revisit
+               // this design in the future.
+               #[allow(unused_mut)]
+               let mut max_field: u64 = 0;
+               $(
+                       if $type >= max_field { max_field = $type + 1; }
+               )*
                $(
-                       BigSize($type).write($stream)?;
-                       let mut len_calc = LengthCalculatingWriter(0);
-                       $field.write(&mut len_calc)?;
-                       BigSize(len_calc.0 as u64).write($stream)?;
-                       $field.write($stream)?;
+                       if $optional_type >= max_field { max_field = $optional_type + 1; }
                )*
+               #[allow(unused_variables)]
+               for i in 0..max_field {
+                       $(
+                               if i == $type {
+                                       BigSize($type).write($stream)?;
+                                       let mut len_calc = LengthCalculatingWriter(0);
+                                       $field.write(&mut len_calc)?;
+                                       BigSize(len_calc.0 as u64).write($stream)?;
+                                       $field.write($stream)?;
+                               }
+                       )*
+                       $(
+                               if i == $optional_type {
+                                       if let Some(ref field) = $optional_field {
+                                               BigSize($optional_type).write($stream)?;
+                                               let mut len_calc = LengthCalculatingWriter(0);
+                                               field.write(&mut len_calc)?;
+                                               BigSize(len_calc.0 as u64).write($stream)?;
+                                               field.write($stream)?;
+                                       }
+                               }
+                       )*
+               }
        } }
 }
 
 macro_rules! encode_varint_length_prefixed_tlv {
-       ($stream: expr, {$(($type: expr, $field: expr)),*}) => { {
+       ($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { {
                use util::ser::{BigSize, LengthCalculatingWriter};
                #[allow(unused_mut)]
                let mut len = LengthCalculatingWriter(0);
@@ -34,12 +64,19 @@ macro_rules! encode_varint_length_prefixed_tlv {
                                BigSize(field_len.0 as u64).write(&mut len)?;
                                len.0 += field_len.0;
                        )*
+                       $(
+                               if let Some(ref field) = $optional_field {
+                                       BigSize($optional_type).write(&mut len)?;
+                                       let mut field_len = LengthCalculatingWriter(0);
+                                       field.write(&mut field_len)?;
+                                       BigSize(field_len.0 as u64).write(&mut len)?;
+                                       len.0 += field_len.0;
+                               }
+                       )*
                }
 
                BigSize(len.0 as u64).write($stream)?;
-               encode_tlv!($stream, {
-                       $(($type, $field)),*
-               });
+               encode_tlv!($stream, { $(($type, $field)),* }, { $(($optional_type, $optional_field)),* });
        } }
 }
 
@@ -209,8 +246,8 @@ macro_rules! write_ver_prefix {
 /// This is the preferred method of adding new fields that old nodes can ignore and still function
 /// correctly.
 macro_rules! write_tlv_fields {
-       ($stream: expr, {$(($type: expr, $field: expr)),*}) => {
-               encode_varint_length_prefixed_tlv!($stream, {$(($type, $field)),*});
+       ($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => {
+               encode_varint_length_prefixed_tlv!($stream, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*});
        }
 }
 
@@ -435,19 +472,27 @@ mod tests {
                let mut stream = VecWriter(Vec::new());
 
                stream.0.clear();
-               encode_varint_length_prefixed_tlv!(&mut stream, { (1, 1u8) });
+               encode_varint_length_prefixed_tlv!(&mut stream, { (1, 1u8) }, { (42, None::<u64>) });
                assert_eq!(stream.0, ::hex::decode("03010101").unwrap());
 
                stream.0.clear();
-               encode_varint_length_prefixed_tlv!(&mut stream, { (4, 0xabcdu16) });
+               encode_varint_length_prefixed_tlv!(&mut stream, { }, { (1, Some(1u8)) });
+               assert_eq!(stream.0, ::hex::decode("03010101").unwrap());
+
+               stream.0.clear();
+               encode_varint_length_prefixed_tlv!(&mut stream, { (4, 0xabcdu16) }, { (42, None::<u64>) });
                assert_eq!(stream.0, ::hex::decode("040402abcd").unwrap());
 
                stream.0.clear();
-               encode_varint_length_prefixed_tlv!(&mut stream, { (0xff, 0xabcdu16) });
+               encode_varint_length_prefixed_tlv!(&mut stream, { (0xff, 0xabcdu16) }, { (42, None::<u64>) });
                assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());
 
                stream.0.clear();
-               encode_varint_length_prefixed_tlv!(&mut stream, { (0, 1u64), (0xff, HighZeroBytesDroppedVarInt(0u64)) });
+               encode_varint_length_prefixed_tlv!(&mut stream, { (0, 1u64), (0xff, HighZeroBytesDroppedVarInt(0u64)) }, { (42, None::<u64>) });
+               assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());
+
+               stream.0.clear();
+               encode_varint_length_prefixed_tlv!(&mut stream, { (0xff, HighZeroBytesDroppedVarInt(0u64)) }, { (0, Some(1u64)) });
                assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());
 
                Ok(())