X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=src%2Fln%2Fchannelmanager.rs;h=912e22abb63d06eccf3f95c8d93a98f5e8ddcb2f;hb=3117e1812b2e448ac1386d7c37b673c99d4ef723;hp=ba2bfefccebaad2f68d1d2fee27681eee6a21229;hpb=4cceb58f91e8cb0e91b9c1fd2e90646a00c21541;p=rust-lightning diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index ba2bfefc..912e22ab 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -161,6 +161,16 @@ impl MsgHandleErrInternal { } } #[inline] + fn ignore_no_close(err: &'static str) -> Self { + Self { + err: HandleError { + err, + action: Some(msgs::ErrorAction::IgnoreError), + }, + shutdown_finish: None, + } + } + #[inline] fn from_no_close(err: msgs::HandleError) -> Self { Self { err, shutdown_finish: None } } @@ -239,7 +249,7 @@ pub(super) struct ChannelHolder { pub(super) next_forward: Instant, /// short channel id -> forward infos. Key of 0 means payments received /// Note that while this is held in the same mutex as the channels themselves, no consistency - /// guarantees are made about there existing a channel with the short id here, nor the short + /// guarantees are made about the existence of a channel with the short id here, nor the short /// ids in the PendingForwardHTLCInfo! pub(super) forward_htlcs: HashMap>, /// Note that while this is held in the same mutex as the channels themselves, no consistency @@ -334,7 +344,7 @@ pub struct ChannelManager { /// HTLC's CLTV. This should always be a few blocks greater than channelmonitor::CLTV_CLAIM_BUFFER, /// ie the node we forwarded the payment on to should always have enough room to reliably time out /// the HTLC via a full update_fail_htlc/commitment_signed dance before we hit the -/// CLTV_CLAIM_BUFFER point (we static assert that its at least 3 blocks more). +/// CLTV_CLAIM_BUFFER point (we static assert that it's at least 3 blocks more). const CLTV_EXPIRY_DELTA: u16 = 6 * 12; //TODO? pub(super) const CLTV_FAR_FAR_AWAY: u32 = 6 * 24 * 7; //TODO? @@ -461,6 +471,12 @@ macro_rules! return_monitor_err { return Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure", channel_id, chan.force_shutdown(), $self.get_channel_update(&chan).ok())) }, ChannelMonitorUpdateErr::TemporaryFailure => { + if !$resend_commitment { + debug_assert!($action_type == RAACommitmentOrder::RevokeAndACKFirst || !$resend_raa); + } + if !$resend_raa { + debug_assert!($action_type == RAACommitmentOrder::CommitmentFirst || !$resend_commitment); + } $entry.get_mut().monitor_update_failed($action_type, $resend_raa, $resend_commitment, $failed_forwards, $failed_fails); return Err(MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore("Failed to update ChannelMonitor"), *$entry.key())); }, @@ -734,7 +750,7 @@ impl ChannelManager { if msg.onion_routing_packet.version != 0 { //TODO: Spec doesn't indicate if we should only hash hop_data here (and in other //sha256_of_onion error data packets), or the entire onion_routing_packet. Either way, - //the hash doesn't really serve any purpuse - in the case of hashing all data, the + //the hash doesn't really serve any purpose - in the case of hashing all data, the //receiving node would have to brute force to figure out which version was put in the //packet by the node that send us the message, in the case of hashing the hop_data, the //node knows the HMAC matched, so they already know what is there... @@ -1136,7 +1152,7 @@ impl ChannelManager { /// Processes HTLCs which are pending waiting on random forward delay. /// - /// Should only really ever be called in response to an PendingHTLCsForwardable event. + /// Should only really ever be called in response to a PendingHTLCsForwardable event. /// Will likely generate further events. pub fn process_pending_htlc_forwards(&self) { let _ = self.total_consistency_lock.read().unwrap(); @@ -1241,7 +1257,7 @@ impl ChannelManager { // messages when we can. // We don't need any kind of timer here as they should fail // the channel onto the chain if they can't get our - // update_fail_htlc in time, its not our problem. + // update_fail_htlc in time, it's not our problem. } } }, @@ -1470,7 +1486,7 @@ impl ChannelManager { None => { // TODO: There is probably a channel manager somewhere that needs to // learn the preimage as the channel already hit the chain and that's - // why its missing. + // why it's missing. return } }; @@ -1480,7 +1496,7 @@ impl ChannelManager { Ok((msgs, monitor_option)) => { if let Some(chan_monitor) = monitor_option { if let Err(_e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) { - unimplemented!();// but def dont push the event... + unimplemented!();// but def don't push the event... } } if let Some((msg, commitment_signed)) = msgs { @@ -1538,7 +1554,7 @@ impl ChannelManager { // knowledge of those gets moved into the appropriate in-memory // ChannelMonitor and they get failed backwards once we get // on-chain confirmations. - // Note I think #198 addresses this, so once its merged a test + // Note I think #198 addresses this, so once it's merged a test // should be written. if let Some(short_id) = channel.get_short_channel_id() { short_to_id.remove(&short_id); @@ -1838,7 +1854,7 @@ impl ChannelManager { // //TODO: There exists a further attack where a node may garble the onion data, forward it to //us repeatedly garbled in different ways, and compare our error messages, which are - //encrypted with the same key. Its not immediately obvious how to usefully exploit that, + //encrypted with the same key. It's not immediately obvious how to usefully exploit that, //but we should prevent it anyway. let (mut pending_forward_info, mut channel_state_lock) = self.decode_update_add_htlc_onion(msg); @@ -2030,10 +2046,16 @@ impl ChannelManager { //TODO: here and below MsgHandleErrInternal, #153 case return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a message for a channel from the wrong node!", msg.channel_id)); } + let was_frozen_for_monitor = chan.get().is_awaiting_monitor_update(); let (commitment_update, pending_forwards, pending_failures, closing_signed, chan_monitor) = try_chan_entry!(self, chan.get_mut().revoke_and_ack(&msg, &*self.fee_estimator), channel_state, chan); if let Err(e) = self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor) { - return_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::CommitmentFirst, false, commitment_update.is_some(), pending_forwards, pending_failures); + if was_frozen_for_monitor { + assert!(commitment_update.is_none() && closing_signed.is_none() && pending_forwards.is_empty() && pending_failures.is_empty()); + return Err(MsgHandleErrInternal::ignore_no_close("Previous monitor update failure prevented responses to RAA")); + } else { + return_monitor_err!(self, e, channel_state, chan, RAACommitmentOrder::CommitmentFirst, false, commitment_update.is_some(), pending_forwards, pending_failures); + } } if let Some(updates) = commitment_update { channel_state.pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs { @@ -2256,7 +2278,7 @@ impl ChannelManager { impl events::MessageSendEventsProvider for ChannelManager { fn get_and_clear_pending_msg_events(&self) -> Vec { - // TODO: Event release to users and serialization is currently race-y: its very easy for a + // TODO: Event release to users and serialization is currently race-y: it's very easy for a // user to serialize a ChannelManager with pending events in it and lose those events on // restart. This is doubly true for the fail/fulfill-backs from monitor events! { @@ -2281,7 +2303,7 @@ impl events::MessageSendEventsProvider for ChannelManager { impl events::EventsProvider for ChannelManager { fn get_and_clear_pending_events(&self) -> Vec { - // TODO: Event release to users and serialization is currently race-y: its very easy for a + // TODO: Event release to users and serialization is currently race-y: it's very easy for a // user to serialize a ChannelManager with pending events in it and lose those events on // restart. This is doubly true for the fail/fulfill-backs from monitor events! { @@ -2605,12 +2627,7 @@ const MIN_SERIALIZATION_VERSION: u8 = 1; impl Writeable for PendingForwardHTLCInfo { fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { - if let &Some(ref onion) = &self.onion_packet { - 1u8.write(writer)?; - onion.write(writer)?; - } else { - 0u8.write(writer)?; - } + self.onion_packet.write(writer)?; self.incoming_shared_secret.write(writer)?; self.payment_hash.write(writer)?; self.short_channel_id.write(writer)?; @@ -2622,13 +2639,8 @@ impl Writeable for PendingForwardHTLCInfo { impl Readable for PendingForwardHTLCInfo { fn read(reader: &mut R) -> Result { - let onion_packet = match >::read(reader)? { - 0 => None, - 1 => Some(msgs::OnionPacket::read(reader)?), - _ => return Err(DecodeError::InvalidValue), - }; Ok(PendingForwardHTLCInfo { - onion_packet, + onion_packet: Readable::read(reader)?, incoming_shared_secret: Readable::read(reader)?, payment_hash: Readable::read(reader)?, short_channel_id: Readable::read(reader)?, @@ -2891,7 +2903,7 @@ pub struct ChannelManagerReadArgs<'a> { /// value.get_funding_txo() should be the key). /// /// If a monitor is inconsistent with the channel state during deserialization the channel will - /// be force-closed using the data in the channelmonitor and the Channel will be dropped. This + /// be force-closed using the data in the ChannelMonitor and the channel will be dropped. This /// is true for missing channels as well. If there is a monitor missing for which we find /// channel data Err(DecodeError::InvalidValue) will be returned. ///