Skip to content
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(sims): Use liveness matrix for val sign status in sims #21952

Merged
merged 2 commits into from
Sep 27, 2024
Merged

Conversation

alpe
Copy link
Contributor

@alpe alpe commented Sep 27, 2024

Description

Closes: #17912

Use liveness matrix for validator sign status in sims

Summary by CodeRabbit

  • Documentation

    • Updated changelog to reflect changes in the "sims" and "cli" modules, including new handling for validator sign status and query updates.
  • Bug Fixes

    • Improved accuracy of voting information by dynamically reflecting the signing status of blocks in the simulation module.

@alpe alpe requested a review from a team as a code owner September 27, 2024 10:17
@alpe alpe marked this pull request as draft September 27, 2024 10:17
Copy link
Contributor

coderabbitai bot commented Sep 27, 2024

📝 Walkthrough

Walkthrough

The pull request introduces modifications to the "sims" module and the x/simulation/mock_cometbft.go file. It updates the changelog to reflect changes in validator sign status handling using a liveness matrix and clarifies the query for address-by-acc-num in the "cli" module. Additionally, it alters the RandomRequestFinalizeBlock function to dynamically set the commit status based on whether a block is signed, replacing a previous hardcoded value.

Changes

Files Change Summary
CHANGELOG.md Updated to reflect changes in the "sims" module regarding validator sign status and "cli" module.
x/simulation/mock_cometbft.go Modified RandomRequestFinalizeBlock to dynamically assign commit status based on block signing.

Assessment against linked issues

Objective Addressed Explanation
x/simulation: the liveness matrix no longer affects simulation (#17912)

Possibly related PRs

Suggested labels

C:x/tx, backport/v0.50.x

Suggested reviewers

  • sontrinh16
  • tac0turtle
  • aaronc
  • kocubinski

📜 Recent review details

Configuration used: .coderabbit.yml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between dcf00cf and 0ef5352.

📒 Files selected for processing (2)
  • CHANGELOG.md (1 hunks)
  • x/simulation/mock_cometbft.go (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
CHANGELOG.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

x/simulation/mock_cometbft.go (1)

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

🪛 Markdownlint
CHANGELOG.md

57-57: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

🔇 Additional comments (5)
x/simulation/mock_cometbft.go (1)

154-168: LGTM: Improved simulation accuracy for validator signing status

The changes effectively implement the PR objective of using the liveness matrix for validator sign status in simulations. By introducing the commitStatus variable and setting it dynamically based on whether a validator signed or not, the code now more accurately reflects real-world validator behavior in the simulation.

This improvement enhances the fidelity of the simulation and addresses the issue mentioned in #17912, where the liveness matrix was not impacting the simulation process.

The implementation is clean, follows Go best practices, and integrates well with the existing code structure.

CHANGELOG.md (4)

Line range hint 1-1170: Well-structured changelog with comprehensive updates for v0.47.0

The CHANGELOG.md file provides a detailed account of changes, improvements, and bug fixes for the Cosmos SDK v0.47.0 release. The structure is clear and organized, making it easier for developers to understand the impact of the update on their applications.

Key highlights for developers:

  1. Significant API breaking changes, including updates to modules like x/gov, x/staking, and x/auth.
  2. New features such as the implementation of ADR-038 for state streaming and support for SIGN_MODE_TEXTUAL.
  3. Important bug fixes, including security patches and improvements to various modules.

Suggestions for improvement:

  1. Consider adding more context or examples for complex changes to help developers understand their impact.
  2. Ensure consistent formatting throughout the document, particularly for bullet points and code references.
  3. Add links to relevant documentation or ADRs for major changes to provide additional context.
🧰 Tools
🪛 Markdownlint

56-56: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


57-57: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


Line range hint 324-349: Critical state machine breaking changes require careful consideration

The "State Machine Breaking" section contains several important changes that developers must carefully review and address when upgrading to v0.47.0. Some of the most impactful changes include:

  1. Changes to x/gov and x/group modules, affecting proposal handling and voting periods.
  2. Updates to x/staking module, modifying validator and delegation behavior.
  3. Modifications to x/bank module, including changes to balance and supply tracking.

These changes may require significant updates to application logic and data migration strategies. Developers should pay close attention to:

  • The new approach for tracking proposals in voting periods in x/gov.
  • Changes to validator distribution info and unbonding delegation entries in x/staking.
  • Updates to balance and supply tracking in x/bank.

It's crucial to thoroughly test applications after implementing these changes to ensure compatibility and correct functionality with the new state machine behavior.

🧰 Tools
🪛 Markdownlint

56-56: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


57-57: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


Line range hint 351-426: Significant API breaking changes require code updates

The "API Breaking Changes" section outlines numerous modifications that will require developers to update their code. Some of the most notable changes include:

  1. Updates to the AppModule interface and module manager, affecting how modules are registered and managed.
  2. Changes to various keeper methods across modules, often adding new parameters or modifying return types.
  3. Renaming and restructuring of packages, such as moving x/params functionality into individual modules.

These changes will have a substantial impact on existing applications. Developers should be prepared to:

  • Review and update all usage of affected APIs, particularly focusing on module integration and keeper interactions.
  • Adapt to new patterns for module management and parameter handling.
  • Update import paths and package references to align with the restructured SDK.

It's recommended to approach the upgrade process methodically, addressing one module or component at a time, and thoroughly testing each change to ensure compatibility with the new API structure.

🧰 Tools
🪛 Markdownlint

56-56: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


57-57: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


Line range hint 536-624: Notable improvements enhance functionality and performance

The "Improvements" section showcases several positive changes that developers and users can benefit from in v0.47.0. Some standout improvements include:

  1. Enhanced support for vote extensions and custom panic recovery in baseapp.
  2. Improved CLI commands, including new options for key management and transaction signing.
  3. Performance optimizations in various modules, such as x/bank and store operations.
  4. New query endpoints and improved error handling across multiple modules.

These improvements offer several advantages:

  • Developers can leverage new features like vote extensions to build more sophisticated governance mechanisms.
  • Enhanced CLI commands provide better flexibility and control for users interacting with the SDK.
  • Performance optimizations should lead to more efficient execution of transactions and queries.
  • Improved error handling and new query endpoints will make it easier to debug issues and retrieve necessary information.

To fully benefit from these improvements, developers should review their existing code and consider updating relevant sections to utilize the new features and optimizations. Users of SDK-based applications may experience improved performance and functionality as these changes are adopted.

🧰 Tools
🪛 Markdownlint

56-56: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


57-57: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Warning

Review ran into problems

🔥 Problems

Error running LanguageTool: LanguageTool error: Unknown error


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

This comment has been minimized.

@alpe alpe marked this pull request as ready for review September 27, 2024 10:19
@julienrbrt julienrbrt added the backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release label Sep 27, 2024
@julienrbrt julienrbrt added this pull request to the merge queue Sep 27, 2024
Merged via the queue into main with commit 0d9b416 Sep 27, 2024
74 of 75 checks passed
@julienrbrt julienrbrt deleted the alex/17912 branch September 27, 2024 11:17
mergify bot pushed a commit that referenced this pull request Sep 27, 2024
(cherry picked from commit 0d9b416)

# Conflicts:
#	CHANGELOG.md
julienrbrt added a commit that referenced this pull request Sep 27, 2024
@coderabbitai coderabbitai bot mentioned this pull request Sep 27, 2024
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release C:Simulations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x/simulation: the liveness matrix no longer affects simulation
3 participants