-
Notifications
You must be signed in to change notification settings - Fork 45
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 listchannels to execute_dev_command #949
base: main
Are you sure you want to change the base?
Conversation
Isn't it overkill to include If the goal is to extend dev commands with flexible calls that support optional parameters, then I suppose there's no way around it. But if we instead want to add selected command variants that are especially useful, maybe we can instead go for e.g.
What are your thoughts on this? |
cc @erdemyerebasmaz : this is also relevant for Breez Cloud. |
beb6875
to
69f350c
Compare
69f350c
to
c5d1696
Compare
My reasoning is: either we write parsing logic for parameterized commands ourselves, or we add a dependency to do so. The advantage is it maps to the enum fields easily. If we would do
I thought a cli parsing library was easier and more robust, also considering potential quoted strings with spaces (even though they are not part of the interface currently) |
The only other option I would see is to implement a nested subcommand into the CLI to handle the params specific to |
That would work too. It would be a breaking interface change, but maybe favorable over magic commandline strings? |
True, but the method should only be used for testing/debugging anyway |
@roeierez What do you think? |
What do you think about exposing the |
It does tie us to the core lightning implementation. Thinking about the 'LDK support' in the roadmap. Also I wonder how that plays with language bindings. I don't personally mind the maintenance burden of supporting this enum. |
We can do that without exposing the implementation by returning |
The existing implementation of execute_dev_command did not allow for parameterized commands. The following things were changed:
"listchannels --source xxx"
listchannels
function was added, with optional parameterssource
,destination
, andshort_channel_id
, so we are able to query the graph on greenlight