Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid retrying over previously failed blinded paths #2818

Merged
merged 7 commits into from
Jan 17, 2024
Merged

Avoid retrying over previously failed blinded paths #2818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ae1288d
Add failed_within_blinded_path to onion util internal struct.
valentinewallace Jan 12, 2024
75a1c47
Add failed_within_blinded_path to DecodedOnionFailure.
valentinewallace Jan 12, 2024
5c5d691
Persist previously failed blinded paths in RouteParameters.
valentinewallace Jan 9, 2024
23ef253
Store previously failed blinded paths on outbound payment failure.
valentinewallace Jan 9, 2024
32ab7a9
Avoid building routes over previously failed blinded payment paths.
valentinewallace Jan 9, 2024
8fcfaeb
Test util: separate out code to construct a blinded path.
valentinewallace Jan 9, 2024
5c87f40
Test that we won't retry over previously failed blinded paths.
valentinewallace Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
127 changes: 120 additions & 7 deletions lightning/src/ln/blinded_payment_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
use crate::blinded_path::BlindedPath;
use crate::blinded_path::payment::{ForwardNode, ForwardTlvs, PaymentConstraints, PaymentRelay, ReceiveTlvs};
use crate::events::{HTLCDestination, MessageSendEvent, MessageSendEventsProvider};
use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PaymentFailureReason};
use crate::ln::PaymentSecret;
use crate::ln::channelmanager;
use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
Expand All @@ -21,15 +21,16 @@ use crate::ln::msgs::ChannelMessageHandler;
use crate::ln::onion_utils;
use crate::ln::onion_utils::INVALID_ONION_BLINDING;
use crate::ln::outbound_payment::Retry;
use crate::offers::invoice::BlindedPayInfo;
use crate::prelude::*;
use crate::routing::router::{Payee, PaymentParameters, RouteParameters};
use crate::util::config::UserConfig;
use crate::util::test_utils;

pub fn get_blinded_route_parameters(
amt_msat: u64, payment_secret: PaymentSecret, node_ids: Vec<PublicKey>,
fn blinded_payment_path(
payment_secret: PaymentSecret, node_ids: Vec<PublicKey>,
channel_upds: &[&msgs::UnsignedChannelUpdate], keys_manager: &test_utils::TestKeysInterface
) -> RouteParameters {
) -> (BlindedPayInfo, BlindedPath) {
let mut intermediate_nodes = Vec::new();
for (node_id, chan_upd) in node_ids.iter().zip(channel_upds) {
intermediate_nodes.push(ForwardNode {
Expand Down Expand Up @@ -58,13 +59,20 @@ pub fn get_blinded_route_parameters(
},
};
let mut secp_ctx = Secp256k1::new();
let blinded_path = BlindedPath::new_for_payment(
BlindedPath::new_for_payment(
&intermediate_nodes[..], *node_ids.last().unwrap(), payee_tlvs,
channel_upds.last().unwrap().htlc_maximum_msat, keys_manager, &secp_ctx
).unwrap();
).unwrap()
}

pub fn get_blinded_route_parameters(
amt_msat: u64, payment_secret: PaymentSecret, node_ids: Vec<PublicKey>,
channel_upds: &[&msgs::UnsignedChannelUpdate], keys_manager: &test_utils::TestKeysInterface
) -> RouteParameters {
RouteParameters::from_payment_params_and_value(
PaymentParameters::blinded(vec![blinded_path]), amt_msat
PaymentParameters::blinded(vec![
blinded_payment_path(payment_secret, node_ids, channel_upds, keys_manager)
]), amt_msat
)
}

Expand Down Expand Up @@ -700,3 +708,108 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) {
expect_payment_failed_conditions(&nodes[0], payment_hash, false,
PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32]));
}

#[test]
fn blinded_path_retries() {
let chanmon_cfgs = create_chanmon_cfgs(4);
// Make one blinded path's fees slightly higher so they are tried in a deterministic order.
let mut higher_fee_chan_cfg = test_default_channel_config();
higher_fee_chan_cfg.channel_config.forwarding_fee_base_msat += 1;
let node_cfgs = create_node_cfgs(4, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, Some(higher_fee_chan_cfg), None]);
let mut nodes = create_network(4, &node_cfgs, &node_chanmgrs);

// Create this network topology so nodes[0] has a blinded route hint to retry over.
// n1
// / \
// n0 n3
// \ /
// n2
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
create_announced_chan_between_nodes_with_value(&nodes, 0, 2, 1_000_000, 0);
let chan_1_3 = create_announced_chan_between_nodes_with_value(&nodes, 1, 3, 1_000_000, 0);
let chan_2_3 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0);

let amt_msat = 5000;
let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[3], Some(amt_msat), None);
let route_params = {
let pay_params = PaymentParameters::blinded(
vec![
blinded_payment_path(payment_secret,
vec![nodes[1].node.get_our_node_id(), nodes[3].node.get_our_node_id()], &[&chan_1_3.0.contents],
&chanmon_cfgs[3].keys_manager
),
blinded_payment_path(payment_secret,
vec![nodes[2].node.get_our_node_id(), nodes[3].node.get_our_node_id()], &[&chan_2_3.0.contents],
&chanmon_cfgs[3].keys_manager
),
]
)
.with_bolt12_features(channelmanager::provided_bolt12_invoice_features(&UserConfig::default()))
.unwrap();
RouteParameters::from_payment_params_and_value(pay_params, amt_msat)
};

nodes[0].node.send_payment(payment_hash, RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0), route_params.clone(), Retry::Attempts(2)).unwrap();
check_added_monitors(&nodes[0], 1);
pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]]], amt_msat, payment_hash, payment_secret);

macro_rules! fail_payment_back {
($intro_node: expr) => {
nodes[3].node.fail_htlc_backwards(&payment_hash);
expect_pending_htlcs_forwardable_conditions(
nodes[3].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash }]
);
nodes[3].node.process_pending_htlc_forwards();
check_added_monitors!(nodes[3], 1);

let updates = get_htlc_update_msgs!(nodes[3], $intro_node.node.get_our_node_id());
assert_eq!(updates.update_fail_malformed_htlcs.len(), 1);
let update_malformed = &updates.update_fail_malformed_htlcs[0];
assert_eq!(update_malformed.sha256_of_onion, [0; 32]);
assert_eq!(update_malformed.failure_code, INVALID_ONION_BLINDING);
$intro_node.node.handle_update_fail_malformed_htlc(&nodes[3].node.get_our_node_id(), update_malformed);
do_commitment_signed_dance(&$intro_node, &nodes[3], &updates.commitment_signed, true, false);

let updates = get_htlc_update_msgs!($intro_node, nodes[0].node.get_our_node_id());
assert_eq!(updates.update_fail_htlcs.len(), 1);
nodes[0].node.handle_update_fail_htlc(&$intro_node.node.get_our_node_id(), &updates.update_fail_htlcs[0]);
do_commitment_signed_dance(&nodes[0], &$intro_node, &updates.commitment_signed, false, false);

let mut events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
match events[0] {
Event::PaymentPathFailed { payment_hash: ev_payment_hash, payment_failed_permanently, .. } => {
assert_eq!(payment_hash, ev_payment_hash);
assert_eq!(payment_failed_permanently, false);
},
_ => panic!("Unexpected event"),
}
match events[1] {
Event::PendingHTLCsForwardable { .. } => {},
_ => panic!("Unexpected event"),
}
nodes[0].node.process_pending_htlc_forwards();
}
}

fail_payment_back!(nodes[1]);

// Pass the retry along.
check_added_monitors!(nodes[0], 1);
let mut msg_events = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(msg_events.len(), 1);
pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], amt_msat, payment_hash, Some(payment_secret), msg_events.pop().unwrap(), true, None);

fail_payment_back!(nodes[2]);
let evs = nodes[0].node.get_and_clear_pending_events();
assert_eq!(evs.len(), 1);
match evs[0] {
Event::PaymentFailed { payment_hash: ev_payment_hash, reason, .. } => {
assert_eq!(ev_payment_hash, payment_hash);
// We have 1 retry attempt remaining, but we're out of blinded paths to try.
assert_eq!(reason, Some(PaymentFailureReason::RouteNotFound));
},
_ => panic!()
}
}
20 changes: 14 additions & 6 deletions lightning/src/ln/onion_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ pub(crate) struct DecodedOnionFailure {
pub(crate) network_update: Option<NetworkUpdate>,
pub(crate) short_channel_id: Option<u64>,
pub(crate) payment_failed_permanently: bool,
pub(crate) failed_within_blinded_path: bool,
#[cfg(test)]
pub(crate) onion_error_code: Option<u16>,
#[cfg(test)]
Expand Down Expand Up @@ -463,6 +464,7 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(
network_update: Option<NetworkUpdate>,
short_channel_id: Option<u64>,
payment_failed_permanently: bool,
failed_within_blinded_path: bool,
}
let mut res: Option<FailureLearnings> = None;
let mut htlc_msat = *first_hop_htlc_msat;
Expand All @@ -488,7 +490,8 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(
error_code_ret = Some(BADONION | PERM | 24); // invalid_onion_blinding
error_packet_ret = Some(vec![0; 32]);
res = Some(FailureLearnings {
network_update: None, short_channel_id: None, payment_failed_permanently: false
network_update: None, short_channel_id: None, payment_failed_permanently: false,
failed_within_blinded_path: true,
});
return
},
Expand Down Expand Up @@ -520,7 +523,8 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(
}

res = Some(FailureLearnings {
network_update: None, short_channel_id: None, payment_failed_permanently: false
network_update: None, short_channel_id: None, payment_failed_permanently: false,
failed_within_blinded_path: true,
});
return
}
Expand Down Expand Up @@ -550,7 +554,8 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(
});
let short_channel_id = Some(route_hop.short_channel_id);
res = Some(FailureLearnings {
network_update, short_channel_id, payment_failed_permanently: is_from_final_node
network_update, short_channel_id, payment_failed_permanently: is_from_final_node,
failed_within_blinded_path: false
});
return
}
Expand Down Expand Up @@ -706,7 +711,8 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(

res = Some(FailureLearnings {
network_update, short_channel_id,
payment_failed_permanently: error_code & PERM == PERM && is_from_final_node
payment_failed_permanently: error_code & PERM == PERM && is_from_final_node,
failed_within_blinded_path: false
});

let (description, title) = errors::get_onion_error_description(error_code);
Expand All @@ -717,10 +723,10 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(
}
}).expect("Route that we sent via spontaneously grew invalid keys in the middle of it?");
if let Some(FailureLearnings {
network_update, short_channel_id, payment_failed_permanently
network_update, short_channel_id, payment_failed_permanently, failed_within_blinded_path
}) = res {
DecodedOnionFailure {
network_update, short_channel_id, payment_failed_permanently,
network_update, short_channel_id, payment_failed_permanently, failed_within_blinded_path,
#[cfg(test)]
onion_error_code: error_code_ret,
#[cfg(test)]
Expand All @@ -731,6 +737,7 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(
// payment not retryable only when garbage is from the final node
DecodedOnionFailure {
network_update: None, short_channel_id: None, payment_failed_permanently: is_from_final_node,
failed_within_blinded_path: false,
#[cfg(test)]
onion_error_code: None,
#[cfg(test)]
Expand Down Expand Up @@ -878,6 +885,7 @@ impl HTLCFailReason {
network_update: None,
payment_failed_permanently: false,
short_channel_id: Some(path.hops[0].short_channel_id),
failed_within_blinded_path: false,
#[cfg(test)]
onion_error_code: Some(*failure_code),
#[cfg(test)]
Expand Down
22 changes: 17 additions & 5 deletions lightning/src/ln/outbound_payment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret};
use crate::ln::channelmanager::{ChannelDetails, EventCompletionAction, HTLCSource, PaymentId};
use crate::ln::onion_utils::{DecodedOnionFailure, HTLCFailReason};
use crate::offers::invoice::Bolt12Invoice;
use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteParameters, Router};
use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, PaymentParameters, Route, RouteParameters, Router};
use crate::util::errors::APIError;
use crate::util::logger::Logger;
use crate::util::time::Time;
Expand Down Expand Up @@ -129,6 +129,11 @@ impl PendingOutboundPayment {
params.previously_failed_channels.push(scid);
}
}
pub fn insert_previously_failed_blinded_path(&mut self, blinded_tail: &BlindedTail) {
if let PendingOutboundPayment::Retryable { payment_params: Some(params), .. } = self {
params.insert_previously_failed_blinded_path(blinded_tail);
}
}
fn is_awaiting_invoice(&self) -> bool {
match self {
PendingOutboundPayment::AwaitingInvoice { .. } => true,
Expand Down Expand Up @@ -243,7 +248,7 @@ impl PendingOutboundPayment {
if insert_res {
if let PendingOutboundPayment::Retryable {
ref mut pending_amt_msat, ref mut pending_fee_msat,
ref mut remaining_max_total_routing_fee_msat, ..
ref mut remaining_max_total_routing_fee_msat, ..
} = self {
*pending_amt_msat += path.final_value_msat();
let path_fee_msat = path.fee_msat();
Expand Down Expand Up @@ -1604,11 +1609,12 @@ impl OutboundPayments {
#[cfg(test)]
let DecodedOnionFailure {
network_update, short_channel_id, payment_failed_permanently, onion_error_code,
onion_error_data
onion_error_data, failed_within_blinded_path
} = onion_error.decode_onion_failure(secp_ctx, logger, &source);
#[cfg(not(test))]
let DecodedOnionFailure { network_update, short_channel_id, payment_failed_permanently } =
onion_error.decode_onion_failure(secp_ctx, logger, &source);
let DecodedOnionFailure {
network_update, short_channel_id, payment_failed_permanently, failed_within_blinded_path
} = onion_error.decode_onion_failure(secp_ctx, logger, &source);

let payment_is_probe = payment_is_probe(payment_hash, &payment_id, probing_cookie_secret);
let mut session_priv_bytes = [0; 32];
Expand Down Expand Up @@ -1647,6 +1653,12 @@ impl OutboundPayments {
// next-hop is needlessly blaming us!
payment.get_mut().insert_previously_failed_scid(scid);
}
if failed_within_blinded_path {
debug_assert!(short_channel_id.is_none());
if let Some(bt) = &path.blinded_tail {
payment.get_mut().insert_previously_failed_blinded_path(&bt);
} else { debug_assert!(false); }
}

if payment_is_probe || !is_retryable_now || payment_failed_permanently {
let reason = if payment_failed_permanently {
Expand Down