ADR 026: Rosetta API Support #6922

jgimeno · 2020-08-03T10:49:53Z

Description

extends: ##6831

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer
Review Codecov Report in the comment section below once CI passes

codecov · 2020-08-03T10:56:43Z

Codecov Report

Merging #6922 into master will increase coverage by 6.96%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #6922      +/-   ##
==========================================
+ Coverage   54.61%   61.57%   +6.96%     
==========================================
  Files         547      518      -29     
  Lines       37107    31996    -5111     
==========================================
- Hits        20266    19703     -563     
+ Misses      15185    10716    -4469     
+ Partials     1656     1577      -79

docs/architecture/adr-026-rosetta-api.md

Co-authored-by: Alessio Treglia <[email protected]>

docs/architecture/adr-026-rosetta-api.md

Co-authored-by: Alessio Treglia <[email protected]>

docs/architecture/adr-026-rosetta-api.md

Co-authored-by: Alessio Treglia <[email protected]>

docs/architecture/adr-026-rosetta-api.md

Co-authored-by: Alessio Treglia <[email protected]>

docs/architecture/adr-026-rosetta-api.md

alessio · 2020-08-06T11:12:25Z

docs/architecture/adr-026-rosetta-api.md

+}
+```
+
+And we will provide different implementations of this `adapter` like for 0.38, for 0.39


Suggested change

And we will provide different implementations of this `adapter` like for 0.38, for 0.39

We will provide implementations of the `Adapter` interface for each of the Cosmos SDK supported release series. Each implementation needs to support historical data queries.

In this case we can just create an Adapter of adapter that query different adapters and return the merged response.

Query -> AdapterOfAdapters --> Adapter1 (cosmos-sdk 0.37) (Hub1)
-------------------------------> Adapter2 (cosmos-sdk 0.38) (Hub2)
-------------------------------> Adapter3 (cosmos-sdk 0.38) (Hub3)

The implementation maybe we need to offer for that is one for every SDK and a wrapper adapter of adapters to build complex use cases.

alessio · 2020-08-06T11:16:21Z

docs/architecture/adr-026-rosetta-api.md

+```
+
+And we will provide different implementations of this `adapter` like for 0.38, for 0.39
+and even a `CosmosHub` implementation that will call different versions of the hub.


Suggested change

and even a `CosmosHub` implementation that will call different versions of the hub.

docs/architecture/adr-026-rosetta-api.md

Co-authored-by: Alessio Treglia <[email protected]>

docs/architecture/adr-026-rosetta-api.md

tac0turtle · 2020-08-06T15:25:34Z

docs/architecture/adr-026-rosetta-api.md

+
+## Decision
+
+We intend to develop a library that could be extended and used by application


I'm a bit lost. if its a seperate repo why is there an ADR in the sdk repo?

Yeah I'm a bit lost here as well.

The idea was first to be part of the same sdk repo, but @zmanian pointed out that there is one use case where we need to be able to query multiple versions of the hub at the same time, so living in its own repo is more "easy" to maintain than to have the same code in multiple versions of the SDK.

But in the end is very related to the SDK so I opened the ADR here. But if you think is not correct to keep it here we can change it.

This ADR should be in the repo for where the code lives, IMO. What do others think?

I think that is correct, meanwhile because the repo is not created yet (no permissions) we can use this as the draft discussion. Even I am not sure if finally everyone agrees on using another repo, this will be clarified in the cosmos-sdk call today I hope! Thanks @marbar3778

alexanderbez · 2020-08-06T16:27:25Z

docs/architecture/adr-026-rosetta-api.md

+
+- Support multiple versions of Cosmos SDK.
+- Support Cosmos Hub.
+- Implement querying of historical data sets.


Historical datasets of what? Previous hub versions? We can probably condense the last two points into one -- support current and previous versions of the Cosmos Hub.

alexanderbez · 2020-08-06T16:28:50Z

docs/architecture/adr-026-rosetta-api.md

+
+## Context
+
+We think it'd be greatly valuable to application developers to have the Cosmos SDK


I would probably add a bit more to the "why?". Rosetta does a pretty good job at this and so you may be able to just take some snippets directly from their page. But essentially anything along the lines of API interop and striving towards a common API standard should suffice.

alexanderbez · 2020-08-06T16:30:09Z

docs/architecture/adr-026-rosetta-api.md

+
+We intend to develop a library that could be extended and used by application
+developers to integrate an in-process Rosetta API-compliant server with the
+application main binaries. We also intend to provide a standalone gateway server


What are application main binaries?

alexanderbez · 2020-08-06T16:31:31Z

docs/architecture/adr-026-rosetta-api.md

+We intend to develop a library that could be extended and used by application
+developers to integrate an in-process Rosetta API-compliant server with the
+application main binaries. We also intend to provide a standalone gateway server
+program that supports a Cosmos SDK's minimum feature set. Such program could


What's the point in a gateway and the main server? You should describe those, specifically the "why".

Also note from the Common Mistakes:

Any implementation of the Rosetta APIs must connect exclusively to a running blockchain node in the same Dockerfile. It is not ok to connect to some external service that preprocesses blockchain data or to a blockchain node running elsewhere.

It's not clear if this means in-process or just the same host, but you should note this. Also, I don't understand what the gateway is, so unless there is a strong reason for it, we should remove this.

Thanks for pointing this out.

I understand both ways are compatible as long as they live in the same Dockerfile (localhost or in-process), but if we had the Rosetta as external process in one Dockerfile and the node in another Dockerfile would not be possible.

alexanderbez · 2020-08-06T16:54:57Z

docs/architecture/adr-026-rosetta-api.md

+
+## Decision
+
+We intend to develop a library that could be extended and used by application


Yeah I'm a bit lost here as well.

alexanderbez · 2020-08-06T16:57:24Z

docs/architecture/adr-026-rosetta-api.md

+program that supports a Cosmos SDK's minimum feature set. Such program could
+run alongside the client applications main binaries.
+
+### Implementation


There is no mention of how to handle "offline" mode a la multiple modes.

I think a lot is missing from this section, particularly:

Mode specification.

How it's bootstrapped and started with a blockchain node (it should be via a sub-command of the app binary).

Types should be defined before their use (e.g. Options and Router)

What are Options and how are they used?

How are multiple networks supported from the same process/server?

alexanderbez · 2020-08-19T16:10:39Z

@jgimeno what is your plan with this ADR? Are you still planning on tackling this? Take a look at https://community.rosetta-api.org/t/multiple-networks/167/2. It looks like we should first tackle supporting a single network with future support for running a gateway that can serve historical queries.

alessio · 2020-08-19T16:27:54Z

@alexanderbez hey there! As announced in Discord, work has started and is currently ongoing here: https://github.com/tendermint/cosmos-rosetta-gateway.

It looks like we should first tackle supporting a single network with future support for running a gateway that can serve historical queries.

I agree and I believe this is the approach @jgimeno and @ilgooz have taken so far.

Note: the repository is just a temporary location, we'll reach out ICF and arrange the transfer into the cosmos Github organization as soon as we have a POC ready.

In conclusion, considering that this work in being carried out in an external repo this PR should just be closed.

Wdyt?

alexanderbez · 2020-08-19T16:30:52Z

Why is work being started before the ADR has been properly vetted and reviewed!? There are many holes and missing pieces in this ADR.

alessio · 2020-08-19T16:59:02Z

We've heard no strong objections to get this started in a separate repo, and we also thought that both ADR and the work on a POC should actually be done in parallel, i.e. pretotyping an implementation as quickly as possible would give us the possibility to write even better specs. Furthermore, developing the library in a separate repo gives us the possibility to support multiple SDK releases without messing around with go modules versioning.

Rest assured everything will be up for review (it actually is already!) and transferring the repo to an ICF-owned organization is on our roadmap (we could even do it now, if that helps)

Does this address your concerns? I am happy to keep this PR open

tac0turtle · 2020-08-19T17:17:02Z

I think we can close this PR. The rosetta wrapper/proxy is not part of the sdk but a tool, tools don't have ADR's in the sdk.

alessio · 2020-08-19T17:21:56Z

I'm OK with that @marbar3778. @alexanderbez wdyt?

alexanderbez · 2020-08-19T18:06:11Z

OK, if AiB wants to take lead on this outside the scope of the SDK then by all means proceed, just know based on this ADR as-is, I'm highly skeptical of the approach design and overall approach. Good luck!

jgimeno added 2 commits August 3, 2020 12:48

First draft for adr.

840cdc1

Update date.

ee79d63

Add some implementation details.

981950c

alessio reviewed Aug 3, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 3, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

jgimeno and others added 3 commits August 3, 2020 18:24

Update docs/architecture/adr-026-rosetta-api.md

13b392f

Co-authored-by: Alessio Treglia <[email protected]>

Update docs/architecture/adr-026-rosetta-api.md

d437a93

Co-authored-by: Alessio Treglia <[email protected]>

Merge branch 'master' into jonathan/adr-rosetta

9797fbe

alessio reviewed Aug 3, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 3, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio changed the title ~~Adr 026 Roseetta.~~ Adr 026 Rosetta. Aug 3, 2020

alessio reviewed Aug 3, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

jgimeno and others added 6 commits August 3, 2020 21:30

Update docs/architecture/adr-026-rosetta-api.md

06bcefd

Co-authored-by: Alessio Treglia <[email protected]>

Update docs/architecture/adr-026-rosetta-api.md

77e32d1

Co-authored-by: Alessio Treglia <[email protected]>

Update docs/architecture/adr-026-rosetta-api.md

06cf545

Co-authored-by: Alessio Treglia <[email protected]>

Rosetta more detailed.

1fd90c7

Router maybe does not need to be exposed.

1a03d1f

Merge branch 'master' into jonathan/adr-rosetta

d9b9a9c

alessio reviewed Aug 4, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 4, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

Update docs/architecture/adr-026-rosetta-api.md

163e30c

Co-authored-by: Alessio Treglia <[email protected]>

alessio reviewed Aug 4, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 4, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 4, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 4, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 4, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 4, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

jgimeno and others added 2 commits August 4, 2020 16:48

Update docs/architecture/adr-026-rosetta-api.md

6787e12

Co-authored-by: Alessio Treglia <[email protected]>

Update docs/architecture/adr-026-rosetta-api.md

15c6817

Co-authored-by: Alessio Treglia <[email protected]>

jgimeno requested review from alessio and alexanderbez August 4, 2020 15:08

jgimeno marked this pull request as ready for review August 5, 2020 08:05

jgimeno requested review from aaronc, clevinson and fedekunze as code owners August 5, 2020 08:05

alessio changed the title ~~Adr 026 Rosetta.~~ ADR 026: Rosetta API Support Aug 6, 2020

alessio reviewed Aug 6, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 6, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 6, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

alessio reviewed Aug 6, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Outdated Show resolved Hide resolved

jgimeno and others added 4 commits August 6, 2020 14:02

Update docs/architecture/adr-026-rosetta-api.md

b08fe90

Co-authored-by: Alessio Treglia <[email protected]>

Update docs/architecture/adr-026-rosetta-api.md

bab1c5c

Co-authored-by: Alessio Treglia <[email protected]>

Update docs/architecture/adr-026-rosetta-api.md

fbd0210

Co-authored-by: Alessio Treglia <[email protected]>

Update docs/architecture/adr-026-rosetta-api.md

9cb1b28

Co-authored-by: Alessio Treglia <[email protected]>

tac0turtle reviewed Aug 6, 2020

View reviewed changes

docs/architecture/adr-026-rosetta-api.md Show resolved Hide resolved

tac0turtle reviewed Aug 6, 2020

View reviewed changes

alexanderbez suggested changes Aug 6, 2020

View reviewed changes

Merge branch 'master' into jonathan/adr-rosetta

c28e610

alexanderbez closed this Aug 19, 2020

alexanderbez deleted the jonathan/adr-rosetta branch August 19, 2020 18:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ADR 026: Rosetta API Support #6922

ADR 026: Rosetta API Support #6922

jgimeno commented Aug 3, 2020 •

edited

Loading

codecov bot commented Aug 3, 2020 •

edited

Loading

alessio Aug 6, 2020 •

edited

Loading

jgimeno Aug 6, 2020 •

edited

Loading

alessio Aug 6, 2020

tac0turtle Aug 6, 2020

alexanderbez Aug 6, 2020

jgimeno Aug 7, 2020

tac0turtle Aug 7, 2020 •

edited

Loading

jgimeno Aug 7, 2020

alexanderbez Aug 6, 2020

alexanderbez Aug 6, 2020

alexanderbez Aug 6, 2020

alexanderbez Aug 6, 2020

alexanderbez Aug 6, 2020

jgimeno Aug 7, 2020

alexanderbez Aug 6, 2020

alexanderbez Aug 6, 2020

alexanderbez Aug 6, 2020

alexanderbez commented Aug 19, 2020

alessio commented Aug 19, 2020

alexanderbez commented Aug 19, 2020

alessio commented Aug 19, 2020

tac0turtle commented Aug 19, 2020

alessio commented Aug 19, 2020

alexanderbez commented Aug 19, 2020 •

edited

Loading

	And we will provide different implementations of this `adapter` like for 0.38, for 0.39
	We will provide implementations of the `Adapter` interface for each of the Cosmos SDK supported release series. Each implementation needs to support historical data queries.


		## Decision

		We intend to develop a library that could be extended and used by application


		## Context

		We think it'd be greatly valuable to application developers to have the Cosmos SDK

ADR 026: Rosetta API Support #6922

ADR 026: Rosetta API Support #6922

Conversation

jgimeno commented Aug 3, 2020 • edited Loading

Description

codecov bot commented Aug 3, 2020 • edited Loading

Codecov Report

alessio Aug 6, 2020 • edited Loading

Choose a reason for hiding this comment

jgimeno Aug 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tac0turtle Aug 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexanderbez commented Aug 19, 2020

alessio commented Aug 19, 2020

alexanderbez commented Aug 19, 2020

alessio commented Aug 19, 2020

tac0turtle commented Aug 19, 2020

alessio commented Aug 19, 2020

alexanderbez commented Aug 19, 2020 • edited Loading

jgimeno commented Aug 3, 2020 •

edited

Loading

codecov bot commented Aug 3, 2020 •

edited

Loading

alessio Aug 6, 2020 •

edited

Loading

jgimeno Aug 6, 2020 •

edited

Loading

tac0turtle Aug 7, 2020 •

edited

Loading

alexanderbez commented Aug 19, 2020 •

edited

Loading