Skip to content

Commit

Permalink
Add Path struct and use it everywhere instead of Vec<RouteHop>
Browse files Browse the repository at this point in the history
  • Loading branch information
valentinewallace committed Apr 3, 2023
1 parent 2e82123 commit 84ffb4c
Show file tree
Hide file tree
Showing 17 changed files with 799 additions and 697 deletions.
10 changes: 5 additions & 5 deletions fuzz/src/chanmon_consistency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use lightning::util::errors::APIError;
use lightning::util::logger::Logger;
use lightning::util::config::UserConfig;
use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer};
use lightning::routing::router::{InFlightHtlcs, Route, RouteHop, RouteParameters, Router};
use lightning::routing::router::{InFlightHtlcs, Path, Route, RouteHop, RouteParameters, Router};

use crate::utils::test_logger::{self, Output};
use crate::utils::test_persister::TestPersister;
Expand Down Expand Up @@ -352,14 +352,14 @@ fn send_payment(source: &ChanMan, dest: &ChanMan, dest_chan_id: u64, amt: u64, p
payment_id[0..8].copy_from_slice(&payment_idx.to_ne_bytes());
*payment_idx += 1;
if let Err(err) = source.send_payment(&Route {
paths: vec![vec![RouteHop {
paths: vec![Path { hops: vec![RouteHop {
pubkey: dest.get_our_node_id(),
node_features: dest.node_features(),
short_channel_id: dest_chan_id,
channel_features: dest.channel_features(),
fee_msat: amt,
cltv_expiry_delta: 200,
}]],
}], blinded_tail: None }],
payment_params: None,
}, payment_hash, &Some(payment_secret), PaymentId(payment_id)) {
check_payment_err(err);
Expand All @@ -374,7 +374,7 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
payment_id[0..8].copy_from_slice(&payment_idx.to_ne_bytes());
*payment_idx += 1;
if let Err(err) = source.send_payment(&Route {
paths: vec![vec![RouteHop {
paths: vec![Path { hops: vec![RouteHop {
pubkey: middle.get_our_node_id(),
node_features: middle.node_features(),
short_channel_id: middle_chan_id,
Expand All @@ -388,7 +388,7 @@ fn send_hop_payment(source: &ChanMan, middle: &ChanMan, middle_chan_id: u64, des
channel_features: dest.channel_features(),
fee_msat: amt,
cltv_expiry_delta: 200,
}]],
}], blinded_tail: None }],
payment_params: None,
}, payment_hash, &Some(payment_secret), PaymentId(payment_id)) {
check_payment_err(err);
Expand Down
2 changes: 1 addition & 1 deletion lightning-invoice/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1173,7 +1173,7 @@ mod test {
nodes[fwd_idx].node.process_pending_htlc_forwards();

let payment_preimage_opt = if user_generated_pmt_hash { None } else { Some(payment_preimage) };
expect_payment_claimable!(&nodes[fwd_idx], payment_hash, payment_secret, payment_amt, payment_preimage_opt, route.paths[0].last().unwrap().pubkey);
expect_payment_claimable!(&nodes[fwd_idx], payment_hash, payment_secret, payment_amt, payment_preimage_opt, route.paths[0].hops.last().unwrap().pubkey);
do_claim_payment_along_route(&nodes[0], &[&vec!(&nodes[fwd_idx])[..]], false, payment_preimage);
let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
Expand Down
12 changes: 6 additions & 6 deletions lightning/src/ln/chanmon_update_fail_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1974,12 +1974,12 @@ fn test_path_paused_mpp() {
// Set us up to take multiple routes, one 0 -> 1 -> 3 and one 0 -> 2 -> 3:
let path = route.paths[0].clone();
route.paths.push(path);
route.paths[0][0].pubkey = nodes[1].node.get_our_node_id();
route.paths[0][0].short_channel_id = chan_1_id;
route.paths[0][1].short_channel_id = chan_3_id;
route.paths[1][0].pubkey = nodes[2].node.get_our_node_id();
route.paths[1][0].short_channel_id = chan_2_ann.contents.short_channel_id;
route.paths[1][1].short_channel_id = chan_4_id;
route.paths[0].hops[0].pubkey = nodes[1].node.get_our_node_id();
route.paths[0].hops[0].short_channel_id = chan_1_id;
route.paths[0].hops[1].short_channel_id = chan_3_id;
route.paths[1].hops[0].pubkey = nodes[2].node.get_our_node_id();
route.paths[1].hops[0].short_channel_id = chan_2_ann.contents.short_channel_id;
route.paths[1].hops[1].short_channel_id = chan_4_id;

// Set it so that the first monitor update (for the path 0 -> 1 -> 3) succeeds, but the second
// (for the path 0 -> 2 -> 3) fails.
Expand Down
3 changes: 2 additions & 1 deletion lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6977,6 +6977,7 @@ mod tests {
use crate::chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator, ConfirmationTarget};
use crate::chain::keysinterface::{ChannelSigner, InMemorySigner, EntropySource, SignerProvider};
use crate::chain::transaction::OutPoint;
use crate::routing::router::Path;
use crate::util::config::UserConfig;
use crate::util::enforcing_trait_impls::EnforcingSigner;
use crate::util::errors::APIError;
Expand Down Expand Up @@ -7154,7 +7155,7 @@ mod tests {
cltv_expiry: 200000000,
state: OutboundHTLCState::Committed,
source: HTLCSource::OutboundRoute {
path: Vec::new(),
path: Path { hops: Vec::new(), blinded_tail: None },
session_priv: SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(),
first_hop_htlc_msat: 548,
payment_id: PaymentId([42; 32]),
Expand Down
49 changes: 26 additions & 23 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, No
#[cfg(any(feature = "_test_utils", test))]
use crate::ln::features::InvoiceFeatures;
use crate::routing::gossip::NetworkGraph;
use crate::routing::router::{DefaultRouter, InFlightHtlcs, PaymentParameters, Route, RouteHop, RouteParameters, RoutePath, Router};
use crate::routing::router::{BlindedTail, DefaultRouter, InFlightHtlcs, Path, PaymentParameters, Route, RouteHop, RouteParameters, Router};
use crate::routing::scoring::ProbabilisticScorer;
use crate::ln::msgs;
use crate::ln::onion_utils;
Expand Down Expand Up @@ -280,7 +280,7 @@ impl_writeable_tlv_based_enum!(SentHTLCId,
pub(crate) enum HTLCSource {
PreviousHopData(HTLCPreviousHopData),
OutboundRoute {
path: Vec<RouteHop>,
path: Path,
session_priv: SecretKey,
/// 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
Expand Down Expand Up @@ -313,7 +313,7 @@ impl core::hash::Hash for HTLCSource {
impl HTLCSource {
pub fn dummy() -> Self {
HTLCSource::OutboundRoute {
path: Vec::new(),
path: Path { hops: Vec::new(), blinded_tail: None },
session_priv: SecretKey::from_slice(&[1; 32]).unwrap(),
first_hop_htlc_msat: 0,
payment_id: PaymentId([2; 32]),
Expand Down Expand Up @@ -2513,16 +2513,16 @@ where
}

#[cfg(test)]
pub(crate) fn test_send_payment_along_path(&self, path: &Vec<RouteHop>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>, session_priv_bytes: [u8; 32]) -> Result<(), APIError> {
pub(crate) fn test_send_payment_along_path(&self, path: &Path, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>, session_priv_bytes: [u8; 32]) -> Result<(), APIError> {
let _lck = self.total_consistency_lock.read().unwrap();
self.send_payment_along_path(path, payment_hash, payment_secret, total_value, cur_height, payment_id, keysend_preimage, session_priv_bytes)
}

fn send_payment_along_path(&self, path: &Vec<RouteHop>, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>, session_priv_bytes: [u8; 32]) -> Result<(), APIError> {
fn send_payment_along_path(&self, path: &Path, payment_hash: &PaymentHash, payment_secret: &Option<PaymentSecret>, total_value: u64, cur_height: u32, payment_id: PaymentId, keysend_preimage: &Option<PaymentPreimage>, session_priv_bytes: [u8; 32]) -> Result<(), APIError> {
// The top-level caller should hold the total_consistency_lock read lock.
debug_assert!(self.total_consistency_lock.try_write().is_err());

log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.first().unwrap().short_channel_id);
log_trace!(self.logger, "Attempting to send payment for path with next hop {}", path.first_hop_scid());
let prng_seed = self.entropy_source.get_secure_random_bytes();
let session_priv = SecretKey::from_slice(&session_priv_bytes[..]).expect("RNG is busted");

Expand All @@ -2535,7 +2535,7 @@ where
let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash);

let err: Result<(), _> = loop {
let (counterparty_node_id, id) = match self.short_to_chan_info.read().unwrap().get(&path.first().unwrap().short_channel_id) {
let (counterparty_node_id, id) = match self.short_to_chan_info.read().unwrap().get(&path.first_hop_scid()) {
None => return Err(APIError::ChannelUnavailable{err: "No channel available with first hop!".to_owned()}),
Some((cp_id, chan_id)) => (cp_id.clone(), chan_id.clone()),
};
Expand Down Expand Up @@ -2587,7 +2587,7 @@ where
return Ok(());
};

match handle_error!(self, err, path.first().unwrap().pubkey) {
match handle_error!(self, err, path.first_hop_node_id()) {
Ok(_) => unreachable!(),
Err(e) => {
Err(APIError::ChannelUnavailable { err: e.err })
Expand Down Expand Up @@ -6902,23 +6902,25 @@ impl Readable for HTLCSource {
let mut payment_id = None;
let mut payment_secret = None;
let mut payment_params: Option<PaymentParameters> = None;
let mut blinded_tail: Option<BlindedTail> = None;
read_tlv_fields!(reader, {
(0, session_priv, required),
(1, payment_id, option),
(2, first_hop_htlc_msat, required),
(3, payment_secret, option),
(4, path, vec_type),
(5, payment_params, (option: ReadableArgs, 0)),
(6, blinded_tail, option),
});
if payment_id.is_none() {
// For backwards compat, if there was no payment_id written, use the session_priv bytes
// instead.
payment_id = Some(PaymentId(*session_priv.0.unwrap().as_ref()));
}
if path.is_none() || path.as_ref().unwrap().is_empty() {
let path = path.unwrap_or(Vec::new());
if path.is_empty() && blinded_tail.is_none() {
return Err(DecodeError::InvalidValue);
}
let path = path.unwrap();
if let Some(params) = payment_params.as_mut() {
if params.final_cltv_expiry_delta == 0 {
params.final_cltv_expiry_delta = path.last().unwrap().cltv_expiry_delta;
Expand All @@ -6927,7 +6929,7 @@ impl Readable for HTLCSource {
Ok(HTLCSource::OutboundRoute {
session_priv: session_priv.0.unwrap(),
first_hop_htlc_msat,
path,
path: Path { hops: path, blinded_tail },
payment_id: payment_id.unwrap(),
payment_secret,
})
Expand All @@ -6949,8 +6951,9 @@ impl Writeable for HTLCSource {
(1, payment_id_opt, option),
(2, first_hop_htlc_msat, required),
(3, payment_secret, option),
(4, *path, vec_type),
(4, path.hops, vec_type),
(5, None::<PaymentParameters>, option), // payment_params in LDK versions prior to 0.0.115
(6, path.blinded_tail, option),
});
}
HTLCSource::PreviousHopData(ref field) => {
Expand Down Expand Up @@ -7612,12 +7615,12 @@ where
if id_to_peer.get(&monitor.get_funding_txo().0.to_channel_id()).is_none() {
for (htlc_source, (htlc, _)) in monitor.get_pending_or_resolved_outbound_htlcs() {
if let HTLCSource::OutboundRoute { payment_id, session_priv, path, payment_secret, .. } = htlc_source {
if path.is_empty() {
if path.len() == 0 {
log_error!(args.logger, "Got an empty path for a pending payment");
return Err(DecodeError::InvalidValue);
}

let path_amt = path.last().unwrap().fee_msat;
let path_amt = path.final_value_msat();
let mut session_priv_bytes = [0; 32];
session_priv_bytes[..].copy_from_slice(&session_priv[..]);
match pending_outbounds.pending_outbound_payments.lock().unwrap().entry(payment_id) {
Expand All @@ -7627,7 +7630,7 @@ where
if newly_added { "Added" } else { "Had" }, path_amt, log_bytes!(session_priv_bytes), log_bytes!(htlc.payment_hash.0));
},
hash_map::Entry::Vacant(entry) => {
let path_fee = path.get_path_fees();
let path_fee = path.fee_msat();
entry.insert(PendingOutboundPayment::Retryable {
retry_strategy: None,
attempts: PaymentAttempts::new(),
Expand Down Expand Up @@ -8130,15 +8133,15 @@ mod tests {
Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
assert_eq!(payment_id, *actual_payment_id);
assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
assert_eq!(route.paths[0], *path);
assert_eq!(route.paths[0].hops, *path);
},
_ => panic!("Unexpected event"),
}
match events[2] {
Event::PaymentPathSuccessful { payment_id: ref actual_payment_id, ref payment_hash, ref path } => {
assert_eq!(payment_id, *actual_payment_id);
assert_eq!(our_payment_hash, *payment_hash.as_ref().unwrap());
assert_eq!(route.paths[0], *path);
assert_eq!(route.paths[0].hops, *path);
},
_ => panic!("Unexpected event"),
}
Expand Down Expand Up @@ -8342,12 +8345,12 @@ mod tests {
let (mut route, payment_hash, _, _) = get_route_and_payment_hash!(&nodes[0], nodes[3], 100000);
let path = route.paths[0].clone();
route.paths.push(path);
route.paths[0][0].pubkey = nodes[1].node.get_our_node_id();
route.paths[0][0].short_channel_id = chan_1_id;
route.paths[0][1].short_channel_id = chan_3_id;
route.paths[1][0].pubkey = nodes[2].node.get_our_node_id();
route.paths[1][0].short_channel_id = chan_2_id;
route.paths[1][1].short_channel_id = chan_4_id;
route.paths[0].hops[0].pubkey = nodes[1].node.get_our_node_id();
route.paths[0].hops[0].short_channel_id = chan_1_id;
route.paths[0].hops[1].short_channel_id = chan_3_id;
route.paths[1].hops[0].pubkey = nodes[2].node.get_our_node_id();
route.paths[1].hops[0].short_channel_id = chan_2_id;
route.paths[1].hops[1].short_channel_id = chan_4_id;

match nodes[0].node.send_payment(&route, payment_hash, &None, PaymentId(payment_hash.0)).unwrap_err() {
PaymentSendFailure::ParameterError(APIError::APIMisuseError { ref err }) => {
Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2189,7 +2189,7 @@ pub fn route_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route:
let route = get_route(origin_node, &payment_params, recv_value, TEST_FINAL_CLTV).unwrap();
assert_eq!(route.paths.len(), 1);
assert_eq!(route.paths[0].len(), expected_route.len());
for (node, hop) in expected_route.iter().zip(route.paths[0].iter()) {
for (node, hop) in expected_route.iter().zip(route.paths[0].hops.iter()) {
assert_eq!(hop.pubkey, node.node.get_our_node_id());
}

Expand All @@ -2210,7 +2210,7 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
None, recv_value, TEST_FINAL_CLTV, origin_node.logger, &scorer, &random_seed_bytes).unwrap();
assert_eq!(route.paths.len(), 1);
assert_eq!(route.paths[0].len(), expected_route.len());
for (node, hop) in expected_route.iter().zip(route.paths[0].iter()) {
for (node, hop) in expected_route.iter().zip(route.paths[0].hops.iter()) {
assert_eq!(hop.pubkey, node.node.get_our_node_id());
}

Expand Down
Loading

0 comments on commit 84ffb4c

Please sign in to comment.