-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
fix(x/tx): add an option to encode maps using amino json (ref #23513) #23539
Conversation
📝 WalkthroughWalkthroughThis pull request introduces changes to the Changes
Sequence DiagramsequenceDiagram
participant Encoder
participant Writer
participant MapData
Encoder->>Encoder: Check map validity
alt Map is valid
Encoder->>MapData: Construct map data
MapData->>Writer: Marshal key-value pairs
else Map is invalid
Encoder->>Writer: Write null
end
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
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
🧹 Nitpick comments (1)
x/tx/signing/aminojson/json_marshal.go (1)
245-256
: Robust implementation with room for optimization.The changes correctly handle invalid map states and provide proper null handling. Consider using a pre-allocated capacity for the map to optimize memory allocation.
-mapData := make(map[string]interface{}) +mapData := make(map[string]interface{}, val.Len())
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
x/tx/CHANGELOG.md
(2 hunks)x/tx/signing/aminojson/json_marshal.go
(1 hunks)x/tx/signing/aminojson/json_marshal_test.go
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
x/tx/CHANGELOG.md (1)
Pattern **/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
x/tx/signing/aminojson/json_marshal_test.go (2)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern **/*_test.go
: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
x/tx/signing/aminojson/json_marshal.go (1)
Pattern **/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: tests (03)
- GitHub Check: tests (02)
- GitHub Check: tests (01)
- GitHub Check: tests (00)
- GitHub Check: test-simapp-v2
- GitHub Check: test-sim-nondeterminism
- GitHub Check: test-integration
- GitHub Check: test-system-v2
- GitHub Check: build (arm64)
- GitHub Check: build (amd64)
- GitHub Check: Analyze
- GitHub Check: golangci-lint
- GitHub Check: Summary
🔇 Additional comments (2)
x/tx/signing/aminojson/json_marshal_test.go (1)
416-440
: Well-structured test implementation!The test covers both valid and invalid cases for map marshalling, with clear assertions and error checks.
x/tx/CHANGELOG.md (1)
36-39
: Well-documented changelog entries!The version entries are properly formatted, follow semantic versioning, and include appropriate PR references.
Also applies to: 64-67
…#23539) Co-authored-by: Alex | Interchain Labs <[email protected]> (cherry picked from commit aef1a2a) # Conflicts: # x/tx/CHANGELOG.md # x/tx/signing/aminojson/json_marshal.go # x/tx/signing/aminojson/json_marshal_test.go
…23539) (#23540) Co-authored-by: Julien Robert <[email protected]>
* main: (61 commits) build(deps): Bump cosmossdk.io/x/tx from 1.0.1 to 1.1.0 (#23547) feat(client/v2): add map support (#23544) fix(db): Fix pebbleDB integration (#23552) build(deps): Bump buf.build/gen/go/cometbft/cometbft/protocolbuffers/go from 1.36.3-20241120201313-68e42a58b301.1 to 1.36.4-20241120201313-68e42a58b301.1 in /api (#23520) build(deps): Bump github.com/golang/glog from 1.2.3 to 1.2.4 in /tools/hubl (#23541) fix(x/tx): add an option to encode maps using amino json (ref #23513) (#23539) build(deps): Bump cosmossdk.io/collections from 1.0.0 to 1.1.0 (#23534) build(deps): Bump github.com/bytedance/sonic from 1.12.7 to 1.12.8 in /log (#23516) chore: fix changelog for `v0.52.0-rc.2` (#23502) build(deps): Bump google.golang.org/protobuf from 1.36.3 to 1.36.4 in /x/upgrade (#23512) chore(collections): bring in `protocodec` in collections directly (#23515) feat(x/tx): add an option to encode maps using amino json (#23513) fix(codec): provide legacy amino og type (#23504) chore: remove unecessary SDKGasMeter impl (#23503) refactor: replace `golang.org/x/exp` with stdlib (#23501) build(deps): Bump google.golang.org/grpc from 1.69.4 to 1.70.0 in /tools/cosmovisor (#23496) build(deps): Bump google.golang.org/grpc from 1.69.4 to 1.70.0 in /store (#23494) build(deps): Bump github.com/hashicorp/go-plugin from 1.6.2 to 1.6.3 in /store (#23493) ci: Fix Ci job target (#23500) feat(server/v2): add swagger server component (#23486) ...
Follow-up of #23513
Afterwards i'll cherry-pick this in x/tx v0.13 after Noble's request. v0.13 is present in release/v0.50.x branch, hence the backport to that branch. v1 is tagged from main.
Then I'll bump in autocli.
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
Release Notes
New Features
Testing
Changelog
These updates improve the robustness and flexibility of the JSON encoding process, particularly for map and list data types.