Update Runtime Weights

[liamaharon]

For #68 (comment).

Uses instructions introduced to the readme in #127 to re-bench all runtimes.

TODO

• Is this much change % expected? Is benchmarking on vCPUs flawed because they are arbitrarily shared with other cloud customers?
  • It seems to be fine, the changes align with actual changes in configuration of the runtimes (thanks @ggwpez!).
• Polkadot
• Kusama
• Asset Hub Polkadot
• Asset Hub Kusama
• Bridge Hub Polkadot
• Bridge Hub Kusama
• Collectives Polkadot
• Look into why subweight shows some errors

Polkadot Changes

Kusama Changes

Asset Hub Polkadot Changes

Asset Hub Kusama Changes

Bridge Hub Polkadot

Bridge Hub Kusama

Collectives Polkadot

[bkontur]

it would be cool to include also this cleaning PR: #57

[liamaharon]

Weights are all updated now. One small thing I'm not sure of is why subweight sometimes fails for extrinsics which seem to have been updated without issue, e.g.

@ggwpez any ideas?

[ggwpez]

/merge

[fellowship-merge-bot]

Enabled auto-merge in Pull Request

Available commands

• /merge: Enables auto-merge for Pull Request
• /merge cancel: Cancels auto-merge for Pull Request
• /merge help: Shows this menu

For more information see the documentation

[ggwpez]

The two remaining failing tests are from #114 and could be commented for now.

[acatangiu]

The two remaining failing tests are from #114 and could be commented for now.

I'm looking into this now, so far looks like actual weights issue with Error(WeightNotComputable):

AssetHubPolkadot TRACE xcm::weight: WeightInfoBounds message: Xcm([ReserveAssetDeposited(MultiAssets([MultiAsset { id: Concrete(MultiLocation { parents: 1, interior: Here }), fun: Fungible(10000000000000) }])), ClearOrigin, BuyExecution { fees: MultiAsset { id: Concrete(MultiLocation { parents: 1, interior: Here }), fun: Fungible(10000000000000) }, weight_limit: Unlimited }, DepositAsset { assets: Wild(AllCounted(1)), beneficiary: MultiLocation { parents: 0, interior: X1(AccountId32 { network: None, id: [142, 175, 4, 21, 22, 135, 115, 99, 38, 201, 254, 161, 126, 37, 252, 82, 135, 97, 54, 147, 201, 18, 144, 156, 178, 38, 170, 71, 148, 242, 106, 72] }) } }, SetTopic([201, 32, 171, 168, 92, 206, 203, 109, 135, 135, 85, 253, 130, 152, 157, 65, 206, 56, 147, 183, 121, 12, 173, 10, 97, 204, 25, 152, 187, 87, 194, 148])])

AssetHubPolkadot events: [RuntimeEvent::DmpQueue(Event::ExecutedDownward { message_hash: [39, 4, 119, 101, 74, 2, 62, 188, 202, 127, 77, 91, 0, 138, 171, 118, 232, 89, 9, 132, 77, 17, 6, 77, 108, 102, 102, 30, 56, 102, 16, 78], message_id: [39, 4, 119, 101, 74, 2, 62, 188, 202, 127, 77, 91, 0, 138, 171, 118, 232, 89, 9, 132, 77, 17, 6, 77, 108, 102, 102, 30, 56, 102, 16, 78], outcome: Error(WeightNotComputable) })]

[bkontur]

ReserveAssetDeposited

AssetHubPolkadot/Kusama does not recognize any reserve locations IsReserve = () here,
tests::reserve_transfer::limited_reserve_transfer_native_asset_from_relay_to_system_para_fails fails which is ok, but we need to just adjust assert here

[acatangiu]

This is now incorrectly set to Weight::MAX - looks like the issue is with benchmarks - I can look into it later today unless somebody knows off the top of their heads what the issue is

[acatangiu]

ReserveAssetDeposited

AssetHubPolkadot/Kusama does not recognize any reserve locations IsReserve = () here, tests::reserve_transfer::limited_reserve_transfer_native_asset_from_relay_to_system_para_fails fails which is ok, but we need to just adjust assert here

no, the assert is correct - it works fine without the bad weights - the problem is certain XCMs are unweighable with this PR because multiple XCM instructions have been benchmarked to Weight::MAX

this is a valid weights issue, not a test issue

[bkontur]

ReserveAssetDeposited

AssetHubPolkadot/Kusama does not recognize any reserve locations IsReserve = () here, tests::reserve_transfer::limited_reserve_transfer_native_asset_from_relay_to_system_para_fails fails which is ok, but we need to just adjust assert here

no, the assert is correct - it works fine without the bad weights - the problem is certain XCMs are unweighable with this PR because multiple XCM instructions have been benchmarked to Weight::MAX

this is a valid weights issue, not a test issue

That previous value 500_000_000_000 (T::BlockWeights::get().max_block) was kind of hack as a part of remote weight estimation which was removed.

This Weight::MAX is correct expected value for ReserveAssetDeposited instruction which is not supported on AssetHub because of IsReserve = (), so benchmark and weight is ok. And XcmExecutor really reports WeightNotComputable for Weight::MAX.

paritytech/polkadot-sdk#1726

[bkontur]

ReserveAssetDeposited

AssetHubPolkadot/Kusama does not recognize any reserve locations IsReserve = () here, tests::reserve_transfer::limited_reserve_transfer_native_asset_from_relay_to_system_para_fails fails which is ok, but we need to just adjust assert here

no, the assert is correct - it works fine without the bad weights - the problem is certain XCMs are unweighable with this PR because multiple XCM instructions have been benchmarked to Weight::MAX
this is a valid weights issue, not a test issue

That previous value 500_000_000_000 (T::BlockWeights::get().max_block) was kind of hack as a part of remote weight estimation which was removed.

This Weight::MAX is correct expected value for ReserveAssetDeposited instruction which is not supported on AssetHub because of IsReserve = (), so benchmark and weight is ok. And XcmExecutor really reports WeightNotComputable for Weight::MAX.

paritytech/polkadot-sdk#1726

My point is thatReserveAssetDeposited is not supported on AssetHub (IsReserve = ()), so the benchmark weight is unknown aka Weight::MAX (because we don't know real implementation - how many reads...), so the XcmExecutor does not even try to execute it.
Previously with T::BlockWeights::get().max_block = 500_000_000_000, it tried to execute it (is it ok?) but with UntrustedReserveLocation failure from processing of ReserveAssetDeposited.

I changed that T::BlockWeights::get().max_block to Weight::MAX according to the discussion here: paritytech/cumulus#2934 (comment)

Maybe I am wrong, but I think it is safer not to execute at all, if we don't know the real benchmark weight beforehand.

[acatangiu]

		let (trusted_reserve, transferable_reserve_asset) = T::TrustedReserve::get()
			.ok_or(BenchmarkError::Override(
				BenchmarkResult::from_weight(T::BlockWeights::get().max_block)
				BenchmarkResult::from_weight(Weight::MAX)
			))?;

Ok, so runtimes which don't recognize any reserve locations, will fail on initial XCM weighing instead of failing on barrier a bit later. That's fine, and I agree it's better to use Weight::MAX instead of some dummy value.
Expectation in this case is that if we ever add trusted locations we also need to change benchmarking code - which again is fine and this test will actually force us to. 👍

Changing test to expect weight failure instead of barrier failure.

[acatangiu]

@liamaharon I'm not allowed to push into your fork:

kata@MacBook-Pro runtimes % git push
ERROR: Permission to liamaharon/runtimes.git denied to acatangiu.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Please add the fix yourself, you can find it here: acatangiu@48787a1

[ggwpez]

@liamaharon is OOO but AFAIK i can push 🤷
Will run the test locally and then push.