From 08bb0b302fb2318188f821970309713fcdfce29d Mon Sep 17 00:00:00 2001 From: Franck Royer Date: Tue, 21 Apr 2020 19:51:35 +1000 Subject: [PATCH 1/6] New enum to type message failures All message failure are constructed on the fly in the channel manager. Instead, we add the `MessageFailure` enum. The enum will list the possible message failures and type the data to be included. The code would also be defined once in an impl block and this enum can directly provide the data byte array and code using `Writeable` trait. This commit adds the skeleton. --- lightning/src/ln/channelmanager.rs | 43 ++++++++++++++++++++++-------- lightning/src/ln/onion_utils.rs | 33 ++++++++++++++++++++++- 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5d52e4a8d16..2620e2247ff 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -162,7 +162,8 @@ pub(super) enum HTLCFailReason { Reason { failure_code: u16, data: Vec, - } + }, + TypedReason(onion_utils::MessageFailure) } /// payment_hash type, use to cross-lock hop @@ -1733,19 +1734,14 @@ impl ChannelMan } if total_value >= msgs::MAX_VALUE_MSAT || total_value > data.total_msat { for htlc in htlcs.iter() { - let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec(); - htlc_msat_height_data.extend_from_slice( - &byte_utils::be32_to_array( - self.latest_block_height.load(Ordering::Acquire) - as u32, - ), - ); + let htlc_msat = htlc.value; + let height = self.latest_block_height.load(Ordering::Acquire) as u32; failed_forwards.push((HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id: htlc.prev_hop.short_channel_id, htlc_id: htlc.prev_hop.htlc_id, incoming_packet_shared_secret: htlc.prev_hop.incoming_packet_shared_secret, }), payment_hash, - HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data } + HTLCFailReason::TypedReason(onion_utils::MessageFailure::IncorrectOrUnknownPaymentDetails { htlc_msat, height}) )); } } else if total_value == data.total_msat { @@ -1904,15 +1900,36 @@ impl ChannelMan } ); } + &HTLCFailReason::TypedReason(ref _message_failure) => { + self.pending_events + .lock() + .unwrap() + .push(events::Event::PaymentFailed { + payment_hash: payment_hash.clone(), + rejected_by_dest: path.len() == 1, + #[cfg(test)] + error_code: Some(_message_failure.code()), + #[cfg(test)] + error_data: Some(_message_failure.data()), + }); + } } - }, + } + HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret }) => { let err_packet = match onion_error { HTLCFailReason::Reason { failure_code, data } => { log_trace!(self, "Failing HTLC with payment_hash {} backwards from us with code {}", log_bytes!(payment_hash.0), failure_code); let packet = onion_utils::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode(); onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &packet) - }, + } + HTLCFailReason::TypedReason(ref message_failure) => { + let failure_code = message_failure.code(); + log_trace!(self, "Failing HTLC with payment_hash {} backwards from us with code {}", log_bytes!(payment_hash.0), failure_code); + let data = message_failure.data(); + let packet = onion_utils::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode(); + onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &packet) + } HTLCFailReason::LightningError { err } => { log_trace!(self, "Failing HTLC with payment_hash {} backwards with pre-built LightningError", log_bytes!(payment_hash.0)); onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &err.data) @@ -3487,6 +3504,10 @@ impl Writeable for HTLCFailReason { failure_code.write(writer)?; data.write(writer)?; } + &HTLCFailReason::TypedReason (ref message_failure) => { + 1u8.write(writer)?; + message_failure.write(writer)?; + } } Ok(()) } diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index dc3b2f90225..1276cc7d4e1 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -4,7 +4,7 @@ use ln::router::RouteHop; use util::byte_utils; use util::chacha20::ChaCha20; use util::errors::{self, APIError}; -use util::ser::{Readable, Writeable, LengthCalculatingWriter}; +use util::ser::{Readable, Writeable, Writer, LengthCalculatingWriter}; use util::logger::{Logger, LogHolder}; use bitcoin_hashes::{Hash, HashEngine}; @@ -314,6 +314,37 @@ pub(super) fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type: encrypt_failure_packet(shared_secret, &failure_packet.encode()[..]) } +#[derive(Clone)] +pub(crate) enum MessageFailure { + IncorrectOrUnknownPaymentDetails { htlc_msat: u64, height: u32 }, +} + +impl MessageFailure { + pub fn code(&self) -> u16 { + use ln::onion_utils::MessageFailure::*; + match *self { + IncorrectOrUnknownPaymentDetails { .. } => 0x4000 | 15, + } + } + + pub fn data(&self) -> Vec { + use ln::onion_utils::MessageFailure::*; + match self { + &IncorrectOrUnknownPaymentDetails { htlc_msat, height } => { + let mut data = byte_utils::be64_to_array(htlc_msat).to_vec(); + data.extend_from_slice(&byte_utils::be32_to_array(height)); + data + } + } + } +} + +impl Writeable for MessageFailure { + fn write(&self, writer: &mut W) -> Result<(), ::std::io::Error> { + writer.write_all(&self.data()) + } +} + /// Process failure we got back from upstream on a payment we sent (implying htlc_source is an /// OutboundRoute). /// Returns update, a boolean indicating that the payment itself failed, and the error code. From 61d45d4a13eeffb14c991218b673e3eaa91a0822 Mon Sep 17 00:00:00 2001 From: Franck Royer Date: Tue, 21 Apr 2020 19:51:39 +1000 Subject: [PATCH 2/6] Use TypedReason for `incorrect_or_unknown_payment_details` --- lightning/src/ln/channelmanager.rs | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2620e2247ff..0a9c023b7df 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -32,7 +32,7 @@ use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpd use ln::features::{InitFeatures, NodeFeatures}; use ln::router::{Route, RouteHop}; use ln::msgs; -use ln::onion_utils; +use ln::onion_utils::{self, MessageFailure}; use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError}; use chain::keysinterface::{ChannelKeys, KeysInterface, KeysManager, InMemoryChannelKeys}; use util::config::UserConfig; @@ -1741,7 +1741,7 @@ impl ChannelMan htlc_id: htlc.prev_hop.htlc_id, incoming_packet_shared_secret: htlc.prev_hop.incoming_packet_shared_secret, }), payment_hash, - HTLCFailReason::TypedReason(onion_utils::MessageFailure::IncorrectOrUnknownPaymentDetails { htlc_msat, height}) + HTLCFailReason::TypedReason(MessageFailure::IncorrectOrUnknownPaymentDetails { htlc_msat, height}) )); } } else if total_value == data.total_msat { @@ -1822,13 +1822,11 @@ impl ChannelMan if let Some(mut sources) = removed_source { for htlc in sources.drain(..) { if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); } - let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec(); - htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array( - self.latest_block_height.load(Ordering::Acquire) as u32, - )); + let htlc_msat = htlc.value; + let height = self.latest_block_height.load(Ordering::Acquire) as u32; self.fail_htlc_backwards_internal(channel_state.take().unwrap(), HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash, - HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data }); + HTLCFailReason::TypedReason(MessageFailure::IncorrectOrUnknownPaymentDetails { htlc_msat, height})); } true } else { false } @@ -2016,13 +2014,11 @@ impl ChannelMan for htlc in sources.drain(..) { if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); } if (is_mpp && !valid_mpp) || (!is_mpp && (htlc.value < expected_amount || htlc.value > expected_amount * 2)) { - let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec(); - htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array( - self.latest_block_height.load(Ordering::Acquire) as u32, - )); + let htlc_msat = htlc.value; + let height = self.latest_block_height.load(Ordering::Acquire) as u32; self.fail_htlc_backwards_internal(channel_state.take().unwrap(), HTLCSource::PreviousHopData(htlc.prev_hop), &payment_hash, - HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_height_data }); + HTLCFailReason::TypedReason(MessageFailure::IncorrectOrUnknownPaymentDetails { htlc_msat, height })); } else { match self.claim_funds_from_hop(channel_state.as_mut().unwrap(), htlc.prev_hop, payment_preimage) { Err(Some(e)) => { From e5645a6d8fe3ac394f37bb1a67140209d7d07886 Mon Sep 17 00:00:00 2001 From: Franck Royer Date: Tue, 21 Apr 2020 19:50:58 +1000 Subject: [PATCH 3/6] Use TypedReason for `permanent_channel_failure` --- lightning/src/ln/channelmanager.rs | 43 +++++++++++++++++++++++++----- lightning/src/ln/onion_utils.rs | 5 +++- 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0a9c023b7df..51e83e2c420 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -868,7 +868,13 @@ impl ChannelMan } }; for htlc_source in failed_htlcs.drain(..) { - 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() }); + self.fail_htlc_backwards_internal( + self.channel_state.lock().unwrap(), + htlc_source.0, + &htlc_source.1, + HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure), + ); + } let chan_update = if let Some(chan) = chan_option { if let Ok(update) = self.get_channel_update(&chan) { @@ -891,7 +897,12 @@ impl ChannelMan let (funding_txo_option, monitor_update, mut failed_htlcs) = shutdown_res; log_trace!(self, "Finishing force-closure of channel {} HTLCs to fail", failed_htlcs.len()); for htlc_source in failed_htlcs.drain(..) { - 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() }); + self.fail_htlc_backwards_internal( + self.channel_state.lock().unwrap(), + htlc_source.0, + &htlc_source.1, + HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure) + ); } if let Some(funding_txo) = funding_txo_option { // There isn't anything we can do if we get an update failure - we're already @@ -2441,7 +2452,12 @@ impl ChannelMan } }; for htlc_source in dropped_htlcs.drain(..) { - 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() }); + self.fail_htlc_backwards_internal( + self.channel_state.lock().unwrap(), + htlc_source.0, + &htlc_source.1, + HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure) + ); } if let Some(chan) = chan_option { if let Ok(update) = self.get_channel_update(&chan) { @@ -2942,7 +2958,12 @@ impl events::Me self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage); } else { log_trace!(self, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0)); - self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); + self.fail_htlc_backwards_internal( + self.channel_state.lock().unwrap(), + htlc_update.source, + &htlc_update.payment_hash, + HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure) + ); } } } @@ -2972,7 +2993,12 @@ impl events::Ev self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage); } else { log_trace!(self, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0)); - self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); + self.fail_htlc_backwards_internal( + self.channel_state.lock().unwrap(), + htlc_update.source, + &htlc_update.payment_hash, + HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure) + ); } } } @@ -3827,7 +3853,12 @@ impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: De }; for htlc_source in failed_htlcs.drain(..) { - channel_manager.fail_htlc_backwards_internal(channel_manager.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); + channel_manager.fail_htlc_backwards_internal( + channel_manager.channel_state.lock().unwrap(), + htlc_source.0, + &htlc_source.1, + HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure) + ); } //TODO: Broadcast channel update for closed channels, but only after we've made a diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index 1276cc7d4e1..825d82ea1e2 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -317,6 +317,7 @@ pub(super) fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type: #[derive(Clone)] pub(crate) enum MessageFailure { IncorrectOrUnknownPaymentDetails { htlc_msat: u64, height: u32 }, + PermanentChannelFailure, } impl MessageFailure { @@ -324,6 +325,7 @@ impl MessageFailure { use ln::onion_utils::MessageFailure::*; match *self { IncorrectOrUnknownPaymentDetails { .. } => 0x4000 | 15, + PermanentChannelFailure => 0x4000 | 8, } } @@ -334,7 +336,8 @@ impl MessageFailure { let mut data = byte_utils::be64_to_array(htlc_msat).to_vec(); data.extend_from_slice(&byte_utils::be32_to_array(height)); data - } + }, + &PermanentChannelFailure => Vec::new() } } } From fb82ff5dd50deb97dfda6a66a43548bea5d54900 Mon Sep 17 00:00:00 2001 From: Franck Royer Date: Tue, 21 Apr 2020 19:51:03 +1000 Subject: [PATCH 4/6] Use TypedReason for `unknown_next_peer` --- lightning/src/ln/channelmanager.rs | 9 +++++++-- lightning/src/ln/onion_utils.rs | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 51e83e2c420..ef8e92741c4 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1577,9 +1577,14 @@ impl ChannelMan htlc_id: prev_htlc_id, incoming_packet_shared_secret: forward_info.incoming_shared_secret, }); - failed_forwards.push((htlc_source, forward_info.payment_hash, - HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() } + failed_forwards.push(( + htlc_source, + forward_info.payment_hash, + HTLCFailReason::TypedReason( + MessageFailure::UnknownNextPeer, + ), )); + }, HTLCForwardInfo::FailHTLC { .. } => { // Channel went away before we could fail it. This implies diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index 825d82ea1e2..ddc9f1d40dc 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -318,6 +318,7 @@ pub(super) fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type: pub(crate) enum MessageFailure { IncorrectOrUnknownPaymentDetails { htlc_msat: u64, height: u32 }, PermanentChannelFailure, + UnknownNextPeer, } impl MessageFailure { @@ -326,6 +327,7 @@ impl MessageFailure { match *self { IncorrectOrUnknownPaymentDetails { .. } => 0x4000 | 15, PermanentChannelFailure => 0x4000 | 8, + UnknownNextPeer => 0x4000 | 10, } } @@ -337,7 +339,8 @@ impl MessageFailure { data.extend_from_slice(&byte_utils::be32_to_array(height)); data }, - &PermanentChannelFailure => Vec::new() + &PermanentChannelFailure => Vec::new(), + &UnknownNextPeer => Vec::new() } } } From 930a325d283a2362af214081962001f178bd584f Mon Sep 17 00:00:00 2001 From: Franck Royer Date: Tue, 21 Apr 2020 19:51:07 +1000 Subject: [PATCH 5/6] Use TypedReason for `temporary_channel_failure` --- lightning/src/ln/channelmanager.rs | 23 +++++++++++++++++------ lightning/src/ln/onion_utils.rs | 4 ++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ef8e92741c4..0283e0410d7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1619,9 +1619,15 @@ impl ChannelMan } else { panic!("Stated return value requirements in send_htlc() were not met"); } - let chan_update = self.get_channel_update(chan.get()).unwrap(); - failed_forwards.push((htlc_source, payment_hash, - HTLCFailReason::Reason { failure_code: 0x1000 | 7, data: chan_update.encode_with_len() } + let channel_update = self.get_channel_update(chan.get()).unwrap(); + failed_forwards.push(( + htlc_source, + payment_hash, + HTLCFailReason::TypedReason( + MessageFailure::TemporaryChannelFailure { + channel_update, + }, + ), )); continue; }, @@ -3271,7 +3277,7 @@ impl u16 { use ln::onion_utils::MessageFailure::*; match *self { + TemporaryChannelFailure { .. } => 0x1000 | 7, IncorrectOrUnknownPaymentDetails { .. } => 0x4000 | 15, PermanentChannelFailure => 0x4000 | 8, UnknownNextPeer => 0x4000 | 10, @@ -334,6 +337,7 @@ impl MessageFailure { pub fn data(&self) -> Vec { use ln::onion_utils::MessageFailure::*; match self { + &TemporaryChannelFailure { ref channel_update } => channel_update.encode_with_len(), &IncorrectOrUnknownPaymentDetails { htlc_msat, height } => { let mut data = byte_utils::be64_to_array(htlc_msat).to_vec(); data.extend_from_slice(&byte_utils::be32_to_array(height)); From b60842670611c9724e2e4e3cc7644529e3189b38 Mon Sep 17 00:00:00 2001 From: Franck Royer Date: Tue, 21 Apr 2020 19:51:16 +1000 Subject: [PATCH 6/6] Set top bytes of `failure_code` in constants These values are used at several places of the code. Constants usage is preferred to avoid typos and make code more readable. --- lightning/src/ln/functional_tests.rs | 4 +--- lightning/src/ln/onion_utils.rs | 16 ++++++++-------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 11cf69b40c4..6e11db8522a 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -5408,12 +5408,10 @@ impl Writeable for BogusOnionHopData { fn test_onion_failure() { use ln::msgs::ChannelUpdate; use ln::channelmanager::CLTV_FAR_FAR_AWAY; + use ln::onion_utils::{NODE, PERM, UPDATE}; use secp256k1; const BADONION: u16 = 0x8000; - const PERM: u16 = 0x4000; - const NODE: u16 = 0x2000; - const UPDATE: u16 = 0x1000; let chanmon_cfgs = create_chanmon_cfgs(3); let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index 90c78be6387..36095e4f3e8 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -315,6 +315,10 @@ pub(super) fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type: encrypt_failure_packet(shared_secret, &failure_packet.encode()[..]) } +pub const UPDATE: u16 = 0x1000; +pub const NODE: u16 = 0x2000; +pub const PERM: u16 = 0x4000; + #[derive(Clone)] pub(crate) enum MessageFailure { TemporaryChannelFailure { channel_update: ChannelUpdate }, @@ -327,10 +331,10 @@ impl MessageFailure { pub fn code(&self) -> u16 { use ln::onion_utils::MessageFailure::*; match *self { - TemporaryChannelFailure { .. } => 0x1000 | 7, - IncorrectOrUnknownPaymentDetails { .. } => 0x4000 | 15, - PermanentChannelFailure => 0x4000 | 8, - UnknownNextPeer => 0x4000 | 10, + TemporaryChannelFailure { .. } => UPDATE | 7, + IncorrectOrUnknownPaymentDetails { .. } => PERM | 15, + PermanentChannelFailure => PERM | 8, + UnknownNextPeer => PERM | 10, } } @@ -393,10 +397,6 @@ pub(super) fn process_onion_failure(secp_ctx: &Secp256k1< if fixed_time_eq(&Hmac::from_engine(hmac).into_inner(), &err_packet.hmac) { if let Some(error_code_slice) = err_packet.failuremsg.get(0..2) { - const PERM: u16 = 0x4000; - const NODE: u16 = 0x2000; - const UPDATE: u16 = 0x1000; - let error_code = byte_utils::slice_to_be16(&error_code_slice); error_code_ret = Some(error_code); error_packet_ret = Some(err_packet.failuremsg[2..].to_vec());