Merge pull request #2128 from valentinewallace/2023-03-route-blinding-groundwork

Route blinding groundwork

Author: TheBlueMatt

fuzz/src/bin/gen_target.sh

@@ -20,6 +20,7 @@ GEN_TEST refund_deser
 GEN_TEST router
 GEN_TEST zbase32
 GEN_TEST indexedmap
+GEN_TEST onion_hop_data
 
 GEN_TEST msg_accept_channel msg_targets::
 GEN_TEST msg_announcement_signatures msg_targets::
@@ -51,7 +52,6 @@ GEN_TEST msg_update_add_htlc msg_targets::
 GEN_TEST msg_error_message msg_targets::
 GEN_TEST msg_channel_update msg_targets::
 
-GEN_TEST msg_onion_hop_data msg_targets::
 GEN_TEST msg_ping msg_targets::
 GEN_TEST msg_pong msg_targets::
 

fuzz/src/bin/msg_onion_hop_data_target.rs renamed to fuzz/src/bin/onion_hop_data_target.rs

@@ -16,14 +16,14 @@
 compile_error!("Fuzz targets need cfg=fuzzing");
 
 extern crate lightning_fuzz;
-use lightning_fuzz::msg_targets::msg_onion_hop_data::*;
+use lightning_fuzz::onion_hop_data::*;
 
 #[cfg(feature = "afl")]
 #[macro_use] extern crate afl;
 #[cfg(feature = "afl")]
 fn main() {
 	fuzz!(|data| {
-		msg_onion_hop_data_run(data.as_ptr(), data.len());
+		onion_hop_data_run(data.as_ptr(), data.len());
 	});
 }
 
@@ -33,7 +33,7 @@ fn main() {
 fn main() {
 	loop {
 		fuzz!(|data| {
-			msg_onion_hop_data_run(data.as_ptr(), data.len());
+			onion_hop_data_run(data.as_ptr(), data.len());
 		});
 	}
 }
@@ -42,7 +42,7 @@ fn main() {
 #[macro_use] extern crate libfuzzer_sys;
 #[cfg(feature = "libfuzzer_fuzz")]
 fuzz_target!(|data: &[u8]| {
-	msg_onion_hop_data_run(data.as_ptr(), data.len());
+	onion_hop_data_run(data.as_ptr(), data.len());
 });
 
 #[cfg(feature = "stdin_fuzz")]
@@ -51,7 +51,7 @@ fn main() {
 
 	let mut data = Vec::with_capacity(8192);
 	std::io::stdin().read_to_end(&mut data).unwrap();
-	msg_onion_hop_data_run(data.as_ptr(), data.len());
+	onion_hop_data_run(data.as_ptr(), data.len());
 }
 
 #[test]
@@ -63,11 +63,11 @@ fn run_test_cases() {
 	use std::sync::{atomic, Arc};
 	{
 		let data: Vec<u8> = vec![0];
-		msg_onion_hop_data_run(data.as_ptr(), data.len());
+		onion_hop_data_run(data.as_ptr(), data.len());
 	}
 	let mut threads = Vec::new();
 	let threads_running = Arc::new(atomic::AtomicUsize::new(0));
-	if let Ok(tests) = fs::read_dir("test_cases/msg_onion_hop_data") {
+	if let Ok(tests) = fs::read_dir("test_cases/onion_hop_data") {
 		for test in tests {
 			let mut data: Vec<u8> = Vec::new();
 			let path = test.unwrap().path();
@@ -82,7 +82,7 @@ fn run_test_cases() {
 
 					let panic_logger = string_logger.clone();
 					let res = if ::std::panic::catch_unwind(move || {
-						msg_onion_hop_data_test(&data, panic_logger);
+						onion_hop_data_test(&data, panic_logger);
 					}).is_err() {
 						Some(string_logger.into_string())
 					} else { None };

fuzz/src/lib.rs

@@ -28,5 +28,6 @@ pub mod process_network_graph;
 pub mod refund_deser;
 pub mod router;
 pub mod zbase32;
+pub mod onion_hop_data;
 
 pub mod msg_targets;

fuzz/src/msg_targets/gen_target.sh

@@ -20,7 +20,6 @@ GEN_TEST lightning::ln::msgs::ChannelReady test_msg_simple ""
 GEN_TEST lightning::ln::msgs::FundingSigned test_msg_simple ""
 GEN_TEST lightning::ln::msgs::GossipTimestampFilter test_msg_simple ""
 GEN_TEST lightning::ln::msgs::Init test_msg_simple ""
-GEN_TEST lightning::ln::msgs::OnionHopData test_msg_simple ""
 GEN_TEST lightning::ln::msgs::OpenChannel test_msg_simple ""
 GEN_TEST lightning::ln::msgs::Ping test_msg_simple ""
 GEN_TEST lightning::ln::msgs::Pong test_msg_simple ""

fuzz/src/msg_targets/mod.rs

@@ -8,7 +8,6 @@ pub mod msg_channel_ready;
 pub mod msg_funding_signed;
 pub mod msg_gossip_timestamp_filter;
 pub mod msg_init;
-pub mod msg_onion_hop_data;
 pub mod msg_open_channel;
 pub mod msg_ping;
 pub mod msg_pong;

fuzz/src/msg_targets/msg_onion_hop_data.rs renamed to fuzz/src/onion_hop_data.rs

@@ -10,16 +10,19 @@
 // This file is auto-generated by gen_target.sh based on msg_target_template.txt
 // To modify it, modify msg_target_template.txt and run gen_target.sh instead.
 
-use crate::msg_targets::utils::VecWriter;
 use crate::utils::test_logger;
 
 #[inline]
-pub fn msg_onion_hop_data_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
-	test_msg_simple!(lightning::ln::msgs::OnionHopData, data);
+pub fn onion_hop_data_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
+	use lightning::util::ser::Readable;
+	let mut r = ::std::io::Cursor::new(data);
+	let _ =  <lightning::ln::msgs::InboundOnionPayload as Readable>::read(&mut r);
 }
 
 #[no_mangle]
-pub extern "C" fn msg_onion_hop_data_run(data: *const u8, datalen: usize) {
+pub extern "C" fn onion_hop_data_run(data: *const u8, datalen: usize) {
+	use lightning::util::ser::Readable;
 	let data = unsafe { std::slice::from_raw_parts(data, datalen) };
-	test_msg_simple!(lightning::ln::msgs::OnionHopData, data);
+	let mut r = ::std::io::Cursor::new(data);
+	let _ =  <lightning::ln::msgs::InboundOnionPayload as Readable>::read(&mut r);
 }

fuzz/targets.h

@@ -13,6 +13,7 @@ void refund_deser_run(const unsigned char* data, size_t data_len);
 void router_run(const unsigned char* data, size_t data_len);
 void zbase32_run(const unsigned char* data, size_t data_len);
 void indexedmap_run(const unsigned char* data, size_t data_len);
+void onion_hop_data_run(const unsigned char* data, size_t data_len);
 void msg_accept_channel_run(const unsigned char* data, size_t data_len);
 void msg_announcement_signatures_run(const unsigned char* data, size_t data_len);
 void msg_channel_reestablish_run(const unsigned char* data, size_t data_len);
@@ -40,7 +41,6 @@ void msg_gossip_timestamp_filter_run(const unsigned char* data, size_t data_len)
 void msg_update_add_htlc_run(const unsigned char* data, size_t data_len);
 void msg_error_message_run(const unsigned char* data, size_t data_len);
 void msg_channel_update_run(const unsigned char* data, size_t data_len);
-void msg_onion_hop_data_run(const unsigned char* data, size_t data_len);
 void msg_ping_run(const unsigned char* data, size_t data_len);
 void msg_pong_run(const unsigned char* data, size_t data_len);
 void msg_channel_details_run(const unsigned char* data, size_t data_len);

lightning/src/ln/channel.rs

@@ -3119,9 +3119,9 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 
 			let mut htlc_updates = Vec::new();
 			mem::swap(&mut htlc_updates, &mut self.context.holding_cell_htlc_updates);
-			let mut update_add_htlcs = Vec::with_capacity(htlc_updates.len());
-			let mut update_fulfill_htlcs = Vec::with_capacity(htlc_updates.len());
-			let mut update_fail_htlcs = Vec::with_capacity(htlc_updates.len());
+			let mut update_add_count = 0;
+			let mut update_fulfill_count = 0;
+			let mut update_fail_count = 0;
 			let mut htlcs_to_fail = Vec::new();
 			for htlc_update in htlc_updates.drain(..) {
 				// Note that this *can* fail, though it should be due to rather-rare conditions on
@@ -3137,7 +3137,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 						match self.send_htlc(amount_msat, *payment_hash, cltv_expiry, source.clone(),
 							onion_routing_packet.clone(), false, skimmed_fee_msat, fee_estimator, logger)
 						{
-							Ok(update_add_msg_option) => update_add_htlcs.push(update_add_msg_option.unwrap()),
+							Ok(_) => update_add_count += 1,
 							Err(e) => {
 								match e {
 									ChannelError::Ignore(ref msg) => {
@@ -3164,11 +3164,11 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 						// not fail - any in between attempts to claim the HTLC will have resulted
 						// in it hitting the holding cell again and we cannot change the state of a
 						// holding cell HTLC from fulfill to anything else.
-						let (update_fulfill_msg_option, mut additional_monitor_update) =
-							if let UpdateFulfillFetch::NewClaim { msg, monitor_update, .. } = self.get_update_fulfill_htlc(htlc_id, *payment_preimage, logger) {
-								(msg, monitor_update)
-							} else { unreachable!() };
-						update_fulfill_htlcs.push(update_fulfill_msg_option.unwrap());
+						let mut additional_monitor_update =
+							if let UpdateFulfillFetch::NewClaim { monitor_update, .. } =
+								self.get_update_fulfill_htlc(htlc_id, *payment_preimage, logger)
+							{ monitor_update } else { unreachable!() };
+						update_fulfill_count += 1;
 						monitor_update.updates.append(&mut additional_monitor_update.updates);
 					},
 					&HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
@@ -3179,7 +3179,8 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 								// not fail - we should never end up in a state where we double-fail
 								// an HTLC or fail-then-claim an HTLC as it indicates we didn't wait
 								// for a full revocation before failing.
-								update_fail_htlcs.push(update_fail_msg_option.unwrap())
+								debug_assert!(update_fail_msg_option.is_some());
+								update_fail_count += 1;
 							},
 							Err(e) => {
 								if let ChannelError::Ignore(_) = e {}
@@ -3191,7 +3192,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 					},
 				}
 			}
-			if update_add_htlcs.is_empty() && update_fulfill_htlcs.is_empty() && update_fail_htlcs.is_empty() && self.context.holding_cell_update_fee.is_none() {
+			if update_add_count == 0 && update_fulfill_count == 0 && update_fail_count == 0 && self.context.holding_cell_update_fee.is_none() {
 				return (None, htlcs_to_fail);
 			}
 			let update_fee = if let Some(feerate) = self.context.holding_cell_update_fee.take() {
@@ -3208,7 +3209,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
 
 			log_debug!(logger, "Freeing holding cell in channel {} resulted in {}{} HTLCs added, {} HTLCs fulfilled, and {} HTLCs failed.",
 				log_bytes!(self.context.channel_id()), if update_fee.is_some() { "a fee update, " } else { "" },
-				update_add_htlcs.len(), update_fulfill_htlcs.len(), update_fail_htlcs.len());
+				update_add_count, update_fulfill_count, update_fail_count);
 
 			self.monitor_updating_paused(false, true, false, Vec::new(), Vec::new(), Vec::new());
 			(self.push_ret_blockable_mon_update(monitor_update), htlcs_to_fail)