Skip to content

Fix MessageQueue pallet order in mocks #934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Agusrodri
Copy link
Contributor

@Agusrodri Agusrodri commented Jul 12, 2023

This change will prevent getting some encoding errors with XCM related tests in the future.

Edit: To give more context, the reason why this change is needed is because the new MessageQueue pallet should be placed at the same index where the old ParasUmp pallet was situated before it was removed in the upgrade to v0.9.43.

If we test the execution of an upward message with the current mock configuration, in most of the cases it will pass without any problem, but in some other ones it will fail while processing the XCM Transact instruction, more specifically while decoding the call that this instruction receives. This was the case in Moonbeam for instance, where we encountered that some XCM tests were failing due to this while testing the upgrade to v0.9.43. These issues got solved by placing the new MessageQueue pallet in the correct index.

@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #934 (8309e18) into master (7ecebea) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #934   +/-   ##
=======================================
  Coverage   78.44%   78.44%           
=======================================
  Files         105      105           
  Lines       10771    10771           
=======================================
  Hits         8449     8449           
  Misses       2322     2322           
Impacted Files Coverage Δ
asset-registry/src/mock/relay.rs 10.00% <ø> (ø)
xtokens/src/mock/relay.rs 61.53% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xlc
Copy link
Member

xlc commented Jul 12, 2023

Can you provide elaborate bit more why this is needed? If the orders are important, we need some code comments to explicitly document the requirement.

@Agusrodri
Copy link
Contributor Author

Can you provide elaborate bit more why this is needed?

Yes of course, I've just updated the PR's description.

@xlc xlc merged commit 669d188 into open-web3-stack:master Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants