X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=src%2Fln%2Fchannelmanager.rs;h=5216882d4a90126be786776f60781443eb2b31f2;hb=8783a748bb952168623cb674f8ea3230d7c94b25;hp=f349afc18d07339843ca9a2eb5ccbd159f9de604;hpb=7a483e597c9079a280b1ea31d9762d89300097ce;p=rust-lightning diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index f349afc1..5216882d 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -22,7 +22,7 @@ use secp256k1; use chain::chaininterface::{BroadcasterInterface,ChainListener,ChainWatchInterface,FeeEstimator}; use chain::transaction::OutPoint; use ln::channel::{Channel, ChannelError}; -use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS}; +use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, HTLC_FAIL_ANTI_REORG_DELAY}; use ln::router::{Route,RouteHop}; use ln::msgs; use ln::msgs::{ChannelMessageHandler, DecodeError, HandleError}; @@ -34,6 +34,7 @@ use util::ser::{Readable, ReadableArgs, Writeable, Writer}; use util::chacha20poly1305rfc::ChaCha20; use util::logger::Logger; use util::errors::APIError; +use util::errors; use crypto; use crypto::mac::{Mac,MacResult}; @@ -341,16 +342,17 @@ pub struct ChannelManager { /// ie the node we forwarded the payment on to should always have enough room to reliably time out /// the HTLC via a full update_fail_htlc/commitment_signed dance before we hit the /// CLTV_CLAIM_BUFFER point (we static assert that its at least 3 blocks more). -const CLTV_EXPIRY_DELTA: u16 = 6 * 24 * 2; //TODO? +const CLTV_EXPIRY_DELTA: u16 = 6 * 12; //TODO? const CLTV_FAR_FAR_AWAY: u32 = 6 * 24 * 7; //TODO? -// Check that our CLTV_EXPIRY is at least CLTV_CLAIM_BUFFER + 2*HTLC_FAIL_TIMEOUT_BLOCKS, ie that -// if the next-hop peer fails the HTLC within HTLC_FAIL_TIMEOUT_BLOCKS then we'll still have -// HTLC_FAIL_TIMEOUT_BLOCKS left to fail it backwards ourselves before hitting the -// CLTV_CLAIM_BUFFER point and failing the channel on-chain to time out the HTLC. +// Check that our CLTV_EXPIRY is at least CLTV_CLAIM_BUFFER + 2*HTLC_FAIL_TIMEOUT_BLOCKS + +// HTLC_FAIL_ANTI_REORG_DELAY, ie that if the next-hop peer fails the HTLC within +// HTLC_FAIL_TIMEOUT_BLOCKS then we'll still have HTLC_FAIL_TIMEOUT_BLOCKS left to fail it +// backwards ourselves before hitting the CLTV_CLAIM_BUFFER point and failing the channel +// on-chain to time out the HTLC. #[deny(const_err)] #[allow(dead_code)] -const CHECK_CLTV_EXPIRY_SANITY: u32 = CLTV_EXPIRY_DELTA as u32 - 2*HTLC_FAIL_TIMEOUT_BLOCKS - CLTV_CLAIM_BUFFER; +const CHECK_CLTV_EXPIRY_SANITY: u32 = CLTV_EXPIRY_DELTA as u32 - 2*HTLC_FAIL_TIMEOUT_BLOCKS - CLTV_CLAIM_BUFFER - HTLC_FAIL_ANTI_REORG_DELAY; // Check for ability of an attacker to make us fail on-chain by delaying inbound claim. See // ChannelMontior::would_broadcast_at_height for a description of why this is needed. @@ -423,6 +425,7 @@ macro_rules! break_chan_entry { break Err(MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore(msg), $entry.key().clone())) }, Err(ChannelError::Close(msg)) => { + log_trace!($self, "Closing channel {} due to Close-required error: {}", log_bytes!($entry.key()[..]), msg); let (channel_id, mut chan) = $entry.remove_entry(); if let Some(short_id) = chan.get_short_channel_id() { $channel_state.short_to_id.remove(&short_id); @@ -441,6 +444,7 @@ macro_rules! try_chan_entry { return Err(MsgHandleErrInternal::from_chan_no_close(ChannelError::Ignore(msg), $entry.key().clone())) }, Err(ChannelError::Close(msg)) => { + log_trace!($self, "Closing channel {} due to Close-required error: {}", log_bytes!($entry.key()[..]), msg); let (channel_id, mut chan) = $entry.remove_entry(); if let Some(short_id) = chan.get_short_channel_id() { $channel_state.short_to_id.remove(&short_id); @@ -659,8 +663,7 @@ impl ChannelManager { } }; for htlc_source in failed_htlcs.drain(..) { - // unknown_next_peer...I dunno who that is anymore.... - self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() }); + self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); } let chan_update = if let Some(chan) = chan_option { if let Ok(update) = self.get_channel_update(&chan) { @@ -681,9 +684,9 @@ impl ChannelManager { #[inline] fn finish_force_close_channel(&self, shutdown_res: ShutdownResult) { let (local_txn, mut failed_htlcs) = shutdown_res; + log_trace!(self, "Finishing force-closure of channel with {} transactions to broadcast and {} HTLCs to fail", local_txn.len(), failed_htlcs.len()); for htlc_source in failed_htlcs.drain(..) { - // unknown_next_peer...I dunno who that is anymore.... - self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() }); + self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); } for tx in local_txn { self.tx_broadcaster.broadcast_transaction(&tx); @@ -707,6 +710,7 @@ impl ChannelManager { return; } }; + log_trace!(self, "Force-closing channel {}", log_bytes!(channel_id[..])); self.finish_force_close_channel(chan.force_shutdown()); if let Ok(update) = self.get_channel_update(&chan) { let mut channel_state = self.channel_state.lock().unwrap(); @@ -968,26 +972,26 @@ impl ChannelManager { } fn decode_update_add_htlc_onion(&self, msg: &msgs::UpdateAddHTLC) -> (PendingHTLCStatus, MutexGuard) { - macro_rules! get_onion_hash { - () => { + macro_rules! return_malformed_err { + ($msg: expr, $err_code: expr) => { { + log_info!(self, "Failed to accept/forward incoming HTLC: {}", $msg); + let mut sha256_of_onion = [0; 32]; let mut sha = Sha256::new(); sha.input(&msg.onion_routing_packet.hop_data); - let mut onion_hash = [0; 32]; - sha.result(&mut onion_hash); - onion_hash + sha.result(&mut sha256_of_onion); + return (PendingHTLCStatus::Fail(HTLCFailureMsg::Malformed(msgs::UpdateFailMalformedHTLC { + channel_id: msg.channel_id, + htlc_id: msg.htlc_id, + sha256_of_onion, + failure_code: $err_code, + })), self.channel_state.lock().unwrap()); } } } if let Err(_) = msg.onion_routing_packet.public_key { - log_info!(self, "Failed to accept/forward incoming HTLC with invalid ephemeral pubkey"); - return (PendingHTLCStatus::Fail(HTLCFailureMsg::Malformed(msgs::UpdateFailMalformedHTLC { - channel_id: msg.channel_id, - htlc_id: msg.htlc_id, - sha256_of_onion: get_onion_hash!(), - failure_code: 0x8000 | 0x4000 | 6, - })), self.channel_state.lock().unwrap()); + return_malformed_err!("invalid ephemeral pubkey", 0x8000 | 0x4000 | 6); } let shared_secret = { @@ -997,6 +1001,23 @@ impl ChannelManager { }; let (rho, mu) = ChannelManager::gen_rho_mu_from_shared_secret(&shared_secret); + if msg.onion_routing_packet.version != 0 { + //TODO: Spec doesn't indicate if we should only hash hop_data here (and in other + //sha256_of_onion error data packets), or the entire onion_routing_packet. Either way, + //the hash doesn't really serve any purpuse - in the case of hashing all data, the + //receiving node would have to brute force to figure out which version was put in the + //packet by the node that send us the message, in the case of hashing the hop_data, the + //node knows the HMAC matched, so they already know what is there... + return_malformed_err!("Unknown onion packet version", 0x8000 | 0x4000 | 4); + } + + let mut hmac = Hmac::new(Sha256::new(), &mu); + hmac.input(&msg.onion_routing_packet.hop_data); + hmac.input(&msg.payment_hash.0[..]); + if hmac.result() != MacResult::new(&msg.onion_routing_packet.hmac) { + return_malformed_err!("HMAC Check failed", 0x8000 | 0x4000 | 5); + } + let mut channel_state = None; macro_rules! return_err { ($msg: expr, $err_code: expr, $data: expr) => { @@ -1014,23 +1035,6 @@ impl ChannelManager { } } - if msg.onion_routing_packet.version != 0 { - //TODO: Spec doesn't indicate if we should only hash hop_data here (and in other - //sha256_of_onion error data packets), or the entire onion_routing_packet. Either way, - //the hash doesn't really serve any purpuse - in the case of hashing all data, the - //receiving node would have to brute force to figure out which version was put in the - //packet by the node that send us the message, in the case of hashing the hop_data, the - //node knows the HMAC matched, so they already know what is there... - return_err!("Unknown onion packet version", 0x8000 | 0x4000 | 4, &get_onion_hash!()); - } - - let mut hmac = Hmac::new(Sha256::new(), &mu); - hmac.input(&msg.onion_routing_packet.hop_data); - hmac.input(&msg.payment_hash.0[..]); - if hmac.result() != MacResult::new(&msg.onion_routing_packet.hmac) { - return_err!("HMAC Check failed", 0x8000 | 0x4000 | 5, &get_onion_hash!()); - } - let mut chacha = ChaCha20::new(&rho, &[0u8; 8]); let next_hop_data = { let mut decoded = [0; 65]; @@ -1088,21 +1092,16 @@ impl ChannelManager { sha.input(&shared_secret); let mut res = [0u8; 32]; sha.result(&mut res); - match SecretKey::from_slice(&self.secp_ctx, &res) { - Err(_) => { - return_err!("Blinding factor is an invalid private key", 0x8000 | 0x4000 | 6, &get_onion_hash!()); - }, - Ok(key) => key - } + SecretKey::from_slice(&self.secp_ctx, &res).expect("SHA-256 is broken?") }; - if let Err(_) = new_pubkey.mul_assign(&self.secp_ctx, &blinding_factor) { - return_err!("New blinding factor is an invalid private key", 0x8000 | 0x4000 | 6, &get_onion_hash!()); - } + let public_key = if let Err(e) = new_pubkey.mul_assign(&self.secp_ctx, &blinding_factor) { + Err(e) + } else { Ok(new_pubkey) }; let outgoing_packet = msgs::OnionPacket { version: 0, - public_key: Ok(new_pubkey), + public_key, hop_data: new_packet_data, hmac: next_hop_data.hmac.clone(), }; @@ -1160,13 +1159,16 @@ impl ChannelManager { } { let mut res = Vec::with_capacity(8 + 128); - if code == 0x1000 | 11 || code == 0x1000 | 12 { - res.extend_from_slice(&byte_utils::be64_to_array(msg.amount_msat)); - } - else if code == 0x1000 | 13 { - res.extend_from_slice(&byte_utils::be32_to_array(msg.cltv_expiry)); - } if let Some(chan_update) = chan_update { + if code == 0x1000 | 11 || code == 0x1000 | 12 { + res.extend_from_slice(&byte_utils::be64_to_array(msg.amount_msat)); + } + else if code == 0x1000 | 13 { + res.extend_from_slice(&byte_utils::be32_to_array(msg.cltv_expiry)); + } + else if code == 0x1000 | 20 { + res.extend_from_slice(&byte_utils::be16_to_array(chan_update.contents.flags)); + } res.extend_from_slice(&chan_update.encode_with_len()[..]); } return_err!(err, code, &res[..]); @@ -1551,36 +1553,65 @@ impl ChannelManager { /// still-available channels. fn fail_htlc_backwards_internal(&self, mut channel_state_lock: MutexGuard, source: HTLCSource, payment_hash: &PaymentHash, onion_error: HTLCFailReason) { match source { - HTLCSource::OutboundRoute { .. } => { + HTLCSource::OutboundRoute { ref route, .. } => { + log_trace!(self, "Failing outbound payment HTLC with payment_hash {}", log_bytes!(payment_hash.0)); mem::drop(channel_state_lock); - if let &HTLCFailReason::ErrorPacket { ref err } = &onion_error { - let (channel_update, payment_retryable) = self.process_onion_failure(&source, err.data.clone()); - if let Some(update) = channel_update { - self.channel_state.lock().unwrap().pending_msg_events.push( - events::MessageSendEvent::PaymentFailureNetworkUpdate { - update, + match &onion_error { + &HTLCFailReason::ErrorPacket { ref err } => { +#[cfg(test)] + let (channel_update, payment_retryable, onion_error_code) = self.process_onion_failure(&source, err.data.clone()); +#[cfg(not(test))] + let (channel_update, payment_retryable, _) = self.process_onion_failure(&source, err.data.clone()); + // TODO: If we decided to blame ourselves (or one of our channels) in + // process_onion_failure we should close that channel as it implies our + // next-hop is needlessly blaming us! + if let Some(update) = channel_update { + self.channel_state.lock().unwrap().pending_msg_events.push( + events::MessageSendEvent::PaymentFailureNetworkUpdate { + update, + } + ); + } + self.pending_events.lock().unwrap().push( + events::Event::PaymentFailed { + payment_hash: payment_hash.clone(), + rejected_by_dest: !payment_retryable, +#[cfg(test)] + error_code: onion_error_code + } + ); + }, + &HTLCFailReason::Reason { +#[cfg(test)] + ref failure_code, + .. } => { + // we get a fail_malformed_htlc from the first hop + // TODO: We'd like to generate a PaymentFailureNetworkUpdate for temporary + // failures here, but that would be insufficient as Router::get_route + // generally ignores its view of our own channels as we provide them via + // ChannelDetails. + // TODO: For non-temporary failures, we really should be closing the + // channel here as we apparently can't relay through them anyway. + self.pending_events.lock().unwrap().push( + events::Event::PaymentFailed { + payment_hash: payment_hash.clone(), + rejected_by_dest: route.hops.len() == 1, +#[cfg(test)] + error_code: Some(*failure_code), } ); } - self.pending_events.lock().unwrap().push(events::Event::PaymentFailed { - payment_hash: payment_hash.clone(), - rejected_by_dest: !payment_retryable, - }); - } else { - //TODO: Pass this back (see GH #243) - self.pending_events.lock().unwrap().push(events::Event::PaymentFailed { - payment_hash: payment_hash.clone(), - rejected_by_dest: false, // We failed it ourselves, can't blame them - }); } }, HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret }) => { let err_packet = match onion_error { HTLCFailReason::Reason { failure_code, data } => { + log_trace!(self, "Failing HTLC with payment_hash {} backwards from us with code {}", log_bytes!(payment_hash.0), failure_code); let packet = ChannelManager::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode(); ChannelManager::encrypt_failure_packet(&incoming_packet_shared_secret, &packet) }, HTLCFailReason::ErrorPacket { err } => { + log_trace!(self, "Failing HTLC with payment_hash {} backwards with pre-built ErrorPacket", log_bytes!(payment_hash.0)); ChannelManager::encrypt_failure_packet(&incoming_packet_shared_secret, &err.data) } }; @@ -1963,8 +1994,7 @@ impl ChannelManager { } }; for htlc_source in dropped_htlcs.drain(..) { - // unknown_next_peer...I dunno who that is anymore.... - self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() }); + self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); } if let Some(chan) = chan_option { if let Ok(update) = self.get_channel_update(&chan) { @@ -2052,7 +2082,17 @@ impl ChannelManager { channel_id: msg.channel_id, htlc_id: msg.htlc_id, reason: if let Ok(update) = chan_update { - ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &update.encode_with_len()[..]) + // TODO: Note that |20 is defined as "channel FROM the processing + // node has been disabled" (emphasis mine), which seems to imply + // that we can't return |20 for an inbound channel being disabled. + // This probably needs a spec update but should definitely be + // allowed. + ChannelManager::build_first_hop_failure_packet(&incoming_shared_secret, 0x1000|20, &{ + let mut res = Vec::with_capacity(8 + 128); + res.extend_from_slice(&byte_utils::be16_to_array(update.contents.flags)); + res.extend_from_slice(&update.encode_with_len()[..]); + res + }[..]) } else { // This can only happen if the channel isn't in the fully-funded // state yet, implying our counterparty is trying to route payments @@ -2092,29 +2132,20 @@ impl ChannelManager { // Process failure we got back from upstream on a payment we sent. Returns update and a boolean // indicating that the payment itself failed - fn process_onion_failure(&self, htlc_source: &HTLCSource, mut packet_decrypted: Vec) -> (Option, bool) { + fn process_onion_failure(&self, htlc_source: &HTLCSource, mut packet_decrypted: Vec) -> (Option, bool, Option) { if let &HTLCSource::OutboundRoute { ref route, ref session_priv, ref first_hop_htlc_msat } = htlc_source { - macro_rules! onion_failure_log { - ( $error_code_textual: expr, $error_code: expr, $reported_name: expr, $reported_value: expr ) => { - log_trace!(self, "{}({:#x}) {}({})", $error_code_textual, $error_code, $reported_name, $reported_value); - }; - ( $error_code_textual: expr, $error_code: expr ) => { - log_trace!(self, "{}({})", $error_code_textual, $error_code); - }; - } - - const BADONION: u16 = 0x8000; - const PERM: u16 = 0x4000; - const UPDATE: u16 = 0x1000; let mut res = None; let mut htlc_msat = *first_hop_htlc_msat; + let mut error_code_ret = None; + let mut next_route_hop_ix = 0; + let mut is_from_final_node = false; // Handle packed channel/node updates for passing back for the route handler Self::construct_onion_keys_callback(&self.secp_ctx, route, session_priv, |shared_secret, _, _, route_hop| { + next_route_hop_ix += 1; if res.is_some() { return; } - let incoming_htlc_msat = htlc_msat; let amt_to_forward = htlc_msat - route_hop.fee_msat; htlc_msat = amt_to_forward; @@ -2126,7 +2157,7 @@ impl ChannelManager { chacha.process(&packet_decrypted, &mut decryption_tmp[..]); packet_decrypted = decryption_tmp; - let is_from_final_node = route.hops.last().unwrap().pubkey == route_hop.pubkey; + is_from_final_node = route.hops.last().unwrap().pubkey == route_hop.pubkey; if let Ok(err_packet) = msgs::DecodedOnionErrorPacket::read(&mut Cursor::new(&packet_decrypted)) { let um = ChannelManager::gen_um_from_shared_secret(&shared_secret[..]); @@ -2136,155 +2167,118 @@ impl ChannelManager { hmac.raw_result(&mut calc_tag); if crypto::util::fixed_time_eq(&calc_tag, &err_packet.hmac) { - if err_packet.failuremsg.len() < 2 { - // Useless packet that we can't use but it passed HMAC, so it - // definitely came from the peer in question - res = Some((None, !is_from_final_node)); - } else { - let error_code = byte_utils::slice_to_be16(&err_packet.failuremsg[0..2]); - - match error_code & 0xff { - 1|2|3 => { - // either from an intermediate or final node - // invalid_realm(PERM|1), - // temporary_node_failure(NODE|2) - // permanent_node_failure(PERM|NODE|2) - // required_node_feature_mssing(PERM|NODE|3) - res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure { - node_id: route_hop.pubkey, - is_permanent: error_code & PERM == PERM, - }), !(error_code & PERM == PERM && is_from_final_node))); - // node returning invalid_realm is removed from network_map, - // although NODE flag is not set, TODO: or remove channel only? - // retry payment when removed node is not a final node - return; - }, - _ => {} - } + if let Some(error_code_slice) = err_packet.failuremsg.get(0..2) { + const PERM: u16 = 0x4000; + const NODE: u16 = 0x2000; + const UPDATE: u16 = 0x1000; - if is_from_final_node { - let payment_retryable = match error_code { - c if c == PERM|15 => false, // unknown_payment_hash - c if c == PERM|16 => false, // incorrect_payment_amount - 17 => true, // final_expiry_too_soon - 18 if err_packet.failuremsg.len() == 6 => { // final_incorrect_cltv_expiry - let _reported_cltv_expiry = byte_utils::slice_to_be32(&err_packet.failuremsg[2..2+4]); - true - }, - 19 if err_packet.failuremsg.len() == 10 => { // final_incorrect_htlc_amount - let _reported_incoming_htlc_msat = byte_utils::slice_to_be64(&err_packet.failuremsg[2..2+8]); - true - }, - _ => { - // A final node has sent us either an invalid code or an error_code that - // MUST be sent from the processing node, or the formmat of failuremsg - // does not coform to the spec. - // Remove it from the network map and don't may retry payment - res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure { - node_id: route_hop.pubkey, - is_permanent: true, - }), false)); - return; - } - }; - res = Some((None, payment_retryable)); - return; - } + let error_code = byte_utils::slice_to_be16(&error_code_slice); + error_code_ret = Some(error_code); - // now, error_code should be only from the intermediate nodes - match error_code { - _c if error_code & PERM == PERM => { - res = Some((Some(msgs::HTLCFailChannelUpdate::ChannelClosed { - short_channel_id: route_hop.short_channel_id, - is_permanent: true, - }), false)); - }, - _c if error_code & UPDATE == UPDATE => { - let offset = match error_code { - c if c == UPDATE|7 => 0, // temporary_channel_failure - c if c == UPDATE|11 => 8, // amount_below_minimum - c if c == UPDATE|12 => 8, // fee_insufficient - c if c == UPDATE|13 => 4, // incorrect_cltv_expiry - c if c == UPDATE|14 => 0, // expiry_too_soon - c if c == UPDATE|20 => 2, // channel_disabled - _ => { - // node sending unknown code - res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure { - node_id: route_hop.pubkey, - is_permanent: true, - }), false)); - return; - } - }; - - if err_packet.failuremsg.len() >= offset + 2 { - let update_len = byte_utils::slice_to_be16(&err_packet.failuremsg[offset+2..offset+4]) as usize; - if err_packet.failuremsg.len() >= offset + 4 + update_len { - if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&err_packet.failuremsg[offset + 4..offset + 4 + update_len])) { - // if channel_update should NOT have caused the failure: - // MAY treat the channel_update as invalid. - let is_chan_update_invalid = match error_code { - c if c == UPDATE|7 => { // temporary_channel_failure - false - }, - c if c == UPDATE|11 => { // amount_below_minimum - let reported_htlc_msat = byte_utils::slice_to_be64(&err_packet.failuremsg[2..2+8]); - onion_failure_log!("amount_below_minimum", UPDATE|11, "htlc_msat", reported_htlc_msat); - incoming_htlc_msat > chan_update.contents.htlc_minimum_msat - }, - c if c == UPDATE|12 => { // fee_insufficient - let reported_htlc_msat = byte_utils::slice_to_be64(&err_packet.failuremsg[2..2+8]); - 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) }); - onion_failure_log!("fee_insufficient", UPDATE|12, "htlc_msat", reported_htlc_msat); - new_fee.is_none() || incoming_htlc_msat >= new_fee.unwrap() && incoming_htlc_msat >= amt_to_forward + new_fee.unwrap() - } - c if c == UPDATE|13 => { // incorrect_cltv_expiry - let reported_cltv_expiry = byte_utils::slice_to_be32(&err_packet.failuremsg[2..2+4]); - onion_failure_log!("incorrect_cltv_expiry", UPDATE|13, "cltv_expiry", reported_cltv_expiry); - route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta - }, - c if c == UPDATE|20 => { // channel_disabled - let reported_flags = byte_utils::slice_to_be16(&err_packet.failuremsg[2..2+2]); - onion_failure_log!("channel_disabled", UPDATE|20, "flags", reported_flags); - chan_update.contents.flags & 0x01 == 0x01 - }, - c if c == UPDATE|21 => true, // expiry_too_far - _ => { unreachable!(); }, - }; - - let msg = if is_chan_update_invalid { None } else { - Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage { - msg: chan_update, - }) - }; - res = Some((msg, true)); - return; - } + let (debug_field, debug_field_size) = errors::get_onion_debug_field(error_code); + + // indicate that payment parameter has failed and no need to + // update Route object + let payment_failed = (match error_code & 0xff { + 15|16|17|18|19 => true, + _ => false, + } && is_from_final_node) // PERM bit observed below even this error is from the intermediate nodes + || error_code == 21; // Special case error 21 as the Route object is bogus, TODO: Maybe fail the node if the CLTV was reasonable? + + let mut fail_channel_update = None; + + if error_code & NODE == NODE { + fail_channel_update = Some(msgs::HTLCFailChannelUpdate::NodeFailure { node_id: route_hop.pubkey, is_permanent: error_code & PERM == PERM }); + } + else if error_code & PERM == PERM { + fail_channel_update = if payment_failed {None} else {Some(msgs::HTLCFailChannelUpdate::ChannelClosed { + short_channel_id: route.hops[next_route_hop_ix - if next_route_hop_ix == route.hops.len() { 1 } else { 0 }].short_channel_id, + is_permanent: true, + })}; + } + else if error_code & UPDATE == UPDATE { + if let Some(update_len_slice) = err_packet.failuremsg.get(debug_field_size+2..debug_field_size+4) { + let update_len = byte_utils::slice_to_be16(&update_len_slice) as usize; + if let Some(update_slice) = err_packet.failuremsg.get(debug_field_size + 4..debug_field_size + 4 + update_len) { + if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&update_slice)) { + // if channel_update should NOT have caused the failure: + // MAY treat the channel_update as invalid. + let is_chan_update_invalid = match error_code & 0xff { + 7 => false, + 11 => amt_to_forward > chan_update.contents.htlc_minimum_msat, + 12 => { + 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) }); + new_fee.is_some() && route_hop.fee_msat >= new_fee.unwrap() + } + 13 => route_hop.cltv_expiry_delta as u16 >= chan_update.contents.cltv_expiry_delta, + 14 => false, // expiry_too_soon; always valid? + 20 => chan_update.contents.flags & 2 == 0, + _ => false, // unknown error code; take channel_update as valid + }; + fail_channel_update = if is_chan_update_invalid { + // This probably indicates the node which forwarded + // to the node in question corrupted something. + Some(msgs::HTLCFailChannelUpdate::ChannelClosed { + short_channel_id: route_hop.short_channel_id, + is_permanent: true, + }) + } else { + Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage { + msg: chan_update, + }) + }; } } - }, - _c if error_code & BADONION == BADONION => { - //TODO - }, - 14 => { // expiry_too_soon - res = Some((None, true)); - return; } - _ => { - // node sending unknown code - res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure { + if fail_channel_update.is_none() { + // They provided an UPDATE which was obviously bogus, not worth + // trying to relay through them anymore. + fail_channel_update = Some(msgs::HTLCFailChannelUpdate::NodeFailure { node_id: route_hop.pubkey, is_permanent: true, - }), false)); - return; + }); } + } else if !payment_failed { + // We can't understand their error messages and they failed to + // forward...they probably can't understand our forwards so its + // really not worth trying any further. + fail_channel_update = Some(msgs::HTLCFailChannelUpdate::NodeFailure { + node_id: route_hop.pubkey, + is_permanent: true, + }); + } + + // TODO: Here (and a few other places) we assume that BADONION errors + // are always "sourced" from the node previous to the one which failed + // to decode the onion. + res = Some((fail_channel_update, !(error_code & PERM == PERM && is_from_final_node))); + + let (description, title) = errors::get_onion_error_description(error_code); + if debug_field_size > 0 && err_packet.failuremsg.len() >= 4 + debug_field_size { + log_warn!(self, "Onion Error[{}({:#x}) {}({})] {}", title, error_code, debug_field, log_bytes!(&err_packet.failuremsg[4..4+debug_field_size]), description); } + else { + log_warn!(self, "Onion Error[{}({:#x})] {}", title, error_code, description); + } + } else { + // Useless packet that we can't use but it passed HMAC, so it + // definitely came from the peer in question + res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure { + node_id: route_hop.pubkey, + is_permanent: true, + }), !is_from_final_node)); } } } }).expect("Route that we sent via spontaneously grew invalid keys in the middle of it?"); - res.unwrap_or((None, true)) - } else { ((None, true)) } + if let Some((channel_update, payment_retryable)) = res { + (channel_update, payment_retryable, error_code_ret) + } else { + // only not set either packet unparseable or hmac does not match with any + // payment not retryable only when garbage is from the final node + (None, !is_from_final_node, None) + } + } else { unreachable!(); } } fn internal_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<(), MsgHandleErrInternal> { @@ -2642,9 +2636,11 @@ impl events::MessageSendEventsProvider for ChannelManager { //TODO: This behavior should be documented. for htlc_update in self.monitor.fetch_pending_htlc_updated() { if let Some(preimage) = htlc_update.payment_preimage { + log_trace!(self, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0)); self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage); } else { - self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() }); + log_trace!(self, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0)); + self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); } } } @@ -2665,9 +2661,11 @@ impl events::EventsProvider for ChannelManager { //TODO: This behavior should be documented. for htlc_update in self.monitor.fetch_pending_htlc_updated() { if let Some(preimage) = htlc_update.payment_preimage { + log_trace!(self, "Claiming HTLC with preimage {} from our monitor", log_bytes!(preimage.0)); self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage); } else { - self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() }); + log_trace!(self, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0)); + self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() }); } } } @@ -2681,6 +2679,8 @@ impl events::EventsProvider for ChannelManager { impl ChainListener for ChannelManager { fn block_connected(&self, header: &BlockHeader, height: u32, txn_matched: &[&Transaction], indexes_of_txn_matched: &[u32]) { + let header_hash = header.bitcoin_hash(); + log_trace!(self, "Block {} at height {} connected with {} txn matched", header_hash, height, txn_matched.len()); let _ = self.total_consistency_lock.read().unwrap(); let mut failed_channels = Vec::new(); { @@ -2713,6 +2713,7 @@ impl ChainListener for ChannelManager { for tx in txn_matched { for inp in tx.input.iter() { if inp.previous_output == funding_txo.into_bitcoin_outpoint() { + log_trace!(self, "Detected channel-closing tx {} spending {}:{}, closing channel {}", tx.txid(), inp.previous_output.txid, inp.previous_output.vout, log_bytes!(channel.channel_id())); if let Some(short_id) = channel.get_short_channel_id() { short_to_id.remove(&short_id); } @@ -2753,7 +2754,7 @@ impl ChainListener for ChannelManager { self.finish_force_close_channel(failure); } self.latest_block_height.store(height as usize, Ordering::Release); - *self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = header.bitcoin_hash(); + *self.last_block_hash.try_lock().expect("block_(dis)connected must not be called in parallel") = header_hash; } /// We force-close the channel without letting our counterparty participate in the shutdown @@ -3569,6 +3570,7 @@ mod tests { chain_monitor: Arc, tx_broadcaster: Arc, chan_monitor: Arc, + keys_manager: Arc, node: Arc, router: Router, node_seed: [u8; 32], @@ -3776,6 +3778,8 @@ mod tests { let as_update = match events_8[0] { MessageSendEvent::BroadcastChannelAnnouncement { ref msg, ref update_msg } => { assert!(*announcement == *msg); + assert_eq!(update_msg.contents.short_channel_id, announcement.contents.short_channel_id); + assert_eq!(update_msg.contents.short_channel_id, bs_update.contents.short_channel_id); update_msg }, _ => panic!("Unexpected event"), @@ -4272,7 +4276,7 @@ mod tests { let events = origin_node.node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentFailed { payment_hash, rejected_by_dest } => { + Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => { assert_eq!(payment_hash, our_payment_hash); assert!(rejected_by_dest); }, @@ -4289,25 +4293,25 @@ mod tests { let mut nodes = Vec::new(); let mut rng = thread_rng(); let secp_ctx = Secp256k1::new(); - let logger: Arc = Arc::new(test_utils::TestLogger::new()); let chan_count = Rc::new(RefCell::new(0)); let payment_count = Rc::new(RefCell::new(0)); - for _ in 0..node_count { + for i in 0..node_count { + let logger: Arc = Arc::new(test_utils::TestLogger::with_id(format!("node {}", i))); let feeest = Arc::new(test_utils::TestFeeEstimator { sat_per_kw: 253 }); let chain_monitor = Arc::new(chaininterface::ChainWatchInterfaceUtil::new(Network::Testnet, Arc::clone(&logger))); let tx_broadcaster = Arc::new(test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new())}); let mut seed = [0; 32]; rng.fill_bytes(&mut seed); - let keys_manager = Arc::new(keysinterface::KeysManager::new(&seed, Network::Testnet, Arc::clone(&logger))); + let keys_manager = Arc::new(test_utils::TestKeysInterface::new(&seed, Network::Testnet, Arc::clone(&logger))); let chan_monitor = Arc::new(test_utils::TestChannelMonitor::new(chain_monitor.clone(), tx_broadcaster.clone(), logger.clone())); let mut config = UserConfig::new(); config.channel_options.announced_channel = true; config.channel_limits.force_announced_channel_preference = false; let node = ChannelManager::new(Network::Testnet, feeest.clone(), chan_monitor.clone(), chain_monitor.clone(), tx_broadcaster.clone(), Arc::clone(&logger), keys_manager.clone(), config).unwrap(); let router = Router::new(PublicKey::from_secret_key(&secp_ctx, &keys_manager.get_node_secret()), chain_monitor.clone(), Arc::clone(&logger)); - nodes.push(Node { chain_monitor, tx_broadcaster, chan_monitor, node, router, node_seed: seed, + nodes.push(Node { chain_monitor, tx_broadcaster, chan_monitor, node, router, keys_manager, node_seed: seed, network_payment_count: payment_count.clone(), network_chan_count: chan_count.clone(), }); @@ -5012,15 +5016,30 @@ mod tests { let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); match events[0] { - Event::PaymentFailed { ref payment_hash, ref rejected_by_dest } => { + Event::PaymentFailed { ref payment_hash, ref rejected_by_dest, .. } => { assert_eq!(our_payment_hash, *payment_hash); assert!(!rejected_by_dest); }, _ => panic!("Unexpected event"), } + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2); + let node_0_closing_signed = match msg_events[0] { + MessageSendEvent::SendClosingSigned { ref node_id, ref msg } => { + assert_eq!(*node_id, nodes[1].node.get_our_node_id()); + (*msg).clone() + }, + _ => panic!("Unexpected event"), + }; + match msg_events[1] { + MessageSendEvent::PaymentFailureNetworkUpdate { update: msgs::HTLCFailChannelUpdate::ChannelUpdateMessage { ref msg }} => { + assert_eq!(msg.contents.short_channel_id, chan_1.0.contents.short_channel_id); + }, + _ => panic!("Unexpected event"), + } + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); - let node_0_closing_signed = get_event_msg!(nodes[0], MessageSendEvent::SendClosingSigned, nodes[1].node.get_our_node_id()); nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_closing_signed).unwrap(); let (_, node_1_closing_signed) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id()); nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed.unwrap()).unwrap(); @@ -7164,7 +7183,7 @@ mod tests { _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentFailed { payment_hash, rejected_by_dest } => { + Event::PaymentFailed { payment_hash, rejected_by_dest, .. } => { assert_eq!(payment_hash, payment_hash_5); assert!(rejected_by_dest); }, @@ -8128,7 +8147,7 @@ mod tests { // Tests handling of a monitor update failure when processing an incoming RAA let mut nodes = create_network(3); create_announced_chan_between_nodes(&nodes, 0, 1); - create_announced_chan_between_nodes(&nodes, 1, 2); + let chan_2 = create_announced_chan_between_nodes(&nodes, 1, 2); // Rebalance a bit so that we can send backwards from 2 to 1. send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000); @@ -8209,11 +8228,21 @@ mod tests { assert!(updates.update_fee.is_none()); nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]).unwrap(); - commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false); + commitment_signed_dance!(nodes[0], nodes[1], updates.commitment_signed, false, true); + + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1); + match msg_events[0] { + MessageSendEvent::PaymentFailureNetworkUpdate { update: msgs::HTLCFailChannelUpdate::ChannelUpdateMessage { ref msg }} => { + assert_eq!(msg.contents.short_channel_id, chan_2.0.contents.short_channel_id); + assert_eq!(msg.contents.flags & 2, 2); // temp disabled + }, + _ => panic!("Unexpected event"), + } let events = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); - if let Event::PaymentFailed { payment_hash, rejected_by_dest } = events[0] { + if let Event::PaymentFailed { payment_hash, rejected_by_dest, .. } = events[0] { assert_eq!(payment_hash, payment_hash_3); assert!(!rejected_by_dest); } else { panic!("Unexpected event!"); } @@ -8283,7 +8312,7 @@ mod tests { commitment_signed_dance!(nodes[0], nodes[1], messages_a.1, false); let events_4 = nodes[0].node.get_and_clear_pending_events(); assert_eq!(events_4.len(), 1); - if let Event::PaymentFailed { payment_hash, rejected_by_dest } = events_4[0] { + if let Event::PaymentFailed { payment_hash, rejected_by_dest, .. } = events_4[0] { assert_eq!(payment_hash, payment_hash_1); assert!(rejected_by_dest); } else { panic!("Unexpected event!"); }