Skip to content
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

Improve fuzzer for XCMv4 #2424

Closed
wants to merge 10 commits into from

Conversation

louismerlin
Copy link
Contributor

@louismerlin louismerlin commented Nov 21, 2023

Description

This PR includes improvements to the xcm-simulator-fuzzer to make it find more coverage in XCMv4.

This includes switching the fuzzing engine from honggfuzz-rs to both honggfuzz-rs and afl.rs through ziggy, a fuzzer manager.

@louismerlin louismerlin added the T6-XCM This PR/Issue is related to XCM. label Nov 21, 2023
@louismerlin louismerlin requested a review from a team as a code owner November 21, 2023 13:41
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/4419542

@@ -155,6 +155,35 @@ fn run_input(xcm_messages: [XcmMessage; 5]) {
println!();

for xcm_message in xcm_messages {
// We block Transact messages because they can cause panics that are due to the test
// environment, and not XCM
Copy link
Member

Choose a reason for hiding this comment

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

For example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just checked, and got the following:

Unexpected event: RuntimeEvent::MessageQueue(
    Event::ProcessingFailed {
        ...
        error: ProcessMessageError::Unsupported,
    },
)

in this line of the event processing code when processing an invalid Transact call.
@ggwpez @franciscoaguirre do you think this should be handled differently, e.g. by allowing Unsupported messages instead of panicking, and then removing the blocklist?

Copy link
Contributor

Choose a reason for hiding this comment

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

We could allow that. The thing is arbitrary Transacts will call any type of extrinsic, with wrong attributes or anything of the sort. We could record which Transacts give an error and which ones don't and why. In that way we would know much more than just by blocklisting them

@franciscoaguirre franciscoaguirre deleted the branch paritytech:cis-xcm-v4 January 16, 2024 19:31
@ggwpez
Copy link
Member

ggwpez commented Jan 18, 2024

Why was this closed?

github-merge-queue bot pushed a commit that referenced this pull request Feb 8, 2024
This adds `try_state()` and `integrity_test()` to the four runtimes of
the XCM-simulator fuzzer.

With this, we are able to stress-test [message-queue's
try_state](https://github.com/paritytech/polkadot-sdk/blob/7df1ae3b8111d534cce108b2b405b6a33fcdedc3/substrate/frame/message-queue/src/lib.rs#L1245-L1347).

This also adds the `Transact` block-listing from #2424 to avoid
false-positives.

Thank you @ggwpez for the help with the runtime configurations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T6-XCM This PR/Issue is related to XCM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants