Refactor childkeys/parentkeys, add migration to clean state#2116
Refactor childkeys/parentkeys, add migration to clean state#2116sam0x17 merged 8 commits intodevnet-readyfrom
Conversation
gztensor
added
the
skip-cargo-audit
| // Migrate subnet burn cost to 2500 | ||
| .saturating_add(migrations::migrate_network_lock_cost_2500::migrate_network_lock_cost_2500::<T>()) | ||
| // Cleanup child/parent keys | ||
| .saturating_add(migrations::migrate_fix_childkeys::migrate_fix_childkeys::<T>()), |
There was a problem hiding this comment.
| .saturating_add(migrations::migrate_fix_childkeys::migrate_fix_childkeys::<T>()), | |
| .saturating_add(migrations::migrate_fix_childkeys::migrate_fix_childkeys::<T>()) |
ales-otf
left a comment
ales-otf
left a comment
There was a problem hiding this comment.
feel free to ignore my comments, they are just hints
| Self::set_parentkeys(pivot.clone(), netuid, new_parents_vec.clone()); | ||
|
|
||
| let prev_parents_set: BTreeSet<T::AccountId> = | ||
| prev_parents_vec.iter().map(|(_, p)| p.clone()).collect(); |
There was a problem hiding this comment.
since prev_parents_vec goes out of scope here, you can probably avoid cloning by using into_iter. but it depends on prev_parents_vec ownership.
|
|
||
| // Updated parents = intersection where proportion changed | ||
| for common in new_parents_set.intersection(&prev_parents_set) { | ||
| let new_p = match new_parents_map.get(common) { |
There was a problem hiding this comment.
just a hint. this can be probably replaced by a one-liner using ok_or method. something like: new_parents_map.get(common).ok_or(Error::<T>::ChildParentInconsistency.into())?. you just need to figure out dereferencing of the inner value
| //////////////////////////////////////////////////////// | ||
| // Actual migration | ||
|
|
||
| Pallet::<T>::clean_zero_childkey_vectors(&mut weight); |
There was a problem hiding this comment.
Did you consider iterating through the child and parent key maps once and passing collected values into the substeps?
There was a problem hiding this comment.
Yes, but after clean_zero_childkey_vectors the number of keys in both maps is significantly reduced, so it is not worth the added complexity. Also, we've done heavier migrations in the past, so this one will pass.
|
@basfroman |
6 participants
Description
Refactor childkeys and parentkeys.
Type of Change
Checklist
cargo fmtandcargo clippyto ensure my code is formatted and linted correctly