Skip to content

Commit 64159b3

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #1045 from TheBlueMatt/2021-08-chanmon-ser-upgradability
Make `ChannelMonitor` serialization slightly more upgradable
2 parents a369f9e + ee43975 commit 64159b3

File tree

5 files changed

+167
-41
lines changed

5 files changed

+167
-41
lines changed

‎lightning/src/chain/channelmonitor.rs

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ impl Readable for ChannelMonitorUpdate {
106106
let len: u64 = Readable::read(r)?;
107107
let mut updates = Vec::with_capacity(cmp::min(len as usize, MAX_ALLOC_SIZE / ::core::mem::size_of::<ChannelMonitorUpdateStep>()));
108108
for _ in 0..len {
109-
updates.push(Readable::read(r)?);
109+
if let Some(upd) = MaybeReadable::read(r)? {
110+
updates.push(upd);
111+
}
110112
}
111113
read_tlv_fields!(r, {});
112114
Ok(Self { update_id, updates })
@@ -394,13 +396,36 @@ enum OnchainEvent {
394396
},
395397
}
396398

397-
impl_writeable_tlv_based!(OnchainEventEntry, {
398-
(0, txid, required),
399-
(2, height, required),
400-
(4, event, required),
401-
});
399+
impl Writeable for OnchainEventEntry {
400+
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
401+
write_tlv_fields!(writer, {
402+
(0, self.txid, required),
403+
(2, self.height, required),
404+
(4, self.event, required),
405+
});
406+
Ok(())
407+
}
408+
}
402409

403-
impl_writeable_tlv_based_enum!(OnchainEvent,
410+
impl MaybeReadable for OnchainEventEntry {
411+
fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
412+
let mut txid = Default::default();
413+
let mut height = 0;
414+
let mut event = None;
415+
read_tlv_fields!(reader, {
416+
(0, txid, required),
417+
(2, height, required),
418+
(4, event, ignorable),
419+
});
420+
if let Some(ev) = event {
421+
Ok(Some(Self { txid, height, event: ev }))
422+
} else {
423+
Ok(None)
424+
}
425+
}
426+
}
427+
428+
impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
404429
(0, HTLCUpdate) => {
405430
(0, source, required),
406431
(1, onchain_value_satoshis, option),
@@ -409,7 +434,7 @@ impl_writeable_tlv_based_enum!(OnchainEvent,
409434
(1, MaturingOutput) => {
410435
(0, descriptor, required),
411436
},
412-
;);
437+
);
413438

414439
#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
415440
#[derive(Clone)]
@@ -443,7 +468,7 @@ pub(crate) enum ChannelMonitorUpdateStep {
443468
},
444469
}
445470

446-
impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
471+
impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
447472
(0, LatestHolderCommitmentTXInfo) => {
448473
(0, commitment_tx, required),
449474
(2, htlc_outputs, vec_type),
@@ -467,7 +492,7 @@ impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
467492
(5, ShutdownScript) => {
468493
(0, scriptpubkey, required),
469494
},
470-
;);
495+
);
471496

472497
/// A ChannelMonitor handles chain events (blocks connected and disconnected) and generates
473498
/// on-chain transactions to ensure no loss of funds occurs.
@@ -2731,7 +2756,9 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
27312756
let waiting_threshold_conf_len: u64 = Readable::read(reader)?;
27322757
let mut onchain_events_awaiting_threshold_conf = Vec::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128));
27332758
for _ in 0..waiting_threshold_conf_len {
2734-
onchain_events_awaiting_threshold_conf.push(Readable::read(reader)?);
2759+
if let Some(val) = MaybeReadable::read(reader)? {
2760+
onchain_events_awaiting_threshold_conf.push(val);
2761+
}
27352762
}
27362763

27372764
let outputs_to_watch_len: u64 = Readable::read(reader)?;

‎lightning/src/chain/onchaintx.rs

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER};
2929
use chain::keysinterface::{Sign, KeysInterface};
3030
use chain::package::PackageTemplate;
3131
use util::logger::Logger;
32-
use util::ser::{Readable, ReadableArgs, Writer, Writeable, VecWriter};
32+
use util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable, VecWriter};
3333
use util::byte_utils;
3434

3535
use io;
@@ -79,20 +79,43 @@ enum OnchainEvent {
7979
}
8080
}
8181

82-
impl_writeable_tlv_based!(OnchainEventEntry, {
83-
(0, txid, required),
84-
(2, height, required),
85-
(4, event, required),
86-
});
82+
impl Writeable for OnchainEventEntry {
83+
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
84+
write_tlv_fields!(writer, {
85+
(0, self.txid, required),
86+
(2, self.height, required),
87+
(4, self.event, required),
88+
});
89+
Ok(())
90+
}
91+
}
92+
93+
impl MaybeReadable for OnchainEventEntry {
94+
fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
95+
let mut txid = Default::default();
96+
let mut height = 0;
97+
let mut event = None;
98+
read_tlv_fields!(reader, {
99+
(0, txid, required),
100+
(2, height, required),
101+
(4, event, ignorable),
102+
});
103+
if let Some(ev) = event {
104+
Ok(Some(Self { txid, height, event: ev }))
105+
} else {
106+
Ok(None)
107+
}
108+
}
109+
}
87110

88-
impl_writeable_tlv_based_enum!(OnchainEvent,
111+
impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
89112
(0, Claim) => {
90113
(0, claim_request, required),
91114
},
92115
(1, ContentiousOutpoint) => {
93116
(0, package, required),
94117
},
95-
;);
118+
);
96119

97120
impl Readable for Option<Vec<Option<(usize, Signature)>>> {
98121
fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
@@ -296,7 +319,9 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> for OnchainTxHandler<K::Signer> {
296319
let waiting_threshold_conf_len: u64 = Readable::read(reader)?;
297320
let mut onchain_events_awaiting_threshold_conf = Vec::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128));
298321
for _ in 0..waiting_threshold_conf_len {
299-
onchain_events_awaiting_threshold_conf.push(Readable::read(reader)?);
322+
if let Some(val) = MaybeReadable::read(reader)? {
323+
onchain_events_awaiting_threshold_conf.push(val);
324+
}
300325
}
301326

302327
read_tlv_fields!(reader, {});

‎lightning/src/ln/channelmanager.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,8 +1477,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
14771477

14781478
let mut chacha = ChaCha20::new(&rho, &[0u8; 8]);
14791479
let mut chacha_stream = ChaChaReader { chacha: &mut chacha, read: Cursor::new(&msg.onion_routing_packet.hop_data[..]) };
1480-
let (next_hop_data, next_hop_hmac) = {
1481-
match msgs::OnionHopData::read(&mut chacha_stream) {
1480+
let (next_hop_data, next_hop_hmac): (msgs::OnionHopData, _) = {
1481+
match <msgs::OnionHopData as Readable>::read(&mut chacha_stream) {
14821482
Err(err) => {
14831483
let error_code = match err {
14841484
msgs::DecodeError::UnknownVersion => 0x4000 | 1, // unknown realm byte

‎lightning/src/util/ser.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,13 @@ pub trait MaybeReadable
241241
fn read<R: Read>(reader: &mut R) -> Result<Option<Self>, DecodeError>;
242242
}
243243

244+
impl<T: Readable> MaybeReadable for T {
245+
#[inline]
246+
fn read<R: Read>(reader: &mut R) -> Result<Option<T>, DecodeError> {
247+
Ok(Some(Readable::read(reader)?))
248+
}
249+
}
250+
244251
pub(crate) struct OptionDeserWrapper<T: Readable>(pub Option<T>);
245252
impl<T: Readable> Readable for OptionDeserWrapper<T> {
246253
#[inline]
@@ -262,15 +269,16 @@ impl<'a, T: Writeable> Writeable for VecWriteWrapper<'a, T> {
262269
}
263270

264271
/// Wrapper to read elements from a given stream until it reaches the end of the stream.
265-
pub(crate) struct VecReadWrapper<T: Readable>(pub Vec<T>);
266-
impl<T: Readable> Readable for VecReadWrapper<T> {
272+
pub(crate) struct VecReadWrapper<T>(pub Vec<T>);
273+
impl<T: MaybeReadable> Readable for VecReadWrapper<T> {
267274
#[inline]
268275
fn read<R: Read>(mut reader: &mut R) -> Result<Self, DecodeError> {
269276
let mut values = Vec::new();
270277
loop {
271278
let mut track_read = ReadTrackingReader::new(&mut reader);
272-
match Readable::read(&mut track_read) {
273-
Ok(v) => { values.push(v); },
279+
match MaybeReadable::read(&mut track_read) {
280+
Ok(Some(v)) => { values.push(v); },
281+
Ok(None) => { },
274282
// If we failed to read any bytes at all, we reached the end of our TLV
275283
// stream and have simply exhausted all entries.
276284
Err(ref e) if e == &DecodeError::ShortRead && !track_read.have_read => break,
@@ -561,7 +569,7 @@ impl Readable for Vec<Signature> {
561569
return Err(DecodeError::BadLengthDescriptor);
562570
}
563571
let mut ret = Vec::with_capacity(len as usize);
564-
for _ in 0..len { ret.push(Signature::read(r)?); }
572+
for _ in 0..len { ret.push(Readable::read(r)?); }
565573
Ok(ret)
566574
}
567575
}
@@ -726,7 +734,8 @@ impl<T: Writeable> Writeable for Option<T> {
726734
impl<T: Readable> Readable for Option<T>
727735
{
728736
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
729-
match BigSize::read(r)?.0 {
737+
let len: BigSize = Readable::read(r)?;
738+
match len.0 {
730739
0 => Ok(None),
731740
len => {
732741
let mut reader = FixedLengthReader::new(r, len - 1);

‎lightning/src/util/ser_macros.rs

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ macro_rules! check_tlv_order {
115115
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, vec_type) => {{
116116
// no-op
117117
}};
118+
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, ignorable) => {{
119+
// no-op
120+
}};
118121
}
119122

120123
macro_rules! check_missing_tlv {
@@ -138,6 +141,9 @@ macro_rules! check_missing_tlv {
138141
($last_seen_type: expr, $type: expr, $field: ident, option) => {{
139142
// no-op
140143
}};
144+
($last_seen_type: expr, $type: expr, $field: ident, ignorable) => {{
145+
// no-op
146+
}};
141147
}
142148

143149
macro_rules! decode_tlv {
@@ -153,6 +159,9 @@ macro_rules! decode_tlv {
153159
($reader: expr, $field: ident, option) => {{
154160
$field = Some(ser::Readable::read(&mut $reader)?);
155161
}};
162+
($reader: expr, $field: ident, ignorable) => {{
163+
$field = ser::MaybeReadable::read(&mut $reader)?;
164+
}};
156165
}
157166

158167
macro_rules! decode_tlv_stream {
@@ -371,7 +380,7 @@ macro_rules! read_ver_prefix {
371380
/// Reads a suffix added by write_tlv_fields.
372381
macro_rules! read_tlv_fields {
373382
($stream: expr, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { {
374-
let tlv_len = ::util::ser::BigSize::read($stream)?;
383+
let tlv_len: ::util::ser::BigSize = ::util::ser::Readable::read($stream)?;
375384
let mut rd = ::util::ser::FixedLengthReader::new($stream, tlv_len.0);
376385
decode_tlv_stream!(&mut rd, {$(($type, $field, $fieldty)),*});
377386
rd.eat_remaining().map_err(|_| ::ln::msgs::DecodeError::ShortRead)?;
@@ -405,7 +414,7 @@ macro_rules! init_tlv_field_var {
405414
};
406415
($field: ident, option) => {
407416
let mut $field = None;
408-
}
417+
};
409418
}
410419

411420
/// Implements Readable/Writeable for a struct storing it as a set of TLVs
@@ -458,17 +467,7 @@ macro_rules! impl_writeable_tlv_based {
458467
}
459468
}
460469

461-
/// Implement Readable and Writeable for an enum, with struct variants stored as TLVs and tuple
462-
/// variants stored directly.
463-
/// The format is, for example
464-
/// impl_writeable_tlv_based_enum!(EnumName,
465-
/// (0, StructVariantA) => {(0, required_variant_field, required), (1, optional_variant_field, option)},
466-
/// (1, StructVariantB) => {(0, variant_field_a, required), (1, variant_field_b, required), (2, variant_vec_field, vec_type)};
467-
/// (2, TupleVariantA), (3, TupleVariantB),
468-
/// );
469-
/// The type is written as a single byte, followed by any variant data.
470-
/// Attempts to read an unknown type byte result in DecodeError::UnknownRequiredFeature.
471-
macro_rules! impl_writeable_tlv_based_enum {
470+
macro_rules! _impl_writeable_tlv_based_enum_common {
472471
($st: ident, $(($variant_id: expr, $variant_name: ident) =>
473472
{$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
474473
),* $(,)*;
@@ -492,6 +491,72 @@ macro_rules! impl_writeable_tlv_based_enum {
492491
Ok(())
493492
}
494493
}
494+
}
495+
}
496+
497+
/// Implement MaybeReadable and Writeable for an enum, with struct variants stored as TLVs and
498+
/// tuple variants stored directly.
499+
///
500+
/// This is largely identical to `impl_writeable_tlv_based_enum`, except that odd variants will
501+
/// return `Ok(None)` instead of `Err(UnknownRequiredFeature)`. It should generally be preferred
502+
/// when `MaybeReadable` is practical instead of just `Readable` as it provides an upgrade path for
503+
/// new variants to be added which are simply ignored by existing clients.
504+
macro_rules! impl_writeable_tlv_based_enum_upgradable {
505+
($st: ident, $(($variant_id: expr, $variant_name: ident) =>
506+
{$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
507+
),* $(,)*) => {
508+
_impl_writeable_tlv_based_enum_common!($st,
509+
$(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*; );
510+
511+
impl ::util::ser::MaybeReadable for $st {
512+
fn read<R: $crate::io::Read>(reader: &mut R) -> Result<Option<Self>, ::ln::msgs::DecodeError> {
513+
let id: u8 = ::util::ser::Readable::read(reader)?;
514+
match id {
515+
$($variant_id => {
516+
// Because read_tlv_fields creates a labeled loop, we cannot call it twice
517+
// in the same function body. Instead, we define a closure and call it.
518+
let f = || {
519+
$(
520+
init_tlv_field_var!($field, $fieldty);
521+
)*
522+
read_tlv_fields!(reader, {
523+
$(($type, $field, $fieldty)),*
524+
});
525+
Ok(Some($st::$variant_name {
526+
$(
527+
$field: init_tlv_based_struct_field!($field, $fieldty)
528+
),*
529+
}))
530+
};
531+
f()
532+
}),*
533+
_ if id % 2 == 1 => Ok(None),
534+
_ => Err(DecodeError::UnknownRequiredFeature),
535+
}
536+
}
537+
}
538+
539+
}
540+
}
541+
542+
/// Implement Readable and Writeable for an enum, with struct variants stored as TLVs and tuple
543+
/// variants stored directly.
544+
/// The format is, for example
545+
/// impl_writeable_tlv_based_enum!(EnumName,
546+
/// (0, StructVariantA) => {(0, required_variant_field, required), (1, optional_variant_field, option)},
547+
/// (1, StructVariantB) => {(0, variant_field_a, required), (1, variant_field_b, required), (2, variant_vec_field, vec_type)};
548+
/// (2, TupleVariantA), (3, TupleVariantB),
549+
/// );
550+
/// The type is written as a single byte, followed by any variant data.
551+
/// Attempts to read an unknown type byte result in DecodeError::UnknownRequiredFeature.
552+
macro_rules! impl_writeable_tlv_based_enum {
553+
($st: ident, $(($variant_id: expr, $variant_name: ident) =>
554+
{$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
555+
),* $(,)*;
556+
$(($tuple_variant_id: expr, $tuple_variant_name: ident)),* $(,)*) => {
557+
_impl_writeable_tlv_based_enum_common!($st,
558+
$(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*;
559+
$(($tuple_variant_id, $tuple_variant_name)),*);
495560

496561
impl ::util::ser::Readable for $st {
497562
fn read<R: $crate::io::Read>(reader: &mut R) -> Result<Self, ::ln::msgs::DecodeError> {

0 commit comments

Comments
 (0)