chore: PR review comments

Author: Samuel Dare

pallets/subtensor/src/lib.rs

@@ -628,8 +628,15 @@ pub mod pallet {
         StorageMap<_, Blake2_128Concat, T::AccountId, u16, ValueQuery, DefaultDelegateTake<T>>;
     #[pallet::storage]
     /// DMAP ( hot, netuid ) --> take | Returns the hotkey childkey take for a specific subnet
-    pub type ChildkeyTake<T: Config> =
-        StorageMap<_, Blake2_128Concat, (T::AccountId, u16), u16, ValueQuery>;
+    pub type ChildkeyTake<T: Config> = StorageDoubleMap<
+        _,
+        Blake2_128Concat,
+        T::AccountId, // First key: hotkey
+        Identity,
+        u16, // Second key: netuid
+        u16, // Value: take
+        ValueQuery,
+    >;
 
     #[pallet::storage]
     /// DMAP ( hot, cold ) --> stake | Returns the stake under a coldkey prefixed by hotkey.

pallets/subtensor/src/staking/set_children.rs

@@ -130,6 +130,11 @@ impl<T: Config> Pallet<T> {
         // --- 7.1. Insert my new children + proportion list into the map.
         ChildKeys::<T>::insert(hotkey.clone(), netuid, children.clone());
 
+        if children.is_empty() {
+            // If there are no children, remove the ChildkeyTake value
+            ChildkeyTake::<T>::remove(hotkey.clone(), netuid);
+        }
+
         // --- 7.2. Update the parents list for my new children.
         for (proportion, new_child_i) in children.clone().iter() {
             // --- 8.2.1. Get the child's parents on this network.
@@ -243,7 +248,7 @@ impl<T: Config> Pallet<T> {
         );
 
         // Set the new childkey take value for the given hotkey and network
-        ChildkeyTake::<T>::insert((hotkey.clone(), netuid), take);
+        ChildkeyTake::<T>::insert(hotkey.clone(), netuid, take);
 
         // TODO: Consider adding a check to ensure the hotkey is registered on the specified network (netuid)
         // before setting the childkey take. This could prevent setting takes for non-existent or
@@ -286,6 +291,6 @@ impl<T: Config> Pallet<T> {
     /// * `u16` - The childkey take value. This is a percentage represented as a value between 0 and 10000,
     ///           where 10000 represents 100%.
     pub fn get_childkey_take(hotkey: &T::AccountId, netuid: u16) -> u16 {
-        ChildkeyTake::<T>::get((hotkey, netuid))
+        ChildkeyTake::<T>::get(hotkey, netuid)
     }
 }

pallets/subtensor/tests/children.rs

@@ -1497,7 +1497,13 @@ fn test_get_parents_chain() {
         let num_keys: usize = 5;
         let proportion = u64::MAX / 2; // 50% stake allocation
 
-        log::info!("Test setup: netuid={}, coldkey={}, num_keys={}, proportion={}", netuid, coldkey, num_keys, proportion);
+        log::info!(
+            "Test setup: netuid={}, coldkey={}, num_keys={}, proportion={}",
+            netuid,
+            coldkey,
+            num_keys,
+            proportion
+        );
 
         // Create a vector of hotkeys
         let hotkeys: Vec<U256> = (0..num_keys).map(|i| U256::from(i as u64 + 2)).collect();
@@ -1512,7 +1518,12 @@ fn test_get_parents_chain() {
         // Register all neurons
         for hotkey in &hotkeys {
             register_ok_neuron(netuid, *hotkey, coldkey, 0);
-            log::info!("Registered neuron: hotkey={}, coldkey={}, netuid={}", hotkey, coldkey, netuid);
+            log::info!(
+                "Registered neuron: hotkey={}, coldkey={}, netuid={}",
+                hotkey,
+                coldkey,
+                netuid
+            );
         }
 
         // Set up parent-child relationships
@@ -1523,13 +1534,22 @@ fn test_get_parents_chain() {
                 netuid,
                 vec![(proportion, hotkeys[i + 1])]
             ));
-            log::info!("Set parent-child relationship: parent={}, child={}, proportion={}", hotkeys[i], hotkeys[i + 1], proportion);
+            log::info!(
+                "Set parent-child relationship: parent={}, child={}, proportion={}",
+                hotkeys[i],
+                hotkeys[i + 1],
+                proportion
+            );
         }
 
         // Test get_parents for each hotkey
         for i in 1..num_keys {
             let parents = SubtensorModule::get_parents(&hotkeys[i], netuid);
-            log::info!("Testing get_parents for hotkey {}: {:?}", hotkeys[i], parents);
+            log::info!(
+                "Testing get_parents for hotkey {}: {:?}",
+                hotkeys[i],
+                parents
+            );
             assert_eq!(
                 parents.len(),
                 1,
@@ -1546,7 +1566,11 @@ fn test_get_parents_chain() {
 
         // Test get_parents for the root (should be empty)
         let root_parents = SubtensorModule::get_parents(&hotkeys[0], netuid);
-        log::info!("Testing get_parents for root hotkey {}: {:?}", hotkeys[0], root_parents);
+        log::info!(
+            "Testing get_parents for root hotkey {}: {:?}",
+            hotkeys[0],
+            root_parents
+        );
         assert!(
             root_parents.is_empty(),
             "Root hotkey should have no parents"
@@ -1556,18 +1580,32 @@ fn test_get_parents_chain() {
         let last_hotkey = hotkeys[num_keys - 1];
         let new_parent = U256::from(num_keys as u64 + 2);
         register_ok_neuron(netuid, new_parent, coldkey, 0);
-        log::info!("Registered new parent neuron: new_parent={}, coldkey={}, netuid={}", new_parent, coldkey, netuid);
+        log::info!(
+            "Registered new parent neuron: new_parent={}, coldkey={}, netuid={}",
+            new_parent,
+            coldkey,
+            netuid
+        );
 
         assert_ok!(SubtensorModule::do_set_children(
             RuntimeOrigin::signed(coldkey),
             new_parent,
             netuid,
             vec![(proportion / 2, last_hotkey)]
         ));
-        log::info!("Set additional parent-child relationship: parent={}, child={}, proportion={}", new_parent, last_hotkey, proportion / 2);
+        log::info!(
+            "Set additional parent-child relationship: parent={}, child={}, proportion={}",
+            new_parent,
+            last_hotkey,
+            proportion / 2
+        );
 
         let last_hotkey_parents = SubtensorModule::get_parents(&last_hotkey, netuid);
-        log::info!("Testing get_parents for last hotkey {} with multiple parents: {:?}", last_hotkey, last_hotkey_parents);
+        log::info!(
+            "Testing get_parents for last hotkey {} with multiple parents: {:?}",
+            last_hotkey,
+            last_hotkey_parents
+        );
         assert_eq!(
             last_hotkey_parents.len(),
             2,
@@ -1583,3 +1621,55 @@ fn test_get_parents_chain() {
         );
     });
 }
+
+// SKIP_WASM_BUILD=1 RUST_LOG=info cargo test --test children -- test_childkey_take_removal_on_empty_children --exact --nocapture
+#[test]
+fn test_childkey_take_removal_on_empty_children() {
+    new_test_ext(1).execute_with(|| {
+        let coldkey = U256::from(1);
+        let hotkey = U256::from(2);
+        let child = U256::from(3);
+        let netuid: u16 = 1;
+        let proportion: u64 = 1000;
+
+        // Add network and register hotkey
+        add_network(netuid, 13, 0);
+        register_ok_neuron(netuid, hotkey, coldkey, 0);
+
+        // Set a child and childkey take
+        assert_ok!(SubtensorModule::do_set_children(
+            RuntimeOrigin::signed(coldkey),
+            hotkey,
+            netuid,
+            vec![(proportion, child)]
+        ));
+
+        let take: u16 = u16::MAX / 10; // 10% take
+        assert_ok!(SubtensorModule::set_childkey_take(
+            RuntimeOrigin::signed(coldkey),
+            hotkey,
+            netuid,
+            take
+        ));
+
+        // Verify childkey take is set
+        assert_eq!(SubtensorModule::get_childkey_take(&hotkey, netuid), take);
+
+        // Remove all children
+        assert_ok!(SubtensorModule::do_set_children(
+            RuntimeOrigin::signed(coldkey),
+            hotkey,
+            netuid,
+            vec![]
+        ));
+
+        // Verify children are removed
+        let children = SubtensorModule::get_children(&hotkey, netuid);
+        assert!(children.is_empty());
+
+        // Verify childkey take is removed
+        assert_eq!(SubtensorModule::get_childkey_take(&hotkey, netuid), 0);
+        // Verify childkey take storage is empty
+        assert_eq!(ChildkeyTake::<Test>::get(hotkey, netuid), 0);
+    });
+}

runtime/src/lib.rs

@@ -858,8 +858,8 @@ parameter_types! {
     pub const SubtensorInitialPruningScore : u16 = u16::MAX;
     pub const SubtensorInitialBondsMovingAverage: u64 = 900_000;
     pub const SubtensorInitialDefaultTake: u16 = 11_796; // 18% honest number.
-    pub const SubtensorInitialMinDelegateTake: u16 = 5_898; // 9%
-    pub const SubtensorInitialDefaultChildKeyTake: u16 = 11_796; // 18% honest number.
+    pub const SubtensorInitialMinDelegateTake: u16 = 0; // Allow 0% delegate take
+    pub const SubtensorInitialDefaultChildKeyTake: u16 = 0; // Allow 0% childkey take
     pub const SubtensorInitialMinChildKeyTake: u16 = 5_898; // 9%
     pub const SubtensorInitialWeightsVersionKey: u64 = 0;
     pub const SubtensorInitialMinDifficulty: u64 = 10_000_000;

scripts/test_specific.sh

@@ -1,6 +1,4 @@
 pallet="${3:-pallet-subtensor}"
 features="${4:-pow-faucet}"
 
-# RUST_LOG="pallet_subtensor=info" cargo test --release --features=$features -p $pallet --test $1 -- $2 --nocapture --exact
-
-RUST_LOG=DEBUG cargo test --release --features=$features -p $pallet --test $1 -- $2 --nocapture --exact
+SKIP_WASM_BUILD=1 RUST_LOG=DEBUG cargo test --release --features=$features -p $pallet --test $1 -- $2 --nocapture --exact