-
Notifications
You must be signed in to change notification settings - Fork 349
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 all celestia-node required endpoints through gRPC #3421
Comments
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as outdated.
This comment was marked as outdated.
Closes: #3422 We seemed to have forgotten to add the query function which means `ABCIQuery` wasn't working even although the grpc endpoint was exposed also related to: #3421<hr>This is an automatic backport of pull request #3423 done by [Mergify](https://mergify.com). Co-authored-by: Callum Waters <cmwaters19@gmail.com>
is it possible to expose these two methods from core, and eventually move the logic to node if possible? |
Yes, should be very, very easy!
|
In addition to the EventsClient:
SignClient:
Additional functionalities like |
Current services exposed by app (click to expand):
Decide on adding missing functionality to existing services (e.g. cosmos.base.tendermint.v1beta1.Service) or add a new service that is specific to node/celestia. The latter is probably cleaner and preferable. |
An additional point is that the block gRPC API should be designed to support larger blocks, so we won't need to rework it as block sizes increase. |
Full nodes should be able to handle these queries themselves since they have all the relevant data. I think |
I think we should just add it as a new service. Generally we should reduce the amount of bespoke code we have in cosmos-sdk if we eventually plan to unfork oursleves |
Closes: #3753 This in some ways ties into https://github.com/celestiaorg/celestia-app/issues/3421<hr>This is an automatic backport of pull request #3754 done by [Mergify](https://mergify.com). --------- Co-authored-by: Callum Waters <cmwaters19@gmail.com>
Closes: #3753 This in some ways ties into https://github.com/celestiaorg/celestia-app/issues/3421<hr>This is an automatic backport of pull request #3754 done by [Mergify](https://mergify.com). --------- Co-authored-by: Callum Waters <cmwaters19@gmail.com>
assigned myself as I opened this: celestiaorg/celestia-core#1513 If there are other parts that need to be done, let me know to see if I can get them done while I am at it |
celestiaorg/celestia-node#3916 is relevant to this as well. |
Summary
Node has to use core's/comet's RPC additionally to gRPC. That is very confusing.
The current app API is not sufficient and lead to bad design decisions in celestia-node which led to proposals that will manifest these even further celestiaorg/celestia-node#2931
Problem Definition
Node currently uses both RPC (comet) and gRPC (app). Apparently,
the onlyone reason for that is only comet's RPC exposes proofs for state queries currently, or rather ABCI queries (see #3422).Another reason, is that bridge nodes currently get their block data via RPC. That is also not ideal as the block data should be binary/protobuf encoded instead of a large JSON file.
If these two changed, celestia-node could solely rely on app's gRPC.
One of the places
The only placewhere node uses rpc is here:Proposal
The text was updated successfully, but these errors were encountered: