From bea1220f099cb8dc3cab662a1758868506647948 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Chuda=C5=9B?= Date: Thu, 26 Oct 2023 13:53:20 +0200 Subject: [PATCH] Add corresponding ETH-implicit account tests --- Cargo.lock | 2 +- chain/rosetta-rpc/src/lib.rs | 2 +- core/account-id/src/lib.rs | 67 ++++++++++--- core/crypto/src/signature.rs | 7 ++ core/crypto/src/test_utils.rs | 7 +- core/primitives-core/src/runtime/fees.rs | 4 +- core/primitives/Cargo.toml | 1 + core/primitives/src/errors.rs | 6 +- core/primitives/src/test_utils.rs | 23 +++-- core/primitives/src/utils.rs | 31 ++++++ .../access_key_nonce_for_implicit_accounts.rs | 51 ++++++---- .../tests/client/features/delegate_action.rs | 95 +++++++++++++------ .../src/tests/client/features/restrict_tla.rs | 2 +- .../src/tests/standard_cases/mod.rs | 62 ++++++++---- .../src/tests/standard_cases/runtime.rs | 24 ++++- integration-tests/src/user/mod.rs | 19 +++- pytest/lib/key.py | 2 +- pytest/tests/loadtest/locust/common/sweat.py | 2 +- pytest/tests/replay/replay.py | 2 +- pytest/tests/sanity/rosetta.py | 12 +-- pytest/tools/mirror/mirror_utils.py | 2 + runtime/runtime/Cargo.toml | 1 - runtime/runtime/src/actions.rs | 2 +- runtime/runtime/src/config.rs | 1 - runtime/runtime/src/verifier.rs | 27 +++--- test-utils/runtime-tester/src/fuzzing.rs | 1 + test-utils/testlib/src/fees_utils.rs | 27 +++++- tools/fork-network/src/cli.rs | 16 ++-- tools/mirror/src/genesis.rs | 20 ++-- tools/mirror/src/key_mapping.rs | 89 +++++++++++++---- tools/mirror/src/lib.rs | 25 ++--- 31 files changed, 445 insertions(+), 187 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 227515dbbf5..63c43ae5841 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4119,6 +4119,7 @@ dependencies = [ "serde_json", "serde_with", "serde_yaml", + "sha3", "smart-default", "strum", "thiserror", @@ -4713,7 +4714,6 @@ dependencies = [ "serde", "serde_json", "sha2 0.10.6", - "sha3", "tempfile", "testlib", "thiserror", diff --git a/chain/rosetta-rpc/src/lib.rs b/chain/rosetta-rpc/src/lib.rs index 7d6829ed245..d3bf9e950f9 100644 --- a/chain/rosetta-rpc/src/lib.rs +++ b/chain/rosetta-rpc/src/lib.rs @@ -502,7 +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 should ETH-implicit account be handled there? + // TODO Rosetta-RPC: should we handle ETH-implicit accounts here? let address = if let near_crypto::KeyType::ED25519 = public_key.key_type() { hex::encode(public_key.key_data()) } else { diff --git a/core/account-id/src/lib.rs b/core/account-id/src/lib.rs index 22a4d31ce9a..de00a852232 100644 --- a/core/account-id/src/lib.rs +++ b/core/account-id/src/lib.rs @@ -143,13 +143,13 @@ impl AccountId { /// assert!(!alice_app.is_sub_account_of(&near_tla)); /// ``` pub fn is_sub_account_of(&self, parent: &AccountId) -> bool { - // TODO what if this is EthImplicitAccount ? + // TODO Is it ok that an account could be a sub account of an EthImplicitAccount ? self.strip_suffix(parent.as_str()) .and_then(|s| s.strip_suffix('.')) .map_or(false, |s| !s.contains('.')) } - /// Returns `true` if the `AccountId` is a 40 characters long hexadecimal, possibly with capital letters. + /// Returns `true` if the `AccountId` is a 40 characters long hexadecimal prefixed with '0x'. /// /// See [Implicit-Accounts](https://docs.near.org/docs/concepts/account#implicit-accounts). /// @@ -161,7 +161,7 @@ impl AccountId { /// let alice: AccountId = "alice.near".parse().unwrap(); /// assert!(!alice.is_eth_implicit()); /// - /// let rando = "0xb794f5eA0ba39494ce839613FFfba74279579268" + /// let rando = "0xb794f5ea0ba39494ce839613fffba74279579268" /// .parse::() /// .unwrap(); /// assert!(rando.is_eth_implicit()); @@ -169,7 +169,7 @@ impl AccountId { fn is_eth_implicit(&self) -> bool { self.len() == 42 && self.starts_with("0x") - && self[2..].as_bytes().iter().all(|b| matches!(b, b'a'..=b'f' | b'A'..=b'F' | b'0'..=b'9')) + && self[2..].as_bytes().iter().all(|b| matches!(b, b'a'..=b'f' | b'0'..=b'9')) } /// Returns `true` if the `AccountId` is a 64 characters long hexadecimal. @@ -475,7 +475,7 @@ mod tests { "b-o_w_e-n", "no_lols", "0123456789012345678901234567890123456789012345678901234567890123", - "0xb794f5eA0ba39494ce839613FFfba74279579268", + "0xb794f5ea0ba39494ce839613fffba74279579268", // Valid, but can't be created "near.a", ]; @@ -591,6 +591,7 @@ mod tests { "alex-skidanov", "b-o_w_e-n", "no_lols", + "0xb794f5ea0ba39494ce839613fffba74279579268", "0123456789012345678901234567890123456789012345678901234567890123", ]; for account_id in ok_top_level_account_ids { @@ -715,6 +716,10 @@ mod tests { "123456789012345678901234567890123456789012345678901234567890", "1234567890.123456789012345678901234567890123456789012345678901234567890", ), + ( + "b794f5ea0ba39494ce839613fffba74279579268", + "0xb794f5ea0ba39494ce839613fffba74279579268", + ), ("aa", "ъ@aa"), ("aa", "ъ.aa"), ]; @@ -731,17 +736,16 @@ mod tests { } } - // TODO add corresponding test for 42 len hex ETH accounts #[test] - fn test_is_account_id_64_len_hex() { - let valid_64_len_hex_account_ids = &[ + fn test_is_account_id_near_implicit() { + let valid_near_implicit_account_ids = &[ "0000000000000000000000000000000000000000000000000000000000000000", "6174617461746174617461746174617461746174617461746174617461746174", "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", "20782e20662e64666420482123494b6b6c677573646b6c66676a646b6c736667", ]; - for valid_account_id in valid_64_len_hex_account_ids { + for valid_account_id in valid_near_implicit_account_ids { assert!( matches!( valid_account_id.parse::(), @@ -752,7 +756,7 @@ mod tests { ); } - let invalid_64_len_hex_account_ids = &[ + let invalid_near_implicit_account_ids = &[ "000000000000000000000000000000000000000000000000000000000000000", "6.74617461746174617461746174617461746174617461746174617461746174", "012-456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", @@ -760,7 +764,7 @@ mod tests { "oooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo", "00000000000000000000000000000000000000000000000000000000000000", ]; - for invalid_account_id in invalid_64_len_hex_account_ids { + for invalid_account_id in invalid_near_implicit_account_ids { assert!( !matches!( invalid_account_id.parse::(), @@ -772,6 +776,47 @@ mod tests { } } + #[test] + fn test_is_account_id_eth_implicit() { + let valid_eth_implicit_account_ids = &[ + "0x0000000000000000000000000000000000000000", + "0x6174617461746174617461746174617461746174", + "0x0123456789abcdef0123456789abcdef01234567", + "0xffffffffffffffffffffffffffffffffffffffff", + "0x20782e20662e64666420482123494b6b6c677573", + ]; + for valid_account_id in valid_eth_implicit_account_ids { + assert!( + matches!( + valid_account_id.parse::(), + Ok(account_id) if account_id.get_account_type() == AccountType::EthImplicitAccount + ), + "Account ID {} should be valid 42-len hex, starting with 0x", + valid_account_id + ); + } + + let invalid_eth_implicit_account_ids = &[ + "04b794f5ea0ba39494ce839613fffba74279579268", + "0x000000000000000000000000000000000000000", + "0x6.74617461746174617461746174617461746174", + "0x012-456789abcdef0123456789abcdef01234567", + "0xfffff_ffffffffffffffffffffffffffffffffff", + "0xoooooooooooooooooooooooooooooooooooooooo", + "0x00000000000000000000000000000000000000000", + ]; + for invalid_account_id in invalid_eth_implicit_account_ids { + assert!( + !matches!( + invalid_account_id.parse::(), + Ok(account_id) if account_id.get_account_type() == AccountType::EthImplicitAccount + ), + "Account ID {} is not an implicit account", + invalid_account_id + ); + } + } + #[test] #[cfg(feature = "arbitrary")] fn test_arbitrary() { diff --git a/core/crypto/src/signature.rs b/core/crypto/src/signature.rs index 966559c1fb5..c9010dd6b5b 100644 --- a/core/crypto/src/signature.rs +++ b/core/crypto/src/signature.rs @@ -156,6 +156,13 @@ impl PublicKey { Self::SECP256K1(_) => panic!(), } } + + pub fn unwrap_as_secp256k1(&self) -> &Secp256K1PublicKey { + match self { + Self::SECP256K1(key) => key, + Self::ED25519(_) => panic!(), + } + } } // This `Hash` implementation is safe since it retains the property diff --git a/core/crypto/src/test_utils.rs b/core/crypto/src/test_utils.rs index 08e8435f5fa..999eebb0697 100644 --- a/core/crypto/src/test_utils.rs +++ b/core/crypto/src/test_utils.rs @@ -29,8 +29,11 @@ impl PublicKey { KeyType::ED25519 => { let keypair = ed25519_key_pair_from_seed(seed); PublicKey::ED25519(ED25519PublicKey(keypair.public.to_bytes())) - } - _ => unimplemented!(), + }, + KeyType::SECP256K1 => { + let secret_key = SecretKey::SECP256K1(secp256k1_secret_key_from_seed(seed)); + PublicKey::SECP256K1(secret_key.public_key().unwrap_as_secp256k1().clone()) + }, } } } diff --git a/core/primitives-core/src/runtime/fees.rs b/core/primitives-core/src/runtime/fees.rs index e6637754d63..aa5e245107a 100644 --- a/core/primitives-core/src/runtime/fees.rs +++ b/core/primitives-core/src/runtime/fees.rs @@ -214,7 +214,7 @@ pub fn transfer_exec_fee( let mut result = cfg.fee(ActionCosts::transfer).exec_fee(); if is_receiver_implicit { result += cfg.fee(ActionCosts::create_account).exec_fee(); - if receiver_account_type != AccountType::EthImplicitAccount { + if receiver_account_type == AccountType::NearImplicitAccount { result += cfg.fee(ActionCosts::add_full_access_key).exec_fee(); } } @@ -230,7 +230,7 @@ pub fn transfer_send_fee( let mut result = cfg.fee(ActionCosts::transfer).send_fee(sender_is_receiver); if is_receiver_implicit { result += cfg.fee(ActionCosts::create_account).send_fee(sender_is_receiver); - if receiver_account_type != AccountType::EthImplicitAccount { + if receiver_account_type == AccountType::NearImplicitAccount { result += cfg.fee(ActionCosts::add_full_access_key).send_fee(sender_is_receiver); } } diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml index 4334d4f3447..77472213c04 100644 --- a/core/primitives/Cargo.toml +++ b/core/primitives/Cargo.toml @@ -29,6 +29,7 @@ serde.workspace = true serde_json.workspace = true serde_with.workspace = true serde_yaml.workspace = true +sha3.workspace = true smart-default.workspace = true stdx.workspace = true strum.workspace = true diff --git a/core/primitives/src/errors.rs b/core/primitives/src/errors.rs index 8e581346e1d..959dc09936c 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -209,7 +209,7 @@ pub enum InvalidAccessKeyError { }, /// Having a deposit with a function call action is not allowed with a function call access key. DepositWithFunctionCall, - /// Transaction is from ETH-implicit `account_id` and uses invalid `public_key` for that address. + /// ETH-implicit `account_id` isn't derived from the `public_key`. InvalidPkForEthAddress { account_id: AccountId, public_key: PublicKey }, } @@ -487,7 +487,7 @@ pub enum ActionErrorKind { /// receipt validation. NewReceiptValidationError(ReceiptValidationError), /// Error occurs when a `CreateAccount` action is called on hex-characters - /// account of length 64 or 42. See implicit account creation NEP: + /// account of length 64 or 40 (when prefixed with '0x'). See implicit account creation NEP: /// . /// /// TODO(#8598): This error is named very poorly. A better name would be @@ -614,7 +614,7 @@ impl Display for InvalidAccessKeyError { }, InvalidAccessKeyError::InvalidPkForEthAddress { account_id, public_key } => write!( f, - "Address {:?} is ETH-implicit and does not correspond to the public_key {}", + "ETH-implicit address {:?} isn't derived from the public_key {}", account_id, public_key ), } diff --git a/core/primitives/src/test_utils.rs b/core/primitives/src/test_utils.rs index d088fb74a9c..e2734796872 100644 --- a/core/primitives/src/test_utils.rs +++ b/core/primitives/src/test_utils.rs @@ -555,26 +555,35 @@ pub fn create_test_signer(account_name: &str) -> InMemoryValidatorSigner { /// Should be used only in tests. pub fn create_user_test_signer(account_name: &str) -> InMemorySigner { let account_id = account_name.parse().unwrap(); - // TODO add support for ETH-implicit test account if account_id == near_implicit_test_account() { - InMemorySigner::from_secret_key(account_id, implicit_test_account_secret()) + InMemorySigner::from_secret_key(account_id, near_implicit_test_account_secret()) + } else if account_id == eth_implicit_test_account() { + InMemorySigner::from_secret_key(account_id, eth_implicit_test_account_secret()) } else { InMemorySigner::from_seed(account_id, KeyType::ED25519, account_name) } } -// TODO add eth_implicit_test_account - -/// A fixed implicit account for which tests can know the private key. +/// A fixed NEAR-implicit account for which tests can know the private key. pub fn near_implicit_test_account() -> AccountId { "061b1dd17603213b00e1a1e53ba060ad427cef4887bd34a5e0ef09010af23b0a".parse().unwrap() } -/// Private key for the fixed implicit test account. -pub fn implicit_test_account_secret() -> SecretKey { +/// Private key for the fixed NEAR-implicit test account. +pub fn near_implicit_test_account_secret() -> SecretKey { "ed25519:5roj6k68kvZu3UEJFyXSfjdKGrodgZUfFLZFpzYXWtESNsLWhYrq3JGi4YpqeVKuw1m9R2TEHjfgWT1fjUqB1DNy".parse().unwrap() } +/// A fixed ETH-implicit account for which tests can know the private key. +pub fn eth_implicit_test_account() -> AccountId { + "0x96791e923f8cf697ad9c3290f2c9059f0231b24c".parse().unwrap() +} + +/// Private key for the fixed ETH-implicit test account. +pub fn eth_implicit_test_account_secret() -> SecretKey { + "secp256k1:X4ETFKtQkSGVoZEnkn7bZ3LyajJaK2b3eweXaKmynGx".parse().unwrap() +} + impl FinalExecutionOutcomeView { #[track_caller] /// Check transaction and all transitive receipts for success status. diff --git a/core/primitives/src/utils.rs b/core/primitives/src/utils.rs index 749320f7e33..ac31defdb05 100644 --- a/core/primitives/src/utils.rs +++ b/core/primitives/src/utils.rs @@ -16,6 +16,10 @@ use crate::version::{ ProtocolVersion, CORRECT_RANDOM_VALUE_PROTOCOL_VERSION, CREATE_HASH_PROTOCOL_VERSION, CREATE_RECEIPT_ID_SWITCH_TO_CURRENT_BLOCK_VERSION, }; + +use near_crypto::{KeyType, PublicKey}; +use near_primitives_core::account::id::AccountId; + use std::mem::size_of; use std::ops::Deref; @@ -465,10 +469,37 @@ where Serializable(object) } +pub fn derive_account_id_from_public_key(public_key: &PublicKey) -> AccountId { + match public_key.key_type() { + KeyType::ED25519 => { + hex::encode(public_key.key_data()).parse().unwrap() + }, + KeyType::SECP256K1 => { + use sha3::Digest; + let pk_hash = sha3::Keccak256::digest(&public_key.key_data()); + format!("0x{}", hex::encode(&pk_hash[12..32])).parse().unwrap() + }, + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn test_derive_account_id_from_ed25519_public_key() { + let public_key = PublicKey::from_seed(KeyType::ED25519, "test"); + let expected: AccountId = "bb4dc639b212e075a751685b26bdcea5920a504181ff2910e8549742127092a0".parse().unwrap(); + assert_eq!(derive_account_id_from_public_key(&public_key), expected); + } + + #[test] + fn test_derive_account_id_from_secp256k1_public_key() { + let public_key = PublicKey::from_seed(KeyType::SECP256K1, "test"); + let expected: AccountId = "0x96791e923f8cf697ad9c3290f2c9059f0231b24c".parse().unwrap(); + assert_eq!(derive_account_id_from_public_key(&public_key), expected); + } + #[test] fn test_num_chunk_producers() { for num_seats in 1..50 { diff --git a/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs b/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs index 67c519f9964..47e8841219c 100644 --- a/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs +++ b/integration-tests/src/tests/client/features/access_key_nonce_for_implicit_accounts.rs @@ -7,7 +7,7 @@ use near_chain_configs::Genesis; use near_chunks::metrics::PARTIAL_ENCODED_CHUNK_FORWARD_CACHED_WITHOUT_HEADER; use near_client::test_utils::{create_chunk_with_transactions, TestEnv}; use near_client::ProcessTxResponse; -use near_crypto::{InMemorySigner, KeyType, Signer}; +use near_crypto::{InMemorySigner, KeyType, Signer, SecretKey}; use near_network::shards_manager::ShardsManagerRequestFromNetwork; use near_network::types::{NetworkRequests, PeerManagerMessageRequest}; use near_o11y::testonly::init_test_logger; @@ -18,6 +18,7 @@ use near_primitives::shard_layout::ShardLayout; use near_primitives::sharding::ChunkHash; use near_primitives::transaction::SignedTransaction; use near_primitives::types::{AccountId, BlockHeight}; +use near_primitives::utils::derive_account_id_from_public_key; use near_primitives::version::{ProtocolFeature, ProtocolVersion}; use near_primitives::views::FinalExecutionStatus; use nearcore::config::GenesisExt; @@ -112,13 +113,13 @@ fn test_transaction_hash_collision() { ); } -// TODO add corresponding method for ETH-implicit accounts? /// Helper for checking that duplicate transactions from implicit accounts are properly rejected. /// It creates implicit account, deletes it and creates again, so that nonce of the access /// key is updated. Then it tries to send tx from implicit account with invalid nonce, which /// should fail since the protocol upgrade. -fn get_status_of_tx_hash_collision_for_near_implicit_account( +fn get_status_of_tx_hash_collision_for_implicit_account( protocol_version: ProtocolVersion, + implicit_account_signer: InMemorySigner, ) -> ProcessTxResponse { let epoch_length = 100; let mut genesis = Genesis::test(vec!["test0".parse().unwrap(), "test1".parse().unwrap()], 1); @@ -129,17 +130,11 @@ fn get_status_of_tx_hash_collision_for_near_implicit_account( .nightshade_runtimes(&genesis) .build(); let genesis_block = env.clients[0].chain.get_block_by_height(0).unwrap(); - - let signer1 = InMemorySigner::from_seed("test1".parse().unwrap(), KeyType::ED25519, "test1"); - - let public_key = &signer1.public_key; - let raw_public_key = public_key.unwrap_as_ed25519().0.to_vec(); - let implicit_account_id = AccountId::try_from(hex::encode(&raw_public_key)).unwrap(); - let implicit_account_signer = - InMemorySigner::from_secret_key(implicit_account_id.clone(), signer1.secret_key.clone()); let deposit_for_account_creation = 10u128.pow(23); let mut height = 1; let blocks_number = 5; + let signer1 = InMemorySigner::from_seed("test1".parse().unwrap(), KeyType::ED25519, "test1"); + let implicit_account_id = implicit_account_signer.account_id.clone(); // Send money to implicit account, invoking its creation. let send_money_tx = SignedTransaction::send_money( @@ -203,25 +198,43 @@ fn get_status_of_tx_hash_collision_for_near_implicit_account( response } -// TODO add corresponding test for ETH-implicit accounts? -/// Test that duplicate transactions from implicit accounts are properly rejected. +/// Test that duplicate transactions from NEAR-implicit accounts are properly rejected. #[test] fn test_transaction_hash_collision_for_near_implicit_account_fail() { let protocol_version = ProtocolFeature::AccessKeyNonceForImplicitAccounts.protocol_version(); + let secret_key = SecretKey::from_seed(KeyType::ED25519, "test"); + let implicit_account_id = derive_account_id_from_public_key(&secret_key.public_key()); + let implicit_account_signer = InMemorySigner::from_secret_key(implicit_account_id, secret_key); + assert_matches!( + get_status_of_tx_hash_collision_for_implicit_account(protocol_version, implicit_account_signer), + ProcessTxResponse::InvalidTx(InvalidTxError::InvalidNonce { .. }) + ); +} + +// TODO, does not pass, see `verify_and_charge_transaction(...)` +/// Test that duplicate transactions from ETH-implicit accounts are properly rejected. +#[test] +fn test_transaction_hash_collision_for_eth_implicit_account_fail() { + let protocol_version = ProtocolFeature::AccessKeyNonceForImplicitAccounts.protocol_version(); + let secret_key = SecretKey::from_seed(KeyType::SECP256K1, "test"); + let implicit_account_id = derive_account_id_from_public_key(&secret_key.public_key()); + let implicit_account_signer = InMemorySigner::from_secret_key(implicit_account_id, secret_key); assert_matches!( - get_status_of_tx_hash_collision_for_near_implicit_account(protocol_version), + get_status_of_tx_hash_collision_for_implicit_account(protocol_version, implicit_account_signer), ProcessTxResponse::InvalidTx(InvalidTxError::InvalidNonce { .. }) ); } -// TODO add corresponding test for ETH-implicit accounts? -/// 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 ? +/// Test that duplicate transactions from NEAR-implicit accounts are not rejected until protocol upgrade. #[test] fn test_transaction_hash_collision_for_near_implicit_account_ok() { - let protocol_version = - ProtocolFeature::AccessKeyNonceForImplicitAccounts.protocol_version() - 1; + let protocol_version = ProtocolFeature::AccessKeyNonceForImplicitAccounts.protocol_version() - 1; + let secret_key = SecretKey::from_seed(KeyType::ED25519, "test"); + let implicit_account_id = derive_account_id_from_public_key(&secret_key.public_key()); + let implicit_account_signer = InMemorySigner::from_secret_key(implicit_account_id, secret_key); assert_matches!( - get_status_of_tx_hash_collision_for_near_implicit_account(protocol_version), + get_status_of_tx_hash_collision_for_implicit_account(protocol_version, implicit_account_signer), ProcessTxResponse::ValidTx ); } diff --git a/integration-tests/src/tests/client/features/delegate_action.rs b/integration-tests/src/tests/client/features/delegate_action.rs index 0d1263cb71f..b181a7d61e5 100644 --- a/integration-tests/src/tests/client/features/delegate_action.rs +++ b/integration-tests/src/tests/client/features/delegate_action.rs @@ -9,13 +9,13 @@ use near_chain::ChainGenesis; use near_chain_configs::Genesis; use near_client::test_utils::TestEnv; use near_crypto::{KeyType, PublicKey, Signer}; -use near_primitives::account::{AccessKey, AccessKeyPermission, FunctionCallPermission}; +use near_primitives::account::{AccessKey, AccessKeyPermission, FunctionCallPermission, id::AccountType}; use near_primitives::config::ActionCosts; use near_primitives::errors::{ ActionError, ActionErrorKind, ActionsValidationError, InvalidAccessKeyError, InvalidTxError, TxExecutionError, }; -use near_primitives::test_utils::{create_user_test_signer, near_implicit_test_account}; +use near_primitives::test_utils::{create_user_test_signer, eth_implicit_test_account, near_implicit_test_account}; use near_primitives::transaction::{ Action, AddKeyAction, CreateAccountAction, DeleteAccountAction, DeleteKeyAction, DeployContractAction, FunctionCallAction, StakeAction, TransferAction, @@ -133,15 +133,17 @@ fn check_meta_tx_execution( .get_access_key(&relayer, &PublicKey::from_seed(KeyType::ED25519, &relayer)) .unwrap() .nonce; - let user_pubk = if sender.is_near_implicit() { - PublicKey::from_near_implicit_account(&sender).unwrap() - } else if sender.is_eth() { - // TODO - panic!("check_meta_tx_execution for eth sender address"); - } else { - PublicKey::from_seed(KeyType::ED25519, &sender) + + let user_pubk = match sender.get_account_type() { + AccountType::NearImplicitAccount => Some(PublicKey::from_near_implicit_account(&sender).unwrap()), + // Cannot infer public key from ETH-implicit address, because ETH-implicit address itself is inferred from the public key. + // However, it is for testing purposes only (checking nonce, that might not exist for ETH-implicit account anyway). + AccountType::EthImplicitAccount => None, + AccountType::NamedAccount => Some(PublicKey::from_seed(KeyType::ED25519, &sender)), }; - let user_nonce_before = node_user.get_access_key(&sender, &user_pubk).unwrap().nonce; + let user_nonce_before = user_pubk.map(|user_pubk| + node_user.get_access_key(&sender, &user_pubk).unwrap().nonce + ); let tx_result = node_user.meta_tx(sender.clone(), receiver.clone(), relayer.clone(), actions).unwrap(); @@ -155,12 +157,15 @@ fn check_meta_tx_execution( .unwrap() .nonce; assert_eq!(relayer_nonce, relayer_nonce_before + 1); - // user key must be checked for existence (to test DeleteKey action) - if let Ok(user_nonce) = node_user - .get_access_key(&sender, &PublicKey::from_seed(KeyType::ED25519, &sender)) - .map(|key| key.nonce) - { - assert_eq!(user_nonce, user_nonce_before + 1); + + if let Some(user_nonce_before) = user_nonce_before { + // user key must be checked for existence (to test DeleteKey action) + if let Ok(user_nonce) = node_user + .get_access_key(&sender, &PublicKey::from_seed(KeyType::ED25519, &sender)) + .map(|key| key.nonce) + { + assert_eq!(user_nonce, user_nonce_before + 1); + } } let sender_after = node_user.view_balance(&sender).unwrap_or(0); @@ -780,14 +785,11 @@ fn meta_tx_create_named_account() { node.view_account(&new_account).expect("failed looking up account"); } -// TODO add corresponding test for ETH-implicit account -/// Try creating a NEAR-implicit account with `CreateAction` which is not allowed in +/// Try creating an implicit account with `CreateAction` which is not allowed in /// or outside meta transactions and must fail with `OnlyImplicitAccountCreationAllowed`. -#[test] -fn meta_tx_create_near_implicit_account_fails() { +fn meta_tx_create_implicit_account_fails(new_account: AccountId) { let relayer = bob_account(); let sender = alice_account(); - let new_account: AccountId = near_implicit_test_account(); let node = RuntimeNode::new(&relayer); let actions = vec![Action::CreateAccount(CreateAccountAction {})]; @@ -802,18 +804,25 @@ fn meta_tx_create_near_implicit_account_fails() { )); } -// TODO add corresponding test for ETH-implicit account +#[test] +fn meta_tx_create_near_implicit_account_fails() { + meta_tx_create_implicit_account_fails(near_implicit_test_account()); +} + +#[test] +fn meta_tx_create_eth_implicit_account_fails() { + meta_tx_create_implicit_account_fails(eth_implicit_test_account()); +} + /// Try creating an implicit account with a meta tx transfer and use the account /// in the same meta transaction. /// /// This is expected to fail with `AccountDoesNotExist`, known limitation of NEP-366. /// It only works with accounts that already exists because it needs to do a /// nonce check against the access key, which can only exist if the account exists. -#[test] -fn meta_tx_create_and_use_implicit_account() { +fn meta_tx_create_and_use_implicit_account(new_account: AccountId) { let relayer = bob_account(); let sender = alice_account(); - let new_account: AccountId = near_implicit_test_account(); let node = RuntimeNode::new(&relayer); // Check the account doesn't exist, yet. We will attempt creating it. @@ -837,18 +846,25 @@ fn meta_tx_create_and_use_implicit_account() { )); } -// TODO add corresponding test for ETH-implicit account -/// Creating a NEAR-implicit account with a meta tx transfer and use the account in +#[test] +fn meta_tx_create_and_use_near_implicit_account() { + meta_tx_create_and_use_implicit_account(near_implicit_test_account()); +} + +#[test] +fn meta_tx_create_and_use_eth_implicit_account() { + meta_tx_create_and_use_implicit_account(eth_implicit_test_account()); +} + +/// Creating a implicit account with a meta tx transfer and use the account in /// a second meta transaction. /// /// Creation through a meta tx should work as normal, it's just that the relayer /// pays for the storage and the user could delete the account and cash in, /// hence this workflow is not ideal from all circumstances. -#[test] -fn meta_tx_create_near_implicit_account() { +fn meta_tx_create_implicit_account(new_account: AccountId) { let relayer = bob_account(); let sender = alice_account(); - let new_account: AccountId = near_implicit_test_account(); let node = RuntimeNode::new(&relayer); // Check account doesn't exist, yet @@ -857,7 +873,13 @@ fn meta_tx_create_near_implicit_account() { let fee_helper = fee_helper(&node); let initial_amount = nearcore::NEAR_BASE; let actions = vec![Action::Transfer(TransferAction { deposit: initial_amount })]; - let tx_cost = fee_helper.create_account_transfer_full_key_cost(); + + let tx_cost = match new_account.get_account_type() { + AccountType::NearImplicitAccount => fee_helper.create_account_transfer_full_key_cost(), + AccountType::EthImplicitAccount => fee_helper.create_account_transfer_cost(), + AccountType::NamedAccount => panic!("must be implicit"), + }; + check_meta_tx_no_fn_call( &node, actions, @@ -893,3 +915,14 @@ fn meta_tx_create_near_implicit_account() { let balance = node.view_balance(&new_account).expect("failed looking up balance"); assert_eq!(balance, initial_amount); } + +#[test] +fn meta_tx_create_near_implicit_account() { + meta_tx_create_implicit_account(near_implicit_test_account()); +} + +// TODO make the test passes +#[test] +fn meta_tx_create_eth_implicit_account() { + meta_tx_create_implicit_account(eth_implicit_test_account()); +} diff --git a/integration-tests/src/tests/client/features/restrict_tla.rs b/integration-tests/src/tests/client/features/restrict_tla.rs index 8ca0cadf08d..5e1d6ce5cd6 100644 --- a/integration-tests/src/tests/client/features/restrict_tla.rs +++ b/integration-tests/src/tests/client/features/restrict_tla.rs @@ -23,7 +23,7 @@ fn test_create_top_level_accounts() { .build(); // These accounts cannot be created because they are top level accounts that are not implicit. - // Note that implicit accounts have to be 64 or 42 characters long. + // Note that implicit accounts have to be 64 or 42 (in case start with '0x') characters long. let top_level_accounts = [ "0x06012c8cf97bead5deae237070f9587f8e7a266da", "0a5e97870f263700f46aa00d967821199b9bc5a120", diff --git a/integration-tests/src/tests/standard_cases/mod.rs b/integration-tests/src/tests/standard_cases/mod.rs index 0dd7925a39e..31af4a4f785 100644 --- a/integration-tests/src/tests/standard_cases/mod.rs +++ b/integration-tests/src/tests/standard_cases/mod.rs @@ -1,12 +1,13 @@ +use core::panic; use std::sync::Arc; mod rpc; mod runtime; use assert_matches::assert_matches; -use near_crypto::{InMemorySigner, KeyType}; +use near_crypto::{InMemorySigner, KeyType, SecretKey, PublicKey}; use near_jsonrpc_primitives::errors::ServerError; -use near_primitives::account::{AccessKey, AccessKeyPermission, FunctionCallPermission}; +use near_primitives::account::{AccessKey, AccessKeyPermission, FunctionCallPermission, id::AccountType}; use near_primitives::config::{ActionCosts, ExtCosts}; use near_primitives::errors::{ ActionError, ActionErrorKind, FunctionCallError, InvalidAccessKeyError, InvalidTxError, @@ -14,6 +15,7 @@ use near_primitives::errors::{ }; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::types::{AccountId, Balance, TrieNodesCount}; +use near_primitives::utils::derive_account_id_from_public_key; use near_primitives::views::{ AccessKeyView, AccountView, ExecutionMetadataView, FinalExecutionOutcomeView, FinalExecutionStatus, @@ -327,17 +329,19 @@ pub fn test_send_money(node: impl Node) { ); } -// TODO add coresponding method for ETH-implicit account -pub fn transfer_tokens_near_implicit_account(node: impl Node) { +pub fn transfer_tokens_implicit_account(node: impl Node, public_key: PublicKey) { let account_id = &node.account_id().unwrap(); let node_user = node.user(); let root = node_user.get_state_root(); let tokens_used = 10u128.pow(25); let fee_helper = fee_helper(&node); - let transfer_cost = fee_helper.transfer_cost_64len_hex(); - let public_key = node_user.signer().public_key(); - let raw_public_key = public_key.unwrap_as_ed25519().0.to_vec(); - let receiver_id = AccountId::try_from(hex::encode(&raw_public_key)).unwrap(); + let receiver_id = derive_account_id_from_public_key(&public_key); + + let transfer_cost = match receiver_id.get_account_type() { + AccountType::NearImplicitAccount => fee_helper.create_account_transfer_full_key_cost(), + AccountType::EthImplicitAccount => fee_helper.create_account_transfer_cost(), + AccountType::NamedAccount => std::panic!("must be implicit"), + }; let transaction_result = node_user.send_money(account_id.clone(), receiver_id.clone(), tokens_used).unwrap(); assert_eq!(transaction_result.status, FinalExecutionStatus::SuccessValue(Vec::new())); @@ -358,8 +362,16 @@ pub fn transfer_tokens_near_implicit_account(node: impl Node) { let AccountView { amount, locked, .. } = node_user.view_account(&receiver_id).unwrap(); assert_eq!((amount, locked), (tokens_used, 0)); - let view_access_key = node_user.get_access_key(&receiver_id, &public_key).unwrap(); - assert_eq!(view_access_key, AccessKey::full_access().into()); + let view_access_key = node_user.get_access_key(&receiver_id, &public_key); + match receiver_id.get_account_type() { + AccountType::NearImplicitAccount => { + assert_eq!(view_access_key.unwrap(), AccessKey::full_access().into()); + }, + AccountType::EthImplicitAccount => { + assert!(node_user.get_access_key(&receiver_id, &public_key).is_err()); + }, + AccountType::NamedAccount => std::panic!("must be implicit"), + } let transaction_result = node_user.send_money(account_id.clone(), receiver_id.clone(), tokens_used).unwrap(); @@ -383,17 +395,13 @@ pub fn transfer_tokens_near_implicit_account(node: impl Node) { assert_eq!((amount, locked), (tokens_used * 2, 0)); } -// TODO add coresponding method for ETH-implicit account -pub fn trying_to_create_near_implicit_account(node: impl Node) { +pub fn trying_to_create_implicit_account(node: impl Node, public_key: PublicKey) { let account_id = &node.account_id().unwrap(); let node_user = node.user(); let root = node_user.get_state_root(); let tokens_used = 10u128.pow(25); let fee_helper = fee_helper(&node); - - let public_key = node_user.signer().public_key(); - let raw_public_key = public_key.unwrap_as_ed25519().0.to_vec(); - let receiver_id = AccountId::try_from(hex::encode(&raw_public_key)).unwrap(); + let receiver_id = derive_account_id_from_public_key(&public_key); let transaction_result = node_user .create_account( @@ -404,14 +412,28 @@ pub fn trying_to_create_near_implicit_account(node: impl Node) { ) .unwrap(); - let cost = fee_helper.create_account_transfer_full_key_cost_fail_on_create_account() - + fee_helper.gas_to_balance( - fee_helper.cfg().fee(ActionCosts::create_account).send_fee(false) + let cost = match receiver_id.get_account_type() { + AccountType::NearImplicitAccount => + fee_helper.create_account_transfer_full_key_cost_fail_on_create_account() + + fee_helper.gas_to_balance( + fee_helper.cfg().fee(ActionCosts::create_account).send_fee(false) + fee_helper .cfg() .fee(near_primitives::config::ActionCosts::add_full_access_key) .send_fee(false), - ); + ), + AccountType::EthImplicitAccount => + fee_helper.create_account_transfer_cost_fail_on_create_account() + + fee_helper.gas_to_balance( + fee_helper.cfg().fee(ActionCosts::create_account).send_fee(false) + // TODO The test passes, although the fee for add_full_access_key probably should not be included + + fee_helper + .cfg() + .fee(near_primitives::config::ActionCosts::add_full_access_key) + .send_fee(false), + ), + AccountType::NamedAccount => std::panic!("must be implicit"), + }; assert_eq!( transaction_result.status, diff --git a/integration-tests/src/tests/standard_cases/runtime.rs b/integration-tests/src/tests/standard_cases/runtime.rs index 2e106a1f1d5..468872b0545 100644 --- a/integration-tests/src/tests/standard_cases/runtime.rs +++ b/integration-tests/src/tests/standard_cases/runtime.rs @@ -24,7 +24,7 @@ fn create_runtime_with_expensive_storage() -> RuntimeNode { match &mut records[0] { StateRecord::Account { account, .. } => account.set_amount(TESTING_INIT_BALANCE * 10000), _ => { - panic!("the first record is expected to be alice account creation!"); + std::panic!("the first record is expected to be alice account creation!"); } } records.push(StateRecord::Data { @@ -113,18 +113,32 @@ fn test_send_money_runtime() { test_send_money(node); } -// TODO add corresponding test for ETH-implicit account #[test] fn test_transfer_tokens_near_implicit_account_runtime() { let node = create_runtime_node(); - transfer_tokens_near_implicit_account(node); + let public_key = node.user().signer().public_key(); + transfer_tokens_implicit_account(node, public_key); +} + +#[test] +fn test_transfer_tokens_eth_implicit_account_runtime() { + let node = create_runtime_node(); + let secret_key = SecretKey::from_seed(KeyType::SECP256K1, "test"); + transfer_tokens_implicit_account(node, secret_key.public_key()); } -// TODO add corresponding test for ETH-implicit account #[test] fn test_trying_to_create_near_implicit_account_runtime() { let node = create_runtime_node(); - trying_to_create_near_implicit_account(node); + let public_key = node.user().signer().public_key(); + trying_to_create_implicit_account(node, public_key); +} + +#[test] +fn test_trying_to_create_eth_implicit_account_runtime() { + let node = create_runtime_node(); + let secret_key = SecretKey::from_seed(KeyType::SECP256K1, "test"); + trying_to_create_implicit_account(node, secret_key.public_key()); } #[test] diff --git a/integration-tests/src/user/mod.rs b/integration-tests/src/user/mod.rs index cc1da5b1a8f..3f9442212c2 100644 --- a/integration-tests/src/user/mod.rs +++ b/integration-tests/src/user/mod.rs @@ -4,7 +4,7 @@ use futures::{future::LocalBoxFuture, FutureExt}; use near_crypto::{PublicKey, Signer}; use near_jsonrpc_primitives::errors::ServerError; -use near_primitives::account::AccessKey; +use near_primitives::account::{AccessKey, id::AccountType}; use near_primitives::action::delegate::{DelegateAction, NonDelegateAction, SignedDelegateAction}; use near_primitives::hash::CryptoHash; use near_primitives::receipt::Receipt; @@ -262,10 +262,19 @@ pub trait User { actions: Vec, ) -> Result { let inner_signer = create_user_test_signer(&signer_id); - let user_nonce = self - .get_access_key(&signer_id, &inner_signer.public_key) - .expect("failed reading user's nonce for access key") - .nonce; + let access_key = self.get_access_key(&signer_id, &inner_signer.public_key); + + let user_nonce = match access_key { + Ok(access_key) => access_key.nonce, + Err(_) => match signer_id.get_account_type() { + AccountType::EthImplicitAccount => { + // TODO Integration-tests/MetaTx: What nonce should we use here? + 0 + } + _ => panic!("failed reading user's nonce for access key"), + }, + }; + let delegate_action = DelegateAction { sender_id: signer_id.clone(), receiver_id, diff --git a/pytest/lib/key.py b/pytest/lib/key.py index fc61a8286b1..f2f12163511 100644 --- a/pytest/lib/key.py +++ b/pytest/lib/key.py @@ -22,7 +22,7 @@ def from_random(cls, account_id: str) -> 'Key': keys = ed25519.create_keypair(entropy=os.urandom) return cls.from_keypair(account_id, keys) - # TODO what about ETH-implicit account? + # TODO Pytest: add method for creating ETH-implicit accounts @classmethod def implicit_account(cls) -> 'Key': keys = ed25519.create_keypair(entropy=os.urandom) diff --git a/pytest/tests/loadtest/locust/common/sweat.py b/pytest/tests/loadtest/locust/common/sweat.py index b1af3bbe7f2..aad54013cb3 100644 --- a/pytest/tests/loadtest/locust/common/sweat.py +++ b/pytest/tests/loadtest/locust/common/sweat.py @@ -56,7 +56,7 @@ class InitSweat(FunctionCall): def __init__(self, sweat_account: Account): super().__init__(sweat_account, sweat_account.key.account_id, "new") - # TODO How does Sweat contract would work with ETH-implicit accounts + # TODO See the description below. Does introducing ETH-implicit accounts impact Sweat contract? def args(self) -> dict: # Technical details about Sweat contract initialization: # diff --git a/pytest/tests/replay/replay.py b/pytest/tests/replay/replay.py index 6d643570e9c..af8bac0fdf4 100644 --- a/pytest/tests/replay/replay.py +++ b/pytest/tests/replay/replay.py @@ -194,9 +194,9 @@ def send_resigned_transactions(tx_path, home_dir): help="Path of the new home directory") args = parser.parse_args() - # TODO support for ETH-implicit accounts? if args.operation == 'generate': if args.genesis: + # TODO Pytest/replay: Test save_genesis with ETH-implicit account key? key_pair = generate_new_key() save_genesis_with_new_key_pair(args.genesis, key_pair, args.home_dir) diff --git a/pytest/tests/sanity/rosetta.py b/pytest/tests/sanity/rosetta.py index 76e8cff5be2..9cab47fd962 100644 --- a/pytest/tests/sanity/rosetta.py +++ b/pytest/tests/sanity/rosetta.py @@ -427,7 +427,7 @@ def setUpClass(cls) -> None: def tearDownClass(cls) -> None: cls.node.cleanup() - # For ETH-implicit account, we will not store access keys, so not additional storage cost. + # TODO Pytest/Rosetta: Test with ETH-implicit account? def test_zero_balance_account(self) -> None: """Tests storage staking requirements for low-storage accounts. @@ -774,9 +774,9 @@ def _get_account_balance(self, self.fail(f'Account {account.account_id} does not exist') return None - # TODO add corresponding test for ETH-implicit account - def test_near_implicit_account(self) -> None: - """Tests creating and deleting NEAR-implicit account + # TODO Pytest/Rosetta: Test with ETH-implicit account? + def test_implicit_account(self) -> None: + """Tests creating and deleting implicit account First sends some funds from validator’s account to an implicit account, then checks how the transaction looks through Data API and finally @@ -787,7 +787,7 @@ def test_near_implicit_account(self) -> None: implicit = key.Key.implicit_account() # 1. Create implicit account. - logger.info(f'Creating NEAR-implicit account: {implicit.account_id}') + logger.info(f'Creating implicit account: {implicit.account_id}') result = self.rosetta.transfer(src=validator, dst=implicit, amount=test_amount) @@ -934,7 +934,7 @@ def test_near_implicit_account(self) -> None: }, related.transaction()) # 2. Delete the account. - logger.info(f'Deleting NEAR-implicit account: {implicit.account_id}') + logger.info(f'Deleting implicit account: {implicit.account_id}') result = self.rosetta.delete_account(implicit, refund_to=validator) self.assertEqual( diff --git a/pytest/tools/mirror/mirror_utils.py b/pytest/tools/mirror/mirror_utils.py index 0c43d427ea1..6e6426ba708 100644 --- a/pytest/tools/mirror/mirror_utils.py +++ b/pytest/tools/mirror/mirror_utils.py @@ -553,6 +553,7 @@ def added_keys_send_transfers(nodes, added_keys, receivers, amount, block_hash): node_idx %= len(nodes) +# TODO Pytest/Mirror: This function uses implicit account def start_source_chain(config, num_source_validators=3): # for now we need at least 2 because we're sending traffic for source_nodes[1].signer_key # Could fix that but for now this assert is fine @@ -632,6 +633,7 @@ def start_source_chain(config, num_source_validators=3): return near_root, source_nodes, target_node_dirs, traffic_data +# TODO Pytest/Mirror: This function uses implicit account # callback will be called once for every iteration of the utils.poll_blocks() # loop, and we break if it returns False def send_traffic(near_root, source_nodes, traffic_data, callback): diff --git a/runtime/runtime/Cargo.toml b/runtime/runtime/Cargo.toml index 47d3c22ff12..58e79b8abc3 100644 --- a/runtime/runtime/Cargo.toml +++ b/runtime/runtime/Cargo.toml @@ -20,7 +20,6 @@ rayon.workspace = true serde.workspace = true serde_json.workspace = true sha2.workspace = true -sha3.workspace = true thiserror.workspace = true tracing.workspace = true diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index 104ad0ac1c1..9556890a558 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -477,7 +477,7 @@ pub(crate) fn action_implicit_account_creation_transfer( + fee_config.storage_usage_config.num_extra_bytes_record, )); }, - AccountType::NamedAccount => panic!("Should only be called for an implicit account"), + AccountType::NamedAccount => panic!("must be implicit"), } diff --git a/runtime/runtime/src/config.rs b/runtime/runtime/src/config.rs index 8d6bf588fb7..0cfb4b6e33f 100644 --- a/runtime/runtime/src/config.rs +++ b/runtime/runtime/src/config.rs @@ -2,7 +2,6 @@ use near_primitives::account::AccessKeyPermission; use near_primitives::errors::IntegerOverflowError; -use near_primitives_core::account::id::AccountType; use near_primitives_core::config::ActionCosts; use num_bigint::BigUint; use num_traits::cast::ToPrimitive; diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index 0778a2e150b..b01584ba63c 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -1,7 +1,6 @@ use crate::config::{total_prepaid_gas, tx_cost, TransactionCost}; use crate::near_primitives::account::Account; use crate::VerificationResult; -use near_crypto::{PublicKey, KeyType}; use near_crypto::key_conversion::is_valid_staking_key; use near_primitives::account::{AccessKey, AccessKeyPermission}; use near_primitives::action::delegate::SignedDelegateAction; @@ -18,6 +17,7 @@ use near_primitives::transaction::{ }; use near_primitives::types::{AccountId, Balance}; use near_primitives::types::{BlockHeight, StorageUsage}; +use near_primitives::utils::derive_account_id_from_public_key; use near_primitives::version::ProtocolFeature; use near_primitives::version::ProtocolVersion; use near_primitives_core::account::id::AccountType; @@ -125,18 +125,6 @@ pub fn validate_transaction( .map_err(|_| InvalidTxError::CostOverflow.into()) } -fn is_pk_valid_for_eth_address(public_key: &PublicKey, account_id: &AccountId) -> bool { - match public_key.key_type() { - KeyType::SECP256K1 => { - use sha3::Digest; - let pk_hash = sha3::Keccak256::digest(&public_key.key_data()); - format!("0x{}", hex::encode(&pk_hash[12..32])) == account_id.as_str() - }, - // Should panic instead? - KeyType::ED25519 => false, - } -} - /// Verifies the signed transaction on top of given state, charges transaction fees /// and balances, and updates the state for the used account and access keys. pub fn verify_and_charge_transaction( @@ -170,9 +158,16 @@ pub fn verify_and_charge_transaction( Some(access_key) => access_key, None => match signer_id.get_account_type() { AccountType::EthImplicitAccount => { - if is_pk_valid_for_eth_address(public_key, signer_id) { - // TODO what about increasing storage usage for that account - AccessKey::full_access() + if derive_account_id_from_public_key(public_key) == *signer_id { + // TODO What about increasing storage usage for that account, because we added access key? + // TODO Can we assume there can only be one access key added to ETH-implicit account, the one from which the address has been derived? + let mut access_key = AccessKey::full_access(); + // TODO Transaction nonce must be greater than access_key nonce, so I used 'transaction.nonce - 1'. + // However, there is a potential for replay attack (test_transaction_hash_collision_for_eth_implicit_account_fail does not pass because of that). + // I would set nonce to (block_height - 1) * near_primitives::account::AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER, + // but block_height is passed here as an Option. + access_key.nonce = transaction.nonce - 1; + access_key } else { return Err(InvalidTxError::InvalidAccessKeyError( InvalidAccessKeyError::InvalidPkForEthAddress { diff --git a/test-utils/runtime-tester/src/fuzzing.rs b/test-utils/runtime-tester/src/fuzzing.rs index 9f6d99443c3..1e266286285 100644 --- a/test-utils/runtime-tester/src/fuzzing.rs +++ b/test-utils/runtime-tester/src/fuzzing.rs @@ -518,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. pub fn new_account(&mut self, u: &mut Unstructured) -> Result { let account = if u.arbitrary::()? { self.new_implicit_account(u)? diff --git a/test-utils/testlib/src/fees_utils.rs b/test-utils/testlib/src/fees_utils.rs index ddd964946a5..f07ad46db69 100644 --- a/test-utils/testlib/src/fees_utils.rs +++ b/test-utils/testlib/src/fees_utils.rs @@ -56,10 +56,24 @@ impl FeeHelper { exec_gas + send_gas } + pub fn create_account_transfer_fee(&self) -> Gas { + let exec_gas = self.cfg().fee(ActionCosts::new_action_receipt).exec_fee() + + self.cfg().fee(ActionCosts::create_account).exec_fee() + + self.cfg().fee(ActionCosts::transfer).exec_fee(); + let send_gas = self.cfg().fee(ActionCosts::new_action_receipt).send_fee(false) + + self.cfg().fee(ActionCosts::create_account).send_fee(false) + + self.cfg().fee(ActionCosts::transfer).send_fee(false); + exec_gas + send_gas + } + pub fn create_account_transfer_full_key_cost(&self) -> Balance { self.gas_to_balance(self.create_account_transfer_full_key_fee()) } + pub fn create_account_transfer_cost(&self) -> Balance { + self.gas_to_balance(self.create_account_transfer_fee()) + } + pub fn create_account_transfer_full_key_cost_no_reward(&self) -> Balance { let exec_gas = self.cfg().fee(ActionCosts::new_action_receipt).exec_fee() + self.cfg().fee(ActionCosts::create_account).exec_fee() @@ -82,6 +96,15 @@ impl FeeHelper { self.gas_to_balance(exec_gas + send_gas) } + pub fn create_account_transfer_cost_fail_on_create_account(&self) -> Balance { + let exec_gas = self.cfg().fee(ActionCosts::new_action_receipt).exec_fee() + + self.cfg().fee(ActionCosts::create_account).exec_fee(); + let send_gas = self.cfg().fee(ActionCosts::new_action_receipt).send_fee(false) + + self.cfg().fee(ActionCosts::create_account).send_fee(false) + + self.cfg().fee(ActionCosts::transfer).send_fee(false); + self.gas_to_balance(exec_gas + send_gas) + } + pub fn deploy_contract_cost(&self, num_bytes: u64) -> Balance { let exec_gas = self.cfg().fee(ActionCosts::new_action_receipt).exec_fee() + self.cfg().fee(ActionCosts::deploy_contract_base).exec_fee() @@ -118,10 +141,6 @@ impl FeeHelper { self.gas_to_balance(self.transfer_fee()) } - pub fn transfer_cost_64len_hex(&self) -> Balance { - self.create_account_transfer_full_key_cost() - } - pub fn stake_cost(&self) -> Balance { let exec_gas = self.cfg().fee(ActionCosts::new_action_receipt).exec_fee() + self.cfg().fee(ActionCosts::stake).exec_fee(); diff --git a/tools/fork-network/src/cli.rs b/tools/fork-network/src/cli.rs index 27a766d464f..47fb456e0ff 100644 --- a/tools/fork-network/src/cli.rs +++ b/tools/fork-network/src/cli.rs @@ -486,13 +486,13 @@ impl ForkNetworkCommand { if let Some(sr) = StateRecord::from_raw_key_value(key.clone(), value.clone()) { match sr { StateRecord::AccessKey { account_id, public_key, access_key } => { - if !account_id.is_implicit() + if !account_id.get_account_type().is_implicit() && access_key.permission == AccessKeyPermission::FullAccess { has_full_key.insert(account_id.clone()); } let new_account_id = map_account(&account_id, None); - let replacement = map_key(&public_key, None); + let replacement = map_key(&public_key, None, &account_id); storage_mutator.delete_access_key(account_id, public_key)?; storage_mutator.set_access_key( new_account_id, @@ -503,7 +503,7 @@ impl ForkNetworkCommand { } StateRecord::Account { account_id, account } => { - if account_id.is_implicit() { + if account_id.get_account_type().is_implicit() { let new_account_id = map_account(&account_id, None); storage_mutator.delete_account(account_id)?; storage_mutator.set_account(new_account_id, account)?; @@ -511,7 +511,7 @@ impl ForkNetworkCommand { } } StateRecord::Data { account_id, data_key, value } => { - if account_id.is_implicit() { + if account_id.get_account_type().is_implicit() { let new_account_id = map_account(&account_id, None); storage_mutator.delete_data(account_id, &data_key)?; storage_mutator.set_data(new_account_id, &data_key, value)?; @@ -519,7 +519,7 @@ impl ForkNetworkCommand { } } StateRecord::Contract { account_id, code } => { - if account_id.is_implicit() { + if account_id.get_account_type().is_implicit() { let new_account_id = map_account(&account_id, None); storage_mutator.delete_code(account_id)?; storage_mutator.set_code(new_account_id, code)?; @@ -527,7 +527,7 @@ impl ForkNetworkCommand { } } StateRecord::PostponedReceipt(receipt) => { - if receipt.predecessor_id.is_implicit() || receipt.receiver_id.is_implicit() + if receipt.predecessor_id.get_account_type().is_implicit() || receipt.receiver_id.get_account_type().is_implicit() { let new_receipt = Receipt { predecessor_id: map_account(&receipt.predecessor_id, None), @@ -541,7 +541,7 @@ impl ForkNetworkCommand { } } StateRecord::ReceivedData { account_id, data_id, data } => { - if account_id.is_implicit() { + if account_id.get_account_type().is_implicit() { let new_account_id = map_account(&account_id, None); storage_mutator.delete_received_data(account_id, data_id)?; storage_mutator.set_received_data(new_account_id, data_id, &data)?; @@ -549,7 +549,7 @@ impl ForkNetworkCommand { } } StateRecord::DelayedReceipt(receipt) => { - if receipt.predecessor_id.is_implicit() || receipt.receiver_id.is_implicit() + if receipt.predecessor_id.get_account_type().is_implicit() || receipt.receiver_id.get_account_type().is_implicit() { let new_receipt = Receipt { predecessor_id: map_account(&receipt.predecessor_id, None), diff --git a/tools/mirror/src/genesis.rs b/tools/mirror/src/genesis.rs index bdc17ab138c..ecf33732794 100644 --- a/tools/mirror/src/genesis.rs +++ b/tools/mirror/src/genesis.rs @@ -1,5 +1,5 @@ use near_primitives::state_record::StateRecord; -use near_primitives_core::account::{AccessKey, AccessKeyPermission, id::AccountType}; +use near_primitives_core::account::{AccessKey, AccessKeyPermission}; use serde::ser::{SerializeSeq, Serializer}; use std::collections::HashSet; use std::fs::File; @@ -32,7 +32,7 @@ pub fn map_records>( near_chain_configs::stream_records_from_file(reader, |mut r| { match &mut r { StateRecord::AccessKey { account_id, public_key, access_key } => { - let replacement = crate::key_mapping::map_key(&public_key, secret.as_ref()); + let replacement = crate::key_mapping::map_key(&public_key, secret.as_ref(), account_id); let new_record = StateRecord::AccessKey { account_id: crate::key_mapping::map_account(&account_id, secret.as_ref()), public_key: replacement.public_key(), @@ -46,7 +46,7 @@ pub fn map_records>( records_seq.serialize_element(&new_record).unwrap(); } StateRecord::Account { account_id, .. } => { - if account_id.get_account_type() == AccountType::NearImplicitAccount { + if account_id.get_account_type().is_implicit() { *account_id = crate::key_mapping::map_account(&account_id, secret.as_ref()); } else { accounts.insert(account_id.clone()); @@ -54,20 +54,20 @@ pub fn map_records>( records_seq.serialize_element(&r).unwrap(); } StateRecord::Data { account_id, .. } => { - if account_id.get_account_type() == AccountType::NearImplicitAccount { + if account_id.get_account_type().is_implicit() { *account_id = crate::key_mapping::map_account(&account_id, secret.as_ref()); } records_seq.serialize_element(&r).unwrap(); } StateRecord::Contract { account_id, .. } => { - if account_id.get_account_type() == AccountType::NearImplicitAccount { + if account_id.get_account_type().is_implicit() { *account_id = crate::key_mapping::map_account(&account_id, secret.as_ref()); } records_seq.serialize_element(&r).unwrap(); } StateRecord::PostponedReceipt(receipt) => { - if receipt.predecessor_id.get_account_type() == AccountType::NearImplicitAccount - || receipt.receiver_id.get_account_type() == AccountType::NearImplicitAccount + if receipt.predecessor_id.get_account_type().is_implicit() + || receipt.receiver_id.get_account_type().is_implicit() { receipt.predecessor_id = crate::key_mapping::map_account(&receipt.predecessor_id, secret.as_ref()); @@ -77,14 +77,14 @@ pub fn map_records>( records_seq.serialize_element(&r).unwrap(); } StateRecord::ReceivedData { account_id, .. } => { - if account_id.get_account_type() == AccountType::NearImplicitAccount { + if account_id.get_account_type().is_implicit() { *account_id = crate::key_mapping::map_account(&account_id, secret.as_ref()); } records_seq.serialize_element(&r).unwrap(); } StateRecord::DelayedReceipt(receipt) => { - if receipt.predecessor_id.get_account_type() == AccountType::NearImplicitAccount - || receipt.receiver_id.get_account_type() == AccountType::NearImplicitAccount + if receipt.predecessor_id.get_account_type().is_implicit() + || receipt.receiver_id.get_account_type().is_implicit() { receipt.predecessor_id = crate::key_mapping::map_account(&receipt.predecessor_id, secret.as_ref()); diff --git a/tools/mirror/src/key_mapping.rs b/tools/mirror/src/key_mapping.rs index d9c352c74b2..bff0b5020d3 100644 --- a/tools/mirror/src/key_mapping.rs +++ b/tools/mirror/src/key_mapping.rs @@ -2,7 +2,9 @@ use hkdf::Hkdf; use near_crypto::{ED25519PublicKey, ED25519SecretKey, PublicKey, Secp256K1PublicKey, SecretKey}; use near_primitives_core::account::id::AccountType; use near_primitives::types::AccountId; +use near_primitives::utils::derive_account_id_from_public_key; use sha2::Sha256; +use std::fmt::Debug; // there is nothing special about this key, it's just some randomly generated one. // We will ensure that every account in the target chain has at least one full access @@ -47,11 +49,11 @@ fn map_ed25519( ED25519SecretKey(buf) } -fn secp256k1_from_slice(buf: &mut [u8], public: &Secp256K1PublicKey) -> secp256k1::SecretKey { +fn secp256k1_from_slice(buf: &mut [u8], mapped_from: &T) -> secp256k1::SecretKey { match secp256k1::SecretKey::from_slice(buf) { Ok(s) => s, Err(_) => { - tracing::warn!(target: "mirror", "Something super unlikely occurred! SECP256K1 key mapped from {:?} is too large. Flipping most significant bit.", public); + tracing::warn!(target: "mirror", "Something super unlikely occurred! SECP256K1 key mapped from {:?} is too large. Flipping most significant bit.", mapped_from); // If we got an error, it means that either `buf` is all zeros, or that when interpreted as a 256-bit // int, it is larger than the order of the secp256k1 curve. Since the order of the curve starts with 0xFF, // in either case flipping the first bit should work, and we can unwrap() below. @@ -81,30 +83,81 @@ fn map_secp256k1( } // This maps the public key to a secret key so that we can sign -// transactions on the target chain. If secret is None, then we just +// transactions on the target chain. If secret is None, then we just // use the bytes of the public key directly, otherwise we feed the // public key to a key derivation function. -pub fn map_key(key: &PublicKey, secret: Option<&[u8; crate::secret::SECRET_LEN]>) -> SecretKey { - match key { - PublicKey::ED25519(k) => SecretKey::ED25519(map_ed25519(k, secret)), - PublicKey::SECP256K1(k) => SecretKey::SECP256K1(map_secp256k1(k, secret)), +// If the `key` corresponds to a ETH-implicit `account_id`, +// then the new key will be derived from that ETH address. +pub fn map_key( + key: &PublicKey, + secret: Option<&[u8; crate::secret::SECRET_LEN]>, + account_id: &AccountId, +) -> SecretKey { + match account_id.get_account_type() { + AccountType::NearImplicitAccount => { + match key { + PublicKey::ED25519(k) => SecretKey::ED25519(map_ed25519(k, secret)), + // TODO Is it true? + _ => panic!("NEAR-implicit account can only have ED25519 access key added"), + } + }, + AccountType::EthImplicitAccount => { + match key { + PublicKey::SECP256K1(_) => map_secp256k1_from_eth_address(account_id, secret), + // TODO Is it true? + _ => panic!("ETH-implicit account can only have SECP256K1 access key added"), + } + }, + AccountType::NamedAccount => { + match key { + PublicKey::ED25519(k) => SecretKey::ED25519(map_ed25519(k, secret)), + PublicKey::SECP256K1(k) => SecretKey::SECP256K1(map_secp256k1(k, secret)), + } + } } } -// If it's an implicit account, interprets it as an ed25519 public key, maps that and then returns -// the resulting implicit account. Otherwise does nothing. We do this so that transactions creating -// an implicit account by sending money will generate an account that we can control +// TODO Deriving secret keys from ETH addresses reduces potential entropy. +// However, it is only for the Mirror tool. Are we ok with that? +fn map_secp256k1_from_eth_address( + account_id: &AccountId, + secret: Option<&[u8; crate::secret::SECRET_LEN]>, +) -> SecretKey { + let mut buf = [0; secp256k1::constants::SECRET_KEY_SIZE]; + + match secret { + Some(secret) => { + let hk = Hkdf::::new(None, secret); + hk.expand(account_id.as_bytes(), &mut buf).unwrap(); + } + None => { + // We zero-pad ETH address. + buf[0..21].copy_from_slice(account_id.as_bytes()); + } + }; + + let secret_key = secp256k1_from_slice(&mut buf, account_id); + SecretKey::SECP256K1(secret_key) +} + +// If it's a NEAR-implicit account, interprets it as an ed25519 public key, maps that and then returns the resulting implicit account. +// If it's an ETH-implicit account, derive a new secp256k1 secret key from the address, and use the new public key to derive a new ETH-address. +// For named account does nothing. We do this so that transactions creating an implicit account by sending money will generate an account that we can control. pub fn map_account( account_id: &AccountId, secret: Option<&[u8; crate::secret::SECRET_LEN]>, ) -> AccountId { - if account_id.get_account_type() == AccountType::NearImplicitAccount { - let public_key = - PublicKey::from_near_implicit_account(account_id).expect("must be implicit"); - let mapped_key = map_key(&public_key, secret); - hex::encode(mapped_key.public_key().key_data()).parse().unwrap() - } else { - // TODO what if it is ETH-implicit? - account_id.clone() + match account_id.get_account_type() { + AccountType::NearImplicitAccount => { + let public_key = + PublicKey::from_near_implicit_account(account_id).expect("must be implicit"); + let mapped_key = map_key(&public_key, secret, account_id); + derive_account_id_from_public_key(&mapped_key.public_key()) + }, + AccountType::EthImplicitAccount => { + let mapped_key = map_secp256k1_from_eth_address(account_id, secret); + derive_account_id_from_public_key(&mapped_key.public_key()) + }, + AccountType::NamedAccount => account_id.clone() } } diff --git a/tools/mirror/src/lib.rs b/tools/mirror/src/lib.rs index 7fae7845e5d..0d298e4d665 100644 --- a/tools/mirror/src/lib.rs +++ b/tools/mirror/src/lib.rs @@ -26,12 +26,14 @@ use near_primitives::views::{ ExecutionOutcomeWithIdView, ExecutionStatusView, QueryRequest, QueryResponseKind, SignedTransactionView, }; -use near_primitives_core::account::{AccessKey, AccessKeyPermission, id::AccountType}; +use near_primitives_core::account::id::AccountType; +use near_primitives_core::account::{AccessKey, AccessKeyPermission}; use near_primitives_core::types::{Nonce, ShardId}; use nearcore::config::NearConfig; use rocksdb::DB; use std::borrow::Cow; use std::collections::{HashMap, HashSet}; +use std::panic; use std::path::Path; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -53,7 +55,7 @@ pub use cli::MirrorCommand; enum DBCol { Misc, // This tracks nonces for Access Keys added by AddKey transactions - // or transfers to NEAR-implicit accounts (not present in the genesis state). + // or transfers to implicit accounts (not present in the genesis state). // For a given (account ID, public key), if we're preparing a transaction // and there's no entry in the DB, then the key was present in the genesis // state. Otherwise, we map tx nonces according to the values in this column. @@ -970,7 +972,7 @@ impl TxMirror { full_key_added = true; } let public_key = - crate::key_mapping::map_key(&add_key.public_key, self.secret.as_ref()) + crate::key_mapping::map_key(&add_key.public_key, self.secret.as_ref(), tx.receiver_id()) .public_key(); let receiver_id = crate::key_mapping::map_account(tx.receiver_id(), self.secret.as_ref()); @@ -983,7 +985,7 @@ impl TxMirror { } Action::DeleteKey(delete_key) => { let replacement = - crate::key_mapping::map_key(&delete_key.public_key, self.secret.as_ref()); + crate::key_mapping::map_key(&delete_key.public_key, self.secret.as_ref(), tx.receiver_id()); let public_key = replacement.public_key(); actions.push(Action::DeleteKey(Box::new(DeleteKeyAction { public_key }))); @@ -1001,10 +1003,11 @@ impl TxMirror { ) })? { - // TODO handle eth-implicit account, no public key would be added to `nonce_updates` - let public_key = PublicKey::from_near_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. + if target_account.get_account_type() == AccountType::NearImplicitAccount { + let public_key = PublicKey::from_near_implicit_account(&target_account).expect("must be implicit"); + nonce_updates.insert((target_account, public_key)); + } } } actions.push(action.clone()); @@ -1135,7 +1138,7 @@ impl TxMirror { let mut key = None; let mut first_key = None; for k in keys.iter() { - let target_secret_key = crate::key_mapping::map_key(k, self.secret.as_ref()); + let target_secret_key = crate::key_mapping::map_key(k, self.secret.as_ref(), &receiver_id); if fetch_access_key_nonce( &self.target_view_client, &target_signer_id, @@ -1194,7 +1197,7 @@ impl TxMirror { full_key_added = true; } let target_public_key = - crate::key_mapping::map_key(&a.public_key, self.secret.as_ref()) + crate::key_mapping::map_key(&a.public_key, self.secret.as_ref(), &receiver_id) .public_key(); nonce_updates.insert((target_receiver_id.clone(), target_public_key.clone())); @@ -1515,7 +1518,7 @@ impl TxMirror { continue; } let target_private_key = - crate::key_mapping::map_key(source_tx.public_key(), self.secret.as_ref()); + crate::key_mapping::map_key(source_tx.public_key(), self.secret.as_ref(), source_tx.receiver_id()); let target_signer_id = crate::key_mapping::map_account(source_tx.signer_id(), self.secret.as_ref());