Skip to content
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

Check in static analysis for removals from Keys map #915

Closed
gztensor opened this issue Nov 5, 2024 · 5 comments
Closed

Check in static analysis for removals from Keys map #915

gztensor opened this issue Nov 5, 2024 · 5 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@gztensor
Copy link
Contributor

gztensor commented Nov 5, 2024

Check in static analysis that we never remove from Keys unless we have matching insert at the same function that inserts with the same map key like in replace_neuron:

// Remove uid_to_replace
Keys::<T>::remove(netuid, uid_to_replace);
...
// Insert with the same key: uid_to_replace
Keys::<T>::insert(netuid, uid_to_replace, new_hotkey.clone());

If we remove from Keys, epoch function is going to break because it relies on neuron IDs being consecutive.

Note: Currently we don't have such removals anywhere but replace_neuron function, so we can limit such replacements that use Keys::<T>::remove to only be present in replace_neuron.

@sam0x17 sam0x17 added the good first issue Good for newcomers label Nov 6, 2024
@sam0x17
Copy link
Contributor

sam0x17 commented Nov 6, 2024

@eagr want this one?

@eagr
Copy link
Contributor

eagr commented Nov 6, 2024

  • + custom lint to forbid Keys::<T>::remove
  • disable lint for occurrences of Keys::<T>::remove in replace_neuron

does that sound right? and which is the target branch for dev now?

@eagr eagr mentioned this issue Nov 6, 2024
5 tasks
@sam0x17
Copy link
Contributor

sam0x17 commented Nov 6, 2024

that sounds right to me (cc @gregzaitsev), and yes it's devnet-ready now 👍🏻

@eagr
Copy link
Contributor

eagr commented Nov 11, 2024

anything else? @sam0x17 maybe something slightly more involved that could help me learn more about the project? :)

@sam0x17
Copy link
Contributor

sam0x17 commented Nov 22, 2024

anything else? @sam0x17 maybe something slightly more involved that could help me learn more about the project? :)

if you @ me on the OTF discord would love to talk further :)

same as my gh username

@sam0x17 sam0x17 closed this as completed Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants