-
Notifications
You must be signed in to change notification settings - Fork 368
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.
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
Complete route blinding support #2812
Changes from all commits
4a0170a
f1e4645
8ad292e
f09ac19
c37d109
a76bb51
6e6bd44
027aef2
aae39b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -490,6 +490,29 @@ fn two_hop_blinded_path_success() { | |||||
claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage); | ||||||
} | ||||||
|
||||||
#[test] | ||||||
fn three_hop_blinded_path_success() { | ||||||
let chanmon_cfgs = create_chanmon_cfgs(5); | ||||||
let node_cfgs = create_node_cfgs(5, &chanmon_cfgs); | ||||||
let node_chanmgrs = create_node_chanmgrs(5, &node_cfgs, &[None, None, None, None, None]); | ||||||
let mut nodes = create_network(5, &node_cfgs, &node_chanmgrs); | ||||||
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); | ||||||
create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0); | ||||||
let chan_upd_2_3 = create_announced_chan_between_nodes_with_value(&nodes, 2, 3, 1_000_000, 0).0.contents; | ||||||
let chan_upd_3_4 = create_announced_chan_between_nodes_with_value(&nodes, 3, 4, 1_000_000, 0).0.contents; | ||||||
|
||||||
let amt_msat = 5000; | ||||||
let (payment_preimage, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[4], Some(amt_msat), None); | ||||||
let route_params = get_blinded_route_parameters(amt_msat, payment_secret, | ||||||
nodes.iter().skip(2).map(|n| n.node.get_our_node_id()).collect(), | ||||||
&[&chan_upd_2_3, &chan_upd_3_4], &chanmon_cfgs[4].keys_manager); | ||||||
|
||||||
nodes[0].node.send_payment(payment_hash, RecipientOnionFields::spontaneous_empty(), PaymentId(payment_hash.0), route_params, Retry::Attempts(0)).unwrap(); | ||||||
check_added_monitors(&nodes[0], 1); | ||||||
pass_along_route(&nodes[0], &[&[&nodes[1], &nodes[2], &nodes[3], &nodes[4]]], amt_msat, payment_hash, payment_secret); | ||||||
claim_payment(&nodes[0], &[&nodes[1], &nodes[2], &nodes[3], &nodes[4]], payment_preimage); | ||||||
} | ||||||
|
||||||
#[derive(PartialEq)] | ||||||
enum ReceiveCheckFail { | ||||||
// The recipient fails the payment upon `PaymentClaimable`. | ||||||
|
@@ -537,19 +560,20 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { | |||||
}; | ||||||
|
||||||
let amt_msat = 5000; | ||||||
let final_cltv_delta = if check == ReceiveCheckFail::ProcessPendingHTLCsCheck { | ||||||
let excess_final_cltv_delta_opt = if check == ReceiveCheckFail::ProcessPendingHTLCsCheck { | ||||||
// Set the final CLTV expiry too low to trigger the failure in process_pending_htlc_forwards. | ||||||
Some(TEST_FINAL_CLTV as u16 - 2) | ||||||
} else { None }; | ||||||
let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), final_cltv_delta); | ||||||
let (_, payment_hash, payment_secret) = get_payment_preimage_hash(&nodes[2], Some(amt_msat), excess_final_cltv_delta_opt); | ||||||
let mut route_params = get_blinded_route_parameters(amt_msat, payment_secret, | ||||||
nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(), &[&chan_upd_1_2], | ||||||
&chanmon_cfgs[2].keys_manager); | ||||||
|
||||||
let route = if check == ReceiveCheckFail::ProcessPendingHTLCsCheck { | ||||||
let mut route = get_route(&nodes[0], &route_params).unwrap(); | ||||||
// Set the final CLTV expiry too low to trigger the failure in process_pending_htlc_forwards. | ||||||
route.paths[0].blinded_tail.as_mut().map(|bt| bt.excess_final_cltv_expiry_delta = TEST_FINAL_CLTV - 2); | ||||||
route.paths[0].hops.last_mut().map(|h| h.cltv_expiry_delta += excess_final_cltv_delta_opt.unwrap() as u32); | ||||||
route.paths[0].blinded_tail.as_mut().map(|bt| bt.excess_final_cltv_expiry_delta = excess_final_cltv_delta_opt.unwrap() as u32); | ||||||
route | ||||||
} else if check == ReceiveCheckFail::PaymentConstraints { | ||||||
// Create a blinded path where the receiver's encrypted payload has an htlc_minimum_msat that is | ||||||
|
@@ -657,6 +681,7 @@ fn do_multi_hop_receiver_fail(check: ReceiveCheckFail) { | |||||
commitment_signed_dance!(nodes[2], nodes[1], (), false, true, false, false); | ||||||
}, | ||||||
ReceiveCheckFail::ProcessPendingHTLCsCheck => { | ||||||
assert_eq!(payment_event_1_2.msgs[0].cltv_expiry, nodes[0].best_block_info().1 + 1 + excess_final_cltv_delta_opt.unwrap() as u32); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The assertion on line 684 checks the - assert_eq!(payment_event_1_2.msgs[0].cltv_expiry, nodes[0].best_block_info().1 + 1 + excess_final_cltv_delta_opt.unwrap() as u32);
+ assert_eq!(payment_event_1_2.msgs[0].cltv_expiry, nodes[0].best_block_info().1 + 1 + insufficient_final_cltv_delta_opt.unwrap() as u32); Committable suggestion
Suggested change
|
||||||
nodes[2].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event_1_2.msgs[0]); | ||||||
check_added_monitors!(nodes[2], 0); | ||||||
do_commitment_signed_dance(&nodes[2], &nodes[1], &payment_event_1_2.commitment_msg, true, true); | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -202,15 +202,16 @@ pub struct BlindedForward { | |
/// onion payload if we're the introduction node. Useful for calculating the next hop's | ||
/// [`msgs::UpdateAddHTLC::blinding_point`]. | ||
pub inbound_blinding_point: PublicKey, | ||
// Another field will be added here when we support forwarding as a non-intro node. | ||
/// If needed, this determines how this HTLC should be failed backwards, based on whether we are | ||
/// the introduction node. | ||
pub failure: BlindedFailure, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm... maybe this should be named There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, it should only be used in a failure context. Could you point me to where you're seeing that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By "used" I meant more that it is used in this struct even though described "if needed". I suppose the struct name is fine given how it is actually used, but the variable name |
||
} | ||
|
||
impl PendingHTLCRouting { | ||
// Used to override the onion failure code and data if the HTLC is blinded. | ||
fn blinded_failure(&self) -> Option<BlindedFailure> { | ||
// TODO: needs update when we support forwarding blinded HTLCs as non-intro node | ||
match self { | ||
Self::Forward { blinded: Some(_), .. } => Some(BlindedFailure::FromIntroductionNode), | ||
Self::Forward { blinded: Some(BlindedForward { failure, .. }), .. } => Some(*failure), | ||
Self::Receive { requires_blinded_error: true, .. } => Some(BlindedFailure::FromBlindedNode), | ||
_ => None, | ||
} | ||
|
@@ -305,10 +306,15 @@ pub(super) enum HTLCForwardInfo { | |
}, | ||
} | ||
|
||
// Used for failing blinded HTLCs backwards correctly. | ||
/// Whether this blinded HTLC is being failed backwards by the introduction node or a blinded node, | ||
/// which determines the failure message that should be used. | ||
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] | ||
enum BlindedFailure { | ||
pub enum BlindedFailure { | ||
/// This HTLC is being failed backwards by the introduction node, and thus should be failed with | ||
/// [`msgs::UpdateFailHTLC`] and error code `0x8000|0x4000|24`. | ||
FromIntroductionNode, | ||
/// This HTLC is being failed backwards by a blinded node within the path, and thus should be | ||
/// failed with [`msgs::UpdateFailMalformedHTLC`] and error code `0x8000|0x4000|24`. | ||
FromBlindedNode, | ||
} | ||
|
||
|
@@ -3024,8 +3030,9 @@ where | |
|
||
let is_intro_node_forward = match next_hop { | ||
onion_utils::Hop::Forward { | ||
// TODO: update this when we support blinded forwarding as non-intro node | ||
next_hop_data: msgs::InboundOnionPayload::BlindedForward { .. }, .. | ||
next_hop_data: msgs::InboundOnionPayload::BlindedForward { | ||
intro_node_blinding_point: Some(_), .. | ||
}, .. | ||
} => true, | ||
_ => false, | ||
}; | ||
|
@@ -4370,7 +4377,7 @@ where | |
incoming_packet_shared_secret: incoming_shared_secret, | ||
// Phantom payments are only PendingHTLCRouting::Receive. | ||
phantom_shared_secret: None, | ||
blinded_failure: blinded.map(|_| BlindedFailure::FromIntroductionNode), | ||
blinded_failure: blinded.map(|b| b.failure), | ||
}); | ||
let next_blinding_point = blinded.and_then(|b| { | ||
let encrypted_tlvs_ss = self.node_signer.ecdh( | ||
|
@@ -9350,6 +9357,7 @@ pub fn provided_init_features(config: &UserConfig) -> InitFeatures { | |
features.set_channel_type_optional(); | ||
features.set_scid_privacy_optional(); | ||
features.set_zero_conf_optional(); | ||
features.set_route_blinding_optional(); | ||
if config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx { | ||
features.set_anchors_zero_fee_htlc_tx_optional(); | ||
} | ||
|
@@ -9495,6 +9503,7 @@ impl_writeable_tlv_based!(PhantomRouteHints, { | |
|
||
impl_writeable_tlv_based!(BlindedForward, { | ||
(0, inbound_blinding_point, required), | ||
(1, failure, (default_value, BlindedFailure::FromIntroductionNode)), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't we previously only support forwarding as an intermediate node, so shouldn't the default here be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, no we previously only supported intro node forwarding. |
||
}); | ||
|
||
impl_writeable_tlv_based_enum!(PendingHTLCRouting, | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -188,11 +188,10 @@ pub(super) fn build_onion_payloads(path: &Path, total_msat: u64, mut recipient_o | |||||
for (i, blinded_hop) in hops.iter().enumerate() { | ||||||
if i == hops.len() - 1 { | ||||||
cur_value_msat += final_value_msat; | ||||||
cur_cltv += excess_final_cltv_expiry_delta; | ||||||
res.push(msgs::OutboundOnionPayload::BlindedReceive { | ||||||
sender_intended_htlc_amt_msat: *final_value_msat, | ||||||
total_msat, | ||||||
cltv_expiry_height: cltv, | ||||||
cltv_expiry_height: cur_cltv + excess_final_cltv_expiry_delta, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The To address this, verify that - cltv_expiry_height: cur_cltv,
+ cltv_expiry_height: cur_cltv + excess_final_cltv_expiry_delta, Committable suggestion
Suggested change
|
||||||
encrypted_tlvs: blinded_hop.encrypted_payload.clone(), | ||||||
intro_node_blinding_point: blinding_point.take(), | ||||||
}); | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable
excess_final_cltv_delta_opt
is used to set a CLTV delta that is too low, but the name does not clearly reflect its purpose. Consider renaming it to more accurately describe its function, such asinsufficient_final_cltv_delta_opt
.Committable suggestion