diff --git a/cairo/src/precompiles/ec_recover.cairo b/cairo/src/precompiles/ec_recover.cairo index 4a5d5b40..036154de 100644 --- a/cairo/src/precompiles/ec_recover.cairo +++ b/cairo/src/precompiles/ec_recover.cairo @@ -79,24 +79,14 @@ namespace PrecompileEcRecover { let (r_bigint) = uint256_to_bigint(r); let (s_bigint) = uint256_to_bigint(s); - let (public_key_point, success) = Signature.try_recover_public_key( + let (success, recovered_address) = Signature.try_recover_eth_address( msg_hash_bigint, r_bigint, s_bigint, v - 27 ); + if (success == 0) { let (output) = alloc(); return (0, output, GAS_COST_EC_RECOVER, 0); } - let (is_public_key_invalid) = EcRecoverHelpers.ec_point_equal( - public_key_point, EcPoint(BigInt3(0, 0, 0), BigInt3(0, 0, 0)) - ); - if (is_public_key_invalid != 0) { - let (output) = alloc(); - return (0, output, GAS_COST_EC_RECOVER, 0); - } - - let (recovered_address) = EcRecoverHelpers.public_key_point_to_eth_address( - public_key_point - ); let (output) = alloc(); memset(output, 0, 12); @@ -115,26 +105,4 @@ namespace EcRecoverHelpers { } return (is_equal=0); } - - // @notice Convert a public key point to the corresponding Ethereum address. - // @dev Uses the `KeccakBuiltin` builtin, while the one in Starkware's CairoZero library does not. - func public_key_point_to_eth_address{ - pedersen_ptr: HashBuiltin*, - range_check_ptr, - bitwise_ptr: BitwiseBuiltin*, - keccak_ptr: KeccakBuiltin*, - }(public_key_point: EcPoint) -> (eth_address: felt) { - alloc_locals; - let (local elements: Uint256*) = alloc(); - let (x_uint256: Uint256) = bigint_to_uint256(public_key_point.x); - assert elements[0] = x_uint256; - let (y_uint256: Uint256) = bigint_to_uint256(public_key_point.y); - assert elements[1] = y_uint256; - - let (point_hash) = keccak_uint256s_bigend(n_elements=2, elements=elements); - - // The Ethereum address is the 20 least significant bytes of the keccak of the public key. - let (high_high, high_low) = unsigned_div_rem(point_hash.high, 2 ** 32); - return (eth_address=point_hash.low + RC_BOUND * high_low); - } } diff --git a/cairo/src/utils/signature.cairo b/cairo/src/utils/signature.cairo index a8302d7b..1d161126 100644 --- a/cairo/src/utils/signature.cairo +++ b/cairo/src/utils/signature.cairo @@ -1,5 +1,5 @@ -from starkware.cairo.common.cairo_builtins import BitwiseBuiltin -from starkware.cairo.common.cairo_secp.bigint3 import BigInt3, UnreducedBigInt3 +from starkware.cairo.common.cairo_builtins import BitwiseBuiltin, KeccakBuiltin +from starkware.cairo.common.cairo_secp.bigint3 import BigInt3 from starkware.cairo.common.cairo_secp.ec_point import EcPoint from starkware.cairo.common.cairo_secp.signature import ( validate_signature_entry, @@ -7,19 +7,21 @@ from starkware.cairo.common.cairo_secp.signature import ( get_generator_point, div_mod_n, ) +from starkware.cairo.common.math_cmp import RC_BOUND +from starkware.cairo.common.cairo_secp.bigint import bigint_to_uint256, uint256_to_bigint +from starkware.cairo.common.builtin_keccak.keccak import keccak_uint256s_bigend from starkware.cairo.common.cairo_secp.ec import ec_add, ec_mul, ec_negate from starkware.cairo.common.uint256 import Uint256 from starkware.cairo.common.alloc import alloc -from starkware.cairo.common.cairo_secp.bigint import uint256_to_bigint +from src.utils.maths import unsigned_div_rem from src.interfaces.interfaces import ICairo1Helpers namespace Signature { - // A version of verify_eth_signature, with that msg_hash, r and s as Uint256 and - // using the Cairo1 helpers class. - func verify_eth_signature_uint256{range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - msg_hash: Uint256, r: Uint256, s: Uint256, y_parity: felt, eth_address: felt - ) { + // A version of verify_eth_signature that uses the keccak builtin. + func verify_eth_signature_uint256{ + range_check_ptr, bitwise_ptr: BitwiseBuiltin*, keccak_ptr: KeccakBuiltin* + }(msg_hash: Uint256, r: Uint256, s: Uint256, y_parity: felt, eth_address: felt) { alloc_locals; let (msg_hash_bigint: BigInt3) = uint256_to_bigint(msg_hash); let (r_bigint: BigInt3) = uint256_to_bigint(r); @@ -35,34 +37,34 @@ namespace Signature { } with_attr error_message("Invalid signature.") { - let (success, recovered_address) = ICairo1Helpers.recover_eth_address( - msg_hash=msg_hash, r=r, s=s, y_parity=y_parity + let (success, recovered_address) = try_recover_eth_address( + msg_hash=msg_hash_bigint, r=r_bigint, s=s_bigint, y_parity=y_parity ); - // TODO: uncomment when we have a working recover_eth_address - // assert success = 1; - // assert eth_address = recovered_address; + assert success = 1; } + + assert eth_address = recovered_address; return (); } - // Similar to `recover_public_key`, but handles the case where 'x' does not correspond to a point on the + // @notice Similar to `recover_public_key`, but handles the case where 'x' does not correspond to a point on the // curve gracefully. - // Receives a signature and the signed message hash. - // Returns the public key associated with the signer, represented as a point on the curve, and `true` if valid. - // Returns the point (0, 0) and `false` otherwise. - // Note: - // Some places use the values 27 and 28 instead of 0 and 1 for v. - // In that case, a subtraction by 27 returns a v that can be used by this function. - // Prover assumptions: - // * r is the x coordinate of some nonzero point on the curve. - // * All the limbs of s and msg_hash are in the range (-2 ** 210.99, 2 ** 210.99). - // * All the limbs of r are in the range (-2 ** 124.99, 2 ** 124.99). + // @param msg_hash The signed message hash. + // @param r The r value of the signature. + // @param s The s value of the signature. + // @param y_parity The y parity value of the signature. true if odd, false if even. + // @return The public key associated with the signer, represented as a point on the curve, and `true` if valid. + // @return The point (0, 0) and `false` otherwise. + // @dev Prover assumptions: + // @dev * r is the x coordinate of some nonzero point on the curve. + // @dev * All the limbs of s and msg_hash are in the range (-2 ** 210.99, 2 ** 210.99). + // @dev * All the limbs of r are in the range (-2 ** 124.99, 2 ** 124.99). func try_recover_public_key{range_check_ptr}( - msg_hash: BigInt3, r: BigInt3, s: BigInt3, v: felt + msg_hash: BigInt3, r: BigInt3, s: BigInt3, y_parity: felt ) -> (public_key_point: EcPoint, success: felt) { alloc_locals; let (local r_point: EcPoint*) = alloc(); - let (is_on_curve) = try_get_point_from_x(x=r, v=v, result=r_point); + let (is_on_curve) = try_get_point_from_x(x=r, v=y_parity, result=r_point); if (is_on_curve == 0) { return (public_key_point=EcPoint(x=BigInt3(0, 0, 0), y=BigInt3(0, 0, 0)), success=0); } @@ -84,4 +86,49 @@ namespace Signature { let (public_key_point) = ec_add(minus_point1, point2); return (public_key_point=public_key_point, success=1); } + + // @notice Recovers the Ethereum address from a signature. + // @dev If the public key point is not on the curve, the function returns success=0. + // @dev: This function does not validate the r, s values. + // @param msg_hash The signed message hash. + // @param r The r value of the signature. + // @param s The s value of the signature. + // @param y_parity The y parity value of the signature. true if odd, false if even. + // @return The Ethereum address. + func try_recover_eth_address{ + range_check_ptr, bitwise_ptr: BitwiseBuiltin*, keccak_ptr: KeccakBuiltin* + }(msg_hash: BigInt3, r: BigInt3, s: BigInt3, y_parity: felt) -> (success: felt, address: felt) { + alloc_locals; + let (public_key_point, success) = try_recover_public_key( + msg_hash=msg_hash, r=r, s=s, y_parity=y_parity + ); + if (success == 0) { + return (success=0, address=0); + } + let (x_uint256) = bigint_to_uint256(public_key_point.x); + let (y_uint256) = bigint_to_uint256(public_key_point.y); + let address = Internals.public_key_point_to_eth_address(x=x_uint256, y=y_uint256); + return (success=success, address=address); + } +} + +namespace Internals { + // @notice Converts a public key point to the corresponding Ethereum address. + // @param x The x coordinate of the public key point. + // @param y The y coordinate of the public key point. + // @return The Ethereum address. + func public_key_point_to_eth_address{ + range_check_ptr, bitwise_ptr: BitwiseBuiltin*, keccak_ptr: KeccakBuiltin* + }(x: Uint256, y: Uint256) -> felt { + alloc_locals; + let (local elements: Uint256*) = alloc(); + assert elements[0] = x; + assert elements[1] = y; + let (point_hash: Uint256) = keccak_uint256s_bigend(n_elements=2, elements=elements); + + // The Ethereum address is the 20 least significant bytes of the keccak of the public key. + let (_, high_low) = unsigned_div_rem(point_hash.high, 2 ** 32); + let eth_address = point_hash.low + RC_BOUND * high_low; + return eth_address; + } } diff --git a/cairo/tests/ethereum/cancun/test_trie.py b/cairo/tests/ethereum/cancun/test_trie.py index 8a2730a6..e88d78b2 100644 --- a/cairo/tests/ethereum/cancun/test_trie.py +++ b/cairo/tests/ethereum/cancun/test_trie.py @@ -105,11 +105,14 @@ def test_bytes_to_nibble_list(self, cairo_run, bytes_: Bytes): # def test_root(self, cairo_run, trie, get_storage_root): # assert root(trie, get_storage_root) == cairo_run("root", trie, get_storage_root) - @given(obj=st.dictionaries(nibble, bytes32), nibble=uint4, level=uint4) + @given( + obj=st.dictionaries(nibble, bytes32).filter(lambda x: len(x) > 0), + nibble=uint4, + level=uint4, + ) def test_get_branche_for_nibble_at_level(self, cairo_run, obj, nibble, level): assume( - len(obj) > 0 # no empty objects - and min(len(k) for k in obj) > level # longer than level + min(len(k) for k in obj) > level # longer than level and len({k[:level] for k in obj}) == 1 # same prefix at level ) branche, value = cairo_run( diff --git a/cairo/tests/fixtures/runner.py b/cairo/tests/fixtures/runner.py index c66114a7..6e7f9f33 100644 --- a/cairo/tests/fixtures/runner.py +++ b/cairo/tests/fixtures/runner.py @@ -126,7 +126,8 @@ def _factory(entrypoint, *args, **kwargs): output_ptr = runner.segments.add() stack.append(output_ptr) else: - stack.append(gen_arg(python_type, kwargs.get(arg_name, args[i]))) + arg_value = kwargs[arg_name] if arg_name in kwargs else args[i] + stack.append(gen_arg(python_type, arg_value)) return_fp = runner.execution_base + 2 end = runner.program_base + len(runner.program.data) diff --git a/cairo/tests/src/precompiles/test_ec_recover.py b/cairo/tests/src/precompiles/test_ec_recover.py index f80d5130..fe0b2950 100644 --- a/cairo/tests/src/precompiles/test_ec_recover.py +++ b/cairo/tests/src/precompiles/test_ec_recover.py @@ -17,17 +17,17 @@ def ecrecover(data): s = U256.from_be_bytes(data[96:128]) if v != 27 and v != 28: - return + return [] if 0 >= r or r >= SECP256K1N: - return + return [] if 0 >= s or s >= SECP256K1N: - return + return [] try: public_key = secp256k1_recover(r, s, v - 27, message_hash) except ValueError: # unable to extract public key - return + return [] address = keccak256(public_key)[12:32] padded_address = left_pad_zero_bytes(address, 32) @@ -40,11 +40,11 @@ class TestEcRecover: def test_valid_signature(self, message, cairo_run): """Test with valid signatures generated from random messages.""" private_key = generate_random_private_key() - msg = keccak256(message) - (v, r, s) = ec_sign(msg, private_key) + msg_hash = keccak256(message) + (v, r, s) = ec_sign(msg_hash, private_key) input_data = [ - *msg, + *msg_hash, *v.to_bytes(32, "big"), *r, *s, @@ -75,8 +75,9 @@ def test_invalid_v(self, v, msg, r, s, cairo_run): *r.to_bytes(32, "big"), *s.to_bytes(32, "big"), ] + py_result = ecrecover(input_data) [output] = cairo_run("test__ec_recover", input=input_data) - assert output == [] + assert output == py_result @given( v=st.integers(min_value=27, max_value=28), @@ -96,8 +97,8 @@ def test_parameter_boundaries(self, cairo_run, v, msg, r, s): py_result = ecrecover(input_data) [cairo_result] = cairo_run("test__ec_recover", input=input_data) - if py_result is None: - assert cairo_result == [] + if len(py_result) == 0: + assert cairo_result == py_result else: py_address, _ = py_result assert bytes(cairo_result) == bytes(py_address) diff --git a/cairo/tests/src/utils/test_signature.cairo b/cairo/tests/src/utils/test_signature.cairo index b5fe9316..95e8875f 100644 --- a/cairo/tests/src/utils/test_signature.cairo +++ b/cairo/tests/src/utils/test_signature.cairo @@ -1,13 +1,37 @@ -from starkware.cairo.common.cairo_builtins import BitwiseBuiltin - from ethereum.base_types import U256 -from src.utils.signature import Signature -func test__verify_eth_signature_uint256{range_check_ptr, bitwise_ptr: BitwiseBuiltin*}( - msg_hash: U256, r: U256, s: U256, y_parity: felt, eth_address: felt -) { +from starkware.cairo.common.cairo_builtins import BitwiseBuiltin, KeccakBuiltin +from starkware.cairo.common.uint256 import Uint256 +from src.utils.signature import Signature, Internals +from starkware.cairo.common.cairo_secp.bigint import uint256_to_bigint + +func test__public_key_point_to_eth_address{ + range_check_ptr, bitwise_ptr: BitwiseBuiltin*, keccak_ptr: KeccakBuiltin* +}(x: U256, y: U256) -> felt { + let eth_address = Internals.public_key_point_to_eth_address(x=[x.value], y=[y.value]); + + return eth_address; +} + +func test__verify_eth_signature_uint256{ + range_check_ptr, bitwise_ptr: BitwiseBuiltin*, keccak_ptr: KeccakBuiltin* +}(msg_hash: U256, r: U256, s: U256, y_parity: felt, eth_address: felt) { Signature.verify_eth_signature_uint256( [msg_hash.value], [r.value], [s.value], y_parity, eth_address ); return (); } + +func test__try_recover_eth_address{ + range_check_ptr, bitwise_ptr: BitwiseBuiltin*, keccak_ptr: KeccakBuiltin* +}(msg_hash: U256, r: U256, s: U256, y_parity: felt) -> (success: felt, address: felt) { + let (msg_hash_bigint) = uint256_to_bigint([msg_hash.value]); + let (r_bigint) = uint256_to_bigint([r.value]); + let (s_bigint) = uint256_to_bigint([s.value]); + + let (success, address) = Signature.try_recover_eth_address( + msg_hash=msg_hash_bigint, r=r_bigint, s=s_bigint, y_parity=y_parity + ); + + return (success=success, address=address); +} diff --git a/cairo/tests/src/utils/test_signature.py b/cairo/tests/src/utils/test_signature.py index 937aa2ea..eca55c81 100644 --- a/cairo/tests/src/utils/test_signature.py +++ b/cairo/tests/src/utils/test_signature.py @@ -1,32 +1,143 @@ -from hypothesis import assume, given -from hypothesis.strategies import integers +import pytest +from eth_keys.datatypes import PrivateKey +from hypothesis import given +from hypothesis import strategies as st from starkware.cairo.lang.cairo_constants import DEFAULT_PRIME -from ethereum.base_types import U256 +from ethereum.base_types import U256, Bytes32 from ethereum.crypto.elliptic_curve import SECP256K1N from tests.utils.errors import cairo_error from tests.utils.strategies import felt class TestSignature: - @given( - msg_hash=..., - r=..., - s=..., - y_parity=integers(min_value=2, max_value=DEFAULT_PRIME - 1), - eth_address=felt, - ) - def test_should_raise_with_invalid_y_parity( - self, cairo_run, msg_hash: U256, r: U256, s: U256, y_parity, eth_address - ): - assume(r != 0 and s != 0) - assume(r < SECP256K1N and s < SECP256K1N) - with cairo_error("Invalid y_parity"): + class TestPublicKeyPointToEthAddress: + @given(private_key=...) + def test__public_key_point_to_eth_address( + self, cairo_run, private_key: PrivateKey + ): + public_key = private_key.public_key + public_key_bytes = public_key.to_bytes() + x = U256.from_be_bytes(public_key_bytes[0:32]) + y = U256.from_be_bytes(public_key_bytes[32:64]) + + expected_address = public_key.to_address() + result = cairo_run( + "test__public_key_point_to_eth_address", + x=x, + y=y, + ) + assert result == int(expected_address, 16) + + class TestVerifyEthSignature: + @pytest.mark.slow + @given(private_key=..., message=...) + def test__verify_eth_signature_uint256( + self, cairo_run, private_key: PrivateKey, message: Bytes32 + ): + signature = private_key.sign_msg_hash(message) + eth_address = int(private_key.public_key.to_address(), 16) cairo_run( "test__verify_eth_signature_uint256", - msg_hash, - r, - s, - y_parity, - eth_address, + msg_hash=U256.from_be_bytes(message), + r=U256(signature.r), + s=U256(signature.s), + y_parity=signature.v, + eth_address=eth_address, + ) + + @given( + msg_hash=..., + r=st.integers(min_value=1, max_value=SECP256K1N - 1).map(U256), + s=st.integers(min_value=1, max_value=SECP256K1N - 1).map(U256), + y_parity=st.integers(min_value=2, max_value=DEFAULT_PRIME - 1), + eth_address=felt, + ) + def test_should_raise_with_invalid_y_parity( + self, cairo_run, msg_hash: U256, r: U256, s: U256, y_parity, eth_address + ): + with cairo_error("Invalid y_parity"): + cairo_run( + "test__verify_eth_signature_uint256", + msg_hash, + r, + s, + y_parity, + eth_address, + ) + + @given( + msg_hash=..., + r=st.one_of( + st.just(U256(0)), + st.integers(min_value=SECP256K1N, max_value=2**256 - 1).map(U256), + ), + s=..., + y_parity=..., + eth_address=felt, + ) + def test_should_raise_with_out_of_bounds_r( + self, + cairo_run, + msg_hash: U256, + r: U256, + s: U256, + y_parity: bool, + eth_address, + ): + with cairo_error("Signature out of range."): + cairo_run( + "test__verify_eth_signature_uint256", + msg_hash, + r, + s, + y_parity, + eth_address, + ) + + @given( + msg_hash=..., + r=..., + s=st.one_of( + st.just(U256(0)), + st.integers(min_value=SECP256K1N, max_value=2**256 - 1).map(U256), + ), + y_parity=..., + eth_address=felt, + ) + def test_should_raise_with_out_of_bounds_s( + self, + cairo_run, + msg_hash: U256, + r: U256, + s: U256, + y_parity: bool, + eth_address, + ): + with cairo_error("Signature out of range."): + cairo_run( + "test__verify_eth_signature_uint256", + msg_hash, + r, + s, + y_parity, + eth_address, + ) + + class TestTryRecoverEthAddress: + @pytest.mark.slow + @given(private_key=..., message=...) + def test__try_recover_eth_address( + self, cairo_run, private_key: PrivateKey, message: Bytes32 + ): + signature = private_key.sign_msg_hash(message) + expected_address = int(private_key.public_key.to_address(), 16) + success, address = cairo_run( + "test__try_recover_eth_address", + msg_hash=U256.from_be_bytes(message), + r=U256(signature.r), + s=U256(signature.s), + y_parity=signature.v, ) + assert success == 1 + assert address == expected_address diff --git a/cairo/tests/utils/strategies.py b/cairo/tests/utils/strategies.py index b659e5b8..adbdd588 100644 --- a/cairo/tests/utils/strategies.py +++ b/cairo/tests/utils/strategies.py @@ -4,6 +4,7 @@ from typing import ForwardRef, Sequence, TypeAlias, Union from unittest.mock import patch +from eth_keys.datatypes import PrivateKey from hypothesis import strategies as st from starkware.cairo.lang.cairo_constants import DEFAULT_PRIME @@ -18,6 +19,7 @@ FixedUint, Uint, ) +from ethereum.crypto.elliptic_curve import SECP256K1N # Mock the Extended type because hypothesis cannot handle the RLP Protocol # Needs to be done before importing the types from ethereum.cancun.trie @@ -101,6 +103,12 @@ def st_from_type(cls): ) ) +private_key = ( + st.integers(min_value=1, max_value=SECP256K1N - 1) + .map(lambda x: int.to_bytes(x, 32, "big")) + .map(PrivateKey) +) + def register_type_strategies(): st.register_type_strategy(U64, uint64) @@ -144,3 +152,4 @@ def register_type_strategies(): ), ) st.register_type_strategy(BranchNode, st_from_type(BranchNode)) + st.register_type_strategy(PrivateKey, private_key)