diff --git a/transaction-multi-payment/Cargo.toml b/transaction-multi-payment/Cargo.toml index effce4b6..ceb2e3d3 100644 --- a/transaction-multi-payment/Cargo.toml +++ b/transaction-multi-payment/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-transaction-multi-payment" -version = "7.1.0" +version = "8.0.0" description = "Transaction multi currency payment support module" authors = ["GalacticCoucil"] edition = "2021" @@ -18,7 +18,7 @@ scale-info = { version = "1.0", default-features = false, features = ["derive"] orml-traits = { git = "https://github.com/open-web3-stack/open-runtime-module-library", rev = "aac79b3b31953381669a2ffa9b3e9bfe48e87f38", default-features = false } # HydraDX traits -hydradx-traits = { path="../traits", default-features = false } +hydradx-traits = { path = "../traits", default-features = false } # Substrate dependencies frame-support = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.17", default-features = false } diff --git a/transaction-multi-payment/src/lib.rs b/transaction-multi-payment/src/lib.rs index 98f70862..eab817b3 100644 --- a/transaction-multi-payment/src/lib.rs +++ b/transaction-multi-payment/src/lib.rs @@ -61,17 +61,16 @@ use frame_support::traits::IsSubType; use scale_info::TypeInfo; -use crate::traits::{CurrencyWithdraw, PaymentWithdrawResult}; +pub use crate::traits::*; use frame_support::dispatch::DispatchError; type AssetIdOf = <::Currencies as MultiCurrency<::AccountId>>::CurrencyId; type BalanceOf = <::Currencies as MultiCurrency<::AccountId>>::Balance; +type NegativeImbalanceOf = ::AccountId>>::NegativeImbalance; /// Spot price type pub type Price = FixedU128; -type NegativeImbalanceOf = ::AccountId>>::NegativeImbalance; - // Re-export pallet items so that they can be accessed from the crate namespace. pub use pallet::*; @@ -137,6 +136,10 @@ pub mod pallet { /// Native Asset #[pallet::constant] type NativeAssetId: Get>; + + /// Account where fees are deposited + #[pallet::constant] + type FeeReceiver: Get; } #[pallet::event] @@ -190,9 +193,6 @@ pub mod pallet { /// Math overflow Overflow, - - /// Fallback account is not set. - FallbackAccountNotSet, } /// Account currency map @@ -200,25 +200,19 @@ pub mod pallet { #[pallet::getter(fn get_currency)] pub type AccountCurrencyMap = StorageMap<_, Blake2_128Concat, T::AccountId, AssetIdOf, OptionQuery>; - /// Curated list of currencies which fees can be paid with + /// Curated list of currencies which fees can be paid mapped to corresponding fallback price #[pallet::storage] #[pallet::getter(fn currencies)] pub type AcceptedCurrencies = StorageMap<_, Twox64Concat, AssetIdOf, Price, OptionQuery>; - /// Block storage for accepted currency price + /// Asset prices from the spot price provider or the fallback price if the price is not available. Updated at the beginning of every block. #[pallet::storage] #[pallet::getter(fn currency_price)] pub type AcceptedCurrencyPrice = StorageMap<_, Twox64Concat, AssetIdOf, Price, OptionQuery>; - /// Account to use when pool does not exist. - #[pallet::storage] - #[pallet::getter(fn fallback_account)] - pub type FallbackAccount = StorageValue<_, T::AccountId, OptionQuery>; - #[pallet::genesis_config] pub struct GenesisConfig { pub currencies: Vec<(AssetIdOf, Price)>, - pub fallback_account: Option, pub account_currencies: Vec<(T::AccountId, AssetIdOf)>, } @@ -227,7 +221,6 @@ pub mod pallet { fn default() -> Self { GenesisConfig { currencies: vec![], - fallback_account: None, account_currencies: vec![], } } @@ -236,12 +229,6 @@ pub mod pallet { #[pallet::genesis_build] impl GenesisBuild for GenesisConfig { fn build(&self) { - if self.fallback_account == None { - panic!("Fallback account is not set"); - } - - FallbackAccount::::put(self.fallback_account.clone().expect("Fallback account is not set")); - for (asset, price) in &self.currencies { AcceptedCurrencies::::insert(asset, price); } @@ -279,7 +266,7 @@ pub mod pallet { >::insert(who.clone(), currency); if T::WithdrawFeeForSetCurrency::get() == Pays::Yes { - Self::withdraw_set_fee(&who)?; + Self::transfer_set_fee(&who)?; } Self::deposit_event(Event::CurrencySet { @@ -352,15 +339,52 @@ where Pallet::::get_currency(who).unwrap_or_else(T::NativeAssetId::get) } - /// Execute a trade to buy HDX and sell selected currency. - pub fn withdraw_fee_non_native( - who: &T::AccountId, - fee: BalanceOf, - ) -> Result { - let currency = Self::account_currency(who); + /// Transfer fee without executing an AMM trade + pub fn transfer_set_fee(who: &T::AccountId) -> DispatchResult { + let base_fee = Self::weight_to_fee(T::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic); + let adjusted_weight_fee = Self::weight_to_fee(T::WeightInfo::set_currency()); + let fee = base_fee.saturating_add(adjusted_weight_fee); + let (currency, maybe_price) = Self::get_currency_and_price(who)?; + + let amount = match maybe_price { + None => fee, + Some(price) => price.checked_mul_int(fee).ok_or(Error::::Overflow)?, + }; + + T::Currencies::transfer(currency, who, &T::FeeReceiver::get(), amount)?; + + Self::deposit_event(Event::FeeWithdrawn { + account_id: who.clone(), + asset_id: currency, + native_fee_amount: fee, + non_native_fee_amount: amount, + destination_account_id: T::FeeReceiver::get(), + }); + + Ok(()) + } + + fn weight_to_fee(weight: Weight) -> BalanceOf { + // cap the weight to the maximum defined in runtime, otherwise it will be the + // `Bounded` maximum of its data type, which is not desired. + let capped_weight: Weight = weight.min(T::BlockWeights::get().max_block); + ::WeightToFee::calc(&capped_weight) + } + + fn check_balance(account: &T::AccountId, currency: AssetIdOf) -> Result<(), Error> { + if T::Currencies::free_balance(currency, account) == BalanceOf::::zero() { + return Err(Error::::ZeroBalance); + }; + Ok(()) + } + fn get_currency_and_price( + who: &::AccountId, + ) -> Result<(AssetIdOf, Option), DispatchError> { + let native_currency = T::NativeAssetId::get(); + let currency = Self::account_currency(who); if currency == T::NativeAssetId::get() { - Ok(PaymentWithdrawResult::Native) + Ok((native_currency, None)) } else { let price = if let Some(spot_price) = Self::currency_price(currency) { spot_price @@ -374,67 +398,144 @@ where } }; - let amount = price.checked_mul_int(fee).ok_or(Error::::Overflow)?; - - T::Currencies::transfer( - currency, - who, - &Self::fallback_account().ok_or(Error::::FallbackAccountNotSet)?, - amount, - )?; - - Self::deposit_event(Event::FeeWithdrawn { - account_id: who.clone(), - asset_id: currency, - native_fee_amount: fee, - non_native_fee_amount: amount, - destination_account_id: Self::fallback_account().ok_or(Error::::FallbackAccountNotSet)?, - }); - - Ok(PaymentWithdrawResult::Transferred) + Ok((currency, Some(price))) } } +} - pub fn withdraw_set_fee(who: &T::AccountId) -> DispatchResult { - let base_fee = Self::weight_to_fee(T::BlockWeights::get().get(DispatchClass::Normal).base_extrinsic); - let adjusted_weight_fee = Self::weight_to_fee(T::WeightInfo::set_currency()); - let fee = base_fee.saturating_add(adjusted_weight_fee); - - let result = Self::withdraw(who, fee)?; - match result { - PaymentWithdrawResult::Transferred => Ok(()), - PaymentWithdrawResult::Native => T::Currencies::withdraw(T::NativeAssetId::get(), who, fee), - } +impl TransactionMultiPaymentDataProvider<::AccountId, AssetIdOf, Price> + for Pallet +where + BalanceOf: FixedPointOperand, +{ + fn get_currency_and_price( + who: &::AccountId, + ) -> Result<(AssetIdOf, Option), DispatchError> { + Self::get_currency_and_price(who) } - fn weight_to_fee(weight: Weight) -> BalanceOf { - // cap the weight to the maximum defined in runtime, otherwise it will be the - // `Bounded` maximum of its data type, which is not desired. - let capped_weight: Weight = weight.min(T::BlockWeights::get().max_block); - ::WeightToFee::calc(&capped_weight) + fn get_fee_receiver() -> ::AccountId { + T::FeeReceiver::get() } +} - fn check_balance(account: &T::AccountId, currency: AssetIdOf) -> Result<(), Error> { - if T::Currencies::free_balance(currency, account) == BalanceOf::::zero() { - return Err(Error::::ZeroBalance); - }; +/// Deposits all fees to some account +pub struct DepositAll(PhantomData); + +impl DepositFee, BalanceOf> for DepositAll { + fn deposit_fee(who: &T::AccountId, currency: AssetIdOf, amount: BalanceOf) -> DispatchResult { + ::Currencies::deposit(currency, who, amount)?; Ok(()) } } -impl CurrencyWithdraw<::AccountId, BalanceOf> for Pallet +/// Implements the transaction payment for native as well as non-native currencies +pub struct TransferFees(PhantomData<(MC, DP, DF)>); + +impl OnChargeTransaction for TransferFees where - BalanceOf: FixedPointOperand, + T: Config, + MC: MultiCurrency<::AccountId>, + AssetIdOf: Into, + MC::Balance: FixedPointOperand, + DP: TransactionMultiPaymentDataProvider, Price>, + DF: DepositFee, { - fn withdraw(who: &T::AccountId, fee: BalanceOf) -> Result { - Self::withdraw_fee_non_native(who, fee) + type LiquidityInfo = Option, Price>>; + type Balance = ::AccountId>>::Balance; + + /// Withdraw the predicted fee from the transaction origin. + /// + /// Note: The `fee` already includes the `tip`. + fn withdraw_fee( + who: &T::AccountId, + _call: &T::Call, + _info: &DispatchInfoOf, + fee: Self::Balance, + _tip: Self::Balance, + ) -> Result { + if fee.is_zero() { + return Ok(None); + } + // get the currency in which fees are paid. In case of non-native currency, the price is required to calculate final fee. + let currency_data = DP::get_currency_and_price(who) + .map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Payment))?; + + match currency_data { + (_, None) => match MC::withdraw(T::NativeAssetId::get().into(), who, fee) { + Ok(()) => Ok(Some(PaymentInfo::Native(fee))), + Err(_) => Err(InvalidTransaction::Payment.into()), + }, + (currency, Some(price)) => { + let converted_fee = price + .checked_mul_int(fee) + .ok_or(TransactionValidityError::Invalid(InvalidTransaction::Payment))?; + match MC::withdraw(currency.into(), who, converted_fee) { + Ok(()) => Ok(Some(PaymentInfo::NonNative(converted_fee, currency, price))), + Err(_) => Err(InvalidTransaction::Payment.into()), + } + } + } + } + + /// Since the predicted fee might have been too high, parts of the fee may + /// be refunded. + /// + /// Note: The `fee` already includes the `tip`. + fn correct_and_deposit_fee( + who: &T::AccountId, + _dispatch_info: &DispatchInfoOf, + _post_info: &PostDispatchInfoOf, + corrected_fee: Self::Balance, + tip: Self::Balance, + already_withdrawn: Self::LiquidityInfo, + ) -> Result<(), TransactionValidityError> { + let fee_receiver = DP::get_fee_receiver(); + + if let Some(paid) = already_withdrawn { + // Calculate how much refund we should return + let (currency, refund, fee, tip) = match paid { + PaymentInfo::Native(paid_fee) => ( + T::NativeAssetId::get().into(), + paid_fee.saturating_sub(corrected_fee), + corrected_fee.saturating_sub(tip), + tip, + ), + PaymentInfo::NonNative(paid_fee, currency, price) => { + // calculate corrected_fee in the non-native currency + let converted_corrected_fee = price + .checked_mul_int(corrected_fee) + .ok_or(TransactionValidityError::Invalid(InvalidTransaction::Payment))?; + let refund = paid_fee.saturating_sub(converted_corrected_fee); + let converted_tip = price + .checked_mul_int(tip) + .ok_or(TransactionValidityError::Invalid(InvalidTransaction::Payment))?; + ( + currency.into(), + refund, + converted_corrected_fee.saturating_sub(converted_tip), + converted_tip, + ) + } + }; + + // refund to the account that paid the fees + MC::deposit(currency, who, refund) + .map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Payment))?; + + // deposit the fee + DF::deposit_fee(&fee_receiver, currency, fee + tip) + .map_err(|_| TransactionValidityError::Invalid(InvalidTransaction::Payment))?; + } + + Ok(()) } } /// Implements the transaction payment for native as well as non-native currencies -pub struct MultiCurrencyAdapter(PhantomData<(C, OU, SW)>); +pub struct WithdrawFees(PhantomData<(C, OU, SW)>); -impl OnChargeTransaction for MultiCurrencyAdapter +impl OnChargeTransaction for WithdrawFees where T: Config, T::TransactionByteFee: Get<::AccountId>>::Balance>, diff --git a/transaction-multi-payment/src/mock.rs b/transaction-multi-payment/src/mock.rs index 25a0c201..e647d158 100644 --- a/transaction-multi-payment/src/mock.rs +++ b/transaction-multi-payment/src/mock.rs @@ -17,14 +17,14 @@ use super::*; pub use crate as multi_payment; -use crate::{Config, MultiCurrencyAdapter}; +use crate::{Config, TransferFees}; use frame_support::{parameter_types, weights::DispatchClass}; use frame_system as system; use orml_traits::parameter_type_with_key; use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, IdentityLookup, One}, + traits::{BlakeTwo256, IdentityLookup}, Perbill, }; @@ -46,7 +46,7 @@ pub const INITIAL_BALANCE: Balance = 1_000_000_000_000_000u128; pub const ALICE: AccountId = 1; pub const BOB: AccountId = 2; -pub const FALLBACK_ACCOUNT: AccountId = 300; +pub const FEE_RECEIVER: AccountId = 300; pub const HDX: AssetId = 0; pub const SUPPORTED_CURRENCY: AssetId = 2000; @@ -92,10 +92,11 @@ parameter_types! { pub const SS58Prefix: u8 = 63; pub const HdxAssetId: u32 = 0; - pub const ExistentialDeposit: u128 = 1; + pub const ExistentialDeposit: u128 = 2; pub const MaxLocks: u32 = 50; pub const TransactionByteFee: Balance = 1; pub const RegistryStringLimit: u32 = 100; + pub const FeeReceiver: AccountId = FEE_RECEIVER; pub RuntimeBlockWeights: system::limits::BlockWeights = system::limits::BlockWeights::builder() .base_block(10) @@ -154,6 +155,7 @@ impl Config for Test { type WithdrawFeeForSetCurrency = PayForSetCurrency; type WeightToFee = IdentityFee; type NativeAssetId = HdxAssetId; + type FeeReceiver = FeeReceiver; } impl pallet_balances::Config for Test { @@ -171,7 +173,7 @@ impl pallet_balances::Config for Test { } impl pallet_transaction_payment::Config for Test { - type OnChargeTransaction = MultiCurrencyAdapter; + type OnChargeTransaction = TransferFees>; type TransactionByteFee = TransactionByteFee; type OperationalFeeMultiplier = (); type WeightToFee = IdentityFee; @@ -209,7 +211,7 @@ impl SpotPriceProvider for SpotPrice { parameter_type_with_key! { pub ExistentialDeposits: |_currency_id: AssetId| -> Balance { - One::one() + 2 }; } @@ -305,7 +307,6 @@ impl ExtBuilder { (SUPPORTED_CURRENCY, Price::from_float(1.5)), (SUPPORTED_CURRENCY_WITH_PRICE, Price::from_float(0.5)), ], - fallback_account: Some(FALLBACK_ACCOUNT), account_currencies: self.account_currencies, } .assimilate_storage(&mut t) diff --git a/transaction-multi-payment/src/tests.rs b/transaction-multi-payment/src/tests.rs index 6317f458..32e0046e 100644 --- a/transaction-multi-payment/src/tests.rs +++ b/transaction-multi-payment/src/tests.rs @@ -15,16 +15,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -pub use crate::{mock::*, Error}; -use frame_support::{assert_noop, assert_ok, assert_storage_noop}; +pub use crate::{mock::*, Config, Error}; +use frame_support::{assert_err, assert_noop, assert_ok}; use pallet_transaction_payment::ChargeTransactionPayment; use sp_runtime::traits::SignedExtension; -use crate::traits::{CurrencyWithdraw, PaymentWithdrawResult}; -use crate::CurrencyBalanceCheck; -use crate::Price; +use crate::traits::TransactionMultiPaymentDataProvider; +use crate::{CurrencyBalanceCheck, PaymentInfo, Price}; use frame_support::sp_runtime::transaction_validity::{InvalidTransaction, ValidTransaction}; -use frame_support::weights::DispatchInfo; +use frame_support::weights::{DispatchInfo, PostDispatchInfo, Weight}; use orml_traits::MultiCurrency; use pallet_balances::Call as BalancesCall; use sp_runtime::traits::BadOrigin; @@ -55,7 +54,7 @@ fn set_supported_currency_without_spot_price() { Currencies::free_balance(SUPPORTED_CURRENCY, &ALICE), 999_999_999_998_457 ); - assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &FALLBACK_ACCOUNT), 1_543); + assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &FEE_RECEIVER), 1_543); }); } @@ -142,10 +141,8 @@ fn fee_payment_in_native_currency() { .build() .execute_with(|| { let len = 10; - let info = DispatchInfo { - weight: 5, - ..Default::default() - }; + let info = info_from_weight(5); + assert!(ChargeTransactionPayment::::from(0) .pre_dispatch(&CHARLIE, CALL, &info, len) .is_ok()); @@ -154,28 +151,6 @@ fn fee_payment_in_native_currency() { }); } -#[test] -fn fee_payment_in_native_currency_with_no_balance() { - const CHARLIE: AccountId = 5; - - ExtBuilder::default() - .base_weight(5) - .account_native_balance(CHARLIE, 10) - .build() - .execute_with(|| { - let len = 10; - let info = DispatchInfo { - weight: 5, - ..Default::default() - }; - assert!(ChargeTransactionPayment::::from(0) - .pre_dispatch(&CHARLIE, CALL, &info, len) - .is_err()); - - assert_eq!(Balances::free_balance(CHARLIE), 10); - }); -} - #[test] fn fee_payment_in_non_native_currency() { const CHARLIE: AccountId = 5; @@ -190,10 +165,7 @@ fn fee_payment_in_non_native_currency() { assert_eq!(Balances::free_balance(CHARLIE), 0); let len = 1000; - let info = DispatchInfo { - weight: 5, - ..Default::default() - }; + let info = info_from_weight(5); assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY_WITH_PRICE, &CHARLIE), 10_000); @@ -219,10 +191,7 @@ fn fee_payment_non_native_insufficient_balance() { .build() .execute_with(|| { let len = 1000; - let info = DispatchInfo { - weight: 5, - ..Default::default() - }; + let info = info_from_weight(5); assert!(ChargeTransactionPayment::::from(0) .pre_dispatch(&CHARLIE, CALL, &info, len) @@ -273,63 +242,6 @@ fn removed_accepted_currency() { }); } -#[test] -fn fee_payment_in_non_native_currency_with_no_price() { - const CHARLIE: AccountId = 5; - - ExtBuilder::default() - .base_weight(5) - .account_tokens(CHARLIE, SUPPORTED_CURRENCY, 10_000) - .with_currencies(vec![(CHARLIE, SUPPORTED_CURRENCY)]) - .build() - .execute_with(|| { - // Make sure Charlie ain't got a penny! - assert_eq!(Balances::free_balance(CHARLIE), 0); - - let len = 10; - let info = DispatchInfo { - weight: 5, - ..Default::default() - }; - - assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &FALLBACK_ACCOUNT), 0); - - assert!(ChargeTransactionPayment::::from(0) - .pre_dispatch(&CHARLIE, CALL, &info, len) - .is_ok()); - - //Native balance check - Charlie should be still broke! - assert_eq!(Balances::free_balance(CHARLIE), 0); - - assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 9970); - assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &FALLBACK_ACCOUNT), 30); - }); -} - -#[test] -fn fee_payment_non_native_insufficient_balance_with_no_pool() { - const CHARLIE: AccountId = 5; - - ExtBuilder::default() - .base_weight(5) - .account_tokens(CHARLIE, SUPPORTED_CURRENCY, 100) - .with_currencies(vec![(CHARLIE, SUPPORTED_CURRENCY)]) - .build() - .execute_with(|| { - let len = 1000; - let info = DispatchInfo { - weight: 5, - ..Default::default() - }; - - assert!(ChargeTransactionPayment::::from(0) - .pre_dispatch(&CHARLIE, CALL, &info, len) - .is_err()); - - assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 100); - }); -} - #[test] fn check_balance_extension_works() { const CHARLIE: AccountId = 5; @@ -391,43 +303,44 @@ fn account_currency_works() { } #[test] -fn withdraw_currency_should_work() { - ExtBuilder::default().base_weight(5).build().execute_with(|| { - assert_storage_noop!(PaymentPallet::withdraw_fee_non_native(&ALICE, 10000).unwrap()); - - assert_ok!(PaymentPallet::set_currency( - Origin::signed(ALICE), - SUPPORTED_CURRENCY_WITH_PRICE - )); - - assert_ok!(PaymentPallet::withdraw_fee_non_native(&ALICE, 10000)); - +fn data_provider_works() { + ExtBuilder::default().build().execute_with(|| { + assert_eq!(PaymentPallet::get_fee_receiver(), FEE_RECEIVER); assert_eq!( - 999999999998898, - Currencies::free_balance(SUPPORTED_CURRENCY_WITH_PRICE, &ALICE) + PaymentPallet::get_currency_and_price(&ALICE), + Ok((::NativeAssetId::get(), None)) ); + assert_ok!(PaymentPallet::set_currency(Origin::signed(ALICE), SUPPORTED_CURRENCY)); assert_eq!( - 1102, - Currencies::free_balance( - SUPPORTED_CURRENCY_WITH_PRICE, - &PaymentPallet::fallback_account().unwrap() - ) + PaymentPallet::get_currency_and_price(&ALICE), + Ok((SUPPORTED_CURRENCY, Some(Price::from_float(1.5)))) + ); + + assert_ok!(PaymentPallet::remove_currency(Origin::root(), SUPPORTED_CURRENCY)); + assert_err!( + PaymentPallet::get_currency_and_price(&ALICE), + Error::::FallbackPriceNotFound ); }); } #[test] -fn withdraw_set_fee_with_core_asset_should_work() { +fn transfer_set_fee_with_core_asset_should_work() { ExtBuilder::default().base_weight(5).build().execute_with(|| { + let fb_account = ::FeeReceiver::get(); let hdx_balance_before = Currencies::free_balance(HDX, &ALICE); - assert_ok!(PaymentPallet::withdraw_set_fee(&ALICE)); + let fb_balance_before = Currencies::free_balance(HDX, &fb_account); + + assert_ok!(PaymentPallet::transfer_set_fee(&ALICE)); + assert_eq!(hdx_balance_before - 1029, Currencies::free_balance(HDX, &ALICE)); + assert_eq!(fb_balance_before + 1029, Currencies::free_balance(HDX, &fb_account)); }); } #[test] -fn withdraw_set_fee_should_work() { +fn transfer_set_fee_should_work() { ExtBuilder::default().base_weight(5).build().execute_with(|| { assert_ok!(PaymentPallet::set_currency( Origin::signed(ALICE), @@ -435,22 +348,17 @@ fn withdraw_set_fee_should_work() { )); let balance_before = Currencies::free_balance(SUPPORTED_CURRENCY_WITH_PRICE, &ALICE); - let fb_acc_balance_before = Currencies::free_balance( - SUPPORTED_CURRENCY_WITH_PRICE, - &PaymentPallet::fallback_account().unwrap(), - ); + let fb_acc_balance_before = + Currencies::free_balance(SUPPORTED_CURRENCY_WITH_PRICE, &::FeeReceiver::get()); - assert_ok!(PaymentPallet::withdraw_set_fee(&ALICE)); + assert_ok!(PaymentPallet::transfer_set_fee(&ALICE)); assert_eq!( balance_before - 102, Currencies::free_balance(SUPPORTED_CURRENCY_WITH_PRICE, &ALICE) ); assert_eq!( fb_acc_balance_before + 102, - Currencies::free_balance( - SUPPORTED_CURRENCY_WITH_PRICE, - &PaymentPallet::fallback_account().unwrap() - ) + Currencies::free_balance(SUPPORTED_CURRENCY_WITH_PRICE, &::FeeReceiver::get()) ); }); } @@ -479,34 +387,395 @@ fn check_balance_should_work() { }); } +/// create a transaction info struct from weight. Handy to avoid building the whole struct. +pub fn info_from_weight(w: Weight) -> DispatchInfo { + // pays_fee: Pays::Yes -- class: DispatchClass::Normal + DispatchInfo { + weight: w, + ..Default::default() + } +} + +fn post_info_from_weight(w: Weight) -> PostDispatchInfo { + PostDispatchInfo { + actual_weight: Some(w), + pays_fee: Default::default(), + } +} + +fn default_post_info() -> PostDispatchInfo { + PostDispatchInfo { + actual_weight: None, + pays_fee: Default::default(), + } +} + #[test] -fn withdraw_with_price_should_work() { - ExtBuilder::default().base_weight(5).build().execute_with(|| { - assert_eq!( - PaymentPallet::withdraw(&ALICE, 1000).unwrap(), - PaymentWithdrawResult::Native - ); +fn fee_payment_in_native_currency_work() { + const CHARLIE: AccountId = 5; - assert_ok!(PaymentPallet::set_currency( - Origin::signed(ALICE), - SUPPORTED_CURRENCY_WITH_PRICE - )); - assert_eq!( - PaymentPallet::withdraw(&ALICE, 1000).unwrap(), - PaymentWithdrawResult::Transferred - ); - }); + ExtBuilder::default() + .account_native_balance(CHARLIE, 100) + .base_weight(5) + .build() + .execute_with(|| { + let len = 10; + let tip = 0; + let dispatch_info = info_from_weight(15); + + let pre = ChargeTransactionPayment::::from(tip) + .pre_dispatch(&CHARLIE, CALL, &dispatch_info, len) + .unwrap(); + assert_eq!(pre, (tip, CHARLIE, Some(PaymentInfo::Native(5 + 15 + 10)))); + + assert_eq!(Balances::free_balance(CHARLIE), 100 - 30); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 0); + + assert_ok!(ChargeTransactionPayment::::post_dispatch( + Some(pre), + &dispatch_info, + &default_post_info(), + len, + &Ok(()) + )); + assert_eq!(Balances::free_balance(CHARLIE), 100 - 30); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 30); + }); } #[test] -fn withdraw_should_not_work() { - ExtBuilder::default().base_weight(5).build().execute_with(|| { - assert_ok!(PaymentPallet::set_currency(Origin::signed(ALICE), SUPPORTED_CURRENCY)); +fn fee_payment_in_native_currency_with_tip_work() { + const CHARLIE: AccountId = 5; - assert_ok!(PaymentPallet::remove_currency(Origin::root(), SUPPORTED_CURRENCY)); - assert_noop!( - PaymentPallet::withdraw(&ALICE, 1000), - Error::::FallbackPriceNotFound - ); - }); + ExtBuilder::default() + .account_native_balance(CHARLIE, 100) + .base_weight(5) + .build() + .execute_with(|| { + let len = 10; + let tip = 5; + let dispatch_info = info_from_weight(15); + let post_dispatch_info = post_info_from_weight(10); + let pre = ChargeTransactionPayment::::from(tip) + .pre_dispatch(&CHARLIE, CALL, &dispatch_info, len) + .unwrap(); + assert_eq!(pre, (tip, CHARLIE, Some(PaymentInfo::Native(5 + 15 + 10 + tip)))); + + assert_eq!(Balances::free_balance(CHARLIE), 100 - 5 - 10 - 15 - tip); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 0); + + assert_ok!(ChargeTransactionPayment::::post_dispatch( + Some(pre), + &dispatch_info, + &post_dispatch_info, + len, + &Ok(()) + )); + + assert_eq!(Balances::free_balance(CHARLIE), 100 - 5 - 10 - 10 - tip); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 30); + }); +} + +#[test] +fn fee_payment_in_non_native_currency_work() { + const CHARLIE: AccountId = 5; + + ExtBuilder::default() + .with_currencies(vec![(CHARLIE, SUPPORTED_CURRENCY)]) + .account_tokens(CHARLIE, SUPPORTED_CURRENCY, 10_000) + .base_weight(5) + .build() + .execute_with(|| { + let len = 10; + let tip = 0; + let dispatch_info = info_from_weight(15); + + let pre = ChargeTransactionPayment::::from(tip) + .pre_dispatch(&CHARLIE, CALL, &dispatch_info, len) + .unwrap(); + assert_eq!( + pre, + ( + tip, + CHARLIE, + Some(PaymentInfo::NonNative(45, SUPPORTED_CURRENCY, Price::from_float(1.5))) + ) + ); + + assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 10_000 - 45); + assert_eq!( + Currencies::free_balance(SUPPORTED_CURRENCY, &::FeeReceiver::get()), + 0 + ); + assert_eq!(Balances::free_balance(CHARLIE), 0); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 0); + + assert_ok!(ChargeTransactionPayment::::post_dispatch( + Some(pre), + &dispatch_info, + &default_post_info(), + len, + &Ok(()) + )); + assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 10_000 - 45); + assert_eq!( + Currencies::free_balance(SUPPORTED_CURRENCY, &::FeeReceiver::get()), + 45 + ); + assert_eq!(Balances::free_balance(CHARLIE), 0); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 0); + }); +} + +#[test] +fn fee_payment_in_non_native_currency_with_tip_work() { + const CHARLIE: AccountId = 5; + + ExtBuilder::default() + .with_currencies(vec![(CHARLIE, SUPPORTED_CURRENCY)]) + .account_tokens(CHARLIE, SUPPORTED_CURRENCY, 10_000) + .base_weight(5) + .build() + .execute_with(|| { + let len = 10; + let tip = 5; + let dispatch_info = info_from_weight(15); + let post_dispatch_info = post_info_from_weight(10); + + let pre = ChargeTransactionPayment::::from(tip) + .pre_dispatch(&CHARLIE, CALL, &dispatch_info, len) + .unwrap(); + assert_eq!( + pre, + ( + tip, + CHARLIE, + Some(PaymentInfo::NonNative(52, SUPPORTED_CURRENCY, Price::from_float(1.5))) + ) + ); + + assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 10_000 - 52); + assert_eq!( + Currencies::free_balance(SUPPORTED_CURRENCY, &::FeeReceiver::get()), + 0 + ); + assert_eq!(Balances::free_balance(CHARLIE), 0); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 0); + + assert_ok!(ChargeTransactionPayment::::post_dispatch( + Some(pre), + &dispatch_info, + &post_dispatch_info, + len, + &Ok(()) + )); + + assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 10_000 - 45); + assert_eq!( + Currencies::free_balance(SUPPORTED_CURRENCY, &::FeeReceiver::get()), + 45 + ); + assert_eq!(Balances::free_balance(CHARLIE), 0); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 0); + }); +} + +#[test] +fn fee_payment_in_native_currency_with_no_balance() { + const CHARLIE: AccountId = 5; + + ExtBuilder::default() + .base_weight(5) + .account_native_balance(CHARLIE, 10) + .build() + .execute_with(|| { + let len = 10; + let info = info_from_weight(5); + + assert!(ChargeTransactionPayment::::from(0) + .pre_dispatch(&CHARLIE, CALL, &info, len) + .is_err()); + + assert_eq!(Balances::free_balance(CHARLIE), 10); + assert_eq!(Balances::free_balance(::FeeReceiver::get()), 0); + }); +} + +#[test] +fn fee_payment_in_non_native_currency_with_no_balance() { + const CHARLIE: AccountId = 5; + + ExtBuilder::default() + .base_weight(5) + .account_tokens(CHARLIE, SUPPORTED_CURRENCY, 100) + .with_currencies(vec![(CHARLIE, SUPPORTED_CURRENCY)]) + .build() + .execute_with(|| { + let len = 1000; + let info = info_from_weight(5); + + assert!(ChargeTransactionPayment::::from(0) + .pre_dispatch(&CHARLIE, CALL, &info, len) + .is_err()); + + assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 100); + assert_eq!( + Tokens::free_balance(SUPPORTED_CURRENCY, &::FeeReceiver::get()), + 0 + ); + }); +} + +#[test] +fn fee_payment_in_non_native_currency_with_no_price() { + const CHARLIE: AccountId = 5; + + ExtBuilder::default() + .base_weight(5) + .account_tokens(CHARLIE, SUPPORTED_CURRENCY, 10_000) + .with_currencies(vec![(CHARLIE, SUPPORTED_CURRENCY)]) + .build() + .execute_with(|| { + // Make sure Charlie ain't got a penny! + assert_eq!(Balances::free_balance(CHARLIE), 0); + + let len = 10; + let info = info_from_weight(5); + + assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &FEE_RECEIVER), 0); + + assert!(ChargeTransactionPayment::::from(0) + .pre_dispatch(&CHARLIE, CALL, &info, len) + .is_ok()); + + //Native balance check - Charlie should be still broke! + assert_eq!(Balances::free_balance(CHARLIE), 0); + + assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 9970); + assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &FEE_RECEIVER), 0); + }); +} + +#[test] +fn fee_payment_in_unregistered_currency() { + const CHARLIE: AccountId = 5; + + ExtBuilder::default() + .base_weight(5) + .account_tokens(CHARLIE, SUPPORTED_CURRENCY, 100) + .with_currencies(vec![(CHARLIE, SUPPORTED_CURRENCY)]) + .build() + .execute_with(|| { + let len = 1000; + let info = info_from_weight(5); + + assert_ok!(PaymentPallet::remove_currency(Origin::root(), SUPPORTED_CURRENCY)); + + assert!(ChargeTransactionPayment::::from(0) + .pre_dispatch(&CHARLIE, CALL, &info, len) + .is_err()); + + assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 100); + }); +} + +#[test] +fn fee_payment_non_native_insufficient_balance_with_no_pool() { + const CHARLIE: AccountId = 5; + + ExtBuilder::default() + .base_weight(5) + .account_tokens(CHARLIE, SUPPORTED_CURRENCY, 100) + .with_currencies(vec![(CHARLIE, SUPPORTED_CURRENCY)]) + .build() + .execute_with(|| { + let len = 1000; + let info = info_from_weight(5); + + assert!(ChargeTransactionPayment::::from(0) + .pre_dispatch(&CHARLIE, CALL, &info, len) + .is_err()); + + assert_eq!(Tokens::free_balance(SUPPORTED_CURRENCY, &CHARLIE), 100); + }); +} + +#[test] +fn fee_in_native_can_kill_account() { + const CHARLIE: AccountId = 5; + + ExtBuilder::default() + .account_native_balance(CHARLIE, 30) + .base_weight(5) + .build() + .execute_with(|| { + let len = 10; + let tip = 0; + let dispatch_info = info_from_weight(15); + + assert_eq!(Balances::free_balance(FEE_RECEIVER), 0); + + let pre = ChargeTransactionPayment::::from(0) + .pre_dispatch(&CHARLIE, CALL, &dispatch_info, len) + .unwrap(); + assert_eq!(pre, (tip, CHARLIE, Some(PaymentInfo::Native(30)))); + + assert_eq!(Balances::free_balance(CHARLIE), 0); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 0); + + assert_ok!(ChargeTransactionPayment::::post_dispatch( + Some(pre), + &dispatch_info, + &default_post_info(), + len, + &Ok(()) + )); + + // zero balance indicates that the account can be killed + assert_eq!(Balances::free_balance(CHARLIE), 0); + assert_eq!(Balances::free_balance(FEE_RECEIVER), 30); + }); +} + +#[test] +fn fee_in_non_native_can_kill_account() { + ExtBuilder::default() + .with_currencies(vec![(ALICE, SUPPORTED_CURRENCY)]) + .base_weight(5) + .build() + .execute_with(|| { + let len = 10; + let tip = 0; + let dispatch_info = info_from_weight(15); + + assert_ok!(Currencies::withdraw(SUPPORTED_CURRENCY, &ALICE, INITIAL_BALANCE - 45)); + + let pre = ChargeTransactionPayment::::from(tip) + .pre_dispatch(&ALICE, CALL, &dispatch_info, len) + .unwrap(); + assert_eq!( + pre, + ( + tip, + ALICE, + Some(PaymentInfo::NonNative(45, SUPPORTED_CURRENCY, Price::from_float(1.5))) + ) + ); + + assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &ALICE), 0); + assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &FEE_RECEIVER), 0); + + assert_ok!(ChargeTransactionPayment::::post_dispatch( + Some(pre), + &dispatch_info, + &default_post_info(), + len, + &Ok(()) + )); + + // zero balance indicates that the account can be killed + assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &ALICE), 0); + assert_eq!(Currencies::free_balance(SUPPORTED_CURRENCY, &FEE_RECEIVER), 45); + }); } diff --git a/transaction-multi-payment/src/traits.rs b/transaction-multi-payment/src/traits.rs index b66ffd5b..9f0afef3 100644 --- a/transaction-multi-payment/src/traits.rs +++ b/transaction-multi-payment/src/traits.rs @@ -1,3 +1,20 @@ +use frame_support::sp_runtime::{DispatchError, DispatchResult}; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PaymentInfo { + Native(Balance), + NonNative(Balance, AssetId, Price), +} + +/// Helper method for providing some data that are needed in OnChargeTransaction +pub trait TransactionMultiPaymentDataProvider { + /// Get a fee currency set by an account and its price + fn get_currency_and_price(who: &AccountId) -> Result<(AssetId, Option), DispatchError>; + + /// Returns the account where fees are deposited + fn get_fee_receiver() -> AccountId; +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum PaymentWithdrawResult { Native, @@ -5,8 +22,10 @@ pub enum PaymentWithdrawResult { } pub trait CurrencyWithdraw { - fn withdraw( - who: &AccountId, - fee: Balance, - ) -> Result; + fn withdraw(who: &AccountId, fee: Balance) -> Result; +} + +/// Handler for dealing with fees +pub trait DepositFee { + fn deposit_fee(who: &AccountId, currency: AssetId, amount: Balance) -> DispatchResult; }