Skip to content

Commit

Permalink
Redesign fix usage
Browse files Browse the repository at this point in the history
  • Loading branch information
dmitrylavrenov committed Mar 28, 2024
1 parent 4f5acfd commit 60bbc02
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 25 deletions.
46 changes: 31 additions & 15 deletions frame/evm-system/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use frame_support::traits::StoredMap;
use scale_codec::{Decode, Encode, FullCodec, MaxEncodedLen};
use scale_info::TypeInfo;
use sp_runtime::{traits::One, DispatchError, DispatchResult, RuntimeDebug};
use sp_runtime::{traits::One, DispatchError, RuntimeDebug};

#[cfg(test)]
mod mock;
Expand Down Expand Up @@ -115,6 +115,26 @@ pub mod pallet {
}
}

/// Some resultant status relevant to account creation.
#[derive(Eq, PartialEq, RuntimeDebug)]
pub enum AccountCreationStatus {
/// Account was created.
Created,
/// Account already existed.
Existed,
}

/// Some resultant status relevant to account removal.
#[derive(Eq, PartialEq, RuntimeDebug)]
pub enum AccountRemovalStatus {
/// Account was destroyed.
Reaped,
/// Account still exists.
Exists,
/// Account doesn't exist.
NotExists,
}

impl<T: Config> Pallet<T> {
/// Check the account existence.
pub fn account_exists(who: &<T as Config>::AccountId) -> bool {
Expand Down Expand Up @@ -144,30 +164,29 @@ impl<T: Config> Pallet<T> {
}

/// Create an account.
pub fn create_account(who: &<T as Config>::AccountId) -> DispatchResult {
pub fn create_account(who: &<T as Config>::AccountId) -> AccountCreationStatus {
if Self::account_exists(who) {
return Err(Error::<T>::AccountAlreadyExist.into());
return AccountCreationStatus::Existed;
}

Account::<T>::insert(who.clone(), AccountInfo::<_, _>::default());
Self::on_created_account(who.clone());
Ok(())
AccountCreationStatus::Created
}

/// Remove an account.
pub fn remove_account(who: &<T as Config>::AccountId) -> DispatchResult {
pub fn remove_account(who: &<T as Config>::AccountId) -> AccountRemovalStatus {
if !Self::account_exists(who) {
return Err(Error::<T>::AccountNotExist.into());
return AccountRemovalStatus::NotExists;
}

if Account::<T>::get(who).data != <T as Config>::AccountData::default() {
return Err(Error::<T>::AccountDataNotEmpty.into());
return AccountRemovalStatus::Exists;
}

Account::<T>::remove(who);
Self::on_killed_account(who.clone());

Ok(())
AccountRemovalStatus::Reaped
}
}

Expand All @@ -180,13 +199,10 @@ impl<T: Config> StoredMap<<T as Config>::AccountId, <T as Config>::AccountData>
k: &<T as Config>::AccountId,
f: impl FnOnce(&mut Option<<T as Config>::AccountData>) -> Result<R, E>,
) -> Result<R, E> {
let account = Account::<T>::get(k);
let was_providing = account.data != <T as Config>::AccountData::default();

let mut maybe_account_data = if was_providing {
Some(account.data)
let (mut maybe_account_data, was_providing) = if Self::account_exists(k) {
(Some(Account::<T>::get(k).data), true)
} else {
None
(None, false)
};

let result = f(&mut maybe_account_data)?;
Expand Down
24 changes: 14 additions & 10 deletions frame/evm-system/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use sp_std::str::FromStr;

use frame_support::{assert_noop, assert_ok};
use frame_support::assert_noop;
use mockall::predicate;
use sp_core::H160;

Expand Down Expand Up @@ -30,7 +30,10 @@ fn create_account_works() {
.return_const(());

// Invoke the function under test.
assert_ok!(EvmSystem::create_account(&account_id));
assert_eq!(
EvmSystem::create_account(&account_id),
AccountCreationStatus::Created
);

// Assert state changes.
assert!(EvmSystem::account_exists(&account_id));
Expand All @@ -52,9 +55,9 @@ fn create_account_fails() {
<Account<Test>>::insert(account_id.clone(), AccountInfo::<_, _>::default());

// Invoke the function under test.
assert_noop!(
assert_eq!(
EvmSystem::create_account(&account_id),
Error::<Test>::AccountAlreadyExist
AccountCreationStatus::Existed
);
});
}
Expand All @@ -79,7 +82,10 @@ fn remove_account_works() {
.return_const(());

// Invoke the function under test.
assert_ok!(EvmSystem::remove_account(&account_id));
assert_eq!(
EvmSystem::remove_account(&account_id),
AccountRemovalStatus::Reaped
);

// Assert state changes.
assert!(!EvmSystem::account_exists(&account_id));
Expand All @@ -100,9 +106,9 @@ fn remove_account_fails() {
let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap();

// Invoke the function under test.
assert_noop!(
assert_eq!(
EvmSystem::remove_account(&account_id),
Error::<Test>::AccountNotExist
AccountRemovalStatus::NotExists
);
});
}
Expand Down Expand Up @@ -205,9 +211,7 @@ fn try_mutate_exists_account_removed() {
new_test_ext().execute_with_ext(|_| {
// Prepare test data.
let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap();
let nonce = 1;
let data = 1;
<Account<Test>>::insert(account_id.clone(), AccountInfo { nonce, data });
<Account<Test>>::insert(account_id.clone(), AccountInfo { nonce: 1, data: 0 });

// Check test preconditions.
assert!(EvmSystem::account_exists(&account_id));
Expand Down

0 comments on commit 60bbc02

Please sign in to comment.