From 1d4464ce4e42fe565b6bc125202d35e742c3e745 Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Sat, 14 May 2022 15:00:15 +0400 Subject: [PATCH 1/3] support multiple fee recipients --- payments/README.md | 11 +- payments/src/lib.rs | 73 ++++++++++--- payments/src/mock.rs | 44 +++++++- payments/src/tests.rs | 240 ++++++++++++++++++++++++++++++++++++++---- payments/src/types.rs | 19 +++- 5 files changed, 342 insertions(+), 45 deletions(-) diff --git a/payments/README.md b/payments/README.md index e28fbf26c..f0f840adf 100644 --- a/payments/README.md +++ b/payments/README.md @@ -51,17 +51,18 @@ pub struct PaymentDetail { /// type of asset used for payment pub asset: AssetIdOf, /// amount of asset used for payment + #[codec(compact)] pub amount: BalanceOf, /// incentive amount that is credited to creator for resolving + #[codec(compact)] pub incentive_amount: BalanceOf, - /// enum to track payment lifecycle [Created, NeedsReview] - pub state: PaymentState, + /// enum to track payment lifecycle [Created, NeedsReview, RefundRequested, + /// Requested] + 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)>, - /// remarks to give context to payment - pub remark: Option>, + pub fee_detail: Option>, } ``` diff --git a/payments/src/lib.rs b/payments/src/lib.rs index c2175e421..67728a1bb 100644 --- a/payments/src/lib.rs +++ b/payments/src/lib.rs @@ -68,7 +68,10 @@ pub mod weights; #[frame_support::pallet] pub mod pallet { pub use crate::{ - types::{DisputeResolver, FeeHandler, PaymentDetail, PaymentHandler, PaymentState, ScheduledTask, Task}, + types::{ + DisputeResolver, FeeHandler, FeeRecipient, FeeRecipientShare, PaymentDetail, PaymentHandler, PaymentState, + ScheduledTask, Task, + }, weights::WeightInfo, }; use frame_support::{ @@ -76,7 +79,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, @@ -99,6 +102,12 @@ pub mod pallet { ScheduledTaskOf, ::MaxRemarkLength, >; + pub type FeeRecipientList = BoundedVec< + FeeRecipient<::AccountId, BalanceOf>, + ::FeeRecipientLimit, + >; + pub type FeeRecipientShareList = + BoundedVec::AccountId>, ::FeeRecipientLimit>; #[pallet::config] pub trait Config: frame_system::Config { @@ -125,6 +134,9 @@ pub mod pallet { /// canceled payment #[pallet::constant] type MaxScheduledTaskListLength: Get; + /// The maximum fee recipient accounts that be specified + #[pallet::constant] + type FeeRecipientLimit: Get; //// Type representing the weight of this pallet type WeightInfo: WeightInfo; } @@ -209,6 +221,8 @@ pub mod pallet { DisputePeriodNotPassed, /// The automatic cancelation queue cannot accept RefundQueueFull, + /// Error in fee calculation + FeeCalculationFailed, } #[pallet::hooks] @@ -583,10 +597,20 @@ pub mod pallet { // 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)); + let recipients = T::FeeHandler::apply_fees(from, recipient, &new_payment, remark); + let mut fee_recipient_list: FeeRecipientList = Default::default(); + + for fee_recipient in recipients { + fee_recipient_list + .try_push(FeeRecipient { + account_id: fee_recipient.account_id, + fee_amount: fee_recipient.percent_of_fees.mul_floor(amount), + }) + .map_err(|_| Error::::FeeCalculationFailed)?; + } + + new_payment.fee_detail = Some(fee_recipient_list); *maybe_payment = Some(new_payment.clone()); // increment provider to prevent sender data from getting reaped @@ -602,7 +626,18 @@ 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()); + // calculate total fee amount + let fee_amount = match payment.fee_detail { + Some(recipient_list) => { + recipient_list + .iter() + .fold(Zero::zero(), |mut sum: BalanceOf, fee_recipient| { + sum += fee_recipient.fee_amount; + sum + }) + } + None => Zero::zero(), + }; let total_fee_amount = payment.incentive_amount.saturating_add(fee_amount); let total_amount = total_fee_amount.saturating_add(payment.amount); @@ -634,21 +669,33 @@ pub mod pallet { // unreserve the incentive amount and fees from the owner account match payment.fee_detail { - Some((fee_recipient, fee_amount)) => { + Some(recipient_list) => { + // calculate total fee amount + let fee_amount = + recipient_list + .iter() + .fold(Zero::zero(), |mut sum: BalanceOf, fee_recipient| { + sum += fee_recipient.fee_amount; + sum + }); + 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 - )?; + for fee_recipient in recipient_list { + T::Asset::transfer( + payment.asset, + from, // fee is paid by payment creator + &fee_recipient.account_id, // account of fee recipient + fee_recipient.fee_amount, // amount of fee + )?; + } } } None => { diff --git a/payments/src/mock.rs b/payments/src/mock.rs index e26626e89..6b1dfb1ca 100644 --- a/payments/src/mock.rs +++ b/payments/src/mock.rs @@ -1,5 +1,5 @@ use crate as payment; -use crate::PaymentDetail; +use crate::*; use frame_support::{ parameter_types, traits::{ConstU32, Contains, Everything, GenesisBuild, Hooks, OnFinalize}, @@ -27,6 +27,8 @@ pub const CURRENCY_ID: u32 = 1; pub const RESOLVER_ACCOUNT: AccountId = 12; pub const FEE_RECIPIENT_ACCOUNT: AccountId = 20; pub const PAYMENT_RECIPENT_FEE_CHARGED: AccountId = 21; +pub const PAYMENT_RECIPENT_MULTIPLE_FEE_CHARGED: AccountId = 22; +pub const SECOND_FEE_RECIPIENT_ACCOUNT: AccountId = 23; pub const INCENTIVE_PERCENTAGE: u8 = 10; pub const MARKETPLACE_FEE_PERCENTAGE: u8 = 10; pub const CANCEL_BLOCK_BUFFER: u64 = 600; @@ -120,11 +122,41 @@ impl crate::types::FeeHandler for MockFeeHandler { to: &AccountId, _detail: &PaymentDetail, _remark: Option<&[u8]>, - ) -> (AccountId, Percent) { + ) -> FeeRecipientShareList { + let mut fee_recipient_share_list: FeeRecipientShareList = Default::default(); match to { - &PAYMENT_RECIPENT_FEE_CHARGED => (FEE_RECIPIENT_ACCOUNT, Percent::from_percent(MARKETPLACE_FEE_PERCENTAGE)), - _ => (FEE_RECIPIENT_ACCOUNT, Percent::from_percent(0)), - } + // to test a single fee recipient + &PAYMENT_RECIPENT_FEE_CHARGED => fee_recipient_share_list + .try_push(FeeRecipientShare { + account_id: FEE_RECIPIENT_ACCOUNT, + percent_of_fees: Percent::from_percent(MARKETPLACE_FEE_PERCENTAGE), + }) + .unwrap(), + // to test multiple fee recipients, both recipient receive 50% of fee + &PAYMENT_RECIPENT_MULTIPLE_FEE_CHARGED => { + fee_recipient_share_list + .try_push(FeeRecipientShare { + account_id: FEE_RECIPIENT_ACCOUNT, + percent_of_fees: Percent::from_percent(MARKETPLACE_FEE_PERCENTAGE / 2), + }) + .unwrap(); + + fee_recipient_share_list + .try_push(FeeRecipientShare { + account_id: SECOND_FEE_RECIPIENT_ACCOUNT, + percent_of_fees: Percent::from_percent(MARKETPLACE_FEE_PERCENTAGE / 2), + }) + .unwrap(); + } + // to test no fee charged + _ => fee_recipient_share_list + .try_push(FeeRecipientShare { + account_id: FEE_RECIPIENT_ACCOUNT, + percent_of_fees: Percent::from_percent(0), + }) + .unwrap(), + }; + fee_recipient_share_list } } @@ -133,6 +165,7 @@ parameter_types! { pub const MaxRemarkLength: u32 = 50; pub const CancelBufferBlockLength: u64 = CANCEL_BLOCK_BUFFER; pub const MaxScheduledTaskListLength : u32 = 5; + pub const FeeRecipientLimit : u32 = 3; } impl payment::Config for Test { @@ -144,6 +177,7 @@ impl payment::Config for Test { type MaxRemarkLength = MaxRemarkLength; type CancelBufferBlockLength = CancelBufferBlockLength; type MaxScheduledTaskListLength = MaxScheduledTaskListLength; + type FeeRecipientLimit = FeeRecipientLimit; type WeightInfo = (); } diff --git a/payments/src/tests.rs b/payments/src/tests.rs index a87e79f6d..6703a54c2 100644 --- a/payments/src/tests.rs +++ b/payments/src/tests.rs @@ -1,7 +1,7 @@ use crate::{ mock::*, types::{PaymentDetail, PaymentState}, - Payment as PaymentStore, PaymentHandler, ScheduledTask, ScheduledTasks, Task, PALLET_RESERVE_ID, + FeeRecipient, Payment as PaymentStore, PaymentHandler, ScheduledTask, ScheduledTasks, Task, PALLET_RESERVE_ID, }; use frame_support::{assert_noop, assert_ok, storage::with_transaction}; use orml_traits::{MultiCurrency, MultiReservableCurrency, NamedMultiReservableCurrency}; @@ -54,7 +54,14 @@ fn test_pay_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); // the payment amount should be reserved correctly @@ -102,7 +109,14 @@ fn test_pay_works() { incentive_amount: 2, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); @@ -134,7 +148,14 @@ fn test_cancel_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); // the payment amount should be reserved @@ -205,7 +226,14 @@ fn test_release_works() { incentive_amount: expected_incentive_amount, state: PaymentState::Created, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); // the payment amount should be reserved @@ -367,7 +395,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: expected_fee_amount + }] + .try_into() + .unwrap() + ) }) ); // the payment amount should be reserved @@ -402,6 +437,82 @@ fn test_charging_fee_payment_works() { }); } +#[test] +fn test_charging_fee_payment_works_for_multiple_recipients() { + new_test_ext().execute_with(|| { + let creator_initial_balance = 100; + let payment_amount = 40; + let expected_incentive_amount = payment_amount / INCENTIVE_PERCENTAGE as u128; + let expected_fee_amount = payment_amount / MARKETPLACE_FEE_PERCENTAGE as u128; + + // should be able to create a payment with available balance + assert_ok!(Payment::pay( + Origin::signed(PAYMENT_CREATOR), + PAYMENT_RECIPENT_MULTIPLE_FEE_CHARGED, + CURRENCY_ID, + payment_amount, + None + )); + assert_eq!( + PaymentStore::::get(PAYMENT_CREATOR, PAYMENT_RECIPENT_MULTIPLE_FEE_CHARGED), + Some(PaymentDetail { + asset: CURRENCY_ID, + amount: payment_amount, + incentive_amount: expected_incentive_amount, + state: PaymentState::Created, + resolver_account: RESOLVER_ACCOUNT, + fee_detail: Some( + vec![ + FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: expected_fee_amount / 2 + }, + FeeRecipient { + account_id: SECOND_FEE_RECIPIENT_ACCOUNT, + fee_amount: expected_fee_amount / 2 + } + ] + .try_into() + .unwrap() + ) + }) + ); + // the payment amount should be reserved + assert_eq!( + Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), + creator_initial_balance - payment_amount - expected_fee_amount - expected_incentive_amount + ); + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_RECIPENT_FEE_CHARGED), 0); + + // should succeed for valid payment + assert_ok!(Payment::release( + Origin::signed(PAYMENT_CREATOR), + PAYMENT_RECIPENT_MULTIPLE_FEE_CHARGED + )); + // the payment amount should be transferred + assert_eq!( + Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), + creator_initial_balance - payment_amount - expected_fee_amount + ); + assert_eq!( + Tokens::total_balance(CURRENCY_ID, &PAYMENT_CREATOR), + creator_initial_balance - payment_amount - expected_fee_amount + ); + assert_eq!( + Tokens::free_balance(CURRENCY_ID, &PAYMENT_RECIPENT_MULTIPLE_FEE_CHARGED), + payment_amount + ); + assert_eq!( + Tokens::free_balance(CURRENCY_ID, &FEE_RECIPIENT_ACCOUNT), + expected_fee_amount / 2 + ); + assert_eq!( + Tokens::free_balance(CURRENCY_ID, &SECOND_FEE_RECIPIENT_ACCOUNT), + expected_fee_amount / 2 + ); + }); +} + #[test] fn test_charging_fee_payment_works_when_canceled() { new_test_ext().execute_with(|| { @@ -426,7 +537,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: expected_fee_amount + }] + .try_into() + .unwrap() + ), }) ); // the payment amount should be reserved @@ -475,7 +593,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); // the payment amount should be reserved correctly @@ -553,7 +678,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into(), + }] + .try_into() + .unwrap(), + ), }, ); @@ -613,7 +745,14 @@ fn test_request_refund() { cancel_block: expected_cancel_block }, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); @@ -678,7 +817,14 @@ fn test_dispute_refund() { incentive_amount: expected_incentive_amount, state: PaymentState::NeedsReview, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); @@ -723,7 +869,14 @@ fn test_request_payment() { incentive_amount: expected_incentive_amount, state: PaymentState::PaymentRequested, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); @@ -799,7 +952,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); @@ -870,7 +1030,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: expected_fee_amount + }] + .try_into() + .unwrap() + ), }) ); @@ -947,7 +1114,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: expected_fee_amount + }] + .try_into() + .unwrap() + ), }) ); @@ -975,7 +1149,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: expected_fee_amount + }] + .try_into() + .unwrap() + ), }) ); }); @@ -1015,7 +1196,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: expected_fee_amount + }] + .try_into() + .unwrap() + ), }) ); @@ -1065,7 +1253,14 @@ 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_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: expected_fee_amount + }] + .try_into() + .unwrap() + ), }) ); }); @@ -1302,7 +1497,14 @@ fn test_automatic_refund_works() { cancel_block: CANCEL_BLOCK }, resolver_account: RESOLVER_ACCOUNT, - fee_detail: Some((FEE_RECIPIENT_ACCOUNT, 0)), + fee_detail: Some( + vec![FeeRecipient { + account_id: FEE_RECIPIENT_ACCOUNT, + fee_amount: 0_u32.into() + }] + .try_into() + .unwrap() + ), }) ); diff --git a/payments/src/types.rs b/payments/src/types.rs index a4e5058d4..7ead9c51a 100644 --- a/payments/src/types.rs +++ b/payments/src/types.rs @@ -1,5 +1,5 @@ #![allow(unused_qualifications)] -use crate::{pallet, AssetIdOf, BalanceOf}; +use crate::{pallet, AssetIdOf, BalanceOf, FeeRecipientList, FeeRecipientShareList}; use parity_scale_codec::{Decode, Encode, HasCompact, MaxEncodedLen}; use scale_info::TypeInfo; use sp_runtime::{DispatchResult, Percent}; @@ -28,7 +28,7 @@ pub struct PaymentDetail { /// 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)>, + pub fee_detail: Option>, } /// The `PaymentState` enum tracks the possible states that a payment can be in. @@ -92,6 +92,19 @@ pub trait DisputeResolver { fn get_resolver_account() -> Account; } +/// FeeRecipient details +#[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, MaxEncodedLen, TypeInfo)] +pub struct FeeRecipientShare { + pub account_id: AccountId, + pub percent_of_fees: Percent, +} + +#[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, MaxEncodedLen, TypeInfo)] +pub struct FeeRecipient { + pub account_id: AccountId, + pub fee_amount: Balance, +} + /// Fee Handler trait that defines how to handle marketplace fees to every /// payment/swap pub trait FeeHandler { @@ -101,7 +114,7 @@ pub trait FeeHandler { to: &T::AccountId, detail: &PaymentDetail, remark: Option<&[u8]>, - ) -> (T::AccountId, Percent); + ) -> FeeRecipientShareList; } /// Types of Tasks that can be scheduled in the pallet From 583db33167926b1ca2e5526dc0fbba554aeb436d Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Sat, 14 May 2022 15:04:18 +0400 Subject: [PATCH 2/3] add docs --- payments/README.md | 31 +++++++++++++++++++++++++++++++ payments/src/types.rs | 4 +++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/payments/README.md b/payments/README.md index f0f840adf..2310e780d 100644 --- a/payments/README.md +++ b/payments/README.md @@ -79,6 +79,37 @@ pub enum PaymentState { } ``` +The `FeeHandler` trait lets the implementation specify how much fees to charge for a payment and to which account it should be credited. +The below example charges a 10% fee for every payment and distributes it evenly to two accounts. + +```rust +pub struct MockFeeHandler; +impl orml_payment::types::FeeHandler for MockFeeHandler { + fn apply_fees( + _from: &AccountId, + _to: &AccountId, + _detail: &PaymentDetail, + _remark: Option<&[u8]>, + ) -> FeeRecipientShareList { + pub const MARKETPLACE_FEE_PERCENTAGE: u8 = 10; + let mut fee_recipient_share_list: FeeRecipientShareList = Default::default(); + fee_recipient_share_list + .try_push(FeeRecipientShare { + account_id: FEE_RECIPIENT_ACCOUNT, + percent_of_fees: Percent::from_percent(MARKETPLACE_FEE_PERCENTAGE / 2), + }).unwrap(); + + fee_recipient_share_list + .try_push(FeeRecipientShare { + account_id: SECOND_FEE_RECIPIENT_ACCOUNT, + percent_of_fees: Percent::from_percent(MARKETPLACE_FEE_PERCENTAGE / 2), + }).unwrap(); + + fee_recipient_share_list + } +} +``` + ## GenesisConfig The rates_provider pallet does not depend on the `GenesisConfig` diff --git a/payments/src/types.rs b/payments/src/types.rs index 7ead9c51a..95beda2d1 100644 --- a/payments/src/types.rs +++ b/payments/src/types.rs @@ -92,13 +92,15 @@ pub trait DisputeResolver { fn get_resolver_account() -> Account; } -/// FeeRecipient details +/// FeeRecipientShare details - specifies the share of fees each account +/// receives #[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, MaxEncodedLen, TypeInfo)] pub struct FeeRecipientShare { pub account_id: AccountId, pub percent_of_fees: Percent, } +/// FeeRecipient details #[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, MaxEncodedLen, TypeInfo)] pub struct FeeRecipient { pub account_id: AccountId, From 1af1749e19cbfdcd6d67cafbaba36b13246a766d Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Sat, 21 May 2022 00:11:26 +0400 Subject: [PATCH 3/3] implement review suggestions --- payments/src/lib.rs | 43 ++++++++++++++----------------------------- payments/src/types.rs | 12 +++++++++++- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/payments/src/lib.rs b/payments/src/lib.rs index 67728a1bb..eff5a108c 100644 --- a/payments/src/lib.rs +++ b/payments/src/lib.rs @@ -69,8 +69,8 @@ pub mod weights; pub mod pallet { pub use crate::{ types::{ - DisputeResolver, FeeHandler, FeeRecipient, FeeRecipientShare, PaymentDetail, PaymentHandler, PaymentState, - ScheduledTask, Task, + calculate_fee_amount, DisputeResolver, FeeHandler, FeeRecipient, FeeRecipientShare, PaymentDetail, + PaymentHandler, PaymentState, ScheduledTask, Task, }, weights::WeightInfo, }; @@ -221,8 +221,6 @@ pub mod pallet { DisputePeriodNotPassed, /// The automatic cancelation queue cannot accept RefundQueueFull, - /// Error in fee calculation - FeeCalculationFailed, } #[pallet::hooks] @@ -599,16 +597,16 @@ pub mod pallet { // implementation of the fee provider let recipients = T::FeeHandler::apply_fees(from, recipient, &new_payment, remark); - let mut fee_recipient_list: FeeRecipientList = Default::default(); - - for fee_recipient in recipients { - fee_recipient_list - .try_push(FeeRecipient { - account_id: fee_recipient.account_id, - fee_amount: fee_recipient.percent_of_fees.mul_floor(amount), - }) - .map_err(|_| Error::::FeeCalculationFailed)?; - } + let fee_recipient_list: FeeRecipientList = recipients + .iter() + .map(|fee_recipient| FeeRecipient { + account_id: fee_recipient.account_id.clone(), + fee_amount: fee_recipient.percent_of_fees.mul_floor(amount), + }) + .collect::>() + .try_into() + // this should not happen since max length of both vecs are FeeRecipientLimit + .expect("recipients should not exceed limit; qed"); new_payment.fee_detail = Some(fee_recipient_list); *maybe_payment = Some(new_payment.clone()); @@ -628,14 +626,7 @@ pub mod pallet { fn reserve_payment_amount(from: &T::AccountId, to: &T::AccountId, payment: PaymentDetail) -> DispatchResult { // calculate total fee amount let fee_amount = match payment.fee_detail { - Some(recipient_list) => { - recipient_list - .iter() - .fold(Zero::zero(), |mut sum: BalanceOf, fee_recipient| { - sum += fee_recipient.fee_amount; - sum - }) - } + Some(recipient_list) => calculate_fee_amount::(&recipient_list), None => Zero::zero(), }; @@ -671,13 +662,7 @@ pub mod pallet { match payment.fee_detail { Some(recipient_list) => { // calculate total fee amount - let fee_amount = - recipient_list - .iter() - .fold(Zero::zero(), |mut sum: BalanceOf, fee_recipient| { - sum += fee_recipient.fee_amount; - sum - }); + let fee_amount = calculate_fee_amount::(&recipient_list); T::Asset::unreserve_named( &PALLET_RESERVE_ID, diff --git a/payments/src/types.rs b/payments/src/types.rs index 95beda2d1..25dd4bdf1 100644 --- a/payments/src/types.rs +++ b/payments/src/types.rs @@ -2,7 +2,7 @@ use crate::{pallet, AssetIdOf, BalanceOf, FeeRecipientList, FeeRecipientShareList}; use parity_scale_codec::{Decode, Encode, HasCompact, MaxEncodedLen}; use scale_info::TypeInfo; -use sp_runtime::{DispatchResult, Percent}; +use sp_runtime::{traits::Zero, DispatchResult, Percent}; /// The PaymentDetail struct stores information about the payment/escrow /// A "payment" in virto network is similar to an escrow, it is used to @@ -134,3 +134,13 @@ pub struct ScheduledTask { /// the 'time' at which the task should be executed pub when: Time, } + +// helper function to calculate the total fee amount +pub fn calculate_fee_amount(fee_recipient_list: &FeeRecipientList) -> BalanceOf { + fee_recipient_list + .iter() + .fold(Zero::zero(), |mut sum: BalanceOf, fee_recipient| { + sum += fee_recipient.fee_amount; + sum + }) +}