Switch node side to v2 candidate receipts #5679

sandreim · 2024-09-11T14:38:25Z

on top of #5423

This PR implements the plumbing work required for #5047 . I also added additional helper methods gated by feature "test" in primitives.

TODO:

PRDoc

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

…ode_v2_descriptors

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

…aritytech/polkadot-sdk into sandreim/node_v2_descriptors

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

…aritytech/polkadot-sdk into sandreim/node_v2_descriptors Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

alindima

Man was this a tough one to review 😄

Thanks for pushing through the tedious work!

Why is this stacked on top of #5423?
From a quick look it could be merged independently

alindima · 2024-10-02T08:27:39Z

polkadot/node/network/collator-protocol/src/validator_side/collation.rs

-	pub collator_id: CollatorId,
+	/// This will be missing when we create the struct from
+	/// a v2 candidate receipt. instance
+	pub collator_id: Option<CollatorId>,


FetchedCollation is used as a key in a HashMap (fetched_candidates). I think we still need some collator identifier here to differentiate between two collators advertising the same collation.

Since this is a fetched collation, we should have access to the collator id which is declared in CollatorProtocolMessage::Declare

Took me a while to remember why I did this.

The gist of it is that for v2 descriptors, you cannot have the same candidate with different candidate hash, because there is no collator id and singature fields. This mapping is currently needed only for reputation changes and sending a Seconded message back to the collator that provided the collation. In slot based collator and even look ahead we don't make use of the seconded message AFAIK, and the reputation system just doesn't make much sense given that currently there is no way to really punish them.

@ordian @eskimor any opinions on this change ?

alindima · 2024-10-02T08:32:20Z

polkadot/primitives/src/vstaging/mod.rs

+	a
+}
+
+impl<H: Copy> From<CandidateDescriptor<H>> for CandidateDescriptorV2<H> {


what's the difference between this and the previous implementation?
is it just for performance?

Yes, initially for testing it was good with encode/decode. At this point I am not sure if it is faster, but in theory should be.

alindima · 2024-10-02T08:33:55Z

polkadot/primitives/src/vstaging/mod.rs

@@ -368,7 +471,7 @@ pub enum CandidateReceiptError {

 macro_rules! impl_getter {
 	($field:ident, $type:ident) => {
-		/// Returns the value of $field field.
+		/// Returns the value of `$field`` field.


Suggested change

/// Returns the value of `$field`` field.

/// Returns the value of `$field` field.

alindima · 2024-10-02T08:35:36Z

polkadot/primitives/src/vstaging/mod.rs

+	fn set_para_head(&mut self, para_head: Hash);
+}
+
+#[cfg(feature = "test")]


I'd rather directly add these methods onto the CandidateDescriptorV2 and guard each of them with #[cfg(test)], which would save us the import of MutateDescriptorV2. But fine either way really

alindima · 2024-10-02T08:38:57Z

polkadot/runtime/parachains/src/runtime_api_impl/v10.rs

@@ -451,7 +451,16 @@ pub fn backing_state<T: initializer::Config>(
 	//
 	// Thus, minimum relay parent is ensured to have asynchronous backing enabled.
 	let now = frame_system::Pallet::<T>::block_number();
-	let min_relay_parent_number = shared::AllowedRelayParents::<T>::get()
+
+	// Workaround for issue #64.


I think you meant to add this to #5423

Yeah, and this explains why I messed up in #5423 (comment)

alindima · 2024-10-02T08:39:23Z

prdoc/pr_5679.prdoc

+  - audience: Node Dev
+    description: |
+      This change is just plumbing work and updates all crate interfaces to use the new primitives.
+      Id doesn't alter any functionality and is required before implementing RFC103 on the 


Suggested change

Id doesn't alter any functionality and is required before implementing RFC103 on the

It doesn't alter any functionality and is required before implementing RFC103 on the

alindima · 2024-10-02T08:41:14Z

prdoc/pr_5679.prdoc

+  - name: cumulus-client-consensus-common
+    bump: major
+  - name: cumulus-client-pov-recovery
+    bump: major


why are these two major? I think they should be none

actually, why is everything a major bump? :D

I've switched the subsystem messages and cumulus traits to new primtives so this breaks things.

Also I'm missing many crates here and the check fails.

sandreim added 30 commits August 12, 2024 15:07

WIP primitives

793142e

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

WIP

3a29fdf

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Working version.

4a53577

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Better version

8285ae7

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Add missing primitives and fix things

2831c5e

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Implement v2 receipts in polkadot-runtime-parachains

c767d60

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

add missing stuff

96999e3

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Switch parachains runtime to use new primitives

c5f2dc3

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

use vstaging primitives

dbb0160

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

update rococo and westend

5efab68

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

client keeps using the old primitives

c2232e4

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

no unsafe pls

87b079f

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

move async backing primtiives to own file

00e8c13

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix

cd4d02f

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix test build

5509e33

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix test-runtime

f8b86d2

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

self review feedback

fe2fbfb

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

review feedback

975e13b

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

feedback

1c7ac55

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

feedback

653873b

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

clippy

dc98149

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

chores

0a6bce3

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Filter v2 candidate descriptors

5e4dac2

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix

f12ca7a

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix prospective parachains tests

13734de

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix fix

effb1cc

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fmt

3f75cba

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix comment

75a47bb

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

another one

12ed853

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix build

f2c0882

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim and others added 20 commits September 13, 2024 15:52

refactor

218f530

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix try-runtime

216937a

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Finally all tests compile

e169d0d

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

🤦

ceac8b2

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

check ump signal count and test

c0aee8c

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

remove unused

1ef7952

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix prdoc

5790b8e

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

more tests cases

ba9d3ff

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Merge branch 'sandreim/runtime_v2_descriptor_support' into sandreim/n…

888880f

…ode_v2_descriptors

workaround #64

0dcf739

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

stricter UMP signal checks and tests

604273b

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

stricter UMP signal checks and tests

9c4e2ae

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

type alias

43bbb9d

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Merge branch 'sandreim/runtime_v2_descriptor_support' of github.com:p…

64febf4

…aritytech/polkadot-sdk into sandreim/node_v2_descriptors

remove unused

a493225

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Merge branch 'master' into sandreim/runtime_v2_descriptor_support

d7e57fd

Merge branch 'sandreim/runtime_v2_descriptor_support' of github.com:p…

7e3a943

…aritytech/polkadot-sdk into sandreim/node_v2_descriptors Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

add prdoc

7352577

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix merge damage

c9e44d1

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix

bf948a7

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim mentioned this pull request Sep 26, 2024

candidate-validation: RFC103 implementation #5847

Draft

sandreim added 2 commits September 26, 2024 17:23

fix some builds

fd9dfee

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

fix the fix

5bd465b

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim marked this pull request as ready for review September 26, 2024 14:51

toml fixes

53bb5b8

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim mentioned this pull request Oct 1, 2024

statement-distribution: RFC103 implementation #5883

Draft

alindima reviewed Oct 2, 2024

View reviewed changes

alindima mentioned this pull request Oct 2, 2024

collation-generation: use v2 receipts #5908

Open

Base automatically changed from sandreim/runtime_v2_descriptor_support to master October 7, 2024 07:22

alindima mentioned this pull request Oct 10, 2024

collator protocol: validate descriptor version on the validator side #6011

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch node side to v2 candidate receipts #5679

Switch node side to v2 candidate receipts #5679

sandreim commented Sep 11, 2024 •

edited

Loading

alindima left a comment

alindima Oct 2, 2024

sandreim Oct 3, 2024 •

edited

Loading

sandreim Oct 3, 2024

alindima Oct 2, 2024

sandreim Oct 3, 2024

alindima Oct 2, 2024

alindima Oct 2, 2024

alindima Oct 2, 2024

sandreim Oct 2, 2024

alindima Oct 2, 2024

alindima Oct 2, 2024

sandreim Oct 2, 2024

	/// Returns the value of `$field`` field.
	/// Returns the value of `$field` field.

	Id doesn't alter any functionality and is required before implementing RFC103 on the
	It doesn't alter any functionality and is required before implementing RFC103 on the

Switch node side to v2 candidate receipts #5679

Are you sure you want to change the base?

Switch node side to v2 candidate receipts #5679

Conversation

sandreim commented Sep 11, 2024 • edited Loading

alindima left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandreim Oct 3, 2024 • edited Loading

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

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

sandreim commented Sep 11, 2024 •

edited

Loading

sandreim Oct 3, 2024 •

edited

Loading