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

Implement GraphQL query layer #930

Open
1 of 6 tasks
ryanchristo opened this issue Mar 24, 2022 · 8 comments
Open
1 of 6 tasks

Implement GraphQL query layer #930

ryanchristo opened this issue Mar 24, 2022 · 8 comments
Labels
Status: Backlog Type: Feature New feature or request

Comments

@ryanchristo
Copy link
Member

ryanchristo commented Mar 24, 2022

Summary

We are currently storing addresses as bytes in state proto messages for storage efficiency. We are also using the same messages in query responses and therefore returning addresses that are not human-readable.

How do we make addresses readable in query responses?

Short-term solution:

Potential long-term solutions:

  • Expose a graphql query layer that has a straightforward way to get an address string from bytes

For Admin Use

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

IMO, by default we should not think that stored object = presented object.
So we should not be afraid to create a different structure for stored object and presentation object (the one we send out).

@robert-zaremba
Copy link
Collaborator

robert-zaremba commented Mar 25, 2022

Hacky solution to this particular problem would be some proto annotations and having 2 codec modes which will understand that annotation and handle the coding. .... but to be honest, it looks too hacky ;)

@aaronc
Copy link
Member

aaronc commented Mar 30, 2022

With the ORM, the stored object is effectively the presented object because the ORM will expose its own direct query layer to clients. So we will need to figure out a way to solve the presentation problem with the ORM.

My current thinking has been:

  • use cosmos_proto.scalar annotations to indicate a bytes field is an address
  • expose a graphql query layer that has a straightforward way to get an address string from bytes (similarly to how Postgraphile uses postgres helper functions to allow the same thing with I believe GeoJSON)

We could also do gRPC where we auto-generated presentation types based on scalar annotations, but that seems more heavy-handed... and graphql has other benefits...

Sorry this isn't well documented on the ORM side but we should write it up in one of the Framework WG calls

@ryanchristo
Copy link
Member Author

We spoke about this briefly on our standup this morning. The graphql solution sounds interesting but may require more work. @technicallyty is planning to look into the scalar annotation later today if he has the time.

@ryanchristo
Copy link
Member Author

ryanchristo commented Apr 12, 2022

Notes from product call:

For v4.0 release, we will need to create separate proto messages where addresses are represented as strings.

@ryanchristo ryanchristo self-assigned this Apr 14, 2022
@ryanchristo ryanchristo changed the title How should we store addresses in state? Addresses as strings in query responses Apr 16, 2022
@ryanchristo ryanchristo changed the title Addresses as strings in query responses Return addresses as strings in query responses Apr 21, 2022
@aaronc
Copy link
Member

aaronc commented Apr 22, 2022

Moving my comments from #1022 here

This makes me want to spend a bit of time on the query infrastructure in the SDK using Cosmos scalars so we don't have to manually write all this stuff. But generally looking good @ryanchristo

There are a few design questions though if I'm going to spend some time. Do we auto-generate gRPC endpoints that just convert bytes to strings for cosmos.AddressBytes scalars? Do we relate this to the value renderer work for sign mode textual? Or maybe we jump straight to graphql?

What do you think @ryanchristo ?

@ryanchristo
Copy link
Member Author

What do you think @ryanchristo ?

I think we should consider jumping straight to graphql given we need more than just addresses converted from bytes to strings. Scalars will not eliminate our need for writing out queries as I have done in #1022.

@ryanchristo ryanchristo changed the title Return addresses as strings in query responses Converting state to human-readable query responses Apr 22, 2022
@aaronc
Copy link
Member

aaronc commented Apr 22, 2022

What do you think @ryanchristo ?

I think we should consider jumping straight to graphql given we need more than just addresses converted from bytes to strings. Scalars will not eliminate our need for writing out queries as I have done in #1022.

I've sort of been leaning towards graphql as well

@ryanchristo ryanchristo removed this from the v4.0 - Llangorse Upgrade milestone Apr 28, 2022
@ryanchristo ryanchristo changed the title Converting state to human-readable query responses Implement GraphQL query layer Sep 4, 2022
@ryanchristo ryanchristo added Scope: app Issues that update the root module Status: Backlog Type: Feature New feature or request and removed Scope: app Issues that update the root module labels Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Backlog Type: Feature New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants