[nc-1756] jsonrpc enabling #91
Conversation
| package tech.pegasys.pantheon.ethereum.jsonrpc.rpcapi; | ||
|
|
||
| public interface RpcApi { | ||
| String name(); |
There was a problem hiding this comment.
This isn't really the name, it's the CLI value for turning the service on/off.
A nice future feature would be to have this different depending on the method of interaction i.e. Pantheon CLI / other API(s)
There was a problem hiding this comment.
forgetting the latter part, does renaming it to cliValue() work?
| } | ||
|
|
||
| public static final String getValue(final RpcApi rpcapi) { | ||
| return rpcapi.name(); |
There was a problem hiding this comment.
Doesn't this need to lowercase the name to duplicate the original logic? (as the lowercase name is used in the CLI)
There was a problem hiding this comment.
from the docs it looked like the uppercase was used, just we have logic in place that normalises it to the uppercase
| public Map<String, JsonRpcMethod> methods( | ||
| final ProtocolContext<CliqueContext> context, final Collection<RpcApi> jsonRpcApis) { | ||
| final Map<String, JsonRpcMethod> rpcMethods = new HashMap<>(); | ||
| if (!jsonRpcApis.contains(CliqueRpcApis.CLIQUE)) { |
There was a problem hiding this comment.
For this to be reliable, does there need to be only a single instance of the CliqueRpcApis throughout the system? ...and there doesn't seem to a guard on creating a Clique instance (I'm thinking otherwise the contains() is using equals(), which is the default implementation on RpcApi)
There was a problem hiding this comment.
yup. Now that we're using RpcApi("NAME") I've got an equals & hashcode on RpcApi that fixes the problem
| Arrays.asList(RpcApis.ETH, RpcApis.NET, RpcApis.WEB3); | ||
|
|
||
| private boolean enabled; | ||
| private int port; | ||
| private String host; | ||
| private Collection<String> corsAllowedDomains = Collections.emptyList(); | ||
| private Collection<RpcApis> rpcApis; | ||
|
|
||
| public enum RpcApis { |
There was a problem hiding this comment.
not sure I understand the benefit in replacing the enum with an interface in this case, what benefit does the new type provide?
There was a problem hiding this comment.
that each module can define its own RPC APIs enable options by implementing the RpcApi interface.
With the enum, you are obliged to define all of the possible values here
There was a problem hiding this comment.
as per Roberto. Then as per our discussion I'm refactoring to rather than lots of class instances just the RpcApis packages creating sets of RpcApi("NAME") objects, then having an equals/hashcode that knows how to deal with that
PR Details
Clique and Ibft need to be toggle'able by the cli, adds 'IBFT' & 'CLIQUE' as options for --rpc-api and --ws-api
Description
Needed to refactor the way RpcApi's get listed so we could use them across new modules, bunch of changes to support that.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Related Issue
Motivation and Context
Ibft & Clique json rpc should work like the mainnet json rpc
Types of changes
Special notes for your reviewer:
Release note
Prerequisites: