Avoid cloning committee (#9065)

User Arc instead

crates/sui-core/src/authority.rs

@@ -1650,7 +1650,7 @@ impl AuthorityState {
         let cache_metrics = Arc::new(ResolverMetrics::new(&registry));
         let epoch_store = AuthorityPerEpochStore::new(
             name,
-            genesis_committee.clone(),
+            Arc::new(genesis_committee.clone()),
             &path.join("store"),
             None,
             EpochMetrics::new(&registry),

crates/sui-core/src/authority/authority_per_epoch_store.rs

@@ -97,8 +97,7 @@ pub struct ExecutionComponents {
 }
 
 pub struct AuthorityPerEpochStore {
-    // TODO: Make this Arc<Committee> for more efficient pass-around.
-    committee: Committee,
+    committee: Arc<Committee>,
     tables: AuthorityEpochTables,
 
     protocol_config: ProtocolConfig,
@@ -379,7 +378,7 @@ impl AuthorityEpochTables {
 impl AuthorityPerEpochStore {
     pub fn new(
         name: AuthorityName,
-        committee: Committee,
+        committee: Arc<Committee>,
         parent_path: &Path,
         db_options: Option<Options>,
         metrics: Arc<EpochMetrics>,
@@ -391,7 +390,7 @@ impl AuthorityPerEpochStore {
         let epoch_id = committee.epoch;
         let tables = AuthorityEpochTables::open(epoch_id, parent_path, db_options.clone());
         let end_of_publish =
-            StakeAggregator::from_iter(Arc::new(committee.clone()), tables.end_of_publish.iter());
+            StakeAggregator::from_iter(committee.clone(), tables.end_of_publish.iter());
         let reconfig_state = tables
             .load_reconfig_state()
             .expect("Load reconfig state at initialization cannot fail");
@@ -467,7 +466,7 @@ impl AuthorityPerEpochStore {
         self.record_epoch_total_duration_metric();
         Self::new(
             name,
-            new_committee,
+            Arc::new(new_committee),
             &self.parent_path,
             self.db_options.clone(),
             self.metrics.clone(),

crates/sui-core/src/authority/authority_store.rs

@@ -94,7 +94,7 @@ impl AuthorityStore {
         Self::open_inner(
             genesis,
             perpetual_tables,
-            committee,
+            &committee,
             indirect_objects_threshold,
         )
         .await
@@ -114,7 +114,7 @@ impl AuthorityStore {
         Self::open_inner(
             genesis,
             perpetual_tables,
-            committee.clone(),
+            committee,
             indirect_objects_threshold,
         )
         .await
@@ -123,7 +123,7 @@ impl AuthorityStore {
     async fn open_inner(
         genesis: &Genesis,
         perpetual_tables: Arc<AuthorityPerpetualTables>,
-        committee: Committee,
+        committee: &Committee,
         indirect_objects_threshold: usize,
     ) -> SuiResult<Self> {
         let epoch = committee.epoch;

crates/sui-core/src/epoch/committee_store.rs

@@ -5,6 +5,7 @@ use parking_lot::RwLock;
 use rocksdb::Options;
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
+use std::sync::Arc;
 use sui_storage::default_db_options;
 use sui_types::base_types::ObjectID;
 use sui_types::committee::{Committee, EpochId};
@@ -19,7 +20,7 @@ use sui_macros::nondeterministic;
 
 pub struct CommitteeStore {
     tables: CommitteeStoreTables,
-    cache: RwLock<HashMap<EpochId, Committee>>,
+    cache: RwLock<HashMap<EpochId, Arc<Committee>>>,
 }
 
 #[derive(DBMapUtils)]
@@ -63,32 +64,33 @@ impl CommitteeStore {
     pub fn init_genesis_committee(&self, genesis_committee: Committee) -> SuiResult {
         assert_eq!(genesis_committee.epoch, 0);
         self.tables.committee_map.insert(&0, &genesis_committee)?;
-        self.cache.write().insert(0, genesis_committee);
+        self.cache.write().insert(0, Arc::new(genesis_committee));
         Ok(())
     }
 
     pub fn insert_new_committee(&self, new_committee: &Committee) -> SuiResult {
         if let Some(old_committee) = self.get_committee(&new_committee.epoch)? {
             // If somehow we already have this committee in the store, they must be the same.
-            assert_eq!(&old_committee, new_committee);
+            assert_eq!(&*old_committee, new_committee);
         } else {
             self.tables
                 .committee_map
                 .insert(&new_committee.epoch, new_committee)?;
             self.cache
                 .write()
-                .insert(new_committee.epoch, new_committee.clone());
+                .insert(new_committee.epoch, Arc::new(new_committee.clone()));
         }
         Ok(())
     }
 
-    pub fn get_committee(&self, epoch_id: &EpochId) -> SuiResult<Option<Committee>> {
+    pub fn get_committee(&self, epoch_id: &EpochId) -> SuiResult<Option<Arc<Committee>>> {
         if let Some(committee) = self.cache.read().get(epoch_id) {
-            return Ok(Some(committee.clone())); // todo use Arc
+            return Ok(Some(committee.clone()));
         }
         let committee = self.tables.committee_map.get(epoch_id)?;
+        let committee = committee.map(Arc::new);
         if let Some(committee) = committee.as_ref() {
-            self.cache.write().insert(*epoch_id, committee.clone()); // todo use Arc
+            self.cache.write().insert(*epoch_id, committee.clone());
         }
         Ok(committee)
     }
@@ -111,7 +113,8 @@ impl CommitteeStore {
         Ok(match epoch {
             Some(epoch) => self
                 .get_committee(&epoch)?
-                .ok_or(SuiError::MissingCommitteeAtEpoch(epoch))?,
+                .ok_or(SuiError::MissingCommitteeAtEpoch(epoch))
+                .map(|c| Committee::clone(&*c))?,
             None => self.get_latest_committee(),
         })
     }

crates/sui-core/src/safe_client.rs

@@ -168,7 +168,7 @@ impl<C, S: SignatureVerifier> SafeClient<C, S> {
         &mut self.authority_client
     }
 
-    fn get_committee(&self, epoch_id: &EpochId) -> SuiResult<Committee> {
+    fn get_committee(&self, epoch_id: &EpochId) -> SuiResult<Arc<Committee>> {
         self.committee_store
             .get_committee(epoch_id)?
             .ok_or(SuiError::MissingCommitteeAtEpoch(*epoch_id))
@@ -446,7 +446,7 @@ where
         match checkpoint {
             Some(c) => {
                 let epoch_id = c.epoch;
-                c.verify_with_contents(&self.get_committee(&epoch_id)?, contents.as_ref())
+                c.verify_with_contents(&*self.get_committee(&epoch_id)?, contents.as_ref())
             }
             None => Ok(()),
         }

crates/sui-core/src/storage.rs

@@ -92,7 +92,7 @@ impl ReadStore for RocksDbStore {
             .map(|contents| contents.flatten())
     }
 
-    fn get_committee(&self, epoch: EpochId) -> Result<Option<Committee>, Self::Error> {
+    fn get_committee(&self, epoch: EpochId) -> Result<Option<Arc<Committee>>, Self::Error> {
         Ok(self.committee_store.get_committee(&epoch).unwrap())
     }
 

crates/sui-core/src/unit_tests/authority_tests.rs

@@ -2731,7 +2731,7 @@ async fn test_authority_persist() {
         let cache_metrics = Arc::new(ResolverMetrics::new(&registry));
         let epoch_store = AuthorityPerEpochStore::new(
             name,
-            committee,
+            Arc::new(committee),
             &epoch_store_path,
             None,
             EpochMetrics::new(&registry),
@@ -4804,7 +4804,7 @@ async fn test_tallying_rule_score_updates() {
     let cache_metrics = Arc::new(ResolverMetrics::new(&registry));
     let epoch_store = AuthorityPerEpochStore::new(
         auth_0_name,
-        committee.clone(),
+        Arc::new(committee.clone()),
         &path,
         None,
         metrics.clone(),

crates/sui-types/src/storage.rs

@@ -30,6 +30,7 @@ use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
 use std::collections::{BTreeMap, HashMap};
 use std::convert::Infallible;
+use std::sync::Arc;
 use tap::Pipe;
 
 #[derive(Debug, PartialEq, Eq, Clone, Copy, Serialize, Deserialize)]
@@ -215,7 +216,7 @@ pub trait ReadStore {
         digest: &CheckpointContentsDigest,
     ) -> Result<Option<FullCheckpointContents>, Self::Error>;
 
-    fn get_committee(&self, epoch: EpochId) -> Result<Option<Committee>, Self::Error>;
+    fn get_committee(&self, epoch: EpochId) -> Result<Option<Arc<Committee>>, Self::Error>;
 
     fn get_transaction(
         &self,
@@ -265,7 +266,7 @@ impl<T: ReadStore> ReadStore for &T {
         ReadStore::get_full_checkpoint_contents(*self, digest)
     }
 
-    fn get_committee(&self, epoch: EpochId) -> Result<Option<Committee>, Self::Error> {
+    fn get_committee(&self, epoch: EpochId) -> Result<Option<Arc<Committee>>, Self::Error> {
         ReadStore::get_committee(*self, epoch)
     }
 
@@ -548,8 +549,12 @@ impl ReadStore for SharedInMemoryStore {
             .map(|contents| contents.flatten())
     }
 
-    fn get_committee(&self, epoch: EpochId) -> Result<Option<Committee>, Self::Error> {
-        self.inner().get_committee_by_epoch(epoch).cloned().pipe(Ok)
+    fn get_committee(&self, epoch: EpochId) -> Result<Option<Arc<Committee>>, Self::Error> {
+        self.inner()
+            .get_committee_by_epoch(epoch)
+            .cloned()
+            .map(Arc::new)
+            .pipe(Ok)
     }
 
     fn get_transaction(