-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat(schema): specify JSON mapping #21243
Conversation
WalkthroughWalkthroughThe recent changes enhance the documentation of the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Kind
participant Validator
Developer->>Kind: Access Kind documentation
Kind->>Developer: Provide detailed encoding specifications
Developer->>Validator: Ensure data integrity during serialization
Validator->>Kind: Validate data formats
Kind->>Validator: Return encoding rules
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
schema/kind.go (4)
12-18
: Clarify the purpose of encoding specifications.The documentation provides a clear overview of Go and JSON encodings for each kind. Ensure that the purpose of specifying canonical deterministic encoding is well-understood, as it is crucial for maintaining data integrity.
30-33
: Clarify base64 encoding details forBytesKind
.The documentation specifies base64 encoding with standard encoding and padding. Ensure that this is consistently applied and that any deviations are documented.
84-89
: TheDecimalStringKind
regex does not enforce rules for leading/trailing zeros.The
DecimalFormat
regex pattern allows leading and trailing zeros, which contradicts the documentation requirements. Consider updating the regex to enforce these rules.
- Location:
schema/kind.go
DecimalFormat = ^-?[0-9]{1,50}(\.[0-9]{1,50})?([eE][-+]?[0-9]{1,2})?$
Analysis chain
Ensure correct handling of
DecimalStringKind
.The documentation specifies no leading or trailing zeros and specific exponential notation. Verify that these rules are enforced in the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify handling of `DecimalStringKind`. # Test: Search for `DecimalStringKind` usage. Expect: Compliance with encoding rules. rg --type go -A 5 $'DecimalStringKind'Length of output: 5979
Script:
#!/bin/bash # Find the definition of `decimalRegex` to verify its pattern. rg --type go 'decimalRegex'Length of output: 158
Script:
#!/bin/bash # Find the definition of `DecimalFormat` to inspect its regex pattern. rg --type go 'DecimalFormat'Length of output: 460
123-128
: AddressKind encoding does not match documentation requirements.The
fmtValue
function formatsAddressKind
using hexadecimal representation (0x%x
), which does not conform to the human-readable string encoding (e.g., bech32) specified in the documentation. Consider implementing a proper encoding mechanism forAddressKind
to ensure compliance.
- Location:
schema/testing/statesim/object_coll.go
, functionfmtValue
Analysis chain
Ensure proper address encoding for
AddressKind
.The documentation specifies a human-readable string encoding for addresses. Verify that this is correctly implemented.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify address encoding for `AddressKind`. # Test: Search for `AddressKind` usage. Expect: Compliance with human-readable string encoding. rg --type go -A 5 $'AddressKind'Length of output: 4156
Script:
#!/bin/bash # Search for functions or methods that handle encoding for `AddressKind`. rg --type go 'func ' -A 10 | grep -i -B 5 'AddressKind'Length of output: 742
Script:
#!/bin/bash # Retrieve the full implementation of the fmtValue function to understand how it handles AddressKind. ast-grep --lang go --pattern 'func fmtValue($_, $_) $_ { $$$ }'Length of output: 748
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- schema/kind.go (1 hunks)
Additional context used
Path-based instructions (1)
schema/kind.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (5)
schema/kind.go (5)
25-27
: Ensure UTF-8 string compliance forStringKind
.The documentation for
StringKind
specifies a UTF-8 string with no null characters. Verify that this requirement is consistently enforced throughout the codebase.Verification successful
UTF-8 Compliance for
StringKind
is EnforcedThe codebase includes checks to ensure that
StringKind
values are valid UTF-8 strings without null characters. This is verified by the presence of a UTF-8 validation check inschema/kind.go
and corresponding test cases inschema/kind_test.go
that handle invalid UTF-8 and null character scenarios.
schema/kind.go
: UTF-8 validation is performed forStringKind
.schema/kind_test.go
: Test cases ensure strings with null characters and invalid UTF-8 are correctly handled.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify enforcement of UTF-8 string compliance for `StringKind`. # Test: Search for `StringKind` usage. Expect: Compliance with UTF-8 string requirements. rg --type go -A 5 $'StringKind'Length of output: 28589
68-69
: Verify canonical encoding forInt64Kind
.The documentation specifies no leading zeros for canonical encoding. Ensure this is enforced in serialization logic.
106-110
: ReviewDurationKind
encoding specification.The documentation specifies a decimal string followed by 's'. Verify that this encoding is consistently applied.
97-103
: Check ISO 8601 compliance forTimeKind
.The documentation specifies ISO 8601 encoding with specific rules for canonical values. Ensure these are correctly implemented.
36-38
: Confirm numeric encoding forInt8Kind
.The documentation specifies numeric JSON encoding for
Int8Kind
. Verify that this is correctly implemented and tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK
* main: (76 commits) docs: more app v2 renaming (#21336) chore: update link in disclaimer (#21339) refactor(x/distribution): audit QA (#21316) docs: rename app v2 to app di when talking about runtime v0 (#21329) feat(schema): specify JSON mapping (#21243) fix(x/authz): bring back msg response in `DispatchActions` (#21044) chore: fix all lint issue since golangci-lint bump (#21326) refactor(x/mint): v0.52 audit x/mint (#21301) chore: fix spelling errors (#21327) feat: export genesis in simapp v2 (#21199) fix(baseapp)!: Halt at height now does not produce the halt height block (#21256) refactor(scripts): remove unused variable (#21320) chore(schema/testing): upgrade to go 1.23 iterators (#21282) chore: readmes + upgrading docs (#21271) feat(client): add auto cli for node service (#21074) ci: fix github workflow vulnerable to script injection (#21304) build(deps): Bump github.com/prometheus/client_golang from 1.19.1 to 1.20.0 (#21307) build(deps): use Go 1.23 instead of Go 1.22 (#21280) refactor(x/auth): audit x/auth changes (#21146) chore: remove todo: "abstract out staking message back to staking" (#21266) ...
Description
This PR specifies the default JSON encoding of schema values. This can be used for JSON bootstrapping initially and possibly other purposes. Unlike proto JSON, it specifies a canonical deterministic encoding and deals with addresses and decimals properly. Nothing is implemented here yet, just specification.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
Kind
type and its constants to clarify Go and JSON encodings, enhancing understanding of data types and serialization practices.StringKind
,BytesKind
, andDecimalStringKind
, specifying their expected formats and handling.TimeKind
andDurationKind
, detailing valid ranges and encoding practices.