pezkuwichain/pezkuwi-subxt
1
0
Fork 0
mirror of https://github.com/pezkuwichain/pezkuwi-subxt.git synced 2026-05-31 07:31:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

[pallet-xcm] Adjust benchmarks (teleport_assets/reserve_transfer_assets) not relying on ED (#3464)

Browse Source 
## Problem
During the bumping of the `polkadot-fellows` repository to
`polkadot-sdk@1.6.0`, I encountered a situation where the benchmarks
`teleport_assets` and `reserve_transfer_assets` in AssetHubKusama
started to fail. This issue arose due to a decreased ED balance for
AssetHubs introduced
[here](https://github.com/polkadot-fellows/runtimes/pull/158/files#diff-80668ff8e793b64f36a9a3ec512df5cbca4ad448c157a5d81abda1b15f35f1daR213),
and also because of a [missing CI
pipeline](https://github.com/polkadot-fellows/runtimes/issues/197) to
check the benchmarks, which went unnoticed.

These benchmarks expect the `caller` to have enough:
1. balance to transfer (BTT)
2. balance for paying delivery (BFPD).
 
So the initial balance was calculated as `ED * 100`, which seems
reasonable:
```
const ED_MULTIPLIER: u32 = 100;
let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());`
```
The problem arises when the price for delivery is 100 times higher than
the existential deposit. In other words, when `ED * 100` does not cover
`BTT` + `BFPD`.

I check AHR/AHW/AHK/AHP and this problem has only AssetHubKusama
```
ED: 3333333
calculated price to parent delivery:  1031666634  (from xcm logs from the benchmark)
---

3333333 * 100 - BTT(3333333) - BFPD(1031666634) = −701666667
```
which results in the error;
```
2024-02-23 09:19:42 Unable to charge fee with error Module(ModuleError { index: 31, error: [17, 0, 0, 0], message: Some("FeesNotMet") })
Error: Input("Benchmark pallet_xcm::reserve_transfer_assets failed: FeesNotMet")
     
```

## Solution

The benchmarks `teleport_assets` and `reserve_transfer_assets` were
fixed by removing `ED * 100` and replacing it with `DeliveryHelper`
logic, which calculates the (almost real) price for delivery and sets it
along with the existential deposit as the initial balance for the
account used in the benchmark.


## TODO

- [ ] patch for 1.6 -
https://github.com/paritytech/polkadot-sdk/pull/3466
- [ ] patch for 1.7 -
https://github.com/paritytech/polkadot-sdk/pull/3465
- [ ] patch for 1.8 - TODO: PR

---------

Co-authored-by: Francisco Aguirre <franciscoaguirreperez@gmail.com>
This commit is contained in:
Branislav Kontur
2024-02-26 09:12:28 +01:00
committed by GitHub
parent b9c792a4b8
commit 3d9439f646
28 changed files with 329 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files
polkadot/xcm/pallet-xcm/src/benchmarking.rs
+45 -28
View File
@@ -17,21 +17,26 @@
use super::*;
use bounded_collections::{ConstU32, WeakBoundedVec};
use frame_benchmarking::{benchmarks, whitelisted_caller, BenchmarkError, BenchmarkResult};
use frame_support::{traits::Currency, weights::Weight};
use frame_support::{
traits::fungible::{Inspect, Mutate},
weights::Weight,
};
use frame_system::RawOrigin;
use sp_std::prelude::*;
use xcm::{latest::prelude::*, v2};
use xcm_builder::EnsureDelivery;
use xcm_executor::traits::FeeReason;
type RuntimeOrigin<T> = <T as frame_system::Config>::RuntimeOrigin;
// existential deposit multiplier
const ED_MULTIPLIER: u32 = 100;
/// Pallet we're benchmarking here.
pub struct Pallet<T: Config>(crate::Pallet<T>);
/// Trait that must be implemented by runtime to be able to benchmark pallet properly.
pub trait Config: crate::Config {
/// Helper that ensures successful delivery for extrinsics/benchmarks which need `SendXcm`.
type DeliveryHelper: EnsureDelivery;
/// A `Location` that can be reached via `XcmRouter`. Used only in benchmarks.
///
/// If `None`, the benchmarks that depend on a reachable destination will be skipped.
@@ -107,23 +112,29 @@ benchmarks! {
}.into();
let assets: Assets = asset.into();
let existential_deposit = T::ExistentialDeposit::get();
let caller = whitelisted_caller();
// Give some multiple of the existential deposit
let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
assert!(balance >= transferred_amount);
let _ = <pallet_balances::Pallet<T> as Currency<_>>::make_free_balance_be(&caller, balance);
// verify initial balance
assert_eq!(pallet_balances::Pallet::<T>::free_balance(&caller), balance);
let caller: T::AccountId = whitelisted_caller();
let send_origin = RawOrigin::Signed(caller.clone());
let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into())
.map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?;
if !T::XcmTeleportFilter::contains(&(origin_location, assets.clone().into_inner())) {
if !T::XcmTeleportFilter::contains(&(origin_location.clone(), assets.clone().into_inner())) {
return Err(BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))
}
// Ensure that origin can send to destination (e.g. setup delivery fees, ensure router setup, ...)
let (_, _) = T::DeliveryHelper::ensure_successful_delivery(
&origin_location,
&destination,
FeeReason::ChargeFees,
);
// Actual balance (e.g. `ensure_successful_delivery` could drip delivery fees, ...)
let balance = <pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller);
// Add transferred_amount to origin
<pallet_balances::Pallet<T> as Mutate<_>>::mint_into(&caller, transferred_amount)?;
// verify initial balance
let balance = balance + transferred_amount;
assert_eq!(<pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller), balance);
let recipient = [0u8; 32];
let versioned_dest: VersionedLocation = destination.into();
let versioned_beneficiary: VersionedLocation =
@@ -132,7 +143,7 @@ benchmarks! {
}: _<RuntimeOrigin<T>>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0)
verify {
// verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees)
assert!(pallet_balances::Pallet::<T>::free_balance(&caller) <= balance - transferred_amount);
assert!(<pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller) <= balance - transferred_amount);
}
reserve_transfer_assets {
@@ -146,23 +157,29 @@ benchmarks! {
}.into();
let assets: Assets = asset.into();
let existential_deposit = T::ExistentialDeposit::get();
let caller = whitelisted_caller();
// Give some multiple of the existential deposit
let balance = existential_deposit.saturating_mul(ED_MULTIPLIER.into());
assert!(balance >= transferred_amount);
let _ = <pallet_balances::Pallet<T> as Currency<_>>::make_free_balance_be(&caller, balance);
// verify initial balance
assert_eq!(pallet_balances::Pallet::<T>::free_balance(&caller), balance);
let caller: T::AccountId = whitelisted_caller();
let send_origin = RawOrigin::Signed(caller.clone());
let origin_location = T::ExecuteXcmOrigin::try_origin(send_origin.clone().into())
.map_err(|_| BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))?;
if !T::XcmReserveTransferFilter::contains(&(origin_location, assets.clone().into_inner())) {
if !T::XcmReserveTransferFilter::contains(&(origin_location.clone(), assets.clone().into_inner())) {
return Err(BenchmarkError::Override(BenchmarkResult::from_weight(Weight::MAX)))
}
// Ensure that origin can send to destination (e.g. setup delivery fees, ensure router setup, ...)
let (_, _) = T::DeliveryHelper::ensure_successful_delivery(
&origin_location,
&destination,
FeeReason::ChargeFees,
);
// Actual balance (e.g. `ensure_successful_delivery` could drip delivery fees, ...)
let balance = <pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller);
// Add transferred_amount to origin
<pallet_balances::Pallet<T> as Mutate<_>>::mint_into(&caller, transferred_amount)?;
// verify initial balance
let balance = balance + transferred_amount;
assert_eq!(<pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller), balance);
let recipient = [0u8; 32];
let versioned_dest: VersionedLocation = destination.into();
let versioned_beneficiary: VersionedLocation =
@@ -171,7 +188,7 @@ benchmarks! {
}: _<RuntimeOrigin<T>>(send_origin.into(), Box::new(versioned_dest), Box::new(versioned_beneficiary), Box::new(versioned_assets), 0)
verify {
// verify balance after transfer, decreased by transferred amount (+ maybe XCM delivery fees)
assert!(pallet_balances::Pallet::<T>::free_balance(&caller) <= balance - transferred_amount);
assert!(<pallet_balances::Pallet<T> as Inspect<_>>::balance(&caller) <= balance - transferred_amount);
}
transfer_assets {
polkadot/xcm/pallet-xcm/src/mock.rs
+22
View File
@@ -563,8 +563,30 @@ impl pallet_test_notifier::Config for Test {
type RuntimeCall = RuntimeCall;
}
#[cfg(feature = "runtime-benchmarks")]
pub struct TestDeliveryHelper;
#[cfg(feature = "runtime-benchmarks")]
impl xcm_builder::EnsureDelivery for TestDeliveryHelper {
fn ensure_successful_delivery(
origin_ref: &Location,
_dest: &Location,
_fee_reason: xcm_executor::traits::FeeReason,
) -> (Option<xcm_executor::FeesMode>, Option<Assets>) {
use xcm_executor::traits::ConvertLocation;
let account = SovereignAccountOf::convert_location(origin_ref).expect("Valid location");
// Give the existential deposit at least
let balance = ExistentialDeposit::get();
let _ = <Balances as frame_support::traits::Currency<_>>::make_free_balance_be(
&account, balance,
);
(None, None)
}
}
#[cfg(feature = "runtime-benchmarks")]
impl super::benchmarking::Config for Test {
type DeliveryHelper = TestDeliveryHelper;
fn reachable_dest() -> Option<Location> {
Some(Parachain(1000).into())
}