Skip to content

Fix Router Serialization Roundtrip #523

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 4 commits into from
Mar 4, 2020
Merged

Fix Router Serialization Roundtrip #523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
39b6233
Impl ReadableArgs for Arc<ChannelManager>, not just ChannelManager.
TheBlueMatt Jan 27, 2020
32ca8ec
Make Readable::read a templated on the stream, not Readable itself
TheBlueMatt Feb 24, 2020
b80d3d9
Change Option<T> serialization format to include length
TheBlueMatt Feb 24, 2020
5f3986d
Test Router serialization round-trip in functional_tests.
TheBlueMatt Feb 24, 2020
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
10 changes: 5 additions & 5 deletions fuzz/src/msg_targets/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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[..]);
Expand All @@ -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[..]);
Expand All @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/chain/keysinterface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ impl Writeable for SpendableOutputDescriptor {
}
}

impl<R: ::std::io::Read> Readable<R> for SpendableOutputDescriptor {
fn read(reader: &mut R) -> Result<Self, DecodeError> {
impl Readable for SpendableOutputDescriptor {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
match Readable::read(reader)? {
0u8 => Ok(SpendableOutputDescriptor::StaticOutput {
outpoint: Readable::read(reader)?,
Expand Down Expand Up @@ -381,8 +381,8 @@ impl Writeable for InMemoryChannelKeys {
}
}

impl<R: ::std::io::Read> Readable<R> for InMemoryChannelKeys {
fn read(reader: &mut R) -> Result<Self, DecodeError> {
impl Readable for InMemoryChannelKeys {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
let funding_key = Readable::read(reader)?;
let revocation_base_key = Readable::read(reader)?;
let payment_base_key = Readable::read(reader)?;
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/ln/chan_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ impl Writeable for CounterpartyCommitmentSecrets {
Ok(())
}
}
impl<R: ::std::io::Read> Readable<R> for CounterpartyCommitmentSecrets {
fn read(reader: &mut R) -> Result<Self, DecodeError> {
impl Readable for CounterpartyCommitmentSecrets {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
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)?;
Expand Down Expand Up @@ -607,8 +607,8 @@ impl Writeable for LocalCommitmentTransaction {
Ok(())
}
}
impl<R: ::std::io::Read> Readable<R> for LocalCommitmentTransaction {
fn read(reader: &mut R) -> Result<Self, DecodeError> {
impl Readable for LocalCommitmentTransaction {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
let tx = match Transaction::consensus_decode(reader.by_ref()) {
Ok(tx) => tx,
Err(e) => match e {
Expand Down
56 changes: 22 additions & 34 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3811,9 +3811,9 @@ impl Writeable for InboundHTLCRemovalReason {
}
}

impl<R: ::std::io::Read> Readable<R> for InboundHTLCRemovalReason {
fn read(reader: &mut R) -> Result<Self, DecodeError> {
Ok(match <u8 as Readable<R>>::read(reader)? {
impl Readable for InboundHTLCRemovalReason {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
Ok(match <u8 as Readable>::read(reader)? {
0 => InboundHTLCRemovalReason::FailRelay(Readable::read(reader)?),
1 => InboundHTLCRemovalReason::FailMalformed((Readable::read(reader)?, Readable::read(reader)?)),
2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
Expand Down Expand Up @@ -3883,18 +3883,6 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
}
}

macro_rules! write_option {
($thing: expr) => {
match &$thing {
&None => 0u8.write(writer)?,
&Some(ref v) => {
1u8.write(writer)?;
v.write(writer)?;
},
}
}
}

(self.pending_outbound_htlcs.len() as u64).write(writer)?;
for htlc in self.pending_outbound_htlcs.iter() {
htlc.htlc_id.write(writer)?;
Expand All @@ -3912,15 +3900,15 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
},
&OutboundHTLCState::RemoteRemoved(ref fail_reason) => {
2u8.write(writer)?;
write_option!(*fail_reason);
fail_reason.write(writer)?;
},
&OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref fail_reason) => {
3u8.write(writer)?;
write_option!(*fail_reason);
fail_reason.write(writer)?;
},
&OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref fail_reason) => {
4u8.write(writer)?;
write_option!(*fail_reason);
fail_reason.write(writer)?;
},
}
}
Expand Down Expand Up @@ -3971,8 +3959,8 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
fail_reason.write(writer)?;
}

write_option!(self.pending_update_fee);
write_option!(self.holding_cell_update_fee);
self.pending_update_fee.write(writer)?;
self.holding_cell_update_fee.write(writer)?;

self.next_local_htlc_id.write(writer)?;
(self.next_remote_htlc_id - dropped_inbound_htlcs).write(writer)?;
Expand All @@ -3989,9 +3977,9 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
None => 0u8.write(writer)?,
}

write_option!(self.funding_txo);
write_option!(self.funding_tx_confirmed_in);
write_option!(self.short_channel_id);
self.funding_txo.write(writer)?;
self.funding_tx_confirmed_in.write(writer)?;
self.short_channel_id.write(writer)?;

self.last_block_connected.write(writer)?;
self.funding_tx_confirmations.write(writer)?;
Expand All @@ -4007,13 +3995,13 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
self.their_max_accepted_htlcs.write(writer)?;
self.minimum_depth.write(writer)?;

write_option!(self.their_pubkeys);
write_option!(self.their_cur_commitment_point);
self.their_pubkeys.write(writer)?;
self.their_cur_commitment_point.write(writer)?;

write_option!(self.their_prev_commitment_point);
self.their_prev_commitment_point.write(writer)?;
self.their_node_id.write(writer)?;

write_option!(self.their_shutdown_scriptpubkey);
self.their_shutdown_scriptpubkey.write(writer)?;

self.commitment_secrets.write(writer)?;

Expand All @@ -4022,8 +4010,8 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
}
}

impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R, Arc<Logger>> for Channel<ChanSigner> {
fn read(reader: &mut R, logger: Arc<Logger>) -> Result<Self, DecodeError> {
impl<ChanSigner: ChannelKeys + Readable> ReadableArgs<Arc<Logger>> for Channel<ChanSigner> {
fn read<R : ::std::io::Read>(reader: &mut R, logger: Arc<Logger>) -> Result<Self, DecodeError> {
let _ver: u8 = Readable::read(reader)?;
let min_ver: u8 = Readable::read(reader)?;
if min_ver > SERIALIZATION_VERSION {
Expand Down Expand Up @@ -4056,7 +4044,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
amount_msat: Readable::read(reader)?,
cltv_expiry: Readable::read(reader)?,
payment_hash: Readable::read(reader)?,
state: match <u8 as Readable<R>>::read(reader)? {
state: match <u8 as Readable>::read(reader)? {
1 => InboundHTLCState::AwaitingRemoteRevokeToAnnounce(Readable::read(reader)?),
2 => InboundHTLCState::AwaitingAnnouncedRemoteRevoke(Readable::read(reader)?),
3 => InboundHTLCState::Committed,
Expand All @@ -4075,7 +4063,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
cltv_expiry: Readable::read(reader)?,
payment_hash: Readable::read(reader)?,
source: Readable::read(reader)?,
state: match <u8 as Readable<R>>::read(reader)? {
state: match <u8 as Readable>::read(reader)? {
0 => OutboundHTLCState::LocalAnnounced(Box::new(Readable::read(reader)?)),
1 => OutboundHTLCState::Committed,
2 => OutboundHTLCState::RemoteRemoved(Readable::read(reader)?),
Expand All @@ -4089,7 +4077,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
let holding_cell_htlc_update_count: u64 = Readable::read(reader)?;
let mut holding_cell_htlc_updates = Vec::with_capacity(cmp::min(holding_cell_htlc_update_count as usize, OUR_MAX_HTLCS as usize*2));
for _ in 0..holding_cell_htlc_update_count {
holding_cell_htlc_updates.push(match <u8 as Readable<R>>::read(reader)? {
holding_cell_htlc_updates.push(match <u8 as Readable>::read(reader)? {
0 => HTLCUpdateAwaitingACK::AddHTLC {
amount_msat: Readable::read(reader)?,
cltv_expiry: Readable::read(reader)?,
Expand All @@ -4109,7 +4097,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
});
}

let resend_order = match <u8 as Readable<R>>::read(reader)? {
let resend_order = match <u8 as Readable>::read(reader)? {
0 => RAACommitmentOrder::CommitmentFirst,
1 => RAACommitmentOrder::RevokeAndACKFirst,
_ => return Err(DecodeError::InvalidValue),
Expand Down Expand Up @@ -4139,7 +4127,7 @@ impl<R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>> ReadableArgs<R,
let channel_update_count = Readable::read(reader)?;
let feerate_per_kw = Readable::read(reader)?;

let last_sent_closing_fee = match <u8 as Readable<R>>::read(reader)? {
let last_sent_closing_fee = match <u8 as Readable>::read(reader)? {
0 => None,
1 => Some((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)),
_ => return Err(DecodeError::InvalidValue),
Expand Down
55 changes: 35 additions & 20 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3062,8 +3062,8 @@ impl Writeable for PendingHTLCInfo {
}
}

impl<R: ::std::io::Read> Readable<R> for PendingHTLCInfo {
fn read(reader: &mut R) -> Result<PendingHTLCInfo, DecodeError> {
impl Readable for PendingHTLCInfo {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<PendingHTLCInfo, DecodeError> {
Ok(PendingHTLCInfo {
onion_packet: Readable::read(reader)?,
incoming_shared_secret: Readable::read(reader)?,
Expand Down Expand Up @@ -3091,9 +3091,9 @@ impl Writeable for HTLCFailureMsg {
}
}

impl<R: ::std::io::Read> Readable<R> for HTLCFailureMsg {
fn read(reader: &mut R) -> Result<HTLCFailureMsg, DecodeError> {
match <u8 as Readable<R>>::read(reader)? {
impl Readable for HTLCFailureMsg {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<HTLCFailureMsg, DecodeError> {
match <u8 as Readable>::read(reader)? {
0 => Ok(HTLCFailureMsg::Relay(Readable::read(reader)?)),
1 => Ok(HTLCFailureMsg::Malformed(Readable::read(reader)?)),
_ => Err(DecodeError::InvalidValue),
Expand All @@ -3117,9 +3117,9 @@ impl Writeable for PendingHTLCStatus {
}
}

impl<R: ::std::io::Read> Readable<R> for PendingHTLCStatus {
fn read(reader: &mut R) -> Result<PendingHTLCStatus, DecodeError> {
match <u8 as Readable<R>>::read(reader)? {
impl Readable for PendingHTLCStatus {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<PendingHTLCStatus, DecodeError> {
match <u8 as Readable>::read(reader)? {
0 => Ok(PendingHTLCStatus::Forward(Readable::read(reader)?)),
1 => Ok(PendingHTLCStatus::Fail(Readable::read(reader)?)),
_ => Err(DecodeError::InvalidValue),
Expand Down Expand Up @@ -3151,9 +3151,9 @@ impl Writeable for HTLCSource {
}
}

impl<R: ::std::io::Read> Readable<R> for HTLCSource {
fn read(reader: &mut R) -> Result<HTLCSource, DecodeError> {
match <u8 as Readable<R>>::read(reader)? {
impl Readable for HTLCSource {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<HTLCSource, DecodeError> {
match <u8 as Readable>::read(reader)? {
0 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)),
1 => Ok(HTLCSource::OutboundRoute {
route: Readable::read(reader)?,
Expand Down Expand Up @@ -3182,9 +3182,9 @@ impl Writeable for HTLCFailReason {
}
}

impl<R: ::std::io::Read> Readable<R> for HTLCFailReason {
fn read(reader: &mut R) -> Result<HTLCFailReason, DecodeError> {
match <u8 as Readable<R>>::read(reader)? {
impl Readable for HTLCFailReason {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<HTLCFailReason, DecodeError> {
match <u8 as Readable>::read(reader)? {
0 => Ok(HTLCFailReason::LightningError { err: Readable::read(reader)? }),
1 => Ok(HTLCFailReason::Reason {
failure_code: Readable::read(reader)?,
Expand Down Expand Up @@ -3214,9 +3214,9 @@ impl Writeable for HTLCForwardInfo {
}
}

impl<R: ::std::io::Read> Readable<R> for HTLCForwardInfo {
fn read(reader: &mut R) -> Result<HTLCForwardInfo, DecodeError> {
match <u8 as Readable<R>>::read(reader)? {
impl Readable for HTLCForwardInfo {
fn read<R: ::std::io::Read>(reader: &mut R) -> Result<HTLCForwardInfo, DecodeError> {
match <u8 as Readable>::read(reader)? {
0 => Ok(HTLCForwardInfo::AddHTLC {
prev_short_channel_id: Readable::read(reader)?,
prev_htlc_id: Readable::read(reader)?,
Expand Down Expand Up @@ -3353,14 +3353,29 @@ pub struct ChannelManagerReadArgs<'a, ChanSigner: 'a + ChannelKeys, M: Deref, T:
pub channel_monitors: &'a mut HashMap<OutPoint, &'a mut ChannelMonitor<ChanSigner>>,
}

impl<'a, R : ::std::io::Read, ChanSigner: ChannelKeys + Readable<R>, M: Deref, T: Deref, K: Deref, F: Deref>
ReadableArgs<R, ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>> for (Sha256dHash, ChannelManager<ChanSigner, M, T, K, F>)
// Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the
// SipmleArcChannelManager type:
impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: Deref>
ReadableArgs<ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>> for (Sha256dHash, Arc<ChannelManager<ChanSigner, M, T, K, F>>)
where M::Target: ManyChannelMonitor<ChanSigner>,
T::Target: BroadcasterInterface,
K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
F::Target: FeeEstimator,
{
fn read(reader: &mut R, args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>) -> Result<Self, DecodeError> {
fn read<R: ::std::io::Read>(reader: &mut R, args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>) -> Result<Self, DecodeError> {
let (blockhash, chan_manager) = <(Sha256dHash, ChannelManager<ChanSigner, M, T, K, F>)>::read(reader, args)?;
Ok((blockhash, Arc::new(chan_manager)))
}
}

impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: Deref>
ReadableArgs<ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>> for (Sha256dHash, ChannelManager<ChanSigner, M, T, K, F>)
where M::Target: ManyChannelMonitor<ChanSigner>,
T::Target: BroadcasterInterface,
K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
F::Target: FeeEstimator,
{
fn read<R: ::std::io::Read>(reader: &mut R, args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F>) -> Result<Self, DecodeError> {
let _ver: u8 = Readable::read(reader)?;
let min_ver: u8 = Readable::read(reader)?;
if min_ver > SERIALIZATION_VERSION {
Expand Down
Loading