Ethereum account

[PhilippeR26]

Motivation and Resolution

OpenZeppelin has released an Ethereum account contract v0.9.0 :
https://github.com/OpenZeppelin/cairo-contracts/tree/v0.9.0

This contract has been included in the test suite, for EthSigner validation.

A new cairo 2.5.3 core::starknet::secp256k1::Secp256k1Point type is necessary for the Ethereum public key, and has been added in Starknet.js. (@haroune-mohammedi )
https://github.com/OpenZeppelin/cairo-contracts/blob/7684fb0ca81a718d262145be6722e6f9f9493c54/src/account/utils/secp256k1.cairo#L42-L51 . So, an Eth public key is represented in Starknet.js by an hex string representing a 512 bits number, and is represented in Cairo by a tuple of 2 u256 : (u256,u256), so a raw format of 4 felts.

The format of the Eth signature is specific, and has to be conform to https://github.com/starkware-libs/cairo/blob/bd7cca1c3332daddc738682de0fea26da1b1973d/corelib/src/starknet/secp256_trait.cairo#L10-L18 . it's a struct with 2 u256 and a boolean, so coded on 5 felts.

Usage related changes

To connect to an Eth account :

const privateKeyETHraw="0x97ee6ca34cb49060f1c303c6cb7ee2d6123e617601ef3e31ccf7bf5bef1f9";
const ethSigner = new EthSigner(privateKeyETHraw);
const ETHaccount = new Account(provider, contractETHaddress, ethSigner, undefined, constants.TRANSACTION_VERSION.V2);

Development related changes

• Eth signer modified to use Secp256k1Point and Eth signature.
• Add tests for ETHSigner, core::starknet::secp256k1::Secp256k1Point` and OpenZeppelin Ethereum account.
• some few cleanup in code and doc.

Checklist:

• Performed a self-review of the code
• Rebased to the last commit of the target branch (or merged it into my branch)
• Documented the changes in code (API docs will be generated automatically)
• Updated the tests
• All tests are passing

[tabaktoni]

LGTM with a caveat.

• Can we add a separate test for the new Cairo type in test/utils/CairoType or an independent type test (Secp256k1Point)
• blind compile of this type will not work?
• Do we need to open an Issue on Abi Wan to support it?

[PhilippeR26]

LGTM with a caveat.

• Can we add a separate test for the new Cairo type in test/utils/CairoType or an independent type test (Secp256k1Point)

I will extract this test in a separate file (there is no specific class for this cairo type).

• blind compile of this type will not work?

Do you mean CallData.compile()?
No, it can't work, as the CallData.compile() method is not aware that it's a Secp256k1Point.

• Do we need to open an Issue on Abi Wan to support it?

I alerted Haroun in my first message. I don't know at all Abiwan, but I can tell that a hex string or a Uint8Array has to be provided.

[PhilippeR26]

Secp256k1Point test file created.

[tabaktoni]

LGTM!
Res. conflict to pass the test and can be merged

[PhilippeR26]

LGTM! Res. conflict to pass the test and can be merged

Done

[penovicp]

In general, lgtm

Left a suggestion for the docs.

Also note that there is an issue with the maxFee estimation for the ethAccount deploy in the beforeAll step of ETH account tx V2 suite. It can be bypassed by manually providing the maxFee, but it probably makes sense to check the origin of the issue, if it's down to the code or devnet.

[PhilippeR26]

Tests where OK 3 weeks ago.
MaxFee is something clearly not stable in Testnets and Devnet-rs.

[haroune-mohammedi]

So @PhilippeR26, If I understand well, we have to add core::starknet::secp256k1::Secp256k1Point and accept a string for it ?

[PhilippeR26]

A BigNumberish

[penovicp]

Looks like adjusting the fee estimate for the tests to pass should be ok.

After that is done this PR should be ok to merge.

[PhilippeR26]

Looks like adjusting the fee estimate for the tests to pass should be ok.

After that is done this PR should be ok to merge.

Done.

[haroune-mohammedi]

core::starknet::secp256k1::Secp256k1Point

OK. It'll be address here keep-starknet-strange/abi-wan-kanabi#68