Skip to content

Commit

Permalink
Add checks for min value to all create_inbound_payment methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@valentinewallace
valentinewallace committed Dec 1, 2021
1 parent 1e575f8 commit 369fbda
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 11 deletions.
2 changes: 1 addition & 1 deletion lightning-invoice/src/utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
25 changes: 18 additions & 7 deletions lightning/src/ln/channelmanager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -4648,6 +4648,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);
Expand Down Expand Up @@ -4697,7 +4701,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],
Expand Down Expand Up @@ -4735,7 +4743,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
Expand All @@ -4745,12 +4753,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
Expand Down Expand Up @@ -4797,6 +4804,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],
Expand Down
6 changes: 3 additions & 3 deletions lightning/src/ln/functional_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 369fbda

Please sign in to comment.