From eab9b3452657cef788eb54e3f46eda4fb06f9197 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Mon, 13 Sep 2021 10:35:47 -0400 Subject: [PATCH] -f fix new Matt's comments --- lightning/src/ln/channelmanager.rs | 27 +++++--- lightning/src/util/events.rs | 102 ++++++----------------------- lightning/src/util/ser.rs | 17 +++++ 3 files changed, 56 insertions(+), 90 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9a25af266..e4dd740da 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -52,7 +52,7 @@ use ln::onion_utils; use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, OptionalField}; use chain::keysinterface::{Sign, KeysInterface, KeysManager, InMemorySigner}; use util::config::UserConfig; -use util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureDescriptor}; +use util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason}; use util::{byte_utils, events}; use util::ser::{Readable, ReadableArgs, MaybeReadable, Writeable, Writer}; use util::chacha20::{ChaCha20, ChaChaReader}; @@ -837,7 +837,7 @@ macro_rules! handle_error { }); } if let Some(channel_id) = chan_id { - $self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id, err: ClosureDescriptor::ProcessingError { err: err.err.clone() } }); + $self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id, err: ClosureReason::ProcessingError { err: err.err.clone() } }); } } @@ -1460,6 +1460,16 @@ impl ChannelMana if let Some(short_id) = chan.get().get_short_channel_id() { channel_state.short_to_id.remove(&short_id); } + let mut pending_events_lock = self.pending_events.lock().unwrap(); + if peer_node_id.is_some() { + if let Some(peer_msg) = peer_msg { + pending_events_lock.push(events::Event::ChannelClosed { channel_id: *channel_id, err: ClosureReason::CounterpartyForceClosed { peer_msg: Some(peer_msg.to_string()) } }); + } else { + pending_events_lock.push(events::Event::ChannelClosed { channel_id: *channel_id, err: ClosureReason::CounterpartyForceClosed { peer_msg: None } }); + } + } else { + pending_events_lock.push(events::Event::ChannelClosed { channel_id: *channel_id, err: ClosureReason::HolderForceClosed }); + } chan.remove_entry().1 } else { return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()}); @@ -1473,7 +1483,6 @@ impl ChannelMana msg: update }); } - self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: *channel_id, err: ClosureDescriptor::ForceClosed { peer_msg: if peer_msg.is_some() { Some(peer_msg.unwrap().clone()) } else { None }}}); Ok(chan.get_counterparty_node_id()) } @@ -3557,7 +3566,7 @@ impl ChannelMana }); } //TODO: split between CounterpartyInitiated/LocallyInitiated - self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: msg.channel_id, err: ClosureDescriptor::CooperativeClosure }); + self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: msg.channel_id, err: ClosureReason::CooperativeClosure }); } Ok(()) } @@ -3969,7 +3978,7 @@ impl ChannelMana msg: update }); } - self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), err: ClosureDescriptor::CommitmentTxBroadcasted }); + self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), err: ClosureReason::CommitmentTxBroadcasted }); pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: chan.get_counterparty_node_id(), action: msgs::ErrorAction::SendErrorMessage { @@ -4505,7 +4514,7 @@ where msg: update }); } - self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: channel.channel_id(), err: ClosureDescriptor::CommitmentTxBroadcasted }); + self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: channel.channel_id(), err: ClosureReason::CommitmentTxBroadcasted }); pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: channel.get_counterparty_node_id(), action: msgs::ErrorAction::SendErrorMessage { msg: e }, @@ -4696,7 +4705,7 @@ impl msg: update }); } - self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), err: ClosureDescriptor::DisconnectedPeer }); + self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), err: ClosureReason::DisconnectedPeer }); false } else { true @@ -4711,7 +4720,7 @@ impl if let Some(short_id) = chan.get_short_channel_id() { short_to_id.remove(&short_id); } - self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), err: ClosureDescriptor::DisconnectedPeer }); + self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), err: ClosureReason::DisconnectedPeer }); return false; } else { no_channels_remain = false; @@ -4807,7 +4816,7 @@ impl } } else { // Untrusted messages from peer, we throw away the error if id points to a non-existent channel - let _ = self.force_close_channel_with_peer(&msg.channel_id, Some(counterparty_node_id), None); + let _ = self.force_close_channel_with_peer(&msg.channel_id, Some(counterparty_node_id), Some(&msg.data)); } } } diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index d4453d9f2..2e54e2f3f 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -70,13 +70,15 @@ pub enum PaymentPurpose { } #[derive(Clone, Debug)] -pub enum ClosureDescriptor { - /// Closure generated from ChannelManager::force_close_channel or receiving a peer error - /// message by ChannelManager::handle_error - ForceClosed { - /// If the error is coming from the peer, there should be a human-readable msg +/// Some information provided on the closure source of the channel halting. +pub enum ClosureReason { + /// Closure generated from receiving a peer error message by ChannelManager::handle_error + CounterpartyForceClosed { + /// The error is coming from the peer, there *might* be a human-readable msg peer_msg: Option, }, + /// Closure generated from ChannelManager::force_close_channel + HolderForceClosed, /// Closure generated from receiving a peer's ClosingSigned message. Note the shutdown /// sequence might have been initially initiated by us. CooperativeClosure, @@ -90,73 +92,14 @@ pub enum ClosureDescriptor { DisconnectedPeer, } -impl Writeable for ClosureDescriptor { - fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { - match self { - ClosureDescriptor::ForceClosed { peer_msg } => { - 0u8.write(writer)?; - if let Some(peer_msg) = peer_msg { - 0u8.write(writer)?; - let bytes = peer_msg.clone().into_bytes(); - (bytes.len() as u64).write(writer)?; - for b in bytes.iter() { - b.write(writer)?; - } - } else { - 1u8.write(writer)?; - } - } - ClosureDescriptor::CooperativeClosure => 1u8.write(writer)?, - ClosureDescriptor::CommitmentTxBroadcasted => 2u8.write(writer)?, - ClosureDescriptor::ProcessingError { err } => { - 3u8.write(writer)?; - let bytes = err.clone().into_bytes(); - (bytes.len() as u64).write(writer)?; - for b in bytes.iter() { - b.write(writer)?; - } - }, - ClosureDescriptor::DisconnectedPeer => 4u8.write(writer)?, - } - Ok(()) - } -} - -impl Readable for ClosureDescriptor { - fn read(reader: &mut R) -> Result { - Ok(match ::read(reader)? { - 0 => { - let peer_msg = match ::read(reader)? { - 0 => { - let bytes_len: u64 = Readable::read(reader)?; - let mut bytes: Vec = Vec::with_capacity(bytes_len as usize); - for _ in 0..bytes_len { - bytes.push(Readable::read(reader)?); - } - let err = String::from_utf8(bytes).unwrap(); - Some(err) - }, - 1 => None, - _ => return Err(DecodeError::InvalidValue), - }; - ClosureDescriptor::ForceClosed { peer_msg } - }, - 1 => ClosureDescriptor::CooperativeClosure, - 2 => ClosureDescriptor::CommitmentTxBroadcasted, - 3 => { - let bytes_len: u64 = Readable::read(reader)?; - let mut bytes: Vec = Vec::with_capacity(bytes_len as usize); - for _ in 0..bytes_len { - bytes.push(Readable::read(reader)?); - } - let err = String::from_utf8(bytes).unwrap(); - ClosureDescriptor::ProcessingError { err } - }, - 4 => ClosureDescriptor::DisconnectedPeer, - _ => return Err(DecodeError::InvalidValue), - }) - } -} +impl_writeable_tlv_based_enum_upgradable!(ClosureReason, + (0, CounterpartyForceClosed) => { (1, peer_msg, option) }, + (2, HolderForceClosed) => {}, + (6, CommitmentTxBroadcasted) => {}, + (4, CooperativeClosure) => {}, + (8, ProcessingError) => { (1, err, required) }, + (10, DisconnectedPeer) => {}, +); /// An Event which you should probably take some action in response to. /// @@ -280,16 +223,12 @@ pub enum Event { claim_from_onchain_tx: bool, }, /// Used to indicate that a channel with the given `channel_id` is in the process of closure. - /// Note that if you try to force-close multiple times a channel through - /// [`ChannelManager::force_close_channel`] before receiving the corresponding monitor - /// event for the broadcast of the commitment transaction, multiple `ChannelClosed` events - /// can be generated. ChannelClosed { /// The channel_id which has been barren from further off-chain updates but /// funding output might still be not resolved yet. channel_id: [u8; 32], /// A machine-readable error message - err: ClosureDescriptor + err: ClosureReason } } @@ -368,7 +307,7 @@ impl Writeable for Event { }); }, &Event::ChannelClosed { ref channel_id, ref err } => { - 9u8.write(writer)?; + 8u8.write(writer)?; channel_id.write(writer)?; err.write(writer)?; write_tlv_fields!(writer, {}); @@ -486,11 +425,12 @@ impl MaybeReadable for Event { }; f() }, - 9u8 => { + 8u8 => { let channel_id = Readable::read(reader)?; - let err = Readable::read(reader)?; + let err = MaybeReadable::read(reader)?; read_tlv_fields!(reader, {}); - Ok(Some(Event::ChannelClosed { channel_id, err})) + if err.is_none() { return Ok(None); } + Ok(Some(Event::ChannelClosed { channel_id, err: err.unwrap() })) }, // Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue. x if x % 2 == 1 => Ok(None), diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index c76b70181..83059620e 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -897,3 +897,20 @@ impl Readable for () { Ok(()) } } + +impl Writeable for String { + #[inline] + fn write(&self, w: &mut W) -> Result<(), io::Error> { + (self.len() as u16).write(w)?; + w.write_all(self.as_bytes()) + } +} + +impl Readable for String { + #[inline] + fn read(r: &mut R) -> Result { + let v: Vec = Readable::read(r)?; + let ret = String::from_utf8(v).map_err(|_| DecodeError::InvalidValue)?; + Ok(ret) + } +} -- 2.39.5