diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 46b35a940..ba250145f 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2142,6 +2142,14 @@ impl ChannelContext where SP::Target: SignerProvider { self.is_usable() && !self.channel_state.is_peer_disconnected() } + /// Returns false if our last broadcasted channel_update message has the "channel disabled" bit set + pub fn is_enabled(&self) -> bool { + self.is_usable() && match self.channel_update_status { + ChannelUpdateStatus::Enabled | ChannelUpdateStatus::DisabledStaged(_) => true, + ChannelUpdateStatus::Disabled | ChannelUpdateStatus::EnabledStaged(_) => false, + } + } + // Public utilities: pub fn channel_id(&self) -> ChannelId { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 7e13d5666..8f6fe68a1 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -62,7 +62,6 @@ use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError}; #[cfg(test)] use crate::ln::outbound_payment; use crate::ln::outbound_payment::{OutboundPayments, PendingOutboundPayment, RetryableInvoiceRequest, SendAlongPathArgs, StaleExpiration}; -use crate::ln::wire::Encode; use crate::offers::invoice::{Bolt12Invoice, DEFAULT_RELATIVE_EXPIRY, DerivedSigningPubkey, ExplicitSigningPubkey, InvoiceBuilder, UnsignedBolt12Invoice}; use crate::offers::invoice_error::InvoiceError; use crate::offers::invoice_request::{DerivedPayerSigningPubkey, InvoiceRequest, InvoiceRequestBuilder}; @@ -677,7 +676,7 @@ pub enum FailureCode { } impl Into for FailureCode { - fn into(self) -> u16 { + fn into(self) -> u16 { match self { FailureCode::TemporaryNodeFailure => 0x2000 | 2, FailureCode::RequiredNodeFeatureMissing => 0x4000 | 0x2000 | 3, @@ -3841,18 +3840,18 @@ where fn can_forward_htlc_to_outgoing_channel( &self, chan: &mut Channel, msg: &msgs::UpdateAddHTLC, next_packet: &NextPacketDetails - ) -> Result<(), (&'static str, u16, Option)> { + ) -> Result<(), (&'static str, u16)> { if !chan.context.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. - return Err(("Refusing to forward to a private channel based on our config.", 0x4000 | 10, None)); + return Err(("Refusing to forward to a private channel based on our config.", 0x4000 | 10)); } if chan.context.get_channel_type().supports_scid_privacy() && next_packet.outgoing_scid != chan.context.outbound_scid_alias() { // `option_scid_alias` (referred to in LDK as `scid_privacy`) means // "refuse to forward unless the SCID alias was used", so we pretend // we don't have the channel here. - return Err(("Refusing to forward over real channel SCID as our counterparty requested.", 0x4000 | 10, None)); + return Err(("Refusing to forward over real channel SCID as our counterparty requested.", 0x4000 | 10)); } // Note that we could technically not return an error yet here and just hope @@ -3860,24 +3859,20 @@ where // 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.context.is_live() { // channel_disabled - // If the channel_update we're going to return is disabled (i.e. the - // peer has been disabled for some time), return `channel_disabled`, - // otherwise return `temporary_channel_failure`. - let chan_update_opt = self.get_channel_update_for_onion(next_packet.outgoing_scid, chan).ok(); - if chan_update_opt.as_ref().map(|u| u.contents.channel_flags & 2 == 2).unwrap_or(false) { - return Err(("Forwarding channel has been disconnected for some time.", 0x1000 | 20, chan_update_opt)); + if !chan.context.is_live() { + if !chan.context.is_enabled() { + // channel_disabled + return Err(("Forwarding channel has been disconnected for some time.", 0x1000 | 20)); } else { - return Err(("Forwarding channel is not in a ready state.", 0x1000 | 7, chan_update_opt)); + // temporary_channel_failure + return Err(("Forwarding channel is not in a ready state.", 0x1000 | 7)); } } if next_packet.outgoing_amt_msat < chan.context.get_counterparty_htlc_minimum_msat() { // amount_below_minimum - let chan_update_opt = self.get_channel_update_for_onion(next_packet.outgoing_scid, chan).ok(); - return Err(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, chan_update_opt)); + return Err(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11)); } if let Err((err, code)) = chan.htlc_satisfies_config(msg, next_packet.outgoing_amt_msat, next_packet.outgoing_cltv_value) { - let chan_update_opt = self.get_channel_update_for_onion(next_packet.outgoing_scid, chan).ok(); - return Err((err, code, chan_update_opt)); + return Err((err, code)); } Ok(()) @@ -3909,7 +3904,7 @@ where fn can_forward_htlc( &self, msg: &msgs::UpdateAddHTLC, next_packet_details: &NextPacketDetails - ) -> Result<(), (&'static str, u16, Option)> { + ) -> Result<(), (&'static str, u16)> { match self.do_funded_channel_callback(next_packet_details.outgoing_scid, |chan: &mut Channel| { self.can_forward_htlc_to_outgoing_channel(chan, msg, next_packet_details) }) { @@ -3922,7 +3917,7 @@ where fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, next_packet_details.outgoing_scid, &self.chain_hash)) || fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, next_packet_details.outgoing_scid, &self.chain_hash) {} else { - return Err(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None)); + return Err(("Don't have available channel for forwarding as requested.", 0x4000 | 10)); } } } @@ -3931,10 +3926,7 @@ where if let Err((err_msg, err_code)) = check_incoming_htlc_cltv( cur_height, next_packet_details.outgoing_cltv_value, msg.cltv_expiry ) { - let chan_update_opt = self.do_funded_channel_callback(next_packet_details.outgoing_scid, |chan: &mut Channel| { - self.get_channel_update_for_onion(next_packet_details.outgoing_scid, chan).ok() - }).flatten(); - return Err((err_msg, err_code, chan_update_opt)); + return Err((err_msg, err_code)); } Ok(()) @@ -3942,12 +3934,12 @@ where fn htlc_failure_from_update_add_err( &self, msg: &msgs::UpdateAddHTLC, counterparty_node_id: &PublicKey, err_msg: &'static str, - mut err_code: u16, chan_update: Option, is_intro_node_blinded_forward: bool, + err_code: u16, is_intro_node_blinded_forward: bool, shared_secret: &[u8; 32] ) -> HTLCFailureMsg { - let mut res = VecWriter(Vec::with_capacity(chan_update.serialized_length() + 2 + 8 + 2)); - if chan_update.is_some() && err_code & 0x1000 == 0x1000 { - let chan_update = chan_update.unwrap(); + // at capacity, we write fields `htlc_msat` and `len` + let mut res = VecWriter(Vec::with_capacity(8 + 2)); + if err_code & 0x1000 == 0x1000 { if err_code == 0x1000 | 11 || err_code == 0x1000 | 12 { msg.amount_msat.write(&mut res).expect("Writes cannot fail"); } @@ -3958,15 +3950,8 @@ where // TODO: underspecified, follow https://github.com/lightning/bolts/issues/791 0u16.write(&mut res).expect("Writes cannot fail"); } - (chan_update.serialized_length() as u16 + 2).write(&mut res).expect("Writes cannot fail"); - msgs::ChannelUpdate::TYPE.write(&mut res).expect("Writes cannot fail"); - chan_update.write(&mut res).expect("Writes cannot fail"); - } else if err_code & 0x1000 == 0x1000 { - // If we're trying to return an error that requires a `channel_update` but - // we're forwarding to a phantom or intercept "channel" (i.e. cannot - // generate an update), just use the generic "temporary_node_failure" - // instead. - err_code = 0x2000 | 2; + // See https://github.com/lightning/bolts/blob/247e83d/04-onion-routing.md?plain=1#L1414-L1415 + (0u16).write(&mut res).expect("Writes cannot fail"); } log_info!( @@ -4014,9 +3999,9 @@ where // Perform outbound checks here instead of in [`Self::construct_pending_htlc_info`] because we // can't hold the outbound peer state lock at the same time as the inbound peer state lock. self.can_forward_htlc(&msg, &next_packet_details).map_err(|e| { - let (err_msg, err_code, chan_update_opt) = e; + let (err_msg, err_code) = e; self.htlc_failure_from_update_add_err( - msg, counterparty_node_id, err_msg, err_code, chan_update_opt, + msg, counterparty_node_id, err_msg, err_code, next_hop.is_intro_node_blinded_forward(), &shared_secret ) })?; @@ -4125,20 +4110,10 @@ where Some(id) => id, }; - self.get_channel_update_for_onion(short_channel_id, chan) - } - - fn get_channel_update_for_onion(&self, short_channel_id: u64, chan: &Channel) -> Result { let logger = WithChannelContext::from(&self.logger, &chan.context, None); log_trace!(logger, "Generating channel update for channel {}", chan.context.channel_id()); let were_node_one = self.our_network_pubkey.serialize()[..] < chan.context.get_counterparty_node_id().serialize()[..]; - - let enabled = chan.context.is_usable() && match chan.channel_update_status() { - ChannelUpdateStatus::Enabled => true, - ChannelUpdateStatus::DisabledStaged(_) => true, - ChannelUpdateStatus::Disabled => false, - ChannelUpdateStatus::EnabledStaged(_) => false, - }; + let enabled = chan.context.is_enabled(); let unsigned = msgs::UnsignedChannelUpdate { chain_hash: self.chain_hash, @@ -5326,16 +5301,9 @@ where }) { Some(Ok(_)) => {}, Some(Err((err, code))) => { - let outgoing_chan_update_opt = if let Some(outgoing_scid) = outgoing_scid_opt.as_ref() { - self.do_funded_channel_callback(*outgoing_scid, |chan: &mut Channel| { - self.get_channel_update_for_onion(*outgoing_scid, chan).ok() - }).flatten() - } else { - None - }; let htlc_fail = self.htlc_failure_from_update_add_err( &update_add_htlc, &incoming_counterparty_node_id, err, code, - outgoing_chan_update_opt, is_intro_node_blinded_forward, &shared_secret, + is_intro_node_blinded_forward, &shared_secret, ); let htlc_destination = get_failed_htlc_destination(outgoing_scid_opt, update_add_htlc.payment_hash); htlc_fails.push((htlc_fail, htlc_destination)); @@ -5347,12 +5315,12 @@ where // Now process the HTLC on the outgoing channel if it's a forward. if let Some(next_packet_details) = next_packet_details_opt.as_ref() { - if let Err((err, code, chan_update_opt)) = self.can_forward_htlc( + if let Err((err, code)) = self.can_forward_htlc( &update_add_htlc, next_packet_details ) { let htlc_fail = self.htlc_failure_from_update_add_err( &update_add_htlc, &incoming_counterparty_node_id, err, code, - chan_update_opt, is_intro_node_blinded_forward, &shared_secret, + is_intro_node_blinded_forward, &shared_secret, ); let htlc_destination = get_failed_htlc_destination(outgoing_scid_opt, update_add_htlc.payment_hash); htlc_fails.push((htlc_fail, htlc_destination)); @@ -5633,7 +5601,8 @@ where } if let Some(ChannelPhase::Funded(ref mut chan)) = peer_state.channel_by_id.get_mut(&forward_chan_id) { - let (failure_code, data) = self.get_htlc_temp_fail_err_and_data(0x1000|7, short_chan_id, chan); + let failure_code = 0x1000|7; + let data = self.get_htlc_inbound_temp_fail_data(failure_code); failed_forwards.push((htlc_source, payment_hash, HTLCFailReason::reason(failure_code, data), HTLCDestination::NextHopChannel { node_id: Some(chan.context.get_counterparty_node_id()), channel_id: forward_chan_id } @@ -6448,46 +6417,21 @@ where /// /// This is for failures on the channel on which the HTLC was *received*, not failures /// forwarding - fn get_htlc_inbound_temp_fail_err_and_data(&self, desired_err_code: u16, chan: &Channel) -> (u16, Vec) { - // We can't be sure what SCID was used when relaying inbound towards us, so we have to - // guess somewhat. If its a public channel, we figure best to just use the real SCID (as - // we're not leaking that we have a channel with the counterparty), otherwise we try to use - // an inbound SCID alias before the real SCID. - let scid_pref = if chan.context.should_announce() { - chan.context.get_short_channel_id().or(chan.context.latest_inbound_scid_alias()) - } else { - chan.context.latest_inbound_scid_alias().or(chan.context.get_short_channel_id()) - }; - if let Some(scid) = scid_pref { - self.get_htlc_temp_fail_err_and_data(desired_err_code, scid, chan) - } else { - (0x4000|10, Vec::new()) - } - } - - - /// Gets an HTLC onion failure code and error data for an `UPDATE` error, given the error code - /// that we want to return and a channel. - fn get_htlc_temp_fail_err_and_data(&self, desired_err_code: u16, scid: u64, chan: &Channel) -> (u16, Vec) { - debug_assert_eq!(desired_err_code & 0x1000, 0x1000); - if let Ok(upd) = self.get_channel_update_for_onion(scid, chan) { - let mut enc = VecWriter(Vec::with_capacity(upd.serialized_length() + 6)); - if desired_err_code == 0x1000 | 20 { - // No flags for `disabled_flags` are currently defined so they're always two zero bytes. - // See https://github.com/lightning/bolts/blob/341ec84/04-onion-routing.md?plain=1#L1008 - 0u16.write(&mut enc).expect("Writes cannot fail"); - } - (upd.serialized_length() as u16 + 2).write(&mut enc).expect("Writes cannot fail"); - msgs::ChannelUpdate::TYPE.write(&mut enc).expect("Writes cannot fail"); - upd.write(&mut enc).expect("Writes cannot fail"); - (desired_err_code, enc.0) - } else { - // If we fail to get a unicast channel_update, it implies we don't yet have an SCID, - // which means we really shouldn't have gotten a payment to be forwarded over this - // channel yet, or if we did it's from a route hint. Either way, returning an error of - // PERM|no_such_channel should be fine. - (0x4000|10, Vec::new()) + fn get_htlc_inbound_temp_fail_data(&self, err_code: u16) -> Vec { + debug_assert_eq!(err_code & 0x1000, 0x1000); + debug_assert_ne!(err_code, 0x1000|11); + debug_assert_ne!(err_code, 0x1000|12); + debug_assert_ne!(err_code, 0x1000|13); + // at capacity, we write fields `disabled_flags` and `len` + let mut enc = VecWriter(Vec::with_capacity(4)); + if err_code == 0x1000 | 20 { + // No flags for `disabled_flags` are currently defined so they're always two zero bytes. + // See https://github.com/lightning/bolts/blob/341ec84/04-onion-routing.md?plain=1#L1008 + 0u16.write(&mut enc).expect("Writes cannot fail"); } + // See https://github.com/lightning/bolts/blob/247e83d/04-onion-routing.md?plain=1#L1414-L1415 + (0u16).write(&mut enc).expect("Writes cannot fail"); + enc.0 } // Fail a list of HTLCs that were just freed from the holding cell. The HTLCs need to be @@ -6504,8 +6448,10 @@ where let peer_state = &mut *peer_state_lock; match peer_state.channel_by_id.entry(channel_id) { hash_map::Entry::Occupied(chan_phase_entry) => { - if let ChannelPhase::Funded(chan) = chan_phase_entry.get() { - self.get_htlc_inbound_temp_fail_err_and_data(0x1000|7, &chan) + if let ChannelPhase::Funded(_chan) = chan_phase_entry.get() { + let failure_code = 0x1000|7; + let data = self.get_htlc_inbound_temp_fail_data(failure_code); + (failure_code, data) } else { // We shouldn't be trying to fail holding cell HTLCs on an unfunded channel. debug_assert!(false); @@ -8164,8 +8110,8 @@ where let reason = if routing.blinded_failure().is_some() { HTLCFailReason::reason(INVALID_ONION_BLINDING, vec![0; 32]) } else if (error_code & 0x1000) != 0 { - let (real_code, error_data) = self.get_htlc_inbound_temp_fail_err_and_data(error_code, chan); - HTLCFailReason::reason(real_code, error_data) + let error_data = self.get_htlc_inbound_temp_fail_data(error_code); + HTLCFailReason::reason(error_code, error_data) } else { HTLCFailReason::from_failure_code(error_code) }.get_encrypted_failure_packet(incoming_shared_secret, &None); @@ -10190,7 +10136,8 @@ where let res = f(channel); if let Ok((channel_ready_opt, mut timed_out_pending_htlcs, announcement_sigs)) = res { for (source, payment_hash) in timed_out_pending_htlcs.drain(..) { - let (failure_code, data) = self.get_htlc_inbound_temp_fail_err_and_data(0x1000|14 /* expiry_too_soon */, &channel); + let failure_code = 0x1000|14; /* expiry_too_soon */ + let data = self.get_htlc_inbound_temp_fail_data(failure_code); timed_out_htlcs.push((source, payment_hash, HTLCFailReason::reason(failure_code, data), HTLCDestination::NextHopChannel { node_id: Some(channel.context.get_counterparty_node_id()), channel_id: channel.context.channel_id() })); } diff --git a/lightning/src/ln/onion_route_tests.rs b/lightning/src/ln/onion_route_tests.rs index 081cca2ef..1e292a186 100644 --- a/lightning/src/ln/onion_route_tests.rs +++ b/lightning/src/ln/onion_route_tests.rs @@ -1397,9 +1397,12 @@ fn test_phantom_failure_modified_cltv() { commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false); // Ensure the payment fails with the expected error. + let mut err_data = Vec::new(); + err_data.extend_from_slice(&update_add.cltv_expiry.to_be_bytes()); + err_data.extend_from_slice(&0u16.to_be_bytes()); let mut fail_conditions = PaymentFailedConditions::new() .blamed_scid(phantom_scid) - .expected_htlc_error_data(0x2000 | 2, &[]); + .expected_htlc_error_data(0x1000 | 13, &err_data); expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions); } @@ -1437,9 +1440,10 @@ fn test_phantom_failure_expires_too_soon() { commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false); // Ensure the payment fails with the expected error. + let err_data = 0u16.to_be_bytes(); let mut fail_conditions = PaymentFailedConditions::new() .blamed_scid(phantom_scid) - .expected_htlc_error_data(0x2000 | 2, &[]); + .expected_htlc_error_data(0x1000 | 14, &err_data); expect_payment_failed_conditions(&nodes[0], payment_hash, false, fail_conditions); } @@ -1535,11 +1539,7 @@ fn do_test_phantom_dust_exposure_failure(multiplier_dust_limit: bool) { commitment_signed_dance!(nodes[0], nodes[1], update_1.commitment_signed, false); // Ensure the payment fails with the expected error. - let mut err_data = Vec::new(); - err_data.extend_from_slice(&(channel.1.serialized_length() as u16 + 2).to_be_bytes()); - err_data.extend_from_slice(&ChannelUpdate::TYPE.to_be_bytes()); - err_data.extend_from_slice(&channel.1.encode()); - + let err_data = 0u16.to_be_bytes(); let mut fail_conditions = PaymentFailedConditions::new() .blamed_scid(route.paths[0].hops.last().as_ref().unwrap().short_channel_id) .blamed_chan_closed(false) diff --git a/lightning/src/ln/priv_short_conf_tests.rs b/lightning/src/ln/priv_short_conf_tests.rs index 22854952c..22f69dfc4 100644 --- a/lightning/src/ln/priv_short_conf_tests.rs +++ b/lightning/src/ln/priv_short_conf_tests.rs @@ -12,7 +12,6 @@ //! LSP). use crate::chain::ChannelMonitorUpdateStatus; -use crate::sign::NodeSigner; use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider}; use crate::ln::channelmanager::{MIN_CLTV_EXPIRY_DELTA, PaymentId, RecipientOnionFields}; use crate::routing::gossip::RoutingFees; @@ -20,8 +19,7 @@ use crate::routing::router::{PaymentParameters, RouteHint, RouteHintHop}; use crate::ln::features::ChannelTypeFeatures; use crate::ln::msgs; use crate::ln::types::ChannelId; -use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ChannelUpdate, ErrorAction}; -use crate::ln::wire::Encode; +use crate::ln::msgs::{ChannelMessageHandler, RoutingMessageHandler, ErrorAction}; use crate::util::config::{UserConfig, MaxDustHTLCExposure}; use crate::util::ser::Writeable; @@ -29,9 +27,6 @@ use crate::prelude::*; use crate::ln::functional_test_utils::*; -use bitcoin::constants::ChainHash; -use bitcoin::network::Network; - #[test] fn test_priv_forwarding_rejection() { // If we have a private channel with outbound liquidity, and @@ -503,28 +498,7 @@ fn test_scid_alias_returned() { nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &bs_updates.update_fail_htlcs[0]); commitment_signed_dance!(nodes[0], nodes[1], bs_updates.commitment_signed, false, true); - // Build the expected channel update - let contents = msgs::UnsignedChannelUpdate { - chain_hash: ChainHash::using_genesis_block(Network::Testnet), - short_channel_id: last_hop[0].inbound_scid_alias.unwrap(), - timestamp: 21, - message_flags: 1, // Only must_be_one - channel_flags: 1, - cltv_expiry_delta: accept_forward_cfg.channel_config.cltv_expiry_delta, - htlc_minimum_msat: 1_000, - htlc_maximum_msat: 1_000_000, // Defaults to 10% of the channel value - fee_base_msat: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_base_msat, - fee_proportional_millionths: last_hop[0].counterparty.forwarding_info.as_ref().unwrap().fee_proportional_millionths, - excess_data: Vec::new(), - }; - let signature = nodes[1].keys_manager.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelUpdate(&contents)).unwrap(); - let msg = msgs::ChannelUpdate { signature, contents }; - - let mut err_data = Vec::new(); - err_data.extend_from_slice(&(msg.serialized_length() as u16 + 2).to_be_bytes()); - err_data.extend_from_slice(&ChannelUpdate::TYPE.to_be_bytes()); - err_data.extend_from_slice(&msg.encode()); - + let err_data = 0u16.to_be_bytes(); expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().blamed_scid(last_hop[0].inbound_scid_alias.unwrap()) .blamed_chan_closed(false).expected_htlc_error_data(0x1000|7, &err_data)); @@ -546,9 +520,7 @@ fn test_scid_alias_returned() { let mut err_data = Vec::new(); err_data.extend_from_slice(&10_000u64.to_be_bytes()); - err_data.extend_from_slice(&(msg.serialized_length() as u16 + 2).to_be_bytes()); - err_data.extend_from_slice(&ChannelUpdate::TYPE.to_be_bytes()); - err_data.extend_from_slice(&msg.encode()); + err_data.extend_from_slice(&0u16.to_be_bytes()); expect_payment_failed_conditions(&nodes[0], payment_hash, false, PaymentFailedConditions::new().blamed_scid(last_hop[0].inbound_scid_alias.unwrap()) .blamed_chan_closed(false).expected_htlc_error_data(0x1000|12, &err_data));