Skip to content
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

Add max_total_routing_fee_msat for pay_invoice #2719

Closed

Conversation

benthecarman
Copy link
Contributor

@benthecarman benthecarman commented Nov 8, 2023

In #2417 support for setting a max fee was added but it is not exposed when using the pay_invoice utility functions. This makes it so the user can pass in that parameter. One downside is that this removes the default value form before.

If we want to keep the default there aren't many pretty options (either making pay_invoice_with_max_fee or having the param be Option<Option<u64>>) so I thought it'd be easier to just remove the default.

Also does a small cleanup I found

In lightningdevkit#2417 support for setting a max fee was added but it is not exposed
when using the `pay_invoice` utility functions. This makes it so the
user can pass in that parameter. One downside is that this removes the
defualt value form before.
@codecov-commenter
Copy link

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (b3e7aac) 88.75% compared to head (9f7733f) 88.80%.
Report is 35 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    #2719      +/-   ##
==========================================
+ Coverage   88.75%   88.80%   +0.05%     
==========================================
  Files         112      113       +1     
  Lines       88504    89172     +668     
  Branches    88504    89172     +668     
==========================================
+ Hits        78548    79186     +638     
- Misses       7717     7742      +25     
- Partials     2239     2244       +5     
Files Coverage Δ
lightning-invoice/src/payment.rs 77.61% <87.50%> (+0.54%) ⬆️

... and 25 files with indirect coverage changes

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

@tnull
Copy link
Contributor

tnull commented Nov 9, 2023

Mh, let's first discuss options and decide on a course of action in #2718 before we go ahead including it in the API like this. If we want to do it, we also may want to see if we want to more generally provide a way to override default route/payment parameters.

@TheBlueMatt
Copy link
Collaborator

How about #2727 instead?

@benthecarman benthecarman deleted the max-fee-pay-invoice branch November 11, 2023 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants