Specific error cases on point aggregation

packages/core/src/errors/mod.rs

@@ -13,4 +13,6 @@ pub use core_error::{
 };
 pub use recover_pubkey_error::RecoverPubkeyError;
 pub use system_error::SystemError;
-pub use verification_error::{AggregationPairingEqualityError, VerificationError};
+pub use verification_error::{
+    AggregationError, AggregationPairingEqualityError, VerificationError,
+};

packages/core/src/errors/recover_pubkey_error.rs

@@ -65,7 +65,8 @@ impl From<CryptoError> for RecoverPubkeyError {
             }
             CryptoError::GenericErr { .. } => RecoverPubkeyError::unknown_err(original.code()),
             CryptoError::InvalidRecoveryParam { .. } => RecoverPubkeyError::InvalidRecoveryParam,
-            CryptoError::AggregationPairingEquality { .. }
+            CryptoError::Aggregation { .. }
+            | CryptoError::AggregationPairingEquality { .. }
             | CryptoError::BatchErr { .. }
             | CryptoError::InvalidPubkeyFormat { .. }
             | CryptoError::InvalidPoint { .. }

packages/core/src/errors/verification_error.rs

@@ -6,6 +6,15 @@ use super::BT;
 #[cfg(not(target_arch = "wasm32"))]
 use cosmwasm_crypto::CryptoError;
 
+#[derive(Display, Debug, PartialEq)]
+#[cfg_attr(feature = "std", derive(thiserror::Error))]
+pub enum AggregationError {
+    #[display("List of points is empty")]
+    Empty,
+    #[display("List is not an expected multiple")]
+    NotMultiple,
+}
+
 #[derive(Display, Debug, PartialEq)]
 #[cfg_attr(feature = "std", derive(thiserror::Error))]
 pub enum AggregationPairingEqualityError {
@@ -24,6 +33,8 @@ pub enum AggregationPairingEqualityError {
 #[derive(Display, Debug)]
 #[cfg_attr(feature = "std", derive(thiserror::Error))]
 pub enum VerificationError {
+    #[display("Aggregation error: {source}")]
+    Aggregation { source: AggregationError },
     #[display("Aggregation pairing equality error: {source}")]
     AggregationPairingEquality {
         source: AggregationPairingEqualityError,
@@ -61,6 +72,9 @@ impl VerificationError {
 impl PartialEq<VerificationError> for VerificationError {
     fn eq(&self, rhs: &VerificationError) -> bool {
         match self {
+            VerificationError::Aggregation { source: lhs_source } => {
+                matches!(rhs, VerificationError::Aggregation { source: rhs_source } if rhs_source == lhs_source)
+            }
             VerificationError::AggregationPairingEquality { source: lhs_source } => {
                 matches!(rhs, VerificationError::AggregationPairingEquality { source: rhs_source } if rhs_source == lhs_source)
             }
@@ -101,6 +115,18 @@ impl PartialEq<VerificationError> for VerificationError {
 impl From<CryptoError> for VerificationError {
     fn from(original: CryptoError) -> Self {
         match original {
+            CryptoError::Aggregation {
+                source: cosmwasm_crypto::AggregationError::Empty,
+                ..
+            } => VerificationError::Aggregation {
+                source: AggregationError::Empty,
+            },
+            CryptoError::Aggregation {
+                source: cosmwasm_crypto::AggregationError::NotMultiple { .. },
+                ..
+            } => VerificationError::Aggregation {
+                source: AggregationError::NotMultiple,
+            },
             CryptoError::AggregationPairingEquality {
                 source: cosmwasm_crypto::AggregationPairingEqualityError::EmptyG1,
                 ..

packages/core/src/lib.rs

@@ -28,11 +28,11 @@ pub use crate::addresses::{instantiate2_address, Addr, CanonicalAddr, Instantiat
 pub use crate::binary::Binary;
 pub use crate::encoding::{from_base64, from_hex, to_base64, to_hex};
 pub use crate::errors::{
-    AggregationPairingEqualityError, CheckedFromRatioError, CheckedMultiplyFractionError,
-    CheckedMultiplyRatioError, CoinFromStrError, CoinsError, ConversionOverflowError, CoreError,
-    CoreResult, DivideByZeroError, DivisionError, OverflowError, OverflowOperation,
-    RecoverPubkeyError, RoundDownOverflowError, RoundUpOverflowError, SystemError,
-    VerificationError,
+    AggregationError, AggregationPairingEqualityError, CheckedFromRatioError,
+    CheckedMultiplyFractionError, CheckedMultiplyRatioError, CoinFromStrError, CoinsError,
+    ConversionOverflowError, CoreError, CoreResult, DivideByZeroError, DivisionError,
+    OverflowError, OverflowOperation, RecoverPubkeyError, RoundDownOverflowError,
+    RoundUpOverflowError, SystemError, VerificationError,
 };
 pub use crate::hex_binary::HexBinary;
 pub use crate::math::{

packages/crypto/src/bls12_318/aggregate.rs

@@ -1,4 +1,7 @@
-use crate::{errors::InvalidPoint, CryptoError};
+use crate::{
+    errors::{Aggregation, InvalidPoint},
+    CryptoError,
+};
 
 use super::points::{g1_from_fixed, g2_from_fixed, G1, G2};
 
@@ -10,8 +13,14 @@ const G2_POINT_SIZE: usize = 96;
 /// This is like Aggregate from <https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-05>
 /// but works for signatures as well as public keys.
 pub fn bls12_381_aggregate_g1(points: &[u8]) -> Result<[u8; 48], CryptoError> {
-    if points.len() % G1_POINT_SIZE != 0 {
-        return Err(InvalidPoint::DecodingError {}.into());
+    if points.is_empty() {
+        return Err(Aggregation::Empty.into());
+    } else if points.len() % G1_POINT_SIZE != 0 {
+        return Err(Aggregation::NotMultiple {
+            expected_multiple: G1_POINT_SIZE,
+            remainder: points.len() % G1_POINT_SIZE,
+        }
+        .into());
     }
 
     let points_count = points.len() / G1_POINT_SIZE;
@@ -46,8 +55,14 @@ pub fn bls12_381_aggregate_g1(points: &[u8]) -> Result<[u8; 48], CryptoError> {
 /// This is like Aggregate from <https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-05>
 /// but works for signatures as well as public keys.
 pub fn bls12_381_aggregate_g2(points: &[u8]) -> Result<[u8; 96], CryptoError> {
-    if points.len() % G2_POINT_SIZE != 0 {
-        return Err(InvalidPoint::DecodingError {}.into());
+    if points.is_empty() {
+        return Err(Aggregation::Empty.into());
+    } else if points.len() % G2_POINT_SIZE != 0 {
+        return Err(Aggregation::NotMultiple {
+            expected_multiple: G2_POINT_SIZE,
+            remainder: points.len() % G2_POINT_SIZE,
+        }
+        .into());
     }
 
     let points_count = points.len() / G2_POINT_SIZE;
@@ -138,15 +153,15 @@ mod tests {
     }
 
     #[test]
-    fn bls12_318_aggregate_g1_works() {
-        let sum = bls12_381_aggregate_g1(b"").unwrap();
-        assert_eq!(sum, G1::identity().to_compressed());
+    fn bls12_318_aggregate_g1_empty_err() {
+        let res = bls12_381_aggregate_g1(b"");
+        assert!(res.is_err());
     }
 
     #[test]
-    fn bls12_318_aggregate_g2_works() {
-        let sum = bls12_381_aggregate_g2(b"").unwrap();
-        assert_eq!(sum, G2::identity().to_compressed());
+    fn bls12_318_aggregate_g2_empty_err() {
+        let res = bls12_381_aggregate_g2(b"");
+        assert!(res.is_err());
     }
 
     #[test]

packages/crypto/src/errors.rs

@@ -6,6 +6,18 @@ use crate::BT;
 
 pub type CryptoResult<T> = core::result::Result<T, CryptoError>;
 
+#[derive(Debug, Display)]
+#[cfg_attr(feature = "std", derive(thiserror::Error))]
+pub enum Aggregation {
+    #[display("List of points is empty")]
+    Empty,
+    #[display("List is not a multiple of {expected_multiple}. Remainder: {remainder}")]
+    NotMultiple {
+        expected_multiple: usize,
+        remainder: usize,
+    },
+}
+
 #[derive(Debug, Display)]
 #[cfg_attr(feature = "std", derive(thiserror::Error))]
 pub enum AggregationPairingEquality {
@@ -33,6 +45,8 @@ pub enum InvalidPoint {
 #[derive(Display, Debug)]
 #[cfg_attr(feature = "std", derive(thiserror::Error))]
 pub enum CryptoError {
+    #[display("Point aggregation error: {source}")]
+    Aggregation { source: Aggregation, backtrace: BT },
     #[display("Aggregation pairing equality error: {source}")]
     AggregationPairingEquality {
         source: AggregationPairingEquality,
@@ -133,6 +147,24 @@ impl CryptoError {
                 source: AggregationPairingEquality::EmptyG2 { .. },
                 ..
             } => 15,
+            CryptoError::Aggregation {
+                source: Aggregation::Empty,
+                ..
+            } => 16,
+            CryptoError::Aggregation {
+                source: Aggregation::NotMultiple { .. },
+                ..
+            } => 17,
+        }
+    }
+}
+
+impl From<Aggregation> for CryptoError {
+    #[track_caller]
+    fn from(value: Aggregation) -> Self {
+        Self::Aggregation {
+            source: value,
+            backtrace: BT::capture(),
         }
     }
 }

packages/crypto/src/lib.rs

@@ -36,7 +36,8 @@ pub use crate::ed25519::EDDSA_PUBKEY_LEN;
 pub use crate::ed25519::{ed25519_batch_verify, ed25519_verify};
 #[doc(hidden)]
 pub use crate::errors::{
-    AggregationPairingEquality as AggregationPairingEqualityError, CryptoError, CryptoResult,
+    Aggregation as AggregationError, AggregationPairingEquality as AggregationPairingEqualityError,
+    CryptoError, CryptoResult,
 };
 #[doc(hidden)]
 pub use crate::secp256k1::{secp256k1_recover_pubkey, secp256k1_verify};

packages/crypto/tests/bls12_381.rs

@@ -162,6 +162,12 @@ fn bls12_381_aggregate_g2_works() {
         let test = read_aggregate_test(json);
         let signatures: Vec<&[u8]> = test.input.iter().map(|m| m.as_slice()).collect();
         let signatures_combined: Vec<u8> = signatures.concat();
+
+        // Skip empty signatures since we explicitly error on empty inputs
+        if signatures_combined.is_empty() {
+            continue;
+        }
+
         let sum = bls12_381_aggregate_g2(&signatures_combined).unwrap();
         match test.output {
             Some(expected) => assert_eq!(sum.as_slice(), expected),
@@ -388,6 +394,14 @@ fn bls12_381_fast_aggregate_verify_works() {
                 pubkeys.extend(pubkey);
             }
 
+            // Reject cases with empty public keys since the aggregation will:
+            //
+            // 1. error out with our implementation specifically
+            // 2. if it wouldn't error out, it would return the identity element of G1, making the
+            //    signature validation return invalid anyway
+            if pubkeys.is_empty() {
+                return Ok(false);
+            }
             let pubkey = bls12_381_aggregate_g1(&pubkeys).unwrap();
 
             if bls12_381_g2_is_identity(&signature)? {

packages/std/src/imports.rs

@@ -17,8 +17,8 @@ use crate::{
     memory::get_optional_region_address,
 };
 use crate::{
-    AggregationPairingEqualityError, RecoverPubkeyError, StdError, StdResult, SystemError,
-    VerificationError,
+    AggregationError, AggregationPairingEqualityError, RecoverPubkeyError, StdError, StdResult,
+    SystemError, VerificationError,
 };
 
 /// An upper bound for typical canonical address lengths (e.g. 20 in Cosmos SDK/Ethereum or 32 in Nano/Substrate)
@@ -405,6 +405,12 @@ impl Api for ExternalApi {
         match result {
             0 => Ok(point),
             8 => Err(VerificationError::InvalidPoint),
+            16 => Err(VerificationError::Aggregation {
+                source: AggregationError::Empty,
+            }),
+            17 => Err(VerificationError::Aggregation {
+                source: AggregationError::NotMultiple,
+            }),
             error_code => Err(VerificationError::unknown_err(error_code)),
         }
     }
@@ -422,6 +428,12 @@ impl Api for ExternalApi {
         match result {
             0 => Ok(point),
             8 => Err(VerificationError::InvalidPoint),
+            16 => Err(VerificationError::Aggregation {
+                source: AggregationError::Empty,
+            }),
+            17 => Err(VerificationError::Aggregation {
+                source: AggregationError::NotMultiple,
+            }),
             error_code => Err(VerificationError::unknown_err(error_code)),
         }
     }

packages/vm/src/imports.rs

@@ -283,7 +283,7 @@ pub fn do_bls12_381_aggregate_g1<
             0
         }
         Err(err) => match err {
-            CryptoError::InvalidPoint { .. } => err.code(),
+            CryptoError::InvalidPoint { .. } | CryptoError::Aggregation { .. } => err.code(),
             CryptoError::AggregationPairingEquality { .. }
             | CryptoError::BatchErr { .. }
             | CryptoError::GenericErr { .. }
@@ -327,7 +327,7 @@ pub fn do_bls12_381_aggregate_g2<
             0
         }
         Err(err) => match err {
-            CryptoError::InvalidPoint { .. } => err.code(),
+            CryptoError::InvalidPoint { .. } | CryptoError::Aggregation { .. } => err.code(),
             CryptoError::AggregationPairingEquality { .. }
             | CryptoError::BatchErr { .. }
             | CryptoError::GenericErr { .. }
@@ -380,7 +380,8 @@ pub fn do_bls12_381_aggregate_pairing_equality<
             CryptoError::AggregationPairingEquality { .. } | CryptoError::InvalidPoint { .. } => {
                 err.code()
             }
-            CryptoError::BatchErr { .. }
+            CryptoError::Aggregation { .. }
+            | CryptoError::BatchErr { .. }
             | CryptoError::GenericErr { .. }
             | CryptoError::InvalidHashFormat { .. }
             | CryptoError::InvalidPubkeyFormat { .. }
@@ -487,7 +488,8 @@ pub fn do_secp256k1_verify<A: BackendApi + 'static, S: Storage + 'static, Q: Que
             | CryptoError::InvalidPubkeyFormat { .. }
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::GenericErr { .. } => err.code(),
-            CryptoError::AggregationPairingEquality { .. }
+            CryptoError::Aggregation { .. }
+            | CryptoError::AggregationPairingEquality { .. }
             | CryptoError::BatchErr { .. }
             | CryptoError::InvalidPoint { .. }
             | CryptoError::InvalidRecoveryParam { .. }
@@ -531,7 +533,8 @@ pub fn do_secp256k1_recover_pubkey<
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::InvalidRecoveryParam { .. }
             | CryptoError::GenericErr { .. } => Ok(to_high_half(err.code())),
-            CryptoError::AggregationPairingEquality { .. }
+            CryptoError::Aggregation { .. }
+            | CryptoError::AggregationPairingEquality { .. }
             | CryptoError::BatchErr { .. }
             | CryptoError::InvalidPoint { .. }
             | CryptoError::InvalidPubkeyFormat { .. }
@@ -576,7 +579,8 @@ pub fn do_secp256r1_verify<A: BackendApi + 'static, S: Storage + 'static, Q: Que
             | CryptoError::InvalidPubkeyFormat { .. }
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::GenericErr { .. } => err.code(),
-            CryptoError::AggregationPairingEquality { .. }
+            CryptoError::Aggregation { .. }
+            | CryptoError::AggregationPairingEquality { .. }
             | CryptoError::BatchErr { .. }
             | CryptoError::InvalidPoint { .. }
             | CryptoError::InvalidRecoveryParam { .. }
@@ -620,7 +624,8 @@ pub fn do_secp256r1_recover_pubkey<
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::InvalidRecoveryParam { .. }
             | CryptoError::GenericErr { .. } => Ok(to_high_half(err.code())),
-            CryptoError::AggregationPairingEquality { .. }
+            CryptoError::Aggregation { .. }
+            | CryptoError::AggregationPairingEquality { .. }
             | CryptoError::BatchErr { .. }
             | CryptoError::InvalidPoint { .. }
             | CryptoError::InvalidPubkeyFormat { .. }
@@ -672,7 +677,8 @@ pub fn do_ed25519_verify<A: BackendApi + 'static, S: Storage + 'static, Q: Queri
             CryptoError::InvalidPubkeyFormat { .. }
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::GenericErr { .. } => err.code(),
-            CryptoError::AggregationPairingEquality { .. }
+            CryptoError::Aggregation { .. }
+            | CryptoError::AggregationPairingEquality { .. }
             | CryptoError::BatchErr { .. }
             | CryptoError::InvalidPoint { .. }
             | CryptoError::InvalidHashFormat { .. }
@@ -738,7 +744,8 @@ pub fn do_ed25519_batch_verify<
             | CryptoError::InvalidPubkeyFormat { .. }
             | CryptoError::InvalidSignatureFormat { .. }
             | CryptoError::GenericErr { .. } => err.code(),
-            CryptoError::AggregationPairingEquality { .. }
+            CryptoError::Aggregation { .. }
+            | CryptoError::AggregationPairingEquality { .. }
             | CryptoError::InvalidHashFormat { .. }
             | CryptoError::InvalidPoint { .. }
             | CryptoError::InvalidRecoveryParam { .. }