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

Relax Copy to Clone on asset_id (companion) #1900

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion parachains/common/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ where
Assets: fungibles::Inspect<AccountId>,
{
fn contains(id: &<Assets as fungibles::Inspect<AccountId>>::AssetId) -> bool {
!Assets::total_issuance(*id).is_zero()
!Assets::total_issuance(id.clone()).is_zero()
Copy link
Contributor

Choose a reason for hiding this comment

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

@gilescope
is there any reason why not to change trait?
from:
fn total_issuance(asset: Self::AssetId)
to:
fn total_issuance(asset: &Self::AssetId)

like InspectMetadata

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a good question - for copy types the clone is as fast as before. I'm in two minds of whether it would be less or more disruptive if we did that, though I agree if they're not Copy types then that would minimise clones.

}
}

Expand Down
2 changes: 1 addition & 1 deletion parachains/runtimes/testing/penpal/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ where
Assets: fungibles::Inspect<AccountId>,
{
fn contains(id: &<Assets as fungibles::Inspect<AccountId>>::AssetId) -> bool {
!Assets::total_issuance(*id).is_zero()
!Assets::total_issuance(id.clone()).is_zero()
}
}

Expand Down
4 changes: 2 additions & 2 deletions primitives/utility/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl<
// Calculate how much we should charge in the asset_id for such amount of weight
// Require at least a payment of minimum_balance
// Necessary for fully collateral-backed assets
let asset_balance: u128 = FeeCharger::charge_weight_in_fungibles(local_asset_id, weight)
let asset_balance: u128 = FeeCharger::charge_weight_in_fungibles(local_asset_id.clone(), weight)
.map(|amount| {
let minimum_balance = ConcreteAssets::minimum_balance(local_asset_id);
if amount < minimum_balance {
Expand Down Expand Up @@ -173,7 +173,7 @@ impl<
let (local_asset_id, outstanding_balance) =
Matcher::matches_fungibles(&(id.clone(), fun).into()).ok()?;

let minimum_balance = ConcreteAssets::minimum_balance(local_asset_id);
let minimum_balance = ConcreteAssets::minimum_balance(local_asset_id.clone());

// Calculate asset_balance
// This read should have already be cached in buy_weight
Expand Down