Skip to content

Commit 67d45df

Browse files
committed
Fix process_onion_failure
- blockify error log macros - return node failure macros - make error_code branching tighter - fill more error logs - log onion hash with BADONION bit - fix bug reporting wrong short_channel_id for ChannleClosed - handle amount_below_minimum return error code for debugging/testing
1 parent 02f805a commit 67d45df

File tree

1 file changed

+125
-85
lines changed

1 file changed

+125
-85
lines changed

src/ln/channelmanager.rs

Lines changed: 125 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use util::sha2::Sha256;
3434
use util::ser::{Readable, ReadableArgs, Writeable, Writer};
3535
use util::chacha20poly1305rfc::ChaCha20;
3636
use util::logger::Logger;
37+
use util::macro_logger::DebugBytes;
3738
use util::errors::APIError;
3839

3940
use crypto;
@@ -2020,26 +2021,41 @@ impl ChannelManager {
20202021

20212022
// Process failure we got back from upstream on a payment we sent. Returns update and a boolean
20222023
// indicating that the payment itself failed
2023-
fn process_onion_failure(&self, htlc_source: &HTLCSource, mut packet_decrypted: Vec<u8>) -> (Option<msgs::HTLCFailChannelUpdate>, bool) {
2024+
fn process_onion_failure(&self, htlc_source: &HTLCSource, mut packet_decrypted: Vec<u8>) -> (Option<msgs::HTLCFailChannelUpdate>, bool, Option<u16>) {
20242025
if let &HTLCSource::OutboundRoute { ref route, ref session_priv, ref first_hop_htlc_msat } = htlc_source {
20252026
macro_rules! onion_failure_log {
2026-
( $error_code_textual: expr, $error_code: expr, $reported_name: expr, $reported_value: expr ) => {
2027-
log_trace!(self, "{}({:#x}) {}({})", $error_code_textual, $error_code, $reported_name, $reported_value);
2028-
};
2029-
( $error_code_textual: expr, $error_code: expr ) => {
2030-
log_trace!(self, "{}({})", $error_code_textual, $error_code);
2031-
};
2027+
( $error_code_textual: expr, $error_code: expr, $reported_name: expr, $reported_value: expr ) => {{
2028+
log_info!(self, "{}({:#x}) {}({})", $error_code_textual, $error_code, $reported_name, $reported_value);
2029+
}};
2030+
( $error_code_textual: expr, $error_code: expr ) => {{
2031+
log_info!(self, "{}({:#x})", $error_code_textual, $error_code);
2032+
}};
2033+
}
2034+
2035+
// when a node has sent invalid code or malformed failure msg
2036+
macro_rules! return_node_fail_update {
2037+
($res:ident, $node_id:expr, $retry:expr) => {{
2038+
$res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure {
2039+
node_id: $node_id,
2040+
is_permanent: true,
2041+
}), $retry));
2042+
return;
2043+
}}
20322044
}
20332045

20342046
const BADONION: u16 = 0x8000;
20352047
const PERM: u16 = 0x4000;
2048+
const NODE: u16 = 0x2000;
20362049
const UPDATE: u16 = 0x1000;
20372050

20382051
let mut res = None;
20392052
let mut htlc_msat = *first_hop_htlc_msat;
2053+
let mut error_code_ret = None;
2054+
let mut next_route_hop_ix = 0;
20402055

20412056
// Handle packed channel/node updates for passing back for the route handler
20422057
Self::construct_onion_keys_callback(&self.secp_ctx, route, session_priv, |shared_secret, _, _, route_hop| {
2058+
next_route_hop_ix += 1;
20432059
if res.is_some() { return; }
20442060

20452061
let incoming_htlc_msat = htlc_msat;
@@ -2067,52 +2083,58 @@ impl ChannelManager {
20672083
if err_packet.failuremsg.len() < 2 {
20682084
// Useless packet that we can't use but it passed HMAC, so it
20692085
// definitely came from the peer in question
2070-
res = Some((None, !is_from_final_node));
2086+
return_node_fail_update!(res, route_hop.pubkey, !is_from_final_node);
20712087
} else {
20722088
let error_code = byte_utils::slice_to_be16(&err_packet.failuremsg[0..2]);
2073-
2074-
match error_code & 0xff {
2075-
1|2|3 => {
2076-
// either from an intermediate or final node
2077-
// invalid_realm(PERM|1),
2078-
// temporary_node_failure(NODE|2)
2079-
// permanent_node_failure(PERM|NODE|2)
2080-
// required_node_feature_mssing(PERM|NODE|3)
2089+
error_code_ret = Some(error_code);
2090+
2091+
// eiter from intermediate or final node
2092+
if error_code&0xff == 1 || error_code&0xff == 2 || error_code&0xff == 3 {
2093+
match error_code {
2094+
_c if _c == PERM|1 => onion_failure_log!("invalid_realm", error_code),
2095+
_c if _c == NODE|2 => onion_failure_log!("temporary_node_failure", error_code),
2096+
_c if _c == PERM|NODE|2 => onion_failure_log!("permanent_node_failure", error_code),
2097+
_c if _c == PERM|NODE|3 => onion_failure_log!("required_node_feature_mssing", error_code),
2098+
_ => return_node_fail_update!(res, route_hop.pubkey, !is_from_final_node),
2099+
}
2100+
// node returning invalid_realm is removed from network_map,
2101+
// although NODE flag is not set, TODO: or remove channel only?
2102+
// retry payment when removed node is not a final node
2103+
let is_permanent = error_code & PERM == PERM;
2104+
if error_code & 0xff00 & NODE == NODE {
20812105
res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure {
20822106
node_id: route_hop.pubkey,
2083-
is_permanent: error_code & PERM == PERM,
2107+
is_permanent,
20842108
}), !(error_code & PERM == PERM && is_from_final_node)));
2085-
// node returning invalid_realm is removed from network_map,
2086-
// although NODE flag is not set, TODO: or remove channel only?
2087-
// retry payment when removed node is not a final node
2088-
return;
2089-
},
2090-
_ => {}
2109+
} else {
2110+
res = Some((Some(msgs::HTLCFailChannelUpdate::ChannelClosed {
2111+
// failing incoming channel to the erring node
2112+
short_channel_id: route_hop.short_channel_id,
2113+
is_permanent,
2114+
}), !(error_code & PERM == PERM && is_from_final_node)));
2115+
}
2116+
return;
20912117
}
20922118

20932119
if is_from_final_node {
20942120
let payment_retryable = match error_code {
2095-
c if c == PERM|15 => false, // unknown_payment_hash
2096-
c if c == PERM|16 => false, // incorrect_payment_amount
2097-
17 => true, // final_expiry_too_soon
2098-
18 if err_packet.failuremsg.len() == 6 => { // final_incorrect_cltv_expiry
2099-
let _reported_cltv_expiry = byte_utils::slice_to_be32(&err_packet.failuremsg[2..2+4]);
2121+
_c if _c == PERM|15 => { onion_failure_log!("unknwon_payment_hash", error_code); false },
2122+
_c if _c == PERM|16 => { onion_failure_log!("incorrect_payment_amount", error_code); false },
2123+
17 => { onion_failure_log!("final_expiry_too_soon", error_code); true },
2124+
18 if err_packet.failuremsg.len() == 6 => {
2125+
onion_failure_log!("final_incorrect_cltv_expiry", error_code, "cltv_expiry", byte_utils::slice_to_be32(&err_packet.failuremsg[2..2+4]));
21002126
true
21012127
},
2102-
19 if err_packet.failuremsg.len() == 10 => { // final_incorrect_htlc_amount
2103-
let _reported_incoming_htlc_msat = byte_utils::slice_to_be64(&err_packet.failuremsg[2..2+8]);
2128+
19 if err_packet.failuremsg.len() == 10 => {
2129+
onion_failure_log!("final_incorrect_htlc_amount", error_code, "incoming_htlc_msat", byte_utils::slice_to_be64(&err_packet.failuremsg[2..2+8]));
21042130
true
21052131
},
21062132
_ => {
21072133
// A final node has sent us either an invalid code or an error_code that
21082134
// MUST be sent from the processing node, or the formmat of failuremsg
21092135
// does not coform to the spec.
21102136
// Remove it from the network map and don't may retry payment
2111-
res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure {
2112-
node_id: route_hop.pubkey,
2113-
is_permanent: true,
2114-
}), false));
2115-
return;
2137+
return_node_fail_update!(res, route_hop.pubkey, false);
21162138
}
21172139
};
21182140
res = Some((None, payment_retryable));
@@ -2121,63 +2143,82 @@ impl ChannelManager {
21212143

21222144
// now, error_code should be only from the intermediate nodes
21232145
match error_code {
2124-
_c if error_code & PERM == PERM => {
2146+
_c if _c & 0xff00 == PERM|BADONION => {
2147+
if err_packet.failuremsg.len() == 2 + 32 {
2148+
let ref onion_hash = DebugBytes(&err_packet.failuremsg[2..2+32]);
2149+
match error_code & 0xff {
2150+
4 => onion_failure_log!("invalid_onion_version", error_code, "sha256_of_onion", onion_hash),
2151+
5 => onion_failure_log!("invalid_onion_hmac", error_code, "sha256_of_onion", onion_hash),
2152+
6 => onion_failure_log!("invalid_onion_key", error_code, "sha256_of_onion", onion_hash),
2153+
_ => return_node_fail_update!(res, route_hop.pubkey, true),
2154+
}
2155+
} else { return_node_fail_update!(res, route_hop.pubkey, true); }
2156+
21252157
res = Some((Some(msgs::HTLCFailChannelUpdate::ChannelClosed {
21262158
short_channel_id: route_hop.short_channel_id,
21272159
is_permanent: true,
2128-
}), false));
2160+
}), true));
2161+
return;
21292162
},
2130-
_c if error_code & UPDATE == UPDATE => {
2131-
let offset = match error_code {
2132-
c if c == UPDATE|7 => 0, // temporary_channel_failure
2133-
c if c == UPDATE|11 => 8, // amount_below_minimum
2134-
c if c == UPDATE|12 => 8, // fee_insufficient
2135-
c if c == UPDATE|13 => 4, // incorrect_cltv_expiry
2136-
c if c == UPDATE|14 => 0, // expiry_too_soon
2137-
c if c == UPDATE|20 => 2, // channel_disabled
2138-
_ => {
2139-
// node sending unknown code
2140-
res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure {
2141-
node_id: route_hop.pubkey,
2142-
is_permanent: true,
2143-
}), false));
2144-
return;
2145-
}
2163+
_c if _c & 0xff00 == PERM => {
2164+
match error_code & 0xff {
2165+
8 => onion_failure_log!("permanent_channel_failure", error_code),
2166+
9 => onion_failure_log!("required_channel_feature_missing", error_code),
2167+
10 => onion_failure_log!("unknown_next_peer", error_code),
2168+
_ => return_node_fail_update!(res, route_hop.pubkey, true),
2169+
}
2170+
assert!(next_route_hop_ix < route.hops.len());
2171+
res = Some((Some(msgs::HTLCFailChannelUpdate::ChannelClosed {
2172+
short_channel_id: route.hops[next_route_hop_ix].short_channel_id,
2173+
is_permanent: true,
2174+
}), true));
2175+
return;
2176+
},
2177+
_c if _c & 0xff00 == UPDATE => {
2178+
let offset = match error_code & 0xff{
2179+
7 => 0, // temporary_channel_failure
2180+
11 => 8, // amount_below_minimum
2181+
12 => 8, // fee_insufficient
2182+
13 => 4, // incorrect_cltv_expiry
2183+
14 => 0, // expiry_too_soon
2184+
20 => 2, // channel_disabled
2185+
_ => return_node_fail_update!(res, route_hop.pubkey, true),
21462186
};
2147-
21482187
if err_packet.failuremsg.len() >= offset + 2 {
21492188
let update_len = byte_utils::slice_to_be16(&err_packet.failuremsg[offset+2..offset+4]) as usize;
21502189
if err_packet.failuremsg.len() >= offset + 4 + update_len {
21512190
if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&err_packet.failuremsg[offset + 4..offset + 4 + update_len])) {
2191+
21522192
// if channel_update should NOT have caused the failure:
21532193
// MAY treat the channel_update as invalid.
2154-
let is_chan_update_invalid = match error_code {
2155-
c if c == UPDATE|7 => { // temporary_channel_failure
2194+
let is_chan_update_invalid = match error_code & 0xff {
2195+
7 => {
2196+
onion_failure_log!("temporary_channel_failure", error_code);
21562197
false
21572198
},
2158-
c if c == UPDATE|11 => { // amount_below_minimum
2159-
let reported_htlc_msat = byte_utils::slice_to_be64(&err_packet.failuremsg[2..2+8]);
2160-
onion_failure_log!("amount_below_minimum", UPDATE|11, "htlc_msat", reported_htlc_msat);
2161-
incoming_htlc_msat > chan_update.contents.htlc_minimum_msat
2199+
11 => {
2200+
onion_failure_log!("amount_below_minimum", error_code, "htlc_msat", byte_utils::slice_to_be64(&err_packet.failuremsg[2..2+8]));
2201+
amt_to_forward > chan_update.contents.htlc_minimum_msat
21622202
},
2163-
c if c == UPDATE|12 => { // fee_insufficient
2164-
let reported_htlc_msat = byte_utils::slice_to_be64(&err_packet.failuremsg[2..2+8]);
2203+
12 => {
21652204
let new_fee = amt_to_forward.checked_mul(chan_update.contents.fee_proportional_millionths as u64).and_then(|prop_fee| { (prop_fee / 1000000).checked_add(chan_update.contents.fee_base_msat as u64) });
2166-
onion_failure_log!("fee_insufficient", UPDATE|12, "htlc_msat", reported_htlc_msat);
2205+
onion_failure_log!("fee_insufficient", error_code, "htlc_msat", byte_utils::slice_to_be64(&err_packet.failuremsg[2..2+8]));
21672206
new_fee.is_none() || incoming_htlc_msat >= new_fee.unwrap() && incoming_htlc_msat >= amt_to_forward + new_fee.unwrap()
21682207
}
2169-
c if c == UPDATE|13 => { // incorrect_cltv_expiry
2170-
let reported_cltv_expiry = byte_utils::slice_to_be32(&err_packet.failuremsg[2..2+4]);
2171-
onion_failure_log!("incorrect_cltv_expiry", UPDATE|13, "cltv_expiry", reported_cltv_expiry);
2208+
13 => {
2209+
onion_failure_log!("incorrect_cltv_expiry", error_code, "cltv_expiry", byte_utils::slice_to_be32(&err_packet.failuremsg[2..2+4]));
21722210
route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta
21732211
},
2174-
c if c == UPDATE|20 => { // channel_disabled
2175-
let reported_flags = byte_utils::slice_to_be16(&err_packet.failuremsg[2..2+2]);
2176-
onion_failure_log!("channel_disabled", UPDATE|20, "flags", reported_flags);
2177-
chan_update.contents.flags & 0x01 == 0x01
2212+
14 => { // expiry_too_soon
2213+
onion_failure_log!("expiry_too_soon", error_code);
2214+
// alwayws valid?
2215+
false
2216+
},
2217+
20 => {
2218+
onion_failure_log!("channel_disabled", error_code, "flags", byte_utils::slice_to_be16(&err_packet.failuremsg[2..2+2]));
2219+
chan_update.contents.flags & 0x0200 == 1
21782220
},
2179-
c if c == UPDATE|21 => true, // expiry_too_far
2180-
_ => { unreachable!(); },
2221+
_ => return_node_fail_update!(res, route_hop.pubkey, true),
21812222
};
21822223

21832224
let msg = if is_chan_update_invalid { None } else {
@@ -2190,29 +2231,28 @@ impl ChannelManager {
21902231
}
21912232
}
21922233
}
2234+
return_node_fail_update!(res, route_hop.pubkey, true);
21932235
},
2194-
_c if error_code & BADONION == BADONION => {
2195-
//TODO
2196-
},
2197-
14 => { // expiry_too_soon
2236+
21 => {
2237+
onion_failure_log!("expiry_too_far", error_code);
21982238
res = Some((None, true));
21992239
return;
2200-
}
2240+
},
22012241
_ => {
2202-
// node sending unknown code
2203-
res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure {
2204-
node_id: route_hop.pubkey,
2205-
is_permanent: true,
2206-
}), false));
2207-
return;
2242+
return_node_fail_update!(res, route_hop.pubkey, true);
22082243
}
22092244
}
22102245
}
22112246
}
22122247
}
22132248
}).expect("Route that we sent via spontaneously grew invalid keys in the middle of it?");
2214-
res.unwrap_or((None, true))
2215-
} else { ((None, true)) }
2249+
//res.unwrap_or((None, true))
2250+
let (channel_update, payment_retryable) = res.expect("should have been set!");
2251+
(channel_update, payment_retryable, error_code_ret)
2252+
} else {
2253+
// htlc_source should have been matched in the callsite
2254+
unreachable!();
2255+
}
22162256
}
22172257

22182258
fn internal_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<(), MsgHandleErrInternal> {

0 commit comments

Comments
 (0)