Make ec_recover funcs return Results (#1171)

* feat: return Reslut from er_recover funcs

* test: update tests to handle returned results

* style: forc fmt

* fix: improve check for empty pubkey

* refactor: remove asm from ec_recover_address

* style: forc fmt

* cleanup: remove Option import

* refactor: move cast to Address type to implicit return

* fix: update ec_recover functions to handle failures correctly

* test: add tests for failure to recover

* fix: remove unused error variant

* cleanup: remove unused import of constants

* test: clean up test with is_err()

sway-lib-std/src/ecr.sw

@@ -1,31 +1,42 @@
 library ecr;
 
-use ::b512::B512;
 use ::address::Address;
+use ::b512::B512;
+use ::context::registers::error;
+use ::hash::{HashMethod, hash_pair};
+use ::result::*;
 
-/// Recover the public key derived from the private key used to sign a message
-pub fn ec_recover(signature: B512, msg_hash: b256) -> B512 {
-    let public_key = ~B512::new();
+pub enum EcRecoverError {
+    UnrecoverablePublicKey: (),
+}
 
-    asm(buffer: public_key.bytes, sig: signature.bytes, hash: msg_hash) {
+/// Recover the public key derived from the private key used to sign a message.
+/// Returns a `Result` to let the caller choose an error handling strategy.
+pub fn ec_recover(signature: B512, msg_hash: b256) -> Result<B512, EcRecoverError> {
+    let public_key = ~B512::new();
+    let was_error = asm(buffer: public_key.bytes, sig: signature.bytes, hash: msg_hash) {
         ecr buffer sig hash;
+        err
     };
-
-    public_key
+    // check the $err register to see if the `ecr` opcode succeeded
+    if was_error == 1 {
+        Result::Err(EcRecoverError::UnrecoverablePublicKey)
+    } else {
+        Result::Ok(public_key)
+    }
 }
 
-/// Recover the address derived from the private key used to sign a message
-pub fn ec_recover_address(signature: B512, msg_hash: b256) -> Address {
-    let address = asm(sig: signature.bytes, hash: msg_hash, addr_buffer, pub_key_buffer, hash_len: 64) {
-        move addr_buffer sp; // Buffer for address.
-        cfei i32;
-        move pub_key_buffer sp; // Temporary buffer for recovered key.
-        cfei i64;
-        ecr pub_key_buffer sig hash; // Recover public_key from sig & hash.
-        s256 addr_buffer pub_key_buffer hash_len; // Hash 64 bytes to the addr_buffer.
-        cfsi i64; // Free temporary key buffer.
-        addr_buffer: b256
-    };
+/// Recover the address derived from the private key used to sign a message.
+/// Returns a `Result` to let the caller choose an error handling strategy.
+pub fn ec_recover_address(signature: B512, msg_hash: b256) -> Result<Address, EcRecoverError> {
+    let pub_key_result = ec_recover(signature, msg_hash);
 
-    ~Address::from(address)
+    if let Result::Err(e) = pub_key_result {
+        // propagate the error if it exists
+        Result::Err(e)
+    } else {
+        let pub_key = pub_key_result.unwrap();
+        let address = hash_pair((pub_key.bytes)[0], (pub_key.bytes)[1], HashMethod::Sha256);
+        Result::Ok(~Address::from(address))
+    }
 }

test/src/e2e_vm_tests/test_programs/should_pass/stdlib/ec_recover_test/Forc.toml

@@ -1,8 +1,8 @@
 [project]
 authors = ["Fuel Labs <contact@fuel.sh>"]
+entry = "main.sw"
 license = "Apache-2.0"
 name = "ec_recover_test"
-entry = "main.sw"
 
 [dependencies]
 std = { path = "../../../../../../../sway-lib-std" }

test/src/e2e_vm_tests/test_programs/should_pass/stdlib/ec_recover_test/src/main.sw

@@ -1,16 +1,16 @@
 script;
 
-use std::b512::B512;
 use std::address::Address;
-use std::ecr::ec_recover_address;
-use std::ecr::ec_recover;
 use std::assert::assert;
+use std::b512::B512;
+use std::ecr::*;
+use std::panic::panic;
+use std::result::Result;
 
 fn main() -> bool {
-
-   //======================================================
-   // test data from sig-gen-util: /sway/sig_gen_util/src/main.rs
-   /**
+    //======================================================
+    // test data from sig-gen-util: /sway/sig_gen_util/src/main.rs
+    /**
    Secret Key: SecretKey(3b940b5586823dfd02ae3b461bb4336b5ecbaefd6627aa922efc048fec0c881c)
    Public Key: 1d152307c6b72b0ed0418b0e70cd80e7f5295b8d86f5722d3f5213fbd2394f36
                b7ce9c3e45905178455900b44abb308f3ef480481a4b2ee3f70aca157fde396a
@@ -33,13 +33,37 @@ fn main() -> bool {
     let signature: B512 = ~B512::from(sig_hi, sig_lo);
 
     // recover the address:
-    let recovered_address: Address = ec_recover_address(signature, msg_hash);
-    assert(recovered_address.value == address.value);
+    let address_result: Result<Address, EcRecoverError> = ec_recover_address(signature, msg_hash);
+    if let Result::Ok(a) = address_result {
+        assert(a.value == address.value);
+    } else {
+        panic(0);
+    };
 
     // recover the public key:
-    let recovered_pubkey: B512 = ec_recover(signature, msg_hash);
-    assert((recovered_pubkey.bytes)[0] == (pubkey.bytes)[0]);
-    assert((recovered_pubkey.bytes)[1] == (pubkey.bytes)[1]);
+    let pubkey_result: Result<B512, EcRecoverError> = ec_recover(signature, msg_hash);
+    if let Result::Ok(p) = pubkey_result {
+        assert(p == pubkey);
+    } else {
+        panic(0);
+    };
+
+    /////////////////////////////////////////
+    ///  Failure to recover
+    /////////////////////////////////////////
+
+    // using invalid data here to test the handling of failed pubkey/address recovery.
+    let bad_sig_hi = 0x000000000_8d8fe7dd522d88_000000000000000_34b6326ff51129776_000000000;
+    let bad_sig_lo = 0x000000000_4a11e49a2ee69f_000000000000000_dba8d779d323ab2a5_000000000;
+    let bad_signature: B512 = ~B512::from(bad_sig_hi, bad_sig_lo);
+
+    // this should return a Result::Err, so if it returns Result::Ok, we panic.
+    let pubkey_result1: Result<B512, EcRecoverError> = ec_recover(bad_signature, msg_hash);
+    assert(pubkey_result1.is_err());
+
+    // this should return a Result::Err, so if it returns Result::Ok, we panic.
+    let pubkey_result2: Result<Address, EcRecoverError> = ec_recover_address(bad_signature, msg_hash);
+    assert(pubkey_result2.is_err());
 
     true
 }