From 6304ca7ed9e3a4e16f9dda96f80249d312604665 Mon Sep 17 00:00:00 2001 From: Ariel Elperin Date: Thu, 21 Sep 2023 17:41:32 +0300 Subject: [PATCH 1/3] add verify_signature function for secp curves --- corelib/src/starknet/secp256_trait.cairo | 107 +++++++++-------------- corelib/src/starknet/secp256k1.cairo | 70 ++++++++++++++- corelib/src/starknet/secp256r1.cairo | 11 +++ corelib/src/test/secp256k1_test.cairo | 44 ++++++++-- corelib/src/test/secp256r1_test.cairo | 71 +++++++++------ 5 files changed, 200 insertions(+), 103 deletions(-) diff --git a/corelib/src/starknet/secp256_trait.cairo b/corelib/src/starknet/secp256_trait.cairo index fe8c0ced65b..dafcf42de75 100644 --- a/corelib/src/starknet/secp256_trait.cairo +++ b/corelib/src/starknet/secp256_trait.cairo @@ -1,5 +1,4 @@ use array::ArrayTrait; -use keccak::keccak_u256s_be_inputs; use math::{u256_mul_mod_n, inv_mod}; use option::OptionTrait; use starknet::{eth_address::U256IntoEthAddress, EthAddress, SyscallResult, SyscallResultTrait}; @@ -35,6 +34,9 @@ trait Secp256Trait { fn secp256_ec_get_point_from_x_syscall( x: u256, y_parity: bool ) -> SyscallResult>; + fn secp256_ec_get_point_from_xy_syscall( + x: u256, y: u256 + ) -> SyscallResult>; } trait Secp256PointTrait { @@ -43,6 +45,43 @@ trait Secp256PointTrait { fn mul(self: Secp256Point, scalar: u256) -> SyscallResult; } +/// Checks whether `value` is in the range [1, N), where N is the size of the curve. +fn is_signature_entry_valid< + Secp256Point, +Drop, impl Secp256Impl: Secp256Trait +>( + value: u256 +) -> bool { + value != 0_u256 && value < Secp256Impl::get_curve_size() +} + +fn verify_signature< + Secp256Point, + +Drop, + impl Secp256Impl: Secp256Trait, + +Secp256PointTrait +>( + msg_hash: u256, r: u256, s: u256, public_key: Secp256Point +) -> Result { + if !is_signature_entry_valid::(r) { + return Result::Err('Signature out of range'); + } + if !is_signature_entry_valid::(s) { + return Result::Err('Signature out of range'); + } + + let n_nz = Secp256Impl::get_curve_size().try_into().unwrap(); + let s_inv = inv_mod(s.try_into().unwrap(), n_nz).unwrap(); + let u1 = u256_mul_mod_n(msg_hash, s_inv, n_nz); + let u2 = u256_mul_mod_n(r, s_inv, n_nz); + + let generator_point = Secp256Impl::get_generator_point(); + let point1 = generator_point.mul(u1).unwrap_syscall(); + let point2 = public_key.mul(u2).unwrap_syscall(); + let sum = point1.add(point2).unwrap_syscall(); + + let (x, y) = sum.get_coordinates().unwrap_syscall(); + return Result::Ok(x == r); +} /// Receives a signature and the signed message hash. /// Returns the public key associated with the signer, represented as a point on the curve. @@ -85,69 +124,3 @@ fn secp256_ec_negate_scalar< ) -> u256 { Secp256Impl::get_curve_size() - c } - - -/// Checks a Secp256 ECDSA signature. -/// Also verifies that r and s components of the signature are in the range (0, N), -/// where N is the size of the curve. -/// Returns a Result with an error string if the signature is invalid. -fn is_eth_signature_valid< - Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait ->( - msg_hash: u256, signature: Signature, eth_address: EthAddress -) -> Result<(), felt252> { - if !is_signature_entry_valid::(signature.r) { - return Result::Err('Signature out of range'); - } - if !is_signature_entry_valid::(signature.s) { - return Result::Err('Signature out of range'); - } - - let public_key_point = recover_public_key::(:msg_hash, :signature).unwrap(); - let calculated_eth_address = public_key_point_to_eth_address(:public_key_point); - if eth_address != calculated_eth_address { - return Result::Err('Invalid signature'); - } - Result::Ok(()) -} - -/// Asserts that a Secp256 ECDSA signature is valid. -/// Also verifies that r and s components of the signature are in the range (0, N), -/// where N is the size of the curve. -fn verify_eth_signature< - Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait ->( - msg_hash: u256, signature: Signature, eth_address: EthAddress -) { - match is_eth_signature_valid::(:msg_hash, :signature, :eth_address) { - Result::Ok(()) => {}, - Result::Err(err) => panic_with_felt252(err), - } -} - -/// Checks whether `value` is in the range [1, N), where N is the size of the curve. -fn is_signature_entry_valid< - Secp256Point, +Drop, impl Secp256Impl: Secp256Trait ->( - value: u256 -) -> bool { - value != 0_u256 && value < Secp256Impl::get_curve_size() -} - -/// Converts a public key point to the corresponding Ethereum address. -fn public_key_point_to_eth_address< - Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait ->( - public_key_point: Secp256Point -) -> EthAddress { - let (x, y) = public_key_point.get_coordinates().unwrap_syscall(); - - // Keccak output is little endian. - let point_hash_le = keccak_u256s_be_inputs(array![x, y].span()); - let point_hash = u256 { - low: integer::u128_byte_reverse(point_hash_le.high), - high: integer::u128_byte_reverse(point_hash_le.low) - }; - - point_hash.into() -} diff --git a/corelib/src/starknet/secp256k1.cairo b/corelib/src/starknet/secp256k1.cairo index 7030844f6de..38e88764b72 100644 --- a/corelib/src/starknet/secp256k1.cairo +++ b/corelib/src/starknet/secp256k1.cairo @@ -3,8 +3,14 @@ use option::OptionTrait; use starknet::{ - EthAddress, secp256_trait::{Secp256Trait, Secp256PointTrait}, SyscallResult, SyscallResultTrait + EthAddress, + secp256_trait::{ + Secp256Trait, Secp256PointTrait, recover_public_key, is_signature_entry_valid, Signature + }, + SyscallResult, SyscallResultTrait }; +use keccak::keccak_u256s_be_inputs; + #[derive(Copy, Drop)] extern type Secp256k1Point; @@ -32,6 +38,17 @@ impl Secp256k1Impl of Secp256Trait { ) -> SyscallResult> { secp256k1_get_point_from_x_syscall(x, y_parity) } + fn secp256_ec_get_point_from_xy_syscall( + x: u256, y: u256 + ) -> SyscallResult> { + let point = secp256k1_get_point_from_x_syscall(x, false).unwrap_syscall().unwrap(); + let (_, point_y) = point.get_coordinates().unwrap_syscall(); + if (point_y == y) { + return SyscallResult::Ok(Option::Some(point)); + } else { + return secp256k1_get_point_from_x_syscall(x, true); + } + } } impl Secp256k1PointImpl of Secp256PointTrait { @@ -72,3 +89,54 @@ extern fn secp256k1_get_point_from_x_syscall( extern fn secp256k1_get_xy_syscall( p: Secp256k1Point ) -> SyscallResult<(u256, u256)> implicits(GasBuiltin, System) nopanic; + +/// Asserts that a Secp256 ECDSA signature is valid. +/// Also verifies that r and s components of the signature are in the range (0, N), +/// where N is the size of the curve. +fn verify_eth_signature(msg_hash: u256, signature: Signature, eth_address: EthAddress) { + match is_eth_signature_valid(:msg_hash, :signature, :eth_address) { + Result::Ok(()) => {}, + Result::Err(err) => panic_with_felt252(err), + } +} + +/// Checks a Secp256 ECDSA signature. +/// Also verifies that r and s components of the signature are in the range (0, N), +/// where N is the size of the curve. +/// Returns a Result with an error string if the signature is invalid. +fn is_eth_signature_valid( + msg_hash: u256, signature: Signature, eth_address: EthAddress +) -> Result<(), felt252> { + if !is_signature_entry_valid::(signature.r) { + return Result::Err('Signature out of range'); + } + if !is_signature_entry_valid::(signature.s) { + return Result::Err('Signature out of range'); + } + + let public_key_point = recover_public_key::(:msg_hash, :signature).unwrap(); + let calculated_eth_address = public_key_point_to_eth_address(:public_key_point); + if eth_address != calculated_eth_address { + return Result::Err('Invalid signature'); + } + Result::Ok(()) +} + + +/// Converts a public key point to the corresponding Ethereum address. +fn public_key_point_to_eth_address< + Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait +>( + public_key_point: Secp256Point +) -> EthAddress { + let (x, y) = public_key_point.get_coordinates().unwrap_syscall(); + + // Keccak output is little endian. + let point_hash_le = keccak_u256s_be_inputs(array![x, y].span()); + let point_hash = u256 { + low: integer::u128_byte_reverse(point_hash_le.high), + high: integer::u128_byte_reverse(point_hash_le.low) + }; + + point_hash.into() +} diff --git a/corelib/src/starknet/secp256r1.cairo b/corelib/src/starknet/secp256r1.cairo index 65fdfcc86a6..44ec6059c3a 100644 --- a/corelib/src/starknet/secp256r1.cairo +++ b/corelib/src/starknet/secp256r1.cairo @@ -32,6 +32,17 @@ impl Secp256r1Impl of Secp256Trait { ) -> SyscallResult> { secp256r1_get_point_from_x_syscall(x, y_parity) } + fn secp256_ec_get_point_from_xy_syscall( + x: u256, y: u256 + ) -> SyscallResult> { + let point = secp256r1_get_point_from_x_syscall(x, false).unwrap_syscall().unwrap(); + let (_, point_y) = point.get_coordinates().unwrap_syscall(); + if (point_y == y) { + return SyscallResult::Ok(Option::Some(point)); + } else { + return secp256r1_get_point_from_x_syscall(x, true); + } + } } impl Secp256r1PointImpl of Secp256PointTrait { diff --git a/corelib/src/test/secp256k1_test.cairo b/corelib/src/test/secp256k1_test.cairo index 9991e9fc321..a2c55126f10 100644 --- a/corelib/src/test/secp256k1_test.cairo +++ b/corelib/src/test/secp256k1_test.cairo @@ -2,9 +2,9 @@ use starknet::{ eth_address::U256IntoEthAddress, EthAddress, secp256k1::Secp256k1Impl, SyscallResultTrait }; use starknet::secp256_trait::{ - Signature, recover_public_key, verify_eth_signature, Secp256PointTrait, signature_from_vrs + Signature, recover_public_key, Secp256PointTrait, signature_from_vrs, verify_signature }; -use starknet::secp256k1::{Secp256k1Point, Secp256k1PointImpl}; +use starknet::secp256k1::{Secp256k1Point, Secp256k1PointImpl, verify_eth_signature}; #[test] fn test_secp256k1_recover_public_key() { @@ -68,7 +68,7 @@ fn test_verify_eth_signature() { get_message_and_signature( :y_parity ); - verify_eth_signature::(:msg_hash, :signature, :eth_address); + verify_eth_signature(:msg_hash, :signature, :eth_address); } #[test] @@ -80,7 +80,7 @@ fn test_verify_eth_signature_wrong_eth_address() { :y_parity ); let eth_address = (eth_address.into() + 1).try_into().unwrap(); - verify_eth_signature::(:msg_hash, :signature, :eth_address); + verify_eth_signature(:msg_hash, :signature, :eth_address); } #[test] @@ -92,7 +92,7 @@ fn test_verify_eth_signature_overflowing_signature_r() { :y_parity ); signature.r = Secp256k1Impl::get_curve_size() + 1; - verify_eth_signature::(:msg_hash, :signature, :eth_address); + verify_eth_signature(:msg_hash, :signature, :eth_address); } #[test] @@ -104,5 +104,37 @@ fn test_verify_eth_signature_overflowing_signature_s() { :y_parity ); signature.s = Secp256k1Impl::get_curve_size() + 1; - verify_eth_signature::(:msg_hash, :signature, :eth_address); + verify_eth_signature(:msg_hash, :signature, :eth_address); +} + +#[test] +#[available_gas(100000000)] +fn test_verify_signature() { + let (msg_hash, signature, public_key_x, public_key_y, _) = get_message_and_signature(false); + + let public_key = Secp256k1Impl::secp256_ec_get_point_from_xy_syscall(public_key_x, public_key_y) + .unwrap_syscall() + .unwrap(); + + let is_valid = verify_signature::( + msg_hash, signature.r, signature.s, public_key + ) + .unwrap(); + assert(is_valid, 'Signature should be valid'); +} + +#[test] +#[available_gas(100000000)] +fn test_verify_signature_invalid_signature() { + let (msg_hash, signature, public_key_x, public_key_y, _) = get_message_and_signature(false); + + let public_key = Secp256k1Impl::secp256_ec_get_point_from_xy_syscall(public_key_x, public_key_y) + .unwrap_syscall() + .unwrap(); + + let is_valid = verify_signature::( + msg_hash, signature.r + 1, signature.s, public_key + ) + .unwrap(); + assert(!is_valid, 'Signature should be invalid'); } diff --git a/corelib/src/test/secp256r1_test.cairo b/corelib/src/test/secp256r1_test.cairo index ac44f5c05ae..345644b4b59 100644 --- a/corelib/src/test/secp256r1_test.cairo +++ b/corelib/src/test/secp256r1_test.cairo @@ -1,9 +1,5 @@ -use starknet::{ - eth_address::U256IntoEthAddress, EthAddress, secp256r1::Secp256r1Impl, SyscallResultTrait -}; -use starknet::secp256_trait::{ - recover_public_key, verify_eth_signature, Secp256PointTrait, Signature -}; +use starknet::{secp256r1::Secp256r1Impl, SyscallResultTrait}; +use starknet::secp256_trait::{recover_public_key, Secp256PointTrait, Signature, verify_signature}; use starknet::secp256r1::{Secp256r1Point, Secp256r1PointImpl}; use test::test_utils::assert_eq; @@ -19,7 +15,7 @@ fn test_secp256r1_recover_public_key() { /// Returns a golden valid message hash and its signature, for testing. -fn get_message_and_signature() -> (u256, Signature, u256, u256, EthAddress) { +fn get_message_and_signature() -> (u256, Signature, u256, u256, Secp256r1Point) { // msg = "" // public key: (0x04aaec73635726f213fb8a9e64da3b8632e41495a944d0045b522eba7240fad5, // 0x0087d9315798aaa3a5ba01775787ced05eaaf7b4e09fc81d6d1aa546e8365d525d) @@ -31,43 +27,60 @@ fn get_message_and_signature() -> (u256, Signature, u256, u256, EthAddress) { 0x04aaec73635726f213fb8a9e64da3b8632e41495a944d0045b522eba7240fad5, 0x0087d9315798aaa3a5ba01775787ced05eaaf7b4e09fc81d6d1aa546e8365d525d ); - let eth_address = 0x492882426e1cda979008bfaf874ff796eb3bb1c0_u256.into(); - (msg_hash, Signature { r, s, y_parity: true }, public_key_x, public_key_y, eth_address) + let public_key = Secp256r1Impl::secp256_ec_get_point_from_xy_syscall(public_key_x, public_key_y) + .unwrap_syscall() + .unwrap(); + + (msg_hash, Signature { r, s, y_parity: true }, public_key_x, public_key_y, public_key) } #[test] -fn test_verify_eth_signature() { - let (msg_hash, signature, expected_public_key_x, expected_public_key_y, eth_address) = - get_message_and_signature(); - verify_eth_signature::(:msg_hash, :signature, :eth_address); +#[available_gas(100000000)] +fn test_verify_signature() { + let (msg_hash, signature, _, _, public_key) = get_message_and_signature(); + let is_valid = verify_signature::< + Secp256r1Point + >(msg_hash, signature.r, signature.s, public_key) + .unwrap(); + assert(is_valid, 'Signature should be valid'); } #[test] -#[should_panic(expected: ('Invalid signature',))] -fn test_verify_eth_signature_wrong_eth_address() { - let (msg_hash, signature, expected_public_key_x, expected_public_key_y, eth_address) = - get_message_and_signature(); - let eth_address = (eth_address.into() + 1).try_into().unwrap(); - verify_eth_signature::(:msg_hash, :signature, :eth_address); +#[available_gas(100000000)] +fn test_verify_signature_invalid_signature() { + let (msg_hash, signature, _, _, public_key) = get_message_and_signature(); + let is_valid = verify_signature::< + Secp256r1Point + >(msg_hash, signature.r + 1, signature.s, public_key) + .unwrap(); + assert(!is_valid, 'Signature should be invalid'); } #[test] #[should_panic(expected: ('Signature out of range',))] -fn test_verify_eth_signature_overflowing_signature_r() { - let (msg_hash, mut signature, expected_public_key_x, expected_public_key_y, eth_address) = - get_message_and_signature(); - signature.r = Secp256r1Impl::get_curve_size() + 1; - verify_eth_signature::(:msg_hash, :signature, :eth_address); +#[available_gas(100000000)] +fn test_verify_signature_overflowing_signature_r() { + let (msg_hash, mut signature, _, _, public_key) = get_message_and_signature(); + match verify_signature::< + Secp256r1Point + >(msg_hash, Secp256r1Impl::get_curve_size() + 1, signature.s, public_key) { + Result::Ok(_) => {}, + Result::Err(err) => panic_with_felt252(err) + } } #[test] #[should_panic(expected: ('Signature out of range',))] -fn test_verify_eth_signature_overflowing_signature_s() { - let (msg_hash, mut signature, expected_public_key_x, expected_public_key_y, eth_address) = - get_message_and_signature(); - signature.s = Secp256r1Impl::get_curve_size() + 1; - verify_eth_signature::(:msg_hash, :signature, :eth_address); +#[available_gas(100000000)] +fn test_verify_signature_overflowing_signature_s() { + let (msg_hash, mut signature, _, _, public_key) = get_message_and_signature(); + match verify_signature::< + Secp256r1Point + >(msg_hash, signature.r, Secp256r1Impl::get_curve_size() + 1, public_key) { + Result::Ok(_) => {}, + Result::Err(err) => panic_with_felt252(err) + } } From e1ad2588b5147c6a8092bc59bdb86a60c24ad135 Mon Sep 17 00:00:00 2001 From: Ariel Elperin Date: Thu, 19 Oct 2023 09:39:34 +0300 Subject: [PATCH 2/3] use existing syscall to generate point and minor refactor to verify_sig --- corelib/src/starknet/secp256_trait.cairo | 17 +++++-------- corelib/src/starknet/secp256k1.cairo | 11 -------- corelib/src/starknet/secp256r1.cairo | 11 -------- corelib/src/test/secp256k1_test.cairo | 20 +++++++-------- corelib/src/test/secp256r1_test.cairo | 32 +++++++++--------------- 5 files changed, 27 insertions(+), 64 deletions(-) diff --git a/corelib/src/starknet/secp256_trait.cairo b/corelib/src/starknet/secp256_trait.cairo index dafcf42de75..181cd3400a0 100644 --- a/corelib/src/starknet/secp256_trait.cairo +++ b/corelib/src/starknet/secp256_trait.cairo @@ -34,9 +34,6 @@ trait Secp256Trait { fn secp256_ec_get_point_from_x_syscall( x: u256, y_parity: bool ) -> SyscallResult>; - fn secp256_ec_get_point_from_xy_syscall( - x: u256, y: u256 - ) -> SyscallResult>; } trait Secp256PointTrait { @@ -54,19 +51,17 @@ fn is_signature_entry_valid< value != 0_u256 && value < Secp256Impl::get_curve_size() } -fn verify_signature< +fn is_valid_signature< Secp256Point, +Drop, impl Secp256Impl: Secp256Trait, +Secp256PointTrait >( msg_hash: u256, r: u256, s: u256, public_key: Secp256Point -) -> Result { - if !is_signature_entry_valid::(r) { - return Result::Err('Signature out of range'); - } - if !is_signature_entry_valid::(s) { - return Result::Err('Signature out of range'); +) -> bool { + if !is_signature_entry_valid::(r) + || !is_signature_entry_valid::(s) { + return false; } let n_nz = Secp256Impl::get_curve_size().try_into().unwrap(); @@ -80,7 +75,7 @@ fn verify_signature< let sum = point1.add(point2).unwrap_syscall(); let (x, y) = sum.get_coordinates().unwrap_syscall(); - return Result::Ok(x == r); + x == r } /// Receives a signature and the signed message hash. diff --git a/corelib/src/starknet/secp256k1.cairo b/corelib/src/starknet/secp256k1.cairo index 38e88764b72..d8f52079e7c 100644 --- a/corelib/src/starknet/secp256k1.cairo +++ b/corelib/src/starknet/secp256k1.cairo @@ -38,17 +38,6 @@ impl Secp256k1Impl of Secp256Trait { ) -> SyscallResult> { secp256k1_get_point_from_x_syscall(x, y_parity) } - fn secp256_ec_get_point_from_xy_syscall( - x: u256, y: u256 - ) -> SyscallResult> { - let point = secp256k1_get_point_from_x_syscall(x, false).unwrap_syscall().unwrap(); - let (_, point_y) = point.get_coordinates().unwrap_syscall(); - if (point_y == y) { - return SyscallResult::Ok(Option::Some(point)); - } else { - return secp256k1_get_point_from_x_syscall(x, true); - } - } } impl Secp256k1PointImpl of Secp256PointTrait { diff --git a/corelib/src/starknet/secp256r1.cairo b/corelib/src/starknet/secp256r1.cairo index 44ec6059c3a..65fdfcc86a6 100644 --- a/corelib/src/starknet/secp256r1.cairo +++ b/corelib/src/starknet/secp256r1.cairo @@ -32,17 +32,6 @@ impl Secp256r1Impl of Secp256Trait { ) -> SyscallResult> { secp256r1_get_point_from_x_syscall(x, y_parity) } - fn secp256_ec_get_point_from_xy_syscall( - x: u256, y: u256 - ) -> SyscallResult> { - let point = secp256r1_get_point_from_x_syscall(x, false).unwrap_syscall().unwrap(); - let (_, point_y) = point.get_coordinates().unwrap_syscall(); - if (point_y == y) { - return SyscallResult::Ok(Option::Some(point)); - } else { - return secp256r1_get_point_from_x_syscall(x, true); - } - } } impl Secp256r1PointImpl of Secp256PointTrait { diff --git a/corelib/src/test/secp256k1_test.cairo b/corelib/src/test/secp256k1_test.cairo index a2c55126f10..999ce10f2e3 100644 --- a/corelib/src/test/secp256k1_test.cairo +++ b/corelib/src/test/secp256k1_test.cairo @@ -2,7 +2,7 @@ use starknet::{ eth_address::U256IntoEthAddress, EthAddress, secp256k1::Secp256k1Impl, SyscallResultTrait }; use starknet::secp256_trait::{ - Signature, recover_public_key, Secp256PointTrait, signature_from_vrs, verify_signature + Signature, recover_public_key, Secp256PointTrait, signature_from_vrs, is_valid_signature }; use starknet::secp256k1::{Secp256k1Point, Secp256k1PointImpl, verify_eth_signature}; @@ -112,14 +112,13 @@ fn test_verify_eth_signature_overflowing_signature_s() { fn test_verify_signature() { let (msg_hash, signature, public_key_x, public_key_y, _) = get_message_and_signature(false); - let public_key = Secp256k1Impl::secp256_ec_get_point_from_xy_syscall(public_key_x, public_key_y) + let public_key = Secp256k1Impl::secp256_ec_new_syscall(public_key_x, public_key_y) .unwrap_syscall() .unwrap(); - let is_valid = verify_signature::( - msg_hash, signature.r, signature.s, public_key - ) - .unwrap(); + let is_valid = is_valid_signature::< + Secp256k1Point + >(msg_hash, signature.r, signature.s, public_key); assert(is_valid, 'Signature should be valid'); } @@ -128,13 +127,12 @@ fn test_verify_signature() { fn test_verify_signature_invalid_signature() { let (msg_hash, signature, public_key_x, public_key_y, _) = get_message_and_signature(false); - let public_key = Secp256k1Impl::secp256_ec_get_point_from_xy_syscall(public_key_x, public_key_y) + let public_key = Secp256k1Impl::secp256_ec_new_syscall(public_key_x, public_key_y) .unwrap_syscall() .unwrap(); - let is_valid = verify_signature::( - msg_hash, signature.r + 1, signature.s, public_key - ) - .unwrap(); + let is_valid = is_valid_signature::< + Secp256k1Point + >(msg_hash, signature.r + 1, signature.s, public_key); assert(!is_valid, 'Signature should be invalid'); } diff --git a/corelib/src/test/secp256r1_test.cairo b/corelib/src/test/secp256r1_test.cairo index 345644b4b59..47c4ab227bc 100644 --- a/corelib/src/test/secp256r1_test.cairo +++ b/corelib/src/test/secp256r1_test.cairo @@ -1,5 +1,5 @@ use starknet::{secp256r1::Secp256r1Impl, SyscallResultTrait}; -use starknet::secp256_trait::{recover_public_key, Secp256PointTrait, Signature, verify_signature}; +use starknet::secp256_trait::{recover_public_key, Secp256PointTrait, Signature, is_valid_signature}; use starknet::secp256r1::{Secp256r1Point, Secp256r1PointImpl}; use test::test_utils::assert_eq; @@ -28,7 +28,7 @@ fn get_message_and_signature() -> (u256, Signature, u256, u256, Secp256r1Point) 0x0087d9315798aaa3a5ba01775787ced05eaaf7b4e09fc81d6d1aa546e8365d525d ); - let public_key = Secp256r1Impl::secp256_ec_get_point_from_xy_syscall(public_key_x, public_key_y) + let public_key = Secp256r1Impl::secp256_ec_new_syscall(public_key_x, public_key_y) .unwrap_syscall() .unwrap(); @@ -39,10 +39,9 @@ fn get_message_and_signature() -> (u256, Signature, u256, u256, Secp256r1Point) #[available_gas(100000000)] fn test_verify_signature() { let (msg_hash, signature, _, _, public_key) = get_message_and_signature(); - let is_valid = verify_signature::< + let is_valid = is_valid_signature::< Secp256r1Point - >(msg_hash, signature.r, signature.s, public_key) - .unwrap(); + >(msg_hash, signature.r, signature.s, public_key); assert(is_valid, 'Signature should be valid'); } @@ -50,37 +49,30 @@ fn test_verify_signature() { #[available_gas(100000000)] fn test_verify_signature_invalid_signature() { let (msg_hash, signature, _, _, public_key) = get_message_and_signature(); - let is_valid = verify_signature::< + let is_valid = is_valid_signature::< Secp256r1Point - >(msg_hash, signature.r + 1, signature.s, public_key) - .unwrap(); + >(msg_hash, signature.r + 1, signature.s, public_key); assert(!is_valid, 'Signature should be invalid'); } #[test] -#[should_panic(expected: ('Signature out of range',))] #[available_gas(100000000)] fn test_verify_signature_overflowing_signature_r() { let (msg_hash, mut signature, _, _, public_key) = get_message_and_signature(); - match verify_signature::< + let is_valid = is_valid_signature::< Secp256r1Point - >(msg_hash, Secp256r1Impl::get_curve_size() + 1, signature.s, public_key) { - Result::Ok(_) => {}, - Result::Err(err) => panic_with_felt252(err) - } + >(msg_hash, Secp256r1Impl::get_curve_size() + 1, signature.s, public_key); + assert(!is_valid, 'Signature out of range'); } #[test] -#[should_panic(expected: ('Signature out of range',))] #[available_gas(100000000)] fn test_verify_signature_overflowing_signature_s() { let (msg_hash, mut signature, _, _, public_key) = get_message_and_signature(); - match verify_signature::< + let is_valid = is_valid_signature::< Secp256r1Point - >(msg_hash, signature.r, Secp256r1Impl::get_curve_size() + 1, public_key) { - Result::Ok(_) => {}, - Result::Err(err) => panic_with_felt252(err) - } + >(msg_hash, signature.r, Secp256r1Impl::get_curve_size() + 1, public_key); + assert(!is_valid, 'Signature out of range'); } From b4b3d100e2bad518060b0c2b226454b5ce29fdd3 Mon Sep 17 00:00:00 2001 From: Ariel Elperin Date: Fri, 20 Oct 2023 11:55:01 +0300 Subject: [PATCH 3/3] move eth_signature to a separate file --- corelib/src/starknet.cairo | 4 ++ corelib/src/starknet/eth_signature.cairo | 59 ++++++++++++++++++++++++ corelib/src/starknet/secp256k1.cairo | 54 ---------------------- corelib/src/test/secp256k1_test.cairo | 3 +- 4 files changed, 65 insertions(+), 55 deletions(-) create mode 100644 corelib/src/starknet/eth_signature.cairo diff --git a/corelib/src/starknet.cairo b/corelib/src/starknet.cairo index 273bded0232..36a11c627de 100644 --- a/corelib/src/starknet.cairo +++ b/corelib/src/starknet.cairo @@ -41,6 +41,10 @@ use eth_address::{ EthAddress, EthAddressIntoFelt252, EthAddressSerde, EthAddressZeroable, Felt252TryIntoEthAddress }; +// EthSignature +mod eth_signature; +use eth_signature::verify_eth_signature; + // ClassHash mod class_hash; use class_hash::{ diff --git a/corelib/src/starknet/eth_signature.cairo b/corelib/src/starknet/eth_signature.cairo new file mode 100644 index 00000000000..a24ff161818 --- /dev/null +++ b/corelib/src/starknet/eth_signature.cairo @@ -0,0 +1,59 @@ +use option::OptionTrait; +use starknet::{ + EthAddress, + secp256_trait::{ + Secp256Trait, Secp256PointTrait, recover_public_key, is_signature_entry_valid, Signature + }, + secp256k1::Secp256k1Point, SyscallResult, SyscallResultTrait +}; +use keccak::keccak_u256s_be_inputs; + +/// Asserts that an Ethereum signature is valid w.r.t. a given Eth address +/// Also verifies that r and s components of the signature are in the range (0, N), +/// where N is the size of the curve. +fn verify_eth_signature(msg_hash: u256, signature: Signature, eth_address: EthAddress) { + match is_eth_signature_valid(:msg_hash, :signature, :eth_address) { + Result::Ok(()) => {}, + Result::Err(err) => panic_with_felt252(err), + } +} + +/// Asserts that an Ethereum signature is valid w.r.t. a given Eth address +/// Also verifies that r and s components of the signature are in the range (0, N), +/// where N is the size of the curve. +/// Returns a Result with an error string if the signature is invalid. +fn is_eth_signature_valid( + msg_hash: u256, signature: Signature, eth_address: EthAddress +) -> Result<(), felt252> { + if !is_signature_entry_valid::(signature.r) { + return Result::Err('Signature out of range'); + } + if !is_signature_entry_valid::(signature.s) { + return Result::Err('Signature out of range'); + } + + let public_key_point = recover_public_key::(:msg_hash, :signature).unwrap(); + let calculated_eth_address = public_key_point_to_eth_address(:public_key_point); + if eth_address != calculated_eth_address { + return Result::Err('Invalid signature'); + } + Result::Ok(()) +} + +/// Converts a public key point to the corresponding Ethereum address. +fn public_key_point_to_eth_address< + Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait +>( + public_key_point: Secp256Point +) -> EthAddress { + let (x, y) = public_key_point.get_coordinates().unwrap_syscall(); + + // Keccak output is little endian. + let point_hash_le = keccak_u256s_be_inputs(array![x, y].span()); + let point_hash = u256 { + low: integer::u128_byte_reverse(point_hash_le.high), + high: integer::u128_byte_reverse(point_hash_le.low) + }; + + point_hash.into() +} diff --git a/corelib/src/starknet/secp256k1.cairo b/corelib/src/starknet/secp256k1.cairo index d8f52079e7c..d02e0a20243 100644 --- a/corelib/src/starknet/secp256k1.cairo +++ b/corelib/src/starknet/secp256k1.cairo @@ -3,14 +3,11 @@ use option::OptionTrait; use starknet::{ - EthAddress, secp256_trait::{ Secp256Trait, Secp256PointTrait, recover_public_key, is_signature_entry_valid, Signature }, SyscallResult, SyscallResultTrait }; -use keccak::keccak_u256s_be_inputs; - #[derive(Copy, Drop)] extern type Secp256k1Point; @@ -78,54 +75,3 @@ extern fn secp256k1_get_point_from_x_syscall( extern fn secp256k1_get_xy_syscall( p: Secp256k1Point ) -> SyscallResult<(u256, u256)> implicits(GasBuiltin, System) nopanic; - -/// Asserts that a Secp256 ECDSA signature is valid. -/// Also verifies that r and s components of the signature are in the range (0, N), -/// where N is the size of the curve. -fn verify_eth_signature(msg_hash: u256, signature: Signature, eth_address: EthAddress) { - match is_eth_signature_valid(:msg_hash, :signature, :eth_address) { - Result::Ok(()) => {}, - Result::Err(err) => panic_with_felt252(err), - } -} - -/// Checks a Secp256 ECDSA signature. -/// Also verifies that r and s components of the signature are in the range (0, N), -/// where N is the size of the curve. -/// Returns a Result with an error string if the signature is invalid. -fn is_eth_signature_valid( - msg_hash: u256, signature: Signature, eth_address: EthAddress -) -> Result<(), felt252> { - if !is_signature_entry_valid::(signature.r) { - return Result::Err('Signature out of range'); - } - if !is_signature_entry_valid::(signature.s) { - return Result::Err('Signature out of range'); - } - - let public_key_point = recover_public_key::(:msg_hash, :signature).unwrap(); - let calculated_eth_address = public_key_point_to_eth_address(:public_key_point); - if eth_address != calculated_eth_address { - return Result::Err('Invalid signature'); - } - Result::Ok(()) -} - - -/// Converts a public key point to the corresponding Ethereum address. -fn public_key_point_to_eth_address< - Secp256Point, +Drop, +Secp256Trait, +Secp256PointTrait ->( - public_key_point: Secp256Point -) -> EthAddress { - let (x, y) = public_key_point.get_coordinates().unwrap_syscall(); - - // Keccak output is little endian. - let point_hash_le = keccak_u256s_be_inputs(array![x, y].span()); - let point_hash = u256 { - low: integer::u128_byte_reverse(point_hash_le.high), - high: integer::u128_byte_reverse(point_hash_le.low) - }; - - point_hash.into() -} diff --git a/corelib/src/test/secp256k1_test.cairo b/corelib/src/test/secp256k1_test.cairo index 999ce10f2e3..1852c808629 100644 --- a/corelib/src/test/secp256k1_test.cairo +++ b/corelib/src/test/secp256k1_test.cairo @@ -4,7 +4,8 @@ use starknet::{ use starknet::secp256_trait::{ Signature, recover_public_key, Secp256PointTrait, signature_from_vrs, is_valid_signature }; -use starknet::secp256k1::{Secp256k1Point, Secp256k1PointImpl, verify_eth_signature}; +use starknet::secp256k1::{Secp256k1Point, Secp256k1PointImpl}; +use starknet::eth_signature::verify_eth_signature; #[test] fn test_secp256k1_recover_public_key() {