Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

dispute-coordinator: fix earliest session checks for pruning and import #6358

Merged
merged 4 commits into from
Nov 28, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions node/core/dispute-coordinator/src/db/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::{
backend::{Backend, BackendWriteOp, OverlayedBackend},
error::{FatalError, FatalResult},
metrics::Metrics,
DISPUTE_WINDOW, LOG_TARGET,
LOG_TARGET,
};

const RECENT_DISPUTES_KEY: &[u8; 15] = b"recent-disputes";
Expand Down Expand Up @@ -318,25 +318,24 @@ pub(crate) fn load_recent_disputes(
///
/// If one or more ancient sessions are pruned, all metadata on candidates within the ancient
/// session will be deleted.
pub(crate) fn note_current_session(
pub(crate) fn note_earliest_session(
overlay_db: &mut OverlayedBackend<'_, impl Backend>,
current_session: SessionIndex,
new_earliest_session: SessionIndex,
) -> SubsystemResult<()> {
let new_earliest = current_session.saturating_sub(DISPUTE_WINDOW.get());
match overlay_db.load_earliest_session()? {
None => {
// First launch - write new-earliest.
overlay_db.write_earliest_session(new_earliest);
overlay_db.write_earliest_session(new_earliest_session);
},
Some(prev_earliest) if new_earliest > prev_earliest => {
Some(prev_earliest) if new_earliest_session > prev_earliest => {
// Prune all data in the outdated sessions.
overlay_db.write_earliest_session(new_earliest);
overlay_db.write_earliest_session(new_earliest_session);

// Clear recent disputes metadata.
{
let mut recent_disputes = overlay_db.load_recent_disputes()?.unwrap_or_default();

let lower_bound = (new_earliest, CandidateHash(Hash::repeat_byte(0x00)));
let lower_bound = (new_earliest_session, CandidateHash(Hash::repeat_byte(0x00)));

let new_recent_disputes = recent_disputes.split_off(&lower_bound);
// Any remanining disputes are considered ancient and must be pruned.
Expand Down Expand Up @@ -373,6 +372,7 @@ mod tests {

use super::*;
use ::test_helpers::{dummy_candidate_receipt, dummy_hash};
use polkadot_node_primitives::DISPUTE_WINDOW;
use polkadot_primitives::v2::{Hash, Id as ParaId};

fn make_db() -> DbBackend {
Expand Down Expand Up @@ -422,7 +422,7 @@ mod tests {
let mut overlay_db = OverlayedBackend::new(&backend);

gum::trace!(target: LOG_TARGET, ?current_session, "Noting current session");
note_current_session(&mut overlay_db, current_session).unwrap();
note_earliest_session(&mut overlay_db, earliest_session).unwrap();

let write_ops = overlay_db.into_write_ops();
backend.write(write_ops).unwrap();
Expand All @@ -442,7 +442,7 @@ mod tests {
let current_session = current_session + 1;
let earliest_session = earliest_session + 1;

note_current_session(&mut overlay_db, current_session).unwrap();
note_earliest_session(&mut overlay_db, earliest_session).unwrap();

let write_ops = overlay_db.into_write_ops();
backend.write(write_ops).unwrap();
Expand Down Expand Up @@ -599,7 +599,7 @@ mod tests {
}

#[test]
fn note_current_session_prunes_old() {
fn note_earliest_session_prunes_old() {
let mut backend = make_db();

let hash_a = CandidateHash(Hash::repeat_byte(0x0a));
Expand Down Expand Up @@ -648,7 +648,7 @@ mod tests {
backend.write(write_ops).unwrap();

let mut overlay_db = OverlayedBackend::new(&backend);
note_current_session(&mut overlay_db, current_session).unwrap();
note_earliest_session(&mut overlay_db, new_earliest_session).unwrap();

assert_eq!(overlay_db.load_earliest_session().unwrap(), Some(new_earliest_session));

Expand Down
6 changes: 3 additions & 3 deletions node/core/dispute-coordinator/src/initialized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use sc_keystore::LocalKeystore;

use polkadot_node_primitives::{
CandidateVotes, DisputeMessage, DisputeMessageCheckError, DisputeStatus,
SignedDisputeStatement, Timestamp, DISPUTE_WINDOW,
SignedDisputeStatement, Timestamp,
};
use polkadot_node_subsystem::{
messages::{
Expand Down Expand Up @@ -299,7 +299,7 @@ impl Initialized {

self.highest_session = session;

db::v1::note_current_session(overlay_db, session)?;
db::v1::note_earliest_session(overlay_db, new_window_start)?;
self.spam_slots.prune_old(new_window_start);
}
},
Expand Down Expand Up @@ -708,7 +708,7 @@ impl Initialized {
now: Timestamp,
) -> Result<ImportStatementsResult> {
gum::trace!(target: LOG_TARGET, ?statements, "In handle import statements");
if session + DISPUTE_WINDOW.get() < self.highest_session {
if !self.rolling_session_window.contains(session) {
// It is not valid to participate in an ancient dispute (spam?).
sandreim marked this conversation as resolved.
Show resolved Hide resolved
return Ok(ImportStatementsResult::InvalidImport)
}
Expand Down
4 changes: 2 additions & 2 deletions node/core/dispute-coordinator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use futures::FutureExt;

use sc_keystore::LocalKeystore;

use polkadot_node_primitives::{CandidateVotes, DISPUTE_WINDOW};
use polkadot_node_primitives::CandidateVotes;
use polkadot_node_subsystem::{
overseer, ActivatedLeaf, FromOrchestra, OverseerSignal, SpawnedSubsystem, SubsystemError,
};
Expand Down Expand Up @@ -272,7 +272,7 @@ impl DisputeCoordinatorSubsystem {
ChainScraper,
)> {
// Prune obsolete disputes:
db::v1::note_current_session(overlay_db, rolling_session_window.latest_session())?;
db::v1::note_earliest_session(overlay_db, rolling_session_window.earliest_session())?;

let active_disputes = match overlay_db.load_recent_disputes() {
Ok(Some(disputes)) =>
Expand Down
20 changes: 20 additions & 0 deletions node/subsystem-util/src/rolling_session_window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,11 @@ impl RollingSessionWindow {
self.earliest_session + (self.session_info.len() as SessionIndex).saturating_sub(1)
}

/// Returns `true` if `session_index` is contained in the window.
pub fn contains(&self, session_index: SessionIndex) -> bool {
session_index >= self.earliest_session() && session_index <= self.latest_session()
}

async fn earliest_non_finalized_block_session<Sender>(
sender: &mut Sender,
) -> Result<u32, SessionsUnavailable>
Expand Down Expand Up @@ -783,6 +788,21 @@ mod tests {
cache_session_info_test(1, 2, Some(window), 2, None);
}

#[test]
fn cache_session_window_contains() {
let window = RollingSessionWindow {
earliest_session: 10,
session_info: vec![dummy_session_info(1)],
window_size: SESSION_WINDOW_SIZE,
db_params: Some(dummy_db_params()),
};

assert!(!window.contains(0));
assert!(!window.contains(10 + SESSION_WINDOW_SIZE.get()));
assert!(!window.contains(11));
assert!(!window.contains(10 + SESSION_WINDOW_SIZE.get() - 1));
}

#[test]
fn cache_session_info_first_late() {
cache_session_info_test(
Expand Down