From 29a4fb16eb4eaeb04e28fd9a1343d2c6d113cc3c Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Wed, 17 May 2023 21:29:20 +0300 Subject: [PATCH] Freeze chain if there are byzantine threshold + 1 invalid votes against a local candidate (#7225) --- runtime/parachains/src/disputes.rs | 10 +- runtime/parachains/src/disputes/tests.rs | 170 ++++++++++++++++++++++- 2 files changed, 176 insertions(+), 4 deletions(-) diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index 3141c1e24741..6a3d61ff8047 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -560,6 +560,8 @@ bitflags::bitflags! { const FOR_SUPERMAJORITY = 0b0010; /// Is the supermajority against the validity of the block reached. const AGAINST_SUPERMAJORITY = 0b0100; + /// Is there f+1 against the validity of the block reached + const AGAINST_BYZANTINE = 0b1000; } } @@ -582,6 +584,10 @@ impl DisputeStateFlags { flags |= DisputeStateFlags::FOR_SUPERMAJORITY; } + if state.validators_against.count_ones() > byzantine_threshold { + flags |= DisputeStateFlags::AGAINST_BYZANTINE; + } + if state.validators_against.count_ones() >= supermajority_threshold { flags |= DisputeStateFlags::AGAINST_SUPERMAJORITY; } @@ -1243,8 +1249,8 @@ impl Pallet { >::insert(&session, &candidate_hash, &summary.state); - // Freeze if just concluded against some local candidate - if summary.new_flags.contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { + // Freeze if the INVALID votes against some local candidate are above the byzantine threshold + if summary.new_flags.contains(DisputeStateFlags::AGAINST_BYZANTINE) { if let Some(revert_to) = >::get(&session, &candidate_hash) { Self::revert_and_freeze(revert_to); } diff --git a/runtime/parachains/src/disputes/tests.rs b/runtime/parachains/src/disputes/tests.rs index b40a5325ef64..9ca3468083c8 100644 --- a/runtime/parachains/src/disputes/tests.rs +++ b/runtime/parachains/src/disputes/tests.rs @@ -123,6 +123,16 @@ fn test_dispute_state_flag_from_state() { DisputeStateFlags::FOR_SUPERMAJORITY | DisputeStateFlags::CONFIRMED, ); + assert_eq!( + DisputeStateFlags::from_state(&DisputeState { + validators_for: bitvec![u8, BitOrderLsb0; 0, 0, 0, 0, 0, 0, 0], + validators_against: bitvec![u8, BitOrderLsb0; 1, 1, 1, 0, 0, 0, 0], + start: 0, + concluded_at: None, + }), + DisputeStateFlags::CONFIRMED | DisputeStateFlags::AGAINST_BYZANTINE, + ); + assert_eq!( DisputeStateFlags::from_state(&DisputeState { validators_for: bitvec![u8, BitOrderLsb0; 0, 0, 0, 0, 0, 0, 0], @@ -130,7 +140,9 @@ fn test_dispute_state_flag_from_state() { start: 0, concluded_at: None, }), - DisputeStateFlags::AGAINST_SUPERMAJORITY | DisputeStateFlags::CONFIRMED, + DisputeStateFlags::AGAINST_SUPERMAJORITY | + DisputeStateFlags::CONFIRMED | + DisputeStateFlags::AGAINST_BYZANTINE, ); } @@ -248,7 +260,9 @@ fn test_import_prev_participant_confirmed_slash_for() { assert_eq!(summary.new_participants, bitvec![u8, BitOrderLsb0; 0, 0, 1, 1, 1, 1, 1, 0]); assert_eq!( summary.new_flags, - DisputeStateFlags::CONFIRMED | DisputeStateFlags::AGAINST_SUPERMAJORITY, + DisputeStateFlags::CONFIRMED | + DisputeStateFlags::AGAINST_SUPERMAJORITY | + DisputeStateFlags::AGAINST_BYZANTINE, ); } @@ -674,6 +688,158 @@ fn test_freeze_provided_against_supermajority_for_included() { }); } +#[test] +fn test_freeze_provided_against_byzantine_threshold_for_included() { + new_test_ext(Default::default()).execute_with(|| { + let v0 = ::Pair::generate().0; + let v1 = ::Pair::generate().0; + let v2 = ::Pair::generate().0; + let v3 = ::Pair::generate().0; + let v4 = ::Pair::generate().0; + let v5 = ::Pair::generate().0; + let v6 = ::Pair::generate().0; + + let active_set = vec![ + (&0, v0.public()), + (&1, v1.public()), + (&2, v2.public()), + (&3, v3.public()), + (&4, v4.public()), + (&5, v5.public()), + (&6, v6.public()), + ]; + + run_to_block(6, |b| Some((true, b, active_set.clone(), Some(active_set.clone())))); + + // A candidate which will be disputed + let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); + let inclusion_parent = sp_core::H256::repeat_byte(0xff); + let session = 3; + + // A byzantine threshold of INVALID + let stmts = vec![DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session, + statements: vec![ + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session, + } + .signing_payload(), + ), + ), + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(1), + v1.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session, + } + .signing_payload(), + ), + ), + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(2), + v2.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session, + } + .signing_payload(), + ), + ), + ( + DisputeStatement::Valid(ValidDisputeStatementKind::BackingValid( + inclusion_parent, + )), + ValidatorIndex(1), + v0.sign(&CompactStatement::Valid(candidate_hash).signing_payload( + &SigningContext { session_index: session, parent_hash: inclusion_parent }, + )), + ), + ], + }]; + + // Include the candidate and import the votes + Pallet::::note_included(3, candidate_hash.clone(), 3); + assert!(Pallet::::process_checked_multi_dispute_data( + &stmts + .into_iter() + .map(CheckedDisputeStatementSet::unchecked_from_unchecked) + .collect() + ) + .is_ok()); + // Successful import should freeze the chain + assert_eq!(Frozen::::get(), Some(2)); + + // Now include one more block + run_to_block(7, |b| Some((true, b, active_set.clone(), Some(active_set.clone())))); + Pallet::::note_included(3, CandidateHash(sp_core::H256::repeat_byte(2)), 3); + + // And generate enough votes to reach supermajority of invalid votes + let stmts = vec![DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session, + statements: vec![ + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(3), + v3.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session, + } + .signing_payload(), + ), + ), + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(4), + v4.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session, + } + .signing_payload(), + ), + ), + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(5), + v5.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session, + } + .signing_payload(), + ), + ), + ], + }]; + assert!(Pallet::::process_checked_multi_dispute_data( + &stmts + .into_iter() + .map(CheckedDisputeStatementSet::unchecked_from_unchecked) + .collect() + ) + .is_ok()); + // Chain should still be frozen + assert_eq!(Frozen::::get(), Some(2)); + }); +} + mod unconfirmed_disputes { use super::*; use assert_matches::assert_matches;