Add test_util for overriding session privs for onion crypt
[rust-lightning] / src / ln / channelmanager.rs
index ccfb9f776973a217b812fbdf59eb8058bee0c96a..5216882d4a90126be786776f60781443eb2b31f2 100644 (file)
@@ -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};
@@ -662,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) {
@@ -686,8 +686,7 @@ impl ChannelManager {
                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);
@@ -973,26 +972,26 @@ impl ChannelManager {
        }
 
        fn decode_update_add_htlc_onion(&self, msg: &msgs::UpdateAddHTLC) -> (PendingHTLCStatus, MutexGuard<ChannelHolder>) {
-               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 = {
@@ -1002,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) => {
@@ -1019,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];
@@ -1093,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(),
                                };
@@ -1165,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[..]);
@@ -1556,28 +1553,54 @@ impl ChannelManager {
        /// still-available channels.
        fn fail_htlc_backwards_internal(&self, mut channel_state_lock: MutexGuard<ChannelHolder>, 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 }) => {
@@ -1971,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) {
@@ -2060,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
@@ -2100,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<u8>) -> (Option<msgs::HTLCFailChannelUpdate>, bool) {
+       fn process_onion_failure(&self, htlc_source: &HTLCSource, mut packet_decrypted: Vec<u8>) -> (Option<msgs::HTLCFailChannelUpdate>, bool, Option<u16>) {
                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;
 
@@ -2134,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[..]);
@@ -2144,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> {
@@ -2654,7 +2640,7 @@ impl events::MessageSendEventsProvider for ChannelManager {
                                        self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage);
                                } else {
                                        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 | 10, data: Vec::new() });
+                                       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() });
                                }
                        }
                }
@@ -2679,7 +2665,7 @@ impl events::EventsProvider for ChannelManager {
                                        self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage);
                                } else {
                                        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 | 10, data: Vec::new() });
+                                       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() });
                                }
                        }
                }
@@ -3584,6 +3570,7 @@ mod tests {
                chain_monitor: Arc<chaininterface::ChainWatchInterfaceUtil>,
                tx_broadcaster: Arc<test_utils::TestBroadcaster>,
                chan_monitor: Arc<test_utils::TestChannelMonitor>,
+               keys_manager: Arc<test_utils::TestKeysInterface>,
                node: Arc<ChannelManager>,
                router: Router,
                node_seed: [u8; 32],
@@ -3791,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"),
@@ -4287,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);
                                },
@@ -4315,14 +4304,14 @@ mod tests {
                        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(),
                        });
@@ -5027,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();
@@ -7179,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);
                                },
@@ -8143,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);
@@ -8224,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!"); }
@@ -8298,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!"); }