-
Notifications
You must be signed in to change notification settings - Fork 961
Libplugin improvements #7750
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
Merged
rustyrussell
merged 18 commits into
ElementsProject:master
from
rustyrussell:libplugin-improvements
Nov 7, 2024
Merged
Libplugin improvements #7750
rustyrussell
merged 18 commits into
ElementsProject:master
from
rustyrussell:libplugin-improvements
Nov 7, 2024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
392bc7f to
c765ab8
Compare
adi2011
reviewed
Oct 30, 2024
c765ab8 to
8a7cee5
Compare
35b3aee to
f63de5e
Compare
7fdfe72 to
f63de5e
Compare
f63de5e to
36c9bce
Compare
And fix the fallout! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Sometimes we want to clean up *after* a command has completed, but we're moving to a model where all libplugin operations require a `struct command`. This adds `aux_command` to create an independent-lifetime command with the same id. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We didn't set this previously when it was a notification. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is clearer than the previous "two booleans". Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is cleaner: everything can now be associated with a command context. You're supposed to eventually dispose of it using timer_complete(). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
All `struct command` should be terminated properly. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This does not code changes, but makes the next changes easier. We short-cut the "we are a child" case and de-indent the main cases. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means we replace p->cmd with an auxillary command after we've finished, so we have a valid command to use. It also means we weave `struct command_result` returns back through all the callers. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were dereferencing the first character of the id, (always '"') which meant everything was id 34. Before: plugin-pay: cmd 34 partid 5 After: cmd pytest:pay#62/cln:pay#105 partid 0 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Fixed: `pay`: debug logging now uses correct JSON ids.
And remove command_done() which was used when there was no cmd. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ng/complete. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When we used to allow cmd to be NULL, we had to hand the plugin everywhere. We no longer do. 1. Various jsonrpc_ functions no longer need the plugin arg. 2. send_outreq no longer needs a plugin arg. 3. The init function takes a command, not a plugin. 4. Remove command_deprecated_in_nocmd_ok. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
36c9bce to
d906205
Compare
This avoids jsonrpc_request_start() double-evaluating its cmd arg. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…helpers. Without knowing what method was called, we can't have useful general logging methods, so go through the pain of adding "const char *method" everywhere, and add: 1. ignore_and_complete - we're done when jsonrpc returned 2. log_broken_and_complete - we're done, but emit BROKEN log. 3. plugin_broken_cb - if this happens, fail the plugin. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…command hook. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Fixed: libplugin: plugins can now intercept `rpc_command` hook without deadlocking.
On `dev-memleak`, if someone is using rpc_command_hook, we'll call it when the hook returns. But it will see these contexts as a leak. So attach them to tmpctx (which is excluded from leak detection). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
d906205 to
0b5f37d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes libplugin to always have (and require!) a non-NULL
struct commandcontext for operations. This is usually easy, but required some work for pay, which didn't pass the command_result codes though.Also, a driveby fix for pay debug logs (not everything is command 34!).