-
Notifications
You must be signed in to change notification settings - Fork 431
Add custom TLV in Bolt11 Payer API #4263
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -615,12 +615,12 @@ mod test { | |
| use super::*; | ||
| use crate::chain::channelmonitor::HTLC_FAIL_BACK_BUFFER; | ||
| use crate::ln::channelmanager::{ | ||
| Bolt11InvoiceParameters, PaymentId, PhantomRouteHints, RecipientOnionFields, Retry, | ||
| MIN_FINAL_CLTV_EXPIRY_DELTA, | ||
| Bolt11InvoiceParameters, OptionalPaymentParams, PaymentId, PhantomRouteHints, | ||
| RecipientOnionFields, Retry, MIN_FINAL_CLTV_EXPIRY_DELTA, | ||
| }; | ||
| use crate::ln::functional_test_utils::*; | ||
| use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, MessageSendEvent}; | ||
| use crate::routing::router::{PaymentParameters, RouteParameters}; | ||
| use crate::routing::router::{PaymentParameters, RouteParameters, RouteParametersConfig}; | ||
| use crate::sign::PhantomKeysManager; | ||
| use crate::types::payment::{PaymentHash, PaymentPreimage}; | ||
| use crate::util::config::UserConfig; | ||
|
|
@@ -663,26 +663,26 @@ mod test { | |
| } | ||
|
|
||
| #[test] | ||
| fn create_and_pay_for_bolt11_invoice() { | ||
| fn create_and_pay_for_bolt11_invoice_with_custom_tlvs() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this intentional that the original test (without custom TLVs) is now gone?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since checking for custom TLVs is a parallel check to the other values and does not affect their behavior, I folded it into the existing test rather than keeping a separate one. I also cleaned up the test in .04 by using helper test functions to improve readability. Happy to adjust if you think keeping the original test separate would be better. |
||
| let chanmon_cfgs = create_chanmon_cfgs(2); | ||
| let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); | ||
| 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); | ||
|
|
||
| let node_a_id = nodes[0].node.get_our_node_id(); | ||
|
|
||
| let amt_msat = 10_000; | ||
| let description = | ||
| Bolt11InvoiceDescription::Direct(Description::new("test".to_string()).unwrap()); | ||
| let non_default_invoice_expiry_secs = 4200; | ||
|
|
||
| let invoice_params = Bolt11InvoiceParameters { | ||
| amount_msats: Some(10_000), | ||
| amount_msats: Some(amt_msat), | ||
| description, | ||
| invoice_expiry_delta_secs: Some(non_default_invoice_expiry_secs), | ||
| ..Default::default() | ||
| }; | ||
| let invoice = nodes[1].node.create_bolt11_invoice(invoice_params).unwrap(); | ||
| assert_eq!(invoice.amount_milli_satoshis(), Some(10_000)); | ||
| assert_eq!(invoice.amount_milli_satoshis(), Some(amt_msat)); | ||
| // If no `min_final_cltv_expiry_delta` is specified, then it should be `MIN_FINAL_CLTV_EXPIRY_DELTA`. | ||
| assert_eq!(invoice.min_final_cltv_expiry_delta(), MIN_FINAL_CLTV_EXPIRY_DELTA as u64); | ||
| assert_eq!( | ||
|
|
@@ -694,6 +694,11 @@ mod test { | |
| Duration::from_secs(non_default_invoice_expiry_secs.into()) | ||
| ); | ||
|
|
||
| let (payment_hash, payment_secret) = | ||
| (PaymentHash(invoice.payment_hash().to_byte_array()), *invoice.payment_secret()); | ||
|
|
||
| let preimage = nodes[1].node.get_payment_preimage(payment_hash, payment_secret).unwrap(); | ||
|
|
||
| // Invoice SCIDs should always use inbound SCID aliases over the real channel ID, if one is | ||
| // available. | ||
| let chan = &nodes[1].node.list_usable_channels()[0]; | ||
|
|
@@ -707,21 +712,34 @@ mod test { | |
| assert_eq!(invoice.route_hints()[0].0[0].htlc_minimum_msat, chan.inbound_htlc_minimum_msat); | ||
| assert_eq!(invoice.route_hints()[0].0[0].htlc_maximum_msat, chan.inbound_htlc_maximum_msat); | ||
|
|
||
| let retry = Retry::Attempts(0); | ||
| let custom_tlvs = vec![(65537, vec![42; 42])]; | ||
| let optional_params = OptionalPaymentParams { | ||
| custom_tlvs: custom_tlvs.clone(), | ||
| route_params_config: RouteParametersConfig::default(), | ||
| retry_strategy: Retry::Attempts(0), | ||
| }; | ||
|
|
||
| nodes[0] | ||
| .node | ||
| .pay_for_bolt11_invoice(&invoice, PaymentId([42; 32]), None, Default::default(), retry) | ||
| .pay_for_bolt11_invoice(&invoice, PaymentId([42; 32]), None, optional_params) | ||
| .unwrap(); | ||
| check_added_monitors(&nodes[0], 1); | ||
|
|
||
| let mut events = nodes[0].node.get_and_clear_pending_msg_events(); | ||
| assert_eq!(events.len(), 1); | ||
| let payment_event = SendEvent::from_event(events.remove(0)); | ||
| nodes[1].node.handle_update_add_htlc(node_a_id, &payment_event.msgs[0]); | ||
| nodes[1].node.handle_commitment_signed_batch_test(node_a_id, &payment_event.commitment_msg); | ||
| check_added_monitors(&nodes[1], 1); | ||
| let events = nodes[1].node.get_and_clear_pending_msg_events(); | ||
| assert_eq!(events.len(), 2); | ||
| let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); | ||
|
|
||
| let path = &[&nodes[1]]; | ||
| let args = PassAlongPathArgs::new(&nodes[0], path, amt_msat, payment_hash, ev) | ||
| .with_payment_preimage(preimage) | ||
| .with_payment_secret(payment_secret) | ||
| .with_custom_tlvs(custom_tlvs.clone()); | ||
|
|
||
| do_pass_along_path(args); | ||
| claim_payment_along_route( | ||
| ClaimAlongRouteArgs::new(&nodes[0], &[&[&nodes[1]]], preimage) | ||
| .with_custom_tlvs(custom_tlvs), | ||
| ); | ||
| } | ||
|
|
||
| fn do_create_invoice_min_final_cltv_delta(with_custom_delta: bool) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -543,6 +543,8 @@ pub enum RetryableSendFailure { | |
| /// | ||
| /// [`BlindedPaymentPath`]: crate::blinded_path::payment::BlindedPaymentPath | ||
| OnionPacketSizeExceeded, | ||
| /// The provided [`RecipientOnionFields::custom_tlvs`] are of invalid range | ||
| InvalidCustomTlvs, | ||
| } | ||
|
|
||
| /// If a payment fails to send to a route, it can be in one of several states. This enum is returned | ||
|
|
@@ -919,6 +921,7 @@ where | |
| pub(super) fn pay_for_bolt11_invoice<R: Deref, ES: Deref, NS: Deref, IH, SP>( | ||
| &self, invoice: &Bolt11Invoice, payment_id: PaymentId, | ||
| amount_msats: Option<u64>, | ||
| custom_tlvs: Vec<(u64, Vec<u8>)>, | ||
| route_params_config: RouteParametersConfig, | ||
| retry_strategy: Retry, | ||
|
Comment on lines
+924
to
926
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we pass |
||
| router: &R, | ||
|
|
@@ -942,7 +945,9 @@ where | |
| (None, None) => return Err(Bolt11PaymentError::InvalidAmount), | ||
| }; | ||
|
|
||
| let mut recipient_onion = RecipientOnionFields::secret_only(*invoice.payment_secret()); | ||
| let mut recipient_onion = RecipientOnionFields::secret_only(*invoice.payment_secret()) | ||
| .with_custom_tlvs(custom_tlvs) | ||
| .map_err(|_| Bolt11PaymentError::SendingFailed(RetryableSendFailure::InvalidCustomTlvs))?; | ||
| recipient_onion.payment_metadata = invoice.payment_metadata().map(|v| v.clone()); | ||
|
|
||
| let payment_params = PaymentParameters::from_bolt11_invoice(invoice) | ||
|
|
@@ -1061,6 +1066,7 @@ where | |
| RetryableSendFailure::RouteNotFound => PaymentFailureReason::RouteNotFound, | ||
| RetryableSendFailure::DuplicatePayment => PaymentFailureReason::UnexpectedError, | ||
| RetryableSendFailure::OnionPacketSizeExceeded => PaymentFailureReason::UnexpectedError, | ||
| RetryableSendFailure::InvalidCustomTlvs => PaymentFailureReason::UnexpectedError, | ||
| }; | ||
| self.abandon_payment(payment_id, reason, pending_events); | ||
| return Err(Bolt12PaymentError::SendingFailed(e)); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than making this public and adding a
RetryableSendFailure::InvalidCustomTlvs, can we have a constructor that enforces these TLVs are in the experimental range?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make a wrapper type for giving to
RecipientOnionFields::with_custom_tlvs?