fix(stf): fixes to make init genesis pass #21088
Conversation
WalkthroughWalkthroughThe changes primarily enhance error handling and serialization practices across the application. Key updates include improved checks for nil pointers before unmarshalling configurations, more informative error reporting, and a transition to standardized JSON marshalling methods. These modifications aim to strengthen the application's robustness, maintainability, and overall reliability. Changes
Sequence Diagram(s)sequenceDiagram
participant Builder
participant Manager
participant EventService
participant Store
Builder->>Store: Check for store options
alt options exist
Store->>Builder: Unmarshal options
else options do not exist
Store-->>Builder: Skip unmarshalling
end
Manager->>Manager: Initialize Genesis
alt error occurs
Manager-->>Manager: Wrap error with context
end
EventService->>EventService: Marshal typed event
EventService->>EventService: Use jsonpb.Marshaler
Possibly related issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
@alpe your pull request is missing a changelog! |
julienrbrt
added
the
backport/v0.52.x
simapp/v2/app_di.go
Outdated
| @@ -31,6 +29,8 @@ import ( | |||
| slashingkeeper "cosmossdk.io/x/slashing/keeper" | |||
| stakingkeeper "cosmossdk.io/x/staking/keeper" | |||
| upgradekeeper "cosmossdk.io/x/upgrade/keeper" | |||
| _ "github.com/cosmos/cosmos-sdk/x/genutil" | |||
| "github.com/spf13/viper" | |||
There was a problem hiding this comment.
linter won't be happy of this
server/v2/stf/core_event_service.go
Outdated
| @@ -5,6 +5,7 @@ import ( | |||
| "encoding/json" | |||
| "slices" | |||
|
|
|||
| "github.com/cosmos/cosmos-sdk/codec" | |||
There was a problem hiding this comment.
we shouldn't bring in a cosmos-sdk dependency, can we use directly gogoproto jsonpb?
julienrbrt
changed the title
|
Adding the backport label as simapp/v2 and comet server imports the SDK and we have replaces everywhere still in release/v052.x |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (5)
- runtime/v2/builder.go (1 hunks)
- runtime/v2/manager.go (1 hunks)
- server/v2/stf/core_event_service.go (2 hunks)
- server/v2/store/commands.go (1 hunks)
- simapp/v2/app_di.go (2 hunks)
Files skipped from review due to trivial changes (1)
- simapp/v2/app_di.go
Additional context used
Path-based instructions (4)
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.server/v2/store/commands.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/v2/builder.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.runtime/v2/manager.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Learnings (1)
Common learnings
Learnt from: likhita-809 PR: cosmos/cosmos-sdk#18471 File: x/protocolpool/keeper/genesis.go:12-51 Timestamp: 2023-11-22T12:32:39.368Z Learning: - The user `likhita-809` has confirmed the changes suggested in the previous interaction. - The file in question is `x/protocolpool/keeper/genesis.go` from a Cosmos SDK module. - The changes involve optimizing the `InitGenesis` function by removing redundant code and ensuring proper handling of start times for budget proposals.
Additional comments not posted (5)
server/v2/stf/core_event_service.go (2)
8-8: Import statement added for codec packageThe import statement for the Cosmos SDK's codec package has been added to support the new marshalling method.
59-61: Change to JSON marshalling methodThe function now uses
codec.ProtoMarshalJSONinstead ofgogoproto.Marshalfor marshalling the event. This change aligns with standardized JSON marshalling practices in the Cosmos SDK, which can improve maintainability and compatibility.server/v2/store/commands.go (1)
132-134: Enhanced nil check for sub-configurationThe condition now ensures that
v.Sub("store.options")is not nil before attempting to unmarshal the store options. This change prevents potential nil pointer dereferences, improving the robustness of the code.runtime/v2/builder.go (1)
131-134: Enhanced nil check for sub-configurationThe condition now ensures that
v.Sub("store.options")is not nil before attempting to unmarshal the store options. This change prevents potential nil pointer dereferences, improving the robustness of the code.runtime/v2/manager.go (1)
169-169: Enhanced error reporting approved.The change improves error reporting by wrapping the error with the module name, which enhances the clarity of error logs and aids in debugging.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Commits
Files that changed from the base of the PR and between d741d2e and da16f20a87bef4088560696393438aad7b41cf79.
Files selected for processing (1)
- server/v2/stf/core_event_service.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- server/v2/stf/core_event_service.go
| evtJSON, err := gogoproto.Marshal(tev) | ||
| if err != nil { | ||
| buf := new(bytes.Buffer) | ||
| jm := &jsonpb.Marshaler{OrigName: true, EmitDefaults: true, AnyResolver: nil} |
There was a problem hiding this comment.
I am using jsonpb now but without the types.UnpackInterfaces to not add codec types.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Commits
Files that changed from the base of the PR and between da16f20a87bef4088560696393438aad7b41cf79 and b458062.
Files selected for processing (2)
- server/v2/stf/core_event_service.go (2 hunks)
- simapp/v2/app_di.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- server/v2/stf/core_event_service.go
- simapp/v2/app_di.go
|
Lint is failing. Once fixed should be good to merge |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (6)
- schema/appdata/async.go (1 hunks)
- schema/appdata/mux_test.go (1 hunks)
- schema/module_schema_test.go (2 hunks)
- server/v2/cometbft/client/grpc/cmtservice/service.go (2 hunks)
- server/v2/stf/core_event_service.go (2 hunks)
- simapp/v2/app_di.go (1 hunks)
Files skipped from review due to trivial changes (5)
- schema/appdata/async.go
- schema/appdata/mux_test.go
- schema/module_schema_test.go
- server/v2/cometbft/client/grpc/cmtservice/service.go
- simapp/v2/app_di.go
Files skipped from review as they are similar to previous changes (1)
- server/v2/stf/core_event_service.go
Co-authored-by: marbar3778 <[email protected]> (cherry picked from commit b05bb26) # Conflicts: # runtime/v2/builder.go # schema/appdata/async.go # schema/appdata/mux_test.go # schema/module_schema_test.go # server/v2/store/commands.go
Description
Some minor fixes while working on v2 sims integration.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Bug Fixes
Chores