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

reset neuron data on dereg #755

Open
wants to merge 1 commit into
base: devnet-ready
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions pallets/subtensor/src/subnets/uids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,23 @@ impl<T: Config> Pallet<T> {
SubnetworkN::<T>::get(netuid)
}

/// Sets value for the element at the given position if it exists.
pub fn set_element_at<N>(vec: &mut [N], position: usize, value: N) {
if let Some(element) = vec.get_mut(position) {
*element = value;
}
}

/// Resets the trust, emission, consensus, incentive, dividends of the neuron to default
pub fn clear_neuron(netuid: u16, neuron_uid: u16) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we optimise this by reducing the number of storage / writes , and number of function calls ?

pub fn clear_neuron(netuid: u16, neuron_uid: u16) {
    let neuron_index: usize = neuron_uid.into();
    let default_value = 0;

    for storage in &mut [
        &mut Emission::<T>::get(netuid),
        &mut Trust::<T>::get(netuid),
        &mut Consensus::<T>::get(netuid),
        &mut Incentive::<T>::get(netuid),
        &mut Dividends::<T>::get(netuid),
    ] {
        set_element_at(storage, neuron_index, default_value);
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that does not quite work as the vectors have different int types

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's the same number of calls to the set_element_at. is there much difference between get vs mutate calls? aren't they compiled to similar things?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyway made the change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted - that does not work

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not just clear the netuid prefix? Or is that the same runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does not modify the vector - &mut Emission::::get(netuid) just gives back the value & StorageMap does not have a way to get a mut ref to the value

let neuron_index: usize = neuron_uid.into();
Emission::<T>::mutate(netuid, |v| Self::set_element_at(v, neuron_index, 0));
Trust::<T>::mutate(netuid, |v| Self::set_element_at(v, neuron_index, 0));
Consensus::<T>::mutate(netuid, |v| Self::set_element_at(v, neuron_index, 0));
Incentive::<T>::mutate(netuid, |v| Self::set_element_at(v, neuron_index, 0));
Dividends::<T>::mutate(netuid, |v| Self::set_element_at(v, neuron_index, 0));
}

/// Replace the neuron under this uid.
pub fn replace_neuron(
netuid: u16,
Expand Down Expand Up @@ -48,6 +65,12 @@ impl<T: Config> Pallet<T> {

// 4. Clear neuron certificates
NeuronCertificates::<T>::remove(netuid, old_hotkey.clone());

// 5. Reset new neuron's values.
Self::clear_neuron(netuid, uid_to_replace);

// 5a. reset axon info for the new uid.
Axons::<T>::remove(netuid, old_hotkey);
}

/// Appends the uid to the network.
Expand Down
74 changes: 67 additions & 7 deletions pallets/subtensor/tests/uids.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,25 +52,58 @@ fn test_replace_neuron() {
// Get UID
let neuron_uid = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey_account_id);
assert_ok!(neuron_uid);
let neuron_uid = neuron_uid.unwrap();

// set non-default values
Trust::<Test>::mutate(netuid, |v| {
SubtensorModule::set_element_at(v, neuron_uid as usize, 5u16)
});
Emission::<Test>::mutate(netuid, |v| {
SubtensorModule::set_element_at(v, neuron_uid as usize, 5u64)
});
Consensus::<Test>::mutate(netuid, |v| {
SubtensorModule::set_element_at(v, neuron_uid as usize, 5u16)
});
Incentive::<Test>::mutate(netuid, |v| {
SubtensorModule::set_element_at(v, neuron_uid as usize, 5u16)
});
Dividends::<Test>::mutate(netuid, |v| {
SubtensorModule::set_element_at(v, neuron_uid as usize, 5u16)
});

// serve axon mock address
let ip: u128 = 1676056785;
let port: u16 = 9999;
let ip_type: u8 = 4;
let hotkey = SubtensorModule::get_hotkey_for_net_and_uid(netuid, neuron_uid).unwrap();
assert!(SubtensorModule::serve_axon(
<<Test as Config>::RuntimeOrigin>::signed(hotkey_account_id),
netuid,
0,
ip,
port,
ip_type,
0,
0,
0
)
.is_ok());

// Set a neuron certificate for it
NeuronCertificates::<Test>::insert(netuid, hotkey_account_id, certificate);

// Replace the neuron.
SubtensorModule::replace_neuron(
netuid,
neuron_uid.unwrap(),
&new_hotkey_account_id,
block_number,
);
SubtensorModule::replace_neuron(netuid, neuron_uid, &new_hotkey_account_id, block_number);

assert!(!SubtensorModule::has_axon_info(netuid, &hotkey));

// Check old hotkey is not registered on any network.
assert!(SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey_account_id).is_err());
assert!(!SubtensorModule::is_hotkey_registered_on_any_network(
&hotkey_account_id
));

let curr_hotkey = SubtensorModule::get_hotkey_for_net_and_uid(netuid, neuron_uid.unwrap());
let curr_hotkey = SubtensorModule::get_hotkey_for_net_and_uid(netuid, neuron_uid);
assert_ok!(curr_hotkey);
assert_ne!(curr_hotkey.unwrap(), hotkey_account_id);

Expand All @@ -86,6 +119,33 @@ fn test_replace_neuron() {
// Check neuron certificate was reset
let certificate = NeuronCertificates::<Test>::get(netuid, hotkey_account_id);
assert_eq!(certificate, None);

// Check trust, emission, consensus, incentive, dividends have been reset to 0.
assert_eq!(SubtensorModule::get_trust_for_uid(netuid, neuron_uid), 0);
assert_eq!(SubtensorModule::get_emission_for_uid(netuid, neuron_uid), 0);
assert_eq!(
SubtensorModule::get_consensus_for_uid(netuid, neuron_uid),
0
);
assert_eq!(
SubtensorModule::get_incentive_for_uid(netuid, neuron_uid),
0
);
assert_eq!(
SubtensorModule::get_dividends_for_uid(netuid, neuron_uid),
0
);

assert!(!SubtensorModule::has_axon_info(
netuid,
&new_hotkey_account_id
));

// Check axon info is reset.
let axon_info = SubtensorModule::get_axon_info(netuid, &curr_hotkey.unwrap());
assert_eq!(axon_info.ip, 0);
assert_eq!(axon_info.port, 0);
assert_eq!(axon_info.ip_type, 0);
});
}

Expand Down
2 changes: 1 addition & 1 deletion runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// `spec_version`, and `authoring_version` are the same between Wasm and native.
// This value is set to 100 to notify Polkadot-JS App (https://polkadot.js.org/apps) to use
// the compatible custom types.
spec_version: 208,
spec_version: 209,
impl_version: 1,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand Down
6 changes: 3 additions & 3 deletions support/procedural-fork/src/pallet/parse/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,11 +718,11 @@ fn process_generics(
"CountedStorageNMap" => StorageKind::CountedNMap,
found => {
let msg = format!(
"Invalid pallet::storage, expected ident: `StorageValue` or \
"Invalid pallet::storage, expected ident: `StorageValue` or \
`StorageMap` or `CountedStorageMap` or `StorageDoubleMap` or `StorageNMap` or `CountedStorageNMap` \
in order to expand metadata, found `{}`.",
found,
);
found,
);
return Err(syn::Error::new(segment.ident.span(), msg));
}
};
Expand Down
18 changes: 9 additions & 9 deletions support/procedural-fork/src/runtime/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,19 +255,19 @@ impl Def {
};

let def = Def {
input,
runtime_struct: runtime_struct.ok_or_else(|| {
syn::Error::new(item_span,
input,
runtime_struct: runtime_struct.ok_or_else(|| {
syn::Error::new(item_span,
"Missing Runtime. Please add a struct inside the module and annotate it with `#[runtime::runtime]`"
)
})?,
pallets,
runtime_types: runtime_types.ok_or_else(|| {
syn::Error::new(item_span,
})?,
pallets,
runtime_types: runtime_types.ok_or_else(|| {
syn::Error::new(item_span,
"Missing Runtime Types. Please annotate the runtime struct with `#[runtime::derive]`"
)
})?,
};
})?,
};

Ok(def)
}
Expand Down