Json schema for Coin and some parts of Transfer #772
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
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #772 +/- ##
=======================================
Coverage 71.32% 71.33%
=======================================
Files 124 124
Lines 14796 14828 +32
=======================================
+ Hits 10553 10577 +24
- Misses 4243 4251 +8
☔ View full report in Codecov by Sentry. |
dzmitry-lahoda
changed the title
dzmitry-lahoda
changed the title
Farhad-Shabani
requested changes
Jul 21, 2023
There was a problem hiding this comment.
We do coin router kind of thing in CosmWasm. It needs some IBC things, and it requests JsonSchema on types.
Great seeing you're using ibc-rs! Thanks for featuring this :)
Also, I made serde of Amount consistently decimal(decimal string). It was from str decimal, and in coin it was decimal, but on its own if was hex string. So made on its own also decimal. For some it can be breaking change. But other way schema (and CosmWasm messages) would be bad/inconsistent.
Just got a deserialization issue when tested locally. Otherwise, we are good.
- Before, serialization roundtrip (ser > deser) on both
CoinandAmountwas ending up with a genuine value. Now, on deserialization, it fails. I think we should add a tailoreddeserializeforU256instead of using this. - Also, It would be great if you include a test for it.
.changelog/unreleased/improvement/772-json-schema-for-channels-and-coin.md
Outdated
Show resolved
Hide resolved
Farhad-Shabani
requested changes
Jul 28, 2023
dzmitry-lahoda
added a commit
to dzmitry-lahoda-forks/ibc-rs
that referenced
this pull request
Aug 1, 2023
* support for optional json schema * some comment about using json schema * more schema * prefixed denom schema and amount fixes * final transfer things * release note * connection is needed too * std is for schema * fixed de of u256 * tests behind flag * clean up
Farhad-Shabani
pushed a commit
that referenced
this pull request
Sep 9, 2024
* support for optional json schema * some comment about using json schema * more schema * prefixed denom schema and amount fixes * final transfer things * release note * connection is needed too * std is for schema * fixed de of u256 * tests behind flag * clean up
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.
Closes: #772
Description
We do coin router kind of thing in CosmWasm. It needs some IBC things, and it requests JsonSchema on types.
So I did it.
Also, I made serde of Amount consistently decimal(decimal string). It was from str decimal, and in coin it was decimal, but on its own if was hex string. So made on its own also decimal. For some it can be breaking change. But other way schema (and CosmWasm messages) would be bad/inconsistent.
PR author checklist:
unclog.docs/).Reviewer checklist:
Files changedin the GitHub PR explorer.