chore: remove duplicate proto files for the same proto file #21648
Conversation
WalkthroughWalkthroughThe changes primarily involve a transition from using the Cosmos SDK's client context to a more direct RPC interface for blockchain interactions. Key modifications include the introduction of a new Changes
Possibly related PRs
Suggested labels
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
|
github-actions
bot
added
C:CLI
C:x/auth
C:x/consensus
C:server/v2
tac0turtle
requested review from
a team,
testinginprod,
raynaudoe,
facundomedica,
aaronc,
sontrinh16,
julienrbrt and
JulianToledano
as code owners
julienrbrt
added
backport/v0.50.x
![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
Outside diff range and nitpick comments (1)
server/v2/cometbft/go.mod (1)
Line range hint
4-16: Reminder: Update replace directives before release.Several replace directives have been added to the file, specifying local package paths for development purposes or to use modified versions of the packages.
Please ensure that these replace directives are removed or updated before releasing the project or when the local changes are merged into the main packages to avoid any conflicts or issues.
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (4)
server/v2/cometbft/client/grpc/cmtservice/query.pb.gois excluded by!**/*.pb.goserver/v2/cometbft/client/grpc/cmtservice/query.pb.gw.gois excluded by!**/*.pb.gw.goserver/v2/cometbft/client/grpc/cmtservice/types.pb.gois excluded by!**/*.pb.goserver/v2/cometbft/go.sumis excluded by!**/*.sum
Files selected for processing (10)
- client/grpc/cmtservice/block.go (1 hunks)
- client/grpc/cmtservice/rpc.go (1 hunks)
- client/grpc/cmtservice/service.go (13 hunks)
- client/grpc/cmtservice/status.go (1 hunks)
- client/grpc/cmtservice/validator.go (1 hunks)
- server/cmt_cmds.go (1 hunks)
- server/v2/cometbft/client/grpc/cmtservice/service.go (1 hunks)
- server/v2/cometbft/go.mod (6 hunks)
- x/auth/tx/service.go (1 hunks)
- x/consensus/keeper/keeper.go (1 hunks)
Files skipped from review due to trivial changes (1)
- x/consensus/keeper/keeper.go
Additional context used
Path-based instructions (8)
client/grpc/cmtservice/status.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/grpc/cmtservice/validator.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/grpc/cmtservice/block.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.client/grpc/cmtservice/rpc.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.server/v2/cometbft/client/grpc/cmtservice/service.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.x/auth/tx/service.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.server/cmt_cmds.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Additional comments not posted (30)
client/grpc/cmtservice/status.go (1)
10-12: LGTM!The changes to the
GetNodeStatusfunction are approved. The modifications simplify the function's implementation, enhance clarity, and improve efficiency by directly calling theStatusmethod on therpcparameter.The changes also conform to the Uber Golang style guide.
client/grpc/cmtservice/validator.go (1)
9-10: LGTM, but verify the function usage.The changes to the
getValidatorsfunction enhance clarity and efficiency by streamlining the RPC interactions. The code changes are approved.However, ensure that this change is thoroughly tested to confirm that it behaves as expected and that all function calls to
getValidatorshave been updated to pass theCometRPCparameter instead ofclient.Context.Run the following script to verify the function usage:
Verification successful
All instances of
getValidatorsare updated correctly.The function
getValidatorsis used only once in the codebase, and it correctly uses the new signature withCometRPCas a parameter. No further updates are necessary.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `getValidators` pass the `CometRPC` parameter. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'getValidators'Length of output: 662
Script:
#!/bin/bash # Description: Search for all instances of `getValidators` usage across the codebase. # Test: Ensure all calls to `getValidators` use the new signature with `CometRPC`. rg --type go 'getValidators'Length of output: 280
client/grpc/cmtservice/block.go (3)
10-17: LGTM!The code changes are approved. The refactoring from
client.ContexttoCometRPCis consistent with the overall goal of streamlining RPC interactions.
19-20: LGTM!The code changes are approved. The refactoring from
client.ContexttoCometRPCis consistent with the overall goal of streamlining RPC interactions. The direct call to theBlockmethod on theCometRPCtype is a cleaner approach.
Line range hint
23-35: LGTM!The code changes are approved. The refactoring from
client.ContexttoCometRPCis consistent with the overall goal of streamlining RPC interactions. The function correctly handles errors and follows the Uber Golang style guide.client/grpc/cmtservice/rpc.go (1)
12-36: LGTM!The
CometRPCinterface definition looks good:
- It consolidates relevant methods in one place, which improves modularity and maintainability.
- The methods have clear and descriptive names, making the interface easy to understand.
- The methods take a
context.Contextparameter, which is a good practice for handling timeouts and cancellations.- The methods return appropriate result types from the
coretypespackage, ensuring type safety.- The
TxandTxSearchmethods have parameters for enabling proofs, which is important for security.- The
BlockSearchandTxSearchmethods have pagination parameters, which is important for performance.- The
BlockSearchandTxSearchmethods have anorderByparameter, which provides flexibility in sorting results.server/v2/cometbft/client/grpc/cmtservice/service.go (5)
12-15: LGTM!The changes to the import statements are approved. The addition of the
addressandcmtservicepackages is necessary to support the refactored code.
18-18: LGTM!The type assertion is approved. It ensures that the
GetLatestValidatorSetResponsetype correctly implements theUnpackInterfacesMessageinterface, enhancing the handling of validator set responses.
26-26: LGTM!The
abciQueryFntype definition is approved. It provides a clean abstraction for the ABCI query functionality, enhancing code readability and maintainability.
28-36: LGTM!The changes to the
RegisterTendermintServicefunction are approved. The inclusion of theconsensusCodecparameter and the use ofcmtservice.NewQueryServeralign with the overall refactoring effort to leverage thecmtservicepackage's functionalities, promoting a more modular and extensible design.
38-42: LGTM!The changes to the
RegisterGRPCGatewayRoutesfunction are approved. The use ofcmtservice.RegisterServiceHandlerClientandcmtservice.NewServiceClientaligns with the overall refactoring effort to leverage thecmtservicepackage, ensuring proper registration and handling of the GRPC-gateway routes.client/grpc/cmtservice/service.go (14)
6-6: LGTM!The import of
cosmossdk.io/core/addresspackage is approved.
32-32: LGTM!The replacement of
clientCtxfield withrpcfield of typeCometRPCis approved.
33-33: LGTM!The renaming of
queryFnfield is approved.
34-34: LGTM!The addition of
consensusCodecfield of typeaddress.Codecis approved.
40-42: LGTM!The updates to the
NewQueryServerfunction parameters are approved. They reflect the shift towards using the newCometRPCtype for RPC interactions and handling consensus address encoding.
45-46: LGTM!The updates to the
NewQueryServerfunction to use the newrpcfield are approved. They promote a more cohesive and modular approach to handling RPC interactions.
52-52: LGTM!The update to the
GetSyncingmethod to use the newrpcfield is approved. It aligns with the overall refactoring to use the new RPC structure.
Line range hint
64-75: LGTM!The updates to the
GetLatestBlockmethod to use the newrpcfield andconsensusCodecare approved. They align with the overall refactoring to use the new RPC structure and handle consensus address encoding.
Line range hint
89-98: LGTM!The updates to the
GetBlockByHeightmethod to use the newrpcfield andconsensusCodecare approved. They align with the overall refactoring to use the new RPC structure and handle consensus address encoding.Also applies to: 103-103
122-122: LGTM!The update to the
GetLatestValidatorSetmethod to pass the newrpcandconsensusCodecfields to theValidatorsOutputfunction is approved. It aligns with the overall refactoring to use the new RPC structure and handle consensus address encoding.
Line range hint
144-153: LGTM!The updates to the
GetValidatorSetByHeightmethod to use the newrpcfield and pass the newrpcandconsensusCodecfields to theValidatorsOutputfunction are approved. They align with the overall refactoring to use the new RPC structure and handle consensus address encoding.
Line range hint
165-189: LGTM!The updates to the
ValidatorsOutputfunction signature and implementation to use the newrpcandconsCodecparameters are approved. They align with the overall refactoring to use the new RPC structure and handle consensus address encoding.
Line range hint
207-291: LGTM!The update to the
GetNodeInfomethod to use the newrpcfield is approved. It aligns with the overall refactoring to use the new RPC structure. The rest of the changes in this section are just code movements without any functional impact.
292-292: LGTM!The update to the
RegisterTendermintServicefunction to pass the newnodeandclientCtx.ConsensusAddressCodecto theNewQueryServerfunction is approved. It aligns with the overall refactoring to use the new RPC structure and handle consensus address encoding.server/v2/cometbft/go.mod (3)
59-61: Verify the necessity and compatibility of the added dependencies.Several new dependencies have been added to the require block, including
github.com/DataDog/datadog-go,github.com/Microsoft/go-winio,github.com/felixge/httpsnoop,github.com/gogo/googleapis,github.com/gorilla/handlers,github.com/gorilla/mux,github.com/grpc-ecosystem/go-grpc-middleware, andgit.luolix.top/huandu/skiplist.Please ensure that these dependencies are necessary for the project and align with its goals and architecture. Also, verify that they are compatible with the existing dependencies and the project's license.
Also applies to: 91-91, 98-98, 109-112, 123-123
75-75: Verify the impact of removing dependencies from the require block.
github.com/cosmos/cosmos-protoandgit.luolix.top/golang/protobufhave been removed from the require block but reintroduced in the indirect dependencies section.Please ensure that removing these dependencies from the require block does not have any unintended consequences on the project's functionality and that their versions in the indirect dependencies section are compatible with the project's requirements.
Also applies to: 102-102
175-175: Verify the compatibility and security of the added indirect dependencies.Several new indirect dependencies have been added, including
golang.org/x/mod,golang.org/x/tools,google.golang.org/genproto/googleapis/api, andgoogle.golang.org/grpc.Please ensure that these indirect dependencies are compatible with the project and do not introduce any security vulnerabilities or licensing issues. Also, verify that their versions are appropriate for the project's requirements.
Also applies to: 181-181, 183-183, 185-185
x/auth/tx/service.go (1)
152-157: LGTM!The changes to retrieve the node using
s.clientCtx.GetNode()and handle potential errors before callingGetProtoBlockare approved. This enhances the function's robustness and improves error handling.server/cmt_cmds.go (1)
44-48: Approved: The change improves the control flow by separating node retrieval from status retrieval.Retrieving the node object using
clientCtx.GetNode()before passing it tocmtservice.GetNodeStatusenhances clarity and could potentially improve error handling related to node retrieval.
|
Adding the backport v0.52 labels due to change in the SDK and in cometbft server. |
|
I think this should be reverted: 13819f5. It is used. |
This reverts commit 13819f5.
(cherry picked from commit c0e84af) # Conflicts: # server/v2/cometbft/go.mod
* main: test: fix sims (#21735) build: bump proto-builder (#21730) refactor(schema)!: rename IntegerStringKind and DecimalStringKind (#21694) feat(types/collections): add `LegacyDec` collection value (#21693) refactor(server): alias AppOptions to coreserver.DynamicConfig (#21711) refactor(simapp): simplify simapp di (#21718) feat: replace the cosmos-db usecases in the tests with `core/testing` (#21525) feat(runtime/v2): store loader on simappv2 (#21704) docs(x/auth): vesting (#21715) build(deps): Bump google.golang.org/grpc from 1.66.1 to 1.66.2 (#21670) refactor(systemtest): Add cli.RunAndWait for common operations (#21689) fix(runtime/v2): provide default factory options if unset in app builder (#21690) chore: remove duplicate proto files for the same proto file (#21648) feat(x/genutil): add better error messages for genesis validation (#21701) build(deps): Bump cosmossdk.io/core from 1.0.0-alpha.1 to 1.0.0-alpha.2 (#21698) test: migrate e2e/bank to system tests (#21607) chore: fix the gci lint issue in testutil (#21695) docs(x/authz): update grant docs (#21677)
Description
this pr removes duplicate registration of proto files
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
New Features
CometRPCinterface for enhanced interaction with the CometBFT blockchain.Improvements
Bug Fixes