Skip to content

lightning-invoice/utils: Actually add expiry to invoices #1474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 10, 2022
Merged

lightning-invoice/utils: Actually add expiry to invoices #1474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15a8401
Actually add expiry to phantom invoice utils
dunxen May 10, 2022
3369b29
Add expiry to non-phantom invoice utils
dunxen May 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lightning-invoice/src/payment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1558,7 +1558,7 @@ mod tests {

assert!(invoice_payer.pay_invoice(&create_invoice_from_channelmanager_and_duration_since_epoch(
&nodes[1].node, nodes[1].keys_manager, Currency::Bitcoin, Some(100_010_000), "Invoice".to_string(),
duration_since_epoch()).unwrap())
duration_since_epoch(), 3600).unwrap())
.is_ok());
let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(htlc_msgs.len(), 2);
Expand Down Expand Up @@ -1604,7 +1604,7 @@ mod tests {

assert!(invoice_payer.pay_invoice(&create_invoice_from_channelmanager_and_duration_since_epoch(
&nodes[1].node, nodes[1].keys_manager, Currency::Bitcoin, Some(100_010_000), "Invoice".to_string(),
duration_since_epoch()).unwrap())
duration_since_epoch(), 3600).unwrap())
.is_ok());
let htlc_msgs = nodes[0].node.get_and_clear_pending_msg_events();
assert_eq!(htlc_msgs.len(), 2);
Expand Down Expand Up @@ -1686,7 +1686,7 @@ mod tests {

assert!(invoice_payer.pay_invoice(&create_invoice_from_channelmanager_and_duration_since_epoch(
&nodes[1].node, nodes[1].keys_manager, Currency::Bitcoin, Some(100_010_000), "Invoice".to_string(),
duration_since_epoch()).unwrap())
duration_since_epoch(), 3600).unwrap())
.is_ok());
let htlc_updates = SendEvent::from_node(&nodes[0]);
check_added_monitors!(nodes[0], 1);
Expand Down
62 changes: 45 additions & 17 deletions lightning-invoice/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Convenient utilities to create an invoice.

use {CreationError, Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError};
use {CreationError, Currency, Invoice, InvoiceBuilder, SignOrCreationError};
use payment::{Payer, Router};

use crate::{prelude::*, Description, InvoiceDescription, Sha256};
Expand All @@ -20,7 +20,6 @@ use lightning::routing::network_graph::{NetworkGraph, RoutingFees};
use lightning::routing::router::{Route, RouteHint, RouteHintHop, RouteParameters, find_route};
use lightning::util::logger::Logger;
use secp256k1::PublicKey;
use core::convert::TryInto;
use core::ops::Deref;
use core::time::Duration;
use sync::Mutex;
Expand Down Expand Up @@ -162,7 +161,8 @@ fn _create_phantom_invoice<Signer: Sign, K: Deref>(
.current_timestamp()
.payment_hash(Hash::from_slice(&payment_hash.0).unwrap())
.payment_secret(payment_secret)
.min_final_cltv_expiry(MIN_FINAL_CLTV_EXPIRY.into());
.min_final_cltv_expiry(MIN_FINAL_CLTV_EXPIRY.into())
.expiry_time(Duration::from_secs(invoice_expiry_delta_secs.into()));
if let Some(amt) = amt_msat {
invoice = invoice.amount_milli_satoshis(amt);
}
Expand Down Expand Up @@ -212,9 +212,12 @@ fn _create_phantom_invoice<Signer: Sign, K: Deref>(
/// method stores the invoice's payment secret and preimage in `ChannelManager`, so (a) the user
/// doesn't have to store preimage/payment secret information and (b) `ChannelManager` can verify
/// that the payment secret is valid when the invoice is paid.
///
/// `invoice_expiry_delta_secs` describes the number of seconds that the invoice is valid for
/// in excess of the current time.
pub fn create_invoice_from_channelmanager<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>(
channelmanager: &ChannelManager<Signer, M, T, K, F, L>, keys_manager: K, network: Currency,
amt_msat: Option<u64>, description: String
amt_msat: Option<u64>, description: String, invoice_expiry_delta_secs: u32
) -> Result<Invoice, SignOrCreationError<()>>
where
M::Target: chain::Watch<Signer>,
Expand All @@ -227,7 +230,8 @@ where
let duration = SystemTime::now().duration_since(SystemTime::UNIX_EPOCH)
.expect("for the foreseeable future this shouldn't happen");
create_invoice_from_channelmanager_and_duration_since_epoch(
channelmanager, keys_manager, network, amt_msat, description, duration
channelmanager, keys_manager, network, amt_msat,
description, duration, invoice_expiry_delta_secs
Comment on lines -230 to +234
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would just wrap at 100 chars instead of making them evenly distributed to ease future automation

)
}

Expand All @@ -238,9 +242,12 @@ where
/// doesn't have to store preimage/payment secret information and (b) `ChannelManager` can verify
/// that the payment secret is valid when the invoice is paid.
/// Use this variant if you want to pass the `description_hash` to the invoice.
///
/// `invoice_expiry_delta_secs` describes the number of seconds that the invoice is valid for
/// in excess of the current time.
pub fn create_invoice_from_channelmanager_with_description_hash<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>(
channelmanager: &ChannelManager<Signer, M, T, K, F, L>, keys_manager: K, network: Currency,
amt_msat: Option<u64>, description_hash: Sha256,
amt_msat: Option<u64>, description_hash: Sha256, invoice_expiry_delta_secs: u32
) -> Result<Invoice, SignOrCreationError<()>>
where
M::Target: chain::Watch<Signer>,
Expand All @@ -256,7 +263,8 @@ where
.expect("for the foreseeable future this shouldn't happen");

create_invoice_from_channelmanager_with_description_hash_and_duration_since_epoch(
channelmanager, keys_manager, network, amt_msat, description_hash, duration,
channelmanager, keys_manager, network, amt_msat,
description_hash, duration, invoice_expiry_delta_secs
)
}

Expand All @@ -266,6 +274,7 @@ where
pub fn create_invoice_from_channelmanager_with_description_hash_and_duration_since_epoch<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>(
channelmanager: &ChannelManager<Signer, M, T, K, F, L>, keys_manager: K, network: Currency,
amt_msat: Option<u64>, description_hash: Sha256, duration_since_epoch: Duration,
invoice_expiry_delta_secs: u32
) -> Result<Invoice, SignOrCreationError<()>>
where
M::Target: chain::Watch<Signer>,
Expand All @@ -277,7 +286,7 @@ where
_create_invoice_from_channelmanager_and_duration_since_epoch(
channelmanager, keys_manager, network, amt_msat,
InvoiceDescription::Hash(&description_hash),
duration_since_epoch,
duration_since_epoch, invoice_expiry_delta_secs
)
}

Expand All @@ -287,6 +296,7 @@ where
pub fn create_invoice_from_channelmanager_and_duration_since_epoch<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>(
channelmanager: &ChannelManager<Signer, M, T, K, F, L>, keys_manager: K, network: Currency,
amt_msat: Option<u64>, description: String, duration_since_epoch: Duration,
invoice_expiry_delta_secs: u32
) -> Result<Invoice, SignOrCreationError<()>>
where
M::Target: chain::Watch<Signer>,
Expand All @@ -300,13 +310,14 @@ where
InvoiceDescription::Direct(
&Description::new(description).map_err(SignOrCreationError::CreationError)?,
),
duration_since_epoch,
duration_since_epoch, invoice_expiry_delta_secs
)
}

fn _create_invoice_from_channelmanager_and_duration_since_epoch<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>(
channelmanager: &ChannelManager<Signer, M, T, K, F, L>, keys_manager: K, network: Currency,
amt_msat: Option<u64>, description: InvoiceDescription, duration_since_epoch: Duration,
invoice_expiry_delta_secs: u32
) -> Result<Invoice, SignOrCreationError<()>>
where
M::Target: chain::Watch<Signer>,
Expand All @@ -320,7 +331,7 @@ where
// `create_inbound_payment` only returns an error if the amount is greater than the total bitcoin
// supply.
let (payment_hash, payment_secret) = channelmanager
.create_inbound_payment(amt_msat, DEFAULT_EXPIRY_TIME.try_into().unwrap())
.create_inbound_payment(amt_msat, invoice_expiry_delta_secs)
.map_err(|()| SignOrCreationError::CreationError(CreationError::InvalidAmount))?;
let our_node_pubkey = channelmanager.get_our_node_id();

Expand All @@ -337,7 +348,8 @@ where
.payment_hash(Hash::from_slice(&payment_hash.0).unwrap())
.payment_secret(payment_secret)
.basic_mpp()
.min_final_cltv_expiry(MIN_FINAL_CLTV_EXPIRY.into());
.min_final_cltv_expiry(MIN_FINAL_CLTV_EXPIRY.into())
.expiry_time(Duration::from_secs(invoice_expiry_delta_secs.into()));
if let Some(amt) = amt_msat {
invoice = invoice.amount_milli_satoshis(amt);
}
Expand Down Expand Up @@ -526,12 +538,14 @@ mod test {
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 100000, 10001, InitFeatures::known(), InitFeatures::known());
let non_default_invoice_expiry_secs = 4200;
let invoice = create_invoice_from_channelmanager_and_duration_since_epoch(
&nodes[1].node, nodes[1].keys_manager, Currency::BitcoinTestnet, Some(10_000), "test".to_string(),
Duration::from_secs(1234567)).unwrap();
Duration::from_secs(1234567), non_default_invoice_expiry_secs).unwrap();
assert_eq!(invoice.amount_pico_btc(), Some(100_000));
assert_eq!(invoice.min_final_cltv_expiry(), MIN_FINAL_CLTV_EXPIRY as u64);
assert_eq!(invoice.description(), InvoiceDescription::Direct(&Description("test".to_string())));
assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into()));

// Invoice SCIDs should always use inbound SCID aliases over the real channel ID, if one is
// available.
Expand Down Expand Up @@ -592,7 +606,7 @@ mod test {
let description_hash = crate::Sha256(Hash::hash("Testing description_hash".as_bytes()));
let invoice = ::utils::create_invoice_from_channelmanager_with_description_hash_and_duration_since_epoch(
&nodes[1].node, nodes[1].keys_manager, Currency::BitcoinTestnet, Some(10_000),
description_hash, Duration::from_secs(1234567),
description_hash, Duration::from_secs(1234567), 3600
).unwrap();
assert_eq!(invoice.amount_pico_btc(), Some(100_000));
assert_eq!(invoice.min_final_cltv_expiry(), MIN_FINAL_CLTV_EXPIRY as u64);
Expand Down Expand Up @@ -752,7 +766,7 @@ mod test {
) {
let invoice = create_invoice_from_channelmanager_and_duration_since_epoch(
&invoice_node.node, invoice_node.keys_manager, Currency::BitcoinTestnet, invoice_amt, "test".to_string(),
Duration::from_secs(1234567)).unwrap();
Duration::from_secs(1234567), 3600).unwrap();
let hints = invoice.private_routes();

for hint in hints {
Expand Down Expand Up @@ -799,8 +813,14 @@ mod test {
} else {
None
};
let non_default_invoice_expiry_secs = 4200;

let invoice = ::utils::create_phantom_invoice::<EnforcingSigner, &test_utils::TestKeysInterface>(Some(payment_amt), payment_hash, "test".to_string(), 3600, route_hints, &nodes[1].keys_manager, Currency::BitcoinTestnet).unwrap();
let invoice = ::utils::create_phantom_invoice::<
EnforcingSigner, &test_utils::TestKeysInterface
>(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: bleh, I hate these kinds of blank lines. Better to \n\t after the = or run over 100 chars than this :(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll get to these in a follow-up today :) (also Jeff's)

Some(payment_amt), payment_hash, "test".to_string(), non_default_invoice_expiry_secs,
route_hints, &nodes[1].keys_manager, Currency::BitcoinTestnet
).unwrap();
let (payment_hash, payment_secret) = (PaymentHash(invoice.payment_hash().into_inner()), *invoice.payment_secret());
let payment_preimage = if user_generated_pmt_hash {
user_payment_preimage
Expand All @@ -811,6 +831,7 @@ mod test {
assert_eq!(invoice.min_final_cltv_expiry(), MIN_FINAL_CLTV_EXPIRY as u64);
assert_eq!(invoice.description(), InvoiceDescription::Direct(&Description("test".to_string())));
assert_eq!(invoice.route_hints().len(), 2);
assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into()));
assert!(!invoice.features().unwrap().supports_basic_mpp());

let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
Expand Down Expand Up @@ -931,10 +952,17 @@ mod test {
];

let description_hash = crate::Sha256(Hash::hash("Description hash phantom invoice".as_bytes()));
let invoice = ::utils::create_phantom_invoice_with_description_hash::<EnforcingSigner,&test_utils::TestKeysInterface>(Some(payment_amt), None, 3600, description_hash, route_hints, &nodes[1].keys_manager, Currency::BitcoinTestnet).unwrap();

let non_default_invoice_expiry_secs = 4200;
let invoice = ::utils::create_phantom_invoice_with_description_hash::<
EnforcingSigner, &test_utils::TestKeysInterface,
>(
Some(payment_amt), None, non_default_invoice_expiry_secs, description_hash,
route_hints, &nodes[1].keys_manager, Currency::BitcoinTestnet
)
.unwrap();
assert_eq!(invoice.amount_pico_btc(), Some(200_000));
assert_eq!(invoice.min_final_cltv_expiry(), MIN_FINAL_CLTV_EXPIRY as u64);
assert_eq!(invoice.expiry_time(), Duration::from_secs(non_default_invoice_expiry_secs.into()));
assert_eq!(invoice.description(), InvoiceDescription::Hash(&crate::Sha256(Sha256::hash("Description hash phantom invoice".as_bytes()))));
}

Expand Down