[core] Audit all reads to system state object

crates/sui-core/src/authority.rs

@@ -546,7 +546,7 @@ impl AuthorityState {
             return Err(SuiError::ValidatorHaltedAtEpochEnd);
         }
 
-        // Checks to see if the transaciton has expired
+        // Checks to see if the transactions has expired
         if match &transaction.inner().data().transaction_data().expiration {
             TransactionExpiration::None => false,
             TransactionExpiration::Epoch(epoch) => *epoch < epoch_store.epoch(),
@@ -1035,12 +1035,13 @@ impl AuthorityState {
         &self,
         sender: SuiAddress,
         transaction_kind: TransactionKind,
-        gas_price: u64,
+        gas_price: Option<u64>,
     ) -> Result<DevInspectResults, anyhow::Error> {
         let epoch_store = self.load_epoch_store_one_call_per_task();
         if !self.is_fullnode(&epoch_store) {
             return Err(anyhow!("dev-inspect is only supported on fullnodes"));
         }
+        let gas_price = gas_price.unwrap_or_else(|| epoch_store.reference_gas_price());
 
         let protocol_config = epoch_store.protocol_config();
 
@@ -1832,8 +1833,14 @@ impl AuthorityState {
             .compute_object_reference())
     }
 
-    // TODO: Audit every call to this function to make sure there are no data races during reconfig.
-    pub fn get_sui_system_state_object(&self) -> SuiResult<SuiSystemState> {
+    /// This function should be called once and exactly once during reconfiguration.
+    pub fn get_sui_system_state_object_during_reconfig(&self) -> SuiResult<SuiSystemState> {
+        self.database.get_sui_system_state_object()
+    }
+
+    // This function is only used for testing.
+    #[cfg(test)]
+    pub fn get_sui_system_state_object_for_testing(&self) -> SuiResult<SuiSystemState> {
         self.database.get_sui_system_state_object()
     }
 
@@ -2674,8 +2681,7 @@ impl AuthorityState {
             "Effects summary of the change epoch transaction: {:?}",
             signed_effects.summary_for_debug()
         );
-        epoch_store
-            .record_is_safe_mode_metric(self.get_sui_system_state_object().unwrap().safe_mode);
+        epoch_store.record_is_safe_mode_metric(system_obj.safe_mode);
         // The change epoch transaction cannot fail to execute.
         assert!(signed_effects.status.is_ok());
         Ok((system_obj, signed_effects.into_message()))

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

@@ -472,10 +472,12 @@ impl AuthorityPerEpochStore {
             .insert(checkpoint, accumulator)?)
     }
 
+    pub fn system_state_object(&self) -> &SuiSystemState {
+        &self.epoch_start_configuration.system_state
+    }
+
     pub fn reference_gas_price(&self) -> u64 {
-        self.epoch_start_configuration
-            .system_state
-            .reference_gas_price
+        self.system_state_object().reference_gas_price
     }
 
     pub async fn acquire_tx_guard(

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

@@ -1151,6 +1151,7 @@ impl AuthorityStore {
             .map(|v| v.map(|v| v.into()))
     }
 
+    // TODO: Transaction Orchestrator also calls this, which is not ideal.
     pub fn get_sui_system_state_object(&self) -> SuiResult<SuiSystemState> {
         self.perpetual_tables.get_sui_system_state_object()
     }

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

@@ -729,7 +729,9 @@ async fn test_dev_inspect_uses_unbound_object() {
         ))],
     }));
 
-    let result = fullnode.dev_inspect_transaction(sender, kind, 1).await;
+    let result = fullnode
+        .dev_inspect_transaction(sender, kind, Some(1))
+        .await;
     let Err(err) = result else { panic!() };
     assert!(err
         .to_string()
@@ -4234,7 +4236,9 @@ pub async fn call_dev_inspect(
         type_arguments,
         arguments,
     }));
-    authority.dev_inspect_transaction(*sender, kind, 1).await
+    authority
+        .dev_inspect_transaction(*sender, kind, Some(1))
+        .await
 }
 
 #[cfg(test)]

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

@@ -98,7 +98,7 @@ async fn test_narwhal_manager() {
             AuthorityState::new_for_testing(genesis_committee, &secret, None, genesis).await;
 
         let system_state = state
-            .get_sui_system_state_object()
+            .get_sui_system_state_object_for_testing()
             .expect("Reading Sui system state object cannot fail");
 
         let transactions_addr = &config.consensus_config.as_ref().unwrap().address;
@@ -174,7 +174,7 @@ async fn test_narwhal_manager() {
             .is_empty());
 
         let system_state = state
-            .get_sui_system_state_object()
+            .get_sui_system_state_object_for_testing()
             .expect("Reading Sui system state object cannot fail");
 
         let mut narwhal_committee = system_state.get_current_epoch_narwhal_committee();

crates/sui-json-rpc/src/coin_api.rs

@@ -274,13 +274,11 @@ impl CoinReadApiServer for CoinReadApi {
     }
 
     async fn get_total_supply(&self, coin_type: String) -> RpcResult<Supply> {
+        let epoch_store = self.state.load_epoch_store_one_call_per_task();
         let coin_struct = parse_sui_struct_tag(&coin_type)?;
 
         Ok(if GAS::is_gas(&coin_struct) {
-            self.state
-                .get_sui_system_state_object()
-                .map_err(Error::from)?
-                .treasury_cap
+            epoch_store.system_state_object().treasury_cap.clone()
         } else {
             let treasury_cap_object = self
                 .find_package_object(&coin_struct.address.into(), TreasuryCap::type_(coin_struct))

crates/sui-json-rpc/src/governance_api.rs

@@ -88,10 +88,8 @@ impl GovernanceReadApiServer for GovernanceReadApi {
     }
 
     async fn get_sui_system_state(&self) -> RpcResult<SuiSystemState> {
-        Ok(self
-            .state
-            .get_sui_system_state_object()
-            .map_err(Error::from)?)
+        let epoch_store = self.state.load_epoch_store_one_call_per_task();
+        Ok(epoch_store.system_state_object().clone())
     }
 
     async fn get_reference_gas_price(&self) -> RpcResult<u64> {

crates/sui-json-rpc/src/transaction_builder_api.rs

@@ -72,7 +72,8 @@ impl DataReader for AuthorityStateDataReader {
     }
 
     async fn get_reference_gas_price(&self) -> Result<u64, anyhow::Error> {
-        Ok(self.0.get_sui_system_state_object()?.reference_gas_price)
+        let epoch_store = self.0.load_epoch_store_one_call_per_task();
+        Ok(epoch_store.reference_gas_price())
     }
 }
 

crates/sui-json-rpc/src/transaction_execution_api.rs

@@ -44,14 +44,6 @@ impl TransactionExecutionApi {
             module_cache,
         }
     }
-
-    fn get_sui_system_state_object_epoch_and_gas_price(&self) -> RpcResult<(EpochId, u64)> {
-        let sys_state = self
-            .state
-            .get_sui_system_state_object()
-            .map_err(|e| anyhow!("Unable to retrieve sui system state object: {e}"))?;
-        Ok((sys_state.epoch, sys_state.reference_gas_price))
-    }
 }
 
 #[async_trait]
@@ -130,23 +122,13 @@ impl WriteApiServer for TransactionExecutionApi {
         sender_address: SuiAddress,
         tx_bytes: Base64,
         gas_price: Option<u64>,
-        epoch: Option<EpochId>,
+        _epoch: Option<EpochId>,
     ) -> RpcResult<DevInspectResults> {
-        // Only fetch from DB if necessary
-        let reference_gas_price = if gas_price.is_none() || epoch.is_none() {
-            self.get_sui_system_state_object_epoch_and_gas_price()?.1
-        } else {
-            0
-        };
         let tx_kind: TransactionKind =
             bcs::from_bytes(&tx_bytes.to_vec().map_err(|e| anyhow!(e))?).map_err(|e| anyhow!(e))?;
         Ok(self
             .state
-            .dev_inspect_transaction(
-                sender_address,
-                tx_kind,
-                gas_price.unwrap_or(reference_gas_price),
-            )
+            .dev_inspect_transaction(sender_address, tx_kind, gas_price)
             .await?)
     }
 

crates/sui-node/src/lib.rs

@@ -54,7 +54,7 @@ use tokio::sync::broadcast;
 use tokio::sync::{watch, Mutex};
 use tokio::task::JoinHandle;
 use tower::ServiceBuilder;
-use tracing::{error_span, info, warn, Instrument};
+use tracing::{error_span, info, Instrument};
 use typed_store::DBMetrics;
 pub mod admin;
 mod handle;
@@ -550,9 +550,7 @@ impl SuiNode {
         // TODO: The following is a bug and could potentially lead to data races.
         // We need to put the narwhal committee in the epoch store, so that we could read it here,
         // instead of reading from the system state.
-        let system_state = state
-            .get_sui_system_state_object()
-            .expect("Reading Sui system state object cannot fail");
+        let system_state = epoch_store.system_state_object();
         let committee = system_state.get_current_epoch_narwhal_committee();
 
         let transactions_addr = &config
@@ -562,24 +560,18 @@ impl SuiNode {
             .address;
         let worker_cache = system_state.get_current_epoch_narwhal_worker_cache(transactions_addr);
 
-        if committee.epoch == epoch_store.epoch() {
-            narwhal_manager
-                .start(
-                    committee.clone(),
-                    SharedWorkerCache::from(worker_cache),
-                    consensus_handler,
-                    SuiTxValidator::new(
-                        epoch_store,
-                        state.transaction_manager().clone(),
-                        sui_tx_validator_metrics.clone(),
-                    ),
-                )
-                .await;
-        } else {
-            warn!(
-                "Current Sui epoch doesn't match the system state epoch. Not starting Narwhal yet"
-            );
-        }
+        narwhal_manager
+            .start(
+                committee.clone(),
+                SharedWorkerCache::from(worker_cache),
+                consensus_handler,
+                SuiTxValidator::new(
+                    epoch_store,
+                    state.transaction_manager().clone(),
+                    sui_tx_validator_metrics.clone(),
+                ),
+            )
+            .await;
 
         Ok(ValidatorComponents {
             validator_server_handle,
@@ -790,7 +782,7 @@ impl SuiNode {
             assert_eq!(cur_epoch_store.epoch() + 1, next_epoch);
             let system_state = self
                 .state
-                .get_sui_system_state_object()
+                .get_sui_system_state_object_during_reconfig()
                 .expect("Read Sui System State object cannot fail");
             // Double check that the committee in the last checkpoint is identical to what's on-chain.
             assert_eq!(