Skip to content

Fix benchmarks of saft-registry#327

Merged
clearloop merged 13 commits intomainfrom
cl/fix-saft-registry
Sep 3, 2021
Merged

Fix benchmarks of saft-registry#327
clearloop merged 13 commits intomainfrom
cl/fix-saft-registry

Conversation

@clearloop
Copy link
Contributor

@clearloop clearloop commented Sep 3, 2021

Changes

Tests

cargo b --release --features runtime-benchmarks

Issues

@clearloop clearloop added the needs review PR needs reviewing label Sep 3, 2021
@clearloop clearloop requested a review from mattsse September 3, 2021 07:58
Comment on lines +122 to +152
// TODO:
//
// This extrinsic requires `remote-asset-manager`
//
// ----
//
// remove_asset {
// let asset_id = T::try_convert(2u8).unwrap();
// let units = 100_u32.into();
// let amount = 1_000u32.into();
// let origin = T::AdminOrigin::successful_origin();
// let origin_account_id = T::AdminOrigin::ensure_origin(origin.clone()).unwrap();
// let receiver = whitelisted_account::<T>("receiver", 0);
//
// // create liquid assets
// assert_ok!(<AssetIndex<T>>::add_asset(
// origin.clone(),
// asset_id,
// units,
// MultiLocation::Null,
// amount
// ));
//
// // create price feed
// T::PriceFeedBenchmarks::create_feed(origin_account_id.clone(), asset_id).unwrap();
//
// // construct call
// let call = Call::<T>::remove_asset(asset_id, units, Some(receiver));
// }: { call.dispatch_bypass_filter(origin.clone())? } verify {
// assert_eq!(T::IndexToken::total_balance(&origin_account_id), 0u32.into());
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

is this blocked by something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This requires calls in remote-asset-manager, got InvalidDest error of xcm

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yes, you're right, the XCM dependent calls can't be benchmarked like that. voting to even remove them.

@clearloop clearloop requested a review from mattsse September 3, 2021 08:30
@clearloop clearloop merged commit 2154e1b into main Sep 3, 2021
@clearloop clearloop deleted the cl/fix-saft-registry branch September 3, 2021 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review PR needs reviewing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix the benchmarks of pallet saft-registry Fix benchmarks for pallet asset-index and pallet saft-registry

2 participants