From 3e51b4affc20b41e0c53391b7628a855d9d8e965 Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Mon, 13 Dec 2021 19:15:05 +0000 Subject: [PATCH 01/10] move payment types to pallet --- Cargo.lock | 1 - pallets/payment/Cargo.toml | 5 ++--- pallets/payment/src/benchmarking.rs | 7 +++---- pallets/payment/src/lib.rs | 4 +++- pallets/payment/src/mock.rs | 9 ++++----- pallets/payment/src/tests.rs | 7 +++++-- .../src/payment.rs => pallets/payment/src/types.rs | 0 primitives/src/lib.rs | 2 -- runtime/src/lib.rs | 4 ++-- 9 files changed, 19 insertions(+), 20 deletions(-) rename primitives/src/payment.rs => pallets/payment/src/types.rs (100%) diff --git a/Cargo.lock b/Cargo.lock index 7c504db8..1c13ea93 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10341,7 +10341,6 @@ dependencies = [ "sp-core", "sp-io", "sp-runtime", - "virto-primitives", ] [[package]] diff --git a/pallets/payment/Cargo.toml b/pallets/payment/Cargo.toml index c78da626..6c2c8a27 100644 --- a/pallets/payment/Cargo.toml +++ b/pallets/payment/Cargo.toml @@ -13,13 +13,12 @@ readme = "README.md" targets = ["x86_64-unknown-linux-gnu"] [dependencies] -codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] } +parity-scale-codec = { default-features = false, features = ['derive'], version = "2.0.0" } frame-support = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.13", default-features = false } frame-system = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.13", default-features = false } sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.13", default-features = false } frame-benchmarking = { git = "https://github.com/paritytech/substrate", branch = "polkadot-v0.9.13", default-features = false, optional = true } orml-traits = { git = "https://github.com/open-web3-stack/open-runtime-module-library", branch = "polkadot-v0.9.13", default-features = false } -virto-primitives = { version = "0.3.0", path = "../../primitives" } scale-info = { version = "1.0.0", default-features = false, features = ["derive"] } [dev-dependencies] @@ -31,7 +30,7 @@ orml-tokens = { git = "https://github.com/open-web3-stack/open-runtime-module-li [features] default = ['std'] std = [ - 'codec/std', + 'parity-scale-codec/std', 'frame-support/std', 'frame-system/std', 'sp-runtime/std', diff --git a/pallets/payment/src/benchmarking.rs b/pallets/payment/src/benchmarking.rs index ac9993b0..a2d08258 100644 --- a/pallets/payment/src/benchmarking.rs +++ b/pallets/payment/src/benchmarking.rs @@ -1,13 +1,12 @@ use super::*; -use crate::Pallet as Payment; +use crate::{Pallet as Payment, PaymentState}; use frame_benchmarking::{account, benchmarks, impl_benchmark_test_suite, whitelisted_caller}; use frame_system::RawOrigin; use orml_traits::MultiCurrency; -use virto_primitives::{Asset, NetworkAsset, PaymentState}; const SEED: u32 = 0; -const CURRENCY_ID: Asset = Asset::Network(NetworkAsset::KSM); +const CURRENCY_ID: u32 = 1u32; const INITIAL_AMOUNT: u32 = 100; const SOME_AMOUNT: u32 = 80; @@ -22,7 +21,7 @@ fn assert_last_event(generic_event: ::Event) { benchmarks! { where_clause { where T::Asset: MultiCurrency< ::AccountId, - CurrencyId = Asset, Balance = u32 + CurrencyId = u32, Balance = u32 > } // create a new payment succesfully diff --git a/pallets/payment/src/lib.rs b/pallets/payment/src/lib.rs index e8764f4d..45b6da6a 100644 --- a/pallets/payment/src/lib.rs +++ b/pallets/payment/src/lib.rs @@ -11,13 +11,15 @@ mod tests; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; +mod types; + #[frame_support::pallet] pub mod pallet { + pub use crate::types::{DisputeResolver, PaymentDetail, PaymentHandler, PaymentState}; use frame_support::{dispatch::DispatchResultWithPostInfo, pallet_prelude::*}; use frame_system::pallet_prelude::*; use orml_traits::{MultiCurrency, MultiReservableCurrency}; use sp_runtime::Percent; - use virto_primitives::{DisputeResolver, PaymentDetail, PaymentHandler, PaymentState}; type BalanceOf = <::Asset as MultiCurrency<::AccountId>>::Balance; diff --git a/pallets/payment/src/mock.rs b/pallets/payment/src/mock.rs index 2a9dcacd..e22ce942 100644 --- a/pallets/payment/src/mock.rs +++ b/pallets/payment/src/mock.rs @@ -11,14 +11,13 @@ use sp_runtime::{ traits::{BlakeTwo256, IdentityLookup}, Percent, }; -use virto_primitives::{Asset, DisputeResolver, NetworkAsset}; type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; pub type AccountId = u8; pub const PAYMENT_CREATOR: AccountId = 10; pub const PAYMENT_RECIPENT: AccountId = 11; -pub const CURRENCY_ID: Asset = Asset::Network(NetworkAsset::KSM); +pub const CURRENCY_ID: u32 = 1u32; pub const RESOLVER_ACCOUNT: AccountId = 12; frame_support::construct_runtime!( @@ -65,7 +64,7 @@ impl system::Config for Test { } parameter_type_with_key! { - pub ExistentialDeposits: |_currency_id: Asset| -> u32 { + pub ExistentialDeposits: |_currency_id: u32| -> u32 { 0u32 }; } @@ -83,7 +82,7 @@ impl Contains for MockDustRemovalWhitelist { impl orml_tokens::Config for Test { type Amount = i64; type Balance = u32; - type CurrencyId = Asset; + type CurrencyId = u32; type Event = Event; type ExistentialDeposits = ExistentialDeposits; type OnDust = (); @@ -93,7 +92,7 @@ impl orml_tokens::Config for Test { } pub struct MockDisputeResolver; -impl DisputeResolver for MockDisputeResolver { +impl crate::types::DisputeResolver for MockDisputeResolver { fn get_origin() -> AccountId { RESOLVER_ACCOUNT } diff --git a/pallets/payment/src/tests.rs b/pallets/payment/src/tests.rs index 1de676a4..68906855 100644 --- a/pallets/payment/src/tests.rs +++ b/pallets/payment/src/tests.rs @@ -1,7 +1,10 @@ -use crate::{mock::*, Payment as PaymentStore}; +use crate::{ + mock::*, + types::{PaymentDetail, PaymentState}, + Payment as PaymentStore, +}; use frame_support::{assert_noop, assert_ok}; use orml_traits::MultiCurrency; -use virto_primitives::{PaymentDetail, PaymentState}; #[test] fn test_create_payment_works() { diff --git a/primitives/src/payment.rs b/pallets/payment/src/types.rs similarity index 100% rename from primitives/src/payment.rs rename to pallets/payment/src/types.rs diff --git a/primitives/src/lib.rs b/primitives/src/lib.rs index 445f889a..cf48cae1 100644 --- a/primitives/src/lib.rs +++ b/primitives/src/lib.rs @@ -5,7 +5,5 @@ #![cfg_attr(not(feature = "std"), no_std)] mod asset; -mod payment; pub use asset::*; -pub use payment::*; diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 3ee0055f..74aa1172 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -63,7 +63,7 @@ mod proxy_type; use currency_id_convert::CurrencyIdConvert; use orml_traits::{arithmetic::Zero, parameter_type_with_key}; use proxy_type::ProxyType; -use virto_primitives::{Asset, DisputeResolver}; +use virto_primitives::Asset; /// Alias to 512-bit hash when used in the context of a transaction signature on the chain. pub type Signature = MultiSignature; @@ -596,7 +596,7 @@ impl orml_unknown_tokens::Config for Runtime { } pub struct VirtoDisputeResolver; -impl DisputeResolver for VirtoDisputeResolver { +impl virto_payment::DisputeResolver for VirtoDisputeResolver { fn get_origin() -> AccountId { Sudo::key() } From 3f6c5d5c7d3cda384595f0650c86a93831a2e6c2 Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Wed, 15 Dec 2021 11:43:46 +0000 Subject: [PATCH 02/10] add feehandler and feerecipient --- pallets/payment/src/lib.rs | 14 +++++++++++--- pallets/payment/src/types.rs | 4 ++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pallets/payment/src/lib.rs b/pallets/payment/src/lib.rs index 45b6da6a..1480a314 100644 --- a/pallets/payment/src/lib.rs +++ b/pallets/payment/src/lib.rs @@ -15,7 +15,7 @@ mod types; #[frame_support::pallet] pub mod pallet { - pub use crate::types::{DisputeResolver, PaymentDetail, PaymentHandler, PaymentState}; + pub use crate::types::{DisputeResolver, PaymentDetail, PaymentHandler, PaymentState, FeeHandler}; use frame_support::{dispatch::DispatchResultWithPostInfo, pallet_prelude::*}; use frame_system::pallet_prelude::*; use orml_traits::{MultiCurrency, MultiReservableCurrency}; @@ -34,7 +34,12 @@ pub mod pallet { type Asset: MultiReservableCurrency; /// Dispute resolution account type DisputeResolver: DisputeResolver; - + /// Fee handler trait + type FeeHandler: FeeHandler, BalanceOf>; + /// Fee recipent account + #[pallet::constant] + type FeeRecipientAccount: Get; + /// Incentive percentage - amount witheld from sender #[pallet::constant] type IncentivePercentage: Get; } @@ -233,7 +238,10 @@ pub mod pallet { T::Asset::unreserve(payment.asset, &from, payment.incentive_amount); // unreserve the amount to the recipent T::Asset::unreserve(payment.asset, &to, payment.amount); - + // calculate fee charged for transaction + let fee_amount = T::FeeHandler::apply_fees(&from, &to, payment.asset, payment.amount); + // transfer fee amount to marketplace + T::Asset::transfer(payment.asset, &from, &T::FeeRecipientAccount::get(), fee_amount)?; payment.state = PaymentState::Released; Ok(()) diff --git a/pallets/payment/src/types.rs b/pallets/payment/src/types.rs index c4a3aa4c..6f2b507e 100644 --- a/pallets/payment/src/types.rs +++ b/pallets/payment/src/types.rs @@ -59,3 +59,7 @@ pub trait DisputeResolver { /// Get a DisputeResolver (Judge) account fn get_origin() -> Account; } + +pub trait FeeHandler { + fn apply_fees(from: &Account, to: &Account, asset: Asset, amount: Amount) -> Amount; +} From baeb3acdcb266118175894e5512a695c1d732911 Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Wed, 15 Dec 2021 15:36:01 +0000 Subject: [PATCH 03/10] add tests --- pallets/payment/Cargo.toml | 1 + pallets/payment/src/mock.rs | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/pallets/payment/Cargo.toml b/pallets/payment/Cargo.toml index 6c2c8a27..ec3bdbc4 100644 --- a/pallets/payment/Cargo.toml +++ b/pallets/payment/Cargo.toml @@ -37,5 +37,6 @@ std = [ 'scale-info/std', 'orml-traits/std', 'frame-benchmarking/std', + 'orml-tokens/std' ] runtime-benchmarks = ["frame-benchmarking"] diff --git a/pallets/payment/src/mock.rs b/pallets/payment/src/mock.rs index e22ce942..ecc81334 100644 --- a/pallets/payment/src/mock.rs +++ b/pallets/payment/src/mock.rs @@ -19,6 +19,7 @@ pub const PAYMENT_CREATOR: AccountId = 10; pub const PAYMENT_RECIPENT: AccountId = 11; pub const CURRENCY_ID: u32 = 1u32; pub const RESOLVER_ACCOUNT: AccountId = 12; +pub const FEE_RECIPIENT_ACCOUNT : AccountId = 20; frame_support::construct_runtime!( pub enum Test where @@ -98,8 +99,16 @@ impl crate::types::DisputeResolver for MockDisputeResolver { } } +pub struct MockFeeHandler; +impl crate::types::FeeHandler for MockFeeHandler { + fn apply_fees(_from: &AccountId, _to: &AccountId, _asset: u32, _amount: u32) -> u32 { + 0u32 + } +} + parameter_types! { pub const IncentivePercentage: Percent = Percent::from_percent(10); + pub const FeeRecipientAccount: AccountId = FEE_RECIPIENT_ACCOUNT; } impl payment::Config for Test { @@ -107,6 +116,8 @@ impl payment::Config for Test { type Asset = Tokens; type DisputeResolver = MockDisputeResolver; type IncentivePercentage = IncentivePercentage; + type FeeHandler = MockFeeHandler; + type FeeRecipientAccount = FeeRecipientAccount; } // Build genesis storage according to the mock runtime. From c648c938a5bf26bab78d0d902182dc874e24c6bb Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Wed, 15 Dec 2021 15:43:05 +0000 Subject: [PATCH 04/10] add fee_payment test --- pallets/payment/src/lib.rs | 14 ++++++++++--- pallets/payment/src/mock.rs | 10 +++++++--- pallets/payment/src/tests.rs | 38 ++++++++++++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/pallets/payment/src/lib.rs b/pallets/payment/src/lib.rs index 1480a314..37056e54 100644 --- a/pallets/payment/src/lib.rs +++ b/pallets/payment/src/lib.rs @@ -15,7 +15,9 @@ mod types; #[frame_support::pallet] pub mod pallet { - pub use crate::types::{DisputeResolver, PaymentDetail, PaymentHandler, PaymentState, FeeHandler}; + pub use crate::types::{ + DisputeResolver, FeeHandler, PaymentDetail, PaymentHandler, PaymentState, + }; use frame_support::{dispatch::DispatchResultWithPostInfo, pallet_prelude::*}; use frame_system::pallet_prelude::*; use orml_traits::{MultiCurrency, MultiReservableCurrency}; @@ -239,9 +241,15 @@ pub mod pallet { // unreserve the amount to the recipent T::Asset::unreserve(payment.asset, &to, payment.amount); // calculate fee charged for transaction - let fee_amount = T::FeeHandler::apply_fees(&from, &to, payment.asset, payment.amount); + let fee_amount = + T::FeeHandler::apply_fees(&from, &to, payment.asset, payment.amount); // transfer fee amount to marketplace - T::Asset::transfer(payment.asset, &from, &T::FeeRecipientAccount::get(), fee_amount)?; + T::Asset::transfer( + payment.asset, + &from, + &T::FeeRecipientAccount::get(), + fee_amount, + )?; payment.state = PaymentState::Released; Ok(()) diff --git a/pallets/payment/src/mock.rs b/pallets/payment/src/mock.rs index ecc81334..3bfa2d0e 100644 --- a/pallets/payment/src/mock.rs +++ b/pallets/payment/src/mock.rs @@ -19,7 +19,8 @@ pub const PAYMENT_CREATOR: AccountId = 10; pub const PAYMENT_RECIPENT: AccountId = 11; pub const CURRENCY_ID: u32 = 1u32; pub const RESOLVER_ACCOUNT: AccountId = 12; -pub const FEE_RECIPIENT_ACCOUNT : AccountId = 20; +pub const FEE_RECIPIENT_ACCOUNT: AccountId = 20; +pub const PAYMENT_RECIPENT_FEE_CHARGED: AccountId = 21; frame_support::construct_runtime!( pub enum Test where @@ -101,8 +102,11 @@ impl crate::types::DisputeResolver for MockDisputeResolver { pub struct MockFeeHandler; impl crate::types::FeeHandler for MockFeeHandler { - fn apply_fees(_from: &AccountId, _to: &AccountId, _asset: u32, _amount: u32) -> u32 { - 0u32 + fn apply_fees(_from: &AccountId, to: &AccountId, _asset: u32, _amount: u32) -> u32 { + match to { + &PAYMENT_RECIPENT_FEE_CHARGED => 1u32, + _ => 0u32, + } } } diff --git a/pallets/payment/src/tests.rs b/pallets/payment/src/tests.rs index 68906855..9052b76f 100644 --- a/pallets/payment/src/tests.rs +++ b/pallets/payment/src/tests.rs @@ -59,7 +59,7 @@ fn test_create_payment_works() { } #[test] -fn test_release_payment_works() { +fn test_cancel_payment_works() { new_test_ext().execute_with(|| { // should be able to create a payment with available balance assert_ok!(Payment::create( @@ -115,7 +115,7 @@ fn test_release_payment_works() { } #[test] -fn test_cancel_payment_works() { +fn test_release_payment_works() { new_test_ext().execute_with(|| { // should be able to create a payment with available balance assert_ok!(Payment::create( @@ -255,3 +255,37 @@ fn test_set_state_payment_works() { ); }); } + +#[test] +fn test_charging_fee_payment_works() { + new_test_ext().execute_with(|| { + // should be able to create a payment with available balance + assert_ok!(Payment::create( + Origin::signed(PAYMENT_CREATOR), + PAYMENT_RECIPENT_FEE_CHARGED, + CURRENCY_ID, + 40, + )); + assert_eq!( + PaymentStore::::get(PAYMENT_CREATOR, PAYMENT_RECIPENT_FEE_CHARGED), + Some(PaymentDetail { + asset: CURRENCY_ID, + amount: 40, + incentive_amount: 4, + state: PaymentState::Created, + resolver_account: RESOLVER_ACCOUNT + }) + ); + // the payment amount should be reserved + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), 56); + 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_FEE_CHARGED)); + // the payment amount should be transferred + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), 59); + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_RECIPENT_FEE_CHARGED), 40); + assert_eq!(Tokens::free_balance(CURRENCY_ID, &FEE_RECIPIENT_ACCOUNT), 1); + assert_eq!(Tokens::total_issuance(CURRENCY_ID), 100); + }); +} From c340bbf84555eb8f638ae84a8a87189717da8952 Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Mon, 20 Dec 2021 23:54:51 +0000 Subject: [PATCH 05/10] update review suggestions --- pallets/payment/Cargo.toml | 1 - pallets/payment/src/lib.rs | 19 ++++++++----------- pallets/payment/src/mock.rs | 10 ++++------ pallets/payment/src/tests.rs | 31 ++++++++++++++++++++----------- pallets/payment/src/types.rs | 9 ++++++--- 5 files changed, 38 insertions(+), 32 deletions(-) diff --git a/pallets/payment/Cargo.toml b/pallets/payment/Cargo.toml index ec3bdbc4..6c2c8a27 100644 --- a/pallets/payment/Cargo.toml +++ b/pallets/payment/Cargo.toml @@ -37,6 +37,5 @@ std = [ 'scale-info/std', 'orml-traits/std', 'frame-benchmarking/std', - 'orml-tokens/std' ] runtime-benchmarks = ["frame-benchmarking"] diff --git a/pallets/payment/src/lib.rs b/pallets/payment/src/lib.rs index 37056e54..0296ad42 100644 --- a/pallets/payment/src/lib.rs +++ b/pallets/payment/src/lib.rs @@ -37,10 +37,7 @@ pub mod pallet { /// Dispute resolution account type DisputeResolver: DisputeResolver; /// Fee handler trait - type FeeHandler: FeeHandler, BalanceOf>; - /// Fee recipent account - #[pallet::constant] - type FeeRecipientAccount: Get; + type FeeHandler: FeeHandler; /// Incentive percentage - amount witheld from sender #[pallet::constant] type IncentivePercentage: Get; @@ -184,12 +181,15 @@ pub mod pallet { recipient.clone(), |maybe_payment| -> DispatchResult { let incentive_amount = T::IncentivePercentage::get() * amount; + let (fee_recipient, fee_percent) = T::FeeHandler::apply_fees(&from, &recipient); + let fee_amount = fee_percent * amount; let new_payment = Some(PaymentDetail { asset, amount, incentive_amount, state: PaymentState::Created, resolver_account: T::DisputeResolver::get_origin(), + fee_detail: (fee_recipient, fee_amount) }); match maybe_payment { Some(x) => { @@ -200,8 +200,8 @@ pub mod pallet { x.state != PaymentState::Created, Error::::PaymentAlreadyInProcess ); - // reserve the incentive amount from the payment creator - T::Asset::reserve(asset, &from, incentive_amount)?; + // reserve the incentive + fees amount from the payment creator + T::Asset::reserve(asset, &from, incentive_amount + fee_amount)?; // transfer amount to recipient T::Asset::transfer(asset, &from, &recipient, amount)?; // reserved the amount in the recipient account @@ -240,15 +240,12 @@ pub mod pallet { T::Asset::unreserve(payment.asset, &from, payment.incentive_amount); // unreserve the amount to the recipent T::Asset::unreserve(payment.asset, &to, payment.amount); - // calculate fee charged for transaction - let fee_amount = - T::FeeHandler::apply_fees(&from, &to, payment.asset, payment.amount); // transfer fee amount to marketplace T::Asset::transfer( payment.asset, &from, - &T::FeeRecipientAccount::get(), - fee_amount, + &payment.fee_detail.0, // account + payment.fee_detail.1, // amount )?; payment.state = PaymentState::Released; diff --git a/pallets/payment/src/mock.rs b/pallets/payment/src/mock.rs index 3bfa2d0e..9fdb5d47 100644 --- a/pallets/payment/src/mock.rs +++ b/pallets/payment/src/mock.rs @@ -101,18 +101,17 @@ impl crate::types::DisputeResolver for MockDisputeResolver { } pub struct MockFeeHandler; -impl crate::types::FeeHandler for MockFeeHandler { - fn apply_fees(_from: &AccountId, to: &AccountId, _asset: u32, _amount: u32) -> u32 { +impl crate::types::FeeHandler for MockFeeHandler { + fn apply_fees(_from: &AccountId, to: &AccountId) -> (AccountId, Percent) { match to { - &PAYMENT_RECIPENT_FEE_CHARGED => 1u32, - _ => 0u32, + &PAYMENT_RECIPENT_FEE_CHARGED => (FEE_RECIPIENT_ACCOUNT, Percent::from_percent(10)), + _ => (FEE_RECIPIENT_ACCOUNT, Percent::from_percent(0)), } } } parameter_types! { pub const IncentivePercentage: Percent = Percent::from_percent(10); - pub const FeeRecipientAccount: AccountId = FEE_RECIPIENT_ACCOUNT; } impl payment::Config for Test { @@ -121,7 +120,6 @@ impl payment::Config for Test { type DisputeResolver = MockDisputeResolver; type IncentivePercentage = IncentivePercentage; type FeeHandler = MockFeeHandler; - type FeeRecipientAccount = FeeRecipientAccount; } // Build genesis storage according to the mock runtime. diff --git a/pallets/payment/src/tests.rs b/pallets/payment/src/tests.rs index 9052b76f..475a20f5 100644 --- a/pallets/payment/src/tests.rs +++ b/pallets/payment/src/tests.rs @@ -27,7 +27,8 @@ fn test_create_payment_works() { amount: 20, incentive_amount: 2, state: PaymentState::Created, - resolver_account: RESOLVER_ACCOUNT + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 0) }) ); // the payment amount should be reserved correctly @@ -52,7 +53,8 @@ fn test_create_payment_works() { amount: 20, incentive_amount: 2, state: PaymentState::Created, - resolver_account: RESOLVER_ACCOUNT + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 0) }) ); }); @@ -75,7 +77,8 @@ fn test_cancel_payment_works() { amount: 40, incentive_amount: 4, state: PaymentState::Created, - resolver_account: RESOLVER_ACCOUNT + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 0) }) ); // the payment amount should be reserved @@ -103,7 +106,8 @@ fn test_cancel_payment_works() { amount: 40, incentive_amount: 4, state: PaymentState::Cancelled, - resolver_account: RESOLVER_ACCOUNT + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 0) }) ); // cannot call cancel again @@ -131,7 +135,8 @@ fn test_release_payment_works() { amount: 40, incentive_amount: 4, state: PaymentState::Created, - resolver_account: RESOLVER_ACCOUNT + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 0) }) ); // the payment amount should be reserved @@ -153,7 +158,8 @@ fn test_release_payment_works() { amount: 40, incentive_amount: 4, state: PaymentState::Released, - resolver_account: RESOLVER_ACCOUNT + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 0) }) ); // cannot call release again @@ -218,7 +224,8 @@ fn test_set_state_payment_works() { amount: 40, incentive_amount: 4, state: PaymentState::Released, - resolver_account: RESOLVER_ACCOUNT + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 0) }) ); @@ -250,7 +257,8 @@ fn test_set_state_payment_works() { amount: 40, incentive_amount: 4, state: PaymentState::Cancelled, - resolver_account: RESOLVER_ACCOUNT + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 0) }) ); }); @@ -273,7 +281,8 @@ fn test_charging_fee_payment_works() { amount: 40, incentive_amount: 4, state: PaymentState::Created, - resolver_account: RESOLVER_ACCOUNT + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 4) }) ); // the payment amount should be reserved @@ -283,9 +292,9 @@ fn test_charging_fee_payment_works() { // should succeed for valid payment assert_ok!(Payment::release(Origin::signed(PAYMENT_CREATOR), PAYMENT_RECIPENT_FEE_CHARGED)); // the payment amount should be transferred - assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), 59); + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), 56); assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_RECIPENT_FEE_CHARGED), 40); - assert_eq!(Tokens::free_balance(CURRENCY_ID, &FEE_RECIPIENT_ACCOUNT), 1); + assert_eq!(Tokens::free_balance(CURRENCY_ID, &FEE_RECIPIENT_ACCOUNT), 4); assert_eq!(Tokens::total_issuance(CURRENCY_ID), 100); }); } diff --git a/pallets/payment/src/types.rs b/pallets/payment/src/types.rs index 6f2b507e..1798fa63 100644 --- a/pallets/payment/src/types.rs +++ b/pallets/payment/src/types.rs @@ -1,7 +1,7 @@ #![allow(unused_qualifications)] use parity_scale_codec::{Decode, Encode}; use scale_info::TypeInfo; -use sp_runtime::DispatchResult; +use sp_runtime::{DispatchResult, Percent}; #[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, TypeInfo)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] @@ -11,6 +11,7 @@ pub struct PaymentDetail { pub incentive_amount: Amount, pub state: PaymentState, pub resolver_account: Account, + pub fee_detail: (Account, Amount) } #[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, TypeInfo)] @@ -60,6 +61,8 @@ pub trait DisputeResolver { fn get_origin() -> Account; } -pub trait FeeHandler { - fn apply_fees(from: &Account, to: &Account, asset: Asset, amount: Amount) -> Amount; +/// 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(from: &Account, to: &Account) -> (Account, Percent); } From 62d31d20885f278215ebf2141c8682f7a77d84a0 Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Mon, 20 Dec 2021 23:58:13 +0000 Subject: [PATCH 06/10] add to runtime --- pallets/payment/src/lib.rs | 4 ++-- pallets/payment/src/types.rs | 2 +- runtime/src/lib.rs | 8 ++++++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pallets/payment/src/lib.rs b/pallets/payment/src/lib.rs index 0296ad42..5ee73f97 100644 --- a/pallets/payment/src/lib.rs +++ b/pallets/payment/src/lib.rs @@ -189,7 +189,7 @@ pub mod pallet { incentive_amount, state: PaymentState::Created, resolver_account: T::DisputeResolver::get_origin(), - fee_detail: (fee_recipient, fee_amount) + fee_detail: (fee_recipient, fee_amount), }); match maybe_payment { Some(x) => { @@ -245,7 +245,7 @@ pub mod pallet { payment.asset, &from, &payment.fee_detail.0, // account - payment.fee_detail.1, // amount + payment.fee_detail.1, // amount )?; payment.state = PaymentState::Released; diff --git a/pallets/payment/src/types.rs b/pallets/payment/src/types.rs index 1798fa63..3bb36ada 100644 --- a/pallets/payment/src/types.rs +++ b/pallets/payment/src/types.rs @@ -11,7 +11,7 @@ pub struct PaymentDetail { pub incentive_amount: Amount, pub state: PaymentState, pub resolver_account: Account, - pub fee_detail: (Account, Amount) + pub fee_detail: (Account, Amount), } #[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, TypeInfo)] diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 74aa1172..77069c7f 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -602,6 +602,13 @@ impl virto_payment::DisputeResolver for VirtoDisputeResolver { } } +pub struct VirtoFeeHandler; +impl virto_payment::FeeHandler for VirtoFeeHandler { + fn apply_fees(_from: &AccountId, _to: &AccountId) -> (AccountId, Percent) { + (Sudo::key(), Percent::from_percent(0)) + } +} + parameter_types! { pub const IncentivePercentage: Percent = Percent::from_percent(10); } @@ -611,6 +618,7 @@ impl virto_payment::Config for Runtime { type Asset = Assets; type DisputeResolver = VirtoDisputeResolver; type IncentivePercentage = IncentivePercentage; + type FeeHandler = VirtoFeeHandler; } parameter_types! { From 4e1001ee4e719f365175f4d6b58bcbbac5213b87 Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Tue, 21 Dec 2021 00:01:34 +0000 Subject: [PATCH 07/10] lint fixes --- pallets/payment/src/lib.rs | 6 +++--- runtime/src/lib.rs | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pallets/payment/src/lib.rs b/pallets/payment/src/lib.rs index 5ee73f97..fe889cef 100644 --- a/pallets/payment/src/lib.rs +++ b/pallets/payment/src/lib.rs @@ -243,9 +243,9 @@ pub mod pallet { // transfer fee amount to marketplace T::Asset::transfer( payment.asset, - &from, - &payment.fee_detail.0, // account - payment.fee_detail.1, // amount + &from, // fee is paid by payment creator + &payment.fee_detail.0, // account of fee recipient + payment.fee_detail.1, // amount of fee )?; payment.state = PaymentState::Released; diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 77069c7f..5cebce18 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -605,7 +605,8 @@ impl virto_payment::DisputeResolver for VirtoDisputeResolver { pub struct VirtoFeeHandler; impl virto_payment::FeeHandler for VirtoFeeHandler { fn apply_fees(_from: &AccountId, _to: &AccountId) -> (AccountId, Percent) { - (Sudo::key(), Percent::from_percent(0)) + const VIRTO_MARKETPLACE_FEE_PERCENT: Percent = Percent::from_percent(0); + (Sudo::key(), VIRTO_MARKETPLACE_FEE_PERCENT) } } From 45411557d2e1a3b8807d51407293ce474584357f Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Tue, 21 Dec 2021 00:18:56 +0000 Subject: [PATCH 08/10] improve docs --- pallets/payment/src/types.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pallets/payment/src/types.rs b/pallets/payment/src/types.rs index 3bb36ada..794d969c 100644 --- a/pallets/payment/src/types.rs +++ b/pallets/payment/src/types.rs @@ -3,22 +3,38 @@ use parity_scale_codec::{Decode, Encode}; use scale_info::TypeInfo; use sp_runtime::{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 guarantee proof of funds +and can be released once an agreed upon condition has reached between the payment creator +and recipient. The payment lifecycle is tracked using the state field. +*/ #[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, TypeInfo)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct PaymentDetail { + /// type of asset used for payment pub asset: Asset, + /// amount of asset used for payment pub amount: Amount, + /// incentive amount that is credited to creator for resolving pub incentive_amount: Amount, + /// enum to track payment lifecycle [Created, Released, Cancelled, NeedsReview] pub state: PaymentState, + /// account that can settle any disputes created in the payment pub resolver_account: Account, + /// fee charged and recipient account details pub fee_detail: (Account, Amount), } + #[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, TypeInfo)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum PaymentState { + /// Amounts have been reserved and waiting for release/cancel Created, + /// Payment has been completed and amount transferred Released, + /// All funds unreserved and sent to original owners Cancelled, /// A judge needs to review and release manually NeedsReview, From 07e66201ab23df22017aae7fa7a45e69827344a5 Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Tue, 21 Dec 2021 08:20:36 +0000 Subject: [PATCH 09/10] cargo fmt --- pallets/payment/src/types.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pallets/payment/src/types.rs b/pallets/payment/src/types.rs index 794d969c..05457e2a 100644 --- a/pallets/payment/src/types.rs +++ b/pallets/payment/src/types.rs @@ -3,7 +3,7 @@ use parity_scale_codec::{Decode, Encode}; use scale_info::TypeInfo; use sp_runtime::{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 guarantee proof of funds and can be released once an agreed upon condition has reached between the payment creator @@ -26,7 +26,6 @@ pub struct PaymentDetail { pub fee_detail: (Account, Amount), } - #[derive(Encode, Decode, Debug, Clone, PartialEq, Eq, TypeInfo)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum PaymentState { From 716a8a73e40c89018b2e58a8b977d3d7eb5c8cba Mon Sep 17 00:00:00 2001 From: Stanly Johnson Date: Tue, 21 Dec 2021 12:35:39 +0000 Subject: [PATCH 10/10] fix unreserve bug --- pallets/payment/src/lib.rs | 14 ++++++++++--- pallets/payment/src/tests.rs | 39 +++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/pallets/payment/src/lib.rs b/pallets/payment/src/lib.rs index fe889cef..f7a59765 100644 --- a/pallets/payment/src/lib.rs +++ b/pallets/payment/src/lib.rs @@ -210,7 +210,7 @@ pub mod pallet { }, None => { // reserve the incentive amount from the payment creator - T::Asset::reserve(asset, &from, incentive_amount)?; + T::Asset::reserve(asset, &from, incentive_amount + fee_amount)?; // transfer amount to recipient T::Asset::transfer(asset, &from, &recipient, amount)?; // reserved the amount in the recipient account @@ -237,7 +237,11 @@ pub mod pallet { // ensure the payment is in created state ensure!(payment.state == Created, Error::::PaymentAlreadyReleased); // unreserve the incentive amount back to the creator - T::Asset::unreserve(payment.asset, &from, payment.incentive_amount); + T::Asset::unreserve( + payment.asset, + &from, + payment.incentive_amount + payment.fee_detail.1, + ); // unreserve the amount to the recipent T::Asset::unreserve(payment.asset, &to, payment.amount); // transfer fee amount to marketplace @@ -274,7 +278,11 @@ pub mod pallet { Error::::PaymentAlreadyReleased ); // unreserve the incentive amount from the owner account - T::Asset::unreserve(payment.asset, &from, payment.incentive_amount); + T::Asset::unreserve( + payment.asset, + &from, + payment.incentive_amount + payment.fee_detail.1, + ); T::Asset::unreserve(payment.asset, &to, payment.amount); // transfer amount to creator match T::Asset::transfer(payment.asset, &to, &from, payment.amount) { diff --git a/pallets/payment/src/tests.rs b/pallets/payment/src/tests.rs index 475a20f5..39c2d3e1 100644 --- a/pallets/payment/src/tests.rs +++ b/pallets/payment/src/tests.rs @@ -286,15 +286,52 @@ fn test_charging_fee_payment_works() { }) ); // the payment amount should be reserved - assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), 56); + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), 52); 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_FEE_CHARGED)); // the payment amount should be transferred assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), 56); + assert_eq!(Tokens::total_balance(CURRENCY_ID, &PAYMENT_CREATOR), 56); assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_RECIPENT_FEE_CHARGED), 40); assert_eq!(Tokens::free_balance(CURRENCY_ID, &FEE_RECIPIENT_ACCOUNT), 4); assert_eq!(Tokens::total_issuance(CURRENCY_ID), 100); }); } + +#[test] +fn test_charging_fee_payment_works_when_canceled() { + new_test_ext().execute_with(|| { + // should be able to create a payment with available balance + assert_ok!(Payment::create( + Origin::signed(PAYMENT_CREATOR), + PAYMENT_RECIPENT_FEE_CHARGED, + CURRENCY_ID, + 40, + )); + assert_eq!( + PaymentStore::::get(PAYMENT_CREATOR, PAYMENT_RECIPENT_FEE_CHARGED), + Some(PaymentDetail { + asset: CURRENCY_ID, + amount: 40, + incentive_amount: 4, + state: PaymentState::Created, + resolver_account: RESOLVER_ACCOUNT, + fee_detail: (FEE_RECIPIENT_ACCOUNT, 4) + }) + ); + // the payment amount should be reserved + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), 52); + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_RECIPENT_FEE_CHARGED), 0); + + // should succeed for valid payment + assert_ok!(Payment::cancel(Origin::signed(PAYMENT_RECIPENT_FEE_CHARGED), PAYMENT_CREATOR)); + // the payment amount should be transferred + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_CREATOR), 100); + assert_eq!(Tokens::total_balance(CURRENCY_ID, &PAYMENT_CREATOR), 100); + assert_eq!(Tokens::free_balance(CURRENCY_ID, &PAYMENT_RECIPENT_FEE_CHARGED), 0); + assert_eq!(Tokens::free_balance(CURRENCY_ID, &FEE_RECIPIENT_ACCOUNT), 0); + assert_eq!(Tokens::total_issuance(CURRENCY_ID), 100); + }); +}