Introduce ErrorHop enum

When we start handling Trampoline, the hops in our error decryption path
could be either `RouteHop`s or `TrampolineHop`s. To avoid excessive code
duplication, we introduce an enum with some methods for common accessors.

Author: arik-so

lightning/src/ln/onion_utils.rs

@@ -974,14 +974,39 @@ where
 	const NODE: u16 = 0x2000;
 	const UPDATE: u16 = 0x1000;
 
+	enum ErrorHop {
+		RouteHop(RouteHop),
+	}
+
+	impl ErrorHop {
+		fn fee_msat(&self) -> u64 {
+			match self {
+				ErrorHop::RouteHop(rh) => rh.fee_msat,
+			}
+		}
+
+		fn pubkey(&self) -> &PublicKey {
+			match self {
+				ErrorHop::RouteHop(rh) => rh.node_pubkey(),
+			}
+		}
+
+		fn short_channel_id(&self) -> Option<u64> {
+			match self {
+				ErrorHop::RouteHop(rh) => Some(rh.short_channel_id),
+			}
+		}
+	}
+
 	let mut onion_keys = Vec::with_capacity(path.hops.len());
 	construct_onion_keys_generic_callback(
 		secp_ctx,
 		&path.hops,
 		path.blinded_tail.as_ref(),
 		session_priv,
 		|shared_secret, _, _, route_hop_option: Option<&RouteHop>, _| {
-			onion_keys.push((route_hop_option.cloned(), shared_secret))
+			onion_keys
+				.push((route_hop_option.map(|rh| ErrorHop::RouteHop(rh.clone())), shared_secret))
 		},
 	)
 	.expect("Route we used spontaneously grew invalid keys in the middle of it?");
@@ -1048,7 +1073,7 @@ where
 			}
 		};
 
-		let amt_to_forward = htlc_msat - route_hop.fee_msat;
+		let amt_to_forward = htlc_msat - route_hop.fee_msat();
 		htlc_msat = amt_to_forward;
 
 		crypt_failure_packet(shared_secret.as_ref(), &mut encrypted_packet);
@@ -1065,13 +1090,13 @@ where
 			match msgs::DecodedOnionErrorPacket::read(&mut Cursor::new(&encrypted_packet.data)) {
 				Ok(p) => p,
 				Err(_) => {
-					log_warn!(logger, "Unreadable failure from {}", route_hop.pubkey);
+					log_warn!(logger, "Unreadable failure from {}", route_hop.pubkey());
 
 					let network_update = Some(NetworkUpdate::NodeFailure {
-						node_id: route_hop.pubkey,
+						node_id: route_hop.pubkey().clone(),
 						is_permanent: true,
 					});
-					let short_channel_id = Some(route_hop.short_channel_id);
+					let short_channel_id = route_hop.short_channel_id();
 					res = Some(FailureLearnings {
 						network_update,
 						short_channel_id,
@@ -1087,13 +1112,13 @@ where
 			None => {
 				// Useless packet that we can't use but it passed HMAC, so it definitely came from the peer
 				// in question
-				log_warn!(logger, "Missing error code in failure from {}", route_hop.pubkey);
+				log_warn!(logger, "Missing error code in failure from {}", route_hop.pubkey());
 
 				let network_update = Some(NetworkUpdate::NodeFailure {
-					node_id: route_hop.pubkey,
+					node_id: route_hop.pubkey().clone(),
 					is_permanent: true,
 				});
-				let short_channel_id = Some(route_hop.short_channel_id);
+				let short_channel_id = route_hop.short_channel_id();
 				res = Some(FailureLearnings {
 					network_update,
 					short_channel_id,
@@ -1127,22 +1152,28 @@ where
 			// entirely, but we can't be confident in that, as it would allow any node to get us to
 			// completely ban one of its counterparties. Instead, we simply remove the channel in
 			// question.
-			network_update = Some(NetworkUpdate::ChannelFailure {
-				short_channel_id: failing_route_hop.short_channel_id,
-				is_permanent: true,
-			});
-		} else if error_code & NODE == NODE {
-			let is_permanent = error_code & PERM == PERM;
-			network_update =
-				Some(NetworkUpdate::NodeFailure { node_id: route_hop.pubkey, is_permanent });
-			short_channel_id = Some(route_hop.short_channel_id);
-		} else if error_code & PERM == PERM {
-			if !payment_failed {
+			if let ErrorHop::RouteHop(failing_route_hop) = failing_route_hop {
 				network_update = Some(NetworkUpdate::ChannelFailure {
 					short_channel_id: failing_route_hop.short_channel_id,
 					is_permanent: true,
 				});
-				short_channel_id = Some(failing_route_hop.short_channel_id);
+			}
+		} else if error_code & NODE == NODE {
+			let is_permanent = error_code & PERM == PERM;
+			network_update = Some(NetworkUpdate::NodeFailure {
+				node_id: route_hop.pubkey().clone(),
+				is_permanent,
+			});
+			short_channel_id = route_hop.short_channel_id();
+		} else if error_code & PERM == PERM {
+			if !payment_failed {
+				if let ErrorHop::RouteHop(failing_route_hop) = failing_route_hop {
+					network_update = Some(NetworkUpdate::ChannelFailure {
+						short_channel_id: failing_route_hop.short_channel_id,
+						is_permanent: true,
+					});
+				}
+				short_channel_id = failing_route_hop.short_channel_id();
 			}
 		} else if error_code & UPDATE == UPDATE {
 			if let Some(update_len_slice) =
@@ -1155,37 +1186,41 @@ where
 					.get(debug_field_size + 4..debug_field_size + 4 + update_len)
 					.is_some()
 				{
-					network_update = Some(NetworkUpdate::ChannelFailure {
-						short_channel_id: failing_route_hop.short_channel_id,
-						is_permanent: false,
-					});
-					short_channel_id = Some(failing_route_hop.short_channel_id);
+					if let ErrorHop::RouteHop(failing_route_hop) = failing_route_hop {
+						network_update = Some(NetworkUpdate::ChannelFailure {
+							short_channel_id: failing_route_hop.short_channel_id,
+							is_permanent: false,
+						});
+					}
+					short_channel_id = failing_route_hop.short_channel_id();
 				}
 			}
 			if network_update.is_none() {
 				// They provided an UPDATE which was obviously bogus, not worth
 				// trying to relay through them anymore.
 				network_update = Some(NetworkUpdate::NodeFailure {
-					node_id: route_hop.pubkey,
+					node_id: route_hop.pubkey().clone(),
 					is_permanent: true,
 				});
 			}
 			if short_channel_id.is_none() {
-				short_channel_id = Some(route_hop.short_channel_id);
+				short_channel_id = route_hop.short_channel_id();
 			}
 		} else if payment_failed {
 			// Only blame the hop when a value in the HTLC doesn't match the corresponding value in the
 			// onion.
 			short_channel_id = match error_code & 0xff {
-				18 | 19 => Some(route_hop.short_channel_id),
+				18 | 19 => route_hop.short_channel_id(),
 				_ => None,
 			};
 		} else {
 			// We can't understand their error messages and they failed to forward...they probably can't
 			// understand our forwards so it's really not worth trying any further.
-			network_update =
-				Some(NetworkUpdate::NodeFailure { node_id: route_hop.pubkey, is_permanent: true });
-			short_channel_id = Some(route_hop.short_channel_id);
+			network_update = Some(NetworkUpdate::NodeFailure {
+				node_id: route_hop.pubkey().clone(),
+				is_permanent: true,
+			});
+			short_channel_id = route_hop.short_channel_id()
 		}
 
 		res = Some(FailureLearnings {
@@ -1200,7 +1235,7 @@ where
 			log_info!(
 				logger,
 				"Onion Error[from {}: {}({:#x}) {}({})] {}",
-				route_hop.pubkey,
+				route_hop.pubkey(),
 				title,
 				error_code,
 				debug_field,
@@ -1211,7 +1246,7 @@ where
 			log_info!(
 				logger,
 				"Onion Error[from {}: {}({:#x})] {}",
-				route_hop.pubkey,
+				route_hop.pubkey(),
 				title,
 				error_code,
 				description