fix: sort on address modification #32
Merged
Conversation
This file contains hidden or 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
alstjd0921
reviewed
Dec 13, 2023
Comment on lines
+555
to
569
| /// The active validator set (full and basic) selected for the current round. This storage is sorted by address. | ||
| pub type SelectedCandidates<T: Config> = | ||
| StorageValue<_, BoundedVec<T::AccountId, ConstU32<MAX_AUTHORITIES>>, ValueQuery>; | ||
|
|
||
| #[pallet::storage] | ||
| #[pallet::getter(fn selected_full_candidates)] | ||
| /// The active full validator set selected for the current round | ||
| /// The active full validator set selected for the current round. This storage is sorted by address. | ||
| pub type SelectedFullCandidates<T: Config> = | ||
| StorageValue<_, BoundedVec<T::AccountId, ConstU32<MAX_AUTHORITIES>>, ValueQuery>; | ||
|
|
||
| #[pallet::storage] | ||
| #[pallet::getter(fn selected_basic_candidates)] | ||
| /// The active basic validator set selected for the current round | ||
| /// The active basic validator set selected for the current round. This storage is sorted by address. | ||
| pub type SelectedBasicCandidates<T: Config> = | ||
| StorageValue<_, BoundedVec<T::AccountId, ConstU32<MAX_AUTHORITIES>>, ValueQuery>; |
Member
There was a problem hiding this comment.
BoundedVec -> BoundedBTreeSet으로 변경을 고려해봐도 좋을 것 같습니다
Collaborator
Author
There was a problem hiding this comment.
적용을 하려고 했는데, 아무래도 타입이 기존 벡터와 다르다보니�사용하는 곳에서 로직 수정이 많이 요구가 되긴 하네요. 다른 스토리지들과 호환이 안되는 부분도 있기도 하고요, 이를테면 CachedSelectedCandidates 처럼요.
그래서 우선은 BoundedVec으로 유지하고, 이후에 저희 스토리지 값들 전부 migration하는 것은 어떨까요? CachedSelectedCandidates 이런것들도 전부 BTreeMap 같은것으로 수정해야 될것 같아서요.
Collaborator
Author
There was a problem hiding this comment.
살짝 대규모 작업이 될수도 있을것 같긴 하네요
Member
There was a problem hiding this comment.
네 당장 바꾸자는 뜻은 아니었습니다. 지금 보니 더 나은 collection들을 쓸 여지가 보여서 의견남겨봤습니다.
alstjd0921
approved these changes
Dec 13, 2023
dnjscksdn98
added a commit
that referenced
this pull request
Dec 19, 2023
* sc-executor: Increase maximum instance count (polkadot-sdk#1856) * remove `pallet::without_storage_info` macros * Update Cargo.toml, Cargo.lock (#31) * fix: sort on address modification (#32) * fix: sort vector on address modification * chore: increase testnet runtime version * test: update package.json version --------- Co-authored-by: alstjd0921 <kwonarseus@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Type of change
Checklist
cargo-clippyand linted my code.