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

Commit

Permalink
dispute-coordinator: fix earliest session checks for pruning and impo…
Browse files Browse the repository at this point in the history
…rt (#6358)

* RollingSession: add fn contains

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* handle_import_statements fix ancient dispute check

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* work with earliest session instead of latest

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* update comment

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
  • Loading branch information
sandreim authored Nov 28, 2022
1 parent 7538b76 commit 1b1bc62
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 18 deletions.
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
8 changes: 4 additions & 4 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,8 +708,8 @@ impl Initialized {
now: Timestamp,
) -> Result<ImportStatementsResult> {
gum::trace!(target: LOG_TARGET, ?statements, "In handle import statements");
if session + DISPUTE_WINDOW.get() < self.highest_session {
// It is not valid to participate in an ancient dispute (spam?).
if !self.rolling_session_window.contains(session) {
// It is not valid to participate in an ancient dispute (spam?) or too new.
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

0 comments on commit 1b1bc62

Please sign in to comment.