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

Invalid flags after payinvoice's pay_req are being ignored #9440

Open
arturgontijo opened this issue Jan 23, 2025 · 2 comments
Open

Invalid flags after payinvoice's pay_req are being ignored #9440

arturgontijo opened this issue Jan 23, 2025 · 2 comments
Labels
enhancement Improvements to existing features / behaviour

Comments

@arturgontijo
Copy link

arturgontijo commented Jan 23, 2025

Background

Flags after lncli payinvoice [command options] pay_req are being ignored.

Your environment

  • version of lnd: lnd-darwin-arm64-v0.18.4-beta
  • which operating system (uname -a on *Nix): Darwin 24.2.0
  • version of btcd, bitcoind, or other backend: [email protected]
  • any other relevant environment details

Steps to reproduce

These fail as expected:

lncli -n=signet payinvoice --last_hop=123 lntbs60030...hj32cppyeeac
>>> [lncli] invalid vertex string length of 3, want 66

lncli -n=signet payinvoice lntbs60030...hj32cppyeeac --last_hop=123
>>> [lncli] invalid vertex string length of 3, want 66

lncli -n=signet payinvoice --last_hop_non_existing=123 lntbs60030...hj32cppyeeac
>>> [lncli] flag provided but not defined: -last_hop_non_existing

But this doesn't:

lncli -n=signet payinvoice lntbs60030...hj32cppyeeac --last_hop_non_existing=123
>>> Payment hash: ...
Description: ...
Amount (in satoshis): 1
Fee limit (in satoshis): 300
Destination: ...
Confirm payment (yes/no): no
[lncli] payment not confirmed

Expected behaviour

Shouldn't we do the same check as we do for sendpayment here?

lncli -n=signet sendpayment --last_hop=123 --pay_req=lntbs60030...hj32cppyeeac
>>> [lncli] invalid vertex string length of 3, want 66

lncli -n=signet sendpayment --pay_req=lntbs60030...hj32cppyeeac --last_hop=123
>>> [lncli] invalid vertex string length of 3, want 66

lncli -n=signet sendpayment --last_hop_non_existing=123 --pay_req=lntbs60030...hj32cppyeeac
>>> [lncli] flag provided but not defined: -last_hop_non_existing

lncli -n=signet sendpayment --pay_req=lntbs60030...hj32cppyeeac --last_hop_non_existing=123
>>> [lncli] flag provided but not defined: -last_hop_non_existing

I think we should output the same error for payinvoice, eg:

lncli -n=signet payinvoice lntbs60030...hj32cppyeeac --last_hop_non_existing=123
>>> [lncli] flag provided but not defined: -last_hop_non_existing

If it makes sense, I can try to implement that check.

@arturgontijo arturgontijo added bug Unintended code behaviour needs triage labels Jan 23, 2025
@arturgontijo arturgontijo changed the title [bug]: [bug]: Invalid flags after payinvoice's pay_req are being ignored Jan 23, 2025
@ViktorTigerstrom
Copy link
Collaborator

The behavior you're describing for payinvoice actually applies to sendpayment as well. For example, if you use the sendpayment command like this:

lncli sendpayment "024e44d23895414408da994c33b790bda76fc138ec6fef3663d786c2fcf524ff96" 100000 "da64ecb9ac16f2b4b60daccf546e1c82f406931ea5b0a097837a00c0c189cf43" 120 "c55d13d3e1d72e3ec049a68b63fb42893581bd3125efad217e12dea5b0639056" --last_hop_non_existing=123

You’ll notice that the --last_hop_non_existing=123 flag is ignored for this command as well. The issue lies in how lncli differentiates between command options (marked with --) and command arguments (passed without --). These are handled and extracted differently in the lncli logic.

As specified in the lncli command descriptions:

lncli payinvoice [command options] pay_req

and

lncli sendpayment [command options] dest amt payment_hash final_cltv_delta pay_addr | --pay_req=R [--pay_addr=H]

it’s described that command options should be included before the command arguments. Therefore, I wouldn’t classify this as a bug. Instead, it appears to be expected behavior based on the way the commands are designed. I'll update this issue by removing the "bug" label and marking it as a potential enhancement instead.

If you'd like to work on a solution that detects non-existing option flags provided after the command arguments, you’re of course more than welcome to do so! :)

@ViktorTigerstrom ViktorTigerstrom added enhancement Improvements to existing features / behaviour and removed bug Unintended code behaviour needs triage labels Jan 27, 2025
@arturgontijo arturgontijo changed the title [bug]: Invalid flags after payinvoice's pay_req are being ignored Invalid flags after payinvoice's pay_req are being ignored Jan 27, 2025
@arturgontijo
Copy link
Author

Oh I see, makes sense!
Ok, I'll take a look on that logic and see if I'm able to come up with something that covers that case.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features / behaviour
Projects
None yet
Development

No branches or pull requests

2 participants