pvf-precheck: PVF pre-checker subsystem #4643

pepyakin · 2021-12-30T17:54:29Z

This commit implements the last major piece of #3211: the subsystem that
tracks PVFs that require voting, issues pre-check requests to
candidate-validation and makes sure that the votes are submitted to the
chain.

TODO:

Run once more with polkadot-launch

I am skipping the cumulus check since this is not related.

skip check-dependent-cumulus

pepyakin · 2021-12-30T17:54:34Z

Current dependencies on/for this PR:

master
- PR pvf-precheck: PVF pre-checker subsystem #4643 (this PR)

This comment was auto-generated by Graphite.

node/core/pvf-checker/src/interest_view.rs

drahnr

Overseer integration looks good, I'll take another pass on the actual impl.

node/core/pvf-checker/src/interest_view.rs

node/core/pvf-checker/src/lib.rs

node/overseer/src/lib.rs

node/overseer/src/dummy.rs

drahnr · 2022-01-03T17:21:29Z

node/overseer/src/tests.rs

-	const NUM_SUBSYSTEMS_MESSAGED: usize = NUM_SUBSYSTEMS - 3;
+	const NUM_SUBSYSTEMS: usize = 21;
+	// -4 for BitfieldSigning, GossipSupport, AvailabilityDistribution and PvfCheckerSubsystem.
+	const NUM_SUBSYSTEMS_MESSAGED: usize = NUM_SUBSYSTEMS - 4;


scripts/gitlab/lingua.dic

node/subsystem-types/src/messages.rs

pepyakin · 2022-01-05T17:03:15Z

I've managed to verify that the thing works as a whole, but only if we change the weight for

polkadot/runtime/parachains/src/paras/mod.rs

Lines 852 to 857 in c8abae0

    
           #[pallet::weight(Weight::MAX)] 
        
           pub fn include_pvf_check_statement( 
        
           	origin: OriginFor<T>, 
        
           	stmt: PvfCheckStatement, 
        
           	signature: ValidatorSignature, 
        
           ) -> DispatchResult {

to something manageable.

eskimor · 2022-01-07T12:52:20Z

node/core/pvf-checker/src/interest_view.rs

+impl PvfData {
+	/// Initialize a new `PvfData` which is awaiting for the initial judgement.
+	fn pending(origin: Hash) -> Self {
+		// Preallocate the hashset with 5 items. This is the anticipated maximum leafs we can


huh i wonder why spell check did not go off )

It's a dev comment and they are currently not checked by CI

node/core/pvf-checker/src/interest_view.rs

eskimor · 2022-01-07T15:17:37Z

node/core/pvf-checker/src/lib.rs

+				} else {
+					// Pre-checking request dropped before replying. That can happen in case the
+					// overseer is shutting down. Our part of shutdown will be handled by the
+					// overseer conclude signal.


Maybe add some debug log, so if this happens for some other reason it will be easier to see what's going on.

eskimor · 2022-01-07T15:20:40Z

node/core/pvf-checker/src/lib.rs

+			// Abstain.
+			//
+			// Returning here will leave the PVF in the view dangling. Since it is there, no new
+			// pre-checking request will be sent.


Some log message?

eskimor · 2022-01-07T15:56:47Z

node/core/pvf-checker/src/tests.rs

+}
+
+#[test]
+fn activation_of_descedant_leaves_pvfs_in_view() {


eskimor · 2022-01-07T16:04:49Z

Really nice! Especially the test suite - reads like a book!

This commit implements the last major piece of #3211: the subsystem that tracks PVFs that require voting, issues pre-check requests to candidate-validation and makes sure that the votes are submitted to the chain.

eskimor · 2022-01-07T17:47:08Z

bot merge

paritytech-processbot · 2022-01-07T17:47:16Z

Waiting for commit status.

This commit implements the last major piece of paritytech#3211: the subsystem that tracks PVFs that require voting, issues pre-check requests to candidate-validation and makes sure that the votes are submitted to the chain.

github-actions bot added the A0-please_review Pull request needs code review. label Dec 30, 2021

pepyakin added 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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Dec 30, 2021

pepyakin force-pushed the pep-pvf-subsystem branch from 86834f5 to 649f134 Compare December 30, 2021 18:17

pepyakin requested review from chevdor and a team as code owners December 30, 2021 18:17

pepyakin force-pushed the pep-pvf-subsystem branch from 649f134 to ef957aa Compare December 30, 2021 18:29

pepyakin requested a review from drahnr January 3, 2022 15:22

drahnr reviewed Jan 3, 2022

View reviewed changes

node/core/pvf-checker/src/interest_view.rs Outdated Show resolved Hide resolved

drahnr reviewed Jan 3, 2022

View reviewed changes

pepyakin force-pushed the pep-pvf-subsystem branch 3 times, most recently from fc7e458 to 645c825 Compare January 4, 2022 14:54

pepyakin requested review from drahnr and eskimor January 4, 2022 16:36

pepyakin force-pushed the pep-pvf-subsystem branch from 645c825 to b3ef387 Compare January 5, 2022 17:14

pepyakin added B1-releasenotes and removed B0-silent Changes should not be mentioned in any release notes labels Jan 5, 2022

pepyakin force-pushed the pep-pvf-subsystem branch from b3ef387 to 7dbc79d Compare January 7, 2022 14:19

eskimor approved these changes Jan 7, 2022

View reviewed changes

pvf-precheck: PVF pre-checker subsystem

a9d2340

This commit implements the last major piece of #3211: the subsystem that tracks PVFs that require voting, issues pre-check requests to candidate-validation and makes sure that the votes are submitted to the chain.

pepyakin force-pushed the pep-pvf-subsystem branch from 7dbc79d to a9d2340 Compare January 7, 2022 17:29

paritytech-processbot bot merged commit 22e5c04 into master Jan 7, 2022

paritytech-processbot bot deleted the pep-pvf-subsystem branch January 7, 2022 18:10

eskimor added D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. and removed D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Jan 11, 2022

jakoblell added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Jan 18, 2022

librelois mentioned this pull request Feb 7, 2022

Update substrate/polkadot/cumulus from v0.9.15 to v0.9.16 moonbeam-foundation/moonbeam#1259

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pvf-precheck: PVF pre-checker subsystem #4643

pvf-precheck: PVF pre-checker subsystem #4643

pepyakin commented Dec 30, 2021 •

edited

Loading

pepyakin commented Dec 30, 2021

drahnr left a comment

drahnr Jan 3, 2022

pepyakin commented Jan 5, 2022

eskimor Jan 7, 2022

pepyakin Jan 7, 2022

drahnr Jan 7, 2022

pepyakin Jan 8, 2022

eskimor Jan 7, 2022

eskimor Jan 7, 2022

eskimor Jan 7, 2022

eskimor commented Jan 7, 2022

eskimor commented Jan 7, 2022

paritytech-processbot bot commented Jan 7, 2022

pvf-precheck: PVF pre-checker subsystem #4643

pvf-precheck: PVF pre-checker subsystem #4643

Conversation

pepyakin commented Dec 30, 2021 • edited Loading

pepyakin commented Dec 30, 2021

drahnr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pepyakin commented Jan 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eskimor commented Jan 7, 2022

eskimor commented Jan 7, 2022

paritytech-processbot bot commented Jan 7, 2022

pepyakin commented Dec 30, 2021 •

edited

Loading