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

feat(server/v2): refactor the server/v2 events #21785

Merged
merged 11 commits into from
Sep 18, 2024

Conversation

cool-develope
Copy link
Contributor

@cool-develope cool-develope commented Sep 17, 2024

Description

Closes: #21527


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
    • Enhanced event attribute handling across various components, improving structure and clarity.
    • Updated dependencies to the latest versions, potentially introducing new features and improvements in functionality.
  • Bug Fixes
    • Improved error handling in attribute retrieval processes to prevent runtime errors.
  • Documentation
    • Updated dependency management to reflect local development and testing enhancements.
  • Refactor
    • Standardized event attribute management by introducing structured representations for block stages.

Copy link
Contributor

coderabbitai bot commented Sep 17, 2024

Walkthrough

Walkthrough

The changes in this pull request involve updates to multiple Go modules, focusing primarily on modifying the go.mod files for dependency management, including version upgrades and the addition of replacement directives. Additionally, there are enhancements to error handling and attribute processing in various functions across several files, improving the robustness and flexibility of the codebase.

Changes

File(s) Change Summary
collections/go.mod, runtime/v2/go.mod, server/v2/cometbft/go.mod, server/v2/go.mod, server/v2/stf/go.mod, simapp/v2/go.mod - Added replacement directives for cosmossdk.io/schema pointing to local paths.
- Updated cosmossdk.io/core version from v1.0.0-alpha.2 to v1.0.0-alpha.3 across multiple go.mod files.
- Introduced new indirect dependencies.
server/v2/cometbft/utils.go - Enhanced error handling in intoABCIEvents and intoABCISimulationResponse functions by checking for errors when retrieving event attributes.
server/v2/stf/core_event_service.go - Modified TypedEventToEvent function to wrap attrs in a closure for deferred execution.
server/v2/stf/stf.go - Updated event attribute handling in preBlock, beginBlock, and endBlock functions to use BlockStage values instead of hardcoded strings.
server/v2/streaming/utils.go - Changed IntoStreamingEvents to retrieve event attributes using event.Attributes() with error handling.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server
    participant Schema
    participant Event

    Client->>Server: Request for event attributes
    Server->>Schema: Retrieve attributes
    Schema-->>Server: Return attributes
    Server->>Event: Process attributes
    Event-->>Server: Return processed event
    Server-->>Client: Send response
Loading

Assessment against linked issues

Objective Addressed Explanation
Setup locations for appdata.Listener callbacks in stf (#[21527])
Integration of appdata.Listener directly into stf (#[21527]) Not clear if the integration has been completed.

Possibly related issues

Possibly related PRs

Suggested labels

C:Store

Suggested reviewers

  • kocubinski
  • lucaslopezf
  • facundomedica
  • sontrinh16
  • testinginprod
  • auricom
  • julienrbrt

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 generate interesting stats about this repository and render them as a table.
    -- @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.

@cool-develope
Copy link
Contributor Author

@julienrbrt we need to create the appropriate version of core and schema module to avoid the replace statement

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Outside diff range and nitpick comments (1)
simapp/v2/go.mod (1)

249-249: Approve the addition of the replacement directive for the cosmossdk.io/schema module, with a reminder.

The replacement directive for the cosmossdk.io/schema module, pointing to a local path ../../schema, indicates that the module is now utilizing a local version of the schema package. This can be beneficial for development or testing purposes, as it allows for quick iterations and debugging.

However, please ensure that:

  1. The local version is kept in sync with the main development branch to avoid introducing any breaking changes.
  2. The reason for using a local version is documented, along with instructions on how to set it up, to ensure a smooth development experience for other team members.
Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ffc1609 and dc2ab6d.

Files ignored due to path filters (6)
  • collections/go.sum is excluded by !**/*.sum
  • runtime/v2/go.sum is excluded by !**/*.sum
  • server/v2/cometbft/go.sum is excluded by !**/*.sum
  • server/v2/go.sum is excluded by !**/*.sum
  • server/v2/stf/go.sum is excluded by !**/*.sum
  • simapp/v2/go.sum is excluded by !**/*.sum
Files selected for processing (12)
  • collections/collections.go (1 hunks)
  • collections/go.mod (1 hunks)
  • collections/indexing.go (2 hunks)
  • runtime/v2/go.mod (3 hunks)
  • server/v2/cometbft/go.mod (2 hunks)
  • server/v2/cometbft/utils.go (2 hunks)
  • server/v2/go.mod (3 hunks)
  • server/v2/stf/core_event_service.go (1 hunks)
  • server/v2/stf/go.mod (2 hunks)
  • server/v2/stf/stf.go (4 hunks)
  • server/v2/streaming/utils.go (1 hunks)
  • simapp/v2/go.mod (2 hunks)
Additional context used
Path-based instructions (6)
server/v2/streaming/utils.go (1)

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

server/v2/stf/core_event_service.go (1)

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

collections/indexing.go (1)

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

collections/collections.go (1)

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

server/v2/cometbft/utils.go (1)

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

server/v2/stf/stf.go (1)

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

GitHub Check: tests (03)
collections/indexing.go

[failure] 70-70:
undefined: schema.StateObjectUpdate


[failure] 90-90:
undefined: schema.StateObjectUpdate


[failure] 97-97:
undefined: schema.StateObjectUpdate


[failure] 103-103:
undefined: schema.StateObjectUpdate


[failure] 110-110:
undefined: schema.StateObjectUpdate


[failure] 115-115:
undefined: schema.StateObjectUpdate

collections/collections.go

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: tests (02)
collections/indexing.go

[failure] 70-70:
undefined: schema.StateObjectUpdate


[failure] 90-90:
undefined: schema.StateObjectUpdate


[failure] 97-97:
undefined: schema.StateObjectUpdate


[failure] 103-103:
undefined: schema.StateObjectUpdate


[failure] 110-110:
undefined: schema.StateObjectUpdate


[failure] 115-115:
undefined: schema.StateObjectUpdate

collections/collections.go

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: tests (01)
collections/indexing.go

[failure] 70-70:
undefined: schema.StateObjectUpdate


[failure] 90-90:
undefined: schema.StateObjectUpdate


[failure] 97-97:
undefined: schema.StateObjectUpdate


[failure] 103-103:
undefined: schema.StateObjectUpdate


[failure] 110-110:
undefined: schema.StateObjectUpdate


[failure] 115-115:
undefined: schema.StateObjectUpdate

collections/collections.go

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: tests (00)
collections/indexing.go

[failure] 70-70:
undefined: schema.StateObjectUpdate


[failure] 90-90:
undefined: schema.StateObjectUpdate


[failure] 97-97:
undefined: schema.StateObjectUpdate


[failure] 103-103:
undefined: schema.StateObjectUpdate


[failure] 110-110:
undefined: schema.StateObjectUpdate


[failure] 115-115:
undefined: schema.StateObjectUpdate

collections/collections.go

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: dependency-review
collections/indexing.go

[failure] 70-70:
undefined: schema.StateObjectUpdate


[failure] 90-90:
undefined: schema.StateObjectUpdate


[failure] 97-97:
undefined: schema.StateObjectUpdate


[failure] 103-103:
undefined: schema.StateObjectUpdate


[failure] 110-110:
undefined: schema.StateObjectUpdate


[failure] 115-115:
undefined: schema.StateObjectUpdate

collections/collections.go

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: golangci-lint
collections/indexing.go

[failure] 70-70:
undefined: schema.StateObjectUpdate


[failure] 90-90:
undefined: schema.StateObjectUpdate


[failure] 97-97:
undefined: schema.StateObjectUpdate


[failure] 103-103:
undefined: schema.StateObjectUpdate


[failure] 110-110:
undefined: schema.StateObjectUpdate


[failure] 115-115:
undefined: schema.StateObjectUpdate) (typecheck)

collections/collections.go

[failure] 107-107:
undefined: schema.StateObjectType

Additional comments not posted (20)
server/v2/stf/go.mod (3)

6-6: Verify the compatibility of the updated cosmossdk.io/core version.

The version of cosmossdk.io/core has been updated to a more specific pseudo-version v1.0.0-alpha.2.0.20240917105208-c9f0e2e4d892. Please ensure that this version is compatible with the rest of the codebase and does not introduce any breaking changes.


7-7: Verify the usage of the new cosmossdk.io/schema dependency.

A new dependency on cosmossdk.io/schema at version v0.2.0 has been introduced. Please ensure that this dependency is being used correctly and does not introduce any issues or conflicts with existing functionality.


17-17: Verify the compatibility of the local schema package.

A replace directive has been added for cosmossdk.io/schema, pointing to a local path ../../../schema. Please ensure that the local version of the schema package is up-to-date and compatible with the rest of the codebase to avoid any inconsistencies or compatibility issues.

collections/go.mod (1)

21-22: LGTM! The new replacement enhances local development and testing.

The addition of the replacement directive for cosmossdk.io/schema allows using a local version of the schema package, enabling more flexible development and testing workflows. This change is consistent with the existing replacement for cosmossdk.io/core/testing, suggesting a pattern of leveraging local packages for development and testing purposes.

Keep in mind that this modification may impact the build process and requires the presence of the schema package in the specified local directory (../schema).

server/v2/streaming/utils.go (1)

Line range hint 1-29: The code conforms to the Uber Golang style guide.

The code follows the basic Golang style guidelines and there are no deviations from the Uber Golang style guide. The function and variable names follow the camelCase convention and are descriptive.

runtime/v2/go.mod (2)

17-17: Ensure the stability of the chosen commit for cosmossdk.io/core.

Using a pseudo-version that points to a specific commit helps ensure reproducible builds and avoids potential breaking changes. However, please ensure that the chosen commit c9f0e2e4d892 is stable and has been thoroughly tested.


34-34: Keep the local version of the schema package in sync with the remote version.

The addition of the cosmossdk.io/schema dependency and the usage of a local version of the schema package allows for faster development. However, please ensure to regularly sync the local version with the remote version to avoid divergence and potential compatibility issues.

Also applies to: 90-90

collections/indexing.go (2)

70-70: Fix the compilation error due to undefined type schema.StateObjectUpdate.

The static analysis hints indicate that the type schema.StateObjectUpdate is undefined, causing a compilation error. Please ensure that the schema package is updated to define this type.

Also, verify that the downstream code is updated to handle the change in return type from []schema.ObjectUpdate to []schema.StateObjectUpdate.

Tools
GitHub Check: tests (03)

[failure] 70-70:
undefined: schema.StateObjectUpdate

GitHub Check: tests (02)

[failure] 70-70:
undefined: schema.StateObjectUpdate

GitHub Check: tests (01)

[failure] 70-70:
undefined: schema.StateObjectUpdate

GitHub Check: tests (00)

[failure] 70-70:
undefined: schema.StateObjectUpdate

GitHub Check: dependency-review

[failure] 70-70:
undefined: schema.StateObjectUpdate

GitHub Check: golangci-lint

[failure] 70-70:
undefined: schema.StateObjectUpdate


90-90: Fix the compilation error due to undefined type schema.StateObjectUpdate.

The static analysis hints indicate that the type schema.StateObjectUpdate is undefined, causing a compilation error. Please ensure that the schema package is updated to define this type.

Also, verify that the downstream code is updated to handle the change in return type from []schema.ObjectUpdate to []schema.StateObjectUpdate.

Tools
GitHub Check: tests (03)

[failure] 90-90:
undefined: schema.StateObjectUpdate

GitHub Check: tests (02)

[failure] 90-90:
undefined: schema.StateObjectUpdate

GitHub Check: tests (01)

[failure] 90-90:
undefined: schema.StateObjectUpdate

GitHub Check: tests (00)

[failure] 90-90:
undefined: schema.StateObjectUpdate

GitHub Check: dependency-review

[failure] 90-90:
undefined: schema.StateObjectUpdate

GitHub Check: golangci-lint

[failure] 90-90:
undefined: schema.StateObjectUpdate

server/v2/go.mod (3)

17-17: LGTM!

The update to a more specific pseudo-version for cosmossdk.io/core dependency is a good practice. It ensures that the project is pinned to a specific commit, providing better reproducibility and control over the exact version being used.


47-47: Looks good!

The addition of cosmossdk.io/schema as an indirect dependency is a necessary update to ensure all required dependencies are properly tracked. Indirect dependencies are an important part of the Go module system and help maintain a complete dependency graph.


116-116: Consider the implications of using a local replace directive.

The replace directive for cosmossdk.io/schema points to a local path ../../schema, indicating the use of a local version of this module. While this is a common practice during development, it can make the project harder to build for other developers or in CI/CD pipelines.

Could you provide more context on the reason for using a local replacement? If the changes made to the local version are necessary, consider publishing the modified module to a remote repository. If the changes are temporary or not required, it might be better to revert to using the upstream version to improve the project's portability and ease of use.

server/v2/cometbft/go.mod (2)

8-8: Ensure compatibility of the local schema package.

The new replace directive points the cosmossdk.io/schema module to a local version in the ../../../schema directory. While this may be necessary for development or testing, it's crucial to verify that the local version is compatible with the rest of the codebase and doesn't introduce any breaking changes.

Consider running the following script to check for any inconsistencies or issues related to the schema package usage:

This script searches for all .go files that import the cosmossdk.io/schema package, and then performs static analysis or runs specific tests to check for any usage of deprecated or changed APIs from the local schema version. Adjust the checks based on the changes made in the local version.

If any issues are found, the script will print a summary and exit with a non-zero status, indicating that further investigation and fixes are needed before proceeding with the local schema version.

Verification successful

Local schema package appears compatible, but exercise caution

The compatibility check for the local schema package has completed successfully, and no issues were detected across the 63 files that import cosmossdk.io/schema. This suggests that the local version is likely compatible with the existing codebase.

However, please note that this check was limited to detecting usage of deprecated APIs. To ensure full compatibility:

  • Run the full test suite to catch any runtime issues.
  • Perform a manual review of any significant changes in the local schema package.
  • Consider gradual rollout or additional testing in a staging environment.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for `cosmossdk.io/schema` usage and verify compatibility

# 1. Find all `.go` files that import the `cosmossdk.io/schema` package
importers=$(fd -e go -x rg -l 'cosmossdk.io/schema')

# 2. For each file, check if the usage is compatible with the local version
for file in $importers; do
  echo "Checking $file"
  # Perform static analysis or run tests specific to the schema usage
  # For example, check for deprecated or changed APIs
  rg -q 'schema\.Deprecated' "$file" && echo "  WARNING: Usage of deprecated schema API"
  # Add more checks as needed
done

# 3. If any issues are found, print a summary and exit with a non-zero status
if [ -n "$(echo $issues)" ]; then
  echo "Compatibility issues found:"
  echo "$issues"
  exit 1
else
  echo "No compatibility issues found"
fi

Length of output: 13600


23-23: Review the changes in the specific core module commit.

The cosmossdk.io/core module version has been updated to a specific commit hash c9f0e2e4d892 from 2024-09-17. This change could include bug fixes, new features, or other improvements that might affect the behavior or compatibility of the codebase.

To ensure a smooth integration, follow these steps:

  1. Review the changes included in the specific commit by visiting the commit URL or running:

  2. Assess the impact of these changes on the dependent code in your project. Look for any breaking changes, deprecated APIs, or new features that might require updates in your codebase.

  3. If necessary, update your code to accommodate the changes introduced by the new core module version. This may involve updating function calls, adapting to new APIs, or handling any deprecated functionality.

  4. Run your project's test suite and perform thorough testing to verify that everything works as expected with the updated core module.

By carefully reviewing and adapting to the changes in the specific core module commit, you can ensure a smooth integration and maintain the stability of your codebase.

server/v2/cometbft/utils.go (2)

130-133: LGTM!

The change improves error handling by explicitly checking for an error when retrieving event attributes and panicking if an error occurs. This aligns with the Uber Go Style Guide's recommendations for error handling.


155-158: LGTM!

Similar to the previous comment, this change improves error handling by explicitly checking for an error when retrieving event attributes and panicking if an error occurs. This aligns with the Uber Go Style Guide's recommendations for error handling.

simapp/v2/go.mod (1)

8-8: Approve the version update for the cosmossdk.io/core dependency.

The version update to v1.0.0-alpha.2.0.20240917105208-c9f0e2e4d892 reflects a more precise versioning scheme, which can help in ensuring compatibility and stability within the module's dependencies. This change aligns with the best practices of dependency management.

server/v2/stf/stf.go (3)

18-18: LGTM!

The import statement is syntactically correct and follows the recommended naming convention for Cosmos SDK modules. The usage of the imported appdata package is evident in the changes made to the preBlock, beginBlock, and endBlock functions.


342-343: Excellent refactor!

The usage of appdata.PreBlockStage, appdata.BeginBlockStage, and appdata.EndBlockStage constants to assign the BlockStage field of each event provides a more structured and standardized approach compared to the previous hardcoded values. This change simplifies the event attribute management and enhances the clarity and maintainability of the code. It aligns well with the overall objective of refactoring the events in the server/v2 module.

Also applies to: 358-359, 381-382


Line range hint 1-583: Code conforms to the Uber Go Style Guide.

Based on the review, the code in this file follows the conventions and recommendations outlined in the Uber Go Style Guide. The formatting, naming, error handling, and commenting practices align with the style guide's guidelines. No obvious deviations were found in the provided code.

Comment on lines 12 to 15
attrs, err := event.Attributes()
if err != nil {
panic(err)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid panicking on errors. Return an error instead to allow graceful error handling.

Panicking on an error is not an ideal error handling strategy as it can lead to unexpected program crashes. Instead of panicking, the function should return an error to allow the caller to handle the error gracefully.

Consider applying this diff to improve the error handling:

-		attrs, err := event.Attributes()
-		if err != nil {
-			panic(err)
-		}
+		attrs, err := event.Attributes()
+		if err != nil {
+			return nil, err
+		}

Also, update the function signature to return an error:

-func IntoStreamingEvents(events []event.Event) []*Event {
+func IntoStreamingEvents(events []event.Event) ([]*Event, error) {

Committable suggestion was skipped due to low confidence.

@@ -81,6 +81,6 @@ func TypedEventToEvent(tev transaction.Msg) (event.Event, error) {

return event.Event{
Type: evtType,
Attributes: attrs,
Attributes: func() ([]event.Attribute, error) { return attrs, nil },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider enhancing the attributes closure for more robust error handling and validation.

The introduction of a closure to encapsulate the attributes allows for deferred execution and the addition of logic without altering the function signature. This is a good approach to make the handling of attributes more flexible.

However, the current implementation doesn't fully leverage the potential benefits of the closure. Consider adding error handling or validation logic within the closure to ensure the attributes are valid and to handle any potential errors gracefully.

For example:

Attributes: func() ([]event.Attribute, error) {
    if err := validateAttributes(attrs); err != nil {
        return nil, err
    }
    return attrs, nil
},

This way, the closure can serve as a centralized point to handle attribute-related errors and validations, making the code more robust and maintainable.

Comment on lines +97 to 99
return []schema.StateObjectUpdate{
{TypeName: c.coll.GetName()},
}, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the compilation error due to undefined type schema.StateObjectUpdate.

The static analysis hints indicate that the type schema.StateObjectUpdate is undefined, causing a compilation error. Please ensure that the schema package is updated to define this type.

Tools
GitHub Check: tests (03)

[failure] 97-97:
undefined: schema.StateObjectUpdate

GitHub Check: tests (02)

[failure] 97-97:
undefined: schema.StateObjectUpdate

GitHub Check: tests (01)

[failure] 97-97:
undefined: schema.StateObjectUpdate

GitHub Check: tests (00)

[failure] 97-97:
undefined: schema.StateObjectUpdate

GitHub Check: dependency-review

[failure] 97-97:
undefined: schema.StateObjectUpdate

GitHub Check: golangci-lint

[failure] 97-97:
undefined: schema.StateObjectUpdate

Comment on lines +103 to 105
return []schema.StateObjectUpdate{
{TypeName: c.coll.GetName(), Key: k, Delete: true},
}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the compilation error due to undefined type schema.StateObjectUpdate.

The static analysis hints indicate that the type schema.StateObjectUpdate is undefined, causing a compilation error. Please ensure that the schema package is updated to define this type.

Tools
GitHub Check: tests (03)

[failure] 103-103:
undefined: schema.StateObjectUpdate

GitHub Check: tests (02)

[failure] 103-103:
undefined: schema.StateObjectUpdate

GitHub Check: tests (01)

[failure] 103-103:
undefined: schema.StateObjectUpdate

GitHub Check: tests (00)

[failure] 103-103:
undefined: schema.StateObjectUpdate

GitHub Check: dependency-review

[failure] 103-103:
undefined: schema.StateObjectUpdate

GitHub Check: golangci-lint

[failure] 103-103:
undefined: schema.StateObjectUpdate

Comment on lines +110 to 112
return []schema.StateObjectUpdate{
{TypeName: c.coll.GetName(), Key: k},
}, err
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the compilation error due to undefined type schema.StateObjectUpdate.

The static analysis hints indicate that the type schema.StateObjectUpdate is undefined, causing a compilation error. Please ensure that the schema package is updated to define this type.

Tools
GitHub Check: tests (03)

[failure] 110-110:
undefined: schema.StateObjectUpdate

GitHub Check: tests (02)

[failure] 110-110:
undefined: schema.StateObjectUpdate

GitHub Check: tests (01)

[failure] 110-110:
undefined: schema.StateObjectUpdate

GitHub Check: tests (00)

[failure] 110-110:
undefined: schema.StateObjectUpdate

GitHub Check: dependency-review

[failure] 110-110:
undefined: schema.StateObjectUpdate

GitHub Check: golangci-lint

[failure] 110-110:
undefined: schema.StateObjectUpdate

Comment on lines +115 to 117
return []schema.StateObjectUpdate{
{TypeName: c.coll.GetName(), Key: k, Value: v},
}, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the compilation error due to undefined type schema.StateObjectUpdate.

The static analysis hints indicate that the type schema.StateObjectUpdate is undefined, causing a compilation error. Please ensure that the schema package is updated to define this type.

Tools
GitHub Check: tests (03)

[failure] 115-115:
undefined: schema.StateObjectUpdate

GitHub Check: tests (02)

[failure] 115-115:
undefined: schema.StateObjectUpdate

GitHub Check: tests (01)

[failure] 115-115:
undefined: schema.StateObjectUpdate

GitHub Check: tests (00)

[failure] 115-115:
undefined: schema.StateObjectUpdate

GitHub Check: dependency-review

[failure] 115-115:
undefined: schema.StateObjectUpdate

GitHub Check: golangci-lint

[failure] 115-115:
undefined: schema.StateObjectUpdate) (typecheck)

@@ -104,7 +104,7 @@
// decoders and encoders to and from schema values and raw kv-store bytes.
type collectionSchemaCodec struct {
coll Collection
objectType schema.ObjectType
objectType schema.StateObjectType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the undefined type schema.StateObjectType.

The static analysis tools indicate that schema.StateObjectType is undefined. This change would cause compilation errors.

Please ensure that schema.StateObjectType is properly defined and imported before using it as the type for the objectType field.

Tools
GitHub Check: tests (03)

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: tests (02)

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: tests (01)

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: tests (00)

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: dependency-review

[failure] 107-107:
undefined: schema.StateObjectType

GitHub Check: golangci-lint

[failure] 107-107:
undefined: schema.StateObjectType

@@ -18,3 +18,5 @@ require (
)

replace cosmossdk.io/core/testing => ../core/testing

replace cosmossdk.io/schema => ../schema
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do this in a separate PR? I'm pretty sure this replace has a global effect on all go.mod's in the SDK. Instead if needed we can tag schema and then update collections.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we need a new tag of schema

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BlockStage is not included in the schema v2.0, after new tag of schema, and also need to tag the core module, maybe alpah3 ??? cc: @julienrbrt

Copy link
Member

@julienrbrt julienrbrt Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure let's have alpha.3.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tagging schema @julienrbrt !

Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@julienrbrt julienrbrt added the backport/v0.52.x PR scheduled for inclusion in the v0.52's next stable release label Sep 18, 2024
@julienrbrt
Copy link
Member

Adding the backport label for the server/v2/CometBFT changes

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (4)
x/slashing/go.mod (1)

Line range hint 186-193: Address the TODO comment and remove replace directives when appropriate.

The replace directives in the go.mod file are used to temporarily substitute certain dependencies with local file paths. This is likely done for development purposes, to use modified or custom versions of those dependencies.

As indicated by the TODO comment, these replace directives should be removed once all the modules are "spun out" (i.e., moved to their own repositories or made independent). Keeping the replace directives long-term can make the module harder to reuse and maintain.

Please ensure to address this TODO comment and remove the replace directives when the modules are ready to be spun out.

client/v2/go.mod (1)

Line range hint 215-222: Address the TODO comment about removing replace statements.

The TODO comment indicates that replace statements should be removed after spinning out all modules. This is a good practice for maintaining clean and efficient dependency management.

Please let me know if you need any assistance in spinning out the modules and removing the replace statements. I'd be happy to help!

x/feegrant/go.mod (1)

Line range hint 208-215: Consider removing the replace directive.

It's generally recommended to avoid using replace directives in production as they can cause issues with dependency management and reproducibility.

Instead, consider creating the appropriate versions of the core and schema modules to avoid the need for local replacements.

x/accounts/go.mod (1)

Line range hint 184-184: Address the TODO comment.

The go.mod file contains a TODO comment indicating that certain replace directives should be removed after spinning out all modules. It's important to create a plan or timeline for addressing this TODO to avoid accumulating technical debt and potential confusion for other developers.

Do you need any assistance in creating a plan to remove the unnecessary replace directives? I'd be happy to help you outline the steps and create GitHub issues to track the progress.

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between dc2ab6d and e59ee1a.

Files ignored due to path filters (31)
  • client/v2/go.sum is excluded by !**/*.sum
  • collections/go.sum is excluded by !**/*.sum
  • go.sum is excluded by !**/*.sum
  • runtime/v2/go.sum is excluded by !**/*.sum
  • server/v2/cometbft/go.sum is excluded by !**/*.sum
  • server/v2/go.sum is excluded by !**/*.sum
  • server/v2/stf/go.sum is excluded by !**/*.sum
  • simapp/go.sum is excluded by !**/*.sum
  • simapp/v2/go.sum is excluded by !**/*.sum
  • tests/go.sum is excluded by !**/*.sum
  • x/accounts/defaults/base/go.sum is excluded by !**/*.sum
  • x/accounts/defaults/lockup/go.sum is excluded by !**/*.sum
  • x/accounts/defaults/multisig/go.sum is excluded by !**/*.sum
  • x/accounts/go.sum is excluded by !**/*.sum
  • x/authz/go.sum is excluded by !**/*.sum
  • x/bank/go.sum is excluded by !**/*.sum
  • x/circuit/go.sum is excluded by !**/*.sum
  • x/consensus/go.sum is excluded by !**/*.sum
  • x/distribution/go.sum is excluded by !**/*.sum
  • x/epochs/go.sum is excluded by !**/*.sum
  • x/evidence/go.sum is excluded by !**/*.sum
  • x/feegrant/go.sum is excluded by !**/*.sum
  • x/gov/go.sum is excluded by !**/*.sum
  • x/group/go.sum is excluded by !**/*.sum
  • x/mint/go.sum is excluded by !**/*.sum
  • x/nft/go.sum is excluded by !**/*.sum
  • x/params/go.sum is excluded by !**/*.sum
  • x/protocolpool/go.sum is excluded by !**/*.sum
  • x/slashing/go.sum is excluded by !**/*.sum
  • x/staking/go.sum is excluded by !**/*.sum
  • x/upgrade/go.sum is excluded by !**/*.sum
Files selected for processing (36)
  • client/v2/go.mod (2 hunks)
  • collections/go.mod (1 hunks)
  • go.mod (1 hunks)
  • runtime/v2/go.mod (2 hunks)
  • server/v2/cometbft/abci.go (1 hunks)
  • server/v2/cometbft/go.mod (2 hunks)
  • server/v2/cometbft/streaming.go (1 hunks)
  • server/v2/cometbft/utils.go (4 hunks)
  • server/v2/go.mod (2 hunks)
  • server/v2/stf/go.mod (1 hunks)
  • server/v2/streaming/utils.go (2 hunks)
  • simapp/go.mod (2 hunks)
  • simapp/v2/go.mod (2 hunks)
  • tests/go.mod (2 hunks)
  • x/accounts/defaults/base/go.mod (2 hunks)
  • x/accounts/defaults/lockup/go.mod (2 hunks)
  • x/accounts/defaults/multisig/go.mod (2 hunks)
  • x/accounts/go.mod (2 hunks)
  • x/authz/go.mod (2 hunks)
  • x/bank/go.mod (2 hunks)
  • x/bank/keeper/keeper_test.go (2 hunks)
  • x/circuit/go.mod (2 hunks)
  • x/consensus/go.mod (2 hunks)
  • x/distribution/go.mod (2 hunks)
  • x/epochs/go.mod (2 hunks)
  • x/evidence/go.mod (2 hunks)
  • x/feegrant/go.mod (2 hunks)
  • x/gov/go.mod (2 hunks)
  • x/group/go.mod (2 hunks)
  • x/mint/go.mod (2 hunks)
  • x/nft/go.mod (2 hunks)
  • x/params/go.mod (2 hunks)
  • x/protocolpool/go.mod (2 hunks)
  • x/slashing/go.mod (2 hunks)
  • x/staking/go.mod (2 hunks)
  • x/upgrade/go.mod (2 hunks)
Files skipped from review as they are similar to previous changes (6)
  • collections/go.mod
  • runtime/v2/go.mod
  • server/v2/cometbft/go.mod
  • server/v2/go.mod
  • server/v2/stf/go.mod
  • simapp/v2/go.mod
Additional context used
Path-based instructions (6)
server/v2/streaming/utils.go (1)

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

server/v2/cometbft/streaming.go (1)

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

server/v2/cometbft/utils.go (1)

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

tests/go.mod (1)

Pattern tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"

server/v2/cometbft/abci.go (1)

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

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

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


Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"

Additional comments not posted (55)
server/v2/streaming/utils.go (1)

Line range hint 5-25: LGTM!

The function now correctly handles the error returned by event.Attributes() and returns it to the caller. The function signature has also been updated to return an error, which is a good practice.

The past review comment suggesting a similar change is no longer applicable as the suggested change has already been implemented.

server/v2/cometbft/streaming.go (3)

24-27: LGTM!

The error handling for event conversion is implemented correctly. Returning early if an error occurs prevents further processing with potentially invalid data, improving the robustness of the function.


32-32: LGTM!

The assignment of the converted streaming events to the Events field is correct.


37-45: LGTM!

The error handling for event conversion is implemented correctly. Returning early if an error occurs prevents further processing with potentially invalid data, improving the robustness of the function. The converted events are correctly passed to the ListenDeliverBlock function of each streaming listener.

x/accounts/defaults/lockup/go.mod (2)

7-7: Verify compatibility with the updated cosmossdk.io/core dependency.

The version of cosmossdk.io/core has been updated from v1.0.0-alpha.2 to v1.0.0-alpha.3. This change may introduce breaking changes or new features that could impact the module's functionality or compatibility with other components.

Ensure that the module has been thoroughly tested with the updated version to confirm that it remains compatible and functions as expected.


25-25: Confirm compatibility with the updated cosmossdk.io/schema dependency.

The version of cosmossdk.io/schema has been updated from v0.2.0 to v0.3.0. This change may introduce updates or modifications in the schema handling that could affect the module's interaction with the schema.

Verify that the module remains compatible with the updated schema handling and that any necessary adjustments have been made to ensure proper functionality.

x/params/go.mod (1)

7-7: Approve version updates, but verify compatibility and impact.

The updates to the cosmossdk.io/core and cosmossdk.io/schema dependencies seem reasonable given the minor version increments. However, it's important to ensure that these updates are compatible with the rest of the codebase and do not introduce any breaking changes.

To verify the compatibility and impact of the version updates, consider the following:

  1. Review the release notes or changelog of the updated dependencies to understand the changes and any potential breaking changes.
  2. Run the unit tests and integration tests of the application to ensure that the updates do not introduce any regressions or failures.
  3. Perform manual testing of the critical functionalities of the application to validate that the behavior remains as expected.
  4. If possible, deploy the application to a staging or test environment to monitor its performance and stability with the updated dependencies.

By thoroughly verifying the compatibility and impact of the version updates, you can minimize the risk of introducing issues or breaking changes in the production environment.

Also applies to: 32-32

x/staking/go.mod (2)

8-8: Verify the impact of updating cosmossdk.io/core to v1.0.0-alpha.3.

Updating to a new alpha version may introduce breaking changes or new features that could affect the functionality of the x/staking module. Ensure that thorough testing has been performed to confirm compatibility with the updated version.


167-167: Verify the impact of updating cosmossdk.io/schema to v0.3.0.

Updating to a new minor version may introduce new features, bug fixes, or minor breaking changes that could affect the schema definitions or serialization logic within the x/staking module. Ensure that proper testing has been performed to confirm compatibility with the updated version.

x/consensus/go.mod (2)

8-8: LGTM!

The version update of cosmossdk.io/core from v1.0.0-alpha.2 to v1.0.0-alpha.3 is approved.


30-30: LGTM!

The version update of cosmossdk.io/schema from v0.2.0 to v0.3.0 is approved.

x/bank/go.mod (2)

8-8: LGTM!

The update to cosmossdk.io/core from v1.0.0-alpha.2 to v1.0.0-alpha.3 is approved. This progression in the alpha release likely introduces new features, bug fixes, or improvements to the core module.


167-167: Verify compatibility with the updated cosmossdk.io/schema version.

The update to cosmossdk.io/schema from v0.2.0 to v0.3.0 is approved. This update likely introduces enhancements or modifications to the schema handling capabilities.

To ensure compatibility with the updated schema version, run the following script:

x/evidence/go.mod (2)

8-8: Approve version update for cosmossdk.io/core.

The minor version update to v1.0.0-alpha.3 should introduce enhancements or bug fixes while maintaining backward compatibility. However, please verify that this updated version is compatible with other dependencies and the codebase to avoid any potential issues.


31-31: Approve version update for cosmossdk.io/schema.

The minor version update to v0.3.0 should introduce enhancements or bug fixes while maintaining backward compatibility. However, please verify that this updated version is compatible with other dependencies and the codebase to avoid any potential issues.

x/epochs/go.mod (2)

8-8: Verify compatibility with the updated cosmossdk.io/core version.

The cosmossdk.io/core module has been updated from v1.0.0-alpha.2 to v1.0.0-alpha.3. Please review the changelog for any breaking changes or new features introduced in this version update. Ensure that the codebase is updated accordingly to maintain compatibility.


23-23: Verify compatibility with the updated cosmossdk.io/schema version.

The cosmossdk.io/schema module has been updated from v0.2.0 to v0.3.0. Please review the changelog for any breaking changes or new features introduced in this version update. Ensure that the codebase is updated accordingly to maintain compatibility.

x/protocolpool/go.mod (2)

8-8: LGTM!

The version update for cosmossdk.io/core is a standard part of the development process and does not raise any immediate concerns.


30-30: LGTM!

The version update for cosmossdk.io/schema is a standard part of the development process and does not raise any immediate concerns.

x/authz/go.mod (2)

7-7: Verify the impact of updating cosmossdk.io/core to v1.0.0-alpha.3.

Ensure that the module's functionality is not adversely affected by the changes in the core SDK. Perform thorough testing to confirm compatibility with the updated version.


33-33: Verify the impact of updating cosmossdk.io/schema to v0.3.0.

Ensure that the module's schema handling is not adversely affected by the changes in the cosmossdk.io/schema package. Perform thorough testing to confirm compatibility with the updated version, especially if the update introduces new features or breaking changes.

x/nft/go.mod (2)

7-7: Verify compatibility with the updated cosmossdk.io/core version.

The version of cosmossdk.io/core has been incremented from v1.0.0-alpha.2 to v1.0.0-alpha.3. Please ensure that the x/nft module is compatible with this updated version and thoroughly test the module to identify any potential issues or breaking changes introduced by the version update.


29-29: Verify compatibility with the updated cosmossdk.io/schema version.

The version of cosmossdk.io/schema has been updated from v0.2.0 to v0.3.0. Please ensure that the x/nft module is compatible with this updated version and thoroughly test the module to identify any potential issues or breaking changes introduced by the version update.

x/slashing/go.mod (1)

8-8: Review the impact of updating cosmossdk.io/core and cosmossdk.io/schema versions.

The go.mod file has been updated to use newer versions of cosmossdk.io/core (v1.0.0-alpha.3) and cosmossdk.io/schema (v0.3.0). It's important to review the release notes or changelog of these dependencies to understand any breaking changes, new features, or bug fixes introduced by the updated versions.

Ensure that the x/slashing module code is compatible with the changes in these dependencies and make necessary adaptations if required.

Also applies to: 33-33

x/circuit/go.mod (2)

8-8: Verify compatibility with the updated cosmossdk.io/core dependency.

The version update to v1.0.0-alpha.3 is approved. However, please ensure that the x/circuit module is fully compatible with any changes introduced in this version of the core module. Thorough testing is recommended to identify and address any potential issues arising from this update.


27-27: Verify compatibility of schema definitions with the updated cosmossdk.io/schema dependency.

The version update to v0.3.0 is approved. However, please review the module's schema definitions to ensure they are fully compatible with any changes introduced in this version of the schema module. Make any necessary adjustments to maintain compatibility and prevent potential issues.

x/distribution/go.mod (2)

8-8: Verify the impact of updating cosmossdk.io/core to v1.0.0-alpha.3.

Incrementing the version of cosmossdk.io/core from v1.0.0-alpha.2 to v1.0.0-alpha.3 may introduce updates, bug fixes, or new features. As this is an alpha version, there is a risk of breaking changes.

Ensure thorough testing is performed to validate compatibility with this new version and check for any breaking changes that may impact the functionality of the module.


33-33: Verify the impact of updating cosmossdk.io/schema to v0.3.0.

Updating the version of cosmossdk.io/schema from v0.2.0 to v0.3.0 suggests enhancements or modifications in the schema handling. While a minor version update should maintain backwards compatibility, it's important to validate this assumption.

Ensure thorough testing is performed to confirm compatibility with this new version and check for any unexpected behavior or breaking changes that may impact the functionality of the module.

x/mint/go.mod (2)

8-8: Verify compatibility with the updated cosmossdk.io/core version.

The cosmossdk.io/core dependency has been updated to version v1.0.0-alpha.3. Please ensure that the x/mint module is compatible with this version and thoroughly test the module to identify any potential issues arising from the update.


32-32: Review usage of schema definitions from the updated cosmossdk.io/schema version.

The cosmossdk.io/schema dependency has been updated to version v0.3.0. Please review any usages of schema definitions from this module in the x/mint module and ensure they are still valid and compatible with the updated version.

x/gov/go.mod (2)

8-8: Approve version update for cosmossdk.io/core.

Updating to v1.0.0-alpha.3 is reasonable, as it likely includes enhancements or bug fixes. However, since this is still an alpha version, ensure thorough testing is performed to validate compatibility and stability with the rest of the codebase.


40-40: Approve version update for cosmossdk.io/schema.

The update to v0.3.0 is a reasonable choice, as it likely introduces new backward-compatible functionality. However, it's still prudent to perform testing to validate that there are no unexpected breaking changes that could affect the codebase.

client/v2/go.mod (1)

7-7: Verify the impact of the dependency updates on the application.

The versions of cosmossdk.io/core and cosmossdk.io/schema have been updated, which may introduce changes in the core SDK functionality and schema handling.

Please ensure that the application has been thoroughly tested with these new versions to confirm that there are no breaking changes or unexpected behaviors.

Also applies to: 29-29

x/feegrant/go.mod (2)

8-8: LGTM!

The version update for cosmossdk.io/core from v1.0.0-alpha.2 to v1.0.0-alpha.3 looks good.


40-40: LGTM!

The version update for cosmossdk.io/schema from v0.2.0 to v0.3.0 looks good.

x/accounts/defaults/multisig/go.mod (1)

7-7: Verify compatibility with updated dependency versions.

The updates to the cosmossdk.io/core and cosmossdk.io/schema dependency versions look good. However, it's important to ensure that the x/accounts/defaults/multisig module remains compatible with these updated versions.

To verify compatibility, consider running the following checks:

If any issues are identified during these checks, please address them to maintain compatibility with the updated dependencies.

Also applies to: 26-26

x/accounts/defaults/base/go.mod (2)

8-8: Review the cosmossdk.io/core version update carefully.

The cosmossdk.io/core dependency has been updated from v1.0.0-alpha.2 to v1.0.0-alpha.3. While this update might bring enhancements or bug fixes, please be cautious as alpha versions may contain breaking changes or instability risks.

Thoroughly test the codebase to ensure compatibility with the updated core module and verify overall stability before proceeding.


26-26: Review the cosmossdk.io/schema version update.

The cosmossdk.io/schema dependency has been updated from v0.2.0 to v0.3.0. This minor version update suggests there might be new backwards-compatible functionality in the schema module.

Please review the changelog to understand the changes and new features introduced in v0.3.0. Ensure thorough testing to verify compatibility with the updated schema module.

go.mod (2)

8-8: Thoroughly test the update to cosmossdk.io/core v1.0.0-alpha.3.

Updating to a new alpha version of the core module may introduce breaking changes that could impact the project's functionality. Ensure comprehensive testing is performed to verify compatibility and stability. Adapt the project's code as necessary to accommodate any breaking changes introduced by the new core version.


14-14: Review the changelog for cosmossdk.io/schema v0.3.0 and ensure compatibility.

Updating to a new minor version of the schema module may introduce changes that affect how data is structured or processed. Review the changelog to understand the new features or enhancements introduced in v0.3.0. Ensure the project's code is compatible with any changes introduced by the new schema version.

x/accounts/go.mod (1)

8-8: Verify compatibility with updated dependencies.

The versions of cosmossdk.io/core and cosmossdk.io/schema have been updated to v1.0.0-alpha.3 and v0.3.0, respectively. While updating dependencies is generally beneficial, it's important to thoroughly test the module to ensure that these updates do not introduce any breaking changes or unexpected behavior.

Please confirm that comprehensive testing has been performed to validate the compatibility and stability of the module with the updated dependencies.

Also applies to: 35-35

x/group/go.mod (2)

41-41: Approve version update for cosmossdk.io/schema.

Upgrading to v0.3.0 is beneficial to leverage potential enhancements and bug fixes in the schema handling package.

Considering the sensitivity of schema changes, ensure comprehensive testing is conducted to validate compatibility and stability. Pay close attention to any schema definitions, migrations, or serialization/deserialization logic. Consider running the following script to identify and analyze all usages of the cosmossdk.io/schema package:


7-7: Approve version update for cosmossdk.io/core.

Upgrading to v1.0.0-alpha.3 is a good move to incorporate the latest enhancements and bug fixes in the core SDK.

Ensure thorough testing is performed to validate compatibility and stability with this alpha version update. Consider running the following script to identify and analyze all usages of the cosmossdk.io/core package:

x/upgrade/go.mod (1)

7-7: Verify the compatibility and impact of the updated versions.

The versions of cosmossdk.io/core and cosmossdk.io/schema have been updated to v1.0.0-alpha.3 and v0.3.0, respectively. These updates could introduce enhancements, bug fixes, or changes in functionality.

To ensure a smooth integration, please verify that:

  • The updated versions are compatible with the rest of the codebase.
  • There are no breaking changes introduced by these updates that could negatively impact the application.

Consider running the following script to analyze the usage of these packages across the codebase:

Review the results to identify any potential incompatibilities or areas that may require updates to align with the new versions.

Also applies to: 45-45

server/v2/cometbft/utils.go (3)

76-85: Improved error handling in finalizeBlockResponse.

The changes enhance the error handling and data retrieval process within the finalizeBlockResponse function. By capturing the results of intoABCIEvents and intoABCITxResults in separate variables and returning immediately if either function encounters an error, the code ensures that any issues encountered while processing events or transaction results are properly communicated to the caller. This improvement increases the robustness and reliability of the code by handling errors gracefully and maintaining data integrity throughout the processing flow.


Line range hint 110-136: Enhanced error handling in intoABCITxResults.

The modifications to the intoABCITxResults function improve error propagation by returning an error alongside the results. The retrieval of event attributes now includes error handling, which prevents the code from accessing potentially invalid data. If an error is encountered while processing events, the function returns immediately with the error, ensuring that any issues encountered while processing transaction results are properly communicated to the caller. These enhancements contribute to the overall robustness and reliability of the code.


Line range hint 139-161: Improved error handling in intoABCIEvents.

The intoABCIEvents function has been enhanced to return an error alongside the results, allowing for better error propagation. The retrieval of event attributes now includes error handling, which prevents the code from accessing potentially invalid data. If an error is encountered while retrieving event attributes, the function returns immediately with the error, ensuring that any issues encountered while processing events are properly communicated to the caller. These modifications contribute to the overall robustness and reliability of the code by gracefully handling errors and maintaining data integrity.

tests/go.mod (2)

69-69: Review the changelog and assess the impact of updating cosmossdk.io/schema to v0.3.0.

The minor version update suggests potential improvements or changes in functionality. Please review the changelog for cosmossdk.io/schema and assess how these changes might impact our codebase. Ensure that any necessary adjustments or migrations are made to maintain compatibility.


8-8: Verify the impact of updating cosmossdk.io/core to v1.0.0-alpha.3.

The version increment suggests potential enhancements or bug fixes. However, as it's still an alpha version, please ensure that this update doesn't introduce any breaking changes or compatibility issues in the codebase.

Run the following script to verify the usage of cosmossdk.io/core across the codebase:

Verification successful

To properly assess the impact of updating cosmossdk.io/core from v1.0.0-alpha.2 to v1.0.0-alpha.3, we need to gather more information about the changes introduced in this version. Let's search for any changelog entries, release notes, or comments in the code that might give us insight into the update.


Based on our search results, we couldn't find any specific information about the changes in cosmossdk.io/core v1.0.0-alpha.3 within the local repository. This lack of information doesn't necessarily mean there are no significant changes, but it suggests that any changes made might not have required extensive modifications or attention in the current codebase.

However, as this is an alpha version update, we should still proceed with caution. Here's what we can conclude:

Proceed with the update, but exercise caution and conduct thorough testing.

While we didn't find any explicit warnings or breaking changes mentioned in the codebase, alpha versions can still introduce subtle changes or new features that might affect the system. Here are some recommendations:

  1. Before applying the update in production, thoroughly test the entire application with the new version in a staging environment.
  2. Pay special attention to modules and components that heavily rely on cosmossdk.io/core.
  3. Review the official documentation or GitHub repository of cosmossdk.io/core for any changelog or release notes that might not be included in this repository.
  4. After updating, monitor the application closely for any unexpected behaviors or performance issues.

If any issues arise during testing or after deployment, be prepared to roll back to the previous version (v1.0.0-alpha.2) if necessary.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of `cosmossdk.io/core` and check for potential breaking changes.

# Test: Search for the import statements of `cosmossdk.io/core`. 
# Expect: Review the usage to ensure compatibility with the updated version.
rg --type go -A 5 $'cosmossdk.io/core'

Length of output: 267499


Script:

#!/bin/bash
# Search for changelog or release notes for cosmossdk.io/core v1.0.0-alpha.3
grep -r "cosmossdk.io/core v1.0.0-alpha.3" . --include="CHANGELOG.md" --include="RELEASE_NOTES.md"

# Look for any TODO comments or update notes related to this core package update
grep -r "TODO.*cosmossdk.io/core" .

# Check for any deprecation notices related to cosmossdk.io/core
grep -r "deprecated.*cosmossdk.io/core" .

# Search for any breaking changes mentioned in comments
grep -r "breaking change.*cosmossdk.io/core" .

Length of output: 227

simapp/go.mod (2)

65-65: Verify compatibility of the cosmossdk.io/schema version update.

The version of cosmossdk.io/schema has been updated from v0.2.0 to v0.3.0. Ensure that this version update is compatible with the rest of the codebase and doesn't introduce any breaking changes.

Run the following script to search for usages of cosmossdk.io/schema and verify compatibility:


9-9: Verify compatibility of the cosmossdk.io/core version update.

The version of cosmossdk.io/core has been incremented from v1.0.0-alpha.2 to v1.0.0-alpha.3. Ensure that this version update is compatible with the rest of the codebase and doesn't introduce any breaking changes.

Run the following script to search for usages of cosmossdk.io/core and verify compatibility:

Verification successful

The update to cosmossdk.io/core v1.0.0-alpha.3 appears to be compatible.

The changelog for this version indicates a single change:

  • The core/event package is now a type alias of schema/appdata.

This modification is unlikely to cause compatibility issues as it seems to be an internal restructuring. The functionality of core/event should remain the same, just accessed through a different package structure.

However, as this is an alpha release, it's always recommended to thoroughly test your application with the new version to ensure everything works as expected.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for usages of `cosmossdk.io/core` and verify compatibility.

# Test: Search for import statements and function calls related to `cosmossdk.io/core`.
rg --type go -A 5 $'cosmossdk\.io/core'

Length of output: 267502


Script:

#!/bin/bash
# Search for changelog or release notes for cosmossdk.io/core v1.0.0-alpha.3
echo "Searching for changelog or release notes..."
grep -r -i "changelog" --include="*.md" || true
grep -r -i "release notes" --include="*.md" || true
echo "Searching for v1.0.0-alpha.3 specific information..."
grep -r -i "v1.0.0-alpha.3" --include="*.md" || true

Length of output: 20659


Script:

#!/bin/bash
# Extract the changelog entry for v1.0.0-alpha.3 from core/CHANGELOG.md
echo "Changelog for core v1.0.0-alpha.3:"
sed -n '/## \[v1.0.0-alpha.3\]/,/## \[v1.0.0-alpha.2\]/p' core/CHANGELOG.md | sed '$d'

Length of output: 391

server/v2/cometbft/abci.go (2)

129-132: Excellent refactoring to improve code readability and error handling!

Extracting the event processing logic into a separate variable events and handling the error makes the code more readable and maintainable. If an error occurs during event conversion, it is properly propagated and handled.


138-138: Proper usage of the events variable.

The events variable, which holds the converted ABCI events, is correctly assigned to the Events field in the CheckTxResponse. This ensures that the processed events are included in the response.

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

1380-1400: LGTM!

The refactoring of event attribute construction using a closure enhances code readability and maintainability. The test logic is correct, and the implementation is accurate.


1461-1496: LGTM!

The refactoring of event attribute construction using closures for each event enhances code readability and maintainability. The test logic is correct, and the implementation is accurate.

@@ -81,6 +81,6 @@ func TypedEventToEvent(tev transaction.Msg) (event.Event, error) {

return event.Event{
Type: evtType,
Attributes: attrs,
Attributes: func() ([]event.Attribute, error) { return attrs, nil },
Copy link
Member

@aaronc aaronc Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can actually wrap all the logic in TypedEventToEvent in this closure so that JSON marshaling is done lazily. Let's do that in a follow up PR though, and merge this.

@cool-develope cool-develope added this pull request to the merge queue Sep 18, 2024
Merged via the queue into main with commit 01212de Sep 18, 2024
93 of 94 checks passed
@cool-develope cool-develope deleted the server/event_new_schema branch September 18, 2024 15:42
mergify bot pushed a commit that referenced this pull request Sep 18, 2024
(cherry picked from commit 01212de)

# Conflicts:
#	client/v2/go.mod
#	client/v2/go.sum
#	collections/collections.go
#	collections/go.mod
#	collections/go.sum
#	collections/indexing.go
#	go.mod
#	go.sum
#	runtime/v2/go.mod
#	runtime/v2/go.sum
#	server/v2/cometbft/go.mod
#	server/v2/cometbft/go.sum
#	server/v2/go.mod
#	server/v2/go.sum
#	server/v2/stf/core_event_service.go
#	server/v2/stf/go.mod
#	server/v2/stf/go.sum
#	server/v2/stf/stf.go
#	server/v2/streaming/utils.go
#	simapp/go.mod
#	simapp/go.sum
#	simapp/v2/go.mod
#	simapp/v2/go.sum
#	tests/go.mod
#	tests/go.sum
#	x/accounts/defaults/base/go.mod
#	x/accounts/defaults/base/go.sum
#	x/accounts/defaults/lockup/go.mod
#	x/accounts/defaults/lockup/go.sum
#	x/accounts/defaults/multisig/go.mod
#	x/accounts/defaults/multisig/go.sum
#	x/accounts/go.mod
#	x/accounts/go.sum
#	x/authz/go.mod
#	x/authz/go.sum
#	x/bank/go.mod
#	x/bank/go.sum
#	x/circuit/go.mod
#	x/circuit/go.sum
#	x/consensus/go.mod
#	x/consensus/go.sum
#	x/distribution/go.mod
#	x/distribution/go.sum
#	x/epochs/go.mod
#	x/epochs/go.sum
#	x/evidence/go.mod
#	x/evidence/go.sum
#	x/feegrant/go.mod
#	x/feegrant/go.sum
#	x/gov/go.mod
#	x/gov/go.sum
#	x/group/go.mod
#	x/group/go.sum
#	x/mint/go.mod
#	x/mint/go.sum
#	x/nft/go.mod
#	x/nft/go.sum
#	x/params/go.mod
#	x/params/go.sum
#	x/protocolpool/go.mod
#	x/protocolpool/go.sum
#	x/slashing/go.mod
#	x/slashing/go.sum
#	x/staking/go.mod
#	x/staking/go.sum
#	x/upgrade/go.mod
#	x/upgrade/go.sum
julienrbrt added a commit that referenced this pull request Sep 18, 2024
)

Co-authored-by: cool-developer <51834436+cool-develope@users.noreply.github.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
@coderabbitai coderabbitai bot mentioned this pull request Sep 19, 2024
12 tasks
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.

server/v2 Indexer Integration
4 participants