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

[Enhancement] Use XCM V3 for initiate_teleport weight calc #2102

Merged
merged 9 commits into from
Feb 9, 2023
Merged

[Enhancement] Use XCM V3 for initiate_teleport weight calc #2102

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
16cd773
[Enhancement] Use XCM V3 for initiate_teleport weight calc
ruseinov Jan 17, 2023
202b067
deref
ruseinov Jan 17, 2023
3fe90ae
replicate in all the runtimes
ruseinov Feb 5, 2023
1f884d1
fmt
ruseinov Feb 5, 2023
9201905
better handling for AllOf
ruseinov Feb 9, 2023
904db9e
Merge remote-tracking branch 'origin/master' into ru/enhancement/iini…
Feb 9, 2023
e59c552
fmt
ruseinov Feb 9, 2023
0cf2bac
small type fix
ruseinov Feb 9, 2023
bdd401e
replicate the fix for all runtimes
ruseinov Feb 9, 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
12 changes: 7 additions & 5 deletions parachains/runtimes/assets/statemine/src/weights/xcm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ impl WeighMultiAssets for MultiAssetFilter {
match self {
Self::Definite(assets) =>
weight.saturating_mul(assets.inner().into_iter().count() as u64),
Self::Wild(_) => weight.saturating_mul(MAX_ASSETS as u64),
Self::Wild(asset) => match asset {
All => weight.saturating_mul(MAX_ASSETS as u64),
AllOf { .. } => weight,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm actually still uncertain about this -- comments on AllOfCounted says that each individual instance of an NFT is considered a different class of asset, and so wouldn't it make more sense to use the number of NFT instances as an upper bound here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, good question. Let me check it out and come back to you.

Copy link
Contributor Author

@ruseinov ruseinov Feb 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I think this makes sense. I'm not sure though where the NFT instance count should be coming from in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

So this requires you to actually look inside the holding register and determine just how many AssetInstances there are... We could match against fun in AllOf and see if it's a NonFungible, and if so, it would basically be weight * MaxAssetsIntoHolding::get(), otherwise it's just weight.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KiChjang One thing that disturbs me a bit is the comment below, should we then do something like weight * MaxAssetsIntoHolding::get() * 2 to be safe?

	/// The maximum number of assets we target to have in the Holding Register at any one time.
	///
	/// NOTE: In the worse case, the Holding Register may contain up to twice as many assets as this
	/// and any benchmarks should take that into account.
	type MaxAssetsIntoHolding: Get<u32>;

AllCounted(count) => weight.saturating_mul(*count as u64),
AllOfCounted { count, .. } => weight.saturating_mul(*count as u64),
},
}
}
}
Expand Down Expand Up @@ -138,10 +143,7 @@ impl<Call> XcmWeightInfo<Call> for StatemineXcmWeight<Call> {
_dest: &MultiLocation,
_xcm: &Xcm<()>,
) -> Weight {
// Hardcoded till the XCM pallet is fixed
let hardcoded_weight = Weight::from_ref_time(200_000_000 as u64);
let weight = assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport());
hardcoded_weight.min(weight)
assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport())
}
fn report_holding(_response_info: &QueryResponseInfo, _assets: &MultiAssetFilter) -> Weight {
XcmGeneric::<Runtime>::report_holding()
Expand Down
12 changes: 7 additions & 5 deletions parachains/runtimes/assets/statemint/src/weights/xcm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ impl WeighMultiAssets for MultiAssetFilter {
match self {
Self::Definite(assets) =>
weight.saturating_mul(assets.inner().into_iter().count() as u64),
Self::Wild(_) => weight.saturating_mul(MAX_ASSETS as u64),
Self::Wild(asset) => match asset {
All => weight.saturating_mul(MAX_ASSETS as u64),
AllOf { .. } => weight,
AllCounted(count) => weight.saturating_mul(*count as u64),
AllOfCounted { count, .. } => weight.saturating_mul(*count as u64),
},
}
}
}
Expand Down Expand Up @@ -138,10 +143,7 @@ impl<Call> XcmWeightInfo<Call> for StatemintXcmWeight<Call> {
_dest: &MultiLocation,
_xcm: &Xcm<()>,
) -> Weight {
// Hardcoded till the XCM pallet is fixed
let hardcoded_weight = Weight::from_ref_time(200_000_000 as u64);
let weight = assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport());
hardcoded_weight.min(weight)
assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport())
}
fn report_holding(_response_info: &QueryResponseInfo, _assets: &MultiAssetFilter) -> Weight {
XcmGeneric::<Runtime>::report_holding()
Expand Down
12 changes: 7 additions & 5 deletions parachains/runtimes/assets/westmint/src/weights/xcm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ impl WeighMultiAssets for MultiAssetFilter {
match self {
Self::Definite(assets) =>
weight.saturating_mul(assets.inner().into_iter().count() as u64),
Self::Wild(_) => weight.saturating_mul(MAX_ASSETS as u64),
Self::Wild(asset) => match asset {
All => weight.saturating_mul(MAX_ASSETS as u64),
AllOf { .. } => weight,
AllCounted(count) => weight.saturating_mul(*count as u64),
AllOfCounted { count, .. } => weight.saturating_mul(*count as u64),
},
}
}
}
Expand Down Expand Up @@ -138,10 +143,7 @@ impl<Call> XcmWeightInfo<Call> for WestmintXcmWeight<Call> {
_dest: &MultiLocation,
_xcm: &Xcm<()>,
) -> Weight {
// Hardcoded till the XCM pallet is fixed
let hardcoded_weight = Weight::from_ref_time(200_000_000 as u64);
let weight = assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport());
hardcoded_weight.min(weight)
assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport())
}
fn report_holding(_response_info: &QueryResponseInfo, _assets: &MultiAssetFilter) -> Weight {
XcmGeneric::<Runtime>::report_holding()
Expand Down
12 changes: 7 additions & 5 deletions parachains/runtimes/bridge-hubs/bridge-hub-kusama/src/weights/xcm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ impl WeighMultiAssets for MultiAssetFilter {
match self {
Self::Definite(assets) =>
weight.saturating_mul(assets.inner().into_iter().count() as u64),
Self::Wild(_) => weight.saturating_mul(MAX_ASSETS as u64),
Self::Wild(asset) => match asset {
All => weight.saturating_mul(MAX_ASSETS as u64),
AllOf { .. } => weight,
AllCounted(count) => weight.saturating_mul(*count as u64),
AllOfCounted { count, .. } => weight.saturating_mul(*count as u64),
},
}
}
}
Expand Down Expand Up @@ -138,10 +143,7 @@ impl<Call> XcmWeightInfo<Call> for BridgeHubKusamaXcmWeight<Call> {
_dest: &MultiLocation,
_xcm: &Xcm<()>,
) -> Weight {
// Hardcoded till the XCM pallet is fixed
let hardcoded_weight = Weight::from_ref_time(200_000_000 as u64);
let weight = assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport());
hardcoded_weight.min(weight)
assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport())
}
fn report_holding(_response_info: &QueryResponseInfo, _assets: &MultiAssetFilter) -> Weight {
XcmGeneric::<Runtime>::report_holding()
Expand Down
12 changes: 7 additions & 5 deletions parachains/runtimes/bridge-hubs/bridge-hub-rococo/src/weights/xcm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ impl WeighMultiAssets for MultiAssetFilter {
match self {
Self::Definite(assets) =>
weight.saturating_mul(assets.inner().into_iter().count() as u64),
Self::Wild(_) => weight.saturating_mul(MAX_ASSETS as u64),
Self::Wild(asset) => match asset {
All => weight.saturating_mul(MAX_ASSETS as u64),
AllOf { .. } => weight,
AllCounted(count) => weight.saturating_mul(*count as u64),
AllOfCounted { count, .. } => weight.saturating_mul(*count as u64),
},
}
}
}
Expand Down Expand Up @@ -138,10 +143,7 @@ impl<Call> XcmWeightInfo<Call> for BridgeHubRococoXcmWeight<Call> {
_dest: &MultiLocation,
_xcm: &Xcm<()>,
) -> Weight {
// Hardcoded till the XCM pallet is fixed
let hardcoded_weight = Weight::from_ref_time(200_000_000 as u64);
let weight = assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport());
hardcoded_weight.min(weight)
assets.weigh_multi_assets(XcmFungibleWeight::<Runtime>::initiate_teleport())
}
fn report_holding(_response_info: &QueryResponseInfo, _assets: &MultiAssetFilter) -> Weight {
XcmGeneric::<Runtime>::report_holding()
Expand Down