clean expired commits

Author: JohnReedV

pallets/subtensor/src/coinbase/run_coinbase.rs

@@ -93,13 +93,15 @@ impl<T: Config> Pallet<T> {
             // --- 4.1 Check to see if the subnet should run its epoch.
             if Self::should_run_epoch(*netuid, current_block) {
                 // --- 4.2 Reveal weights from the n-2nd epoch.
-                if let Err(e) = Self::reveal_crv3_commits(*netuid) {
-                    log::warn!(
-                        "Failed to reveal commits for subnet {} due to error: {:?}",
-                        *netuid,
-                        e
-                    );
-                };
+                if Self::get_commit_reveal_weights_enabled(*netuid) {
+                    if let Err(e) = Self::reveal_crv3_commits(*netuid) {
+                        log::warn!(
+                            "Failed to reveal commits for subnet {} due to error: {:?}",
+                            *netuid,
+                            e
+                        );
+                    };
+                }
 
                 // --- 4.3 Drain the subnet emission.
                 let mut subnet_emission: u64 = PendingEmission::<T>::get(*netuid);
@@ -204,8 +206,6 @@ impl<T: Config> Pallet<T> {
     }
 
     /// The `reveal_crv3_commits` function is run at the very beginning of epoch `n`,
-    /// revealing commitments from epoch `n - 2`.
-    /// n - 2.
     pub fn reveal_crv3_commits(netuid: u16) -> dispatch::DispatchResult {
         use ark_serialize::CanonicalDeserialize;
         use frame_support::traits::OriginTrait;
@@ -216,16 +216,23 @@ impl<T: Config> Pallet<T> {
         let cur_block = Self::get_current_block_as_u64();
         let cur_epoch = Self::get_epoch_index(netuid, cur_block);
 
+        // Weights revealed must have been committed during epoch `cur_epoch - reveal_period`.
+        let reveal_epoch =
+            cur_epoch.saturating_sub(Self::get_reveal_period(netuid).saturating_sub(1));
+
+        // Clean expired commits
+        for (epoch, _) in CRV3WeightCommits::<T>::iter_prefix(netuid) {
+            if epoch < reveal_epoch {
+                CRV3WeightCommits::<T>::remove(netuid, epoch);
+            }
+        }
+
         // No commits to reveal until at least epoch 2.
         if cur_epoch < 2 {
             log::warn!("Failed to reveal commit for subnet {} Too early", netuid);
             return Ok(());
         }
 
-        // Weights revealed must have been committed during epoch `cur_epoch - reveal_period`.
-        let reveal_epoch =
-            cur_epoch.saturating_sub(Self::get_reveal_period(netuid).saturating_sub(1));
-
         let mut entries = CRV3WeightCommits::<T>::take(netuid, reveal_epoch);
 
         // Keep popping item off the end of the queue until we sucessfully reveal a commit.

pallets/subtensor/src/tests/weights.rs

@@ -5320,3 +5320,67 @@ fn test_multiple_commits_by_same_hotkey_within_limit() {
         );
     });
 }
+
+#[test]
+fn test_reveal_crv3_commits_removes_past_epoch_commits() {
+    new_test_ext(100).execute_with(|| {
+        let netuid: u16 = 1;
+        let hotkey: AccountId = U256::from(1);
+        let reveal_round: u64 = 1000;
+
+        // Initialize network and neuron
+        add_network(netuid, 5, 0);
+        register_ok_neuron(netuid, hotkey, U256::from(2), 100_000);
+        SubtensorModule::set_commit_reveal_weights_enabled(netuid, true);
+        SubtensorModule::set_reveal_period(netuid, 1);
+        SubtensorModule::set_weights_set_rate_limit(netuid, 0);
+
+        let current_block = SubtensorModule::get_current_block_as_u64();
+        let current_epoch = SubtensorModule::get_epoch_index(netuid, current_block);
+
+        // Simulate commits in past epochs
+        let past_epochs = vec![current_epoch - 2, current_epoch - 1];
+        for epoch in &past_epochs {
+            let commit_data: Vec<u8> = vec![*epoch as u8; 5];
+            let bounded_commit_data = commit_data
+                .clone()
+                .try_into()
+                .expect("Failed to convert commit data into bounded vector");
+            assert_ok!(CRV3WeightCommits::<Test>::try_mutate(
+                netuid,
+                *epoch,
+                |commits| -> DispatchResult {
+                    commits.push_back((hotkey, bounded_commit_data, reveal_round));
+                    Ok(())
+                }
+            ));
+        }
+
+        for epoch in &past_epochs {
+            let commits = CRV3WeightCommits::<Test>::get(netuid, *epoch);
+            assert!(
+                !commits.is_empty(),
+                "Expected commits to be present for past epoch {}",
+                epoch
+            );
+        }
+
+        assert_ok!(SubtensorModule::reveal_crv3_commits(netuid));
+
+        for epoch in &past_epochs {
+            let commits = CRV3WeightCommits::<Test>::get(netuid, *epoch);
+            assert!(
+                commits.is_empty(),
+                "Expected commits for past epoch {} to be removed",
+                epoch
+            );
+        }
+
+        let current_epoch_commits = CRV3WeightCommits::<Test>::get(netuid, current_epoch);
+        assert!(
+            current_epoch_commits.is_empty(),
+            "Expected no commits for current epoch {}",
+            current_epoch
+        );
+    });
+}