fix: Add sims export/import test and fix #23462
Conversation
alpe
requested review from
julienrbrt,
tac0turtle,
JulianToledano,
ziscky,
a team and
lucaslopezf
as code owners
📝 WalkthroughWalkthroughThis pull request introduces several enhancements across multiple modules in the Cosmos SDK, focusing on improving genesis state management, simulation testing, and module configurations. The changes include adding new message types for tracking unbonding entries, consensus address rotations, and distribution events. The modifications also update store key handling, introduce new utility functions for store comparisons, and refactor some existing simulation and decoding mechanisms. Changes
Suggested Labels
Suggested Reviewers
Possibly Related PRs
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
| @@ -133,7 +133,7 @@ func (AppModule) GenerateGenesisState(simState *module.SimulationState) { | |||
|
|
|||
| // RegisterStoreDecoder registers a decoder for staking module's types | |||
| func (am AppModule) RegisterStoreDecoder(sdr simtypes.StoreDecoderRegistry) { | |||
| sdr[types.StoreKey] = simulation.NewDecodeStore(am.cdc) | |||
| sdr[types.StoreKey] = simtypes.NewStoreDecoderFuncFromCollectionsSchema(am.keeper.Schema) | |||
There was a problem hiding this comment.
Use default Collection based store decoder. The legacy one did not support all fields and panics
julienrbrt
added
the
backport/v0.52.x
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| default: | ||
| if fd.IsExtension() { | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.staking.v1beta1.LastValidatorPower")) | ||
| panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.staking.v1beta1.RotatedConsensusAddresses")) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| } | ||
| panic(fmt.Errorf("message cosmos.staking.v1beta1.LastValidatorPower does not contain field %s", fd.FullName())) | ||
| panic(fmt.Errorf("message cosmos.staking.v1beta1.RotatedConsensusAddresses does not contain field %s", fd.FullName())) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| @@ -331,6 +360,32 @@ | |||
| return nil, err | |||
| } | |||
|
|
|||
| var initConsAddrs, rotatedConsAddrs []types.RotatedConsensusAddresses | |||
| err = k.ConsAddrToValidatorIdentifierMap.Walk(ctx, nil, func(newBz []byte, initBz []byte) (stop bool, err error) { | |||
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
| initConsAddrs = append(initConsAddrs, types.RotatedConsensusAddresses{OldAddress: oldAddr, NewAddress: newAddr}) | ||
| return false, nil | ||
| }) | ||
| err = k.OldToNewConsAddrMap.Walk(ctx, nil, func(oldBz []byte, newBz []byte) (stop bool, err error) { |
Check warning
Code scanning / CodeQL
Useless assignment to local variable Warning
![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
🧹 Nitpick comments (11)
api/cosmos/mint/v1beta1/mint.pulsar.go (1)
Line range hint
679-685: Improve field comments to comply with Go documentation conventions.The comments for the newly added fields should be full sentences starting with a capital letter and ending with a period, following the Uber Go Style Guide.
Apply this diff to update the comments:
- // goal of percent bonded atoms + // Goal of percent bonded atoms. GoalBonded string `protobuf:"bytes,5,opt,name=goal_bonded,json=goalBonded,proto3" json:"goal_bonded,omitempty"` - // expected blocks per year + // Expected blocks per year. BlocksPerYear uint64 `protobuf:"varint,6,opt,name=blocks_per_year,json=blocksPerYear,proto3" json:"blocks_per_year,omitempty"` - // maximum supply for the token + // Maximum supply for the token. MaxSupply string `protobuf:"bytes,7,opt,name=max_supply,json=maxSupply,proto3" json:"max_supply,omitempty"`simapp/v2/sim_test_utils.go (5)
34-38: Simplify the mismatch logging loopThe loop in lines 34-37 logs only the first mismatch due to the
breakstatement. If the intention is to log all mismatches, consider removing thebreak; otherwise, if only the first mismatch should be logged, the loop may be unnecessary.
40-40: Clarify the log message to reflect actual comparison resultsThe log message on line 40 uses
len(failedKVAs), which represents the number of mismatched key/value pairs, but states "compared %d different key/value pairs". Consider adjusting the message to indicate the number of differences found or logging the total number of key/value pairs compared for clarity.
44-77: Use more descriptive variable names for readabilityVariables
aandbinDiffKVStoresandkvAs,kvBscan be renamed tostoreA,storeB,kvsA, andkvsBrespectively. This will enhance code clarity and maintainability, aligning with the Uber Go Style Guide's emphasis on explicit naming.
54-70: Ensure safe concurrent assignment to shared variablesAssigning to
kvAsandkvBswithin goroutines may lead to data races if accessed concurrently. Although currently safe due to theWaitGroup, consider declaringkvAsandkvBswithin the goroutines and returning them to avoid potential concurrency issues and adhere to best practices.
96-140: Improve variable naming ingetDiffFromKVPairIn the
getDiffFromKVPairfunction, variableskvAsandkvBscould be renamed tokvsAandkvsBorkvPairsAandkvPairsBto enhance readability, following the Uber Go Style Guide's recommendation for clear and descriptive variable names.simapp/v2/sim_test.go (2)
122-173: Enhance test coverage for specific staking export/import fixesWhile the
TestAppImportExportfunction verifies overall store equality after export and import, it would be beneficial to include explicit assertions onOldToNewConsAddrMapandConsAddrToValidatorIdentifierMap. This ensures that the specific staking issues addressed in the PR are thoroughly tested.
158-165: Rename thedecodeableinterface to follow Go conventionsPer Go naming conventions, interface names should end with "er". Consider renaming the
decodeableinterface tostoreDecoderfor clarity and consistency with established best practices.runtime/v2/manager.go (1)
87-100: LGTM! Clean implementation with proper store key management.The implementation correctly handles default naming, overrides, and skipped keys. Consider pre-allocating the map with the exact capacity needed.
-storeKeys := make(map[string]string, len(m.modules)) +moduleKeys := maps.Keys(m.modules) +storeKeys := make(map[string]string, len(moduleKeys)-len(m.config.SkipStoreKeys))types/simulation/collections.go (1)
40-49: Refactor to reduce code duplication when handlingvAandvB.The nil checks and stringification logic for
vAandvBare similar. Extracting this logic into a helper function can improve maintainability and readability.For example:
func NewStoreDecoderFuncFromCollectionsSchema(schema collections.Schema) func(kvA, kvB kv.Pair) string { // existing code... return func(kvA, kvB kv.Pair) string { // existing code... - vAString, vBString := "<nil>", "<nil>" - if vA != nil { - if vAString, err = vc.Stringify(vA); err != nil { - panic(err) - } - } - if vB != nil { - if vBString, err = vc.Stringify(vB); err != nil { - panic(err) - } - } + vAString := stringifyValue(vA, vc) + vBString := stringifyValue(vB, vc) return vAString + "\n" + vBString } } + +func stringifyValue(value any, vc collcodec.UntypedValueCodec) string { + if value == nil { + return "<nil>" + } + strValue, err := vc.Stringify(value) + if err != nil { + panic(err) + } + return strValue +}x/staking/proto/cosmos/staking/v1beta1/genesis.proto (1)
Line range hint
95-99: Consider adding validation rules for RotationQueueRecordWhile the message structure is correct, consider adding comments to describe any validation rules or constraints for the
val_addrsfield.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
x/accounts/defaults/lockup/v1/lockup.pb.gois excluded by!**/*.pb.gox/accounts/v1/tx.pb.gois excluded by!**/*.pb.gox/mint/types/mint.pb.gois excluded by!**/*.pb.gox/protocolpool/types/genesis.pb.gois excluded by!**/*.pb.gox/staking/types/genesis.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (22)
api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go(1 hunks)api/cosmos/accounts/v1/tx.pulsar.go(1 hunks)api/cosmos/mint/v1beta1/mint.pulsar.go(1 hunks)api/cosmos/protocolpool/v1/genesis.pulsar.go(2 hunks)api/cosmos/staking/v1beta1/genesis.pulsar.go(48 hunks)runtime/v2/manager.go(1 hunks)scripts/build/simulations.mk(1 hunks)simapp/v2/app_config.go(1 hunks)simapp/v2/sim_runner.go(1 hunks)simapp/v2/sim_test.go(2 hunks)simapp/v2/sim_test_utils.go(1 hunks)types/simulation/collections.go(1 hunks)x/accounts/proto/cosmos/accounts/defaults/lockup/v1/lockup.proto(1 hunks)x/accounts/proto/cosmos/accounts/v1/tx.proto(0 hunks)x/mint/proto/cosmos/mint/v1beta1/mint.proto(0 hunks)x/protocolpool/proto/cosmos/protocolpool/v1/genesis.proto(1 hunks)x/staking/CHANGELOG.md(1 hunks)x/staking/depinject.go(1 hunks)x/staking/keeper/genesis.go(3 hunks)x/staking/proto/cosmos/staking/v1beta1/genesis.proto(2 hunks)x/staking/simulation/decoder.go(0 hunks)x/staking/simulation/decoder_test.go(0 hunks)
💤 Files with no reviewable changes (4)
- x/mint/proto/cosmos/mint/v1beta1/mint.proto
- x/staking/simulation/decoder.go
- x/staking/simulation/decoder_test.go
- x/accounts/proto/cosmos/accounts/v1/tx.proto
🧰 Additional context used
📓 Path-based instructions (14)
simapp/v2/app_config.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
types/simulation/collections.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/sim_runner.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
x/staking/depinject.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.
api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
x/staking/keeper/genesis.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/mint/v1beta1/mint.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/protocolpool/v1/genesis.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
x/staking/CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
api/cosmos/accounts/v1/tx.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/sim_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"
simapp/v2/sim_test_utils.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/staking/v1beta1/genesis.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 golangci-lint (1.62.2)
x/staking/keeper/genesis.go
364-364: File is not gofumpt-ed with -extra
(gofumpt)
376-376: File is not gofumpt-ed with -extra
(gofumpt)
🪛 GitHub Actions: Lint
x/staking/keeper/genesis.go
[warning] 364-364: File is not gofumpt-ed with -extra
[warning] 376-376: File is not gofumpt-ed with -extra
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze
- GitHub Check: Summary
🔇 Additional comments (28)
scripts/build/simulations.mk (1)
31-33: LGTM! Enabled thetest-sim-import-exporttarget correctlyThe uncommented lines properly enable the
test-sim-import-exporttarget, allowing the import/export simulation to run as intended.simapp/v2/app_config.go (1)
191-193: LGTM! Appropriate modules added to SkipStoreKeys.The addition of
genutiltypes,runtime, andvestingmodules toSkipStoreKeysis correct as these modules don't require persistent storage keys.simapp/v2/sim_runner.go (1)
87-87: LGTM! Well-defined interface addition.The
StoreKeys()method is a clean addition to theModuleManagerinterface, providing a way to access module store keys.api/cosmos/staking/v1beta1/genesis.pulsar.go (12)
427-476: New list types forRotatedConsensusAddressesare correctly implementedThe addition of
_GenesisState_12_listand_GenesisState_13_listto handle slices ofRotatedConsensusAddressesfollows the existing patterns in the codebase and is correctly implemented.
Line range hint
542-561: Field descriptors for newGenesisStatefields are properly initializedThe new field descriptors
fd_GenesisState_rotated_cons_addressesandfd_GenesisState_initial_cons_addressesare correctly added and initialized in theinit()function, ensuring proper reflection.
695-706: Updates toRange()method include new fields appropriatelyThe
Range()method infastReflection_GenesisStateis correctly updated to include the new fieldsRotatedConsAddressesandInitialConsAddresses, allowing iteration over these fields.
744-747: Proper implementation ofHas()method for new fieldsThe
Has()method properly checks for the presence of the new fieldsRotatedConsAddressesandInitialConsAddresses.
786-789:Clear()method correctly handles new fieldsThe
Clear()method is updated to clear the new fieldsRotatedConsAddressesandInitialConsAddresses, ensuring they can be reset appropriately.
933-940: Updates toSet()method include new fieldsThe
Set()method correctly handles setting the values ofRotatedConsAddressesandInitialConsAddresses.
1014-1025:Mutable()method adjusted for new fieldsThe
Mutable()method correctly provides mutable access for the new fields inGenesisState.
1074-1079:NewField()method appropriately initializes new fieldsThe
NewField()method returns appropriate default values for the new list fields, ensuring consistency.
Line range hint
1208-1280: Serialization methods updated for new fieldsThe
sizeandmarshalfunctions inProtoMethods()are appropriately updated to handle serialization of the new fields, ensuring that the new data is correctly serialized and deserialized.
1852-1935:Unmarshalfunction correctly parses new fieldsThe
unmarshalfunction is updated to correctly parse the new fieldsRotatedConsAddressesandInitialConsAddressesduring deserialization.
Line range hint
2423-2491: Definition ofRotatedConsensusAddressesis well-structuredThe new struct
RotatedConsensusAddressesand its associated methods are correctly implemented, providing the necessary functionality for handling rotated consensus addresses.
Line range hint
3960-4077: New fields added toGenesisStatestruct are properly definedThe addition of
RotatedConsAddressesandInitialConsAddressesto theGenesisStatestruct is correctly implemented, with appropriate protobuf annotations and JSON tags.api/cosmos/accounts/v1/tx.pulsar.go (1)
4720-4826: LGTM!The changes in the protobuf message definitions appear to be generated code and conform to the expected structure.
x/staking/depinject.go (1)
136-136: Use the default collection-based store decoder.This update replaces the legacy store decoder with
NewStoreDecoderFuncFromCollectionsSchema, ensuring better support for all fields and preventing potential panics.x/staking/keeper/genesis.go (4)
198-211: LGTM! Proper error handling for consensus address rotation.The implementation correctly decodes and stores old-to-new consensus address mappings with appropriate error handling.
212-225: LGTM! Proper error handling for initial consensus addresses.The implementation correctly decodes and stores initial-to-current consensus address mappings with appropriate error handling.
363-388: LGTM! Proper error handling in export logic.The implementation correctly exports both mappings with appropriate error handling and clear error messages.
🧰 Tools
🪛 golangci-lint (1.62.2)
364-364: File is not
gofumpt-ed with-extra(gofumpt)
376-376: File is not
gofumpt-ed with-extra(gofumpt)
🪛 GitHub Actions: Lint
[warning] 364-364: File is not
gofumpt-ed with-extra
[warning] 376-376: File is not
gofumpt-ed with-extra
401-402: LGTM! Proper inclusion in genesis state.The exported mappings are correctly included in the genesis state.
api/cosmos/protocolpool/v1/genesis.pulsar.go (1)
Line range hint
1549-1656: LGTM! Generated code matches protobuf definition.The generated code correctly implements the
Distributionmessage type with all required fields and methods.api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go (1)
Line range hint
1772-2000: LGTM! Generated code matches protobuf definition.The generated code correctly implements the
UnbondingEntriesmessage type with all required fields and methods.x/accounts/proto/cosmos/accounts/defaults/lockup/v1/lockup.proto (1)
26-29: LGTM! Clear and concise message definition.The
UnbondingEntriesmessage type is well-defined and follows protobuf best practices.x/protocolpool/proto/cosmos/protocolpool/v1/genesis.proto (1)
Line range hint
31-39: Well-structured message definition!The
Distributionmessage is well-defined with clear field names, appropriate types, and helpful comments explaining the purpose of each field. The use of gogoproto annotations is correct.x/staking/proto/cosmos/staking/v1beta1/genesis.proto (2)
50-59: Well-structured additions to GenesisState!The new fields for handling consensus address rotation are well-organized with appropriate annotations and clear comments.
74-84: Well-defined RotatedConsensusAddresses message!The message structure clearly represents the old and new consensus addresses with appropriate scalar annotations for address fields.
x/staking/CHANGELOG.md (1)
30-31: Changelog entry is well-formatted!The entry follows the changelog format guidelines and clearly describes the purpose of the changes.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
api/cosmos/staking/v1beta1/genesis.pulsar.go (1)
4065-4077: Getter methods for RotatedConsAddresses and InitialConsAddresses
These accessors match the typical proto-generated code style.Consider returning an empty slice instead of
nil, but this is a minor style preference in some projects.api/cosmos/accounts/v1/tx.pulsar.go (1)
4720-4826: Function and variable naming conventions in auto-generated code.The auto-generated code includes function and variable names with underscores (e.g.,
fastReflection_MsgInit), which deviates from the Uber Golang style guide that recommends usingcamelCasefor naming. While auto-generated code is typically exempt from manual style corrections, consider configuring the code generatorprotoc-gen-go-pulsarto adhere to Go naming conventions to improve code consistency.scripts/build/simulations.mk (1)
33-33: UseSIM_NUM_BLOCKSvariable for consistency.To maintain consistency with other simulation targets and allow for easier configuration, consider using the
SIM_NUM_BLOCKSvariable instead of hardcoding the number of blocks.Apply this diff to use the variable:
-NumBlocks=50 + -NumBlocks=$(SIM_NUM_BLOCKS)simapp/v2/sim_test_utils.go (1)
96-140: Improve code readability with early returns.The function
getDiffFromKVPairhas nested conditions that could be simplified with early returns. The deferred swap logic also makes the flow harder to follow.Consider restructuring for better readability:
func getDiffFromKVPair(kvAs, kvBs []kv.Pair) (diffA, diffB []kv.Pair) { - // we assume that kvBs is equal or larger than kvAs - // if not, we swap the two - if len(kvAs) > len(kvBs) { - kvAs, kvBs = kvBs, kvAs - // we need to swap the diffA and diffB as well - defer func() { - diffA, diffB = diffB, diffA - }() - } + // Handle the case where kvAs is larger + if len(kvAs) > len(kvBs) { + diffB, diffA = getDiffFromKVPair(kvBs, kvAs) + return + } - // in case kvAs is empty we can return early - // since there is nothing to compare - // if kvAs == kvBs, then diffA and diffB will be empty - if len(kvAs) == 0 { - return []kv.Pair{}, kvBs - } + // Handle empty kvAs case + if len(kvAs) == 0 { + return []kv.Pair{}, kvBs + }x/staking/keeper/genesis.go (2)
363-388: Fix gofumpt formatting issues.The code has formatting issues reported by the linter.
Run
gofumpt -w -extra .to fix the formatting:
- Add a blank line before line 364
- Add a blank line before line 376
🧰 Tools
🪛 golangci-lint (1.62.2)
364-364: File is not
gofumpt-ed with-extra(gofumpt)
376-376: File is not
gofumpt-ed with-extra(gofumpt)
🪛 GitHub Actions: Lint
[warning] 364-364: File is not
gofumpt-ed with-extra
[warning] 376-376: File is not
gofumpt-ed with-extra
363-388: Consider adding validation for duplicate addresses.The code should validate that there are no duplicate addresses in the maps to prevent potential conflicts.
Consider adding validation:
var initConsAddrs, rotatedConsAddrs []types.RotatedConsensusAddresses + seen := make(map[string]bool) err = k.ConsAddrToValidatorIdentifierMap.Walk(ctx, nil, func(newBz []byte, initBz []byte) (stop bool, err error) { oldAddr, err2 := k.consensusAddressCodec.BytesToString(initBz) if err2 != nil { return true, fmt.Errorf("decode initial address %X: %w", initBz, err2) } newAddr, err2 := k.consensusAddressCodec.BytesToString(newBz) if err2 != nil { return true, fmt.Errorf("decode new address %X: %w", newBz, err2) } + if seen[oldAddr] || seen[newAddr] { + return true, fmt.Errorf("duplicate address found: old=%s new=%s", oldAddr, newAddr) + } + seen[oldAddr] = true + seen[newAddr] = true initConsAddrs = append(initConsAddrs, types.RotatedConsensusAddresses{OldAddress: oldAddr, NewAddress: newAddr}) return false, nil })🧰 Tools
🪛 golangci-lint (1.62.2)
364-364: File is not
gofumpt-ed with-extra(gofumpt)
376-376: File is not
gofumpt-ed with-extra(gofumpt)
🪛 GitHub Actions: Lint
[warning] 364-364: File is not
gofumpt-ed with-extra
[warning] 376-376: File is not
gofumpt-ed with-extrax/staking/proto/cosmos/staking/v1beta1/genesis.proto (3)
50-52: Consider adding a more descriptive comment for the rotation_queue field.The current comment "RotationQueue with address and time tuples" could be more descriptive about the purpose and significance of this queue in the context of consensus key rotation.
53-59: Ensure consistent naming convention for address-related fields.The field names use inconsistent casing:
rotated_cons_addressesuses snake_caseRotatedConsAddressesin the comment uses PascalCase
Line range hint
95-99: Add field descriptions for RotationQueueRecord.While the message structure is correct, it would benefit from having descriptions for both fields to clarify their purpose and usage.
x/staking/CHANGELOG.md (1)
Line range hint
30-34: Add more details about the genesis import/export fix.The changelog entry for PR #23462 could be more descriptive about what specific data was missing and how it affects users.
Consider expanding the entry to:
-* [#23462](https://github.com/cosmos/cosmos-sdk/pull/23462) fixes missing data for genesis ex-/import on key rotation +* [#23462](https://github.com/cosmos/cosmos-sdk/pull/23462) Fix missing consensus address mappings during genesis export/import when validators have rotated their consensus keys
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
x/accounts/defaults/lockup/v1/lockup.pb.gois excluded by!**/*.pb.gox/accounts/v1/tx.pb.gois excluded by!**/*.pb.gox/mint/types/mint.pb.gois excluded by!**/*.pb.gox/protocolpool/types/genesis.pb.gois excluded by!**/*.pb.gox/staking/types/genesis.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (22)
api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go(1 hunks)api/cosmos/accounts/v1/tx.pulsar.go(1 hunks)api/cosmos/mint/v1beta1/mint.pulsar.go(1 hunks)api/cosmos/protocolpool/v1/genesis.pulsar.go(2 hunks)api/cosmos/staking/v1beta1/genesis.pulsar.go(48 hunks)runtime/v2/manager.go(1 hunks)scripts/build/simulations.mk(1 hunks)simapp/v2/app_config.go(1 hunks)simapp/v2/sim_runner.go(1 hunks)simapp/v2/sim_test.go(2 hunks)simapp/v2/sim_test_utils.go(1 hunks)types/simulation/collections.go(1 hunks)x/accounts/proto/cosmos/accounts/defaults/lockup/v1/lockup.proto(1 hunks)x/accounts/proto/cosmos/accounts/v1/tx.proto(0 hunks)x/mint/proto/cosmos/mint/v1beta1/mint.proto(0 hunks)x/protocolpool/proto/cosmos/protocolpool/v1/genesis.proto(1 hunks)x/staking/CHANGELOG.md(1 hunks)x/staking/depinject.go(1 hunks)x/staking/keeper/genesis.go(3 hunks)x/staking/proto/cosmos/staking/v1beta1/genesis.proto(2 hunks)x/staking/simulation/decoder.go(0 hunks)x/staking/simulation/decoder_test.go(0 hunks)
💤 Files with no reviewable changes (4)
- x/mint/proto/cosmos/mint/v1beta1/mint.proto
- x/staking/simulation/decoder.go
- x/accounts/proto/cosmos/accounts/v1/tx.proto
- x/staking/simulation/decoder_test.go
🧰 Additional context used
📓 Path-based instructions (14)
types/simulation/collections.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/sim_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"
x/staking/CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
simapp/v2/app_config.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/protocolpool/v1/genesis.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
x/staking/depinject.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/sim_runner.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/sim_test_utils.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/accounts/v1/tx.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
x/staking/keeper/genesis.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.
api/cosmos/mint/v1beta1/mint.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/staking/v1beta1/genesis.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 golangci-lint (1.62.2)
x/staking/keeper/genesis.go
364-364: File is not gofumpt-ed with -extra
(gofumpt)
376-376: File is not gofumpt-ed with -extra
(gofumpt)
🪛 GitHub Actions: Lint
x/staking/keeper/genesis.go
[warning] 364-364: File is not gofumpt-ed with -extra
[warning] 376-376: File is not gofumpt-ed with -extra
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze
- GitHub Check: Summary
🔇 Additional comments (27)
api/cosmos/mint/v1beta1/mint.pulsar.go (1)
1526-1588: Auto-generated protobuf descriptor changes
These lines appear to be part of the compiled raw descriptor. The additions don't present functional or syntactical issues.api/cosmos/staking/v1beta1/genesis.pulsar.go (13)
427-476: Introduction of_GenesisState_12_listfor RotatedConsensusAddresses
This auto-generated code segment correctly follows the existing pattern for repeated fields.
478-527: Introduction of_GenesisState_13_listfor InitialConsAddresses
Similarly, this segment properly implements a repeated field list structure.
542-543: Field descriptors for rotated and initial consensus addresses
Defines descriptors for the new repeated fields, adhering to the standard approach.
560-561: Initialization of new field descriptors
Ensures proper registration of new fields in theinitfunction.
695-706: Range logic for new repeated fields
Correctly enumerates the new repeated fields in theRangeinvocation.
744-747:Hasmethod for new repeated fields
Follows the same logic as existing repeated fields. No concerns.
786-789:Clearmethod for new repeated fields
Properly resets the new repeated fields to nil.
933-940:Setmethod for new repeated fields
Implementation is consistent with proto reflection patterns.
1014-1025:Mutablemethod for new repeated fields
Allows for correct creation or extension of repeated fields.
1208-1219: Size calculation for the new repeated fields
Properly accounts for lengths of both message slices.
1249-1280: Marshaling the newly introduced repeated fields
Implements the standard approach by iterating backward and writing each element.
1852-1885: Unmarshal logic forrotated_cons_addresses
Decodes each entry and appends to the slice, following standard patterns.
1886-1919: Unmarshal logic forinitial_cons_addresses
Mirrors the approach used forrotated_cons_addresses.x/staking/depinject.go (1)
136-136: LGTM – updated store decoder registration.The change to use
simtypes.NewStoreDecoderFuncFromCollectionsSchema(am.keeper.Schema)improves the store decoder by utilizing the collections schema, aligning with the latest best practices. Good work on updating the store decoder function.simapp/v2/sim_test_utils.go (1)
36-36: Remove TODO comment or create an issue.The TODO comment
break // todo: removeshould either be removed or tracked in an issue for future follow-up.Would you like me to help create an issue to track this TODO?
simapp/v2/sim_test.go (1)
121-173: LGTM! Well-structured test with comprehensive store comparison.The test function is well-implemented with:
- Clear separation of export and import steps
- Proper handling of store comparisons with skip prefixes
- Comprehensive logging for debugging
simapp/v2/app_config.go (1)
191-193: LGTM! Appropriate additions to SkipStoreKeys.The new entries are correctly added to skip store construction for modules that don't require a store key.
x/staking/keeper/genesis.go (1)
198-225: LGTM! Well-implemented consensus address handling.The implementation correctly:
- Handles address decoding with proper error messages
- Maintains clear mapping between old and new consensus addresses
- Provides informative error context with entry indices
simapp/v2/sim_runner.go (1)
87-87: Verify implementations of the ModuleManager interface.The addition of
StoreKeys()method is a breaking change that requires all implementations of theModuleManagerinterface to provide store key mappings.Run this script to find all implementations that need updating:
✅ Verification successful
No breaking changes detected in ModuleManager implementations
The only implementation of the ModuleManager interface found in the codebase (
MMstruct inruntime/v2/manager.go) already implements theStoreKeys()method with the correct signature. No updates are required for existing implementations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all Go files that may contain ModuleManager implementations ast-grep --pattern 'type $_ struct { $$$ } func ($x $_) Modules() map[string]$_ { $$$ }'Length of output: 95
Script:
#!/bin/bash # First, find the ModuleManager interface definition echo "=== Finding ModuleManager interface ===" ast-grep --pattern 'type ModuleManager interface { $$$ }' echo -e "\n=== Finding potential implementations ===" # Look for struct types that implement methods matching the interface name rg "func \(\w+ \w+\) (?:Modules|StoreKeys)" --type goLength of output: 305
Script:
#!/bin/bash echo "=== Finding interfaces with StoreKeys method ===" ast-grep --pattern 'interface { $$$ StoreKeys() $_ $$$ }' echo -e "\n=== Finding StoreKeys implementations ===" rg "func.*StoreKeys\(\)" --type go -A 2Length of output: 2026
runtime/v2/manager.go (1)
87-100: LGTM! Clean implementation of store key mapping.The implementation:
- Provides default store keys based on module names
- Handles overrides via config
- Supports skipping specific keys
- Is thread-safe by creating a new map
api/cosmos/protocolpool/v1/genesis.pulsar.go (1)
Line range hint
1549-1656: Generated code looks good.This is machine-generated protobuf code that follows the expected patterns.
api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go (1)
Line range hint
1772-1784: Generated code looks good.This is machine-generated protobuf code that follows the expected patterns.
x/staking/proto/cosmos/staking/v1beta1/genesis.proto (1)
74-84: LGTM! Well-structured message definition.The
RotatedConsensusAddressesmessage is well-defined with clear field descriptions and appropriate scalar type for address fields.types/simulation/collections.go (1)
40-49: Good improvement in nil handling!The changes properly handle nil values by setting default strings and checking for nil before stringification, preventing potential panics.
x/accounts/proto/cosmos/accounts/defaults/lockup/v1/lockup.proto (1)
26-28: LGTM! Clear and concise message definition.The
UnbondingEntriesmessage is well-defined with a clear purpose of grouping multiple entries.x/protocolpool/proto/cosmos/protocolpool/v1/genesis.proto (1)
Line range hint
31-38: LGTM! Well-documented message definition.The
Distributionmessage is well-structured with clear comments explaining the purpose of each field and appropriate use of protobuf options.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
x/protocolpool/proto/cosmos/protocolpool/v1/genesis.proto (1)
Line range hint
31-39: LGTM - Well-structured proto definition with clear documentation.The
Distributionmessage type is well-defined with appropriate field types and protobuf options. The documentation clearly explains the purpose of each field and the rationale behind tracking distribution time.Consider adding examples in the documentation to illustrate typical usage scenarios, such as:
// Example: // { // "time": "2024-01-20T15:00:00Z", // "amount": { // "coins": [ // {"denom": "stake", "amount": "1000"} // ] // } // }simapp/v2/sim_test.go (2)
150-157: Consider extracting skipPrefixes to a constant.The skipPrefixes map could be defined as a package-level constant to improve maintainability and reusability.
161-166: Optimize store decoder registration.Consider these improvements:
- Pre-allocate the storeDecoders map with the expected size
- Add error handling for the type assertion
- storeDecoders := make(simtypes.StoreDecoderRegistry) + storeDecoders := make(simtypes.StoreDecoderRegistry, len(ti.ModuleManager.Modules())) for _, m := range ti.ModuleManager.Modules() { - if v, ok := m.(decodeable); ok { + if v, ok := m.(decodeable); !ok { + tb.Logf("module %T does not implement decodeable interface", m) + continue + } else { v.RegisterStoreDecoder(storeDecoders) } }
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
x/accounts/defaults/lockup/v1/lockup.pb.gois excluded by!**/*.pb.gox/accounts/v1/tx.pb.gois excluded by!**/*.pb.gox/mint/types/mint.pb.gois excluded by!**/*.pb.gox/protocolpool/types/genesis.pb.gois excluded by!**/*.pb.gox/staking/types/genesis.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (22)
api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go(1 hunks)api/cosmos/accounts/v1/tx.pulsar.go(1 hunks)api/cosmos/mint/v1beta1/mint.pulsar.go(1 hunks)api/cosmos/protocolpool/v1/genesis.pulsar.go(2 hunks)api/cosmos/staking/v1beta1/genesis.pulsar.go(48 hunks)runtime/v2/manager.go(1 hunks)scripts/build/simulations.mk(1 hunks)simapp/v2/app_config.go(1 hunks)simapp/v2/sim_runner.go(1 hunks)simapp/v2/sim_test.go(2 hunks)simapp/v2/sim_test_utils.go(1 hunks)types/simulation/collections.go(1 hunks)x/accounts/proto/cosmos/accounts/defaults/lockup/v1/lockup.proto(1 hunks)x/accounts/proto/cosmos/accounts/v1/tx.proto(0 hunks)x/mint/proto/cosmos/mint/v1beta1/mint.proto(0 hunks)x/protocolpool/proto/cosmos/protocolpool/v1/genesis.proto(1 hunks)x/staking/CHANGELOG.md(1 hunks)x/staking/depinject.go(1 hunks)x/staking/keeper/genesis.go(3 hunks)x/staking/proto/cosmos/staking/v1beta1/genesis.proto(2 hunks)x/staking/simulation/decoder.go(0 hunks)x/staking/simulation/decoder_test.go(0 hunks)
💤 Files with no reviewable changes (4)
- x/accounts/proto/cosmos/accounts/v1/tx.proto
- x/mint/proto/cosmos/mint/v1beta1/mint.proto
- x/staking/simulation/decoder_test.go
- x/staking/simulation/decoder.go
✅ Files skipped from review due to trivial changes (1)
- api/cosmos/accounts/v1/tx.pulsar.go
🧰 Additional context used
📓 Path-based instructions (13)
simapp/v2/app_config.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
types/simulation/collections.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/sim_runner.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.
x/staking/keeper/genesis.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/sim_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"
api/cosmos/mint/v1beta1/mint.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
x/staking/CHANGELOG.md (1)
Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
x/staking/depinject.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/protocolpool/v1/genesis.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
simapp/v2/sim_test_utils.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
api/cosmos/staking/v1beta1/genesis.pulsar.go (1)
Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
🪛 golangci-lint (1.62.2)
x/staking/keeper/genesis.go
364-364: File is not gofumpt-ed with -extra
(gofumpt)
376-376: File is not gofumpt-ed with -extra
(gofumpt)
🪛 GitHub Check: CodeQL
x/staking/keeper/genesis.go
[warning] 364-364: Useless assignment to local variable
This definition of err is never used.
[warning] 376-376: Useless assignment to local variable
This definition of err is never used.
api/cosmos/staking/v1beta1/genesis.pulsar.go
[warning] 2599-2599: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
[warning] 2601-2601: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
🪛 GitHub Actions: Lint
x/staking/keeper/genesis.go
[warning] 364-364: File is not gofumpt-ed with -extra
[warning] 376-376: File is not gofumpt-ed with -extra
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: main
- GitHub Check: main
- GitHub Check: assign-reviewers
- GitHub Check: Summary
🔇 Additional comments (32)
api/cosmos/protocolpool/v1/genesis.pulsar.go (1)
Line range hint
1549-1656: LGTM - Generated code matches proto definitions.The generated code correctly implements the
Distributionmessage type with all its fields and metadata as defined in the proto file. Since this is an auto-generated file, no manual changes should be made here.simapp/v2/sim_test_utils.go (2)
88-88: 🛠️ Refactor suggestionHandle unexpected keys without panicking.
Using
panicin library code can cause unexpected crashes in applications that use this library. Instead, consider returning an error or logging the issue to allow the calling function to handle it gracefully.Apply this diff to avoid the panic:
- panic(fmt.Errorf("unexpected key %X (%s)", kvA.Key, kvA.Key)) + tb.Fatalf("unexpected key %X (%s)", kvA.Key, kvA.Key)Likely invalid or redundant comment.
74-74: 🛠️ Refactor suggestionReplace
fmt.Printfwithtb.Logffor consistent test logging.Using
fmt.Printfin test code is discouraged as it bypasses the test logging mechanisms provided by the testing framework. Switching totb.Logfensures that logs are properly captured and reported in test outputs.Apply this diff to make the change:
- fmt.Printf("%q:: KV stores are different: %d key/value pairs in store A and %d key/value pairs in store B\n", storeKey, len(kvAs), len(kvBs)) + tb.Logf("%q:: KV stores are different: %d key/value pairs in store A and %d key/value pairs in store B", storeKey, len(kvAs), len(kvBs))Likely invalid or redundant comment.
api/cosmos/staking/v1beta1/genesis.pulsar.go (14)
427-527: New generated code forRotatedConsensusAddressesis correctly implemented.The addition of the
RotatedConsensusAddressestype and its associated methods aligns with the updated protobuf definitions. The methods properly implement the required interfaces, ensuring seamless serialization and deserialization.
560-561:GenesisStatestruct updated to include new fields appropriately.The fields
RotatedConsAddressesandInitialConsAddressesare correctly added to theGenesisStatestruct. This ensures that the genesis state can accurately represent the rotated consensus addresses during initialization and export.
695-706: ExtendedRangemethod to include new fields in iteration.The
Rangemethod offastReflection_GenesisStatenow includes the new fieldsRotatedConsAddressesandInitialConsAddresses. This change ensures that these fields are considered during reflection operations.
744-747: UpdatedHasmethod to support new fields.The
Hasmethod correctly checks for the presence of the new fieldsRotatedConsAddressesandInitialConsAddresses, maintaining consistency in field population checks.
786-789: EnhancedClearmethod to handle new fields appropriately.The
Clearmethod now includes cases to clearRotatedConsAddressesandInitialConsAddresses, ensuring that these fields can be reset when necessary.
863-874: AdjustedGetmethod to retrieve new fields properly.The
Getmethod is updated to return the values ofRotatedConsAddressesandInitialConsAddressesfields, enabling accurate data retrieval.
933-940: ModifiedSetmethod to assign values to new fields.The
Setmethod allows assignment toRotatedConsAddressesandInitialConsAddresses, facilitating correct data storage in theGenesisState.
1014-1025: ImplementedMutablemethod support for new fields.The
Mutablemethod now can initialize and return mutable references to the new fields, ensuring they can be modified as needed.
1074-1079: ProvidedNewFieldmethod implementations for new list fields.The
NewFieldmethod correctly creates new list instances forRotatedConsAddressesandInitialConsAddresses, maintaining consistency in object creation.
1208-1219: Adjusted marshaling size calculation to include new fields.The size calculation in the
ProtoMethodsnow accounts forRotatedConsAddressesandInitialConsAddresses, ensuring accurate buffer allocation during marshaling.
1249-1280: Updated marshaling logic to serialize new fields properly.The marshaling function includes serialization of
RotatedConsAddressesandInitialConsAddresses, guaranteeing that these fields are correctly written to the buffer.
1775-1942: Enhanced unmarshaling logic to deserialize new fields correctly.The unmarshaling function now handles
RotatedConsAddressesandInitialConsAddresses, allowing for accurate reconstruction of theGenesisStatefrom serialized data.
Line range hint
2423-2759: Added new message typeRotatedConsensusAddresseswith necessary methods.The
RotatedConsensusAddressestype includes all required methods for protobuf message handling, ensuring compatibility and proper functioning within the protocol buffers framework.
Line range hint
3960-4069: Protobuf definitions and generated code updated to include new fields and types.The
GenesisStatemessage in the protobuf file now containsrotated_cons_addressesandinitial_cons_addresses, and the generated Go code reflects these additions accurately.types/simulation/collections.go (1)
40-51: Prevent potential panics by checking fornilbefore stringification.The added
nilchecks before stringifyingvAandvBensure that the function handlesnilvalues gracefully, preventing potential panics during simulation store decoding.scripts/build/simulations.mk (1)
31-33: LGTM! Test configuration is well structured.The test command is properly configured with appropriate timeout and block count parameters.
x/staking/depinject.go (1)
136-136: Improved store decoder implementation.The switch to Collection based store decoder is a good improvement as it provides better support for all fields and prevents panics.
simapp/v2/app_config.go (1)
191-193: LGTM! Appropriate modules added to SkipStoreKeys.The added modules (genutil, runtime, vesting) correctly belong in SkipStoreKeys as they don't require their own store keys.
x/staking/keeper/genesis.go (3)
198-211: LGTM! Well-structured error handling for rotated consensus addresses.The implementation properly decodes and stores old-to-new consensus address mappings with clear error messages that include the index of the failing entry.
212-225: LGTM! Well-structured error handling for initial consensus addresses.The implementation properly decodes and stores initial-to-current consensus address mappings with clear error messages that include the index of the failing entry.
401-402: LGTM! Proper inclusion of new fields in genesis state.The new fields
RotatedConsAddressesandInitialConsAddressesare correctly added to the genesis state.simapp/v2/sim_runner.go (1)
87-87: LGTM! Clean interface extension.The addition of
StoreKeys()method to theModuleManagerinterface is well-defined and follows the interface design pattern.runtime/v2/manager.go (1)
87-100: LGTM! Clean and efficient implementation of store keys mapping.The implementation properly handles:
- Default store key naming convention
- Store key overrides from config
- Store key exclusions from config
api/cosmos/mint/v1beta1/mint.pulsar.go (1)
Line range hint
1-1588: Skip review of generated protobuf code.This is an auto-generated protobuf file. Any changes should be made to the source .proto files instead.
api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go (1)
1772-1772: Generated code looks good!The generated protobuf code correctly implements the
UnbondingEntriesmessage with all necessary methods and follows protobuf generation standards.x/accounts/proto/cosmos/accounts/defaults/lockup/v1/lockup.proto (1)
26-29: Well-structured protobuf definition!The
UnbondingEntriesmessage is well-defined with:
- Clear documentation
- Proper field numbering
- Appropriate use of repeated field for entries
x/staking/proto/cosmos/staking/v1beta1/genesis.proto (2)
50-59: Well-structured genesis state additions!The new fields in GenesisState are properly defined with:
- Clear field numbering (11, 12, 13)
- Appropriate protobuf options for nullable and empty value handling
- Good documentation comments explaining the purpose of each field
74-84: Well-designed address rotation message!The
RotatedConsensusAddressesmessage is well-structured with:
- Clear field naming (old_address, new_address)
- Proper use of cosmos.AddressString scalar type
- Good documentation for each field
x/staking/CHANGELOG.md (1)
30-31: Clear and well-placed changelog entry!The changelog entry correctly documents the fix for missing data during genesis export/import on key rotation, following the changelog guidelines.
(cherry picked from commit 5359c8f) # Conflicts: # api/cosmos/accounts/defaults/lockup/v1/lockup.pulsar.go # api/cosmos/accounts/v1/tx.pulsar.go # api/cosmos/mint/v1beta1/mint.pulsar.go # api/cosmos/protocolpool/v1/genesis.pulsar.go # api/cosmos/staking/v1beta1/genesis.pulsar.go # runtime/v2/manager.go # scripts/build/simulations.mk # simapp/sim_test_utils.go # simapp/v2/sim_runner.go # simapp/v2/sim_test.go # x/staking/CHANGELOG.md
Co-authored-by: Alexander Peters <[email protected]> Co-authored-by: Julien Robert <[email protected]>
Description
See #23265
Fixes the genesis export/import for staking
OldToNewConsAddrMapandConsAddrToValidatorIdentifierMap.This issue was discovered by the restored sims tests included in this PR.
make test-sim-import-exportNote: I had to add some doc and import cleanup for
make proto-all.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
Release Notes
New Features
Improvements
Bug Fixes
Technical Enhancements