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

Support Rosetta API #6831

Closed
3 of 7 tasks
alessio opened this issue Jul 23, 2020 · 21 comments · Fixed by #8656 or #8473
Closed
3 of 7 tasks

Support Rosetta API #6831

alessio opened this issue Jul 23, 2020 · 21 comments · Fixed by #8656 or #8473

Comments

@alessio
Copy link
Contributor

alessio commented Jul 23, 2020

Please provide Cosmos SDK app developers with an implementation of the Rosetta API specifications and enable Cosmos SDK applications to provide support for Rosetta clients.

More information:

Remaining tasks for the finalization:

  • Implement full balance tracking via events
  • Rework construction API so it can support all the messages registered in the chain via codec, without extension
  • Unit tests and cleanups

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

I took a look at the API spec and it seems very straightforward. I would propose that we write up an ADR first on how to write and expose this API. My initial intuition is that we have a separate versioned Resetta API that runs alongside the existing API server.

@tac0turtle
Copy link
Member

Something to think about with Rosetta is how to support old chains as well. For the cosmos hub there have been three upgrades that have been for zero height. If an exchange wants to query something from a previous chain how will they do that? A wrapper around the old rest APIs may be needed but then the question is about uniqueness of blocks and height.

Some this may need to be added to the adr so people are aware of what to do for old chains.

@alexanderbez
Copy link
Contributor

I don't think we'd support "uniqueness" of block heights across previous chains. Also, is it a hard requirement that we add this for all major hub versions? If so, where is this documented. @zmanian @jackzampolin

@jackzampolin
Copy link
Member

Support for hub 4 would be the first goal here. Legacy chain support is a nice to have a this point.

@zmanian
Copy link
Member

zmanian commented Jul 25, 2020

Could we get a decision if Rosetta support will be native or via shim/proxy like Oasis's support?

@alessio
Copy link
Contributor Author

alessio commented Jul 26, 2020

Could we get a decision if Rosetta support will be native or via shim/proxy like Oasis's support?

The feedback I received was along the lines of we should develop this as a module. I tend to agree with that. Plus:

Support for hub 4 would be the first goal here. Legacy chain support is a nice to have a this point.

I fully agree on prioritization - no doubt we shall develop a module for master first, yet IMHO support for older cosmos hubs is more than a nice to have. There is quite some interest in the community re: tools for accounting and reconciliation of data that spans across all the hubs. Still, 0.40 implementation must come first.

CC'ing @jgimeno @ilgooz

@alexanderbez
Copy link
Contributor

alexanderbez commented Jul 26, 2020

Can you define module? I would propose that we simply create a small package, rosetta, which should provide a concise, configurable, and clean API to expose Rosetta compliant data.

I'll have an ADR up tomorrow. I'd like to see what others think.

@fadeev
Copy link
Contributor

fadeev commented Jul 26, 2020

Can you define module?

A Cosmos SDK module that exposes :1317/rosetta/* endpoints and uses SDK functionality to send and recieve data that conforms to the Rosetta spec. If it is implemented as a module, it will be easy to import into a chain, it will launch with a "REST" server and won't depend on the existing :1317 API (in contrast with a standalone server, which will).

@alexanderbez
Copy link
Contributor

I see. We should then stick with "package" nomenclature, in the traditional Golang sense, here instead and not "module" as that can lead to confusion as "modules" have a special meaning in the SDK.

I do not see any merit to having a Rosetta compliant API be served as a standalone server. There are currently three "servers" already: legacy, gRPC, and gRPC HTTP Gateway, with gRPC HTTP gateway replacing the legacy after Stargate.

Why have a fourth? It just seems unnecessary. Ultimately, there is a single HTTP API (gRPC HTTP gateway), which we can server Rosetta from.

example:

import (
  "github.com/cosmos/cosmos-sdk/rosetta"
)

rosettaFactory := rosetta.NewRosettaFactory(...)

// apiSrv is either the legacy or gRPC HTTP gateway server

// 1. either mount handlers manually 
apiSrv.Handle("/v1/rosetta/data/network", rosettaFactory.DataAPI.NetworkHandler)
apiSrv.Handle("/v1/rosetta/data/account", rosettaFactory.DataAPI.AccountHandler)
// ...

// 2. Or, through an automatic mount call
rosetta.MountHandlers(apiSrv.Mux, rosettaFactory)

@fedekunze fedekunze added this to the v0.41 milestone Jul 28, 2020
@jackzampolin
Copy link
Member

I think having a standalone shim server would be nice especially for supporting for prior versions. Don't think this is a cosmossdk module, more of a go package as @alexanderbez mentions.

@amaury1093
Copy link
Contributor

amaury1093 commented Jul 31, 2020

I'm also in favor of a shim/proxy server.

tbh long-term, I'd even go as far as removing grpc-gateway, legacy rest, and grpc-web-proxy from the cosmos sdk, and put them all in this proxy. We'd only keep the grpc server in this repo.

My reasons are:

  • Keep the cosmos sdk as lean as possible, for various reasons.
  • Separation of concerns. I saw @ethanfrey recently adding a flag for allowing CORS for the legacy API, and I would expect more flags to come (applies to the gateway and to the grpc-web-proxy): allowed headers, allowed origins, rate-limiting and all other 150 configurations HTTP servers have nowadays.
  • If node operators run a pool of nodes, and only expose one single endpoint (à la Infura), this proxy could even do stuff like load balancing; the proxy & the node(s) don't need to live on the same machine.

Note: I do admit that I'm not super aware on how node operators and validators run their nodes, and what features are important to them. A small user research might be helpful here.

Note 2: this of course doesn't contradict @alexanderbez's comment on having a rosetta package, which is anyways needed.

@ethanfrey
Copy link
Contributor

I am fully in support of a roseta proxy, that can run out-of-process from a node. Whether it is in the same repo or a different one... I have no real opinion.

tbh long-term, I'd even go as far as removing grpc-gateway, legacy rest, and grpc-web-proxy from the cosmos sdk, and put them all in this proxy. We'd only keep the grpc server in this repo.

However, we cannot only support this, as there are many custom modules that will never be properly captured/exposed in roseta. Roseta is designed for exchanges not custom apps. I see the utility of grpc-gateway, legacy rest, and grpc-web-proxy and they have to be different than the Roseta protocol (as we are seeking compatibility with existing cosmos APIs
not Roseta).

I assume you are suggesting a different repo to hold all the proxies and then have circa 4 binaries there for all the use cases, and avoid pulling in all the http dependencies into the core cosmos-sdk repo.

@amaury1093
Copy link
Contributor

amaury1093 commented Jul 31, 2020

I assume you are suggesting a different repo to hold all the proxies and then have circa 4 binaries there for all the use cases, and avoid pulling in all the http dependencies into the core cosmos-sdk repo.

No strong opinion about a separate repo or not. I was proposing a separate optional binary.

circa 4 binaries

No, actually only 2 binaries: one for the node (the current {my_app}d), and one proxy which exposes configurable endpoints for grpc-gateway/web-proxy/rosetta/legacy, and just talks to the node's gRPC server on the other side.

@jgimeno
Copy link
Contributor

jgimeno commented Jul 31, 2020

I am all in for their own repo and a standalone. But we can provide the abstracted service in case a developer wants to embed it in their own binary if they need.

Similar as how it is done with the RPC.

This is an ugly draft of how could this package look like, in orange is what can be included in the new repo. A developer instead of using the standalone binary can import this package into its project and instantiate the Service in his own application binary if he needs.

https://user-images.githubusercontent.com/4056757/89031723-59b44280-d333-11ea-9ce0-361e41279c3b.jpg

@alexanderbez
Copy link
Contributor

alexanderbez commented Jul 31, 2020

Looks reasonable @jgimeno. But again, I want to stress that I'd personally would avoid the necessity to have two binaries -- we literally just put in a bunch of work to have everything run from a single binary.

The legacy API is being ditched in the next release. That leaves us with gRPC running automatically in-process with the app. The gRPC gateway will also be started automatically in-process with the app I'd presume.

However, we can require that operators explicitly start external services (gRPC gateway and rosetta) but from the same binary:

e.g.

$ gaiad grpc-gateway ...
$ gaiad rosetta-proxy ...

I don't really think this merits its own repo as I think the implementation is going to be super trivial and not complex.


@amaurymartiny

If node operators run a pool of nodes, and only expose one single endpoint (à la Infura), this proxy could even do stuff like load balancing; the proxy & the node(s) don't need to live on the same machine.

Typically LB are not used in front of full-nodes because that requires all nodes to be perfectly in sync which is very hard to do, so operators typically don't do this.

@alessio
Copy link
Contributor Author

alessio commented Aug 3, 2020

@alexanderbez dixit:

However, we can require that operators explicitly start external services (gRPC gateway and rosetta) but from the same binary:

That sounds sensible to me.

@amaurymartiny dixit:

this proxy could even do stuff like load balancing

Please, please let's not pack proxying and load balancing together. Really, they are two entirely distinct features (especially load balancing would be completely out of scope of any cosmos-sdk binary).

@alessio
Copy link
Contributor Author

alessio commented Feb 25, 2021

Reopening. Work is not yet done.

@fedekunze
Copy link
Collaborator

@alessio can you update the description with the remaining tasks?

@fdymylja
Copy link
Contributor

fdymylja commented Mar 3, 2021

@alessio can you update the description with the remaining tasks?

Currently remaining tasks are:

  • Implement full balance tracking via events
  • Rework construction API so it can support all the messages registered in the chain via codec, without extension
  • Unit tests and cleanups

@alessio
Copy link
Contributor Author

alessio commented Mar 3, 2021

Thanks @fdymylja - mind updating the issue's description please? 🙏

@clevinson clevinson added backlog and removed backlog labels Mar 3, 2021
@fdymylja
Copy link
Contributor

fdymylja commented Mar 16, 2021

Rosetta API support was merged into master. It supports balance tracking fully, and allows for the construction and delivery of any message the chain supports (including the messages that non-sdk modules accept).

Currently it can support only one chain at time as the logic is bound to the codec, which is app specific.

But we're currently working on an implementation which would allow rosetta to support multiple chains (and different cosmos-sdk versions, as long as they're post v0.40) at the same time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet