[DONTMERGE] Add debug logs and make approval voting failable #3783

Lldenaurois · 2021-09-04T13:40:07Z

Malicious node implementation for burn-in of disputes on Rococo.

This adds substantial logging and causes nodes to falsely raise disputes on good candidate at a frequency of 1/5000 approvals. The next step is to alter candidate-backing to second a deterministically bad candidate at a similar frequency.

rphmeier · 2021-09-04T15:40:31Z

node/core/approval-voting/src/lib.rs

@@ -97,6 +99,7 @@ const APPROVAL_CHECKING_TIMEOUT: Duration = Duration::from_secs(120);
 const APPROVAL_CACHE_SIZE: usize = 1024;
 const TICK_TOO_FAR_IN_FUTURE: Tick = 20; // 10 seconds.
 const LOG_TARGET: &str = "parachain::approval-voting";
+const MALICIOUS_FREQUENCY: usize = 5000;


FYI 5000 is quite a lot. 1 parachain block requires 20ish approvals, so this means 1 in 200 parachain blocks. I'd recommend tweaking this to 50_000 or 100_000 so it's only once every few hours.

rphmeier · 2021-09-04T15:42:31Z

node/core/dispute-coordinator/src/real/mod.rs

 			let _ = rx.send(recent_disputes.keys().cloned().collect());
 		},
 		DisputeCoordinatorMessage::ActiveDisputes(rx) => {
+			// TODO (ladi): Shouldn''t this load active disputes?


I think there's only 'recent disputes' on disk, and then this filters them to be only active

rphmeier · 2021-09-04T15:42:56Z

node/core/dispute-participation/src/lib.rs

@@ -103,7 +104,7 @@ impl Error {
 				tracing::debug!(target: LOG_TARGET, err = ?self)
 			},
 			// it's worth reporting otherwise
-			_ => tracing::warn!(target: LOG_TARGET, err = ?self),
+			_ => tracing::debug!(target: LOG_TARGET, err = ?self),


Why demote this?

rphmeier · 2021-09-04T15:43:15Z

node/core/dispute-participation/src/lib.rs

@@ -119,7 +120,7 @@ where
 		match ctx.recv().await {
 			Err(_) => return,
 			Ok(FromOverseer::Signal(OverseerSignal::Conclude)) => {
-				tracing::info!(target: LOG_TARGET, "Received `Conclude` signal, exiting");
+				tracing::debug!(target: LOG_TARGET, "Received `Conclude` signal, exiting");


And many others in the same file

It's easier (for now) to just display everything in Debug logs imo. I can change it back if you see it differently.

This PR is not intended to get merged at any point

You can match logs with e.g. level =~"DEBUG|INFO|ERROR|WARN" ... just for reference.

node/core/approval-voting/src/lib.rs

rphmeier · 2021-09-06T16:01:24Z

node/core/approval-voting/src/lib.rs

+							.collect();
+						actions.append(&mut approvals);
+					}
+					ApprovalOutcome::Malicious((candidate, session_index)) => {


This seems like a lot of extra machinery to add instead of just sending the message from within the if coin_flip == 0 block

I was debating whether or not to do it simply with the coin_flip == 0, but thought maybe this was a little cleaner.

I realize we aren't going to be re-using this code, but it wasn't a large change. Will keep in mind to keep it simple going forward.

drahnr · 2021-09-07T08:55:53Z

node/core/approval-voting/src/lib.rs

@@ -100,7 +101,8 @@ const APPROVAL_CACHE_SIZE: usize = 1024;
 const TICK_TOO_FAR_IN_FUTURE: Tick = 20; // 10 seconds.
 const LOG_TARGET: &str = "parachain::approval-voting";
 const DEBUG_LOG_TARGET: &str = "parachain::ladi-debug-approval-voting";
-const MALICIOUS_FREQUENCY: usize = 5_000;
+const MALICIOUS_MEAN: f64 = 2_500f64;
+const MALICIOUS_SDEV: f64 = 70f64;


nit: .._STDDEV is the common abbreviation.

Yeah, I wanted MEAN and SDEV to have the same length :)

drahnr · 2021-09-07T09:00:24Z

node/core/approval-voting/src/lib.rs

@@ -346,12 +348,15 @@ where
 {
 	fn start(self, ctx: Context) -> SpawnedSubsystem {
 		let backend = DbBackend::new(self.db.clone(), self.db_config);
+		let normal = Normal::new(MALICIOUS_MEAN, MALICIOUS_SDEV);
+		let offset: u64 = normal.sample(&mut rand::thread_rng()).round() as u64;


I'd recommend adding 3 * STDDEV which means 99.7% are not edited, but you avoid hickups due to huge outliers.

drahnr · 2021-09-07T09:01:26Z

node/core/approval-voting/src/lib.rs

@@ -791,6 +797,7 @@ where
 			&mut approvals_cache,
 			&mut subsystem.mode,
 			actions,
+			if counter.fetch_add(1, Ordering::SeqCst) == offset { true } else { false },


nit: the if case can be dropped :) just use the boolean expression

I added logging in these two cases, so for now I'll leave it as an if-statement. I'll keep the logging for deploying to one node but can drop the logging before deploying to all validators.

drahnr

LGTM (for test)

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

…-info

…adi-logging-disputes

* changed lease period to 1 day * bumped version * bumped version again * changed 356 to 365 days to mimic one year Co-authored-by: Santi Balaguer <[email protected]>

…ng-disputes

* Switch substrate branch to polkadot-v0.9.16 * Bump spec_version * Bump version to 0.9.16

… rk-prep-v0.9.16

* New changelog scripts (paritytech#4491) * Add templates * Add folder for local storage of the digests * Add first draft of the changelog scripts * Enable Audits in the change template * Fixes for Polkadot * Fix templating issue in case there is no high prio change * Fix Ruby setup * Remove shell * Fix chain names * Fix ENV * Fix how to get runtime * Fix runtime_dir * Fix context location * Pin changelogerator to a specific version * New weights for Westend * New weights for Kusama * New weights for Polkadot * deps update

…ng-disputes

Lldenaurois force-pushed the ladi-logging-disputes branch 2 times, most recently from ac8810e to a58867c Compare September 4, 2021 14:15

rphmeier reviewed Sep 4, 2021

View reviewed changes

rphmeier added A1-onice and removed A0-please_review Pull request needs code review. labels Sep 4, 2021

rphmeier changed the title ~~Add debug logs and make approval voting failable~~ [DONTMERGE] Add debug logs and make approval voting failable Sep 4, 2021

Lldenaurois force-pushed the ladi-logging-disputes branch from a58867c to 78abae4 Compare September 4, 2021 23:28

Add debug logs and make approval voting failable

d12cde4

Lldenaurois force-pushed the ladi-logging-disputes branch from 78abae4 to d12cde4 Compare September 4, 2021 23:32

rphmeier reviewed Sep 5, 2021

View reviewed changes

node/core/approval-voting/src/lib.rs Outdated Show resolved Hide resolved

Lldenaurois added 3 commits September 5, 2021 20:57

Merge remote-tracking branch 'origin/master' into ladi-logging-disputes

1f898c7

Increase MALICIOUS_FREQUENCY

10fffc6

Reduce to 5000

bf75f08

Lldenaurois force-pushed the ladi-logging-disputes branch from 603d79f to 0f4d592 Compare September 6, 2021 15:11

IssueLocalStatement on malicious approval vote

65f7767

Lldenaurois force-pushed the ladi-logging-disputes branch from 0f4d592 to 65f7767 Compare September 6, 2021 15:20

rphmeier reviewed Sep 6, 2021

View reviewed changes

Move to uniform distributionary cycle

3333849

drahnr reviewed Sep 7, 2021

View reviewed changes

Update offset calculation

175ed47

drahnr reviewed Sep 7, 2021

View reviewed changes

ggwpez and others added 11 commits January 19, 2022 12:33

Remove generate_storage_info

4c0887a

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

Add more without_storage_info

e0efa29

Signed-off-by: Oliver Tale-Yazdi <[email protected]>

Merge remote-tracking branch 'origin/master' into oty-without-storage…

d1ae091

…-info

update lockfile for {"substrate"}

dd84aa8

Merge remote-tracking branch 'parity/master' into ladi-logging-disputes

f802fd4

Merge remote-tracking branch 'parity/oty-without-storage-info' into l…

b223987

…adi-logging-disputes

Increase Malicious base

05ff8d4

Update Rococo to 1-day lease (paritytech#4761)

7641d6c

* changed lease period to 1 day * bumped version * bumped version again * changed 356 to 365 days to mimic one year Co-authored-by: Santi Balaguer <[email protected]>

Update Rococo to 1-day lease (paritytech#4761)

ca490a1

* changed lease period to 1 day * bumped version * bumped version again * changed 356 to 365 days to mimic one year Co-authored-by: Santi Balaguer <[email protected]>

Decrease malicious base and remove adder collator test

f7d997d

Remove malicious approval voting

e63e448

Lldenaurois force-pushed the ladi-logging-disputes branch from 8aa9955 to e63e448 Compare January 21, 2022 21:48

Lldenaurois and others added 9 commits January 22, 2022 02:10

Enable disputes testing without needing to run collators

c6a16d1

Remove collator protocol tests

58726b9

Merge remote-tracking branch 'parity/rk-prep-v0.9.16' into ladi-loggi…

d6b1965

…ng-disputes

v0.9.16 version bumps (paritytech#4766)

cdfd5be

* Switch substrate branch to polkadot-v0.9.16 * Bump spec_version * Bump version to 0.9.16

Update deps

9dd8afa

Update Substrate for 0.9.16 (paritytech#4769)

8664023

Merge branch 'rk-prep-v0.9.16' of github.com:paritytech/polkadot into…

56a6e68

… rk-prep-v0.9.16

Merge branch 'release-v0.9.16' into rk-prep-v0.9.16

22a7690

Lldenaurois requested a review from chevdor as a code owner January 25, 2022 18:12

Merge remote-tracking branch 'parity/rk-prep-v0.9.16' into ladi-loggi…

cd4b791

…ng-disputes

Lldenaurois force-pushed the ladi-logging-disputes branch from ce4e0b0 to cd4b791 Compare January 25, 2022 18:26

Diener: Update substrate dependencies to a-sync-all-forks

a9414ee

Lldenaurois force-pushed the ladi-logging-disputes branch from 35a6216 to f58d17b Compare January 27, 2022 03:44

Merge remote-tracking branch 'parity/rk-prep-v0.9.16' into ladi-loggi…

61de01c

…ng-disputes

Lldenaurois force-pushed the ladi-logging-disputes branch from f58d17b to 61de01c Compare January 27, 2022 03:47

the-right-joyce added S5-on_ice Work is currently on hold, see comments for further information. and removed A1-onice labels Dec 14, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DONTMERGE] Add debug logs and make approval voting failable #3783

[DONTMERGE] Add debug logs and make approval voting failable #3783

Lldenaurois commented Sep 4, 2021 •

edited by rphmeier

Loading

rphmeier Sep 4, 2021 •

edited

Loading

rphmeier Sep 4, 2021

rphmeier Sep 4, 2021

rphmeier Sep 4, 2021

rphmeier Sep 4, 2021

Lldenaurois Sep 5, 2021

eskimor Sep 9, 2021

rphmeier Sep 6, 2021

Lldenaurois Sep 6, 2021

drahnr Sep 7, 2021

Lldenaurois Sep 7, 2021

drahnr Sep 7, 2021

drahnr Sep 7, 2021 •

edited

Loading

Lldenaurois Sep 7, 2021

drahnr left a comment •

edited

Loading

[DONTMERGE] Add debug logs and make approval voting failable #3783

Are you sure you want to change the base?

[DONTMERGE] Add debug logs and make approval voting failable #3783

Conversation

Lldenaurois commented Sep 4, 2021 • edited by rphmeier Loading

rphmeier Sep 4, 2021 • 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

Choose a reason for hiding this comment

drahnr Sep 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drahnr left a comment • edited Loading

Choose a reason for hiding this comment

Lldenaurois commented Sep 4, 2021 •

edited by rphmeier

Loading

rphmeier Sep 4, 2021 •

edited

Loading

drahnr Sep 7, 2021 •

edited

Loading

drahnr left a comment •

edited

Loading