-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(x/staking): Migrate ValidatorQueue to use Collections #17562
Merged
Merged
Changes from 19 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
bc2da6f
wip: migrate ValidatorQueue to collections
likhita-809 51f9286
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 f6af9fc
add changelog
likhita-809 121c27c
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 34838c6
fix iterator and add diff test for migration
likhita-809 3afd3f1
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 b200277
remove ParseValidatorQueueKey
likhita-809 c346236
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 63db269
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 df6d113
use right hash in diff test
likhita-809 eff5ed6
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 a7cda07
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 d9987a2
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 e037806
fix keeper test
likhita-809 d138a0a
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 18ad606
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 3e044c3
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 3f672aa
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 03c0359
add a note on using 3 keys
likhita-809 6e5a7cf
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 5848e81
remove FormatTimeBytes usage and apply frojdi's suggestion
likhita-809 3b7e386
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 26e2976
refactor(staking): cleanup UnbondAllMatureValidators (#17664)
testinginprod d9641a0
address nits
likhita-809 00af397
Merge branch 'likhita/valQueue' of https://github.com/cosmos/cosmos-s…
likhita-809 ff0261e
Merge branch 'main' of https://github.com/cosmos/cosmos-sdk into likh…
likhita-809 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really unfortunate that we prefix time bytes with their length even when the size is constant and well-known. =(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use here if you want k.ValidatorsQueue.KeyCodec().Size(endTime), instead of using sdk.FormatTimeBytes... At least this way we don't need to rely on format time bytes and we keep it in between collections. I would even comment this in the Keeper why we use three keys instead of using simply two.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can use
k.ValidatorQueue.KeyCodec().Size(endTime)
because key here should be of formatcollections.Triple[uint64, time.Time, uint64]
, not just time(endTime). And it gives size of the combined length of fields in key.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, it's a good workaround, unfortunate that we had to do it but it works lol. Once we finish up with all these migrations we'll most definitely do a lot of refactor so we can do proper migrations then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, then @likhita-809 maybe we can use sdk.TimeKey.Size directly, sorry for being pedantic about this, but I'd rather we remove every form of bytes conversion in state from production code, testing is fine to have it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although, time key size is constant so in theory we could use the same number everywhere: https://github.com/cosmos/cosmos-sdk/blob/main/types/collections.go#L210