Skip to content

Conversation

@DongLieu
Copy link
Contributor

@DongLieu DongLieu commented Mar 17, 2025

Description

Closes: #23758


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

  • Chores

    • Updated core dependency versions to enhance compatibility and stability.
    • Removed legacy configuration files for protocol generation and project analysis.
  • Refactor

    • Streamlined internal dependency management and module initialization.
    • Adjusted public interfaces for improved consistency with current standards.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 17, 2025

📝 Walkthrough

Walkthrough

This pull request updates dependency versions in the benchmark’s Go modules, refactors key injection in the depinject module, and revises the AppModule implementation in the module file. It also removes several protocol buffer and SonarQube configuration files, streamlining both dependency management and build settings in the benchmark tool.

Changes

File(s) Change Summary
tools/benchmark/go.mod Downgrade github.com/cosmos/cosmos-sdk from v0.53.0 to v0.50.11; Upgrade github.com/spf13/cobra from v1.8.1 to v1.9.1.
tools/benchmark/module/depinject.go Remove StoreKeyRegistrar interface and its field; add KVStoreServiceFactory type alias; update the Input struct accordingly.
tools/benchmark/module/module.go Remove obsolete imports, update method signatures (ignoring context parameters), and change the interface registration parameter from registry.InterfaceRegistrar to codectypes.InterfaceRegistry.
tools/benchmark/proto/… Delete configuration files: buf.gen.gogo.yaml, buf.gen.pulsar.yaml, and buf.yaml.
tools/benchmark/sonar-project.properties Remove SonarQube configuration file.

Assessment against linked issues

Objective Addressed Explanation
Refactor tools/benchmark to use new appmodule syntax and updated proto build setup (#23758)

Possibly related PRs

Suggested labels

C:Cosmovisor, C:core/testing, C:collections, C:server/v2, C:x/accounts

Suggested reviewers

  • sontrinh16
  • akhilkumarpilli
  • julienrbrt

📜 Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 70c5e77 and 08d45e4.

⛔ Files ignored due to path filters (2)
  • tools/benchmark/go.sum is excluded by !**/*.sum
  • tools/benchmark/proto/buf.lock is excluded by !**/*.lock
📒 Files selected for processing (7)
  • tools/benchmark/go.mod (1 hunks)
  • tools/benchmark/module/depinject.go (1 hunks)
  • tools/benchmark/module/module.go (4 hunks)
  • tools/benchmark/proto/buf.gen.gogo.yaml (0 hunks)
  • tools/benchmark/proto/buf.gen.pulsar.yaml (0 hunks)
  • tools/benchmark/proto/buf.yaml (0 hunks)
  • tools/benchmark/sonar-project.properties (0 hunks)
💤 Files with no reviewable changes (4)
  • tools/benchmark/proto/buf.yaml
  • tools/benchmark/proto/buf.gen.gogo.yaml
  • tools/benchmark/sonar-project.properties
  • tools/benchmark/proto/buf.gen.pulsar.yaml
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Golang code for conformity with the Ub...

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

  • tools/benchmark/module/depinject.go
  • tools/benchmark/module/module.go
🪛 GitHub Actions: Tests / Code Coverage
tools/benchmark/go.mod

[error] 1-1: updates to go.mod needed, disabled by -mod=readonly; to update it: go mod tidy

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Summary
🔇 Additional comments (9)
tools/benchmark/go.mod (1)

11-11:

✅ Verification successful

Version changes in dependencies need to be verified

The version of cosmos-sdk has been downgraded from v0.53.0 to v0.50.11, while the version of cobra has been upgraded from v1.8.1 to v1.9.1.

Also applies to: 14-14


🏁 Script executed:

#!/bin/bash
# Check for compatibility issues in the dependency changes
# Look at the changelog for spf13/cobra to understand what changed between v1.8.1 and v1.9.1
curl -s https://api.github.com/repos/spf13/cobra/releases/tags/v1.9.1 | jq '.body'

# Check release notes for cosmos-sdk v0.50.11 to understand if it meets requirements
curl -s https://api.github.com/repos/cosmos/cosmos-sdk/releases/tags/v0.50.11 | jq '.body'

Length of output: 1958


Dependency Versions Verified – Proceed with Caution

We've checked the release notes for both dependencies:

  • spf13/cobra: The v1.9.1 release notes show minor bug fixes (e.g., a fix to the CompletionFunc implementation) without any breaking changes compared to v1.8.1.
  • cosmos-sdk: The v0.50.11 release notes confirm this is a patch release with bug fixes and improvements. However, note that the maintenance policy indicates that v0.50.x will soon be limited to bug fixes only, which might be a consideration if broader feature support was expected with the previously used v0.53.0.

Action Items:

  • Verify that downgrading from v0.53.0 to v0.50.11 for cosmos-sdk meets all long-term integration requirements.
  • Confirm that all downstream components are compatible with these dependency versions.

This applies to changes at tools/benchmark/go.mod on lines 11 and 14.

tools/benchmark/module/depinject.go (3)

29-29: New type alias added to replace usage of store.KVStoreServiceFactory

The new KVStoreServiceFactory type alias simplifies the code and makes it more maintainable by localizing the function type definition.


36-36: Store factory type updated to use local type alias

Changed from using store.KVStoreServiceFactory to the local type alias KVStoreServiceFactory. This maintains functionality while making the code more consistent with the backported version.


48-51: Simplified key registration logic

The key registration logic has been simplified by removing the conditional check for in.Registrar and the call to RegisterKey(sk). This change aligns with the removal of the StoreKeyRegistrar interface and simplifies the dependency structure.

tools/benchmark/module/module.go (5)

20-22: Updated imports to match SDK v0.50.11 requirements

The imports have been updated to use the appropriate packages from the SDK v0.50.11. This includes adding codectypes and sdk imports while removing the no longer needed registry and transaction imports.


27-28: Interface compliance assertions reordered

The order of interface assertions has been changed but their functionality remains the same. This is a minor style change that doesn't affect functionality.


56-58: Updated ExportGenesis method to ignore context parameter

The method signature has been modified to ignore the context parameter with an underscore, which conforms to Go conventions for unused parameters and matches the requirements of the v0.50.11 SDK.


80-82: Updated ValidateGenesis method to ignore a parameter

The parameter is now properly ignored with an underscore, following Go conventions for unused parameters.


93-98: Updated RegisterInterfaces method signature and implementation

The method now:

  1. Uses codectypes.InterfaceRegistry instead of registry.InterfaceRegistrar
  2. Registers implementations as (*sdk.Msg)(nil) instead of (*transaction.Msg)(nil)
  3. Updates the call to msgservice.RegisterMsgServiceDesc accordingly

These changes align with the SDK v0.50.11 requirements while maintaining the same functionality.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ 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>, please review it.
    • 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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @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 generate docstrings to generate docstrings for this 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.

@aljo242
Copy link
Contributor

aljo242 commented Mar 17, 2025

@DongLieu this task is to backport it to release/v0.53.x. I can clarify in the issue

@DongLieu
Copy link
Contributor Author

im sorry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backport tools/benchmark

2 participants