feat: making code more robust, fixing incentives #44
Conversation
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
saiintbrisson
requested changes
Mar 31, 2024
pallets/subspace/src/subnet.rs
Outdated
| @@ -10,15 +10,71 @@ use sp_std::vec::Vec; | |||
| use substrate_fixed::types::I64F64; | |||
| extern crate alloc; | |||
|
|
|||
| #[derive(Debug)] | |||
| pub struct SubnetSet<T: Config> { | |||
There was a problem hiding this comment.
Suggested change
| pub struct SubnetSet<T: Config> { | |
| pub struct SubnetChangeset<T: Config> { |
pallets/subspace/src/subnet.rs
Outdated
|
|
||
| impl<T: Config> SubnetSet<T> { | ||
| #[must_use] | ||
| pub fn new(name: &Vec<u8>, founder_key: &T::AccountId, params: SubnetParams<T>) -> Self { |
There was a problem hiding this comment.
Suggested change
| pub fn new(name: &Vec<u8>, founder_key: &T::AccountId, params: SubnetParams<T>) -> Self { | |
| pub fn new(name: &[u8], founder_key: &T::AccountId, params: SubnetParams<T>) -> Self { |
pallets/subspace/src/subnet.rs
Outdated
| // check for name validity | ||
| if let Some(name) = self.name { | ||
| ensure!( | ||
| !Pallet::<T>::if_subnet_name_exists(&name), |
There was a problem hiding this comment.
As for the module, this must only be checked if the subnet name is different from its current name. Check ModuleChangeset::new to see how it's done.
pallets/subspace/src/subnet.rs
Outdated
Comment on lines
57
to
60
| ensure!( | ||
| Pallet::<T>::if_subnet_netuid_exists(netuid), | ||
| Error::<T>::NetuidDoesNotExist | ||
| ); |
There was a problem hiding this comment.
Doing this will make this changeset only work when updating a subnet, and not registering one, meaning the same validations will have to be re-done elsewhere, which fragments validation logic.
aripiprazole
reviewed
Apr 1, 2024
pallets/subspace/src/step.rs
Outdated
| registrations_this_interval, | ||
| target_registrations_per_interval, | ||
| )); | ||
| Self::set_burn( |
There was a problem hiding this comment.
I would split on a variable just to make easier to read
aripiprazole
approved these changes
Apr 1, 2024
|
d03761b |
saiintbrisson
approved these changes
Apr 2, 2024
saiintbrisson
assigned functor-flow and unassigned saiintbrisson, aripiprazole and atomoxetine
functor-flow
added a commit
that referenced
this pull request
Jan 16, 2025
* feat: local subnet burn ! missing migrations * fixing potential problems after reaching global module limit * fix: ensuring subnet naming safety * feat: adding migrations + fixing subnet burn * applying PR suggestions * fix: clippy errors
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.
Changes
I honestly think that we should change the
subnesettosubnetchangesetand also add the update for it, what do you think @saiintbrisson ?