Skip to content

Commit

Permalink
Require two pubkey recoveries to succeed
Browse files Browse the repository at this point in the history
  • Loading branch information
webmaster128 committed Feb 3, 2024
1 parent fa92691 commit 5ecf3d3
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 20 deletions.
19 changes: 8 additions & 11 deletions packages/crypto/src/secp256k1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,17 +380,14 @@ mod tests {
assert_eq!(hash.as_slice(), message_hash.as_slice());

// Since the recovery param is missing in the test vectors, we try both 0 and 1
let try0 = secp256k1_recover_pubkey(&message_hash, &signature, 0);
let try1 = secp256k1_recover_pubkey(&message_hash, &signature, 1);
match (try0, try1) {
(Ok(recovered0), Ok(recovered1)) => {
// Got two different pubkeys. Without the recovery param, we don't know which one is the right one.
assert!(recovered0 == public_key || recovered1 == public_key)
},
(Ok(recovered), Err(_)) => assert_eq!(recovered, public_key),
(Err(_), Ok(recovered)) => assert_eq!(recovered, public_key),
(Err(_), Err(_)) => panic!("secp256k1_recover_pubkey failed (test case {i} in {COSMOS_SECP256K1_TESTS_JSON})"),
}
let recovered0 = secp256k1_recover_pubkey(&message_hash, &signature, 0).unwrap();
let recovered1 = secp256k1_recover_pubkey(&message_hash, &signature, 1).unwrap();
// Got two different pubkeys. Without the recovery param, we don't know which one is the right one.
assert_ne!(recovered0, recovered1);
assert!(
recovered0 == public_key || recovered1 == public_key,
"Did not find correct pubkey (test case {i} in {COSMOS_SECP256K1_TESTS_JSON})"

Check warning on line 389 in packages/crypto/src/secp256k1.rs

View check run for this annotation

Codecov / codecov/patch

packages/crypto/src/secp256k1.rs#L389

Added line #L389 was not covered by tests
);
}
}

Expand Down
14 changes: 5 additions & 9 deletions packages/crypto/tests/wycheproof_secp256k1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,15 +286,11 @@ fn ecdsa_secp256k1_sha3_512() {

fn test_secp256k1_recover_pubkey(message_hash: &[u8], signature: &[u8], public_key: &[u8]) {
// Since the recovery param is missing in the test vectors, we try both 0 and 1
for recovery_param in 0..=1 {
if let Ok(recovered) = secp256k1_recover_pubkey(message_hash, signature, recovery_param) {
if recovered == public_key {
// success, found working recovery param
return;
}
}
}
panic!("secp256k1_recover_pubkey failed for all recovery params");
let recovered0 = secp256k1_recover_pubkey(message_hash, signature, 0).unwrap();
let recovered1 = secp256k1_recover_pubkey(message_hash, signature, 1).unwrap();
// Got two different pubkeys. Without the recovery param, we don't know which one is the right one.
assert_ne!(recovered0, recovered1);
assert!(recovered0 == public_key || recovered1 == public_key);
}

fn from_der(data: &[u8]) -> Result<[u8; 64], String> {
Expand Down

0 comments on commit 5ecf3d3

Please sign in to comment.