Rename ChannelClosed to ChannelFailure

A NetworkUpdate indicating ChannelClosed actually corresponds to a
channel failure as described in BOLT 4:

0x2000 (NODE): node failure (otherwise channel)

Rename the enum variant to ChannelFailure and rename NetworkGraph
methods close_channel_from_update and fail_node to channel_failed and
node_failed, respectively.
This commit is contained in:
Jeffrey Czyz 2021-11-05 12:55:25 -05:00
parent dcffefae3d
commit 3b3a4ba0a6
No known key found for this signature in database
GPG key ID: 3A4E08275D5E96D2
5 changed files with 28 additions and 28 deletions

View file

@ -196,7 +196,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
},
4 => {
let short_channel_id = slice_to_be64(get_slice!(8));
net_graph.close_channel_from_update(short_channel_id, false);
net_graph.channel_failed(short_channel_id, false);
},
_ if node_pks.is_empty() => {},
_ => {

View file

@ -1512,7 +1512,7 @@ macro_rules! expect_payment_failed_conditions {
}
assert_eq!(msg.contents.flags & 2, 0);
},
&Some($crate::routing::network_graph::NetworkUpdate::ChannelClosed { short_channel_id, is_permanent }) if chan_closed => {
&Some($crate::routing::network_graph::NetworkUpdate::ChannelFailure { short_channel_id, is_permanent }) if chan_closed => {
if let Some(scid) = $conditions.expected_blamed_scid {
assert_eq!(short_channel_id, scid);
}

View file

@ -177,8 +177,8 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:
panic!("channel_update not found!");
}
},
&NetworkUpdate::ChannelClosed { ref short_channel_id, ref is_permanent } => {
if let NetworkUpdate::ChannelClosed { short_channel_id: ref expected_short_channel_id, is_permanent: ref expected_is_permanent } = expected_channel_update.unwrap() {
&NetworkUpdate::ChannelFailure { ref short_channel_id, ref is_permanent } => {
if let NetworkUpdate::ChannelFailure { short_channel_id: ref expected_short_channel_id, is_permanent: ref expected_is_permanent } = expected_channel_update.unwrap() {
assert!(*short_channel_id == *expected_short_channel_id);
assert!(*is_permanent == *expected_is_permanent);
} else {
@ -283,7 +283,7 @@ fn test_fee_failures() {
let short_channel_id = channels[0].0.contents.short_channel_id;
run_onion_failure_test("fee_insufficient", 0, &nodes, &route, &payment_hash, &payment_secret, |msg| {
msg.amount_msat -= 1;
}, || {}, true, Some(UPDATE|12), Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent: true}), Some(short_channel_id));
}, || {}, true, Some(UPDATE|12), Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent: true}), Some(short_channel_id));
// In an earlier version, we spuriously failed to forward payments if the expected feerate
// changed between the channel open and the payment.
@ -343,7 +343,7 @@ fn test_onion_failure() {
// describing a length-1 TLV payload, which is obviously bogus.
new_payloads[0].data[0] = 1;
msg.onion_routing_packet = onion_utils::construct_onion_packet_bogus_hopdata(new_payloads, onion_keys, [0; 32], &payment_hash);
}, ||{}, true, Some(PERM|22), Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent: true}), Some(short_channel_id));
}, ||{}, true, Some(PERM|22), Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent: true}), Some(short_channel_id));
// final node failure
let short_channel_id = channels[1].0.contents.short_channel_id;
@ -360,7 +360,7 @@ fn test_onion_failure() {
// length-1 TLV payload, which is obviously bogus.
new_payloads[1].data[0] = 1;
msg.onion_routing_packet = onion_utils::construct_onion_packet_bogus_hopdata(new_payloads, onion_keys, [0; 32], &payment_hash);
}, ||{}, false, Some(PERM|22), Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent: true}), Some(short_channel_id));
}, ||{}, false, Some(PERM|22), Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent: true}), Some(short_channel_id));
// the following three with run_onion_failure_test_with_fail_intercept() test only the origin node
// receiving simulated fail messages
@ -471,7 +471,7 @@ fn test_onion_failure() {
let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap();
msg.reason = onion_utils::build_first_hop_failure_packet(onion_keys[0].shared_secret.as_ref(), PERM|8, &[0;0]);
// short_channel_id from the processing node
}, ||{}, true, Some(PERM|8), Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent: true}), Some(short_channel_id));
}, ||{}, true, Some(PERM|8), Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent: true}), Some(short_channel_id));
let short_channel_id = channels[1].0.contents.short_channel_id;
run_onion_failure_test_with_fail_intercept("required_channel_feature_missing", 100, &nodes, &route, &payment_hash, &payment_secret, |msg| {
@ -481,13 +481,13 @@ fn test_onion_failure() {
let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route.paths[0], &session_priv).unwrap();
msg.reason = onion_utils::build_first_hop_failure_packet(onion_keys[0].shared_secret.as_ref(), PERM|9, &[0;0]);
// short_channel_id from the processing node
}, ||{}, true, Some(PERM|9), Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent: true}), Some(short_channel_id));
}, ||{}, true, Some(PERM|9), Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent: true}), Some(short_channel_id));
let mut bogus_route = route.clone();
bogus_route.paths[0][1].short_channel_id -= 1;
let short_channel_id = bogus_route.paths[0][1].short_channel_id;
run_onion_failure_test("unknown_next_peer", 0, &nodes, &bogus_route, &payment_hash, &payment_secret, |_| {}, ||{}, true, Some(PERM|10),
Some(NetworkUpdate::ChannelClosed{short_channel_id, is_permanent:true}), Some(short_channel_id));
Some(NetworkUpdate::ChannelFailure{short_channel_id, is_permanent:true}), Some(short_channel_id));
let short_channel_id = channels[1].0.contents.short_channel_id;
let amt_to_forward = nodes[1].node.channel_state.lock().unwrap().by_id.get(&channels[1].2).unwrap().get_counterparty_htlc_minimum_msat() - 1;
@ -511,13 +511,13 @@ fn test_onion_failure() {
let short_channel_id = channels[0].0.contents.short_channel_id;
run_onion_failure_test("fee_insufficient", 0, &nodes, &route, &payment_hash, &payment_secret, |msg| {
msg.amount_msat -= 1;
}, || {}, true, Some(UPDATE|12), Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent: true}), Some(short_channel_id));
}, || {}, true, Some(UPDATE|12), Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent: true}), Some(short_channel_id));
let short_channel_id = channels[0].0.contents.short_channel_id;
run_onion_failure_test("incorrect_cltv_expiry", 0, &nodes, &route, &payment_hash, &payment_secret, |msg| {
// need to violate: cltv_expiry - cltv_expiry_delta >= outgoing_cltv_value
msg.cltv_expiry -= 1;
}, || {}, true, Some(UPDATE|13), Some(NetworkUpdate::ChannelClosed { short_channel_id, is_permanent: true}), Some(short_channel_id));
}, || {}, true, Some(UPDATE|13), Some(NetworkUpdate::ChannelFailure { short_channel_id, is_permanent: true}), Some(short_channel_id));
let short_channel_id = channels[1].0.contents.short_channel_id;
run_onion_failure_test("expiry_too_soon", 0, &nodes, &route, &payment_hash, &payment_secret, |msg| {

View file

@ -395,7 +395,7 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
}
else if error_code & PERM == PERM {
if !payment_failed {
network_update = Some(NetworkUpdate::ChannelClosed {
network_update = Some(NetworkUpdate::ChannelFailure {
short_channel_id: failing_route_hop.short_channel_id,
is_permanent: true,
});
@ -440,7 +440,7 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing, L: Deref>(secp_ctx: &
if is_chan_update_invalid {
// This probably indicates the node which forwarded
// to the node in question corrupted something.
network_update = Some(NetworkUpdate::ChannelClosed {
network_update = Some(NetworkUpdate::ChannelFailure {
short_channel_id: route_hop.short_channel_id,
is_permanent: true,
});

View file

@ -162,17 +162,17 @@ pub enum NetworkUpdate {
/// The update to apply via [`NetworkGraph::update_channel`].
msg: ChannelUpdate,
},
/// An error indicating only that a channel has been closed, which should be applied via
/// [`NetworkGraph::close_channel_from_update`].
ChannelClosed {
/// An error indicating that a channel failed to route a payment, which should be applied via
/// [`NetworkGraph::channel_failed`].
ChannelFailure {
/// The short channel id of the closed channel.
short_channel_id: u64,
/// Whether the channel should be permanently removed or temporarily disabled until a new
/// `channel_update` message is received.
is_permanent: bool,
},
/// An error indicating only that a node has failed, which should be applied via
/// [`NetworkGraph::fail_node`].
/// An error indicating that a node failed to route a payment, which should be applied via
/// [`NetworkGraph::node_failed`].
NodeFailure {
/// The node id of the failed node.
node_id: PublicKey,
@ -186,7 +186,7 @@ impl_writeable_tlv_based_enum_upgradable!(NetworkUpdate,
(0, ChannelUpdateMessage) => {
(0, msg, required),
},
(2, ChannelClosed) => {
(2, ChannelFailure) => {
(0, short_channel_id, required),
(2, is_permanent, required),
},
@ -282,15 +282,15 @@ where C::Target: chain::Access, L::Target: Logger
log_debug!(self.logger, "Updating channel with channel_update from a payment failure. Channel {} is {}.", short_channel_id, status);
let _ = self.network_graph.update_channel(msg, &self.secp_ctx);
},
NetworkUpdate::ChannelClosed { short_channel_id, is_permanent } => {
NetworkUpdate::ChannelFailure { short_channel_id, is_permanent } => {
let action = if is_permanent { "Removing" } else { "Disabling" };
log_debug!(self.logger, "{} channel graph entry for {} due to a payment failure.", action, short_channel_id);
self.network_graph.close_channel_from_update(short_channel_id, is_permanent);
self.network_graph.channel_failed(short_channel_id, is_permanent);
},
NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
let action = if is_permanent { "Removing" } else { "Disabling" };
log_debug!(self.logger, "{} node graph entry for {} due to a payment failure.", action, node_id);
self.network_graph.fail_node(node_id, is_permanent);
self.network_graph.node_failed(node_id, is_permanent);
},
}
}
@ -1328,11 +1328,11 @@ impl NetworkGraph {
self.add_channel_between_nodes(msg.short_channel_id, chan_info, utxo_value)
}
/// Close a channel if a corresponding HTLC fail was sent.
/// Marks a channel in the graph as failed if a corresponding HTLC fail was sent.
/// If permanent, removes a channel from the local storage.
/// May cause the removal of nodes too, if this was their last channel.
/// If not permanent, makes channels unavailable for routing.
pub fn close_channel_from_update(&self, short_channel_id: u64, is_permanent: bool) {
pub fn channel_failed(&self, short_channel_id: u64, is_permanent: bool) {
let mut channels = self.channels.write().unwrap();
if is_permanent {
if let Some(chan) = channels.remove(&short_channel_id) {
@ -1352,7 +1352,7 @@ impl NetworkGraph {
}
/// Marks a node in the graph as failed.
pub fn fail_node(&self, _node_id: &PublicKey, is_permanent: bool) {
pub fn node_failed(&self, _node_id: &PublicKey, is_permanent: bool) {
if is_permanent {
// TODO: Wholly remove the node
} else {
@ -2120,7 +2120,7 @@ mod tests {
rejected_by_dest: false,
all_paths_failed: true,
path: vec![],
network_update: Some(NetworkUpdate::ChannelClosed {
network_update: Some(NetworkUpdate::ChannelFailure {
short_channel_id,
is_permanent: false,
}),
@ -2145,7 +2145,7 @@ mod tests {
rejected_by_dest: false,
all_paths_failed: true,
path: vec![],
network_update: Some(NetworkUpdate::ChannelClosed {
network_update: Some(NetworkUpdate::ChannelFailure {
short_channel_id,
is_permanent: true,
}),