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) } } diff --git a/crates/precompile-currency-swap/src/lib.rs b/crates/precompile-currency-swap/src/lib.rs index 34ad450a8..a47b8aff6 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 withdrawal 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(); diff --git a/crates/primitives-currency-swap/src/lib.rs b/crates/primitives-currency-swap/src/lib.rs index 348d644c1..f43653f3f 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 declaration 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 underlying cause of this error. + pub cause: E, + /// The original imbalance that was passed to the swap operation. + pub incoming_imbalance: I, }