Skip to content

Commit edf41e3

Browse files
aumetraaumetra
committed
Specific error cases on point aggregation
1 parent 4552450 commit edf41e3

File tree

10 files changed

+140
-30
lines changed

10 files changed

+140
-30
lines changed

‎packages/core/src/errors/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,6 @@ pub use core_error::{
1313
};
1414
pub use recover_pubkey_error::RecoverPubkeyError;
1515
pub use system_error::SystemError;
16-
pub use verification_error::{AggregationPairingEqualityError, VerificationError};
16+
pub use verification_error::{
17+
AggregationError, AggregationPairingEqualityError, VerificationError,
18+
};

‎packages/core/src/errors/recover_pubkey_error.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ impl From<CryptoError> for RecoverPubkeyError {
6565
}
6666
CryptoError::GenericErr { .. } => RecoverPubkeyError::unknown_err(original.code()),
6767
CryptoError::InvalidRecoveryParam { .. } => RecoverPubkeyError::InvalidRecoveryParam,
68-
CryptoError::AggregationPairingEquality { .. }
68+
CryptoError::Aggregation { .. }
69+
| CryptoError::AggregationPairingEquality { .. }
6970
| CryptoError::BatchErr { .. }
7071
| CryptoError::InvalidPubkeyFormat { .. }
7172
| CryptoError::InvalidPoint { .. }

‎packages/core/src/errors/verification_error.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,15 @@ use super::BT;
66
#[cfg(not(target_arch = "wasm32"))]
77
use cosmwasm_crypto::CryptoError;
88

9+
#[derive(Display, Debug, PartialEq)]
10+
#[cfg_attr(feature = "std", derive(thiserror::Error))]
11+
pub enum AggregationError {
12+
#[display("List of points is empty")]
13+
Empty,
14+
#[display("List is not an expected multiple")]
15+
NotMultiple,
16+
}
17+
918
#[derive(Display, Debug, PartialEq)]
1019
#[cfg_attr(feature = "std", derive(thiserror::Error))]
1120
pub enum AggregationPairingEqualityError {
@@ -24,6 +33,8 @@ pub enum AggregationPairingEqualityError {
2433
#[derive(Display, Debug)]
2534
#[cfg_attr(feature = "std", derive(thiserror::Error))]
2635
pub enum VerificationError {
36+
#[display("Aggregation error: {source}")]
37+
Aggregation { source: AggregationError },
2738
#[display("Aggregation pairing equality error: {source}")]
2839
AggregationPairingEquality {
2940
source: AggregationPairingEqualityError,
@@ -61,6 +72,9 @@ impl VerificationError {
6172
impl PartialEq<VerificationError> for VerificationError {
6273
fn eq(&self, rhs: &VerificationError) -> bool {
6374
match self {
75+
VerificationError::Aggregation { source: lhs_source } => {
76+
matches!(rhs, VerificationError::Aggregation { source: rhs_source } if rhs_source == lhs_source)
77+
}
6478
VerificationError::AggregationPairingEquality { source: lhs_source } => {
6579
matches!(rhs, VerificationError::AggregationPairingEquality { source: rhs_source } if rhs_source == lhs_source)
6680
}
@@ -101,6 +115,18 @@ impl PartialEq<VerificationError> for VerificationError {
101115
impl From<CryptoError> for VerificationError {
102116
fn from(original: CryptoError) -> Self {
103117
match original {
118+
CryptoError::Aggregation {
119+
source: cosmwasm_crypto::AggregationError::Empty,
120+
..
121+
} => VerificationError::Aggregation {
122+
source: AggregationError::Empty,
123+
},
124+
CryptoError::Aggregation {
125+
source: cosmwasm_crypto::AggregationError::NotMultiple { .. },
126+
..
127+
} => VerificationError::Aggregation {
128+
source: AggregationError::NotMultiple,
129+
},
104130
CryptoError::AggregationPairingEquality {
105131
source: cosmwasm_crypto::AggregationPairingEqualityError::EmptyG1,
106132
..

‎packages/core/src/lib.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ pub use crate::addresses::{instantiate2_address, Addr, CanonicalAddr, Instantiat
2828
pub use crate::binary::Binary;
2929
pub use crate::encoding::{from_base64, from_hex, to_base64, to_hex};
3030
pub use crate::errors::{
31-
AggregationPairingEqualityError, CheckedFromRatioError, CheckedMultiplyFractionError,
32-
CheckedMultiplyRatioError, CoinFromStrError, CoinsError, ConversionOverflowError, CoreError,
33-
CoreResult, DivideByZeroError, DivisionError, OverflowError, OverflowOperation,
34-
RecoverPubkeyError, RoundDownOverflowError, RoundUpOverflowError, SystemError,
35-
VerificationError,
31+
AggregationError, AggregationPairingEqualityError, CheckedFromRatioError,
32+
CheckedMultiplyFractionError, CheckedMultiplyRatioError, CoinFromStrError, CoinsError,
33+
ConversionOverflowError, CoreError, CoreResult, DivideByZeroError, DivisionError,
34+
OverflowError, OverflowOperation, RecoverPubkeyError, RoundDownOverflowError,
35+
RoundUpOverflowError, SystemError, VerificationError,
3636
};
3737
pub use crate::hex_binary::HexBinary;
3838
pub use crate::math::{

‎packages/crypto/src/bls12_318/aggregate.rs

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1-
use crate::{errors::InvalidPoint, CryptoError};
1+
use crate::{
2+
errors::{Aggregation, InvalidPoint},
3+
CryptoError,
4+
};
25

36
use super::points::{g1_from_fixed, g2_from_fixed, G1, G2};
47

@@ -10,8 +13,14 @@ const G2_POINT_SIZE: usize = 96;
1013
/// This is like Aggregate from <https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-05>
1114
/// but works for signatures as well as public keys.
1215
pub fn bls12_381_aggregate_g1(points: &[u8]) -> Result<[u8; 48], CryptoError> {
13-
if points.len() % G1_POINT_SIZE != 0 {
14-
return Err(InvalidPoint::DecodingError {}.into());
16+
if points.is_empty() {
17+
return Err(Aggregation::Empty.into());
18+
} else if points.len() % G1_POINT_SIZE != 0 {
19+
return Err(Aggregation::NotMultiple {
20+
expected_multiple: G1_POINT_SIZE,
21+
remainder: points.len() % G1_POINT_SIZE,
22+
}
23+
.into());
1524
}
1625

1726
let points_count = points.len() / G1_POINT_SIZE;
@@ -46,8 +55,14 @@ pub fn bls12_381_aggregate_g1(points: &[u8]) -> Result<[u8; 48], CryptoError> {
4655
/// This is like Aggregate from <https://datatracker.ietf.org/doc/html/draft-irtf-cfrg-bls-signature-05>
4756
/// but works for signatures as well as public keys.
4857
pub fn bls12_381_aggregate_g2(points: &[u8]) -> Result<[u8; 96], CryptoError> {
49-
if points.len() % G2_POINT_SIZE != 0 {
50-
return Err(InvalidPoint::DecodingError {}.into());
58+
if points.is_empty() {
59+
return Err(Aggregation::Empty.into());
60+
} else if points.len() % G2_POINT_SIZE != 0 {
61+
return Err(Aggregation::NotMultiple {
62+
expected_multiple: G2_POINT_SIZE,
63+
remainder: points.len() % G2_POINT_SIZE,
64+
}
65+
.into());
5166
}
5267

5368
let points_count = points.len() / G2_POINT_SIZE;
@@ -138,15 +153,15 @@ mod tests {
138153
}
139154

140155
#[test]
141-
fn bls12_318_aggregate_g1_works() {
142-
let sum = bls12_381_aggregate_g1(b"").unwrap();
143-
assert_eq!(sum, G1::identity().to_compressed());
156+
fn bls12_318_aggregate_g1_empty_err() {
157+
let res = bls12_381_aggregate_g1(b"");
158+
assert!(res.is_err());
144159
}
145160

146161
#[test]
147-
fn bls12_318_aggregate_g2_works() {
148-
let sum = bls12_381_aggregate_g2(b"").unwrap();
149-
assert_eq!(sum, G2::identity().to_compressed());
162+
fn bls12_318_aggregate_g2_empty_err() {
163+
let res = bls12_381_aggregate_g2(b"");
164+
assert!(res.is_err());
150165
}
151166

152167
#[test]

‎packages/crypto/src/errors.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,18 @@ use crate::BT;
66

77
pub type CryptoResult<T> = core::result::Result<T, CryptoError>;
88

9+
#[derive(Debug, Display)]
10+
#[cfg_attr(feature = "std", derive(thiserror::Error))]
11+
pub enum Aggregation {
12+
#[display("List of points is empty")]
13+
Empty,
14+
#[display("List is not a multiple of {expected_multiple}. Remainder: {remainder}")]
15+
NotMultiple {
16+
expected_multiple: usize,
17+
remainder: usize,
18+
},
19+
}
20+
921
#[derive(Debug, Display)]
1022
#[cfg_attr(feature = "std", derive(thiserror::Error))]
1123
pub enum AggregationPairingEquality {
@@ -33,6 +45,8 @@ pub enum InvalidPoint {
3345
#[derive(Display, Debug)]
3446
#[cfg_attr(feature = "std", derive(thiserror::Error))]
3547
pub enum CryptoError {
48+
#[display("Point aggregation error: {source}")]
49+
Aggregation { source: Aggregation, backtrace: BT },
3650
#[display("Aggregation pairing equality error: {source}")]
3751
AggregationPairingEquality {
3852
source: AggregationPairingEquality,
@@ -133,6 +147,24 @@ impl CryptoError {
133147
source: AggregationPairingEquality::EmptyG2 { .. },
134148
..
135149
} => 15,
150+
CryptoError::Aggregation {
151+
source: Aggregation::Empty,
152+
..
153+
} => 16,
154+
CryptoError::Aggregation {
155+
source: Aggregation::NotMultiple { .. },
156+
..
157+
} => 17,
158+
}
159+
}
160+
}
161+
162+
impl From<Aggregation> for CryptoError {
163+
#[track_caller]
164+
fn from(value: Aggregation) -> Self {
165+
Self::Aggregation {
166+
source: value,
167+
backtrace: BT::capture(),
136168
}
137169
}
138170
}

‎packages/crypto/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ pub use crate::ed25519::EDDSA_PUBKEY_LEN;
3636
pub use crate::ed25519::{ed25519_batch_verify, ed25519_verify};
3737
#[doc(hidden)]
3838
pub use crate::errors::{
39-
AggregationPairingEquality as AggregationPairingEqualityError, CryptoError, CryptoResult,
39+
Aggregation as AggregationError, AggregationPairingEquality as AggregationPairingEqualityError,
40+
CryptoError, CryptoResult,
4041
};
4142
#[doc(hidden)]
4243
pub use crate::secp256k1::{secp256k1_recover_pubkey, secp256k1_verify};

‎packages/crypto/tests/bls12_381.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,12 @@ fn bls12_381_aggregate_g2_works() {
162162
let test = read_aggregate_test(json);
163163
let signatures: Vec<&[u8]> = test.input.iter().map(|m| m.as_slice()).collect();
164164
let signatures_combined: Vec<u8> = signatures.concat();
165+
166+
// Skip empty signatures since we explicitly error on empty inputs
167+
if signatures_combined.is_empty() {
168+
continue;
169+
}
170+
165171
let sum = bls12_381_aggregate_g2(&signatures_combined).unwrap();
166172
match test.output {
167173
Some(expected) => assert_eq!(sum.as_slice(), expected),
@@ -388,6 +394,14 @@ fn bls12_381_fast_aggregate_verify_works() {
388394
pubkeys.extend(pubkey);
389395
}
390396

397+
// Reject cases with empty public keys since the aggregation will:
398+
//
399+
// 1. error out with our implementation specifically
400+
// 2. if it wouldn't error out, it would return the identity element of G1, making the
401+
// signature validation return invalid anyway
402+
if pubkeys.is_empty() {
403+
return Ok(false);
404+
}
391405
let pubkey = bls12_381_aggregate_g1(&pubkeys).unwrap();
392406

393407
if bls12_381_g2_is_identity(&signature)? {

‎packages/std/src/imports.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use crate::{
1717
memory::get_optional_region_address,
1818
};
1919
use crate::{
20-
AggregationPairingEqualityError, RecoverPubkeyError, StdError, StdResult, SystemError,
21-
VerificationError,
20+
AggregationError, AggregationPairingEqualityError, RecoverPubkeyError, StdError, StdResult,
21+
SystemError, VerificationError,
2222
};
2323

2424
/// An upper bound for typical canonical address lengths (e.g. 20 in Cosmos SDK/Ethereum or 32 in Nano/Substrate)
@@ -413,6 +413,12 @@ impl Api for ExternalApi {
413413
match result {
414414
0 => Ok(point),
415415
8 => Err(VerificationError::InvalidPoint),
416+
16 => Err(VerificationError::Aggregation {
417+
source: AggregationError::Empty,
418+
}),
419+
17 => Err(VerificationError::Aggregation {
420+
source: AggregationError::NotMultiple,
421+
}),
416422
error_code => Err(VerificationError::unknown_err(error_code)),
417423
}
418424
}
@@ -430,6 +436,12 @@ impl Api for ExternalApi {
430436
match result {
431437
0 => Ok(point),
432438
8 => Err(VerificationError::InvalidPoint),
439+
16 => Err(VerificationError::Aggregation {
440+
source: AggregationError::Empty,
441+
}),
442+
17 => Err(VerificationError::Aggregation {
443+
source: AggregationError::NotMultiple,
444+
}),
433445
error_code => Err(VerificationError::unknown_err(error_code)),
434446
}
435447
}

‎packages/vm/src/imports.rs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ pub fn do_bls12_381_aggregate_g1<
283283
0
284284
}
285285
Err(err) => match err {
286-
CryptoError::InvalidPoint { .. } => err.code(),
286+
CryptoError::InvalidPoint { .. } | CryptoError::Aggregation { .. } => err.code(),
287287
CryptoError::AggregationPairingEquality { .. }
288288
| CryptoError::BatchErr { .. }
289289
| CryptoError::GenericErr { .. }
@@ -327,7 +327,7 @@ pub fn do_bls12_381_aggregate_g2<
327327
0
328328
}
329329
Err(err) => match err {
330-
CryptoError::InvalidPoint { .. } => err.code(),
330+
CryptoError::InvalidPoint { .. } | CryptoError::Aggregation { .. } => err.code(),
331331
CryptoError::AggregationPairingEquality { .. }
332332
| CryptoError::BatchErr { .. }
333333
| CryptoError::GenericErr { .. }
@@ -380,7 +380,8 @@ pub fn do_bls12_381_aggregate_pairing_equality<
380380
CryptoError::AggregationPairingEquality { .. } | CryptoError::InvalidPoint { .. } => {
381381
err.code()
382382
}
383-
CryptoError::BatchErr { .. }
383+
CryptoError::Aggregation { .. }
384+
| CryptoError::BatchErr { .. }
384385
| CryptoError::GenericErr { .. }
385386
| CryptoError::InvalidHashFormat { .. }
386387
| CryptoError::InvalidPubkeyFormat { .. }
@@ -487,7 +488,8 @@ pub fn do_secp256k1_verify<A: BackendApi + 'static, S: Storage + 'static, Q: Que
487488
| CryptoError::InvalidPubkeyFormat { .. }
488489
| CryptoError::InvalidSignatureFormat { .. }
489490
| CryptoError::GenericErr { .. } => err.code(),
490-
CryptoError::AggregationPairingEquality { .. }
491+
CryptoError::Aggregation { .. }
492+
| CryptoError::AggregationPairingEquality { .. }
491493
| CryptoError::BatchErr { .. }
492494
| CryptoError::InvalidPoint { .. }
493495
| CryptoError::InvalidRecoveryParam { .. }
@@ -531,7 +533,8 @@ pub fn do_secp256k1_recover_pubkey<
531533
| CryptoError::InvalidSignatureFormat { .. }
532534
| CryptoError::InvalidRecoveryParam { .. }
533535
| CryptoError::GenericErr { .. } => Ok(to_high_half(err.code())),
534-
CryptoError::AggregationPairingEquality { .. }
536+
CryptoError::Aggregation { .. }
537+
| CryptoError::AggregationPairingEquality { .. }
535538
| CryptoError::BatchErr { .. }
536539
| CryptoError::InvalidPoint { .. }
537540
| CryptoError::InvalidPubkeyFormat { .. }
@@ -576,7 +579,8 @@ pub fn do_secp256r1_verify<A: BackendApi + 'static, S: Storage + 'static, Q: Que
576579
| CryptoError::InvalidPubkeyFormat { .. }
577580
| CryptoError::InvalidSignatureFormat { .. }
578581
| CryptoError::GenericErr { .. } => err.code(),
579-
CryptoError::AggregationPairingEquality { .. }
582+
CryptoError::Aggregation { .. }
583+
| CryptoError::AggregationPairingEquality { .. }
580584
| CryptoError::BatchErr { .. }
581585
| CryptoError::InvalidPoint { .. }
582586
| CryptoError::InvalidRecoveryParam { .. }
@@ -620,7 +624,8 @@ pub fn do_secp256r1_recover_pubkey<
620624
| CryptoError::InvalidSignatureFormat { .. }
621625
| CryptoError::InvalidRecoveryParam { .. }
622626
| CryptoError::GenericErr { .. } => Ok(to_high_half(err.code())),
623-
CryptoError::AggregationPairingEquality { .. }
627+
CryptoError::Aggregation { .. }
628+
| CryptoError::AggregationPairingEquality { .. }
624629
| CryptoError::BatchErr { .. }
625630
| CryptoError::InvalidPoint { .. }
626631
| CryptoError::InvalidPubkeyFormat { .. }
@@ -672,7 +677,8 @@ pub fn do_ed25519_verify<A: BackendApi + 'static, S: Storage + 'static, Q: Queri
672677
CryptoError::InvalidPubkeyFormat { .. }
673678
| CryptoError::InvalidSignatureFormat { .. }
674679
| CryptoError::GenericErr { .. } => err.code(),
675-
CryptoError::AggregationPairingEquality { .. }
680+
CryptoError::Aggregation { .. }
681+
| CryptoError::AggregationPairingEquality { .. }
676682
| CryptoError::BatchErr { .. }
677683
| CryptoError::InvalidPoint { .. }
678684
| CryptoError::InvalidHashFormat { .. }
@@ -738,7 +744,8 @@ pub fn do_ed25519_batch_verify<
738744
| CryptoError::InvalidPubkeyFormat { .. }
739745
| CryptoError::InvalidSignatureFormat { .. }
740746
| CryptoError::GenericErr { .. } => err.code(),
741-
CryptoError::AggregationPairingEquality { .. }
747+
CryptoError::Aggregation { .. }
748+
| CryptoError::AggregationPairingEquality { .. }
742749
| CryptoError::InvalidHashFormat { .. }
743750
| CryptoError::InvalidPoint { .. }
744751
| CryptoError::InvalidRecoveryParam { .. }

0 commit comments

Comments
 (0)