From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Tue, 27 Apr 2021 02:05:29 +0000 (+0000) Subject: Merge pull request #854 from TheBlueMatt/2021-03-fix-lens X-Git-Tag: v0.0.14~13 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=affefb677e3f9f1a586707484ac82b6c86f4f26b;hp=7af9976261436fe2357a187c2aaa6792fb0506c6;p=rust-lightning Merge pull request #854 from TheBlueMatt/2021-03-fix-lens Fix serialization expected lengths and check them in test/fuzzing --- diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 647fc323..da7ba15a 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -39,6 +39,7 @@ use std::io::Read; use std::ops::Deref; use chain; +// Maximum size of a serialized HTLCOutputInCommitment const HTLC_OUTPUT_IN_COMMITMENT_SIZE: usize = 1 + 8 + 4 + 32 + 5; pub(crate) const MAX_HTLCS: u16 = 483; @@ -320,7 +321,8 @@ pub struct TxCreationKeys { /// Broadcaster's Payment Key (which isn't allowed to be spent from for some delay) pub broadcaster_delayed_payment_key: PublicKey, } -impl_writeable!(TxCreationKeys, 33*6, + +impl_writeable!(TxCreationKeys, 33*5, { per_commitment_point, revocation_key, broadcaster_htlc_key, countersignatory_htlc_key, broadcaster_delayed_payment_key }); /// One counterparty's public keys which do not change over the life of a channel. @@ -427,7 +429,10 @@ pub struct HTLCOutputInCommitment { pub transaction_output_index: Option, } -impl_writeable!(HTLCOutputInCommitment, HTLC_OUTPUT_IN_COMMITMENT_SIZE, { +impl_writeable_len_match!(HTLCOutputInCommitment, { + { HTLCOutputInCommitment { transaction_output_index: None, .. }, HTLC_OUTPUT_IN_COMMITMENT_SIZE - 4 }, + { _, HTLC_OUTPUT_IN_COMMITMENT_SIZE } + }, { offered, amount_msat, cltv_expiry, diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 01a43784..8a7713dc 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -1396,7 +1396,7 @@ impl Readable for Pong { impl Writeable for UnsignedChannelAnnouncement { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { - w.size_hint(2 + 2*32 + 4*33 + self.features.byte_count() + self.excess_data.len()); + w.size_hint(2 + 32 + 8 + 4*33 + self.features.byte_count() + self.excess_data.len()); self.features.write(w)?; self.chain_hash.write(w)?; self.short_channel_id.write(w)?; @@ -1430,7 +1430,7 @@ impl Readable for UnsignedChannelAnnouncement { impl_writeable_len_match!(ChannelAnnouncement, { { ChannelAnnouncement { contents: UnsignedChannelAnnouncement {ref features, ref excess_data, ..}, .. }, - 2 + 2*32 + 4*33 + features.byte_count() + excess_data.len() + 4*64 } + 2 + 32 + 8 + 4*33 + features.byte_count() + excess_data.len() + 4*64 } }, { node_signature_1, node_signature_2, @@ -1491,8 +1491,8 @@ impl Readable for UnsignedChannelUpdate { } impl_writeable_len_match!(ChannelUpdate, { - { ChannelUpdate { contents: UnsignedChannelUpdate {ref excess_data, ..}, .. }, - 64 + excess_data.len() + 64 } + { ChannelUpdate { contents: UnsignedChannelUpdate {ref excess_data, ref htlc_maximum_msat, ..}, .. }, + 64 + 64 + excess_data.len() + if let OptionalField::Present(_) = htlc_maximum_msat { 8 } else { 0 } } }, { signature, contents @@ -1528,7 +1528,7 @@ impl Readable for ErrorMessage { impl Writeable for UnsignedNodeAnnouncement { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { - w.size_hint(64 + 76 + self.features.byte_count() + self.addresses.len()*38 + self.excess_address_data.len() + self.excess_data.len()); + w.size_hint(76 + self.features.byte_count() + self.addresses.len()*38 + self.excess_address_data.len() + self.excess_data.len()); self.features.write(w)?; self.timestamp.write(w)?; self.node_id.write(w)?; @@ -1611,7 +1611,7 @@ impl Readable for UnsignedNodeAnnouncement { } } -impl_writeable_len_match!(NodeAnnouncement, { +impl_writeable_len_match!(NodeAnnouncement, <=, { { NodeAnnouncement { contents: UnsignedNodeAnnouncement { ref features, ref addresses, ref excess_address_data, ref excess_data, ..}, .. }, 64 + 76 + features.byte_count() + addresses.len()*(NetAddress::MAX_LEN as usize + 1) + excess_address_data.len() + excess_data.len() } }, { diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 4a9f3006..de83f227 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -223,7 +223,7 @@ impl Default for ChannelConfig { } //Add write and readable traits to channelconfig -impl_writeable!(ChannelConfig, 8+2+1+1, { +impl_writeable!(ChannelConfig, 4+2+1+1, { fee_proportional_millionths, cltv_expiry_delta, announced_channel, diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index ceabcc1d..5062bae7 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -120,6 +120,16 @@ macro_rules! impl_writeable { if $len != 0 { w.size_hint($len); } + #[cfg(any(test, feature = "fuzztarget"))] + { + // In tests, assert that the hard-coded length matches the actual one + if $len != 0 { + use util::ser::LengthCalculatingWriter; + let mut len_calc = LengthCalculatingWriter(0); + $( self.$field.write(&mut len_calc)?; )* + assert_eq!(len_calc.0, $len); + } + } $( self.$field.write(w)?; )* Ok(()) } @@ -135,24 +145,36 @@ macro_rules! impl_writeable { } } macro_rules! impl_writeable_len_match { - ($st:ident, {$({$m: pat, $l: expr}),*}, {$($field:ident),*}) => { - impl Writeable for $st { + ($struct: ident, $cmp: tt, {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => { + impl Writeable for $struct { fn write(&self, w: &mut W) -> Result<(), ::std::io::Error> { - w.size_hint(match *self { - $($m => $l,)* - }); + let len = match *self { + $($match => $length,)* + }; + w.size_hint(len); + #[cfg(any(test, feature = "fuzztarget"))] + { + // In tests, assert that the hard-coded length matches the actual one + use util::ser::LengthCalculatingWriter; + let mut len_calc = LengthCalculatingWriter(0); + $( self.$field.write(&mut len_calc)?; )* + assert!(len_calc.0 $cmp len); + } $( self.$field.write(w)?; )* Ok(()) } } - impl ::util::ser::Readable for $st { + impl ::util::ser::Readable for $struct { fn read(r: &mut R) -> Result { Ok(Self { $($field: Readable::read(r)?),* }) } } + }; + ($struct: ident, {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => { + impl_writeable_len_match!($struct, ==, { $({ $match, $length }),* }, { $($field),* }); } }