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

Allow verification of sent XCMs in benchmarks #4605

Closed
wants to merge 7 commits into from

Conversation

KiChjang
Copy link
Contributor

Partial fix for paritytech/polkadot-sdk#900.

Creates an additional TestSendXcm struct in mocks, which is essentially a global queue of XCMs that are meant to be sent by the XcmSender as configured in the XCM executor. This will allow us to inspect the outgoing messages by reading off of the queue, and thereby verifying the integrity of our XCM benchmarks.

@KiChjang KiChjang added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Dec 27, 2021
@@ -64,7 +62,7 @@ pub fn mock_worst_case_holding() -> MultiAssets {
const HOLDING_FUNGIBLES: u32 = 99;
const HOLDING_NON_FUNGIBLES: u32 = 99;
let fungibles_amount: u128 = 100;
(0..HOLDING_FUNGIBLES)
(1..=HOLDING_FUNGIBLES)
Copy link
Contributor Author

@KiChjang KiChjang Dec 28, 2021

Choose a reason for hiding this comment

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

This is done here because the XCM executor's holding register automatically removes any fungible assets that have an amount of 0, which would cause some tests to fail as the worst case holding expects a non-existent GeneralIndex(0) asset to be there.

@KiChjang KiChjang requested a review from kianenigma December 28, 2021 09:56
@shawntabrizi
Copy link
Member

This unfortunately will not work right?

Because when running verification on an actual production chain, this mock thing wont exist. So this only works for tests, but verification in benchmarking is quite a bit more than that.

We need some general trait / implementation for both tests and the real queue system.

@KiChjang
Copy link
Contributor Author

Yeah, this is definitely not going to work, closing for now.

@KiChjang KiChjang closed this Dec 30, 2021
@shawntabrizi shawntabrizi deleted the kckyeung/xcm-benchmark-verify-sent branch January 1, 2022 13:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants