Remove y from EthAccount signature. (#940)

* feat: remove y from signature

* feat: update CHANGELOG

* feat: apply update review

* refactor: Signature to EthSignature

* fix: tests

* feat: apply update reviews

CHANGELOG.md

@@ -67,6 +67,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - `IERC721::name`, `IERC721::symbol`, and `IERC721Metadata::token_uri` return ByteArrays instead of felts (#857)
   - `InternalTrait::initializer` accepts an additional `base_uri` ByteArray parameter (#857)
   - IERC721Metadata SRC5 interface ID. This is changed because of the ByteArray integration (#857)
+- EthAccount
+  - Expected signature format changed from `(r, s, y)` to `(r, s)` (#940)
 
 ### Removed
 

src/account/utils/signature.cairo

@@ -3,12 +3,15 @@
 
 use ecdsa::check_ecdsa_signature;
 use openzeppelin::account::interface::EthPublicKey;
-use openzeppelin::account::utils::secp256k1::Secp256k1PointPartialEq;
-use starknet::eth_signature::Signature;
-use starknet::secp256_trait::{is_signature_entry_valid, recover_public_key};
-use starknet::secp256k1::Secp256k1Point;
+use starknet::secp256_trait;
 
-/// This function assumes the `s` component of the signature to be positive 
+#[derive(Copy, Drop, Serde)]
+pub struct EthSignature {
+    pub r: u256,
+    pub s: u256,
+}
+
+/// This function assumes the `s` component of the signature to be positive
 /// for efficiency reasons. It is not recommended to use it other than for
 /// validating account signatures over transaction hashes since otherwise
 /// it's not protected against signature malleability.
@@ -25,7 +28,7 @@ fn is_valid_stark_signature(
     }
 }
 
-/// This function assumes the `s` component of the signature to be positive 
+/// This function assumes the `s` component of the signature to be positive
 /// for efficiency reasons. It is not recommended to use it other than for
 /// validating account signatures over transaction hashes since otherwise
 /// it's not protected against signature malleability.
@@ -34,21 +37,8 @@ fn is_valid_eth_signature(
     msg_hash: felt252, public_key: EthPublicKey, signature: Span<felt252>
 ) -> bool {
     let mut signature = signature;
-    let signature: Signature = Serde::deserialize(ref signature)
+    let signature: EthSignature = Serde::deserialize(ref signature)
         .expect('Signature: Invalid format.');
 
-    // Signature out of range
-    if !is_signature_entry_valid::<Secp256k1Point>(signature.r) {
-        return false;
-    }
-    if !is_signature_entry_valid::<Secp256k1Point>(signature.s) {
-        return false;
-    }
-
-    let public_key_point: Secp256k1Point = recover_public_key(msg_hash.into(), signature).unwrap();
-    if public_key_point != public_key {
-        // Invalid signature
-        return false;
-    }
-    true
+    secp256_trait::is_valid_signature(msg_hash.into(), signature.r, signature.s, public_key)
 }

src/tests/account/test_dual_eth_account.cairo

@@ -12,7 +12,6 @@ use openzeppelin::tests::mocks::non_implementing_mock::NonImplementingMock;
 use openzeppelin::tests::utils::constants::{ETH_PUBKEY, NEW_ETH_PUBKEY};
 use openzeppelin::tests::utils;
 use openzeppelin::utils::serde::SerializedAppend;
-use starknet::eth_signature::Signature;
 use starknet::testing;
 
 //

src/tests/account/test_eth_account.cairo

@@ -9,6 +9,7 @@ use openzeppelin::account::interface::{ISRC6, ISRC6_ID};
 use openzeppelin::account::utils::secp256k1::{
     DebugSecp256k1Point, Secp256k1PointPartialEq, Secp256k1PointSerde
 };
+use openzeppelin::account::utils::signature::EthSignature;
 use openzeppelin::introspection::interface::{ISRC5, ISRC5_ID};
 use openzeppelin::tests::mocks::erc20_mocks::DualCaseERC20Mock;
 use openzeppelin::tests::mocks::eth_account_mocks::DualCaseEthAccountMock;
@@ -24,7 +25,6 @@ use poseidon::poseidon_hash_span;
 use starknet::ContractAddress;
 use starknet::SyscallResultTrait;
 use starknet::account::Call;
-use starknet::eth_signature::Signature;
 use starknet::secp256k1::secp256k1_new_syscall;
 use starknet::testing;
 
@@ -33,7 +33,7 @@ struct SignedTransactionData {
     private_key: u256,
     public_key: EthPublicKey,
     transaction_hash: felt252,
-    signature: Signature
+    signature: EthSignature
 }
 
 /// This signature was computed using ethers.js.
@@ -47,10 +47,9 @@ fn SIGNED_TX_DATA() -> SignedTransactionData {
             .unwrap()
             .unwrap(),
         transaction_hash: 0x008f882c63d0396d216d57529fe29ad5e70b6cd51b47bd2458b0a4ccb2ba0957,
-        signature: Signature {
+        signature: EthSignature {
             r: 0x82bb3efc0554ec181405468f273b0dbf935cca47182b22da78967d0770f7dcc3,
             s: 0x6719fef30c11c74add873e4da0e1234deb69eae6a6bd4daa44b816dc199f3e86,
-            y_parity: true
         }
     }
 }