Malus: Implement storing invalid chunks #4711

Lldenaurois · 2022-01-13T03:10:56Z

This PR implements a first attempt at intercepting AvailabilityStoreMessages and writing invalid chunks into the availability store for every StoreAvailableData message intercepted by the MessageInterceptor.

sandreim

Took a first pass, will take a second one and pay more attention to details vs reference impl.

sandreim · 2022-01-13T11:41:59Z

node/core/av-store/src/lib.rs

@@ -1039,6 +1039,7 @@ fn process_message(
 			let _ = tx.send(a);
 		},
 		AvailabilityStoreMessage::QueryChunk(candidate, validator_index, tx) => {
+			println!("THIS IS MOST DEFF WORKING");


Suggested change

println!("THIS IS MOST DEFF WORKING");

println!("THIS IS MOST DEFF WORKING");

sandreim · 2022-01-13T11:43:03Z

node/malus/src/malus.rs

@@ -65,8 +65,8 @@ impl MalusCli {
 		match self.variant {
 			NemesisVariant::BackGarbageCandidate(cmd) =>
 				polkadot_cli::run_node(run_cmd(cmd), BackGarbageCandidate)?,
-			NemesisVariant::SuggestGarbageCandidate(cmd) =>
-				polkadot_cli::run_node(run_cmd(cmd), SuggestGarbageCandidate)?,
+			NemesisVariant::SuggestGarbageCandidate(_cmd) => panic! {},


Is this on purpose ?

sandreim · 2022-01-13T11:43:10Z

node/malus/src/malus.rs

-			NemesisVariant::SuggestGarbageCandidate(cmd) =>
-				polkadot_cli::run_node(run_cmd(cmd), SuggestGarbageCandidate)?,
+			NemesisVariant::SuggestGarbageCandidate(_cmd) => panic! {},
+			//polkadot_cli::run_node(run_cmd(cmd), SuggestGarbageCandidate)?,


Suggested change

//polkadot_cli::run_node(run_cmd(cmd), SuggestGarbageCandidate)?,

//polkadot_cli::run_node(run_cmd(cmd), SuggestGarbageCandidate)?,

sandreim · 2022-01-13T11:44:27Z

node/malus/src/tests.rs

@@ -131,7 +134,8 @@ fn integrity_test_pass() {
 					AvailabilityStoreMessage::QueryChunk(Default::default(), 0.into(), tx),
 				)
 				.await;
-				let _ = rx.timeout(std::time::Duration::from_millis(100)).await.unwrap();
+				let resp = rx.timeout(std::time::Duration::from_secs(10)).await.unwrap();
+				println!("RESP {:?}", resp);


Should we add these as logs ?

sandreim · 2022-01-13T16:23:45Z

node/malus/src/variants/store_malicious_available_data.rs

+
+/// Unix time wrapper with big-endian encoding.
+#[derive(Debug, Clone, Copy, PartialEq, PartialOrd, Eq, Ord)]
+struct BETimestamp(u64);


This duplicates https://github.com/paritytech/polkadot/blob/master/node/core/av-store/src/lib.rs#L78. We can reuse that to reduce the amount of code written in malus.

sandreim · 2022-01-13T16:25:43Z

node/malus/src/variants/store_malicious_available_data.rs

@@ -0,0 +1,353 @@
+// Copyright 2021 Parity Technologies (UK) Ltd.


Can we avoid duplicating this file: https://github.com/paritytech/polkadot/blob/master/node/core/av-store/src/lib.rs ?

sandreim · 2022-01-13T16:27:19Z

node/malus/src/malus.rs

+			NemesisVariant::StoreMaliciousAvailableData(cmd) =>
+				polkadot_cli::run_node(run_cmd(cmd), StoreMaliciousAvailableDataWrapper)?,
 			NemesisVariant::SuggestGarbageCandidate(cmd) =>
-				polkadot_cli::run_node(run_cmd(cmd), SuggestGarbageCandidate)?,
+				polkadot_cli::run_node(run_cmd(cmd), BackGarbageCandidateWrapper)?,


Do we want to maybe combine these eventually? So we can run tests with malicious stored available data and dispute valid candidates for example.

drahnr · 2022-01-14T12:48:21Z

node/malus/src/variants/dispute_valid_candidates.rs

-					filter,
-				)
-			})
+			.replace_candidate_backing(move |cb| InterceptedSubsystem::new(cb, filter))


#4711 Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com> Co-authored-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

…ate` implementation (#5011) * Implement fake validation results Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * refactor Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * cargo lock Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * spell check Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * spellcheck Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * typos Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Review feedback Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * move stuff around Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * chores Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Impl valid - still wip Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fixes Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fmt Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Pull Ladi's implementation: #4711 Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com> Co-authored-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix build Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Logs and comments Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * WIP: suggest garbage candidate + implement validation result caching Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Do commitment hash checks in candidate validation Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Minor refactor in approval, backing, dispute-coord Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Working version of suggest garbage candidate Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Dedup Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * cleanup #1 Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * remove debug leftovers Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fmt Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Accidentally commited some local test Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * spellcheck Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * some more fixes Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Refactor and fix it Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * review feedback Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * typo Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * tests review feedback Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * refactor disputer Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix zombienet disputes test Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * spellcheck Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix ui tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix typo Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com>

drahnr · 2022-05-11T20:33:07Z

@sandreim does this bring any additional coverage to what we have? If not let's close this

Lldenaurois added 2 commits January 13, 2022 04:05

First attempt to implement storing malicious chunks

5367f35

Suggest garbage candidate

f8e2de4

ordian added A3-in_progress Pull request is in progress. No review needed at this stage. 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. labels Jan 13, 2022

Lldenaurois added 4 commits January 13, 2022 09:28

Fix build error

50eae31

Remove comment

6b0bdaf

Fix binary build

98bfb89

Add both variants to the malus binary

258b229

sandreim reviewed Jan 13, 2022

View reviewed changes

drahnr reviewed Jan 14, 2022

View reviewed changes

Lldenaurois added 3 commits January 26, 2022 15:17

Merge remote-tracking branch 'parity/master' into malus_backing

a431471

Merge remote-tracking branch 'parity/master' into malus_backing

2b40707

WORKING

301e566

sandreim added a commit that referenced this pull request Mar 28, 2022

Pull Ladi's implementation:

1c279dc

#4711 Co-authored-by: Lldenaurois <Ljdenaurois@gmail.com> Co-authored-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

sandreim mentioned this pull request Mar 30, 2022

Malus: improvements in dispute ancestor and suggest garbage candidate implementation #5011

Merged

sandreim closed this May 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Malus: Implement storing invalid chunks #4711

Malus: Implement storing invalid chunks #4711

Lldenaurois commented Jan 13, 2022

sandreim left a comment

sandreim Jan 13, 2022

sandreim Jan 13, 2022

sandreim Jan 13, 2022

sandreim Jan 13, 2022

sandreim Jan 13, 2022

sandreim Jan 13, 2022

sandreim Jan 13, 2022

drahnr Jan 14, 2022

drahnr commented May 11, 2022

	println!("THIS IS MOST DEFF WORKING");
	println!("THIS IS MOST DEFF WORKING");

	//polkadot_cli::run_node(run_cmd(cmd), SuggestGarbageCandidate)?,
	//polkadot_cli::run_node(run_cmd(cmd), SuggestGarbageCandidate)?,

		@@ -0,0 +1,353 @@
		// Copyright 2021 Parity Technologies (UK) Ltd.

Malus: Implement storing invalid chunks #4711

Malus: Implement storing invalid chunks #4711

Conversation

Lldenaurois commented Jan 13, 2022

sandreim left a 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

drahnr commented May 11, 2022