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

Move explorer to it's own binary crate #3227

Open
3 tasks
ecioppettini opened this issue Apr 19, 2021 · 5 comments
Open
3 tasks

Move explorer to it's own binary crate #3227

ecioppettini opened this issue Apr 19, 2021 · 5 comments
Labels
A-explorer Area: Explorer API and backend

Comments

@ecioppettini
Copy link
Contributor

partially replaces #2759 (bundling two things in one issue was a bad idea, existing discussion was on the other topic anyway).

About

Make the explorer a separate service instead of a mode of operation for the node (as it works right now per #694).

The idea is for this service to not be a participant on the p2p layer, but to hook to the node and receive messages in the same way that we do right now internally through the intercom.
New blocks and tip changes are enough for the most part. Bootstrapping shouldn't differ much from the way the node does it, I think, except (probably) that we only have one node to pull blocks from.

Requirements

  • Create a new explorer binary crate in this workspace (as in add separate explorer service #2467, but it's out of date and built on different assumptions, and just a PoC anyway).
    We may want to move some things to jormungandr_lib to keep some things consistent (logging setup, some settings, maybe some networking stuff or service stuff), but it's not needed from the beginning.
  • Connect this new service to the explorer through grpc. This may deserve a separate issue to discuss it. I don't have much knowledge about the networking layer myself.
    As far as I can tell, the apis from the BlockService should be enough, but the explorer wouldn't be a full client, and I'm not sure how it will interact with the other stuff (will it get quarantined, for example?). May require a particular ExplorerService for the node?
  • A lot of tests use the explorer, so that would need a rework somehow.

Other considerations

Per design, most of the explorer works independently from the Blockchain (basically the ledger), so migration shouldn't be a problem with just sending blocks (and tips).

There are currently two things that need access to the blockchain (and I didn't add them myself, so I'm not sure what to do with them yet).
One of those is the Block::treasury query, which I think is used in tests at least.
The other one is in the indexing part of the VoteTally certificate. I think this can be ignored for now, but we'll need to think if it needs something that should be provided through an api (I think it needs the tallies, basically), of if it is something that the explorer can keep up-to-date by itself.

@ecioppettini ecioppettini added the A-explorer Area: Explorer API and backend label Apr 19, 2021
@eugene-babichenko
Copy link
Contributor

We may want to move some things to jormungandr_lib to keep some things consistent (logging setup, some settings, maybe some networking stuff or service stuff), but it's not needed from the beginning.

I am not sure about that part. jormungandr_lib has many clients and not all of them would need logging and networking things. Although we can just make these things feature-gated.

Just to make sure: you intend to use gRPC for explorer-to-node communications, right?

@ecioppettini
Copy link
Contributor Author

I am not sure about that part. jormungandr_lib has many clients and not all of them would need logging and networking things. Although we can just make these things feature-gated.

Well, maybe you are right. It could be a different crate too. But overall I was speculating, maybe we don't need it.
It's just that I was thinking than re-using some bits of the codebase could make things more consistent overall. There is a decent deal of tools and helpers in jormungandr, I think.

Just to make sure: you intend to use gRPC for explorer-to-node communications, right?

Yeah, that's what we talked about with @mzabaluev last time, at least. It's already implemented, and websockets are more fit to more user-level clients (with graphql, probably).

Although, nothing is done yet, so if you have any concerns or other ideas you can bring them up.

@mzabaluev
Copy link
Contributor

I support using gRPC for inter-component subscription channels.

BlockService could provide the necessary functionality for bootstrapping and subscription to new block announcements.
The server-side handling of client connections assumes p2p nodes indeed. But as long as the client behaves as an anonymous node, does not subscribe to things it does not need (e.g. gossip), and does not send events up the subscription streams, it should be possible to accommodate non-node clients using this protocol.

@mzabaluev
Copy link
Contributor

mzabaluev commented Apr 21, 2021

One thing that BlockService does not currently provide is tip change notifications. I think this could be added backward compatibly to block announcements on the server side, but the explorer could also apply tip selection logic on its own, and the result should be the same.

Previously, there were requests to accommodate thin clients via grpc-web and bidirectional subscription requests were a sticking point as they are not supported in grpc-web.
Maybe its time to get the node a non-p2p service endpoint to address the needs of such clients.

@eugene-babichenko
Copy link
Contributor

But as long as the client behaves as an anonymous node, does not subscribe to things it does not need (e.g. gossip), and does not send events up the subscription streams, it should be possible to accommodate non-node clients using this protocol.

This is calling for introducing some sort of filtering. Or at least we need to explicitly state in the documentation that we allow anonymous connections to BlockService, and thus the endpoint may be needed to be protected by a firewall if a node operator doesn't like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-explorer Area: Explorer API and backend
Projects
None yet
Development

No branches or pull requests

3 participants