Defer BLS signature decompression until verification

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<bls12381::Signature, CryptoMaterialError> {
+        self.signature.recover_group_element()
     }
 
     // Question: SignatureWithStatus has interior mutability. Is it okay to expose this?

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<bls12381::Signature, CryptoMaterialError> {
+        self.signature.recover_group_element()
     }
 
     /// Returns the signature along with the verification status of the signature.

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<bls12381::Signature, CryptoMaterialError> {
+        self.signature.recover_group_element()
     }
 
     pub fn signature_with_status(&self) -> &SignatureWithStatus {

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<bls12381::Signature, CryptoMaterialError> {
+        self.signature.recover_group_element()
     }
 
     pub fn signature_with_status(&self) -> &SignatureWithStatus {

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,

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(),

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();

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())

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())

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<bls12381::Signature>,
+    sig: Option<WireBlsSignature>,
 }
 
 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<bls12381::Signature> {
-        &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<Option<bls12381::Signature>, 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<bls12381::Signature> {
+        self.try_group_element().ok().flatten()
     }
 }