Skip to content

Commit

Permalink
[core] Make AuthorityState.committee private field (MystenLabs#6484)
Browse files Browse the repository at this point in the history
  • Loading branch information
lxfind authored Dec 1, 2022
1 parent 15d1e30 commit 58a8ae5
Show file tree
Hide file tree
Showing 12 changed files with 27 additions and 31 deletions.
8 changes: 4 additions & 4 deletions crates/sui-core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ pub struct AuthorityState {

// Epoch related information.
/// Committee of this Sui instance.
pub committee: ArcSwap<Committee>,
committee: ArcSwap<Committee>,

/// Move native functions that are available to invoke
pub(crate) _native_functions: NativeFunctionTable,
Expand Down Expand Up @@ -579,7 +579,7 @@ impl AuthorityState {
}

pub fn epoch(&self) -> EpochId {
self.committee.load().epoch
self.committee().epoch
}

pub fn committee_store(&self) -> &Arc<CommitteeStore> {
Expand Down Expand Up @@ -2147,7 +2147,7 @@ impl AuthorityState {
fn verify_narwhal_transaction(&self, certificate: &CertifiedTransaction) -> SuiResult {
// Check the certificate. Remember that Byzantine authorities may input anything into
// consensus.
certificate.verify_signature(&self.committee.load())
certificate.verify_signature(&self.committee())
}

/// Verifies transaction signatures and other data
Expand Down Expand Up @@ -2193,7 +2193,7 @@ impl AuthorityState {
warn!("CheckpointSignature authority {} does not match narwhal certificate source {}", data.summary.auth_signature.authority, consensus_output.certificate.origin() );
return Err(());
}
data.verify(&self.committee.load()).map_err(|err|{
data.verify(&self.committee()).map_err(|err|{
warn!(
"Ignoring malformed checkpoint signature (failed to verify) from {}, sequence {}: {:?}",
transaction.consensus_output.certificate.header.author, data.summary.summary.sequence_number, err
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-core/src/authority_active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
use arc_swap::ArcSwap;
use mysten_metrics::spawn_monitored_task;
use prometheus::Registry;
use std::{collections::HashMap, ops::Deref, sync::Arc, time::Duration};
use std::{collections::HashMap, sync::Arc, time::Duration};
use sui_types::{base_types::AuthorityName, error::SuiResult};
use tokio::{
sync::{oneshot, Mutex, MutexGuard},
Expand Down Expand Up @@ -261,7 +261,7 @@ where
pub async fn spawn_gossip_process(self: Arc<Self>, degree: usize) -> JoinHandle<()> {
// Number of tasks at most "degree" and no more than committee - 1
// (validators do not follow themselves for gossip)
let committee = self.state.committee.load().deref().clone();
let committee = self.state.clone_committee();
let target_num_tasks = usize::min(committee.num_members() - 1, degree);

spawn_monitored_task!(gossip_process(&self, target_num_tasks))
Expand Down Expand Up @@ -313,7 +313,7 @@ where
self: Arc<Self>,
mut lock_guard: MutexGuard<'_, Option<NodeSyncProcessHandle>>,
) {
let epoch = self.state.committee.load().epoch;
let epoch = self.state.epoch();
info!(?epoch, "respawn_node_sync_process");
Self::cancel_node_sync_process_impl(&mut lock_guard).await;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl AuthorityAPI for ConfigurableBatchActionClient {
certificate: CertifiedTransaction,
) -> Result<TransactionInfoResponse, SuiError> {
let state = self.state.clone();
let certificate = certificate.verify(&self.state.committee.load()).unwrap();
let certificate = certificate.verify(&self.state.committee()).unwrap();
state
.handle_certificate(&certificate)
.await
Expand Down
11 changes: 5 additions & 6 deletions crates/sui-core/src/authority_active/gossip/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use prometheus::{
register_int_gauge_with_registry, Histogram, IntCounter, IntGauge, Registry,
};
use std::future::Future;
use std::ops::Deref;
use std::{collections::HashSet, sync::Arc, time::Duration};
use sui_types::committee::StakeUnit;
use sui_types::{
Expand Down Expand Up @@ -149,7 +148,7 @@ async fn follower_process<A, Handler: DigestHandler<A> + Clone>(
{
// Make a clone of the active authority and committee, and keep using it until epoch changes.
let mut local_active = Arc::new(active_authority.clone());
let mut committee = local_active.state.committee.load().deref().clone();
let mut committee = local_active.state.clone_committee();

// Number of tasks at most "degree" and no more than committee - 1
let mut target_num_tasks = usize::min(committee.num_members() - 1, degree);
Expand All @@ -169,15 +168,15 @@ async fn follower_process<A, Handler: DigestHandler<A> + Clone>(
.concurrent_followed_validators;
let metrics_reconnect_interval_ms = &active_authority.gossip_metrics.reconnect_interval_ms;
loop {
if active_authority.state.committee.load().epoch != committee.epoch {
if active_authority.state.epoch() != committee.epoch {
// If epoch has changed, we need to make a new copy of the active authority,
// and update all local variables.
// We also need to remove any authority that's no longer a valid validator
// from the list of peer names.
// It's ok to keep the existing gossip tasks running even for peers that are no longer
// validators, and let them end naturally.
local_active = Arc::new(active_authority.clone());
committee = local_active.state.committee.load().deref().clone();
committee = local_active.state.clone_committee();
target_num_tasks = usize::min(committee.num_members() - 1, degree);
peer_names.retain(|name| committee.authority_exists(name));
}
Expand Down Expand Up @@ -277,9 +276,9 @@ where
{
// Make sure we exit loop by limiting the number of tries to choose peer
// where n is the total number of committee members.
let mut tries_remaining = active_authority.state.committee.load().num_members();
let mut tries_remaining = active_authority.state.committee().num_members();
while tries_remaining > 0 {
let name = *active_authority.state.committee.load().sample();
let name = *active_authority.state.committee().sample();
if peer_names.contains(&name)
|| name == my_name
|| !active_authority.can_contact(name).await
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-core/src/authority_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ impl LocalAuthorityClient {
let response = match state.get_tx_info_already_executed(&tx_digest).await {
Ok(Some(response)) => response,
_ => {
let certificate = certificate.verify(&state.committee.load())?;
let certificate = certificate.verify(&state.committee())?;
state.handle_certificate(&certificate).await?
}
};
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-core/src/authority_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ impl ValidatorService {

// 2) Verify cert signatures
let cert_verif_metrics_guard = start_timer(metrics.cert_verification_latency.clone());
let certificate = certificate.verify(&state.committee.load())?;
let certificate = certificate.verify(&state.committee())?;
drop(cert_verif_metrics_guard);

// 3) All certificates are sent to consensus (at least by some authorities)
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-core/src/consensus_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ impl ConsensusAdapter {
.consensus_message_processed_notify(transaction.key())
.boxed();
let await_submit = Self::await_submit_delay(
&self.authority.committee.load(),
&self.authority.committee(),
&self.authority.name,
&transaction,
)
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-core/src/consensus_validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl TransactionValidator for ConsensusTxValidator {

fn validate(&self, tx: &[u8]) -> Result<(), Self::Error> {
let transaction = tx_from_bytes(tx)?;
let committee = self.state.committee.load();
let committee = self.state.committee();
match &transaction.kind {
ConsensusTransactionKind::UserTransaction(certificate) => {
certificate.verify_signature(&committee).tap_err(|err| {
Expand All @@ -62,7 +62,7 @@ impl TransactionValidator for ConsensusTxValidator {
.iter()
.map(|tx| tx_from_bytes(tx))
.collect::<Result<Vec<_>, _>>()?;
let committee = self.state.committee.load();
let committee = self.state.committee();

let mut obligation = VerificationObligation::default();
for tx in txs.into_iter() {
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-core/src/node_sync/node_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ where
epoch_id: EpochId,
) -> SuiResult<bool> {
// Check if the tx is final.
let committee = self.state().committee.load();
let committee = self.state().committee();
check_epoch!(committee.epoch, epoch_id);
let stake = committee.weight(peer);
let quorum_threshold = committee.quorum_threshold();
Expand Down
11 changes: 4 additions & 7 deletions crates/sui-core/src/transaction_orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ where
}
QuorumDriverResponse::EffectsCert(result) => {
let (tx_cert, effects_cert) = *result;
let tx_cert = tx_cert.verify(&self.validator_state.committee.load())?;
let tx_cert = tx_cert.verify(&self.validator_state.committee())?;
if !wait_for_local_execution {
return Ok(ExecuteTransactionResponse::EffectsCert(Box::new((
tx_cert.into(),
Expand Down Expand Up @@ -240,7 +240,7 @@ where
loop {
match effects_receiver.recv().await {
Ok((tx_cert, effects_cert)) => {
let tx_cert = match tx_cert.verify(&validator_state.committee.load()) {
let tx_cert = match tx_cert.verify(&validator_state.committee()) {
Err(err) => {
error!(
"received certificate from quorum driver with bad signatures! {}",
Expand Down Expand Up @@ -304,13 +304,10 @@ where
Ok(())
}
e @ Err(SuiError::TransactionInputObjectsErrors { .. }) => {
debug!(?tx_digest, "Orchestrator failed to executue transaction optimistically due to missing parents: {:?}", e);
debug!(?tx_digest, "Orchestrator failed to execute transaction optimistically due to missing parents: {:?}", e);

match node_sync_handle
.handle_parents_request(
state.committee.load().epoch,
std::iter::once(*tx_digest),
)
.handle_parents_request(state.epoch(), std::iter::once(*tx_digest))
.await?
.next()
.instrument(tracing::debug_span!(
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-core/src/unit_tests/authority_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ pub async fn send_and_confirm_transaction_with_shared(
let vote = response.signed_transaction.unwrap().into_inner();

// Collect signatures from a quorum of authorities
let committee = authority.committee.load();
let committee = authority.committee();
let certificate = CertifiedTransaction::new(
transaction.into_message(),
vec![vote.auth_sig().clone()],
Expand Down Expand Up @@ -2463,7 +2463,7 @@ fn init_certified_transaction(
authority_state.name,
&*authority_state.secret,
);
let committee = authority_state.committee.load();
let committee = authority_state.committee();
CertifiedTransaction::new(
transaction.into_message(),
vec![vote.auth_sig().clone()],
Expand Down Expand Up @@ -2660,7 +2660,7 @@ async fn make_test_transaction(

let transaction = to_sender_signed_transaction(data, sender_key);

let committee = authorities[0].committee.load();
let committee = authorities[0].committee();
let mut sigs = vec![];

for authority in authorities {
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-core/src/unit_tests/consensus_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub async fn test_certificates(authority: &AuthorityState) -> Vec<CertifiedTrans
let certificate = CertifiedTransaction::new(
transaction.into_message(),
vec![vote.auth_sig().clone()],
&authority.committee.load(),
&authority.committee(),
)
.unwrap();
certificates.push(certificate);
Expand Down

0 comments on commit 58a8ae5

Please sign in to comment.