Skip to content

[bug]: fee_limit_percent missing in SendPaymentV2 #7832

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

Jump to bottom
Open
AndySchroder opened this issue Jul 13, 2023 · 5 comments · May be fixed by #9671
Open

[bug]: fee_limit_percent missing in SendPaymentV2 #7832

AndySchroder opened this issue Jul 13, 2023 · 5 comments · May be fixed by #9671
Labels
bug Unintended code behaviour good first issue Issues suitable for first time contributors to LND P2 should be fixed if one has time rpc Related to the RPC interface

Comments

@AndySchroder
Copy link

SendPayment has a FeeLimit field as part of the request that contains (fixed or fixed_msat) and (percent) values. SendPayment is also listed as depreciated and that SendPaymentV2 should be used instead. SendPaymentV2 does not have a FeeLimit field as part of the request, but instead has a fee_limit_sat or fee_limit_msat field and no fee_limit_percent.

I'm not sure why SendPayment is depreciated over SendPaymentV2 (I can't notice any meaningful differences) and I don't understand why the way fee limits are specified has changed, but we should still have the ability to specify the fee limit in terms of a percentage.
@AndySchroder AndySchroder added bug Unintended code behaviour needs triage labels Jul 13, 2023
@AndySchroder
Copy link
Author

Please consider #7835 when fixing this. It's likely you won't fix #7835 since SendPayment is listed as depreciated, but I don't want the same problem to exist when you fix this issue.

@saubyk saubyk moved this to 🔖 P2 in Issue Triage Jul 18, 2023
@saubyk saubyk added rpc Related to the RPC interface P2 should be fixed if one has time and removed needs triage labels Aug 9, 2023
@saubyk saubyk removed this from Issue Triage Aug 9, 2023
@ziggie1984 ziggie1984 added the good first issue Issues suitable for first time contributors to LND label Nov 19, 2024
@ZZiigguurraatt
Copy link

When implementing this, it might be a good idea to set a default value for fee_limit_percent to 1%. At the same time, set the default fee_limit_sat to 5 sat instead of 0sat. This will allow for reliably making small test payments as well as most payments without risk of too high of a fee.

@ZZiigguurraatt ZZiigguurraatt mentioned this issue Nov 19, 2024
Open
@ZZiigguurraatt ZZiigguurraatt mentioned this issue Dec 6, 2024
Open
@ZZiigguurraatt
Copy link

When implementing this, it might be a good idea to set a default value for fee_limit_percent to 1%. At the same time, set the default fee_limit_sat to 5 sat instead of 0sat. This will allow for reliably making small test payments as well as most payments without risk of too high of a fee.

Actually, these values should be the default defaults, but we should be able to override the default defaults in lnd.conf.

This was referenced Feb 6, 2025
Closed
Open
@PsychoPunkSage
Copy link

Hi everyone.
Is this issue still available? I would like to take up this issue

@Lokeshranjan8
Copy link

Hey, if this issue is available, can I try this?

@asmit27rai asmit27rai linked a pull request Apr 2, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Unintended code behaviour good first issue Issues suitable for first time contributors to LND P2 should be fixed if one has time rpc Related to the RPC interface
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

routerrpc: fix fee limit validation and calculation asmit27rai/lnd
6 participants
@AndySchroder @saubyk @ziggie1984 @PsychoPunkSage @Lokeshranjan8 @ZZiigguurraatt