From da30e3196ca2c76d5f296281933f18a8c0e136d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Chuda=C5=9B?= Date: Fri, 27 Oct 2023 16:34:21 +0200 Subject: [PATCH] ETH-implicit accounts support --- Cargo.lock | 1 + chain/jsonrpc/res/rpc_errors_schema.json | 11 +- core/crypto/src/signature.rs | 7 + core/crypto/src/test_utils.rs | 5 +- core/primitives-core/src/runtime/fees.rs | 12 +- core/primitives/Cargo.toml | 1 + core/primitives/src/errors.rs | 11 +- core/primitives/src/test_utils.rs | 12 + core/primitives/src/utils.rs | 27 +- .../access_key_nonce_for_implicit_accounts.rs | 267 +++++++++++++++++- .../tests/client/features/delegate_action.rs | 100 +++++-- .../src/tests/client/features/restrict_tla.rs | 8 +- .../src/tests/standard_cases/mod.rs | 51 ++-- .../src/tests/standard_cases/runtime.rs | 15 + integration-tests/src/user/mod.rs | 17 +- runtime/runtime/src/actions.rs | 97 ++++--- runtime/runtime/src/verifier.rs | 63 +++-- test-utils/testlib/src/fees_utils.rs | 37 ++- tools/fork-network/src/cli.rs | 30 +- tools/mirror/src/genesis.rs | 26 +- tools/mirror/src/key_mapping.rs | 7 +- tools/mirror/src/lib.rs | 5 +- 22 files changed, 635 insertions(+), 175 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dab97ce3394..95c6d41e0cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4119,6 +4119,7 @@ dependencies = [ "serde_json", "serde_with", "serde_yaml", + "sha3", "smart-default", "strum", "thiserror", diff --git a/chain/jsonrpc/res/rpc_errors_schema.json b/chain/jsonrpc/res/rpc_errors_schema.json index 1279c61751e..b8863eab816 100644 --- a/chain/jsonrpc/res/rpc_errors_schema.json +++ b/chain/jsonrpc/res/rpc_errors_schema.json @@ -446,7 +446,8 @@ "MethodNameMismatch", "RequiresFullAccess", "NotEnoughAllowance", - "DepositWithFunctionCall" + "DepositWithFunctionCall", + "InvalidPkForEthAddress" ], "props": {} }, @@ -487,6 +488,14 @@ "tx_nonce": "" } }, + "InvalidPkForEthAddress": { + "name": "InvalidPkForEthAddress", + "subtypes": [], + "props": { + "account_id": "", + "public_key": "" + } + }, "InvalidPredecessorId": { "name": "InvalidPredecessorId", "subtypes": [], 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..3d325085384 100644 --- a/core/crypto/src/test_utils.rs +++ b/core/crypto/src/test_utils.rs @@ -30,7 +30,10 @@ impl PublicKey { 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 a2b7bf5acd5..818319eac27 100644 --- a/core/primitives-core/src/runtime/fees.rs +++ b/core/primitives-core/src/runtime/fees.rs @@ -217,8 +217,10 @@ pub fn transfer_exec_fee( (_, AccountType::NamedAccount) => transfer_fee, // No account will be created, just a regular transfer. (false, _) => transfer_fee, - // Currently, no account is created on transfer to ETH-implicit account, just a regular transfer. - (true, AccountType::EthImplicitAccount) => transfer_fee, + // Extra fee for the CreateAccount. + (true, AccountType::EthImplicitAccount) => { + transfer_fee + cfg.fee(ActionCosts::create_account).exec_fee() + } // Extra fees for the CreateAccount and AddFullAccessKey. (true, AccountType::NearImplicitAccount) => { transfer_fee @@ -240,8 +242,10 @@ pub fn transfer_send_fee( (_, AccountType::NamedAccount) => transfer_fee, // No account will be created, just a regular transfer. (false, _) => transfer_fee, - // Currently, no account is created on transfer to ETH-implicit account, just a regular transfer. - (true, AccountType::EthImplicitAccount) => transfer_fee, + // Extra fee for the CreateAccount. + (true, AccountType::EthImplicitAccount) => { + transfer_fee + cfg.fee(ActionCosts::create_account).send_fee(sender_is_receiver) + } // Extra fees for the CreateAccount and AddFullAccessKey. (true, AccountType::NearImplicitAccount) => { transfer_fee diff --git a/core/primitives/Cargo.toml b/core/primitives/Cargo.toml index 0896a09001d..74082351104 100644 --- a/core/primitives/Cargo.toml +++ b/core/primitives/Cargo.toml @@ -30,6 +30,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 16ba9beee1c..cc7d87cf454 100644 --- a/core/primitives/src/errors.rs +++ b/core/primitives/src/errors.rs @@ -209,6 +209,8 @@ pub enum InvalidAccessKeyError { }, /// Having a deposit with a function call action is not allowed with a function call access key. DepositWithFunctionCall, + /// ETH-implicit `account_id` isn't derived from the `public_key`. + InvalidPkForEthAddress { account_id: AccountId, public_key: PublicKey }, } /// Describes the error for validating a list of actions. @@ -485,8 +487,8 @@ pub enum ActionErrorKind { /// receipt validation. NewReceiptValidationError(ReceiptValidationError), /// Error occurs when a `CreateAccount` action is called on hex-characters - /// account of length 64. See implicit account creation NEP: - /// . + /// account of length 64 or 42 (when starting with '0x'). + /// See implicit account creation NEP: . /// /// TODO(#8598): This error is named very poorly. A better name would be /// `OnlyNamedAccountCreationAllowed`. @@ -610,6 +612,11 @@ impl Display for InvalidAccessKeyError { InvalidAccessKeyError::DepositWithFunctionCall => { write!(f, "Having a deposit with a function call action is not allowed with a function call access key.") } + InvalidAccessKeyError::InvalidPkForEthAddress { account_id, public_key } => write!( + f, + "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 18d111e127a..327dc1c1524 100644 --- a/core/primitives/src/test_utils.rs +++ b/core/primitives/src/test_utils.rs @@ -558,6 +558,8 @@ pub fn create_user_test_signer(account_name: &AccountIdRef) -> InMemorySigner { let account_id = account_name.to_owned(); if account_id == near_implicit_test_account() { 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.as_str()) } @@ -573,6 +575,16 @@ 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 06894bb9eaf..91d4f25d869 100644 --- a/core/primitives/src/utils.rs +++ b/core/primitives/src/utils.rs @@ -17,7 +17,7 @@ use crate::version::{ CREATE_RECEIPT_ID_SWITCH_TO_CURRENT_BLOCK_VERSION, }; -use near_crypto::ED25519PublicKey; +use near_crypto::{KeyType, PublicKey}; use near_primitives_core::account::id::AccountId; use std::mem::size_of; @@ -469,23 +469,38 @@ where Serializable(object) } -/// Derives `AccountId` from `PublicKey``. +/// Derives `AccountId` from `PublicKey`. /// If the key type is ED25519, returns hex-encoded copy of the key. -pub fn derive_near_implicit_account_id(public_key: &ED25519PublicKey) -> AccountId { - hex::encode(public_key).parse().unwrap() +/// If the key type is SECP256K1, returns '0x' + keccak256(public_key)[12:32].hex(). +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::*; - use near_crypto::{KeyType, PublicKey}; #[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(); - let account_id = derive_near_implicit_account_id(public_key.unwrap_as_ed25519()); + let account_id = derive_account_id_from_public_key(&public_key); + assert_eq!(account_id, 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(); + let account_id = derive_account_id_from_public_key(&public_key); assert_eq!(account_id, expected); } 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 bb07c943c2b..5c784fa7953 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 @@ -12,13 +12,13 @@ use near_network::shards_manager::ShardsManagerRequestFromNetwork; use near_network::types::{NetworkRequests, PeerManagerMessageRequest}; use near_o11y::testonly::init_test_logger; use near_primitives::account::AccessKey; -use near_primitives::errors::InvalidTxError; +use near_primitives::errors::{InvalidAccessKeyError, InvalidTxError}; use near_primitives::runtime::config_store::RuntimeConfigStore; 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_near_implicit_account_id; +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; @@ -198,13 +198,243 @@ fn get_status_of_tx_hash_collision_for_implicit_account( response } +/// Test that duplicate transactions from ETH-implicit account can be properly rejected +/// if we set nonce to `(block_height - 1) * 1e6` for transactions that results in +/// access key being added to the ETH-implicit account. +#[test] +fn test_transaction_eth_implicit_account() { + let epoch_length = 10; + let mut genesis = Genesis::test(vec!["test0".parse().unwrap(), "test1".parse().unwrap()], 1); + genesis.config.epoch_length = epoch_length; + let mut env = TestEnv::builder(ChainGenesis::test()) + .real_epoch_managers(&genesis.config) + .nightshade_runtimes(&genesis) + .build(); + let genesis_block = env.clients[0].chain.get_block_by_height(0).unwrap(); + 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 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.clone(), secret_key); + + // Send money to ETH-implicit account, invoking its creation. + let send_money_tx = SignedTransaction::send_money( + 1, + "test1".parse().unwrap(), + implicit_account_id.clone(), + &signer1, + deposit_for_account_creation, + *genesis_block.hash(), + ); + // Check for tx success status and get new block height. + height = check_tx_processing(&mut env, send_money_tx, height, blocks_number); + let block = env.clients[0].chain.get_block_by_height(height - 1).unwrap(); + + // Sending money from ETH-implicit account using `(block_height - 1) * 1e6` as nonce. + // That will add full access key to this account with this nonce. + let original_access_key_nonce = (height - 1) * AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER; + let send_money_from_implicit_account_tx = SignedTransaction::send_money( + original_access_key_nonce, + implicit_account_id.clone(), + "test0".parse().unwrap(), + &implicit_account_signer, + 100, + *block.hash(), + ); + height = + check_tx_processing(&mut env, send_money_from_implicit_account_tx, height, blocks_number); + let block = env.clients[0].chain.get_block_by_height(height - 1).unwrap(); + + // Try to delete implicit account. Access key is already added, so the transaction nonce will be compared + // with that access key's nonce. We should have been used greater nonce than `access_key_nonce` so this + // transaction should fail. + let try_delete_account_tx = SignedTransaction::delete_account( + original_access_key_nonce, + implicit_account_id.clone(), + implicit_account_id.clone(), + "test0".parse().unwrap(), + &implicit_account_signer, + *block.hash(), + ); + let response = env.clients[0].process_tx(try_delete_account_tx, false, false); + assert_matches!(response, ProcessTxResponse::InvalidTx(InvalidTxError::InvalidNonce { .. })); + + // Delete implicit account. This time we set transaction nonce to `access_key_nonce + 1`. + // We do not have to use `(block_height - 1) * 1e6` if no access key would be added + // to the ETH-implicit account in this transaction. + let delete_account_tx = SignedTransaction::delete_account( + original_access_key_nonce + 1, + implicit_account_id.clone(), + implicit_account_id.clone(), + "test0".parse().unwrap(), + &implicit_account_signer, + *block.hash(), + ); + height = check_tx_processing(&mut env, delete_account_tx, height, blocks_number); + let block = env.clients[0].chain.get_block_by_height(height - 1).unwrap(); + + // Send money to the ETH-implicit account again, but with incorrect nonce. + let try_send_money_again_tx = SignedTransaction::send_money( + 1, + "test1".parse().unwrap(), + implicit_account_id.clone(), + &signer1, + deposit_for_account_creation, + *block.hash(), + ); + let response = env.clients[0].process_tx(try_send_money_again_tx, false, false); + assert_matches!(response, ProcessTxResponse::InvalidTx(InvalidTxError::InvalidNonce { .. })); + + // Send money to the ETH-implicit account again, but with correct nonce. + // This will recreate the ETH-implicit account. + let send_money_again_tx = SignedTransaction::send_money( + 2, + "test1".parse().unwrap(), + implicit_account_id.clone(), + &signer1, + deposit_for_account_creation, + *block.hash(), + ); + height = check_tx_processing(&mut env, send_money_again_tx, height, blocks_number); + let block = env.clients[0].chain.get_block_by_height(height - 1).unwrap(); + + // Sending money from ETH-implicit account again using `(block_height - 1) * 1e6` as nonce. + // That will add full access key to this account again and set its nonce to the transaction nonce. + let send_money_from_implicit_account_again_tx = SignedTransaction::send_money( + (height - 1) * AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER, + implicit_account_id.clone(), + "test0".parse().unwrap(), + &implicit_account_signer, + 100, + *block.hash(), + ); + height = check_tx_processing( + &mut env, + send_money_from_implicit_account_again_tx, + height, + blocks_number, + ); + let block = env.clients[0].chain.get_block_by_height(height - 1).unwrap(); + + // Sending money from ETH-implicit account again, but this time ignoring the fact that the account and access key + // were recreated. Using `original_access_key_nonce + 2` would work, but the access key was recreated + // using `(block_height - 1) * 1e6` as the new nonce (current block height has changed since the last time). + let try_send_money_from_implicit_account_again_tx = SignedTransaction::send_money( + original_access_key_nonce + 2, + implicit_account_id, + "test0".parse().unwrap(), + &implicit_account_signer, + 100, + *block.hash(), + ); + let response = + env.clients[0].process_tx(try_send_money_from_implicit_account_again_tx, false, false); + assert_matches!(response, ProcessTxResponse::InvalidTx(InvalidTxError::InvalidNonce { .. })); +} + +/// Test that the signer is correctly verified for transactions done from an ETH-implicit account. +#[test] +fn test_transaction_eth_implicit_account_invalid_pk() { + let epoch_length = 10; + let mut genesis = Genesis::test(vec!["test0".parse().unwrap(), "test1".parse().unwrap()], 1); + genesis.config.epoch_length = epoch_length; + let mut env = TestEnv::builder(ChainGenesis::test()) + .real_epoch_managers(&genesis.config) + .nightshade_runtimes(&genesis) + .build(); + let genesis_block = env.clients[0].chain.get_block_by_height(0).unwrap(); + 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 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.clone(), secret_key); + + // Send money to ETH-implicit account, invoking its creation. + let send_money_tx = SignedTransaction::send_money( + 1, + "test1".parse().unwrap(), + implicit_account_id.clone(), + &signer1, + deposit_for_account_creation, + *genesis_block.hash(), + ); + height = check_tx_processing(&mut env, send_money_tx, height, blocks_number); + let block = env.clients[0].chain.get_block_by_height(height - 1).unwrap(); + + let secret_key_2 = SecretKey::from_seed(KeyType::SECP256K1, "test2"); + let invalid_signer_2 = + InMemorySigner::from_secret_key(implicit_account_id.clone(), secret_key_2); + + // Try sending money from ETH-implicit account using incorrect signer. + // That will check if signer's account ID is derived from the provided public key. + // As the providede public key does not match the account address, no access key would be added. + let try_send_money_from_implicit_account_tx = SignedTransaction::send_money( + (height - 1) * AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER, + implicit_account_id.clone(), + "test0".parse().unwrap(), + &invalid_signer_2, + 100, + *block.hash(), + ); + let response = env.clients[0].process_tx(try_send_money_from_implicit_account_tx, false, false); + assert_matches!( + response, + ProcessTxResponse::InvalidTx(InvalidTxError::InvalidAccessKeyError( + InvalidAccessKeyError::InvalidPkForEthAddress { .. } + )) + ); + + let secret_key_3 = SecretKey::from_seed(KeyType::ED25519, "test"); + let invalid_signer_3 = + InMemorySigner::from_secret_key(implicit_account_id.clone(), secret_key_3); + + // Now we try sending money from ETH-implicit account providing ED25519 public key. + // The transaction would result in creating access key, so it should be signed by Secp256k1 key. + let try_send_money_from_implicit_account_again_tx = SignedTransaction::send_money( + (height - 1) * AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER, + implicit_account_id.clone(), + "test0".parse().unwrap(), + &invalid_signer_3, + 100, + *block.hash(), + ); + let response = + env.clients[0].process_tx(try_send_money_from_implicit_account_again_tx, false, false); + assert_matches!( + response, + ProcessTxResponse::InvalidTx(InvalidTxError::InvalidAccessKeyError( + InvalidAccessKeyError::InvalidPkForEthAddress { .. } + )) + ); + + // Now we send money from ETH-implicit account using correct signing key. + // As this is the first valid transaction from that account, no access key has been added yet, + // so using 0 nonce should pass (we should not use 0 nonce in real transaction). + let send_money_from_implicit_account_tx = SignedTransaction::send_money( + 0, + implicit_account_id, + "test0".parse().unwrap(), + &implicit_account_signer, + 100, + *block.hash(), + ); + check_tx_processing(&mut env, send_money_from_implicit_account_tx, height, blocks_number); +} + /// Test that duplicate transactions from NEAR-implicit accounts are properly rejected. #[test] -fn test_transaction_hash_collision_for_implicit_account_fail() { +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_near_implicit_account_id(secret_key.public_key().unwrap_as_ed25519()); + 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( @@ -217,12 +447,11 @@ fn test_transaction_hash_collision_for_implicit_account_fail() { /// Test that duplicate transactions from NEAR-implicit accounts are not rejected until protocol upgrade. #[test] -fn test_transaction_hash_collision_for_implicit_account_ok() { +fn test_transaction_hash_collision_for_near_implicit_account_ok() { let protocol_version = ProtocolFeature::AccessKeyNonceForImplicitAccounts.protocol_version() - 1; let secret_key = SecretKey::from_seed(KeyType::ED25519, "test"); - let implicit_account_id = - derive_near_implicit_account_id(secret_key.public_key().unwrap_as_ed25519()); + 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( @@ -233,6 +462,28 @@ fn test_transaction_hash_collision_for_implicit_account_ok() { ); } +/// Test that duplicate transactions from ETH-implicit accounts are not rejected before and after protocol upgrade. +/// It is responsibility of the transaction signer to choose nonce equal to `(block_height - 1) * 1e6` in case the transaction +/// results in adding a new key to an ETH-implicit account (see https://github.com/near/NEPs/issues/498#issuecomment-1782881395). +#[test] +fn test_transaction_hash_collision_for_eth_implicit_account_ok() { + let protocol_version = + ProtocolFeature::AccessKeyNonceForImplicitAccounts.protocol_version() - 1; + for protocol_version in protocol_version..=protocol_version + 1 { + 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_implicit_account( + protocol_version, + implicit_account_signer + ), + ProcessTxResponse::ValidTx + ); + } +} + /// Test that chunks with transactions that have expired are considered invalid. #[test] fn test_chunk_transaction_validity() { diff --git a/integration-tests/src/tests/client/features/delegate_action.rs b/integration-tests/src/tests/client/features/delegate_action.rs index 503b6278541..405a43a070b 100644 --- a/integration-tests/src/tests/client/features/delegate_action.rs +++ b/integration-tests/src/tests/client/features/delegate_action.rs @@ -17,7 +17,10 @@ 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, eth_implicit_test_account_secret, + near_implicit_test_account, +}; use near_primitives::transaction::{ Action, AddKeyAction, CreateAccountAction, DeleteAccountAction, DeleteKeyAction, DeployContractAction, FunctionCallAction, StakeAction, TransferAction, @@ -119,6 +122,8 @@ fn check_meta_tx_execution( sender: AccountId, relayer: AccountId, receiver: AccountId, + // For meta transactions from ETH-implicit account, we require Secp256K1 `public_key` matching the sender address. + public_key: Option, ) -> (FinalExecutionOutcomeView, i128, i128, i128) { let node_user = node.user(); @@ -135,12 +140,17 @@ fn check_meta_tx_execution( .get_access_key(&relayer, &PublicKey::from_seed(KeyType::ED25519, relayer.as_ref())) .unwrap() .nonce; + let user_pubk = match sender.get_account_type() { AccountType::NearImplicitAccount => PublicKey::from_near_implicit_account(&sender).unwrap(), - AccountType::EthImplicitAccount => PublicKey::from_seed(KeyType::ED25519, sender.as_ref()), + // We require that tests sending transactions from ETH-implicit accounts provide the public key. + // It is because we can't infer public key from ETH-implicit account ID, and we need + // the public key from which the ETH-implicit sender address was derived. + AccountType::EthImplicitAccount => public_key.unwrap(), AccountType::NamedAccount => PublicKey::from_seed(KeyType::ED25519, sender.as_ref()), }; - let user_nonce_before = node_user.get_access_key(&sender, &user_pubk).unwrap().nonce; + + let user_nonce_before = node_user.get_access_key(&sender, &user_pubk); let tx_result = node_user.meta_tx(sender.clone(), receiver.clone(), relayer.clone(), actions).unwrap(); @@ -154,12 +164,18 @@ 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.as_ref())) - .map(|key| key.nonce) - { - assert_eq!(user_nonce, user_nonce_before + 1); + + match user_nonce_before { + Ok(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.as_ref())) + .map(|key| key.nonce) + { + assert_eq!(user_nonce, user_nonce_before.nonce + 1); + } + } + Err(_) => assert!(sender.get_account_type() == AccountType::EthImplicitAccount), } let sender_after = node_user.view_balance(&sender).unwrap_or(0); @@ -183,12 +199,13 @@ fn check_meta_tx_no_fn_call( sender: AccountId, relayer: AccountId, receiver: AccountId, + public_key: Option, ) -> FinalExecutionOutcomeView { let fee_helper = fee_helper(node); let gas_cost = normal_tx_cost + fee_helper.meta_tx_overhead_cost(&actions, &receiver); let (tx_result, sender_diff, relayer_diff, receiver_diff) = - check_meta_tx_execution(node, actions, sender, relayer, receiver); + check_meta_tx_execution(node, actions, sender, relayer, receiver, public_key); assert_eq!(sender_diff, 0, "sender should not pay for anything"); assert_eq!(receiver_diff, tokens_transferred as i128, "unexpected receiver balance"); @@ -220,7 +237,7 @@ fn check_meta_tx_fn_call( let meta_tx_overhead_cost = fee_helper.meta_tx_overhead_cost(&actions, &receiver); let (tx_result, sender_diff, relayer_diff, receiver_diff) = - check_meta_tx_execution(node, actions, sender, relayer, receiver); + check_meta_tx_execution(node, actions, sender, relayer, receiver, None); assert_eq!(sender_diff, 0, "sender should not pay for anything"); @@ -274,7 +291,7 @@ fn meta_tx_near_transfer() { let amount = nearcore::NEAR_BASE; let actions = vec![Action::Transfer(TransferAction { deposit: amount })]; let tx_cost = fee_helper.transfer_cost(); - check_meta_tx_no_fn_call(&node, actions, tx_cost, amount, sender, relayer, receiver); + check_meta_tx_no_fn_call(&node, actions, tx_cost, amount, sender, relayer, receiver, None); } /// Call a function on the test contract provided by default in the test environment. @@ -437,7 +454,7 @@ fn meta_tx_deploy() { let code = smallest_rs_contract().to_vec(); let tx_cost = fee_helper.deploy_contract_cost(code.len() as u64); let actions = vec![Action::DeployContract(DeployContractAction { code })]; - check_meta_tx_no_fn_call(&node, actions, tx_cost, 0, sender, relayer, receiver); + check_meta_tx_no_fn_call(&node, actions, tx_cost, 0, sender, relayer, receiver, None); } #[test] @@ -452,7 +469,7 @@ fn meta_tx_stake() { let tx_cost = fee_helper.stake_cost(); let public_key = create_user_test_signer(&sender).public_key; let actions = vec![Action::Stake(Box::new(StakeAction { public_key, stake: 0 }))]; - check_meta_tx_no_fn_call(&node, actions, tx_cost, 0, sender, relayer, receiver); + check_meta_tx_no_fn_call(&node, actions, tx_cost, 0, sender, relayer, receiver, None); } #[test] @@ -472,7 +489,7 @@ fn meta_tx_add_key() { public_key: public_key.clone(), access_key: AccessKey::full_access(), }))]; - check_meta_tx_no_fn_call(&node, actions, tx_cost, 0, sender, relayer, receiver.clone()); + check_meta_tx_no_fn_call(&node, actions, tx_cost, 0, sender, relayer, receiver.clone(), None); let key_view = node .user() @@ -498,7 +515,7 @@ fn meta_tx_delete_key() { let public_key = PublicKey::from_seed(KeyType::ED25519, receiver.as_ref()); let actions = vec![Action::DeleteKey(Box::new(DeleteKeyAction { public_key: public_key.clone() }))]; - check_meta_tx_no_fn_call(&node, actions, tx_cost, 0, sender, relayer, receiver.clone()); + check_meta_tx_no_fn_call(&node, actions, tx_cost, 0, sender, relayer, receiver.clone(), None); let err = node .user() @@ -535,7 +552,7 @@ fn meta_tx_delete_account() { let gas_cost = fee_helper.prepaid_delete_account_cost() + fee_helper.meta_tx_overhead_cost(&actions, &receiver); let (_tx_result, sender_diff, relayer_diff, receiver_diff) = - check_meta_tx_execution(&node, actions, sender, relayer, receiver.clone()); + check_meta_tx_execution(&node, actions, sender, relayer, receiver.clone(), None); assert_eq!( sender_diff, @@ -773,7 +790,16 @@ fn meta_tx_create_named_account() { node.view_account(&new_account).expect_err("account already exists"); let tx_cost = fee_helper.create_account_transfer_full_key_cost(); - check_meta_tx_no_fn_call(&node, actions, tx_cost, amount, sender, relayer, new_account.clone()); + check_meta_tx_no_fn_call( + &node, + actions, + tx_cost, + amount, + sender, + relayer, + new_account.clone(), + None, + ); // Check the account exists after we created it. node.view_account(&new_account).expect("failed looking up account"); @@ -803,6 +829,11 @@ 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. /// @@ -840,13 +871,21 @@ 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 an 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. -fn meta_tx_create_implicit_account(new_account: AccountId) { +fn meta_tx_create_implicit_account( + new_account: AccountId, + new_account_public_key: Option, +) { let relayer = bob_account(); let sender = alice_account(); let node = RuntimeNode::new(&relayer); @@ -860,8 +899,8 @@ fn meta_tx_create_implicit_account(new_account: AccountId) { let tx_cost = match new_account.get_account_type() { AccountType::NearImplicitAccount => fee_helper.create_account_transfer_full_key_cost(), - AccountType::EthImplicitAccount => panic!("must be near-implicit"), - AccountType::NamedAccount => panic!("must be near-implicit"), + AccountType::EthImplicitAccount => fee_helper.create_account_transfer_cost(), + AccountType::NamedAccount => panic!("must be implicit"), }; check_meta_tx_no_fn_call( &node, @@ -871,6 +910,7 @@ fn meta_tx_create_implicit_account(new_account: AccountId) { sender.clone(), relayer.clone(), new_account.clone(), + None, ); // Check account exists with expected balance @@ -881,7 +921,14 @@ fn meta_tx_create_implicit_account(new_account: AccountId) { // Now test we can use this account in a meta transaction that sends back half the tokens to alice. let transfer_amount = initial_amount / 2; let actions = vec![Action::Transfer(TransferAction { deposit: transfer_amount })]; - let tx_cost = fee_helper.transfer_cost(); + + let tx_cost = match new_account.get_account_type() { + AccountType::NearImplicitAccount => fee_helper.transfer_cost(), + // TODO Test did not pass with `transfer_full_key_cost`, only passes with `transfer_cost`. + AccountType::EthImplicitAccount => fee_helper.transfer_cost(), + AccountType::NamedAccount => panic!("must be implicit"), + }; + check_meta_tx_no_fn_call( &node, actions, @@ -890,6 +937,7 @@ fn meta_tx_create_implicit_account(new_account: AccountId) { new_account.clone(), relayer, sender, + new_account_public_key, ) .assert_success(); @@ -901,5 +949,11 @@ fn meta_tx_create_implicit_account(new_account: AccountId) { #[test] fn meta_tx_create_near_implicit_account() { - meta_tx_create_implicit_account(near_implicit_test_account()); + meta_tx_create_implicit_account(near_implicit_test_account(), None); +} + +#[test] +fn meta_tx_create_eth_implicit_account() { + let secret_key = eth_implicit_test_account_secret(); + meta_tx_create_implicit_account(eth_implicit_test_account(), Some(secret_key.public_key())); } diff --git a/integration-tests/src/tests/client/features/restrict_tla.rs b/integration-tests/src/tests/client/features/restrict_tla.rs index 250a2e42af1..b50675845ec 100644 --- a/integration-tests/src/tests/client/features/restrict_tla.rs +++ b/integration-tests/src/tests/client/features/restrict_tla.rs @@ -23,11 +23,11 @@ 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 characters long. + // Note that implicit accounts have to be 64 or 42 (if starts with '0x') characters long. let top_level_accounts = [ - "0x06012c8cf97bead5deae237070f9587f8e7a266d", - "0x5e97870f263700f46aa00d967821199b9bc5a120", - "0x0000000000000000000000000000000000000000", + "0x06012c8cf97bead5deae237070f9587f8e7a266da", + "0a5e97870f263700f46aa00d967821199b9bc5a120", + "0x000000000000000000000000000000000000000", "alice", "thisisaveryverylongtoplevelaccount", ]; diff --git a/integration-tests/src/tests/standard_cases/mod.rs b/integration-tests/src/tests/standard_cases/mod.rs index 4a47ec2c397..432f27bf121 100644 --- a/integration-tests/src/tests/standard_cases/mod.rs +++ b/integration-tests/src/tests/standard_cases/mod.rs @@ -16,7 +16,7 @@ use near_primitives::errors::{ }; use near_primitives::hash::{hash, CryptoHash}; use near_primitives::types::{AccountId, Balance, TrieNodesCount}; -use near_primitives::utils::derive_near_implicit_account_id; +use near_primitives::utils::derive_account_id_from_public_key; use near_primitives::views::{ AccessKeyView, AccountView, ExecutionMetadataView, FinalExecutionOutcomeView, FinalExecutionStatus, @@ -336,12 +336,12 @@ pub fn transfer_tokens_implicit_account(node: impl Node, public_key: PublicKey) let root = node_user.get_state_root(); let tokens_used = 10u128.pow(25); let fee_helper = fee_helper(&node); - let receiver_id = derive_near_implicit_account_id(public_key.unwrap_as_ed25519()); + 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 => std::panic!("must be near-implicit"), - AccountType::NamedAccount => std::panic!("must be near-implicit"), + AccountType::EthImplicitAccount => fee_helper.create_account_transfer_cost(), + AccountType::NamedAccount => std::panic!("must be implicit"), }; let transaction_result = @@ -369,8 +369,11 @@ pub fn transfer_tokens_implicit_account(node: impl Node, public_key: PublicKey) AccountType::NearImplicitAccount => { assert_eq!(view_access_key.unwrap(), AccessKey::full_access().into()); } - AccountType::EthImplicitAccount => std::panic!("must be near-implicit"), - AccountType::NamedAccount => std::panic!("must be near-implicit"), + AccountType::EthImplicitAccount => { + // A transfer to ETH-implicit address does not create access key. + assert!(node_user.get_access_key(&receiver_id, &public_key).is_err()); + } + AccountType::NamedAccount => std::panic!("must be implicit"), } let transaction_result = @@ -401,31 +404,31 @@ pub fn trying_to_create_implicit_account(node: impl Node, public_key: PublicKey) let root = node_user.get_state_root(); let tokens_used = 10u128.pow(25); let fee_helper = fee_helper(&node); - let receiver_id = derive_near_implicit_account_id(public_key.unwrap_as_ed25519()); + let receiver_id = derive_account_id_from_public_key(&public_key); let transaction_result = node_user - .create_account( - account_id.clone(), - receiver_id.clone(), - node.signer().public_key(), - tokens_used, - ) + .create_account(account_id.clone(), receiver_id.clone(), public_key, tokens_used) .unwrap(); + let create_account_fee = fee_helper.cfg().fee(ActionCosts::create_account).send_fee(false); + let add_access_key_fee = fee_helper + .cfg() + .fee(near_primitives::config::ActionCosts::add_full_access_key) + .send_fee(false); + let cost = match receiver_id.get_account_type() { AccountType::NearImplicitAccount => { - let fail_cost = - fee_helper.create_account_transfer_full_key_cost_fail_on_create_account(); - let create_account_fee = - fee_helper.cfg().fee(ActionCosts::create_account).send_fee(false); - let add_access_key_fee = fee_helper - .cfg() - .fee(near_primitives::config::ActionCosts::add_full_access_key) - .send_fee(false); - fail_cost + fee_helper.gas_to_balance(create_account_fee + add_access_key_fee) + fee_helper.create_account_transfer_full_key_cost_fail_on_create_account() + // TODO Create account send fee and add access key send fee are already included + // in create_account_transfer_full_key_cost_fail_on_create_account. Why do we add it again? + + fee_helper.gas_to_balance(create_account_fee + add_access_key_fee) + } + AccountType::EthImplicitAccount => { + fee_helper.create_account_transfer_cost_fail_on_create_account() + // TODO See above. create_account_fee + add_access_key_fee are needed for test to pass. Why? + + fee_helper.gas_to_balance(create_account_fee + add_access_key_fee) } - AccountType::EthImplicitAccount => std::panic!("must be near-implicit"), - AccountType::NamedAccount => std::panic!("must be near-implicit"), + AccountType::NamedAccount => std::panic!("must be implicit"), }; assert_eq!( diff --git a/integration-tests/src/tests/standard_cases/runtime.rs b/integration-tests/src/tests/standard_cases/runtime.rs index c30551085f0..5fe99b9d2e7 100644 --- a/integration-tests/src/tests/standard_cases/runtime.rs +++ b/integration-tests/src/tests/standard_cases/runtime.rs @@ -1,6 +1,7 @@ use crate::node::RuntimeNode; use crate::tests::standard_cases::*; use near_chain_configs::Genesis; +use near_crypto::SecretKey; use near_primitives::state_record::StateRecord; use nearcore::config::{GenesisExt, TESTING_INIT_BALANCE}; use testlib::runtime_utils::{add_test_contract, alice_account, bob_account}; @@ -120,6 +121,13 @@ fn test_transfer_tokens_near_implicit_account_runtime() { 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()); +} + #[test] fn test_trying_to_create_near_implicit_account_runtime() { let node = create_runtime_node(); @@ -127,6 +135,13 @@ fn test_trying_to_create_near_implicit_account_runtime() { 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] fn test_smart_contract_reward_runtime() { let node = create_runtime_node(); diff --git a/integration-tests/src/user/mod.rs b/integration-tests/src/user/mod.rs index cc1da5b1a8f..d5beec891ce 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::{id::AccountType, AccessKey}; use near_primitives::action::delegate::{DelegateAction, NonDelegateAction, SignedDelegateAction}; use near_primitives::hash::CryptoHash; use near_primitives::receipt::Receipt; @@ -262,10 +262,17 @@ 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() { + // Zero nonce is for tests only. In real setting we should use `(block_height - 1) * 1e6`. + AccountType::EthImplicitAccount => 0, + _ => panic!("failed reading user's nonce for access key"), + }, + }; + let delegate_action = DelegateAction { sender_id: signer_id.clone(), receiver_id, diff --git a/runtime/runtime/src/actions.rs b/runtime/runtime/src/actions.rs index df4f6e6616e..a3516cc86f9 100644 --- a/runtime/runtime/src/actions.rs +++ b/runtime/runtime/src/actions.rs @@ -22,7 +22,7 @@ use near_primitives::transaction::{ }; use near_primitives::types::validator_stake::ValidatorStake; use near_primitives::types::{AccountId, BlockHeight, EpochInfoProvider, Gas, TrieCacheMode}; -use near_primitives::utils::create_random_seed; +use near_primitives::utils::{create_random_seed, derive_account_id_from_public_key}; use near_primitives::version::{ ProtocolFeature, ProtocolVersion, DELETE_KEY_STORAGE_USAGE_PROTOCOL_VERSION, }; @@ -473,9 +473,16 @@ pub(crate) fn action_implicit_account_creation_transfer( } // Invariant: The `account_id` is implicit. // It holds because in the only calling site, we've checked the permissions before. - AccountType::EthImplicitAccount | AccountType::NamedAccount => { - panic!("must be near-implicit") + AccountType::EthImplicitAccount => { + *account = Some(Account::new( + transfer.deposit, + 0, + CryptoHash::default(), + fee_config.storage_usage_config.num_bytes_account + + fee_config.storage_usage_config.num_extra_bytes_record, + )); } + AccountType::NamedAccount => panic!("must be implicit"), } } @@ -738,31 +745,58 @@ fn validate_delegate_action_key( result: &mut ActionResult, ) -> Result<(), RuntimeError> { // 'delegate_action.sender_id' account existence must be checked by a caller - let mut access_key = match get_access_key( - state_update, - &delegate_action.sender_id, - &delegate_action.public_key, - )? { - Some(access_key) => access_key, - None => { - result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( - InvalidAccessKeyError::AccessKeyNotFound { - account_id: delegate_action.sender_id.clone(), - public_key: delegate_action.public_key.clone(), - }, - ) + let signer_id = &delegate_action.sender_id; + let public_key = &delegate_action.public_key; + // By default, we always check nonce. + let mut should_check_nonce = true; + let mut access_key = + match get_access_key(state_update, &signer_id, &delegate_action.public_key)? { + Some(access_key) => access_key, + None => match signer_id.get_account_type() { + // Access key is missing, but in the case of a transaction from ETH-implicit account + // there is a possibility that full access key will be created now. + AccountType::EthImplicitAccount => { + if derive_account_id_from_public_key(public_key) == *signer_id { + // This is the only case we do not compare access key nonce with transaction nonce, + // because the access key is created now. + should_check_nonce = false; + // TODO What about increasing storage usage for that account, because we added access key? + AccessKey::full_access() + } else { + // Provided public key is not the one from which the signer address was derived. + result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::InvalidPkForEthAddress { + account_id: signer_id.clone(), + public_key: public_key.clone(), + }, + ) + .into()); + return Ok(()); + } + } + // Transactions done from non-ETH-implicit account must have access key already added. + _ => { + result.result = Err(ActionErrorKind::DelegateActionAccessKeyError( + InvalidAccessKeyError::AccessKeyNotFound { + account_id: signer_id.clone(), + public_key: public_key.clone(), + }, + ) + .into()); + return Ok(()); + } + }, + }; + + if should_check_nonce { + if delegate_action.nonce <= access_key.nonce { + result.result = Err(ActionErrorKind::DelegateActionInvalidNonce { + delegate_nonce: delegate_action.nonce, + ak_nonce: access_key.nonce, + } .into()); return Ok(()); } - }; - - if delegate_action.nonce <= access_key.nonce { - result.result = Err(ActionErrorKind::DelegateActionInvalidNonce { - delegate_nonce: delegate_action.nonce, - ak_nonce: access_key.nonce, - } - .into()); - return Ok(()); } let upper_bound = apply_state.block_height @@ -831,12 +865,7 @@ fn validate_delegate_action_key( } }; - set_access_key( - state_update, - delegate_action.sender_id.clone(), - delegate_action.public_key.clone(), - &access_key, - ); + set_access_key(state_update, signer_id.clone(), public_key.clone(), &access_key); Ok(()) } @@ -897,8 +926,7 @@ pub(crate) fn check_account_existence( } else { // TODO: this should be `config.implicit_account_creation`. if config.wasm_config.implicit_account_creation - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - && account_id.get_account_type() == AccountType::NearImplicitAccount + && account_id.get_account_type().is_implicit() { // If the account doesn't exist and it's implicit, then you // should only be able to create it using single transfer action. @@ -920,8 +948,7 @@ pub(crate) fn check_account_existence( if account.is_none() { return if config.wasm_config.implicit_account_creation && is_the_only_action - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - && account_id.get_account_type() == AccountType::NearImplicitAccount + && account_id.get_account_type().is_implicit() && !is_refund { // OK. It's implicit account creation. diff --git a/runtime/runtime/src/verifier.rs b/runtime/runtime/src/verifier.rs index e020483d6e4..17cbf60e2ce 100644 --- a/runtime/runtime/src/verifier.rs +++ b/runtime/runtime/src/verifier.rs @@ -2,7 +2,7 @@ use crate::config::{total_prepaid_gas, tx_cost, TransactionCost}; use crate::near_primitives::account::Account; use crate::VerificationResult; use near_crypto::key_conversion::is_valid_staking_key; -use near_primitives::account::AccessKeyPermission; +use near_primitives::account::{AccessKey, AccessKeyPermission}; use near_primitives::action::delegate::SignedDelegateAction; use near_primitives::checked_feature; use near_primitives::errors::{ @@ -17,8 +17,10 @@ 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; use near_store::{ get_access_key, get_account, set_access_key, set_account, StorageError, TrieUpdate, }; @@ -144,6 +146,7 @@ pub fn verify_and_charge_transaction( )?; let transaction = &signed_transaction.transaction; let signer_id = &transaction.signer_id; + let public_key = &transaction.public_key; let mut signer = match get_account(state_update, signer_id)? { Some(signer) => signer, @@ -151,26 +154,54 @@ pub fn verify_and_charge_transaction( return Err(InvalidTxError::SignerDoesNotExist { signer_id: signer_id.clone() }.into()); } }; - let mut access_key = match get_access_key(state_update, signer_id, &transaction.public_key)? { + // By default, we always check nonce. + let mut should_check_nonce = true; + let mut access_key = match get_access_key(state_update, signer_id, public_key)? { Some(access_key) => access_key, - None => { - return Err(InvalidTxError::InvalidAccessKeyError( - InvalidAccessKeyError::AccessKeyNotFound { - account_id: signer_id.clone(), - public_key: transaction.public_key.clone(), - }, - ) - .into()); - } + None => match signer_id.get_account_type() { + // Access key is missing, but in the case of a transaction from ETH-implicit account + // there is a possibility that full access key will be created now. + AccountType::EthImplicitAccount => { + if derive_account_id_from_public_key(public_key) == *signer_id { + // This is the only case we do not compare access key nonce with transaction nonce, + // because the access key is created now. + should_check_nonce = false; + // TODO What about increasing storage usage for that account, because we added access key? + AccessKey::full_access() + } else { + // Provided public key is not the one from which the signer address was derived. + return Err(InvalidTxError::InvalidAccessKeyError( + InvalidAccessKeyError::InvalidPkForEthAddress { + account_id: signer_id.clone(), + public_key: public_key.clone(), + }, + ) + .into()); + } + } + // Transactions done from non-ETH-implicit account must have access key already added. + _ => { + return Err(InvalidTxError::InvalidAccessKeyError( + InvalidAccessKeyError::AccessKeyNotFound { + account_id: signer_id.clone(), + public_key: public_key.clone(), + }, + ) + .into()); + } + }, }; - if transaction.nonce <= access_key.nonce { - return Err(InvalidTxError::InvalidNonce { - tx_nonce: transaction.nonce, - ak_nonce: access_key.nonce, + if should_check_nonce { + if transaction.nonce <= access_key.nonce { + return Err(InvalidTxError::InvalidNonce { + tx_nonce: transaction.nonce, + ak_nonce: access_key.nonce, + } + .into()); } - .into()); } + if checked_feature!("stable", AccessKeyNonceRange, current_protocol_version) { if let Some(height) = block_height { let upper_bound = diff --git a/test-utils/testlib/src/fees_utils.rs b/test-utils/testlib/src/fees_utils.rs index ddd964946a5..37b71d90b85 100644 --- a/test-utils/testlib/src/fees_utils.rs +++ b/test-utils/testlib/src/fees_utils.rs @@ -56,10 +56,34 @@ 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 transfer_full_key_fee(&self) -> Gas { + let exec_gas = self.cfg().fee(ActionCosts::new_action_receipt).exec_fee() + + self.cfg().fee(ActionCosts::transfer).exec_fee() + + self.cfg().fee(ActionCosts::add_full_access_key).exec_fee(); + let send_gas = self.cfg().fee(ActionCosts::new_action_receipt).send_fee(false) + + self.cfg().fee(ActionCosts::transfer).send_fee(false) + + self.cfg().fee(ActionCosts::add_full_access_key).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 +106,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,8 +151,8 @@ 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 transfer_full_key_cost(&self) -> Balance { + self.gas_to_balance(self.transfer_full_key_fee()) } pub fn stake_cost(&self) -> Balance { diff --git a/tools/fork-network/src/cli.rs b/tools/fork-network/src/cli.rs index 74ca5f54a65..ee5d617d7b1 100644 --- a/tools/fork-network/src/cli.rs +++ b/tools/fork-network/src/cli.rs @@ -8,7 +8,6 @@ use near_epoch_manager::{EpochManager, EpochManagerAdapter, EpochManagerHandle}; use near_mirror::key_mapping::{map_account, map_key}; use near_o11y::default_subscriber_with_opentelemetry; use near_o11y::env_filter::make_env_filter; -use near_primitives::account::id::AccountType; use near_primitives::account::{AccessKey, AccessKeyPermission, Account}; use near_primitives::borsh; use near_primitives::hash::CryptoHash; @@ -487,8 +486,7 @@ 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 } => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - if account_id.get_account_type() != AccountType::NearImplicitAccount + if !account_id.get_account_type().is_implicit() && access_key.permission == AccessKeyPermission::FullAccess { has_full_key.insert(account_id.clone()); @@ -505,8 +503,7 @@ impl ForkNetworkCommand { } StateRecord::Account { account_id, account } => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - if account_id.get_account_type() == AccountType::NearImplicitAccount { + 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)?; @@ -514,8 +511,7 @@ impl ForkNetworkCommand { } } StateRecord::Data { account_id, data_key, value } => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - if account_id.get_account_type() == AccountType::NearImplicitAccount { + 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)?; @@ -523,8 +519,7 @@ impl ForkNetworkCommand { } } StateRecord::Contract { account_id, code } => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - if account_id.get_account_type() == AccountType::NearImplicitAccount { + 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)?; @@ -532,11 +527,8 @@ impl ForkNetworkCommand { } } StateRecord::PostponedReceipt(receipt) => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - 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() { let new_receipt = Receipt { predecessor_id: map_account(&receipt.predecessor_id, None), @@ -550,8 +542,7 @@ impl ForkNetworkCommand { } } StateRecord::ReceivedData { account_id, data_id, data } => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - if account_id.get_account_type() == AccountType::NearImplicitAccount { + 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)?; @@ -559,11 +550,8 @@ impl ForkNetworkCommand { } } StateRecord::DelayedReceipt(receipt) => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - 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() { 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 38415ac8f5a..6f354d8df76 100644 --- a/tools/mirror/src/genesis.rs +++ b/tools/mirror/src/genesis.rs @@ -1,5 +1,4 @@ use near_primitives::state_record::StateRecord; -use near_primitives_core::account::id::AccountType; use near_primitives_core::account::{AccessKey, AccessKeyPermission}; use serde::ser::{SerializeSeq, Serializer}; use std::collections::HashSet; @@ -39,8 +38,7 @@ pub fn map_records>( public_key: replacement.public_key(), access_key: access_key.clone(), }; - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - if account_id.get_account_type() != AccountType::NearImplicitAccount + if !account_id.get_account_type().is_implicit() && access_key.permission == AccessKeyPermission::FullAccess { has_full_key.insert(account_id.clone()); @@ -50,8 +48,7 @@ pub fn map_records>( records_seq.serialize_element(&new_record).unwrap(); } StateRecord::Account { account_id, .. } => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - 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()); @@ -59,23 +56,20 @@ pub fn map_records>( records_seq.serialize_element(&r).unwrap(); } StateRecord::Data { account_id, .. } => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - 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, .. } => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - 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) => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - 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()); @@ -85,16 +79,14 @@ pub fn map_records>( records_seq.serialize_element(&r).unwrap(); } StateRecord::ReceivedData { account_id, .. } => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - 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) => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - 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 4cbc8608419..3dbc8be63e2 100644 --- a/tools/mirror/src/key_mapping.rs +++ b/tools/mirror/src/key_mapping.rs @@ -1,7 +1,7 @@ use hkdf::Hkdf; use near_crypto::{ED25519PublicKey, ED25519SecretKey, PublicKey, Secp256K1PublicKey, SecretKey}; use near_primitives::types::AccountId; -use near_primitives::utils::derive_near_implicit_account_id; +use near_primitives::utils::derive_account_id_from_public_key; use near_primitives_core::account::id::AccountType; use sha2::Sha256; @@ -103,10 +103,11 @@ pub fn map_account( match account_id.get_account_type() { AccountType::NearImplicitAccount => { let public_key = - PublicKey::from_near_implicit_account(account_id).expect("must be near-implicit"); + PublicKey::from_near_implicit_account(account_id).expect("must be implicit"); let mapped_key = map_key(&public_key, secret); - derive_near_implicit_account_id(mapped_key.public_key().unwrap_as_ed25519()) + derive_account_id_from_public_key(&mapped_key.public_key()) } + // TODO(eth-implicit) map to a new ETH address AccountType::EthImplicitAccount => account_id.clone(), AccountType::NamedAccount => account_id.clone(), } diff --git a/tools/mirror/src/lib.rs b/tools/mirror/src/lib.rs index 84f1c3e0c62..6f829c2161f 100644 --- a/tools/mirror/src/lib.rs +++ b/tools/mirror/src/lib.rs @@ -990,8 +990,7 @@ impl TxMirror { actions.push(Action::DeleteKey(Box::new(DeleteKeyAction { public_key }))); } Action::Transfer(_) => { - // TODO(eth-implicit) Change back to is_implicit() when ETH-implicit accounts are supported. - if tx.receiver_id().get_account_type() == AccountType::NearImplicitAccount + if tx.receiver_id().get_account_type().is_implicit() && source_actions.len() == 1 { let target_account = @@ -1009,7 +1008,7 @@ impl TxMirror { { let public_key = PublicKey::from_near_implicit_account(&target_account) - .expect("must be near-implicit"); + .expect("must be implicit"); nonce_updates.insert((target_account, public_key)); } }