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

rphmeier · 2023-07-28T04:02:24Z

BradleyOlson64 · 2023-07-28T22:12:31Z

pallets/parachain-system/src/lib.rs

+
+			let upward_capacity = &mut messaging_state.relay_dispatch_queue_remaining_capacity;
+			upward_capacity.remaining_count =
+				upward_capacity.remaining_count.saturating_sub(used_bandwidth.ump_msg_count);


Sanity checking, do we want to be subtracting the entire used_bandwidth of the segment here each time?

Yeah. What we read from the relay-chain state is the total capacity as-of the most recently included block. We then subtract the used bandwidth in the unincluded segment from it to get the actual remaining channel capacity.

slumber

Looks correct, although I'd say tests are quite important to have.

pallets/parachain-system/src/lib.rs

rphmeier · 2023-07-31T19:34:31Z

pallets/parachain-system/src/tests.rs

 		let mut result = Vec::new();
 		SENT_MESSAGES.with(|ms| {
 			ms.borrow_mut().retain(|m| {
 				let status = <Pallet<Test> as GetChannelInfo>::get_channel_status(m.0);
-				let ready = matches!(status, ChannelStatus::Ready(..));
-				if ready && !ids.contains(&m.0) && taken < maximum_channels {
+				let ChannelStatus::Ready(max_size_now, max_size_ever) = status else { return false };


This change is wrong. As this is within a retain, returning false (e.g. when the channel is full) would prune the message forever.

yes, my bad, pushed a fix

I pushed a more comprehensive version + added a bunch more to the test you wrote, just while I was playing around with it.

…rh-2471

rphmeier · 2023-07-31T21:02:20Z

bot merge

paritytech-processbot · 2023-07-31T21:02:27Z

Waiting for commit status.

paritytech-processbot · 2023-07-31T21:26:34Z

Merge cancelled due to error. Error: Statuses failed for 2286288

paritytech-cicd-pr · 2023-07-31T21:26:51Z

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/3303140

slumber · 2023-08-02T11:17:16Z

bot merge

* 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>

rphmeier changed the title ~~mostly address 2471 with a bug introduced~~ parachain-system: update RelevantMessagingState according to the unincluded segmetn Jul 28, 2023

rphmeier changed the title ~~parachain-system: update RelevantMessagingState according to the unincluded segmetn~~ parachain-system: update RelevantMessagingState according to the unincluded segment Jul 28, 2023

rphmeier requested a review from slumber July 28, 2023 19:34

rphmeier marked this pull request as ready for review July 28, 2023 19:45

BradleyOlson64 reviewed Jul 28, 2023

View reviewed changes

BradleyOlson64 approved these changes Jul 29, 2023

View reviewed changes

slumber approved these changes Jul 29, 2023

View reviewed changes

pallets/parachain-system/src/lib.rs Show resolved Hide resolved

rphmeier and others added 6 commits July 31, 2023 11:04

mostly address 2471 with a bug introduced

22e92ae

adjust relevant messaging state after computing total

b4754d4

fmt

ad8c0e0

max -> min

b313339

fix test implementation of xcmp source

34499d5

add test

24082a5

rphmeier force-pushed the rh-2471 branch from 341d799 to 24082a5 Compare July 31, 2023 16:04

rphmeier commented Jul 31, 2023

View reviewed changes

rphmeier and others added 5 commits July 31, 2023 14:40

fix test message sending logic

96179b2

fix + test

38de64c

add more to unincluded segment test

be3069c

Merge branch 'rh-2471' of https://github.com/paritytech/cumulus into …

fcb9283

…rh-2471

fmt

2286288

paritytech-processbot bot merged commit 1937edb into slumber-async-backing-feature Aug 2, 2023
38 checks passed

paritytech-processbot bot deleted the rh-2471 branch August 2, 2023 11:17

This was referenced Aug 2, 2023

parachain-system: add tests for unincluded segment message handling #2950

Closed

Ensure outgoing XCMP respects unincluded segment #2471

Closed

rphmeier mentioned this pull request Aug 16, 2023

advance parachain properly in XCM-emulator for tests #3023

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

rphmeier commented Jul 28, 2023 •

edited

BradleyOlson64 Jul 28, 2023

rphmeier Jul 28, 2023 •

edited

slumber left a comment

rphmeier Jul 31, 2023

slumber Jul 31, 2023

rphmeier Jul 31, 2023

rphmeier commented Jul 31, 2023

paritytech-processbot bot commented Jul 31, 2023

paritytech-processbot bot commented Jul 31, 2023

paritytech-cicd-pr commented Jul 31, 2023

slumber commented Aug 2, 2023

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

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

Conversation

rphmeier commented Jul 28, 2023 • edited

BradleyOlson64 Jul 28, 2023

Choose a reason for hiding this comment

rphmeier Jul 28, 2023 • edited

Choose a reason for hiding this comment

slumber left a comment

Choose a reason for hiding this comment

rphmeier Jul 31, 2023

Choose a reason for hiding this comment

slumber Jul 31, 2023

Choose a reason for hiding this comment

rphmeier Jul 31, 2023

Choose a reason for hiding this comment

rphmeier commented Jul 31, 2023

paritytech-processbot bot commented Jul 31, 2023

paritytech-processbot bot commented Jul 31, 2023

paritytech-cicd-pr commented Jul 31, 2023

slumber commented Aug 2, 2023

rphmeier commented Jul 28, 2023 •

edited

rphmeier Jul 28, 2023 •

edited