plugin and parameter enhancements - #2148
Merged
rustyrussell merged 13 commits intoDec 10, 2018
Merged
Conversation
ZmnSCPxj
approved these changes
Dec 7, 2018
ZmnSCPxj
left a comment
ZmnSCPxj
left a comment
Contributor
There was a problem hiding this comment.
did not review much, but changes to invoice and payalgo are minimal.
wythe
reviewed
Dec 7, 2018
| { | ||
| *failed = true; | ||
| } | ||
|
|
rustyrussell
force-pushed
the
guilt/pay-plugin
branch
from
December 8, 2018 00:39
84f28f8 to
6c1f22a
Compare
Contributor
Author
|
Trivial rebase on top of master now requirements have been merged... |
rustyrussell
changed the title
Member
|
ACK c31e59d |
Check if it's a normal file first, then check permissions. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This currently just invokes GDB, but we could generalize it (though pdb doesn't allow attaching to a running process, other python debuggers seem to). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Note that this changes the order of arguments to pipecmd to match the documentation, so we fix all the callers! Also make configure re-run when configurator changes. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we've updated ccan/pipecmd, we can use pipecmd_preserve to preserve stderr for plugins so we see their error spew. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This (will) avoid the plugin having to walk back from the params object as it currently does. No code changes; I removed UNUSED and UNNEEDED labels from the other parameters though (as *every* json_rpc callback needs to call param() these days, they're *always* used). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need these for literal copying of requests between plugin and client. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…them (much). We simply look for the id token, and substitute it on the way in/out. We also need to make sure output is '\n\n' terminated. I started this because we weren't forwarding complex errors properly (we treated them as a string), but it's also a huge simplification. `struct plugin_rpc_request` is eliminated entirely: the information we need is actually inside `struct command` already. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
All callers currently just hand the same arg twice, but plugins might want this different. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
json_tok* is used with 'struct command', so rename this to match the other low-level json tok helpers. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I want to use param functions in plugins, and they don't have struct command. I had to use a special arg to param() for check to flag it as allowing extra parameters, rather than adding a one-use accessor. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can use the 'destructor-canary' trick instead. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
json_escaped.[ch], param.[ch] and jsonrpc_errors.h move from lightningd/ to common/. Tests moved too. We add a new 'common/json_tok.[ch]' for the common parameter parsing routines which a plugin might want, taking them out of lightningd/json.c (which now only contains the lightningd-specific ones). The rest is mainly fixing up includes. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
cdecker
force-pushed
the
guilt/pay-plugin
branch
from
December 9, 2018 16:31
6c1f22a to
c31e59d
Compare
wythe
reviewed
Dec 9, 2018
|
|
||
| #endif /* LIGHTNING_LIGHTNINGD_PARAM_H */ | ||
| /* Special flag for 'check' which allows any parameters. */ | ||
| #define p_opt_any() "", false, NULL, NULL |
Contributor
There was a problem hiding this comment.
Nicely done. This approach crossed my mind at some point but then it was gone. Its clearly better 👍
Contributor
|
ACK c31e59d param/json stuff looks great. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Still a WIP, since it's based on a merge of master and #2141 which I both needed.Goodies inside:
@wythe please take a look too, since some disentangling was required!
(BTW, please accept https://github.com/ElementsProject/lightning/invitations so I can tag you for review and bug you in future :)