Skip to content

Snowbridge - Test pallet order#3381

Merged
bkontur merged 25 commits intoparitytech:masterfrom
Snowfork:snowbridge-test-pallet-order
Feb 21, 2024
Merged

Snowbridge - Test pallet order#3381
bkontur merged 25 commits intoparitytech:masterfrom
Snowfork:snowbridge-test-pallet-order

Conversation

@claravanstaden
Copy link
Copy Markdown
Contributor

@claravanstaden claravanstaden commented Feb 19, 2024

  • Adds a test to check the correct digest for Snowbridge outbound messages. For the correct digest to be in the block, the the MessageQueue pallet should be configured after the EthereumOutbound queue pallet. The added test fails if the EthereumOutbound is configured after the MessageQueue pallet.
  • Adds a helper method run_to_block_with_finalize to simulate the block finalizing. The existing run_to_block method does not finalize and so it cannot successfully test this condition.

Closes: #3208

claravanstaden and others added 11 commits February 8, 2024 13:37
* pallet order

* fmt

* adds test for bridge order

* revert unintended change

---------

Co-authored-by: claravanstaden <Cats 4 life!>
# Conflicts:
#	polkadot/node/subsystem-bench/src/approval/mod.rs
#	polkadot/node/subsystem-bench/src/availability/mod.rs
#	polkadot/node/subsystem-bench/src/cli.rs
#	polkadot/node/subsystem-bench/src/core/environment.rs
#	polkadot/node/subsystem-bench/src/subsystem-bench.rs
@claravanstaden claravanstaden marked this pull request as ready for review February 19, 2024 13:14
@paritytech-review-bot paritytech-review-bot bot requested a review from a team February 19, 2024 13:14
@claravanstaden
Copy link
Copy Markdown
Contributor Author

@bkontur this is the test you requested for #3208.

@bkontur bkontur added the R0-no-crate-publish-required The change does not require any crates to be re-published. label Feb 20, 2024
@paritytech-review-bot paritytech-review-bot bot requested a review from a team February 20, 2024 17:45
#[cfg(feature = "runtime-benchmarks")]
use benchmark_helpers::DoNothingRouter;
#[cfg(not(feature = "runtime-benchmarks"))]
//#[cfg(not(feature = "runtime-benchmarks"))]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove this

Comment on lines -370 to -373
#[cfg(feature = "runtime-benchmarks")]
type MessageProcessor =
pallet_message_queue::mock_helpers::NoopMessageProcessor<AggregateMessageOrigin>;
#[cfg(not(feature = "runtime-benchmarks"))]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bkontur the problem with the benchmark tests lies here. If the NoopMessageProcessor is used, the messages aren't actually processed and so the nonces doesn't increment and all the post-message sending checks fail. It looks like the tests pass with the without the special NoopMessageProcessor, will double check CI.

Copy link
Copy Markdown
Contributor

@bkontur bkontur left a comment

Choose a reason for hiding this comment

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

you can revert that benchmark stuff: #3381 (comment)
and fix it as a separate

@bkontur bkontur enabled auto-merge February 21, 2024 11:38
auto-merge was automatically disabled February 21, 2024 11:44

Head branch was pushed to by a user without write access

@claravanstaden
Copy link
Copy Markdown
Contributor Author

claravanstaden commented Feb 21, 2024

Thanks @bkontur, I'll make a separate PR. Reverted in c950af0

Separate PR here: #3424

@bkontur bkontur enabled auto-merge February 21, 2024 12:19
@bkontur bkontur added this pull request to the merge queue Feb 21, 2024
Merged via the queue into paritytech:master with commit 5a06771 Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

R0-no-crate-publish-required The change does not require any crates to be re-published.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create test case to confirm pallet indexes are correct

5 participants