refactor!: remove duplicate block #23437
Conversation
📝 WalkthroughWalkthroughThis pull request introduces changes to the Cosmos SDK's block data handling, focusing on deprecating and removing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
tac0turtle
added
the
backport/v0.52.x
tac0turtle
requested review from
julienrbrt,
JulianToledano and
a team
as code owners
julienrbrt
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
client/grpc/cmtservice/service.go (1)
113-114: LGTM! Good optimization.Returning an empty block instead of the full block data reduces response size while maintaining backward compatibility. This change aligns with the PR objective to eliminate duplicate block data.
However, consider adding a comment explaining why we're returning an empty block to help future maintainers understand the design decision.
- Block: &v1.Block{}, // fill with empty block to reduce response size + Block: &v1.Block{}, // Return empty block as this field is deprecated. Users should query Comet directly or use SdkBlockCHANGELOG.md (3)
56-56: Fix typo in API Breaking Changes entryThe entry has a typo in "x/distinction" - should be "x/distribution".
- (x/distinction) [#8918](https://github.com/cosmos/cosmos-sdk/pull/8918) Fix module's parameters validation. + (x/distribution) [#8918](https://github.com/cosmos/cosmos-sdk/pull/8918) Fix module's parameters validation.
Line range hint
1-1000: Improve changelog formatting consistencyThe changelog has some inconsistent formatting:
- Some PR links use full URLs while others use relative paths
- Some bullet points use
*while others use-- Indentation is inconsistent in some sections
Consider standardizing the formatting:
- Use consistent bullet point style (prefer
-)- Use consistent PR link format (prefer relative paths)
- Maintain consistent indentation throughout
Line range hint
1-1000: Add missing PR linksSome changelog entries are missing links to their corresponding PRs/issues. For example:
- "Fix negative index accesses in CompactUnmarshal,GetIndex,SetIndex"
- Several entries in the API Breaking Changes section
Add the missing PR/issue links to improve traceability.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
client/grpc/cmtservice/query.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)api/cosmos/base/tendermint/v1beta1/query.pulsar.go(3 hunks)client/grpc/cmtservice/service.go(2 hunks)crypto/keys/multisig/multisig.go(0 hunks)proto/cosmos/base/tendermint/v1beta1/query.proto(1 hunks)
🔥 Files not summarized due to errors (1)
- api/cosmos/base/tendermint/v1beta1/query.pulsar.go: Error: Server error: no LLM provider could handle the message
💤 Files with no reviewable changes (1)
- crypto/keys/multisig/multisig.go
🧰 Additional context used
📓 Path-based instructions (3)
api/cosmos/base/tendermint/v1beta1/query.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
client/grpc/cmtservice/service.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: test-system-v2
- GitHub Check: Analyze
- GitHub Check: Summary
🔇 Additional comments (5)
client/grpc/cmtservice/service.go (1)
7-7: LGTM!The new import is required for creating empty v1.Block objects and follows Go import conventions.
proto/cosmos/base/tendermint/v1beta1/query.proto (1)
109-112: LGTM! Well-structured proto changes.The changes follow protobuf best practices:
- Proper deprecation marking with clear message
- New field addition with correct version tracking
- Maintains backward compatibility while encouraging migration to sdk_block
api/cosmos/base/tendermint/v1beta1/query.pulsar.go (3)
10888-10889: Well-documented deprecation notice!The deprecation notice clearly indicates that users should migrate to
sdk_blockinstead of using the deprecatedBlockfield.
10921-10921: Properly marked deprecated getter methodThe getter method is correctly marked as deprecated, maintaining API consistency with the deprecated field.
11668-11886: Verify API version compatibilityThe protobuf definitions include version information for cosmos-sdk. Please ensure that the version numbers are compatible with the current release.
✅ Verification successful
Version compatibility is correctly maintained
The cosmos-sdk version 0.47 used in the protobuf definitions is consistent with the module's scope and follows the established versioning patterns. Different versions across the codebase represent feature introduction points as per ADR-044 guidelines.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for version compatibility in other proto files # Look for any version mismatches in proto files rg -A 1 "cosmos-sdk [0-9]+" | grep -v "0.47\|0.46\|0.43\|0.52"Length of output: 30178
Co-authored-by: Alex | Interchain Labs <[email protected]> (cherry picked from commit 7f827c5) # Conflicts: # CHANGELOG.md # api/cosmos/base/tendermint/v1beta1/query.pulsar.go
Co-authored-by: Marko <[email protected]> Co-authored-by: Julien Robert <[email protected]>
Description
Closes: #23364
This pr removed the comet block as it is duplicated with the sdk representation. If the user would like the comet block they should query comet.
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.mdReviewers 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
Deprecations
Blockfield inGetBlockByHeightResponseandGetLatestBlockResponsesdk_blockfield as a replacementAPI Changes
GetBlockByHeightmethod to return an empty Comet blockNewLegacyAminoPubKeyfunction for multisig keysVersioning
These changes aim to improve API clarity and streamline block data handling in the Cosmos SDK.