diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index 83ca86b36b04..1f1debee9cd4 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -37,12 +37,6 @@ Disputes: double_map (SessionIndex, CandidateHash) -> Option, // All included blocks on the chain, as well as the block number in this chain that // should be reverted back to if the candidate is disputed and determined to be invalid. Included: double_map (SessionIndex, CandidateHash) -> Option, -// Maps session indices to a vector indicating the number of potentially-spam disputes -// each validator is participating in. Potentially-spam disputes are remote disputes which have -// fewer than `byzantine_threshold + 1` validators. -// -// The i'th entry of the vector corresponds to the i'th validator in the session. -SpamSlots: map SessionIndex -> Option>, // Whether the chain is frozen or not. Starts as `None`. When this is `Some`, // the chain will not accept any new parachain blocks for backing or inclusion, // and its value indicates the last valid block number in the chain. @@ -55,50 +49,45 @@ Frozen: Option, ## Session Change 1. If the current session is not greater than `config.dispute_period + 1`, nothing to do here. -1. Set `pruning_target = current_session - config.dispute_period - 1`. We add the extra `1` because we want to keep things for `config.dispute_period` _full_ sessions. +1. Set `pruning_target = current_session - config.dispute_period - 1`. We add the extra `1` because we want to keep things for `config.dispute_period` _full_ sessions. The stuff at the end of the most recent session has been around for a little over 0 sessions, not a little over 1. 1. If `LastPrunedSession` is `None`, then set `LastPrunedSession` to `Some(pruning_target)` and return. -1. Otherwise, clear out all disputes, included candidates, and `SpamSlots` entries in the range `last_pruned..=pruning_target` and set `LastPrunedSession` to `Some(pruning_target)`. +2. Otherwise, clear out all disputes and included candidates entries in the range `last_pruned..=pruning_target` and set `LastPrunedSession` to `Some(pruning_target)`. ## 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. If the `Included` map does not contain the candidate and there are fewer than `byzantine_threshold + 1` participating validators, reduce `SpamSlots` for all participating validators. +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. ## Routines * `filter_multi_dispute_data(MultiDisputeStatementSet) -> MultiDisputeStatementSet`: 1. Takes a `MultiDisputeStatementSet` and filters it down to a `MultiDisputeStatementSet` that satisfies all the criteria of `provide_multi_dispute_data`. That is, eliminating - ancient votes, votes which overwhelm the maximum amount of spam slots, and duplicates. - This can be used by block authors to create the final submission in a block which is + ancient votes, duplicates and unconfirmed disputes. + This can be used by block authors to create the final submission in a block which is guaranteed to pass the `provide_multi_dispute_data` checks. * `provide_multi_dispute_data(MultiDisputeStatementSet) -> Vec<(SessionIndex, Hash)>`: 1. Pass on each dispute statement set to `provide_dispute_data`, propagating failure. - 1. Return a list of all candidates who just had disputes initiated. + 2. Return a list of all candidates who just had disputes initiated. * `provide_dispute_data(DisputeStatementSet) -> bool`: Provide data to an ongoing dispute or initiate a dispute. - 1. All statements must be issued under the correct session for the correct candidate. + 1. All statements must be issued under the correct session for the correct candidate. 1. `SessionInfo` is used to check statement signatures and this function should fail if any signatures are invalid. 1. If there is no dispute under `Disputes`, create a new `DisputeState` with blank bitfields. 1. If `concluded_at` is `Some`, and is `concluded_at + config.post_conclusion_acceptance_period < now`, return false. - 1. If the overlap of the validators in the `DisputeStatementSet` and those already present in the `DisputeState` is fewer in number than `byzantine_threshold + 1` and the candidate is not present in the `Included` map - 1. increment `SpamSlots` for each validator in the `DisputeStatementSet` which is not already in the `DisputeState`. Initialize the `SpamSlots` to a zeroed vector first, if necessary. do not increment `SpamSlots` if the candidate is local. - 1. If the value for any spam slot exceeds `config.dispute_max_spam_slots`, return false. - 1. If the overlap of the validators in the `DisputeStatementSet` and those already present in the `DisputeState` is at least `byzantine_threshold + 1`, the `DisputeState` has fewer than `byzantine_threshold + 1` validators, and the candidate is not present in the `Included` map, then decrease `SpamSlots` by 1 for each validator in the `DisputeState`. - 1. Import all statements into the dispute. This should fail if any statements are duplicate or if the corresponding bit for the corresponding validator is set in the dispute already. - 1. If `concluded_at` is `None`, reward all statements. - 1. If `concluded_at` is `Some`, reward all statements slightly less. - 1. If either side now has supermajority and did not previously, slash the other side. This may be both sides, and we support this possibility in code, but note that this requires validators to participate on both sides which has negative expected value. Set `concluded_at` to `Some(now)` if it was `None`. - 1. If just concluded against the candidate and the `Included` map contains `(session, candidate)`: invoke `revert_and_freeze` with the stored block number. - 1. Return true if just initiated, false otherwise. + 2. Import all statements into the dispute. This should fail if any statements are duplicate or if the corresponding bit for the corresponding validator is set in the dispute already. + 3. If `concluded_at` is `None`, reward all statements. + 4. If `concluded_at` is `Some`, reward all statements slightly less. + 5. If either side now has supermajority and did not previously, slash the other side. This may be both sides, and we support this possibility in code, but note that this requires validators to participate on both sides which has negative expected value. Set `concluded_at` to `Some(now)` if it was `None`. + 6. If just concluded against the candidate and the `Included` map contains `(session, candidate)`: invoke `revert_and_freeze` with the stored block number. + 7. Return true if just initiated, false otherwise. * `disputes() -> Vec<(SessionIndex, CandidateHash, DisputeState)>`: Get a list of all disputes and info about dispute state. 1. Iterate over all disputes in `Disputes` and collect into a vector. * `note_included(SessionIndex, CandidateHash, included_in: BlockNumber)`: 1. Add `(SessionIndex, CandidateHash)` to the `Included` map with `included_in - 1` as the value. - 1. If there is a dispute under `(Sessionindex, CandidateHash)` with fewer than `byzantine_threshold + 1` participating validators, decrease `SpamSlots` by 1 for each validator in the `DisputeState`. 1. If there is a dispute under `(SessionIndex, CandidateHash)` that has concluded against the candidate, invoke `revert_and_freeze` with the stored block number. * `concluded_invalid(SessionIndex, CandidateHash) -> bool`: Returns whether a candidate has already concluded a dispute in the negative. @@ -111,3 +100,27 @@ Frozen: Option, 1. If `is_frozen()` return. 1. Set `Frozen` to `Some(BlockNumber)` to indicate a rollback to the block number. 1. Issue a `Revert(BlockNumber + 1)` log to indicate a rollback of the block's child in the header chain, which is the same as a rollback to the block number. + +# Disputes filtering + +All disputes delivered to the runtime by the client are filtered before the actual import. In this context actual import +means persisted in the runtime storage. The filtering has got two purposes: +- Limit the amount of data saved onchain. +- Prevent persisting malicious dispute data onchain. + +*Implementation note*: Filtering is performed in function `filter_dispute_data` from `Disputes` pallet. + +The filtering is performed on the whole statement set which is about to be imported onchain. The following filters are +applied: +1. Remove ancient disputes - if a dispute is concluded before the block number indicated in `OLDEST_ACCEPTED` parameter + it is removed from the set. `OLDEST_ACCEPTED` is a runtime configuration option. + *Implementation note*: `dispute_post_conclusion_acceptance_period` from + `HostConfiguration` is used in the current Polkadot/Kusama implementation. +2. Remove votes from unknown validators. If there is a vote from a validator which wasn't an authority in the session + where the dispute was raised - they are removed. Please note that this step removes only single votes instead of + removing the whole dispute. +3. Remove one sided disputes - if a dispute doesn't contain two opposing votes it is not imported onchain. This serves + as a measure not to import one sided disputes. A dispute is raised only if there are two opposing votes so if the + client is not sending them the dispute is a potential spam. +4. Remove unconfirmed disputes - if a dispute contains less votes than the byzantine threshold it is removed. This is + also a spam precaution. A legitimate client will send only confirmed disputes to the runtime. \ No newline at end of file diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 380a2baa5f78..8cf3e95f79bb 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -1493,6 +1493,8 @@ pub type Migrations = ( >, pallet_scheduler::migration::v4::CleanupAgendas, pallet_staking::migrations::v13::MigrateToV13, + parachains_disputes::migration::v1::MigrateToV1, + parachains_configuration::migration::v4::MigrateToV4, ); /// Unchecked extrinsic type as expected by this runtime. diff --git a/runtime/parachains/src/builder.rs b/runtime/parachains/src/builder.rs index 9a1c16e6aa1d..84651a06003e 100644 --- a/runtime/parachains/src/builder.rs +++ b/runtime/parachains/src/builder.rs @@ -575,7 +575,7 @@ impl BenchBuilder { /// Fill cores `start..last` with dispute statement sets. The statement sets will have 3/4th of /// votes be valid, and 1/4th of votes be invalid. - fn create_disputes_with_no_spam( + fn create_disputes( &self, start: u32, last: u32, @@ -664,7 +664,7 @@ impl BenchBuilder { let backed_candidates = builder .create_backed_candidates(&builder.backed_and_concluding_cores, builder.code_upgrade); - let disputes = builder.create_disputes_with_no_spam( + let disputes = builder.create_disputes( builder.backed_and_concluding_cores.len() as u32, used_cores, builder.dispute_sessions.as_slice(), diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs index 2f09deb34ded..b360c108e428 100644 --- a/runtime/parachains/src/configuration.rs +++ b/runtime/parachains/src/configuration.rs @@ -192,8 +192,6 @@ pub struct HostConfiguration { pub dispute_period: SessionIndex, /// How long after dispute conclusion to accept statements. 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 @@ -263,7 +261,6 @@ impl> Default for HostConfiguration, new: u32) -> DispatchResult { - ensure_root(origin)?; - Self::schedule_config_update(|config| { - config.dispute_max_spam_slots = new; - }) - } - /// Set the dispute conclusion by time out period. #[pallet::call_index(17)] #[pallet::weight(( diff --git a/runtime/parachains/src/configuration/migration.rs b/runtime/parachains/src/configuration/migration.rs index 2b754f155966..14c38bf3550b 100644 --- a/runtime/parachains/src/configuration/migration.rs +++ b/runtime/parachains/src/configuration/migration.rs @@ -17,11 +17,7 @@ //! A module that is responsible for migration of storage. use crate::configuration::{self, Config, Pallet, Store, MAX_POV_SIZE}; -use frame_support::{ - pallet_prelude::*, - traits::StorageVersion, - weights::{OldWeight, Weight}, -}; +use frame_support::{pallet_prelude::*, traits::StorageVersion, weights::Weight}; use frame_system::pallet_prelude::BlockNumberFor; /// The current storage version. @@ -29,12 +25,15 @@ use frame_system::pallet_prelude::BlockNumberFor; /// v0-v1: /// v1-v2: /// v2-v3: -pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(3); +/// v3-v4: +pub const STORAGE_VERSION: StorageVersion = StorageVersion::new(4); -pub mod v3 { +pub mod v4 { use super::*; - use frame_support::traits::OnRuntimeUpgrade; + use frame_support::{traits::OnRuntimeUpgrade, weights::constants::WEIGHT_REF_TIME_PER_MILLIS}; use primitives::v2::{Balance, SessionIndex}; + #[cfg(feature = "try-runtime")] + use sp_std::prelude::*; // Copied over from configuration.rs @ de9e147695b9f1be8bd44e07861a31e483c8343a and removed // all the comments, and changed the Weight struct to OldWeight @@ -51,7 +50,7 @@ pub mod v3 { pub validation_upgrade_delay: BlockNumber, pub max_pov_size: u32, pub max_downward_message_size: u32, - pub ump_service_total_weight: OldWeight, + pub ump_service_total_weight: Weight, pub hrmp_max_parachain_outbound_channels: u32, pub hrmp_max_parathread_outbound_channels: u32, pub hrmp_sender_deposit: Balance, @@ -79,7 +78,7 @@ pub mod v3 { pub zeroth_delay_tranche_width: u32, pub needed_approvals: u32, pub relay_vrf_modulo_samples: u32, - pub ump_max_individual_weight: OldWeight, + pub ump_max_individual_weight: Weight, pub pvf_checking_enabled: bool, pub pvf_voting_ttl: SessionIndex, pub minimum_validation_upgrade_delay: BlockNumber, @@ -114,7 +113,7 @@ pub mod v3 { max_upward_queue_count: Default::default(), max_upward_queue_size: Default::default(), max_downward_message_size: Default::default(), - ump_service_total_weight: OldWeight(Default::default()), + ump_service_total_weight: Default::default(), max_upward_message_size: Default::default(), max_upward_message_num_per_candidate: Default::default(), hrmp_sender_deposit: Default::default(), @@ -127,8 +126,9 @@ pub mod v3 { hrmp_max_parachain_outbound_channels: Default::default(), hrmp_max_parathread_outbound_channels: Default::default(), hrmp_max_message_num_per_candidate: Default::default(), - ump_max_individual_weight: OldWeight( - frame_support::weights::constants::WEIGHT_REF_TIME_PER_MILLIS * 20, + ump_max_individual_weight: Weight::from_parts( + 20u64 * WEIGHT_REF_TIME_PER_MILLIS, + MAX_POV_SIZE as u64, ), pvf_checking_enabled: false, pvf_voting_ttl: 2u32.into(), @@ -137,32 +137,51 @@ pub mod v3 { } } - pub struct MigrateToV3(sp_std::marker::PhantomData); - impl OnRuntimeUpgrade for MigrateToV3 { + pub struct MigrateToV4(sp_std::marker::PhantomData); + impl OnRuntimeUpgrade for MigrateToV4 { + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + log::trace!(target: crate::configuration::LOG_TARGET, "Running pre_upgrade()"); + + ensure!(StorageVersion::get::>() == 3, "The migration requires version 3"); + Ok(Vec::new()) + } + fn on_runtime_upgrade() -> Weight { - if StorageVersion::get::>() == 2 { - let weight_consumed = migrate_to_v3::(); + if StorageVersion::get::>() == 3 { + let weight_consumed = migrate_to_v4::(); - log::info!(target: configuration::LOG_TARGET, "MigrateToV3 executed successfully"); + log::info!(target: configuration::LOG_TARGET, "MigrateToV4 executed successfully"); STORAGE_VERSION.put::>(); weight_consumed } else { - log::warn!(target: configuration::LOG_TARGET, "MigrateToV3 should be removed."); + log::warn!(target: configuration::LOG_TARGET, "MigrateToV4 should be removed."); T::DbWeight::get().reads(1) } } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + log::trace!(target: crate::configuration::LOG_TARGET, "Running post_upgrade()"); + ensure!( + StorageVersion::get::>() == STORAGE_VERSION, + "Storage version should be 4 after the migration" + ); + + Ok(()) + } } } -fn migrate_to_v3() -> Weight { +fn migrate_to_v4() -> Weight { // Unusual formatting is justified: // - make it easier to verify that fields assign what they supposed to assign. // - this code is transient and will be removed after all migrations are done. // - this code is important enough to optimize for legibility sacrificing consistency. #[rustfmt::skip] let translate = - |pre: v3::OldHostConfiguration>| -> + |pre: v4::OldHostConfiguration>| -> configuration::HostConfiguration> { super::HostConfiguration { @@ -177,6 +196,7 @@ validation_upgrade_cooldown : pre.validation_upgrade_cooldown, validation_upgrade_delay : pre.validation_upgrade_delay, max_pov_size : pre.max_pov_size, max_downward_message_size : pre.max_downward_message_size, +ump_service_total_weight : pre.ump_service_total_weight, hrmp_max_parachain_outbound_channels : pre.hrmp_max_parachain_outbound_channels, hrmp_max_parathread_outbound_channels : pre.hrmp_max_parathread_outbound_channels, hrmp_sender_deposit : pre.hrmp_sender_deposit, @@ -197,19 +217,17 @@ max_validators_per_core : pre.max_validators_per_core, max_validators : pre.max_validators, dispute_period : pre.dispute_period, dispute_post_conclusion_acceptance_period: pre.dispute_post_conclusion_acceptance_period, -dispute_max_spam_slots : pre.dispute_max_spam_slots, dispute_conclusion_by_time_out_period : pre.dispute_conclusion_by_time_out_period, no_show_slots : pre.no_show_slots, n_delay_tranches : pre.n_delay_tranches, zeroth_delay_tranche_width : pre.zeroth_delay_tranche_width, needed_approvals : pre.needed_approvals, relay_vrf_modulo_samples : pre.relay_vrf_modulo_samples, +ump_max_individual_weight : pre.ump_max_individual_weight, pvf_checking_enabled : pre.pvf_checking_enabled, pvf_voting_ttl : pre.pvf_voting_ttl, minimum_validation_upgrade_delay : pre.minimum_validation_upgrade_delay, -ump_service_total_weight: Weight::from_ref_time(pre.ump_service_total_weight.0).set_proof_size(MAX_POV_SIZE as u64), -ump_max_individual_weight: Weight::from_ref_time(pre.ump_max_individual_weight.0).set_proof_size(MAX_POV_SIZE as u64), } }; @@ -221,7 +239,7 @@ ump_max_individual_weight: Weight::from_ref_time(pre.ump_max_individual_weight.0 // to be unlikely to be caused by this. So we just log. Maybe it'll work out still? log::error!( target: configuration::LOG_TARGET, - "unexpected error when performing translation of the configuration type during storage upgrade to v2." + "unexpected error when performing translation of the configuration type during storage upgrade to v4." ); } @@ -234,31 +252,43 @@ mod tests { use crate::mock::{new_test_ext, Test}; #[test] - fn v2_deserialized_from_actual_data() { - // Fetched at Kusama 14,703,780 (0x3b2c305d01bd4adf1973d32a2d55ca1260a55eea8dfb3168e317c57f2841fdf1) + fn v3_deserialized_from_actual_data() { + // Example how to get new `raw_config`: + // We'll obtain the raw_config hes for block + // 15,772,152 (0xf89d3ab5312c5f70d396dc59612f0aa65806c798346f9db4b35278baed2e0e53) on Kusama. + // Steps: + // 1. Go to Polkadot.js -> Developer -> Chain state -> Storage: https://polkadot.js.org/apps/#/chainstate + // 2. Set these parameters: + // 2.1. selected state query: configuration; activeConfig(): PolkadotRuntimeParachainsConfigurationHostConfiguration + // 2.2. blockhash to query at: 0xf89d3ab5312c5f70d396dc59612f0aa65806c798346f9db4b35278baed2e0e53 (the hash of the block) + // 2.3. Note the value of encoded storage key -> 0x06de3d8a54d27e44a9d5ce189618f22db4b49d95320d9021994c850f25b8e385 for the referenced block. + // 2.4. You'll also need the decoded values to update the test. + // 3. Go to Polkadot.js -> Developer -> Chain state -> Raw storage + // 3.1 Enter the encoded storage key and you get the raw config. + + // Fetched at Kusama 15,772,152 (0xf89d3ab5312c5f70d396dc59612f0aa65806c798346f9db4b35278baed2e0e53) // // This exceeds the maximal line width length, but that's fine, since this is not code and // doesn't need to be read and also leaving it as one line allows to easily copy it. - let raw_config = hex_literal::hex!["0000a000005000000a00000000c8000000c800000a0000000a000000100e0000580200000000500000c8000000e87648170000001e00000000000000005039278c0400000000000000000000005039278c0400000000000000000000e8030000009001001e00000000000000009001008070000000000000000000000a0000000a0000000a00000001000000010500000001c8000000060000005802000002000000580200000200000059000000000000001e0000002800000000c817a804000000000200000014000000"]; + let raw_config = hex_literal::hex!["0000a000005000000a00000000c8000000c800000a0000000a000000100e0000580200000000500000c800000700e8764817020040011e00000000000000005039278c0400000000000000000000005039278c0400000000000000000000e8030000009001001e00000000000000009001008070000000000000000000000a0000000a0000000a00000001000000010500000001c8000000060000005802000002000000580200000200000059000000000000001e000000280000000700c817a80402004001000200000014000000"]; - let v2 = - v3::OldHostConfiguration::::decode(&mut &raw_config[..]) + let v3 = + v4::OldHostConfiguration::::decode(&mut &raw_config[..]) .unwrap(); // We check only a sample of the values here. If we missed any fields or messed up data types // that would skew all the fields coming after. - assert_eq!(v2.max_code_size, 10_485_760); - assert_eq!(v2.validation_upgrade_cooldown, 3600); - assert_eq!(v2.max_pov_size, 5_242_880); - assert_eq!(v2.hrmp_channel_max_message_size, 102_400); - assert_eq!(v2.dispute_max_spam_slots, 2); - assert_eq!(v2.n_delay_tranches, 89); - assert_eq!(v2.ump_max_individual_weight, OldWeight(20_000_000_000)); - assert_eq!(v2.minimum_validation_upgrade_delay, 20); + assert_eq!(v3.max_code_size, 10_485_760); + assert_eq!(v3.validation_upgrade_cooldown, 3600); + assert_eq!(v3.max_pov_size, 5_242_880); + assert_eq!(v3.hrmp_channel_max_message_size, 102_400); + assert_eq!(v3.n_delay_tranches, 89); + assert_eq!(v3.ump_max_individual_weight, Weight::from_parts(20_000_000_000, 5_242_880)); + assert_eq!(v3.minimum_validation_upgrade_delay, 20); } #[test] - fn test_migrate_to_v3() { + fn test_migrate_to_v4() { // Host configuration has lots of fields. However, in this migration we add only a couple of // fields. The most important part to check are a couple of the last fields. We also pick // extra fields to check arbitrarily, e.g. depending on their position (i.e. the middle) and @@ -267,8 +297,8 @@ mod tests { // We specify only the picked fields and the rest should be provided by the `Default` // implementation. That implementation is copied over between the two types and should work // fine. - let v2 = v3::OldHostConfiguration:: { - ump_max_individual_weight: OldWeight(0x71616e6f6e0au64), + let v3 = v4::OldHostConfiguration:: { + ump_max_individual_weight: Weight::from_parts(0x71616e6f6e0au64, 0x71616e6f6e0au64), needed_approvals: 69, thread_availability_period: 55, hrmp_recipient_deposit: 1337, @@ -279,64 +309,61 @@ mod tests { }; new_test_ext(Default::default()).execute_with(|| { - // Implant the v2 version in the state. + // Implant the v3 version in the state. frame_support::storage::unhashed::put_raw( &configuration::ActiveConfig::::hashed_key(), - &v2.encode(), + &v3.encode(), ); - migrate_to_v3::(); + migrate_to_v4::(); - let v3 = configuration::ActiveConfig::::get(); + let v4 = configuration::ActiveConfig::::get(); #[rustfmt::skip] { - assert_eq!(v2.max_code_size , v3.max_code_size); - assert_eq!(v2.max_head_data_size , v3.max_head_data_size); - assert_eq!(v2.max_upward_queue_count , v3.max_upward_queue_count); - assert_eq!(v2.max_upward_queue_size , v3.max_upward_queue_size); - assert_eq!(v2.max_upward_message_size , v3.max_upward_message_size); - assert_eq!(v2.max_upward_message_num_per_candidate , v3.max_upward_message_num_per_candidate); - assert_eq!(v2.hrmp_max_message_num_per_candidate , v3.hrmp_max_message_num_per_candidate); - assert_eq!(v2.validation_upgrade_cooldown , v3.validation_upgrade_cooldown); - assert_eq!(v2.validation_upgrade_delay , v3.validation_upgrade_delay); - assert_eq!(v2.max_pov_size , v3.max_pov_size); - assert_eq!(v2.max_downward_message_size , v3.max_downward_message_size); - assert_eq!(v2.hrmp_max_parachain_outbound_channels , v3.hrmp_max_parachain_outbound_channels); - assert_eq!(v2.hrmp_max_parathread_outbound_channels , v3.hrmp_max_parathread_outbound_channels); - assert_eq!(v2.hrmp_sender_deposit , v3.hrmp_sender_deposit); - assert_eq!(v2.hrmp_recipient_deposit , v3.hrmp_recipient_deposit); - assert_eq!(v2.hrmp_channel_max_capacity , v3.hrmp_channel_max_capacity); - assert_eq!(v2.hrmp_channel_max_total_size , v3.hrmp_channel_max_total_size); - assert_eq!(v2.hrmp_max_parachain_inbound_channels , v3.hrmp_max_parachain_inbound_channels); - assert_eq!(v2.hrmp_max_parathread_inbound_channels , v3.hrmp_max_parathread_inbound_channels); - assert_eq!(v2.hrmp_channel_max_message_size , v3.hrmp_channel_max_message_size); - assert_eq!(v2.code_retention_period , v3.code_retention_period); - assert_eq!(v2.parathread_cores , v3.parathread_cores); - assert_eq!(v2.parathread_retries , v3.parathread_retries); - assert_eq!(v2.group_rotation_frequency , v3.group_rotation_frequency); - assert_eq!(v2.chain_availability_period , v3.chain_availability_period); - assert_eq!(v2.thread_availability_period , v3.thread_availability_period); - assert_eq!(v2.scheduling_lookahead , v3.scheduling_lookahead); - assert_eq!(v2.max_validators_per_core , v3.max_validators_per_core); - assert_eq!(v2.max_validators , v3.max_validators); - assert_eq!(v2.dispute_period , v3.dispute_period); - assert_eq!(v2.dispute_post_conclusion_acceptance_period, v3.dispute_post_conclusion_acceptance_period); - assert_eq!(v2.dispute_max_spam_slots , v3.dispute_max_spam_slots); - assert_eq!(v2.dispute_conclusion_by_time_out_period , v3.dispute_conclusion_by_time_out_period); - assert_eq!(v2.no_show_slots , v3.no_show_slots); - assert_eq!(v2.n_delay_tranches , v3.n_delay_tranches); - assert_eq!(v2.zeroth_delay_tranche_width , v3.zeroth_delay_tranche_width); - assert_eq!(v2.needed_approvals , v3.needed_approvals); - assert_eq!(v2.relay_vrf_modulo_samples , v3.relay_vrf_modulo_samples); - assert_eq!(v2.pvf_checking_enabled , v3.pvf_checking_enabled); - assert_eq!(v2.pvf_voting_ttl , v3.pvf_voting_ttl); - assert_eq!(v2.minimum_validation_upgrade_delay , v3.minimum_validation_upgrade_delay); + assert_eq!(v3.max_code_size , v4.max_code_size); + assert_eq!(v3.max_head_data_size , v4.max_head_data_size); + assert_eq!(v3.max_upward_queue_count , v4.max_upward_queue_count); + assert_eq!(v3.max_upward_queue_size , v4.max_upward_queue_size); + assert_eq!(v3.max_upward_message_size , v4.max_upward_message_size); + assert_eq!(v3.max_upward_message_num_per_candidate , v4.max_upward_message_num_per_candidate); + assert_eq!(v3.hrmp_max_message_num_per_candidate , v4.hrmp_max_message_num_per_candidate); + assert_eq!(v3.validation_upgrade_cooldown , v4.validation_upgrade_cooldown); + assert_eq!(v3.validation_upgrade_delay , v4.validation_upgrade_delay); + assert_eq!(v3.max_pov_size , v4.max_pov_size); + assert_eq!(v3.max_downward_message_size , v4.max_downward_message_size); + assert_eq!(v3.ump_service_total_weight , v4.ump_service_total_weight); + assert_eq!(v3.hrmp_max_parachain_outbound_channels , v4.hrmp_max_parachain_outbound_channels); + assert_eq!(v3.hrmp_max_parathread_outbound_channels , v4.hrmp_max_parathread_outbound_channels); + assert_eq!(v3.hrmp_sender_deposit , v4.hrmp_sender_deposit); + assert_eq!(v3.hrmp_recipient_deposit , v4.hrmp_recipient_deposit); + assert_eq!(v3.hrmp_channel_max_capacity , v4.hrmp_channel_max_capacity); + assert_eq!(v3.hrmp_channel_max_total_size , v4.hrmp_channel_max_total_size); + assert_eq!(v3.hrmp_max_parachain_inbound_channels , v4.hrmp_max_parachain_inbound_channels); + assert_eq!(v3.hrmp_max_parathread_inbound_channels , v4.hrmp_max_parathread_inbound_channels); + assert_eq!(v3.hrmp_channel_max_message_size , v4.hrmp_channel_max_message_size); + assert_eq!(v3.code_retention_period , v4.code_retention_period); + assert_eq!(v3.parathread_cores , v4.parathread_cores); + assert_eq!(v3.parathread_retries , v4.parathread_retries); + assert_eq!(v3.group_rotation_frequency , v4.group_rotation_frequency); + assert_eq!(v3.chain_availability_period , v4.chain_availability_period); + assert_eq!(v3.thread_availability_period , v4.thread_availability_period); + assert_eq!(v3.scheduling_lookahead , v4.scheduling_lookahead); + assert_eq!(v3.max_validators_per_core , v4.max_validators_per_core); + assert_eq!(v3.max_validators , v4.max_validators); + assert_eq!(v3.dispute_period , v4.dispute_period); + assert_eq!(v3.dispute_post_conclusion_acceptance_period, v4.dispute_post_conclusion_acceptance_period); + assert_eq!(v3.dispute_conclusion_by_time_out_period , v4.dispute_conclusion_by_time_out_period); + assert_eq!(v3.no_show_slots , v4.no_show_slots); + assert_eq!(v3.n_delay_tranches , v4.n_delay_tranches); + assert_eq!(v3.zeroth_delay_tranche_width , v4.zeroth_delay_tranche_width); + assert_eq!(v3.needed_approvals , v4.needed_approvals); + assert_eq!(v3.relay_vrf_modulo_samples , v4.relay_vrf_modulo_samples); + assert_eq!(v3.ump_max_individual_weight , v4.ump_max_individual_weight); + assert_eq!(v3.pvf_checking_enabled , v4.pvf_checking_enabled); + assert_eq!(v3.pvf_voting_ttl , v4.pvf_voting_ttl); + assert_eq!(v3.minimum_validation_upgrade_delay , v4.minimum_validation_upgrade_delay); - assert_eq!(v2.ump_service_total_weight, OldWeight(v3.ump_service_total_weight.ref_time())); - assert_eq!(v2.ump_max_individual_weight, OldWeight(v3.ump_max_individual_weight.ref_time())); - assert_eq!(v3.ump_service_total_weight.proof_size(), MAX_POV_SIZE as u64); - assert_eq!(v3.ump_max_individual_weight.proof_size(), MAX_POV_SIZE as u64); }; // ; makes this a statement. `rustfmt::skip` cannot be put on an expression. }); } diff --git a/runtime/parachains/src/configuration/tests.rs b/runtime/parachains/src/configuration/tests.rs index 6f2faf6cb204..d9d0ae6ea267 100644 --- a/runtime/parachains/src/configuration/tests.rs +++ b/runtime/parachains/src/configuration/tests.rs @@ -309,7 +309,6 @@ fn setting_pending_config_members() { max_validators: None, dispute_period: 239, dispute_post_conclusion_acceptance_period: 10, - dispute_max_spam_slots: 2, dispute_conclusion_by_time_out_period: 512, no_show_slots: 240, n_delay_tranches: 241, @@ -402,11 +401,6 @@ fn setting_pending_config_members() { new_config.dispute_post_conclusion_acceptance_period, ) .unwrap(); - Configuration::set_dispute_max_spam_slots( - RuntimeOrigin::root(), - new_config.dispute_max_spam_slots, - ) - .unwrap(); Configuration::set_dispute_conclusion_by_time_out_period( RuntimeOrigin::root(), new_config.dispute_conclusion_by_time_out_period, diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index a01827f33d7c..ca4cc2dda236 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -46,6 +46,10 @@ mod tests; #[cfg(feature = "runtime-benchmarks")] mod benchmarking; +pub mod migration; + +const LOG_TARGET: &str = "runtime::disputes"; + /// Whether the dispute is local or remote. #[derive(Encode, Decode, Clone, PartialEq, Eq, RuntimeDebug, TypeInfo)] pub enum DisputeLocation { @@ -262,7 +266,6 @@ pub trait DisputesHandler { /// accounting for maximum block weight. fn filter_dispute_data( statement_set: DisputeStatementSet, - max_spam_slots: u32, post_conclusion_acceptance_period: BlockNumber, verify_sigs: VerifyDisputeSignatures, ) -> Option; @@ -311,7 +314,6 @@ impl DisputesHandler for () { fn filter_dispute_data( _set: DisputeStatementSet, - _max_spam_slots: u32, _post_conclusion_acceptance_period: BlockNumber, _verify_sigs: VerifyDisputeSignatures, ) -> Option { @@ -361,14 +363,12 @@ where fn filter_dispute_data( set: DisputeStatementSet, - max_spam_slots: u32, post_conclusion_acceptance_period: T::BlockNumber, verify_sigs: VerifyDisputeSignatures, ) -> Option { pallet::Pallet::::filter_dispute_data( &set, post_conclusion_acceptance_period, - max_spam_slots, verify_sigs, ) .filter_statement_set(set) @@ -471,14 +471,6 @@ pub mod pallet { T::BlockNumber, >; - /// Maps session indices to a vector indicating the number of potentially-spam disputes - /// each validator is participating in. Potentially-spam disputes are remote disputes which have - /// fewer than `byzantine_threshold + 1` validators. - /// - /// The i'th entry of the vector corresponds to the i'th validator in the session. - #[pallet::storage] - pub(super) type SpamSlots = StorageMap<_, Twox64Concat, SessionIndex, Vec>; - /// Whether the chain is frozen. Starts as `None`. When this is `Some`, /// the chain will not accept any new parachain blocks for backing or inclusion, /// and its value indicates the last valid block number in the chain. @@ -517,10 +509,10 @@ pub mod pallet { InvalidSignature, /// Validator vote submitted more than once to dispute. DuplicateStatement, - /// Too many spam slots used by some specific validator. - PotentialSpam, /// A dispute where there are only votes on one side. SingleSidedDispute, + /// Unconfirmed dispute statement sets provided + UnconfirmedDispute, } #[pallet::call] @@ -574,19 +566,9 @@ impl DisputeStateFlags { } } -#[derive(PartialEq, RuntimeDebug)] -enum SpamSlotChange { - /// Add a `+1` to the spam slot for a particular validator index in this session. - Inc, - /// Subtract `-1` ... - Dec, -} - struct ImportSummary { /// The new state, with all votes imported. state: DisputeState, - /// Changes to spam slots. Validator index paired with directional change. - spam_slot_changes: Vec<(ValidatorIndex, SpamSlotChange)>, /// Validators to slash for being (wrongly) on the AGAINST side. slash_against: Vec, /// Validators to slash for being (wrongly) on the FOR side. @@ -699,37 +681,7 @@ impl DisputeStateImporter { let pre_post_contains = |flags| (pre_flags.contains(flags), post_flags.contains(flags)); - // 1. Act on confirmed flag state to inform spam slots changes. - let spam_slot_changes: Vec<_> = match pre_post_contains(DisputeStateFlags::CONFIRMED) { - (false, false) => { - // increment spam slots for all new participants. - self.new_participants - .iter_ones() - .map(|i| (ValidatorIndex(i as _), SpamSlotChange::Inc)) - .collect() - }, - (false, true) => { - // all participants, which are not new participants - let prev_participants = (self.state.validators_for.clone() | - self.state.validators_against.clone()) & - !self.new_participants.clone(); - - prev_participants - .iter_ones() - .map(|i| (ValidatorIndex(i as _), SpamSlotChange::Dec)) - .collect() - }, - (true, false) => { - log::error!("Dispute statements are never removed. This is a bug"); - Vec::new() - }, - (true, true) => { - // No change, nothing to do. - Vec::new() - }, - }; - - // 2. Check for fresh FOR supermajority. Only if not already concluded. + // 1. Check for fresh FOR supermajority. Only if not already concluded. let slash_against = if let (false, true) = pre_post_contains(DisputeStateFlags::FOR_SUPERMAJORITY) { if self.state.concluded_at.is_none() { @@ -746,7 +698,7 @@ impl DisputeStateImporter { Vec::new() }; - // 3. Check for fresh AGAINST supermajority. + // 2. Check for fresh AGAINST supermajority. let slash_for = if let (false, true) = pre_post_contains(DisputeStateFlags::AGAINST_SUPERMAJORITY) { if self.state.concluded_at.is_none() { @@ -761,7 +713,6 @@ impl DisputeStateImporter { ImportSummary { state: self.state, - spam_slot_changes, slash_against, slash_for, new_participants: self.new_participants, @@ -831,32 +782,7 @@ impl Pallet { dispute.concluded_at = Some(now); >::insert(session_index, candidate_hash, &dispute); - if >::contains_key(&session_index, &candidate_hash) { - // Local disputes don't count towards spam. - - weight += T::DbWeight::get().reads_writes(1, 1); - continue - } - - // mildly punish all validators involved. they've failed to make - // data available to others, so this is most likely spam. - SpamSlots::::mutate(session_index, |spam_slots| { - let spam_slots = match spam_slots { - Some(ref mut s) => s, - None => return, - }; - - // also reduce spam slots for all validators involved, if the dispute was unconfirmed. - // this does open us up to more spam, but only for validators who are willing - // to be punished more. - // - // it would be unexpected for any change here to occur when the dispute has not concluded - // in time, as a dispute guaranteed to have at least one honest participant should - // conclude quickly. - let _participating = decrement_spam(spam_slots, &dispute); - }); - - weight += T::DbWeight::get().reads_writes(2, 2); + weight += T::DbWeight::get().writes(1); } } @@ -894,7 +820,6 @@ impl Pallet { // TODO: https://github.com/paritytech/polkadot/issues/3469 #[allow(deprecated)] >::remove_prefix(to_prune, None); - SpamSlots::::remove(to_prune); } *last_pruned = Some(pruning_target); @@ -938,10 +863,10 @@ impl Pallet { // // Votes which are duplicate or already known by the chain are filtered out. // The entire set is removed if the dispute is both, ancient and concluded. + // Disputes without enough votes to get confirmed are also filtered out. fn filter_dispute_data( set: &DisputeStatementSet, post_conclusion_acceptance_period: ::BlockNumber, - max_spam_slots: u32, verify_sigs: VerifyDisputeSignatures, ) -> StatementSetFilter { let mut filter = StatementSetFilter::RemoveIndices(Vec::new()); @@ -960,29 +885,26 @@ impl Pallet { let n_validators = session_info.validators.len(); // Check for ancient. - let (first_votes, dispute_state) = { + let dispute_state = { if let Some(dispute_state) = >::get(&set.session, &set.candidate_hash) { if dispute_state.concluded_at.as_ref().map_or(false, |c| c < &oldest_accepted) { return StatementSetFilter::RemoveAll } - (false, dispute_state) + dispute_state } else { // No state in storage, this indicates it's the first dispute statement set as well. - ( - true, - DisputeState { - validators_for: bitvec![u8, BitOrderLsb0; 0; n_validators], - validators_against: bitvec![u8, BitOrderLsb0; 0; n_validators], - start: now, - concluded_at: None, - }, - ) + DisputeState { + validators_for: bitvec![u8, BitOrderLsb0; 0; n_validators], + validators_against: bitvec![u8, BitOrderLsb0; 0; n_validators], + start: now, + concluded_at: None, + } } }; // Check and import all votes. - let mut summary = { + let summary = { let mut importer = DisputeStateImporter::new(dispute_state, now); for (i, (statement, validator_index, signature)) in set.statements.iter().enumerate() { // assure the validator index and is present in the session info @@ -1039,99 +961,11 @@ impl Pallet { return StatementSetFilter::RemoveAll } - // Apply spam slot changes. Bail early if too many occupied. - let is_local = >::contains_key(&set.session, &set.candidate_hash); - if !is_local { - let mut spam_slots: Vec = - SpamSlots::::get(&set.session).unwrap_or_else(|| vec![0; n_validators]); - let mut spam_filter_struck = false; - for (validator_index, spam_slot_change) in summary.spam_slot_changes { - let spam_slot = spam_slots - .get_mut(validator_index.0 as usize) - .expect("index is in-bounds, as checked above; qed"); - - if let SpamSlotChange::Inc = spam_slot_change { - if *spam_slot >= max_spam_slots { - spam_filter_struck = true; - - // Find the vote by this validator and filter it out. - let first_index_in_set = set - .statements - .iter() - .position(|(_statement, v_i, _signature)| &validator_index == v_i) - .expect( - "spam slots are only incremented when a new statement \ - from a validator is included; qed", - ); - - // Note that there may be many votes by the validator in the statement - // set. There are not supposed to be, but the purpose of this function - // is to filter out invalid submissions, after all. - // - // This is fine - we only need to handle the first one, because all - // subsequent votes' indices have been added to the filter already - // by the duplicate checks above. It's only the first one which - // may not already have been filtered out. - filter.remove_index(first_index_in_set); - - // Removing individual statments can cause the dispute to become onesided. - // Checking that (again) is done after the loop. Remove the bit indices. - summary.new_participants.set(validator_index.0 as _, false); - } - - // It's also worth noting that the `DisputeStateImporter` - // which produces these spam slot updates only produces - // one spam slot update per validator because it rejects - // duplicate votes. - // - // So we don't need to worry about spam slots being - // updated incorrectly after receiving duplicates. - *spam_slot += 1; - } else { - *spam_slot = spam_slot.saturating_sub(1); - } - } - - // We write the spam slots here because sequential calls to - // `filter_dispute_data` have a dependency on each other. - // - // For example, if a validator V occupies 1 spam slot and - // max is 2, then 2 sequential calls incrementing spam slot - // cannot be allowed. - // - // However, 3 sequential calls, where the first increments, - // the second decrements, and the third increments would be allowed. - SpamSlots::::insert(&set.session, spam_slots); - - // This is only relevant in cases where it's the first vote and the state - // would hence hold a onesided dispute. If a onesided dispute can never be - // started, by induction, we can never enter a state of a one sided dispute. - if spam_filter_struck && first_votes { - let mut vote_for_count = 0_u64; - let mut vote_against_count = 0_u64; - // Since this is the first set of statements for the dispute, - // it's sufficient to count the votes in the statement set after they - set.statements.iter().for_each(|(statement, v_i, _signature)| { - if Some(true) == - summary.new_participants.get(v_i.0 as usize).map(|b| *b.as_ref()) - { - match statement { - // `summary.new_flags` contains the spam free votes. - // Note that this does not distinguish between pro or con votes, - // since allowing both of them, even if the spam threshold would be reached - // is a good thing. - // Overflow of the counters is no concern, disputes are limited by weight. - DisputeStatement::Valid(_) => vote_for_count += 1, - DisputeStatement::Invalid(_) => vote_against_count += 1, - } - } - }); - if vote_for_count.is_zero() || vote_against_count.is_zero() { - // It wasn't one-sided before the spam filters, but now it is, - // so we need to be thorough and not import that dispute. - return StatementSetFilter::RemoveAll - } - } + // Reject disputes containing less votes than needed for confirmation. + if (summary.state.validators_for.clone() | &summary.state.validators_against).count_ones() <= + byzantine_threshold(summary.state.validators_for.len()) + { + return StatementSetFilter::RemoveAll } filter @@ -1201,13 +1035,17 @@ impl Pallet { Error::::SingleSidedDispute, ); + // Reject disputes containing less votes than needed for confirmation. + ensure!( + (summary.state.validators_for.clone() | &summary.state.validators_against).count_ones() > + byzantine_threshold(summary.state.validators_for.len()), + Error::::UnconfirmedDispute, + ); + let DisputeStatementSet { ref session, ref candidate_hash, .. } = set; let session = *session; let candidate_hash = *candidate_hash; - // we can omit spam slot checks, `fn filter_disputes_data` is - // always called before calling this `fn`. - if fresh { let is_local = >::contains_key(&session, &candidate_hash); @@ -1283,15 +1121,7 @@ impl Pallet { >::insert(&session, &candidate_hash, revert_to); - // If we just included a block locally which has a live dispute, decrement spam slots - // for any involved validators, if the dispute is not already confirmed by f + 1. if let Some(state) = >::get(&session, candidate_hash) { - SpamSlots::::mutate(&session, |spam_slots| { - if let Some(ref mut spam_slots) = *spam_slots { - decrement_spam(spam_slots, &state); - } - }); - if has_supermajority_against(&state) { Self::revert_and_freeze(revert_to); } @@ -1337,29 +1167,6 @@ fn has_supermajority_against(dispute: &DisputeState) - dispute.validators_against.count_ones() >= supermajority_threshold } -// If the dispute had not enough validators to confirm, decrement spam slots for all the participating -// validators. -// -// Returns the set of participating validators as a bitvec. -fn decrement_spam( - spam_slots: &mut [u32], - dispute: &DisputeState, -) -> bitvec::vec::BitVec { - let byzantine_threshold = byzantine_threshold(spam_slots.len()); - - let participating = dispute.validators_for.clone() | dispute.validators_against.clone(); - let decrement_spam = participating.count_ones() <= byzantine_threshold; - for validator_index in participating.iter_ones() { - if decrement_spam { - if let Some(occupied) = spam_slots.get_mut(validator_index as usize) { - *occupied = occupied.saturating_sub(1); - } - } - } - - participating -} - fn check_signature( validator_public: &ValidatorId, candidate_hash: CandidateHash, diff --git a/runtime/parachains/src/disputes/migration.rs b/runtime/parachains/src/disputes/migration.rs new file mode 100644 index 000000000000..584d4b33872e --- /dev/null +++ b/runtime/parachains/src/disputes/migration.rs @@ -0,0 +1,98 @@ +// Copyright 2017-2022 Parity Technologies (UK) Ltd. +// This file is part of Polkadot. + +// Polkadot is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Polkadot is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Polkadot. If not, see . + +//! Storage migration(s) related to disputes pallet + +use frame_support::traits::StorageVersion; + +/// The current storage version. +const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + +pub mod v1 { + use super::*; + use crate::disputes::{Config, Pallet}; + use frame_support::{ + pallet_prelude::*, storage_alias, traits::OnRuntimeUpgrade, weights::Weight, + }; + use primitives::v2::SessionIndex; + use sp_std::prelude::*; + + #[storage_alias] + type SpamSlots = StorageMap, Twox64Concat, SessionIndex, Vec>; + + pub struct MigrateToV1(sp_std::marker::PhantomData); + impl OnRuntimeUpgrade for MigrateToV1 { + fn on_runtime_upgrade() -> Weight { + let mut weight: Weight = Weight::zero(); + + if StorageVersion::get::>() < STORAGE_VERSION { + log::info!(target: crate::disputes::LOG_TARGET, "Migrating disputes storage to v1"); + weight += migrate_to_v1::(); + STORAGE_VERSION.put::>(); + weight = weight.saturating_add(T::DbWeight::get().reads_writes(1, 1)); + } else { + log::info!( + target: crate::disputes::LOG_TARGET, + "Disputes storage up to date - no need for migration" + ); + } + + weight + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, &'static str> { + log::trace!( + target: crate::disputes::LOG_TARGET, + "SpamSlots before migration: {}", + SpamSlots::::iter().count() + ); + ensure!( + StorageVersion::get::>() == 0, + "Storage version should be less than `1` before the migration", + ); + Ok(Vec::new()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(_state: Vec) -> Result<(), &'static str> { + log::trace!(target: crate::disputes::LOG_TARGET, "Running post_upgrade()"); + ensure!( + StorageVersion::get::>() == STORAGE_VERSION, + "Storage version should be `1` after the migration" + ); + ensure!( + SpamSlots::::iter().count() == 0, + "SpamSlots should be empty after the migration" + ); + Ok(()) + } + } + + /// Migrates the pallet storage to the most recent version, checking and setting the `StorageVersion`. + pub fn migrate_to_v1() -> Weight { + let mut weight: Weight = Weight::zero(); + + // SpamSlots should not contain too many keys so removing everything at once should be safe + let res = SpamSlots::::clear(u32::MAX, None); + // `loops` is the number of iterations => used to calculate read weights + // `backend` is the number of keys removed from the backend => used to calculate write weights + weight = weight + .saturating_add(T::DbWeight::get().reads_writes(res.loops as u64, res.backend as u64)); + + weight + } +} diff --git a/runtime/parachains/src/disputes/tests.rs b/runtime/parachains/src/disputes/tests.rs index 4d8ac714cb7b..d1c785223ff6 100644 --- a/runtime/parachains/src/disputes/tests.rs +++ b/runtime/parachains/src/disputes/tests.rs @@ -23,7 +23,6 @@ use crate::{ Test, PUNISH_VALIDATORS_AGAINST, PUNISH_VALIDATORS_FOR, REWARD_VALIDATORS, }, }; -use assert_matches::assert_matches; use frame_support::{ assert_err, assert_noop, assert_ok, traits::{OnFinalize, OnInitialize}, @@ -31,20 +30,16 @@ use frame_support::{ use primitives::v2::BlockNumber; use sp_core::{crypto::CryptoType, Pair}; -/// Filtering updates the spam slots, as such update them. -fn update_spam_slots(stmts: MultiDisputeStatementSet) -> CheckedMultiDisputeStatementSet { +fn filter_dispute_set(stmts: MultiDisputeStatementSet) -> CheckedMultiDisputeStatementSet { let config = >::config(); - let max_spam_slots = config.dispute_max_spam_slots; let post_conclusion_acceptance_period = config.dispute_post_conclusion_acceptance_period; stmts .into_iter() .filter_map(|set| { - // updates spam slots implicitly let filter = Pallet::::filter_dispute_data( &set, post_conclusion_acceptance_period, - max_spam_slots, VerifyDisputeSignatures::Skip, ); filter.filter_statement_set(set) @@ -135,7 +130,7 @@ fn test_dispute_state_flag_from_state() { } #[test] -fn test_import_new_participant_spam_inc() { +fn test_import_new_participant() { let mut importer = DisputeStateImporter::new( DisputeState { validators_for: bitvec![u8, BitOrderLsb0; 1, 0, 0, 0, 0, 0, 0, 0], @@ -171,14 +166,13 @@ fn test_import_new_participant_spam_inc() { concluded_at: None, }, ); - assert_eq!(summary.spam_slot_changes, vec![(ValidatorIndex(2), SpamSlotChange::Inc)]); assert!(summary.slash_for.is_empty()); assert!(summary.slash_against.is_empty()); assert_eq!(summary.new_participants, bitvec![u8, BitOrderLsb0; 0, 0, 1, 0, 0, 0, 0, 0]); } #[test] -fn test_import_prev_participant_spam_dec_confirmed() { +fn test_import_prev_participant_confirmed() { let mut importer = DisputeStateImporter::new( DisputeState { validators_for: bitvec![u8, BitOrderLsb0; 1, 0, 0, 0, 0, 0, 0, 0], @@ -201,10 +195,7 @@ fn test_import_prev_participant_spam_dec_confirmed() { concluded_at: None, }, ); - assert_eq!( - summary.spam_slot_changes, - vec![(ValidatorIndex(0), SpamSlotChange::Dec), (ValidatorIndex(1), SpamSlotChange::Dec),], - ); + assert!(summary.slash_for.is_empty()); assert!(summary.slash_against.is_empty()); assert_eq!(summary.new_participants, bitvec![u8, BitOrderLsb0; 0, 0, 1, 0, 0, 0, 0, 0]); @@ -212,7 +203,7 @@ fn test_import_prev_participant_spam_dec_confirmed() { } #[test] -fn test_import_prev_participant_spam_dec_confirmed_slash_for() { +fn test_import_prev_participant_confirmed_slash_for() { let mut importer = DisputeStateImporter::new( DisputeState { validators_for: bitvec![u8, BitOrderLsb0; 1, 0, 0, 0, 0, 0, 0, 0], @@ -240,10 +231,7 @@ fn test_import_prev_participant_spam_dec_confirmed_slash_for() { concluded_at: Some(0), }, ); - assert_eq!( - summary.spam_slot_changes, - vec![(ValidatorIndex(0), SpamSlotChange::Dec), (ValidatorIndex(1), SpamSlotChange::Dec),], - ); + assert_eq!(summary.slash_for, vec![ValidatorIndex(0), ValidatorIndex(2)]); assert!(summary.slash_against.is_empty()); assert_eq!(summary.new_participants, bitvec![u8, BitOrderLsb0; 0, 0, 1, 1, 1, 1, 1, 0]); @@ -281,224 +269,12 @@ fn test_import_slash_against() { concluded_at: Some(0), }, ); - assert!(summary.spam_slot_changes.is_empty()); assert!(summary.slash_for.is_empty()); assert_eq!(summary.slash_against, vec![ValidatorIndex(1), ValidatorIndex(5)]); assert_eq!(summary.new_participants, bitvec![u8, BitOrderLsb0; 0, 0, 0, 1, 1, 1, 1, 1]); assert_eq!(summary.new_flags, DisputeStateFlags::FOR_SUPERMAJORITY); } -fn generate_dispute_statement_set_entry( - session: u32, - candidate_hash: CandidateHash, - statement: DisputeStatement, - validator: &::Pair, -) -> (DisputeStatement, ValidatorSignature) { - let valid = match &statement { - DisputeStatement::Valid(_) => true, - _ => false, - }; - let signature_bytes = validator - .sign(&ExplicitDisputeStatement { valid, candidate_hash, session }.signing_payload()); - let signature = ValidatorSignature::try_from(signature_bytes).unwrap(); - (statement, signature) -} - -fn generate_dispute_statement_set( - session: SessionIndex, - candidate_hash: CandidateHash, - validators: &[::Pair], - vidxs: Vec<(usize, DisputeStatement)>, -) -> DisputeStatementSet { - let statements = vidxs - .into_iter() - .map(|(v_i, statement)| { - let validator_index = ValidatorIndex(v_i as u32); - let (statement, signature) = generate_dispute_statement_set_entry( - session, - candidate_hash.clone(), - statement, - &validators[v_i], - ); - (statement, validator_index, signature) - }) - .collect::>(); - DisputeStatementSet { candidate_hash: candidate_hash.clone(), session, statements } -} - -#[test] -fn dispute_statement_becoming_onesided_due_to_spamslots_is_accepted() { - let dispute_conclusion_by_time_out_period = 3; - let start = 10; - let session = start - 1; - let dispute_max_spam_slots = 2; - let post_conclusion_acceptance_period = 3; - - let mock_genesis_config = MockGenesisConfig { - configuration: crate::configuration::GenesisConfig { - config: HostConfiguration { - dispute_conclusion_by_time_out_period, - dispute_max_spam_slots, - ..Default::default() - }, - ..Default::default() - }, - ..Default::default() - }; - - new_test_ext(mock_genesis_config).execute_with(|| { - // We need 6 validators for the byzantine threshold to be 2 - static ACCOUNT_IDS: &[AccountId] = &[0, 1, 2, 3, 4, 5, 6, 7]; - let validators = std::iter::repeat(()) - .take(7) - .map(|_| ::Pair::generate().0) - .collect::>(); - let validators = &validators; - - // a new session at each block, but always the same validators - let session_change_callback = |block_number: u32| -> Option> { - let session_validators = - Vec::from_iter(ACCOUNT_IDS.iter().zip(validators.iter().map(|pair| pair.public()))); - Some((true, block_number, session_validators.clone(), Some(session_validators))) - }; - - run_to_block(start, session_change_callback); - - // Must be _foreign_ parachain candidate - // otherwise slots do not trigger. - let candidate_hash_a = CandidateHash(sp_core::H256::repeat_byte(0xA)); - let candidate_hash_b = CandidateHash(sp_core::H256::repeat_byte(0xB)); - let candidate_hash_c = CandidateHash(sp_core::H256::repeat_byte(0xC)); - let candidate_hash_d = CandidateHash(sp_core::H256::repeat_byte(0xD)); - - let stmts = vec![ - // a - generate_dispute_statement_set( - session, - candidate_hash_a, - validators, - vec![ - (3, DisputeStatement::Valid(ValidDisputeStatementKind::Explicit)), - (6, DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit)), - ], - ), - // b - generate_dispute_statement_set( - session, - candidate_hash_b, - validators, - vec![ - (1, DisputeStatement::Valid(ValidDisputeStatementKind::Explicit)), - (6, DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit)), - ], - ), - // c - generate_dispute_statement_set( - session, - candidate_hash_c, - validators, - vec![ - (2, DisputeStatement::Valid(ValidDisputeStatementKind::Explicit)), - (6, DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit)), - ], - ), - // d - generate_dispute_statement_set( - session, - candidate_hash_d, - validators, - vec![ - (4, DisputeStatement::Valid(ValidDisputeStatementKind::Explicit)), - (5, DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit)), - ], - ), - generate_dispute_statement_set( - session, - candidate_hash_d, - validators, - vec![(6, DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit))], - ), - ]; - - // no filtering happens, host config for `dispute_max_spam_slots: 2` is the default - // updates spam slots implicitly - let set = stmts[0].clone(); - let filter = Pallet::::filter_dispute_data( - &set, - post_conclusion_acceptance_period, - dispute_max_spam_slots, - VerifyDisputeSignatures::Skip, - ); - assert_matches!(&filter, StatementSetFilter::RemoveIndices(v) if v.is_empty()); - assert_matches!(filter.filter_statement_set(set.clone()), Some(modified) => { - assert_eq!(&set, modified.as_ref()); - }); - assert_eq!(SpamSlots::::get(session), Some(vec![0, 0, 0, 1, 0, 0, 1])); - - // <-----> - - // 2nd, still ok? Should be - let set = stmts[1].clone(); - let filter = Pallet::::filter_dispute_data( - &set, - post_conclusion_acceptance_period, - dispute_max_spam_slots, - VerifyDisputeSignatures::Skip, - ); - assert_matches!(&filter, StatementSetFilter::RemoveIndices(v) if v.is_empty()); - assert_matches!(filter.filter_statement_set(set.clone()), Some(modified) => { - assert_eq!(&set, modified.as_ref()); - }); - assert_eq!(SpamSlots::::get(session), Some(vec![0, 1, 0, 1, 0, 0, 2])); - - // <-----> - - // now this is the third spammy participation of validator 6 and hence - let set = stmts[2].clone(); - let filter = Pallet::::filter_dispute_data( - &set, - post_conclusion_acceptance_period, - dispute_max_spam_slots, - VerifyDisputeSignatures::Skip, - ); - assert_matches!(&filter, StatementSetFilter::RemoveAll); - // no need to apply the filter, - // we don't do anything with the result, and spam slots were updated already - - // <-----> - - // now there is no pariticipation in this dispute being initiated - // only validator 4 and 5 are part of it - // with 3 validators it's not a an unconfirmed dispute anymore - // so validator 6, while being considered spammy should work again - let set = stmts[3].clone(); - let filter = Pallet::::filter_dispute_data( - &set, - post_conclusion_acceptance_period, - dispute_max_spam_slots, - VerifyDisputeSignatures::Skip, - ); - assert_matches!(&filter, StatementSetFilter::RemoveIndices(v) if v.is_empty()); - // no need to apply the filter, - // we don't do anything with the result, and spam slots were updated already - - // <-----> - - // it's a spammy participant, so a new dispute will not be accepted being initiated by a spammer - let set = stmts[4].clone(); - let filter = Pallet::::filter_dispute_data( - &set, - post_conclusion_acceptance_period, - dispute_max_spam_slots, - VerifyDisputeSignatures::Skip, - ); - assert_matches!(&filter, StatementSetFilter::RemoveAll); - assert_matches!(filter.filter_statement_set(set.clone()), None); - - assert_eq!(SpamSlots::::get(session), Some(vec![0, 1, 1, 1, 1, 1, 3])); - }); -} - // Test that dispute timeout is handled correctly. #[test] fn test_dispute_timeout() { @@ -517,7 +293,7 @@ fn test_dispute_timeout() { }; new_test_ext(mock_genesis_config).execute_with(|| { - // We need 6 validators for the byzantine threshold to be 2 + // We need 7 validators for the byzantine threshold to be 2 let v0 = ::Pair::generate().0; let v1 = ::Pair::generate().0; let v2 = ::Pair::generate().0; @@ -554,10 +330,12 @@ fn test_dispute_timeout() { let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); - // v0 votes for 3, v6 against. + // 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: start - 1, + session, statements: vec![ ( DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), @@ -571,6 +349,18 @@ fn test_dispute_timeout() { .signing_payload(), ), ), + ( + 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), @@ -586,8 +376,7 @@ fn test_dispute_timeout() { ], }]; - let stmts = update_spam_slots(stmts); - assert_eq!(SpamSlots::::get(start - 1), Some(vec![1, 0, 0, 0, 0, 0, 1])); + let stmts = filter_dispute_set(stmts); assert_ok!( Pallet::::process_checked_multi_dispute_data(stmts), @@ -596,11 +385,20 @@ fn test_dispute_timeout() { // Run to timeout period run_to_block(start + dispute_conclusion_by_time_out_period, |_| None); - assert_eq!(SpamSlots::::get(start - 1), Some(vec![1, 0, 0, 0, 0, 0, 1])); + assert!(>::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!(SpamSlots::::get(start - 1), Some(vec![0, 0, 0, 0, 0, 0, 0])); + assert_eq!( + >::get(&session, &candidate_hash) + .expect("dispute should exist") + .concluded_at + .expect("dispute should have concluded"), + start + dispute_conclusion_by_time_out_period + 1 + ); }); } @@ -888,12 +686,122 @@ fn test_freeze_provided_against_supermajority_for_included() { }); } +mod unconfirmed_disputes { + use super::*; + use assert_matches::assert_matches; + use sp_runtime::ModuleError; + + // Shared initialization code between `test_unconfirmed_are_ignored` and `test_unconfirmed_disputes_cause_block_import_error` + fn generate_dispute_statement_set_and_run_to_block() -> DisputeStatementSet { + // 7 validators needed for byzantine threshold of 2. + 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; + + // Mapping between key pair and `ValidatorIndex` + // v0 -> 0 + // v1 -> 3 + // v2 -> 6 + // v3 -> 5 + // v4 -> 1 + // v5 -> 4 + // v6 -> 2 + + run_to_block(6, |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)); + + // v0 votes for 4, v1 votes against 4. + DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 4, + statements: vec![ + ( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(0), + v0.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 4, + } + .signing_payload(), + ), + ), + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(3), + v1.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session: 4, + } + .signing_payload(), + ), + ), + ], + } + } + #[test] + fn test_unconfirmed_are_ignored() { + new_test_ext(Default::default()).execute_with(|| { + let stmts = vec![generate_dispute_statement_set_and_run_to_block()]; + let stmts = filter_dispute_set(stmts); + + // Not confirmed => should be filtered out + assert_ok!(Pallet::::process_checked_multi_dispute_data(stmts), vec![],); + }); + } + + #[test] + fn test_unconfirmed_disputes_cause_block_import_error() { + new_test_ext(Default::default()).execute_with(|| { + + let stmts = generate_dispute_statement_set_and_run_to_block(); + let stmts = vec![CheckedDisputeStatementSet::unchecked_from_unchecked(stmts)]; + + assert_matches!( + Pallet::::process_checked_multi_dispute_data(stmts), + Err(DispatchError::Module(ModuleError{index: _, error: _, message})) => assert_eq!(message, Some("UnconfirmedDispute")) + ); + + }); + } +} + // tests for: // * provide_multi_dispute: with success scenario // * disputes: correctness of datas // * could_be_invalid: correctness of datas -// * note_included: decrement spam correctly -// * spam slots: correctly incremented and decremented // * ensure rewards and punishment are correctly called. #[test] fn test_provide_multi_dispute_success_and_other() { @@ -907,6 +815,7 @@ fn test_provide_multi_dispute_success_and_other() { let v5 = ::Pair::generate().0; let v6 = ::Pair::generate().0; + // Mapping between key pair and `ValidatorIndex` // v0 -> 0 // v1 -> 3 // v2 -> 6 @@ -943,7 +852,7 @@ fn test_provide_multi_dispute_success_and_other() { let candidate_hash = CandidateHash(sp_core::H256::repeat_byte(1)); - // v0 votes for 3, v6 votes against + // v0 and v1 vote for 3, v6 votes against let stmts = vec![DisputeStatementSet { candidate_hash: candidate_hash.clone(), session: 3, @@ -972,53 +881,7 @@ fn test_provide_multi_dispute_success_and_other() { .signing_payload(), ), ), - ], - }]; - - let stmts = update_spam_slots(stmts); - assert_eq!(SpamSlots::::get(3), Some(vec![1, 0, 1, 0, 0, 0, 0])); - - assert_ok!( - Pallet::::process_checked_multi_dispute_data(stmts), - vec![(3, candidate_hash.clone())], - ); - - // v1 votes for 4 and for 3, v6 votes against 4. - let stmts = vec![ - DisputeStatementSet { - candidate_hash: candidate_hash.clone(), - session: 4, - statements: vec![ - ( - DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), - ValidatorIndex(3), - v1.sign( - &ExplicitDisputeStatement { - valid: true, - candidate_hash: candidate_hash.clone(), - session: 4, - } - .signing_payload(), - ), - ), - ( - DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), - ValidatorIndex(2), - v6.sign( - &ExplicitDisputeStatement { - valid: false, - candidate_hash: candidate_hash.clone(), - session: 4, - } - .signing_payload(), - ), - ), - ], - }, - DisputeStatementSet { - candidate_hash: candidate_hash.clone(), - session: 3, - statements: vec![( + ( DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), ValidatorIndex(3), v1.sign( @@ -1029,20 +892,18 @@ fn test_provide_multi_dispute_success_and_other() { } .signing_payload(), ), - )], - }, - ]; + ), + ], + }]; - let stmts = update_spam_slots(stmts); + let stmts = filter_dispute_set(stmts); assert_ok!( Pallet::::process_checked_multi_dispute_data(stmts), - vec![(4, candidate_hash.clone())], + vec![(3, candidate_hash.clone())], ); - assert_eq!(SpamSlots::::get(3), Some(vec![0, 0, 0, 0, 0, 0, 0])); // Confirmed as no longer spam - assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 1, 1, 0, 0, 0])); - // v3 votes against 3 and for 5, v6 votes against 5. + // v3 votes against 3 and for 5, v2 and v6 vote against 5. let stmts = vec![ DisputeStatementSet { candidate_hash: candidate_hash.clone(), @@ -1076,6 +937,18 @@ fn test_provide_multi_dispute_success_and_other() { .signing_payload(), ), ), + ( + DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), + ValidatorIndex(6), + v2.sign( + &ExplicitDisputeStatement { + valid: false, + candidate_hash: candidate_hash.clone(), + session: 5, + } + .signing_payload(), + ), + ), ( DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), ValidatorIndex(2), @@ -1092,55 +965,31 @@ fn test_provide_multi_dispute_success_and_other() { }, ]; - let stmts = update_spam_slots(stmts); + let stmts = filter_dispute_set(stmts); assert_ok!( Pallet::::process_checked_multi_dispute_data(stmts), vec![(5, candidate_hash.clone())], ); - assert_eq!(SpamSlots::::get(3), Some(vec![0, 0, 0, 0, 0, 0, 0])); - assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 1, 1, 0, 0, 0])); - assert_eq!(SpamSlots::::get(5), Some(vec![0, 0, 1, 0, 0, 1, 0])); - // v2 votes for 3 and against 5 - let stmts = vec![ - DisputeStatementSet { - candidate_hash: candidate_hash.clone(), - session: 3, - statements: vec![( - DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), - ValidatorIndex(6), - v2.sign( - &ExplicitDisputeStatement { - valid: true, - candidate_hash: candidate_hash.clone(), - session: 3, - } - .signing_payload(), - ), - )], - }, - DisputeStatementSet { - candidate_hash: candidate_hash.clone(), - session: 5, - statements: vec![( - DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), - ValidatorIndex(6), - v2.sign( - &ExplicitDisputeStatement { - valid: false, - candidate_hash: candidate_hash.clone(), - session: 5, - } - .signing_payload(), - ), - )], - }, - ]; - let stmts = update_spam_slots(stmts); + // v2 votes for 3 + let stmts = vec![DisputeStatementSet { + candidate_hash: candidate_hash.clone(), + session: 3, + statements: vec![( + DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), + ValidatorIndex(6), + v2.sign( + &ExplicitDisputeStatement { + valid: true, + candidate_hash: candidate_hash.clone(), + session: 3, + } + .signing_payload(), + ), + )], + }]; + let stmts = filter_dispute_set(stmts); assert_ok!(Pallet::::process_checked_multi_dispute_data(stmts), vec![]); - assert_eq!(SpamSlots::::get(3), Some(vec![0, 0, 0, 0, 0, 0, 0])); - assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 1, 1, 0, 0, 0])); - assert_eq!(SpamSlots::::get(5), Some(vec![0, 0, 0, 0, 0, 0, 0])); let stmts = vec![ // 0, 4, and 5 vote against 5 @@ -1218,7 +1067,7 @@ fn test_provide_multi_dispute_success_and_other() { ], }, ]; - let stmts = update_spam_slots(stmts); + let stmts = filter_dispute_set(stmts); assert_ok!(Pallet::::process_checked_multi_dispute_data(stmts), vec![]); assert_eq!( @@ -1244,16 +1093,6 @@ fn test_provide_multi_dispute_success_and_other() { concluded_at: Some(6), // 5 vote for } ), - ( - 4, - candidate_hash.clone(), - DisputeState { - validators_for: bitvec![u8, BitOrderLsb0; 0, 0, 0, 1, 0, 0, 0], - validators_against: bitvec![u8, BitOrderLsb0; 0, 0, 1, 0, 0, 0, 0], - start: 6, - concluded_at: None, - } - ), ] ); @@ -1261,22 +1100,14 @@ fn test_provide_multi_dispute_success_and_other() { assert!(!Pallet::::concluded_invalid(4, candidate_hash.clone())); assert!(Pallet::::concluded_invalid(5, candidate_hash.clone())); - // Ensure inclusion removes spam slots - assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 1, 1, 0, 0, 0])); - Pallet::::note_included(4, candidate_hash.clone(), 4); - assert_eq!(SpamSlots::::get(4), Some(vec![0, 0, 0, 0, 0, 0, 0])); - // Ensure the `reward_validator` function was correctly called assert_eq!( REWARD_VALIDATORS.with(|r| r.borrow().clone()), vec![ - (3, vec![ValidatorIndex(0), ValidatorIndex(2)]), - (4, vec![ValidatorIndex(2), ValidatorIndex(3)]), - (3, vec![ValidatorIndex(3)]), + (3, vec![ValidatorIndex(0), ValidatorIndex(2), ValidatorIndex(3)]), (3, vec![ValidatorIndex(5)]), - (5, vec![ValidatorIndex(2), ValidatorIndex(5)]), + (5, vec![ValidatorIndex(2), ValidatorIndex(5), ValidatorIndex(6)]), (3, vec![ValidatorIndex(6)]), - (5, vec![ValidatorIndex(6)]), (5, vec![ValidatorIndex(0), ValidatorIndex(1), ValidatorIndex(4)]), (3, vec![ValidatorIndex(1), ValidatorIndex(4)]), ], @@ -1286,14 +1117,11 @@ fn test_provide_multi_dispute_success_and_other() { assert_eq!( PUNISH_VALIDATORS_AGAINST.with(|r| r.borrow().clone()), vec![ - (3, vec![]), - (4, vec![]), (3, vec![]), (3, vec![]), (5, vec![]), (3, vec![]), (5, vec![]), - (5, vec![]), (3, vec![ValidatorIndex(2), ValidatorIndex(5)]), ], ); @@ -1302,13 +1130,10 @@ fn test_provide_multi_dispute_success_and_other() { assert_eq!( PUNISH_VALIDATORS_FOR.with(|r| r.borrow().clone()), vec![ - (3, vec![]), - (4, vec![]), (3, vec![]), (3, vec![]), (5, vec![]), (3, vec![]), - (5, vec![]), (5, vec![ValidatorIndex(5)]), (3, vec![]), ], @@ -1380,44 +1205,6 @@ fn test_has_supermajority_against() { ); } -#[test] -fn test_decrement_spam() { - let original_spam_slots = vec![0, 1, 2, 3, 4, 5, 6, 7]; - - // Test confirm is no-op - let mut spam_slots = original_spam_slots.clone(); - let dispute_state_confirm = DisputeState { - validators_for: bitvec![u8, BitOrderLsb0; 1, 1, 0, 0, 0, 0, 0, 0], - validators_against: bitvec![u8, BitOrderLsb0; 1, 0, 1, 0, 0, 0, 0, 0], - start: 0, - concluded_at: None, - }; - assert_eq!(DisputeStateFlags::from_state(&dispute_state_confirm), DisputeStateFlags::CONFIRMED); - assert_eq!( - decrement_spam(spam_slots.as_mut(), &dispute_state_confirm), - bitvec![u8, BitOrderLsb0; 1, 1, 1, 0, 0, 0, 0, 0], - ); - assert_eq!(spam_slots, original_spam_slots); - - // Test not confirm is decreasing spam - let mut spam_slots = original_spam_slots.clone(); - let dispute_state_no_confirm = DisputeState { - validators_for: bitvec![u8, BitOrderLsb0; 1, 0, 0, 0, 0, 0, 0, 0], - validators_against: bitvec![u8, BitOrderLsb0; 1, 0, 1, 0, 0, 0, 0, 0], - start: 0, - concluded_at: None, - }; - assert_eq!( - DisputeStateFlags::from_state(&dispute_state_no_confirm), - DisputeStateFlags::default() - ); - assert_eq!( - decrement_spam(spam_slots.as_mut(), &dispute_state_no_confirm), - bitvec![u8, BitOrderLsb0; 1, 0, 1, 0, 0, 0, 0, 0], - ); - assert_eq!(spam_slots, vec![0, 1, 1, 3, 4, 5, 6, 7]); -} - #[test] fn test_check_signature() { let validator_id = ::Pair::generate().0; @@ -1912,14 +1699,12 @@ fn apply_filter_all>( sets: I, ) -> Vec { let config = >::config(); - let max_spam_slots = config.dispute_max_spam_slots; let post_conclusion_acceptance_period = config.dispute_post_conclusion_acceptance_period; let mut acc = Vec::::new(); for dispute_statement in sets { if let Some(checked) = as DisputesHandler<::BlockNumber>>::filter_dispute_data( dispute_statement, - max_spam_slots, post_conclusion_acceptance_period, VerifyDisputeSignatures::Yes, ) { @@ -1993,13 +1778,11 @@ fn filter_removes_duplicates_within_set() { ], }; - let max_spam_slots = 10; let post_conclusion_acceptance_period = 10; let statements = as DisputesHandler< ::BlockNumber, >>::filter_dispute_data( statements, - max_spam_slots, post_conclusion_acceptance_period, VerifyDisputeSignatures::Yes, ); @@ -2085,151 +1868,6 @@ fn filter_bad_signatures_correctly_detects_single_sided() { }) } -#[test] -fn filter_correctly_accounts_spam_slots() { - let dispute_max_spam_slots = 2; - - let mock_genesis_config = MockGenesisConfig { - configuration: crate::configuration::GenesisConfig { - config: HostConfiguration { dispute_max_spam_slots, ..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 = ::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; - - run_to_block(3, |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_a = CandidateHash(sp_core::H256::repeat_byte(1)); - let candidate_hash_b = CandidateHash(sp_core::H256::repeat_byte(2)); - let candidate_hash_c = CandidateHash(sp_core::H256::repeat_byte(3)); - - let payload = |c_hash: &CandidateHash, valid| { - ExplicitDisputeStatement { valid, candidate_hash: c_hash.clone(), session: 1 } - .signing_payload() - }; - - let payload_a = payload(&candidate_hash_a, true); - let payload_b = payload(&candidate_hash_b, true); - let payload_c = payload(&candidate_hash_c, true); - - let payload_a_bad = payload(&candidate_hash_a, false); - let payload_b_bad = payload(&candidate_hash_b, false); - let payload_c_bad = payload(&candidate_hash_c, false); - - let sig_0a = v0.sign(&payload_a); - let sig_0b = v0.sign(&payload_b); - let sig_0c = v0.sign(&payload_c); - - let sig_1b = v1.sign(&payload_b); - - let sig_2a = v2.sign(&payload_a_bad); - let sig_2b = v2.sign(&payload_b_bad); - let sig_2c = v2.sign(&payload_c_bad); - - let statements = vec![ - // validators 0 and 2 get 1 spam slot from this. - DisputeStatementSet { - candidate_hash: candidate_hash_a.clone(), - session: 1, - statements: vec![ - ( - DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), - ValidatorIndex(0), - sig_0a.clone(), - ), - ( - DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), - ValidatorIndex(6), - sig_2a.clone(), - ), - ], - }, - // Validators 0, 2, and 3 get no spam slots for this - DisputeStatementSet { - candidate_hash: candidate_hash_b.clone(), - session: 1, - statements: vec![ - ( - DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), - ValidatorIndex(0), - sig_0b.clone(), - ), - ( - DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), - ValidatorIndex(3), - sig_1b.clone(), - ), - ( - DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), - ValidatorIndex(6), - sig_2b.clone(), - ), - ], - }, - // Validators 0 and 2 get an extra spam slot for this. - DisputeStatementSet { - candidate_hash: candidate_hash_c.clone(), - session: 1, - statements: vec![ - ( - DisputeStatement::Valid(ValidDisputeStatementKind::Explicit), - ValidatorIndex(0), - sig_0c.clone(), - ), - ( - DisputeStatement::Invalid(InvalidDisputeStatementKind::Explicit), - ValidatorIndex(6), - sig_2c.clone(), - ), - ], - }, - ]; - - let old_statements = statements - .clone() - .into_iter() - .map(CheckedDisputeStatementSet::unchecked_from_unchecked) - .collect::>(); - let statements = apply_filter_all::(statements); - - assert_eq!(statements, old_statements); - }) -} - #[test] fn filter_removes_session_out_of_bounds() { new_test_ext(Default::default()).execute_with(|| { diff --git a/runtime/parachains/src/paras_inherent/mod.rs b/runtime/parachains/src/paras_inherent/mod.rs index 15669ec9c15d..9ee51abd9932 100644 --- a/runtime/parachains/src/paras_inherent/mod.rs +++ b/runtime/parachains/src/paras_inherent/mod.rs @@ -349,7 +349,6 @@ impl Pallet { let (checked_disputes, total_consumed_weight) = { // Obtain config params.. let config = >::config(); - let max_spam_slots = config.dispute_max_spam_slots; let post_conclusion_acceptance_period = config.dispute_post_conclusion_acceptance_period; @@ -363,7 +362,6 @@ impl Pallet { let dispute_set_validity_check = move |set| { T::DisputesHandler::filter_dispute_data( set, - max_spam_slots, post_conclusion_acceptance_period, verify_dispute_sigs, ) @@ -595,7 +593,6 @@ impl Pallet { } let config = >::config(); - let max_spam_slots = config.dispute_max_spam_slots; let post_conclusion_acceptance_period = config.dispute_post_conclusion_acceptance_period; // TODO: Better if we can convert this to `with_transactional` and handle an error if @@ -609,7 +606,6 @@ impl Pallet { let dispute_statement_set_valid = move |set: DisputeStatementSet| { T::DisputesHandler::filter_dispute_data( set, - max_spam_slots, post_conclusion_acceptance_period, // `DisputeCoordinator` on the node side only forwards // valid dispute statement sets and hence this does not diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 44bbd365b1ef..dc32a91c7ace 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -1605,6 +1605,8 @@ pub type Migrations = ( crowdloan::migration::MigrateToTrackInactive, pallet_scheduler::migration::v4::CleanupAgendas, pallet_staking::migrations::v13::MigrateToV13, + parachains_disputes::migration::v1::MigrateToV1, + parachains_configuration::migration::v4::MigrateToV4, ); /// Unchecked extrinsic type as expected by this runtime. diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 65ad8223e5d8..38d5ce28d167 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -1484,6 +1484,8 @@ pub type Migrations = ( pallet_balances::migration::MigrateToTrackInactive, crowdloan::migration::MigrateToTrackInactive, pallet_scheduler::migration::v4::CleanupAgendas, + parachains_disputes::migration::v1::MigrateToV1, + parachains_configuration::migration::v4::MigrateToV4, ); /// Executive: handles dispatch to the various modules. diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 5d050e74a6fe..b10f76a2700d 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1219,6 +1219,8 @@ pub type Migrations = ( crowdloan::migration::MigrateToTrackInactive, pallet_scheduler::migration::v4::CleanupAgendas, pallet_staking::migrations::v13::MigrateToV13, + parachains_disputes::migration::v1::MigrateToV1, + parachains_configuration::migration::v4::MigrateToV4, ); /// Unchecked extrinsic type as expected by this runtime.