Skip to content

Make ChannelMonitor serialization slightly more upgradable #1045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 16, 2021
Merged

Make ChannelMonitor serialization slightly more upgradable #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45490d5
Implement `VecReadWrapper` for `MaybeReadable`
TheBlueMatt Jul 28, 2021
ee43975
Migrate OnchainEvent serialization to be MaybeReadable.
TheBlueMatt Jul 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 38 additions & 11 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ impl Readable for ChannelMonitorUpdate {
let len: u64 = Readable::read(r)?;
let mut updates = Vec::with_capacity(cmp::min(len as usize, MAX_ALLOC_SIZE / ::core::mem::size_of::<ChannelMonitorUpdateStep>()));
for _ in 0..len {
updates.push(Readable::read(r)?);
if let Some(upd) = MaybeReadable::read(r)? {
updates.push(upd);
}
}
read_tlv_fields!(r, {});
Ok(Self { update_id, updates })
Expand Down Expand Up @@ -394,13 +396,36 @@ enum OnchainEvent {
},
}

impl_writeable_tlv_based!(OnchainEventEntry, {
(0, txid, required),
(2, height, required),
(4, event, required),
});
impl Writeable for OnchainEventEntry {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
write_tlv_fields!(writer, {
(0, self.txid, required),
(2, self.height, required),
(4, self.event, required),
});
Ok(())
}
}

impl_writeable_tlv_based_enum!(OnchainEvent,
impl MaybeReadable for OnchainEventEntry {
fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
let mut txid = Default::default();
let mut height = 0;
let mut event = None;
read_tlv_fields!(reader, {
(0, txid, required),
(2, height, required),
(4, event, ignorable),
});
if let Some(ev) = event {
Ok(Some(Self { txid, height, event: ev }))
} else {
Ok(None)
}
}
}

impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
(0, HTLCUpdate) => {
(0, source, required),
(1, onchain_value_satoshis, option),
Expand All @@ -409,7 +434,7 @@ impl_writeable_tlv_based_enum!(OnchainEvent,
(1, MaturingOutput) => {
(0, descriptor, required),
},
;);
);

#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
#[derive(Clone)]
Expand Down Expand Up @@ -443,7 +468,7 @@ pub(crate) enum ChannelMonitorUpdateStep {
},
}

impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
(0, LatestHolderCommitmentTXInfo) => {
(0, commitment_tx, required),
(2, htlc_outputs, vec_type),
Expand All @@ -467,7 +492,7 @@ impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
(5, ShutdownScript) => {
(0, scriptpubkey, required),
},
;);
);

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

let outputs_to_watch_len: u64 = Readable::read(reader)?;
Expand Down
43 changes: 34 additions & 9 deletions lightning/src/chain/onchaintx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER};
use chain::keysinterface::{Sign, KeysInterface};
use chain::package::PackageTemplate;
use util::logger::Logger;
use util::ser::{Readable, ReadableArgs, Writer, Writeable, VecWriter};
use util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable, VecWriter};
use util::byte_utils;

use io;
Expand Down Expand Up @@ -79,20 +79,43 @@ enum OnchainEvent {
}
}

impl_writeable_tlv_based!(OnchainEventEntry, {
(0, txid, required),
(2, height, required),
(4, event, required),
});
impl Writeable for OnchainEventEntry {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
write_tlv_fields!(writer, {
(0, self.txid, required),
(2, self.height, required),
(4, self.event, required),
});
Ok(())
}
}

impl MaybeReadable for OnchainEventEntry {
fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
let mut txid = Default::default();
let mut height = 0;
let mut event = None;
read_tlv_fields!(reader, {
(0, txid, required),
(2, height, required),
(4, event, ignorable),
});
if let Some(ev) = event {
Ok(Some(Self { txid, height, event: ev }))
} else {
Ok(None)
}
}
}

impl_writeable_tlv_based_enum!(OnchainEvent,
impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
(0, Claim) => {
(0, claim_request, required),
},
(1, ContentiousOutpoint) => {
(0, package, required),
},
;);
);

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

read_tlv_fields!(reader, {});
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1477,8 +1477,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana

let mut chacha = ChaCha20::new(&rho, &[0u8; 8]);
let mut chacha_stream = ChaChaReader { chacha: &mut chacha, read: Cursor::new(&msg.onion_routing_packet.hop_data[..]) };
let (next_hop_data, next_hop_hmac) = {
match msgs::OnionHopData::read(&mut chacha_stream) {
let (next_hop_data, next_hop_hmac): (msgs::OnionHopData, _) = {
match <msgs::OnionHopData as Readable>::read(&mut chacha_stream) {
Err(err) => {
let error_code = match err {
msgs::DecodeError::UnknownVersion => 0x4000 | 1, // unknown realm byte
Expand Down
21 changes: 15 additions & 6 deletions lightning/src/util/ser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,13 @@ pub trait MaybeReadable
fn read<R: Read>(reader: &mut R) -> Result<Option<Self>, DecodeError>;
}

impl<T: Readable> MaybeReadable for T {
#[inline]
fn read<R: Read>(reader: &mut R) -> Result<Option<T>, DecodeError> {
Ok(Some(Readable::read(reader)?))
}
}

pub(crate) struct OptionDeserWrapper<T: Readable>(pub Option<T>);
impl<T: Readable> Readable for OptionDeserWrapper<T> {
#[inline]
Expand All @@ -262,15 +269,16 @@ impl<'a, T: Writeable> Writeable for VecWriteWrapper<'a, T> {
}

/// Wrapper to read elements from a given stream until it reaches the end of the stream.
pub(crate) struct VecReadWrapper<T: Readable>(pub Vec<T>);
impl<T: Readable> Readable for VecReadWrapper<T> {
pub(crate) struct VecReadWrapper<T>(pub Vec<T>);
impl<T: MaybeReadable> Readable for VecReadWrapper<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, this is not exercised at all for MaybeReadable types now but could be in the future. That way we don't need to write custom decoding for TLVs with a vec containing types implementing MaybeReadable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand the comment. Obviously this PR is a bit awkward because it is just commits from #1034, which does use all the code in it. I tried a few ways to do VecReadWrapper that I thought were cleaner but always ended up hitting the "you may implement MaybeReadable and Readable for this type causing duplicate implementations" stuff every time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No action necessary. Just pointing out this use case.

#[inline]
fn read<R: Read>(mut reader: &mut R) -> Result<Self, DecodeError> {
let mut values = Vec::new();
loop {
let mut track_read = ReadTrackingReader::new(&mut reader);
match Readable::read(&mut track_read) {
Ok(v) => { values.push(v); },
match MaybeReadable::read(&mut track_read) {
Ok(Some(v)) => { values.push(v); },
Ok(None) => { },
// If we failed to read any bytes at all, we reached the end of our TLV
// stream and have simply exhausted all entries.
Err(ref e) if e == &DecodeError::ShortRead && !track_read.have_read => break,
Expand Down Expand Up @@ -561,7 +569,7 @@ impl Readable for Vec<Signature> {
return Err(DecodeError::BadLengthDescriptor);
}
let mut ret = Vec::with_capacity(len as usize);
for _ in 0..len { ret.push(Signature::read(r)?); }
for _ in 0..len { ret.push(Readable::read(r)?); }
Ok(ret)
}
}
Expand Down Expand Up @@ -726,7 +734,8 @@ impl<T: Writeable> Writeable for Option<T> {
impl<T: Readable> Readable for Option<T>
{
fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
match BigSize::read(r)?.0 {
let len: BigSize = Readable::read(r)?;
match len.0 {
0 => Ok(None),
len => {
let mut reader = FixedLengthReader::new(r, len - 1);
Expand Down
91 changes: 78 additions & 13 deletions lightning/src/util/ser_macros.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ macro_rules! check_tlv_order {
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, vec_type) => {{
// no-op
}};
($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, ignorable) => {{
// no-op
}};
}

macro_rules! check_missing_tlv {
Expand All @@ -138,6 +141,9 @@ macro_rules! check_missing_tlv {
($last_seen_type: expr, $type: expr, $field: ident, option) => {{
// no-op
}};
($last_seen_type: expr, $type: expr, $field: ident, ignorable) => {{
// no-op
}};
}

macro_rules! decode_tlv {
Expand All @@ -153,6 +159,9 @@ macro_rules! decode_tlv {
($reader: expr, $field: ident, option) => {{
$field = Some(ser::Readable::read(&mut $reader)?);
}};
($reader: expr, $field: ident, ignorable) => {{
$field = ser::MaybeReadable::read(&mut $reader)?;
}};
}

macro_rules! decode_tlv_stream {
Expand Down Expand Up @@ -371,7 +380,7 @@ macro_rules! read_ver_prefix {
/// Reads a suffix added by write_tlv_fields.
macro_rules! read_tlv_fields {
($stream: expr, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { {
let tlv_len = ::util::ser::BigSize::read($stream)?;
let tlv_len: ::util::ser::BigSize = ::util::ser::Readable::read($stream)?;
let mut rd = ::util::ser::FixedLengthReader::new($stream, tlv_len.0);
decode_tlv_stream!(&mut rd, {$(($type, $field, $fieldty)),*});
rd.eat_remaining().map_err(|_| ::ln::msgs::DecodeError::ShortRead)?;
Expand Down Expand Up @@ -405,7 +414,7 @@ macro_rules! init_tlv_field_var {
};
($field: ident, option) => {
let mut $field = None;
}
};
}

/// Implements Readable/Writeable for a struct storing it as a set of TLVs
Expand Down Expand Up @@ -458,17 +467,7 @@ macro_rules! impl_writeable_tlv_based {
}
}

/// Implement Readable and Writeable for an enum, with struct variants stored as TLVs and tuple
/// variants stored directly.
/// The format is, for example
/// impl_writeable_tlv_based_enum!(EnumName,
/// (0, StructVariantA) => {(0, required_variant_field, required), (1, optional_variant_field, option)},
/// (1, StructVariantB) => {(0, variant_field_a, required), (1, variant_field_b, required), (2, variant_vec_field, vec_type)};
/// (2, TupleVariantA), (3, TupleVariantB),
/// );
/// The type is written as a single byte, followed by any variant data.
/// Attempts to read an unknown type byte result in DecodeError::UnknownRequiredFeature.
macro_rules! impl_writeable_tlv_based_enum {
macro_rules! _impl_writeable_tlv_based_enum_common {
($st: ident, $(($variant_id: expr, $variant_name: ident) =>
{$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
),* $(,)*;
Expand All @@ -492,6 +491,72 @@ macro_rules! impl_writeable_tlv_based_enum {
Ok(())
}
}
}
}

/// Implement MaybeReadable and Writeable for an enum, with struct variants stored as TLVs and
/// tuple variants stored directly.
///
/// This is largely identical to `impl_writeable_tlv_based_enum`, except that odd variants will
/// return `Ok(None)` instead of `Err(UnknownRequiredFeature)`. It should generally be preferred
/// when `MaybeReadable` is practical instead of just `Readable` as it provides an upgrade path for
/// new variants to be added which are simply ignored by existing clients.
macro_rules! impl_writeable_tlv_based_enum_upgradable {
($st: ident, $(($variant_id: expr, $variant_name: ident) =>
{$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
),* $(,)*) => {
_impl_writeable_tlv_based_enum_common!($st,
$(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*; );

impl ::util::ser::MaybeReadable for $st {
fn read<R: $crate::io::Read>(reader: &mut R) -> Result<Option<Self>, ::ln::msgs::DecodeError> {
let id: u8 = ::util::ser::Readable::read(reader)?;
match id {
$($variant_id => {
// Because read_tlv_fields creates a labeled loop, we cannot call it twice
// in the same function body. Instead, we define a closure and call it.
let f = || {
$(
init_tlv_field_var!($field, $fieldty);
)*
read_tlv_fields!(reader, {
$(($type, $field, $fieldty)),*
});
Ok(Some($st::$variant_name {
$(
$field: init_tlv_based_struct_field!($field, $fieldty)
),*
}))
};
f()
}),*
_ if id % 2 == 1 => Ok(None),
_ => Err(DecodeError::UnknownRequiredFeature),
}
}
}

}
}

/// Implement Readable and Writeable for an enum, with struct variants stored as TLVs and tuple
/// variants stored directly.
/// The format is, for example
/// impl_writeable_tlv_based_enum!(EnumName,
/// (0, StructVariantA) => {(0, required_variant_field, required), (1, optional_variant_field, option)},
/// (1, StructVariantB) => {(0, variant_field_a, required), (1, variant_field_b, required), (2, variant_vec_field, vec_type)};
/// (2, TupleVariantA), (3, TupleVariantB),
/// );
/// The type is written as a single byte, followed by any variant data.
/// Attempts to read an unknown type byte result in DecodeError::UnknownRequiredFeature.
macro_rules! impl_writeable_tlv_based_enum {
($st: ident, $(($variant_id: expr, $variant_name: ident) =>
{$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
),* $(,)*;
$(($tuple_variant_id: expr, $tuple_variant_name: ident)),* $(,)*) => {
_impl_writeable_tlv_based_enum_common!($st,
$(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*;
$(($tuple_variant_id, $tuple_variant_name)),*);

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