WIP: started implementing onion failure handling

Author: yuntai

fuzz/fuzz_targets/router_target.rs

@@ -187,7 +187,7 @@ pub fn do_test(data: &[u8]) {
 					},
 					1 => {
 						let short_channel_id = slice_to_be64(get_slice!(8));
-						router.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelClosed {short_channel_id});
+						router.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelClosed {short_channel_id, is_permanent: false});
 					},
 					_ => return,
 				}

src/ln/channelmanager.rs

@@ -1345,11 +1345,6 @@ impl ChannelManager {
 		} else { false }
 	}
 
-	/// Indicates that the amount for payment_hash is incorrect after a PaymentReceived event.
-	pub fn fail_htlc_backwards_incorrect_payment_amount(&self, payment_hash: &[u8; 32]) -> bool {
-		self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 16, data: Vec::new() })
-	}
-
 	/// Fails an HTLC backwards to the sender of it to us.
 	/// Note that while we take a channel_state lock as input, we do *not* assume consistency here.
 	/// There are several callsites that do stupid things like loop over a list of payment_hashes
@@ -1358,6 +1353,8 @@ impl ChannelManager {
 	/// still-available channels.
 	fn fail_htlc_backwards_internal(&self, mut channel_state: MutexGuard<ChannelHolder>, source: HTLCSource, payment_hash: &[u8; 32], onion_error: HTLCFailReason) {
 		match source {
+			//YK TODO: not sure this can happen here as htlc_bacwards_interanl is
+			//always dervied by forwarding process
 			HTLCSource::OutboundRoute { .. } => {
 				mem::drop(channel_state);
 
@@ -1778,7 +1775,113 @@ impl ChannelManager {
 		Ok(())
 	}
 
+	// Process onion peacket processed in only in the origin node. Returns update
+	// for router and boolean flag indicating if payment can be retried
+	fn process_onion_failure(&self, route: &Route, mut packet_decrypted: Vec<u8>, session_priv: &SecretKey) -> (Option<msgs::HTLCFailChannelUpdate>, bool) {
+		use ln::router::{BADONION, PERM, NODE, UPDATE, OnionFailureCode};
+		let mut res = None;
+
+		// 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| {
+			if res.is_some() { return; }
+
+			let ammag = ChannelManager::gen_ammag_from_shared_secret(&shared_secret);
+
+			let mut decryption_tmp = Vec::with_capacity(packet_decrypted.len());
+			decryption_tmp.resize(packet_decrypted.len(), 0);
+			let mut chacha = ChaCha20::new(&ammag, &[0u8; 8]);
+			chacha.process(&packet_decrypted, &mut decryption_tmp[..]);
+			packet_decrypted = decryption_tmp;
+
+			let is_from_final_node = route.hops.last().unwrap().pubkey == route_hop.pubkey;
+
+			match msgs::DecodedOnionErrorPacket::read(&mut Cursor::new(&packet_decrypted)) {
+				Err(e) => { 
+					// TODO: what to do?
+				},
+				Ok(ref err_packet) if err_packet.failuremsg.len() < 2 => {
+					// TODO: what to do?
+				}
+				Ok(ref err_packet) if err_packet.failuremsg.len() >= 2 => {
+					let um = ChannelManager::gen_um_from_shared_secret(&shared_secret);
+
+					let mut hmac = Hmac::new(Sha256::new(), &um);
+					hmac.input(&err_packet.encode()[32..]);
+					let mut calc_tag = [0u8; 32];
+					hmac.raw_result(&mut calc_tag);
+
+					if crypto::util::fixed_time_eq(&calc_tag, &err_packet.hmac) {
+						let error_code = OnionFailureCode::from_code(byte_utils::slice_to_be16(&err_packet.failuremsg[0..2]));
+						if error_code.is_none() {
+							// bogus node sending invalid code
+							res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure {
+								node_id: route_hop.pubkey,
+								is_permanent: true,
+							}), false));
+							return;
+						}
+						let error_code = error_code.unwrap();
+
+						if !error_code.is_from_valid_node(is_from_final_node) {
+							// final node sent an error code that it MUST not send or
+							// intermediate node sent an error code that it MUST not
+							// consider it as bogus
+							res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure {
+								node_id: route_hop.pubkey,
+								is_permanent: true,
+							}), false));
+							return;
+						}
+
+						if is_from_final_node {
+							res = Some((None, !error_code.is_flag_set(PERM)));
+							return;
+						}
+
+						if error_code.is_flag_set(NODE) {
+							res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure {
+								node_id: route_hop.pubkey,
+								is_permanent: error_code.is_flag_set(PERM),
+							}), !error_code.is_flag_set(PERM)));
+						} else if error_code.is_flag_set(UPDATE) {
+							// TODO: parse additinoal parameter and implement
+							// AND the channel_update is valid and more recent than the channel_update used to send the payment
+							// if channel_update should NOT have caused the failure: 
+							// MAY treat the channel_update as invalid.
+							if err_packet.failuremsg.len() >= 4 {
+								let update_len = byte_utils::slice_to_be16(&err_packet.failuremsg[2..4]) as usize;
+								if err_packet.failuremsg.len() >= 4 + update_len {
+									if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&err_packet.failuremsg[4..4 + update_len])) {
+										res = Some((Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage {
+											msg: chan_update,
+										}), true));
+										return;
+									}
+								}
+							}
+							// malformed
+							res = Some((Some(msgs::HTLCFailChannelUpdate::NodeFailure {
+								node_id: route_hop.pubkey,
+								is_permanent: true,
+							}), false));
+							return;
+						} else {
+							debug_assert!(error_code.is_flag_set(PERM));
+							res = Some((Some(msgs::HTLCFailChannelUpdate::ChannelClosed {
+								short_channel_id: route_hop.short_channel_id,
+								is_permanent: true,
+							}), false));
+						}
+					}
+				},
+				_ => { unreachable!() }
+			}
+		});
+		res.unwrap()
+	}
+
 	fn internal_update_fail_htlc(&self, their_node_id: &PublicKey, msg: &msgs::UpdateFailHTLC) -> Result<Option<msgs::HTLCFailChannelUpdate>, MsgHandleErrInternal> {
+
 		let mut channel_state = self.channel_state.lock().unwrap();
 		let htlc_source = match channel_state.by_id.get_mut(&msg.channel_id) {
 			Some(chan) => {
@@ -1792,62 +1895,15 @@ impl ChannelManager {
 			None => return Err(MsgHandleErrInternal::send_err_msg_no_close("Failed to find corresponding channel", msg.channel_id))
 		}?;
 
-		match htlc_source {
-			&HTLCSource::OutboundRoute { ref route, ref session_priv, .. } => {
-				// Handle packed channel/node updates for passing back for the route handler
-				let mut packet_decrypted = msg.reason.data.clone();
-				let mut res = None;
-				Self::construct_onion_keys_callback(&self.secp_ctx, &route, &session_priv, |shared_secret, _, _, route_hop| {
-					if res.is_some() { return; }
-
-					let ammag = ChannelManager::gen_ammag_from_shared_secret(&shared_secret);
-
-					let mut decryption_tmp = Vec::with_capacity(packet_decrypted.len());
-					decryption_tmp.resize(packet_decrypted.len(), 0);
-					let mut chacha = ChaCha20::new(&ammag, &[0u8; 8]);
-					chacha.process(&packet_decrypted, &mut decryption_tmp[..]);
-					packet_decrypted = decryption_tmp;
-
-					if let Ok(err_packet) = msgs::DecodedOnionErrorPacket::read(&mut Cursor::new(&packet_decrypted)) {
-						if err_packet.failuremsg.len() >= 2 {
-							let um = ChannelManager::gen_um_from_shared_secret(&shared_secret);
-
-							let mut hmac = Hmac::new(Sha256::new(), &um);
-							hmac.input(&err_packet.encode()[32..]);
-							let mut calc_tag = [0u8; 32];
-							hmac.raw_result(&mut calc_tag);
-							if crypto::util::fixed_time_eq(&calc_tag, &err_packet.hmac) {
-								const UNKNOWN_CHAN: u16 = 0x4000|10;
-								const TEMP_CHAN_FAILURE: u16 = 0x4000|7;
-								match byte_utils::slice_to_be16(&err_packet.failuremsg[0..2]) {
-									TEMP_CHAN_FAILURE => {
-										if err_packet.failuremsg.len() >= 4 {
-											let update_len = byte_utils::slice_to_be16(&err_packet.failuremsg[2..4]) as usize;
-											if err_packet.failuremsg.len() >= 4 + update_len {
-												if let Ok(chan_update) = msgs::ChannelUpdate::read(&mut Cursor::new(&err_packet.failuremsg[4..4 + update_len])) {
-													res = Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage {
-														msg: chan_update,
-													});
-												}
-											}
-										}
-									},
-									UNKNOWN_CHAN => {
-										// No such next-hop. We know this came from the
-										// current node as the HMAC validated.
-										res = Some(msgs::HTLCFailChannelUpdate::ChannelClosed {
-											short_channel_id: route_hop.short_channel_id
-										});
-									},
-									_ => {}, //TODO: Enumerate all of these!
-								}
-							}
-						}
-					}
-				}).unwrap();
-				Ok(res)
-			},
-			_ => { Ok(None) },
+		// we are the originating node and update route information
+		if let &HTLCSource::OutboundRoute { ref route, ref session_priv } = htlc_source {
+			let (update, _payment_retry) = self.process_onion_failure(route, msg.reason.data.clone(), session_priv);
+			if _payment_retry {
+				// TODO
+			}
+			Ok(update)
+		} else {
+			Ok(None)
 		}
 	}
 
@@ -4224,6 +4280,9 @@ mod tests {
 		assert_eq!(nodes[0].node.list_channels().len(), 0);
 		assert_eq!(nodes[1].node.list_channels().len(), 1);
 
+		let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+		nodes[4].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![] }, 1);
+
 		// One pending HTLC is discarded by the force-close:
 		let payment_preimage_1 = route_payment(&nodes[1], &vec!(&nodes[2], &nodes[3])[..], 3000000).0;
 
@@ -4277,8 +4336,6 @@ mod tests {
 		assert_eq!(nodes[2].node.list_channels().len(), 0);
 		assert_eq!(nodes[3].node.list_channels().len(), 1);
 
-		let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
-		nodes[4].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![] }, 1);
 		// One pending HTLC to time out:
 		let payment_preimage_2 = route_payment(&nodes[3], &vec!(&nodes[4])[..], 3000000).0;
 
@@ -4844,7 +4901,7 @@ mod tests {
 		let as_chan = a_channel_lock.by_id.get(&chan_announcement.3).unwrap();
 		let bs_chan = b_channel_lock.by_id.get(&chan_announcement.3).unwrap();
 
-		let _ = nodes[0].router.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelClosed { short_channel_id : as_chan.get_short_channel_id().unwrap() } );
+		let _ = nodes[0].router.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelClosed { short_channel_id : as_chan.get_short_channel_id().unwrap(), is_permanent: false } );
 
 		let as_bitcoin_key = PublicKey::from_secret_key(&secp_ctx, &as_chan.get_local_keys().funding_key);
 		let bs_bitcoin_key = PublicKey::from_secret_key(&secp_ctx, &bs_chan.get_local_keys().funding_key);
@@ -4891,7 +4948,7 @@ mod tests {
 		let unsigned_msg = dummy_unsigned_msg!();
 		sign_msg!(unsigned_msg);
 		assert_eq!(nodes[0].router.handle_channel_announcement(&chan_announcement).unwrap(), true);
-		let _ = nodes[0].router.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelClosed { short_channel_id : as_chan.get_short_channel_id().unwrap() } );
+		let _ = nodes[0].router.handle_htlc_fail_channel_update(&msgs::HTLCFailChannelUpdate::ChannelClosed { short_channel_id : as_chan.get_short_channel_id().unwrap(), is_permanent: false } );
 
 		// Configured with Network::Testnet
 		let mut unsigned_msg = dummy_unsigned_msg!();

src/ln/msgs.rs

@@ -484,7 +484,19 @@ pub enum HTLCFailChannelUpdate {
 	ChannelClosed {
 		/// The short_channel_id which has now closed.
 		short_channel_id: u64,
+		/// when this true, this channel should be permanently removed from the
+		/// consideration. Otherwise, this channel can be restored as new channel_update is received
+		is_permanent: bool,
 	},
+	/// We received an error which indicated only that a node has failed
+	NodeFailure {
+		/// The node_id that has failed.
+		node_id: PublicKey,
+		/// when this true, node should be permanently removed from the
+		/// consideration. Otherwise, the channels connected to this node can be
+		/// restored as new channel_update is received
+		is_permanent: bool,
+	}
 }
 
 /// A trait to describe an object which can receive channel messages.