Skip to content

Commit

Permalink
Change fastcrypto types in SystemMessage to opaque bytes (MystenL…
Browse files Browse the repository at this point in the history
…abs#14706)

## Description 

This avoids the cost of deserializing the messages in places where the
contents are not needed.
  • Loading branch information
aschran authored Nov 7, 2023
1 parent d42437d commit 2ef6417
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 21 deletions.
29 changes: 27 additions & 2 deletions narwhal/primary/src/primary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use fastcrypto::{
signature_service::SignatureService,
traits::{KeyPair as _, ToFromBytes},
};
use fastcrypto_tbls::{tbls::ThresholdBls, types::ThresholdBls12381MinSig};
use mysten_metrics::metered_channel::{channel_with_total, Receiver, Sender};
use mysten_metrics::monitored_scope;
use mysten_network::{multiaddr::Protocol, Multiaddr};
Expand Down Expand Up @@ -67,8 +68,9 @@ use types::{
now, validate_received_certificate_version, Certificate, CertificateAPI, CertificateDigest,
FetchCertificatesRequest, FetchCertificatesResponse, Header, HeaderAPI, MetadataAPI,
PreSubscribedBroadcastSender, PrimaryToPrimary, PrimaryToPrimaryServer, RequestVoteRequest,
RequestVoteResponse, Round, SendCertificateRequest, SendCertificateResponse, Vote, VoteInfoAPI,
WorkerOthersBatchMessage, WorkerOwnBatchMessage, WorkerToPrimary, WorkerToPrimaryServer,
RequestVoteResponse, Round, SendCertificateRequest, SendCertificateResponse, SystemMessage,
Vote, VoteInfoAPI, WorkerOthersBatchMessage, WorkerOwnBatchMessage, WorkerToPrimary,
WorkerToPrimaryServer,
};

#[cfg(test)]
Expand Down Expand Up @@ -713,6 +715,29 @@ impl PrimaryReceiverHandler {
DagError::HeaderRequiresQuorum(header.digest())
);

// Verify any system messages present in the header.
type DkgG = <ThresholdBls12381MinSig as ThresholdBls>::Public;
for m in header.system_messages().iter() {
match m {
SystemMessage::DkgMessage(bytes) => {
let msg: fastcrypto_tbls::dkg::Message<DkgG, DkgG> =
bcs::from_bytes(bytes).map_err(|_| DagError::InvalidSystemMessage)?;
ensure!(
msg.sender == header.author().0,
DagError::InvalidSystemMessage
);
}
SystemMessage::DkgConfirmation(bytes) => {
let conf: fastcrypto_tbls::dkg::Confirmation<DkgG> =
bcs::from_bytes(bytes).map_err(|_| DagError::InvalidSystemMessage)?;
ensure!(
conf.sender == header.author().0,
DagError::InvalidSystemMessage
);
}
}
}

// Synchronize all batches referenced in the header.
self.synchronizer
.sync_header_batches(header, /* max_age */ 0)
Expand Down
28 changes: 24 additions & 4 deletions narwhal/primary/src/state_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
use config::{AuthorityIdentifier, ChainIdentifier, Committee};
use crypto::RandomnessPrivateKey;
use fastcrypto::groups;
use fastcrypto_tbls::tbls::ThresholdBls;
use fastcrypto_tbls::types::ThresholdBls12381MinSig;
use fastcrypto_tbls::{dkg, nodes};
use mysten_metrics::metered_channel::{Receiver, Sender};
use mysten_metrics::spawn_logged_monitored_task;
Expand Down Expand Up @@ -120,7 +122,9 @@ impl RandomnessState {
let msg = self.party.create_message(&mut rand::thread_rng());
info!("random beacon: sending DKG Message: {msg:?}");
let _ = tx_system_messages
.send(SystemMessage::DkgMessage(msg))
.send(SystemMessage::DkgMessage(
bcs::to_bytes(&msg).expect("message serialization should not fail"),
))
.await;
}

Expand Down Expand Up @@ -165,7 +169,10 @@ impl RandomnessState {
);
self.used_messages = Some(used_msgs);
let _ = tx_system_messages
.send(SystemMessage::DkgConfirmation(conf))
.send(SystemMessage::DkgConfirmation(
bcs::to_bytes(&conf)
.expect("confirmation serialization should not fail"),
))
.await;
}
Err(fastcrypto::error::FastCryptoError::NotEnoughInputs) => (), // wait for more input
Expand Down Expand Up @@ -262,9 +269,22 @@ impl StateHandler {
for certificate in certificates {
let header = certificate.header();
for message in header.system_messages() {
type DkgG = <ThresholdBls12381MinSig as ThresholdBls>::Public;
match message {
SystemMessage::DkgMessage(msg) => randomness_state.add_message(msg.clone()),
SystemMessage::DkgConfirmation(conf) => {
SystemMessage::DkgMessage(bytes) => {
let msg: fastcrypto_tbls::dkg::Message<DkgG, DkgG> = bcs::from_bytes(
bytes,
)
.expect(
"DKG message deserialization from certified header should not fail",
);
randomness_state.add_message(msg.clone());
}
SystemMessage::DkgConfirmation(bytes) => {
let conf: fastcrypto_tbls::dkg::Confirmation<DkgG> =
bcs::from_bytes(bytes).expect(
"DKG confirmation deserialization from certified header should not fail",
);
randomness_state.add_confirmation(conf.clone())
}
}
Expand Down
5 changes: 4 additions & 1 deletion narwhal/primary/src/tests/state_handler_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ async fn start_dkg() {
let (tx_system_messages, mut rx_system_messages) = test_utils::test_channel!(1);
randomness_state.start_dkg(&tx_system_messages).await;

type DkgG = <ThresholdBls12381MinSig as ThresholdBls>::Public;
let dkg_message = rx_system_messages.recv().await.unwrap();
match dkg_message {
SystemMessage::DkgMessage(msg) => {
SystemMessage::DkgMessage(bytes) => {
let msg: fastcrypto_tbls::dkg::Message<DkgG, DkgG> = bcs::from_bytes(&bytes)
.expect("DKG message deserialization from certified header should not fail");
assert_eq!(msg.sender, name.0);
}
_ => panic!("wrong type of message sent"),
Expand Down
20 changes: 6 additions & 14 deletions narwhal/types/src/primary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use fastcrypto::{
signature_service::SignatureService,
traits::{AggregateAuthenticator, Signer, VerifyingKey},
};
use fastcrypto_tbls::{tbls::ThresholdBls, types::ThresholdBls12381MinSig};
use indexmap::IndexMap;
use mysten_util_mem::MallocSizeOf;
use once_cell::sync::{Lazy, OnceCell};
Expand Down Expand Up @@ -384,17 +383,12 @@ impl Hash<{ crypto::DIGEST_LENGTH }> for BatchV2 {
pub enum SystemMessage {
// DKG is used to generate keys for use in the random beacon protocol.
// `DkgMessage` is sent out at start-of-epoch to initiate the process.
DkgMessage(
fastcrypto_tbls::dkg::Message<
<ThresholdBls12381MinSig as ThresholdBls>::Public,
<ThresholdBls12381MinSig as ThresholdBls>::Public,
>,
),
// Contents are a serialized `fastcrypto_tbls::dkg::Message`.
DkgMessage(Vec<u8>),
// `DkgConfirmation` is the second DKG message, sent as soon as a threshold amount of
// `DkgMessages` have been received locally, to complete the key generation process.
DkgConfirmation(
fastcrypto_tbls::dkg::Confirmation<<ThresholdBls12381MinSig as ThresholdBls>::Public>,
),
// Contents are a serialized `fastcrypto_tbls::dkg::Confirmation`.
DkgConfirmation(Vec<u8>),
}

#[derive(Clone, Deserialize, MallocSizeOf, Serialize)]
Expand Down Expand Up @@ -804,14 +798,12 @@ impl HeaderV2 {
let mut has_dkg_confirmation = false;
for m in self.system_messages.iter() {
match m {
SystemMessage::DkgMessage(msg) => {
ensure!(msg.sender == self.author.0, DagError::InvalidSystemMessage);
SystemMessage::DkgMessage(_) => {
// A header must have no more than one DkgMessage.
ensure!(!has_dkg_message, DagError::DuplicateSystemMessage);
has_dkg_message = true;
}
SystemMessage::DkgConfirmation(conf) => {
ensure!(conf.sender == self.author.0, DagError::InvalidSystemMessage);
SystemMessage::DkgConfirmation(_) => {
// A header must have no more than one DkgConfirmation.
ensure!(!has_dkg_confirmation, DagError::DuplicateSystemMessage);
has_dkg_confirmation = true;
Expand Down

0 comments on commit 2ef6417

Please sign in to comment.