From 75c2c7f5b6563c6b9e2f0b2ff62f09b49412c718 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Fri, 31 Jan 2025 15:14:34 +0300 Subject: [PATCH 1/6] Properly handle new account creation related to inc_account_nonce --- frame/evm-system/src/lib.rs | 6 +++--- frame/evm-system/src/tests.rs | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 497f1c1e86..45417e198d 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -160,13 +160,13 @@ impl Pallet { /// Increment a particular account's nonce by 1. pub fn inc_account_nonce(who: &::AccountId) { + let is_account_existed = Self::account_exists(who); + Account::::mutate(who, |a| { a.nonce += ::Index::one(); // Meaning that account is being created. - if a.nonce == ::Index::one() - && a.data == ::AccountData::default() - { + if !is_account_existed { Self::on_created_account(who.clone()); } }); diff --git a/frame/evm-system/src/tests.rs b/frame/evm-system/src/tests.rs index 2a797e966b..34e18378a9 100644 --- a/frame/evm-system/src/tests.rs +++ b/frame/evm-system/src/tests.rs @@ -113,14 +113,17 @@ fn remove_account_fails() { }); } -/// This test verifies that incrementing account nonce works in the happy path. +/// This test verifies that incrementing account nonce works in the happy path +/// in case a new account should be created. #[test] -fn inc_account_nonce_works() { +fn inc_account_nonce_account_created() { new_test_ext().execute_with_ext(|_| { // Prepare test data. let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); // Check test preconditions. + assert!(!EvmSystem::account_exists(&account_id)); + let nonce_before = EvmSystem::account_nonce(&account_id); // Set block number to enable events. @@ -139,6 +142,7 @@ fn inc_account_nonce_works() { // Assert state changes. assert_eq!(EvmSystem::account_nonce(&account_id), nonce_before + 1); + assert!(EvmSystem::account_exists(&account_id)); System::assert_has_event(RuntimeEvent::EvmSystem(Event::NewAccount { account: account_id, })); @@ -147,12 +151,36 @@ fn inc_account_nonce_works() { EvmSystem::inc_account_nonce(&account_id); // Assert state changes. assert_eq!(EvmSystem::account_nonce(&account_id), nonce_before + 2); + assert!(EvmSystem::account_exists(&account_id)); // Assert mock invocations. on_new_account_ctx.checkpoint(); }); } +/// This test verifies that incrementing account nonce works in the happy path +/// in case an account already exists. +#[test] +fn inc_account_nonce_account_exists() { + new_test_ext().execute_with_ext(|_| { + // Prepare test data. + let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + >::insert(account_id.clone(), AccountInfo::<_, _>::default()); + + // Check test preconditions. + assert!(EvmSystem::account_exists(&account_id)); + + let nonce_before = EvmSystem::account_nonce(&account_id); + + // Invoke the function under test. + EvmSystem::inc_account_nonce(&account_id); + + // Assert state changes. + assert!(EvmSystem::account_exists(&account_id)); + assert_eq!(EvmSystem::account_nonce(&account_id), nonce_before + 1); + }); +} + /// This test verifies that try_mutate_exists works as expected in case data wasn't providing /// and returned data is `Some`. As a result, a new account has been created. #[test] From b3d4a50b598f6a3b19d8086bdab3a7c2cac2283f Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Fri, 31 Jan 2025 15:21:16 +0300 Subject: [PATCH 2/6] Move outside mutate --- frame/evm-system/src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 45417e198d..792b1c8145 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -160,16 +160,16 @@ impl Pallet { /// Increment a particular account's nonce by 1. pub fn inc_account_nonce(who: &::AccountId) { - let is_account_existed = Self::account_exists(who); + let was_existed = Self::account_exists(who); Account::::mutate(who, |a| { a.nonce += ::Index::one(); - - // Meaning that account is being created. - if !is_account_existed { - Self::on_created_account(who.clone()); - } }); + + // Meaning that account is being created. + if !was_existed { + Self::on_created_account(who.clone()); + } } /// Create an account. From 4366fb23597608d1b97d1c3ebc1a7819716fd71f Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Mon, 3 Feb 2025 00:26:18 +0300 Subject: [PATCH 3/6] Do the check inside mutate --- frame/evm-system/src/lib.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 792b1c8145..ac4edbfd72 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -160,16 +160,14 @@ impl Pallet { /// Increment a particular account's nonce by 1. pub fn inc_account_nonce(who: &::AccountId) { - let was_existed = Self::account_exists(who); - Account::::mutate(who, |a| { a.nonce += ::Index::one(); - }); - // Meaning that account is being created. - if !was_existed { - Self::on_created_account(who.clone()); - } + // Meaning that account is being created. + if !Self::account_exists(who) { + Self::on_created_account(who.clone()); + } + }); } /// Create an account. From 1235fb2abccb62b4acdc9dca62bea5831b731129 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Mon, 3 Feb 2025 01:16:33 +0300 Subject: [PATCH 4/6] Use mutate_exists --- frame/evm-system/src/lib.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index ac4edbfd72..3a1557c8a5 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -160,14 +160,21 @@ impl Pallet { /// Increment a particular account's nonce by 1. pub fn inc_account_nonce(who: &::AccountId) { - Account::::mutate(who, |a| { - a.nonce += ::Index::one(); + let is_new_account = Account::::mutate_exists(who, |maybe_account| { + let is_new_account = maybe_account.is_none(); - // Meaning that account is being created. - if !Self::account_exists(who) { - Self::on_created_account(who.clone()); - } + let mut account = maybe_account.take().unwrap_or_default(); + account.nonce += ::Index::one(); + + *maybe_account = Some(account); + + is_new_account }); + + // Meaning that account is being created. + if is_new_account { + Self::on_created_account(who.clone()); + } } /// Create an account. From 3fec4dc5fa3db04de14c445153dd056e0f27e538 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov <39522748+dmitrylavrenov@users.noreply.github.com> Date: Mon, 3 Feb 2025 01:37:14 +0300 Subject: [PATCH 5/6] Use get_or_insert_default at mutate_exists Co-authored-by: MOZGIII --- frame/evm-system/src/lib.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index 3a1557c8a5..e80dcd862f 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -163,11 +163,9 @@ impl Pallet { let is_new_account = Account::::mutate_exists(who, |maybe_account| { let is_new_account = maybe_account.is_none(); - let mut account = maybe_account.take().unwrap_or_default(); + let mut account = maybe_account.get_or_insert_default(); account.nonce += ::Index::one(); - *maybe_account = Some(account); - is_new_account }); From 104bb7e4f87ca8c1712a3fc6f7d3c3259937457e Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Mon, 3 Feb 2025 02:05:55 +0300 Subject: [PATCH 6/6] Remove redundant mut --- frame/evm-system/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index e80dcd862f..d69eb10cbb 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -163,7 +163,7 @@ impl Pallet { let is_new_account = Account::::mutate_exists(who, |maybe_account| { let is_new_account = maybe_account.is_none(); - let mut account = maybe_account.get_or_insert_default(); + let account = maybe_account.get_or_insert_default(); account.nonce += ::Index::one(); is_new_account