Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Changed LPFee to Permill type in Asset Conversion Pallet #14823

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

PatricioNapoli
Copy link
Contributor

Fixes paritytech/polkadot-sdk#88

Changed Liquidity Pool fee in Asset Conversion pallet from a Const to a Permill type.

@PatricioNapoli PatricioNapoli added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Aug 24, 2023
@PatricioNapoli PatricioNapoli added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label Aug 24, 2023
@@ -1169,10 +1171,13 @@ pub mod pallet {
.checked_mul(&1000u32.into())
.ok_or(Error::<T>::Overflow)?;

let fee_mult =
T::HigherPrecisionBalance::from(1_000_000u32 - T::LPFee::get().deconstruct());
Copy link
Contributor

@xlc xlc Aug 24, 2023

Choose a reason for hiding this comment

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

this is just confusing and not a good usage of Permill type

@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/3432642

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Asset Conversion LPFee Should Use a Per{Something} Instead of u32
3 participants