diff --git a/integration-tests/src/tests/defaults.rs b/integration-tests/src/tests/defaults.rs index 03b4c32b1..3153edce3 100644 --- a/integration-tests/src/tests/defaults.rs +++ b/integration-tests/src/tests/defaults.rs @@ -23,8 +23,7 @@ use pallet_funding::{ use sp_arithmetic::{FixedPointNumber, Percent}; use macros::generate_accounts; -use pallet_funding::traits::ProvideAssetPrice; -use polimec_common::{USD_DECIMALS, USD_UNIT}; +use polimec_common::{ProvideAssetPrice, USD_DECIMALS, USD_UNIT}; use polimec_runtime::{AccountId, PLMC}; use sp_runtime::{traits::ConstU32, Perquintill}; diff --git a/integration-tests/src/tests/e2e.rs b/integration-tests/src/tests/e2e.rs index d77c229c6..afa727e2d 100644 --- a/integration-tests/src/tests/e2e.rs +++ b/integration-tests/src/tests/e2e.rs @@ -24,12 +24,11 @@ use frame_support::{ }; use itertools::Itertools; use macros::generate_accounts; -use pallet_funding::{traits::ProvideAssetPrice, *}; -use polimec_common::{USD_DECIMALS, USD_UNIT}; +use pallet_funding::*; +use polimec_common::{ProvideAssetPrice, USD_DECIMALS, USD_UNIT}; use sp_arithmetic::{traits::Zero, Percent, Perquintill}; use sp_runtime::{FixedPointNumber, FixedU128}; use xcm_emulator::log; - type UserToCTBalance = Vec<(AccountId, FixedU128, ProjectId)>; generate_accounts!( diff --git a/pallets/funding/src/functions/misc.rs b/pallets/funding/src/functions/misc.rs index a716425c1..4c6a68fc0 100644 --- a/pallets/funding/src/functions/misc.rs +++ b/pallets/funding/src/functions/misc.rs @@ -1,7 +1,6 @@ #[allow(clippy::wildcard_imports)] use super::*; use polimec_common::ProvideAssetPrice; -use sp_runtime::traits::CheckedAdd; // Helper functions // ATTENTION: if this is called directly, it will not be transactional diff --git a/pallets/funding/src/lib.rs b/pallets/funding/src/lib.rs index 78db584b8..b47dad639 100644 --- a/pallets/funding/src/lib.rs +++ b/pallets/funding/src/lib.rs @@ -88,7 +88,7 @@ pub use pallet::*; use pallet_xcm::ensure_response; use polimec_common::{ credentials::{Cid, Did, EnsureOriginWithCredentials, InvestorType, UntrustedToken}, - migration_types::{Migration, MigrationStatusd}, + migration_types::{Migration, MigrationStatus}, }; use polkadot_parachain_primitives::primitives::Id as ParaId; use sp_arithmetic::traits::{One, Saturating}; diff --git a/pallets/funding/src/mock.rs b/pallets/funding/src/mock.rs index 49e8f93d9..ae1d4db9f 100644 --- a/pallets/funding/src/mock.rs +++ b/pallets/funding/src/mock.rs @@ -52,7 +52,6 @@ pub const PLMC: Balance = 10u128.pow(PLMC_DECIMALS as u32); pub const MILLI_PLMC: Balance = PLMC / 10u128.pow(3); pub const MICRO_PLMC: Balance = PLMC / 10u128.pow(6); pub const EXISTENTIAL_DEPOSIT: Balance = 10 * MILLI_PLMC; - pub type Block = frame_system::mocking::MockBlock; pub type AccountId = u32; pub type BlockNumber = u64; diff --git a/pallets/funding/src/runtime_api.rs b/pallets/funding/src/runtime_api.rs index 50e15df79..dd2520862 100644 --- a/pallets/funding/src/runtime_api.rs +++ b/pallets/funding/src/runtime_api.rs @@ -1,5 +1,5 @@ #[allow(clippy::wildcard_imports)] -use crate::{traits::*, *}; +use crate::*; use alloc::collections::BTreeMap; use frame_support::traits::fungibles::{metadata::Inspect as MetadataInspect, Inspect, InspectEnumerable}; use itertools::Itertools; diff --git a/pallets/funding/src/traits.rs b/pallets/funding/src/traits.rs index 8d70b3b42..eb1ab30d0 100644 --- a/pallets/funding/src/traits.rs +++ b/pallets/funding/src/traits.rs @@ -17,10 +17,6 @@ use crate::{Balance, Config, ProjectId}; use frame_support::weights::Weight; use frame_system::pallet_prelude::BlockNumberFor; -use sp_arithmetic::{ - traits::{CheckedDiv, CheckedMul}, - FixedPointNumber, -}; use sp_runtime::DispatchError; pub trait BondingRequirementCalculation { diff --git a/pallets/proxy-bonding/src/functions.rs b/pallets/proxy-bonding/src/functions.rs index cec5a7121..c7c79a8cc 100644 --- a/pallets/proxy-bonding/src/functions.rs +++ b/pallets/proxy-bonding/src/functions.rs @@ -1,40 +1,42 @@ -use frame_support::ensure; -use crate::{AccountIdOf, AssetId, BalanceOf, Config, Pallet, ReleaseType, Releases}; -use frame_support::traits::{ - fungible, - fungible::{Inspect, Mutate, MutateHold}, - fungibles, - fungibles::Mutate as FungiblesMutate, - tokens::{Fortitude, Precision, Preservation}, +use crate::{AccountIdOf, AssetId, BalanceOf, Config, Error, Pallet, ReleaseType, Releases}; +use frame_support::{ + ensure, + traits::{ + fungible, + fungible::{Inspect, Mutate, MutateHold}, + fungibles, + fungibles::Mutate as FungiblesMutate, + tokens::{Fortitude, Precision, Preservation}, + }, }; use frame_system::pallet_prelude::BlockNumberFor; use polimec_common::ProvideAssetPrice; use sp_runtime::{ traits::{AccountIdConversion, Get}, - FixedPointNumber, + DispatchError, FixedPointNumber, }; impl Pallet { /// Calculate the USD fee in `fee_asset` for bonding `bond_amount` of the native token. /// e.g. if the fee is 1%, native token PLMC, fee_asset USDT, bond_amount 1000 PLMC, PLMC price 0.5USD, USDT price 1USD, /// Then the calculated fee would be 1% * 1000 * 0.5 = 5USD, which is 5 USDT at a price of 1USD. - pub fn calculate_fee(bond_amount: BalanceOf, fee_asset: AssetId) -> Result, &'static str> { + pub fn calculate_fee(bond_amount: BalanceOf, fee_asset: AssetId) -> Result, DispatchError> { let bonding_token_price = T::PriceProvider::get_decimals_aware_price( T::BondingTokenId::get(), T::UsdDecimals::get(), T::BondingTokenDecimals::get(), ) - .ok_or("Price not available")?; + .ok_or(Error::::PriceNotAvailable)?; let fee_asset_decimals = >>::decimals(fee_asset); let fee_token_price = T::PriceProvider::get_decimals_aware_price(fee_asset, T::UsdDecimals::get(), fee_asset_decimals) - .ok_or("Price not available")?; + .ok_or(Error::::PriceNotAvailable)?; let bonded_in_usd = bonding_token_price.saturating_mul_int(bond_amount); let fee_in_usd = T::FeePercentage::get() * bonded_in_usd; let fee_in_fee_asset = - fee_token_price.reciprocal().ok_or("Price not available")?.saturating_mul_int(fee_in_usd); + fee_token_price.reciprocal().ok_or(Error::::PriceNotAvailable)?.saturating_mul_int(fee_in_usd); Ok(fee_in_fee_asset) } @@ -47,7 +49,7 @@ impl Pallet { bond_amount: BalanceOf, fee_asset: AssetId, hold_reason: T::RuntimeHoldReason, - ) -> Result<(), &'static str> { + ) -> Result<(), DispatchError> { let treasury = T::Treasury::get(); let bonding_account: AccountIdOf = T::RootId::get().into_sub_account_truncating(derivation_path); let existential_deposit = >::minimum_balance(); @@ -99,9 +101,9 @@ impl Pallet { ) -> Result<(), DispatchError> { let bonding_account: AccountIdOf = T::RootId::get().into_sub_account_truncating(derivation_path); let fee_in_fee_asset = Self::calculate_fee(bond_amount, fee_asset)?; - let release_type = Releases::::get(derivation_path, hold_reason).ok_or("Release type not found")?; + let release_type = Releases::::get(derivation_path, hold_reason).ok_or(Error::::ReleaseTypeNotSet)?; - ensure!(release_type == ReleaseType::Refunded, "Need to set this derivation path / hold reason as `Refunded`"); + ensure!(release_type == ReleaseType::Refunded, Error::::FeeRefundDisallowed); // We know this fee token account is existing thanks to the provider reference of the ED of the native asset, so we can fully move all the funds. // FYI same cannot be said of the `account`. We assume they only hold the fee token so their fee asset balance must not go below the min_balance. diff --git a/pallets/proxy-bonding/src/lib.rs b/pallets/proxy-bonding/src/lib.rs index dab5dda68..a97c4b4af 100644 --- a/pallets/proxy-bonding/src/lib.rs +++ b/pallets/proxy-bonding/src/lib.rs @@ -40,10 +40,7 @@ pub mod pallet { }; use frame_system::pallet_prelude::*; use polimec_common::ProvideAssetPrice; - use sp_runtime::{ - traits::{AccountIdConversion}, - Perbill, TypeId, - }; + use sp_runtime::{traits::AccountIdConversion, Perbill, TypeId}; pub type AssetId = u32; pub type BalanceOf = <::BondingToken as fungible::Inspect>>::Balance; @@ -56,6 +53,7 @@ pub mod pallet { /// Because this pallet emits events, it depends on the runtime's definition of an event. type RuntimeEvent: From> + IsType<::RuntimeEvent>; + /// The overarching hold reason generated by `construct_runtime`. This is used for the bonding. type RuntimeHoldReason: IsType> + Parameter + MaxEncodedLen; /// The pallet giving access to the bonding token @@ -63,8 +61,16 @@ pub mod pallet { + fungible::Mutate + fungible::MutateHold; + /// The number of decimals one unit of the bonding token has. Used to calculate decimal aware prices. + #[pallet::constant] type BondingTokenDecimals: Get; + + /// The number of decimals one unit of USD has. Used to calculate decimal aware prices. USD is not a real asset, but a reference point. + #[pallet::constant] type UsdDecimals: Get; + + /// The id of the bonding token. Used to get the price of the bonding token. + #[pallet::constant] type BondingTokenId: Get; /// The pallet giving access to fee-paying assets, like USDT @@ -72,15 +78,19 @@ pub mod pallet { + fungibles::Mutate, AssetId = AssetId> + fungibles::metadata::Inspect, AssetId = AssetId>; + /// The percentage of the bonded amount in USD that will be taken as a fee in the fee asset. + #[pallet::constant] type FeePercentage: Get; /// Method to get the price of an asset like USDT or PLMC. Likely to come from an oracle type PriceProvider: ProvideAssetPrice; /// The account holding the tokens to be bonded. Normally the treasury + #[pallet::constant] type Treasury: Get; /// The account receiving the fees + #[pallet::constant] type FeeRecipient: Get; /// The id type that can generate sub-accounts @@ -125,9 +135,12 @@ pub mod pallet { ReleaseTypeNotSet, /// Tried to unlock the native tokens and send them back to the treasury, but the release type configured a later unlock block. TooEarlyToUnlock, - /// The release type for the given derivation path / hold reason is set to refunded, which disallows sending fees to the recipient + /// The release type for the given derivation path / hold reason is set to `Refunded`, which disallows sending fees to the recipient FeeToRecipientDisallowed, - + /// The release type for the given derivation path / hold reason is set to `Locked`, which disallows refunding fees + FeeRefundDisallowed, + /// The price of a fee asset or the native token could not be retrieved + PriceNotAvailable, } #[pallet::hooks] diff --git a/pallets/proxy-bonding/src/mock.rs b/pallets/proxy-bonding/src/mock.rs index 02c2306a3..05a70af86 100644 --- a/pallets/proxy-bonding/src/mock.rs +++ b/pallets/proxy-bonding/src/mock.rs @@ -16,10 +16,13 @@ use sp_runtime::{ }; use std::{cell::RefCell, collections::BTreeMap}; pub const NATIVE_DECIMALS: u8 = 10; -pub const FEE_ASSET_DECIMALS: u8 = 6; pub const NATIVE_UNIT: u64 = 1 * 10u64.pow(NATIVE_DECIMALS as u32); pub const MILLI_NATIVE_UNIT: u64 = NATIVE_UNIT / 1_000; -pub const FEE_ASSET_UNIT: u64 = 1 * 10u64.pow(FEE_ASSET_DECIMALS as u32); + +pub const MOCK_FEE_ASSET_ID: u32 = 1337; +pub const MOCK_FEE_ASSET_DECIMALS: u8 = 6; +pub const MOCK_FEE_ASSET_UNIT: u64 = 1 * 10u64.pow(MOCK_FEE_ASSET_DECIMALS as u32); + // Configure a mock runtime to test the pallet. #[frame_support::runtime] mod test_runtime { @@ -165,10 +168,27 @@ impl crate::Config for TestRuntime { type RuntimeEvent = RuntimeEvent; type RuntimeHoldReason = MockRuntimeHoldReason; type Treasury = Treasury; - type UsdDecimals = ConstU8; + type UsdDecimals = ConstU8; } // Build genesis storage according to the mock runtime. pub fn new_test_ext() -> sp_io::TestExternalities { - GenesisConfig::::default().build_storage().unwrap().into() + let mut storage = GenesisConfig::::default().build_storage().unwrap(); + RuntimeGenesisConfig { + assets: AssetsConfig { + assets: vec![(MOCK_FEE_ASSET_ID, 1, true, 100)], + metadata: vec![( + MOCK_FEE_ASSET_ID, + "Tether USD".to_string().into_bytes(), + "USDT".to_string().into_bytes(), + MOCK_FEE_ASSET_DECIMALS, + )], + accounts: vec![], + }, + ..Default::default() + } + .assimilate_storage(&mut storage) + .unwrap(); + + sp_io::TestExternalities::new(storage) } diff --git a/pallets/proxy-bonding/src/tests.rs b/pallets/proxy-bonding/src/tests.rs index 8a93c7edf..7be5ecae0 100644 --- a/pallets/proxy-bonding/src/tests.rs +++ b/pallets/proxy-bonding/src/tests.rs @@ -1,9 +1,12 @@ use crate::{mock::*, AccountIdOf, Error, ReleaseType}; -use frame_support::{assert_noop, assert_ok, traits::{ - fungible::{Inspect, InspectHold, Mutate as FungibleMutate}, - fungibles::{metadata::Mutate as MetadataMutate, Create, Inspect as FungiblesInspect, Mutate}, - Get, -}}; +use frame_support::{ + assert_noop, assert_ok, + traits::{ + fungible::{Inspect, InspectHold, Mutate as FungibleMutate}, + fungibles::{Inspect as FungiblesInspect, Mutate}, + Get, + }, +}; use sp_runtime::traits::AccountIdConversion; #[test] @@ -18,10 +21,10 @@ fn locked_outcome() { // Some pallet calculates he needs to bond 100 native tokens to access some functionality let bond_amount = 200 * NATIVE_UNIT; - let expected_fee = 5 * FEE_ASSET_UNIT; + let expected_fee = 5 * MOCK_FEE_ASSET_UNIT; // User decides to pay the fee with asset 1337 - let fee_asset = 1337u32; + let fee_asset = MOCK_FEE_ASSET_ID; let hold_reason = MockRuntimeHoldReason::Reason; @@ -34,17 +37,17 @@ fn locked_outcome() { // Asset creator needs to pay for metadata which defines decimals >::set_balance(&1, 100 * NATIVE_UNIT); - // The user needs to have 5 + ED of the fee asset on his account - >::create(fee_asset, 1, true, 100).unwrap(); - // Asset creator sets decimals of USDT to 6, same as underlying USD used by the price provider. - >::set( - fee_asset, - &1, - "Tether USD".to_string().into_bytes(), - "USDT".to_string().into_bytes(), - 6, - ) - .unwrap(); + // // The user needs to have 5 + ED of the fee asset on his account + // >::create(fee_asset, 1, true, 100).unwrap(); + // // Asset creator sets decimals of USDT to 6, same as underlying USD used by the price provider. + // >::set( + // fee_asset, + // &1, + // "Tether USD".to_string().into_bytes(), + // "USDT".to_string().into_bytes(), + // 6, + // ) + // .unwrap(); // User should have enough fee tokens to pay the fee, but also some amount for the ED, which is defined above as `min_balance` >::mint_into(fee_asset, &user, expected_fee + 100).unwrap(); @@ -62,15 +65,28 @@ fn locked_outcome() { ProxyBonding::set_release_type(derivation_path, hold_reason.clone(), ReleaseType::Locked(10)); assert_noop!( - ProxyBonding::transfer_bonds_back_to_treasury(RuntimeOrigin::signed(1), derivation_path, hold_reason.clone()), + ProxyBonding::transfer_bonds_back_to_treasury( + RuntimeOrigin::signed(1), + derivation_path, + hold_reason.clone() + ), Error::::TooEarlyToUnlock ); - assert_ok!(ProxyBonding::transfer_fees_to_recipient(RuntimeOrigin::signed(1), derivation_path, hold_reason.clone(), fee_asset)); + assert_ok!(ProxyBonding::transfer_fees_to_recipient( + RuntimeOrigin::signed(1), + derivation_path, + hold_reason.clone(), + fee_asset + )); assert_eq!(>::balance(fee_asset, &fee_recipient), expected_fee); System::set_block_number(10); - assert_ok!(ProxyBonding::transfer_bonds_back_to_treasury(RuntimeOrigin::signed(1), derivation_path, hold_reason.clone())); + assert_ok!(ProxyBonding::transfer_bonds_back_to_treasury( + RuntimeOrigin::signed(1), + derivation_path, + hold_reason.clone() + )); assert_eq!(>::balance(&treasury), ed + bond_amount); }); } @@ -87,10 +103,10 @@ fn refunded_outcome() { // Some pallet calculates he needs to bond 100 native tokens to access some functionality let bond_amount = 200 * NATIVE_UNIT; - let expected_fee = 5 * FEE_ASSET_UNIT; + let expected_fee = 5 * MOCK_FEE_ASSET_UNIT; // User decides to pay the fee with asset 1337 - let fee_asset = 1337u32; + let fee_asset = MOCK_FEE_ASSET_ID; let hold_reason = MockRuntimeHoldReason::Reason; @@ -100,20 +116,7 @@ fn refunded_outcome() { >::set_balance(&treasury, bond_amount + ed * 2); // The fee recipient should exist to receive the USDT >::set_balance(&fee_recipient, ed); - // Asset creator needs to pay for metadata which defines decimals - >::set_balance(&1, 100 * NATIVE_UNIT); - // The user needs to have 5 + ED of the fee asset on his account - >::create(fee_asset, 1, true, 100).unwrap(); - // Asset creator sets decimals of USDT to 6, same as underlying USD used by the price provider. - >::set( - fee_asset, - &1, - "Tether USD".to_string().into_bytes(), - "USDT".to_string().into_bytes(), - 6, - ) - .unwrap(); // User should have enough fee tokens to pay the fee, but also some amount for the ED, which is defined above as `min_balance` >::mint_into(fee_asset, &user, expected_fee + 100).unwrap(); @@ -130,15 +133,23 @@ fn refunded_outcome() { // Mark the release type as Refunded, which leaves the fee to subsequent `refund_fee` calls, and allows to send the bond immediately the treasury. ProxyBonding::set_release_type(derivation_path, hold_reason.clone(), ReleaseType::Refunded); - assert_ok!( - ProxyBonding::transfer_bonds_back_to_treasury(RuntimeOrigin::signed(1), derivation_path, hold_reason.clone()), - ); + assert_ok!(ProxyBonding::transfer_bonds_back_to_treasury( + RuntimeOrigin::signed(1), + derivation_path, + hold_reason.clone() + ),); assert_eq!(>::balance(&treasury), ed + bond_amount); - assert_noop!(ProxyBonding::transfer_fees_to_recipient(RuntimeOrigin::signed(1), derivation_path, hold_reason.clone(), fee_asset), Error::::FeeToRecipientDisallowed); - - assert_ok!( - ProxyBonding::refund_fee(derivation_path, hold_reason.clone(), user, bond_amount, fee_asset) + assert_noop!( + ProxyBonding::transfer_fees_to_recipient( + RuntimeOrigin::signed(1), + derivation_path, + hold_reason.clone(), + fee_asset + ), + Error::::FeeToRecipientDisallowed ); + + assert_ok!(ProxyBonding::refund_fee(derivation_path, hold_reason.clone(), user, bond_amount, fee_asset)); assert_eq!(>::balance(fee_asset, &user), 100 + expected_fee); }); -} \ No newline at end of file +} diff --git a/polimec-common/common/src/lib.rs b/polimec-common/common/src/lib.rs index 925b2d6ae..8533c4459 100644 --- a/polimec-common/common/src/lib.rs +++ b/polimec-common/common/src/lib.rs @@ -242,6 +242,9 @@ impl SendXcm for DummyXcmSender { pub trait ProvideAssetPrice { type AssetId; type Price: FixedPointNumber; + /// Gets the price of an asset. + /// + /// Returns `None` if the price is not available. fn get_price(asset_id: Self::AssetId) -> Option; /// Prices define the relationship between USD/Asset. When to and from that asset, we need to be aware that they might