From 32ca8ec13e0928cbb4f7067a3fb6d41f39691d1c Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 23 Feb 2020 23:12:19 -0500 Subject: [PATCH] Make Readable::read a templated on the stream, not Readable itself This makes Readable symmetric with Writeable and makes sense - something which is Readable should be Readable for any stream which implements std::io::Read, not only for a stream type it decides on. This solves some lifetime-compatibility issues in trying to read() from a LengthLimitingReader in arbitrary Readable impls. --- fuzz/src/msg_targets/utils.rs | 10 +- lightning/src/chain/keysinterface.rs | 8 +- lightning/src/ln/chan_utils.rs | 8 +- lightning/src/ln/channel.rs | 20 ++-- lightning/src/ln/channelmanager.rs | 46 ++++---- lightning/src/ln/channelmonitor.rs | 54 ++++----- lightning/src/ln/features.rs | 4 +- lightning/src/ln/msgs.rs | 56 ++++----- lightning/src/ln/router.rs | 16 +-- lightning/src/ln/wire.rs | 2 +- lightning/src/util/enforcing_trait_impls.rs | 4 +- lightning/src/util/events.rs | 4 +- lightning/src/util/ser.rs | 120 ++++++++++---------- lightning/src/util/ser_macros.rs | 12 +- 14 files changed, 179 insertions(+), 185 deletions(-) diff --git a/fuzz/src/msg_targets/utils.rs b/fuzz/src/msg_targets/utils.rs index 2a7731cc..6125e5af 100644 --- a/fuzz/src/msg_targets/utils.rs +++ b/fuzz/src/msg_targets/utils.rs @@ -28,7 +28,7 @@ macro_rules! test_msg { { use lightning::util::ser::{Writeable, Readable}; let mut r = ::std::io::Cursor::new($data); - if let Ok(msg) = <$MsgType as Readable<::std::io::Cursor<&[u8]>>>::read(&mut r) { + if let Ok(msg) = <$MsgType as Readable>::read(&mut r) { let p = r.position() as usize; let mut w = VecWriter(Vec::new()); msg.write(&mut w).unwrap(); @@ -48,11 +48,11 @@ macro_rules! test_msg_simple { { use lightning::util::ser::{Writeable, Readable}; let mut r = ::std::io::Cursor::new($data); - if let Ok(msg) = <$MsgType as Readable<::std::io::Cursor<&[u8]>>>::read(&mut r) { + if let Ok(msg) = <$MsgType as Readable>::read(&mut r) { let mut w = VecWriter(Vec::new()); msg.write(&mut w).unwrap(); - let msg = <$MsgType as Readable<::std::io::Cursor<&[u8]>>>::read(&mut ::std::io::Cursor::new(&w.0)).unwrap(); + let msg = <$MsgType as Readable>::read(&mut ::std::io::Cursor::new(&w.0)).unwrap(); let mut w_two = VecWriter(Vec::new()); msg.write(&mut w_two).unwrap(); assert_eq!(&w.0[..], &w_two.0[..]); @@ -69,7 +69,7 @@ macro_rules! test_msg_exact { { use lightning::util::ser::{Writeable, Readable}; let mut r = ::std::io::Cursor::new($data); - if let Ok(msg) = <$MsgType as Readable<::std::io::Cursor<&[u8]>>>::read(&mut r) { + if let Ok(msg) = <$MsgType as Readable>::read(&mut r) { let mut w = VecWriter(Vec::new()); msg.write(&mut w).unwrap(); assert_eq!(&r.into_inner()[..], &w.0[..]); @@ -86,7 +86,7 @@ macro_rules! test_msg_hole { { use lightning::util::ser::{Writeable, Readable}; let mut r = ::std::io::Cursor::new($data); - if let Ok(msg) = <$MsgType as Readable<::std::io::Cursor<&[u8]>>>::read(&mut r) { + if let Ok(msg) = <$MsgType as Readable>::read(&mut r) { let mut w = VecWriter(Vec::new()); msg.write(&mut w).unwrap(); let p = w.0.len() as usize; diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 544df015..43a04452 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -115,8 +115,8 @@ impl Writeable for SpendableOutputDescriptor { } } -impl Readable for SpendableOutputDescriptor { - fn read(reader: &mut R) -> Result { +impl Readable for SpendableOutputDescriptor { + fn read(reader: &mut R) -> Result { match Readable::read(reader)? { 0u8 => Ok(SpendableOutputDescriptor::StaticOutput { outpoint: Readable::read(reader)?, @@ -381,8 +381,8 @@ impl Writeable for InMemoryChannelKeys { } } -impl Readable for InMemoryChannelKeys { - fn read(reader: &mut R) -> Result { +impl Readable for InMemoryChannelKeys { + fn read(reader: &mut R) -> Result { let funding_key = Readable::read(reader)?; let revocation_base_key = Readable::read(reader)?; let payment_base_key = Readable::read(reader)?; diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 3fd489fa..561448c8 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -156,8 +156,8 @@ impl Writeable for CounterpartyCommitmentSecrets { Ok(()) } } -impl Readable for CounterpartyCommitmentSecrets { - fn read(reader: &mut R) -> Result { +impl Readable for CounterpartyCommitmentSecrets { + fn read(reader: &mut R) -> Result { let mut old_secrets = [([0; 32], 1 << 48); 49]; for &mut (ref mut secret, ref mut idx) in old_secrets.iter_mut() { *secret = Readable::read(reader)?; @@ -607,8 +607,8 @@ impl Writeable for LocalCommitmentTransaction { Ok(()) } } -impl Readable for LocalCommitmentTransaction { - fn read(reader: &mut R) -> Result { +impl Readable for LocalCommitmentTransaction { + fn read(reader: &mut R) -> Result { let tx = match Transaction::consensus_decode(reader.by_ref()) { Ok(tx) => tx, Err(e) => match e { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 2e4f49ce..3dc5dcd8 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3811,9 +3811,9 @@ impl Writeable for InboundHTLCRemovalReason { } } -impl Readable for InboundHTLCRemovalReason { - fn read(reader: &mut R) -> Result { - Ok(match >::read(reader)? { +impl Readable for InboundHTLCRemovalReason { + fn read(reader: &mut R) -> Result { + Ok(match ::read(reader)? { 0 => InboundHTLCRemovalReason::FailRelay(Readable::read(reader)?), 1 => InboundHTLCRemovalReason::FailMalformed((Readable::read(reader)?, Readable::read(reader)?)), 2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?), @@ -4022,8 +4022,8 @@ impl Writeable for Channel { } } -impl> ReadableArgs> for Channel { - fn read(reader: &mut R, logger: Arc) -> Result { +impl ReadableArgs> for Channel { + fn read(reader: &mut R, logger: Arc) -> Result { let _ver: u8 = Readable::read(reader)?; let min_ver: u8 = Readable::read(reader)?; if min_ver > SERIALIZATION_VERSION { @@ -4056,7 +4056,7 @@ impl> ReadableArgs>::read(reader)? { + state: match ::read(reader)? { 1 => InboundHTLCState::AwaitingRemoteRevokeToAnnounce(Readable::read(reader)?), 2 => InboundHTLCState::AwaitingAnnouncedRemoteRevoke(Readable::read(reader)?), 3 => InboundHTLCState::Committed, @@ -4075,7 +4075,7 @@ impl> ReadableArgs>::read(reader)? { + state: match ::read(reader)? { 0 => OutboundHTLCState::LocalAnnounced(Box::new(Readable::read(reader)?)), 1 => OutboundHTLCState::Committed, 2 => OutboundHTLCState::RemoteRemoved(Readable::read(reader)?), @@ -4089,7 +4089,7 @@ impl> ReadableArgs>::read(reader)? { + holding_cell_htlc_updates.push(match ::read(reader)? { 0 => HTLCUpdateAwaitingACK::AddHTLC { amount_msat: Readable::read(reader)?, cltv_expiry: Readable::read(reader)?, @@ -4109,7 +4109,7 @@ impl> ReadableArgs>::read(reader)? { + let resend_order = match ::read(reader)? { 0 => RAACommitmentOrder::CommitmentFirst, 1 => RAACommitmentOrder::RevokeAndACKFirst, _ => return Err(DecodeError::InvalidValue), @@ -4139,7 +4139,7 @@ impl> ReadableArgs>::read(reader)? { + let last_sent_closing_fee = match ::read(reader)? { 0 => None, 1 => Some((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)), _ => return Err(DecodeError::InvalidValue), diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 476d5be8..5f7e903f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3062,8 +3062,8 @@ impl Writeable for PendingHTLCInfo { } } -impl Readable for PendingHTLCInfo { - fn read(reader: &mut R) -> Result { +impl Readable for PendingHTLCInfo { + fn read(reader: &mut R) -> Result { Ok(PendingHTLCInfo { onion_packet: Readable::read(reader)?, incoming_shared_secret: Readable::read(reader)?, @@ -3091,9 +3091,9 @@ impl Writeable for HTLCFailureMsg { } } -impl Readable for HTLCFailureMsg { - fn read(reader: &mut R) -> Result { - match >::read(reader)? { +impl Readable for HTLCFailureMsg { + fn read(reader: &mut R) -> Result { + match ::read(reader)? { 0 => Ok(HTLCFailureMsg::Relay(Readable::read(reader)?)), 1 => Ok(HTLCFailureMsg::Malformed(Readable::read(reader)?)), _ => Err(DecodeError::InvalidValue), @@ -3117,9 +3117,9 @@ impl Writeable for PendingHTLCStatus { } } -impl Readable for PendingHTLCStatus { - fn read(reader: &mut R) -> Result { - match >::read(reader)? { +impl Readable for PendingHTLCStatus { + fn read(reader: &mut R) -> Result { + match ::read(reader)? { 0 => Ok(PendingHTLCStatus::Forward(Readable::read(reader)?)), 1 => Ok(PendingHTLCStatus::Fail(Readable::read(reader)?)), _ => Err(DecodeError::InvalidValue), @@ -3151,9 +3151,9 @@ impl Writeable for HTLCSource { } } -impl Readable for HTLCSource { - fn read(reader: &mut R) -> Result { - match >::read(reader)? { +impl Readable for HTLCSource { + fn read(reader: &mut R) -> Result { + match ::read(reader)? { 0 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)), 1 => Ok(HTLCSource::OutboundRoute { route: Readable::read(reader)?, @@ -3182,9 +3182,9 @@ impl Writeable for HTLCFailReason { } } -impl Readable for HTLCFailReason { - fn read(reader: &mut R) -> Result { - match >::read(reader)? { +impl Readable for HTLCFailReason { + fn read(reader: &mut R) -> Result { + match ::read(reader)? { 0 => Ok(HTLCFailReason::LightningError { err: Readable::read(reader)? }), 1 => Ok(HTLCFailReason::Reason { failure_code: Readable::read(reader)?, @@ -3214,9 +3214,9 @@ impl Writeable for HTLCForwardInfo { } } -impl Readable for HTLCForwardInfo { - fn read(reader: &mut R) -> Result { - match >::read(reader)? { +impl Readable for HTLCForwardInfo { + fn read(reader: &mut R) -> Result { + match ::read(reader)? { 0 => Ok(HTLCForwardInfo::AddHTLC { prev_short_channel_id: Readable::read(reader)?, prev_htlc_id: Readable::read(reader)?, @@ -3355,27 +3355,27 @@ pub struct ChannelManagerReadArgs<'a, ChanSigner: 'a + ChannelKeys, M: Deref, T: // Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the // SipmleArcChannelManager type: -impl<'a, R : ::std::io::Read, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: Deref> - ReadableArgs> for (Sha256dHash, Arc>) +impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: Deref> + ReadableArgs> for (Sha256dHash, Arc>) where M::Target: ManyChannelMonitor, T::Target: BroadcasterInterface, K::Target: KeysInterface, F::Target: FeeEstimator, { - fn read(reader: &mut R, args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>) -> Result { + fn read(reader: &mut R, args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>) -> Result { let (blockhash, chan_manager) = <(Sha256dHash, ChannelManager)>::read(reader, args)?; Ok((blockhash, Arc::new(chan_manager))) } } -impl<'a, R : ::std::io::Read, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: Deref> - ReadableArgs> for (Sha256dHash, ChannelManager) +impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: Deref> + ReadableArgs> for (Sha256dHash, ChannelManager) where M::Target: ManyChannelMonitor, T::Target: BroadcasterInterface, K::Target: KeysInterface, F::Target: FeeEstimator, { - fn read(reader: &mut R, args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>) -> Result { + fn read(reader: &mut R, args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>) -> Result { let _ver: u8 = Readable::read(reader)?; let min_ver: u8 = Readable::read(reader)?; if min_ver > SERIALIZATION_VERSION { diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 1bc8c76b..c5f4fea5 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -72,8 +72,8 @@ impl Writeable for ChannelMonitorUpdate { Ok(()) } } -impl Readable for ChannelMonitorUpdate { - fn read(r: &mut R) -> Result { +impl Readable for ChannelMonitorUpdate { + fn read(r: &mut R) -> Result { let update_id: u64 = Readable::read(r)?; let len: u64 = Readable::read(r)?; let mut updates = Vec::with_capacity(cmp::min(len as usize, MAX_ALLOC_SIZE / ::std::mem::size_of::())); @@ -517,14 +517,14 @@ impl Writeable for InputMaterial { } } -impl Readable for InputMaterial { - fn read(reader: &mut R) -> Result { - let input_material = match >::read(reader)? { +impl Readable for InputMaterial { + fn read(reader: &mut R) -> Result { + let input_material = match ::read(reader)? { 0 => { let script = Readable::read(reader)?; let pubkey = Readable::read(reader)?; let key = Readable::read(reader)?; - let is_htlc = match >::read(reader)? { + let is_htlc = match ::read(reader)? { 0 => true, 1 => false, _ => return Err(DecodeError::InvalidValue), @@ -624,8 +624,8 @@ impl Writeable for ClaimTxBumpMaterial { } } -impl Readable for ClaimTxBumpMaterial { - fn read(reader: &mut R) -> Result { +impl Readable for ClaimTxBumpMaterial { + fn read(reader: &mut R) -> Result { let height_timer = Readable::read(reader)?; let feerate_previous = Readable::read(reader)?; let soonest_timelock = Readable::read(reader)?; @@ -724,8 +724,8 @@ impl Writeable for ChannelMonitorUpdateStep { Ok(()) } } -impl Readable for ChannelMonitorUpdateStep { - fn read(r: &mut R) -> Result { +impl Readable for ChannelMonitorUpdateStep { + fn read(r: &mut R) -> Result { match Readable::read(r)? { 0u8 => { Ok(ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo { @@ -751,7 +751,7 @@ impl Readable for ChannelMonitorUpdateStep { let len: u64 = Readable::read(r)?; let mut res = Vec::new(); for _ in 0..len { - res.push((Readable::read(r)?, as Readable>::read(r)?.map(|o| Box::new(o)))); + res.push((Readable::read(r)?, as Readable>::read(r)?.map(|o| Box::new(o)))); } res }, @@ -3191,8 +3191,8 @@ impl ChannelMonitor { const MAX_ALLOC_SIZE: usize = 64*1024; -impl> ReadableArgs> for (Sha256dHash, ChannelMonitor) { - fn read(reader: &mut R, logger: Arc) -> Result { +impl ReadableArgs> for (Sha256dHash, ChannelMonitor) { + fn read(reader: &mut R, logger: Arc) -> Result { let secp_ctx = Secp256k1::new(); macro_rules! unwrap_obj { ($key: expr) => { @@ -3210,9 +3210,9 @@ impl> ReadableArgs>::read(reader)?.0; + let commitment_transaction_number_obscure_factor = ::read(reader)?.0; - let key_storage = match >::read(reader)? { + let key_storage = match ::read(reader)? { 0 => { let keys = Readable::read(reader)?; let funding_key = Readable::read(reader)?; @@ -3252,7 +3252,7 @@ impl> ReadableArgs>::read(reader)?.0; + let first_idx = ::read(reader)?.0; if first_idx == 0 { None } else { @@ -3294,7 +3294,7 @@ impl> ReadableArgs as Readable>::read(reader)?.map(|o: HTLCSource| Box::new(o)))); + htlcs.push((read_htlc_in_commitment!(), as Readable>::read(reader)?.map(|o: HTLCSource| Box::new(o)))); } if let Some(_) = remote_claimable_outpoints.insert(txid, htlcs) { return Err(DecodeError::InvalidValue); @@ -3305,8 +3305,8 @@ impl> ReadableArgs>::read(reader)?.0; - let outputs_count = >::read(reader)?; + let commitment_number = ::read(reader)?.0; + let outputs_count = ::read(reader)?; let mut outputs = Vec::with_capacity(cmp::min(outputs_count as usize, MAX_ALLOC_SIZE / 8)); for _ in 0..outputs_count { outputs.push(Readable::read(reader)?); @@ -3320,7 +3320,7 @@ impl> ReadableArgs>::read(reader)?.0; + let commitment_number = ::read(reader)?.0; if let Some(_) = remote_hash_commitment_number.insert(payment_hash, commitment_number) { return Err(DecodeError::InvalidValue); } @@ -3329,7 +3329,7 @@ impl> ReadableArgs { { - let tx = >::read(reader)?; + let tx = ::read(reader)?; let revocation_key = Readable::read(reader)?; let a_htlc_key = Readable::read(reader)?; let b_htlc_key = Readable::read(reader)?; @@ -3341,7 +3341,7 @@ impl> ReadableArgs>::read(reader)? { + let sigs = match ::read(reader)? { 0 => None, 1 => Some(Readable::read(reader)?), _ => return Err(DecodeError::InvalidValue), @@ -3358,7 +3358,7 @@ impl> ReadableArgs>::read(reader)? { + let prev_local_signed_commitment_tx = match ::read(reader)? { 0 => None, 1 => { Some(read_local_tx!()) @@ -3366,7 +3366,7 @@ impl> ReadableArgs return Err(DecodeError::InvalidValue), }; - let current_local_signed_commitment_tx = match >::read(reader)? { + let current_local_signed_commitment_tx = match ::read(reader)? { 0 => None, 1 => { Some(read_local_tx!()) @@ -3374,7 +3374,7 @@ impl> ReadableArgs return Err(DecodeError::InvalidValue), }; - let current_remote_commitment_number = >::read(reader)?.0; + let current_remote_commitment_number = ::read(reader)?.0; let payment_preimages_len: u64 = Readable::read(reader)?; let mut payment_preimages = HashMap::with_capacity(cmp::min(payment_preimages_len as usize, MAX_ALLOC_SIZE / 32)); @@ -3402,7 +3402,7 @@ impl> ReadableArgs>::read(reader)? { + let to_remote_rescue = match ::read(reader)? { 0 => None, 1 => { let to_remote_script = Readable::read(reader)?; @@ -3434,7 +3434,7 @@ impl> ReadableArgs>::read(reader)? { + let ev = match ::read(reader)? { 0 => { let claim_request = Readable::read(reader)?; OnchainEvent::Claim { diff --git a/lightning/src/ln/features.rs b/lightning/src/ln/features.rs index 61a862c0..990e0478 100644 --- a/lightning/src/ln/features.rs +++ b/lightning/src/ln/features.rs @@ -283,8 +283,8 @@ impl Writeable for Features { } } -impl Readable for Features { - fn read(r: &mut R) -> Result { +impl Readable for Features { + fn read(r: &mut R) -> Result { let mut flags: Vec = Readable::read(r)?; flags.reverse(); // Swap to little-endian Ok(Self { diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index b50e35b9..8d23ac9a 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -336,9 +336,9 @@ impl Writeable for NetAddress { } } -impl Readable for Result { - fn read(reader: &mut R) -> Result, DecodeError> { - let byte = >::read(reader)?; +impl Readable for Result { + fn read(reader: &mut R) -> Result, DecodeError> { + let byte = ::read(reader)?; match byte { 1 => { Ok(Ok(NetAddress::IPv4 { @@ -718,9 +718,9 @@ impl Writeable for OptionalField