ETH-implicit accounts tests

core/primitives-core/src/runtime/fees.rs

@@ -228,7 +228,6 @@ pub fn transfer_exec_fee(
                 + cfg.fee(ActionCosts::add_full_access_key).exec_fee()
         }
     }
-    result
 }
 
 pub fn transfer_send_fee(
@@ -254,5 +253,4 @@ pub fn transfer_send_fee(
                 + cfg.fee(ActionCosts::add_full_access_key).send_fee(sender_is_receiver)
         }
     }
-    result
 }

core/primitives/src/errors.rs

@@ -142,6 +142,8 @@ pub enum InvalidTxError {
     SignerDoesNotExist { signer_id: AccountId },
     /// Transaction nonce must be `account[access_key].nonce + 1`.
     InvalidNonce { tx_nonce: Nonce, ak_nonce: Nonce },
+    /// Transaction nonce is less than the lower bound given by the block height.
+    InvalidNonceCreateEthImplicitAccount { tx_nonce: Nonce, lower_bound: Nonce },
     /// Transaction nonce is larger than the upper bound given by the block height
     NonceTooLarge { tx_nonce: Nonce, upper_bound: Nonce },
     /// TX receiver_id is not a valid AccountId
@@ -568,6 +570,13 @@ impl Display for InvalidTxError {
             InvalidTxError::TransactionSizeExceeded { size, limit } => {
                 write!(f, "Size of serialized transaction {} exceeded the limit {}", size, limit)
             }
+            InvalidTxError::InvalidNonceCreateEthImplicitAccount { tx_nonce, lower_bound } => {
+                write!(
+                    f,
+                    "Transaction nonce {} must be at least the access key nonce lower bound {}",
+                    tx_nonce, lower_bound
+                )
+            }
         }
     }
 }

core/primitives/src/utils.rs

@@ -469,7 +469,7 @@ where
     Serializable(object)
 }
 
-/// Derives `AccountId` from `PublicKey``.
+/// Derives `AccountId` from `PublicKey`.
 /// If the key type is ED25519, returns hex-encoded copy of the key.
 /// 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 {

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

@@ -12,7 +12,7 @@ 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;
@@ -151,7 +151,7 @@ fn get_status_of_tx_hash_collision_for_implicit_account(
     // Delete implicit account.
     let delete_account_tx = SignedTransaction::delete_account(
         // Because AccessKeyNonceRange is enabled, correctness of this nonce is guaranteed.
-        (height - 1) * near_primitives::account::AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER,
+        (height - 1) * AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER,
         implicit_account_id.clone(),
         implicit_account_id.clone(),
         "test0".parse().unwrap(),
@@ -198,24 +198,206 @@ fn get_status_of_tx_hash_collision_for_implicit_account(
     response
 }
 
-/// Test that duplicate transactions from NEAR-implicit accounts are properly rejected.
+/// 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_hash_collision_for_near_implicit_account_fail() {
-    let protocol_version = ProtocolFeature::AccessKeyNonceForImplicitAccounts.protocol_version();
-    let secret_key = SecretKey::from_seed(KeyType::ED25519, "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, secret_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 if the access key was not recreated in the meantime
+    // 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 { .. }));
+}
+
+// TODO(eth-implicit) finish
+/// Test that signer is correctly verifier 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 secret_key_2 = SecretKey::from_seed(KeyType::SECP256K1, "test2");
+    let implicit_account_id = derive_account_id_from_public_key(&secret_key.public_key());
+    let implicit_account_id_2 = derive_account_id_from_public_key(&secret_key_2.public_key());
+    let _implicit_account_signer =
+        InMemorySigner::from_secret_key(implicit_account_id.clone(), secret_key);
+    let implicit_account_signer_2 =
+        InMemorySigner::from_secret_key(implicit_account_id_2, secret_key_2);
+
+    // 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();
+
+    // Sending money from ETH-implicit account using.
+    // That will check if signer's account ID is derived from the provided public key.
+    let send_money_from_implicit_account_tx = SignedTransaction::send_money(
+        (height - 1) * AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER,
+        implicit_account_id,
+        "test0".parse().unwrap(),
+        &implicit_account_signer_2,
+        100,
+        *block.hash(),
+    );
+    let response = env.clients[0].process_tx(send_money_from_implicit_account_tx, false, false);
     assert_matches!(
-        get_status_of_tx_hash_collision_for_implicit_account(protocol_version, implicit_account_signer),
-        ProcessTxResponse::InvalidTx(InvalidTxError::InvalidNonce { .. })
+        response,
+        ProcessTxResponse::InvalidTx(InvalidTxError::InvalidAccessKeyError(
+            InvalidAccessKeyError::InvalidPkForEthAddress { .. }
+        ))
     );
 }
 
-/// Test that duplicate transactions from ETH-implicit accounts are properly rejected.
+/// Test that duplicate transactions from NEAR-implicit accounts are properly rejected.
 #[test]
-fn test_transaction_hash_collision_for_eth_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::SECP256K1, "test");
+    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!(
@@ -244,18 +426,26 @@ fn test_transaction_hash_collision_for_near_implicit_account_ok() {
     );
 }
 
-/// Test that duplicate transactions from ETH-implicit accounts are not rejected until protocol upgrade.
+/// 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;
-    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
-    );
+    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.