refactor(staking)!: remove historical info #20845
Conversation
|
Warning Rate limit exceeded@tac0turtle has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 22 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes involve significant updates to the Changes
Sequence Diagram(s)Silently ignored as the changes are too varied and there is no singular new feature or control flow modification that necessitates a sequence diagram. 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 Configration File (
|
![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 (9)
- x/staking/CHANGELOG.md (3 hunks)
- x/staking/keeper/abci.go (1 hunks)
- x/staking/keeper/grpc_query.go (1 hunks)
- x/staking/keeper/keeper.go (3 hunks)
- x/staking/migrations/v6/keys.go (1 hunks)
- x/staking/migrations/v6/store.go (1 hunks)
- x/staking/module.go (2 hunks)
- x/staking/proto/cosmos/staking/v1beta1/query.proto (2 hunks)
- x/staking/types/keys.go (1 hunks)
Files skipped from review due to trivial changes (3)
- x/staking/keeper/keeper.go
- x/staking/migrations/v6/keys.go
- x/staking/types/keys.go
Additional context used
Path-based instructions (5)
x/staking/migrations/v6/store.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/keeper/abci.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/module.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/keeper/grpc_query.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Markdownlint
x/staking/CHANGELOG.md
46-46: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
90-90: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
92-92: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
95-95: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
98-98: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
97-97: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
LanguageTool
x/staking/CHANGELOG.md
[style] ~96-~96: Consider a shorter alternative to avoid wordiness.
Context: ...pi/cosmos/staking/v1beta1".Infraction_` in order to remove dependency between modules on st...(IN_ORDER_TO_PREMIUM)
[misspelling] ~102-~102: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ub.com//pull/18841) In a undelegation or redelegation if the sha...(EN_A_VS_AN)
Additional comments not posted (13)
x/staking/migrations/v6/store.go (1)
15-15: Ensure correct deletion of HistoricalInfoKey.The code correctly deletes
HistoricalInfoKey. Ensure that this key is no longer used elsewhere in the codebase.x/staking/keeper/abci.go (1)
9-13: LGTM! Ensure performance is monitored.The
EndBlockerfunction correctly updates the validator set and measures execution time using telemetry.x/staking/module.go (1)
Line range hint
78-78: Ensure proper handling of migrations.The
RegisterMigrationsmethod correctly registers migrations up to version 6. Ensure that the removal ofHistoricalInfois handled in the migrations.x/staking/CHANGELOG.md (6)
46-46: Remove consecutive blank lines.There are multiple consecutive blank lines that should be reduced to one.
-Tools
Markdownlint
46-46: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
90-98: Fix unordered list indentation.The unordered list items should have consistent indentation.
- * remove from `Keeper`: `GetParams`, `SetParams` + * remove from `Keeper`: `GetParams`, `SetParams` - * remove from `types`: `GetRedelegationTimeKey` + * remove from `types`: `GetRedelegationTimeKey` - * remove from `Keeper`: `RedelegationQueueIterator` + * remove from `Keeper`: `RedelegationQueueIterator` - * remove from `types`: `GetLastValidatorPowerKey` + * remove from `types`: `GetLastValidatorPowerKey` - * remove from `Keeper`: `LastValidatorsIterator`, `IterateLastValidators` + * remove from `Keeper`: `LastValidatorsIterator`, `IterateLastValidators`Tools
LanguageTool
[style] ~96-~96: Consider a shorter alternative to avoid wordiness.
Context: ...pi/cosmos/staking/v1beta1".Infraction_` in order to remove dependency between modules on st...(IN_ORDER_TO_PREMIUM)
Markdownlint
90-90: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
92-92: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
95-95: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
98-98: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
97-97: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
97-97: Remove trailing spaces.There is a trailing space that should be removed.
-Tools
Markdownlint
97-97: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
96-96: Consider a shorter alternative to avoid wordiness.The phrase "in order to" can be shortened to "to".
- in order to + toTools
LanguageTool
[style] ~96-~96: Consider a shorter alternative to avoid wordiness.
Context: ...pi/cosmos/staking/v1beta1".Infraction_` in order to remove dependency between modules on st...(IN_ORDER_TO_PREMIUM)
102-102: Use "an" instead of "a".Use “an” instead of ‘a’ if the following word starts with a vowel sound.
- In a undelegation or redelegation + In an undelegation or redelegationTools
LanguageTool
[misspelling] ~102-~102: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ub.com//pull/18841) In a undelegation or redelegation if the sha...(EN_A_VS_AN)
105-105: Accurate changelog entry for removal of HistoricalInfo.The changelog entry correctly documents the removal of
HistoricalInfofrom the staking module.x/staking/proto/cosmos/staking/v1beta1/query.proto (3)
116-116: Deprecation Notice: HistoricalInfo RPC method.The
HistoricalInfoRPC method is now deprecated. Ensure that all dependent services and clients are updated to handle this deprecation.
371-371: Deprecation Notice: QueryHistoricalInfoRequest message.The
QueryHistoricalInfoRequestmessage is now deprecated. Ensure that all dependent services and clients are updated to handle this deprecation.
379-379: Deprecation Notice: QueryHistoricalInfoResponse message.The
QueryHistoricalInfoResponsemessage is now deprecated. Ensure that all dependent services and clients are updated to handle this deprecation.x/staking/keeper/grpc_query.go (1)
412-412: Deprecation Notice: HistoricalInfo endpoint.The
HistoricalInfoendpoint has been deprecated and removed in version 0.52. Ensure that all dependent services and clients are updated to handle this deprecation.
There was a problem hiding this comment.
Do we need to regen swagger.yaml for deprecation notices?
I think you can also get rid of ErrInvalidHistoricalInfo in staking errors.
There's also a staking param DefaultHistoricalEntries, and some documentation in the staking readme that could be flagged for removal! 🙏🏻 🚀
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range and nitpick comments (11)
api/cosmos/staking/v1beta1/query.pulsar.go (11)
12228-12235: Mark the initialization of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this initialization should also be marked as deprecated.// Deprecated: Do not use. var ( md_QueryHistoricalInfoResponse protoreflect.MessageDescriptor fd_QueryHistoricalInfoResponse_hist protoreflect.FieldDescriptor )
Line range hint
12240-12245: Mark the check of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this check should also be marked as deprecated.// Deprecated: Do not use. switch fd.FullName() { case "cosmos.staking.v1beta1.QueryHistoricalInfoResponse.hist": return x.Hist != nil default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.staking.v1beta1.QueryHistoricalInfoResponse")) } }
Line range hint
12246-12251: Mark the nil assignment of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this operation should also be marked as deprecated.// Deprecated: Do not use. switch fd.FullName() { case "cosmos.staking.v1beta1.QueryHistoricalInfoResponse.hist": x.Hist = nil default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.staking.v1beta1.QueryHistoricalInfoResponse")) } }
Line range hint
12252-12258: Mark the retrieval of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this retrieval should also be marked as deprecated.// Deprecated: Do not use. case "cosmos.staking.v1beta1.QueryHistoricalInfoResponse.hist": value := x.Hist return protoreflect.ValueOfMessage(value.ProtoReflect()) default: if descriptor.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.staking.v1beta1.QueryHistoricalInfoResponse")) }
Line range hint
12259-12265: Mark the value assignment of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this operation should also be marked as deprecated.// Deprecated: Do not use. switch fd.FullName() { case "cosmos.staking.v1beta1.QueryHistoricalInfoResponse.hist": x.Hist = value.Message().Interface().(*HistoricalInfo) default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.staking.v1beta1.QueryHistoricalInfoResponse")) } }
Line range hint
12266-12272: Mark the new instance creation of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this initialization should also be marked as deprecated.// Deprecated: Do not use. x.Hist = new(HistoricalInfo) return protoreflect.ValueOfMessage(x.Hist.ProtoReflect())
Line range hint
12273-12278: Mark the new instance creation of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this initialization should also be marked as deprecated.// Deprecated: Do not use. case "cosmos.staking.v1beta1.QueryHistoricalInfoResponse.hist": m := new(HistoricalInfo) return protoreflect.ValueOfMessage(m.ProtoReflect()) default: if fd.IsExtension() { panic(fmt.Errorf("proto3 declared messages do not support extensions: cosmos.staking.v1beta1.QueryHistoricalInfoResponse")) }
Line range hint
12279-12283: Mark the size calculation of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this calculation should also be marked as deprecated.// Deprecated: Do not use. l = options.Size(x.Hist) n += 1 + l + runtime.Sov(uint64(l))
Line range hint
12284-12289: Mark the marshaling of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this operation should also be marked as deprecated.// Deprecated: Do not use. if x.Hist != nil { encoded, err := options.Marshal(x.Hist) if err != nil { return protoiface.MarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, err } }
Line range hint
12290-12295: Mark the unmarshaling of deprecated fields as deprecated.Since
QueryHistoricalInfoResponseis deprecated, this operation should also be marked as deprecated.// Deprecated: Do not use. switch fieldNum { case 1: if wireType != 2 { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, fmt.Errorf("proto: wrong wireType = %d for field Hist", wireType) } var msglen int for shift := uint(0); ; shift += 7 { if shift >= 64 { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, fmt.Errorf("proto: QueryHistoricalInfoResponse: wiretype end group for non-group") } if iNdEx >= l { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, io.ErrUnexpectedEOF } b := dAtA[iNdEx] iNdEx++ msglen |= int(b&0x7F) << shift if b < 0x80 { break } } if msglen < 0 { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, fmt.Errorf("proto: negative length found during unmarshaling") } postIndex := iNdEx + msglen if postIndex < 0 { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, fmt.Errorf("proto: negative length found during unmarshaling") } if postIndex > l { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, io.ErrUnexpectedEOF } if x.Hist == nil { x.Hist = new(HistoricalInfo) } if err := options.Unmarshal(dAtA[iNdEx:postIndex], x.Hist); err != nil { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, err } iNdEx = postIndex default: iNdEx = preIndex skippy, err := runtime.Skip(dAtA[iNdEx:]) if err != nil { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, err } if (skippy < 0) || (iNdEx+skippy) < 0 { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, fmt.Errorf("proto: negative length found during unmarshaling") } if (iNdEx + skippy) > l { return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, io.ErrUnexpectedEOF } iNdEx += skippy }
16061-16062: Mark the definition of deprecated fields as deprecated.Since
HistoricalInfois deprecated, this definition should also be marked as deprecated.// Deprecated: Do not use. (*HistoricalInfo)(nil), // 35: cosmos.staking.v1beta1.HistoricalInfo
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (3)
api/cosmos/staking/v1beta1/query_grpc.pb.gois excluded by!**/*.pb.gox/staking/types/query.pb.gois excluded by!**/*.pb.gox/staking/types/staking.pb.gois excluded by!**/*.pb.go
Files selected for processing (9)
- api/cosmos/staking/v1beta1/query.pulsar.go (22 hunks)
- client/docs/swagger-ui/swagger.yaml (25 hunks)
- x/staking/README.md (5 hunks)
- x/staking/proto/cosmos/staking/v1beta1/query.proto (3 hunks)
- x/staking/proto/cosmos/staking/v1beta1/staking.proto (4 hunks)
- x/staking/simulation/genesis.go (1 hunks)
- x/staking/simulation/genesis_test.go (1 hunks)
- x/staking/types/errors.go (1 hunks)
- x/staking/types/params.go (3 hunks)
Files not summarized due to errors (1)
- api/cosmos/staking/v1beta1/query.pulsar.go: Error: Message exceeds token limit
Files skipped from review due to trivial changes (3)
- x/staking/simulation/genesis_test.go
- x/staking/types/errors.go
- x/staking/types/params.go
Files skipped from review as they are similar to previous changes (1)
- x/staking/proto/cosmos/staking/v1beta1/query.proto
Additional context used
Path-based instructions (3)
x/staking/simulation/genesis.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.x/staking/README.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"api/cosmos/staking/v1beta1/query.pulsar.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Markdownlint
x/staking/README.md
30-30: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
31-31: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
32-32: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
33-33: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
35-35: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
363-363: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
364-364: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
365-365: Expected: 1; Actual: 4
Multiple consecutive blank lines(MD012, no-multiple-blanks)
786-786: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
Additional comments not posted (14)
x/staking/simulation/genesis.go (1)
67-67: LGTM! The removal ofhistoricalEntriesparameter is consistent with the PR objectives.The change aligns with the removal of historical information as described in the PR summary.
x/staking/proto/cosmos/staking/v1beta1/staking.proto (3)
224-224: LGTM! The comment added for theentriesfield improves clarity.The change enhances documentation for the
UnbondingDelegationmessage.
297-297: LGTM! The comment added for theentriesfield improves clarity.The change enhances documentation for the
Redelegationmessage.
313-313: LGTM! The deprecation of thehistorical_entriesfield is consistent with the PR objectives.The change aligns with the removal of historical information as described in the PR summary.
api/cosmos/staking/v1beta1/query.pulsar.go (10)
Line range hint
12236-12239: LGTM!The code changes are approved.
Line range hint
12296-12298: LGTM!The code changes are approved.
Line range hint
14303-14350: LGTM!The code changes are approved.
Line range hint
15303-15308: LGTM!The code changes are approved.
Line range hint
15343-15348: LGTM!The code changes are approved.
Line range hint
15353-15357: LGTM!The code changes are approved.
Line range hint
15358-15362: LGTM!The code changes are approved.
15791-15801: LGTM!The code changes are approved.
Line range hint
15802-15812: LGTM!The code changes are approved.
15964-16008: LGTM!The code changes are approved.
|
|
||
| HistoricalInfo objects are stored and pruned at each block such that the staking keeper persists | ||
| the `n` most recent historical info defined by staking module parameter: `HistoricalEntries`. | ||
|
|
There was a problem hiding this comment.
Remove extra blank lines.
Multiple consecutive blank lines detected at line 364. Expected: 1 blank line; Actual: 3 blank lines.
-
+ Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
Markdownlint
364-364: Expected: 1; Actual: 3
Multiple consecutive blank lines(MD012, no-multiple-blanks)
| the current block in a `HistoricalInfo` object. The Validators are sorted on their address to ensure that | ||
| they are in a deterministic order. | ||
| The oldest HistoricalEntries will be pruned to ensure that there only exist the parameter-defined number of | ||
| historical entries. | ||
|
|
There was a problem hiding this comment.
Remove extra blank lines.
Multiple consecutive blank lines detected at line 365. Expected: 1 blank line; Actual: 4 blank lines.
-
+ Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
Markdownlint
365-365: Expected: 1; Actual: 4
Multiple consecutive blank lines(MD012, no-multiple-blanks)
|
|
||
| Otherwise, the latest historical info is stored under the key `historicalInfoKey|height`, while any entries older than `height - HistoricalEntries` is deleted. | ||
| In most cases, this results in a single entry being pruned per block. | ||
| However, if the parameter `HistoricalEntries` has changed to a lower value there will be multiple entries in the store that must be pruned. | ||
|
|
There was a problem hiding this comment.
Remove extra blank line.
Multiple consecutive blank lines detected at line 786. Expected: 1 blank line; Actual: 2 blank lines.
-
+ Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
Markdownlint
786-786: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
| @@ -361,20 +360,8 @@ | |||
| https://github.com/cosmos/cosmos-sdk/blob/8f0d5b15f0b10da7645d7fc1aa868fe44e3f3a44/proto/cosmos/staking/v1beta1/staking.proto#L429-L433 | |||
| ``` | |||
|
|
|||
| ### HistoricalInfo | |||
|
|
|||
There was a problem hiding this comment.
Remove extra blank line.
Multiple consecutive blank lines detected at line 363. Expected: 1 blank line; Actual: 2 blank lines.
-
+ Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Tools
Markdownlint
363-363: Expected: 1; Actual: 2
Multiple consecutive blank lines(MD012, no-multiple-blanks)
| @@ -30,7 +30,6 @@ | |||
| * [UnbondingDelegation](#unbondingdelegation) | |||
| * [Redelegation](#redelegation) | |||
| * [Queues](#queues) | |||
There was a problem hiding this comment.
Fix unordered list indentation.
The unordered list item at line 32 has incorrect indentation. Expected: 2 spaces; Actual: 4 spaces.
- * [UnbondingID](#unbondingid)
+ * [UnbondingID](#unbondingid)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * [Queues](#queues) | |
| * [Queues](#queues) |
Tools
Markdownlint
32-32: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
| @@ -30,7 +30,6 @@ | |||
| * [UnbondingDelegation](#unbondingdelegation) | |||
| * [Redelegation](#redelegation) | |||
There was a problem hiding this comment.
Fix unordered list indentation.
The unordered list item at line 31 has incorrect indentation. Expected: 2 spaces; Actual: 4 spaces.
- * [LastTotalPower](#lasttotalpower)
+ * [LastTotalPower](#lasttotalpower)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * [Redelegation](#redelegation) | |
| * [Redelegation](#redelegation) |
Tools
Markdownlint
31-31: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
| @@ -30,7 +30,6 @@ | |||
| * [UnbondingDelegation](#unbondingdelegation) | |||
There was a problem hiding this comment.
Fix unordered list indentation.
The unordered list item at line 30 has incorrect indentation. Expected: 2 spaces; Actual: 4 spaces.
- * [Pool](#pool)
+ * [Pool](#pool)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * [UnbondingDelegation](#unbondingdelegation) | |
| * [UnbondingDelegation](#unbondingdelegation) |
Tools
Markdownlint
30-30: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
| @@ -30,7 +30,6 @@ | |||
| * [UnbondingDelegation](#unbondingdelegation) | |||
| * [Redelegation](#redelegation) | |||
| * [Queues](#queues) | |||
| * [HistoricalInfo](#historicalinfo) | |||
| * [ConsPubkeyRotation](#conspubkeyrotation) | |||
There was a problem hiding this comment.
Fix unordered list indentation.
The unordered list item at line 33 has incorrect indentation. Expected: 2 spaces; Actual: 4 spaces.
- * [Params](#params)
+ * [Params](#params)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * [ConsPubkeyRotation](#conspubkeyrotation) | |
| * [ConsPubkeyRotation](#conspubkeyrotation) |
Tools
Markdownlint
33-33: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
| @@ -30,7 +30,6 @@ | |||
| * [UnbondingDelegation](#unbondingdelegation) | |||
| * [Redelegation](#redelegation) | |||
| * [Queues](#queues) | |||
| * [HistoricalInfo](#historicalinfo) | |||
| * [ConsPubkeyRotation](#conspubkeyrotation) | |||
| * [State Transitions](#state-transitions) | |||
| * [Validators](#validators) | |||
There was a problem hiding this comment.
Fix unordered list indentation.
The unordered list item at line 35 has incorrect indentation. Expected: 2 spaces; Actual: 4 spaces.
- * [ConsPubkeyRotation](#conspubkeyrotation)
+ * [ConsPubkeyRotation](#conspubkeyrotation)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * [Validators](#validators) | |
| * [Validators](#validators) |
Tools
Markdownlint
35-35: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
tests/integration/staking/keeper/grpc_query_test.go (1)
Line range hint
1-1: Remove unused import.The import
github.com/cometbft/cometbft/api/cometbft/types/v1is no longer used and can be removed.- import cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"tests/integration/staking/keeper/deterministic_test.go (1)
Line range hint
1-1: Remove unused import.The import
github.com/cometbft/cometbft/api/cometbft/types/v1is no longer used and can be removed.- import cmtproto "github.com/cometbft/cometbft/api/cometbft/types/v1"
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- tests/integration/staking/keeper/deterministic_test.go (1 hunks)
- tests/integration/staking/keeper/grpc_query_test.go (3 hunks)
Additional context used
Path-based instructions (2)
tests/integration/staking/keeper/grpc_query_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"tests/integration/staking/keeper/deterministic_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
Additional comments not posted (2)
tests/integration/staking/keeper/grpc_query_test.go (1)
Line range hint
1-1: Confirm removal of historical info tests.The removal of historical info tests is consistent with the PR objective.
tests/integration/staking/keeper/deterministic_test.go (1)
Line range hint
1-1: Confirm removal of historical info tests.The removal of historical info tests is consistent with the PR objective.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (4)
- simapp/sim_test.go (1 hunks)
- tests/integration/staking/keeper/deterministic_test.go (5 hunks)
- tests/integration/staking/keeper/grpc_query_test.go (3 hunks)
- x/staking/keeper/keeper_test.go (16 hunks)
Files skipped from review due to trivial changes (1)
- x/staking/keeper/keeper_test.go
Files skipped from review as they are similar to previous changes (1)
- tests/integration/staking/keeper/deterministic_test.go
Additional context used
Path-based instructions (2)
simapp/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"tests/integration/staking/keeper/grpc_query_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
Additional comments not posted (2)
simapp/sim_test.go (1)
97-98: Verify the relevance of the added prefixes.The added prefixes for
stakingtypesshould be relevant to the removal of historical info. Ensure that these changes align with the overall PR objective.tests/integration/staking/keeper/grpc_query_test.go (1)
724-725: Verify the accuracy of the error message.The error message should accurately reflect the deprecation and removal of the historical info endpoint. Ensure that this message is consistent with the expected behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- tests/integration/staking/keeper/grpc_query_test.go (3 hunks)
- x/staking/keeper/grpc_query.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/staking/keeper/grpc_query.go
Additional context used
Path-based instructions (1)
tests/integration/staking/keeper/grpc_query_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
Additional comments not posted (10)
tests/integration/staking/keeper/grpc_query_test.go (10)
Line range hint
1-1: LGTM!The
TestGRPCQueryValidatorsfunction is well-structured and follows best practices for table-driven tests.Also applies to: 9-9
Line range hint
1-1: LGTM!The
TestGRPCQueryDelegatorValidatorsfunction is well-structured and follows best practices for table-driven tests.Also applies to: 48-48
Line range hint
1-1: LGTM!The
TestGRPCQueryDelegatorValidatorfunction is well-structured and follows best practices for table-driven tests.Also applies to: 87-87
Line range hint
1-1: LGTM!The
TestGRPCQueryDelegationfunction is well-structured and follows best practices for table-driven tests.Also applies to: 126-126
Line range hint
1-1: LGTM!The
TestGRPCQueryDelegatorDelegationsfunction is well-structured and follows best practices for table-driven tests.Also applies to: 165-165
Line range hint
1-1: LGTM!The
TestGRPCQueryValidatorDelegationsfunction is well-structured and follows best practices for table-driven tests.Also applies to: 204-204
Line range hint
1-1: LGTM!The
TestGRPCQueryUnbondingDelegationfunction is well-structured and follows best practices for table-driven tests.Also applies to: 243-243
Line range hint
1-1: LGTM!The
TestGRPCQueryDelegatorUnbondingDelegationsfunction is well-structured and follows best practices for table-driven tests.Also applies to: 282-282
Line range hint
1-1: LGTM!The
TestGRPCQueryPoolParametersfunction is well-structured and follows best practices.Also applies to: 321-321
Line range hint
1-1: LGTM!The
TestGRPCQueryHistoricalInfofunction correctly verifies that the deprecated endpoint returns an error.Also applies to: 724-725
| // hist defines the historical info at the given height. | ||
| HistoricalInfo hist = 1 [deprecated = true]; | ||
| HistoricalRecord historical_record = 2; |
x/staking/types/params.go
Outdated
| @@ -28,6 +28,7 @@ const ( | |||
| // DefaultHistorical entries is 10000. Apps that don't use IBC can ignore this | |||
| // value by not adding the staking module to the application module manager's | |||
| // SetOrderBeginBlockers. | |||
| // Deprecated: HistoricalEntries is deprecated | |||
| DefaultHistoricalEntries uint32 = 10000 | |||
There was a problem hiding this comment.
Why keep this? given that we are already API breaking with the NewParams arg changes
x/staking/types/params.go
Outdated
| @@ -28,6 +28,7 @@ const ( | |||
| // DefaultHistorical entries is 10000. Apps that don't use IBC can ignore this | |||
| // value by not adding the staking module to the application module manager's | |||
| // SetOrderBeginBlockers. | |||
| // Deprecated: HistoricalEntries is deprecated | |||
| DefaultHistoricalEntries uint32 = 10000 | |||
There was a problem hiding this comment.
Why keep this? given that we are already API breaking with the NewParams arg changes
Description
remove historicalinfo from staking
this was discussed with the ibc team and will be supported in v10 which will coincide with v0.52
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
Bug Fixes
GetLastValidatorsto handle cases whereMaxValidatorsexceeds bonded validators without errors.CreateValidatorto prevent panics by validating pubkey length.API Changes
HistoricalInfoendpoint in gRPC queries.Documentation
Anytype in documentation, providing detailed examples and JSON representations.HistoricalInfocontent from README and documentation files.Refactor
HistoricalInfoand related methods and storage from the staking module.Tests
Chores
HistoricalInfo.