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

Expose json rpc config to plugins through JsonRpcConfigurationService #2704

Closed
wants to merge 3 commits into from

Conversation

antonydenyer
Copy link
Member

PR description

Expose json rpc config to plugins through JsonRpcConfigurationService.
This is instead of exposing generic untyped arguments as explored in #2661

Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
@antonydenyer antonydenyer marked this pull request as draft August 31, 2021 15:24
@antonydenyer antonydenyer marked this pull request as ready for review August 31, 2021 15:56
*
* @return list of APIs enabled on JSON-RPC HTTP service
*/
Collection<String> getJsonRpcApis();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you also need cors host and allowlist?

Copy link
Member Author

@antonydenyer antonydenyer Sep 1, 2021

Choose a reason for hiding this comment

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

I did ponder that, would you need it? If you're inside the plugin running on the same machine I think all you need to know is what address it's bound to.

Now, you might need all the tls info, but then we're down the path of exposing all the cert stuff too, at that point you may as well just pass over JsonRpcConfiguration. The difficulty with this is that it's built and validated after the register method is called in the plugins.

I think the root cause is #2703 and the bootstrapping process of besu should be something like:

  • load besu config
  • load plugin config
  • clean/normalise config (cast to types etc)
  • validate options (e.g miner-enabled with no coinbase address)
  • make normalised config available to besu and plugins
    ...

but, this addresses what I immediately need.

…nfig

Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
@vmichalik vmichalik added the enhancement New feature or request label Sep 1, 2021
@vmichalik
Copy link

btw if you hadn't seen it already, #1317 requests this functionality

…nfig

Signed-off-by: Antony Denyer <git@antonydenyer.co.uk>
@antonydenyer
Copy link
Member Author

I ended up working around the issue in the end by getting what I wanted from picocli. It's a hack and relies on us continuing to use picocli.

@Option(
      names = {"--plugin-permissioning-rpc-http-port"},
      defaultValue = "--rpc-http-port",
      hidden = true,
      preprocessor = BesuPreprocessor.class)
  private Integer port;


public static class BesuPreprocessor implements CommandLine.IParameterPreprocessor {
    @Override
    public boolean preprocess(
        final Stack<String> args,
        final CommandLine.Model.CommandSpec commandSpec,
        final CommandLine.Model.ArgSpec argSpec,
        final Map<String, Object> info) {
      argSpec.setValue(commandSpec.findOption(argSpec.defaultValue()).getValue());
      return true;
    }
  }

I don't think we should document this as 'the way to do it' but leaving here for reference until an agreement can be reached on how to expose config to plugins.

@antonydenyer antonydenyer deleted the plugin-rpc-config branch September 7, 2021 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants