Skip to content

Commit

Permalink
Replace Vec<PK> in authority quorum with equivalent bitmap (MystenLab…
Browse files Browse the repository at this point in the history
…s#2929)

* replaced pks in AuthorityQuorumSignInfo with bitmaps

* add tests for authority pk bitmaps

* include for roaring bitmap implementation
  • Loading branch information
punwai authored Jul 7, 2022
1 parent 8c931f4 commit e9d494e
Show file tree
Hide file tree
Showing 17 changed files with 512 additions and 78 deletions.
27 changes: 27 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion crates/sui-benchmark/src/benchmark/transaction_creator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ fn make_cert(network_config: &NetworkConfig, tx: &Transaction) -> CertifiedTrans
.key_pair();
let pubx = secx.public_key_bytes();
let sig = AuthoritySignature::new(&certificate.data, secx);
certificate.auth_sign_info.signatures.push((*pubx, sig));
certificate
.auth_sign_info
.add_signature(sig, *pubx, &committee)
.unwrap();
}
certificate
}
Expand Down
22 changes: 16 additions & 6 deletions crates/sui-core/src/authority_active/checkpoint_driver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,10 +445,18 @@ where
}
Action::NewCert => {
let available_authorities: BTreeSet<_> = checkpoint
.signatory_authorities()
.filter(|a| **a != self_name)
.cloned()
.collect();
.signatory_authorities(committee)
.filter_map(|x| match x {
Ok(&a) => {
if a != self_name {
Some(Ok(a))
} else {
None
}
}
Err(e) => Some(Err(e)),
})
.collect::<SuiResult<_>>()?;
let (_, contents) = get_one_checkpoint_with_contents(
active_authority.net.load().clone(),
checkpoint.summary.sequence_number,
Expand Down Expand Up @@ -485,8 +493,10 @@ where
// We use the latest available authorities not the authorities that signed the checkpoint
// since these might be gone after the epoch they were active.
let available_authorities: BTreeSet<_> = latest_known_checkpoint
.signatory_authorities()
.cloned()
.signatory_authorities(&net.committee)
.collect::<SuiResult<BTreeSet<_>>>()?
.iter()
.map(|&&x| x)
.collect();

// Check if the latest checkpoint is merely a signed checkpoint, and if
Expand Down
15 changes: 9 additions & 6 deletions crates/sui-core/src/authority_aggregator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,10 +358,11 @@ where
let mut candidate_source_authorties: HashSet<AuthorityName> = cert
.certificate
.auth_sign_info
.signatures
.authorities(&self.committee)
.collect::<SuiResult<HashSet<_>>>()?
.iter()
.map(|(name, _)| *name)
.collect();
.map(|&&name| name)
.collect::<HashSet<_>>();

// Sample a `retries` number of distinct authorities by stake.
let mut source_authorities: Vec<AuthorityName> = Vec::new();
Expand Down Expand Up @@ -1111,7 +1112,8 @@ where
self.committee.epoch(),
transaction_ref.clone(),
state.signatures.clone(),
));
&self.committee,
)?);
}
}
// If we get back an error, then we aggregate and check
Expand Down Expand Up @@ -1369,11 +1371,12 @@ where
good_stake = stake,
"Found an effect with good stake over threshold"
);
return Ok(CertifiedTransactionEffects::new(
return CertifiedTransactionEffects::new(
certificate.auth_sign_info.epoch,
effects,
signatures,
));
&self.committee,
);
}
}

Expand Down
2 changes: 2 additions & 0 deletions crates/sui-core/src/unit_tests/authority_aggregator_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,9 @@ where
committee.epoch(),
transaction.unwrap().to_transaction(),
votes,
committee,
)
.unwrap()
}

pub async fn do_cert<A>(
Expand Down
4 changes: 4 additions & 0 deletions crates/sui-core/src/unit_tests/authority_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,9 @@ pub async fn send_and_confirm_transaction(

// Collect signatures from a quorum of authorities
let committee = authority.committee.load();
println!("{:?}", committee.voting_rights);
println!("{:?}", committee.index_map);
println!("{:?}", vote.auth_sign_info.authority);
let mut builder = SignatureAggregator::try_new(transaction, &committee).unwrap();
let certificate = builder
.append(vote.auth_sign_info.authority, vote.auth_sign_info.signature)
Expand Down Expand Up @@ -522,6 +525,7 @@ async fn test_publish_dependent_module_ok() {
bytes
};
let authority = init_state_with_objects(vec![gas_payment_object]).await;
println!("{:?}", authority.committee.load().index_map);

let data = TransactionData::new_module(
sender,
Expand Down
8 changes: 2 additions & 6 deletions crates/sui-json-rpc-api/src/rpc_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -991,12 +991,8 @@ impl Display for SuiCertifiedTransaction {
writeln!(writer, "Transaction Signature: {:?}", self.tx_signature)?;
writeln!(
writer,
"Signed Authorities : {:?}",
self.auth_sign_info
.signatures
.iter()
.map(|(name, _)| name)
.collect::<Vec<_>>()
"Signed Authorities Bitmap: {:?}",
self.auth_sign_info.signers_map
)?;
write!(writer, "{}", &self.data)?;
write!(f, "{}", writer)
Expand Down
21 changes: 6 additions & 15 deletions crates/sui-open-rpc/spec/openrpc.json
Original file line number Diff line number Diff line change
Expand Up @@ -1267,7 +1267,8 @@
"type": "object",
"required": [
"epoch",
"signatures"
"signatures",
"signers_map"
],
"properties": {
"epoch": {
Expand All @@ -1278,18 +1279,11 @@
"signatures": {
"type": "array",
"items": {
"type": "array",
"items": [
{
"$ref": "#/components/schemas/PublicKeyBytes"
},
{
"$ref": "#/components/schemas/AuthoritySignature"
}
],
"maxItems": 2,
"minItems": 2
"$ref": "#/components/schemas/AuthoritySignature"
}
},
"signers_map": {
"$ref": "#/components/schemas/Base64"
}
}
},
Expand Down Expand Up @@ -2352,9 +2346,6 @@
}
]
},
"PublicKeyBytes": {
"$ref": "#/components/schemas/Base64"
},
"PublishResponse": {
"type": "object",
"required": [
Expand Down
1 change: 1 addition & 0 deletions crates/sui-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ schemars ="0.8.10"
tonic = "0.7"
strum = "^0.24"
strum_macros = "^0.24"
roaring = "0.9.0"

# This version is incompatible with ed25519-dalek
rand_latest = { version = "0.8.5", package = "rand" }
Expand Down
61 changes: 54 additions & 7 deletions crates/sui-types/src/committee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ pub type StakeUnit = u64;
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct Committee {
pub epoch: EpochId,
voting_rights: Vec<(AuthorityName, StakeUnit)>,
pub voting_rights: Vec<(AuthorityName, StakeUnit)>,
pub total_votes: StakeUnit,
// Note: this is a derived structure, no need to store.
#[serde(skip)]
expanded_keys: HashMap<AuthorityName, PublicKey>,
#[serde(skip)]
pub index_map: HashMap<AuthorityName, usize>,
#[serde(skip)]
loaded: bool,
}

impl Committee {
Expand Down Expand Up @@ -55,20 +59,63 @@ impl Committee {

voting_rights.sort_by_key(|(a, _)| *a);
let total_votes = voting_rights.iter().map(|(_, votes)| *votes).sum();
let expanded_keys: HashMap<_, _> = 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")))
.collect();

let (expanded_keys, index_map) = Self::load_inner(&voting_rights);

Ok(Committee {
epoch,
voting_rights,
total_votes,
expanded_keys,
index_map,
loaded: true,
})
}

// We call this if these have not yet been computed
pub fn load_inner(
voting_rights: &[(AuthorityName, StakeUnit)],
) -> (
HashMap<AuthorityName, PublicKey>,
HashMap<AuthorityName, usize>,
) {
let expanded_keys: HashMap<AuthorityName, PublicKey> = 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")))
.collect();

let index_map: HashMap<AuthorityName, usize> = voting_rights
.iter()
.enumerate()
.map(|(index, (addr, _))| (*addr, index))
.collect();
(expanded_keys, index_map)
}

pub fn reload_fields(&mut self) {
let (expanded_keys, index_map) = Committee::load_inner(&self.voting_rights);
self.expanded_keys = expanded_keys;
self.index_map = index_map;
self.loaded = true;
}

pub fn authority_index(&self, author: &AuthorityName) -> Option<u32> {
if !self.loaded {
return self
.voting_rights
.iter()
.position(|(a, _)| a == author)
.map(|i| i as u32);
}
self.index_map.get(author).map(|i| *i as u32)
}

pub fn authority_by_index(&self, index: u32) -> Option<&AuthorityName> {
self.voting_rights.get(index as usize).map(|(name, _)| name)
}

pub fn epoch(&self) -> EpochId {
self.epoch
}
Expand Down
Loading

0 comments on commit e9d494e

Please sign in to comment.