inherent disputes: remove per block initializer and disputes timeout …

…event (#6937) * Limit disputes weight and remove initializer code Signed-off-by: Andrei Sandu <[email protected]> * const Signed-off-by: Andrei Sandu <[email protected]> * remove timeout test Signed-off-by: Andrei Sandu <[email protected]> * review feedback #1 Signed-off-by: Andrei Sandu <[email protected]> * Remove the new weight limiting for disputes Signed-off-by: Andrei Sandu <[email protected]> * cargo lock Signed-off-by: Andrei Sandu <[email protected]> * Remove dispute_conclusion_by_time_out_period Signed-off-by: Andrei Sandu <[email protected]> * Enable migrations Signed-off-by: Andrei Sandu <[email protected]> * Update guide Signed-off-by: Andrei Sandu <[email protected]> * Fix comment Signed-off-by: Andrei Sandu <[email protected]> * More guide fixes Signed-off-by: Andrei Sandu <[email protected]> * Also migrate pending configs Signed-off-by: Andrei Sandu <[email protected]> * fix build Signed-off-by: Andrei Sandu <[email protected]> * fix test Signed-off-by: Andrei Sandu <[email protected]> --------- Signed-off-by: Andrei Sandu <[email protected]>
paritytech · Mar 24, 2023 · 2375a62 · 2375a62
1 parent 32b9138
commit 2375a62
Show file tree

Hide file tree

Showing 13 changed files with 116 additions and 266 deletions.
diff --git a/roadmap/implementers-guide/src/disputes-flow.md b/roadmap/implementers-guide/src/disputes-flow.md
@@ -8,7 +8,6 @@ stateDiagram-v2
     [*] --> WaitForDisputeVote: backing Vote received
     WaitForBackingVote --> Open: negative Vote received
     WaitForDisputeVote --> Open: backing Vote received
-    Open --> Concluded: Timeout without supermajority
     Open --> Concluded: Incoming Vote via Gossip
     Open --> Open: No ⅔ supermajority
     Open --> [*]

diff --git a/roadmap/implementers-guide/src/protocol-disputes.md b/roadmap/implementers-guide/src/protocol-disputes.md
@@ -62,6 +62,6 @@ Validators are rewarded for providing statements to the chain as well as for par
 
 ## Dispute Conclusion
 
-Disputes, roughly, are over when one side reaches a ⅔ supermajority. They may also conclude after a timeout, without either side witnessing supermajority, which will only happen if the majority of validators are unable to vote for some reason. Furthermore, disputes on-chain will stay open for some fixed amount of time even after concluding, to accept new votes.
+Disputes, roughly, are over when one side reaches a ⅔ supermajority. They may also never conclude without either side witnessing supermajority, which will only happen if the majority of validators are unable to vote for some reason. Furthermore, disputes on-chain will stay open for some fixed amount of time even after concluding, to accept new votes.
 
 Late votes, after the dispute already reached a ⅔ supermajority, must be rewarded (albeit a smaller amount) as well.
diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md
@@ -56,7 +56,7 @@ Frozen: Option<BlockNumber>,
 
 ## Block Initialization
 
-1. Iterate through all disputes. If any have not concluded and started more than `config.dispute_conclusion_by_timeout_period` blocks ago, set them to `Concluded` and mildly punish all validators associated, as they have failed to distribute available data.
+This is currently a `no op`.
 
 ## Routines
 

diff --git a/roadmap/implementers-guide/src/types/runtime.md b/roadmap/implementers-guide/src/types/runtime.md
@@ -44,8 +44,6 @@ struct HostConfiguration {
 	pub dispute_post_conclusion_acceptance_period: BlockNumber,
 	/// The maximum number of dispute spam slots
 	pub dispute_max_spam_slots: u32,
-	/// How long it takes for a dispute to conclude by time-out, if no supermajority is reached.
-	pub dispute_conclusion_by_time_out_period: BlockNumber,
 	/// The amount of consensus slots that must pass between submitting an assignment and
 	/// submitting an approval vote before a validator is considered a no-show.
 	/// Must be at least 1.

diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs
@@ -1481,6 +1481,7 @@ pub type Migrations = (
 		Runtime,
 		NominationPoolsMigrationV4OldPallet,
 	>,
+	parachains_configuration::migration::v5::MigrateToV5<Runtime>,
 );
 
 /// Unchecked extrinsic type as expected by this runtime.

diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs
@@ -193,8 +193,6 @@ pub struct HostConfiguration<BlockNumber> {
 	pub dispute_period: SessionIndex,
 	/// How long after dispute conclusion to accept statements.
 	pub dispute_post_conclusion_acceptance_period: BlockNumber,
-	/// How long it takes for a dispute to conclude by time-out, if no supermajority is reached.
-	pub dispute_conclusion_by_time_out_period: BlockNumber,
 	/// The amount of consensus slots that must pass between submitting an assignment and
 	/// submitting an approval vote before a validator is considered a no-show.
 	///
@@ -262,7 +260,6 @@ impl<BlockNumber: Default + From<u32>> Default for HostConfiguration<BlockNumber
 			max_validators: None,
 			dispute_period: 6,
 			dispute_post_conclusion_acceptance_period: 100.into(),
-			dispute_conclusion_by_time_out_period: 200.into(),
 			n_delay_tranches: Default::default(),
 			zeroth_delay_tranche_width: Default::default(),
 			needed_approvals: Default::default(),
@@ -765,22 +762,6 @@ pub mod pallet {
 			})
 		}
 
-		/// Set the dispute conclusion by time out period.
-		#[pallet::call_index(17)]
-		#[pallet::weight((
-			T::WeightInfo::set_config_with_block_number(),
-			DispatchClass::Operational,
-		))]
-		pub fn set_dispute_conclusion_by_time_out_period(
-			origin: OriginFor<T>,
-			new: T::BlockNumber,
-		) -> DispatchResult {
-			ensure_root(origin)?;
-			Self::schedule_config_update(|config| {
-				config.dispute_conclusion_by_time_out_period = new;
-			})
-		}
-
 		/// Set the no show slots, in number of number of consensus slots.
 		/// Must be at least 1.
 		#[pallet::call_index(18)]

diff --git a/runtime/parachains/src/configuration/migration.rs b/runtime/parachains/src/configuration/migration.rs
diff --git a/runtime/parachains/src/configuration/tests.rs b/runtime/parachains/src/configuration/tests.rs
@@ -297,7 +297,6 @@ fn setting_pending_config_members() {
 			max_validators: None,
 			dispute_period: 239,
 			dispute_post_conclusion_acceptance_period: 10,
-			dispute_conclusion_by_time_out_period: 512,
 			no_show_slots: 240,
 			n_delay_tranches: 241,
 			zeroth_delay_tranche_width: 242,
@@ -389,11 +388,6 @@ fn setting_pending_config_members() {
 			new_config.dispute_post_conclusion_acceptance_period,
 		)
 		.unwrap();
-		Configuration::set_dispute_conclusion_by_time_out_period(
-			RuntimeOrigin::root(),
-			new_config.dispute_conclusion_by_time_out_period,
-		)
-		.unwrap();
 		Configuration::set_no_show_slots(RuntimeOrigin::root(), new_config.no_show_slots).unwrap();
 		Configuration::set_n_delay_tranches(RuntimeOrigin::root(), new_config.n_delay_tranches)
 			.unwrap();

diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs
@@ -18,7 +18,7 @@
 
 use crate::{configuration, initializer::SessionChangeNotification, session_info};
 use bitvec::{bitvec, order::Lsb0 as BitOrderLsb0};
-use frame_support::{ensure, traits::Get, weights::Weight};
+use frame_support::{ensure, weights::Weight};
 use frame_system::pallet_prelude::*;
 use parity_scale_codec::{Decode, Encode};
 use primitives::{
@@ -503,9 +503,6 @@ pub mod pallet {
 		/// A dispute has concluded for or against a candidate.
 		/// `\[para id, candidate hash, dispute result\]`
 		DisputeConcluded(CandidateHash, DisputeResult),
-		/// A dispute has timed out due to insufficient participation.
-		/// `\[para id, candidate hash\]`
-		DisputeTimedOut(CandidateHash),
 		/// A dispute has concluded with supermajority against a candidate.
 		/// Block authors should no longer build on top of this head and should
 		/// instead revert the block at the given height. This should be the
@@ -911,26 +908,8 @@ impl StatementSetFilter {
 
 impl<T: Config> Pallet<T> {
 	/// Called by the initializer to initialize the disputes module.
-	pub(crate) fn initializer_initialize(now: T::BlockNumber) -> Weight {
-		let config = <configuration::Pallet<T>>::config();
-
-		let mut weight = Weight::zero();
-		for (session_index, candidate_hash, mut dispute) in <Disputes<T>>::iter() {
-			weight += T::DbWeight::get().reads_writes(1, 0);
-
-			if dispute.concluded_at.is_none() &&
-				dispute.start + config.dispute_conclusion_by_time_out_period < now
-			{
-				Self::deposit_event(Event::DisputeTimedOut(candidate_hash));
-
-				dispute.concluded_at = Some(now);
-				<Disputes<T>>::insert(session_index, candidate_hash, &dispute);
-
-				weight += T::DbWeight::get().writes(1);
-			}
-		}
-
-		weight
+	pub(crate) fn initializer_initialize(_now: T::BlockNumber) -> Weight {
+		Weight::zero()
 	}
 
 	/// Called by the initializer to finalize the disputes pallet.

diff --git a/runtime/parachains/src/disputes/tests.rs b/runtime/parachains/src/disputes/tests.rs
@@ -329,131 +329,6 @@ fn test_import_backing_votes() {
 	);
 }
 
-// Test that dispute timeout is handled correctly.
-#[test]
-fn test_dispute_timeout() {
-	let dispute_conclusion_by_time_out_period = 3;
-	let start = 10;
-
-	let mock_genesis_config = MockGenesisConfig {
-		configuration: crate::configuration::GenesisConfig {
-			config: HostConfiguration {
-				dispute_conclusion_by_time_out_period,
-				..Default::default()
-			},
-			..Default::default()
-		},
-		..Default::default()
-	};
-
-	new_test_ext(mock_genesis_config).execute_with(|| {
-		// We need 7 validators for the byzantine threshold to be 2
-		let v0 = <ValidatorId as CryptoType>::Pair::generate().0;
-		let v1 = <ValidatorId as CryptoType>::Pair::generate().0;
-		let v2 = <ValidatorId as CryptoType>::Pair::generate().0;
-		let v3 = <ValidatorId as CryptoType>::Pair::generate().0;
-		let v4 = <ValidatorId as CryptoType>::Pair::generate().0;
-		let v5 = <ValidatorId as CryptoType>::Pair::generate().0;
-		let v6 = <ValidatorId as CryptoType>::Pair::generate().0;
-
-		run_to_block(start, |b| {
-			// a new session at each block
-			Some((
-				true,
-				b,
-				vec![
-					(&0, v0.public()),
-					(&1, v1.public()),
-					(&2, v2.public()),
-					(&3, v3.public()),
-					(&4, v4.public()),
-					(&5, v5.public()),
-					(&6, v6.public()),
-				],
-				Some(vec![
-					(&0, v0.public()),
-					(&1, v1.public()),
-					(&2, v2.public()),
-					(&3, v3.public()),
-					(&4, v4.public()),
-					(&5, v5.public()),
-					(&6, v6.public()),
-				]),
-			))
-		});
-
-		let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1));
-		let inclusion_parent = sp_core::H256::repeat_byte(0xff);
-
-		// v0 and v1 vote for 3, v2 against. We need f+1 votes (3) so that the dispute is
-		// confirmed. Otherwise It will be filtered out.
-		let session = start - 1;
-		let stmts = vec![DisputeStatementSet {
-			candidate_hash: candidate_hash.clone(),
-			session,
-			statements: vec![
-				(
-					DisputeStatement::Valid(ValidDisputeStatementKind::BackingValid(
-						inclusion_parent,
-					)),
-					ValidatorIndex(0),
-					v0.sign(&CompactStatement::Valid(candidate_hash).signing_payload(
-						&SigningContext { session_index: start - 1, parent_hash: inclusion_parent },
-					)),
-				),
-				(
-					DisputeStatement::Valid(ValidDisputeStatementKind::Explicit),
-					ValidatorIndex(1),
-					v1.sign(
-						&ExplicitDisputeStatement {
-							valid: true,
-							candidate_hash: candidate_hash.clone(),
-							session: start - 1,
-						}
-						.signing_payload(),
-					),
-				),
-				(
-					DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit),
-					ValidatorIndex(6),
-					v2.sign(
-						&ExplicitDisputeStatement {
-							valid: false,
-							candidate_hash: candidate_hash.clone(),
-							session: start - 1,
-						}
-						.signing_payload(),
-					),
-				),
-			],
-		}];
-
-		let stmts = filter_dispute_set(stmts);
-
-		assert_ok!(
-			Pallet::<Test>::process_checked_multi_dispute_data(&stmts),
-			vec![(9, candidate_hash.clone())],
-		);
-
-		// Run to timeout period
-		run_to_block(start + dispute_conclusion_by_time_out_period, |_| None);
-		assert!(<Disputes<Test>>::get(&session, &candidate_hash)
-			.expect("dispute should exist")
-			.concluded_at
-			.is_none());
-
-		// Run to timeout + 1 in order to executive on_finalize(timeout)
-		run_to_block(start + dispute_conclusion_by_time_out_period + 1, |_| None);
-		assert_eq!(
-			<Disputes<Test>>::get(&session, &candidate_hash)
-				.expect("dispute should exist")
-				.concluded_at
-				.expect("dispute should have concluded"),
-			start + dispute_conclusion_by_time_out_period + 1
-		);
-	});
-}
-
 // Test pruning works
 #[test]
 fn test_initializer_on_new_session() {

diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs
@@ -1418,6 +1418,7 @@ pub type Migrations = (
 		Runtime,
 		NominationPoolsMigrationV4OldPallet,
 	>,
+	parachains_configuration::migration::v5::MigrateToV5<Runtime>,
 );
 
 /// Unchecked extrinsic type as expected by this runtime.

diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs
@@ -1494,7 +1494,7 @@ pub type UncheckedExtrinsic =
 /// All migrations that will run on the next runtime upgrade.
 ///
 /// Should be cleared after every release.
-pub type Migrations = ();
+pub type Migrations = parachains_configuration::migration::v5::MigrateToV5<Runtime>;
 
 /// Executive: handles dispatch to the various modules.
 pub type Executive = frame_executive::Executive<

diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs
@@ -1214,6 +1214,7 @@ pub type Migrations = (
 		Runtime,
 		NominationPoolsMigrationV4OldPallet,
 	>,
+	parachains_configuration::migration::v5::MigrateToV5<Runtime>,
 );
 
 /// Unchecked extrinsic type as expected by this runtime.