From 1177877631a11b64df6f019b5390a8a7018e3a3f Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Wed, 15 Mar 2023 15:42:23 +0000 Subject: [PATCH 1/4] refactor: inconsistent BalanceConversion fn --- frame/assets/src/tests.rs | 9 +++------ frame/assets/src/types.rs | 2 +- frame/support/src/traits/tokens/misc.rs | 5 +++-- .../transaction-payment/asset-tx-payment/src/payment.rs | 6 +++--- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs index bc61810a76ac4..d7ddf7c7b000e 100644 --- a/frame/assets/src/tests.rs +++ b/frame/assets/src/tests.rs @@ -1164,19 +1164,16 @@ fn balance_conversion_should_work() { assert_ok!(Assets::force_create(RuntimeOrigin::root(), not_sufficient, 1, false, 10)); assert_eq!(asset_ids(), vec![23, 42, 999]); assert_eq!( - BalanceToAssetBalance::::to_asset_balance(100, 1234), + BalanceToAssetBalance::::convert(100, 1234), Err(ConversionError::AssetMissing) ); assert_eq!( - BalanceToAssetBalance::::to_asset_balance( - 100, - not_sufficient - ), + BalanceToAssetBalance::::convert(100, not_sufficient), Err(ConversionError::AssetNotSufficient) ); // 10 / 1 == 10 -> the conversion should 10x the value assert_eq!( - BalanceToAssetBalance::::to_asset_balance(100, id), + BalanceToAssetBalance::::convert(100, id), Ok(100 * 10) ); }); diff --git a/frame/assets/src/types.rs b/frame/assets/src/types.rs index d50461ba59898..71e1301a7b5ce 100644 --- a/frame/assets/src/types.rs +++ b/frame/assets/src/types.rs @@ -245,7 +245,7 @@ where /// /// Will return `Err` if the asset is not found, not sufficient or the fungible's minimum /// balance is zero. - fn to_asset_balance( + fn convert( balance: BalanceOf, asset_id: AssetIdOf, ) -> Result, ConversionError> { diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index 6113642c83460..060d541e00c7b 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -189,10 +189,11 @@ impl< { } -/// Converts a balance value into an asset balance. +/// Provides a conversion mechanism between two balances. +/// The most natural conversion would be native to asset balance. pub trait BalanceConversion { type Error; - fn to_asset_balance(balance: InBalance, asset_id: AssetId) -> Result; + fn convert(balance: InBalance, asset_id: AssetId) -> Result; } /// Trait to handle asset locking mechanism to ensure interactions with the asset can be implemented diff --git a/frame/transaction-payment/asset-tx-payment/src/payment.rs b/frame/transaction-payment/asset-tx-payment/src/payment.rs index e273ffcfc565c..475e04f828331 100644 --- a/frame/transaction-payment/asset-tx-payment/src/payment.rs +++ b/frame/transaction-payment/asset-tx-payment/src/payment.rs @@ -118,7 +118,7 @@ where // less than one (e.g. 0.5) but gets rounded down by integer division we introduce a minimum // fee. let min_converted_fee = if fee.is_zero() { Zero::zero() } else { One::one() }; - let converted_fee = CON::to_asset_balance(fee, asset_id) + let converted_fee = CON::convert(fee, asset_id) .map_err(|_| TransactionValidityError::from(InvalidTransaction::Payment))? .max(min_converted_fee); let can_withdraw = @@ -146,10 +146,10 @@ where ) -> Result<(AssetBalanceOf, AssetBalanceOf), TransactionValidityError> { let min_converted_fee = if corrected_fee.is_zero() { Zero::zero() } else { One::one() }; // Convert the corrected fee and tip into the asset used for payment. - let converted_fee = CON::to_asset_balance(corrected_fee, paid.asset()) + let converted_fee = CON::convert(corrected_fee, paid.asset()) .map_err(|_| -> TransactionValidityError { InvalidTransaction::Payment.into() })? .max(min_converted_fee); - let converted_tip = CON::to_asset_balance(tip, paid.asset()) + let converted_tip = CON::convert(tip, paid.asset()) .map_err(|_| -> TransactionValidityError { InvalidTransaction::Payment.into() })?; // Calculate how much refund we should return. From 7dcc60a75c30a9a14d877408cbc9204320737fc7 Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Tue, 28 Mar 2023 13:58:19 +0200 Subject: [PATCH 2/4] Revert "refactor: inconsistent BalanceConversion fn" This reverts commit 1177877631a11b64df6f019b5390a8a7018e3a3f. --- frame/assets/src/tests.rs | 9 ++++++--- frame/assets/src/types.rs | 2 +- frame/support/src/traits/tokens/misc.rs | 5 ++--- .../transaction-payment/asset-tx-payment/src/payment.rs | 6 +++--- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs index d7ddf7c7b000e..bc61810a76ac4 100644 --- a/frame/assets/src/tests.rs +++ b/frame/assets/src/tests.rs @@ -1164,16 +1164,19 @@ fn balance_conversion_should_work() { assert_ok!(Assets::force_create(RuntimeOrigin::root(), not_sufficient, 1, false, 10)); assert_eq!(asset_ids(), vec![23, 42, 999]); assert_eq!( - BalanceToAssetBalance::::convert(100, 1234), + BalanceToAssetBalance::::to_asset_balance(100, 1234), Err(ConversionError::AssetMissing) ); assert_eq!( - BalanceToAssetBalance::::convert(100, not_sufficient), + BalanceToAssetBalance::::to_asset_balance( + 100, + not_sufficient + ), Err(ConversionError::AssetNotSufficient) ); // 10 / 1 == 10 -> the conversion should 10x the value assert_eq!( - BalanceToAssetBalance::::convert(100, id), + BalanceToAssetBalance::::to_asset_balance(100, id), Ok(100 * 10) ); }); diff --git a/frame/assets/src/types.rs b/frame/assets/src/types.rs index 71e1301a7b5ce..d50461ba59898 100644 --- a/frame/assets/src/types.rs +++ b/frame/assets/src/types.rs @@ -245,7 +245,7 @@ where /// /// Will return `Err` if the asset is not found, not sufficient or the fungible's minimum /// balance is zero. - fn convert( + fn to_asset_balance( balance: BalanceOf, asset_id: AssetIdOf, ) -> Result, ConversionError> { diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index 060d541e00c7b..6113642c83460 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -189,11 +189,10 @@ impl< { } -/// Provides a conversion mechanism between two balances. -/// The most natural conversion would be native to asset balance. +/// Converts a balance value into an asset balance. pub trait BalanceConversion { type Error; - fn convert(balance: InBalance, asset_id: AssetId) -> Result; + fn to_asset_balance(balance: InBalance, asset_id: AssetId) -> Result; } /// Trait to handle asset locking mechanism to ensure interactions with the asset can be implemented diff --git a/frame/transaction-payment/asset-tx-payment/src/payment.rs b/frame/transaction-payment/asset-tx-payment/src/payment.rs index 475e04f828331..e273ffcfc565c 100644 --- a/frame/transaction-payment/asset-tx-payment/src/payment.rs +++ b/frame/transaction-payment/asset-tx-payment/src/payment.rs @@ -118,7 +118,7 @@ where // less than one (e.g. 0.5) but gets rounded down by integer division we introduce a minimum // fee. let min_converted_fee = if fee.is_zero() { Zero::zero() } else { One::one() }; - let converted_fee = CON::convert(fee, asset_id) + let converted_fee = CON::to_asset_balance(fee, asset_id) .map_err(|_| TransactionValidityError::from(InvalidTransaction::Payment))? .max(min_converted_fee); let can_withdraw = @@ -146,10 +146,10 @@ where ) -> Result<(AssetBalanceOf, AssetBalanceOf), TransactionValidityError> { let min_converted_fee = if corrected_fee.is_zero() { Zero::zero() } else { One::one() }; // Convert the corrected fee and tip into the asset used for payment. - let converted_fee = CON::convert(corrected_fee, paid.asset()) + let converted_fee = CON::to_asset_balance(corrected_fee, paid.asset()) .map_err(|_| -> TransactionValidityError { InvalidTransaction::Payment.into() })? .max(min_converted_fee); - let converted_tip = CON::convert(tip, paid.asset()) + let converted_tip = CON::to_asset_balance(tip, paid.asset()) .map_err(|_| -> TransactionValidityError { InvalidTransaction::Payment.into() })?; // Calculate how much refund we should return. From b0e812a5c9fa94f5828cc3821ac94a3573f5bc5c Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Tue, 28 Mar 2023 14:08:32 +0200 Subject: [PATCH 3/4] refactor: rename BalanceConversion trait --- frame/assets/src/tests.rs | 2 +- frame/assets/src/types.rs | 4 ++-- frame/support/src/traits/tokens.rs | 2 +- frame/support/src/traits/tokens/misc.rs | 5 +++-- frame/transaction-payment/asset-tx-payment/src/payment.rs | 6 +++--- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/frame/assets/src/tests.rs b/frame/assets/src/tests.rs index bc61810a76ac4..abcc86f3134c5 100644 --- a/frame/assets/src/tests.rs +++ b/frame/assets/src/tests.rs @@ -1156,7 +1156,7 @@ fn set_min_balance_should_work() { #[test] fn balance_conversion_should_work() { new_test_ext().execute_with(|| { - use frame_support::traits::tokens::BalanceConversion; + use frame_support::traits::tokens::ConversionToAssetBalance; let id = 42; assert_ok!(Assets::force_create(RuntimeOrigin::root(), id, 1, true, 10)); diff --git a/frame/assets/src/types.rs b/frame/assets/src/types.rs index d50461ba59898..c83e764f4a68d 100644 --- a/frame/assets/src/types.rs +++ b/frame/assets/src/types.rs @@ -20,7 +20,7 @@ use super::*; use frame_support::{ pallet_prelude::*, - traits::{fungible, tokens::BalanceConversion}, + traits::{fungible, tokens::ConversionToAssetBalance}, }; use sp_runtime::{traits::Convert, FixedPointNumber, FixedPointOperand, FixedU128}; @@ -228,7 +228,7 @@ type BalanceOf = >>::Balance; /// Converts a balance value into an asset balance based on the ratio between the fungible's /// minimum balance and the minimum asset balance. pub struct BalanceToAssetBalance(PhantomData<(F, T, CON, I)>); -impl BalanceConversion, AssetIdOf, AssetBalanceOf> +impl ConversionToAssetBalance, AssetIdOf, AssetBalanceOf> for BalanceToAssetBalance where F: fungible::Inspect>, diff --git a/frame/support/src/traits/tokens.rs b/frame/support/src/traits/tokens.rs index e1a96f621bd4f..b43442d8fac74 100644 --- a/frame/support/src/traits/tokens.rs +++ b/frame/support/src/traits/tokens.rs @@ -28,6 +28,6 @@ pub mod nonfungibles; pub mod nonfungibles_v2; pub use imbalance::Imbalance; pub use misc::{ - AssetId, Balance, BalanceConversion, BalanceStatus, ConvertRank, DepositConsequence, + AssetId, Balance, BalanceStatus, ConversionToAssetBalance, ConvertRank, DepositConsequence, ExistenceRequirement, GetSalary, Locker, WithdrawConsequence, WithdrawReasons, }; diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index 6113642c83460..03ae944f40395 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -190,9 +190,10 @@ impl< } /// Converts a balance value into an asset balance. -pub trait BalanceConversion { +pub trait ConversionToAssetBalance { type Error; - fn to_asset_balance(balance: InBalance, asset_id: AssetId) -> Result; + fn to_asset_balance(balance: InBalance, asset_id: AssetId) + -> Result; } /// Trait to handle asset locking mechanism to ensure interactions with the asset can be implemented diff --git a/frame/transaction-payment/asset-tx-payment/src/payment.rs b/frame/transaction-payment/asset-tx-payment/src/payment.rs index e273ffcfc565c..e937ca98daec3 100644 --- a/frame/transaction-payment/asset-tx-payment/src/payment.rs +++ b/frame/transaction-payment/asset-tx-payment/src/payment.rs @@ -21,7 +21,7 @@ use codec::FullCodec; use frame_support::{ traits::{ fungibles::{Balanced, CreditOf, Inspect}, - tokens::{Balance, BalanceConversion}, + tokens::{Balance, ConversionToAssetBalance}, }, unsigned::TransactionValidityError, }; @@ -85,7 +85,7 @@ impl> HandleCredit for () { } /// Implements the asset transaction for a balance to asset converter (implementing -/// [`BalanceConversion`]) and a credit handler (implementing [`HandleCredit`]). +/// [`ConversionToAssetBalance`]) and a credit handler (implementing [`HandleCredit`]). /// /// The credit handler is given the complete fee in terms of the asset used for the transaction. pub struct FungiblesAdapter(PhantomData<(CON, HC)>); @@ -95,7 +95,7 @@ pub struct FungiblesAdapter(PhantomData<(CON, HC)>); impl OnChargeAssetTransaction for FungiblesAdapter where T: Config, - CON: BalanceConversion, AssetIdOf, AssetBalanceOf>, + CON: ConversionToAssetBalance, AssetIdOf, AssetBalanceOf>, HC: HandleCredit, AssetIdOf: FullCodec + Copy + MaybeSerializeDeserialize + Debug + Default + Eq + TypeInfo, { From 5aec63f443eb9a96731fbb84bbc7cb3bbe23d09e Mon Sep 17 00:00:00 2001 From: William Freudenberger Date: Tue, 28 Mar 2023 14:08:56 +0200 Subject: [PATCH 4/4] feat: add ConversionFromAssetBalance --- frame/support/src/traits/tokens/misc.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/frame/support/src/traits/tokens/misc.rs b/frame/support/src/traits/tokens/misc.rs index 03ae944f40395..ce20b14cbc21f 100644 --- a/frame/support/src/traits/tokens/misc.rs +++ b/frame/support/src/traits/tokens/misc.rs @@ -196,6 +196,15 @@ pub trait ConversionToAssetBalance { -> Result; } +/// Converts an asset balance value into balance. +pub trait ConversionFromAssetBalance { + type Error; + fn from_asset_balance( + balance: AssetBalance, + asset_id: AssetId, + ) -> Result; +} + /// Trait to handle asset locking mechanism to ensure interactions with the asset can be implemented /// downstream to extend logic of Uniques current functionality. pub trait Locker {