From c5c38c04c4677458a70307a111c611efa58e669d Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Thu, 15 Jun 2023 20:31:40 +0800 Subject: [PATCH 1/2] fix fungibles::Inspect::balance --- tokens/src/lib.rs | 23 +++++++++---- tokens/src/tests_events.rs | 2 +- tokens/src/tests_fungibles.rs | 63 ++++++++++++++++++++++++++++++----- 3 files changed, 72 insertions(+), 16 deletions(-) diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index fe2d50857..bdba849e9 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -1768,7 +1768,7 @@ impl fungibles::Inspect for Pallet { } fn balance(asset_id: Self::AssetId, who: &T::AccountId) -> Self::Balance { - Self::accounts(who, asset_id).total() + Self::accounts(who, asset_id).free } fn total_balance(asset_id: Self::AssetId, who: &T::AccountId) -> Self::Balance { @@ -1869,11 +1869,21 @@ impl fungibles::Unbalanced for Pallet { who: &T::AccountId, amount: Self::Balance, ) -> Result, DispatchError> { + let max_reduction = >::reducible_balance( + asset_id, + who, + Preservation::Expendable, + Fortitude::Force, + ); + // Balance is the same type and will not overflow let (_, dust_amount) = Self::try_mutate_account(who, asset_id, |account, _| -> Result<(), DispatchError> { - // free = new_balance - reserved - account.free = amount.checked_sub(&account.reserved).ok_or(TokenError::BelowMinimum)?; + // Make sure the reduction (if there is one) is no more than the maximum + // allowed. + let reduction = account.free.saturating_sub(amount); + ensure!(reduction <= max_reduction, Error::::BalanceTooLow); + account.free = amount; Self::deposit_event(Event::BalanceSet { currency_id: asset_id, who: who.clone(), @@ -1911,9 +1921,10 @@ impl fungibles::Unbalanced for Pallet { amount = amount.min(free); } let new_balance = old_balance.checked_sub(&amount).ok_or(TokenError::FundsUnavailable)?; - // Original implementation was not returning burnt dust in result - let dust_burnt = Self::write_balance(asset, who, new_balance)?.unwrap_or_default(); - Ok(old_balance.saturating_sub(new_balance).saturating_add(dust_burnt)) + let _dust_amount = Self::write_balance(asset, who, new_balance)?.unwrap_or_default(); + + // here just return decreased amount, shouldn't count the dust_amount, + Ok(old_balance.saturating_sub(new_balance)) } } diff --git a/tokens/src/tests_events.rs b/tokens/src/tests_events.rs index 6b89226c4..c75c9b0d8 100644 --- a/tokens/src/tests_events.rs +++ b/tokens/src/tests_events.rs @@ -220,7 +220,7 @@ fn pallet_fungibles_unbalanced_deposit_events() { System::assert_last_event(RuntimeEvent::Tokens(crate::Event::BalanceSet { currency_id: DOT, who: ALICE, - free: 450, + free: 500, reserved: 50, })); diff --git a/tokens/src/tests_fungibles.rs b/tokens/src/tests_fungibles.rs index 07498ac18..8283d1f86 100644 --- a/tokens/src/tests_fungibles.rs +++ b/tokens/src/tests_fungibles.rs @@ -119,12 +119,24 @@ fn fungibles_unbalanced_trait_should_work() { Preservation::Expendable, Fortitude::Polite ), - Ok(5) + Ok(4) ); assert_eq!(>::balance(DOT, &ALICE), 0); + assert_eq!(>::total_balance(DOT, &ALICE), 0); // set reserved assert_ok!(>::write_balance(DOT, &ALICE, 100)); assert_ok!(>::reserve(DOT, &ALICE, 50)); + assert_eq!(>::balance(DOT, &ALICE), 50); + assert_eq!(>::total_balance(DOT, &ALICE), 100); + assert_eq!( + >::reducible_balance( + DOT, + &ALICE, + Preservation::Protect, + Fortitude::Polite + ), + 50 + ); assert_noop!( >::decrease_balance( DOT, @@ -134,7 +146,7 @@ fn fungibles_unbalanced_trait_should_work() { Preservation::Protect, Fortitude::Polite ), - TokenError::BelowMinimum + TokenError::FundsUnavailable ); assert_eq!( >::decrease_balance( @@ -147,16 +159,19 @@ fn fungibles_unbalanced_trait_should_work() { ), Ok(50) ); - assert_eq!(>::balance(DOT, &ALICE), 50); + assert_eq!(>::balance(DOT, &ALICE), 0); + assert_eq!(>::total_balance(DOT, &ALICE), 50); assert_eq!( >::unreserve(DOT, &ALICE, 50), 0 ); assert_eq!(>::balance(DOT, &ALICE), 50); + assert_eq!(>::total_balance(DOT, &ALICE), 50); // decrease_balance_at_most assert_ok!(>::write_balance(DOT, &ALICE, 10)); assert_eq!(>::balance(DOT, &ALICE), 10); + assert_eq!(>::total_balance(DOT, &ALICE), 10); assert_eq!( >::decrease_balance( DOT, @@ -168,6 +183,8 @@ fn fungibles_unbalanced_trait_should_work() { ), Ok(10) ); + assert_eq!(>::balance(DOT, &ALICE), 0); + assert_eq!(>::total_balance(DOT, &ALICE), 0); assert_ok!(>::write_balance(DOT, &ALICE, 10)); assert_eq!( >::decrease_balance( @@ -181,6 +198,8 @@ fn fungibles_unbalanced_trait_should_work() { Ok(5) ); assert_eq!(>::balance(DOT, &ALICE), 5); + assert_eq!(>::total_balance(DOT, &ALICE), 5); + // new balance < ExistentialDeposits, clean dust assert_eq!( >::decrease_balance( @@ -191,12 +210,24 @@ fn fungibles_unbalanced_trait_should_work() { Preservation::Expendable, Fortitude::Polite ), - Ok(5) + Ok(4) ); assert_eq!(>::balance(DOT, &ALICE), 0); + assert_eq!(>::total_balance(DOT, &ALICE), 0); // set reserved assert_ok!(>::write_balance(DOT, &ALICE, 100)); assert_ok!(>::reserve(DOT, &ALICE, 50)); + assert_eq!(>::balance(DOT, &ALICE), 50); + assert_eq!(>::total_balance(DOT, &ALICE), 100); + assert_eq!( + >::reducible_balance( + DOT, + &ALICE, + Preservation::Protect, + Fortitude::Polite + ), + 50 + ); assert_eq!( >::decrease_balance( DOT, @@ -208,12 +239,14 @@ fn fungibles_unbalanced_trait_should_work() { ), Ok(50), ); - assert_eq!(>::balance(DOT, &ALICE), 50); + assert_eq!(>::balance(DOT, &ALICE), 0); + assert_eq!(>::total_balance(DOT, &ALICE), 50); assert_eq!( >::unreserve(DOT, &ALICE, 50), 0 ); assert_eq!(>::balance(DOT, &ALICE), 50); + assert_eq!(>::total_balance(DOT, &ALICE), 50); // increase_balance assert_ok!(>::write_balance(DOT, &ALICE, 0)); @@ -285,11 +318,15 @@ fn fungibles_mutate_hold_trait_should_work() { >::balance_on_hold(DOT, REASON, &ALICE), 0 ); + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_ok!(>::hold(DOT, REASON, &ALICE, 100)); assert_eq!( >::balance_on_hold(DOT, REASON, &ALICE), 100 ); + assert_eq!(>::balance(DOT, &ALICE), 0); + assert_eq!( >::release(DOT, REASON, &ALICE, 40, Precision::Exact), Ok(40) @@ -298,6 +335,7 @@ fn fungibles_mutate_hold_trait_should_work() { >::balance_on_hold(DOT, REASON, &ALICE), 60 ); + assert_eq!(>::balance(DOT, &ALICE), 40); // exceed hold amount when not in best_effort assert_noop!( @@ -314,17 +352,20 @@ fn fungibles_mutate_hold_trait_should_work() { >::balance_on_hold(DOT, REASON, &ALICE), 0 ); + assert_eq!(>::balance(DOT, &ALICE), 100); assert_ok!(>::hold(DOT, REASON, &ALICE, 70)); assert_eq!( >::balance_on_hold(DOT, REASON, &ALICE), 70 ); - assert_eq!(>::balance(DOT, &BOB), 100); + assert_eq!(>::balance(DOT, &ALICE), 30); + assert_eq!( >::balance_on_hold(DOT, REASON, &BOB), 0 ); + assert_eq!(>::balance(DOT, &BOB), 100); assert_eq!( >::transfer_on_hold( DOT, @@ -342,11 +383,13 @@ fn fungibles_mutate_hold_trait_should_work() { >::balance_on_hold(DOT, REASON, &ALICE), 65 ); - assert_eq!(>::balance(DOT, &BOB), 105); + assert_eq!(>::balance(DOT, &ALICE), 30); assert_eq!( >::balance_on_hold(DOT, REASON, &BOB), 0 ); + assert_eq!(>::balance(DOT, &BOB), 105); + assert_eq!( >::transfer_on_hold( DOT, @@ -364,11 +407,12 @@ fn fungibles_mutate_hold_trait_should_work() { >::balance_on_hold(DOT, REASON, &ALICE), 60 ); - assert_eq!(>::balance(DOT, &BOB), 110); + assert_eq!(>::balance(DOT, &ALICE), 30); assert_eq!( >::balance_on_hold(DOT, REASON, &BOB), 5 ); + assert_eq!(>::balance(DOT, &BOB), 105); // exceed hold amount when not in best_effort assert_noop!( @@ -403,11 +447,12 @@ fn fungibles_mutate_hold_trait_should_work() { >::balance_on_hold(DOT, REASON, &ALICE), 0 ); - assert_eq!(>::balance(DOT, &BOB), 170); + assert_eq!(>::balance(DOT, &ALICE), 30); assert_eq!( >::balance_on_hold(DOT, REASON, &BOB), 65 ); + assert_eq!(>::balance(DOT, &BOB), 105); }); } From c4ed561b2ed673f34d3d2cb2a7bd2ae74c7cf031 Mon Sep 17 00:00:00 2001 From: wangjj9219 <183318287@qq.com> Date: Thu, 15 Jun 2023 20:35:01 +0800 Subject: [PATCH 2/2] update --- tokens/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index bdba849e9..9a07389c1 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -1923,7 +1923,7 @@ impl fungibles::Unbalanced for Pallet { let new_balance = old_balance.checked_sub(&amount).ok_or(TokenError::FundsUnavailable)?; let _dust_amount = Self::write_balance(asset, who, new_balance)?.unwrap_or_default(); - // here just return decreased amount, shouldn't count the dust_amount, + // here just return decrease amount, shouldn't count the dust_amount Ok(old_balance.saturating_sub(new_balance)) } }