Make rand a dev-dep by having the user randomize HTLC forward delay

This removes the last calls to rand outside of test and moves the
dep to a dev-dependency, dropping our fuzz rng wrapper in the
process.

Cargo.toml

@@ -24,7 +24,6 @@ max_level_debug = []
 [dependencies]
 bitcoin = "0.18"
 bitcoin_hashes = "0.3"
-rand = "0.4"
 secp256k1 = "0.12"
 
 [dev-dependencies.bitcoin]
@@ -33,6 +32,7 @@ features = ["bitcoinconsensus"]
 
 [dev-dependencies]
 hex = "0.3"
+rand = "0.4"
 
 [profile.dev]
 opt-level = 1

fuzz/fuzz_targets/chanmon_deser_target.rs

@@ -8,7 +8,6 @@ extern crate lightning;
 use bitcoin_hashes::sha256d::Hash as Sha256dHash;
 
 use lightning::ln::channelmonitor;
-use lightning::util::reset_rng_state;
 use lightning::util::ser::{ReadableArgs, Writer};
 
 mod utils;
@@ -30,7 +29,6 @@ impl Writer for VecWriter {
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	let logger = Arc::new(test_logger::TestLogger::new("".to_owned()));
 	if let Ok((latest_block_hash, monitor)) = <(Sha256dHash, channelmonitor::ChannelMonitor)>::read(&mut Cursor::new(data), logger.clone()) {
 		let mut w = VecWriter(Vec::new());

fuzz/fuzz_targets/chanmon_fail_consistency.rs

@@ -37,7 +37,7 @@ use lightning::ln::channelmonitor::{ChannelMonitorUpdateErr, HTLCUpdate};
 use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage};
 use lightning::ln::router::{Route, RouteHop};
 use lightning::ln::msgs::{CommitmentUpdate, ChannelMessageHandler, ErrorAction, HandleError, UpdateAddHTLC, LocalFeatures};
-use lightning::util::{reset_rng_state, events};
+use lightning::util::events;
 use lightning::util::logger::Logger;
 use lightning::util::config::UserConfig;
 use lightning::util::events::{EventsProvider, MessageSendEventsProvider};
@@ -136,8 +136,6 @@ impl KeysInterface for KeyProvider {
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
-
 	let fee_est = Arc::new(FuzzEstimator{});
 	let broadcast = Arc::new(TestBroadcaster{});
 

fuzz/fuzz_targets/full_stack_target.rs

@@ -34,7 +34,6 @@ use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage
 use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor};
 use lightning::ln::router::Router;
 use lightning::util::events::{EventsProvider,Event};
-use lightning::util::reset_rng_state;
 use lightning::util::logger::Logger;
 use lightning::util::config::UserConfig;
 
@@ -292,8 +291,6 @@ impl KeysInterface for KeyProvider {
 
 #[inline]
 pub fn do_test(data: &[u8], logger: &Arc<Logger>) {
-	reset_rng_state();
-
 	let input = Arc::new(InputData {
 		data: data.to_vec(),
 		read_pos: AtomicUsize::new(0),

fuzz/fuzz_targets/msg_targets/msg_accept_channel_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::AcceptChannel, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_announcement_signatures_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::AnnouncementSignatures, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_channel_announcement_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg_exact!(msgs::ChannelAnnouncement, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_channel_reestablish_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::ChannelReestablish, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_channel_update_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg_exact!(msgs::ChannelUpdate, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_closing_signed_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::ClosingSigned, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_commitment_signed_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::CommitmentSigned, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_decoded_onion_error_packet_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::DecodedOnionErrorPacket, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_error_message_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg_hole!(msgs::ErrorMessage, data, 32, 2);
 }
 

fuzz/fuzz_targets/msg_targets/msg_funding_created_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::FundingCreated, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_funding_locked_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::FundingLocked, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_funding_signed_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::FundingSigned, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_init_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::Init, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_node_announcement_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg_exact!(msgs::NodeAnnouncement, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_onion_hop_data_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg_hole!(msgs::OnionHopData, data, 1+8+8+4, 12);
 }
 

fuzz/fuzz_targets/msg_targets/msg_open_channel_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::OpenChannel, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_ping_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg_simple!(msgs::Ping, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_pong_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg_simple!(msgs::Pong, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_revoke_and_ack_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::RevokeAndACK, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_shutdown_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::Shutdown, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_target_template.txt

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	TEST_MSG!(msgs::MSG_TARGET, dataEXTRA_ARGS);
 }
 

fuzz/fuzz_targets/msg_targets/msg_update_add_htlc_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg_hole!(msgs::UpdateAddHTLC, data, 85, 33);
 }
 

fuzz/fuzz_targets/msg_targets/msg_update_fail_htlc_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::UpdateFailHTLC, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_update_fail_malformed_htlc_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::UpdateFailMalformedHTLC, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_update_fee_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::UpdateFee, data);
 }
 

fuzz/fuzz_targets/msg_targets/msg_update_fulfill_htlc_target.rs

@@ -4,14 +4,12 @@
 extern crate lightning;
 
 use lightning::ln::msgs;
-use lightning::util::reset_rng_state;
 
 mod utils;
 use utils::VecWriter;
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
 	test_msg!(msgs::UpdateFulfillHTLC, data);
 }
 

fuzz/fuzz_targets/peer_crypt_target.rs

@@ -2,7 +2,6 @@ extern crate lightning;
 extern crate secp256k1;
 
 use lightning::ln::peer_channel_encryptor::PeerChannelEncryptor;
-use lightning::util::reset_rng_state;
 
 use secp256k1::key::{PublicKey,SecretKey};
 
@@ -14,8 +13,6 @@ fn slice_to_be16(v: &[u8]) -> u16 {
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
-
 	let mut read_pos = 0;
 	macro_rules! get_slice {
 		($len: expr) => {

fuzz/fuzz_targets/router_target.rs

@@ -11,7 +11,6 @@ use lightning::ln::channelmanager::ChannelDetails;
 use lightning::ln::msgs;
 use lightning::ln::msgs::{RoutingMessageHandler};
 use lightning::ln::router::{Router, RouteHint};
-use lightning::util::reset_rng_state;
 use lightning::util::logger::Logger;
 use lightning::util::ser::Readable;
 
@@ -96,8 +95,6 @@ impl ChainWatchInterface for DummyChainWatcher {
 
 #[inline]
 pub fn do_test(data: &[u8]) {
-	reset_rng_state();
-
 	let input = Arc::new(InputData {
 		data: data.to_vec(),
 		read_pos: AtomicUsize::new(0),

src/lib.rs

@@ -13,8 +13,8 @@
 
 extern crate bitcoin;
 extern crate bitcoin_hashes;
-extern crate rand;
 extern crate secp256k1;
+#[cfg(test)] extern crate rand;
 #[cfg(test)] extern crate hex;
 
 #[macro_use]

src/ln/channelmanager.rs

@@ -36,7 +36,7 @@ use ln::onion_utils;
 use ln::msgs::{ChannelMessageHandler, DecodeError, HandleError};
 use chain::keysinterface::KeysInterface;
 use util::config::UserConfig;
-use util::{byte_utils, events, rng};
+use util::{byte_utils, events};
 use util::ser::{Readable, ReadableArgs, Writeable, Writer};
 use util::chacha20::ChaCha20;
 use util::logger::Logger;
@@ -214,11 +214,11 @@ impl MsgHandleErrInternal {
 	}
 }
 
-/// We hold back HTLCs we intend to relay for a random interval in the range (this, 5*this). This
-/// provides some limited amount of privacy. Ideally this would range from somewhere like 1 second
-/// to 30 seconds, but people expect lightning to be, you know, kinda fast, sadly. We could
-/// probably increase this significantly.
-const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u32 = 50;
+/// We hold back HTLCs we intend to relay for a random interval greater than this (see
+/// Event::PendingHTLCsForwardable for the API guidelines indicating how long should be waited).
+/// This provides some limited amount of privacy. Ideally this would range from somewhere like one
+/// second to 30 seconds, but people expect lightning to be, you know, kinda fast, sadly.
+const MIN_HTLC_RELAY_HOLDING_CELL_MILLIS: u64 = 100;
 
 pub(super) enum HTLCForwardInfo {
 	AddHTLC {
@@ -1486,7 +1486,7 @@ impl ChannelManager {
 
 				let mut forward_event = None;
 				if channel_state_lock.forward_htlcs.is_empty() {
-					forward_event = Some(Duration::from_millis(((rng::rand_f32() * 4.0 + 1.0) * MIN_HTLC_RELAY_HOLDING_CELL_MILLIS as f32) as u64));
+					forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS));
 				}
 				match channel_state_lock.forward_htlcs.entry(short_channel_id) {
 					hash_map::Entry::Occupied(mut entry) => {
@@ -2095,7 +2095,7 @@ impl ChannelManager {
 			if !pending_forwards.is_empty() {
 				let mut channel_state = self.channel_state.lock().unwrap();
 				if channel_state.forward_htlcs.is_empty() {
-					forward_event = Some(Duration::from_millis(((rng::rand_f32() * 4.0 + 1.0) * MIN_HTLC_RELAY_HOLDING_CELL_MILLIS as f32) as u64));
+					forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS))
 				}
 				for (forward_info, prev_htlc_id) in pending_forwards.drain(..) {
 					match channel_state.forward_htlcs.entry(forward_info.short_channel_id) {

src/util/events.rs

@@ -92,7 +92,10 @@ pub enum Event {
 	/// Used to indicate that ChannelManager::process_pending_htlc_forwards should be called at a
 	/// time in the future.
 	PendingHTLCsForwardable {
-		/// The amount of time that should be waited prior to calling process_pending_htlc_forwards
+		/// The minimum amount of time that should be waited prior to calling
+		/// process_pending_htlc_forwards. To increase the effort required to correlate payments,
+		/// you should wait a random amount of time in roughly the range (now + time_forwardable,
+		/// now + 5*time_forwardable).
 		time_forwardable: Duration,
 	},
 	/// Used to indicate that an output was generated on-chain which you should know how to spend.

src/util/mod.rs

@@ -10,7 +10,6 @@ pub(crate) mod chacha20;
 pub(crate) mod poly1305;
 pub(crate) mod chacha20poly1305rfc;
 pub(crate) mod internal_traits;
-pub(crate) mod rng;
 pub(crate) mod transaction_utils;
 
 #[macro_use]
@@ -22,9 +21,6 @@ pub(crate) mod macro_logger;
 pub mod logger;
 pub mod config;
 
-#[cfg(feature = "fuzztarget")]
-pub use self::rng::{reset_rng_state, fill_bytes};
-
 #[cfg(test)]
 pub(crate) mod test_utils;
 

src/util/rng.rs

@@ -1,44 +0,0 @@
-#[cfg(not(feature = "fuzztarget"))]
-mod real_rng {
-	use rand::{thread_rng,Rng};
-
-	pub fn fill_bytes(data: &mut [u8]) {
-		let mut rng = thread_rng();
-		rng.fill_bytes(data);
-	}
-
-	pub fn rand_f32() -> f32 {
-		let mut rng = thread_rng();
-		rng.next_f32()
-	}
-}
-#[cfg(not(feature = "fuzztarget"))]
-pub use self::real_rng::*;
-
-#[cfg(feature = "fuzztarget")]
-mod fuzzy_rng {
-	use util::byte_utils;
-
-	static mut RNG_ITER: u64 = 0;
-
-	pub fn fill_bytes(data: &mut [u8]) {
-		let rng = unsafe { RNG_ITER += 1; RNG_ITER -1 };
-		for i in 0..data.len() / 8 {
-			data[i*8..(i+1)*8].copy_from_slice(&byte_utils::be64_to_array(rng));
-		}
-		let rem = data.len() % 8;
-		let off = data.len() - rem;
-		data[off..].copy_from_slice(&byte_utils::be64_to_array(rng)[0..rem]);
-	}
-
-	pub fn rand_f32() -> f32 {
-		let rng = unsafe { RNG_ITER += 1; RNG_ITER - 1 };
-		f64::from_bits(rng) as f32
-	}
-
-	pub fn reset_rng_state() {
-		unsafe { RNG_ITER = 0; }
-	}
-}
-#[cfg(feature = "fuzztarget")]
-pub use self::fuzzy_rng::*;