From d4b72a696766fd5715b5a936b798b00b10374601 Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Sun, 8 May 2022 23:53:41 +0400 Subject: [PATCH] make fee handler generic --- payments/src/lib.rs | 46 ++++++++++++++----------------------------- payments/src/mock.rs | 26 ++++++++++++++++-------- payments/src/tests.rs | 38 +++++++++++++++++------------------ payments/src/types.rs | 13 +++++++----- 4 files changed, 60 insertions(+), 63 deletions(-) diff --git a/payments/src/lib.rs b/payments/src/lib.rs index c2175e421..fe1feffa1 100644 --- a/payments/src/lib.rs +++ b/payments/src/lib.rs @@ -76,7 +76,7 @@ pub mod pallet { storage::bounded_btree_map::BoundedBTreeMap, traits::tokens::BalanceStatus, transactional, }; use frame_system::pallet_prelude::*; - use orml_traits::{LockIdentifier, MultiCurrency, NamedMultiReservableCurrency}; + use orml_traits::{arithmetic::Zero, LockIdentifier, MultiCurrency, NamedMultiReservableCurrency}; use sp_runtime::{ traits::{CheckedAdd, Saturating}, Percent, @@ -578,14 +578,12 @@ pub mod pallet { incentive_amount, state: payment_state, resolver_account: T::DisputeResolver::get_resolver_account(), - fee_detail: None, + fee_amount: Zero::zero(), }; // Calculate fee amount - this will be implemented based on the custom // implementation of the fee provider - let (fee_recipient, fee_percent) = T::FeeHandler::apply_fees(from, recipient, &new_payment, remark); - let fee_amount = fee_percent.mul_floor(amount); - new_payment.fee_detail = Some((fee_recipient, fee_amount)); + new_payment.fee_amount = T::FeeHandler::get_fee_amount(from, recipient, &new_payment, remark); *maybe_payment = Some(new_payment.clone()); @@ -602,9 +600,7 @@ pub mod pallet { /// the recipient but will stay in Reserve state. #[require_transactional] fn reserve_payment_amount(from: &T::AccountId, to: &T::AccountId, payment: PaymentDetail) -> DispatchResult { - let fee_amount = payment.fee_detail.map(|(_, f)| f).unwrap_or_else(|| 0u32.into()); - - let total_fee_amount = payment.incentive_amount.saturating_add(fee_amount); + let total_fee_amount = payment.incentive_amount.saturating_add(payment.fee_amount); let total_amount = total_fee_amount.saturating_add(payment.amount); // reserve the total amount from payment creator @@ -632,29 +628,17 @@ pub mod pallet { Payment::::try_mutate(from, to, |maybe_payment| -> DispatchResult { let payment = maybe_payment.take().ok_or(Error::::InvalidPayment)?; - // unreserve the incentive amount and fees from the owner account - match payment.fee_detail { - Some((fee_recipient, fee_amount)) => { - T::Asset::unreserve_named( - &PALLET_RESERVE_ID, - payment.asset, - from, - payment.incentive_amount + fee_amount, - ); - // transfer fee to marketplace if operation is not cancel - if recipient_share != Percent::zero() { - T::Asset::transfer( - payment.asset, - from, // fee is paid by payment creator - &fee_recipient, // account of fee recipient - fee_amount, // amount of fee - )?; - } - } - None => { - T::Asset::unreserve_named(&PALLET_RESERVE_ID, payment.asset, from, payment.incentive_amount); - } - }; + T::Asset::unreserve_named( + &PALLET_RESERVE_ID, + payment.asset, + from, + payment.incentive_amount + payment.fee_amount, + ); + + // transfer fee to marketplace if operation is not cancel + if recipient_share != Percent::zero() { + T::FeeHandler::apply_fees(from, to, &payment)?; + } // Unreserve the transfer amount T::Asset::unreserve_named(&PALLET_RESERVE_ID, payment.asset, to, payment.amount); diff --git a/payments/src/mock.rs b/payments/src/mock.rs index e26626e89..f308a45f2 100644 --- a/payments/src/mock.rs +++ b/payments/src/mock.rs @@ -6,12 +6,12 @@ use frame_support::{ weights::DispatchClass, }; use frame_system as system; -use orml_traits::parameter_type_with_key; +use orml_traits::{parameter_type_with_key, MultiCurrency}; use sp_core::H256; use sp_runtime::{ testing::Header, - traits::{BlakeTwo256, IdentityLookup}, - Percent, + traits::{BlakeTwo256, IdentityLookup, Zero}, + DispatchResult, Percent, }; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; @@ -115,17 +115,27 @@ impl crate::types::DisputeResolver for MockDisputeResolver { pub struct MockFeeHandler; impl crate::types::FeeHandler for MockFeeHandler { - fn apply_fees( + fn get_fee_amount( _from: &AccountId, to: &AccountId, - _detail: &PaymentDetail, + detail: &PaymentDetail, _remark: Option<&[u8]>, - ) -> (AccountId, Percent) { + ) -> Balance { match to { - &PAYMENT_RECIPENT_FEE_CHARGED => (FEE_RECIPIENT_ACCOUNT, Percent::from_percent(MARKETPLACE_FEE_PERCENTAGE)), - _ => (FEE_RECIPIENT_ACCOUNT, Percent::from_percent(0)), + &PAYMENT_RECIPENT_FEE_CHARGED => Percent::from_percent(MARKETPLACE_FEE_PERCENTAGE) * detail.amount, + _ => Zero::zero(), } } + + fn apply_fees(from: &AccountId, _to: &AccountId, detail: &PaymentDetail) -> DispatchResult { + // transfer the fee amount to fee recipient account + >::transfer( + detail.asset, + from, // fee is paid by payment creator + &FEE_RECIPIENT_ACCOUNT, // account of fee recipient + detail.fee_amount, // amount of fee + ) + } } parameter_types! { diff --git a/payments/src/tests.rs b/payments/src/tests.rs index a87e79f6d..8d761825a 100644 --- a/payments/src/tests.rs +++ b/payments/src/tests.rs @@ -5,7 +5,7 @@ use crate::{ }; use frame_support::{assert_noop, assert_ok, storage::with_transaction}; use orml_traits::{MultiCurrency, MultiReservableCurrency, NamedMultiReservableCurrency}; -use sp_runtime::{Percent, TransactionOutcome}; +use sp_runtime::{traits::Zero, Percent, TransactionOutcome}; type Error = crate::Error; @@ -54,7 +54,7 @@ fn test_pay_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); // the payment amount should be reserved correctly @@ -102,7 +102,7 @@ fn test_pay_works() { incentive_amount: 2, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); @@ -134,7 +134,7 @@ fn test_cancel_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); // the payment amount should be reserved @@ -205,7 +205,7 @@ fn test_release_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); // the payment amount should be reserved @@ -367,7 +367,7 @@ fn test_charging_fee_payment_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, expected_fee_amount)), + fee_amount: expected_fee_amount }) ); // the payment amount should be reserved @@ -426,7 +426,7 @@ fn test_charging_fee_payment_works_when_canceled() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, expected_fee_amount)), + fee_amount: expected_fee_amount, }) ); // the payment amount should be reserved @@ -475,7 +475,7 @@ fn test_pay_with_remark_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); // the payment amount should be reserved correctly @@ -553,7 +553,7 @@ fn test_do_not_overwrite_logic_works() { incentive_amount: expected_incentive_amount, state: PaymentState::NeedsReview, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }, ); @@ -613,7 +613,7 @@ fn test_request_refund() { cancel_block: expected_cancel_block }, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); @@ -678,7 +678,7 @@ fn test_dispute_refund() { incentive_amount: expected_incentive_amount, state: PaymentState::NeedsReview, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); @@ -723,7 +723,7 @@ fn test_request_payment() { incentive_amount: expected_incentive_amount, state: PaymentState::PaymentRequested, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); @@ -799,7 +799,7 @@ fn test_accept_and_pay() { incentive_amount: expected_incentive_amount, state: PaymentState::PaymentRequested, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); @@ -870,7 +870,7 @@ fn test_accept_and_pay_should_charge_fee_correctly() { incentive_amount: expected_incentive_amount, state: PaymentState::PaymentRequested, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, expected_fee_amount)), + fee_amount: expected_fee_amount }) ); @@ -947,7 +947,7 @@ fn test_create_payment_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, expected_fee_amount)), + fee_amount: expected_fee_amount }) ); @@ -975,7 +975,7 @@ fn test_create_payment_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, expected_fee_amount)), + fee_amount: expected_fee_amount }) ); }); @@ -1015,7 +1015,7 @@ fn test_reserve_payment_amount_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, expected_fee_amount)), + fee_amount: expected_fee_amount }) ); @@ -1065,7 +1065,7 @@ fn test_reserve_payment_amount_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, expected_fee_amount)), + fee_amount: expected_fee_amount }) ); }); @@ -1302,7 +1302,7 @@ fn test_automatic_refund_works() { cancel_block: CANCEL_BLOCK }, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_amount: Zero::zero(), }) ); diff --git a/payments/src/types.rs b/payments/src/types.rs index a4e5058d4..0b5cad2b4 100644 --- a/payments/src/types.rs +++ b/payments/src/types.rs @@ -27,8 +27,8 @@ pub struct PaymentDetail { pub state: PaymentState, /// account that can settle any disputes created in the payment pub resolver_account: T::AccountId, - /// fee charged and recipient account details - pub fee_detail: Option<(T::AccountId, BalanceOf)>, + /// fee charged details + pub fee_amount: BalanceOf, } /// The `PaymentState` enum tracks the possible states that a payment can be in. @@ -95,13 +95,16 @@ pub trait DisputeResolver { /// Fee Handler trait that defines how to handle marketplace fees to every /// payment/swap pub trait FeeHandler { - /// Get the distribution of fees to marketplace participants - fn apply_fees( + /// Get the amount of fee to charge user + fn get_fee_amount( from: &T::AccountId, to: &T::AccountId, detail: &PaymentDetail, remark: Option<&[u8]>, - ) -> (T::AccountId, Percent); + ) -> BalanceOf; + + /// Deduct the fee from the user transaction + fn apply_fees(from: &T::AccountId, to: &T::AccountId, detail: &PaymentDetail) -> DispatchResult; } /// Types of Tasks that can be scheduled in the pallet