refactored `payment_parameters_from_zero_amount_invoice` to `amount-less` #2979

srikanth-iyengar · 2024-04-01T17:05:30Z

Fix #2879
Refactored payment_parameters_from_zero_amount_invoice to amount-less as per the discussion in the issue

shaavan

The changes look good. Just one small nit.

I think it will be a good idea to amend the payment_parameters_from_amount_less_invoice documentation so that readers can understand that zero-amount invoices are synonymous with amount-less invoices in this context.

srikanth-iyengar · 2024-04-02T16:13:08Z

The changes look good. Just one small nit.

I think it will be a good idea to amend the payment_parameters_from_amount_less_invoice documentation so that readers can understand that zero-amount invoices are synonymous with amount-less invoices in this context.

Ohk will add the documentation

srikanth-iyengar · 2024-04-02T16:31:55Z

@shaavan should I add the doc just below this ?

rust-lightning/lightning-invoice/src/payment.rs

Lines 19 to 28 in 9325070

    
           /// Builds the necessary parameters to pay or pre-flight probe the given zero-amount 
        
           /// [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or 
        
           /// [`ChannelManager::send_preflight_probes`]. 
        
           /// 
        
           /// Prior to paying, you must ensure that the [`Bolt11Invoice::payment_hash`] is unique and the 
        
           /// same [`PaymentHash`] has never been paid before. 
        
           /// 
        
           /// Will always succeed unless the invoice has an amount specified, in which case 
        
           /// [`payment_parameters_from_invoice`] should be used. 
        
           ///

shaavan · 2024-04-03T05:23:16Z

I was thinking maybe something simple like this may also work

 /// Builds the necessary parameters to pay or pre-flight probe the given zero-amount
-/// [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or
+/// (amount-less) [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or
 /// [`ChannelManager::send_preflight_probes`].

But feel free to handle it in whichever way you prefer!

srikanth-iyengar · 2024-04-03T05:24:39Z

Got it 👍

TheBlueMatt · 2024-04-05T10:22:42Z

lightning-invoice/src/payment.rs

@@ -28,7 +28,7 @@ use lightning::routing::router::{PaymentParameters, RouteParameters};
 ///
 /// [`ChannelManager::send_payment`]: lightning::ln::channelmanager::ChannelManager::send_payment
 /// [`ChannelManager::send_preflight_probes`]: lightning::ln::channelmanager::ChannelManager::send_preflight_probes
-pub fn payment_parameters_from_zero_amount_invoice(invoice: &Bolt11Invoice, amount_msat: u64)
+pub fn payment_parameters_from_amount_less_invoice(invoice: &Bolt11Invoice, amount_msat: u64)


Should be either amountless (one word) or no_amount, I think.

refactoring to amountless

codecov-commenter · 2024-04-05T12:28:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.08%. Comparing base (51926f1) to head (e9d9a81).
Report is 105 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2979      +/-   ##
==========================================
- Coverage   89.38%   89.08%   -0.30%     
==========================================
  Files         117      118       +1     
  Lines       95513    97492    +1979     
  Branches    95513    97492    +1979     
==========================================
+ Hits        85376    86854    +1478     
- Misses       7913     8387     +474     
- Partials     2224     2251      +27

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

shaavan · 2024-04-06T11:45:34Z

lightning-invoice/src/payment.rs

+/// An amount-less invoice, also known as a zero-amount invoice, refers to an invoice
+/// where the amount to be paid is zero. In this context, both terms are used synonymously.


I think the added docs are conveying the wrong message here.
A zero-amount invoice doesn't mean the amount to be paid is zero, but rather any arbitrary amount can be paid by the sender. For context, this answer might be helpful.

Also, FWIW, we don't need to explain the meaning of zero-amount invoice here, as the users of this function would most probably be aware of it.
That's why, I think a simple indication that zero-amount ≈ amountless will suffice here.

Got it will change the docs accordingly

lightning-invoice/src/payment.rs

TheBlueMatt · 2024-04-09T00:07:38Z

lightning-invoice/src/payment.rs

@@ -17,7 +17,7 @@ use lightning::ln::channelmanager::RecipientOnionFields;
 use lightning::routing::router::{PaymentParameters, RouteParameters};

 /// Builds the necessary parameters to pay or pre-flight probe the given zero-amount
-/// [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or
+/// (amountless) [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or


Given the functions are called amountless, maybe we should say "amountless (sometimes referred to as zero-amount)"?

- /// [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or + /// amountless (sometimes referred to as zero-amount) [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or

like this ?

TheBlueMatt · 2024-04-11T13:31:30Z

Please make your commit message more descriptive, see https://cbea.ms/git-commit/ for some guidelines.

TheBlueMatt · 2024-04-19T22:46:48Z

lightning-invoice/src/payment.rs

@@ -17,7 +17,7 @@ use lightning::ln::channelmanager::RecipientOnionFields;
 use lightning::routing::router::{PaymentParameters, RouteParameters};

 /// Builds the necessary parameters to pay or pre-flight probe the given zero-amount
-/// [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or
+/// amountless (sometimes referred to as zero-amount) [`Bolt11Invoice`] using [`ChannelManager::send_payment`] or


This now reads "zero-amount amountless (sometimes referred to as zero-amount)"

refactored the code related to zero_amount in the lightning payment invoice to use the term "amountless" instead

shaavan reviewed Apr 2, 2024

View reviewed changes

TheBlueMatt reviewed Apr 5, 2024

View reviewed changes

shaavan reviewed Apr 6, 2024

View reviewed changes

srikanth-iyengar force-pushed the refactor/2879 branch from 6442618 to 0b07165 Compare April 6, 2024 13:13

shaavan reviewed Apr 6, 2024

View reviewed changes

lightning-invoice/src/payment.rs Outdated Show resolved Hide resolved

srikanth-iyengar force-pushed the refactor/2879 branch 2 times, most recently from 62b0fa9 to e897bf3 Compare April 7, 2024 01:27

TheBlueMatt reviewed Apr 9, 2024

View reviewed changes

srikanth-iyengar force-pushed the refactor/2879 branch 3 times, most recently from bac2f07 to 1c75d98 Compare April 16, 2024 02:28

TheBlueMatt reviewed Apr 19, 2024

View reviewed changes

srikanth-iyengar force-pushed the refactor/2879 branch from 1c75d98 to c9ea33f Compare April 20, 2024 02:02

Refactor zero_amount to imply amountless

e9d9a81

refactored the code related to zero_amount in the lightning payment invoice to use the term "amountless" instead

srikanth-iyengar force-pushed the refactor/2879 branch from c9ea33f to e9d9a81 Compare April 20, 2024 11:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactored `payment_parameters_from_zero_amount_invoice` to `amount-less` #2979

refactored `payment_parameters_from_zero_amount_invoice` to `amount-less` #2979

srikanth-iyengar commented Apr 1, 2024

shaavan left a comment

srikanth-iyengar commented Apr 2, 2024

srikanth-iyengar commented Apr 2, 2024

shaavan commented Apr 3, 2024

srikanth-iyengar commented Apr 3, 2024

TheBlueMatt Apr 5, 2024

srikanth-iyengar Apr 5, 2024

codecov-commenter commented Apr 5, 2024 •

edited

shaavan Apr 6, 2024

srikanth-iyengar Apr 6, 2024

TheBlueMatt Apr 9, 2024

srikanth-iyengar Apr 9, 2024 •

edited

TheBlueMatt Apr 11, 2024

TheBlueMatt commented Apr 11, 2024

TheBlueMatt Apr 19, 2024

		/// An amount-less invoice, also known as a zero-amount invoice, refers to an invoice
		/// where the amount to be paid is zero. In this context, both terms are used synonymously.

refactored payment_parameters_from_zero_amount_invoice to amount-less #2979

Are you sure you want to change the base?

refactored payment_parameters_from_zero_amount_invoice to amount-less #2979

Conversation

srikanth-iyengar commented Apr 1, 2024

shaavan left a comment

Choose a reason for hiding this comment

srikanth-iyengar commented Apr 2, 2024

srikanth-iyengar commented Apr 2, 2024

shaavan commented Apr 3, 2024

srikanth-iyengar commented Apr 3, 2024

TheBlueMatt Apr 5, 2024

Choose a reason for hiding this comment

srikanth-iyengar Apr 5, 2024

Choose a reason for hiding this comment

codecov-commenter commented Apr 5, 2024 • edited

Codecov Report

shaavan Apr 6, 2024

Choose a reason for hiding this comment

srikanth-iyengar Apr 6, 2024

Choose a reason for hiding this comment

TheBlueMatt Apr 9, 2024

Choose a reason for hiding this comment

srikanth-iyengar Apr 9, 2024 • edited

Choose a reason for hiding this comment

TheBlueMatt Apr 11, 2024

Choose a reason for hiding this comment

TheBlueMatt commented Apr 11, 2024

TheBlueMatt Apr 19, 2024

Choose a reason for hiding this comment

refactored `payment_parameters_from_zero_amount_invoice` to `amount-less` #2979

refactored `payment_parameters_from_zero_amount_invoice` to `amount-less` #2979

codecov-commenter commented Apr 5, 2024 •

edited

srikanth-iyengar Apr 9, 2024 •

edited