From 1345f371de57577f52e448ce754482a80057367e Mon Sep 17 00:00:00 2001
From: Simon Warta <simon@warta.it>
Date: Sat, 3 Feb 2024 13:53:57 +0100
Subject: [PATCH] Require two pubkey recoveries to succeed

---
 packages/crypto/src/secp256k1.rs              | 18 +++++++-----------
 packages/crypto/tests/wycheproof_secp256k1.rs | 13 ++++---------
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/packages/crypto/src/secp256k1.rs b/packages/crypto/src/secp256k1.rs
index d77f71181d..06ca31cb28 100644
--- a/packages/crypto/src/secp256k1.rs
+++ b/packages/crypto/src/secp256k1.rs
@@ -380,17 +380,13 @@ 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!(
+                recovered0 == public_key || recovered1 == public_key,
+                "Did not find correct pubkey (test case {i} in {COSMOS_SECP256K1_TESTS_JSON})"
+            );
         }
     }
 
diff --git a/packages/crypto/tests/wycheproof_secp256k1.rs b/packages/crypto/tests/wycheproof_secp256k1.rs
index 2d8a307c5b..25867c557c 100644
--- a/packages/crypto/tests/wycheproof_secp256k1.rs
+++ b/packages/crypto/tests/wycheproof_secp256k1.rs
@@ -286,15 +286,10 @@ 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!(recovered0 == public_key || recovered1 == public_key);
 }
 
 fn from_der(data: &[u8]) -> Result<[u8; 64], String> {