X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannelmanager.rs;h=f068805ade21559bd0873784e9e8294cea2dc004;hb=520b53eb1ca3b187b664de02b095f69b37effbd5;hp=21d13eaefc2d350c81c32ad3c246c4f3823e4c15;hpb=da298e498fc8bdf6af2b2becf82b0123118f46b4;p=rust-lightning diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 21d13eae..f068805a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -602,6 +602,29 @@ const CHECK_CLTV_EXPIRY_SANITY: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRA #[allow(dead_code)] const CHECK_CLTV_EXPIRY_SANITY_2: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER; +/// Channel parameters which apply to our counterparty. These are split out from [`ChannelDetails`] +/// to better separate parameters. +#[derive(Clone, Debug, PartialEq)] +pub struct ChannelCounterparty { + /// The node_id of our counterparty + pub node_id: PublicKey, + /// The Features the channel counterparty provided upon last connection. + /// Useful for routing as it is the most up-to-date copy of the counterparty's features and + /// many routing-relevant features are present in the init context. + pub features: InitFeatures, + /// The value, in satoshis, that must always be held in the channel for our counterparty. This + /// value ensures that if our counterparty broadcasts a revoked state, we can punish them by + /// claiming at least this value on chain. + /// + /// This value is not included in [`inbound_capacity_msat`] as it can never be spent. + /// + /// [`inbound_capacity_msat`]: ChannelDetails::inbound_capacity_msat + pub unspendable_punishment_reserve: u64, + /// Information on the fees and requirements that the counterparty requires when forwarding + /// payments to us through this channel. + pub forwarding_info: Option, +} + /// Details of a channel, as returned by ChannelManager::list_channels and ChannelManager::list_usable_channels #[derive(Clone, Debug, PartialEq)] pub struct ChannelDetails { @@ -610,6 +633,8 @@ pub struct ChannelDetails { /// Note that this means this value is *not* persistent - it can change once during the /// lifetime of the channel. pub channel_id: [u8; 32], + /// Parameters which apply to our counterparty. See individual fields for more information. + pub counterparty: ChannelCounterparty, /// The Channel's funding transaction output, if we've negotiated the funding transaction with /// our counterparty already. /// @@ -619,12 +644,6 @@ pub struct ChannelDetails { /// The position of the funding transaction in the chain. None if the funding transaction has /// not yet been confirmed and the channel fully opened. pub short_channel_id: Option, - /// The node_id of our counterparty - pub remote_network_id: PublicKey, - /// The Features the channel counterparty provided upon last connection. - /// Useful for routing as it is the most up-to-date copy of the counterparty's features and - /// many routing-relevant features are present in the init context. - pub counterparty_features: InitFeatures, /// The value, in satoshis, of this channel as appears in the funding output pub channel_value_satoshis: u64, /// The value, in satoshis, that must always be held in the channel for us. This value ensures @@ -636,15 +655,7 @@ pub struct ChannelDetails { /// This value will be `None` for outbound channels until the counterparty accepts the channel. /// /// [`outbound_capacity_msat`]: ChannelDetails::outbound_capacity_msat - pub to_self_reserve_satoshis: Option, - /// The value, in satoshis, that must always be held in the channel for our counterparty. This - /// value ensures that if our counterparty broadcasts a revoked state, we can punish them by - /// claiming at least this value on chain. - /// - /// This value is not included in [`inbound_capacity_msat`] as it can never be spent. - /// - /// [`inbound_capacity_msat`]: ChannelDetails::inbound_capacity_msat - pub to_remote_reserve_satoshis: u64, + pub unspendable_punishment_reserve: Option, /// The user_id passed in to create_channel, or 0 if the channel was inbound. pub user_id: u64, /// The available outbound capacity for sending HTLCs to the remote peer. This does not include @@ -685,7 +696,7 @@ pub struct ChannelDetails { /// time to claim our non-HTLC-encumbered funds. /// /// This value will be `None` for outbound channels until the counterparty accepts the channel. - pub spend_csv_on_our_commitment_funds: Option, + pub force_close_spend_delay: Option, /// True if the channel was initiated (and thus funded) by us. pub is_outbound: bool, /// True if the channel is confirmed, funding_locked messages have been exchanged, and the @@ -703,9 +714,6 @@ pub struct ChannelDetails { pub is_usable: bool, /// True if this channel is (or will be) publicly-announced. pub is_public: bool, - /// Information on the fees and requirements that the counterparty requires when forwarding - /// payments to us through this channel. - pub counterparty_forwarding_info: Option, } /// If a payment fails to send, it can be in one of several states. This enum is returned as the @@ -800,7 +808,7 @@ macro_rules! convert_chan_err { $short_to_id.remove(&short_id); } let shutdown_res = $channel.force_shutdown(true); - (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $self.get_channel_update(&$channel).ok())) + (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok())) }, ChannelError::CloseDelayBroadcast(msg) => { log_error!($self.logger, "Channel {} need to be shutdown but closing transactions not broadcast due to {}", log_bytes!($channel_id[..]), msg); @@ -808,7 +816,7 @@ macro_rules! convert_chan_err { $short_to_id.remove(&short_id); } let shutdown_res = $channel.force_shutdown(false); - (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $self.get_channel_update(&$channel).ok())) + (true, MsgHandleErrInternal::from_finish_shutdown(msg, *$channel_id, shutdown_res, $self.get_channel_update_for_broadcast(&$channel).ok())) } } } @@ -864,7 +872,8 @@ macro_rules! handle_monitor_err { // splitting hairs we'd prefer to claim payments that were to us, but we haven't // given up the preimage yet, so might as well just wait until the payment is // retried, avoiding the on-chain fees. - let res: Result<(), _> = Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure".to_owned(), *$chan_id, $chan.force_shutdown(true), $self.get_channel_update(&$chan).ok())); + let res: Result<(), _> = Err(MsgHandleErrInternal::from_finish_shutdown("ChannelMonitor storage failure".to_owned(), *$chan_id, + $chan.force_shutdown(true), $self.get_channel_update_for_broadcast(&$chan).ok() )); (res, true) }, ChannelMonitorUpdateErr::TemporaryFailure => { @@ -1170,30 +1179,32 @@ impl ChannelMana channel.get_holder_counterparty_selected_channel_reserve_satoshis(); res.push(ChannelDetails { channel_id: (*channel_id).clone(), + counterparty: ChannelCounterparty { + node_id: channel.get_counterparty_node_id(), + features: InitFeatures::empty(), + unspendable_punishment_reserve: to_remote_reserve_satoshis, + forwarding_info: channel.counterparty_forwarding_info(), + }, funding_txo: channel.get_funding_txo(), short_channel_id: channel.get_short_channel_id(), - remote_network_id: channel.get_counterparty_node_id(), - counterparty_features: InitFeatures::empty(), channel_value_satoshis: channel.get_value_satoshis(), - to_self_reserve_satoshis, - to_remote_reserve_satoshis, + unspendable_punishment_reserve: to_self_reserve_satoshis, inbound_capacity_msat, outbound_capacity_msat, user_id: channel.get_user_id(), confirmations_required: channel.minimum_depth(), - spend_csv_on_our_commitment_funds: channel.get_counterparty_selected_contest_delay(), + force_close_spend_delay: channel.get_counterparty_selected_contest_delay(), is_outbound: channel.is_outbound(), is_funding_locked: channel.is_usable(), is_usable: channel.is_live(), is_public: channel.should_announce(), - counterparty_forwarding_info: channel.counterparty_forwarding_info(), }); } } let per_peer_state = self.per_peer_state.read().unwrap(); for chan in res.iter_mut() { - if let Some(peer_state) = per_peer_state.get(&chan.remote_network_id) { - chan.counterparty_features = peer_state.lock().unwrap().latest_features.clone(); + if let Some(peer_state) = per_peer_state.get(&chan.counterparty.node_id) { + chan.counterparty.features = peer_state.lock().unwrap().latest_features.clone(); } } res @@ -1250,9 +1261,7 @@ impl ChannelMana self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); } let chan_update = if let Some(chan) = chan_option { - if let Ok(update) = self.get_channel_update(&chan) { - Some(update) - } else { None } + self.get_channel_update_for_broadcast(&chan).ok() } else { None }; if let Some(update) = chan_update { @@ -1301,7 +1310,7 @@ impl ChannelMana }; log_error!(self.logger, "Force-closing channel {}", log_bytes!(channel_id[..])); self.finish_force_close_channel(chan.force_shutdown(true)); - if let Ok(update) = self.get_channel_update(&chan) { + if let Ok(update) = self.get_channel_update_for_broadcast(&chan) { let mut channel_state = self.channel_state.lock().unwrap(); channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg: update @@ -1546,46 +1555,56 @@ impl ChannelMana // short_channel_id is non-0 in any ::Forward. if let &PendingHTLCRouting::Forward { ref short_channel_id, .. } = routing { let id_option = channel_state.as_ref().unwrap().short_to_id.get(&short_channel_id).cloned(); - let forwarding_id = match id_option { - None => { // unknown_next_peer - return_err!("Don't have available channel for forwarding as requested.", 0x4000 | 10, &[0;0]); - }, - Some(id) => id.clone(), - }; if let Some((err, code, chan_update)) = loop { + let forwarding_id = match id_option { + None => { // unknown_next_peer + break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None)); + }, + Some(id) => id.clone(), + }; + let chan = channel_state.as_mut().unwrap().by_id.get_mut(&forwarding_id).unwrap(); + if !chan.should_announce() && !self.default_configuration.accept_forwards_to_priv_channels { + // Note that the behavior here should be identical to the above block - we + // should NOT reveal the existence or non-existence of a private channel if + // we don't allow forwards outbound over them. + break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None)); + } + // Note that we could technically not return an error yet here and just hope // that the connection is reestablished or monitor updated by the time we get // around to doing the actual forward, but better to fail early if we can and // hopefully an attacker trying to path-trace payments cannot make this occur // on a small/per-node/per-channel scale. if !chan.is_live() { // channel_disabled - break Some(("Forwarding channel is not in a ready state.", 0x1000 | 20, Some(self.get_channel_update(chan).unwrap()))); + break Some(("Forwarding channel is not in a ready state.", 0x1000 | 20, Some(self.get_channel_update_for_unicast(chan).unwrap()))); } if *amt_to_forward < chan.get_counterparty_htlc_minimum_msat() { // amount_below_minimum - break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, Some(self.get_channel_update(chan).unwrap()))); + break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, Some(self.get_channel_update_for_unicast(chan).unwrap()))); } - let fee = amt_to_forward.checked_mul(chan.get_fee_proportional_millionths() as u64).and_then(|prop_fee| { (prop_fee / 1000000).checked_add(chan.get_holder_fee_base_msat(&self.fee_estimator) as u64) }); + let fee = amt_to_forward.checked_mul(chan.get_fee_proportional_millionths() as u64) + .and_then(|prop_fee| { (prop_fee / 1000000) + .checked_add(chan.get_outbound_forwarding_fee_base_msat() as u64) }); if fee.is_none() || msg.amount_msat < fee.unwrap() || (msg.amount_msat - fee.unwrap()) < *amt_to_forward { // fee_insufficient - break Some(("Prior hop has deviated from specified fees parameters or origin node has obsolete ones", 0x1000 | 12, Some(self.get_channel_update(chan).unwrap()))); + break Some(("Prior hop has deviated from specified fees parameters or origin node has obsolete ones", 0x1000 | 12, Some(self.get_channel_update_for_unicast(chan).unwrap()))); } if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + chan.get_cltv_expiry_delta() as u64 { // incorrect_cltv_expiry - break Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, Some(self.get_channel_update(chan).unwrap()))); + break Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, Some(self.get_channel_update_for_unicast(chan).unwrap()))); } let cur_height = self.best_block.read().unwrap().height() + 1; // Theoretically, channel counterparty shouldn't send us a HTLC expiring now, but we want to be robust wrt to counterparty // packet sanitization (see HTLC_FAIL_BACK_BUFFER rational) if msg.cltv_expiry <= cur_height + HTLC_FAIL_BACK_BUFFER as u32 { // expiry_too_soon - break Some(("CLTV expiry is too close", 0x1000 | 14, Some(self.get_channel_update(chan).unwrap()))); + break Some(("CLTV expiry is too close", 0x1000 | 14, Some(self.get_channel_update_for_unicast(chan).unwrap()))); } if msg.cltv_expiry > cur_height + CLTV_FAR_FAR_AWAY as u32 { // expiry_too_far break Some(("CLTV expiry is too far in the future", 21, None)); } - // In theory, we would be safe against unitentional channel-closure, if we only required a margin of LATENCY_GRACE_PERIOD_BLOCKS. - // But, to be safe against policy reception, we use a longuer delay. + // In theory, we would be safe against unintentional channel-closure, if we only required a margin of LATENCY_GRACE_PERIOD_BLOCKS. + // But, to be safe against policy reception, we use a longer delay. if (*outgoing_cltv_value) as u64 <= (cur_height + HTLC_FAIL_BACK_BUFFER) as u64 { - break Some(("Outgoing CLTV value is too soon", 0x1000 | 14, Some(self.get_channel_update(chan).unwrap()))); + break Some(("Outgoing CLTV value is too soon", 0x1000 | 14, Some(self.get_channel_update_for_unicast(chan).unwrap()))); } break None; @@ -1613,9 +1632,29 @@ impl ChannelMana (pending_forward_info, channel_state.unwrap()) } - /// only fails if the channel does not yet have an assigned short_id + /// Gets the current channel_update for the given channel. This first checks if the channel is + /// public, and thus should be called whenever the result is going to be passed out in a + /// [`MessageSendEvent::BroadcastChannelUpdate`] event. + /// /// May be called with channel_state already locked! - fn get_channel_update(&self, chan: &Channel) -> Result { + fn get_channel_update_for_broadcast(&self, chan: &Channel) -> Result { + if !chan.should_announce() { + return Err(LightningError { + err: "Cannot broadcast a channel_update for a private channel".to_owned(), + action: msgs::ErrorAction::IgnoreError + }); + } + log_trace!(self.logger, "Attempting to generate broadcast channel update for channel {}", log_bytes!(chan.channel_id())); + self.get_channel_update_for_unicast(chan) + } + + /// Gets the current channel_update for the given channel. This does not check if the channel + /// is public (only returning an Err if the channel does not yet have an assigned short_id), + /// and thus MUST NOT be called unless the recipient of the resulting message has already + /// provided evidence that they know about the existence of the channel. + /// May be called with channel_state already locked! + fn get_channel_update_for_unicast(&self, chan: &Channel) -> Result { + log_trace!(self.logger, "Attempting to generate channel update for channel {}", log_bytes!(chan.channel_id())); let short_channel_id = match chan.get_short_channel_id() { None => return Err(LightningError{err: "Channel not yet established".to_owned(), action: msgs::ErrorAction::IgnoreError}), Some(id) => id, @@ -1631,7 +1670,7 @@ impl ChannelMana cltv_expiry_delta: chan.get_cltv_expiry_delta(), htlc_minimum_msat: chan.get_counterparty_htlc_minimum_msat(), htlc_maximum_msat: OptionalField::Present(chan.get_announced_htlc_max_msat()), - fee_base_msat: chan.get_holder_fee_base_msat(&self.fee_estimator), + fee_base_msat: chan.get_outbound_forwarding_fee_base_msat(), fee_proportional_millionths: chan.get_fee_proportional_millionths(), excess_data: Vec::new(), }; @@ -2008,7 +2047,7 @@ impl ChannelMana if let Some(msg) = chan.get_signed_channel_announcement(&self.our_network_key, self.get_our_node_id(), self.genesis_hash.clone()) { channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement { msg, - update_msg: match self.get_channel_update(chan) { + update_msg: match self.get_channel_update_for_broadcast(chan) { Ok(msg) => msg, Err(_) => continue, }, @@ -2100,7 +2139,7 @@ impl ChannelMana } else { panic!("Stated return value requirements in send_htlc() were not met"); } - let chan_update = self.get_channel_update(chan.get()).unwrap(); + let chan_update = self.get_channel_update_for_unicast(chan.get()).unwrap(); failed_forwards.push((htlc_source, payment_hash, HTLCFailReason::Reason { failure_code: 0x1000 | 7, data: chan_update.encode_with_len() } )); @@ -2172,7 +2211,7 @@ impl ChannelMana if let Some(short_id) = channel.get_short_channel_id() { channel_state.short_to_id.remove(&short_id); } - Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, channel.force_shutdown(true), self.get_channel_update(&channel).ok())) + Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, channel.force_shutdown(true), self.get_channel_update_for_broadcast(&channel).ok())) }, ChannelError::CloseDelayBroadcast(_) => { panic!("Wait is only generated on receipt of channel_reestablish, which is handled by try_chan_entry, we don't bother to support it here"); } }; @@ -2375,7 +2414,7 @@ impl ChannelMana ChannelUpdateStatus::DisabledStaged if chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Enabled), ChannelUpdateStatus::EnabledStaged if !chan.is_live() => chan.set_channel_update_status(ChannelUpdateStatus::Disabled), ChannelUpdateStatus::DisabledStaged if !chan.is_live() => { - if let Ok(update) = self.get_channel_update(&chan) { + if let Ok(update) = self.get_channel_update_for_broadcast(&chan) { channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg: update }); @@ -2384,7 +2423,7 @@ impl ChannelMana chan.set_channel_update_status(ChannelUpdateStatus::Disabled); }, ChannelUpdateStatus::EnabledStaged if chan.is_live() => { - if let Ok(update) = self.get_channel_update(&chan) { + if let Ok(update) = self.get_channel_update_for_broadcast(&chan) { channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg: update }); @@ -2434,7 +2473,7 @@ impl ChannelMana let (failure_code, onion_failure_data) = match self.channel_state.lock().unwrap().by_id.entry(channel_id) { hash_map::Entry::Occupied(chan_entry) => { - if let Ok(upd) = self.get_channel_update(&chan_entry.get()) { + if let Ok(upd) = self.get_channel_update_for_unicast(&chan_entry.get()) { (0x1000|7, upd.encode_with_len()) } else { (0x4000|10, Vec::new()) @@ -2798,7 +2837,8 @@ impl ChannelMana pub fn channel_monitor_updated(&self, funding_txo: &OutPoint, highest_applied_update_id: u64) { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier); - let (mut pending_failures, chan_restoration_res) = { + let chan_restoration_res; + let mut pending_failures = { let mut channel_lock = self.channel_state.lock().unwrap(); let channel_state = &mut *channel_lock; let mut channel = match channel_state.by_id.entry(funding_txo.to_channel_id()) { @@ -2810,7 +2850,21 @@ impl ChannelMana } let (raa, commitment_update, order, pending_forwards, pending_failures, funding_broadcastable, funding_locked) = channel.get_mut().monitor_updating_restored(&self.logger); - (pending_failures, handle_chan_restoration_locked!(self, channel_lock, channel_state, channel, raa, commitment_update, order, None, pending_forwards, funding_broadcastable, funding_locked)) + let channel_update = if funding_locked.is_some() && channel.get().is_usable() && !channel.get().should_announce() { + // We only send a channel_update in the case where we are just now sending a + // funding_locked and the channel is in a usable state. Further, we rely on the + // normal announcement_signatures process to send a channel_update for public + // channels, only generating a unicast channel_update if this is a private channel. + Some(events::MessageSendEvent::SendChannelUpdate { + node_id: channel.get().get_counterparty_node_id(), + msg: self.get_channel_update_for_unicast(channel.get()).unwrap(), + }) + } else { None }; + chan_restoration_res = handle_chan_restoration_locked!(self, channel_lock, channel_state, channel, raa, commitment_update, order, None, pending_forwards, funding_broadcastable, funding_locked); + if let Some(upd) = channel_update { + channel_state.pending_msg_events.push(upd); + } + pending_failures }; post_handle_chan_restoration!(self, chan_restoration_res); for failure in pending_failures.drain(..) { @@ -2973,6 +3027,11 @@ impl ChannelMana node_id: counterparty_node_id.clone(), msg: announcement_sigs, }); + } else if chan.get().is_usable() { + channel_state.pending_msg_events.push(events::MessageSendEvent::SendChannelUpdate { + node_id: counterparty_node_id.clone(), + msg: self.get_channel_update_for_unicast(chan.get()).unwrap(), + }); } Ok(()) }, @@ -3017,7 +3076,7 @@ impl ChannelMana self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); } if let Some(chan) = chan_option { - if let Ok(update) = self.get_channel_update(&chan) { + if let Ok(update) = self.get_channel_update_for_broadcast(&chan) { let mut channel_state = self.channel_state.lock().unwrap(); channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg: update @@ -3063,7 +3122,7 @@ impl ChannelMana self.tx_broadcaster.broadcast_transaction(&broadcast_tx); } if let Some(chan) = chan_option { - if let Ok(update) = self.get_channel_update(&chan) { + if let Ok(update) = self.get_channel_update_for_broadcast(&chan) { let mut channel_state = self.channel_state.lock().unwrap(); channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg: update @@ -3101,7 +3160,7 @@ impl ChannelMana // want to reject the new HTLC and fail it backwards instead of forwarding. match pending_forward_info { PendingHTLCStatus::Forward(PendingHTLCInfo { ref incoming_shared_secret, .. }) => { - let reason = if let Ok(upd) = self.get_channel_update(chan) { + let reason = if let Ok(upd) = self.get_channel_update_for_unicast(chan) { onion_utils::build_first_hop_failure_packet(incoming_shared_secret, error_code, &{ let mut res = Vec::with_capacity(8 + 128); // TODO: underspecified, follow https://github.com/lightningnetwork/lightning-rfc/issues/791 @@ -3363,7 +3422,9 @@ impl ChannelMana channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement { msg: try_chan_entry!(self, chan.get_mut().announcement_signatures(&self.our_network_key, self.get_our_node_id(), self.genesis_hash.clone(), msg), channel_state, chan), - update_msg: self.get_channel_update(chan.get()).unwrap(), // can only fail if we're not in a ready state + // Note that announcement_signatures fails if the channel cannot be announced, + // so get_channel_update_for_broadcast will never fail by the time we get here. + update_msg: self.get_channel_update_for_broadcast(chan.get()).unwrap(), }); }, hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id)) @@ -3393,7 +3454,13 @@ impl ChannelMana } return Err(MsgHandleErrInternal::send_err_msg_no_close("Got a channel_update for a channel from the wrong node - it shouldn't know about our private channels!".to_owned(), chan_id)); } - try_chan_entry!(self, chan.get_mut().channel_update(&msg), channel_state, chan); + let were_node_one = self.get_our_node_id().serialize()[..] < chan.get().get_counterparty_node_id().serialize()[..]; + let msg_from_node_one = msg.contents.flags & 1 == 0; + if were_node_one == msg_from_node_one { + return Ok(NotifyOption::SkipPersist); + } else { + try_chan_entry!(self, chan.get_mut().channel_update(&msg), channel_state, chan); + } }, hash_map::Entry::Vacant(_) => unreachable!() } @@ -3401,7 +3468,8 @@ impl ChannelMana } fn internal_channel_reestablish(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(), MsgHandleErrInternal> { - let (htlcs_failed_forward, need_lnd_workaround, chan_restoration_res) = { + let chan_restoration_res; + let (htlcs_failed_forward, need_lnd_workaround) = { let mut channel_state_lock = self.channel_state.lock().unwrap(); let channel_state = &mut *channel_state_lock; @@ -3416,15 +3484,27 @@ impl ChannelMana // add-HTLCs on disconnect, we may be handed HTLCs to fail backwards here. let (funding_locked, revoke_and_ack, commitment_update, monitor_update_opt, order, htlcs_failed_forward, shutdown) = try_chan_entry!(self, chan.get_mut().channel_reestablish(msg, &self.logger), channel_state, chan); + let mut channel_update = None; if let Some(msg) = shutdown { channel_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown { node_id: counterparty_node_id.clone(), msg, }); + } else if chan.get().is_usable() { + // If the channel is in a usable state (ie the channel is not being shut + // down), send a unicast channel_update to our counterparty to make sure + // they have the latest channel parameters. + channel_update = Some(events::MessageSendEvent::SendChannelUpdate { + node_id: chan.get().get_counterparty_node_id(), + msg: self.get_channel_update_for_unicast(chan.get()).unwrap(), + }); } let need_lnd_workaround = chan.get_mut().workaround_lnd_bug_4006.take(); - (htlcs_failed_forward, need_lnd_workaround, - handle_chan_restoration_locked!(self, channel_state_lock, channel_state, chan, revoke_and_ack, commitment_update, order, monitor_update_opt, Vec::new(), None, funding_locked)) + chan_restoration_res = handle_chan_restoration_locked!(self, channel_state_lock, channel_state, chan, revoke_and_ack, commitment_update, order, monitor_update_opt, Vec::new(), None, funding_locked); + if let Some(upd) = channel_update { + channel_state.pending_msg_events.push(upd); + } + (htlcs_failed_forward, need_lnd_workaround) }, hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel".to_owned(), msg.channel_id)) } @@ -3521,7 +3601,7 @@ impl ChannelMana short_to_id.remove(&short_id); } failed_channels.push(chan.force_shutdown(false)); - if let Ok(update) = self.get_channel_update(&chan) { + if let Ok(update) = self.get_channel_update_for_broadcast(&chan) { pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg: update }); @@ -3960,7 +4040,7 @@ where let res = f(channel); if let Ok((chan_res, mut timed_out_pending_htlcs)) = res { for (source, payment_hash) in timed_out_pending_htlcs.drain(..) { - let chan_update = self.get_channel_update(&channel).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe + let chan_update = self.get_channel_update_for_unicast(&channel).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe timed_out_htlcs.push((source, payment_hash, HTLCFailReason::Reason { failure_code: 0x1000 | 14, // expiry_too_soon, or at least it is now data: chan_update, @@ -3977,6 +4057,12 @@ where node_id: channel.get_counterparty_node_id(), msg: announcement_sigs, }); + } else if channel.is_usable() { + log_trace!(self.logger, "Sending funding_locked WITHOUT announcement_signatures but with private channel_update for our counterparty on channel {}", log_bytes!(channel.channel_id())); + pending_msg_events.push(events::MessageSendEvent::SendChannelUpdate { + node_id: channel.get_counterparty_node_id(), + msg: self.get_channel_update_for_unicast(channel).unwrap(), + }); } else { log_trace!(self.logger, "Sending funding_locked WITHOUT announcement_signatures for {}", log_bytes!(channel.channel_id())); } @@ -3989,7 +4075,7 @@ where // It looks like our counterparty went on-chain or funding transaction was // reorged out of the main chain. Close the channel. failed_channels.push(channel.force_shutdown(true)); - if let Ok(update) = self.get_channel_update(&channel) { + if let Ok(update) = self.get_channel_update_for_broadcast(&channel) { pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg: update }); @@ -4179,7 +4265,7 @@ impl short_to_id.remove(&short_id); } failed_channels.push(chan.force_shutdown(true)); - if let Ok(update) = self.get_channel_update(&chan) { + if let Ok(update) = self.get_channel_update_for_broadcast(&chan) { pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate { msg: update }); @@ -4222,6 +4308,7 @@ impl &events::MessageSendEvent::BroadcastChannelAnnouncement { .. } => true, &events::MessageSendEvent::BroadcastNodeAnnouncement { .. } => true, &events::MessageSendEvent::BroadcastChannelUpdate { .. } => true, + &events::MessageSendEvent::SendChannelUpdate { ref node_id, .. } => node_id != counterparty_node_id, &events::MessageSendEvent::HandleError { ref node_id, .. } => node_id != counterparty_node_id, &events::MessageSendEvent::PaymentFailureNetworkUpdate { .. } => true, &events::MessageSendEvent::SendChannelRangeQuery { .. } => false, @@ -4286,7 +4373,7 @@ impl if msg.channel_id == [0; 32] { for chan in self.list_channels() { - if chan.remote_network_id == *counterparty_node_id { + 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)); } @@ -4955,6 +5042,31 @@ mod tests { // At this point the channel info given by peers should still be the same. assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info); assert_eq!(nodes[1].node.list_channels()[0], node_b_chan_info); + + // An earlier version of handle_channel_update didn't check the directionality of the + // update message and would always update the local fee info, even if our peer was + // (spuriously) forwarding us our own channel_update. + let as_node_one = nodes[0].node.get_our_node_id().serialize()[..] < nodes[1].node.get_our_node_id().serialize()[..]; + let as_update = if as_node_one == (chan.0.contents.flags & 1 == 0 /* chan.0 is from node one */) { &chan.0 } else { &chan.1 }; + let bs_update = if as_node_one == (chan.0.contents.flags & 1 == 0 /* chan.0 is from node one */) { &chan.1 } else { &chan.0 }; + + // First deliver each peers' own message, checking that the node doesn't need to be + // persisted and that its channel info remains the same. + nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &as_update); + nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &bs_update); + assert!(!nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1))); + assert!(!nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1))); + assert_eq!(nodes[0].node.list_channels()[0], node_a_chan_info); + assert_eq!(nodes[1].node.list_channels()[0], node_b_chan_info); + + // Finally, deliver the other peers' message, ensuring each node needs to be persisted and + // the channel info has updated. + nodes[0].node.handle_channel_update(&nodes[1].node.get_our_node_id(), &bs_update); + nodes[1].node.handle_channel_update(&nodes[0].node.get_our_node_id(), &as_update); + assert!(nodes[0].node.await_persistable_update_timeout(Duration::from_millis(1))); + assert!(nodes[1].node.await_persistable_update_timeout(Duration::from_millis(1))); + assert_ne!(nodes[0].node.list_channels()[0], node_a_chan_info); + assert_ne!(nodes[1].node.list_channels()[0], node_b_chan_info); } } @@ -5005,7 +5117,7 @@ pub mod bench { let genesis_hash = bitcoin::blockdata::constants::genesis_block(network).header.block_hash(); let tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))}; - let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: 253 }; + let fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }; let mut config: UserConfig = Default::default(); config.own_channel_config.minimum_depth = 1; @@ -5055,7 +5167,19 @@ pub mod bench { Listen::block_connected(&node_b, &block, 1); node_a.handle_funding_locked(&node_b.get_our_node_id(), &get_event_msg!(node_b_holder, MessageSendEvent::SendFundingLocked, node_a.get_our_node_id())); - node_b.handle_funding_locked(&node_a.get_our_node_id(), &get_event_msg!(node_a_holder, MessageSendEvent::SendFundingLocked, node_b.get_our_node_id())); + let msg_events = node_a.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2); + match msg_events[0] { + MessageSendEvent::SendFundingLocked { ref msg, .. } => { + node_b.handle_funding_locked(&node_a.get_our_node_id(), msg); + get_event_msg!(node_b_holder, MessageSendEvent::SendChannelUpdate, node_a.get_our_node_id()); + }, + _ => panic!(), + } + match msg_events[1] { + MessageSendEvent::SendChannelUpdate { .. } => {}, + _ => panic!(), + } let dummy_graph = NetworkGraph::new(genesis_hash);