diff --git a/consensus/consensus-types/src/order_vote.rs b/consensus/consensus-types/src/order_vote.rs index ecf8dbf4901..d56afcc0bbb 100644 --- a/consensus/consensus-types/src/order_vote.rs +++ b/consensus/consensus-types/src/order_vote.rs @@ -4,7 +4,7 @@ use crate::common::Author; use anyhow::{ensure, Context}; -use aptos_crypto::{bls12381, HashValue}; +use aptos_crypto::{bls12381, CryptoMaterialError, HashValue}; use aptos_short_hex_str::AsShortHexStr; use aptos_types::{ ledger_info::{LedgerInfo, SignatureWithStatus}, @@ -63,8 +63,8 @@ impl OrderVote { &self.ledger_info } - pub fn signature(&self) -> &bls12381::Signature { - self.signature.signature() + pub fn signature(&self) -> Result { + self.signature.recover_group_element() } // Question: SignatureWithStatus has interior mutability. Is it okay to expose this? diff --git a/consensus/consensus-types/src/pipeline/commit_vote.rs b/consensus/consensus-types/src/pipeline/commit_vote.rs index 21c466038e4..34e7b139070 100644 --- a/consensus/consensus-types/src/pipeline/commit_vote.rs +++ b/consensus/consensus-types/src/pipeline/commit_vote.rs @@ -79,9 +79,10 @@ impl CommitVote { &self.ledger_info } - /// Return the signature of the vote - pub fn signature(&self) -> &bls12381::Signature { - self.signature.signature() + /// Recover the group element of the commit-vote signature. + /// LedgerInfo matching must use [`Self::ledger_info`] / [`Self::signature_with_status`]. + pub fn signature(&self) -> Result { + self.signature.recover_group_element() } /// Returns the signature along with the verification status of the signature. diff --git a/consensus/consensus-types/src/proof_of_store.rs b/consensus/consensus-types/src/proof_of_store.rs index e8952446950..840d57e8f18 100644 --- a/consensus/consensus-types/src/proof_of_store.rs +++ b/consensus/consensus-types/src/proof_of_store.rs @@ -280,8 +280,8 @@ impl SignedBatchInfo { Ok(validator.optimistic_verify(self.signer, &self.info, &self.signature)?) } - pub fn signature(&self) -> &bls12381::Signature { - self.signature.signature() + pub fn signature(&self) -> Result { + self.signature.recover_group_element() } pub fn signature_with_status(&self) -> &SignatureWithStatus { diff --git a/consensus/consensus-types/src/vote.rs b/consensus/consensus-types/src/vote.rs index 1370fc57713..c3254d36e5f 100644 --- a/consensus/consensus-types/src/vote.rs +++ b/consensus/consensus-types/src/vote.rs @@ -108,9 +108,10 @@ impl Vote { &self.ledger_info } - /// Return the signature of the vote - pub fn signature(&self) -> &bls12381::Signature { - self.signature.signature() + /// Recover the group element of the vote signature. + /// Callers that only match `LedgerInfo` should use [`Self::signature_with_status`]. + pub fn signature(&self) -> Result { + self.signature.recover_group_element() } pub fn signature_with_status(&self) -> &SignatureWithStatus { diff --git a/consensus/safety-rules/src/test_utils.rs b/consensus/safety-rules/src/test_utils.rs index ce161c0a5fb..87fd1d28c71 100644 --- a/consensus/safety-rules/src/test_utils.rs +++ b/consensus/safety-rules/src/test_utils.rs @@ -173,7 +173,7 @@ pub fn make_proposal_with_parent_and_overrides( PartialSignatures::empty(), ); - ledger_info_with_signatures.add_signature(vote.author(), vote.signature().clone()); + ledger_info_with_signatures.add_signature(vote.author(), vote.signature().unwrap()); let qc = QuorumCert::new( vote_data, diff --git a/consensus/src/pending_order_votes.rs b/consensus/src/pending_order_votes.rs index 397e3f0f314..9323c8cb2b0 100644 --- a/consensus/src/pending_order_votes.rs +++ b/consensus/src/pending_order_votes.rs @@ -288,14 +288,14 @@ mod tests { li.clone(), signers[0].sign(&li).expect("Unable to sign ledger info"), ); - partial_signatures.add_signature(signers[0].author(), vote_0.signature().clone()); + partial_signatures.add_signature(signers[0].author(), vote_0.signature().unwrap()); let vote_1 = OrderVote::new_with_signature( signers[1].author(), li.clone(), signers[1].sign(&li).expect("Unable to sign ledger info"), ); - partial_signatures.add_signature(signers[1].author(), vote_1.signature().clone()); + partial_signatures.add_signature(signers[1].author(), vote_1.signature().unwrap()); let vote_2 = OrderVote::new_with_signature( signers[2].author(), @@ -308,7 +308,7 @@ mod tests { li.clone(), signers[3].sign(&li).expect("Unable to sign ledger info"), ); - partial_signatures.add_signature(signers[3].author(), vote_3.signature().clone()); + partial_signatures.add_signature(signers[3].author(), vote_3.signature().unwrap()); let vote_4 = OrderVote::new_with_signature( signers[4].author(), diff --git a/consensus/src/pending_votes.rs b/consensus/src/pending_votes.rs index 300ecd67fae..f4892f10665 100644 --- a/consensus/src/pending_votes.rs +++ b/consensus/src/pending_votes.rs @@ -717,7 +717,7 @@ mod tests { pending_votes.insert_vote(&vote_0, &validator_verifier), VoteReceptionResult::VoteAdded(1) ); - partial_sigs.add_signature(signers[0].author(), vote_0.signature().clone()); + partial_sigs.add_signature(signers[0].author(), vote_0.signature().unwrap()); // same author voting for the same thing -> DuplicateVote assert_eq!( @@ -729,7 +729,7 @@ mod tests { pending_votes.insert_vote(&vote_1, &validator_verifier), VoteReceptionResult::VoteAdded(2) ); - partial_sigs.add_signature(signers[1].author(), vote_1.signature().clone()); + partial_sigs.add_signature(signers[1].author(), vote_1.signature().unwrap()); assert_eq!(validator_verifier.pessimistic_verify_set().len(), 0); @@ -750,7 +750,7 @@ mod tests { }, } - partial_sigs.add_signature(signers[3].author(), vote_3.signature().clone()); + partial_sigs.add_signature(signers[3].author(), vote_3.signature().unwrap()); let aggregated_sig = validator_verifier .aggregate_signatures(partial_sigs.signatures_iter()) .unwrap(); diff --git a/consensus/src/pipeline/buffer_item.rs b/consensus/src/pipeline/buffer_item.rs index 6840b26e43d..d744be0e8da 100644 --- a/consensus/src/pipeline/buffer_item.rs +++ b/consensus/src/pipeline/buffer_item.rs @@ -548,23 +548,23 @@ mod test { let mut partial_signatures = BTreeMap::new(); partial_signatures.insert( validator_signers[0].author(), - commit_votes[0].signature().clone(), + commit_votes[0].signature().unwrap(), ); partial_signatures.insert( validator_signers[1].author(), - commit_votes[1].signature().clone(), + commit_votes[1].signature().unwrap(), ); partial_signatures.insert( validator_signers[2].author(), - commit_votes[2].signature().clone(), + commit_votes[2].signature().unwrap(), ); partial_signatures.insert( validator_signers[3].author(), - commit_votes[3].signature().clone(), + commit_votes[3].signature().unwrap(), ); partial_signatures.insert( validator_signers[4].author(), - commit_votes[4].signature().clone(), + commit_votes[4].signature().unwrap(), ); let li_with_sig = validator_verifier .aggregate_signatures(partial_signatures.iter()) @@ -653,23 +653,23 @@ mod test { let mut partial_signatures = BTreeMap::new(); partial_signatures.insert( validator_signers[0].author(), - commit_votes[0].signature().clone(), + commit_votes[0].signature().unwrap(), ); partial_signatures.insert( validator_signers[1].author(), - commit_votes[1].signature().clone(), + commit_votes[1].signature().unwrap(), ); partial_signatures.insert( validator_signers[2].author(), - commit_votes[2].signature().clone(), + commit_votes[2].signature().unwrap(), ); partial_signatures.insert( validator_signers[4].author(), - commit_votes[4].signature().clone(), + commit_votes[4].signature().unwrap(), ); partial_signatures.insert( validator_signers[6].author(), - commit_votes[6].signature().clone(), + commit_votes[6].signature().unwrap(), ); let li_with_sig = validator_verifier .aggregate_signatures(partial_signatures.iter()) diff --git a/consensus/src/pipeline/signing_phase.rs b/consensus/src/pipeline/signing_phase.rs index 74bf4d208c0..c7459b447d5 100644 --- a/consensus/src/pipeline/signing_phase.rs +++ b/consensus/src/pipeline/signing_phase.rs @@ -85,8 +85,11 @@ impl StatelessPipeline for SigningPhase { fut.commit_vote_fut .clone() .await - .map(|vote| vote.signature().clone()) .map_err(|e| Error::InternalError(e.to_string())) + .and_then(|vote| { + vote.signature() + .map_err(|e| Error::InternalError(e.to_string())) + }) } else { self.safety_rule_handle .sign_commit_vote(ordered_ledger_info, commit_ledger_info.clone()) diff --git a/types/src/aggregate_signature.rs b/types/src/aggregate_signature.rs index 697b3f0074b..7fe8e599e2a 100644 --- a/types/src/aggregate_signature.rs +++ b/types/src/aggregate_signature.rs @@ -1,8 +1,9 @@ // Copyright © Aptos Foundation // SPDX-License-Identifier: Apache-2.0 +use crate::wire_bls::WireBlsSignature; use aptos_bitvec::BitVec; -use aptos_crypto::bls12381; +use aptos_crypto::{bls12381, CryptoMaterialError}; use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher}; use move_core_types::account_address::AccountAddress; use serde::{Deserialize, Serialize}; @@ -12,10 +13,14 @@ use std::collections::BTreeMap; /// it stores a bit mask representing the set of validators participating in the signing process /// and the multi-signature/aggregated signature itself, /// which was aggregated from these validators' partial BLS signatures. +/// +/// The signature payload stays in compressed wire form so a later +/// `LedgerInfo` equality check can inspect bitmask and commit info without +/// paying G2 decompression. Verification paths call [`Self::try_group_element`]. #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, CryptoHasher, BCSCryptoHash)] pub struct AggregateSignature { validator_bitmask: BitVec, - sig: Option, + sig: Option, } impl AggregateSignature { @@ -25,7 +30,7 @@ impl AggregateSignature { ) -> Self { Self { validator_bitmask, - sig: aggregated_signature, + sig: aggregated_signature.map(WireBlsSignature::from), } } @@ -61,8 +66,26 @@ impl AggregateSignature { self.validator_bitmask.count_ones() as usize } - pub fn sig(&self) -> &Option { - &self.sig + /// Compressed payload, if present. Does not recover a group element. + pub fn wire_sig(&self) -> Option<&WireBlsSignature> { + self.sig.as_ref() + } + + /// Recover the aggregated group element. `Ok(None)` means no signature + /// was stored; `Err` means the 96-byte payload is not a G2 point. + pub fn try_group_element(&self) -> Result, CryptoMaterialError> { + match self.sig { + None => Ok(None), + Some(wire) => wire.recover_group_element().map(Some), + } + } + + /// Historical accessor. Recovers the group element when the payload is a + /// valid point; unrecoverable 96-byte payloads are reported as `None`. + /// Verify paths should prefer [`Self::try_group_element`] to distinguish + /// "missing" from "malformed". + pub fn sig(&self) -> Option { + self.try_group_element().ok().flatten() } } diff --git a/types/src/ledger_info.rs b/types/src/ledger_info.rs index facbe165fd6..029703a6da3 100644 --- a/types/src/ledger_info.rs +++ b/types/src/ledger_info.rs @@ -11,10 +11,12 @@ use crate::{ on_chain_config::ValidatorSet, transaction::Version, validator_verifier::{ValidatorVerifier, VerifyError}, + wire_bls::WireBlsSignature, }; use aptos_crypto::{ bls12381, hash::{CryptoHash, HashValue}, + CryptoMaterialError, }; use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher}; use derivative::Derivative; @@ -405,7 +407,9 @@ impl LedgerInfoWithVerifiedSignatures { #[derive(Clone, Debug, Derivative)] #[derivative(PartialEq, Eq)] pub struct SignatureWithStatus { - signature: bls12381::Signature, + /// Compressed encoding. Equality is on these bytes, matching the old + /// `bls12381::Signature` `PartialEq` (which compared `to_bytes()`). + signature: WireBlsSignature, #[derivative(PartialEq = "ignore")] // false if the signature not verified. // true if the signature is verified. @@ -417,13 +421,25 @@ impl SignatureWithStatus { self.verification_status.store(true, Ordering::SeqCst); } - pub fn signature(&self) -> &bls12381::Signature { + /// Compressed payload. Safe to call while matching commit votes to a + /// later full `LedgerInfo`; does not decompress. + pub fn wire_signature(&self) -> &WireBlsSignature { &self.signature } + /// Recover the group element. Call this only on verify / aggregate paths. + pub fn recover_group_element(&self) -> Result { + self.signature.recover_group_element() + } + + /// Historical name for the recovered group element. + pub fn signature(&self) -> Result { + self.recover_group_element() + } + pub fn from(signature: bls12381::Signature) -> Self { Self { - signature, + signature: WireBlsSignature::from(signature), verification_status: Arc::new(AtomicBool::new(false)), } } @@ -447,8 +463,11 @@ impl<'de> Deserialize<'de> for SignatureWithStatus { where D: serde::Deserializer<'de>, { - let signature = bls12381::Signature::deserialize(deserializer)?; - Ok(SignatureWithStatus::from(signature)) + let signature = WireBlsSignature::deserialize(deserializer)?; + Ok(SignatureWithStatus { + signature, + verification_status: Arc::new(AtomicBool::new(false)), + }) } } @@ -521,11 +540,19 @@ impl SignatureAggregator { ) -> Result { self.check_voting_power(verifier, true)?; - let all_signatures = self + // Skip payloads that are not G2 points. `check_voting_power` still + // counted every stored voter; verification of the resulting bitmask + // (or a later filter pass) drops the unrecoverable authors. + let recovered: Vec<(AccountAddress, bls12381::Signature)> = self .signatures .iter() - .map(|(voter, sig)| (voter, sig.signature())); - verifier.aggregate_signatures(all_signatures) + .filter_map(|(voter, sig)| { + sig.recover_group_element() + .ok() + .map(|point| (*voter, point)) + }) + .collect(); + verifier.aggregate_signatures(recovered.iter().map(|(voter, sig)| (voter, sig))) } fn filter_invalid_signatures(&mut self, verifier: &ValidatorVerifier) { @@ -611,11 +638,11 @@ mod tests { fn test_signature_with_status_bcs() { let signature = bls12381::Signature::dummy_signature(); let signature_with_status_1 = SignatureWithStatus { - signature: signature.clone(), + signature: WireBlsSignature::from(&signature), verification_status: Arc::new(AtomicBool::new(true)), }; let signature_with_status_2 = SignatureWithStatus { - signature: signature.clone(), + signature: WireBlsSignature::from(&signature), verification_status: Arc::new(AtomicBool::new(false)), }; let serialized_signature_with_status_1 = @@ -623,11 +650,18 @@ mod tests { let serialized_signature_with_status_2 = bcs::to_bytes(&signature_with_status_2).expect("Failed to serialize signature"); assert!(serialized_signature_with_status_1 == serialized_signature_with_status_2); + assert_eq!( + serialized_signature_with_status_1, + bcs::to_bytes(&signature).expect("Failed to serialize raw signature") + ); let deserialized_signature_with_status: SignatureWithStatus = bcs::from_bytes(&serialized_signature_with_status_1) .expect("Failed to deserialize signature"); - assert_eq!(*deserialized_signature_with_status.signature(), signature); + assert_eq!( + deserialized_signature_with_status.signature().unwrap(), + signature + ); assert!(!deserialized_signature_with_status.is_verified()); } @@ -635,11 +669,11 @@ mod tests { fn test_signature_with_status_serde() { let signature = bls12381::Signature::dummy_signature(); let signature_with_status_1 = SignatureWithStatus { - signature: signature.clone(), + signature: WireBlsSignature::from(&signature), verification_status: Arc::new(AtomicBool::new(true)), }; let signature_with_status_2 = SignatureWithStatus { - signature: signature.clone(), + signature: WireBlsSignature::from(&signature), verification_status: Arc::new(AtomicBool::new(false)), }; let serialized_signature_with_status_1 = @@ -647,11 +681,18 @@ mod tests { let serialized_signature_with_status_2 = serde_json::to_string(&signature_with_status_2).expect("Failed to serialize signature"); assert!(serialized_signature_with_status_1 == serialized_signature_with_status_2); + assert_eq!( + serialized_signature_with_status_1, + serde_json::to_string(&signature).expect("Failed to serialize raw signature") + ); let deserialized_signature_with_status: SignatureWithStatus = serde_json::from_str(&serialized_signature_with_status_1) .expect("Failed to deserialize signature"); - assert_eq!(*deserialized_signature_with_status.signature(), signature); + assert_eq!( + deserialized_signature_with_status.signature().unwrap(), + signature + ); assert!(!deserialized_signature_with_status.is_verified()); } @@ -865,4 +906,50 @@ mod tests { assert_eq!(signature_aggregator.all_voters().count(), 5); assert_eq!(validator_verifier.pessimistic_verify_set().len(), 2); } + + #[test] + fn junk_signature_with_status_decodes_without_group_element() { + let junk = WireBlsSignature::from_compact_array([0x7Fu8; WireBlsSignature::COMPACT_LEN]); + let encoded = bcs::to_bytes(&junk).unwrap(); + let decoded: SignatureWithStatus = + bcs::from_bytes(&encoded).expect("length-valid payload must decode"); + assert!(!decoded.is_verified()); + assert!(decoded.recover_group_element().is_err()); + assert_eq!(decoded.wire_signature(), &junk); + } + + #[test] + fn junk_aggregate_payload_does_not_block_ledger_info_match() { + let ledger_info = LedgerInfo::new(BlockInfo::empty(), HashValue::random()); + let dummy = bls12381::Signature::dummy_signature(); + let valid = AggregateSignature::new(BitVec::from(vec![true]), Some(dummy.clone())); + let mut encoded = bcs::to_bytes(&valid).unwrap(); + + let dummy_bytes = bcs::to_bytes(&dummy).unwrap(); + let junk = WireBlsSignature::from_compact_array([0xEEu8; WireBlsSignature::COMPACT_LEN]); + let junk_bytes = bcs::to_bytes(&junk).unwrap(); + assert_eq!(dummy_bytes.len(), junk_bytes.len()); + let start = encoded.len() - dummy_bytes.len(); + encoded[start..].copy_from_slice(&junk_bytes); + + let decoded: AggregateSignature = + bcs::from_bytes(&encoded).expect("compressed payload must decode"); + assert!(decoded.try_group_element().is_err()); + assert_eq!(decoded.wire_sig(), Some(&junk)); + + let li_with_sigs = LedgerInfoWithSignatures::new(ledger_info.clone(), decoded); + // Commit-vote matching compares ledger fields only; decompression + // must not run (and must not fail) before this equality check. + assert_eq!(li_with_sigs.ledger_info(), &ledger_info); + assert_eq!(li_with_sigs.commit_info(), ledger_info.commit_info()); + + let vote_placeholder = LedgerInfo::new( + ledger_info.commit_info().clone(), + ledger_info.consensus_data_hash(), + ); + assert_eq!(&vote_placeholder, li_with_sigs.ledger_info()); + assert!(li_with_sigs + .verify_signatures(&ValidatorVerifier::new(vec![])) + .is_err()); + } } diff --git a/types/src/lib.rs b/types/src/lib.rs index 4a50857d1f4..4a12346070b 100644 --- a/types/src/lib.rs +++ b/types/src/lib.rs @@ -55,6 +55,7 @@ pub mod validator_verifier; pub mod vesting; pub mod vm_status; pub mod waypoint; +pub mod wire_bls; pub mod write_set; pub use account_address::AccountAddress as PeerId; diff --git a/types/src/validator_verifier.rs b/types/src/validator_verifier.rs index 2e89c9c321a..6bfa33edce9 100644 --- a/types/src/validator_verifier.rs +++ b/types/src/validator_verifier.rs @@ -279,7 +279,10 @@ impl ValidatorVerifier { if (!self.optimistic_sig_verification || self.pessimistic_verify_set.contains(&author)) && !signature_with_status.is_verified() { - self.verify(author, message, signature_with_status.signature())?; + let point = signature_with_status + .recover_group_element() + .map_err(|_| VerifyError::InvalidMultiSignature)?; + self.verify(author, message, &point)?; signature_with_status.set_verified(); } Ok(()) @@ -296,10 +299,11 @@ impl ValidatorVerifier { .into_par_iter() .with_min_len(4) // At least 4 signatures are verified in each task .filter_map(|(account_address, signature)| { + let recovered = signature.recover_group_element().ok(); if signature.is_verified() - || self - .verify(account_address, message, signature.signature()) - .is_ok() + || recovered + .as_ref() + .is_some_and(|point| self.verify(account_address, message, point).is_ok()) { signature.set_verified(); Some((account_address, signature)) @@ -371,10 +375,11 @@ impl ValidatorVerifier { return Ok(()); } } - // Verify empty multi signature + // Verify empty multi signature. Decompression happens here, after the + // bitmask and voting-power checks above. let multi_sig = multi_signature - .sig() - .as_ref() + .try_group_element() + .map_err(|_| VerifyError::InvalidMultiSignature)? .ok_or(VerifyError::EmptySignature)?; // Verify the optimistically aggregated signature. let aggregated_key = @@ -405,10 +410,11 @@ impl ValidatorVerifier { } // Verify the quorum voting power of the authors self.check_voting_power(authors.iter(), true)?; - // Verify empty aggregated signature + // Verify empty aggregated signature. Decompression happens here, after + // the bitmask and voting-power checks above. let aggregated_sig = aggregated_signature - .sig() - .as_ref() + .try_group_element() + .map_err(|_| VerifyError::InvalidMultiSignature)? .ok_or(VerifyError::EmptySignature)?; aggregated_sig diff --git a/types/src/wire_bls.rs b/types/src/wire_bls.rs new file mode 100644 index 00000000000..c14510cb1f8 --- /dev/null +++ b/types/src/wire_bls.rs @@ -0,0 +1,239 @@ +// Copyright © Aptos Foundation +// SPDX-License-Identifier: Apache-2.0 + +//! Compressed BLS12-381 G2 payload used by consensus vote types. +//! +//! `bls12381::Signature` recovers a curve point inside `TryFrom` / serde. That +//! recovery is a field square-root and is far more expensive than reading a +//! `LedgerInfo`. Commit-vote matching only compares the ledger fields of a +//! vote against a later full `LedgerInfoWithSignatures`; it must not depend on +//! the attached signature being a valid group element. +//! +//! This type is the 96-byte encoding only. Serde matches the historical +//! `Signature` newtype so BCS and JSON stay bitwise compatible. Recovering a +//! `bls12381::Signature` is a separate, explicit step used by verification +//! and aggregation. + +use aptos_crypto::{bls12381, CryptoMaterialError}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; +use std::{convert::TryFrom, fmt, hash::Hash}; + +/// AIP-80 prefix accepted on the human-readable path, matching `bls12381::Signature`. +const BLS_SIG_AIP80_PREFIX: &str = "bls12381-sig-"; + +/// 96-byte compressed BLS signature that has not been turned into a group element. +#[derive(Clone, Copy)] +pub struct WireBlsSignature { + compact: [u8; bls12381::Signature::LENGTH], +} + +impl WireBlsSignature { + /// Compressed encoding length, identical to `bls12381::Signature::LENGTH`. + pub const COMPACT_LEN: usize = bls12381::Signature::LENGTH; + + /// Wrap an already-recovered signature by re-encoding it to compressed bytes. + pub fn capture_point(signature: &bls12381::Signature) -> Self { + Self { + compact: signature.to_bytes(), + } + } + + /// Accept a fixed-size compressed payload. No curve checks run here. + pub fn from_compact_array(compact: [u8; Self::COMPACT_LEN]) -> Self { + Self { compact } + } + + /// Accept a slice if and only if it is the compressed encoding length. + pub fn from_compact_slice(bytes: &[u8]) -> Result { + let compact: [u8; Self::COMPACT_LEN] = bytes + .try_into() + .map_err(|_| CryptoMaterialError::WrongLengthError)?; + Ok(Self { compact }) + } + + /// Borrow the compressed encoding. + pub fn compact_bytes(&self) -> &[u8; Self::COMPACT_LEN] { + &self.compact + } + + /// Recover the G2 element. This is the first decompression for values + /// that arrived on the wire. + pub fn recover_group_element(&self) -> Result { + bls12381::Signature::try_from(self.compact.as_slice()) + } +} + +impl From for WireBlsSignature { + fn from(signature: bls12381::Signature) -> Self { + Self::capture_point(&signature) + } +} + +impl From<&bls12381::Signature> for WireBlsSignature { + fn from(signature: &bls12381::Signature) -> Self { + Self::capture_point(signature) + } +} + +impl PartialEq for WireBlsSignature { + fn eq(&self, other: &Self) -> bool { + self.compact == other.compact + } +} + +impl Eq for WireBlsSignature {} + +impl Hash for WireBlsSignature { + fn hash(&self, state: &mut H) { + state.write(&self.compact); + } +} + +impl fmt::Debug for WireBlsSignature { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", hex::encode(self.compact)) + } +} + +impl fmt::Display for WireBlsSignature { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", hex::encode(self.compact)) + } +} + +impl Serialize for WireBlsSignature { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + // Field name and encoding must stay identical to `bls12381::Signature`. + if serializer.is_human_readable() { + serializer.serialize_str(&format!("0x{}", hex::encode(self.compact))) + } else { + serializer.serialize_newtype_struct("Signature", serde_bytes::Bytes::new(&self.compact)) + } + } +} + +impl<'de> Deserialize<'de> for WireBlsSignature { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + use serde::de::Error; + + if deserializer.is_human_readable() { + let encoded = String::deserialize(deserializer)?; + decode_human_readable_compact(&encoded).map_err(D::Error::custom) + } else { + #[derive(Deserialize)] + #[serde(rename = "Signature")] + struct SignatureBytes<'a>(&'a [u8]); + + let SignatureBytes(bytes) = SignatureBytes::deserialize(deserializer)?; + Self::from_compact_slice(bytes).map_err(D::Error::custom) + } + } +} + +fn decode_human_readable_compact(encoded: &str) -> Result { + let mut body = encoded + .strip_prefix(BLS_SIG_AIP80_PREFIX) + .unwrap_or(encoded); + body = body.strip_prefix("0x").unwrap_or(body); + let raw = hex::decode(body).map_err(|_| CryptoMaterialError::DeserializationError)?; + WireBlsSignature::from_compact_slice(&raw) +} + +#[cfg(test)] +mod tests { + use super::*; + use aptos_crypto::{bls12381::Signature, ValidCryptoMaterial, ValidCryptoMaterialStringExt}; + + fn valid_point() -> Signature { + Signature::dummy_signature() + } + + #[test] + fn compact_round_trip_matches_signature_bytes() { + let point = valid_point(); + let wire = WireBlsSignature::capture_point(&point); + assert_eq!(wire.compact_bytes().as_slice(), point.to_bytes().as_slice()); + assert_eq!(wire.recover_group_element().unwrap(), point); + assert_eq!( + ValidCryptoMaterial::to_bytes(&point).len(), + WireBlsSignature::COMPACT_LEN + ); + } + + #[test] + fn bcs_matches_bls_signature_newtype() { + let point = valid_point(); + let wire = WireBlsSignature::from(&point); + assert_eq!( + bcs::to_bytes(&wire).unwrap(), + bcs::to_bytes(&point).unwrap() + ); + } + + #[test] + fn json_matches_bls_signature_string() { + let point = valid_point(); + let wire = WireBlsSignature::from(&point); + assert_eq!( + serde_json::to_string(&wire).unwrap(), + serde_json::to_string(&point).unwrap() + ); + assert_eq!( + serde_json::to_string(&wire).unwrap(), + serde_json::to_string(&point.to_encoded_string().unwrap()).unwrap() + ); + } + + #[test] + fn option_wrapper_stays_bitwise_compatible() { + let point = valid_point(); + let old: Option = Some(point.clone()); + let new: Option = Some(WireBlsSignature::from(&point)); + assert_eq!(bcs::to_bytes(&old).unwrap(), bcs::to_bytes(&new).unwrap()); + assert_eq!( + serde_json::to_string(&old).unwrap(), + serde_json::to_string(&new).unwrap() + ); + } + + #[test] + fn well_formed_length_junk_decodes_and_fails_only_on_recover() { + let junk = [0xFFu8; WireBlsSignature::COMPACT_LEN]; + let wire = WireBlsSignature::from_compact_array(junk); + let encoded = bcs::to_bytes(&wire).unwrap(); + let decoded: WireBlsSignature = bcs::from_bytes(&encoded).unwrap(); + assert_eq!(decoded, wire); + assert!(decoded.recover_group_element().is_err()); + + let json = serde_json::to_string(&wire).unwrap(); + let from_json: WireBlsSignature = serde_json::from_str(&json).unwrap(); + assert_eq!(from_json, wire); + assert!(from_json.recover_group_element().is_err()); + } + + #[test] + fn wrong_length_is_rejected_before_recover() { + assert!(WireBlsSignature::from_compact_slice(&[0u8; 32]).is_err()); + let short = serde_bytes::Bytes::new(&[0u8; 8]); + // Direct slice helper is the length gate used by serde. + assert_eq!( + WireBlsSignature::from_compact_slice(short.as_ref()).unwrap_err(), + CryptoMaterialError::WrongLengthError + ); + } + + #[test] + fn human_readable_accepts_aip80_prefix_without_recovering() { + let junk = [0xAAu8; WireBlsSignature::COMPACT_LEN]; + let encoded = format!("{}0x{}", BLS_SIG_AIP80_PREFIX, hex::encode(junk)); + let wire = decode_human_readable_compact(&encoded).unwrap(); + assert_eq!(wire.compact_bytes(), &junk); + assert!(wire.recover_group_element().is_err()); + } +}