docs: ica tx encoding documentation added #4169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
srdtrk
requested review from
colin-axner,
AdityaSripal,
crodriguezvega,
charleenfei,
damiannolan,
chatton and
DimitrisJim
as code owners
srdtrk
added
backport-to-v7.3.x
docs
crodriguezvega
approved these changes
Jul 25, 2023
There was a problem hiding this comment.
Nice work, @srdtrk. I just have some nits, otherwise looks good to me.
We should also update this line here:
Note the data field is a base64 encoded byte string as per the [proto3 JSON encoding specification](https://developers.google.com/protocol-buffers/docs/proto3#json).
(I just realised that the text is wrong and it mentions proto3 JSON 😓).
| } | ||
| ``` | ||
|
|
||
| The encoding method for `CosmosTx` is determined during the channel handshake process. If the channel version [metadata's encoding field](https://github.com/cosmos/ibc-go/blob/v7.2.0/proto/ibc/applications/interchain_accounts/v1/metadata.proto#L22) is marked as `proto3`, `CosmosTx` undergoes protobuf encoding. Conversely, if the field is set to `proto3json`, pbjson encoding takes place, which generates a JSON representation of the protobuf message. |
There was a problem hiding this comment.
Suggested change
| The encoding method for `CosmosTx` is determined during the channel handshake process. If the channel version [metadata's encoding field](https://github.com/cosmos/ibc-go/blob/v7.2.0/proto/ibc/applications/interchain_accounts/v1/metadata.proto#L22) is marked as `proto3`, `CosmosTx` undergoes protobuf encoding. Conversely, if the field is set to `proto3json`, pbjson encoding takes place, which generates a JSON representation of the protobuf message. | |
| The encoding method for `CosmosTx` is determined during the channel handshake process. If the channel version [metadata's `encoding` field](https://github.com/cosmos/ibc-go/blob/v7.2.0/proto/ibc/applications/interchain_accounts/v1/metadata.proto#L22) is marked as `proto3`, `CosmosTx` undergoes protobuf encoding. Conversely, if the field is set to `proto3json`, pbjson encoding takes place, which generates a JSON representation of the protobuf message. |
There was a problem hiding this comment.
Sorry, I should look into the differences myself between all these packages, but is pbjson encoding here or protojson encoding?
There was a problem hiding this comment.
The SDK uses jsonpb. So I'll use this term instead
DimitrisJim
approved these changes
Jul 26, 2023
| @@ -84,7 +84,7 @@ simd tx interchain-accounts host --help | |||
|
|
|||
| ##### `generate-packet-data` | |||
|
|
|||
| The `generate-packet-data` command allows users to generate interchain accounts packet data for input message(s). The packet data can then be used with the controller submodule's [`send-tx` command](#send-tx). | |||
| The `generate-packet-data` command allows users to generate protobuf encoded interchain accounts packet data for input message(s). The packet data can then be used with the controller submodule's [`send-tx` command](#send-tx). | |||
There was a problem hiding this comment.
might be worth it to tweak the inline docs on this too, I don't see us mentioning the encoding there.
There was a problem hiding this comment.
I added it to the cobra command. There is another issue for more general encoding in those commands #3974
There was a problem hiding this comment.
I need more approvals now that I modified ica with inline docs haha
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4169 +/- ##
=======================================
Coverage 78.86% 78.86%
=======================================
Files 188 188
Lines 12994 12994
=======================================
Hits 10248 10248
Misses 2317 2317
Partials 429 429
|
colin-axner
approved these changes
Jul 27, 2023
|
|
||
| ## (Protobuf) JSON Encoding | ||
|
|
||
| Protojson encoding presents an alternative encoding technique for `CosmosTx`. It is selected if the channel handshake begins with the channel version metadata `encoding` field labeled as `proto3json`. In Golang, protojson encoding employs the `"github.com/cosmos/gogoproto/jsonpb"` package. Within Cosmos SDK, the `ProtoCodec` structure implements the `JSONCodec` interface, leveraging the `jsonpb` package. This method generates a JSON format as follows: |
There was a problem hiding this comment.
I think it'd be nice to reference the proto3json mapping in defining the expected encoding formats
| } | ||
| ``` | ||
|
|
||
| The encoding method for `CosmosTx` is determined during the channel handshake process. If the channel version [metadata's `encoding` field](https://github.com/cosmos/ibc-go/blob/v7.2.0/proto/ibc/applications/interchain_accounts/v1/metadata.proto#L22) is marked as `proto3`, `CosmosTx` undergoes protobuf encoding. Conversely, if the field is set to `proto3json`, jsonpb encoding takes place, which generates a JSON representation of the protobuf message. |
|
|
||
| ## (Protobuf) JSON Encoding | ||
|
|
||
| Protojson encoding presents an alternative encoding technique for `CosmosTx`. It is selected if the channel handshake begins with the channel version metadata `encoding` field labeled as `proto3json`. In Golang, protojson encoding employs the `"github.com/cosmos/gogoproto/jsonpb"` package. Within Cosmos SDK, the `ProtoCodec` structure implements the `JSONCodec` interface, leveraging the `jsonpb` package. This method generates a JSON format as follows: |
There was a problem hiding this comment.
Suggested change
| Protojson encoding presents an alternative encoding technique for `CosmosTx`. It is selected if the channel handshake begins with the channel version metadata `encoding` field labeled as `proto3json`. In Golang, protojson encoding employs the `"github.com/cosmos/gogoproto/jsonpb"` package. Within Cosmos SDK, the `ProtoCodec` structure implements the `JSONCodec` interface, leveraging the `jsonpb` package. This method generates a JSON format as follows: | |
| The proto3 JSON encoding presents an alternative encoding technique for `CosmosTx`. It is selected if the channel handshake begins with the channel version metadata `encoding` field labeled as `proto3json`. In Golang, the Proto3 canonical encoding in JSON is implemented by the `"github.com/cosmos/gogoproto/jsonpb"` package. Within Cosmos SDK, the `ProtoCodec` structure implements the `JSONCodec` interface, leveraging the `jsonpb` package. This method generates a JSON format as follows: |
mergify bot
pushed a commit
that referenced
this pull request
Jul 27, 2023
* docs: added 'tx-encoding.md' file to docs * feat: added tx encoding to frontend * docs: improved tx encoding docs * style(docs): moved tx encoding to bottom * docs: improved tx encoding * docs: improved ica tx encoding docs * docs: in ica tx encoding, added small code blocks around some interface names * docs: improved ica client docs + reordered tx encoding in sidebar * docs(ica/cmd): updated docs of 'generate-packet-data' * docs: improved ica tx encoding docs (cherry picked from commit 585e56b)
crodriguezvega
pushed a commit
that referenced
this pull request
Jul 27, 2023
* docs: added 'tx-encoding.md' file to docs * feat: added tx encoding to frontend * docs: improved tx encoding docs * style(docs): moved tx encoding to bottom * docs: improved tx encoding * docs: improved ica tx encoding docs * docs: in ica tx encoding, added small code blocks around some interface names * docs: improved ica client docs + reordered tx encoding in sidebar * docs(ica/cmd): updated docs of 'generate-packet-data' * docs: improved ica tx encoding docs (cherry picked from commit 585e56b) Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds documentation for ica transaction encoding
closes: #4047
Commit Message / Changelog Entry
see the guidelines for commit messages. (view raw markdown for examples)
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.
Code follows the module structure standards and Go style guide.Wrote unit and integration tests.docs/) or specification (x/<module>/spec/).Added relevantgodoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.