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

coin select: add coin selection strategy option to all on-chain RPCs #8498

Closed
guggero opened this issue Feb 23, 2024 · 3 comments · Fixed by #8515
Closed

coin select: add coin selection strategy option to all on-chain RPCs #8498

guggero opened this issue Feb 23, 2024 · 3 comments · Fixed by #8515
Labels
beginner Issues suitable for new developers cli Related to the command line interface coin selection good first issue Issues suitable for first time contributors to LND P3 might get fixed, nice to have rpc Related to the RPC interface

Comments

@guggero
Copy link
Collaborator

guggero commented Feb 23, 2024

Comment from #8378:

          Yeah, I think that makes sense. Though it feels like that would be a different PR, as we'd probably also want to add such new parameters to the existing RPCs that currently use the config-level strategy (`FundPsbt`, `EstimateFee`, `SendCoins`, `SendOutputs`).

Originally posted by @guggero in #8378 (comment)

Currently, the coin selection strategy can only be configured globally, via the --coin-selection-strategy=[largest|random] config/CLI option.
But because the strategy might be something a user wants to decide upon for every spend, a similar option should be added to all on-chain RPCs such as the ones mentioned in the original comment above.

@guggero guggero added rpc Related to the RPC interface beginner Issues suitable for new developers good first issue Issues suitable for first time contributors to LND cli Related to the command line interface coin selection labels Feb 23, 2024
@saubyk saubyk added the P3 might get fixed, nice to have label Feb 26, 2024
@mohamedawnallah
Copy link
Contributor

mohamedawnallah commented Mar 6, 2024

@guggero I found there is no CLI command for the SendOutputs RPC as in the code and Lightning Engineering API Docs - SendOutputs RPC Call. Is it okay to submit a PR for adding lncli sendoutputs command + configuring the coin selection strategy option?

@guggero
Copy link
Collaborator Author

guggero commented Mar 6, 2024

I don't think that is needed. That would require you to specify the pk scripts on the command line, which is kind of awkward. I think in the CLI the sendcoins command should cover all requirements. Or why exactly do you think we'd need a new command?
Also, please keep in mind that reviewing PRs, especially from new contributors, takes a lot of time from the maintainers. So if you really want to support the project, I'd suggest to also start looking into testing and reviewing pull requests from other people instead of writing more code.

@mohamedawnallah
Copy link
Contributor

Got it @guggero! Thanks for clarifying :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beginner Issues suitable for new developers cli Related to the command line interface coin selection good first issue Issues suitable for first time contributors to LND P3 might get fixed, nice to have rpc Related to the RPC interface
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants