chore(x/staking): revert proto breakage #21666

julienrbrt · 2024-09-11T22:18:12Z

Description

Makes impossible to use latest api from main in 0.52.

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...

included the correct type prefix in the PR title, you can find examples of the prefixes below:
confirmed ! in the type prefix if API or client breaking change
targeted the correct branch (see PR Targeting)
provided a link to the relevant issue or specification
reviewed "Files changed" and left comments if necessary
included the necessary unit and integration tests
added a changelog entry to CHANGELOG.md
updated the relevant documentation or specification, including comments for documenting Go code
confirmed all CI checks have passed

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...

confirmed the correct type prefix in the PR title
confirmed all author checklist items have been addressed
reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

New Features
- Introduced new message types for querying historical staking information, enhancing the staking query API.
- Added support for historical information retrieval based on specified blockchain heights.
- Expanded data structures in the staking module for improved tracking of validator states and updates.
Bug Fixes
- Adjusted message definitions to ensure backward compatibility and correct serialization behavior.
Documentation
- Updated protocol definitions to reflect new message types and their intended usage.

coderabbitai · 2024-09-11T22:19:28Z

Warning

Rate limit exceeded

@julienrbrt has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 22 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between 476dbff and dbe5eeb.

Walkthrough

This pull request introduces significant modifications across multiple files, primarily focusing on protocol buffer definitions related to multisig accounts and staking queries. Key changes include the removal of certain import statements, the addition of new message types for handling historical staking information, and the introduction of new data structures for validator updates. The updates reflect a restructuring of the modules to enhance functionality and accommodate new features, while also marking some existing elements as deprecated.

Changes

Files	Change Summary
`api/cosmos/accounts/defaults/multisig/v1/multisig.pulsar.go`	Removed an import statement; modified protocol buffer definitions with extensive changes to message structures.
`api/cosmos/staking/v1beta1/query.pulsar.go`	Added new message types for querying historical staking information; modified initialization logic.
`x/staking/proto/cosmos/staking/v1beta1/query.proto`	Introduced a new RPC method for historical info queries; added corresponding request and response message types.
`x/staking/proto/cosmos/staking/v1beta1/staking.proto`	Added new message definitions for historical information and validator updates; marked existing messages as deprecated.

Possibly related PRs

refactor: decouple comet from modules #21382: This PR involves the removal of QueryHistoricalInfoRequest and QueryHistoricalInfoResponse types, which are related to the protocol buffer definitions modified in the main PR.
fix(x/accounts/lockup): prevent double withdraw #21619: This PR modifies the multisig.pulsar.go file by renaming field descriptors and adding new message types, aligning with the extensive changes made to protocol buffer definitions in the main PR.

Suggested labels

C:x/staking, C:x/genutil, C:x/authz, C:x/accounts/multisig

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

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)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

Outside diff range and nitpick comments (3)

x/staking/proto/cosmos/staking/v1beta1/staking.proto (2)

21-26: Approve addition of HistoricalInfo message, but note deprecation.

The new HistoricalInfo message is well-structured and serves the purpose of storing historical validator state data. However, it's important to be aware that this message is marked as deprecated, indicating potential future removal or replacement.

Consider adding a comment to clarify the reason for the deprecation and any planned migration path.

397-401: Approve addition of ValidatorUpdates message, but address TODO and deprecation.

The new ValidatorUpdates message serves a clear purpose of holding an array of validator update objects. However, please note:

The message is marked as deprecated, suggesting potential removal or replacement in the future. Consider adding a comment to clarify the reason and any planned migration path.

There is a TODO comment indicating that this message may be moved to a different proto package (proto/cosmos/base) to separate modules from tendermint dependency. It would be beneficial to create a follow-up issue to track this proposed refactoring.

api/cosmos/staking/v1beta1/query.pulsar.go (1)

15301-15382: Code segment looks good, but note the deprecation.

The QueryHistoricalInfoRequest and QueryHistoricalInfoResponse message type definitions look correct and follow the expected structure.

However, note that these message types are marked as deprecated using comments. This indicates that they should not be used in new code and may be removed in a future version of the API.

Consider updating any code that uses these deprecated message types to use the recommended alternatives, if available.

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 4fe934e and 476dbff.

Files ignored due to path filters (5)

api/cosmos/staking/v1beta1/query_grpc.pb.go is excluded by !**/*.pb.go
x/accounts/defaults/multisig/v1/multisig.pb.go is excluded by !**/*.pb.go
x/staking/types/query.pb.go is excluded by !**/*.pb.go
x/staking/types/query.pb.gw.go is excluded by !**/*.pb.gw.go
x/staking/types/staking.pb.go is excluded by !**/*.pb.go

Files selected for processing (4)

api/cosmos/accounts/defaults/multisig/v1/multisig.pulsar.go (2 hunks)
api/cosmos/staking/v1beta1/query.pulsar.go (22 hunks)
x/staking/proto/cosmos/staking/v1beta1/query.proto (2 hunks)
x/staking/proto/cosmos/staking/v1beta1/staking.proto (2 hunks)

Additional context used

Path-based instructions (2)

api/cosmos/accounts/defaults/multisig/v1/multisig.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

api/cosmos/staking/v1beta1/query.pulsar.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

Additional comments not posted (10)

x/staking/proto/cosmos/staking/v1beta1/query.proto (3)

115-119: LGTM, but note the deprecation.

The HistoricalInfo RPC method is correctly defined in the Query service. However, please be aware that it is marked as deprecated, indicating that it may be removed or replaced in future versions. Consider updating any code that relies on this method.

371-375: LGTM, but note the deprecation.

The QueryHistoricalInfoRequest message is correctly defined with a single height field. However, please be aware that it is marked as deprecated, indicating that it may be removed or replaced in future versions. Consider updating any code that relies on this message type.

379-383: LGTM, but note the deprecation.

The QueryHistoricalInfoResponse message is correctly defined with a single hist field of type HistoricalInfo. However, please be aware that both the message and its field are marked as deprecated, indicating that they may be removed or replaced in future versions. Consider updating any code that relies on this message type.

x/staking/proto/cosmos/staking/v1beta1/staking.proto (1)

12-13: Approve addition of new imports.

The new import statements for cometbft/types/v1/types.proto and cometbft/abci/v1/types.proto are necessary to support the newly added HistoricalInfo and ValidatorUpdates messages. The imports are correctly specified and scoped to the required types.

api/cosmos/accounts/defaults/multisig/v1/multisig.pulsar.go (1)

Line range hint 1-1: This file contains generated code and should not be manually edited.

This Go code file is generated from protocol buffer definitions by protoc-gen-go-pulsar. To make changes, update the corresponding .proto files and re-generate this file. Do not manually edit this generated code.

api/cosmos/staking/v1beta1/query.pulsar.go (5)

11823-12225: Code segment looks good.

The generated Protobuf code for QueryHistoricalInfoRequest and QueryHistoricalInfoResponse message types looks correct and follows the expected structure. No issues found.

12680-12680: Code segment looks good.

The generated Protobuf code for QueryPoolRequest message type looks correct and follows the expected structure. No issues found.

13038-13038: Code segment looks good.

The generated Protobuf code for QueryPoolResponse message type looks correct and follows the expected structure. No issues found.

13471-13471: Code segment looks good.

The generated Protobuf code for QueryParamsRequest message type looks correct and follows the expected structure. No issues found.

13829-13829: Code segment looks good.

The generated Protobuf code for QueryParamsResponse message type looks correct and follows the expected structure. No issues found.

sontrinh16 · 2024-09-11T22:40:56Z

just need to add back HistoricalInfo to queryserver and autocli

* main: refactor(server/v2/cometbft): use only protov1 and backport #21084 (#21681) chore(x/staking): revert proto breakage (#21666) build(deps): bump rocksdb & core & update ci codeowners (#21652)

julienrbrt added 2 commits September 12, 2024 00:17

chore(x/staking): revert proto breakage

98bde1a

make proto-gen

476dbff

julienrbrt assigned kocubinski and sontrinh16 Sep 11, 2024

julienrbrt requested review from facundomedica, testinginprod, JulianToledano, ziscky, a team and sontrinh16 as code owners September 11, 2024 22:18

github-actions bot added C:x/staking C:x/accounts C:x/accounts/multisig labels Sep 11, 2024

coderabbitai bot reviewed Sep 11, 2024

View reviewed changes

julienrbrt added 2 commits September 12, 2024 10:43

deprecate query and hide command

f0fb443

updates

dbe5eeb

sontrinh16 approved these changes Sep 12, 2024

View reviewed changes

tac0turtle approved these changes Sep 12, 2024

View reviewed changes

julienrbrt added this pull request to the merge queue Sep 12, 2024

Merged via the queue into main with commit 5475903 Sep 12, 2024
73 of 74 checks passed

julienrbrt deleted the julien/revert-protobreak branch September 12, 2024 09:27

This was referenced Sep 12, 2024

refactor(x/bank/v2): migrate to handlers #21659

Merged

ci: remove duplicate gosec & lint fixes #21685

Merged

This was referenced Sep 12, 2024

feat(sims): Add sims2 framework and factory methods #21613

Merged

chore: remove duplicate proto files for the same proto file #21648

Merged

refactor(genutil): Use sdk types genesis validator #21678

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(x/staking): revert proto breakage #21666

chore(x/staking): revert proto breakage #21666

julienrbrt commented Sep 11, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 11, 2024 •

edited

Loading

Rate limit exceeded

Walkthrough

Changes

Possibly related PRs

Suggested labels

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

coderabbitai bot left a comment

sontrinh16 commented Sep 11, 2024

chore(x/staking): revert proto breakage #21666

chore(x/staking): revert proto breakage #21666

Conversation

julienrbrt commented Sep 11, 2024 • edited by coderabbitai bot Loading

Description

Author Checklist

Reviewers Checklist

Summary by CodeRabbit

coderabbitai bot commented Sep 11, 2024 • edited Loading

Rate limit exceeded

Walkthrough

Changes

Possibly related PRs

Suggested labels

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

Documentation and Community

coderabbitai bot left a comment

Choose a reason for hiding this comment

sontrinh16 commented Sep 11, 2024

julienrbrt commented Sep 11, 2024 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 11, 2024 •

edited

Loading