refactor(evidence): migrate to env var

[tac0turtle]

Description

ref #19290
closes: #19487

migrate evidence module to use env, this leave consensus related things to context for now. This will be removed before the next pr

---

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

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

  • Enhanced the evidence module with an improved environment setup for better logging and event management.

• Refactor

  • Updated evidence keeper creation to utilize a unified environment parameter.
  • Moved BeginBlocker logic and made HandleEquivocation function private for better encapsulation.

• Tests

  • Adjusted integration tests to align with the new evidence keeper initialization parameters and context setup changes.

• Documentation

  • Updated CHANGELOG to reflect the introduction of a new environment parameter for the evidence keeper and structural changes in the evidence module.

[coderabbitai]

Walkthrough

Walkthrough

The recent updates focus on enhancing the evidence keeper in the blockchain application by integrating a new environment parameter, which includes a logger. This change streamlines the passing of environment-related dependencies, such as logging and event management, to various components. Additionally, the x/evidence module has been restructured for better modularity, including extracting it to a separate go.mod file and refining how evidence of infractions is handled and logged.

Changes

File(s)	Change Summary
simapp/app.go	Updated NewSimApp to include environment with logger in evidence keeper creation.
.../evidence/keeper/infraction_test.go, .../keeper/keeper_test.go	Updated function calls to include environment parameter; adjusted logger usage.
x/evidence/CHANGELOG.md	Documented changes including passing appmodule.Environment to NewKeeper, module extraction, and internal function updates.
x/evidence/depinject.go, .../keeper/keeper.go	Replaced StoreService with Environment in structures and updated usage accordingly.
.../evidence/keeper/abci.go, .../keeper/infraction.go	Removed direct SDK context usage; adjusted logger and header service usage based on environment.

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

• X
• Mastodon
• Reddit
• LinkedIn

---

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-tests for this file.
• 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 tests 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 from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[likhita-809]

infraction test is failing in integration tests. Can you please check that ?
Otherwise lgtm

[coderabbitai]

Review Status

Actionable comments generated: 0

Configuration used: .coderabbit.yml

Commits

Files that changed from the base of the PR and between c883624 and c34cdb5.

Files selected for processing (8)

• simapp/app.go (1 hunks)
• tests/integration/evidence/keeper/infraction_test.go (3 hunks)
• x/evidence/CHANGELOG.md (1 hunks)
• x/evidence/depinject.go (3 hunks)
• x/evidence/keeper/abci.go (2 hunks)
• x/evidence/keeper/infraction.go (2 hunks)
• x/evidence/keeper/keeper.go (4 hunks)
• x/evidence/keeper/keeper_test.go (4 hunks)

Additional comments: 16

x/evidence/depinject.go (2)

• 29-30: The replacement of StoreService with Environment in the ModuleInputs struct is a significant architectural change that aligns with modern development practices for better configuration management. Ensure that the Environment provides all necessary functionalities previously offered by StoreService.
• 45-45: The update in the ProvideModule function to utilize the new Environment parameter is consistent with the changes made in the ModuleInputs struct. This modification enhances the modularity of the evidence module by decoupling it from direct service dependencies.

x/evidence/keeper/abci.go (1)

• 34-34: Removing the sdkCtx variable and directly accessing the logger in the BeginBlocker function simplifies the code. Ensure that the logger is correctly initialized and accessible in this context.

x/evidence/CHANGELOG.md (2)

• 30-31: The changelog entry documenting the migration to appmodule.Environment is clear and accurately reflects the significant architectural change. Ensure all significant changes are documented and review the changelog for completeness and accuracy.
• 36-36: The documentation of the x/evidence module extraction to a separate go.mod file and modifications to BeginBlocker and HandleEquivocation are well-documented in the changelog. This provides clear information on the module's evolution and improvements in modularity.

x/evidence/keeper/keeper.go (2)

• 26-26: Replacing the storeService field with environment in the Keeper struct is a significant architectural change that enhances modularity. Ensure thorough testing to verify that environment provides all necessary functionalities and that there are no regressions in event management.
• 109-114: The update in the SubmitEvidence function to use environment for event management aligns with the architectural changes made in the Keeper struct. Ensure that the event management functionality is thoroughly tested to confirm its correctness and efficiency.

x/evidence/keeper/infraction.go (2)

• 28-28: Adjusting how the logger is obtained in the handleEquivocationEvidence function enhances modularity. Ensure that the logger is correctly initialized and accessible in this context.
• 66-71: Using the header service for header information instead of direct context access is a significant improvement that aligns with the PR's objectives. Verify the header service's accuracy and ensure that it provides timely and accurate header information.

x/evidence/keeper/keeper_test.go (2)

• 14-14: The addition of the "cosmossdk.io/log" import aligns with the architectural changes made in the keeper and is necessary for the updated logging functionality.
• 86-92: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [89-103]

Replacing storeService with env in the SetupTest() function and adjusting the Logger() function call are consistent with the changes made in the keeper. Ensure the test suite is reviewed for completeness and adequately covers the new changes.

tests/integration/evidence/keeper/infraction_test.go (3)

• 134-134: The addition of a new parameter for the environment in the NewKeeper function call aligns with the PR's objective to migrate the evidence module to use environment variables. However, it's crucial to ensure that this new environment parameter is correctly initialized and passed throughout the test setup to maintain consistency and avoid potential null pointer exceptions.
• 274-274: The modification in the TestHandleDoubleSign_TooOld function to set the block height and header information differently is a critical change. It's essential to verify that this new context setup accurately reflects the intended test conditions, especially considering the test's focus on handling double sign infractions that are too old. This change should be carefully reviewed to ensure it does not inadvertently affect the test's validity or the accuracy of its assertions.
• 309-309: The adjustment in the TestHandleDoubleSign_TooOld function to increment the block height and adjust the time based on the consensus parameters is a significant change. This modification is crucial for testing the evidence handling logic for outdated infractions. It's important to ensure that the new block height and time are set correctly to test the intended edge cases without introducing inconsistencies in the test logic.

simapp/app.go (2)

• 407-407: The change to the evidenceKeeper initialization in the NewSimApp function to include a new environment parameter with a logger is aligned with the PR's objective of migrating the evidence module to use environment variables for better configuration management and security. This approach enhances modularity and maintainability by decoupling the evidence keeper's dependencies from direct service access, instead relying on an environment abstraction. It's crucial to ensure that all references and usages of the evidence keeper throughout the application are updated to accommodate this change.
• 404-410: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-407]

Overall, the modifications in simapp/app.go appear to be well-integrated and consistent with the stated objectives of the PR. The introduction of an environment parameter for the evidence keeper aligns with modern development practices and enhances the application's configuration management and security posture. It's recommended to thoroughly test these changes, especially in integration tests, to ensure that the new environment-based configuration does not introduce any regressions or unexpected behavior in the evidence module's functionality.