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

add back tx/encode endpoint #10856

Closed
4 of 6 tasks
Tracked by #13085
tac0turtle opened this issue Jan 2, 2022 · 15 comments · Fixed by #13882
Closed
4 of 6 tasks
Tracked by #13085

add back tx/encode endpoint #10856

tac0turtle opened this issue Jan 2, 2022 · 15 comments · Fixed by #13882
Assignees
Labels
T: Client UX T: Dev UX UX for SDK developers (i.e. how to call our code) T:feature-request

Comments

@tac0turtle
Copy link
Member

tac0turtle commented Jan 2, 2022

Summary

tx/encode endpoint was removed without a migration path given to many users. This has caused lots of trouble. It would be good to add an endpoint to grpc for tx/encode. This will help a lot with client interactions.

Proposal

cc @ValarDragon


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@tac0turtle tac0turtle added T:feature-request T: Dev UX UX for SDK developers (i.e. how to call our code) T: Client UX labels Jan 2, 2022
@ValarDragon
Copy link
Contributor

Please, this would be a great time saver for so many integrations

@alexanderbez
Copy link
Contributor

@AmauryM do you have any insight here?

@amaury1093
Copy link
Contributor

amaury1093 commented Jan 3, 2022

The old /tx/encode endpoint took as input a StdTx as JSON, and outputs new proto Tx's proto-binary bytes (as base64). Unfortunately we cannot do that anymore. The StdTx does not have a readable sequence, so we can't populate this field in the proto tx.

We could add a new endpoint for proto tx JSON -> proto tx Binary conversion, but then what's the point of doing that over network when 4 lines of code suffice to do it locally.

@tac0turtle
Copy link
Member Author

4 lines of code in go or js, python, etc as well?

for my use case I don't want to have any codecs on my client, I want to get the format of the message from grpc and be able to broadcast it. To do this currently my understanding is I need codecs of the proto messages.

@amaury1093
Copy link
Contributor

OK, sgtm, then adding an endpoint for proto tx JSON<->binary encodings sounds fine 👍 It should go in the tx service.

@robert-zaremba
Copy link
Collaborator

Are we sure there are no hidden traps? Can we safely encode every message with standard JSON (not a proto-json) without missing anything? Before we were using Amino....

@robert-zaremba
Copy link
Collaborator

Also, if we confirm the above, do we need it in 0.45?

@tac0turtle
Copy link
Member Author

Are we sure there are no hidden traps? Can we safely encode every message with standard JSON (not a proto-json) without missing anything? Before we were using Amino....

I believe the json received in grpc reflection is Proto-json? @fdymylja could you confirm?

Also, if we confirm the above, do we need it in 0.45?

I would love this for 0.45 but also its not breaking so we could backport it(?)

@amaury1093
Copy link
Contributor

Are we sure there are no hidden traps? Can we safely encode every message with standard JSON (not a proto-json) without missing anything? Before we were using Amino....

Yeah, as Marko said, that endpoint would handle proto json only. No std json, no amino json.

@BitHighlander
Copy link

BitHighlander commented Jan 16, 2022

We could add a new endpoint for proto tx JSON -> proto tx Binary conversion, but then what's the point of doing that over network when 4 lines of code suffice to do it locally.

@AmauryM do you have a js/ts example of doing this locally? I maintain https://github.com/shapeshift/hdwallet a library for broad Hardware wallet support (trezor/ledger/keepkey) and i'm unable to broadcast our signed payloads. (example) with the latest node version.

any advice for how to address this offline, in client, would be helpful, thanks.

@amaury1093
Copy link
Contributor

I don't unfortunately. it may be worth asking https://github.com/cosmos/cosmjs

@roccomuso
Copy link

any news about this?

@kevin486
Copy link

kevin486 commented Jun 9, 2022

Sharing an example of how to convert from json to protobuff can be very helpful. Thank you!!

@tac0turtle tac0turtle mentioned this issue Aug 30, 2022
18 tasks
@tac0turtle
Copy link
Member Author

@AmauryM what do you think about injecting an endpoint with grpcgatteway to handle all encoding?

@amaury1093
Copy link
Contributor

SGTM!

@tac0turtle tac0turtle moved this to 📝 Todo in Cosmos-SDK Oct 21, 2022
@likhita-809 likhita-809 self-assigned this Nov 7, 2022
@likhita-809 likhita-809 moved this from 📝 Todo to 💪 In Progress in Cosmos-SDK Nov 9, 2022
@kocubinski kocubinski moved this from 💪 In Progress to 👀 Needs Review in Cosmos-SDK Nov 9, 2022
Repository owner moved this from 👀 Needs Review to 👏 Done in Cosmos-SDK Nov 24, 2022
@tac0turtle tac0turtle removed this from Cosmos-SDK Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: Client UX T: Dev UX UX for SDK developers (i.e. how to call our code) T:feature-request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants