Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed LPFee to Percent type in Asset Conversion Pallet #1237

Closed
wants to merge 2 commits into from
Closed

Changed LPFee to Percent type in Asset Conversion Pallet #1237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df58524
changed LPFee to Percent
PatricioNapoli Aug 29, 2023
1073b1e
fix compiles
PatricioNapoli Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cumulus/parachains/runtimes/assets/asset-hub-kusama/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use sp_runtime::{
create_runtime_str, generic, impl_opaque_keys,
traits::{AccountIdConversion, AccountIdLookup, BlakeTwo256, Block as BlockT, Verify},
transaction_validity::{TransactionSource, TransactionValidity},
ApplyExtrinsicResult, Permill,
ApplyExtrinsicResult, PerThing, Percent, Permill, Rounding,
};

use sp_std::prelude::*;
Expand Down Expand Up @@ -291,6 +291,7 @@ parameter_types! {
pub const AllowMultiAssetPools: bool = false;
// should be non-zero if AllowMultiAssetPools is true, otherwise can be zero
pub const LiquidityWithdrawalFee: Permill = Permill::from_percent(0);
pub LiquidityPoolFee: Percent = Percent::from_rational_with_rounding(997u32, 1000u32, Rounding::NearestPrefDown).unwrap(); // 0.3%
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this be zero?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah undesired, I meant (3, 1000) which is still rounded to either 0% or 1% depending on rounding rule. I probably need more accuracy for a 0.3% using Permill (there is no inbetween Percent and Permill right?) and then the fee calculations need to take this into consideration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it will have to be a PerMill

}

ord_parameter_types! {
Expand Down Expand Up @@ -342,7 +343,7 @@ impl pallet_asset_conversion::Config for Runtime {
type PoolSetupFeeReceiver = AssetConversionOrigin;
// should be non-zero if `AllowMultiAssetPools` is true, otherwise can be zero.
type LiquidityWithdrawalFee = LiquidityWithdrawalFee;
type LPFee = ConstU32<3>;
type LPFee = LiquidityPoolFee;
type PalletId = AssetConversionPalletId;
type AllowMultiAssetPools = AllowMultiAssetPools;
type MaxSwapPathLength = ConstU32<4>;
Expand Down
5 changes: 3 additions & 2 deletions cumulus/parachains/runtimes/assets/asset-hub-westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ use sp_runtime::{
create_runtime_str, generic, impl_opaque_keys,
traits::{AccountIdConversion, AccountIdLookup, BlakeTwo256, Block as BlockT, Verify},
transaction_validity::{TransactionSource, TransactionValidity},
ApplyExtrinsicResult, Permill, RuntimeDebug,
ApplyExtrinsicResult, PerThing, Percent, Permill, Rounding, RuntimeDebug,
};
use sp_std::prelude::*;
#[cfg(feature = "std")]
Expand Down Expand Up @@ -269,6 +269,7 @@ parameter_types! {
pub const AllowMultiAssetPools: bool = false;
// should be non-zero if AllowMultiAssetPools is true, otherwise can be zero
pub const LiquidityWithdrawalFee: Permill = Permill::from_percent(0);
pub LiquidityPoolFee: Percent = Percent::from_rational_with_rounding(997u32, 1000u32, Rounding::NearestPrefDown).unwrap(); // 0.3%
}

ord_parameter_types! {
Expand Down Expand Up @@ -318,7 +319,7 @@ impl pallet_asset_conversion::Config for Runtime {
type PoolSetupFee = ConstU128<0>; // Asset class deposit fees are sufficient to prevent spam
type PoolSetupFeeReceiver = AssetConversionOrigin;
type LiquidityWithdrawalFee = LiquidityWithdrawalFee; // should be non-zero if AllowMultiAssetPools is true, otherwise can be zero.
type LPFee = ConstU32<3>;
type LPFee = LiquidityPoolFee;
type PalletId = AssetConversionPalletId;
type AllowMultiAssetPools = AllowMultiAssetPools;
type MaxSwapPathLength = ConstU32<4>;
Expand Down
7 changes: 4 additions & 3 deletions substrate/bin/node/runtime/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ use sp_runtime::{
OpaqueKeys, SaturatedConversion, StaticLookup,
},
transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity},
ApplyExtrinsicResult, FixedPointNumber, FixedU128, Perbill, Percent, Permill, Perquintill,
RuntimeDebug,
ApplyExtrinsicResult, FixedPointNumber, FixedU128, PerThing, Perbill, Percent, Permill,
Perquintill, Rounding, RuntimeDebug,
};
use sp_std::prelude::*;
#[cfg(any(feature = "std", test))]
Expand Down Expand Up @@ -1617,6 +1617,7 @@ parameter_types! {
pub const PoolSetupFee: Balance = 1 * DOLLARS; // should be more or equal to the existential deposit
pub const MintMinLiquidity: Balance = 100; // 100 is good enough when the main currency has 10-12 decimals.
pub const LiquidityWithdrawalFee: Permill = Permill::from_percent(0); // should be non-zero if AllowMultiAssetPools is true, otherwise can be zero.
pub LiquidityPoolFee: Percent = Percent::from_rational_with_rounding(997u32, 1000u32, Rounding::NearestPrefDown).unwrap(); // 0.3%
}

impl pallet_asset_conversion::Config for Runtime {
Expand All @@ -1631,7 +1632,7 @@ impl pallet_asset_conversion::Config for Runtime {
type MultiAssetId = NativeOrAssetId<u32>;
type PoolAssetId = <Self as pallet_assets::Config<Instance2>>::AssetId;
type PalletId = AssetConversionPalletId;
type LPFee = ConstU32<3>; // means 0.3%
type LPFee = LiquidityPoolFee;
type PoolSetupFee = PoolSetupFee;
type PoolSetupFeeReceiver = AssetConversionOrigin;
type LiquidityWithdrawalFee = LiquidityWithdrawalFee;
Expand Down
28 changes: 12 additions & 16 deletions substrate/frame/asset-conversion/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub mod pallet {
},
BoundedBTreeSet, PalletId,
};
use sp_arithmetic::Permill;
use sp_arithmetic::{Percent, Permill};
use sp_runtime::{
traits::{IntegerSquareRoot, One, Zero},
Saturating,
Expand Down Expand Up @@ -173,7 +173,7 @@ pub mod pallet {

/// A % the liquidity providers will take of every swap. Represents 10ths of a percent.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// A % the liquidity providers will take of every swap. Represents 10ths of a percent.
/// A % the liquidity providers will take of every swap.

#[pallet::constant]
type LPFee: Get<u32>;
type LPFee: Get<Percent>;

/// A one-time fee to setup the pool.
#[pallet::constant]
Expand Down Expand Up @@ -1124,18 +1124,16 @@ pub mod pallet {
return Err(Error::<T>::ZeroLiquidity.into())
}

let amount_in_with_fee = amount_in
.checked_mul(&(T::HigherPrecisionBalance::from(1000u32) - (T::LPFee::get().into())))
.ok_or(Error::<T>::Overflow)?;
let fee_mult = T::HigherPrecisionBalance::from(T::LPFee::get() * 1u32);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO, temporal, same as below


let amount_in_with_fee =
amount_in.checked_mul(&fee_mult).ok_or(Error::<T>::Overflow)?;

let numerator =
amount_in_with_fee.checked_mul(&reserve_out).ok_or(Error::<T>::Overflow)?;

let denominator = reserve_in
.checked_mul(&1000u32.into())
.ok_or(Error::<T>::Overflow)?
.checked_add(&amount_in_with_fee)
.ok_or(Error::<T>::Overflow)?;
let denominator =
reserve_in.checked_add(&amount_in_with_fee).ok_or(Error::<T>::Overflow)?;

let result = numerator.checked_div(&denominator).ok_or(Error::<T>::Overflow)?;

Expand Down Expand Up @@ -1163,16 +1161,14 @@ pub mod pallet {
Err(Error::<T>::AmountOutTooHigh.into())?
}

let numerator = reserve_in
.checked_mul(&amount_out)
.ok_or(Error::<T>::Overflow)?
.checked_mul(&1000u32.into())
.ok_or(Error::<T>::Overflow)?;
let numerator = reserve_in.checked_mul(&amount_out).ok_or(Error::<T>::Overflow)?;

let fee_mult = T::HigherPrecisionBalance::from(T::LPFee::get() * 1u32);

let denominator = reserve_out
.checked_sub(&amount_out)
.ok_or(Error::<T>::Overflow)?
.checked_mul(&(T::HigherPrecisionBalance::from(1000u32) - T::LPFee::get().into()))
.checked_mul(&fee_mult)
.ok_or(Error::<T>::Overflow)?;

let result = numerator
Expand Down
5 changes: 3 additions & 2 deletions substrate/frame/asset-conversion/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use frame_support::{
PalletId,
};
use frame_system::{EnsureSigned, EnsureSignedBy};
use sp_arithmetic::Permill;
use sp_arithmetic::{PerThing, Percent, Permill, Rounding};
use sp_core::H256;
use sp_runtime::{
traits::{AccountIdConversion, BlakeTwo256, IdentityLookup},
Expand Down Expand Up @@ -143,6 +143,7 @@ parameter_types! {
pub const AssetConversionPalletId: PalletId = PalletId(*b"py/ascon");
pub storage AllowMultiAssetPools: bool = true;
pub storage LiquidityWithdrawalFee: Permill = Permill::from_percent(0); // should be non-zero if AllowMultiAssetPools is true, otherwise can be zero
pub LiquidityPoolFee: Percent = Percent::from_rational_with_rounding(997u32, 1000u32, Rounding::NearestPrefDown).unwrap(); // 0.3%
}

ord_parameter_types! {
Expand All @@ -159,7 +160,7 @@ impl Config for Test {
type PoolAssets = PoolAssets;
type PalletId = AssetConversionPalletId;
type WeightInfo = ();
type LPFee = ConstU32<3>; // means 0.3%
type LPFee = LiquidityPoolFee;
type PoolSetupFee = ConstU128<100>; // should be more or equal to the existential deposit
type PoolSetupFeeReceiver = AssetConversionOrigin;
type LiquidityWithdrawalFee = LiquidityWithdrawalFee;
Expand Down
5 changes: 3 additions & 2 deletions substrate/frame/transaction-payment/asset-conversion-tx-payment/src/mock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use pallet_transaction_payment::CurrencyAdapter;
use sp_core::H256;
use sp_runtime::{
traits::{AccountIdConversion, BlakeTwo256, IdentityLookup, SaturatedConversion},
Permill,
PerThing, Percent, Permill, Rounding,
};

type Block = frame_system::mocking::MockBlock<Runtime>;
Expand Down Expand Up @@ -225,6 +225,7 @@ parameter_types! {
pub storage AllowMultiAssetPools: bool = false;
// should be non-zero if AllowMultiAssetPools is true, otherwise can be zero
pub storage LiquidityWithdrawalFee: Permill = Permill::from_percent(0);
pub LiquidityPoolFee: Percent = Percent::from_rational_with_rounding(997u32, 1000u32, Rounding::NearestPrefDown).unwrap(); // 0.3%
pub const MaxSwapPathLength: u32 = 4;
}

Expand All @@ -242,7 +243,7 @@ impl pallet_asset_conversion::Config for Runtime {
type PoolAssets = PoolAssets;
type PalletId = AssetConversionPalletId;
type WeightInfo = ();
type LPFee = ConstU32<3>; // means 0.3%
type LPFee = LiquidityPoolFee;
type PoolSetupFee = ConstU64<100>; // should be more or equal to the existential deposit
type PoolSetupFeeReceiver = AssetConversionOrigin;
type LiquidityWithdrawalFee = LiquidityWithdrawalFee;
Expand Down