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

Client: Expose Engine API as a separate Server with a dedicated Port Number #1504

Closed
holgerd77 opened this issue Oct 1, 2021 · 3 comments
Closed

Comments

@holgerd77
Copy link
Member

holgerd77 commented Oct 1, 2021

Merge relevant, see Engine API, also take note of the priority and urgency label on the sideline (may change over time).

Feedback from Ryan required before starting on an implementation: recommended, but eventually also some "code can be copied over" case


The Merge engine API spec states:

Client software MUST expose Engine API at a port independent from JSON-RPC API. The default port for the Engine API is 8550 for HTTP and 8551 for WebSocket.

While I guess (but not 100% sure) things should at least work if the engine API would be exposed via the normal JSON-RPC server it will likely be better if we do this separation early on and start a separate server.

Implementation shouldn't be too difficult with some modification of the RPC server instantiation in bin/cli.ts runRpcServer (or by adding a separate small function), some modification of RPCManager.getMethods() (modules should then likely be passed in instead of read statically), maybe rename the modules folder to something like user and move engine.ts to a separate folder engine.

Also to consider:

  • Tests need to be adopted
  • There needs to be new flags to activate (--engineRPC, --engineRPCport, --engineRPCaddress) (not sure, the capitalized RPC is not completely consistent with e.g. --rpcport, I still like it a bit better though due to better readability. Absolutely no blocker though of course. 😋)
@holgerd77 holgerd77 changed the title Client: Client: Expose Engine API as a separate Server with a dedicated Port Number Oct 1, 2021
@holgerd77
Copy link
Member Author

This is still relevant and we would need to address before a next client release, this would otherwise be too insecure to have the engined API exposed on the generic RPC port and address.

Wonder if we generally should restrict the engine API even more for security reasons and e.g. only allow to expose on localhost? Or is this too restrictive? 🤔 Or we might want to at least add a warning if address !== 'localhost'?

@ryanio
Copy link
Contributor

ryanio commented Oct 16, 2021

working on this now :)

@ryanio
Copy link
Contributor

ryanio commented Oct 17, 2021

added in #1530

@ryanio ryanio closed this as completed Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants