diff --git a/node/core/chain-selection/src/lib.rs b/node/core/chain-selection/src/lib.rs index 786454fb9891..af988978ca98 100644 --- a/node/core/chain-selection/src/lib.rs +++ b/node/core/chain-selection/src/lib.rs @@ -466,6 +466,10 @@ where let _ = tx.send(best_containing); } + ChainSelectionMessage::RevertBlocks(blocks_to_revert) => { + let write_ops = handle_revert_blocks(backend, blocks_to_revert)?; + backend.write(write_ops)?; + } } } } @@ -678,6 +682,21 @@ fn handle_approved_block(backend: &mut impl Backend, approved_block: Hash) -> Re backend.write(ops) } +// Here we revert a provided group of blocks. The most common cause for this is that +// the dispute coordinator has notified chain selection of a dispute which concluded +// against a candidate. +fn handle_revert_blocks( + backend: &impl Backend, + blocks_to_revert: Vec<(BlockNumber, Hash)>, +) -> Result, Error> { + let mut overlay = OverlayedBackend::new(backend); + for (block_number, block_hash) in blocks_to_revert { + tree::apply_single_reversion(&mut overlay, block_hash, block_number)?; + } + + Ok(overlay.into_write_ops().collect()) +} + fn detect_stagnant( backend: &mut impl Backend, now: Timestamp, diff --git a/node/core/chain-selection/src/tests.rs b/node/core/chain-selection/src/tests.rs index 404b854d894b..d2b77a2f5d0b 100644 --- a/node/core/chain-selection/src/tests.rs +++ b/node/core/chain-selection/src/tests.rs @@ -2014,3 +2014,106 @@ fn stagnant_makes_childless_parent_leaf() { virtual_overseer }) } + +#[test] +fn revert_blocks_message_triggers_proper_reversion() { + test_harness(|backend, _, mut virtual_overseer| async move { + // Building mini chain with 1 finalized block and 3 unfinalized blocks + let finalized_number = 0; + let finalized_hash = Hash::repeat_byte(0); + + let (head_hash, built_chain) = + construct_chain_on_base(vec![1, 2, 3], finalized_number, finalized_hash, |_| {}); + + import_blocks_into( + &mut virtual_overseer, + &backend, + Some((finalized_number, finalized_hash)), + built_chain.clone(), + ) + .await; + + // Checking mini chain + assert_backend_contains(&backend, built_chain.iter().map(|&(ref h, _)| h)); + assert_leaves(&backend, vec![head_hash]); + assert_leaves_query(&mut virtual_overseer, vec![head_hash]).await; + + let block_1_hash = backend.load_blocks_by_number(1).unwrap().get(0).unwrap().clone(); + let block_2_hash = backend.load_blocks_by_number(2).unwrap().get(0).unwrap().clone(); + + // Sending revert blocks message + let (_, write_rx) = backend.await_next_write(); + virtual_overseer + .send(FromOrchestra::Communication { + msg: ChainSelectionMessage::RevertBlocks(Vec::from([(2, block_2_hash)])), + }) + .await; + + write_rx.await.unwrap(); + + // Checking results: + // Block 2 should be explicitly reverted + assert_eq!( + backend + .load_block_entry(&block_2_hash) + .unwrap() + .unwrap() + .viability + .explicitly_reverted, + true + ); + // Block 3 should be non-viable, with 2 as its earliest unviable ancestor + assert_eq!( + backend + .load_block_entry(&head_hash) + .unwrap() + .unwrap() + .viability + .earliest_unviable_ancestor, + Some(block_2_hash) + ); + // Block 1 should be left as the only leaf + assert_leaves(&backend, vec![block_1_hash]); + + virtual_overseer + }) +} + +#[test] +fn revert_blocks_against_finalized_is_ignored() { + test_harness(|backend, _, mut virtual_overseer| async move { + // Building mini chain with 1 finalized block and 3 unfinalized blocks + let finalized_number = 0; + let finalized_hash = Hash::repeat_byte(0); + + let (head_hash, built_chain) = + construct_chain_on_base(vec![1], finalized_number, finalized_hash, |_| {}); + + import_blocks_into( + &mut virtual_overseer, + &backend, + Some((finalized_number, finalized_hash)), + built_chain.clone(), + ) + .await; + + // Checking mini chain + assert_backend_contains(&backend, built_chain.iter().map(|&(ref h, _)| h)); + + // Sending dispute concluded against message + virtual_overseer + .send(FromOrchestra::Communication { + msg: ChainSelectionMessage::RevertBlocks(Vec::from([( + finalized_number, + finalized_hash, + )])), + }) + .await; + + // Leaf should be head if reversion of finalized was properly ignored + assert_leaves(&backend, vec![head_hash]); + assert_leaves_query(&mut virtual_overseer, vec![head_hash]).await; + + virtual_overseer + }) +} diff --git a/node/core/chain-selection/src/tree.rs b/node/core/chain-selection/src/tree.rs index aafd75de5f97..3db06eb4916b 100644 --- a/node/core/chain-selection/src/tree.rs +++ b/node/core/chain-selection/src/tree.rs @@ -247,7 +247,7 @@ pub(crate) fn import_block( stagnant_at: Timestamp, ) -> Result<(), Error> { add_block(backend, block_hash, block_number, parent_hash, weight, stagnant_at)?; - apply_reversions(backend, block_hash, block_number, reversion_logs)?; + apply_ancestor_reversions(backend, block_hash, block_number, reversion_logs)?; Ok(()) } @@ -347,9 +347,9 @@ fn add_block( Ok(()) } -// Assuming that a block is already imported, accepts the number of the block -// as well as a list of reversions triggered by the block in ascending order. -fn apply_reversions( +/// Assuming that a block is already imported, accepts the number of the block +/// as well as a list of reversions triggered by the block in ascending order. +fn apply_ancestor_reversions( backend: &mut OverlayedBackend, block_hash: Hash, block_number: BlockNumber, @@ -358,39 +358,76 @@ fn apply_reversions( // Note: since revert numbers are in ascending order, the expensive propagation // of unviability is only heavy on the first log. for revert_number in reversions { - let mut ancestor_entry = - match load_ancestor(backend, block_hash, block_number, revert_number)? { - None => { - gum::warn!( - target: LOG_TARGET, - ?block_hash, - block_number, - revert_target = revert_number, - "The hammer has dropped. \ - A block has indicated that its finalized ancestor be reverted. \ - Please inform an adult.", - ); + let maybe_block_entry = load_ancestor(backend, block_hash, block_number, revert_number)?; + revert_single_block_entry_if_present( + backend, + maybe_block_entry, + None, + revert_number, + Some(block_hash), + Some(block_number), + )?; + } - continue - }, - Some(ancestor_entry) => { - gum::info!( - target: LOG_TARGET, - ?block_hash, - block_number, - revert_target = revert_number, - revert_hash = ?ancestor_entry.block_hash, - "A block has signaled that its ancestor be reverted due to a bad parachain block.", - ); + Ok(()) +} - ancestor_entry - }, - }; +/// Marks a single block as explicitly reverted, then propagates viability updates +/// to all its children. This is triggered when the disputes subsystem signals that +/// a dispute has concluded against a candidate. +pub(crate) fn apply_single_reversion( + backend: &mut OverlayedBackend, + revert_hash: Hash, + revert_number: BlockNumber, +) -> Result<(), Error> { + let maybe_block_entry = backend.load_block_entry(&revert_hash)?; + revert_single_block_entry_if_present( + backend, + maybe_block_entry, + Some(revert_hash), + revert_number, + None, + None, + )?; + Ok(()) +} - ancestor_entry.viability.explicitly_reverted = true; - propagate_viability_update(backend, ancestor_entry)?; - } +fn revert_single_block_entry_if_present( + backend: &mut OverlayedBackend, + maybe_block_entry: Option, + maybe_revert_hash: Option, + revert_number: BlockNumber, + maybe_reporting_hash: Option, + maybe_reporting_number: Option, +) -> Result<(), Error> { + match maybe_block_entry { + None => { + gum::warn!( + target: LOG_TARGET, + ?maybe_revert_hash, + revert_target = revert_number, + ?maybe_reporting_hash, + ?maybe_reporting_number, + "The hammer has dropped. \ + The protocol has indicated that a finalized block be reverted. \ + Please inform an adult.", + ); + }, + Some(mut block_entry) => { + gum::info!( + target: LOG_TARGET, + ?maybe_revert_hash, + revert_target = revert_number, + ?maybe_reporting_hash, + ?maybe_reporting_number, + "Unfinalized block reverted due to a bad parachain block.", + ); + block_entry.viability.explicitly_reverted = true; + // Marks children of reverted block as non-viable + propagate_viability_update(backend, block_entry)?; + }, + } Ok(()) } diff --git a/node/core/dispute-coordinator/src/initialized.rs b/node/core/dispute-coordinator/src/initialized.rs index 9f4415ba36a6..703ff1030319 100644 --- a/node/core/dispute-coordinator/src/initialized.rs +++ b/node/core/dispute-coordinator/src/initialized.rs @@ -31,7 +31,7 @@ use polkadot_node_primitives::{ }; use polkadot_node_subsystem::{ messages::{ - ApprovalVotingMessage, BlockDescription, DisputeCoordinatorMessage, + ApprovalVotingMessage, BlockDescription, ChainSelectionMessage, DisputeCoordinatorMessage, DisputeDistributionMessage, ImportStatementsResult, }, overseer, ActivatedLeaf, ActiveLeavesUpdate, FromOrchestra, OverseerSignal, @@ -1027,6 +1027,22 @@ impl Initialized { } } + // Notify ChainSelection if a dispute has concluded against a candidate. ChainSelection + // will need to mark the candidate's relay parent as reverted. + if import_result.is_freshly_concluded_against() { + let blocks_including = self.scraper.get_blocks_including_candidate(&candidate_hash); + if blocks_including.len() > 0 { + ctx.send_message(ChainSelectionMessage::RevertBlocks(blocks_including)).await; + } else { + gum::debug!( + target: LOG_TARGET, + ?candidate_hash, + ?session, + "Could not find an including block for candidate against which a dispute has concluded." + ); + } + } + // Update metrics: if import_result.is_freshly_disputed() { self.metrics.on_open(); diff --git a/node/core/dispute-coordinator/src/scraping/candidates.rs b/node/core/dispute-coordinator/src/scraping/candidates.rs index 2fe797768cc2..de54a4da905c 100644 --- a/node/core/dispute-coordinator/src/scraping/candidates.rs +++ b/node/core/dispute-coordinator/src/scraping/candidates.rs @@ -102,15 +102,18 @@ impl ScrapedCandidates { } // Removes all candidates up to a given height. The candidates at the block height are NOT removed. - pub fn remove_up_to_height(&mut self, height: &BlockNumber) { + pub fn remove_up_to_height(&mut self, height: &BlockNumber) -> HashSet { + let mut candidates_modified: HashSet = HashSet::new(); let not_stale = self.candidates_by_block_number.split_off(&height); let stale = std::mem::take(&mut self.candidates_by_block_number); self.candidates_by_block_number = not_stale; for candidates in stale.values() { for c in candidates { self.candidates.remove(c); + candidates_modified.insert(*c); } } + candidates_modified } pub fn insert(&mut self, block_number: BlockNumber, candidate_hash: CandidateHash) { diff --git a/node/core/dispute-coordinator/src/scraping/mod.rs b/node/core/dispute-coordinator/src/scraping/mod.rs index 29b217e46e4a..3504fba842f5 100644 --- a/node/core/dispute-coordinator/src/scraping/mod.rs +++ b/node/core/dispute-coordinator/src/scraping/mod.rs @@ -14,7 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . -use std::num::NonZeroUsize; +use std::{ + collections::{BTreeMap, HashSet}, + num::NonZeroUsize, +}; use futures::channel::oneshot; use lru::LruCache; @@ -69,9 +72,73 @@ impl ScrapedUpdates { } } +/// A structure meant to facilitate chain reversions in the event of a dispute +/// concluding against a candidate. Each candidate hash maps to a number of +/// block heights, which in turn map to vectors of blocks at those heights. +pub struct Inclusions { + inclusions_inner: BTreeMap>>, +} + +impl Inclusions { + pub fn new() -> Self { + Self { inclusions_inner: BTreeMap::new() } + } + + // Add parent block to the vector which has CandidateHash as an outer key and + // BlockNumber as an inner key + pub fn insert( + &mut self, + candidate_hash: CandidateHash, + block_number: BlockNumber, + block_hash: Hash, + ) { + if let Some(blocks_including) = self.inclusions_inner.get_mut(&candidate_hash) { + if let Some(blocks_at_height) = blocks_including.get_mut(&block_number) { + blocks_at_height.push(block_hash); + } else { + blocks_including.insert(block_number, Vec::from([block_hash])); + } + } else { + let mut blocks_including: BTreeMap> = BTreeMap::new(); + blocks_including.insert(block_number, Vec::from([block_hash])); + self.inclusions_inner.insert(candidate_hash, blocks_including); + } + } + + pub fn remove_up_to_height( + &mut self, + height: &BlockNumber, + candidates_modified: HashSet, + ) { + for candidate in candidates_modified { + if let Some(blocks_including) = self.inclusions_inner.get_mut(&candidate) { + // Returns everything after the given key, including the key. This works because the blocks are sorted in ascending order. + *blocks_including = blocks_including.split_off(height); + } + } + self.inclusions_inner + .retain(|_, blocks_including| blocks_including.keys().len() > 0); + } + + pub fn get(&mut self, candidate: &CandidateHash) -> Vec<(BlockNumber, Hash)> { + let mut inclusions_as_vec: Vec<(BlockNumber, Hash)> = Vec::new(); + if let Some(blocks_including) = self.inclusions_inner.get(candidate) { + for (height, blocks_at_height) in blocks_including.iter() { + for block in blocks_at_height { + inclusions_as_vec.push((*height, *block)); + } + } + } + inclusions_as_vec + } +} + /// Chain scraper /// -/// Scrapes unfinalized chain in order to collect information from blocks. +/// Scrapes unfinalized chain in order to collect information from blocks. Chain scraping +/// during disputes enables critical spam prevention. It does so by updating two important +/// criteria determining whether a vote sent during dispute distribution is potential +/// spam. Namely, whether the candidate being voted on is backed or included. /// /// Concretely: /// @@ -95,6 +162,11 @@ pub struct ChainScraper { /// We assume that ancestors of cached blocks are already processed, i.e. we have saved /// corresponding included candidates. last_observed_blocks: LruCache, + /// Maps included candidate hashes to one or more relay block heights and hashes. + /// These correspond to all the relay blocks which marked a candidate as included, + /// and are needed to apply reversions in case a dispute is concluded against the + /// candidate. + inclusions: Inclusions, } impl ChainScraper { @@ -119,6 +191,7 @@ impl ChainScraper { included_candidates: candidates::ScrapedCandidates::new(), backed_candidates: candidates::ScrapedCandidates::new(), last_observed_blocks: LruCache::new(LRU_OBSERVED_BLOCKS_CAPACITY), + inclusions: Inclusions::new(), }; let update = ActiveLeavesUpdate { activated: Some(initial_head), deactivated: Default::default() }; @@ -195,7 +268,9 @@ impl ChainScraper { { Some(key_to_prune) => { self.backed_candidates.remove_up_to_height(&key_to_prune); - self.included_candidates.remove_up_to_height(&key_to_prune); + let candidates_modified = + self.included_candidates.remove_up_to_height(&key_to_prune); + self.inclusions.remove_up_to_height(&key_to_prune, candidates_modified); }, None => { // Nothing to prune. We are still in the beginning of the chain and there are not @@ -233,6 +308,7 @@ impl ChainScraper { "Processing included event" ); self.included_candidates.insert(block_number, candidate_hash); + self.inclusions.insert(candidate_hash, block_number, block_hash); included_receipts.push(receipt); }, CandidateEvent::CandidateBacked(receipt, _, _, _) => { @@ -318,6 +394,13 @@ impl ChainScraper { } return Ok(ancestors) } + + pub fn get_blocks_including_candidate( + &mut self, + candidate: &CandidateHash, + ) -> Vec<(BlockNumber, Hash)> { + self.inclusions.get(candidate) + } } async fn get_finalized_block_number(sender: &mut Sender) -> FatalResult diff --git a/node/core/dispute-coordinator/src/scraping/tests.rs b/node/core/dispute-coordinator/src/scraping/tests.rs index 3a6befa2002d..86ade56203c4 100644 --- a/node/core/dispute-coordinator/src/scraping/tests.rs +++ b/node/core/dispute-coordinator/src/scraping/tests.rs @@ -578,3 +578,73 @@ fn scraper_handles_the_same_candidate_incuded_in_two_different_block_heights() { assert!(!scraper.is_candidate_included(&magic_candidate.hash())); }); } + +#[test] +fn inclusions_per_candidate_properly_adds_and_prunes() { + const TEST_TARGET_BLOCK_NUMBER: BlockNumber = 2; + const TEST_TARGET_BLOCK_NUMBER_2: BlockNumber = 3; + + // How many blocks should we skip before sending a leaf update. + const BLOCKS_TO_SKIP: usize = 4; + + futures::executor::block_on(async { + let (state, mut virtual_overseer) = TestState::new().await; + + let TestState { mut chain, mut scraper, mut ctx } = state; + + // 1 because `TestState` starts at leaf 1. + let next_update = (1..BLOCKS_TO_SKIP).map(|_| next_leaf(&mut chain)).last().unwrap(); + + let mut finalized_block_number = 1; + let expected_ancestry_len = BLOCKS_TO_SKIP - finalized_block_number as usize; + let overseer_fut = overseer_process_active_leaves_update( + &mut virtual_overseer, + &chain, + finalized_block_number, + expected_ancestry_len, + |block_num| { + if block_num == TEST_TARGET_BLOCK_NUMBER || block_num == TEST_TARGET_BLOCK_NUMBER_2 + { + get_backed_and_included_candidate_events(TEST_TARGET_BLOCK_NUMBER) + } else { + vec![] + } + }, + ); + join(process_active_leaves_update(ctx.sender(), &mut scraper, next_update), overseer_fut) + .await; + + let candidate = make_candidate_receipt(get_block_number_hash(TEST_TARGET_BLOCK_NUMBER)); + + // We included the same candidate at two different block heights. So both blocks in which + // the candidate is included are recorded + assert_eq!( + scraper.get_blocks_including_candidate(&candidate.hash()), + Vec::from([ + (TEST_TARGET_BLOCK_NUMBER, get_block_number_hash(TEST_TARGET_BLOCK_NUMBER)), + (TEST_TARGET_BLOCK_NUMBER_2, get_block_number_hash(TEST_TARGET_BLOCK_NUMBER_2)) + ]) + ); + + // After `DISPUTE_CANDIDATE_LIFETIME_AFTER_FINALIZATION` blocks the earlier inclusion should be removed + finalized_block_number = + TEST_TARGET_BLOCK_NUMBER + DISPUTE_CANDIDATE_LIFETIME_AFTER_FINALIZATION; + process_finalized_block(&mut scraper, &finalized_block_number); + + // The later inclusion should still be present, as we haven't exceeded its lifetime + assert_eq!( + scraper.get_blocks_including_candidate(&candidate.hash()), + Vec::from([( + TEST_TARGET_BLOCK_NUMBER_2, + get_block_number_hash(TEST_TARGET_BLOCK_NUMBER_2) + )]) + ); + + finalized_block_number = + TEST_TARGET_BLOCK_NUMBER_2 + DISPUTE_CANDIDATE_LIFETIME_AFTER_FINALIZATION; + process_finalized_block(&mut scraper, &finalized_block_number); + + // Now both inclusions have exceeded their lifetimes after finalization and should be purged + assert!(scraper.get_blocks_including_candidate(&candidate.hash()).len() == 0); + }); +} diff --git a/node/core/dispute-coordinator/src/tests.rs b/node/core/dispute-coordinator/src/tests.rs index b5c2a6bd8e3f..aa423c5e4eaa 100644 --- a/node/core/dispute-coordinator/src/tests.rs +++ b/node/core/dispute-coordinator/src/tests.rs @@ -36,7 +36,7 @@ use polkadot_node_primitives::{ }; use polkadot_node_subsystem::{ messages::{ - ApprovalVotingMessage, ChainApiMessage, DisputeCoordinatorMessage, + ApprovalVotingMessage, ChainApiMessage, ChainSelectionMessage, DisputeCoordinatorMessage, DisputeDistributionMessage, ImportStatementsResult, }, overseer::FromOrchestra, @@ -3337,3 +3337,152 @@ fn participation_requests_reprioritized_for_newly_included() { }) }); } + +// When a dispute has concluded against a parachain block candidate we want to notify +// the chain selection subsystem. Then chain selection can revert the relay parents of +// the disputed candidate and mark all descendants as non-viable. This direct +// notification saves time compared to letting chain selection learn about a dispute +// conclusion from an on chain revert log. +#[test] +fn informs_chain_selection_when_dispute_concluded_against() { + test_harness(|mut test_state, mut virtual_overseer| { + Box::pin(async move { + let session = 1; + + test_state.handle_resume_sync(&mut virtual_overseer, session).await; + + let candidate_receipt = make_invalid_candidate_receipt(); + let parent_1_number = 1; + let parent_2_number = 2; + + let candidate_hash = candidate_receipt.hash(); + + // Including test candidate in 2 different parent blocks + let block_1_header = Header { + parent_hash: test_state.last_block, + number: parent_1_number, + digest: dummy_digest(), + state_root: dummy_hash(), + extrinsics_root: dummy_hash(), + }; + let parent_1_hash = block_1_header.hash(); + + test_state + .activate_leaf_at_session( + &mut virtual_overseer, + session, + parent_1_number, + vec![make_candidate_included_event(candidate_receipt.clone())], + ) + .await; + + let block_2_header = Header { + parent_hash: test_state.last_block, + number: parent_2_number, + digest: dummy_digest(), + state_root: dummy_hash(), + extrinsics_root: dummy_hash(), + }; + let parent_2_hash = block_2_header.hash(); + + test_state + .activate_leaf_at_session( + &mut virtual_overseer, + session, + parent_2_number, + vec![make_candidate_included_event(candidate_receipt.clone())], + ) + .await; + + let supermajority_threshold = + polkadot_primitives::v2::supermajority_threshold(test_state.validators.len()); + + let (valid_vote, invalid_vote) = generate_opposing_votes_pair( + &test_state, + ValidatorIndex(2), + ValidatorIndex(1), + candidate_hash, + session, + VoteType::Explicit, + ) + .await; + + let (pending_confirmation, confirmation_rx) = oneshot::channel(); + virtual_overseer + .send(FromOrchestra::Communication { + msg: DisputeCoordinatorMessage::ImportStatements { + candidate_receipt: candidate_receipt.clone(), + session, + statements: vec![ + (valid_vote, ValidatorIndex(2)), + (invalid_vote, ValidatorIndex(1)), + ], + pending_confirmation: Some(pending_confirmation), + }, + }) + .await; + handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new()) + .await; + assert_matches!(confirmation_rx.await.unwrap(), + ImportStatementsResult::ValidImport => {} + ); + + // Use a different expected commitments hash to ensure the candidate validation returns invalid. + participation_with_distribution( + &mut virtual_overseer, + &candidate_hash, + CandidateCommitments::default().hash(), + ) + .await; + + let mut statements = Vec::new(); + // minus 2, because of local vote and one previously imported invalid vote. + for i in (0_u32..supermajority_threshold as u32 - 2).map(|i| i + 3) { + let vote = test_state + .issue_explicit_statement_with_index( + ValidatorIndex(i), + candidate_hash, + session, + false, + ) + .await; + + statements.push((vote, ValidatorIndex(i as _))); + } + + virtual_overseer + .send(FromOrchestra::Communication { + msg: DisputeCoordinatorMessage::ImportStatements { + candidate_receipt: candidate_receipt.clone(), + session, + statements, + pending_confirmation: None, + }, + }) + .await; + handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new()) + .await; + + // Checking that concluded dispute has signaled the reversion of all parent blocks. + assert_matches!( + virtual_overseer.recv().await, + AllMessages::ChainSelection( + ChainSelectionMessage::RevertBlocks(revert_set) + ) => { + assert!(revert_set.contains(&(parent_1_number, parent_1_hash))); + assert!(revert_set.contains(&(parent_2_number, parent_2_hash))); + }, + "Overseer did not receive `ChainSelectionMessage::RevertBlocks` message" + ); + + // Wrap up + virtual_overseer.send(FromOrchestra::Signal(OverseerSignal::Conclude)).await; + assert_matches!( + virtual_overseer.try_recv().await, + None => {} + ); + + test_state + }) + }); +} diff --git a/node/overseer/src/lib.rs b/node/overseer/src/lib.rs index 6253f8b3e264..209a1479f3d6 100644 --- a/node/overseer/src/lib.rs +++ b/node/overseer/src/lib.rs @@ -593,6 +593,7 @@ pub struct Overseer { ApprovalVotingMessage, AvailabilityStoreMessage, AvailabilityRecoveryMessage, + ChainSelectionMessage, ])] dispute_coordinator: DisputeCoordinator, diff --git a/node/subsystem-types/src/messages.rs b/node/subsystem-types/src/messages.rs index bf1dbe552f5f..1397795436c0 100644 --- a/node/subsystem-types/src/messages.rs +++ b/node/subsystem-types/src/messages.rs @@ -518,6 +518,9 @@ pub enum ChainSelectionMessage { /// Request the best leaf containing the given block in its ancestry. Return `None` if /// there is no such leaf. BestLeafContaining(Hash, oneshot::Sender>), + /// The passed blocks must be marked as reverted, and their children must be marked + /// as non-viable. + RevertBlocks(Vec<(BlockNumber, Hash)>), } /// A sender for the result of a runtime API request. diff --git a/roadmap/implementers-guide/src/node/disputes/dispute-coordinator.md b/roadmap/implementers-guide/src/node/disputes/dispute-coordinator.md index dc01664c295a..33ac78ee7077 100644 --- a/roadmap/implementers-guide/src/node/disputes/dispute-coordinator.md +++ b/roadmap/implementers-guide/src/node/disputes/dispute-coordinator.md @@ -9,7 +9,11 @@ In particular the dispute-coordinator is responsible for: - Ensuring that the node is able to raise a dispute in case an invalid candidate is found during approval checking. -- Ensuring approval votes will be recorded. +- Ensuring that backing and approval votes will be recorded on chain. With these + votes on chain we can be certain that appropriate targets for slashing will be + available for concluded disputes. Also, scraping these votes during a dispute + is necessary for critical spam prevention measures. +- Ensuring backing votes will never get overridden by explicit votes. - Coordinating actual participation in a dispute, ensuring that the node participates in any justified dispute in a way that ensures resolution of disputes on the network even in the case of many disputes raised (flood/DoS @@ -17,15 +21,13 @@ In particular the dispute-coordinator is responsible for: - Ensuring disputes resolve, even for candidates on abandoned forks as much as reasonably possible, to rule out "free tries" and thus guarantee our gambler's ruin property. -- Provide an API for chain selection, so we can prevent finalization of any +- Providing an API for chain selection, so we can prevent finalization of any chain which has included candidates for which a dispute is either ongoing or concluded invalid and avoid building on chains with an included invalid candidate. -- Provide an API for retrieving (resolved) disputes, including all votes, both +- Providing an API for retrieving (resolved) disputes, including all votes, both implicit (approval, backing) and explicit dispute votes. So validators can get rewarded/slashed accordingly. -- Ensure backing votes are recorded and will never get overridden by explicit - votes. ## Ensuring That Disputes Can Be Raised diff --git a/roadmap/implementers-guide/src/node/utility/chain-selection.md b/roadmap/implementers-guide/src/node/utility/chain-selection.md index fc6a9820143b..640691e55961 100644 --- a/roadmap/implementers-guide/src/node/utility/chain-selection.md +++ b/roadmap/implementers-guide/src/node/utility/chain-selection.md @@ -8,6 +8,7 @@ This subsystem needs to update its information on the unfinalized chain: * On every leaf-activated signal * On every block-finalized signal * On every `ChainSelectionMessage::Approve` + * On every `ChainSelectionMessage::RevertBlocks` * Periodically, to detect stagnation. Simple implementations of these updates do `O(n_unfinalized_blocks)` disk operations. If the amount of unfinalized blocks is relatively small, the updates should not take very much time. However, in cases where there are hundreds or thousands of unfinalized blocks the naive implementations of these update algorithms would have to be replaced with more sophisticated versions. @@ -32,6 +33,10 @@ Gets all leaves of the chain, i.e. block hashes that are suitable to build upon If the required block is unknown or not viable, then return `None`. Iterate over all leaves in order of descending weight, returning the first leaf containing the required block in its chain, and `None` otherwise. +### `ChainSelectionMessage::RevertBlocks` +This message indicates that a dispute has concluded against a parachain block candidate. The message passes along a vector containing the block number and block hash of each block where the disputed candidate was included. The passed blocks will be marked as reverted, and their descendants will be marked as non-viable. + + ### Periodically Detect stagnant blocks and apply the stagnant definition to all descendants. Update the set of viable leaves accordingly.