Skip to content

Commit

Permalink
Use bytes representation for Certificate::aggregate_signature (Mysten…
Browse files Browse the repository at this point in the history
…Labs#9697)

## Description 

Profiling has shown that validators spend a significant amount of CPU
resources deserialising the `aggregate_signature` field in NW's
`Certificate` object when read from disk. This is because signatures by
default are stored as compressed points, so deserialisation needs to
decompress points which is a non-trivial computation. However, in most
cases the signature does not need to be verified, and in these cases the
binary representation suffices.

This PR replaces the `aggregate_signature` field in NW's `Certificate`
object with its `AsBytes` representation, and the signature is only
decompressed when the signature actually have to be verified. According
to recently produced perf flame graphs, this could save ~2% CPU usage
for validators.

## Test Plan 

Unit tests.

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

- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [X] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [X] necessitate either a data wipe or data migration
  • Loading branch information
jonas-lj authored Mar 22, 2023
1 parent 684a689 commit 9e0de41
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 15 deletions.
6 changes: 3 additions & 3 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,8 @@ move-prover-boogie-backend = { git = "https://github.com/move-language/move", re
move-stackless-bytecode = { git = "https://github.com/move-language/move", rev = "3ebc7eef2c41d5360cc5c0ea08ee9a4002cdba54" }
move-symbol-pool = { git = "https://github.com/move-language/move", rev = "3ebc7eef2c41d5360cc5c0ea08ee9a4002cdba54" }

fastcrypto = { git = "https://github.com/MystenLabs/fastcrypto", rev = "b42ed07f24ac2b98afdb80b1d9cceb6e75d36a42" }
fastcrypto-zkp = { git = "https://github.com/MystenLabs/fastcrypto", rev = "b42ed07f24ac2b98afdb80b1d9cceb6e75d36a42", package = "fastcrypto-zkp" }
fastcrypto = { git = "https://github.com/MystenLabs/fastcrypto", rev = "c2f79b1807bff7d09517b631191b61f2614c641c" }
fastcrypto-zkp = { git = "https://github.com/MystenLabs/fastcrypto", rev = "c2f79b1807bff7d09517b631191b61f2614c641c", package = "fastcrypto-zkp" }

# anemo dependencies
anemo = { git = "https://github.com/mystenlabs/anemo.git", rev = "4ebf4a86952827ff0fcce6a2d8a80f42f34efed9" }
Expand Down
1 change: 1 addition & 0 deletions crates/mysten-util-mem/src/external_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ malloc_size_of_is_0!(fastcrypto::bls12381::min_sig::BLS12381PublicKey);
malloc_size_of_is_0!(fastcrypto::bls12381::min_sig::BLS12381PublicKeyAsBytes);
malloc_size_of_is_0!(fastcrypto::bls12381::min_sig::BLS12381Signature);
malloc_size_of_is_0!(fastcrypto::bls12381::min_sig::BLS12381AggregateSignature);
malloc_size_of_is_0!(fastcrypto::bls12381::min_sig::BLS12381AggregateSignatureAsBytes);
malloc_size_of_is_0!(fastcrypto::bls12381::min_pk::BLS12381PublicKey);
malloc_size_of_is_0!(fastcrypto::bls12381::min_pk::BLS12381Signature);
malloc_size_of_is_0!(fastcrypto::bls12381::min_pk::BLS12381AggregateSignature);
Expand Down
10 changes: 5 additions & 5 deletions crates/workspace-hack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,8 @@ expect-test = { version = "1", default-features = false }
eyre = { version = "0.6" }
fail-9fbad63c4bcf4a8f = { package = "fail", version = "0.4", default-features = false }
fail-d8f496e17d97b5cb = { package = "fail", version = "0.5", default-features = false }
fastcrypto = { git = "https://github.com/MystenLabs/fastcrypto", rev = "b42ed07f24ac2b98afdb80b1d9cceb6e75d36a42", features = ["copy_key"] }
fastcrypto-zkp = { git = "https://github.com/MystenLabs/fastcrypto", rev = "b42ed07f24ac2b98afdb80b1d9cceb6e75d36a42", default-features = false }
fastcrypto = { git = "https://github.com/MystenLabs/fastcrypto", rev = "c2f79b1807bff7d09517b631191b61f2614c641c", features = ["copy_key"] }
fastcrypto-zkp = { git = "https://github.com/MystenLabs/fastcrypto", rev = "c2f79b1807bff7d09517b631191b61f2614c641c", default-features = false }
fastrand = { version = "1", default-features = false }
fd-lock = { version = "3", default-features = false }
fdlimit = { version = "0.2", default-features = false }
Expand Down Expand Up @@ -829,9 +829,9 @@ expect-test = { version = "1", default-features = false }
eyre = { version = "0.6" }
fail-9fbad63c4bcf4a8f = { package = "fail", version = "0.4", default-features = false }
fail-d8f496e17d97b5cb = { package = "fail", version = "0.5", default-features = false }
fastcrypto = { git = "https://github.com/MystenLabs/fastcrypto", rev = "b42ed07f24ac2b98afdb80b1d9cceb6e75d36a42", features = ["copy_key"] }
fastcrypto-derive = { git = "https://github.com/MystenLabs/fastcrypto", rev = "b42ed07f24ac2b98afdb80b1d9cceb6e75d36a42", default-features = false }
fastcrypto-zkp = { git = "https://github.com/MystenLabs/fastcrypto", rev = "b42ed07f24ac2b98afdb80b1d9cceb6e75d36a42", default-features = false }
fastcrypto = { git = "https://github.com/MystenLabs/fastcrypto", rev = "c2f79b1807bff7d09517b631191b61f2614c641c", features = ["copy_key"] }
fastcrypto-derive = { git = "https://github.com/MystenLabs/fastcrypto", rev = "c2f79b1807bff7d09517b631191b61f2614c641c", default-features = false }
fastcrypto-zkp = { git = "https://github.com/MystenLabs/fastcrypto", rev = "c2f79b1807bff7d09517b631191b61f2614c641c", default-features = false }
fastrand = { version = "1", default-features = false }
fd-lock = { version = "3", default-features = false }
fdlimit = { version = "0.2", default-features = false }
Expand Down
1 change: 1 addition & 0 deletions narwhal/crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub type PublicKey = bls12381::min_sig::BLS12381PublicKey;
pub type PublicKeyBytes = bls12381::min_sig::BLS12381PublicKeyAsBytes;
pub type Signature = bls12381::min_sig::BLS12381Signature;
pub type AggregateSignature = bls12381::min_sig::BLS12381AggregateSignature;
pub type AggregateSignatureBytes = bls12381::min_sig::BLS12381AggregateSignatureAsBytes;
pub type PrivateKey = bls12381::min_sig::BLS12381PrivateKey;
pub type KeyPair = bls12381::min_sig::BLS12381KeyPair;

Expand Down
12 changes: 7 additions & 5 deletions narwhal/types/src/primary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ use crate::{
use bytes::Bytes;
use config::{Committee, Epoch, Stake, WorkerCache, WorkerId, WorkerInfo};
use crypto::{
to_intent_message, AggregateSignature, NarwhalAuthorityAggregateSignature,
NarwhalAuthoritySignature, PublicKey, PublicKeyBytes, Signature,
to_intent_message, AggregateSignature, AggregateSignatureBytes,
NarwhalAuthorityAggregateSignature, NarwhalAuthoritySignature, PublicKey, PublicKeyBytes,
Signature,
};
use dag::node_dag::Affiliated;
use derive_builder::Builder;
Expand Down Expand Up @@ -499,7 +500,7 @@ impl PartialEq for Vote {
#[derive(Clone, Serialize, Deserialize, Default, MallocSizeOf)]
pub struct Certificate {
pub header: Header,
aggregated_signature: AggregateSignature,
aggregated_signature: AggregateSignatureBytes,
#[serde_as(as = "NarwhalBitmap")]
signed_authorities: roaring::RoaringBitmap,
pub metadata: Metadata,
Expand Down Expand Up @@ -605,7 +606,7 @@ impl Certificate {

Ok(Certificate {
header,
aggregated_signature,
aggregated_signature: AggregateSignatureBytes::from(&aggregated_signature),
signed_authorities,
metadata: Metadata::default(),
})
Expand Down Expand Up @@ -669,7 +670,8 @@ impl Certificate {

// Verify the signatures
let certificate_digest: Digest<{ crypto::DIGEST_LENGTH }> = Digest::from(self.digest());
self.aggregated_signature
AggregateSignature::try_from(&self.aggregated_signature)
.map_err(|_| DagError::InvalidSignature)?
.verify_secure(&to_intent_message(certificate_digest), &pks[..])
.map_err(|_| DagError::InvalidSignature)?;

Expand Down

0 comments on commit 9e0de41

Please sign in to comment.