From 9d2be1b03541264f45a595ec37fdc48e2a3b770c Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Tue, 4 Jul 2023 13:29:17 +0300 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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; } From e1e2de41df83af1cdf1027d3bf4ac75027fda0d1 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Tue, 4 Jul 2023 16:22:41 +0300 Subject: [PATCH 04/10] Add estimated swapped balances logic into CurrencySwap interface --- crates/primitives-currency-swap/src/lib.rs | 24 +++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/crates/primitives-currency-swap/src/lib.rs b/crates/primitives-currency-swap/src/lib.rs index f43653f3f..7b8601c9f 100644 --- a/crates/primitives-currency-swap/src/lib.rs +++ b/crates/primitives-currency-swap/src/lib.rs @@ -3,15 +3,20 @@ // Either generate code at stadard mode, or `no_std`, based on the `std` feature presence. #![cfg_attr(not(feature = "std"), no_std)] -use frame_support::{sp_runtime::DispatchError, traits::Currency}; +use frame_support::{ + sp_runtime::DispatchError, + traits::{fungible::Inspect, Currency}, +}; /// Currency swap interface. pub trait CurrencySwap { /// The currency to convert from. - type From: Currency; + type From: Currency + + Inspect>::Balance>; /// The currency to convert to. - type To: Currency; + type To: Currency + + Inspect>::Balance>; /// A possible error happens during the actual swap logic. type Error: Into; @@ -23,14 +28,27 @@ pub trait CurrencySwap { ToNegativeImbalanceFor, ErrorFor, >; + + /// Obtain the estimated resulted balance value. + fn estimate_swapped_balance( + balance: FromBalanceFor, + ) -> ToBalanceFor; } +/// An easy way to access the [`Currency::Balance`] of [`CurrencySwap::From`] of `T`. +pub type FromBalanceFor = + <>::From as Currency>::Balance; + /// An easy way to access the [`Currency::NegativeImbalance`] of [`CurrencySwap::From`] of `T`. pub type FromNegativeImbalanceFor = <>::From as Currency>::NegativeImbalance; +/// An easy way to access the [`Currency::Balance`] of [`CurrencySwap::To`] of `T`. +pub type ToBalanceFor = + <>::To as Currency>::Balance; + /// An easy way to access the [`Currency::NegativeImbalance`] of [`CurrencySwap::To`] of `T`. pub type ToNegativeImbalanceFor = < Date: Tue, 4 Jul 2023 16:23:24 +0300 Subject: [PATCH 05/10] Apply required changes into bridge pot currency swap logic --- .../src/existence_optional.rs | 6 ++++++ .../src/existence_required.rs | 6 ++++++ crates/bridge-pot-currency-swap/src/lib.rs | 14 +++++++++++--- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/crates/bridge-pot-currency-swap/src/existence_optional.rs b/crates/bridge-pot-currency-swap/src/existence_optional.rs index fb3a5e515..0a7bd3693 100644 --- a/crates/bridge-pot-currency-swap/src/existence_optional.rs +++ b/crates/bridge-pot-currency-swap/src/existence_optional.rs @@ -45,4 +45,10 @@ impl primitives_currency_swap::CurrencySwap>::Balance, + ) -> >::Balance { + T::BalanceConverter::convert(balance) + } } diff --git a/crates/bridge-pot-currency-swap/src/existence_required.rs b/crates/bridge-pot-currency-swap/src/existence_required.rs index 7f2dbd5b7..fb0103999 100644 --- a/crates/bridge-pot-currency-swap/src/existence_required.rs +++ b/crates/bridge-pot-currency-swap/src/existence_required.rs @@ -73,4 +73,10 @@ impl primitives_currency_swap::CurrencySwap>::Balance, + ) -> >::Balance { + T::BalanceConverter::convert(balance) + } } diff --git a/crates/bridge-pot-currency-swap/src/lib.rs b/crates/bridge-pot-currency-swap/src/lib.rs index 3d8f76dfb..c9da023f2 100644 --- a/crates/bridge-pot-currency-swap/src/lib.rs +++ b/crates/bridge-pot-currency-swap/src/lib.rs @@ -6,7 +6,7 @@ use frame_support::{ sp_runtime::traits::Convert, sp_std::marker::PhantomData, - traits::{Currency, Get}, + traits::{fungible::Inspect, Currency, Get}, }; pub mod existence_optional; @@ -24,10 +24,18 @@ pub trait Config { type AccountIdTo; /// The currency to swap from. - type CurrencyFrom: Currency; + type CurrencyFrom: Currency + + Inspect< + Self::AccountIdFrom, + Balance = >::Balance, + >; /// The currency to swap to. - type CurrencyTo: Currency; + type CurrencyTo: Currency + + Inspect< + Self::AccountIdTo, + Balance = >::Balance, + >; /// The converter to determine how the balance amount should be converted from one currency to /// another. From 0ff654e73f1f2f90f321fde7ef6e7ceed8ec8006 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Tue, 4 Jul 2023 16:24:45 +0300 Subject: [PATCH 06/10] Fix depositting with balance below ed at pallet-currency-swap --- crates/pallet-currency-swap/src/lib.rs | 7 +++++-- crates/pallet-currency-swap/src/mock.rs | 4 ++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/pallet-currency-swap/src/lib.rs b/crates/pallet-currency-swap/src/lib.rs index df97777de..0cb1b4f21 100644 --- a/crates/pallet-currency-swap/src/lib.rs +++ b/crates/pallet-currency-swap/src/lib.rs @@ -2,7 +2,7 @@ #![cfg_attr(not(feature = "std"), no_std)] -use frame_support::traits::Currency; +use frame_support::traits::{fungible::Inspect, Currency}; pub use pallet::*; use primitives_currency_swap::CurrencySwap as CurrencySwapT; pub use weights::*; @@ -140,6 +140,9 @@ pub mod pallet { amount: FromBalanceOf, existence_requirement: ExistenceRequirement, ) -> DispatchResult { + let estimated_swapped_balance = T::CurrencySwap::estimate_swapped_balance(amount); + ToCurrencyOf::::can_deposit(&to, estimated_swapped_balance, false).into_result()?; + let withdrawed_imbalance = FromCurrencyOf::::withdraw( &who, amount, @@ -150,7 +153,7 @@ pub mod pallet { let deposited_imbalance = T::CurrencySwap::swap(withdrawed_imbalance).map_err(|error| { - // Here we undo the withdrawl to avoid having a dangling imbalance. + // Here we undo the withdrawal to avoid having a dangling imbalance. FromCurrencyOf::::resolve_creating(&who, error.incoming_imbalance); error.cause.into() })?; diff --git a/crates/pallet-currency-swap/src/mock.rs b/crates/pallet-currency-swap/src/mock.rs index d43932396..ff3619e5d 100644 --- a/crates/pallet-currency-swap/src/mock.rs +++ b/crates/pallet-currency-swap/src/mock.rs @@ -109,6 +109,10 @@ mock! { primitives_currency_swap::ToNegativeImbalanceFor, primitives_currency_swap::ErrorFor >; + + fn estimate_swapped_balance( + balance: >::Balance, + ) -> >::Balance; } } From 68a996fc3f4185d18bd8828d3f4236922a4a67a1 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Tue, 4 Jul 2023 16:25:09 +0300 Subject: [PATCH 07/10] Fix depositting with balance below ed at precompile-currency-swap --- crates/precompile-currency-swap/src/lib.rs | 16 +++++++++++++++- crates/precompile-currency-swap/src/mock.rs | 4 ++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/precompile-currency-swap/src/lib.rs b/crates/precompile-currency-swap/src/lib.rs index a47b8aff6..dec771735 100644 --- a/crates/precompile-currency-swap/src/lib.rs +++ b/crates/precompile-currency-swap/src/lib.rs @@ -5,7 +5,7 @@ use frame_support::{ sp_runtime, sp_std::{marker::PhantomData, prelude::*}, - traits::tokens::currency::Currency, + traits::{fungible::Inspect, tokens::currency::Currency}, }; use pallet_evm::{ ExitError, ExitRevert, Precompile, PrecompileFailure, PrecompileHandle, PrecompileOutput, @@ -117,6 +117,20 @@ where }); } + let estimated_swapped_balance = CurrencySwapT::estimate_swapped_balance(value); + CurrencySwapT::To::can_deposit(&to, estimated_swapped_balance, false) + .into_result() + .map_err(|error| match error { + sp_runtime::DispatchError::Token(sp_runtime::TokenError::BelowMinimum) => { + PrecompileFailure::Error { + exit_status: ExitError::OutOfFund, + } + } + _ => PrecompileFailure::Error { + exit_status: ExitError::Other("unable to deposit funds".into()), + }, + })?; + let imbalance = CurrencySwapT::From::withdraw( &from, value, diff --git a/crates/precompile-currency-swap/src/mock.rs b/crates/precompile-currency-swap/src/mock.rs index 44fc45ca0..cf73bbac8 100644 --- a/crates/precompile-currency-swap/src/mock.rs +++ b/crates/precompile-currency-swap/src/mock.rs @@ -195,6 +195,10 @@ mock! { primitives_currency_swap::ToNegativeImbalanceFor, primitives_currency_swap::ErrorFor, >; + + fn estimate_swapped_balance( + balance: primitives_currency_swap::FromBalanceFor, + ) -> primitives_currency_swap::ToBalanceFor; } } From 2d73cceb68f8513b99613e3acf24e4cb542ac1ca Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Tue, 11 Jul 2023 19:34:36 +0300 Subject: [PATCH 08/10] Fix tests --- crates/pallet-currency-swap/src/tests.rs | 35 ++++++++++++++++ crates/precompile-currency-swap/src/tests.rs | 42 ++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/crates/pallet-currency-swap/src/tests.rs b/crates/pallet-currency-swap/src/tests.rs index 1764410dc..7f7aaa83d 100644 --- a/crates/pallet-currency-swap/src/tests.rs +++ b/crates/pallet-currency-swap/src/tests.rs @@ -29,6 +29,12 @@ fn swap_works() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(swap_balance); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx .expect() @@ -61,6 +67,7 @@ fn swap_works() { })); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -86,6 +93,12 @@ fn swap_works_kill_origin() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(swap_balance); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx .expect() @@ -115,6 +128,7 @@ fn swap_works_kill_origin() { })); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -139,6 +153,12 @@ fn swap_keep_alive_works() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(swap_balance); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx .expect() @@ -171,6 +191,7 @@ fn swap_keep_alive_works() { })); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -188,6 +209,12 @@ fn swap_fails() { Balances::make_free_balance_be(&alice, alice_balance); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(swap_balance); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx .expect() @@ -213,6 +240,7 @@ fn swap_fails() { assert_eq!(EvmBalances::total_balance(&alice_evm), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -231,6 +259,12 @@ fn swap_keep_alive_fails() { Balances::make_free_balance_be(&alice, alice_balance); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(swap_balance); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx.expect().never(); @@ -245,6 +279,7 @@ fn swap_keep_alive_fails() { assert_eq!(EvmBalances::total_balance(&alice_evm), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } diff --git a/crates/precompile-currency-swap/src/tests.rs b/crates/precompile-currency-swap/src/tests.rs index 4566c3ee3..9f5d60cae 100644 --- a/crates/precompile-currency-swap/src/tests.rs +++ b/crates/precompile-currency-swap/src/tests.rs @@ -39,6 +39,12 @@ fn swap_works() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(swap_balance); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx .expect() @@ -91,6 +97,7 @@ fn swap_works() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -125,6 +132,12 @@ fn swap_works_almost_full_balance() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(swap_balance); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx .expect() @@ -178,6 +191,7 @@ fn swap_works_almost_full_balance() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -208,6 +222,8 @@ fn swap_fail_no_funds() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx.expect().never(); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx.expect().never(); @@ -248,6 +264,7 @@ fn swap_fail_no_funds() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -282,6 +299,12 @@ fn swap_fail_trait_error() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(swap_balance); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx .expect() @@ -336,6 +359,7 @@ fn swap_fail_trait_error() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -373,6 +397,8 @@ fn swap_fail_full_balance() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx.expect().never(); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx.expect().never(); @@ -413,6 +439,7 @@ fn swap_fail_full_balance() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -446,6 +473,8 @@ fn swap_fail_bad_selector() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx.expect().never(); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx.expect().never(); @@ -488,6 +517,7 @@ fn swap_fail_bad_selector() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -522,6 +552,8 @@ fn swap_fail_value_overflow() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx.expect().never(); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx.expect().never(); @@ -561,6 +593,7 @@ fn swap_fail_value_overflow() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -594,6 +627,8 @@ fn swap_fail_no_arguments() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx.expect().never(); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx.expect().never(); @@ -636,6 +671,7 @@ fn swap_fail_no_arguments() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -669,6 +705,8 @@ fn swap_fail_short_argument() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx.expect().never(); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx.expect().never(); @@ -712,6 +750,7 @@ fn swap_fail_short_argument() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } @@ -745,6 +784,8 @@ fn swap_fail_trailing_junk() { System::set_block_number(1); // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx.expect().never(); let swap_ctx = MockCurrencySwap::swap_context(); swap_ctx.expect().never(); @@ -788,6 +829,7 @@ fn swap_fail_trailing_junk() { assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); }); } From 4f7d577994c0ef03c7d6efbe3eae8218f5cabe36 Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Wed, 12 Jul 2023 16:46:39 +0300 Subject: [PATCH 09/10] Add more tests to check introduced flow --- crates/pallet-currency-swap/src/mock.rs | 6 +- crates/pallet-currency-swap/src/tests.rs | 80 ++++++++++++++++++- crates/precompile-currency-swap/src/tests.rs | 83 ++++++++++++++++++++ 3 files changed, 166 insertions(+), 3 deletions(-) diff --git a/crates/pallet-currency-swap/src/mock.rs b/crates/pallet-currency-swap/src/mock.rs index ff3619e5d..2a73c5b2d 100644 --- a/crates/pallet-currency-swap/src/mock.rs +++ b/crates/pallet-currency-swap/src/mock.rs @@ -17,6 +17,8 @@ use sp_core::{H160, H256}; use crate::{self as pallet_currency_swap}; +pub(crate) const EXISTENTIAL_DEPOSIT: u64 = 10; + type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; @@ -69,7 +71,7 @@ impl pallet_balances::Config for Test { type Balance = u64; type RuntimeEvent = RuntimeEvent; type DustRemoval = (); - type ExistentialDeposit = ConstU64<10>; + type ExistentialDeposit = ConstU64; type AccountStore = System; type MaxLocks = (); type MaxReserves = (); @@ -90,7 +92,7 @@ impl pallet_evm_balances::Config for Test { type RuntimeEvent = RuntimeEvent; type AccountId = EvmAccountId; type Balance = Balance; - type ExistentialDeposit = ConstU64<10>; + type ExistentialDeposit = ConstU64; type AccountStore = EvmSystem; type DustRemoval = (); } diff --git a/crates/pallet-currency-swap/src/tests.rs b/crates/pallet-currency-swap/src/tests.rs index 7f7aaa83d..f7aba7d6d 100644 --- a/crates/pallet-currency-swap/src/tests.rs +++ b/crates/pallet-currency-swap/src/tests.rs @@ -3,7 +3,7 @@ use frame_support::{assert_noop, assert_ok, traits::Currency}; use mockall::predicate; use sp_core::H160; -use sp_runtime::DispatchError; +use sp_runtime::{DispatchError, TokenError}; use sp_std::str::FromStr; use crate::{mock::*, *}; @@ -245,6 +245,45 @@ fn swap_fails() { }); } +/// This test verifies that swap call fails in case estimated swapped balance less or equal +/// than target currency existential deposit. +#[test] +fn swap_below_ed_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 = 100; + + // Prepare the test state. + Balances::make_free_balance_be(&alice, alice_balance); + + // // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(EXISTENTIAL_DEPOSIT - 1); + let swap_ctx = MockCurrencySwap::swap_context(); + swap_ctx.expect().never(); + + // Invoke the function under test. + assert_noop!( + CurrencySwap::swap(RuntimeOrigin::signed(alice), alice_evm, swap_balance), + DispatchError::Token(TokenError::BelowMinimum) + ); + + // Assert state changes. + assert_eq!(Balances::total_balance(&alice), alice_balance); + assert_eq!(EvmBalances::total_balance(&alice_evm), 0); + + // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); + 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] @@ -283,3 +322,42 @@ fn swap_keep_alive_fails() { swap_ctx.checkpoint(); }); } + +/// This test verifies that `swap_keep_alive` call fails in case estimated swapped balance less or equal +/// than target currency existential deposit. +#[test] +fn swap_keep_alive_below_ed_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 = 100; + + // Prepare the test state. + Balances::make_free_balance_be(&alice, alice_balance); + + // // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(EXISTENTIAL_DEPOSIT - 1); + 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), + DispatchError::Token(TokenError::BelowMinimum) + ); + + // Assert state changes. + assert_eq!(Balances::total_balance(&alice), alice_balance); + assert_eq!(EvmBalances::total_balance(&alice_evm), 0); + + // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); + swap_ctx.checkpoint(); + }); +} diff --git a/crates/precompile-currency-swap/src/tests.rs b/crates/precompile-currency-swap/src/tests.rs index 9f5d60cae..a59d9fea0 100644 --- a/crates/precompile-currency-swap/src/tests.rs +++ b/crates/precompile-currency-swap/src/tests.rs @@ -269,6 +269,89 @@ fn swap_fail_no_funds() { }); } +/// This test verifies that the swap precompile call behaves as expected when +/// estimated swapped balance less or equal than target currency existential deposit. +/// All fee (up to specified max fee limit!) will be consumed, but not the value. +#[test] +fn swap_fail_below_ed() { + new_test_ext().execute_with_ext(|_| { + let alice_evm = H160::from(hex_literal::hex!( + "1000000000000000000000000000000000000001" + )); + let alice = AccountId::from(hex_literal::hex!( + "1000000000000000000000000000000000000000000000000000000000000001" + )); + + let expected_gas_usage: u64 = 50_123; // all fee will be consumed + let expected_fee: Balance = gas_to_fee(expected_gas_usage); + + let alice_evm_balance = 100 * 10u128.pow(18); + let swap_balance = 10 * 10u128.pow(18); + + // Prepare the test state. + EvmBalances::make_free_balance_be(&alice_evm, alice_evm_balance); + + // Check test preconditions. + assert_eq!(EvmBalances::total_balance(&alice_evm), alice_evm_balance); + assert_eq!(Balances::total_balance(&alice), 0); + + // Set block number to enable events. + System::set_block_number(1); + + // Set mock expectations. + let estimate_swapped_balance_ctx = MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .once() + .with(predicate::eq(swap_balance)) + .return_const(1_u128); + let swap_ctx = MockCurrencySwap::swap_context(); + swap_ctx.expect().never(); + + // Prepare EVM call. + let input = EvmDataWriter::new_with_selector(Action::Swap) + .write(H256::from(alice.as_ref())) + .build(); + + // Invoke the function under test. + let config = ::config(); + let execinfo = ::Runner::call( + alice_evm, + *PRECOMPILE_ADDRESS, + input, + swap_balance.into(), + 50_123, // a reasonable upper bound for tests + Some(*GAS_PRICE), + Some(*GAS_PRICE), + None, + Vec::new(), + true, + true, + config, + ) + .unwrap(); + assert_eq!( + execinfo.exit_reason, + fp_evm::ExitReason::Error(fp_evm::ExitError::OutOfFund) + ); + assert_eq!(execinfo.used_gas, expected_gas_usage.into()); + assert_eq!(execinfo.value, Vec::::new()); + assert_eq!(execinfo.logs, Vec::new()); + + // Assert state changes. + assert_eq!( + EvmBalances::total_balance(&alice_evm), + alice_evm_balance - expected_fee + ); + assert_eq!(Balances::total_balance(&alice), 0); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); + + // Assert mock invocations. + estimate_swapped_balance_ctx.checkpoint(); + swap_ctx.checkpoint(); + }); +} + /// This test verifies that the swap precompile call behaves as expected when the currency swap /// implementation fails. /// The fee is consumed (and not all of it - just what was actually used), but the value is not. From 3eb08dc4c2ba9522ca5dffd6070b664ee285a20d Mon Sep 17 00:00:00 2001 From: Dmitry Lavrenov Date: Wed, 12 Jul 2023 16:57:33 +0300 Subject: [PATCH 10/10] Fix benchmarking --- crates/pallet-currency-swap/src/benchmarking.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/pallet-currency-swap/src/benchmarking.rs b/crates/pallet-currency-swap/src/benchmarking.rs index df90412d4..f00f9058f 100644 --- a/crates/pallet-currency-swap/src/benchmarking.rs +++ b/crates/pallet-currency-swap/src/benchmarking.rs @@ -74,12 +74,19 @@ benchmarks! { impl Interface for crate::mock::Test { type Data = ( std::sync::MutexGuard<'static, ()>, + mock::__mock_MockCurrencySwap_CurrencySwap_9230394375286242749::__estimate_swapped_balance::Context, mock::__mock_MockCurrencySwap_CurrencySwap_9230394375286242749::__swap::Context, ); fn prepare() -> Self::Data { let mock_runtime_guard = mock::runtime_lock(); + let estimate_swapped_balance_ctx = + mock::MockCurrencySwap::estimate_swapped_balance_context(); + estimate_swapped_balance_ctx + .expect() + .times(1..) + .return_const(Self::to_balance()); let swap_ctx = mock::MockCurrencySwap::swap_context(); swap_ctx.expect().times(1..).return_once(move |_| { Ok( @@ -89,11 +96,12 @@ impl Interface for crate::mock::Test { ) }); - (mock_runtime_guard, swap_ctx) + (mock_runtime_guard, estimate_swapped_balance_ctx, swap_ctx) } fn verify(data: Self::Data) -> DispatchResult { - let (mock_runtime_guard, swap_ctx) = data; + let (mock_runtime_guard, estimate_swapped_balance_ctx, swap_ctx) = data; + estimate_swapped_balance_ctx.checkpoint(); swap_ctx.checkpoint(); drop(mock_runtime_guard); Ok(())