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

Make xcm-emulator test pass for new consensus hook #2722

Conversation

NachoPal
Copy link
Contributor

@NachoPal NachoPal commented Jun 9, 2023

This PR tries to solve: #2703

Tests are not panicking anymore with slot info is inserted on block initialization, but now they are running into another issue. Tests are failing with included head not present in relay storage proof.

Should I add the rest of the hooks (on_finalize, etc)?

@NachoPal NachoPal requested review from slumber and rphmeier June 9, 2023 15:25
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: test-linux-stable
Logs: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/2964712

@slumber
Copy link
Contributor

slumber commented Jun 9, 2023

We need a full parent header to be included into the storage proof. For example, this is how it's done for parachain system tests

let header = System::finalize();
let head_data = relay_chain::HeadData(header.encode());

sproof_builder.included_para_head = self
.included_para_head
.clone()
.unwrap_or_else(|| parent_head_data.clone())
.into();

To my understanding, headers are not accessible from the runtime, so we need system pallet to be finalized.

@muharem
Copy link
Contributor

muharem commented Jun 11, 2023

I would say that the emulator should behave as close as possible to the real environment, while only mocking the network layer. on_finalize should also run for all pallets, in a right order.

parachains/integration-tests/emulated/common/src/lib.rs Outdated Show resolved Hide resolved
xcm/xcm-emulator/src/lib.rs Outdated Show resolved Hide resolved
@NachoPal NachoPal added B0-silent Changes should not be mentioned in any release notes A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. labels Jun 14, 2023
@NachoPal
Copy link
Contributor Author

bot fmt

@command-bot
Copy link

command-bot bot commented Jun 14, 2023

@NachoPal https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/2989870 was started for your command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh". Check out https://gitlab.parity.io/parity/mirrors/cumulus/-/pipelines?page=1&scope=all&username=group_605_bot to know what else is being executed currently.

Comment bot cancel 9-dd2ced3f-1709-486b-aa37-89165fcaa547 to cancel this command or bot cancel to cancel all commands in this pull request.

@command-bot
Copy link

command-bot bot commented Jun 14, 2023

@NachoPal Command "$PIPELINE_SCRIPTS_DIR/commands/fmt/fmt.sh" has finished. Result: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/2989870 has finished. If any artifacts were generated, you can download them from https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/2989870/artifacts/download.

@NachoPal
Copy link
Contributor Author

NachoPal commented Jun 14, 2023

I dropped the idea of running all pallets on_initialized and on_finalized with AllPalletsWithSystem. The reason is I was running into more errors because of multiple reasons. One of them was the slot verification when using defaults pre-runtime digest. It forced me to manually build them + having to set inherent for timestamp etc...

I realised I was building something very specific for our runtimes using Aura and AuraExt. One of the goals of xcm-emulator is to be as generic as possible, so other parachains (that are maybe using another consensus system) can use it too.

I created this new issue #2734 to add on_initialize and on_finalize optional hooks to xcm-emulator.

The changes in this PR should be good enough to make the test pass, and therefore not being a blocker anymore.

Please merge it if you are ok with it.

@NachoPal NachoPal changed the title Add pallets hooks to xcm-emulator Make xcm-emulator test pass for new consensus hook Jun 14, 2023
Copy link
Contributor

@gilescope gilescope left a comment

Choose a reason for hiding this comment

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

It's a closer emulation than what we've currently got so it gets my vote.

@slumber
Copy link
Contributor

slumber commented Jun 15, 2023

Thanks @NachoPal!

@slumber slumber merged commit f1cbacd into slumber-async-backing-check-aura-slot Jun 15, 2023
@slumber slumber deleted the nacho/xcm-emulator-add-pallets-hooks branch June 15, 2023 13:11
rphmeier pushed a commit that referenced this pull request Jul 3, 2023
…` logic (#2658)

* aura-ext: check slot in consensus hook

* convert relay chain slot

* Make relay chain slot duration generic

* use fixed velocity hook for pallets with aura

* purge timestamp inherent

* fix warning

* adjust runtime tests

* fix slots in tests

* Make `xcm-emulator` test pass for new consensus hook (#2722)

* add pallets on_initialize

* tests pass

* add AuraExt on_init

* ".git/.scripts/commands/fmt/fmt.sh"

---------

Co-authored-by: command-bot <>

---------

Co-authored-by: Ignacio Palacios <ignacio.palacios.santos@gmail.com>
paritytech-processbot bot pushed a commit that referenced this pull request Aug 18, 2023
* Update substrate & polkadot

* min changes to make async backing compile

* (async backing) parachain-system: track limitations for unincluded blocks (#2438)

* unincluded segment draft

* read para head from storage proof

* read_para_head -> read_included_para_head

* Provide pub interface

* add errors

* fix unincluded segment update

* BlockTracker -> Ancestor

* add a dmp limit

* Read para head depending on the storage switch

* doc comments

* storage items docs

* add a sanity check on block initialize

* Check watermark

* append to the segment on block finalize

* Move segment update into set_validation_data

* Resolve para head todo

* option watermark

* fix comment

* Drop dmq check

* fix weight

* doc-comments on inherent invariant

* Remove TODO

* add todo

* primitives tests

* pallet tests

* doc comments

* refactor unincluded segment length into a ConsensusHook (#2501)

* refactor unincluded segment length into a ConsensusHook

* add docs

* refactor bandwidth_out calculation

Co-authored-by: Chris Sosnin <48099298+slumber@users.noreply.github.com>

* test for limits from impl

* fmt

* make tests compile

* update comment

* uncomment test

* fix collator test by adding parent to state proof

* patch HRMP watermark rules for unincluded segment

* get consensus-common tests to pass, using unincluded segment

* fix unincluded segment tests

* get all tests passing

* fmt

* rustdoc CI

* aura-ext: limit the number of authored blocks per slot (#2551)

* aura_ext consensus hook

* reverse dependency

* include weight into hook

* fix tests

* remove stray println

Co-authored-by: Chris Sosnin <48099298+slumber@users.noreply.github.com>

* fix test warning

* fix doc link

---------

Co-authored-by: Chris Sosnin <48099298+slumber@users.noreply.github.com>
Co-authored-by: Chris Sosnin <chris125_@live.com>

* parachain-system: ignore go ahead signal once upgrade is processed (#2594)

* handle goahead signal for unincluded segment

* doc comment

* add test

* parachain-system: drop processed messages from inherent data (#2590)

* implement `drop_processed_messages`

* drop messages based on relay parent number

* adjust tests

* drop changes to mqc

* fix comment

* drop test

* drop more dead code

* clippy

* aura-ext: check slot in consensus hook and remove all `CheckInherents` logic (#2658)

* aura-ext: check slot in consensus hook

* convert relay chain slot

* Make relay chain slot duration generic

* use fixed velocity hook for pallets with aura

* purge timestamp inherent

* fix warning

* adjust runtime tests

* fix slots in tests

* Make `xcm-emulator` test pass for new consensus hook (#2722)

* add pallets on_initialize

* tests pass

* add AuraExt on_init

* ".git/.scripts/commands/fmt/fmt.sh"

---------

Co-authored-by: command-bot <>

---------

Co-authored-by: Ignacio Palacios <ignacio.palacios.santos@gmail.com>

* update polkadot git refs

* CollationGenerationConfig closure is now optional (#2772)

* CollationGenerationConfig closure is now optional

* fix test

* propagate network-protocol-staging feature (#2899)

* Feature Flagging Consensus Hook Type Parameter (#2911)

* First pass

* fmt

* Added as default feature in tomls

* Changed to direct dependency feature

* Dealing with clippy error

* Update pallets/parachain-system/src/lib.rs

Co-authored-by: asynchronous rob <rphmeier@gmail.com>

---------

Co-authored-by: asynchronous rob <rphmeier@gmail.com>

* fmt

* bump deps and remove warning

* parachain-system: update RelevantMessagingState according to the unincluded segment (#2948)

* mostly address 2471 with a bug introduced

* adjust relevant messaging state after computing total

* fmt

* max -> min

* fix test implementation of xcmp source

* add test

* fix test message sending logic

* fix + test

* add more to unincluded segment test

* fmt

---------

Co-authored-by: Chris Sosnin <chris125_@live.com>

* Integrate new Aura / Parachain Consensus Logic in Parachain-Template / Polkadot-Parachain (#2864)

* add a comment

* refactor client/service utilities

* deprecate start_collator

* update parachain-template

* update test-service in the same way

* update polkadot-parachain crate

* fmt

* wire up new SubmitCollation message

* some runtime utilities for implementing unincluded segment runtime APIs

* allow parachains to configure their level of sybil-resistance when starting the network

* make aura-ext compile

* update to specify sybil resistance levels

* fmt

* specify relay chain slot duration in milliseconds

* update Aura to explicitly produce Send futures

also, make relay_chain_slot_duration a Duration

* add authoring duration to basic collator and document params

* integrate new basic collator into parachain-template

* remove assert_send used for testing

* basic-aura: only author when parent included

* update polkadot-parachain-bin

* fmt

* some fixes

* fixes

* add a RelayNumberMonotonicallyIncreases

* add a utility function for initializing subsystems

* some logging for timestamp adjustment

* fmt

* some fixes for lookahead collator

* add a log

* update `find_potential_parents` to account for sessions

* bound the loop

* restore & deprecate old start_collator and start_full_node functions.

* remove unnecessary await calls

* fix warning

* clippy

* more clippy

* remove unneeded logic

* ci

* update comment

Co-authored-by: Marcin S. <marcin@bytedude.com>

* (async backing) restore `CheckInherents` for backwards-compatibility (#2977)

* bring back timestamp

* Restore CheckInherents

* revert to empty CheckInherents

* make CheckInherents optional

* attempt

* properly end system blocks

* add some more comments

* ignore failing system parachain tests

* update refs after main feature branch merge

* comment out the offending tests because CI runs ignored tests

* fix warnings

* fmt

* revert to polkadot master

* cargo update -p polkadot-primitives -p sp-io

---------

Co-authored-by: asynchronous rob <rphmeier@gmail.com>
Co-authored-by: Ignacio Palacios <ignacio.palacios.santos@gmail.com>
Co-authored-by: Bradley Olson <34992650+BradleyOlson64@users.noreply.github.com>
Co-authored-by: Marcin S. <marcin@bytedude.com>
Co-authored-by: eskimor <eskimor@users.noreply.github.com>
Co-authored-by: Andronik <write@reusable.software>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants