From bf144408959889ac8284a2b7892a7d48f6ae60c6 Mon Sep 17 00:00:00 2001
From: HaoranYi <haoran.yi@gmail.com>
Date: Tue, 12 Jul 2022 14:27:28 -0500
Subject: [PATCH] clean up and optimize account hash verify (#26560)

* remove unused code

* extract test related fault hash inject fn

* use rotate to optimize hashes removal

* use rotate to optimize snapshot hashes removal

* address code reveiw feedbacks

* revise comments

* inline
---
 core/src/accounts_hash_verifier.rs    | 28 ++++++++++++++-------------
 core/src/snapshot_packager_service.rs | 20 ++++++++++++-------
 runtime/src/snapshot_package.rs       | 11 +++++++++++
 3 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/core/src/accounts_hash_verifier.rs b/core/src/accounts_hash_verifier.rs
index 9d59e4072..8b557202f 100644
--- a/core/src/accounts_hash_verifier.rs
+++ b/core/src/accounts_hash_verifier.rs
@@ -11,8 +11,8 @@ use {
         accounts_hash::{CalcAccountsHashConfig, HashStats},
         snapshot_config::SnapshotConfig,
         snapshot_package::{
-            AccountsPackage, PendingAccountsPackage, PendingSnapshotPackage, SnapshotPackage,
-            SnapshotType,
+            retain_max_n_elements, AccountsPackage, PendingAccountsPackage, PendingSnapshotPackage,
+            SnapshotPackage, SnapshotType,
         },
         sorted_storages::SortedStorages,
     },
@@ -193,6 +193,16 @@ impl AccountsHashVerifier {
         accounts_hash
     }
 
+    fn generate_fault_hash(original_hash: &Hash) -> Hash {
+        use {
+            rand::{thread_rng, Rng},
+            solana_sdk::hash::extend_and_hash,
+        };
+
+        let rand = thread_rng().gen_range(0, 10);
+        extend_and_hash(original_hash, &[rand])
+    }
+
     fn push_accounts_hashes_to_cluster(
         accounts_package: &AccountsPackage,
         cluster_info: &ClusterInfo,
@@ -207,21 +217,14 @@ impl AccountsHashVerifier {
             && accounts_package.slot % fault_injection_rate_slots == 0
         {
             // For testing, publish an invalid hash to gossip.
-            use {
-                rand::{thread_rng, Rng},
-                solana_sdk::hash::extend_and_hash,
-            };
+            let fault_hash = Self::generate_fault_hash(&accounts_hash);
             warn!("inserting fault at slot: {}", accounts_package.slot);
-            let rand = thread_rng().gen_range(0, 10);
-            let hash = extend_and_hash(&accounts_hash, &[rand]);
-            hashes.push((accounts_package.slot, hash));
+            hashes.push((accounts_package.slot, fault_hash));
         } else {
             hashes.push((accounts_package.slot, accounts_hash));
         }
 
-        while hashes.len() > MAX_SNAPSHOT_HASHES {
-            hashes.remove(0);
-        }
+        retain_max_n_elements(hashes, MAX_SNAPSHOT_HASHES);
 
         if halt_on_known_validator_accounts_hash_mismatch {
             let mut slot_to_hash = HashMap::new();
@@ -251,7 +254,6 @@ impl AccountsHashVerifier {
 
         let snapshot_package = SnapshotPackage::new(accounts_package, accounts_hash);
         let pending_snapshot_package = pending_snapshot_package.unwrap();
-        let _snapshot_config = snapshot_config.unwrap();
 
         // If the snapshot package is an Incremental Snapshot, do not submit it if there's already
         // a pending Full Snapshot.
diff --git a/core/src/snapshot_packager_service.rs b/core/src/snapshot_packager_service.rs
index cce9632ed..7077362e4 100644
--- a/core/src/snapshot_packager_service.rs
+++ b/core/src/snapshot_packager_service.rs
@@ -10,7 +10,7 @@ use {
             FullSnapshotHash, FullSnapshotHashes, IncrementalSnapshotHash,
             IncrementalSnapshotHashes, StartingSnapshotHashes,
         },
-        snapshot_package::{PendingSnapshotPackage, SnapshotType},
+        snapshot_package::{retain_max_n_elements, PendingSnapshotPackage, SnapshotType},
         snapshot_utils,
     },
     solana_sdk::{clock::Slot, hash::Hash},
@@ -166,9 +166,12 @@ impl SnapshotGossipManager {
         self.full_snapshot_hashes
             .hashes
             .push(full_snapshot_hash.hash);
-        while self.full_snapshot_hashes.hashes.len() > self.max_full_snapshot_hashes {
-            self.full_snapshot_hashes.hashes.remove(0);
-        }
+
+        retain_max_n_elements(
+            &mut self.full_snapshot_hashes.hashes,
+            self.max_full_snapshot_hashes,
+        );
+
         self.cluster_info
             .push_snapshot_hashes(self.full_snapshot_hashes.hashes.clone());
     }
@@ -190,9 +193,12 @@ impl SnapshotGossipManager {
         self.incremental_snapshot_hashes
             .hashes
             .push(incremental_snapshot_hash.hash);
-        while self.incremental_snapshot_hashes.hashes.len() > self.max_incremental_snapshot_hashes {
-            self.incremental_snapshot_hashes.hashes.remove(0);
-        }
+
+        retain_max_n_elements(
+            &mut self.incremental_snapshot_hashes.hashes,
+            self.max_incremental_snapshot_hashes,
+        );
+
         // Pushing incremental snapshot hashes to the cluster should never fail.  The only error
         // case is when the length of the hashes is too big, but we account for that with
         // `max_incremental_snapshot_hashes`.  If this call ever does error, it's a programmer bug!
diff --git a/runtime/src/snapshot_package.rs b/runtime/src/snapshot_package.rs
index cb050bc7f..8dec38f18 100644
--- a/runtime/src/snapshot_package.rs
+++ b/runtime/src/snapshot_package.rs
@@ -209,3 +209,14 @@ impl SnapshotType {
         matches!(self, SnapshotType::IncrementalSnapshot(_))
     }
 }
+
+/// Helper function to retain only max n of elements to the right of a vector,
+/// viz. remove v.len() - n elements from the left of the vector.
+#[inline(always)]
+pub fn retain_max_n_elements<T>(v: &mut Vec<T>, n: usize) {
+    if v.len() > n {
+        let to_truncate = v.len() - n;
+        v.rotate_left(to_truncate);
+        v.truncate(n);
+    }
+}