Skip to content

Commit

Permalink
[Consensus 2.0] fix mysticeti committee member ordering (MystenLabs#1…
Browse files Browse the repository at this point in the history
…6679)

## Description 

Committee members should be ordered in the same order as the SUI
committee (protocol public key order ascending).

## Test Plan 

CI

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
akichidis authored Mar 18, 2024
1 parent 1c60177 commit 67b4c83
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 2 deletions.
1 change: 1 addition & 0 deletions consensus/config/src/committee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ impl Committee {
"Too many authorities ({})!",
authorities.len()
);

let total_stake = authorities.iter().map(|a| a.stake).sum();
assert_ne!(total_stake, 0, "Total stake cannot be zero!");
let quorum_threshold = 2 * total_stake / 3 + 1;
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-core/src/consensus_manager/mysticeti_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ impl ConsensusManagerTrait for MysticetiManager {

let name: AuthorityName = self.keypair.public().into();

let sui_committee = system_state.get_sui_committee();
let authority_index: AuthorityIndex = committee
.to_authority_index(
epoch_store
.committee()
sui_committee
.authority_index(&name)
.expect("Should have valid index for own authority") as usize,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,10 @@ impl EpochStartSystemStateTrait for EpochStartSystemStateV1 {
});
}

// Sort the authorities by their protocol (public) key in ascending order. That's the sorting
// SUI committee follows and we
authorities.sort_by(|a1, a2| a1.protocol_key.cmp(&a2.protocol_key));

ConsensusCommittee::new(self.epoch as consensus_config::Epoch, authorities)
}

Expand Down Expand Up @@ -298,3 +302,87 @@ impl EpochStartValidatorInfoV1 {
(&self.protocol_pubkey).into()
}
}

#[cfg(test)]
mod test {
use crate::base_types::SuiAddress;
use crate::committee::CommitteeTrait;
use crate::crypto::{get_key_pair, AuthorityKeyPair};
use crate::sui_system_state::epoch_start_sui_system_state::{
EpochStartSystemStateTrait, EpochStartSystemStateV1, EpochStartValidatorInfoV1,
};
use fastcrypto::traits::KeyPair;
use mysten_network::Multiaddr;
use narwhal_crypto::NetworkKeyPair;
use rand::thread_rng;
use sui_protocol_config::ProtocolVersion;

#[test]
fn test_sui_and_mysticeti_committee_are_same() {
// GIVEN
let mut active_validators = vec![];

for i in 0..10 {
let (sui_address, protocol_key): (SuiAddress, AuthorityKeyPair) = get_key_pair();
let narwhal_network_key = NetworkKeyPair::generate(&mut thread_rng());

active_validators.push(EpochStartValidatorInfoV1 {
sui_address,
protocol_pubkey: protocol_key.public().clone(),
narwhal_network_pubkey: narwhal_network_key.public().clone(),
narwhal_worker_pubkey: narwhal_network_key.public().clone(),
sui_net_address: Multiaddr::empty(),
p2p_address: Multiaddr::empty(),
narwhal_primary_address: Multiaddr::empty(),
narwhal_worker_address: Multiaddr::empty(),
voting_power: 1_000,
hostname: format!("host-{i}").to_string(),
})
}

let state = EpochStartSystemStateV1 {
epoch: 10,
protocol_version: ProtocolVersion::MAX.as_u64(),
reference_gas_price: 0,
safe_mode: false,
epoch_start_timestamp_ms: 0,
epoch_duration_ms: 0,
active_validators,
};

// WHEN
let sui_committee = state.get_sui_committee();
let mysticeti_committee = state.get_mysticeti_committee();

// THEN
// assert the validators details
assert_eq!(sui_committee.num_members(), 10);
assert_eq!(sui_committee.num_members(), mysticeti_committee.size());
assert_eq!(
sui_committee.validity_threshold(),
mysticeti_committee.validity_threshold()
);
assert_eq!(
sui_committee.quorum_threshold(),
mysticeti_committee.quorum_threshold()
);
assert_eq!(state.epoch, mysticeti_committee.epoch());

for (authority_index, mysticeti_authority) in mysticeti_committee.authorities() {
let sui_authority_name = sui_committee
.authority_by_index(authority_index.value() as u32)
.unwrap();

assert_eq!(
mysticeti_authority.protocol_key.pubkey.to_bytes(),
sui_authority_name.0,
"Mysten & SUI committee member of same index correspond to different public key"
);
assert_eq!(
mysticeti_authority.stake,
sui_committee.weight(sui_authority_name),
"Mysten & SUI committee member stake differs"
);
}
}
}

0 comments on commit 67b4c83

Please sign in to comment.