fix: set access key nonce on implicit account creation

chain/chain/Cargo.toml

@@ -43,9 +43,9 @@ expensive_tests = []
 test_features = []
 delay_detector = ["delay-detector"]
 no_cache = ["near-store/no_cache"]
-protocol_feature_chunk_only_producers = ["near-primitives/protocol_feature_chunk_only_producers"]
+protocol_feature_chunk_only_producers = ["near-chain-configs/protocol_feature_chunk_only_producers", "near-primitives/protocol_feature_chunk_only_producers"]
 
 protocol_feature_routing_exchange_algorithm = []
 nightly_protocol_features = ["nightly_protocol", "protocol_feature_chunk_only_producers", "protocol_feature_routing_exchange_algorithm"]
-nightly_protocol = []
+nightly_protocol = ["near-store/nightly_protocol", "near-primitives/nightly_protocol"]
 sandbox = []

chain/chain/src/tests/simple_chain.rs

@@ -23,7 +23,7 @@ fn empty_chain() {
     assert_eq!(chain.head().unwrap().height, 0);
     let hash = chain.head().unwrap().last_block_hash;
     #[cfg(feature = "nightly_protocol")]
-    assert_eq!(hash, CryptoHash::from_str("DHFJfDH25yEPmEpKrUN9upXY3gFcCmsGyHMTk2sugyXd").unwrap());
+    assert_eq!(hash, CryptoHash::from_str("3wFoFbJPPv7Jg1ZERuBWTCok4H9uRZ7Uagm1wiXdKpvV").unwrap());
     #[cfg(not(feature = "nightly_protocol"))]
     assert_eq!(hash, CryptoHash::from_str("BkrZeGCDnYJGAdc3G1fb1P8FMbnNDhBn6w5DjhQeNAdp").unwrap());
     assert_eq!(count_utc, 1);
@@ -44,7 +44,7 @@ fn build_chain() {
     #[cfg(feature = "nightly_protocol")]
     assert_eq!(
         prev_hash,
-        CryptoHash::from_str("FrAMjjHhjJAB1N6BiDVpRfAMsjLHbTYZnkYKUs39JgEx").unwrap()
+        CryptoHash::from_str("5VroU43sRYCkttuJfpdP6iC57fsD4q2TCbWaronBWpz7").unwrap()
     );
     #[cfg(not(feature = "nightly_protocol"))]
     assert_eq!(
@@ -67,7 +67,7 @@ fn build_chain() {
     #[cfg(feature = "nightly_protocol")]
     assert_eq!(
         chain.head().unwrap().last_block_hash,
-        CryptoHash::from_str("2zngdBZU9YEZKvtfpkZb3vjjxf7j7szMdo5jtPLDcg5Z").unwrap()
+        CryptoHash::from_str("BNap11nsM7PEqYQertYU487g7gkCteQ8Fmee6QfyRdVQ").unwrap()
     );
     #[cfg(not(feature = "nightly_protocol"))]
     assert_eq!(

core/primitives-core/src/account.rs

@@ -155,10 +155,9 @@ impl BorshSerialize for Account {
     BorshSerialize, BorshDeserialize, Serialize, Deserialize, PartialEq, Eq, Hash, Clone, Debug,
 )]
 pub struct AccessKey {
-    /// The nonce for this access key.
-    /// NOTE: In some cases the access key needs to be recreated. If the new access key reuses the
-    /// same public key, the nonce of the new access key should be equal to the nonce of the old
-    /// access key. It's required to avoid replaying old transactions again.
+    /// Nonce for this access key, used for tx nonce generation. When access key is created, nonce
+    /// is set to `(block_height - 1) * 1e6` to avoid tx hash collision on access key re-creation.
+    /// See <https://github.com/near/nearcore/issues/3779> for more details.
     pub nonce: Nonce,
 
     /// Defines permissions for this access key.

core/primitives/Cargo.toml

@@ -41,7 +41,8 @@ dump_errors_schema = ["near-rpc-error-macro/dump_errors_schema"]
 protocol_feature_alt_bn128 = ["near-primitives-core/protocol_feature_alt_bn128", "near-vm-errors/protocol_feature_alt_bn128"]
 protocol_feature_chunk_only_producers = []
 protocol_feature_routing_exchange_algorithm = ["near-primitives-core/protocol_feature_routing_exchange_algorithm"]
-nightly_protocol_features = ["nightly_protocol", "protocol_feature_alt_bn128", "protocol_feature_chunk_only_producers", "protocol_feature_routing_exchange_algorithm"]
+protocol_feature_access_key_nonce_for_implicit_accounts = []
+nightly_protocol_features = ["nightly_protocol", "protocol_feature_alt_bn128", "protocol_feature_chunk_only_producers", "protocol_feature_routing_exchange_algorithm", "protocol_feature_access_key_nonce_for_implicit_accounts"]
 nightly_protocol = []
 deepsize_feature = ["deepsize", "near-vm-errors/deepsize_feature", "near-primitives-core/deepsize_feature", "near-crypto/deepsize_feature"]
 

core/primitives/src/version.rs

@@ -86,6 +86,8 @@ pub enum ProtocolFeature {
     // stable features
     ForwardChunkParts,
     RectifyInflation,
+    /// Add `AccessKey` nonce range by setting nonce to `(block_height - 1) * 1e6`, see
+    /// <https://github.com/near/nearcore/issues/3779>.
     AccessKeyNonceRange,
     FixApplyChunks,
     LowerStorageCost,
@@ -136,6 +138,9 @@ pub enum ProtocolFeature {
     ChunkOnlyProducers,
     #[cfg(feature = "protocol_feature_routing_exchange_algorithm")]
     RoutingExchangeAlgorithm,
+    #[cfg(feature = "protocol_feature_access_key_nonce_for_implicit_accounts")]
+    /// Add `AccessKey` nonce range for implicit accounts, as in `AccessKeyNonceRange` feature.
+    AccessKeyNonceForImplicitAccounts,
 }
 
 /// Current latest stable version of the protocol.
@@ -146,7 +151,7 @@ pub const PROTOCOL_VERSION: ProtocolVersion = 49;
 
 /// Current latest nightly version of the protocol.
 #[cfg(feature = "nightly_protocol")]
-pub const PROTOCOL_VERSION: ProtocolVersion = 124;
+pub const PROTOCOL_VERSION: ProtocolVersion = 125;
 
 impl ProtocolFeature {
     pub const fn protocol_version(self) -> ProtocolVersion {
@@ -181,6 +186,8 @@ impl ProtocolFeature {
             ProtocolFeature::ChunkOnlyProducers => 124,
             #[cfg(feature = "protocol_feature_routing_exchange_algorithm")]
             ProtocolFeature::RoutingExchangeAlgorithm => 117,
+            #[cfg(feature = "protocol_feature_access_key_nonce_for_implicit_accounts")]
+            ProtocolFeature::AccessKeyNonceForImplicitAccounts => 125,
         }
     }
 }

integration-tests/Cargo.toml

@@ -63,7 +63,8 @@ protocol_feature_alt_bn128 = [
     "near-vm-errors/protocol_feature_alt_bn128",
 ]
 protocol_feature_chunk_only_producers = ["near-client/protocol_feature_chunk_only_producers", "near-primitives/protocol_feature_chunk_only_producers"]
-nightly_protocol_features = ["nearcore/nightly_protocol_features", "protocol_feature_alt_bn128", "protocol_feature_chunk_only_producers"]
+protocol_feature_access_key_nonce_for_implicit_accounts = ["near-primitives/protocol_feature_access_key_nonce_for_implicit_accounts", "node-runtime/protocol_feature_access_key_nonce_for_implicit_accounts"]
+nightly_protocol_features = ["nearcore/nightly_protocol_features", "protocol_feature_alt_bn128", "protocol_feature_chunk_only_producers", "protocol_feature_access_key_nonce_for_implicit_accounts"]
 nightly_protocol = ["nearcore/nightly_protocol"]
 sandbox = ["near-network/sandbox", "near-chain/sandbox", "node-runtime/sandbox", "near-client/sandbox"]
 no_cache = ["nearcore/no_cache"]

integration-tests/tests/client/process_blocks.rs

@@ -95,13 +95,14 @@ pub fn create_nightshade_runtimes(genesis: &Genesis, n: usize) -> Vec<Arc<dyn Ru
         .collect()
 }
 
-fn produce_epochs(
+/// Produce `blocks_number` block in the given environment, starting from the given height.
+/// Returns the first unoccupied height in the chain after this operation.
+fn produce_blocks_from_height(
     env: &mut TestEnv,
-    epoch_number: u64,
-    epoch_length: u64,
+    blocks_number: u64,
     height: BlockHeight,
 ) -> BlockHeight {
-    let next_height = height + epoch_number * epoch_length;
+    let next_height = height + blocks_number;
     for i in height..next_height {
         let block = env.clients[0].produce_block(i).unwrap().unwrap();
         env.process_block(0, block.clone(), Provenance::PRODUCED);
@@ -112,6 +113,22 @@ fn produce_epochs(
     next_height
 }
 
+/// Try to process tx in the next blocks, check that tx and all generated receipts succeed.
+/// Return height of the next block.
+fn check_tx_processing(
+    env: &mut TestEnv,
+    tx: SignedTransaction,
+    height: BlockHeight,
+    blocks_number: u64,
+) -> BlockHeight {
+    let tx_hash = tx.get_hash().clone();
+    env.clients[0].process_tx(tx, false, false);
+    let next_height = produce_blocks_from_height(env, blocks_number, height);
+    let final_outcome = env.clients[0].chain.get_final_transaction_result(&tx_hash).unwrap();
+    assert!(matches!(final_outcome.status, FinalExecutionStatus::SuccessValue(_)));
+    next_height
+}
+
 fn deploy_test_contract(
     env: &mut TestEnv,
     account_id: AccountId,
@@ -132,7 +149,7 @@ fn deploy_test_contract(
         *block.hash(),
     );
     env.clients[0].process_tx(tx, false, false);
-    produce_epochs(env, 1, epoch_length, height)
+    produce_blocks_from_height(env, epoch_length, height)
 }
 
 /// Create environment and set of transactions which cause congestion on the chain.
@@ -3331,6 +3348,125 @@ mod access_key_nonce_range_tests {
         assert!(matches!(res, NetworkClientResponses::InvalidTx(_)));
     }
 
+    /// 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_implicit_account(
+        protocol_version: ProtocolVersion,
+    ) -> NetworkClientResponses {
+        let epoch_length = 100;
+        let mut genesis =
+            Genesis::test(vec!["test0".parse().unwrap(), "test1".parse().unwrap()], 1);
+        genesis.config.epoch_length = epoch_length;
+        genesis.config.protocol_version = protocol_version;
+        let mut env = TestEnv::builder(ChainGenesis::test())
+            .runtime_adapters(create_nightshade_runtimes(&genesis, 1))
+            .build();
+        let genesis_block = env.clients[0].chain.get_block_by_height(0).unwrap().clone();
+
+        let signer1 =
+            InMemorySigner::from_seed("test1".parse().unwrap(), KeyType::ED25519, "test1");
+
+        let public_key = signer1.public_key.clone();
+        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;
+
+        // Send money to 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.clone(),
+            *genesis_block.hash(),
+        );
+        height = check_tx_processing(&mut env, send_money_tx, height, blocks_number.clone());
+        let block = env.clients[0].chain.get_block_by_height(height - 1).unwrap().clone();
+
+        // 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,
+            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.clone());
+        let block = env.clients[0].chain.get_block_by_height(height - 1).unwrap().clone();
+
+        // Send money to implicit account again, invoking its second creation.
+        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().clone();
+
+        // Send money from implicit account with incorrect nonce.
+        let send_money_from_implicit_account_tx = SignedTransaction::send_money(
+            1,
+            implicit_account_id.clone(),
+            "test0".parse().unwrap(),
+            &implicit_account_signer,
+            100,
+            *block.hash(),
+        );
+        let status = env.clients[0].process_tx(send_money_from_implicit_account_tx, false, false);
+
+        // Check that sending money from implicit account with correct nonce is still valid.
+        let send_money_from_implicit_account_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(),
+        );
+        check_tx_processing(&mut env, send_money_from_implicit_account_tx, height, blocks_number);
+
+        status
+    }
+
+    /// Test that duplicate transactions from implicit accounts are properly rejected.
+    #[cfg(feature = "protocol_feature_access_key_nonce_for_implicit_accounts")]
+    #[test]
+    fn test_transaction_hash_collision_for_implicit_account_fail() {
+        let protocol_version =
+            ProtocolFeature::AccessKeyNonceForImplicitAccounts.protocol_version();
+        assert!(matches!(
+            get_status_of_tx_hash_collision_for_implicit_account(protocol_version),
+            NetworkClientResponses::InvalidTx(InvalidTxError::InvalidNonce { .. })
+        ));
+    }
+
+    /// Test that duplicate transactions from implicit accounts are not rejected until protocol upgrade.
+    #[test]
+    fn test_transaction_hash_collision_for_implicit_account_ok() {
+        #[cfg(feature = "protocol_feature_access_key_nonce_for_implicit_accounts")]
+        let protocol_version =
+            ProtocolFeature::AccessKeyNonceForImplicitAccounts.protocol_version() - 1;
+        #[cfg(not(feature = "protocol_feature_access_key_nonce_for_implicit_accounts"))]
+        let protocol_version = PROTOCOL_VERSION;
+        assert!(matches!(
+            get_status_of_tx_hash_collision_for_implicit_account(protocol_version),
+            NetworkClientResponses::ValidTx
+        ));
+    }
+
     /// Test that chunks with transactions that have expired are considered invalid.
     #[test]
     fn test_chunk_transaction_validity() {
@@ -3833,7 +3969,7 @@ mod contract_precompilation_tests {
         );
 
         // Wait 3 epochs.
-        height = produce_epochs(&mut env, 3, EPOCH_LENGTH, height);
+        height = produce_blocks_from_height(&mut env, 3 * EPOCH_LENGTH, height);
 
         // Process test contract deployment on the first client.
         let wasm_code = near_test_contracts::rs_contract().to_vec();
@@ -3926,7 +4062,7 @@ mod contract_precompilation_tests {
             *block.hash(),
         );
         env.clients[0].process_tx(delete_account_tx, false, false);
-        height = produce_epochs(&mut env, 1, EPOCH_LENGTH, height);
+        height = produce_blocks_from_height(&mut env, EPOCH_LENGTH, height);
 
         // Perform state sync for the second client.
         state_sync_on_height(&mut env, height - 1);

nearcore/Cargo.toml

@@ -75,7 +75,8 @@ json_rpc = ["near-jsonrpc"]
 protocol_feature_alt_bn128 = ["near-primitives/protocol_feature_alt_bn128", "node-runtime/protocol_feature_alt_bn128"]
 protocol_feature_chunk_only_producers = ["near-chain-configs/protocol_feature_chunk_only_producers", "near-epoch-manager/protocol_feature_chunk_only_producers", "near-chain/protocol_feature_chunk_only_producers", "near-client/protocol_feature_chunk_only_producers", "node-runtime/protocol_feature_chunk_only_producers", "near-rosetta-rpc/protocol_feature_chunk_only_producers", "near-primitives/protocol_feature_chunk_only_producers"]
 protocol_feature_routing_exchange_algorithm = ["near-primitives/protocol_feature_routing_exchange_algorithm", "near-chain/protocol_feature_routing_exchange_algorithm", "near-network/protocol_feature_routing_exchange_algorithm", "near-client/protocol_feature_routing_exchange_algorithm", "near-jsonrpc/protocol_feature_routing_exchange_algorithm"]
-nightly_protocol_features = ["nightly_protocol", "near-primitives/nightly_protocol_features", "near-client/nightly_protocol_features", "near-epoch-manager/nightly_protocol_features", "near-store/nightly_protocol_features", "protocol_feature_alt_bn128", "protocol_feature_chunk_only_producers", "protocol_feature_routing_exchange_algorithm"]
+protocol_feature_access_key_nonce_for_implicit_accounts = ["near-primitives/protocol_feature_access_key_nonce_for_implicit_accounts", "node-runtime/protocol_feature_access_key_nonce_for_implicit_accounts"]
+nightly_protocol_features = ["nightly_protocol", "near-primitives/nightly_protocol_features", "near-client/nightly_protocol_features", "near-epoch-manager/nightly_protocol_features", "near-store/nightly_protocol_features", "protocol_feature_alt_bn128", "protocol_feature_chunk_only_producers", "protocol_feature_routing_exchange_algorithm", "protocol_feature_access_key_nonce_for_implicit_accounts"]
 nightly_protocol = ["near-primitives/nightly_protocol", "near-jsonrpc/nightly_protocol"]
 
 # Force usage of a specific wasm vm irrespective of protocol version.

runtime/runtime/Cargo.toml

@@ -36,7 +36,7 @@ near-vm-errors = { path = "../../runtime/near-vm-errors" }
 default = []
 dump_errors_schema = ["near-vm-errors/dump_errors_schema"]
 protocol_feature_chunk_only_producers = ["near-primitives/protocol_feature_chunk_only_producers", "near-store/protocol_feature_chunk_only_producers", "near-chain-configs/protocol_feature_chunk_only_producers"]
-
+protocol_feature_access_key_nonce_for_implicit_accounts = ["near-primitives/protocol_feature_access_key_nonce_for_implicit_accounts"]
 no_cpu_compatibility_checks = ["near-vm-runner/no_cpu_compatibility_checks"]
 
 no_cache = ["near-vm-runner/no_cache", "near-store/no_cache"]

runtime/runtime/src/actions.rs

@@ -16,7 +16,7 @@ use near_primitives::transaction::{
     FunctionCallAction, StakeAction, TransferAction,
 };
 use near_primitives::types::validator_stake::ValidatorStake;
-use near_primitives::types::{AccountId, EpochInfoProvider};
+use near_primitives::types::{AccountId, BlockHeight, EpochInfoProvider};
 use near_primitives::utils::create_random_seed;
 use near_primitives::version::{
     is_implicit_account_creation_enabled, ProtocolFeature, ProtocolVersion,
@@ -392,13 +392,26 @@ pub(crate) fn action_implicit_account_creation_transfer(
     actor_id: &mut AccountId,
     account_id: &AccountId,
     transfer: &TransferAction,
+    block_height: BlockHeight,
+    current_protocol_version: ProtocolVersion,
 ) {
     // NOTE: The account_id is hex like, because we've checked the permissions before.
     debug_assert!(AccountId::is_implicit(account_id.as_ref()));
 
     *actor_id = account_id.clone();
 
-    let access_key = AccessKey::full_access();
+    let mut access_key = AccessKey::full_access();
+    // Set default nonce for newly created access key to avoid transaction hash collision.
+    // See <https://github.com/near/nearcore/issues/3779>.
+    if checked_feature!(
+        "protocol_feature_access_key_nonce_for_implicit_accounts",
+        AccessKeyNonceForImplicitAccounts,
+        current_protocol_version
+    ) {
+        access_key.nonce = (block_height - 1)
+            * near_primitives::account::AccessKey::ACCESS_KEY_NONCE_RANGE_MULTIPLIER;
+    }
+
     // 0 for ED25519
     let mut public_key_data = Vec::with_capacity(33);
     public_key_data.push(0u8);

runtime/runtime/src/lib.rs

@@ -399,6 +399,8 @@ impl Runtime {
                         actor_id,
                         &receipt.receiver_id,
                         transfer,
+                        apply_state.block_index,
+                        apply_state.current_protocol_version,
                     );
                 }
             }