From c986e52ce83e9aeaa9447abebc5f6600470337cf Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 19 Aug 2021 16:45:55 -0400 Subject: [PATCH] Add MppId field to HTLCSource as a way to correlate mpp payment paths --- lightning/src/ln/channel.rs | 3 +- lightning/src/ln/channelmanager.rs | 93 ++++++++++++++++++++++++---- lightning/src/ln/functional_tests.rs | 5 +- lightning/src/ln/onion_utils.rs | 2 +- 4 files changed, 86 insertions(+), 17 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 930511c0d..24b1eafaf 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -5525,7 +5525,7 @@ mod tests { use bitcoin::hashes::hex::FromHex; use hex; use ln::{PaymentPreimage, PaymentHash}; - use ln::channelmanager::HTLCSource; + use ln::channelmanager::{HTLCSource, MppId}; use ln::channel::{Channel,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,HTLCCandidate,HTLCInitiator,TxCreationKeys}; use ln::channel::MAX_FUNDING_SATOSHIS; use ln::features::InitFeatures; @@ -5699,6 +5699,7 @@ mod tests { path: Vec::new(), session_priv: SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(), first_hop_htlc_msat: 548, + mpp_id: MppId([42; 32]), } }); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b772c5e9a..583ebfb9b 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -173,6 +173,22 @@ struct ClaimableHTLC { onion_payload: OnionPayload, } +/// A payment identifier used to correlate an MPP payment's per-path HTLC sources internally. +#[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)] +pub(crate) struct MppId(pub [u8; 32]); + +impl Writeable for MppId { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.0.write(w) + } +} + +impl Readable for MppId { + fn read(r: &mut R) -> Result { + let buf: [u8; 32] = Readable::read(r)?; + Ok(MppId(buf)) + } +} /// Tracks the inbound corresponding to an outbound HTLC #[derive(Clone, PartialEq)] pub(crate) enum HTLCSource { @@ -183,6 +199,7 @@ pub(crate) enum HTLCSource { /// Technically we can recalculate this from the route, but we cache it here to avoid /// doing a double-pass on route when we get a failure back first_hop_htlc_msat: u64, + mpp_id: MppId, }, } #[cfg(test)] @@ -192,6 +209,7 @@ impl HTLCSource { path: Vec::new(), session_priv: SecretKey::from_slice(&[1; 32]).unwrap(), first_hop_htlc_msat: 0, + mpp_id: MppId([2; 32]), } } } @@ -1820,7 +1838,7 @@ impl ChannelMana } // Only public for testing, this should otherwise never be called direcly - pub(crate) fn send_payment_along_path(&self, path: &Vec, payment_hash: &PaymentHash, payment_secret: &Option, total_value: u64, cur_height: u32, keysend_preimage: &Option) -> Result<(), APIError> { + pub(crate) fn send_payment_along_path(&self, path: &Vec, payment_hash: &PaymentHash, payment_secret: &Option, total_value: u64, cur_height: u32, mpp_id: MppId, keysend_preimage: &Option) -> Result<(), APIError> { log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.first().unwrap().short_channel_id); let prng_seed = self.keys_manager.get_secure_random_bytes(); let session_priv_bytes = self.keys_manager.get_secure_random_bytes(); @@ -1857,6 +1875,7 @@ impl ChannelMana path: path.clone(), session_priv: session_priv.clone(), first_hop_htlc_msat: htlc_msat, + mpp_id, }, onion_packet, &self.logger), channel_state, chan) } { Some((update_add, commitment_signed, monitor_update)) => { @@ -1956,6 +1975,7 @@ impl ChannelMana let mut total_value = 0; let our_node_id = self.get_our_node_id(); let mut path_errs = Vec::with_capacity(route.paths.len()); + let mpp_id = MppId(self.keys_manager.get_secure_random_bytes()); 'path_check: for path in route.paths.iter() { if path.len() < 1 || path.len() > 20 { path_errs.push(Err(APIError::RouteError{err: "Path didn't go anywhere/had bogus size"})); @@ -1977,7 +1997,7 @@ impl ChannelMana let cur_height = self.best_block.read().unwrap().height() + 1; let mut results = Vec::new(); for path in route.paths.iter() { - results.push(self.send_payment_along_path(&path, &payment_hash, payment_secret, total_value, cur_height, &keysend_preimage)); + results.push(self.send_payment_along_path(&path, &payment_hash, payment_secret, total_value, cur_height, mpp_id, &keysend_preimage)); } let mut has_ok = false; let mut has_err = false; @@ -4911,14 +4931,60 @@ impl Readable for ClaimableHTLC { } } -impl_writeable_tlv_based_enum!(HTLCSource, - (0, OutboundRoute) => { - (0, session_priv, required), - (2, first_hop_htlc_msat, required), - (4, path, vec_type), - }, ; - (1, PreviousHopData) -); +impl Readable for HTLCSource { + fn read(reader: &mut R) -> Result { + let id: u8 = Readable::read(reader)?; + match id { + 0 => { + let mut session_priv: ::util::ser::OptionDeserWrapper = ::util::ser::OptionDeserWrapper(None); + let mut first_hop_htlc_msat: u64 = 0; + let mut path = Some(Vec::new()); + let mut mpp_id = None; + read_tlv_fields!(reader, { + (0, session_priv, required), + (1, mpp_id, option), + (2, first_hop_htlc_msat, required), + (4, path, vec_type), + }); + if mpp_id.is_none() { + // For backwards compat, if there was no mpp_id written, use the session_priv bytes + // instead. + mpp_id = Some(MppId(*session_priv.0.unwrap().as_ref())); + } + Ok(HTLCSource::OutboundRoute { + session_priv: session_priv.0.unwrap(), + first_hop_htlc_msat: first_hop_htlc_msat, + path: path.unwrap(), + mpp_id: mpp_id.unwrap(), + }) + } + 1 => Ok(HTLCSource::PreviousHopData(Readable::read(reader)?)), + _ => Err(DecodeError::UnknownRequiredFeature), + } + } +} + +impl Writeable for HTLCSource { + fn write(&self, writer: &mut W) -> Result<(), ::io::Error> { + match self { + HTLCSource::OutboundRoute { ref session_priv, ref first_hop_htlc_msat, ref path, mpp_id } => { + 0u8.write(writer)?; + let mpp_id_opt = Some(mpp_id); + write_tlv_fields!(writer, { + (0, session_priv, required), + (1, mpp_id_opt, option), + (2, first_hop_htlc_msat, required), + (4, path, vec_type), + }); + } + HTLCSource::PreviousHopData(ref field) => { + 1u8.write(writer)?; + field.write(writer)?; + } + } + Ok(()) + } +} impl_writeable_tlv_based_enum!(HTLCFailReason, (0, LightningError) => { @@ -5364,7 +5430,7 @@ mod tests { use bitcoin::hashes::sha256::Hash as Sha256; use core::time::Duration; use ln::{PaymentPreimage, PaymentHash, PaymentSecret}; - use ln::channelmanager::PaymentSendFailure; + use ln::channelmanager::{MppId, PaymentSendFailure}; use ln::features::{InitFeatures, InvoiceFeatures}; use ln::functional_test_utils::*; use ln::msgs; @@ -5515,10 +5581,11 @@ mod tests { let net_graph_msg_handler = &nodes[0].net_graph_msg_handler; let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph, &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 100_000, TEST_FINAL_CLTV, &logger).unwrap(); let (payment_preimage, our_payment_hash, payment_secret) = get_payment_preimage_hash!(&nodes[1]); + let mpp_id = MppId([42; 32]); // Use the utility function send_payment_along_path to send the payment with MPP data which // indicates there are more HTLCs coming. let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match. - nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, &None).unwrap(); + nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, mpp_id, &None).unwrap(); check_added_monitors!(nodes[0], 1); let mut events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); @@ -5548,7 +5615,7 @@ mod tests { expect_payment_failed!(nodes[0], our_payment_hash, true); // Send the second half of the original MPP payment. - nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, &None).unwrap(); + nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200_000, cur_height, mpp_id, &None).unwrap(); check_added_monitors!(nodes[0], 1); let mut events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 452b2db5d..7db74900c 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -19,7 +19,7 @@ use chain::transaction::OutPoint; use chain::keysinterface::BaseSign; use ln::{PaymentPreimage, PaymentSecret, PaymentHash}; use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC}; -use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA}; +use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, MppId, RAACommitmentOrder, PaymentSendFailure, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA}; use ln::channel::{Channel, ChannelError}; use ln::{chan_utils, onion_utils}; use ln::chan_utils::HTLC_SUCCESS_TX_WEIGHT; @@ -3886,7 +3886,8 @@ fn do_test_htlc_timeout(send_partial_mpp: bool) { // Use the utility function send_payment_along_path to send the payment with MPP data which // indicates there are more HTLCs coming. let cur_height = CHAN_CONFIRM_DEPTH + 1; // route_payment calls send_payment, which adds 1 to the current height. So we do the same here to match. - nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200000, cur_height, &None).unwrap(); + let mpp_id = MppId([42; 32]); + nodes[0].node.send_payment_along_path(&route.paths[0], &our_payment_hash, &Some(payment_secret), 200000, cur_height, mpp_id, &None).unwrap(); check_added_monitors!(nodes[0], 1); let mut events = nodes[0].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index f6c62cb83..ee3ed96b5 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -332,7 +332,7 @@ pub(super) fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type: /// Returns update, a boolean indicating that the payment itself failed, and the error code. #[inline] pub(super) fn process_onion_failure(secp_ctx: &Secp256k1, logger: &L, htlc_source: &HTLCSource, mut packet_decrypted: Vec) -> (Option, bool, Option, Option>) where L::Target: Logger { - if let &HTLCSource::OutboundRoute { ref path, ref session_priv, ref first_hop_htlc_msat } = htlc_source { + if let &HTLCSource::OutboundRoute { ref path, ref session_priv, ref first_hop_htlc_msat, .. } = htlc_source { let mut res = None; let mut htlc_msat = *first_hop_htlc_msat; let mut error_code_ret = None; -- 2.39.5