From 0192e5c88e0e8c42edc304177c8b29e00e098f6a Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Fri, 30 Jun 2023 21:38:55 -0300 Subject: [PATCH 1/7] Make currency swap return the original imbalance on failure --- crates/primitives-currency-swap/src/lib.rs | 34 ++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/primitives-currency-swap/src/lib.rs b/crates/primitives-currency-swap/src/lib.rs index 348d644c1..aa3480cfd 100644 --- a/crates/primitives-currency-swap/src/lib.rs +++ b/crates/primitives-currency-swap/src/lib.rs @@ -18,6 +18,36 @@ pub trait CurrencySwap { /// The actual swap logic. fn swap( - imbalance: >::NegativeImbalance, - ) -> Result<>::NegativeImbalance, Self::Error>; + imbalance: FromNegativeImbalanceFor, + ) -> Result< + ToNegativeImbalanceFor, + ErrorFor, + >; +} + +/// 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::NegativeImbalance`] of [`CurrencySwap::To`] of `T`. +pub type ToNegativeImbalanceFor = <>::To as Currency>::NegativeImbalance; + +/// A type alias for compact delcaration of the error type for the [`CurrencySwap::swap`] call. +pub type ErrorFor = Error< + FromNegativeImbalanceFor, + >::Error, +>; + +/// An error that can occur while doing a currency swap. +#[derive(Debug)] +pub struct Error { + /// The inderlying cause of this error. + pub cause: E, + /// The original imbalances that was passed to the swap operation. + pub incoming_imbalance: I, } From 5166fa5c8b17e475e861c5749d32983eeb2279b2 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Fri, 30 Jun 2023 21:39:12 -0300 Subject: [PATCH 2/7] Adjust the precompile-currency-swap --- crates/precompile-currency-swap/src/lib.rs | 10 +++++++--- crates/precompile-currency-swap/src/mock.rs | 9 ++++++--- crates/precompile-currency-swap/src/tests.rs | 17 ++++++++++++++++- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/crates/precompile-currency-swap/src/lib.rs b/crates/precompile-currency-swap/src/lib.rs index 34ad450a8..d85031177 100644 --- a/crates/precompile-currency-swap/src/lib.rs +++ b/crates/precompile-currency-swap/src/lib.rs @@ -134,9 +134,13 @@ where }, })?; - let imbalance = CurrencySwapT::swap(imbalance).map_err(|_| PrecompileFailure::Revert { - exit_status: ExitRevert::Reverted, - output: "unable to swap the currency".into(), + let imbalance = CurrencySwapT::swap(imbalance).map_err(|error| { + // Here we undo the withdrawl to avoid having a dangling imbalance. + CurrencySwapT::From::resolve_creating(&from, error.incoming_imbalance); + PrecompileFailure::Revert { + exit_status: ExitRevert::Reverted, + output: "unable to swap the currency".into(), + } })?; CurrencySwapT::To::resolve_creating(&to, imbalance); diff --git a/crates/precompile-currency-swap/src/mock.rs b/crates/precompile-currency-swap/src/mock.rs index a71652f26..44fc45ca0 100644 --- a/crates/precompile-currency-swap/src/mock.rs +++ b/crates/precompile-currency-swap/src/mock.rs @@ -13,7 +13,7 @@ use frame_support::{ traits::{BlakeTwo256, IdentityLookup}, BuildStorage, DispatchError, }, - traits::{ConstU16, ConstU32, ConstU64, Currency}, + traits::{ConstU16, ConstU32, ConstU64}, weights::Weight, }; use frame_system as system; @@ -190,8 +190,11 @@ mock! { type Error = DispatchError; fn swap( - imbalance: >::NegativeImbalance, - ) -> Result<>::NegativeImbalance, DispatchError>; + imbalance: primitives_currency_swap::FromNegativeImbalanceFor, + ) -> Result< + primitives_currency_swap::ToNegativeImbalanceFor, + primitives_currency_swap::ErrorFor, + >; } } diff --git a/crates/precompile-currency-swap/src/tests.rs b/crates/precompile-currency-swap/src/tests.rs index 0644f5cf5..4566c3ee3 100644 --- a/crates/precompile-currency-swap/src/tests.rs +++ b/crates/precompile-currency-swap/src/tests.rs @@ -88,6 +88,7 @@ fn swap_works() { alice_evm_balance - swap_balance - expected_fee ); assert_eq!(Balances::total_balance(&alice), swap_balance); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); @@ -174,6 +175,7 @@ fn swap_works_almost_full_balance() { ); assert_eq!(EvmBalances::total_balance(&alice_evm), 1); assert_eq!(Balances::total_balance(&alice), swap_balance); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); @@ -243,6 +245,7 @@ fn swap_fail_no_funds() { // Assert state changes. assert_eq!(EvmBalances::total_balance(&alice_evm), alice_evm_balance); assert_eq!(Balances::total_balance(&alice), 0); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); @@ -286,7 +289,12 @@ fn swap_fail_trait_error() { .with(predicate::eq( >::NegativeImbalance::new(swap_balance), )) - .return_once(move |_| Err(sp_runtime::DispatchError::Other("test"))); + .return_once(move |incoming_imbalance| { + Err(primitives_currency_swap::Error { + cause: sp_runtime::DispatchError::Other("test"), + incoming_imbalance, + }) + }); // Prepare EVM call. let input = EvmDataWriter::new_with_selector(Action::Swap) @@ -325,6 +333,7 @@ fn swap_fail_trait_error() { alice_evm_balance - expected_fee ); assert_eq!(Balances::total_balance(&alice), 0); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); @@ -401,6 +410,7 @@ fn swap_fail_full_balance() { // Assert state changes. assert_eq!(EvmBalances::total_balance(&alice_evm), alice_evm_balance); assert_eq!(Balances::total_balance(&alice), 0); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); @@ -475,6 +485,7 @@ fn swap_fail_bad_selector() { alice_evm_balance - expected_fee ); assert_eq!(Balances::total_balance(&alice), 0); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); @@ -547,6 +558,7 @@ fn swap_fail_value_overflow() { // Assert state changes. assert_eq!(EvmBalances::total_balance(&alice_evm), alice_evm_balance); assert_eq!(Balances::total_balance(&alice), 0); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); @@ -621,6 +633,7 @@ fn swap_fail_no_arguments() { alice_evm_balance - expected_fee ); assert_eq!(Balances::total_balance(&alice), 0); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); @@ -696,6 +709,7 @@ fn swap_fail_short_argument() { alice_evm_balance - expected_fee ); assert_eq!(Balances::total_balance(&alice), 0); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); @@ -771,6 +785,7 @@ fn swap_fail_trailing_junk() { alice_evm_balance - expected_fee ); assert_eq!(Balances::total_balance(&alice), 0); + assert_eq!(EvmBalances::total_balance(&PRECOMPILE_ADDRESS), 0); // Assert mock invocations. swap_ctx.checkpoint(); From 937ad4f48e215adf2a6c13e98e4befe60817e824 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Sat, 1 Jul 2023 11:01:54 -0300 Subject: [PATCH 3/7] Update crates/primitives-currency-swap/src/lib.rs Co-authored-by: Dmitry Lavrenov <39522748+dmitrylavrenov@users.noreply.github.com> --- crates/primitives-currency-swap/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/primitives-currency-swap/src/lib.rs b/crates/primitives-currency-swap/src/lib.rs index aa3480cfd..cd75624c0 100644 --- a/crates/primitives-currency-swap/src/lib.rs +++ b/crates/primitives-currency-swap/src/lib.rs @@ -37,7 +37,7 @@ pub type ToNegativeImbalanceFor = <>::To as Currency>::NegativeImbalance; -/// A type alias for compact delcaration of the error type for the [`CurrencySwap::swap`] call. +/// A type alias for compact declaration of the error type for the [`CurrencySwap::swap`] call. pub type ErrorFor = Error< FromNegativeImbalanceFor, >::Error, From d97eacfac238bc81ae76bce3d397b97e98d7b573 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Sat, 1 Jul 2023 11:02:04 -0300 Subject: [PATCH 4/7] Update crates/primitives-currency-swap/src/lib.rs Co-authored-by: Dmitry Lavrenov <39522748+dmitrylavrenov@users.noreply.github.com> --- crates/primitives-currency-swap/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/primitives-currency-swap/src/lib.rs b/crates/primitives-currency-swap/src/lib.rs index cd75624c0..3f303844f 100644 --- a/crates/primitives-currency-swap/src/lib.rs +++ b/crates/primitives-currency-swap/src/lib.rs @@ -46,7 +46,7 @@ pub type ErrorFor = Error< /// An error that can occur while doing a currency swap. #[derive(Debug)] pub struct Error { - /// The inderlying cause of this error. + /// The underlying cause of this error. pub cause: E, /// The original imbalances that was passed to the swap operation. pub incoming_imbalance: I, From 10d35eeeb1c2050ca8fe67db9053ac5d1c8b9518 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Sat, 1 Jul 2023 11:02:15 -0300 Subject: [PATCH 5/7] Update crates/precompile-currency-swap/src/lib.rs Co-authored-by: Dmitry Lavrenov <39522748+dmitrylavrenov@users.noreply.github.com> --- crates/precompile-currency-swap/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/precompile-currency-swap/src/lib.rs b/crates/precompile-currency-swap/src/lib.rs index d85031177..a47b8aff6 100644 --- a/crates/precompile-currency-swap/src/lib.rs +++ b/crates/precompile-currency-swap/src/lib.rs @@ -135,7 +135,7 @@ where })?; let imbalance = CurrencySwapT::swap(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. CurrencySwapT::From::resolve_creating(&from, error.incoming_imbalance); PrecompileFailure::Revert { exit_status: ExitRevert::Reverted, From d9e274cb0874e4312cf3292a08983665ff261f50 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Sat, 1 Jul 2023 11:02:23 -0300 Subject: [PATCH 6/7] Update crates/primitives-currency-swap/src/lib.rs Co-authored-by: Dmitry Lavrenov <39522748+dmitrylavrenov@users.noreply.github.com> --- crates/primitives-currency-swap/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/primitives-currency-swap/src/lib.rs b/crates/primitives-currency-swap/src/lib.rs index 3f303844f..f43653f3f 100644 --- a/crates/primitives-currency-swap/src/lib.rs +++ b/crates/primitives-currency-swap/src/lib.rs @@ -48,6 +48,6 @@ pub type ErrorFor = Error< pub struct Error { /// The underlying cause of this error. pub cause: E, - /// The original imbalances that was passed to the swap operation. + /// The original imbalance that was passed to the swap operation. pub incoming_imbalance: I, } From a69987a06ac5b27a650eeb997714f3940c91d242 Mon Sep 17 00:00:00 2001 From: MOZGIII Date: Sat, 1 Jul 2023 11:40:00 -0300 Subject: [PATCH 7/7] Adjust the bridge-pot-currency-swap --- .../src/existence_optional.rs | 31 +++++++---- .../src/existence_required.rs | 53 +++++++++++++------ 2 files changed, 57 insertions(+), 27 deletions(-) diff --git a/crates/bridge-pot-currency-swap/src/existence_optional.rs b/crates/bridge-pot-currency-swap/src/existence_optional.rs index 27db74dcf..3544bbfda 100644 --- a/crates/bridge-pot-currency-swap/src/existence_optional.rs +++ b/crates/bridge-pot-currency-swap/src/existence_optional.rs @@ -19,19 +19,30 @@ impl primitives_currency_swap::CurrencySwap>::NegativeImbalance, - ) -> Result<>::NegativeImbalance, Self::Error> { - let amount = imbalance.peek(); - - T::CurrencyFrom::resolve_creating(&T::PotFrom::get(), imbalance); - - let imbalance = T::CurrencyTo::withdraw( + incoming_imbalance: >::NegativeImbalance, + ) -> Result< + >::NegativeImbalance, + primitives_currency_swap::ErrorFor, + > { + let amount = incoming_imbalance.peek(); + + let outgoing_imbalance = match T::CurrencyTo::withdraw( &T::PotTo::get(), T::BalanceCoverter::convert(amount), WithdrawReasons::TRANSFER, ExistenceRequirement::AllowDeath, - )?; - - Ok(imbalance) + ) { + Ok(imbalance) => imbalance, + Err(error) => { + return Err(primitives_currency_swap::Error { + cause: error, + incoming_imbalance, + }) + } + }; + + T::CurrencyFrom::resolve_creating(&T::PotFrom::get(), incoming_imbalance); + + Ok(outgoing_imbalance) } } diff --git a/crates/bridge-pot-currency-swap/src/existence_required.rs b/crates/bridge-pot-currency-swap/src/existence_required.rs index 12a954508..25d085f78 100644 --- a/crates/bridge-pot-currency-swap/src/existence_required.rs +++ b/crates/bridge-pot-currency-swap/src/existence_required.rs @@ -12,17 +12,19 @@ pub enum Marker {} /// An error that can occur while doing the swap operation. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum Error { +pub enum Error { /// Unable to resolve the incoming balance into the corresponding pot. - ResolvingIncomingImbalance(ImbalanceFrom), + ResolvingIncomingImbalance, /// Unable to withdraw the outgoing balance from the corresponding pot. IssuingOutgoingImbalance(DispatchError), } -impl From> for DispatchError { - fn from(value: Error) -> Self { +impl From for DispatchError { + fn from(value: Error) -> Self { match value { - Error::ResolvingIncomingImbalance(_) => DispatchError::NoProviders, + Error::ResolvingIncomingImbalance => { + DispatchError::Other("swap pot account does not exist") + } Error::IssuingOutgoingImbalance(err) => err, } } @@ -33,25 +35,42 @@ impl primitives_currency_swap::CurrencySwap::CurrencyFrom as Currency>::NegativeImbalance>; + type Error = Error; fn swap( - imbalance: >::NegativeImbalance, - ) -> Result<>::NegativeImbalance, Self::Error> { - let amount = imbalance.peek(); + incoming_imbalance: >::NegativeImbalance, + ) -> Result< + >::NegativeImbalance, + primitives_currency_swap::ErrorFor, + > { + let amount = incoming_imbalance.peek(); - T::CurrencyFrom::resolve_into_existing(&T::PotFrom::get(), imbalance) - .map_err(Error::ResolvingIncomingImbalance)?; - - let imbalance = T::CurrencyTo::withdraw( + let outgoing_imbalance = match T::CurrencyTo::withdraw( &T::PotTo::get(), T::BalanceCoverter::convert(amount), WithdrawReasons::TRANSFER, ExistenceRequirement::KeepAlive, - ) - .map_err(Error::IssuingOutgoingImbalance)?; + ) { + Ok(imbalance) => imbalance, + Err(error) => { + return Err(primitives_currency_swap::Error { + cause: Error::IssuingOutgoingImbalance(error), + incoming_imbalance, + }); + } + }; + + match T::CurrencyFrom::resolve_into_existing(&T::PotFrom::get(), incoming_imbalance) { + Ok(()) => {} + Err(imbalance) => { + T::CurrencyTo::resolve_creating(&T::PotTo::get(), outgoing_imbalance); + return Err(primitives_currency_swap::Error { + cause: Error::ResolvingIncomingImbalance, + incoming_imbalance: imbalance, + }); + } + } - Ok(imbalance) + Ok(outgoing_imbalance) } }