From 9d2be1b03541264f45a595ec37fdc48e2a3b770c Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Tue, 4 Jul 2023 13:29:17 +0300 Subject: [PATCH 1/3] Add swap_keep_alive call --- crates/pallet-currency-swap/src/lib.rs | 79 +++++++++++++++------- crates/pallet-currency-swap/src/weights.rs | 9 ++- 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/crates/pallet-currency-swap/src/lib.rs b/crates/pallet-currency-swap/src/lib.rs index cc14ed219..0116aac02 100644 --- a/crates/pallet-currency-swap/src/lib.rs +++ b/crates/pallet-currency-swap/src/lib.rs @@ -108,33 +108,64 @@ pub mod pallet { let who = ensure_signed(origin)?; with_storage_layer(move || { - let withdrawed_imbalance = FromCurrencyOf::::withdraw( - &who, - amount, - WithdrawReasons::TRANSFER, - ExistenceRequirement::AllowDeath, - )?; - let withdrawed_amount = withdrawed_imbalance.peek(); - - let deposited_imbalance = - T::CurrencySwap::swap(withdrawed_imbalance).map_err(|error| { - // Here we undo the withdrawl to avoid having a dangling imbalance. - FromCurrencyOf::::resolve_creating(&who, error.incoming_imbalance); - error.cause.into() - })?; - let deposited_amount = deposited_imbalance.peek(); - - ToCurrencyOf::::resolve_creating(&to, deposited_imbalance); - - Self::deposit_event(Event::BalancesSwapped { - from: who, - withdrawed_amount, - to, - deposited_amount, - }); + Self::do_swap(who, to, amount, ExistenceRequirement::AllowDeath)?; Ok(()) }) } + + /// Same as the [`swap`] call, but with a check that the swap will not kill the origin account. + #[pallet::call_index(1)] + #[pallet::weight(T::WeightInfo::swap_keep_alive())] + pub fn swap_keep_alive( + origin: OriginFor, + to: T::AccountIdTo, + amount: FromBalanceOf, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + + with_storage_layer(move || { + Self::do_swap(who, to, amount, ExistenceRequirement::KeepAlive)?; + + Ok(()) + }) + } + } + + impl Pallet { + /// General swap balances implementation. + pub fn do_swap( + who: T::AccountId, + to: T::AccountIdTo, + amount: FromBalanceOf, + existence_requirement: ExistenceRequirement, + ) -> DispatchResult { + let withdrawed_imbalance = FromCurrencyOf::::withdraw( + &who, + amount, + WithdrawReasons::TRANSFER, + existence_requirement, + )?; + let withdrawed_amount = withdrawed_imbalance.peek(); + + let deposited_imbalance = + T::CurrencySwap::swap(withdrawed_imbalance).map_err(|error| { + // Here we undo the withdrawl to avoid having a dangling imbalance. + FromCurrencyOf::::resolve_creating(&who, error.incoming_imbalance); + error.cause.into() + })?; + let deposited_amount = deposited_imbalance.peek(); + + ToCurrencyOf::::resolve_creating(&to, deposited_imbalance); + + Self::deposit_event(Event::BalancesSwapped { + from: who, + withdrawed_amount, + to, + deposited_amount, + }); + + Ok(()) + } } } diff --git a/crates/pallet-currency-swap/src/weights.rs b/crates/pallet-currency-swap/src/weights.rs index 37daf0790..0cbef516f 100644 --- a/crates/pallet-currency-swap/src/weights.rs +++ b/crates/pallet-currency-swap/src/weights.rs @@ -4,12 +4,19 @@ use frame_support::weights::Weight; /// Weight functions needed for pallet-currency-swap. pub trait WeightInfo { - /// A function to calculate required weights for swap call. + /// A function to calculate required weights for [`swap`] call. fn swap() -> Weight; + + /// A function to calculate required weights for [`swap_keep_alive`] call. + fn swap_keep_alive() -> Weight; } impl WeightInfo for () { fn swap() -> Weight { Weight::zero() } + + fn swap_keep_alive() -> Weight { + Weight::zero() + } } From 61182c9b8c39cd816a3fa3ff96a0c000b192997a Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Tue, 4 Jul 2023 13:51:30 +0300 Subject: [PATCH 2/3] Add tests --- crates/pallet-currency-swap/src/mock.rs | 4 +- crates/pallet-currency-swap/src/tests.rs | 147 ++++++++++++++++++++++- 2 files changed, 147 insertions(+), 4 deletions(-) diff --git a/crates/pallet-currency-swap/src/mock.rs b/crates/pallet-currency-swap/src/mock.rs index 99b987586..d43932396 100644 --- a/crates/pallet-currency-swap/src/mock.rs +++ b/crates/pallet-currency-swap/src/mock.rs @@ -69,7 +69,7 @@ impl pallet_balances::Config for Test { type Balance = u64; type RuntimeEvent = RuntimeEvent; type DustRemoval = (); - type ExistentialDeposit = ConstU64<1>; + type ExistentialDeposit = ConstU64<10>; type AccountStore = System; type MaxLocks = (); type MaxReserves = (); @@ -90,7 +90,7 @@ impl pallet_evm_balances::Config for Test { type RuntimeEvent = RuntimeEvent; type AccountId = EvmAccountId; type Balance = Balance; - type ExistentialDeposit = ConstU64<1>; + type ExistentialDeposit = ConstU64<10>; type AccountStore = EvmSystem; type DustRemoval = (); } diff --git a/crates/pallet-currency-swap/src/tests.rs b/crates/pallet-currency-swap/src/tests.rs index 6b143a248..e3bff543d 100644 --- a/crates/pallet-currency-swap/src/tests.rs +++ b/crates/pallet-currency-swap/src/tests.rs @@ -8,7 +8,8 @@ use sp_std::str::FromStr; use crate::{mock::*, *}; -/// This test verifies that swap call works in the happy path. +/// This test verifies that [`swap`] call works as expected in case origin left balances amount +/// is greater or equal than existential deposit. #[test] fn swap_works() { new_test_ext().execute_with_ext(|_| { @@ -64,7 +65,117 @@ fn swap_works() { }); } -/// This test verifies that swap call fails in case some error happens during the actual swap logic. +/// This test verifies that [`swap`] call works as expected in case origin left balances amount +/// is less than existential deposit. The origin account should be killed. +#[test] +fn swap_works_kill_origin() { + new_test_ext().execute_with_ext(|_| { + let alice = 42; + let alice_evm = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + let alice_balance = 1000; + let swap_balance = 999; + + // Prepare the test state. + Balances::make_free_balance_be(&alice, alice_balance); + + // Check test preconditions. + assert_eq!(Balances::total_balance(&alice), alice_balance); + assert_eq!(EvmBalances::total_balance(&alice_evm), 0); + + // Set block number to enable events. + System::set_block_number(1); + + // Set mock expectations. + let swap_ctx = MockCurrencySwap::swap_context(); + swap_ctx + .expect() + .once() + .with(predicate::eq( + >::NegativeImbalance::new(swap_balance), + )) + .return_once(move |_| { + Ok(>::NegativeImbalance::new(swap_balance)) + }); + + // Invoke the function under test. + assert_ok!(CurrencySwap::swap( + RuntimeOrigin::signed(alice), + alice_evm, + swap_balance + )); + + // Assert state changes. + assert!(!System::account_exists(&alice)); + assert_eq!(EvmBalances::total_balance(&alice_evm), swap_balance); + System::assert_has_event(RuntimeEvent::CurrencySwap(Event::BalancesSwapped { + from: alice, + withdrawed_amount: swap_balance, + to: alice_evm, + deposited_amount: swap_balance, + })); + + // Assert mock invocations. + swap_ctx.checkpoint(); + }); +} + +/// This test verifies that [`swap_keep_alive`] call works in the happy path. +#[test] +fn swap_keep_alive_works() { + new_test_ext().execute_with_ext(|_| { + let alice = 42; + let alice_evm = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + let alice_balance = 1000; + let swap_balance = 100; + + // Prepare the test state. + Balances::make_free_balance_be(&alice, alice_balance); + + // Check test preconditions. + assert_eq!(Balances::total_balance(&alice), alice_balance); + assert_eq!(EvmBalances::total_balance(&alice_evm), 0); + + // Set block number to enable events. + System::set_block_number(1); + + // Set mock expectations. + let swap_ctx = MockCurrencySwap::swap_context(); + swap_ctx + .expect() + .once() + .with(predicate::eq( + >::NegativeImbalance::new(swap_balance), + )) + .return_once(move |_| { + Ok(>::NegativeImbalance::new(swap_balance)) + }); + + // Invoke the function under test. + assert_ok!(CurrencySwap::swap( + RuntimeOrigin::signed(alice), + alice_evm, + swap_balance + )); + + // Assert state changes. + assert_eq!( + Balances::total_balance(&alice), + alice_balance - swap_balance + ); + assert_eq!(EvmBalances::total_balance(&alice_evm), swap_balance); + System::assert_has_event(RuntimeEvent::CurrencySwap(Event::BalancesSwapped { + from: alice, + withdrawed_amount: swap_balance, + to: alice_evm, + deposited_amount: swap_balance, + })); + + // Assert mock invocations. + swap_ctx.checkpoint(); + }); +} + +/// This test verifies that [`swap`] call fails in case some error happens during the actual swap logic. #[test] fn swap_fails() { new_test_ext().execute_with_ext(|_| { @@ -105,3 +216,35 @@ fn swap_fails() { swap_ctx.checkpoint(); }); } + +/// This test verifies that [`swap_keep_alive`] call fails in case origin left balances amount +/// is less than existential deposit. The call should prevent swap operation. +#[test] +fn swap_keep_alive_fails() { + new_test_ext().execute_with_ext(|_| { + let alice = 42; + let alice_evm = H160::from_str("1000000000000000000000000000000000000001").unwrap(); + let alice_balance = 1000; + let swap_balance = 999; + + // Prepare the test state. + Balances::make_free_balance_be(&alice, alice_balance); + + // Set mock expectations. + let swap_ctx = MockCurrencySwap::swap_context(); + swap_ctx.expect().never(); + + // Invoke the function under test. + assert_noop!( + CurrencySwap::swap_keep_alive(RuntimeOrigin::signed(alice), alice_evm, swap_balance), + pallet_balances::Error::::KeepAlive + ); + + // Assert state changes. + assert_eq!(Balances::total_balance(&alice), alice_balance); + assert_eq!(EvmBalances::total_balance(&alice_evm), 0); + + // Assert mock invocations. + swap_ctx.checkpoint(); + }); +} From 113bc0e6a172351f5c3aac85f22614e6c632acaf Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Tue, 4 Jul 2023 16:32:49 +0300 Subject: [PATCH 3/3] Fix docs --- crates/pallet-currency-swap/src/lib.rs | 2 +- crates/pallet-currency-swap/src/tests.rs | 10 +++++----- crates/pallet-currency-swap/src/weights.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/pallet-currency-swap/src/lib.rs b/crates/pallet-currency-swap/src/lib.rs index 0116aac02..df97777de 100644 --- a/crates/pallet-currency-swap/src/lib.rs +++ b/crates/pallet-currency-swap/src/lib.rs @@ -114,7 +114,7 @@ pub mod pallet { }) } - /// Same as the [`swap`] call, but with a check that the swap will not kill the origin account. + /// Same as the swap call, but with a check that the swap will not kill the origin account. #[pallet::call_index(1)] #[pallet::weight(T::WeightInfo::swap_keep_alive())] pub fn swap_keep_alive( diff --git a/crates/pallet-currency-swap/src/tests.rs b/crates/pallet-currency-swap/src/tests.rs index e3bff543d..1764410dc 100644 --- a/crates/pallet-currency-swap/src/tests.rs +++ b/crates/pallet-currency-swap/src/tests.rs @@ -8,7 +8,7 @@ use sp_std::str::FromStr; use crate::{mock::*, *}; -/// This test verifies that [`swap`] call works as expected in case origin left balances amount +/// This test verifies that swap call works as expected in case origin left balances amount /// is greater or equal than existential deposit. #[test] fn swap_works() { @@ -65,7 +65,7 @@ fn swap_works() { }); } -/// This test verifies that [`swap`] call works as expected in case origin left balances amount +/// This test verifies that swap call works as expected in case origin left balances amount /// is less than existential deposit. The origin account should be killed. #[test] fn swap_works_kill_origin() { @@ -119,7 +119,7 @@ fn swap_works_kill_origin() { }); } -/// This test verifies that [`swap_keep_alive`] call works in the happy path. +/// This test verifies that `swap_keep_alive` call works in the happy path. #[test] fn swap_keep_alive_works() { new_test_ext().execute_with_ext(|_| { @@ -175,7 +175,7 @@ fn swap_keep_alive_works() { }); } -/// This test verifies that [`swap`] call fails in case some error happens during the actual swap logic. +/// This test verifies that swap call fails in case some error happens during the actual swap logic. #[test] fn swap_fails() { new_test_ext().execute_with_ext(|_| { @@ -217,7 +217,7 @@ fn swap_fails() { }); } -/// This test verifies that [`swap_keep_alive`] call fails in case origin left balances amount +/// This test verifies that `swap_keep_alive` call fails in case origin left balances amount /// is less than existential deposit. The call should prevent swap operation. #[test] fn swap_keep_alive_fails() { diff --git a/crates/pallet-currency-swap/src/weights.rs b/crates/pallet-currency-swap/src/weights.rs index 0cbef516f..11071103e 100644 --- a/crates/pallet-currency-swap/src/weights.rs +++ b/crates/pallet-currency-swap/src/weights.rs @@ -4,10 +4,10 @@ use frame_support::weights::Weight; /// Weight functions needed for pallet-currency-swap. pub trait WeightInfo { - /// A function to calculate required weights for [`swap`] call. + /// A function to calculate required weights for swap call. fn swap() -> Weight; - /// A function to calculate required weights for [`swap_keep_alive`] call. + /// A function to calculate required weights for `swap_keep_alive` call. fn swap_keep_alive() -> Weight; }