From 48ca45d526cee0e294e71d442f805ecaafe2e6ab Mon Sep 17 00:00:00 2001 From: Connor Davis <17688291+PoisonPhang@users.noreply.github.com> Date: Mon, 7 Nov 2022 15:45:53 -0600 Subject: [PATCH] Applied fixes for unbalanced traits --- tokens/src/lib.rs | 17 ++- tokens/src/tests_events.rs | 2 +- tokens/src/tests_fungibles.rs | 280 ++++++++++++++++++++++++++++++++++ 3 files changed, 292 insertions(+), 7 deletions(-) diff --git a/tokens/src/lib.rs b/tokens/src/lib.rs index 06fefa10c..7587d7998 100644 --- a/tokens/src/lib.rs +++ b/tokens/src/lib.rs @@ -1729,18 +1729,23 @@ impl fungibles::Transfer for Pallet { impl fungibles::Unbalanced for Pallet { fn set_balance(asset_id: Self::AssetId, who: &T::AccountId, amount: Self::Balance) -> DispatchResult { - // Balance is the same type and will not overflow - Self::mutate_account(who, asset_id, |account, _| { - account.free = amount; + // Balance is the same type and will not overflow. + // Provided `fungibles::Unbalanced` functions always provide `set_balance` with `free + reserved`. + // free = new_balance - reserved + Self::mutate_account(who, asset_id, |account, _| -> DispatchResult { + account.free = amount + .checked_sub(&account.reserved) + .ok_or(ArithmeticError::Underflow)?; Self::deposit_event(Event::BalanceSet { currency_id: asset_id, who: who.clone(), - free: amount, + free: account.free, reserved: account.reserved, }); - }); - Ok(()) + + Ok(()) + }) } fn set_total_issuance(asset_id: Self::AssetId, amount: Self::Balance) { diff --git a/tokens/src/tests_events.rs b/tokens/src/tests_events.rs index a03c3c0bf..400630d67 100644 --- a/tokens/src/tests_events.rs +++ b/tokens/src/tests_events.rs @@ -202,7 +202,7 @@ fn pallet_fungibles_unbalanced_deposit_events() { System::assert_last_event(Event::Tokens(crate::Event::BalanceSet { currency_id: DOT, who: ALICE, - free: 500, + free: 450, reserved: 50, })); diff --git a/tokens/src/tests_fungibles.rs b/tokens/src/tests_fungibles.rs index 69b862165..7659930bd 100644 --- a/tokens/src/tests_fungibles.rs +++ b/tokens/src/tests_fungibles.rs @@ -5,6 +5,7 @@ use super::*; use frame_support::{assert_noop, assert_ok}; use mock::*; +use sp_runtime::TokenError; #[test] fn fungibles_inspect_trait_should_work() { @@ -64,6 +65,285 @@ fn fungibles_unbalanced_trait_should_work() { }); } +mod fungibles_unbalanced { + use super::*; + + #[test] + fn set_balance_should_update_balance() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 100)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_ok!(>::set_balance(DOT, &ALICE, 10)); + assert_eq!(>::balance(DOT, &ALICE), 10); + }); + } + + #[test] + fn set_total_issuance_should_update_total_issuance() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 100)]) + .build() + .execute_with(|| { + assert_eq!(>::total_issuance(DOT), 100); + >::set_total_issuance(DOT, 10); + assert_eq!(>::total_issuance(DOT), 10); + }); + } + + mod decrease_balance { + + use super::*; + + #[test] + fn should_fail_when_not_enough_funds() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 10)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 10); + assert_noop!( + >::decrease_balance(DOT, &ALICE, 20), + TokenError::NoFunds + ); + }); + } + + #[test] + fn should_update_balance_when_successful() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 10)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 10); + assert_ok!( + >::decrease_balance(DOT, &ALICE, 5), + 5 + ); + assert_eq!(>::balance(DOT, &ALICE), 5); + }); + } + + #[test] + fn should_remove_accounts_that_drop_below_ed() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 5)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 5); + assert_ok!( + >::decrease_balance(DOT, &ALICE, 4), + 5 + ); + assert_eq!(>::balance(DOT, &ALICE), 0); + }); + } + + #[test] + fn should_fail_when_not_enough_free_funds() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 100)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_ok!(>::hold(DOT, &ALICE, 50)); + assert_noop!( + >::decrease_balance(DOT, &ALICE, 60), + ArithmeticError::Underflow + ); + }); + } + + #[test] + fn should_update_balance_when_enough_free_funds() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 100)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_ok!(>::hold(DOT, &ALICE, 50)); + assert_ok!( + >::decrease_balance(DOT, &ALICE, 50), + 50 + ); + assert_eq!(>::balance(DOT, &ALICE), 50); + assert_ok!(>::release(DOT, &ALICE, 50, false)); + assert_eq!(>::balance(DOT, &ALICE), 50); + }); + } + } + + mod decrease_balance_at_most { + use super::*; + + #[test] + fn should_decrease_by_part_available_when_not_enough_for_most() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 10)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 10); + assert_eq!( + >::decrease_balance_at_most(DOT, &ALICE, 20), + 10 + ); + assert_eq!(>::balance(DOT, &ALICE), 0); + }) + } + + #[test] + fn should_decrease_by_most_when_available() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 10)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 10); + assert_eq!( + >::decrease_balance_at_most(DOT, &ALICE, 5), + 5 + ); + assert_eq!(>::balance(DOT, &ALICE), 5); + }) + } + + #[test] + fn should_remove_accounts_that_drop_below_ed() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 5)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 5); + assert_eq!( + >::decrease_balance_at_most(DOT, &ALICE, 4), + 5 + ); + assert_eq!(>::balance(DOT, &ALICE), 0); + }) + } + + // Ignore while `decrease_balance_at_most` is broken + #[ignore] + #[test] + fn should_remove_free_when_part_is_reserved() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 100)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_ok!(>::hold(DOT, &ALICE, 50)); + assert_eq!( + >::decrease_balance_at_most(DOT, &ALICE, 60), + 50 + ); + assert_eq!(>::balance(DOT, &ALICE), 50); + }) + } + + #[test] + fn should_remove_most_when_part_is_reserved_and_free_has_enough() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 100)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 100); + assert_ok!(>::hold(DOT, &ALICE, 50)); + assert_eq!( + >::decrease_balance_at_most(DOT, &ALICE, 50), + 50 + ); + assert_eq!(>::balance(DOT, &ALICE), 50); + assert_ok!(>::release(DOT, &ALICE, 50, false)); + assert_eq!(>::balance(DOT, &ALICE), 50); + }) + } + } + + mod increase_balance { + use super::*; + + #[test] + fn should_error_when_below_ed() { + ExtBuilder::default().build().execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 0); + assert_noop!( + >::increase_balance(DOT, &ALICE, 1), + TokenError::BelowMinimum + ); + }) + } + + #[test] + fn should_update_balance() { + ExtBuilder::default().build().execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 0); + assert_ok!( + >::increase_balance(DOT, &ALICE, 2), + 2 + ); + assert_eq!(>::balance(DOT, &ALICE), 2); + }) + } + + #[test] + fn should_error_when_overflowing_balance_type() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 2)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 2); + assert_noop!( + >::increase_balance(DOT, &ALICE, Balance::MAX), + ArithmeticError::Overflow + ); + }) + } + } + + mod increase_balance_at_most { + use super::*; + + #[test] + fn should_not_create_dust_account() { + ExtBuilder::default().build().execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 0); + assert_eq!( + >::increase_balance_at_most(DOT, &ALICE, 1), + 0 + ); + assert_eq!(>::balance(DOT, &ALICE), 0); + }) + } + + #[test] + fn should_update_balance() { + ExtBuilder::default().build().execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 0); + assert_eq!( + >::increase_balance_at_most(DOT, &ALICE, 2), + 2 + ); + assert_eq!(>::balance(DOT, &ALICE), 2); + }) + } + + #[test] + fn should_not_overflow() { + ExtBuilder::default() + .balances(vec![(ALICE, DOT, 2)]) + .build() + .execute_with(|| { + assert_eq!(>::balance(DOT, &ALICE), 2); + assert_eq!( + >::increase_balance_at_most(DOT, &ALICE, Balance::MAX), + Balance::MAX - 2 + ); + assert_eq!(>::balance(DOT, &ALICE), Balance::MAX); + }) + } + } +} + #[test] fn fungibles_inspect_hold_trait_should_work() { ExtBuilder::default()