From cb89dfc039880c3490fd0c73df038f4433dc2520 Mon Sep 17 00:00:00 2001 From: Andronik Ordian <write@reusable.software> Date: Wed, 29 Sep 2021 11:53:44 +0200 Subject: [PATCH] approval-voting: populate session cache in advance (#3954) * try populating session cache in advance * remove unused arg * fmt * fix compilation * fix tests * Revert "fix tests" This reverts commit e8222b1108e09a39727a38e3b4e4c3061642a213. * fix tests * bump dispute window const by 1 * fix tests --- .../node/core/approval-voting/src/import.rs | 4 +-- .../node/core/approval-voting/src/tests.rs | 4 +-- .../core/dispute-coordinator/src/real/mod.rs | 17 +----------- polkadot/node/primitives/src/lib.rs | 3 ++- .../src/rolling_session_window.rs | 27 ++++++++----------- 5 files changed, 18 insertions(+), 37 deletions(-) diff --git a/polkadot/node/core/approval-voting/src/import.rs b/polkadot/node/core/approval-voting/src/import.rs index 52b0211a86a..710c1ab55cf 100644 --- a/polkadot/node/core/approval-voting/src/import.rs +++ b/polkadot/node/core/approval-voting/src/import.rs @@ -324,7 +324,7 @@ pub(crate) async fn handle_new_head( } }; - match state.session_window.cache_session_info_for_head(ctx, head, &header).await { + match state.session_window.cache_session_info_for_head(ctx, head).await { Err(e) => { tracing::debug!( target: LOG_TARGET, @@ -1236,7 +1236,7 @@ pub(crate) mod tests { h, RuntimeApiRequest::SessionIndexForChild(c_tx), )) => { - assert_eq!(h, parent_hash.clone()); + assert_eq!(h, hash); let _ = c_tx.send(Ok(session)); } ); diff --git a/polkadot/node/core/approval-voting/src/tests.rs b/polkadot/node/core/approval-voting/src/tests.rs index c40d62ecedf..fe0725384ba 100644 --- a/polkadot/node/core/approval-voting/src/tests.rs +++ b/polkadot/node/core/approval-voting/src/tests.rs @@ -786,8 +786,8 @@ async fn import_block( RuntimeApiRequest::SessionIndexForChild(s_tx) ) ) => { - let hash = &hashes[number.saturating_sub(1) as usize]; - assert_eq!(req_block_hash, hash.0.clone()); + let hash = &hashes[number as usize]; + assert_eq!(req_block_hash, hash.0); s_tx.send(Ok(number.into())).unwrap(); } ); diff --git a/polkadot/node/core/dispute-coordinator/src/real/mod.rs b/polkadot/node/core/dispute-coordinator/src/real/mod.rs index 7dd07201792..f0d0f5b8597 100644 --- a/polkadot/node/core/dispute-coordinator/src/real/mod.rs +++ b/polkadot/node/core/dispute-coordinator/src/real/mod.rs @@ -478,22 +478,7 @@ async fn handle_new_activations( new_activations: impl IntoIterator<Item = Hash>, ) -> Result<(), Error> { for new_leaf in new_activations { - let block_header = { - let (tx, rx) = oneshot::channel(); - - ctx.send_message(ChainApiMessage::BlockHeader(new_leaf, tx)).await; - - match rx.await?? { - None => continue, - Some(header) => header, - } - }; - - match state - .rolling_session_window - .cache_session_info_for_head(ctx, new_leaf, &block_header) - .await - { + match state.rolling_session_window.cache_session_info_for_head(ctx, new_leaf).await { Err(e) => { tracing::warn!( target: LOG_TARGET, diff --git a/polkadot/node/primitives/src/lib.rs b/polkadot/node/primitives/src/lib.rs index c2c300fca74..8d00e8b0cb3 100644 --- a/polkadot/node/primitives/src/lib.rs +++ b/polkadot/node/primitives/src/lib.rs @@ -69,7 +69,8 @@ pub const POV_BOMB_LIMIT: usize = (MAX_POV_SIZE * 4u32) as usize; /// On Polkadot this is 1 day, and on Kusama it's 6 hours. /// /// Number of sessions we want to consider in disputes. -pub const DISPUTE_WINDOW: SessionIndex = 6; +/// + 1 for the child's session. +pub const DISPUTE_WINDOW: SessionIndex = 6 + 1; /// The cumulative weight of a block in a fork-choice rule. pub type BlockWeight = u32; diff --git a/polkadot/node/subsystem-util/src/rolling_session_window.rs b/polkadot/node/subsystem-util/src/rolling_session_window.rs index a7afc4f8c94..1c73ff58c71 100644 --- a/polkadot/node/subsystem-util/src/rolling_session_window.rs +++ b/polkadot/node/subsystem-util/src/rolling_session_window.rs @@ -19,7 +19,7 @@ //! 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 polkadot_primitives::v1::{Hash, Header, SessionIndex, SessionInfo}; +use polkadot_primitives::v1::{Hash, SessionIndex, SessionInfo}; use futures::channel::oneshot; use polkadot_node_subsystem::{ @@ -131,7 +131,7 @@ impl RollingSessionWindow { } /// When inspecting a new import notification, updates the session info cache to match - /// the session of the imported block. + /// the session of the imported block's child. /// /// this only needs to be called on heads where we are directly notified about import, as sessions do /// not change often and import notifications are expected to be typically increasing in session number. @@ -141,7 +141,6 @@ impl RollingSessionWindow { &mut self, ctx: &mut (impl SubsystemContext + overseer::SubsystemContext), block_hash: Hash, - block_header: &Header, ) -> Result<SessionWindowUpdate, SessionsUnavailable> { if self.window_size == 0 { return Ok(SessionWindowUpdate::Unchanged) @@ -150,11 +149,9 @@ impl RollingSessionWindow { let session_index = { let (s_tx, s_rx) = oneshot::channel(); - // The genesis is guaranteed to be at the beginning of the session and its parent state - // is non-existent. Therefore if we're at the genesis, we request using its state and - // not the parent. + // We're requesting session index of a child to populate the cache in advance. ctx.send_message(RuntimeApiMessage::Request( - if block_header.number == 0 { block_hash } else { block_header.parent_hash }, + block_hash, RuntimeApiRequest::SessionIndexForChild(s_tx), )) .await; @@ -289,9 +286,10 @@ mod tests { use assert_matches::assert_matches; use polkadot_node_subsystem::messages::{AllMessages, AvailabilityRecoveryMessage}; use polkadot_node_subsystem_test_helpers::make_subsystem_context; + use polkadot_primitives::v1::Header; use sp_core::testing::TaskExecutor; - const TEST_WINDOW_SIZE: SessionIndex = 6; + const TEST_WINDOW_SIZE: SessionIndex = 6 + 1; fn dummy_session_info(index: SessionIndex) -> SessionInfo { SessionInfo { @@ -329,9 +327,8 @@ mod tests { let hash = header.hash(); let test_fut = { - let header = header.clone(); Box::pin(async move { - window.cache_session_info_for_head(&mut ctx, hash, &header).await.unwrap(); + window.cache_session_info_for_head(&mut ctx, hash).await.unwrap(); assert_eq!(window.earliest_session, Some(expected_start_session)); assert_eq!( @@ -348,7 +345,7 @@ mod tests { h, RuntimeApiRequest::SessionIndexForChild(s_tx), )) => { - assert_eq!(h, header.parent_hash); + assert_eq!(h, hash); let _ = s_tx.send(Ok(session)); } ); @@ -497,9 +494,8 @@ mod tests { let hash = header.hash(); let test_fut = { - let header = header.clone(); Box::pin(async move { - let res = window.cache_session_info_for_head(&mut ctx, hash, &header).await; + let res = window.cache_session_info_for_head(&mut ctx, hash).await; assert!(res.is_err()); }) @@ -512,7 +508,7 @@ mod tests { h, RuntimeApiRequest::SessionIndexForChild(s_tx), )) => { - assert_eq!(h, header.parent_hash); + assert_eq!(h, hash); let _ = s_tx.send(Ok(session)); } ); @@ -559,9 +555,8 @@ mod tests { let hash = header.hash(); let test_fut = { - let header = header.clone(); Box::pin(async move { - window.cache_session_info_for_head(&mut ctx, hash, &header).await.unwrap(); + window.cache_session_info_for_head(&mut ctx, hash).await.unwrap(); assert_eq!(window.earliest_session, Some(session)); assert_eq!(window.session_info, vec![dummy_session_info(session)]); -- GitLab