Skip to content

Commit

Permalink
Audit sui system state related TODOs
Browse files Browse the repository at this point in the history
  • Loading branch information
lxfind committed Mar 13, 2023
1 parent 472c86c commit 56ff6a5
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 18 deletions.
14 changes: 11 additions & 3 deletions crates/sui-types/src/committee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ pub type CommitteeDigest = [u8; 32];
pub struct Committee {
pub epoch: EpochId,
pub voting_rights: Vec<(AuthorityName, StakeUnit)>,
// TODO: Remove and replace with constant.
pub total_votes: StakeUnit,
expanded_keys: HashMap<AuthorityName, AuthorityPublicKey>,
index_map: HashMap<AuthorityName, usize>,
// TODO: This is no longer needed. This file needs a cleanup since we removed the optional
// cached expanded keys.
loaded: bool,
}

Expand Down Expand Up @@ -89,9 +92,14 @@ impl Committee {
) {
let expanded_keys: HashMap<AuthorityName, AuthorityPublicKey> = voting_rights
.iter()
// TODO: Verify all code path to make sure we always have valid public keys.
// e.g. when a new validator is registering themself on-chain.
.map(|(addr, _)| (*addr, (*addr).try_into().expect("Invalid Authority Key")))
.map(|(addr, _)| {
(
*addr,
(*addr)
.try_into()
.expect("Validator pubkey is always verified on-chain"),
)
})
.collect();

let index_map: HashMap<AuthorityName, usize> = voting_rights
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ impl EpochStartSystemStateTrait for EpochStartSystemStateV1 {
.collect();
Committee::new(self.epoch, voting_rights)
// unwrap is safe because we should have verified the committee on-chain.
// TODO: Make sure we actually verify it.
// MUSTFIX: Make sure we always have a valid committee.
.unwrap()
}

Expand Down
16 changes: 2 additions & 14 deletions crates/sui-types/src/sui_system_state/sui_system_state_inner_v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,12 @@ const E_METADATA_INVALID_WORKER_ADDR: u64 = 7;

/// Rust version of the Move sui::sui_system::SystemParameters type
#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)]
// TODO: Get rid of json schema once we deprecate getSuiSystemState RPC API.
#[serde(rename = "SystemParameters")]
pub struct SystemParametersV1 {
pub governance_start_epoch: u64,
pub epoch_duration_ms: u64,
}

#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)]
// TODO: Get rid of json schema once we deprecate getSuiSystemState RPC API.
#[serde(rename = "ValidatorMetadata")]
pub struct ValidatorMetadataV1 {
pub sui_address: SuiAddress,
pub protocol_pubkey_bytes: Vec<u8>,
Expand Down Expand Up @@ -122,7 +118,7 @@ impl ValidatorMetadataV1 {
let p2p_address = Multiaddr::try_from(self.p2p_address.clone())
.map_err(|_| E_METADATA_INVALID_P2P_ADDR)?;
// Also make sure that the p2p address is a valid anemo address.
// TODO: This will trigger a bunch of Move test failures today since we did not give proper
// MUSTFIX: This will trigger a bunch of Move test failures today since we did not give proper
// value for p2p address.
// multiaddr_to_anemo_address(&p2p_address).ok_or(E_METADATA_INVALID_P2P_ADDR)?;
let primary_address = Multiaddr::try_from(self.primary_address.clone())
Expand Down Expand Up @@ -235,8 +231,6 @@ impl ValidatorMetadataV1 {

/// Rust version of the Move sui::validator::Validator type
#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)]
// TODO: Get rid of json schema once we deprecate getSuiSystemState RPC API.
#[serde(rename = "Validator")]
pub struct ValidatorV1 {
metadata: ValidatorMetadataV1,
#[serde(skip)]
Expand Down Expand Up @@ -359,8 +353,6 @@ impl ValidatorV1 {

/// Rust version of the Move sui::staking_pool::StakingPool type
#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)]
// TODO: Get rid of json schema once we deprecate getSuiSystemState RPC API.
#[serde(rename = "StakingPool")]
pub struct StakingPoolV1 {
pub id: ObjectID,
pub activation_epoch: Option<u64>,
Expand All @@ -376,8 +368,6 @@ pub struct StakingPoolV1 {

/// Rust version of the Move sui::validator_set::ValidatorSet type
#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)]
// TODO: Get rid of json schema once we deprecate getSuiSystemState RPC API.
#[serde(rename = "ValidatorSet")]
pub struct ValidatorSetV1 {
pub total_stake: u64,
pub active_validators: Vec<ValidatorV1>,
Expand Down Expand Up @@ -417,8 +407,6 @@ impl SuiSystemStateInnerV1 {
}

#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)]
// TODO: Get rid of json schema once we deprecate getSuiSystemState RPC API.
#[serde(rename = "StakeSubsidy")]
pub struct StakeSubsidyV1 {
pub epoch_counter: u64,
pub balance: Balance,
Expand Down Expand Up @@ -471,7 +459,7 @@ impl SuiSystemStateTrait for SuiSystemStateInnerV1 {
CommitteeWithNetworkMetadata {
committee: Committee::new(self.epoch, voting_rights)
// unwrap is safe because we should have verified the committee on-chain.
// TODO: Make sure we actually verify it.
// MUSTFIX: Make sure we always have a valid committee
.unwrap(),
network_metadata,
}
Expand Down

0 comments on commit 56ff6a5

Please sign in to comment.