Add checks for min value to all create_inbound_payment methods

Author: valentinewallace

lightning-invoice/src/utils.rs

@@ -63,7 +63,7 @@ where
 	let (payment_hash, payment_secret) = channelmanager.create_inbound_payment(
 		amt_msat,
 		DEFAULT_EXPIRY_TIME.try_into().unwrap(),
-	);
+	).unwrap();
 	let our_node_pubkey = channelmanager.get_our_node_id();
 	let mut invoice = InvoiceBuilder::new(network)
 		.description(description)

lightning/src/ln/channelmanager.rs

@@ -49,7 +49,7 @@ use routing::router::{Payee, Route, RouteHop, RoutePath, RouteParameters};
 use ln::msgs;
 use ln::msgs::NetAddress;
 use ln::onion_utils;
-use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, OptionalField};
+use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT, OptionalField};
 use chain::keysinterface::{Sign, KeyMaterial, KeysInterface, KeysManager, InMemorySigner};
 use util::config::UserConfig;
 use util::events::{EventHandler, EventsProvider, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
@@ -4649,6 +4649,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option<PaymentPreimage>, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
 		assert!(invoice_expiry_delta_secs <= 60*60*24*365); // Sadly bitcoin timestamps are u32s, so panic before 2106
 
+		if min_value_msat.is_some() && min_value_msat.unwrap() > MAX_VALUE_MSAT {
+			return Err(APIError::APIMisuseError { err: format!("min_value_msat of {} greater than total 21 million bitcoin supply", min_value_msat.unwrap()) });
+		}
+
 		let payment_secret = PaymentSecret(self.keys_manager.get_secure_random_bytes());
 
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
@@ -4698,7 +4702,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	/// [`PaymentReceived::payment_preimage`]: events::Event::PaymentReceived::payment_preimage
 	/// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
 	// For details on the implementation of this method, see `verify_inbound_payment_data`.
-	pub fn create_inbound_payment(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> (PaymentHash, PaymentSecret) {
+	pub fn create_inbound_payment(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<(PaymentHash, PaymentSecret), APIError> {
+		if min_value_msat.is_some() && min_value_msat.unwrap() > MAX_VALUE_MSAT {
+			return Err(APIError::APIMisuseError { err: format!("min_value_msat of {} greater than total 21 million bitcoin supply", min_value_msat.unwrap()) });
+		}
+
 		let min_amt_msat_bytes: [u8; 8] = match min_value_msat {
 			Some(amt) => amt.to_be_bytes(),
 			None => [0; 8],
@@ -4736,7 +4744,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		}
 
 		let payment_hash = PaymentHash(Sha256::hash(&payment_preimage_bytes).into_inner());
-		(payment_hash, PaymentSecret(payment_secret_bytes))
+		Ok((payment_hash, PaymentSecret(payment_secret_bytes)))
 	}
 
 	/// Legacy version of [`create_inbound_payment`]. Use this method if you wish to share
@@ -4746,12 +4754,11 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	/// This method will be deprecated in the next few versions.
 	///
 	/// [`create_inbound_payment`]: Self::create_inbound_payment
-	pub fn create_inbound_payment_legacy(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> (PaymentHash, PaymentSecret) {
+	pub fn create_inbound_payment_legacy(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<(PaymentHash, PaymentSecret), APIError> {
 		let payment_preimage = PaymentPreimage(self.keys_manager.get_secure_random_bytes());
 		let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
-		(payment_hash,
-		 self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs)
-		     .expect("RNG Generated Duplicate PaymentHash"))
+		let payment_secret = self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs)?;
+		Ok((payment_hash, payment_secret))
 	}
 
 	/// Gets a [`PaymentSecret`] for a given [`PaymentHash`], for which the payment preimage is
@@ -4798,6 +4805,10 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	/// [`PaymentReceived`]: events::Event::PaymentReceived
 	// For details on the implementation of this method, see `verify_inbound_payment_data`.
 	pub fn create_inbound_payment_for_hash(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
+		if min_value_msat.is_some() && min_value_msat.unwrap() > MAX_VALUE_MSAT {
+			return Err(APIError::APIMisuseError { err: format!("min_value_msat of {} greater than total 21 million bitcoin supply", min_value_msat.unwrap()) });
+		}
+
 		let mut min_amt_msat_bytes: [u8; 8] = match min_value_msat {
 			Some(amt) => amt.to_be_bytes(),
 			None => [0; 8],

lightning/src/ln/functional_tests.rs

@@ -8073,7 +8073,7 @@ fn test_preimage_storage() {
 	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
 
 	{
-		let (payment_hash, payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 7200);
+		let (payment_hash, payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 7200).unwrap();
 		let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
 		nodes[0].node.send_payment(&route, payment_hash, &Some(payment_secret)).unwrap();
 		check_added_monitors!(nodes[0], 1);
@@ -8111,7 +8111,7 @@ fn test_secret_timeout() {
 
 	create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).0.contents.short_channel_id;
 
-	let (payment_hash, payment_secret_1) = nodes[1].node.create_inbound_payment_legacy(Some(100_000), 2);
+	let (payment_hash, payment_secret_1) = nodes[1].node.create_inbound_payment_legacy(Some(100_000), 2).unwrap();
 
 	// We should fail to register the same payment hash twice, at least until we've connected a
 	// block with time 7200 + CHAN_CONFIRM_DEPTH + 1.
@@ -8178,7 +8178,7 @@ fn test_bad_secret_hash() {
 
 	let random_payment_hash = PaymentHash([42; 32]);
 	let random_payment_secret = PaymentSecret([43; 32]);
-	let (our_payment_hash, our_payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 2);
+	let (our_payment_hash, our_payment_secret) = nodes[1].node.create_inbound_payment(Some(100_000), 2).unwrap();
 	let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
 
 	// All the below cases should end up being handled exactly identically, so we macro the