From e488f529df7ae3f9672b084706775a808c430c62 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 26 Sep 2022 11:23:31 +0000 Subject: [PATCH 01/42] Impl dynamic window size. Keep sessions for unfinalized chain Signed-off-by: Andrei Sandu --- .../src/rolling_session_window.rs | 117 ++++++++++++++++-- 1 file changed, 110 insertions(+), 7 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index dd9282b50fe5..31c8bcc370bc 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -24,11 +24,13 @@ use polkadot_primitives::v2::{Hash, SessionIndex, SessionInfo}; use futures::channel::oneshot; use polkadot_node_subsystem::{ - errors::RuntimeApiError, - messages::{RuntimeApiMessage, RuntimeApiRequest}, + errors::{ChainApiError, RuntimeApiError}, + messages::{ChainApiMessage, RuntimeApiMessage, RuntimeApiRequest}, overseer, }; +const LOG_TARGET: &str = "parachain::rolling-session-window"; + /// Sessions unavailable in state to cache. #[derive(Debug, Clone, thiserror::Error)] pub enum SessionsUnavailableReason { @@ -38,9 +40,18 @@ pub enum SessionsUnavailableReason { /// The runtime API itself returned an error. #[error(transparent)] RuntimeApi(#[from] RuntimeApiError), + /// The chain API itself returned an error. + #[error(transparent)] + ChainApi(#[from] ChainApiError), /// Missing session info from runtime API for given `SessionIndex`. #[error("Missing session index {0:?}")] Missing(SessionIndex), + /// Missing last finalized block number. + #[error("Missing last finalized block number")] + MissingLastFinalizedBlock, + /// Missing last finalized block hash. + #[error("Missing last finalized block hash")] + MissingLastFinalizedBlockHash, } /// Information about the sessions being fetched. @@ -98,7 +109,8 @@ impl RollingSessionWindow { block_hash: Hash, ) -> Result where - Sender: overseer::SubsystemSender, + Sender: overseer::SubsystemSender + + overseer::SubsystemSender, { let session_index = get_session_index_for_child(&mut sender, block_hash).await?; @@ -146,6 +158,87 @@ impl RollingSessionWindow { self.earliest_session + (self.session_info.len() as SessionIndex).saturating_sub(1) } + async fn last_finalized_block_session( + &self, + sender: &mut Sender, + ) -> Result + where + Sender: overseer::SubsystemSender + + overseer::SubsystemSender, + { + let last_finalized_height = { + let (tx, rx) = oneshot::channel(); + sender.send_message(ChainApiMessage::FinalizedBlockNumber(tx)).await; + match rx.await { + Ok(Ok(number)) => number, + Ok(Err(e)) => + return Err(SessionsUnavailable { + kind: SessionsUnavailableReason::ChainApi(e), + info: None, + }), + Err(err) => { + gum::warn!( + target: LOG_TARGET, + ?err, + "Failed fetching last finalized block number" + ); + return Err(SessionsUnavailable { + kind: SessionsUnavailableReason::MissingLastFinalizedBlock, + info: None, + }) + }, + } + }; + + let (tx, rx) = oneshot::channel(); + // We want to get the parent of the last finalized block. + sender + .send_message(ChainApiMessage::FinalizedBlockHash( + last_finalized_height.saturating_sub(1), + tx, + )) + .await; + let last_finalized_hash_parent = match rx.await { + Ok(Ok(maybe_hash)) => maybe_hash, + Ok(Err(e)) => + return Err(SessionsUnavailable { + kind: SessionsUnavailableReason::ChainApi(e), + info: None, + }), + Err(err) => { + gum::warn!(target: LOG_TARGET, ?err, "Failed fetching last finalized block hash"); + return Err(SessionsUnavailable { + kind: SessionsUnavailableReason::MissingLastFinalizedBlockHash, + info: None, + }) + }, + }; + + // Get the session in which the last finalized block was authored. + if let Some(last_finalized_hash_parent) = last_finalized_hash_parent { + let session = + match get_session_index_for_child(sender, last_finalized_hash_parent).await { + Ok(session_index) => session_index, + Err(err) => { + gum::warn!( + target: LOG_TARGET, + ?err, + ?last_finalized_hash_parent, + "Failed fetching session index" + ); + return Err(err) + }, + }; + + Ok(session) + } else { + return Err(SessionsUnavailable { + kind: SessionsUnavailableReason::MissingLastFinalizedBlockHash, + info: None, + }) + } + } + /// When inspecting a new import notification, updates the session info cache to match /// the session of the imported block's child. /// @@ -153,12 +246,17 @@ impl RollingSessionWindow { /// not change often and import notifications are expected to be typically increasing in session number. /// /// some backwards drift in session index is acceptable. - pub async fn cache_session_info_for_head( + pub async fn cache_session_info_for_head( &mut self, - sender: &mut impl overseer::SubsystemSender, + sender: &mut Sender, block_hash: Hash, - ) -> Result { + ) -> Result + where + Sender: overseer::SubsystemSender + + overseer::SubsystemSender, + { let session_index = get_session_index_for_child(sender, block_hash).await?; + let last_finalized_block_session = self.last_finalized_block_session(sender).await?; let old_window_start = self.earliest_session; @@ -171,7 +269,12 @@ impl RollingSessionWindow { let old_window_end = latest; - let window_start = session_index.saturating_sub(self.window_size.get() - 1); + // Ensure we keep sessions up to last finalized block by adjusting the window start. + // This will increase the session window to cover the full unfinalized chain. + let window_start = std::cmp::min( + session_index.saturating_sub(self.window_size.get() - 1), + last_finalized_block_session, + ); // keep some of the old window, if applicable. let overlap_start = window_start.saturating_sub(old_window_start); From 5780cc96f37171e10db59001813770a9192099cc Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 26 Sep 2022 15:30:25 +0000 Subject: [PATCH 02/42] feedback Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 31c8bcc370bc..06951fe3a216 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -158,7 +158,7 @@ impl RollingSessionWindow { self.earliest_session + (self.session_info.len() as SessionIndex).saturating_sub(1) } - async fn last_finalized_block_session( + async fn earliest_non_finalized_block_session( &self, sender: &mut Sender, ) -> Result @@ -193,10 +193,7 @@ impl RollingSessionWindow { let (tx, rx) = oneshot::channel(); // We want to get the parent of the last finalized block. sender - .send_message(ChainApiMessage::FinalizedBlockHash( - last_finalized_height.saturating_sub(1), - tx, - )) + .send_message(ChainApiMessage::FinalizedBlockHash(last_finalized_height, tx)) .await; let last_finalized_hash_parent = match rx.await { Ok(Ok(maybe_hash)) => maybe_hash, @@ -256,7 +253,8 @@ impl RollingSessionWindow { + overseer::SubsystemSender, { let session_index = get_session_index_for_child(sender, block_hash).await?; - let last_finalized_block_session = self.last_finalized_block_session(sender).await?; + let earliest_non_finalized_block_session = + self.earliest_non_finalized_block_session(sender).await?; let old_window_start = self.earliest_session; @@ -273,7 +271,7 @@ impl RollingSessionWindow { // This will increase the session window to cover the full unfinalized chain. let window_start = std::cmp::min( session_index.saturating_sub(self.window_size.get() - 1), - last_finalized_block_session, + earliest_non_finalized_block_session, ); // keep some of the old window, if applicable. From 2132d9c8d34b2781dd4cc925fca88eaa9a19de8c Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 27 Sep 2022 08:00:02 +0000 Subject: [PATCH 03/42] Stretch also in contructor plus tests Signed-off-by: Andrei Sandu --- .../src/rolling_session_window.rs | 222 +++++++++++++++++- 1 file changed, 219 insertions(+), 3 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 06951fe3a216..17b8a24739f9 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -113,8 +113,14 @@ impl RollingSessionWindow { + overseer::SubsystemSender, { let session_index = get_session_index_for_child(&mut sender, block_hash).await?; + let earliest_non_finalized_block_session = + Self::earliest_non_finalized_block_session(&mut sender).await?; - let window_start = session_index.saturating_sub(window_size.get() - 1); + // This will increase the session window to cover the full unfinalized chain. + let window_start = std::cmp::min( + session_index.saturating_sub(window_size.get() - 1), + earliest_non_finalized_block_session, + ); match load_all_sessions(&mut sender, block_hash, window_start, session_index).await { Err(kind) => Err(SessionsUnavailable { @@ -159,7 +165,6 @@ impl RollingSessionWindow { } async fn earliest_non_finalized_block_session( - &self, sender: &mut Sender, ) -> Result where @@ -254,7 +259,7 @@ impl RollingSessionWindow { { let session_index = get_session_index_for_child(sender, block_hash).await?; let earliest_non_finalized_block_session = - self.earliest_non_finalized_block_session(sender).await?; + Self::earliest_non_finalized_block_session(sender).await?; let old_window_start = self.earliest_session; @@ -420,6 +425,14 @@ mod tests { parent_hash: Default::default(), }; + let finalized_header = Header { + digest: Default::default(), + extrinsics_root: Default::default(), + number: 0, + state_root: Default::default(), + parent_hash: Default::default(), + }; + let pool = TaskExecutor::new(); let (mut ctx, mut handle) = make_subsystem_context::(pool.clone()); @@ -459,6 +472,37 @@ mod tests { } ); + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(finalized_header.number)); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + block_number, + s_tx, + )) => { + assert_eq!(block_number, finalized_header.number); + let _ = s_tx.send(Ok(Some(finalized_header.hash()))); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, finalized_header.hash()); + let _ = s_tx.send(Ok(session)); + } + ); + for i in expect_requests_from..=session { assert_matches!( handle.recv().await, @@ -586,6 +630,108 @@ mod tests { cache_session_info_test(0, 3, Some(window), 2); } + #[test] + fn any_session_stretch_for_unfinalized_chain() { + // Session index of the tip of our fake test chain. + let session: SessionIndex = 100; + let genesis_session: SessionIndex = 0; + + let header = Header { + digest: Default::default(), + extrinsics_root: Default::default(), + number: 5, + state_root: Default::default(), + parent_hash: Default::default(), + }; + + let finalized_header = Header { + digest: Default::default(), + extrinsics_root: Default::default(), + number: 0, + state_root: Default::default(), + parent_hash: Default::default(), + }; + + let pool = TaskExecutor::new(); + let (mut ctx, mut handle) = make_subsystem_context::<(), _>(pool.clone()); + + let hash = header.hash(); + + let test_fut = { + let sender = ctx.sender().clone(); + Box::pin(async move { + let res = RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash).await; + assert!(res.is_err()); + }) + }; + + let aux_fut = Box::pin(async move { + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, hash); + let _ = s_tx.send(Ok(session)); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(finalized_header.number)); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + block_number, + s_tx, + )) => { + assert_eq!(block_number, finalized_header.number); + let _ = s_tx.send(Ok(Some(finalized_header.hash()))); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, finalized_header.hash()); + let _ = s_tx.send(Ok(0)); + } + ); + + // Unfinalized chain starts at geneisis block, so session 0 is how far we stretch. + for i in genesis_session..=session { + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionInfo(j, s_tx), + )) => { + assert_eq!(h, hash); + assert_eq!(i, j); + + let _ = s_tx.send(Ok(if i == session { + None + } else { + Some(dummy_session_info(i)) + })); + } + ); + } + }); + + futures::executor::block_on(futures::future::join(test_fut, aux_fut)); + } + #[test] fn any_session_unavailable_for_caching_means_no_change() { let session: SessionIndex = 6; @@ -599,6 +745,14 @@ mod tests { parent_hash: Default::default(), }; + let finalized_header = Header { + digest: Default::default(), + extrinsics_root: Default::default(), + number: 0, + state_root: Default::default(), + parent_hash: Default::default(), + }; + let pool = TaskExecutor::new(); let (mut ctx, mut handle) = make_subsystem_context::<(), _>(pool.clone()); @@ -624,6 +778,37 @@ mod tests { } ); + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(finalized_header.number)); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + block_number, + s_tx, + )) => { + assert_eq!(block_number, finalized_header.number); + let _ = s_tx.send(Ok(Some(finalized_header.hash()))); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, finalized_header.hash()); + let _ = s_tx.send(Ok(session)); + } + ); + for i in start_session..=session { assert_matches!( handle.recv().await, @@ -687,6 +872,37 @@ mod tests { } ); + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(header.number)); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + block_number, + s_tx, + )) => { + assert_eq!(block_number, header.number); + let _ = s_tx.send(Ok(Some(header.hash()))); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, header.hash()); + let _ = s_tx.send(Ok(session)); + } + ); + assert_matches!( handle.recv().await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( From eb0695940a654c3587c1184342499b58bf773453 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 27 Sep 2022 08:06:43 +0000 Subject: [PATCH 04/42] review feedback Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 17b8a24739f9..0ff2dc6deb13 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -20,7 +20,7 @@ //! care about the state of particular blocks. pub use polkadot_node_primitives::{new_session_window_size, SessionWindowSize}; -use polkadot_primitives::v2::{Hash, SessionIndex, SessionInfo}; +use polkadot_primitives::v2::{BlockNumber, Hash, SessionIndex, SessionInfo}; use futures::channel::oneshot; use polkadot_node_subsystem::{ @@ -51,7 +51,7 @@ pub enum SessionsUnavailableReason { MissingLastFinalizedBlock, /// Missing last finalized block hash. #[error("Missing last finalized block hash")] - MissingLastFinalizedBlockHash, + MissingLastFinalizedBlockHash(BlockNumber), } /// Information about the sessions being fetched. @@ -196,7 +196,7 @@ impl RollingSessionWindow { }; let (tx, rx) = oneshot::channel(); - // We want to get the parent of the last finalized block. + // We want to get the session index for the child of the last finalized block. sender .send_message(ChainApiMessage::FinalizedBlockHash(last_finalized_height, tx)) .await; @@ -210,7 +210,9 @@ impl RollingSessionWindow { Err(err) => { gum::warn!(target: LOG_TARGET, ?err, "Failed fetching last finalized block hash"); return Err(SessionsUnavailable { - kind: SessionsUnavailableReason::MissingLastFinalizedBlockHash, + kind: SessionsUnavailableReason::MissingLastFinalizedBlockHash( + last_finalized_height, + ), info: None, }) }, @@ -235,7 +237,9 @@ impl RollingSessionWindow { Ok(session) } else { return Err(SessionsUnavailable { - kind: SessionsUnavailableReason::MissingLastFinalizedBlockHash, + kind: SessionsUnavailableReason::MissingLastFinalizedBlockHash( + last_finalized_height, + ), info: None, }) } From 88ac66f8255371e06d763964dbf7356fdde29cf8 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 27 Sep 2022 08:14:40 +0000 Subject: [PATCH 05/42] fix approval-voting tests Signed-off-by: Andrei Sandu --- node/core/approval-voting/src/import.rs | 32 +++++++++++++++++++++++++ node/core/approval-voting/src/tests.rs | 31 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/node/core/approval-voting/src/import.rs b/node/core/approval-voting/src/import.rs index d43bf40546ae..5413c271e0d6 100644 --- a/node/core/approval-voting/src/import.rs +++ b/node/core/approval-voting/src/import.rs @@ -1296,6 +1296,38 @@ pub(crate) mod tests { } ); + // Caching of sesssions needs sessoion of first unfinalied block. + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(header.number)); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + block_number, + s_tx, + )) => { + assert_eq!(block_number, header.number); + let _ = s_tx.send(Ok(Some(header.hash()))); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, header.hash()); + let _ = s_tx.send(Ok(session)); + } + ); + // determine_new_blocks exits early as the parent_hash is in the DB assert_matches!( diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs index 66d1402ed6dc..bdb7a8c929b3 100644 --- a/node/core/approval-voting/src/tests.rs +++ b/node/core/approval-voting/src/tests.rs @@ -807,6 +807,37 @@ async fn import_block( } ); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(number)); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + block_number, + s_tx, + )) => { + assert_eq!(block_number, number); + let _ = s_tx.send(Ok(Some(hashes[number as usize].0))); + } + ); + + assert_matches!( + overseer_recv(overseer).await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, hashes[number as usize].0); + let _ = s_tx.send(Ok(number.into())); + } + ); + if !fork { assert_matches!( overseer_recv(overseer).await, From a7d97d085ff81ad1bea7bf2d7caf480564c35938 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 27 Sep 2022 08:44:14 +0000 Subject: [PATCH 06/42] grunting: dispute coordinator tests Signed-off-by: Andrei Sandu --- node/core/dispute-coordinator/src/tests.rs | 41 ++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/node/core/dispute-coordinator/src/tests.rs b/node/core/dispute-coordinator/src/tests.rs index ff85319599ce..aaef00999259 100644 --- a/node/core/dispute-coordinator/src/tests.rs +++ b/node/core/dispute-coordinator/src/tests.rs @@ -239,13 +239,15 @@ impl TestState { ))) .await; - self.handle_sync_queries(virtual_overseer, block_hash, session).await; + self.handle_sync_queries(virtual_overseer, block_hash, block_number, session) + .await; } async fn handle_sync_queries( &mut self, virtual_overseer: &mut VirtualOverseer, block_hash: Hash, + block_number: BlockNumber, session: SessionIndex, ) { // Order of messages is not fixed (different on initializing): @@ -278,11 +280,45 @@ impl TestState { finished_steps.got_session_information = true; assert_eq!(h, block_hash); let _ = tx.send(Ok(session)); + + // Queries for fetching earliest unfinalized block session. See `RollingSessionWindow`. + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(block_number)); + } + ); + + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + number, + s_tx, + )) => { + assert_eq!(block_number, number); + let _ = s_tx.send(Ok(Some(block_hash))); + } + ); + + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, block_hash); + let _ = s_tx.send(Ok(session)); + } + ); + // No queries, if subsystem knows about this session already. if self.known_session == Some(session) { continue } self.known_session = Some(session); + loop { // answer session info queries until the current session is reached. assert_matches!( @@ -361,7 +397,8 @@ impl TestState { ))) .await; - self.handle_sync_queries(virtual_overseer, *leaf, session).await; + self.handle_sync_queries(virtual_overseer, *leaf, n as BlockNumber, session) + .await; } } From 3e4d9d58b6109458b628ebc4f0692cb9a7ce4263 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 3 Oct 2022 15:08:27 +0000 Subject: [PATCH 07/42] add session window column Signed-off-by: Andrei Sandu --- node/service/src/parachains_db/mod.rs | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/node/service/src/parachains_db/mod.rs b/node/service/src/parachains_db/mod.rs index de12a8ac1a32..de1919c0a0b4 100644 --- a/node/service/src/parachains_db/mod.rs +++ b/node/service/src/parachains_db/mod.rs @@ -30,15 +30,20 @@ pub(crate) mod columns { } pub mod v1 { - pub const NUM_COLUMNS: u32 = 5; + pub const NUM_COLUMNS: u32 = 6; pub const COL_AVAILABILITY_DATA: u32 = 0; pub const COL_AVAILABILITY_META: u32 = 1; pub const COL_APPROVAL_DATA: u32 = 2; pub const COL_CHAIN_SELECTION_DATA: u32 = 3; pub const COL_DISPUTE_COORDINATOR_DATA: u32 = 4; - pub const ORDERED_COL: &[u32] = - &[COL_AVAILABILITY_META, COL_CHAIN_SELECTION_DATA, COL_DISPUTE_COORDINATOR_DATA]; + pub const ORDERED_COL: &[u32] = &[ + COL_AVAILABILITY_META, + COL_CHAIN_SELECTION_DATA, + COL_DISPUTE_COORDINATOR_DATA, + COL_SESSION_WINDOW_DATA, + ]; + pub const COL_SESSION_WINDOW_DATA: u32 = 5; } } @@ -56,6 +61,8 @@ pub struct ColumnsConfig { pub col_chain_selection_data: u32, /// The column used by dispute coordinator for data. pub col_dispute_coordinator_data: u32, + /// The column used for session window data. + pub col_session_window_data: u32, } /// The real columns used by the parachains DB. @@ -66,6 +73,7 @@ pub const REAL_COLUMNS: ColumnsConfig = ColumnsConfig { col_approval_data: columns::v1::COL_APPROVAL_DATA, col_chain_selection_data: columns::v1::COL_CHAIN_SELECTION_DATA, col_dispute_coordinator_data: columns::v1::COL_DISPUTE_COORDINATOR_DATA, + col_session_window_data: columns::v1::COL_SESSION_WINDOW_DATA, }; #[derive(PartialEq)] @@ -83,11 +91,18 @@ pub struct CacheSizes { pub availability_meta: usize, /// Cache used by approval data. pub approval_data: usize, + /// Cache used by session window data + pub session_data: usize, } impl Default for CacheSizes { fn default() -> Self { - CacheSizes { availability_data: 25, availability_meta: 1, approval_data: 5 } + CacheSizes { + availability_data: 25, + availability_meta: 1, + approval_data: 5, + session_data: 1, + } } } @@ -117,6 +132,9 @@ pub fn open_creating_rocksdb( let _ = db_config .memory_budget .insert(columns::v1::COL_APPROVAL_DATA, cache_sizes.approval_data); + let _ = db_config + .memory_budget + .insert(columns::v1::COL_SESSION_WINDOW_DATA, cache_sizes.session_data); let path_str = path .to_str() From 1732baa52265e5fdfb02671bee5ac162380a1223 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 4 Oct 2022 16:00:56 +0000 Subject: [PATCH 08/42] integrate approval vote and fix tests Signed-off-by: Andrei Sandu --- .../approval-voting/src/approval_db/v1/mod.rs | 2 + .../src/approval_db/v1/tests.rs | 6 +- node/core/approval-voting/src/import.rs | 11 +- node/core/approval-voting/src/lib.rs | 38 +++- node/core/approval-voting/src/tests.rs | 9 +- .../src/rolling_session_window.rs | 166 ++++++++++++++++-- 6 files changed, 203 insertions(+), 29 deletions(-) diff --git a/node/core/approval-voting/src/approval_db/v1/mod.rs b/node/core/approval-voting/src/approval_db/v1/mod.rs index 03b7aa68f134..0af76d28c362 100644 --- a/node/core/approval-voting/src/approval_db/v1/mod.rs +++ b/node/core/approval-voting/src/approval_db/v1/mod.rs @@ -150,6 +150,8 @@ pub type Bitfield = BitVec; pub struct Config { /// The column family in the database where data is stored. pub col_data: u32, + /// The column of the database where rolling session window data is stored. + pub col_session_data: u32, } /// Details pertaining to our assignment on a block. diff --git a/node/core/approval-voting/src/approval_db/v1/tests.rs b/node/core/approval-voting/src/approval_db/v1/tests.rs index 548c64bcef03..d939b3cf5a05 100644 --- a/node/core/approval-voting/src/approval_db/v1/tests.rs +++ b/node/core/approval-voting/src/approval_db/v1/tests.rs @@ -28,9 +28,11 @@ use std::{collections::HashMap, sync::Arc}; use ::test_helpers::{dummy_candidate_receipt, dummy_candidate_receipt_bad_sig, dummy_hash}; const DATA_COL: u32 = 0; -const NUM_COLUMNS: u32 = 1; +const SESSION_DATA_COL: u32 = 1; -const TEST_CONFIG: Config = Config { col_data: DATA_COL }; +const NUM_COLUMNS: u32 = 2; + +const TEST_CONFIG: Config = Config { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; fn make_db() -> (DbBackend, Arc) { let db = kvdb_memorydb::create(NUM_COLUMNS); diff --git a/node/core/approval-voting/src/import.rs b/node/core/approval-voting/src/import.rs index 5413c271e0d6..bfa69c5eff09 100644 --- a/node/core/approval-voting/src/import.rs +++ b/node/core/approval-voting/src/import.rs @@ -635,9 +635,11 @@ pub(crate) mod tests { }; const DATA_COL: u32 = 0; - const NUM_COLUMNS: u32 = 1; + const SESSION_DATA_COL: u32 = 0; - const TEST_CONFIG: DatabaseConfig = DatabaseConfig { col_data: DATA_COL }; + const NUM_COLUMNS: u32 = 2; + + const TEST_CONFIG: DatabaseConfig = DatabaseConfig { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; #[derive(Default)] struct MockClock; @@ -652,12 +654,17 @@ pub(crate) mod tests { } fn blank_state() -> State { + let db = kvdb_memorydb::create(NUM_COLUMNS); + let db = polkadot_node_subsystem_util::database::kvdb_impl::DbAdapter::new(db, &[]); + let db: Arc = Arc::new(db); State { session_window: None, keystore: Arc::new(LocalKeystore::in_memory()), slot_duration_millis: 6_000, clock: Box::new(MockClock::default()), assignment_criteria: Box::new(MockAssignmentCriteria), + db, + db_config: TEST_CONFIG, } } diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index 467d8be612e9..9d814c2cf6c4 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -44,8 +44,8 @@ use polkadot_node_subsystem_util::{ database::Database, metrics::{self, prometheus}, rolling_session_window::{ - new_session_window_size, RollingSessionWindow, SessionWindowSize, SessionWindowUpdate, - SessionsUnavailable, + new_session_window_size, DatabaseParams, RollingSessionWindow, SessionWindowSize, + SessionWindowUpdate, SessionsUnavailable, }, TimeoutExt, }; @@ -119,6 +119,8 @@ const LOG_TARGET: &str = "parachain::approval-voting"; pub struct Config { /// The column family in the DB where approval-voting data is stored. pub col_data: u32, + /// The of the DB where rolling session info is stored. + pub col_session_data: u32, /// The slot duration of the consensus algorithm, in milliseconds. Should be evenly /// divisible by 500. pub slot_duration_millis: u64, @@ -358,7 +360,10 @@ impl ApprovalVotingSubsystem { keystore, slot_duration_millis: config.slot_duration_millis, db, - db_config: DatabaseConfig { col_data: config.col_data }, + db_config: DatabaseConfig { + col_data: config.col_data, + col_session_data: config.col_session_data, + }, mode: Mode::Syncing(sync_oracle), metrics, } @@ -367,7 +372,10 @@ impl ApprovalVotingSubsystem { /// Revert to the block corresponding to the specified `hash`. /// The operation is not allowed for blocks older than the last finalized one. pub fn revert_to(&self, hash: Hash) -> Result<(), SubsystemError> { - let config = approval_db::v1::Config { col_data: self.db_config.col_data }; + let config = approval_db::v1::Config { + col_data: self.db_config.col_data, + col_session_data: self.db_config.col_session_data, + }; let mut backend = approval_db::v1::DbBackend::new(self.db.clone(), config); let mut overlay = OverlayedBackend::new(&backend); @@ -615,6 +623,9 @@ struct State { slot_duration_millis: u64, clock: Box, assignment_criteria: Box, + // Require for `RollingSessionWindow`. + db_config: DatabaseConfig, + db: Arc, } #[overseer::contextbounds(ApprovalVoting, prefix = self::overseer)] @@ -636,8 +647,18 @@ impl State { match session_window { None => { let sender = ctx.sender().clone(); - self.session_window = - Some(RollingSessionWindow::new(sender, APPROVAL_SESSIONS, head).await?); + self.session_window = Some( + RollingSessionWindow::new( + sender, + APPROVAL_SESSIONS, + head, + DatabaseParams { + db: self.db.clone(), + db_column: self.db_config.col_session_data, + }, + ) + .await?, + ); Ok(None) }, Some(mut session_window) => { @@ -732,12 +753,17 @@ async fn run( where B: Backend, { + let db = subsystem.db.clone(); + let db_config = subsystem.db_config.clone(); + let mut state = State { session_window: None, keystore: subsystem.keystore, slot_duration_millis: subsystem.slot_duration_millis, clock, assignment_criteria, + db_config, + db, }; let mut wakeups = Wakeups::default(); diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs index bdb7a8c929b3..b2e5a4724482 100644 --- a/node/core/approval-voting/src/tests.rs +++ b/node/core/approval-voting/src/tests.rs @@ -14,6 +14,8 @@ // You should have received a copy of the GNU General Public License // along with Polkadot. If not, see . +use crate::tests::test_constants::TEST_CONFIG; + use super::*; use polkadot_node_primitives::{ approval::{ @@ -111,9 +113,11 @@ fn make_sync_oracle(val: bool) -> (Box, TestSyncOracleHan pub mod test_constants { use crate::approval_db::v1::Config as DatabaseConfig; const DATA_COL: u32 = 0; - pub(crate) const NUM_COLUMNS: u32 = 1; + const SESSION_DATA_COL: u32 = 1; + + pub(crate) const NUM_COLUMNS: u32 = 2; - pub(crate) const TEST_CONFIG: DatabaseConfig = DatabaseConfig { col_data: DATA_COL }; + pub(crate) const TEST_CONFIG: DatabaseConfig = DatabaseConfig { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; } struct MockSupportsParachains; @@ -489,6 +493,7 @@ fn test_harness>( Config { col_data: test_constants::TEST_CONFIG.col_data, slot_duration_millis: SLOT_DURATION_MILLIS, + col_session_data: TEST_CONFIG.col_session_data, }, Arc::new(db), Arc::new(keystore), diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 0ff2dc6deb13..47e314ee1d5a 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -19,8 +19,13 @@ //! This is useful for consensus components which need to stay up-to-date about recent sessions but don't //! care about the state of particular blocks. +use super::database::{DBTransaction, Database}; +use kvdb::{DBKey, DBOp}; + +use parity_scale_codec::{Decode, Encode}; pub use polkadot_node_primitives::{new_session_window_size, SessionWindowSize}; use polkadot_primitives::v2::{BlockNumber, Hash, SessionIndex, SessionInfo}; +use std::sync::Arc; use futures::channel::oneshot; use polkadot_node_subsystem::{ @@ -30,6 +35,7 @@ use polkadot_node_subsystem::{ }; const LOG_TARGET: &str = "parachain::rolling-session-window"; +const STORED_ROLLING_SESSION_WINDOW: &[u8] = b"Rolling_session_window"; /// Sessions unavailable in state to cache. #[derive(Debug, Clone, thiserror::Error)] @@ -94,55 +100,174 @@ pub enum SessionWindowUpdate { Unchanged, } +/// A structure to store rolling session database parameters. +#[derive(Clone)] +pub struct DatabaseParams { + /// Databae reference. + pub db: Arc, + /// The column which stores the rolling session info. + pub db_column: u32, +} /// A rolling window of sessions and cached session info. pub struct RollingSessionWindow { earliest_session: SessionIndex, session_info: Vec, window_size: SessionWindowSize, + // These params just to enable some approval-voting tests to force feed sessions + // in the window. + db_params: Option, +} + +/// The rolling session data we persist in the database. +#[derive(Encode, Decode, Default)] +struct StoredWindow { + earliest_session: SessionIndex, + session_info: Vec, } impl RollingSessionWindow { /// Initialize a new session info cache with the given window size. + /// Invariant: The database always contains the earliest session. Then, + /// we can always extend the session info vector using chain state. pub async fn new( mut sender: Sender, window_size: SessionWindowSize, block_hash: Hash, + db_params: DatabaseParams, ) -> Result where Sender: overseer::SubsystemSender + overseer::SubsystemSender, { + let maybe_stored_window = Self::db_load(db_params.clone()); + let session_index = get_session_index_for_child(&mut sender, block_hash).await?; let earliest_non_finalized_block_session = Self::earliest_non_finalized_block_session(&mut sender).await?; - // This will increase the session window to cover the full unfinalized chain. - let window_start = std::cmp::min( - session_index.saturating_sub(window_size.get() - 1), - earliest_non_finalized_block_session, - ); + let (window_start, mut stored_sessions) = + if let Some(mut stored_window) = maybe_stored_window { + // Check if DB is ancient. + if earliest_non_finalized_block_session > + stored_window.earliest_session + stored_window.session_info.len() as u32 + { + // If ancient, we scrap it and fetch from chain state. + stored_window.session_info.clear(); + } - match load_all_sessions(&mut sender, block_hash, window_start, session_index).await { - Err(kind) => Err(SessionsUnavailable { - kind, - info: Some(SessionsUnavailableInfo { - window_start, - window_end: session_index, - block_hash, + // The session window might extend beyond the last finalized block, but that's fine as we'll prune it at + // next update. + let window_start = if stored_window.session_info.len() > 0 { + // If there is at least one entry in db, we always take the DB as source of truth. + stored_window.earliest_session + } else { + // This will increase the session window to cover the full unfinalized chain. + std::cmp::min( + session_index.saturating_sub(window_size.get() - 1), + earliest_non_finalized_block_session, + ) + }; + + (window_start, stored_window.session_info) + } else { + ( + std::cmp::min( + session_index.saturating_sub(window_size.get() - 1), + earliest_non_finalized_block_session, + ), + Vec::new(), + ) + }; + + // Try to load sessions from DB first and load more from chain state if needed. + let sessions_missing_count = session_index + .saturating_sub(window_start) + .saturating_add(1) + .saturating_sub(stored_sessions.len() as u32); + + // Load remaining from chain state. + let mut chain_sessions = if sessions_missing_count > 0 { + match load_all_sessions_from_chain_state( + &mut sender, + block_hash, + window_start, + session_index, + ) + .await + { + Err(kind) => Err(SessionsUnavailable { + kind, + info: Some(SessionsUnavailableInfo { + window_start, + window_end: session_index, + block_hash, + }), }), - }), - Ok(s) => Ok(Self { earliest_session: window_start, session_info: s, window_size }), + Ok(sessions) => Ok(sessions), + }? + } else { + // There are no new sessions to be fetched from chain state. + Vec::new() + }; + + stored_sessions.append(&mut chain_sessions); + + Ok(Self { + earliest_session: window_start, + session_info: stored_sessions, + window_size, + db_params: Some(db_params), + }) + } + + // Load session information from the parachains db. + fn db_load(db_params: DatabaseParams) -> Option { + match db_params.db.get(db_params.db_column, STORED_ROLLING_SESSION_WINDOW).ok()? { + None => None, + Some(raw) => { + let maybe_decoded = StoredWindow::decode(&mut &raw[..]).map(Some); + match maybe_decoded { + Ok(decoded) => decoded, + Err(err) => { + gum::warn!( + target: LOG_TARGET, + ?err, + "Failed decoding db entry; will start with onchain session infos and self-heal DB entry on next update." + ); + None + }, + } + }, + } + } + + // Saves/Updates all sessions in the database. + fn db_save(&mut self, stored_window: StoredWindow) { + if let Some(db_params) = self.db_params.as_ref() { + match db_params.db.write(DBTransaction { + ops: vec![DBOp::Insert { + col: db_params.db_column, + key: DBKey::from_slice(STORED_ROLLING_SESSION_WINDOW), + value: stored_window.encode(), + }], + }) { + Ok(_) => {}, + Err(err) => { + gum::warn!(target: LOG_TARGET, ?err, "Failed writing db entry"); + }, + } } } /// Initialize a new session info cache with the given window size and /// initial data. + /// This is only used in `approval voting` tests. pub fn with_session_info( window_size: SessionWindowSize, earliest_session: SessionIndex, session_info: Vec, ) -> Self { - RollingSessionWindow { earliest_session, session_info, window_size } + RollingSessionWindow { earliest_session, session_info, window_size, db_params: None } } /// Access the session info for the given session index, if stored within the window. @@ -288,7 +413,9 @@ impl RollingSessionWindow { let fresh_start = if latest < window_start { window_start } else { latest + 1 }; - match load_all_sessions(sender, block_hash, fresh_start, session_index).await { + match load_all_sessions_from_chain_state(sender, block_hash, fresh_start, session_index) + .await + { Err(kind) => Err(SessionsUnavailable { kind, info: Some(SessionsUnavailableInfo { @@ -308,12 +435,17 @@ impl RollingSessionWindow { let outdated = std::cmp::min(overlap_start as usize, self.session_info.len()); self.session_info.drain(..outdated); self.session_info.extend(s); + // we need to account for this case: // window_start ................................... session_index // old_window_start ........... latest let new_earliest = std::cmp::max(window_start, old_window_start); self.earliest_session = new_earliest; + self.db_save(StoredWindow { + earliest_session: self.earliest_session, + session_info: self.session_info.clone(), + }); Ok(update) }, } @@ -354,7 +486,7 @@ async fn get_session_index_for_child( } } -async fn load_all_sessions( +async fn load_all_sessions_from_chain_state( sender: &mut impl overseer::SubsystemSender, block_hash: Hash, start: SessionIndex, From c9045cd1d02a793c63c859f73b55e4ac855395c5 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 4 Oct 2022 16:14:51 +0000 Subject: [PATCH 09/42] fix rolling session tests Signed-off-by: Andrei Sandu --- Cargo.lock | 1 + node/core/approval-voting/src/import.rs | 5 ++- node/core/approval-voting/src/tests.rs | 3 +- node/subsystem-util/Cargo.toml | 1 + .../src/rolling_session_window.rs | 39 ++++++++++++++++--- 5 files changed, 40 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 99033d5da5da..d365a54d97ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6908,6 +6908,7 @@ dependencies = [ "futures", "itertools", "kvdb", + "kvdb-memorydb", "kvdb-shared-tests", "lazy_static", "log", diff --git a/node/core/approval-voting/src/import.rs b/node/core/approval-voting/src/import.rs index bfa69c5eff09..e98cea0c4db4 100644 --- a/node/core/approval-voting/src/import.rs +++ b/node/core/approval-voting/src/import.rs @@ -635,11 +635,12 @@ pub(crate) mod tests { }; const DATA_COL: u32 = 0; - const SESSION_DATA_COL: u32 = 0; + const SESSION_DATA_COL: u32 = 1; const NUM_COLUMNS: u32 = 2; - const TEST_CONFIG: DatabaseConfig = DatabaseConfig { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; + const TEST_CONFIG: DatabaseConfig = + DatabaseConfig { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; #[derive(Default)] struct MockClock; diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs index b2e5a4724482..2b72e235c9bc 100644 --- a/node/core/approval-voting/src/tests.rs +++ b/node/core/approval-voting/src/tests.rs @@ -117,7 +117,8 @@ pub mod test_constants { pub(crate) const NUM_COLUMNS: u32 = 2; - pub(crate) const TEST_CONFIG: DatabaseConfig = DatabaseConfig { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; + pub(crate) const TEST_CONFIG: DatabaseConfig = + DatabaseConfig { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; } struct MockSupportsParachains; diff --git a/node/subsystem-util/Cargo.toml b/node/subsystem-util/Cargo.toml index 6f120beec7cb..41e84e58efad 100644 --- a/node/subsystem-util/Cargo.toml +++ b/node/subsystem-util/Cargo.toml @@ -46,3 +46,4 @@ lazy_static = "1.4.0" polkadot-primitives-test-helpers = { path = "../../primitives/test-helpers" } kvdb-shared-tests = "0.9.0" tempfile = "3.1.0" +kvdb-memorydb = "0.11.0" diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 47e314ee1d5a..47719b5fc1c0 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -518,6 +518,7 @@ async fn load_all_sessions_from_chain_state( #[cfg(test)] mod tests { use super::*; + use crate::database::kvdb_impl::DbAdapter; use assert_matches::assert_matches; use polkadot_node_subsystem::{ messages::{AllMessages, AvailabilityRecoveryMessage}, @@ -528,7 +529,16 @@ mod tests { use sp_core::testing::TaskExecutor; pub const TEST_WINDOW_SIZE: SessionWindowSize = new_session_window_size!(6); + const SESSION_DATA_COL: u32 = 0; + const NUM_COLUMNS: u32 = 1; + + fn dummy_db_params() -> DatabaseParams { + let db = kvdb_memorydb::create(NUM_COLUMNS); + let db = DbAdapter::new(db, &[]); + let db: Arc = Arc::new(db); + DatabaseParams { db, db_column: SESSION_DATA_COL } + } fn dummy_session_info(index: SessionIndex) -> SessionInfo { SessionInfo { validators: Vec::new(), @@ -580,9 +590,14 @@ mod tests { let test_fut = { Box::pin(async move { let window = match window { - None => RollingSessionWindow::new(sender.clone(), TEST_WINDOW_SIZE, hash) - .await - .unwrap(), + None => RollingSessionWindow::new( + sender.clone(), + TEST_WINDOW_SIZE, + hash, + dummy_db_params(), + ) + .await + .unwrap(), Some(mut window) => { window.cache_session_info_for_head(sender, hash).await.unwrap(); window @@ -668,6 +683,7 @@ mod tests { earliest_session: 1, session_info: vec![dummy_session_info(1)], window_size: TEST_WINDOW_SIZE, + db_params: Some(dummy_db_params()), }; cache_session_info_test(1, 2, Some(window), 2); @@ -693,6 +709,7 @@ mod tests { dummy_session_info(52), ], window_size: TEST_WINDOW_SIZE, + db_params: Some(dummy_db_params()), }; cache_session_info_test( @@ -710,6 +727,7 @@ mod tests { earliest_session: start, session_info: (start..=99).map(dummy_session_info).collect(), window_size: TEST_WINDOW_SIZE, + db_params: Some(dummy_db_params()), }; cache_session_info_test( @@ -727,6 +745,7 @@ mod tests { earliest_session: start, session_info: (start..=97).map(dummy_session_info).collect(), window_size: TEST_WINDOW_SIZE, + db_params: Some(dummy_db_params()), }; cache_session_info_test( @@ -744,6 +763,7 @@ mod tests { earliest_session: start, session_info: (0..=1).map(dummy_session_info).collect(), window_size: TEST_WINDOW_SIZE, + db_params: Some(dummy_db_params()), }; cache_session_info_test( @@ -761,6 +781,7 @@ mod tests { earliest_session: start, session_info: (0..=1).map(dummy_session_info).collect(), window_size: TEST_WINDOW_SIZE, + db_params: Some(dummy_db_params()), }; cache_session_info_test(0, 3, Some(window), 2); @@ -796,7 +817,9 @@ mod tests { let test_fut = { let sender = ctx.sender().clone(); Box::pin(async move { - let res = RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash).await; + let res = + RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash, dummy_db_params()) + .await; assert!(res.is_err()); }) }; @@ -897,7 +920,9 @@ mod tests { let test_fut = { let sender = ctx.sender().clone(); Box::pin(async move { - let res = RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash).await; + let res = + RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash, dummy_db_params()) + .await; assert!(res.is_err()); }) }; @@ -989,7 +1014,9 @@ mod tests { Box::pin(async move { let sender = ctx.sender().clone(); let window = - RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash).await.unwrap(); + RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash, dummy_db_params()) + .await + .unwrap(); assert_eq!(window.earliest_session, session); assert_eq!(window.session_info, vec![dummy_session_info(session)]); From 781ef474cd9c423169bff33cda93ecb314361297 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 4 Oct 2022 16:21:00 +0000 Subject: [PATCH 10/42] Small refactor Signed-off-by: Andrei Sandu --- .../src/rolling_session_window.rs | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 47719b5fc1c0..e44a593e0dc3 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -139,12 +139,21 @@ impl RollingSessionWindow { Sender: overseer::SubsystemSender + overseer::SubsystemSender, { - let maybe_stored_window = Self::db_load(db_params.clone()); - + // At first, determine session window start using the chain state. let session_index = get_session_index_for_child(&mut sender, block_hash).await?; let earliest_non_finalized_block_session = Self::earliest_non_finalized_block_session(&mut sender).await?; + // This will increase the session window to cover the full unfinalized chain. + let on_chain_window_start = std::cmp::min( + session_index.saturating_sub(window_size.get() - 1), + earliest_non_finalized_block_session, + ); + + // Fetch session information from DB. + let maybe_stored_window = Self::db_load(db_params.clone()); + + // Get the DB stored sessions and recompute window start based on DB data. let (window_start, mut stored_sessions) = if let Some(mut stored_window) = maybe_stored_window { // Check if DB is ancient. @@ -161,22 +170,12 @@ impl RollingSessionWindow { // If there is at least one entry in db, we always take the DB as source of truth. stored_window.earliest_session } else { - // This will increase the session window to cover the full unfinalized chain. - std::cmp::min( - session_index.saturating_sub(window_size.get() - 1), - earliest_non_finalized_block_session, - ) + on_chain_window_start }; (window_start, stored_window.session_info) } else { - ( - std::cmp::min( - session_index.saturating_sub(window_size.get() - 1), - earliest_non_finalized_block_session, - ), - Vec::new(), - ) + (on_chain_window_start, Vec::new()) }; // Try to load sessions from DB first and load more from chain state if needed. From 89d4d0076056264410250d4964a4f7afc6f6b138 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 5 Oct 2022 19:28:09 +0000 Subject: [PATCH 11/42] WIP, tests failing Signed-off-by: Andrei Sandu --- .../src/rolling_session_window.rs | 140 ++++++++++++------ 1 file changed, 94 insertions(+), 46 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index e44a593e0dc3..68d77bac24dc 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -154,39 +154,39 @@ impl RollingSessionWindow { let maybe_stored_window = Self::db_load(db_params.clone()); // Get the DB stored sessions and recompute window start based on DB data. - let (window_start, mut stored_sessions) = - if let Some(mut stored_window) = maybe_stored_window { - // Check if DB is ancient. - if earliest_non_finalized_block_session > - stored_window.earliest_session + stored_window.session_info.len() as u32 - { - // If ancient, we scrap it and fetch from chain state. - stored_window.session_info.clear(); - } - - // The session window might extend beyond the last finalized block, but that's fine as we'll prune it at - // next update. - let window_start = if stored_window.session_info.len() > 0 { - // If there is at least one entry in db, we always take the DB as source of truth. - stored_window.earliest_session - } else { - on_chain_window_start - }; + let (window_start, stored_sessions) = if let Some(mut stored_window) = maybe_stored_window { + // Check if DB is ancient. + if earliest_non_finalized_block_session > + stored_window.earliest_session + stored_window.session_info.len() as u32 + { + // If ancient, we scrap it and fetch from chain state. + stored_window.session_info.clear(); + } - (window_start, stored_window.session_info) + // The session window might extend beyond the last finalized block, but that's fine as we'll prune it at + // next update. + let window_start = if stored_window.session_info.len() > 0 { + // If there is at least one entry in db, we always take the DB as source of truth. + stored_window.earliest_session } else { - (on_chain_window_start, Vec::new()) + on_chain_window_start }; + (window_start, stored_window.session_info) + } else { + (on_chain_window_start, Vec::new()) + }; + // Try to load sessions from DB first and load more from chain state if needed. let sessions_missing_count = session_index .saturating_sub(window_start) .saturating_add(1) .saturating_sub(stored_sessions.len() as u32); - // Load remaining from chain state. - let mut chain_sessions = if sessions_missing_count > 0 { - match load_all_sessions_from_chain_state( + // Extend from chain state. + let sessions = if sessions_missing_count > 0 { + match extend_sessions_from_chain_state( + stored_sessions, &mut sender, block_hash, window_start, @@ -209,11 +209,9 @@ impl RollingSessionWindow { Vec::new() }; - stored_sessions.append(&mut chain_sessions); - Ok(Self { earliest_session: window_start, - session_info: stored_sessions, + session_info: sessions, window_size, db_params: Some(db_params), }) @@ -386,11 +384,6 @@ impl RollingSessionWindow { + overseer::SubsystemSender, { let session_index = get_session_index_for_child(sender, block_hash).await?; - let earliest_non_finalized_block_session = - Self::earliest_non_finalized_block_session(sender).await?; - - let old_window_start = self.earliest_session; - let latest = self.latest_session(); // Either cached or ancient. @@ -398,6 +391,10 @@ impl RollingSessionWindow { return Ok(SessionWindowUpdate::Unchanged) } + let earliest_non_finalized_block_session = + Self::earliest_non_finalized_block_session(sender).await?; + + let old_window_start = self.earliest_session; let old_window_end = latest; // Ensure we keep sessions up to last finalized block by adjusting the window start. @@ -412,8 +409,14 @@ impl RollingSessionWindow { let fresh_start = if latest < window_start { window_start } else { latest + 1 }; - match load_all_sessions_from_chain_state(sender, block_hash, fresh_start, session_index) - .await + match extend_sessions_from_chain_state( + self.session_info.clone(), + sender, + block_hash, + fresh_start, + session_index, + ) + .await { Err(kind) => Err(SessionsUnavailable { kind, @@ -441,6 +444,7 @@ impl RollingSessionWindow { let new_earliest = std::cmp::max(window_start, old_window_start); self.earliest_session = new_earliest; + // Update current window in DB. self.db_save(StoredWindow { earliest_session: self.earliest_session, session_info: self.session_info.clone(), @@ -485,13 +489,26 @@ async fn get_session_index_for_child( } } -async fn load_all_sessions_from_chain_state( +/// Attempts to extend db stored sessions with sessions missing between `start` and up to `end_inclusive`. +/// If `allow_failure` is true, fetching errors are ignored until we get a first session, then +/// the function would return error. This allows us to be more relaxed wrt sessions no longer +/// available in the runtime, but not for other types of errors. +async fn extend_sessions_from_chain_state( + db_sessions: Vec, sender: &mut impl overseer::SubsystemSender, block_hash: Hash, - start: SessionIndex, + mut start: SessionIndex, end_inclusive: SessionIndex, ) -> Result, SessionsUnavailableReason> { - let mut v = Vec::new(); + println!("Extending {} sessions, start: {}, end: {}", db_sessions.len(), start, end_inclusive); + + // Start from the db sessions. + let mut sessions = db_sessions; + // We allow session fetch failures only if we won't create a gap in the window by doing so. + let allow_failure = false; + + start += sessions.len() as u32; + for i in start..=end_inclusive { let (tx, rx) = oneshot::channel(); sender @@ -501,17 +518,44 @@ async fn load_all_sessions_from_chain_state( )) .await; - let session_info = match rx.await { - Ok(Ok(Some(s))) => s, - Ok(Ok(None)) => return Err(SessionsUnavailableReason::Missing(i)), - Ok(Err(e)) => return Err(SessionsUnavailableReason::RuntimeApi(e)), - Err(canceled) => return Err(SessionsUnavailableReason::RuntimeApiUnavailable(canceled)), + match rx.await { + Ok(Ok(Some(session_info))) => { + sessions.push(session_info); + }, + Ok(Ok(None)) if !allow_failure => return Err(SessionsUnavailableReason::Missing(i)), + Ok(Ok(None)) => { + /* handle `allow_failure` true */ + gum::debug!( + target: LOG_TARGET, + session = ?i, + "Session info missing from runtime." + ); + }, + Ok(Err(e)) if !allow_failure => return Err(SessionsUnavailableReason::RuntimeApi(e)), + Err(canceled) if !allow_failure => + return Err(SessionsUnavailableReason::RuntimeApiUnavailable(canceled)), + Ok(Err(err)) => { + /* handle `allow_failure` true */ + gum::debug!( + target: LOG_TARGET, + session = ?i, + ?err, + "Error while fetching session information." + ); + }, + Err(err) => { + /* handle `allow_failure` true */ + gum::debug!( + target: LOG_TARGET, + session = ?i, + ?err, + "Channel error while fetching session information." + ); + }, }; - - v.push(session_info); } - Ok(v) + Ok(sessions) } #[cfg(test)] @@ -711,11 +755,13 @@ mod tests { db_params: Some(dummy_db_params()), }; + let actual_window_size = window.session_info.len() as u32; + cache_session_info_test( (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), 100, Some(window), - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1 - actual_window_size), ); } @@ -783,7 +829,9 @@ mod tests { db_params: Some(dummy_db_params()), }; - cache_session_info_test(0, 3, Some(window), 2); + let actual_window_size = window.session_info.len() as u32; + + cache_session_info_test(0, 3, Some(window), actual_window_size); } #[test] From 09d25e6073f379e20cf75528e41fb7585089db40 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 6 Oct 2022 08:23:15 +0000 Subject: [PATCH 12/42] Fix approval voting tests Signed-off-by: Andrei Sandu --- node/core/approval-voting/src/import.rs | 32 ---------- node/core/approval-voting/src/tests.rs | 58 +++++++++---------- .../src/rolling_session_window.rs | 30 ++++++---- 3 files changed, 47 insertions(+), 73 deletions(-) diff --git a/node/core/approval-voting/src/import.rs b/node/core/approval-voting/src/import.rs index e98cea0c4db4..2b51ff5f3af9 100644 --- a/node/core/approval-voting/src/import.rs +++ b/node/core/approval-voting/src/import.rs @@ -1304,38 +1304,6 @@ pub(crate) mod tests { } ); - // Caching of sesssions needs sessoion of first unfinalied block. - assert_matches!( - handle.recv().await, - AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( - s_tx, - )) => { - let _ = s_tx.send(Ok(header.number)); - } - ); - - assert_matches!( - handle.recv().await, - AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( - block_number, - s_tx, - )) => { - assert_eq!(block_number, header.number); - let _ = s_tx.send(Ok(Some(header.hash()))); - } - ); - - assert_matches!( - handle.recv().await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - h, - RuntimeApiRequest::SessionIndexForChild(s_tx), - )) => { - assert_eq!(h, header.hash()); - let _ = s_tx.send(Ok(session)); - } - ); - // determine_new_blocks exits early as the parent_hash is in the DB assert_matches!( diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs index 2b72e235c9bc..51af1c04641f 100644 --- a/node/core/approval-voting/src/tests.rs +++ b/node/core/approval-voting/src/tests.rs @@ -813,38 +813,38 @@ async fn import_block( } ); - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( - s_tx, - )) => { - let _ = s_tx.send(Ok(number)); - } - ); + if !fork { + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(number)); + } + ); - assert_matches!( - overseer_recv(overseer).await, - AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( - block_number, - s_tx, - )) => { - assert_eq!(block_number, number); - let _ = s_tx.send(Ok(Some(hashes[number as usize].0))); - } - ); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + block_number, + s_tx, + )) => { + assert_eq!(block_number, number); + let _ = s_tx.send(Ok(Some(hashes[number as usize].0))); + } + ); - assert_matches!( - overseer_recv(overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - h, - RuntimeApiRequest::SessionIndexForChild(s_tx), - )) => { - assert_eq!(h, hashes[number as usize].0); - let _ = s_tx.send(Ok(number.into())); - } - ); + assert_matches!( + overseer_recv(overseer).await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, hashes[number as usize].0); + let _ = s_tx.send(Ok(number.into())); + } + ); - if !fork { assert_matches!( overseer_recv(overseer).await, AllMessages::RuntimeApi( diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 68d77bac24dc..3994711558d2 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -404,16 +404,26 @@ impl RollingSessionWindow { earliest_non_finalized_block_session, ); - // keep some of the old window, if applicable. - let overlap_start = window_start.saturating_sub(old_window_start); + // Never look back past earliest session, since if sessions beyond were not needed or available + // in the past remains valid for the future (window only advanced forward). + let window_start = std::cmp::max(window_start, self.earliest_session); - let fresh_start = if latest < window_start { window_start } else { latest + 1 }; + let mut sessions = self.session_info.clone(); + let sessions_out_of_window = window_start.saturating_sub(old_window_start) as usize; + + let sessions = if sessions_out_of_window < sessions.len() { + // Drop sessions based on how much the window advanced. + sessions.split_off((window_start as usize).saturating_sub(old_window_start as usize)) + } else { + // Window has jumped such that we need to fetch all sessions from on chain. + Vec::new() + }; match extend_sessions_from_chain_state( - self.session_info.clone(), + sessions, sender, block_hash, - fresh_start, + window_start, session_index, ) .await @@ -421,7 +431,7 @@ impl RollingSessionWindow { Err(kind) => Err(SessionsUnavailable { kind, info: Some(SessionsUnavailableInfo { - window_start: fresh_start, + window_start, window_end: session_index, block_hash, }), @@ -434,9 +444,7 @@ impl RollingSessionWindow { new_window_end: session_index, }; - let outdated = std::cmp::min(overlap_start as usize, self.session_info.len()); - self.session_info.drain(..outdated); - self.session_info.extend(s); + self.session_info = s; // we need to account for this case: // window_start ................................... session_index @@ -755,13 +763,11 @@ mod tests { db_params: Some(dummy_db_params()), }; - let actual_window_size = window.session_info.len() as u32; - cache_session_info_test( (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), 100, Some(window), - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1 - actual_window_size), + (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), ); } From 6ea92ed730a610097feb617a31fbcda318bc5232 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 12:54:32 +0000 Subject: [PATCH 13/42] fix dispute-coordinator tests Signed-off-by: Andrei Sandu --- node/core/dispute-coordinator/src/db/v1.rs | 4 +- node/core/dispute-coordinator/src/lib.rs | 39 ++-- node/core/dispute-coordinator/src/tests.rs | 218 +++++++++++++-------- 3 files changed, 168 insertions(+), 93 deletions(-) diff --git a/node/core/dispute-coordinator/src/db/v1.rs b/node/core/dispute-coordinator/src/db/v1.rs index 2c643d341de2..c637524bd7ce 100644 --- a/node/core/dispute-coordinator/src/db/v1.rs +++ b/node/core/dispute-coordinator/src/db/v1.rs @@ -196,6 +196,8 @@ fn candidate_votes_session_prefix(session: SessionIndex) -> [u8; 15 + 4] { pub struct ColumnConfiguration { /// The column in the key-value DB where data is stored. pub col_data: u32, + /// The column in the key-value DB where session data is stored. + pub col_session_data: u32, } /// Tracked votes on candidates, for the purposes of dispute resolution. @@ -362,7 +364,7 @@ mod tests { let db = kvdb_memorydb::create(1); let db = polkadot_node_subsystem_util::database::kvdb_impl::DbAdapter::new(db, &[0]); let store = Arc::new(db); - let config = ColumnConfiguration { col_data: 0 }; + let config = ColumnConfiguration { col_data: 0, col_session_data: 1 }; DbBackend::new(store, config, Metrics::default()) } diff --git a/node/core/dispute-coordinator/src/lib.rs b/node/core/dispute-coordinator/src/lib.rs index 6289eb2f11a2..3bdfa0f2363f 100644 --- a/node/core/dispute-coordinator/src/lib.rs +++ b/node/core/dispute-coordinator/src/lib.rs @@ -35,7 +35,8 @@ use polkadot_node_subsystem::{ overseer, ActivatedLeaf, FromOrchestra, OverseerSignal, SpawnedSubsystem, SubsystemError, }; use polkadot_node_subsystem_util::{ - database::Database, rolling_session_window::RollingSessionWindow, + database::Database, + rolling_session_window::{DatabaseParams, RollingSessionWindow}, }; use polkadot_primitives::v2::{ScrapedOnChainVotes, ValidatorIndex, ValidatorPair}; @@ -118,11 +119,16 @@ pub struct DisputeCoordinatorSubsystem { pub struct Config { /// The data column in the store to use for dispute data. pub col_data: u32, + /// The data column in the store to use for session data. + pub col_session_data: u32, } impl Config { fn column_config(&self) -> db::v1::ColumnConfiguration { - db::v1::ColumnConfiguration { col_data: self.col_data } + db::v1::ColumnConfiguration { + col_data: self.col_data, + col_session_data: self.col_session_data, + } } } @@ -199,17 +205,21 @@ impl DisputeCoordinatorSubsystem { B: Backend + 'static, { loop { - let (first_leaf, rolling_session_window) = match get_rolling_session_window(ctx).await { - Ok(Some(update)) => update, - Ok(None) => { - gum::info!(target: LOG_TARGET, "received `Conclude` signal, exiting"); - return Ok(None) - }, - Err(e) => { - e.split()?.log(); - continue - }, - }; + let db_params = + DatabaseParams { db: self.store.clone(), db_column: self.config.col_session_data }; + + let (first_leaf, rolling_session_window) = + match get_rolling_session_window(ctx, db_params).await { + Ok(Some(update)) => update, + Ok(None) => { + gum::info!(target: LOG_TARGET, "received `Conclude` signal, exiting"); + return Ok(None) + }, + Err(e) => { + e.split()?.log(); + continue + }, + }; let mut overlay_db = OverlayedBackend::new(&mut backend); let (participations, votes, spam_slots, ordering_provider) = match self @@ -352,12 +362,13 @@ impl DisputeCoordinatorSubsystem { #[overseer::contextbounds(DisputeCoordinator, prefix = self::overseer)] async fn get_rolling_session_window( ctx: &mut Context, + db_params: DatabaseParams, ) -> Result> { if let Some(leaf) = { wait_for_first_leaf(ctx) }.await? { let sender = ctx.sender().clone(); Ok(Some(( leaf.clone(), - RollingSessionWindow::new(sender, DISPUTE_WINDOW, leaf.hash) + RollingSessionWindow::new(sender, DISPUTE_WINDOW, leaf.hash, db_params) .await .map_err(JfyiError::RollingSessionWindow)?, ))) diff --git a/node/core/dispute-coordinator/src/tests.rs b/node/core/dispute-coordinator/src/tests.rs index aaef00999259..ec6ec311484c 100644 --- a/node/core/dispute-coordinator/src/tests.rs +++ b/node/core/dispute-coordinator/src/tests.rs @@ -176,7 +176,7 @@ impl Default for TestState { let db = kvdb_memorydb::create(1); let db = polkadot_node_subsystem_util::database::kvdb_impl::DbAdapter::new(db, &[]); let db = Arc::new(db); - let config = Config { col_data: 0 }; + let config = Config { col_data: 0, col_session_data: 1 }; let genesis_header = Header { parent_hash: Hash::zero(), @@ -212,6 +212,7 @@ impl TestState { virtual_overseer: &mut VirtualOverseer, session: SessionIndex, block_number: BlockNumber, + got_session_information: bool, ) { assert!(block_number > 0); @@ -239,8 +240,14 @@ impl TestState { ))) .await; - self.handle_sync_queries(virtual_overseer, block_hash, block_number, session) - .await; + self.handle_sync_queries( + virtual_overseer, + block_hash, + block_number, + session, + got_session_information, + ) + .await; } async fn handle_sync_queries( @@ -249,8 +256,10 @@ impl TestState { block_hash: Hash, block_number: BlockNumber, session: SessionIndex, + got_session_information: bool, ) { // Order of messages is not fixed (different on initializing): + #[derive(Debug)] struct FinishedSteps { got_session_information: bool, got_scraping_information: bool, @@ -268,7 +277,8 @@ impl TestState { let mut finished_steps = FinishedSteps::new(); while !finished_steps.is_done() { - match overseer_recv(virtual_overseer).await { + let recv = overseer_recv(virtual_overseer).await; + match recv { AllMessages::RuntimeApi(RuntimeApiMessage::Request( h, RuntimeApiRequest::SessionIndexForChild(tx), @@ -282,36 +292,38 @@ impl TestState { let _ = tx.send(Ok(session)); // Queries for fetching earliest unfinalized block session. See `RollingSessionWindow`. - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( - s_tx, - )) => { - let _ = s_tx.send(Ok(block_number)); - } - ); + if self.known_session.is_none() { + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(block_number)); + } + ); - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( - number, - s_tx, - )) => { - assert_eq!(block_number, number); - let _ = s_tx.send(Ok(Some(block_hash))); - } - ); + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + number, + s_tx, + )) => { + assert_eq!(block_number, number); + let _ = s_tx.send(Ok(Some(block_hash))); + } + ); - assert_matches!( - overseer_recv(virtual_overseer).await, - AllMessages::RuntimeApi(RuntimeApiMessage::Request( - h, - RuntimeApiRequest::SessionIndexForChild(s_tx), - )) => { - assert_eq!(h, block_hash); - let _ = s_tx.send(Ok(session)); - } - ); + assert_matches!( + overseer_recv(virtual_overseer).await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, block_hash); + let _ = s_tx.send(Ok(session)); + } + ); + } // No queries, if subsystem knows about this session already. if self.known_session == Some(session) { @@ -379,6 +391,7 @@ impl TestState { &mut self, virtual_overseer: &mut VirtualOverseer, session: SessionIndex, + got_session_information: bool, ) { let leaves: Vec = self.headers.keys().cloned().collect(); for (n, leaf) in leaves.iter().enumerate() { @@ -397,8 +410,14 @@ impl TestState { ))) .await; - self.handle_sync_queries(virtual_overseer, *leaf, n as BlockNumber, session) - .await; + self.handle_sync_queries( + virtual_overseer, + *leaf, + n as BlockNumber, + session, + got_session_information, + ) + .await; } } @@ -576,14 +595,16 @@ fn too_many_unconfirmed_statements_are_considered_spam() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt1 = make_valid_candidate_receipt(); let candidate_hash1 = candidate_receipt1.hash(); let candidate_receipt2 = make_invalid_candidate_receipt(); let candidate_hash2 = candidate_receipt2.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote1 = test_state .issue_backing_statement_with_index(ValidatorIndex(3), candidate_hash1, session) @@ -709,12 +730,14 @@ fn approval_vote_import_works() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt1 = make_valid_candidate_receipt(); let candidate_hash1 = candidate_receipt1.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote1 = test_state .issue_backing_statement_with_index(ValidatorIndex(3), candidate_hash1, session) @@ -754,6 +777,7 @@ fn approval_vote_import_works() { let approval_votes = [(ValidatorIndex(4), approval_vote.into_validator_signature())] .into_iter() .collect(); + handle_approval_vote_request(&mut virtual_overseer, &candidate_hash1, approval_votes) .await; @@ -802,14 +826,16 @@ fn dispute_gets_confirmed_via_participation() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt1 = make_valid_candidate_receipt(); let candidate_hash1 = candidate_receipt1.hash(); let candidate_receipt2 = make_invalid_candidate_receipt(); let candidate_hash2 = candidate_receipt2.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote1 = test_state .issue_explicit_statement_with_index( @@ -951,14 +977,16 @@ fn dispute_gets_confirmed_at_byzantine_threshold() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt1 = make_valid_candidate_receipt(); let candidate_hash1 = candidate_receipt1.hash(); let candidate_receipt2 = make_invalid_candidate_receipt(); let candidate_hash2 = candidate_receipt2.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote1 = test_state .issue_explicit_statement_with_index( @@ -1114,12 +1142,14 @@ fn backing_statements_import_works_and_no_spam() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote1 = test_state .issue_backing_statement_with_index(ValidatorIndex(3), candidate_hash, session) @@ -1218,12 +1248,14 @@ fn conflicting_votes_lead_to_dispute_participation() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -1343,12 +1375,14 @@ fn positive_votes_dont_trigger_participation() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -1456,12 +1490,14 @@ fn wrong_validator_index_is_ignored() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -1534,12 +1570,14 @@ fn finality_votes_ignore_disputed_candidates() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -1644,12 +1682,14 @@ fn supermajority_valid_dispute_may_be_finalized() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let supermajority_threshold = polkadot_primitives::v2::supermajority_threshold(test_state.validators.len()); @@ -1784,12 +1824,14 @@ fn concluded_supermajority_for_non_active_after_time() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let supermajority_threshold = polkadot_primitives::v2::supermajority_threshold(test_state.validators.len()); @@ -1901,13 +1943,15 @@ fn concluded_supermajority_against_non_active_after_time() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_invalid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let supermajority_threshold = polkadot_primitives::v2::supermajority_threshold(test_state.validators.len()); @@ -2026,12 +2070,14 @@ fn resume_dispute_without_local_statement() { test_harness(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -2095,7 +2141,7 @@ fn resume_dispute_without_local_statement() { // local statement for the active dispute. .resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); @@ -2206,12 +2252,14 @@ fn resume_dispute_with_local_statement() { test_harness(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let local_valid_vote = test_state .issue_explicit_statement_with_index( @@ -2255,6 +2303,7 @@ fn resume_dispute_with_local_statement() { }, }) .await; + handle_approval_vote_request(&mut virtual_overseer, &candidate_hash, HashMap::new()) .await; @@ -2282,7 +2331,7 @@ fn resume_dispute_with_local_statement() { // local statement for the active dispute. .resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; // Assert that subsystem is not sending Participation messages because we issued a local statement assert!(virtual_overseer.recv().timeout(TEST_TIMEOUT).await.is_none()); @@ -2304,12 +2353,14 @@ fn resume_dispute_without_local_statement_or_local_key() { test_state .resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -2377,7 +2428,7 @@ fn resume_dispute_without_local_statement_or_local_key() { // validator in that dispute. .resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, true).await; // Assert that subsystem is not sending Participation messages because we issued a local statement assert!(virtual_overseer.recv().timeout(TEST_TIMEOUT).await.is_none()); @@ -2397,12 +2448,14 @@ fn resume_dispute_with_local_statement_without_local_key() { let test_state = TestState::default(); let mut test_state = test_state.resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let local_valid_vote = test_state .issue_explicit_statement_with_index( @@ -2469,6 +2522,7 @@ fn resume_dispute_with_local_statement_without_local_key() { test_state }) }); + // No keys: test_state.subsystem_keystore = make_keystore(vec![Sr25519Keyring::Two.to_seed()].into_iter()).into(); @@ -2476,7 +2530,7 @@ fn resume_dispute_with_local_statement_without_local_key() { // her a non existing key. test_state.resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; // Assert that subsystem is not sending Participation messages because we don't // have a key. @@ -2505,12 +2559,14 @@ fn issue_local_statement_does_cause_distribution_but_not_duplicate_participation Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let other_vote = test_state .issue_explicit_statement_with_index( @@ -2578,12 +2634,14 @@ fn own_approval_vote_gets_distributed_on_dispute() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let statement = test_state.issue_approval_vote_with_index( ValidatorIndex(0), @@ -2669,12 +2727,14 @@ fn negative_issue_local_statement_only_triggers_import() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_invalid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; virtual_overseer .send(FromOrchestra::Communication { @@ -2717,12 +2777,14 @@ fn redundant_votes_ignored() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session).await; + test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; + test_state + .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .await; let valid_vote = test_state .issue_backing_statement_with_index(ValidatorIndex(1), candidate_hash, session) From cecd5e0e81d378cfcef9072480bd1cbf0728c21a Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 12:57:45 +0000 Subject: [PATCH 14/42] remove uneeded param Signed-off-by: Andrei Sandu --- node/core/dispute-coordinator/src/tests.rs | 93 ++++++++++------------ 1 file changed, 44 insertions(+), 49 deletions(-) diff --git a/node/core/dispute-coordinator/src/tests.rs b/node/core/dispute-coordinator/src/tests.rs index ec6ec311484c..0efb85fe0913 100644 --- a/node/core/dispute-coordinator/src/tests.rs +++ b/node/core/dispute-coordinator/src/tests.rs @@ -212,7 +212,6 @@ impl TestState { virtual_overseer: &mut VirtualOverseer, session: SessionIndex, block_number: BlockNumber, - got_session_information: bool, ) { assert!(block_number > 0); @@ -245,7 +244,6 @@ impl TestState { block_hash, block_number, session, - got_session_information, ) .await; } @@ -256,7 +254,6 @@ impl TestState { block_hash: Hash, block_number: BlockNumber, session: SessionIndex, - got_session_information: bool, ) { // Order of messages is not fixed (different on initializing): #[derive(Debug)] @@ -391,7 +388,6 @@ impl TestState { &mut self, virtual_overseer: &mut VirtualOverseer, session: SessionIndex, - got_session_information: bool, ) { let leaves: Vec = self.headers.keys().cloned().collect(); for (n, leaf) in leaves.iter().enumerate() { @@ -415,7 +411,6 @@ impl TestState { *leaf, n as BlockNumber, session, - got_session_information, ) .await; } @@ -595,7 +590,7 @@ fn too_many_unconfirmed_statements_are_considered_spam() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt1 = make_valid_candidate_receipt(); let candidate_hash1 = candidate_receipt1.hash(); @@ -603,7 +598,7 @@ fn too_many_unconfirmed_statements_are_considered_spam() { let candidate_hash2 = candidate_receipt2.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote1 = test_state @@ -730,13 +725,13 @@ fn approval_vote_import_works() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt1 = make_valid_candidate_receipt(); let candidate_hash1 = candidate_receipt1.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote1 = test_state @@ -826,7 +821,7 @@ fn dispute_gets_confirmed_via_participation() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt1 = make_valid_candidate_receipt(); let candidate_hash1 = candidate_receipt1.hash(); @@ -834,7 +829,7 @@ fn dispute_gets_confirmed_via_participation() { let candidate_hash2 = candidate_receipt2.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote1 = test_state @@ -977,7 +972,7 @@ fn dispute_gets_confirmed_at_byzantine_threshold() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt1 = make_valid_candidate_receipt(); let candidate_hash1 = candidate_receipt1.hash(); @@ -985,7 +980,7 @@ fn dispute_gets_confirmed_at_byzantine_threshold() { let candidate_hash2 = candidate_receipt2.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote1 = test_state @@ -1142,13 +1137,13 @@ fn backing_statements_import_works_and_no_spam() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote1 = test_state @@ -1248,13 +1243,13 @@ fn conflicting_votes_lead_to_dispute_participation() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote = test_state @@ -1375,13 +1370,13 @@ fn positive_votes_dont_trigger_participation() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote = test_state @@ -1490,13 +1485,13 @@ fn wrong_validator_index_is_ignored() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote = test_state @@ -1570,13 +1565,13 @@ fn finality_votes_ignore_disputed_candidates() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote = test_state @@ -1682,13 +1677,13 @@ fn supermajority_valid_dispute_may_be_finalized() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let supermajority_threshold = @@ -1824,13 +1819,13 @@ fn concluded_supermajority_for_non_active_after_time() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let supermajority_threshold = @@ -1943,14 +1938,14 @@ fn concluded_supermajority_against_non_active_after_time() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_invalid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let supermajority_threshold = @@ -2070,13 +2065,13 @@ fn resume_dispute_without_local_statement() { test_harness(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote = test_state @@ -2141,7 +2136,7 @@ fn resume_dispute_without_local_statement() { // local statement for the active dispute. .resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); @@ -2252,13 +2247,13 @@ fn resume_dispute_with_local_statement() { test_harness(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let local_valid_vote = test_state @@ -2331,7 +2326,7 @@ fn resume_dispute_with_local_statement() { // local statement for the active dispute. .resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; // Assert that subsystem is not sending Participation messages because we issued a local statement assert!(virtual_overseer.recv().timeout(TEST_TIMEOUT).await.is_none()); @@ -2353,13 +2348,13 @@ fn resume_dispute_without_local_statement_or_local_key() { test_state .resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote = test_state @@ -2428,7 +2423,7 @@ fn resume_dispute_without_local_statement_or_local_key() { // validator in that dispute. .resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session, true).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; // Assert that subsystem is not sending Participation messages because we issued a local statement assert!(virtual_overseer.recv().timeout(TEST_TIMEOUT).await.is_none()); @@ -2448,13 +2443,13 @@ fn resume_dispute_with_local_statement_without_local_key() { let test_state = TestState::default(); let mut test_state = test_state.resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let local_valid_vote = test_state @@ -2530,7 +2525,7 @@ fn resume_dispute_with_local_statement_without_local_key() { // her a non existing key. test_state.resume(|mut test_state, mut virtual_overseer| { Box::pin(async move { - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; // Assert that subsystem is not sending Participation messages because we don't // have a key. @@ -2559,13 +2554,13 @@ fn issue_local_statement_does_cause_distribution_but_not_duplicate_participation Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let other_vote = test_state @@ -2634,13 +2629,13 @@ fn own_approval_vote_gets_distributed_on_dispute() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let statement = test_state.issue_approval_vote_with_index( @@ -2727,13 +2722,13 @@ fn negative_issue_local_statement_only_triggers_import() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_invalid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; virtual_overseer @@ -2777,13 +2772,13 @@ fn redundant_votes_ignored() { Box::pin(async move { let session = 1; - test_state.handle_resume_sync(&mut virtual_overseer, session, false).await; + test_state.handle_resume_sync(&mut virtual_overseer, session).await; let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1, true) + .activate_leaf_at_session(&mut virtual_overseer, session, 1) .await; let valid_vote = test_state From e0aee8be671a558c48e1bb50a6e57cac7e05c214 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 12:58:11 +0000 Subject: [PATCH 15/42] fmt Signed-off-by: Andrei Sandu --- node/core/dispute-coordinator/src/tests.rs | 98 ++++++---------------- 1 file changed, 24 insertions(+), 74 deletions(-) diff --git a/node/core/dispute-coordinator/src/tests.rs b/node/core/dispute-coordinator/src/tests.rs index 0efb85fe0913..1b0d51f5719d 100644 --- a/node/core/dispute-coordinator/src/tests.rs +++ b/node/core/dispute-coordinator/src/tests.rs @@ -239,13 +239,8 @@ impl TestState { ))) .await; - self.handle_sync_queries( - virtual_overseer, - block_hash, - block_number, - session, - ) - .await; + self.handle_sync_queries(virtual_overseer, block_hash, block_number, session) + .await; } async fn handle_sync_queries( @@ -406,13 +401,8 @@ impl TestState { ))) .await; - self.handle_sync_queries( - virtual_overseer, - *leaf, - n as BlockNumber, - session, - ) - .await; + self.handle_sync_queries(virtual_overseer, *leaf, n as BlockNumber, session) + .await; } } @@ -597,9 +587,7 @@ fn too_many_unconfirmed_statements_are_considered_spam() { let candidate_receipt2 = make_invalid_candidate_receipt(); let candidate_hash2 = candidate_receipt2.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote1 = test_state .issue_backing_statement_with_index(ValidatorIndex(3), candidate_hash1, session) @@ -730,9 +718,7 @@ fn approval_vote_import_works() { let candidate_receipt1 = make_valid_candidate_receipt(); let candidate_hash1 = candidate_receipt1.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote1 = test_state .issue_backing_statement_with_index(ValidatorIndex(3), candidate_hash1, session) @@ -828,9 +814,7 @@ fn dispute_gets_confirmed_via_participation() { let candidate_receipt2 = make_invalid_candidate_receipt(); let candidate_hash2 = candidate_receipt2.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote1 = test_state .issue_explicit_statement_with_index( @@ -979,9 +963,7 @@ fn dispute_gets_confirmed_at_byzantine_threshold() { let candidate_receipt2 = make_invalid_candidate_receipt(); let candidate_hash2 = candidate_receipt2.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote1 = test_state .issue_explicit_statement_with_index( @@ -1142,9 +1124,7 @@ fn backing_statements_import_works_and_no_spam() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote1 = test_state .issue_backing_statement_with_index(ValidatorIndex(3), candidate_hash, session) @@ -1248,9 +1228,7 @@ fn conflicting_votes_lead_to_dispute_participation() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -1375,9 +1353,7 @@ fn positive_votes_dont_trigger_participation() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -1490,9 +1466,7 @@ fn wrong_validator_index_is_ignored() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -1570,9 +1544,7 @@ fn finality_votes_ignore_disputed_candidates() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -1682,9 +1654,7 @@ fn supermajority_valid_dispute_may_be_finalized() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let supermajority_threshold = polkadot_primitives::v2::supermajority_threshold(test_state.validators.len()); @@ -1824,9 +1794,7 @@ fn concluded_supermajority_for_non_active_after_time() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let supermajority_threshold = polkadot_primitives::v2::supermajority_threshold(test_state.validators.len()); @@ -1944,9 +1912,7 @@ fn concluded_supermajority_against_non_active_after_time() { let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let supermajority_threshold = polkadot_primitives::v2::supermajority_threshold(test_state.validators.len()); @@ -2070,9 +2036,7 @@ fn resume_dispute_without_local_statement() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -2252,9 +2216,7 @@ fn resume_dispute_with_local_statement() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let local_valid_vote = test_state .issue_explicit_statement_with_index( @@ -2353,9 +2315,7 @@ fn resume_dispute_without_local_statement_or_local_key() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote = test_state .issue_explicit_statement_with_index( @@ -2448,9 +2408,7 @@ fn resume_dispute_with_local_statement_without_local_key() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let local_valid_vote = test_state .issue_explicit_statement_with_index( @@ -2559,9 +2517,7 @@ fn issue_local_statement_does_cause_distribution_but_not_duplicate_participation let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let other_vote = test_state .issue_explicit_statement_with_index( @@ -2634,9 +2590,7 @@ fn own_approval_vote_gets_distributed_on_dispute() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let statement = test_state.issue_approval_vote_with_index( ValidatorIndex(0), @@ -2727,9 +2681,7 @@ fn negative_issue_local_statement_only_triggers_import() { let candidate_receipt = make_invalid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; virtual_overseer .send(FromOrchestra::Communication { @@ -2777,9 +2729,7 @@ fn redundant_votes_ignored() { let candidate_receipt = make_valid_candidate_receipt(); let candidate_hash = candidate_receipt.hash(); - test_state - .activate_leaf_at_session(&mut virtual_overseer, session, 1) - .await; + test_state.activate_leaf_at_session(&mut virtual_overseer, session, 1).await; let valid_vote = test_state .issue_backing_statement_with_index(ValidatorIndex(1), candidate_hash, session) From c6e727d91f88c37e6d7c8e52a40d3829565c830e Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 13:02:04 +0000 Subject: [PATCH 16/42] fix loose ends Signed-off-by: Andrei Sandu --- node/service/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index 6425ee7a7536..99b33ccddb58 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -944,6 +944,7 @@ where let approval_voting_config = ApprovalVotingConfig { col_data: parachains_db::REAL_COLUMNS.col_approval_data, + col_session_data: parachains_db::REAL_COLUMNS.col_session_window_data, slot_duration_millis: slot_duration.as_millis() as u64, }; @@ -967,6 +968,7 @@ where let dispute_coordinator_config = DisputeCoordinatorConfig { col_data: parachains_db::REAL_COLUMNS.col_dispute_coordinator_data, + col_session_data: parachains_db::REAL_COLUMNS.col_session_window_data, }; let rpc_handlers = service::spawn_tasks(service::SpawnTasksParams { @@ -1536,6 +1538,7 @@ fn revert_chain_selection(db: Arc, hash: Hash) -> sp_blockchain::R fn revert_approval_voting(db: Arc, hash: Hash) -> sp_blockchain::Result<()> { let config = approval_voting_subsystem::Config { col_data: parachains_db::REAL_COLUMNS.col_approval_data, + col_session_data: parachains_db::REAL_COLUMNS.col_session_window_data, slot_duration_millis: Default::default(), }; From 90cdad79e75b99502d223cfaff4adafb003badca Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 13:39:53 +0000 Subject: [PATCH 17/42] allow failure and tests for it Signed-off-by: Andrei Sandu --- .../src/rolling_session_window.rs | 201 +++++++++++++++--- 1 file changed, 171 insertions(+), 30 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 3994711558d2..49b533f5f4ed 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -154,29 +154,30 @@ impl RollingSessionWindow { let maybe_stored_window = Self::db_load(db_params.clone()); // Get the DB stored sessions and recompute window start based on DB data. - let (window_start, stored_sessions) = if let Some(mut stored_window) = maybe_stored_window { - // Check if DB is ancient. - if earliest_non_finalized_block_session > - stored_window.earliest_session + stored_window.session_info.len() as u32 - { - // If ancient, we scrap it and fetch from chain state. - stored_window.session_info.clear(); - } + let (mut window_start, stored_sessions) = + if let Some(mut stored_window) = maybe_stored_window { + // Check if DB is ancient. + if earliest_non_finalized_block_session > + stored_window.earliest_session + stored_window.session_info.len() as u32 + { + // If ancient, we scrap it and fetch from chain state. + stored_window.session_info.clear(); + } + + // The session window might extend beyond the last finalized block, but that's fine as we'll prune it at + // next update. + let window_start = if stored_window.session_info.len() > 0 { + // If there is at least one entry in db, we always take the DB as source of truth. + stored_window.earliest_session + } else { + on_chain_window_start + }; - // The session window might extend beyond the last finalized block, but that's fine as we'll prune it at - // next update. - let window_start = if stored_window.session_info.len() > 0 { - // If there is at least one entry in db, we always take the DB as source of truth. - stored_window.earliest_session + (window_start, stored_window.session_info) } else { - on_chain_window_start + (on_chain_window_start, Vec::new()) }; - (window_start, stored_window.session_info) - } else { - (on_chain_window_start, Vec::new()) - }; - // Try to load sessions from DB first and load more from chain state if needed. let sessions_missing_count = session_index .saturating_sub(window_start) @@ -189,7 +190,7 @@ impl RollingSessionWindow { stored_sessions, &mut sender, block_hash, - window_start, + &mut window_start, session_index, ) .await @@ -406,7 +407,7 @@ impl RollingSessionWindow { // Never look back past earliest session, since if sessions beyond were not needed or available // in the past remains valid for the future (window only advanced forward). - let window_start = std::cmp::max(window_start, self.earliest_session); + let mut window_start = std::cmp::max(window_start, self.earliest_session); let mut sessions = self.session_info.clone(); let sessions_out_of_window = window_start.saturating_sub(old_window_start) as usize; @@ -423,7 +424,7 @@ impl RollingSessionWindow { sessions, sender, block_hash, - window_start, + &mut window_start, session_index, ) .await @@ -505,17 +506,15 @@ async fn extend_sessions_from_chain_state( db_sessions: Vec, sender: &mut impl overseer::SubsystemSender, block_hash: Hash, - mut start: SessionIndex, + window_start: &mut SessionIndex, end_inclusive: SessionIndex, ) -> Result, SessionsUnavailableReason> { - println!("Extending {} sessions, start: {}, end: {}", db_sessions.len(), start, end_inclusive); - // Start from the db sessions. let mut sessions = db_sessions; // We allow session fetch failures only if we won't create a gap in the window by doing so. - let allow_failure = false; + let mut allow_failure = sessions.is_empty(); - start += sessions.len() as u32; + let start = *window_start + sessions.len() as u32; for i in start..=end_inclusive { let (tx, rx) = oneshot::channel(); @@ -528,11 +527,17 @@ async fn extend_sessions_from_chain_state( match rx.await { Ok(Ok(Some(session_info))) => { + // We do not allow failure anymore after having at least 1 session in window. + allow_failure = false; sessions.push(session_info); }, - Ok(Ok(None)) if !allow_failure => return Err(SessionsUnavailableReason::Missing(i)), + Ok(Ok(None)) if !allow_failure => { + return Err(SessionsUnavailableReason::Missing(i)) + }, Ok(Ok(None)) => { /* handle `allow_failure` true */ + // If we didn't get the session, we advance window start. + *window_start += 1; gum::debug!( target: LOG_TARGET, session = ?i, @@ -544,6 +549,8 @@ async fn extend_sessions_from_chain_state( return Err(SessionsUnavailableReason::RuntimeApiUnavailable(canceled)), Ok(Err(err)) => { /* handle `allow_failure` true */ + // If we didn't get the session, we advance window start. + *window_start += 1; gum::debug!( target: LOG_TARGET, session = ?i, @@ -553,6 +560,8 @@ async fn extend_sessions_from_chain_state( }, Err(err) => { /* handle `allow_failure` true */ + // If we didn't get the session, we advance window start. + *window_start += 1; gum::debug!( target: LOG_TARGET, session = ?i, @@ -841,7 +850,7 @@ mod tests { } #[test] - fn any_session_stretch_for_unfinalized_chain() { + fn cache_session_fails_for_gap_in_window() { // Session index of the tip of our fake test chain. let session: SessionIndex = 100; let genesis_session: SessionIndex = 0; @@ -873,6 +882,7 @@ mod tests { let res = RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash, dummy_db_params()) .await; + assert!(res.is_err()); }) }; @@ -921,6 +931,137 @@ mod tests { ); // Unfinalized chain starts at geneisis block, so session 0 is how far we stretch. + // First 50 sessions are missing. + for i in genesis_session..=50 { + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionInfo(j, s_tx), + )) => { + assert_eq!(h, hash); + assert_eq!(i, j); + let _ = s_tx.send(Ok(None)); + } + ); + } + // next 10 sessions are present + for i in 51..=60 { + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionInfo(j, s_tx), + )) => { + assert_eq!(h, hash); + assert_eq!(i, j); + let _ = s_tx.send(Ok(Some(dummy_session_info(i)))); + } + ); + } + // gap of 1 session + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionInfo(j, s_tx), + )) => { + assert_eq!(h, hash); + assert_eq!(61, j); + let _ = s_tx.send(Ok(None)); + } + ); + }); + + futures::executor::block_on(futures::future::join(test_fut, aux_fut)); + } + + #[test] + fn any_session_stretch_with_failure_allowed_for_unfinalized_chain() { + // Session index of the tip of our fake test chain. + let session: SessionIndex = 100; + let genesis_session: SessionIndex = 0; + + let header = Header { + digest: Default::default(), + extrinsics_root: Default::default(), + number: 5, + state_root: Default::default(), + parent_hash: Default::default(), + }; + + let finalized_header = Header { + digest: Default::default(), + extrinsics_root: Default::default(), + number: 0, + state_root: Default::default(), + parent_hash: Default::default(), + }; + + let pool = TaskExecutor::new(); + let (mut ctx, mut handle) = make_subsystem_context::<(), _>(pool.clone()); + + let hash = header.hash(); + + let test_fut = { + let sender = ctx.sender().clone(); + Box::pin(async move { + let res = + RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash, dummy_db_params()) + .await; + assert!(res.is_ok()); + let rsw = res.unwrap(); + // Since first 50 sessions are missing the earliest should be 50. + assert_eq!(rsw.earliest_session, 50); + assert_eq!(rsw.session_info.len(), 51); + }) + }; + + let aux_fut = Box::pin(async move { + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, hash); + let _ = s_tx.send(Ok(session)); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockNumber( + s_tx, + )) => { + let _ = s_tx.send(Ok(finalized_header.number)); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::ChainApi(ChainApiMessage::FinalizedBlockHash( + block_number, + s_tx, + )) => { + assert_eq!(block_number, finalized_header.number); + let _ = s_tx.send(Ok(Some(finalized_header.hash()))); + } + ); + + assert_matches!( + handle.recv().await, + AllMessages::RuntimeApi(RuntimeApiMessage::Request( + h, + RuntimeApiRequest::SessionIndexForChild(s_tx), + )) => { + assert_eq!(h, finalized_header.hash()); + let _ = s_tx.send(Ok(0)); + } + ); + + // Unfinalized chain starts at geneisis block, so session 0 is how far we stretch. + // We also test if failure is allowed for 50 first missing sessions. for i in genesis_session..=session { assert_matches!( handle.recv().await, @@ -931,7 +1072,7 @@ mod tests { assert_eq!(h, hash); assert_eq!(i, j); - let _ = s_tx.send(Ok(if i == session { + let _ = s_tx.send(Ok(if i < 50 { None } else { Some(dummy_session_info(i)) From 1ae389bb12436335ed69a4434b4bffaff2af78ae Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 13:42:59 +0000 Subject: [PATCH 18/42] fix comment Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 49b533f5f4ed..a358a0838ea6 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -113,8 +113,8 @@ pub struct RollingSessionWindow { earliest_session: SessionIndex, session_info: Vec, window_size: SessionWindowSize, - // These params just to enable some approval-voting tests to force feed sessions - // in the window. + // The option is just to enable some approval-voting tests to force feed sessions + // in the window without dealing with the DB. db_params: Option, } @@ -531,9 +531,7 @@ async fn extend_sessions_from_chain_state( allow_failure = false; sessions.push(session_info); }, - Ok(Ok(None)) if !allow_failure => { - return Err(SessionsUnavailableReason::Missing(i)) - }, + Ok(Ok(None)) if !allow_failure => return Err(SessionsUnavailableReason::Missing(i)), Ok(Ok(None)) => { /* handle `allow_failure` true */ // If we didn't get the session, we advance window start. From ee093f890e04237c8a1de155af27b1a7dbae2bb0 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 13:44:56 +0000 Subject: [PATCH 19/42] comment fix Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index a358a0838ea6..cba6339c1734 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -406,7 +406,7 @@ impl RollingSessionWindow { ); // Never look back past earliest session, since if sessions beyond were not needed or available - // in the past remains valid for the future (window only advanced forward). + // in the past remains valid for the future (window only advances forward). let mut window_start = std::cmp::max(window_start, self.earliest_session); let mut sessions = self.session_info.clone(); From 5ccc67563e360b6629f5b3f1eeaac50c4436429f Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 13:46:15 +0000 Subject: [PATCH 20/42] style fix Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index cba6339c1734..61f341ad2458 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -533,7 +533,7 @@ async fn extend_sessions_from_chain_state( }, Ok(Ok(None)) if !allow_failure => return Err(SessionsUnavailableReason::Missing(i)), Ok(Ok(None)) => { - /* handle `allow_failure` true */ + // Handle `allow_failure` true. // If we didn't get the session, we advance window start. *window_start += 1; gum::debug!( @@ -546,7 +546,7 @@ async fn extend_sessions_from_chain_state( Err(canceled) if !allow_failure => return Err(SessionsUnavailableReason::RuntimeApiUnavailable(canceled)), Ok(Err(err)) => { - /* handle `allow_failure` true */ + // Handle `allow_failure` true. // If we didn't get the session, we advance window start. *window_start += 1; gum::debug!( @@ -557,7 +557,7 @@ async fn extend_sessions_from_chain_state( ); }, Err(err) => { - /* handle `allow_failure` true */ + // Handle `allow_failure` true. // If we didn't get the session, we advance window start. *window_start += 1; gum::debug!( From 762be95ee475bb5390ba860926a7192e9762c9d9 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 13:49:14 +0000 Subject: [PATCH 21/42] new col doesn't need to be ordered Signed-off-by: Andrei Sandu --- node/service/src/parachains_db/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/node/service/src/parachains_db/mod.rs b/node/service/src/parachains_db/mod.rs index de1919c0a0b4..47f540b04159 100644 --- a/node/service/src/parachains_db/mod.rs +++ b/node/service/src/parachains_db/mod.rs @@ -41,7 +41,6 @@ pub(crate) mod columns { COL_AVAILABILITY_META, COL_CHAIN_SELECTION_DATA, COL_DISPUTE_COORDINATOR_DATA, - COL_SESSION_WINDOW_DATA, ]; pub const COL_SESSION_WINDOW_DATA: u32 = 5; } From f6d37a960da9d9013173cf59152c6c70c99c23c2 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 7 Oct 2022 14:21:52 +0000 Subject: [PATCH 22/42] fmt and spellcheck Signed-off-by: Andrei Sandu --- node/service/src/parachains_db/mod.rs | 7 ++----- node/subsystem-util/src/rolling_session_window.rs | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/node/service/src/parachains_db/mod.rs b/node/service/src/parachains_db/mod.rs index 47f540b04159..7b1ed0e519be 100644 --- a/node/service/src/parachains_db/mod.rs +++ b/node/service/src/parachains_db/mod.rs @@ -37,11 +37,8 @@ pub(crate) mod columns { pub const COL_APPROVAL_DATA: u32 = 2; pub const COL_CHAIN_SELECTION_DATA: u32 = 3; pub const COL_DISPUTE_COORDINATOR_DATA: u32 = 4; - pub const ORDERED_COL: &[u32] = &[ - COL_AVAILABILITY_META, - COL_CHAIN_SELECTION_DATA, - COL_DISPUTE_COORDINATOR_DATA, - ]; + pub const ORDERED_COL: &[u32] = + &[COL_AVAILABILITY_META, COL_CHAIN_SELECTION_DATA, COL_DISPUTE_COORDINATOR_DATA]; pub const COL_SESSION_WINDOW_DATA: u32 = 5; } } diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 61f341ad2458..67fc3486a670 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -103,7 +103,7 @@ pub enum SessionWindowUpdate { /// A structure to store rolling session database parameters. #[derive(Clone)] pub struct DatabaseParams { - /// Databae reference. + /// Database reference. pub db: Arc, /// The column which stores the rolling session info. pub db_column: u32, @@ -500,7 +500,7 @@ async fn get_session_index_for_child( /// Attempts to extend db stored sessions with sessions missing between `start` and up to `end_inclusive`. /// If `allow_failure` is true, fetching errors are ignored until we get a first session, then -/// the function would return error. This allows us to be more relaxed wrt sessions no longer +/// the function would return error. This allows us to be more relaxed `wrt` sessions no longer /// available in the runtime, but not for other types of errors. async fn extend_sessions_from_chain_state( db_sessions: Vec, From 8de8d66b6bfb07f8cecfb88c9989e16645cd650b Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 10 Oct 2022 16:06:37 +0000 Subject: [PATCH 23/42] db persist tests Signed-off-by: Andrei Sandu --- .../src/rolling_session_window.rs | 98 ++++++++++++++++--- 1 file changed, 85 insertions(+), 13 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 67fc3486a670..48542e9209d4 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -597,6 +597,7 @@ mod tests { let db: Arc = Arc::new(db); DatabaseParams { db, db_column: SESSION_DATA_COL } } + fn dummy_session_info(index: SessionIndex) -> SessionInfo { SessionInfo { validators: Vec::new(), @@ -620,7 +621,10 @@ mod tests { session: SessionIndex, window: Option, expect_requests_from: SessionIndex, - ) { + db_params: Option, + ) -> RollingSessionWindow { + let db_params = db_params.unwrap_or(dummy_db_params()); + let header = Header { digest: Default::default(), extrinsics_root: Default::default(), @@ -648,14 +652,10 @@ mod tests { let test_fut = { Box::pin(async move { let window = match window { - None => RollingSessionWindow::new( - sender.clone(), - TEST_WINDOW_SIZE, - hash, - dummy_db_params(), - ) - .await - .unwrap(), + None => + RollingSessionWindow::new(sender.clone(), TEST_WINDOW_SIZE, hash, db_params) + .await + .unwrap(), Some(mut window) => { window.cache_session_info_for_head(sender, hash).await.unwrap(); window @@ -666,6 +666,8 @@ mod tests { window.session_info, (expected_start_session..=session).map(dummy_session_info).collect::>(), ); + + window }) }; @@ -727,12 +729,43 @@ mod tests { } }); - futures::executor::block_on(futures::future::join(test_fut, aux_fut)); + let (window, _) = futures::executor::block_on(futures::future::join(test_fut, aux_fut)); + window + } + + #[test] + fn cache_session_info_start_empty_db() { + let db_params = dummy_db_params(); + + let window = cache_session_info_test( + (10 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + 10, + None, + (10 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + Some(db_params.clone()), + ); + + let window = cache_session_info_test( + (11 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + 11, + Some(window), + 11, + None, + ); + assert_eq!(window.session_info.len(), TEST_WINDOW_SIZE.get() as usize); + + cache_session_info_test( + (11 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + 12, + None, + 12, + Some(db_params), + ); } #[test] fn cache_session_info_first_early() { - cache_session_info_test(0, 1, None, 0); + cache_session_info_test(0, 1, None, 0, None); } #[test] @@ -744,7 +777,7 @@ mod tests { db_params: Some(dummy_db_params()), }; - cache_session_info_test(1, 2, Some(window), 2); + cache_session_info_test(1, 2, Some(window), 2, None); } #[test] @@ -754,6 +787,7 @@ mod tests { 100, None, (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + None, ); } @@ -775,6 +809,7 @@ mod tests { 100, Some(window), (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + None, ); } @@ -793,9 +828,44 @@ mod tests { 100, Some(window), 100, // should only make one request. + None, ); } + #[test] + fn cache_session_info_roll_many_full_db() { + let db_params = dummy_db_params(); + let start = 97 - (TEST_WINDOW_SIZE.get() - 1); + let window = RollingSessionWindow { + earliest_session: start, + session_info: (start..=97).map(dummy_session_info).collect(), + window_size: TEST_WINDOW_SIZE, + db_params: Some(db_params.clone()), + }; + + cache_session_info_test( + (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + 100, + Some(window), + 98, + None, + ); + + // We expect the session to be populated from DB, and only fetch 101 from on chain. + cache_session_info_test( + (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + 101, + None, + 101, + Some(db_params.clone()), + ); + + // Session warps in the future. + let window = cache_session_info_test(195, 200, None, 195, Some(db_params)); + + assert_eq!(window.session_info.len(), TEST_WINDOW_SIZE.get() as usize); + } + #[test] fn cache_session_info_roll_many_full() { let start = 97 - (TEST_WINDOW_SIZE.get() - 1); @@ -811,6 +881,7 @@ mod tests { 100, Some(window), 98, + None, ); } @@ -829,6 +900,7 @@ mod tests { 2, Some(window), 2, // should only make one request. + None, ); } @@ -844,7 +916,7 @@ mod tests { let actual_window_size = window.session_info.len() as u32; - cache_session_info_test(0, 3, Some(window), actual_window_size); + cache_session_info_test(0, 3, Some(window), actual_window_size, None); } #[test] From 5c41d356877e9915419b08ebe45907496f8ed873 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 10 Oct 2022 16:07:26 +0000 Subject: [PATCH 24/42] Add v2 config and cols Signed-off-by: Andrei Sandu --- node/service/src/parachains_db/mod.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/node/service/src/parachains_db/mod.rs b/node/service/src/parachains_db/mod.rs index 7b1ed0e519be..48193309c311 100644 --- a/node/service/src/parachains_db/mod.rs +++ b/node/service/src/parachains_db/mod.rs @@ -30,7 +30,7 @@ pub(crate) mod columns { } pub mod v1 { - pub const NUM_COLUMNS: u32 = 6; + pub const NUM_COLUMNS: u32 = 5; pub const COL_AVAILABILITY_DATA: u32 = 0; pub const COL_AVAILABILITY_META: u32 = 1; @@ -39,6 +39,10 @@ pub(crate) mod columns { pub const COL_DISPUTE_COORDINATOR_DATA: u32 = 4; pub const ORDERED_COL: &[u32] = &[COL_AVAILABILITY_META, COL_CHAIN_SELECTION_DATA, COL_DISPUTE_COORDINATOR_DATA]; + } + + pub mod v2 { + pub const NUM_COLUMNS: u32 = 6; pub const COL_SESSION_WINDOW_DATA: u32 = 5; } } @@ -69,7 +73,7 @@ pub const REAL_COLUMNS: ColumnsConfig = ColumnsConfig { col_approval_data: columns::v1::COL_APPROVAL_DATA, col_chain_selection_data: columns::v1::COL_CHAIN_SELECTION_DATA, col_dispute_coordinator_data: columns::v1::COL_DISPUTE_COORDINATOR_DATA, - col_session_window_data: columns::v1::COL_SESSION_WINDOW_DATA, + col_session_window_data: columns::v2::COL_SESSION_WINDOW_DATA, }; #[derive(PartialEq)] @@ -117,7 +121,7 @@ pub fn open_creating_rocksdb( let path = root.join("parachains").join("db"); - let mut db_config = DatabaseConfig::with_columns(columns::v1::NUM_COLUMNS); + let mut db_config = DatabaseConfig::with_columns(columns::v2::NUM_COLUMNS); let _ = db_config .memory_budget @@ -130,7 +134,7 @@ pub fn open_creating_rocksdb( .insert(columns::v1::COL_APPROVAL_DATA, cache_sizes.approval_data); let _ = db_config .memory_budget - .insert(columns::v1::COL_SESSION_WINDOW_DATA, cache_sizes.session_data); + .insert(columns::v2::COL_SESSION_WINDOW_DATA, cache_sizes.session_data); let path_str = path .to_str() @@ -161,7 +165,7 @@ pub fn open_creating_paritydb( std::fs::create_dir_all(&path_str)?; upgrade::try_upgrade_db(&path, DatabaseKind::ParityDB)?; - let db = parity_db::Db::open_or_create(&upgrade::paritydb_version_1_config(&path)) + let db = parity_db::Db::open_or_create(&upgrade::paritydb_version_2_config(&path)) .map_err(|err| io::Error::new(io::ErrorKind::Other, format!("{:?}", err)))?; let db = polkadot_node_subsystem_util::database::paritydb_impl::DbAdapter::new( From 11fb82519cb3418fffc97b35a270397aac0bce38 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 11 Oct 2022 10:21:19 +0000 Subject: [PATCH 25/42] DB upgrade WIP Signed-off-by: Andrei Sandu --- node/service/src/parachains_db/upgrade.rs | 84 ++++++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/node/service/src/parachains_db/upgrade.rs b/node/service/src/parachains_db/upgrade.rs index ad995f41ed82..ae774ad10043 100644 --- a/node/service/src/parachains_db/upgrade.rs +++ b/node/service/src/parachains_db/upgrade.rs @@ -28,7 +28,7 @@ type Version = u32; const VERSION_FILE_NAME: &'static str = "parachain_db_version"; /// Current db version. -const CURRENT_VERSION: Version = 1; +const CURRENT_VERSION: Version = 2; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -56,6 +56,8 @@ pub(crate) fn try_upgrade_db(db_path: &Path, db_kind: DatabaseKind) -> Result<() match get_db_version(db_path)? { // 0 -> 1 migration Some(0) => migrate_from_version_0_to_1(db_path, db_kind)?, + // 1 -> 2 migration + Some(1) => migrate_from_version_1_to_2(db_path, db_kind)?, // Already at current version, do nothing. Some(CURRENT_VERSION) => (), // This is an arbitrary future version, we don't handle it. @@ -112,6 +114,19 @@ fn migrate_from_version_0_to_1(path: &Path, db_kind: DatabaseKind) -> Result<(), }) } +fn migrate_from_version_1_to_2(path: &Path, db_kind: DatabaseKind) -> Result<(), Error> { + gum::info!(target: LOG_TARGET, "Migrating parachains db from version 1 to version 2 ..."); + + match db_kind { + DatabaseKind::ParityDB => paritydb_migrate_from_version_1_to_2(path), + DatabaseKind::RocksDB => rocksdb_migrate_from_version_1_to_2(path), + } + .and_then(|result| { + gum::info!(target: LOG_TARGET, "Migration complete! "); + Ok(result) + }) +} + /// Migration from version 0 to version 1: /// * the number of columns has changed from 3 to 5; fn rocksdb_migrate_from_version_0_to_1(path: &Path) -> Result<(), Error> { @@ -129,6 +144,22 @@ fn rocksdb_migrate_from_version_0_to_1(path: &Path) -> Result<(), Error> { Ok(()) } +/// Migration from version 1 to version 2: +/// * the number of columns has changed from 5 to 6; +fn rocksdb_migrate_from_version_1_to_2(path: &Path) -> Result<(), Error> { + use kvdb_rocksdb::{Database, DatabaseConfig}; + + let db_path = path + .to_str() + .ok_or_else(|| super::other_io_error("Invalid database path".into()))?; + let db_cfg = DatabaseConfig::with_columns(super::columns::v0::NUM_COLUMNS); + let db = Database::open(&db_cfg, db_path)?; + + db.add_column()?; + + Ok(()) +} + // This currently clears columns which had their configs altered between versions. // The columns to be changed are constrained by the `allowed_columns` vector. fn paritydb_fix_columns( @@ -197,6 +228,17 @@ pub(crate) fn paritydb_version_1_config(path: &Path) -> parity_db::Options { options } +/// Database configuration for version 2. +pub(crate) fn paritydb_version_2_config(path: &Path) -> parity_db::Options { + let mut options = + parity_db::Options::with_columns(&path, super::columns::v2::NUM_COLUMNS as u8); + for i in columns::v1::ORDERED_COL { + options.columns[*i as usize].btree_index = true; + } + + options +} + /// Database configuration for version 0. This is useful just for testing. #[cfg(test)] pub(crate) fn paritydb_version_0_config(path: &Path) -> parity_db::Options { @@ -224,6 +266,12 @@ fn paritydb_migrate_from_version_0_to_1(path: &Path) -> Result<(), Error> { Ok(()) } +/// Migration from version 1 to version 2. +/// Cases covered: a no-op since `paritydb` handles new columns automatically. +fn paritydb_migrate_from_version_1_to_2(_path: &Path) -> Result<(), Error> { + Ok(()) +} + #[cfg(test)] mod tests { #[test] @@ -255,4 +303,38 @@ mod tests { Some("somevalue".as_bytes().to_vec()) ); } + + #[test] + fn test_paritydb_migrate_1_2() { + use super::{columns::v1::*, *}; + use parity_db::Db; + + let db_dir = tempfile::tempdir().unwrap(); + let path = db_dir.path(); + { + let db = Db::open_or_create(&paritydb_version_1_config(&path)).unwrap(); + + db.commit(vec![ + (COL_DISPUTE_COORDINATOR_DATA as u8, b"1234".to_vec(), Some(b"somevalue".to_vec())), + (COL_AVAILABILITY_META as u8, b"5678".to_vec(), Some(b"somevalue".to_vec())), + ]) + .unwrap(); + + assert_eq!(db.num_columns(), super::columns::v1::NUM_COLUMNS as u8); + } + + try_upgrade_db(&path, DatabaseKind::ParityDB).unwrap(); + + let db = Db::open(&paritydb_version_2_config(&path)).unwrap(); + assert_eq!( + db.get(super::columns::v1::COL_DISPUTE_COORDINATOR_DATA as u8, b"1234").unwrap(), + None + ); + assert_eq!( + db.get(super::columns::v1::COL_AVAILABILITY_META as u8, b"5678").unwrap(), + Some("somevalue".as_bytes().to_vec()) + ); + + assert_eq!(db.num_columns(), super::columns::v2::NUM_COLUMNS as u8); + } } From 0a2ecd667631b86822f1caa3dbf4ebcb65dcc583 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 11 Oct 2022 15:48:23 +0000 Subject: [PATCH 26/42] Fix comments Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 48542e9209d4..6b353e7ee8be 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -499,9 +499,7 @@ async fn get_session_index_for_child( } /// Attempts to extend db stored sessions with sessions missing between `start` and up to `end_inclusive`. -/// If `allow_failure` is true, fetching errors are ignored until we get a first session, then -/// the function would return error. This allows us to be more relaxed `wrt` sessions no longer -/// available in the runtime, but not for other types of errors. +/// Runtime session info fetching errors are ignored if that doesn't create a gap in the window. async fn extend_sessions_from_chain_state( db_sessions: Vec, sender: &mut impl overseer::SubsystemSender, @@ -512,6 +510,7 @@ async fn extend_sessions_from_chain_state( // Start from the db sessions. let mut sessions = db_sessions; // We allow session fetch failures only if we won't create a gap in the window by doing so. + // If `allow_failure` is set to true here, fetching errors are ignored until we get a first session. let mut allow_failure = sessions.is_empty(); let start = *window_start + sessions.len() as u32; From c887eca9ed7a0f1da5e1291330be27bfb4a30035 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 12 Oct 2022 16:09:24 +0000 Subject: [PATCH 27/42] add todo Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 6b353e7ee8be..407c570c2e6c 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -240,6 +240,7 @@ impl RollingSessionWindow { } // Saves/Updates all sessions in the database. + // TODO: https://github.com/paritytech/polkadot/issues/6144 fn db_save(&mut self, stored_window: StoredWindow) { if let Some(db_params) = self.db_params.as_ref() { match db_params.db.write(DBTransaction { @@ -501,14 +502,14 @@ async fn get_session_index_for_child( /// Attempts to extend db stored sessions with sessions missing between `start` and up to `end_inclusive`. /// Runtime session info fetching errors are ignored if that doesn't create a gap in the window. async fn extend_sessions_from_chain_state( - db_sessions: Vec, + stored_sessions: Vec, sender: &mut impl overseer::SubsystemSender, block_hash: Hash, window_start: &mut SessionIndex, end_inclusive: SessionIndex, ) -> Result, SessionsUnavailableReason> { // Start from the db sessions. - let mut sessions = db_sessions; + let mut sessions = stored_sessions; // We allow session fetch failures only if we won't create a gap in the window by doing so. // If `allow_failure` is set to true here, fetching errors are ignored until we get a first session. let mut allow_failure = sessions.is_empty(); From 32947c936edd530607320ebff52d622b2baab8e8 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Oct 2022 08:51:24 +0000 Subject: [PATCH 28/42] update to parity-db to "0.4.2" Signed-off-by: Andrei Sandu --- Cargo.lock | 35 ++++++++++++++++++++++++++-------- node/service/Cargo.toml | 2 +- node/subsystem-util/Cargo.toml | 2 +- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d365a54d97ad..60d6e41db168 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4099,9 +4099,9 @@ dependencies = [ [[package]] name = "lz4" -version = "1.23.2" +version = "1.24.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aac20ed6991e01bf6a2e68cc73df2b389707403662a8ba89f68511fb340f724c" +checksum = "7e9e2dd86df36ce760a60f6ff6ad526f7ba1f14ba0356f8254fb6905e6494df1" dependencies = [ "libc", "lz4-sys", @@ -4109,9 +4109,9 @@ dependencies = [ [[package]] name = "lz4-sys" -version = "1.9.2" +version = "1.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dca79aa95d8b3226213ad454d328369853be3a1382d89532a854f4d69640acae" +checksum = "57d27b317e207b10f69f5e75494119e391a96f48861ae870d1da6edac98ca900" dependencies = [ "cc", "libc", @@ -5814,6 +5814,25 @@ dependencies = [ "snap", ] +[[package]] +name = "parity-db" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a7511a0bec4a336b5929999d02b560d2439c993cccf98c26481484e811adc43" +dependencies = [ + "blake2", + "crc32fast", + "fs2", + "hex", + "libc", + "log", + "lz4", + "memmap2 0.5.0", + "parking_lot 0.12.1", + "rand 0.8.5", + "snap", +] + [[package]] name = "parity-scale-codec" version = "3.1.5" @@ -6913,7 +6932,7 @@ dependencies = [ "lazy_static", "log", "lru 0.8.0", - "parity-db", + "parity-db 0.4.2", "parity-scale-codec", "parity-util-mem", "parking_lot 0.11.2", @@ -7308,7 +7327,7 @@ dependencies = [ "pallet-im-online", "pallet-staking", "pallet-transaction-payment-rpc-runtime-api", - "parity-db", + "parity-db 0.4.2", "polkadot-approval-distribution", "polkadot-availability-bitfield-distribution", "polkadot-availability-distribution", @@ -8797,7 +8816,7 @@ dependencies = [ "kvdb-rocksdb", "linked-hash-map", "log", - "parity-db", + "parity-db 0.3.16", "parity-scale-codec", "parking_lot 0.12.1", "sc-client-api", @@ -11922,7 +11941,7 @@ version = "1.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "97fee6b57c6a41524a810daee9286c02d7752c4253064d0b05472833a438f675" dependencies = [ - "cfg-if 0.1.10", + "cfg-if 1.0.0", "digest 0.10.3", "rand 0.8.5", "static_assertions", diff --git a/node/service/Cargo.toml b/node/service/Cargo.toml index 1d2613537814..2fddc5735b7f 100644 --- a/node/service/Cargo.toml +++ b/node/service/Cargo.toml @@ -71,7 +71,7 @@ serde_json = "1.0.81" thiserror = "1.0.31" kvdb = "0.11.0" kvdb-rocksdb = { version = "0.15.2", optional = true } -parity-db = { version = "0.3.16", optional = true } +parity-db = { version = "0.4.2", optional = true } async-trait = "0.1.57" lru = "0.8" diff --git a/node/subsystem-util/Cargo.toml b/node/subsystem-util/Cargo.toml index 41e84e58efad..2365e36eb453 100644 --- a/node/subsystem-util/Cargo.toml +++ b/node/subsystem-util/Cargo.toml @@ -34,7 +34,7 @@ sp-keystore = { git = "https://github.com/paritytech/substrate", branch = "maste kvdb = "0.11.0" parity-util-mem = { version = "0.11", default-features = false } -parity-db = { version = "0.3.13" } +parity-db = { version = "0.4.2"} [dev-dependencies] assert_matches = "1.4.0" From 3d75b6911b5e7b694798fa1172c465c908ff21f1 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Oct 2022 10:46:34 +0000 Subject: [PATCH 29/42] migration complete Signed-off-by: Andrei Sandu --- node/service/src/parachains_db/upgrade.rs | 128 ++++++++++++++++++---- 1 file changed, 105 insertions(+), 23 deletions(-) diff --git a/node/service/src/parachains_db/upgrade.rs b/node/service/src/parachains_db/upgrade.rs index ae774ad10043..9439391c441f 100644 --- a/node/service/src/parachains_db/upgrade.rs +++ b/node/service/src/parachains_db/upgrade.rs @@ -152,7 +152,7 @@ fn rocksdb_migrate_from_version_1_to_2(path: &Path) -> Result<(), Error> { let db_path = path .to_str() .ok_or_else(|| super::other_io_error("Invalid database path".into()))?; - let db_cfg = DatabaseConfig::with_columns(super::columns::v0::NUM_COLUMNS); + let db_cfg = DatabaseConfig::with_columns(super::columns::v1::NUM_COLUMNS); let db = Database::open(&db_cfg, db_path)?; db.add_column()?; @@ -266,17 +266,27 @@ fn paritydb_migrate_from_version_0_to_1(path: &Path) -> Result<(), Error> { Ok(()) } -/// Migration from version 1 to version 2. -/// Cases covered: a no-op since `paritydb` handles new columns automatically. -fn paritydb_migrate_from_version_1_to_2(_path: &Path) -> Result<(), Error> { +/// Migration from version 1 to version 2: +/// - add a new column for session information storage +fn paritydb_migrate_from_version_1_to_2(path: &Path) -> Result<(), Error> { + let mut options = paritydb_version_1_config(path); + + // Adds the session info column. + parity_db::Db::add_column(&mut options, Default::default()) + .map_err(|e| other_io_error(format!("Error adding column {:?}", e)))?; + Ok(()) } #[cfg(test)] mod tests { + use super::{ + columns::{v1::*, v2::*}, + *, + }; + #[test] - fn test_paritydb_migrate_0_1() { - use super::{columns::v1::*, *}; + fn test_paritydb_migrate_0_to_1() { use parity_db::Db; let db_dir = tempfile::tempdir().unwrap(); @@ -294,47 +304,119 @@ mod tests { try_upgrade_db(&path, DatabaseKind::ParityDB).unwrap(); let db = Db::open(&paritydb_version_1_config(&path)).unwrap(); + assert_eq!(db.get(COL_DISPUTE_COORDINATOR_DATA as u8, b"1234").unwrap(), None); assert_eq!( - db.get(super::columns::v1::COL_DISPUTE_COORDINATOR_DATA as u8, b"1234").unwrap(), - None - ); - assert_eq!( - db.get(super::columns::v1::COL_AVAILABILITY_META as u8, b"5678").unwrap(), + db.get(COL_AVAILABILITY_META as u8, b"5678").unwrap(), Some("somevalue".as_bytes().to_vec()) ); } #[test] - fn test_paritydb_migrate_1_2() { - use super::{columns::v1::*, *}; + fn test_paritydb_migrate_1_to_2() { use parity_db::Db; let db_dir = tempfile::tempdir().unwrap(); let path = db_dir.path(); + + // We need to properly set db version for upgrade to work. + fs::write(version_file_path(path), "1").expect("Failed to write DB version"); + { let db = Db::open_or_create(&paritydb_version_1_config(&path)).unwrap(); - db.commit(vec![ - (COL_DISPUTE_COORDINATOR_DATA as u8, b"1234".to_vec(), Some(b"somevalue".to_vec())), - (COL_AVAILABILITY_META as u8, b"5678".to_vec(), Some(b"somevalue".to_vec())), - ]) + // Write some dummy data + db.commit(vec![( + COL_DISPUTE_COORDINATOR_DATA as u8, + b"1234".to_vec(), + Some(b"somevalue".to_vec()), + )]) .unwrap(); - assert_eq!(db.num_columns(), super::columns::v1::NUM_COLUMNS as u8); + assert_eq!(db.num_columns(), columns::v1::NUM_COLUMNS as u8); } try_upgrade_db(&path, DatabaseKind::ParityDB).unwrap(); let db = Db::open(&paritydb_version_2_config(&path)).unwrap(); + + assert_eq!(db.num_columns(), columns::v2::NUM_COLUMNS as u8); + assert_eq!( - db.get(super::columns::v1::COL_DISPUTE_COORDINATOR_DATA as u8, b"1234").unwrap(), - None + db.get(COL_DISPUTE_COORDINATOR_DATA as u8, b"1234").unwrap(), + Some("somevalue".as_bytes().to_vec()) ); + + // Test we can write the new column. + db.commit(vec![( + COL_SESSION_WINDOW_DATA as u8, + b"1337".to_vec(), + Some(b"0xdeadb00b".to_vec()), + )]) + .unwrap(); + + // Read back data from new column. assert_eq!( - db.get(super::columns::v1::COL_AVAILABILITY_META as u8, b"5678").unwrap(), - Some("somevalue".as_bytes().to_vec()) + db.get(COL_SESSION_WINDOW_DATA as u8, b"1337").unwrap(), + Some("0xdeadb00b".as_bytes().to_vec()) ); + } + + #[test] + fn test_rocksdb_migrate_1_to_2() { + use kvdb::{DBKey, DBOp}; + use kvdb_rocksdb::{Database, DatabaseConfig}; + use polkadot_node_subsystem_util::database::{ + kvdb_impl::DbAdapter, DBTransaction, KeyValueDB, + }; + + let db_dir = tempfile::tempdir().unwrap(); + let db_path = db_dir.path().to_str().unwrap(); + let db_cfg = DatabaseConfig::with_columns(super::columns::v1::NUM_COLUMNS); + let db = Database::open(&db_cfg, db_path).unwrap(); + assert_eq!(db.num_columns(), super::columns::v1::NUM_COLUMNS as u32); + + // We need to properly set db version for upgrade to work. + fs::write(version_file_path(db_dir.path()), "1").expect("Failed to write DB version"); + { + let db = DbAdapter::new(db, columns::v1::ORDERED_COL); + db.write(DBTransaction { + ops: vec![DBOp::Insert { + col: COL_DISPUTE_COORDINATOR_DATA, + key: DBKey::from_slice(b"1234"), + value: b"0xdeadb00b".to_vec(), + }], + }) + .unwrap(); + } - assert_eq!(db.num_columns(), super::columns::v2::NUM_COLUMNS as u8); + try_upgrade_db(&db_dir.path(), DatabaseKind::RocksDB).unwrap(); + + let db_cfg = DatabaseConfig::with_columns(super::columns::v2::NUM_COLUMNS); + let db = Database::open(&db_cfg, db_path).unwrap(); + + assert_eq!(db.num_columns(), super::columns::v2::NUM_COLUMNS); + + let db = DbAdapter::new(db, columns::v1::ORDERED_COL); + + assert_eq!( + db.get(COL_DISPUTE_COORDINATOR_DATA, b"1234").unwrap(), + Some("0xdeadb00b".as_bytes().to_vec()) + ); + + // Test we can write the new column. + db.write(DBTransaction { + ops: vec![DBOp::Insert { + col: COL_SESSION_WINDOW_DATA, + key: DBKey::from_slice(b"1337"), + value: b"0xdeadb00b".to_vec(), + }], + }) + .unwrap(); + + // Read back data from new column. + assert_eq!( + db.get(COL_SESSION_WINDOW_DATA, b"1337").unwrap(), + Some("0xdeadb00b".as_bytes().to_vec()) + ); } } From a17770d10d76263c0e9e2db806f2304ef7d46664 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Oct 2022 10:54:37 +0000 Subject: [PATCH 30/42] One session window size Signed-off-by: Andrei Sandu --- node/core/approval-voting/src/import.rs | 27 ++---- node/core/approval-voting/src/lib.rs | 7 +- node/core/dispute-coordinator/src/lib.rs | 2 +- .../src/rolling_session_window.rs | 85 +++++++++---------- 4 files changed, 49 insertions(+), 72 deletions(-) diff --git a/node/core/approval-voting/src/import.rs b/node/core/approval-voting/src/import.rs index 2b51ff5f3af9..a86171e2db63 100644 --- a/node/core/approval-voting/src/import.rs +++ b/node/core/approval-voting/src/import.rs @@ -671,11 +671,7 @@ pub(crate) mod tests { fn single_session_state(index: SessionIndex, info: SessionInfo) -> State { State { - session_window: Some(RollingSessionWindow::with_session_info( - APPROVAL_SESSIONS, - index, - vec![info], - )), + session_window: Some(RollingSessionWindow::with_session_info(index, vec![info])), ..blank_state() } } @@ -788,11 +784,8 @@ pub(crate) mod tests { .map(|(r, c, g)| (r.hash(), r.clone(), *c, *g)) .collect::>(); - let session_window = RollingSessionWindow::with_session_info( - APPROVAL_SESSIONS, - session, - vec![session_info], - ); + let session_window = + RollingSessionWindow::with_session_info(session, vec![session_info]); let header = header.clone(); Box::pin(async move { @@ -897,11 +890,8 @@ pub(crate) mod tests { .collect::>(); let test_fut = { - let session_window = RollingSessionWindow::with_session_info( - APPROVAL_SESSIONS, - session, - vec![session_info], - ); + let session_window = + RollingSessionWindow::with_session_info(session, vec![session_info]); let header = header.clone(); Box::pin(async move { @@ -1095,11 +1085,8 @@ pub(crate) mod tests { .map(|(r, c, g)| (r.hash(), r.clone(), *c, *g)) .collect::>(); - let session_window = Some(RollingSessionWindow::with_session_info( - APPROVAL_SESSIONS, - session, - vec![session_info], - )); + let session_window = + Some(RollingSessionWindow::with_session_info(session, vec![session_info])); let header = header.clone(); Box::pin(async move { diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index 9d814c2cf6c4..a0c417af5725 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -44,8 +44,8 @@ use polkadot_node_subsystem_util::{ database::Database, metrics::{self, prometheus}, rolling_session_window::{ - new_session_window_size, DatabaseParams, RollingSessionWindow, SessionWindowSize, - SessionWindowUpdate, SessionsUnavailable, + DatabaseParams, RollingSessionWindow, SessionWindowSize, SessionWindowUpdate, + SessionsUnavailable, }, TimeoutExt, }; @@ -97,8 +97,6 @@ use crate::{ #[cfg(test)] mod tests; -pub const APPROVAL_SESSIONS: SessionWindowSize = new_session_window_size!(6); - const APPROVAL_CHECKING_TIMEOUT: Duration = Duration::from_secs(120); /// How long are we willing to wait for approval signatures? /// @@ -650,7 +648,6 @@ impl State { self.session_window = Some( RollingSessionWindow::new( sender, - APPROVAL_SESSIONS, head, DatabaseParams { db: self.db.clone(), diff --git a/node/core/dispute-coordinator/src/lib.rs b/node/core/dispute-coordinator/src/lib.rs index 3bdfa0f2363f..aa434636644c 100644 --- a/node/core/dispute-coordinator/src/lib.rs +++ b/node/core/dispute-coordinator/src/lib.rs @@ -368,7 +368,7 @@ async fn get_rolling_session_window( let sender = ctx.sender().clone(); Ok(Some(( leaf.clone(), - RollingSessionWindow::new(sender, DISPUTE_WINDOW, leaf.hash, db_params) + RollingSessionWindow::new(sender, leaf.hash, db_params) .await .map_err(JfyiError::RollingSessionWindow)?, ))) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 407c570c2e6c..26245522158b 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -34,6 +34,7 @@ use polkadot_node_subsystem::{ overseer, }; +const SESSION_WINDOW_SIZE: SessionWindowSize = new_session_window_size!(6); const LOG_TARGET: &str = "parachain::rolling-session-window"; const STORED_ROLLING_SESSION_WINDOW: &[u8] = b"Rolling_session_window"; @@ -131,7 +132,6 @@ impl RollingSessionWindow { /// we can always extend the session info vector using chain state. pub async fn new( mut sender: Sender, - window_size: SessionWindowSize, block_hash: Hash, db_params: DatabaseParams, ) -> Result @@ -146,7 +146,7 @@ impl RollingSessionWindow { // This will increase the session window to cover the full unfinalized chain. let on_chain_window_start = std::cmp::min( - session_index.saturating_sub(window_size.get() - 1), + session_index.saturating_sub(SESSION_WINDOW_SIZE.get() - 1), earliest_non_finalized_block_session, ); @@ -213,7 +213,7 @@ impl RollingSessionWindow { Ok(Self { earliest_session: window_start, session_info: sessions, - window_size, + window_size: SESSION_WINDOW_SIZE, db_params: Some(db_params), }) } @@ -262,11 +262,15 @@ impl RollingSessionWindow { /// initial data. /// This is only used in `approval voting` tests. pub fn with_session_info( - window_size: SessionWindowSize, earliest_session: SessionIndex, session_info: Vec, ) -> Self { - RollingSessionWindow { earliest_session, session_info, window_size, db_params: None } + RollingSessionWindow { + earliest_session, + session_info, + window_size: SESSION_WINDOW_SIZE, + db_params: None, + } } /// Access the session info for the given session index, if stored within the window. @@ -586,7 +590,6 @@ mod tests { use polkadot_primitives::v2::Header; use sp_core::testing::TaskExecutor; - pub const TEST_WINDOW_SIZE: SessionWindowSize = new_session_window_size!(6); const SESSION_DATA_COL: u32 = 0; const NUM_COLUMNS: u32 = 1; @@ -653,9 +656,7 @@ mod tests { Box::pin(async move { let window = match window { None => - RollingSessionWindow::new(sender.clone(), TEST_WINDOW_SIZE, hash, db_params) - .await - .unwrap(), + RollingSessionWindow::new(sender.clone(), hash, db_params).await.unwrap(), Some(mut window) => { window.cache_session_info_for_head(sender, hash).await.unwrap(); window @@ -738,24 +739,24 @@ mod tests { let db_params = dummy_db_params(); let window = cache_session_info_test( - (10 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (10 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), 10, None, - (10 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (10 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), Some(db_params.clone()), ); let window = cache_session_info_test( - (11 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (11 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), 11, Some(window), 11, None, ); - assert_eq!(window.session_info.len(), TEST_WINDOW_SIZE.get() as usize); + assert_eq!(window.session_info.len(), SESSION_WINDOW_SIZE.get() as usize); cache_session_info_test( - (11 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (11 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), 12, None, 12, @@ -773,7 +774,7 @@ mod tests { let window = RollingSessionWindow { earliest_session: 1, session_info: vec![dummy_session_info(1)], - window_size: TEST_WINDOW_SIZE, + window_size: SESSION_WINDOW_SIZE, db_params: Some(dummy_db_params()), }; @@ -783,10 +784,10 @@ mod tests { #[test] fn cache_session_info_first_late() { cache_session_info_test( - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (100 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), 100, None, - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (100 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), None, ); } @@ -800,31 +801,31 @@ mod tests { dummy_session_info(51), dummy_session_info(52), ], - window_size: TEST_WINDOW_SIZE, + window_size: SESSION_WINDOW_SIZE, db_params: Some(dummy_db_params()), }; cache_session_info_test( - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (100 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), 100, Some(window), - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (100 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), None, ); } #[test] fn cache_session_info_roll_full() { - let start = 99 - (TEST_WINDOW_SIZE.get() - 1); + let start = 99 - (SESSION_WINDOW_SIZE.get() - 1); let window = RollingSessionWindow { earliest_session: start, session_info: (start..=99).map(dummy_session_info).collect(), - window_size: TEST_WINDOW_SIZE, + window_size: SESSION_WINDOW_SIZE, db_params: Some(dummy_db_params()), }; cache_session_info_test( - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (100 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), 100, Some(window), 100, // should only make one request. @@ -835,16 +836,16 @@ mod tests { #[test] fn cache_session_info_roll_many_full_db() { let db_params = dummy_db_params(); - let start = 97 - (TEST_WINDOW_SIZE.get() - 1); + let start = 97 - (SESSION_WINDOW_SIZE.get() - 1); let window = RollingSessionWindow { earliest_session: start, session_info: (start..=97).map(dummy_session_info).collect(), - window_size: TEST_WINDOW_SIZE, + window_size: SESSION_WINDOW_SIZE, db_params: Some(db_params.clone()), }; cache_session_info_test( - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (100 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), 100, Some(window), 98, @@ -853,7 +854,7 @@ mod tests { // We expect the session to be populated from DB, and only fetch 101 from on chain. cache_session_info_test( - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (100 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), 101, None, 101, @@ -863,21 +864,21 @@ mod tests { // Session warps in the future. let window = cache_session_info_test(195, 200, None, 195, Some(db_params)); - assert_eq!(window.session_info.len(), TEST_WINDOW_SIZE.get() as usize); + assert_eq!(window.session_info.len(), SESSION_WINDOW_SIZE.get() as usize); } #[test] fn cache_session_info_roll_many_full() { - let start = 97 - (TEST_WINDOW_SIZE.get() - 1); + let start = 97 - (SESSION_WINDOW_SIZE.get() - 1); let window = RollingSessionWindow { earliest_session: start, session_info: (start..=97).map(dummy_session_info).collect(), - window_size: TEST_WINDOW_SIZE, + window_size: SESSION_WINDOW_SIZE, db_params: Some(dummy_db_params()), }; cache_session_info_test( - (100 as SessionIndex).saturating_sub(TEST_WINDOW_SIZE.get() - 1), + (100 as SessionIndex).saturating_sub(SESSION_WINDOW_SIZE.get() - 1), 100, Some(window), 98, @@ -891,7 +892,7 @@ mod tests { let window = RollingSessionWindow { earliest_session: start, session_info: (0..=1).map(dummy_session_info).collect(), - window_size: TEST_WINDOW_SIZE, + window_size: SESSION_WINDOW_SIZE, db_params: Some(dummy_db_params()), }; @@ -910,7 +911,7 @@ mod tests { let window = RollingSessionWindow { earliest_session: start, session_info: (0..=1).map(dummy_session_info).collect(), - window_size: TEST_WINDOW_SIZE, + window_size: SESSION_WINDOW_SIZE, db_params: Some(dummy_db_params()), }; @@ -949,9 +950,7 @@ mod tests { let test_fut = { let sender = ctx.sender().clone(); Box::pin(async move { - let res = - RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash, dummy_db_params()) - .await; + let res = RollingSessionWindow::new(sender, hash, dummy_db_params()).await; assert!(res.is_err()); }) @@ -1076,9 +1075,7 @@ mod tests { let test_fut = { let sender = ctx.sender().clone(); Box::pin(async move { - let res = - RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash, dummy_db_params()) - .await; + let res = RollingSessionWindow::new(sender, hash, dummy_db_params()).await; assert!(res.is_ok()); let rsw = res.unwrap(); // Since first 50 sessions are missing the earliest should be 50. @@ -1158,7 +1155,7 @@ mod tests { #[test] fn any_session_unavailable_for_caching_means_no_change() { let session: SessionIndex = 6; - let start_session = session.saturating_sub(TEST_WINDOW_SIZE.get() - 1); + let start_session = session.saturating_sub(SESSION_WINDOW_SIZE.get() - 1); let header = Header { digest: Default::default(), @@ -1184,9 +1181,7 @@ mod tests { let test_fut = { let sender = ctx.sender().clone(); Box::pin(async move { - let res = - RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash, dummy_db_params()) - .await; + let res = RollingSessionWindow::new(sender, hash, dummy_db_params()).await; assert!(res.is_err()); }) }; @@ -1278,9 +1273,7 @@ mod tests { Box::pin(async move { let sender = ctx.sender().clone(); let window = - RollingSessionWindow::new(sender, TEST_WINDOW_SIZE, hash, dummy_db_params()) - .await - .unwrap(); + RollingSessionWindow::new(sender, hash, dummy_db_params()).await.unwrap(); assert_eq!(window.earliest_session, session); assert_eq!(window.session_info, vec![dummy_session_info(session)]); From f91681c3a99ef46939b93abd888e41a5da36f4a7 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Oct 2022 10:57:47 +0000 Subject: [PATCH 31/42] fix merge damage Signed-off-by: Andrei Sandu --- node/subsystem-util/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/subsystem-util/Cargo.toml b/node/subsystem-util/Cargo.toml index 2b2fd3d12087..d5654701d2e6 100644 --- a/node/subsystem-util/Cargo.toml +++ b/node/subsystem-util/Cargo.toml @@ -46,4 +46,4 @@ lazy_static = "1.4.0" polkadot-primitives-test-helpers = { path = "../../primitives/test-helpers" } kvdb-shared-tests = "0.10.0" tempfile = "3.1.0" -kvdb-memorydb = "0.11.0" +kvdb-memorydb = "0.12.0" From 5a5e7c678ef1bf677dc7dceef315046215486286 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Oct 2022 11:51:00 +0000 Subject: [PATCH 32/42] fix build errors Signed-off-by: Andrei Sandu --- node/core/approval-voting/src/lib.rs | 2 +- node/service/src/parachains_db/upgrade.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index a0c417af5725..9652cfeaacf8 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -44,7 +44,7 @@ use polkadot_node_subsystem_util::{ database::Database, metrics::{self, prometheus}, rolling_session_window::{ - DatabaseParams, RollingSessionWindow, SessionWindowSize, SessionWindowUpdate, + DatabaseParams, RollingSessionWindow, SessionWindowUpdate, SessionsUnavailable, }, TimeoutExt, diff --git a/node/service/src/parachains_db/upgrade.rs b/node/service/src/parachains_db/upgrade.rs index f0161664d688..2ff9fb8edafe 100644 --- a/node/service/src/parachains_db/upgrade.rs +++ b/node/service/src/parachains_db/upgrade.rs @@ -153,7 +153,7 @@ fn rocksdb_migrate_from_version_1_to_2(path: &Path) -> Result<(), Error> { .to_str() .ok_or_else(|| super::other_io_error("Invalid database path".into()))?; let db_cfg = DatabaseConfig::with_columns(super::columns::v1::NUM_COLUMNS); - let db = Database::open(&db_cfg, db_path)?; + let mut db = Database::open(&db_cfg, db_path)?; db.add_column()?; From cf05ac64f8d0b74078b27193e5b2b0200b1dcccb Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Oct 2022 11:51:12 +0000 Subject: [PATCH 33/42] fmt Signed-off-by: Andrei Sandu --- node/core/approval-voting/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index 9652cfeaacf8..a45d0d026bb2 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -44,8 +44,7 @@ use polkadot_node_subsystem_util::{ database::Database, metrics::{self, prometheus}, rolling_session_window::{ - DatabaseParams, RollingSessionWindow, SessionWindowUpdate, - SessionsUnavailable, + DatabaseParams, RollingSessionWindow, SessionWindowUpdate, SessionsUnavailable, }, TimeoutExt, }; From dcb31c9a3a7f7042c33f3046230199b469d8d059 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Oct 2022 11:58:21 +0000 Subject: [PATCH 34/42] comment fix Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 26245522158b..2186c8fc77f2 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -178,7 +178,7 @@ impl RollingSessionWindow { (on_chain_window_start, Vec::new()) }; - // Try to load sessions from DB first and load more from chain state if needed. + // Compute the amount of sessions missing from the window that will be fetched from chain state. let sessions_missing_count = session_index .saturating_sub(window_start) .saturating_add(1) From 47765bc5b8eb49c3361d5427af09cd0bbf9dd7c9 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Thu, 13 Oct 2022 12:54:51 +0000 Subject: [PATCH 35/42] fix build Signed-off-by: Andrei Sandu --- node/core/approval-voting/src/import.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/node/core/approval-voting/src/import.rs b/node/core/approval-voting/src/import.rs index a86171e2db63..01524b714a0e 100644 --- a/node/core/approval-voting/src/import.rs +++ b/node/core/approval-voting/src/import.rs @@ -630,9 +630,7 @@ pub(crate) mod tests { pub(crate) use sp_runtime::{Digest, DigestItem}; use std::{pin::Pin, sync::Arc}; - use crate::{ - approval_db::v1::Config as DatabaseConfig, criteria, BlockEntry, APPROVAL_SESSIONS, - }; + use crate::{approval_db::v1::Config as DatabaseConfig, criteria, BlockEntry}; const DATA_COL: u32 = 0; const SESSION_DATA_COL: u32 = 1; From 81d7e05aa9ab262a9ee9615c96ead70d70b4ea96 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 14 Oct 2022 14:13:06 +0000 Subject: [PATCH 36/42] make error more explicit Signed-off-by: Andrei Sandu --- node/service/src/parachains_db/upgrade.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/service/src/parachains_db/upgrade.rs b/node/service/src/parachains_db/upgrade.rs index 2ff9fb8edafe..05e6ea4cb725 100644 --- a/node/service/src/parachains_db/upgrade.rs +++ b/node/service/src/parachains_db/upgrade.rs @@ -36,7 +36,7 @@ pub enum Error { Io(#[from] io::Error), #[error("The version file format is incorrect")] CorruptedVersionFile, - #[error("Future version (expected {current:?}, found {got:?})")] + #[error("Parachains DB has a future version (expected {current:?}, found {got:?})")] FutureVersion { current: Version, got: Version }, } From 516f23efe38b8d985ec6808be5f76cf3f512c7b7 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Fri, 14 Oct 2022 14:13:23 +0000 Subject: [PATCH 37/42] add comment Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 2186c8fc77f2..4c3791f1efd2 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -34,6 +34,8 @@ use polkadot_node_subsystem::{ overseer, }; +// The window size is equal to the `approval-voting` and `dispute-coordinator` constants that +// have been obsoleted. const SESSION_WINDOW_SIZE: SessionWindowSize = new_session_window_size!(6); const LOG_TARGET: &str = "parachain::rolling-session-window"; const STORED_ROLLING_SESSION_WINDOW: &[u8] = b"Rolling_session_window"; From 316f5ed57698e79f37aa27d8925af1047b59f7e1 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 8 Nov 2022 11:59:22 +0000 Subject: [PATCH 38/42] refactor conflict merge Signed-off-by: Andrei Sandu --- node/core/approval-voting/src/lib.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index 37e0f1dd8f7f..ec20a4b79502 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -768,10 +768,7 @@ async fn run( where B: Backend, { - let db = subsystem.db.clone(); - let db_config = subsystem.db_config.clone(); - - if let Err(err) = db_sanity_check(subsystem.db, subsystem.db_config) { + if let Err(err) = db_sanity_check(subsystem.db.clone(), subsystem.db_config.clone()) { gum::warn!(target: LOG_TARGET, ?err, "Could not run approval vote DB sanity check"); } @@ -781,8 +778,8 @@ where slot_duration_millis: subsystem.slot_duration_millis, clock, assignment_criteria, - db_config, - db, + db_config: subsystem.db_config, + db: subsystem.db, }; let mut wakeups = Wakeups::default(); From 3b45f235238ddef2d9b52a5b518b7d3180513fe2 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 8 Nov 2022 12:10:33 +0000 Subject: [PATCH 39/42] rename col_data Signed-off-by: Andrei Sandu --- .../approval-voting/src/approval_db/v1/mod.rs | 34 ++++++++------- .../src/approval_db/v1/tests.rs | 3 +- node/core/approval-voting/src/import.rs | 2 +- node/core/approval-voting/src/lib.rs | 6 +-- node/core/approval-voting/src/tests.rs | 4 +- node/core/dispute-coordinator/src/db/v1.rs | 43 +++++++++++++------ node/core/dispute-coordinator/src/lib.rs | 4 +- node/core/dispute-coordinator/src/tests.rs | 2 +- node/service/src/lib.rs | 4 +- 9 files changed, 61 insertions(+), 41 deletions(-) diff --git a/node/core/approval-voting/src/approval_db/v1/mod.rs b/node/core/approval-voting/src/approval_db/v1/mod.rs index 0af76d28c362..858bcb8c36fe 100644 --- a/node/core/approval-voting/src/approval_db/v1/mod.rs +++ b/node/core/approval-voting/src/approval_db/v1/mod.rs @@ -90,41 +90,45 @@ impl Backend for DbBackend { match op { BackendWriteOp::WriteStoredBlockRange(stored_block_range) => { tx.put_vec( - self.config.col_data, + self.config.col_approval_data, &STORED_BLOCKS_KEY, stored_block_range.encode(), ); }, BackendWriteOp::DeleteStoredBlockRange => { - tx.delete(self.config.col_data, &STORED_BLOCKS_KEY); + tx.delete(self.config.col_approval_data, &STORED_BLOCKS_KEY); }, BackendWriteOp::WriteBlocksAtHeight(h, blocks) => { - tx.put_vec(self.config.col_data, &blocks_at_height_key(h), blocks.encode()); + tx.put_vec( + self.config.col_approval_data, + &blocks_at_height_key(h), + blocks.encode(), + ); }, BackendWriteOp::DeleteBlocksAtHeight(h) => { - tx.delete(self.config.col_data, &blocks_at_height_key(h)); + tx.delete(self.config.col_approval_data, &blocks_at_height_key(h)); }, BackendWriteOp::WriteBlockEntry(block_entry) => { let block_entry: BlockEntry = block_entry.into(); tx.put_vec( - self.config.col_data, + self.config.col_approval_data, &block_entry_key(&block_entry.block_hash), block_entry.encode(), ); }, BackendWriteOp::DeleteBlockEntry(hash) => { - tx.delete(self.config.col_data, &block_entry_key(&hash)); + tx.delete(self.config.col_approval_data, &block_entry_key(&hash)); }, BackendWriteOp::WriteCandidateEntry(candidate_entry) => { let candidate_entry: CandidateEntry = candidate_entry.into(); tx.put_vec( - self.config.col_data, + self.config.col_approval_data, &candidate_entry_key(&candidate_entry.candidate.hash()), candidate_entry.encode(), ); }, BackendWriteOp::DeleteCandidateEntry(candidate_hash) => { - tx.delete(self.config.col_data, &candidate_entry_key(&candidate_hash)); + tx.delete(self.config.col_approval_data, &candidate_entry_key(&candidate_hash)); }, } } @@ -149,7 +153,7 @@ pub type Bitfield = BitVec; #[derive(Debug, Clone, Copy)] pub struct Config { /// The column family in the database where data is stored. - pub col_data: u32, + pub col_approval_data: u32, /// The column of the database where rolling session window data is stored. pub col_session_data: u32, } @@ -245,10 +249,10 @@ pub type Result = std::result::Result; pub(crate) fn load_decode( store: &dyn Database, - col_data: u32, + col_approval_data: u32, key: &[u8], ) -> Result> { - match store.get(col_data, key)? { + match store.get(col_approval_data, key)? { None => Ok(None), Some(raw) => D::decode(&mut &raw[..]).map(Some).map_err(Into::into), } @@ -305,7 +309,7 @@ pub fn load_stored_blocks( store: &dyn Database, config: &Config, ) -> SubsystemResult> { - load_decode(store, config.col_data, STORED_BLOCKS_KEY) + load_decode(store, config.col_approval_data, STORED_BLOCKS_KEY) .map_err(|e| SubsystemError::with_origin("approval-voting", e)) } @@ -315,7 +319,7 @@ pub fn load_blocks_at_height( config: &Config, block_number: &BlockNumber, ) -> SubsystemResult> { - load_decode(store, config.col_data, &blocks_at_height_key(*block_number)) + load_decode(store, config.col_approval_data, &blocks_at_height_key(*block_number)) .map(|x| x.unwrap_or_default()) .map_err(|e| SubsystemError::with_origin("approval-voting", e)) } @@ -326,7 +330,7 @@ pub fn load_block_entry( config: &Config, block_hash: &Hash, ) -> SubsystemResult> { - load_decode(store, config.col_data, &block_entry_key(block_hash)) + load_decode(store, config.col_approval_data, &block_entry_key(block_hash)) .map(|u: Option| u.map(|v| v.into())) .map_err(|e| SubsystemError::with_origin("approval-voting", e)) } @@ -337,7 +341,7 @@ pub fn load_candidate_entry( config: &Config, candidate_hash: &CandidateHash, ) -> SubsystemResult> { - load_decode(store, config.col_data, &candidate_entry_key(candidate_hash)) + load_decode(store, config.col_approval_data, &candidate_entry_key(candidate_hash)) .map(|u: Option| u.map(|v| v.into())) .map_err(|e| SubsystemError::with_origin("approval-voting", e)) } diff --git a/node/core/approval-voting/src/approval_db/v1/tests.rs b/node/core/approval-voting/src/approval_db/v1/tests.rs index d939b3cf5a05..06923c6a539f 100644 --- a/node/core/approval-voting/src/approval_db/v1/tests.rs +++ b/node/core/approval-voting/src/approval_db/v1/tests.rs @@ -32,7 +32,8 @@ const SESSION_DATA_COL: u32 = 1; const NUM_COLUMNS: u32 = 2; -const TEST_CONFIG: Config = Config { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; +const TEST_CONFIG: Config = + Config { col_approval_data: DATA_COL, col_session_data: SESSION_DATA_COL }; fn make_db() -> (DbBackend, Arc) { let db = kvdb_memorydb::create(NUM_COLUMNS); diff --git a/node/core/approval-voting/src/import.rs b/node/core/approval-voting/src/import.rs index 1989f9ec56f8..20629dd022d4 100644 --- a/node/core/approval-voting/src/import.rs +++ b/node/core/approval-voting/src/import.rs @@ -640,7 +640,7 @@ pub(crate) mod tests { const NUM_COLUMNS: u32 = 2; const TEST_CONFIG: DatabaseConfig = - DatabaseConfig { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; + DatabaseConfig { col_approval_data: DATA_COL, col_session_data: SESSION_DATA_COL }; #[derive(Default)] struct MockClock; diff --git a/node/core/approval-voting/src/lib.rs b/node/core/approval-voting/src/lib.rs index ec20a4b79502..b96992df2c88 100644 --- a/node/core/approval-voting/src/lib.rs +++ b/node/core/approval-voting/src/lib.rs @@ -115,7 +115,7 @@ const LOG_TARGET: &str = "parachain::approval-voting"; #[derive(Debug, Clone)] pub struct Config { /// The column family in the DB where approval-voting data is stored. - pub col_data: u32, + pub col_approval_data: u32, /// The of the DB where rolling session info is stored. pub col_session_data: u32, /// The slot duration of the consensus algorithm, in milliseconds. Should be evenly @@ -358,7 +358,7 @@ impl ApprovalVotingSubsystem { slot_duration_millis: config.slot_duration_millis, db, db_config: DatabaseConfig { - col_data: config.col_data, + col_approval_data: config.col_approval_data, col_session_data: config.col_session_data, }, mode: Mode::Syncing(sync_oracle), @@ -370,7 +370,7 @@ impl ApprovalVotingSubsystem { /// The operation is not allowed for blocks older than the last finalized one. pub fn revert_to(&self, hash: Hash) -> Result<(), SubsystemError> { let config = approval_db::v1::Config { - col_data: self.db_config.col_data, + col_approval_data: self.db_config.col_approval_data, col_session_data: self.db_config.col_session_data, }; let mut backend = approval_db::v1::DbBackend::new(self.db.clone(), config); diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs index 0bbfec039ca3..b9063c8ade25 100644 --- a/node/core/approval-voting/src/tests.rs +++ b/node/core/approval-voting/src/tests.rs @@ -118,7 +118,7 @@ pub mod test_constants { pub(crate) const NUM_COLUMNS: u32 = 2; pub(crate) const TEST_CONFIG: DatabaseConfig = - DatabaseConfig { col_data: DATA_COL, col_session_data: SESSION_DATA_COL }; + DatabaseConfig { col_approval_data: DATA_COL, col_session_data: SESSION_DATA_COL }; } struct MockSupportsParachains; @@ -492,7 +492,7 @@ fn test_harness>( context, ApprovalVotingSubsystem::with_config( Config { - col_data: test_constants::TEST_CONFIG.col_data, + col_approval_data: test_constants::TEST_CONFIG.col_approval_data, slot_duration_millis: SLOT_DURATION_MILLIS, col_session_data: TEST_CONFIG.col_session_data, }, diff --git a/node/core/dispute-coordinator/src/db/v1.rs b/node/core/dispute-coordinator/src/db/v1.rs index c637524bd7ce..bb1456a59745 100644 --- a/node/core/dispute-coordinator/src/db/v1.rs +++ b/node/core/dispute-coordinator/src/db/v1.rs @@ -99,10 +99,10 @@ impl DbBackend { encoded = ?candidate_votes_session_prefix(index), "Cleaning votes for session index" ); - tx.delete_prefix(self.config.col_data, &candidate_votes_session_prefix(index)); + tx.delete_prefix(self.config.col_dispute_data, &candidate_votes_session_prefix(index)); } // New watermark: - tx.put_vec(self.config.col_data, CLEANED_VOTES_WATERMARK_KEY, clean_until.encode()); + tx.put_vec(self.config.col_dispute_data, CLEANED_VOTES_WATERMARK_KEY, clean_until.encode()); Ok(()) } } @@ -148,21 +148,32 @@ impl Backend for DbBackend { self.add_vote_cleanup_tx(&mut tx, session)?; // Actually write the earliest session. - tx.put_vec(self.config.col_data, EARLIEST_SESSION_KEY, session.encode()); + tx.put_vec( + self.config.col_dispute_data, + EARLIEST_SESSION_KEY, + session.encode(), + ); }, BackendWriteOp::WriteRecentDisputes(recent_disputes) => { - tx.put_vec(self.config.col_data, RECENT_DISPUTES_KEY, recent_disputes.encode()); + tx.put_vec( + self.config.col_dispute_data, + RECENT_DISPUTES_KEY, + recent_disputes.encode(), + ); }, BackendWriteOp::WriteCandidateVotes(session, candidate_hash, votes) => { gum::trace!(target: LOG_TARGET, ?session, "Writing candidate votes"); tx.put_vec( - self.config.col_data, + self.config.col_dispute_data, &candidate_votes_key(session, &candidate_hash), votes.encode(), ); }, BackendWriteOp::DeleteCandidateVotes(session, candidate_hash) => { - tx.delete(self.config.col_data, &candidate_votes_key(session, &candidate_hash)); + tx.delete( + self.config.col_dispute_data, + &candidate_votes_key(session, &candidate_hash), + ); }, } } @@ -195,7 +206,7 @@ fn candidate_votes_session_prefix(session: SessionIndex) -> [u8; 15 + 4] { #[derive(Debug, Clone)] pub struct ColumnConfiguration { /// The column in the key-value DB where data is stored. - pub col_data: u32, + pub col_dispute_data: u32, /// The column in the key-value DB where session data is stored. pub col_session_data: u32, } @@ -259,8 +270,12 @@ impl From for crate::error::Error { /// Result alias for DB errors. pub type Result = std::result::Result; -fn load_decode(db: &dyn Database, col_data: u32, key: &[u8]) -> Result> { - match db.get(col_data, key)? { +fn load_decode( + db: &dyn Database, + col_dispute_data: u32, + key: &[u8], +) -> Result> { + match db.get(col_dispute_data, key)? { None => Ok(None), Some(raw) => D::decode(&mut &raw[..]).map(Some).map_err(Into::into), } @@ -273,7 +288,7 @@ pub(crate) fn load_candidate_votes( session: SessionIndex, candidate_hash: &CandidateHash, ) -> SubsystemResult> { - load_decode(db, config.col_data, &candidate_votes_key(session, candidate_hash)) + load_decode(db, config.col_dispute_data, &candidate_votes_key(session, candidate_hash)) .map_err(|e| SubsystemError::with_origin("dispute-coordinator", e)) } @@ -282,7 +297,7 @@ pub(crate) fn load_earliest_session( db: &dyn Database, config: &ColumnConfiguration, ) -> SubsystemResult> { - load_decode(db, config.col_data, EARLIEST_SESSION_KEY) + load_decode(db, config.col_dispute_data, EARLIEST_SESSION_KEY) .map_err(|e| SubsystemError::with_origin("dispute-coordinator", e)) } @@ -291,7 +306,7 @@ pub(crate) fn load_recent_disputes( db: &dyn Database, config: &ColumnConfiguration, ) -> SubsystemResult> { - load_decode(db, config.col_data, RECENT_DISPUTES_KEY) + load_decode(db, config.col_dispute_data, RECENT_DISPUTES_KEY) .map_err(|e| SubsystemError::with_origin("dispute-coordinator", e)) } @@ -349,7 +364,7 @@ fn load_cleaned_votes_watermark( db: &dyn Database, config: &ColumnConfiguration, ) -> FatalResult> { - load_decode(db, config.col_data, CLEANED_VOTES_WATERMARK_KEY) + load_decode(db, config.col_dispute_data, CLEANED_VOTES_WATERMARK_KEY) .map_err(|e| FatalError::DbReadFailed(e)) } @@ -364,7 +379,7 @@ mod tests { let db = kvdb_memorydb::create(1); let db = polkadot_node_subsystem_util::database::kvdb_impl::DbAdapter::new(db, &[0]); let store = Arc::new(db); - let config = ColumnConfiguration { col_data: 0, col_session_data: 1 }; + let config = ColumnConfiguration { col_dispute_data: 0, col_session_data: 1 }; DbBackend::new(store, config, Metrics::default()) } diff --git a/node/core/dispute-coordinator/src/lib.rs b/node/core/dispute-coordinator/src/lib.rs index aa434636644c..03abd8f59d60 100644 --- a/node/core/dispute-coordinator/src/lib.rs +++ b/node/core/dispute-coordinator/src/lib.rs @@ -118,7 +118,7 @@ pub struct DisputeCoordinatorSubsystem { #[derive(Debug, Clone, Copy)] pub struct Config { /// The data column in the store to use for dispute data. - pub col_data: u32, + pub col_dispute_data: u32, /// The data column in the store to use for session data. pub col_session_data: u32, } @@ -126,7 +126,7 @@ pub struct Config { impl Config { fn column_config(&self) -> db::v1::ColumnConfiguration { db::v1::ColumnConfiguration { - col_data: self.col_data, + col_dispute_data: self.col_dispute_data, col_session_data: self.col_session_data, } } diff --git a/node/core/dispute-coordinator/src/tests.rs b/node/core/dispute-coordinator/src/tests.rs index 32c813d9d460..a1ad315d2ea0 100644 --- a/node/core/dispute-coordinator/src/tests.rs +++ b/node/core/dispute-coordinator/src/tests.rs @@ -176,7 +176,7 @@ impl Default for TestState { let db = kvdb_memorydb::create(1); let db = polkadot_node_subsystem_util::database::kvdb_impl::DbAdapter::new(db, &[]); let db = Arc::new(db); - let config = Config { col_data: 0, col_session_data: 1 }; + let config = Config { col_dispute_data: 0, col_session_data: 1 }; let genesis_header = Header { parent_hash: Hash::zero(), diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index 6d451ae8a472..d87c87079b62 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -943,7 +943,7 @@ where let parachains_db = open_database(&config.database)?; let approval_voting_config = ApprovalVotingConfig { - col_data: parachains_db::REAL_COLUMNS.col_approval_data, + col_approval_data: parachains_db::REAL_COLUMNS.col_approval_data, col_session_data: parachains_db::REAL_COLUMNS.col_session_window_data, slot_duration_millis: slot_duration.as_millis() as u64, }; @@ -967,7 +967,7 @@ where }; let dispute_coordinator_config = DisputeCoordinatorConfig { - col_data: parachains_db::REAL_COLUMNS.col_dispute_coordinator_data, + col_dispute_data: parachains_db::REAL_COLUMNS.col_dispute_coordinator_data, col_session_data: parachains_db::REAL_COLUMNS.col_session_window_data, }; From 8305677fd931e47c47b112de85bdfdf1a12708d4 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 8 Nov 2022 12:20:23 +0000 Subject: [PATCH 40/42] add doc comment Signed-off-by: Andrei Sandu --- node/service/src/parachains_db/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/node/service/src/parachains_db/mod.rs b/node/service/src/parachains_db/mod.rs index 48193309c311..d6309e5f17a6 100644 --- a/node/service/src/parachains_db/mod.rs +++ b/node/service/src/parachains_db/mod.rs @@ -23,6 +23,8 @@ mod upgrade; const LOG_TARGET: &str = "parachain::db"; +/// Column configuration per each version. Each module contains the additions or changes +/// specific to that version. #[cfg(any(test, feature = "full-node"))] pub(crate) mod columns { pub mod v0 { From f8b6023865383eaaa82a7c90a3212a48d8d5c623 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 8 Nov 2022 14:54:04 +0000 Subject: [PATCH 41/42] fix build Signed-off-by: Andrei Sandu --- node/service/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index d87c87079b62..18218a8aba8e 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -1539,7 +1539,7 @@ fn revert_chain_selection(db: Arc, hash: Hash) -> sp_blockchain::R fn revert_approval_voting(db: Arc, hash: Hash) -> sp_blockchain::Result<()> { let config = approval_voting_subsystem::Config { - col_data: parachains_db::REAL_COLUMNS.col_approval_data, + col_approval_data: parachains_db::REAL_COLUMNS.col_approval_data, col_session_data: parachains_db::REAL_COLUMNS.col_session_window_data, slot_duration_millis: Default::default(), }; From 43e18236296e9a9408a07544f672a9ac7c7d87d6 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 8 Nov 2022 14:54:31 +0000 Subject: [PATCH 42/42] migration: move all cols to v2 Signed-off-by: Andrei Sandu --- node/service/src/parachains_db/mod.rs | 33 +++++++++++------------ node/service/src/parachains_db/upgrade.rs | 19 ++++++------- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/node/service/src/parachains_db/mod.rs b/node/service/src/parachains_db/mod.rs index d6309e5f17a6..74e7e13dc657 100644 --- a/node/service/src/parachains_db/mod.rs +++ b/node/service/src/parachains_db/mod.rs @@ -23,8 +23,7 @@ mod upgrade; const LOG_TARGET: &str = "parachain::db"; -/// Column configuration per each version. Each module contains the additions or changes -/// specific to that version. +/// Column configuration per version. #[cfg(any(test, feature = "full-node"))] pub(crate) mod columns { pub mod v0 { @@ -33,20 +32,20 @@ pub(crate) mod columns { pub mod v1 { pub const NUM_COLUMNS: u32 = 5; + } + pub mod v2 { + pub const NUM_COLUMNS: u32 = 6; pub const COL_AVAILABILITY_DATA: u32 = 0; pub const COL_AVAILABILITY_META: u32 = 1; pub const COL_APPROVAL_DATA: u32 = 2; pub const COL_CHAIN_SELECTION_DATA: u32 = 3; pub const COL_DISPUTE_COORDINATOR_DATA: u32 = 4; + pub const COL_SESSION_WINDOW_DATA: u32 = 5; + pub const ORDERED_COL: &[u32] = &[COL_AVAILABILITY_META, COL_CHAIN_SELECTION_DATA, COL_DISPUTE_COORDINATOR_DATA]; } - - pub mod v2 { - pub const NUM_COLUMNS: u32 = 6; - pub const COL_SESSION_WINDOW_DATA: u32 = 5; - } } /// Columns used by different subsystems. @@ -70,11 +69,11 @@ pub struct ColumnsConfig { /// The real columns used by the parachains DB. #[cfg(any(test, feature = "full-node"))] pub const REAL_COLUMNS: ColumnsConfig = ColumnsConfig { - col_availability_data: columns::v1::COL_AVAILABILITY_DATA, - col_availability_meta: columns::v1::COL_AVAILABILITY_META, - col_approval_data: columns::v1::COL_APPROVAL_DATA, - col_chain_selection_data: columns::v1::COL_CHAIN_SELECTION_DATA, - col_dispute_coordinator_data: columns::v1::COL_DISPUTE_COORDINATOR_DATA, + col_availability_data: columns::v2::COL_AVAILABILITY_DATA, + col_availability_meta: columns::v2::COL_AVAILABILITY_META, + col_approval_data: columns::v2::COL_APPROVAL_DATA, + col_chain_selection_data: columns::v2::COL_CHAIN_SELECTION_DATA, + col_dispute_coordinator_data: columns::v2::COL_DISPUTE_COORDINATOR_DATA, col_session_window_data: columns::v2::COL_SESSION_WINDOW_DATA, }; @@ -127,13 +126,13 @@ pub fn open_creating_rocksdb( let _ = db_config .memory_budget - .insert(columns::v1::COL_AVAILABILITY_DATA, cache_sizes.availability_data); + .insert(columns::v2::COL_AVAILABILITY_DATA, cache_sizes.availability_data); let _ = db_config .memory_budget - .insert(columns::v1::COL_AVAILABILITY_META, cache_sizes.availability_meta); + .insert(columns::v2::COL_AVAILABILITY_META, cache_sizes.availability_meta); let _ = db_config .memory_budget - .insert(columns::v1::COL_APPROVAL_DATA, cache_sizes.approval_data); + .insert(columns::v2::COL_APPROVAL_DATA, cache_sizes.approval_data); let _ = db_config .memory_budget .insert(columns::v2::COL_SESSION_WINDOW_DATA, cache_sizes.session_data); @@ -147,7 +146,7 @@ pub fn open_creating_rocksdb( let db = Database::open(&db_config, &path_str)?; let db = polkadot_node_subsystem_util::database::kvdb_impl::DbAdapter::new( db, - columns::v1::ORDERED_COL, + columns::v2::ORDERED_COL, ); Ok(Arc::new(db)) @@ -172,7 +171,7 @@ pub fn open_creating_paritydb( let db = polkadot_node_subsystem_util::database::paritydb_impl::DbAdapter::new( db, - columns::v1::ORDERED_COL, + columns::v2::ORDERED_COL, ); Ok(Arc::new(db)) } diff --git a/node/service/src/parachains_db/upgrade.rs b/node/service/src/parachains_db/upgrade.rs index 05e6ea4cb725..01d4fb62f7f6 100644 --- a/node/service/src/parachains_db/upgrade.rs +++ b/node/service/src/parachains_db/upgrade.rs @@ -221,7 +221,7 @@ fn paritydb_fix_columns( pub(crate) fn paritydb_version_1_config(path: &Path) -> parity_db::Options { let mut options = parity_db::Options::with_columns(&path, super::columns::v1::NUM_COLUMNS as u8); - for i in columns::v1::ORDERED_COL { + for i in columns::v2::ORDERED_COL { options.columns[*i as usize].btree_index = true; } @@ -232,7 +232,7 @@ pub(crate) fn paritydb_version_1_config(path: &Path) -> parity_db::Options { pub(crate) fn paritydb_version_2_config(path: &Path) -> parity_db::Options { let mut options = parity_db::Options::with_columns(&path, super::columns::v2::NUM_COLUMNS as u8); - for i in columns::v1::ORDERED_COL { + for i in columns::v2::ORDERED_COL { options.columns[*i as usize].btree_index = true; } @@ -244,8 +244,8 @@ pub(crate) fn paritydb_version_2_config(path: &Path) -> parity_db::Options { pub(crate) fn paritydb_version_0_config(path: &Path) -> parity_db::Options { let mut options = parity_db::Options::with_columns(&path, super::columns::v1::NUM_COLUMNS as u8); - options.columns[super::columns::v1::COL_AVAILABILITY_META as usize].btree_index = true; - options.columns[super::columns::v1::COL_CHAIN_SELECTION_DATA as usize].btree_index = true; + options.columns[super::columns::v2::COL_AVAILABILITY_META as usize].btree_index = true; + options.columns[super::columns::v2::COL_CHAIN_SELECTION_DATA as usize].btree_index = true; options } @@ -260,7 +260,7 @@ fn paritydb_migrate_from_version_0_to_1(path: &Path) -> Result<(), Error> { paritydb_fix_columns( path, paritydb_version_1_config(path), - vec![super::columns::v1::COL_DISPUTE_COORDINATOR_DATA], + vec![super::columns::v2::COL_DISPUTE_COORDINATOR_DATA], )?; Ok(()) @@ -280,10 +280,7 @@ fn paritydb_migrate_from_version_1_to_2(path: &Path) -> Result<(), Error> { #[cfg(test)] mod tests { - use super::{ - columns::{v1::*, v2::*}, - *, - }; + use super::{columns::v2::*, *}; #[test] fn test_paritydb_migrate_0_to_1() { @@ -378,7 +375,7 @@ mod tests { // We need to properly set db version for upgrade to work. fs::write(version_file_path(db_dir.path()), "1").expect("Failed to write DB version"); { - let db = DbAdapter::new(db, columns::v1::ORDERED_COL); + let db = DbAdapter::new(db, columns::v2::ORDERED_COL); db.write(DBTransaction { ops: vec![DBOp::Insert { col: COL_DISPUTE_COORDINATOR_DATA, @@ -396,7 +393,7 @@ mod tests { assert_eq!(db.num_columns(), super::columns::v2::NUM_COLUMNS); - let db = DbAdapter::new(db, columns::v1::ORDERED_COL); + let db = DbAdapter::new(db, columns::v2::ORDERED_COL); assert_eq!( db.get(COL_DISPUTE_COORDINATOR_DATA, b"1234").unwrap(),