Skip to content

Commit 7a4f741

Browse files
TheBlueMattTheBlueMatt
committed
Migrate OnchainEvent serialization to be MaybeReadable.
This adds a new TLV-based enum serialization macro entitled `impl_writeable_tlv_based_enum_upgradable`. As the name implies, the new macro allows us to ignore odd-numbered variant entries. Because the new macro implements only `MaybeReadable` and not `Readable`, it is not applicable in many contexts, here only being added for the two `OnchainEvent` structs.
1 parent 24dc510 commit 7a4f741

File tree

3 files changed

+144
-32
lines changed

3 files changed

+144
-32
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 })
@@ -391,21 +393,44 @@ enum OnchainEvent {
391393
},
392394
}
393395

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

400-
impl_writeable_tlv_based_enum!(OnchainEvent,
407+
impl MaybeReadable for OnchainEventEntry {
408+
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
409+
let mut txid = Default::default();
410+
let mut height = 0;
411+
let mut event = None;
412+
read_tlv_fields!(reader, {
413+
(0, txid, required),
414+
(2, height, required),
415+
(4, event, maybe_readable),
416+
});
417+
if let Some(ev) = event {
418+
Ok(Some(Self { txid, height, event: ev }))
419+
} else {
420+
Ok(None)
421+
}
422+
}
423+
}
424+
425+
impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
401426
(0, HTLCUpdate) => {
402427
(0, source, required),
403428
(2, payment_hash, required),
404429
},
405430
(1, MaturingOutput) => {
406431
(0, descriptor, required),
407432
},
408-
;);
433+
);
409434

410435
#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
411436
#[derive(Clone)]
@@ -436,7 +461,7 @@ pub(crate) enum ChannelMonitorUpdateStep {
436461
},
437462
}
438463

439-
impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
464+
impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
440465
(0, LatestHolderCommitmentTXInfo) => {
441466
(0, commitment_tx, required),
442467
(2, htlc_outputs, vec_type),
@@ -457,7 +482,7 @@ impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
457482
(4, ChannelForceClosed) => {
458483
(0, should_broadcast, required),
459484
},
460-
;);
485+
);
461486

462487
/// A ChannelMonitor handles chain events (blocks connected and disconnected) and generates
463488
/// on-chain transactions to ensure no loss of funds occurs.
@@ -2689,7 +2714,9 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
26892714
let waiting_threshold_conf_len: u64 = Readable::read(reader)?;
26902715
let mut onchain_events_awaiting_threshold_conf = Vec::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128));
26912716
for _ in 0..waiting_threshold_conf_len {
2692-
onchain_events_awaiting_threshold_conf.push(Readable::read(reader)?);
2717+
if let Some(val) = MaybeReadable::read(reader)? {
2718+
onchain_events_awaiting_threshold_conf.push(val);
2719+
}
26932720
}
26942721

26952722
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<(), ::std::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+
}
8792

88-
impl_writeable_tlv_based_enum!(OnchainEvent,
93+
impl MaybeReadable for OnchainEventEntry {
94+
fn read<R: ::std::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, maybe_readable),
102+
});
103+
if let Some(ev) = event {
104+
Ok(Some(Self { txid, height, event: ev }))
105+
} else {
106+
Ok(None)
107+
}
108+
}
109+
}
110+
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/util/ser_macros.rs

Lines changed: 72 additions & 12 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, maybe_readable) => {{
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, maybe_readable) => {{
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, maybe_readable) => {{
163+
$field = ser::MaybeReadable::read(&mut $reader)?;
164+
}};
156165
}
157166

158167
macro_rules! decode_tlv_stream {
@@ -406,7 +415,7 @@ macro_rules! init_tlv_field_var {
406415
};
407416
($field: ident, option) => {
408417
let mut $field = None;
409-
}
418+
};
410419
}
411420

412421
/// Implements Readable/Writeable for a struct storing it as a set of TLVs
@@ -459,17 +468,7 @@ macro_rules! impl_writeable_tlv_based {
459468
}
460469
}
461470

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

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

0 commit comments

Comments
 (0)