Ethereum address implicit accounts #9969

staffik · 2023-10-19T11:25:52Z

Part of near/NEPs#498

Approach:

Add AccountType enum: NamedAccount, NearImplicitAccount, or EthImplicitAccount.
Parse 40 characters long hexadecimal addresses prefixed with '0x' as EthImplicitAccount.
Add derive_account_id_from_public_key function that returns '0x' + keccak256(public_key)[12:32].hex() if the public key type is SECP256K1.
action_implicit_account_creation_transfer: in case of EthImplicitAccount creates an account without an access key. Fees are the same as for NearImplicitAccount minus ActionCosts::add_full_access_key.
verify_and_charge_transaction: if access key not found for transaction.public_key, and signer_id is EthImplicitAccount, and derive_account_id_from_public_key(transaction.public_key) == *signer_id, then add full access key to that account, with nonce equal to the transaction's nonce.
tools/mirror: EthImplicitAccount account has to be mapped to a new address, for which we know the secret key. Because the new address has to be derived from the secret key, we need to map the existing ETH address to a new secret key first. Because the ETH address length is 20 bytes, and secret key length is 32 bytes, the mapped secret key is less secure.

To do:

Make test_transaction_hash_collision_for_eth_implicit_account_fail passes.
Make meta_tx_create_eth_implicit_account test passes.
Verify why test_trying_to_create_eth_implicit_account_runtime passes, because it probably shouldn’t.
Discuss what nonce should be used for a transaction from EthImplicitAccountwhen there is no access key added to that account.
Test ETH-implicit addresses in Python tests (Nayduck).
Verify if the fuzzing testing util works correctly (runtime-tester).
Verify if the fork-network tool works correctly.
Verify if the mirror tool works correctly. First discuss the way ETH addresses (and keys) are mapped and whether it is secure enough.
Change protocol version, update documentation.

core/account-id/src/lib.rs

staffik

Questions and things left to be done.
@birchmd can you look at some of the questions?

staffik · 2023-10-26T14:47:46Z

chain/rosetta-rpc/src/lib.rs

@@ -502,6 +502,7 @@ async fn construction_derive(
    let public_key: near_crypto::PublicKey = (&public_key)
        .try_into()
        .map_err(|_| errors::ErrorKind::InvalidInput("Invalid PublicKey".to_string()))?;
+    // TODO Rosetta-RPC: should we handle ETH-implicit accounts here?


Should we handle ETH-implicit accounts here?

staffik · 2023-10-26T14:49:18Z

core/account-id/src/lib.rs

@@ -127,11 +143,35 @@ impl AccountId {
    /// assert!(!alice_app.is_sub_account_of(&near_tla));
    /// ```
    pub fn is_sub_account_of(&self, parent: &AccountId) -> bool {
+        // TODO Is it ok that an account could be a sub account of an EthImplicitAccount ?


Is it ok that an account could be a sub account of an ETH-implicit account?

staffik · 2023-10-26T14:52:19Z

integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs

+    );
+}
+
+// TODO, does not pass, see `verify_and_charge_transaction(...)`


TODO: does not pass because a transaction is replayed.

staffik · 2023-10-26T14:54:37Z

integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs

        ProcessTxResponse::InvalidTx(InvalidTxError::InvalidNonce { .. })
    );
 }

-/// Test that duplicate transactions from implicit accounts are not rejected until protocol upgrade.
+// TODO Since we set access key nonce differently when creating ETH-implicit accounts, we cannot repeat the below test for ETH-implicit account ?


TODO Verify after we decide on what nonce should be used by user.

staffik · 2023-10-26T14:57:55Z

integration-tests/src/tests/client/features/delegate_action.rs

+    meta_tx_create_implicit_account(near_implicit_test_account());
+}
+
+// TODO make the test passes


staffik · 2023-10-26T15:37:56Z

test-utils/runtime-tester/src/fuzzing.rs

@@ -517,6 +518,7 @@ impl Scope {
        Ok(self.accounts[self.accounts.len() - 1].clone())
    }

+    // Test-utils/fuzzing: used for runtime tests, make sure everything is ok.


staffik · 2023-10-26T15:40:48Z

tools/mirror/src/key_mapping.rs

+        AccountType::NearImplicitAccount => {
+            match key {
+                PublicKey::ED25519(k) => SecretKey::ED25519(map_ed25519(k, secret)),
+                // TODO Is it true?


Does NEAR-implicit account can only have ED25519 access key added?

staffik · 2023-10-26T15:41:30Z

tools/mirror/src/key_mapping.rs

+        AccountType::EthImplicitAccount => {
+            match key {
+                PublicKey::SECP256K1(_) => map_secp256k1_from_eth_address(account_id, secret),
+                // TODO Is it true?


Does ETH-implicit account can only have SECP256K1 access key added?

staffik · 2023-10-26T15:45:40Z

tools/mirror/src/key_mapping.rs

+// TODO Deriving secret keys from ETH addresses reduces potential entropy.
+// However, it is only for the Mirror tool. Are we ok with that?


ETH address is 20 bytes length, shorter than public key (32 bytes) from which it is derived.
It reduces potential entropy so it has security impact.
But mirror needs to map ETH address to a new one, and we first need to map the address to a new public key, from which we then derive a new ETH address.
Are we ok with the security impact for the mirror tool?

staffik · 2023-10-26T15:46:06Z

tools/mirror/src/lib.rs

-                            let public_key = PublicKey::from_implicit_account(&target_account)
-                                .expect("must be implicit");
-                            nonce_updates.insert((target_account, public_key));
+                            // TODO Tools/Mirror: For ETH-implicit account we do not add access key on creation, but make sure it is ok for Mirror.


staffik · 2023-10-27T11:31:33Z

Moving to #10018, in order to split this PR into several smaller PRs.

Part of near/nearcore#10018. This PR introduces some changes from near/nearcore#9969 laying groundwork for real protocol changes to be done in a separate PR. Summary: - Add `AccountType` enum: `NamedAccount`, `NearImplicitAccount`, or `EthImplicitAccount`. - Parse 40 characters long hexadecimal addresses prefixed with `'0x'` as `EthImplicitAccount`.

Part of #10018. This PR introduces some changes from #9969 laying groundwork for real protocol changes to be done in a separate PR. Some changes might look redundant (especially `match receiver_id.get_account_type() { ...`) but they will make it easier to read changes done in the next PR. Changes done in [near-account-id 1.0.0-alpha.2](near/near-account-id-rs#14): - Add `AccountType` enum: `NamedAccount`, `NearImplicitAccount`, or `EthImplicitAccount`. - Parse 40 characters long hexadecimal addresses prefixed with `'0x'` as `EthImplicitAccount`. - `AccountType::is_implicit()` returns true for both `NearImplicitAccount` and `EthImplicitAccount`. Summary of this PR: - Bump version of `near-account-id` to `1.0.0-alpha.2`. - Do not use `is_implicit()` for now, as we do not want to treat `EthImplicitAccount` as implicit as for now. - Add `derive_account_id_from_public_key` function that currently only supports `ED25519` key type and returns hex-encoded copy of the key. - Refactor/rename tests a bit.

staffik force-pushed the eth-wallet-n-transaction-support branch 2 times, most recently from 3981e27 to 3e47bf0 Compare October 19, 2023 15:03

walnut-the-cat reviewed Oct 19, 2023

View reviewed changes

core/account-id/src/lib.rs Outdated Show resolved Hide resolved

wacban reviewed Oct 20, 2023

View reviewed changes

core/account-id/src/lib.rs Outdated Show resolved Hide resolved

core/account-id/src/lib.rs Outdated Show resolved Hide resolved

staffik force-pushed the eth-wallet-n-transaction-support branch from dd17891 to 36a58be Compare October 23, 2023 14:00

staffik added 3 commits October 26, 2023 13:53

Mark implicit-account related code

993edb2

Add AccountType enum

0616f1f

Verify public key for ETH-implicit account

4797eec

staffik force-pushed the eth-wallet-n-transaction-support branch 3 times, most recently from bea1220 to 3b6c95e Compare October 26, 2023 14:46

staffik commented Oct 26, 2023

View reviewed changes

Add corresponding ETH-implicit account tests

f7b9af7

staffik force-pushed the eth-wallet-n-transaction-support branch from 3b6c95e to f7b9af7 Compare October 27, 2023 09:01

staffik closed this Oct 27, 2023

This was referenced Oct 27, 2023

Setup for ETH-implicit accounts #10020

Merged

AccountType, add EthImplicitAccount near/near-account-id-rs#14

Merged

Ekleog-NEAR deleted the eth-wallet-n-transaction-support branch March 29, 2024 14:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ethereum address implicit accounts #9969

Ethereum address implicit accounts #9969

staffik commented Oct 19, 2023 •

edited

Loading

staffik left a comment •

edited

Loading

staffik Oct 26, 2023

staffik Oct 26, 2023

staffik Oct 26, 2023 •

edited

Loading

staffik Oct 26, 2023 •

edited

Loading

staffik Oct 26, 2023

staffik Oct 26, 2023

staffik Oct 26, 2023

staffik Oct 26, 2023 •

edited

Loading

staffik Oct 26, 2023

staffik Oct 26, 2023

staffik commented Oct 27, 2023

		// TODO Deriving secret keys from ETH addresses reduces potential entropy.
		// However, it is only for the Mirror tool. Are we ok with that?

Ethereum address implicit accounts #9969

Ethereum address implicit accounts #9969

Conversation

staffik commented Oct 19, 2023 • edited Loading

staffik left a comment • edited Loading

Choose a reason for hiding this comment

staffik Oct 26, 2023

Choose a reason for hiding this comment

staffik Oct 26, 2023

Choose a reason for hiding this comment

staffik Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

staffik Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

staffik Oct 26, 2023

Choose a reason for hiding this comment

staffik Oct 26, 2023

Choose a reason for hiding this comment

staffik Oct 26, 2023

Choose a reason for hiding this comment

staffik Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

staffik Oct 26, 2023

Choose a reason for hiding this comment

staffik Oct 26, 2023

Choose a reason for hiding this comment

staffik commented Oct 27, 2023

staffik commented Oct 19, 2023 •

edited

Loading

staffik left a comment •

edited

Loading

staffik Oct 26, 2023 •

edited

Loading

staffik Oct 26, 2023 •

edited

Loading

staffik Oct 26, 2023 •

edited

Loading