diff --git a/frame/evm-balances/src/tests.rs b/frame/evm-balances/src/tests.rs index b23d54de07..e9f7370aae 100644 --- a/frame/evm-balances/src/tests.rs +++ b/frame/evm-balances/src/tests.rs @@ -8,6 +8,16 @@ use sp_std::str::FromStr; use crate::{mock::*, *}; +fn assert_total_issuance_invariant() { + let iterated_total_issuance: u64 = >::iter_values() + .map(|account_data| account_data.data.total()) + .sum(); + + let total_issuance = EvmBalances::total_issuance(); + + assert_eq!(iterated_total_issuance, total_issuance); +} + #[test] fn basic_setup_works() { new_test_ext().execute_with_ext(|_| { @@ -23,6 +33,8 @@ fn basic_setup_works() { // Check the total balance value. assert_eq!(EvmBalances::total_issuance(), 2 * INIT_BALANCE); + + assert_total_issuance_invariant(); }); } @@ -72,6 +84,8 @@ fn currency_deactivate_reactivate_works() { EvmBalances::reactivate(40); // Assert state changes. assert_eq!(>::get(), 60); + + assert_total_issuance_invariant(); }); } @@ -88,6 +102,8 @@ fn currency_burn_works() { assert_eq!(EvmBalances::total_issuance(), 2 * INIT_BALANCE - 100); drop(imbalance); assert_eq!(EvmBalances::total_issuance(), 2 * INIT_BALANCE); + + assert_total_issuance_invariant(); }); } @@ -104,6 +120,8 @@ fn currency_issue_works() { assert_eq!(EvmBalances::total_issuance(), 2 * INIT_BALANCE + 100); drop(imbalance); assert_eq!(EvmBalances::total_issuance(), 2 * INIT_BALANCE); + + assert_total_issuance_invariant(); }); } @@ -140,6 +158,8 @@ fn currency_transfer_works() { to: bob(), amount: transfered_amount, })); + + assert_total_issuance_invariant(); }); } @@ -166,6 +186,8 @@ fn currency_slash_works() { who: alice(), amount: slashed_amount, })); + + assert_total_issuance_invariant(); }); } @@ -181,10 +203,8 @@ fn currency_deposit_into_existing_works() { System::set_block_number(1); // Invoke the function under test. - assert_ok!(EvmBalances::deposit_into_existing( - &alice(), - deposited_amount - )); + let imbalance = EvmBalances::deposit_into_existing(&alice(), deposited_amount).unwrap(); + drop(imbalance); // Assert state changes. assert_eq!( @@ -195,6 +215,8 @@ fn currency_deposit_into_existing_works() { who: alice(), amount: deposited_amount, })); + + assert_total_issuance_invariant(); }); } @@ -222,6 +244,8 @@ fn currency_deposit_creating_works() { System::assert_has_event(RuntimeEvent::EvmSystem( pallet_evm_system::Event::NewAccount { account: charlie }, )); + + assert_total_issuance_invariant(); }); } @@ -237,12 +261,14 @@ fn currency_withdraw_works() { System::set_block_number(1); // Invoke the function under test. - assert_ok!(EvmBalances::withdraw( + let imbalance = EvmBalances::withdraw( &alice(), 1000, WithdrawReasons::FEE, - ExistenceRequirement::KeepAlive - )); + ExistenceRequirement::KeepAlive, + ) + .unwrap(); + drop(imbalance); // Assert state changes. assert_eq!( @@ -253,6 +279,8 @@ fn currency_withdraw_works() { who: alice(), amount: withdrawed_amount, })); + + assert_total_issuance_invariant(); }); } @@ -271,6 +299,8 @@ fn currency_make_free_balance_be_works() { // Assert state changes. assert_eq!(EvmBalances::total_balance(&charlie), made_free_balance); + + assert_total_issuance_invariant(); }); } @@ -297,6 +327,8 @@ fn evm_system_account_should_be_reaped() { System::assert_has_event(RuntimeEvent::EvmSystem( pallet_evm_system::Event::KilledAccount { account: bob() }, )); + + assert_total_issuance_invariant(); }); } @@ -317,6 +349,34 @@ fn evm_balances_transferring_too_high_value_should_not_panic() { }); } +#[test] +fn evm_system_removing_account_non_zero_balance() { + new_test_ext().execute_with_ext(|_| { + // Prepare test preconditions. + let contract = H160::from_str("1000000000000000000000000000000000000003").unwrap(); + EVM::create_account(contract, vec![1, 2, 3]); + + // Transfer some balance to contract address. + assert_ok!(EvmBalances::transfer( + &alice(), + &contract, + 1000, + ExistenceRequirement::KeepAlive + )); + + assert_eq!(EvmBalances::free_balance(&contract), 1000); + + // Invoke the function under test. + EVM::remove_account(&contract); + + // Assert state changes. + assert_eq!(EvmBalances::free_balance(&contract), 1000); + assert!(EvmSystem::account_exists(&contract)); + + assert_total_issuance_invariant(); + }); +} + #[test] fn evm_fee_deduction() { new_test_ext().execute_with_ext(|_| { @@ -338,6 +398,8 @@ fn evm_fee_deduction() { // Refund fees as 5 units <::OnChargeTransaction as pallet_evm::OnChargeEVMTransaction>::correct_and_deposit_fee(&charlie, U256::from(5), U256::from(5), imbalance); assert_eq!(EvmBalances::free_balance(&charlie), 95); + + assert_total_issuance_invariant(); }); } @@ -374,6 +436,8 @@ fn evm_issuance_after_tip() { let after_tip = ::Currency::total_issuance(); assert_eq!(after_tip, (before_tip - (base_fee * 21_000))); + + assert_total_issuance_invariant(); }); } @@ -410,6 +474,8 @@ fn evm_refunds_should_work() { let total_cost = (U256::from(21_000) * base_fee) + U256::from(1); let after_call = EVM::account_basic(&alice()).0.balance; assert_eq!(after_call, before_call - total_cost); + + assert_total_issuance_invariant(); }); } @@ -451,6 +517,8 @@ fn evm_refunds_and_priority_should_work() { let after_call = EVM::account_basic(&alice()).0.balance; // The tip is deducted but never refunded to the caller. assert_eq!(after_call, before_call - total_cost); + + assert_total_issuance_invariant(); }); } @@ -482,6 +550,8 @@ fn evm_call_should_fail_with_priority_greater_than_max_fee() { assert!(result.is_err()); // Some used weight is returned as part of the error. assert_eq!(result.unwrap_err().weight, Weight::from_parts(7, 0)); + + assert_total_issuance_invariant(); }); } @@ -512,5 +582,7 @@ fn evm_call_should_succeed_with_priority_equal_to_max_fee() { ::config(), ); assert!(result.is_ok()); + + assert_total_issuance_invariant(); }); } diff --git a/frame/evm-system/src/lib.rs b/frame/evm-system/src/lib.rs index fe25d3aed3..fe2c422ca4 100644 --- a/frame/evm-system/src/lib.rs +++ b/frame/evm-system/src/lib.rs @@ -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; @@ -115,6 +115,26 @@ pub mod pallet { } } +/// The outcome of the account creation operation. +#[derive(Eq, PartialEq, RuntimeDebug)] +pub enum AccountCreationOutcome { + /// Account was created. + Created, + /// Account already exists in the system, so action was taken. + AlreadyExists, +} + +/// The outcome of the account removal operation. +#[derive(Eq, PartialEq, RuntimeDebug)] +pub enum AccountRemovalOutcome { + /// Account was destroyed and no longer exists. + Reaped, + /// Account was non-empty, and it was retained and still exists in the system. + Retained, + /// Account did not exist in the first place, so no action was taken. + DidNotExist, +} + impl Pallet { /// Check the account existence. pub fn account_exists(who: &::AccountId) -> bool { @@ -144,25 +164,29 @@ impl Pallet { } /// Create an account. - pub fn create_account(who: &::AccountId) -> DispatchResult { + pub fn create_account(who: &::AccountId) -> AccountCreationOutcome { if Self::account_exists(who) { - return Err(Error::::AccountAlreadyExist.into()); + return AccountCreationOutcome::AlreadyExists; } Account::::insert(who.clone(), AccountInfo::<_, _>::default()); Self::on_created_account(who.clone()); - Ok(()) + AccountCreationOutcome::Created } /// Remove an account. - pub fn remove_account(who: &::AccountId) -> DispatchResult { + pub fn remove_account(who: &::AccountId) -> AccountRemovalOutcome { if !Self::account_exists(who) { - return Err(Error::::AccountNotExist.into()); + return AccountRemovalOutcome::DidNotExist; + } + + if Account::::get(who).data != ::AccountData::default() { + return AccountRemovalOutcome::Retained; } Account::::remove(who); Self::on_killed_account(who.clone()); - Ok(()) + AccountRemovalOutcome::Reaped } } diff --git a/frame/evm-system/src/tests.rs b/frame/evm-system/src/tests.rs index b175d5e6c0..a7e747661a 100644 --- a/frame/evm-system/src/tests.rs +++ b/frame/evm-system/src/tests.rs @@ -2,7 +2,7 @@ use sp_std::str::FromStr; -use frame_support::{assert_noop, assert_ok}; +use frame_support::{assert_noop, assert_storage_noop}; use mockall::predicate; use sp_core::H160; @@ -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), + AccountCreationOutcome::Created + ); // Assert state changes. assert!(EvmSystem::account_exists(&account_id)); @@ -52,10 +55,10 @@ fn create_account_fails() { >::insert(account_id.clone(), AccountInfo::<_, _>::default()); // Invoke the function under test. - assert_noop!( + assert_storage_noop!(assert_eq!( EvmSystem::create_account(&account_id), - Error::::AccountAlreadyExist - ); + AccountCreationOutcome::AlreadyExists + )); }); } @@ -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), + AccountRemovalOutcome::Reaped + ); // Assert state changes. assert!(!EvmSystem::account_exists(&account_id)); @@ -100,10 +106,10 @@ fn remove_account_fails() { let account_id = H160::from_str("1000000000000000000000000000000000000001").unwrap(); // Invoke the function under test. - assert_noop!( + assert_storage_noop!(assert_eq!( EvmSystem::remove_account(&account_id), - Error::::AccountNotExist - ); + AccountRemovalOutcome::DidNotExist + )); }); }