From 81fa3d27f352ec9ad802ebfca7e8b993e3a23fd8 Mon Sep 17 00:00:00 2001 From: brenzi Date: Wed, 23 Aug 2023 13:49:34 +0200 Subject: [PATCH] fix sufficients error (#345) * added currently failing unit test for sufficients, reproducing #333 * fix sufficients issue * clippy --- balances/src/lib.rs | 4 +++ balances/src/mock.rs | 2 ++ balances/src/tests.rs | 72 ++++++++++++++++++++++++++++++++++++++----- faucet/src/mock.rs | 17 +--------- test-utils/src/lib.rs | 23 ++++---------- 5 files changed, 78 insertions(+), 40 deletions(-) diff --git a/balances/src/lib.rs b/balances/src/lib.rs index 9ddd144f..63b1d109 100644 --- a/balances/src/lib.rs +++ b/balances/src/lib.rs @@ -322,6 +322,9 @@ impl Pallet { who: &T::AccountId, amount: BalanceType, ) -> DispatchResult { + if !Balance::::contains_key(community_id, who) { + Self::new_account(who)?; + } let mut entry_who = Self::balance_entry_updated(community_id, who); let mut entry_tot = Self::total_issuance_entry_updated(community_id); ensure!( @@ -332,6 +335,7 @@ impl Pallet { entry_tot.principal += amount; >::insert(community_id, entry_tot); >::insert(community_id, who, entry_who); + Self::deposit_event(Event::Issued(community_id, who.clone(), amount)); debug!(target: LOG, "issue {:?} for {:?}", amount, who); Ok(()) diff --git a/balances/src/mock.rs b/balances/src/mock.rs index bcd43e47..6d567072 100644 --- a/balances/src/mock.rs +++ b/balances/src/mock.rs @@ -34,6 +34,7 @@ frame_support::construct_runtime!( { System: frame_system::{Pallet, Call, Config, Storage, Event}, Timestamp: pallet_timestamp::{Pallet, Call, Storage, Inherent}, + Balances: pallet_balances::{Pallet, Call, Storage, Config, Event}, EncointerScheduler: encointer_scheduler::{Pallet, Call, Storage, Config, Event}, EncointerBalances: dut::{Pallet, Call, Storage, Event, Config}, } @@ -51,6 +52,7 @@ impl dut::Config for TestRuntime { impl_frame_system!(TestRuntime); impl_timestamp!(TestRuntime, EncointerScheduler); impl_encointer_scheduler!(TestRuntime); +impl_balances!(TestRuntime, System); // genesis values pub fn new_test_ext() -> sp_io::TestExternalities { diff --git a/balances/src/tests.rs b/balances/src/tests.rs index 1dee7002..7a433485 100644 --- a/balances/src/tests.rs +++ b/balances/src/tests.rs @@ -16,8 +16,8 @@ //! Unit tests for the encointer_balances module. -use super::*; -use crate::mock::DefaultDemurrage; +use super::{Balance as EncointerBalanceStorage, *}; +use crate::mock::{Balances, DefaultDemurrage}; use approx::{assert_abs_diff_eq, assert_relative_eq}; use encointer_primitives::{ communities::CommunityIdentifier, @@ -25,7 +25,7 @@ use encointer_primitives::{ }; use frame_support::{ assert_err, assert_noop, assert_ok, - traits::{tokens::fungibles::Unbalanced, OnInitialize}, + traits::{tokens::fungibles::Unbalanced, Currency, OnInitialize}, }; use mock::{master, new_test_ext, EncointerBalances, RuntimeOrigin, System, TestRuntime}; use sp_runtime::{app_crypto::Pair, testing::sr25519, AccountId32, DispatchError}; @@ -305,7 +305,7 @@ fn transfer_all_works() { System::set_block_number(3); - assert!(!Balance::::contains_key(cid, alice)); + assert!(!EncointerBalanceStorage::::contains_key(cid, alice)); let balance: f64 = EncointerBalances::balance(cid, &bob).lossy_into(); let demurrage_factor: f64 = @@ -341,7 +341,7 @@ fn remove_account_works() { BalanceType::from_num(50) )); EncointerBalances::remove_account(cid, &alice).ok(); - assert!(!Balance::::contains_key(cid, alice)); + assert!(!EncointerBalanceStorage::::contains_key(cid, alice)); }) } @@ -363,7 +363,7 @@ fn transfer_removes_account_if_source_below_existential_deposit() { BalanceType::from_num(20) )); - assert!(Balance::::contains_key(cid, alice.clone())); + assert!(EncointerBalanceStorage::::contains_key(cid, alice.clone())); let balance: f64 = EncointerBalances::balance(cid, &alice).lossy_into(); assert_eq!(balance, 30.0); @@ -373,7 +373,65 @@ fn transfer_removes_account_if_source_below_existential_deposit() { cid, BalanceType::from_num(30) )); - assert!(!Balance::::contains_key(cid, alice)); + assert!(!EncointerBalanceStorage::::contains_key(cid, alice)); + }) +} + +#[test] +fn transfer_all_native_wont_remove_account_with_remaining_community_balance() { + new_test_ext().execute_with(|| { + System::set_block_number(0); + System::on_initialize(System::block_number()); + + let alice = AccountKeyring::Alice.to_account_id(); + let bob = AccountKeyring::Bob.to_account_id(); + let charlie = AccountKeyring::Charlie.to_account_id(); + let cid = CommunityIdentifier::default(); + assert!(!frame_system::Account::::contains_key(&alice)); + assert!(!frame_system::Account::::contains_key(&bob)); + assert!(!frame_system::Account::::contains_key(&charlie)); + // issue native + assert_ok!(Balances::force_set_balance( + RuntimeOrigin::root(), + alice.clone(), + Balances::minimum_balance() * 100, + )); + assert!(frame_system::Account::::contains_key(&alice)); + assert_eq!(System::account(&alice).providers, 1); + // issue CC + assert_ok!(EncointerBalances::issue(cid, &alice, BalanceType::from_num(50))); + assert_eq!(System::account(&alice).sufficients, 1); + assert!(EncointerBalanceStorage::::contains_key(cid, &alice)); + + // create bob account by sending him some CC + assert_ok!(EncointerBalances::transfer( + Some(alice.clone()).into(), + bob.clone(), + cid, + BalanceType::from_num(20) + )); + assert!(frame_system::Account::::contains_key(&bob)); + assert!(EncointerBalanceStorage::::contains_key(cid, bob.clone())); + assert_eq!(System::account(&bob).sufficients, 1); + + // reap Alice native but keep CC, so Alice should stay alive + assert_ok!(Balances::transfer_all(Some(alice.clone()).into(), charlie.clone(), false)); + assert!(frame_system::Account::::contains_key(&alice)); + assert_eq!(System::account(&alice).providers, 0); + assert_eq!(System::account(&alice).sufficients, 1); + + // reap Bob's CC so his account should be killed + assert_ok!(EncointerBalances::transfer_all(Some(bob.clone()).into(), charlie.clone(), cid)); + assert!(!frame_system::Account::::contains_key(&bob)); + assert!(!EncointerBalanceStorage::::contains_key(cid, &bob)); + + // reap Alice CC so her account should be killed + assert_ok!(EncointerBalances::transfer_all( + Some(alice.clone()).into(), + charlie.clone(), + cid + )); + assert!(!frame_system::Account::::contains_key(&alice)); }) } diff --git a/faucet/src/mock.rs b/faucet/src/mock.rs index ad0fa57e..24c02d30 100644 --- a/faucet/src/mock.rs +++ b/faucet/src/mock.rs @@ -64,22 +64,6 @@ impl pallet_treasury::Config for TestRuntime { type SpendOrigin = frame_support::traits::NeverEnsureOrigin; } -impl pallet_balances::Config for TestRuntime { - type MaxLocks = (); - type MaxReserves = ConstU32<1000>; - type ReserveIdentifier = [u8; 8]; - type Balance = u64; - type RuntimeEvent = RuntimeEvent; - type DustRemoval = (); - type ExistentialDeposit = ConstU64<1>; - type AccountStore = System; - type WeightInfo = (); - type FreezeIdentifier = (); - type MaxHolds = (); - type MaxFreezes = (); - type RuntimeHoldReason = (); -} - parameter_types! { pub const FaucetPalletId: PalletId = PalletId(*b"faucetId"); } @@ -94,6 +78,7 @@ impl dut::Config for TestRuntime { // boilerplate impl_frame_system!(TestRuntime); +impl_balances!(TestRuntime, System); impl_timestamp!(TestRuntime, EncointerScheduler); impl_encointer_scheduler!(TestRuntime); impl_encointer_balances!(TestRuntime); diff --git a/test-utils/src/lib.rs b/test-utils/src/lib.rs index a9ea122a..27c85fb5 100644 --- a/test-utils/src/lib.rs +++ b/test-utils/src/lib.rs @@ -19,14 +19,10 @@ //extern crate node_primitives; use encointer_primitives::balances::{BalanceType, Demurrage}; -use frame_support::{ - ord_parameter_types, parameter_types, - traits::{EitherOfDiverse, Get}, -}; +use frame_support::{ord_parameter_types, parameter_types, traits::EitherOfDiverse}; use frame_system::{EnsureRoot, EnsureSignedBy}; use sp_core::crypto::AccountId32; use sp_runtime::{generic, traits::IdentifyAccount, MultiSignature, Perbill}; -use std::cell::RefCell; // convenience reexport such that the tests do not need to put sp-keyring in the Cargo.toml. pub use sp_keyring::AccountKeyring; @@ -56,9 +52,6 @@ pub const TIME_TOLERANCE: u64 = 600000; // [ms] pub const LOCATION_TOLERANCE: u32 = 1000; // [m] pub const ZERO: BalanceType = BalanceType::from_bits(0x0); -thread_local! { - static EXISTENTIAL_DEPOSIT: RefCell = RefCell::new(0); -} /// The signature type used by accounts/transactions. pub type Signature = MultiSignature; /// An identifier for an account on this system. @@ -70,13 +63,6 @@ pub type Balance = u64; pub type Header = generic::Header; -pub struct ExistentialDeposit; -impl Get for ExistentialDeposit { - fn get() -> u64 { - EXISTENTIAL_DEPOSIT.with(|v| *v.borrow()) - } -} - parameter_types! { pub const BlockHashCount: u64 = 250; pub const MaximumBlockWeight: u32 = 1024; @@ -155,13 +141,16 @@ macro_rules! impl_balances { type Balance = Balance; type RuntimeEvent = RuntimeEvent; type DustRemoval = (); - type ExistentialDeposit = ExistentialDeposit; + type ExistentialDeposit = frame_support::traits::ConstU64<1>; type AccountStore = System; type WeightInfo = (); type MaxLocks = (); - type MaxReserves = (); + type MaxReserves = frame_support::traits::ConstU32<1000>; type ReserveIdentifier = [u8; 8]; type RuntimeHoldReason = (); + type FreezeIdentifier = (); + type MaxHolds = frame_support::traits::ConstU32<0>; + type MaxFreezes = frame_support::traits::ConstU32<0>; } }; }