From: Antoine Riard Date: Tue, 7 Sep 2021 23:52:10 +0000 (-0400) Subject: -f Matt's comments X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=1ad3eae91c641e7ace4bb01f5fddabe6b78904ce;p=rust-lightning -f Matt's comments --- diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 482c1a2a..9a25af26 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -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 }); + $self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id, err: ClosureDescriptor::ProcessingError { err: err.err.clone() } }); } } @@ -1447,7 +1447,7 @@ impl ChannelMana } } - fn force_close_channel_with_peer(&self, channel_id: &[u8; 32], peer_node_id: Option<&PublicKey>) -> Result { + fn force_close_channel_with_peer(&self, channel_id: &[u8; 32], peer_node_id: Option<&PublicKey>, peer_msg: Option<&String>) -> Result { let mut chan = { let mut channel_state_lock = self.channel_state.lock().unwrap(); let channel_state = &mut *channel_state_lock; @@ -1473,7 +1473,7 @@ impl ChannelMana msg: update }); } - self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: *channel_id, err: ClosureDescriptor::ForceClosed }); + 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()) } @@ -1482,7 +1482,7 @@ impl ChannelMana /// the chain and rejecting new HTLCs on the given channel. Fails if channel_id is unknown to the manager. pub fn force_close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError> { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); - match self.force_close_channel_with_peer(channel_id, None) { + match self.force_close_channel_with_peer(channel_id, None, None) { Ok(counterparty_node_id) => { self.channel_state.lock().unwrap().pending_msg_events.push( events::MessageSendEvent::HandleError { @@ -3556,6 +3556,7 @@ impl ChannelMana msg: update }); } + //TODO: split between CounterpartyInitiated/LocallyInitiated self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: msg.channel_id, err: ClosureDescriptor::CooperativeClosure }); } Ok(()) @@ -3968,7 +3969,7 @@ impl ChannelMana msg: update }); } - self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), err: ClosureDescriptor::UnknownOnchainCommitment }); + self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), err: ClosureDescriptor::CommitmentTxBroadcasted }); pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: chan.get_counterparty_node_id(), action: msgs::ErrorAction::SendErrorMessage { @@ -4504,7 +4505,7 @@ where msg: update }); } - self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: channel.channel_id(), err: ClosureDescriptor::UnknownOnchainCommitment }); + self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: channel.channel_id(), err: ClosureDescriptor::CommitmentTxBroadcasted }); pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: channel.get_counterparty_node_id(), action: msgs::ErrorAction::SendErrorMessage { msg: e }, @@ -4710,7 +4711,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::ProcessingError }); + self.pending_events.lock().unwrap().push(events::Event::ChannelClosed { channel_id: chan.channel_id(), err: ClosureDescriptor::DisconnectedPeer }); return false; } else { no_channels_remain = false; @@ -4801,12 +4802,12 @@ impl for chan in self.list_channels() { if chan.counterparty.node_id == *counterparty_node_id { // Untrusted messages from peer, we throw away the error if id points to a non-existent channel - let _ = self.force_close_channel_with_peer(&chan.channel_id, Some(counterparty_node_id)); + let _ = self.force_close_channel_with_peer(&chan.channel_id, Some(counterparty_node_id), Some(&msg.data)); } } } 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)); + let _ = self.force_close_channel_with_peer(&msg.channel_id, Some(counterparty_node_id), None); } } } diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 675b8391..d4453d9f 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -71,25 +71,52 @@ pub enum PaymentPurpose { #[derive(Clone, Debug)] pub enum ClosureDescriptor { - ForceClosed, - UserInitiated, - CounterpartyInitiated, + /// 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 + peer_msg: Option, + }, + /// Closure generated from receiving a peer's ClosingSigned message. Note the shutdown + /// sequence might have been initially initiated by us. CooperativeClosure, - UnknownOnchainCommitment, - ProcessingError, + /// Closure generated from receiving chain::Watch's CommitmentTxBroadcast event. + CommitmentTxBroadcasted, + /// Closure generated from processing an event, likely a HTLC forward/relay/reception. + ProcessingError { + err: String, + }, + /// Closure generated from ChannelManager::peer_disconnected. DisconnectedPeer, } impl Writeable for ClosureDescriptor { fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { match self { - ClosureDescriptor::ForceClosed => 0u8.write(writer)?, - ClosureDescriptor::UserInitiated => 1u8.write(writer)?, - ClosureDescriptor::CounterpartyInitiated => 2u8.write(writer)?, - ClosureDescriptor::CooperativeClosure => 3u8.write(writer)?, - ClosureDescriptor::UnknownOnchainCommitment => 4u8.write(writer)?, - ClosureDescriptor::ProcessingError => 5u8.write(writer)?, - ClosureDescriptor::DisconnectedPeer => 6u8.write(writer)?, + 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(()) } @@ -98,13 +125,34 @@ impl Writeable for ClosureDescriptor { impl Readable for ClosureDescriptor { fn read(reader: &mut R) -> Result { Ok(match ::read(reader)? { - 0 => ClosureDescriptor::ForceClosed, - 1 => ClosureDescriptor::UserInitiated, - 2 => ClosureDescriptor::CounterpartyInitiated, - 3 => ClosureDescriptor::CooperativeClosure, - 4 => ClosureDescriptor::UnknownOnchainCommitment, - 5 => ClosureDescriptor::ProcessingError, - 6 => ClosureDescriptor::DisconnectedPeer, + 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), }) } @@ -320,7 +368,7 @@ impl Writeable for Event { }); }, &Event::ChannelClosed { ref channel_id, ref err } => { - 6u8.write(writer)?; + 9u8.write(writer)?; channel_id.write(writer)?; err.write(writer)?; write_tlv_fields!(writer, {}); @@ -438,14 +486,14 @@ impl MaybeReadable for Event { }; f() }, - // Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue. - x if x % 2 == 1 => Ok(None), - 6u8 => { + 9u8 => { let channel_id = Readable::read(reader)?; let err = Readable::read(reader)?; read_tlv_fields!(reader, {}); Ok(Some(Event::ChannelClosed { channel_id, err})) }, + // Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue. + x if x % 2 == 1 => Ok(None), _ => Err(msgs::DecodeError::InvalidValue) } }