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

Reckless RPC Plugin #7506

Merged
merged 8 commits into from
Aug 13, 2024

Conversation

endothermicdev
Copy link
Collaborator

@endothermicdev endothermicdev commented Jul 30, 2024

Based on #7484 Merged

Adds a simple plugin to start a reckless process by rpc. Reads relevant network, lightning directory and config file information to invoke reckless with matching parameters.

@endothermicdev endothermicdev added this to the v24.08 milestone Jul 30, 2024
@endothermicdev endothermicdev force-pushed the reckless-rpc branch 7 times, most recently from 5237c0e to 172129a Compare August 2, 2024 14:06
@endothermicdev endothermicdev force-pushed the reckless-rpc branch 7 times, most recently from 8628794 to 34b6414 Compare August 7, 2024 20:55
@endothermicdev endothermicdev marked this pull request as ready for review August 7, 2024 20:55
@endothermicdev endothermicdev requested a review from cdecker as a code owner August 7, 2024 20:55
Trying a single command first - reckless-search to test
launching a reckless process and processing the result.
This would interupt most commands during the config reading
step until the user accepts the prompt to create a new empty
config.
This allows generic subcommands to be passed to reckless-rpc along with
the target to search/install/etc..  These commands are unvalidated so
far and may crash the reckless process.

Changelog-Added: reckless-rpc plugin: issue commands to reckless over rpc.
If the rpc plugin is run while an older version of reckless is found on
PATH, it produces an error:

2024-08-01T18:32:00.849Z DEBUG   plugin-recklessrpc: reckless-stderr:usage: reckless [-h] [-d RECKLESS_DIR] [-l LIGHTNING] [-c CONF] [-r] [--network NETWORK] [-v]
{install,uninstall,search,enable,disable,source,help} ...
reckless: error: unrecognized arguments: --json

Catch this and don't try to parse the output as json.
... before processing the output. It's probably a python backtrace, not
json anyway.
Copy link
Collaborator

@ShahanaFarooqui ShahanaFarooqui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Alex, amazing PR. Already looking forward to use reckless in the UI.

Documentation and code both seem pretty clear to me. Don't have any input on that but below are some of my observations from testing the rpc:

1: Incorrect usage throws crash error. For example:
reckless --regtest -c ~/l1/.lightning/config help source shows replies with a neat try "reckless source -h" message but
cln-cli --regtest --lightning-dir=~/l1/.lightning reckless help source throws the reckless process has crashed error.

reckless --regtest -c ~/l1/.lightning/config source help returns proper usage details but
cln-cli --regtest --lightning-dir=~/l1/.lightning reckless source help return the reckless process has crashed.

2: Uninstalling a non-installed plugin throws the reckless process has crashed in rpc but it also fails with reckless plugin:

[2024-08-09 20:23:10,907] WARNING: failed to disable: Could not find a reckless directory for sling
Traceback (most recent call last):
  File "/usr/local/bin/reckless", line 1803, in <module>
    log.add_result(args.func(target))
  File "/usr/local/bin/reckless", line 1343, in uninstall
    inst = InferInstall(plugin_name)
  File "/usr/local/bin/reckless", line 806, in __init__
    raise NotFoundError("Could not find a reckless directory "
__main__.NotFoundError: Could not find a reckless directory for sling

3: lightning's help with -h:
reckless --regtest -c ~/l1/.lightning/config source -h returns proper usage details but
cln-cli --regtest --lightning-dir=~/l1/.lightning reckless source -h shows lightning-cli help instead.

3: Result from re-enabling an already enable plugin:
{
"result": [
"null"
],
"log": [
"DEBUG: activating sling",
"ERROR: reckless: sling failed to start!",
"ERROR: CLIError(-32602 ~/l1/.lightning/reckless/sling/sling: Invalid argument)"
]
}

4: Installing a disabled plugin should logically just enable it. But reckless goes for re-installation and throws [Errno 17] File exists error after the process is finished.

Running two installation symoultaneously returns invalid character in json output error:

{
   "code": -3,
   "message": "reckless returned invalid character in json output"
}
plugin-recklessrpc: Reckless subprocess complete: {\n   \"result\": [\n      null\n   ],\n   \"log\": [\n      \"DEBUG: Searching for sling\",\n      \"DEBUG: fetching from gh API: https://api.github.com/repos/lightningd/plugins/contents/\",\n      \"DEBUG: fetching from gh API: https://api.github.com/repos/lightningd/plugins/git/trees/294f93d7060799439c994daa84f534c4d1458325\",\n      \"INFO: found sling in source: https://github.com/lightningd/plugins\",\n      \"DEBUG: entry: None\",\n      \"DEBUG: sub-directory: sling\",\n      \"DEBUG: Retrieving sling from https://github.com/lightningd/plugins\",\n      \"DEBUG: Install requested from InstInfo(sling, https://github.com/lightningd/plugins, None, None, None, sling).\",\n      \"INFO: cloning Source.GITHUB_REPO InstInfo(sling, https://github.com/lightningd/plugins, None, None, None, sling)\",\n      \"DEBUG: cloned_src: InstInfo(sling, /tmp/reckless-533844684wlhxlleh/clone, None, Cargo.toml, Cargo.toml, sling/sling)\",\n      \"DEBUG: using latest commit of default branch\",\n      \"DEBUG: checked out HEAD: 8c4579982abb899e5af480a942d6d4ecd1cee16d\",\n      \"DEBUG: using installer rust\",\n      \"DEBUG: copying /tmp/reckless-533844684wlhxlleh/clone/sling/sling tree to ~/l1/.lightning/reckless/sling/source/sling\",\n      \"ERROR: [Errno 17] File exists: '~/l1/.lightning/reckless/sling/source/sling'\"\n   ]\n}\n\u0001

5: How can we recognize if a plugin is dynamic or not?
Re-enabling a disabled dynamic plugin returns error message "ERROR: CLIError(-3 ~/l1/.lightning/reckless/cln-ntfy/source/cln-ntfy/target/release/cln-ntfy: Not a dynamic plugin)".
With that information, we can remove the disable option also for the plugin in the UI.

6: plugins/reckless is not added in .gitignore.

@rustyrussell
Copy link
Contributor

5: How can we recognize if a plugin is dynamic or not?
Re-enabling a disabled dynamic plugin returns error message "ERROR: CLIError(-3 ~/l1/.lightning/reckless/cln-ntfy/source/cln-ntfy/target/release/cln-ntfy: Not a dynamic plugin)".
With that information, we can remove the disable option also for the plugin in the UI.

"plugins list" has a boolean field "dynamic" for this.

@ShahanaFarooqui
Copy link
Collaborator

ACK eda0129

@ShahanaFarooqui ShahanaFarooqui merged commit 88504ea into ElementsProject:master Aug 13, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants