From a1440b67259ba7c31d60809daccae15d15c7c9da Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Mon, 17 Jan 2022 19:25:15 +0000 Subject: [PATCH 01/10] Rename/refactor around get_session_index Signed-off-by: Andrei Sandu --- bridges/bin/rialto/runtime/src/lib.rs | 4 +-- node/core/backing/src/lib.rs | 4 +-- node/core/pvf-checker/src/lib.rs | 2 +- node/core/pvf-checker/src/runtime_api.rs | 2 +- node/core/runtime-api/src/cache.rs | 14 ++++---- node/core/runtime-api/src/lib.rs | 6 ++-- node/core/runtime-api/src/tests.rs | 10 +++--- .../src/variants/back_garbage_candidate.rs | 2 +- .../src/requester/mod.rs | 33 ++++++++++--------- .../src/requester/session_cache.rs | 2 +- .../src/collator_side/mod.rs | 2 +- .../dispute-distribution/src/sender/mod.rs | 2 +- node/network/gossip-support/src/lib.rs | 2 +- .../network/statement-distribution/src/lib.rs | 2 +- node/subsystem-util/src/lib.rs | 6 ++-- .../src/rolling_session_window.rs | 6 ++-- node/subsystem-util/src/runtime/mod.rs | 13 ++++---- primitives/src/v2/mod.rs | 2 +- .../src/node/approval/approval-voting.md | 2 +- .../src/runtime-api/session-index.md | 2 +- runtime/kusama/src/lib.rs | 4 +-- runtime/parachains/src/runtime_api_impl/v1.rs | 6 ++-- runtime/polkadot/src/lib.rs | 4 +-- runtime/rococo/src/lib.rs | 4 +-- runtime/test-runtime/src/lib.rs | 4 +-- runtime/westend/src/lib.rs | 4 +-- 26 files changed, 73 insertions(+), 71 deletions(-) diff --git a/bridges/bin/rialto/runtime/src/lib.rs b/bridges/bin/rialto/runtime/src/lib.rs index 4c1901f5a3d7..a5c629b4a15c 100644 --- a/bridges/bin/rialto/runtime/src/lib.rs +++ b/bridges/bin/rialto/runtime/src/lib.rs @@ -774,8 +774,8 @@ impl_runtime_apis! { polkadot_runtime_parachains::runtime_api_impl::v1::check_validation_outputs::(para_id, outputs) } - fn session_index_for_child() -> polkadot_primitives::v1::SessionIndex { - polkadot_runtime_parachains::runtime_api_impl::v1::session_index_for_child::() + fn child_session_index() -> polkadot_primitives::v1::SessionIndex { + polkadot_runtime_parachains::runtime_api_impl::v1::child_session_index::() } fn validation_code( diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index f3ce93536238..3bb6e3311607 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -37,7 +37,7 @@ use polkadot_node_primitives::{ use polkadot_node_subsystem_util::{ self as util, metrics::{self, prometheus}, - request_from_runtime, request_session_index_for_child, request_validator_groups, + request_from_runtime, request_child_session_index, request_validator_groups, request_validators, FromJobCommand, JobSender, Validator, }; use polkadot_primitives::v1::{ @@ -1222,7 +1222,7 @@ impl util::JobTrait for CandidateBackingJob { let (validators, groups, session_index, cores) = futures::try_join!( request_validators(parent, &mut sender).await, request_validator_groups(parent, &mut sender).await, - request_session_index_for_child(parent, &mut sender).await, + request_child_session_index(parent, &mut sender).await, request_from_runtime(parent, &mut sender, |tx| { RuntimeApiRequest::AvailabilityCores(tx) },) diff --git a/node/core/pvf-checker/src/lib.rs b/node/core/pvf-checker/src/lib.rs index 6256ebbc37e0..3539078adc65 100644 --- a/node/core/pvf-checker/src/lib.rs +++ b/node/core/pvf-checker/src/lib.rs @@ -370,7 +370,7 @@ async fn examine_activation( _ => (leaf_number, leaf_hash), }; - let new_session_index = match runtime_api::session_index_for_child(sender, leaf_hash).await { + let new_session_index = match runtime_api::child_session_index(sender, leaf_hash).await { Ok(session_index) => if state.latest_session.map_or(true, |l| l < session_index) { let signing_credentials = diff --git a/node/core/pvf-checker/src/runtime_api.rs b/node/core/pvf-checker/src/runtime_api.rs index 59f94f6c1ac0..9b30f640a2e7 100644 --- a/node/core/pvf-checker/src/runtime_api.rs +++ b/node/core/pvf-checker/src/runtime_api.rs @@ -26,7 +26,7 @@ use polkadot_primitives::{ v2::PvfCheckStatement, }; -pub(crate) async fn session_index_for_child( +pub(crate) async fn child_session_index( sender: &mut impl SubsystemSender, relay_parent: Hash, ) -> Result { diff --git a/node/core/runtime-api/src/cache.rs b/node/core/runtime-api/src/cache.rs index c075b73d278e..425a7a9f5055 100644 --- a/node/core/runtime-api/src/cache.rs +++ b/node/core/runtime-api/src/cache.rs @@ -38,7 +38,7 @@ const AVAILABILITY_CORES_CACHE_SIZE: usize = 64 * 1024; const PERSISTED_VALIDATION_DATA_CACHE_SIZE: usize = 64 * 1024; const ASSUMED_VALIDATION_DATA_CACHE_SIZE: usize = 64 * 1024; const CHECK_VALIDATION_OUTPUTS_CACHE_SIZE: usize = 64 * 1024; -const SESSION_INDEX_FOR_CHILD_CACHE_SIZE: usize = 64 * 1024; +const CHILD_SESSION_INDEX_CACHE_SIZE: usize = 64 * 1024; const VALIDATION_CODE_CACHE_SIZE: usize = 10 * 1024 * 1024; const CANDIDATE_PENDING_AVAILABILITY_CACHE_SIZE: usize = 64 * 1024; const CANDIDATE_EVENTS_CACHE_SIZE: usize = 64 * 1024; @@ -92,7 +92,7 @@ pub(crate) struct RequestResultCache { >, check_validation_outputs: MemoryLruCache<(Hash, ParaId, CandidateCommitments), ResidentSizeOf>, - session_index_for_child: MemoryLruCache>, + child_session_index: MemoryLruCache>, validation_code: MemoryLruCache< (Hash, ParaId, OccupiedCoreAssumption), ResidentSizeOf>, @@ -128,7 +128,7 @@ impl Default for RequestResultCache { persisted_validation_data: MemoryLruCache::new(PERSISTED_VALIDATION_DATA_CACHE_SIZE), assumed_validation_data: MemoryLruCache::new(ASSUMED_VALIDATION_DATA_CACHE_SIZE), check_validation_outputs: MemoryLruCache::new(CHECK_VALIDATION_OUTPUTS_CACHE_SIZE), - session_index_for_child: MemoryLruCache::new(SESSION_INDEX_FOR_CHILD_CACHE_SIZE), + child_session_index: MemoryLruCache::new(CHILD_SESSION_INDEX_CACHE_SIZE), validation_code: MemoryLruCache::new(VALIDATION_CODE_CACHE_SIZE), validation_code_by_hash: MemoryLruCache::new(VALIDATION_CODE_CACHE_SIZE), candidate_pending_availability: MemoryLruCache::new( @@ -238,16 +238,16 @@ impl RequestResultCache { self.check_validation_outputs.insert(key, ResidentSizeOf(value)); } - pub(crate) fn session_index_for_child(&mut self, relay_parent: &Hash) -> Option<&SessionIndex> { - self.session_index_for_child.get(relay_parent).map(|v| &v.0) + pub(crate) fn child_session_index(&mut self, relay_parent: &Hash) -> Option<&SessionIndex> { + self.child_session_index.get(relay_parent).map(|v| &v.0) } - pub(crate) fn cache_session_index_for_child( + pub(crate) fn cache_child_session_index( &mut self, relay_parent: Hash, index: SessionIndex, ) { - self.session_index_for_child.insert(relay_parent, ResidentSizeOf(index)); + self.child_session_index.insert(relay_parent, ResidentSizeOf(index)); } pub(crate) fn validation_code( diff --git a/node/core/runtime-api/src/lib.rs b/node/core/runtime-api/src/lib.rs index e1e23d162061..b7a077ef54c9 100644 --- a/node/core/runtime-api/src/lib.rs +++ b/node/core/runtime-api/src/lib.rs @@ -135,7 +135,7 @@ where .requests_cache .cache_check_validation_outputs((relay_parent, para_id, commitments), b), SessionIndexForChild(relay_parent, session_index) => - self.requests_cache.cache_session_index_for_child(relay_parent, session_index), + self.requests_cache.cache_child_session_index(relay_parent, session_index), ValidationCode(relay_parent, para_id, assumption, code) => self .requests_cache .cache_validation_code((relay_parent, para_id, assumption), code), @@ -224,7 +224,7 @@ where Request::CheckValidationOutputs(para, commitments, sender) => query!(check_validation_outputs(para, commitments), sender) .map(|sender| Request::CheckValidationOutputs(para, commitments, sender)), - Request::SessionIndexForChild(sender) => query!(session_index_for_child(), sender) + Request::SessionIndexForChild(sender) => query!(child_session_index(), sender) .map(|sender| Request::SessionIndexForChild(sender)), Request::ValidationCode(para, assumption, sender) => query!(validation_code(para, assumption), sender) @@ -430,7 +430,7 @@ where sender ), Request::SessionIndexForChild(sender) => - query!(SessionIndexForChild, session_index_for_child(), ver = 1, sender), + query!(SessionIndexForChild, child_session_index(), ver = 1, sender), Request::ValidationCode(para, assumption, sender) => query!(ValidationCode, validation_code(para, assumption), ver = 1, sender), Request::ValidationCodeByHash(validation_code_hash, sender) => query!( diff --git a/node/core/runtime-api/src/tests.rs b/node/core/runtime-api/src/tests.rs index d9e5700ff691..588ae34cc090 100644 --- a/node/core/runtime-api/src/tests.rs +++ b/node/core/runtime-api/src/tests.rs @@ -43,7 +43,7 @@ struct MockRuntimeApi { availability_cores: Vec, availability_cores_wait: Arc>, validation_data: HashMap, - session_index_for_child: SessionIndex, + child_session_index: SessionIndex, session_info: HashMap, validation_code: HashMap, validation_code_by_hash: HashMap, @@ -121,8 +121,8 @@ sp_api::mock_impl_runtime_apis! { ) } - fn session_index_for_child(&self) -> SessionIndex { - self.session_index_for_child.clone() + fn child_session_index(&self) -> SessionIndex { + self.child_session_index.clone() } fn session_info(&self, index: SessionIndex) -> Option { @@ -483,7 +483,7 @@ fn requests_check_validation_outputs() { } #[test] -fn requests_session_index_for_child() { +fn requests_child_session_index() { let (ctx, mut ctx_handle) = make_subsystem_context(TaskExecutor::new()); let runtime_api = Arc::new(MockRuntimeApi::default()); let relay_parent = [1; 32].into(); @@ -500,7 +500,7 @@ fn requests_session_index_for_child() { }) .await; - assert_eq!(rx.await.unwrap().unwrap(), runtime_api.session_index_for_child); + assert_eq!(rx.await.unwrap().unwrap(), runtime_api.child_session_index); ctx_handle.send(FromOverseer::Signal(OverseerSignal::Conclude)).await; }; diff --git a/node/malus/src/variants/back_garbage_candidate.rs b/node/malus/src/variants/back_garbage_candidate.rs index 06938f2a4fea..b27cf36770a2 100644 --- a/node/malus/src/variants/back_garbage_candidate.rs +++ b/node/malus/src/variants/back_garbage_candidate.rs @@ -141,7 +141,7 @@ where Some("malus"), Box::pin(async move { let relay_parent = candidate_descriptor.relay_parent; - let session_index = util::request_session_index_for_child( + let session_index = util::request_child_session_index( relay_parent, &mut subsystem_sender, ) diff --git a/node/network/availability-distribution/src/requester/mod.rs b/node/network/availability-distribution/src/requester/mod.rs index 6812fa69e954..d78dcf4b936c 100644 --- a/node/network/availability-distribution/src/requester/mod.rs +++ b/node/network/availability-distribution/src/requester/mod.rs @@ -98,36 +98,36 @@ impl Requester { tracing::trace!(target: LOG_TARGET, ?update, "Update fetching heads"); let ActiveLeavesUpdate { activated, deactivated } = update; // Stale leaves happen after a reversion - we don't want to re-run availability there. - let activated = activated.and_then(|h| match h.status { + if let Some(leaf) = activated.and_then(|h| match h.status { LeafStatus::Stale => None, LeafStatus::Fresh => Some(h), - }); + }) { + self.start_requesting_chunks(ctx, runtime, leaf).await?; + } + // Order important! We need to handle activated, prior to deactivated, otherwise we might // cancel still needed jobs. - self.start_requesting_chunks(ctx, runtime, activated.into_iter()).await?; self.stop_requesting_chunks(deactivated.into_iter()); Ok(()) } - /// Start requesting chunks for newly imported heads. + /// Start requesting chunks for newly imported relay chain head. async fn start_requesting_chunks( &mut self, ctx: &mut Context, runtime: &mut RuntimeInfo, - new_heads: impl Iterator, + leaf: ActivatedLeaf, ) -> super::Result<()> where Context: SubsystemContext, { - for ActivatedLeaf { hash: leaf, .. } in new_heads { - let cores = get_occupied_cores(ctx, leaf).await?; - tracing::trace!( - target: LOG_TARGET, - occupied_cores = ?cores, - "Query occupied core" - ); - self.add_cores(ctx, runtime, leaf, cores).await?; - } + let cores = get_occupied_cores(ctx, leaf.hash).await?; + tracing::trace!( + target: LOG_TARGET, + occupied_cores = ?cores, + "Query occupied core" + ); + self.add_cores(ctx, runtime, leaf.hash, cores).await?; Ok(()) } @@ -175,8 +175,9 @@ impl Requester { ctx, runtime, // We use leaf here, as relay_parent must be in the same session as the - // leaf. (Cores are dropped at session boundaries.) At the same time, - // only leaves are guaranteed to be fetchable by the state trie. + // leaf. This is guaranteed by runtime which ensures that cores are cleared + // at session boundaries. At the same time, only leaves are guaranteed to + // be fetchable by the state trie. leaf, |info| FetchTaskConfig::new(leaf, &core, tx, metrics, info), ) diff --git a/node/network/availability-distribution/src/requester/session_cache.rs b/node/network/availability-distribution/src/requester/session_cache.rs index 1dbab38cfff0..a09af708d48b 100644 --- a/node/network/availability-distribution/src/requester/session_cache.rs +++ b/node/network/availability-distribution/src/requester/session_cache.rs @@ -105,7 +105,7 @@ impl SessionCache { Context: SubsystemContext, F: FnOnce(&SessionInfo) -> R, { - let session_index = runtime.get_session_index(ctx.sender(), parent).await?; + let session_index = runtime.get_child_session_index(ctx.sender(), parent).await?; if let Some(o_info) = self.session_info_cache.get(&session_index) { tracing::trace!(target: LOG_TARGET, session_index, "Got session from lru"); diff --git a/node/network/collator-protocol/src/collator_side/mod.rs b/node/network/collator-protocol/src/collator_side/mod.rs index 758a978cc908..a15c31ee538b 100644 --- a/node/network/collator-protocol/src/collator_side/mod.rs +++ b/node/network/collator-protocol/src/collator_side/mod.rs @@ -434,7 +434,7 @@ where Context: SubsystemContext, Context: overseer::SubsystemContext, { - let session_index = runtime.get_session_index(ctx.sender(), relay_parent).await?; + let session_index = runtime.get_child_session_index(ctx.sender(), relay_parent).await?; let info = &runtime .get_session_info_by_index(ctx.sender(), relay_parent, session_index) .await? diff --git a/node/network/dispute-distribution/src/sender/mod.rs b/node/network/dispute-distribution/src/sender/mod.rs index 55d88f1310ae..39d8edc5842d 100644 --- a/node/network/dispute-distribution/src/sender/mod.rs +++ b/node/network/dispute-distribution/src/sender/mod.rs @@ -313,7 +313,7 @@ async fn get_active_session_indeces( ) -> Result> { let mut indeces = HashMap::new(); for head in active_heads { - let session_index = runtime.get_session_index(ctx.sender(), *head).await?; + let session_index = runtime.get_child_session_index(ctx.sender(), *head).await?; indeces.insert(session_index, *head); } Ok(indeces) diff --git a/node/network/gossip-support/src/lib.rs b/node/network/gossip-support/src/lib.rs index a12fbf82d236..9086ceb45cc5 100644 --- a/node/network/gossip-support/src/lib.rs +++ b/node/network/gossip-support/src/lib.rs @@ -200,7 +200,7 @@ where { for leaf in leaves { let current_index = - util::request_session_index_for_child(leaf, ctx.sender()).await.await??; + util::request_child_session_index(leaf, ctx.sender()).await.await??; let since_failure = self.last_failure.map(|i| i.elapsed()).unwrap_or_default(); let force_request = since_failure >= BACKOFF_DURATION; let leaf_session = Some((current_index, leaf)); diff --git a/node/network/statement-distribution/src/lib.rs b/node/network/statement-distribution/src/lib.rs index 2194aa806a60..a5f01e7b3b01 100644 --- a/node/network/statement-distribution/src/lib.rs +++ b/node/network/statement-distribution/src/lib.rs @@ -1799,7 +1799,7 @@ impl StatementDistributionSubsystem { ); let session_index = - runtime.get_session_index(ctx.sender(), relay_parent).await?; + runtime.get_child_session_index(ctx.sender(), relay_parent).await?; let info = runtime .get_session_info_by_index(ctx.sender(), relay_parent, session_index) .await?; diff --git a/node/subsystem-util/src/lib.rs b/node/subsystem-util/src/lib.rs index bf120c945f02..5aa9c0bdc3ec 100644 --- a/node/subsystem-util/src/lib.rs +++ b/node/subsystem-util/src/lib.rs @@ -209,7 +209,7 @@ specialize_requests! { fn request_availability_cores() -> Vec; AvailabilityCores; fn request_persisted_validation_data(para_id: ParaId, assumption: OccupiedCoreAssumption) -> Option; PersistedValidationData; fn request_assumed_validation_data(para_id: ParaId, expected_persisted_validation_data_hash: Hash) -> Option<(PersistedValidationData, ValidationCodeHash)>; AssumedValidationData; - fn request_session_index_for_child() -> SessionIndex; SessionIndexForChild; + fn request_child_session_index() -> SessionIndex; SessionIndexForChild; fn request_validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) -> Option; ValidationCode; fn request_validation_code_by_hash(validation_code_hash: ValidationCodeHash) -> Option; ValidationCodeByHash; fn request_candidate_pending_availability(para_id: ParaId) -> Option; CandidatePendingAvailability; @@ -326,12 +326,12 @@ impl Validator { keystore: SyncCryptoStorePtr, sender: &mut impl SubsystemSender, ) -> Result { - // Note: request_validators and request_session_index_for_child do not and cannot + // Note: request_validators and request_child_session_index do not and cannot // run concurrently: they both have a mutable handle to the same sender. // However, each of them returns a oneshot::Receiver, and those are resolved concurrently. let (validators, session_index) = futures::try_join!( request_validators(parent, sender).await, - request_session_index_for_child(parent, sender).await, + request_child_session_index(parent, sender).await, )?; let signing_context = SigningContext { session_index: session_index?, parent_hash: parent }; diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 95fb633349c1..1df917704198 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -102,7 +102,7 @@ impl RollingSessionWindow { window_size: SessionWindowSize, block_hash: Hash, ) -> Result { - let session_index = get_session_index_for_head(ctx, block_hash).await?; + let session_index = get_child_session_index(ctx, block_hash).await?; let window_start = session_index.saturating_sub(window_size.get() - 1); @@ -160,7 +160,7 @@ impl RollingSessionWindow { ctx: &mut (impl SubsystemContext + overseer::SubsystemContext), block_hash: Hash, ) -> Result { - let session_index = get_session_index_for_head(ctx, block_hash).await?; + let session_index = get_child_session_index(ctx, block_hash).await?; let old_window_start = self.earliest_session; @@ -212,7 +212,7 @@ impl RollingSessionWindow { } } -async fn get_session_index_for_head( +async fn get_child_session_index( ctx: &mut (impl SubsystemContext + overseer::SubsystemContext), block_hash: Hash, ) -> Result { diff --git a/node/subsystem-util/src/runtime/mod.rs b/node/subsystem-util/src/runtime/mod.rs index fc5a34464956..fb0b47b7b830 100644 --- a/node/subsystem-util/src/runtime/mod.rs +++ b/node/subsystem-util/src/runtime/mod.rs @@ -36,7 +36,7 @@ use polkadot_primitives::{ }; use crate::{ - request_availability_cores, request_candidate_events, request_session_index_for_child, + request_availability_cores, request_candidate_events, request_child_session_index, request_session_info, request_validation_code_by_hash, request_validator_groups, }; @@ -117,8 +117,9 @@ impl RuntimeInfo { } } - /// Retrieve the current session index. - pub async fn get_session_index( + /// Retrieve the current session index.- fix the name and comment. + /// Add comment about why using the child session index. + pub async fn get_child_session_index( &mut self, sender: &mut Sender, parent: Hash, @@ -130,7 +131,7 @@ impl RuntimeInfo { Some(index) => Ok(*index), None => { let index = - recv_runtime(request_session_index_for_child(parent, sender).await).await?; + recv_runtime(request_child_session_index(parent, sender).await).await?; self.session_index_cache.put(parent, index); Ok(index) }, @@ -146,7 +147,7 @@ impl RuntimeInfo { where Sender: SubsystemSender, { - let session_index = self.get_session_index(sender, parent).await?; + let session_index = self.get_child_session_index(sender, parent).await?; self.get_session_info_by_index(sender, parent, session_index).await } @@ -195,7 +196,7 @@ impl RuntimeInfo { Payload: EncodeAs + Clone, RealPayload: Encode + Clone, { - let session_index = self.get_session_index(sender, parent).await?; + let session_index = self.get_child_session_index(sender, parent).await?; let info = self.get_session_info_by_index(sender, parent, session_index).await?; Ok(check_signature(session_index, &info.session_info, parent, signed)) } diff --git a/primitives/src/v2/mod.rs b/primitives/src/v2/mod.rs index 065d5cc3c057..a45c46c4b295 100644 --- a/primitives/src/v2/mod.rs +++ b/primitives/src/v2/mod.rs @@ -171,7 +171,7 @@ sp_api::decl_runtime_apis! { /// Returns the session index expected at a child of the block. /// /// This can be used to instantiate a `SigningContext`. - fn session_index_for_child() -> v1::SessionIndex; + fn child_session_index() -> v1::SessionIndex; /// Old method to fetch v1 session info. #[changed_in(2)] diff --git a/roadmap/implementers-guide/src/node/approval/approval-voting.md b/roadmap/implementers-guide/src/node/approval/approval-voting.md index adb95e1f6389..6a11cbc77178 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-voting.md +++ b/roadmap/implementers-guide/src/node/approval/approval-voting.md @@ -177,7 +177,7 @@ On receiving an `OverseerSignal::ActiveLeavesUpdate(update)`: * We update the `StoredBlockRange` and the `BlockNumber` maps. * We use the `RuntimeApiSubsystem` to determine information about these blocks. It is generally safe to assume that runtime state is available for recent, unfinalized blocks. In the case that it isn't, it means that we are catching up to the head of the chain and needn't worry about assignments to those blocks anyway, as the security assumption of the protocol tolerates nodes being temporarily offline or out-of-date. * We fetch the set of candidates included by each block by dispatching a `RuntimeApiRequest::CandidateEvents` and checking the `CandidateIncluded` events. - * We fetch the session of the block by dispatching a `session_index_for_child` request with the parent-hash of the block. + * We fetch the session of the block by dispatching a `child_session_index` request with the parent-hash of the block. * If the `session index - APPROVAL_SESSIONS > state.earliest_session`, then bump `state.earliest_sessions` to that amount and prune earlier sessions. * If the session isn't in our `state.session_info`, load the session info for it and for all sessions since the earliest-session, including the earliest-session, if that is missing. And it can be, just after pruning, if we've done a big jump forward, as is the case when we've just finished chain synchronization. * If any of the runtime API calls fail, we just warn and skip the block. diff --git a/roadmap/implementers-guide/src/runtime-api/session-index.md b/roadmap/implementers-guide/src/runtime-api/session-index.md index 1baf6a167dbb..587e8b7918ea 100644 --- a/roadmap/implementers-guide/src/runtime-api/session-index.md +++ b/roadmap/implementers-guide/src/runtime-api/session-index.md @@ -8,5 +8,5 @@ This session index can be used to derive a [`SigningContext`](../types/candidate ```rust /// Returns the session index expected at a child of the block. -fn session_index_for_child(at: Block) -> SessionIndex; +fn child_session_index(at: Block) -> SessionIndex; ``` diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index c0b6e2d27c4e..d93f1f022bf5 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -3038,8 +3038,8 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::check_validation_outputs::(para_id, outputs) } - fn session_index_for_child() -> SessionIndex { - parachains_runtime_api_impl::session_index_for_child::() + fn child_session_index() -> SessionIndex { + parachains_runtime_api_impl::child_session_index::() } fn validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) diff --git a/runtime/parachains/src/runtime_api_impl/v1.rs b/runtime/parachains/src/runtime_api_impl/v1.rs index 994a720bc590..da9e8e5e1404 100644 --- a/runtime/parachains/src/runtime_api_impl/v1.rs +++ b/runtime/parachains/src/runtime_api_impl/v1.rs @@ -264,8 +264,8 @@ pub fn check_validation_outputs( >::check_validation_outputs_for_runtime_api(para_id, outputs) } -/// Implementation for the `session_index_for_child` function of the runtime API. -pub fn session_index_for_child() -> SessionIndex { +/// Implementation for the `child_session_index` function of the runtime API. +pub fn child_session_index() -> SessionIndex { // Just returns the session index from `inclusion`. Runtime APIs follow // initialization so the initializer will have applied any pending session change // which is expected at the child of the block whose context the runtime API was invoked @@ -281,7 +281,7 @@ pub fn session_index_for_child() -> SessionIndex { /// Gets next, current and some historical authority ids using `session_info` module. pub fn relevant_authority_ids( ) -> Vec { - let current_session_index = session_index_for_child::(); + let current_session_index = child_session_index::(); let earliest_stored_session = >::earliest_stored_session(); // Due to `max_validators`, the `SessionInfo` stores only the validators who are actively diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 984fe1609352..3d664bc918f1 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -1792,8 +1792,8 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::check_validation_outputs::(para_id, outputs) } - fn session_index_for_child() -> SessionIndex { - parachains_runtime_api_impl::session_index_for_child::() + fn child_session_index() -> SessionIndex { + parachains_runtime_api_impl::child_session_index::() } fn validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 8208fa548eae..1b420c6818ec 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -1188,8 +1188,8 @@ sp_api::impl_runtime_apis! { runtime_api_impl::check_validation_outputs::(para_id, outputs) } - fn session_index_for_child() -> SessionIndex { - runtime_api_impl::session_index_for_child::() + fn child_session_index() -> SessionIndex { + runtime_api_impl::child_session_index::() } fn validation_code(para_id: Id, assumption: OccupiedCoreAssumption) diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index eed721ad8896..0120bb085673 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -835,8 +835,8 @@ sp_api::impl_runtime_apis! { runtime_impl::check_validation_outputs::(para_id, outputs) } - fn session_index_for_child() -> SessionIndex { - runtime_impl::session_index_for_child::() + fn child_session_index() -> SessionIndex { + runtime_impl::child_session_index::() } fn validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 318018dd6a5e..fcc0e7a7266d 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1220,8 +1220,8 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::check_validation_outputs::(para_id, outputs) } - fn session_index_for_child() -> SessionIndex { - parachains_runtime_api_impl::session_index_for_child::() + fn child_session_index() -> SessionIndex { + parachains_runtime_api_impl::child_session_index::() } fn validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) From b0e94d14b0002fdef6f5ffce02f35988620eab3e Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 18 Jan 2022 10:21:15 +0000 Subject: [PATCH 02/10] choose proper head for fetching session Signed-off-by: Andrei Sandu --- node/core/backing/src/lib.rs | 2 +- node/core/runtime-api/src/cache.rs | 6 +----- .../dispute-distribution/src/sender/mod.rs | 20 +++++++++++++++++-- node/subsystem-util/src/runtime/mod.rs | 3 +-- 4 files changed, 21 insertions(+), 10 deletions(-) diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index 3bb6e3311607..8ecff2c62bbe 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -37,7 +37,7 @@ use polkadot_node_primitives::{ use polkadot_node_subsystem_util::{ self as util, metrics::{self, prometheus}, - request_from_runtime, request_child_session_index, request_validator_groups, + request_child_session_index, request_from_runtime, request_validator_groups, request_validators, FromJobCommand, JobSender, Validator, }; use polkadot_primitives::v1::{ diff --git a/node/core/runtime-api/src/cache.rs b/node/core/runtime-api/src/cache.rs index 425a7a9f5055..b9a50e5b2c29 100644 --- a/node/core/runtime-api/src/cache.rs +++ b/node/core/runtime-api/src/cache.rs @@ -242,11 +242,7 @@ impl RequestResultCache { self.child_session_index.get(relay_parent).map(|v| &v.0) } - pub(crate) fn cache_child_session_index( - &mut self, - relay_parent: Hash, - index: SessionIndex, - ) { + pub(crate) fn cache_child_session_index(&mut self, relay_parent: Hash, index: SessionIndex) { self.child_session_index.insert(relay_parent, ResidentSizeOf(index)); } diff --git a/node/network/dispute-distribution/src/sender/mod.rs b/node/network/dispute-distribution/src/sender/mod.rs index 39d8edc5842d..dbdc3ccb41b1 100644 --- a/node/network/dispute-distribution/src/sender/mod.rs +++ b/node/network/dispute-distribution/src/sender/mod.rs @@ -190,8 +190,24 @@ impl DisputeSender { dispute: (SessionIndex, CandidateHash), ) -> Result<()> { let (session_index, candidate_hash) = dispute; - // We need some relay chain head for context for receiving session info information: - let ref_head = self.active_sessions.values().next().ok_or(NonFatal::NoActiveHeads)?; + // A relay chain head is required as context for receiving session info information from runtime and + // storage. We will iterate `active_sessions` to find a suitable head. We assume that there is at + // least one active head which, by `session_index`, is at least as recent as the `dispute` passed in. + // We need to avoid picking an older one from a session that might not yet exist in storage. + // Related to https://github.com/paritytech/polkadot/issues/4730. + let ref_head = self + .active_sessions + .iter() + .find_map(|(active_session_index, head_hash)| { + // Pick the first one that is at least as recent as the dispute. + if active_session_index >= &session_index { + Some(head_hash) + } else { + None + } + }) + .ok_or(NonFatal::NoActiveHeads)?; + let info = runtime .get_session_info_by_index(ctx.sender(), *ref_head, session_index) .await?; diff --git a/node/subsystem-util/src/runtime/mod.rs b/node/subsystem-util/src/runtime/mod.rs index fb0b47b7b830..d5aa60040951 100644 --- a/node/subsystem-util/src/runtime/mod.rs +++ b/node/subsystem-util/src/runtime/mod.rs @@ -130,8 +130,7 @@ impl RuntimeInfo { match self.session_index_cache.get(&parent) { Some(index) => Ok(*index), None => { - let index = - recv_runtime(request_child_session_index(parent, sender).await).await?; + let index = recv_runtime(request_child_session_index(parent, sender).await).await?; self.session_index_cache.put(parent, index); Ok(index) }, From c557268b4ae8e5becd1f3f746d1ffb6581b19c4f Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 18 Jan 2022 12:40:05 +0000 Subject: [PATCH 03/10] revert rename Signed-off-by: Andrei Sandu --- bridges/bin/rialto/runtime/src/lib.rs | 4 ++-- node/core/backing/src/lib.rs | 4 ++-- node/core/pvf-checker/src/lib.rs | 2 +- node/core/pvf-checker/src/runtime_api.rs | 2 +- node/core/runtime-api/src/cache.rs | 18 +++++++++++------- node/core/runtime-api/src/lib.rs | 6 +++--- node/core/runtime-api/src/tests.rs | 10 +++++----- .../src/variants/back_garbage_candidate.rs | 2 +- .../src/requester/session_cache.rs | 2 +- .../collator-protocol/src/collator_side/mod.rs | 2 +- .../dispute-distribution/src/sender/mod.rs | 2 +- node/network/gossip-support/src/lib.rs | 2 +- node/network/statement-distribution/src/lib.rs | 2 +- node/subsystem-util/src/lib.rs | 6 +++--- .../src/rolling_session_window.rs | 6 +++--- node/subsystem-util/src/runtime/mod.rs | 11 ++++++----- primitives/src/v2/mod.rs | 2 +- .../src/node/approval/approval-voting.md | 2 +- .../src/runtime-api/session-index.md | 2 +- runtime/kusama/src/lib.rs | 4 ++-- runtime/parachains/src/runtime_api_impl/v1.rs | 6 +++--- runtime/polkadot/src/lib.rs | 4 ++-- runtime/rococo/src/lib.rs | 4 ++-- runtime/test-runtime/src/lib.rs | 4 ++-- runtime/westend/src/lib.rs | 4 ++-- 25 files changed, 59 insertions(+), 54 deletions(-) diff --git a/bridges/bin/rialto/runtime/src/lib.rs b/bridges/bin/rialto/runtime/src/lib.rs index a5c629b4a15c..4c1901f5a3d7 100644 --- a/bridges/bin/rialto/runtime/src/lib.rs +++ b/bridges/bin/rialto/runtime/src/lib.rs @@ -774,8 +774,8 @@ impl_runtime_apis! { polkadot_runtime_parachains::runtime_api_impl::v1::check_validation_outputs::(para_id, outputs) } - fn child_session_index() -> polkadot_primitives::v1::SessionIndex { - polkadot_runtime_parachains::runtime_api_impl::v1::child_session_index::() + fn session_index_for_child() -> polkadot_primitives::v1::SessionIndex { + polkadot_runtime_parachains::runtime_api_impl::v1::session_index_for_child::() } fn validation_code( diff --git a/node/core/backing/src/lib.rs b/node/core/backing/src/lib.rs index 8ecff2c62bbe..f3ce93536238 100644 --- a/node/core/backing/src/lib.rs +++ b/node/core/backing/src/lib.rs @@ -37,7 +37,7 @@ use polkadot_node_primitives::{ use polkadot_node_subsystem_util::{ self as util, metrics::{self, prometheus}, - request_child_session_index, request_from_runtime, request_validator_groups, + request_from_runtime, request_session_index_for_child, request_validator_groups, request_validators, FromJobCommand, JobSender, Validator, }; use polkadot_primitives::v1::{ @@ -1222,7 +1222,7 @@ impl util::JobTrait for CandidateBackingJob { let (validators, groups, session_index, cores) = futures::try_join!( request_validators(parent, &mut sender).await, request_validator_groups(parent, &mut sender).await, - request_child_session_index(parent, &mut sender).await, + request_session_index_for_child(parent, &mut sender).await, request_from_runtime(parent, &mut sender, |tx| { RuntimeApiRequest::AvailabilityCores(tx) },) diff --git a/node/core/pvf-checker/src/lib.rs b/node/core/pvf-checker/src/lib.rs index 3539078adc65..6256ebbc37e0 100644 --- a/node/core/pvf-checker/src/lib.rs +++ b/node/core/pvf-checker/src/lib.rs @@ -370,7 +370,7 @@ async fn examine_activation( _ => (leaf_number, leaf_hash), }; - let new_session_index = match runtime_api::child_session_index(sender, leaf_hash).await { + let new_session_index = match runtime_api::session_index_for_child(sender, leaf_hash).await { Ok(session_index) => if state.latest_session.map_or(true, |l| l < session_index) { let signing_credentials = diff --git a/node/core/pvf-checker/src/runtime_api.rs b/node/core/pvf-checker/src/runtime_api.rs index 9b30f640a2e7..59f94f6c1ac0 100644 --- a/node/core/pvf-checker/src/runtime_api.rs +++ b/node/core/pvf-checker/src/runtime_api.rs @@ -26,7 +26,7 @@ use polkadot_primitives::{ v2::PvfCheckStatement, }; -pub(crate) async fn child_session_index( +pub(crate) async fn session_index_for_child( sender: &mut impl SubsystemSender, relay_parent: Hash, ) -> Result { diff --git a/node/core/runtime-api/src/cache.rs b/node/core/runtime-api/src/cache.rs index b9a50e5b2c29..c075b73d278e 100644 --- a/node/core/runtime-api/src/cache.rs +++ b/node/core/runtime-api/src/cache.rs @@ -38,7 +38,7 @@ const AVAILABILITY_CORES_CACHE_SIZE: usize = 64 * 1024; const PERSISTED_VALIDATION_DATA_CACHE_SIZE: usize = 64 * 1024; const ASSUMED_VALIDATION_DATA_CACHE_SIZE: usize = 64 * 1024; const CHECK_VALIDATION_OUTPUTS_CACHE_SIZE: usize = 64 * 1024; -const CHILD_SESSION_INDEX_CACHE_SIZE: usize = 64 * 1024; +const SESSION_INDEX_FOR_CHILD_CACHE_SIZE: usize = 64 * 1024; const VALIDATION_CODE_CACHE_SIZE: usize = 10 * 1024 * 1024; const CANDIDATE_PENDING_AVAILABILITY_CACHE_SIZE: usize = 64 * 1024; const CANDIDATE_EVENTS_CACHE_SIZE: usize = 64 * 1024; @@ -92,7 +92,7 @@ pub(crate) struct RequestResultCache { >, check_validation_outputs: MemoryLruCache<(Hash, ParaId, CandidateCommitments), ResidentSizeOf>, - child_session_index: MemoryLruCache>, + session_index_for_child: MemoryLruCache>, validation_code: MemoryLruCache< (Hash, ParaId, OccupiedCoreAssumption), ResidentSizeOf>, @@ -128,7 +128,7 @@ impl Default for RequestResultCache { persisted_validation_data: MemoryLruCache::new(PERSISTED_VALIDATION_DATA_CACHE_SIZE), assumed_validation_data: MemoryLruCache::new(ASSUMED_VALIDATION_DATA_CACHE_SIZE), check_validation_outputs: MemoryLruCache::new(CHECK_VALIDATION_OUTPUTS_CACHE_SIZE), - child_session_index: MemoryLruCache::new(CHILD_SESSION_INDEX_CACHE_SIZE), + session_index_for_child: MemoryLruCache::new(SESSION_INDEX_FOR_CHILD_CACHE_SIZE), validation_code: MemoryLruCache::new(VALIDATION_CODE_CACHE_SIZE), validation_code_by_hash: MemoryLruCache::new(VALIDATION_CODE_CACHE_SIZE), candidate_pending_availability: MemoryLruCache::new( @@ -238,12 +238,16 @@ impl RequestResultCache { self.check_validation_outputs.insert(key, ResidentSizeOf(value)); } - pub(crate) fn child_session_index(&mut self, relay_parent: &Hash) -> Option<&SessionIndex> { - self.child_session_index.get(relay_parent).map(|v| &v.0) + pub(crate) fn session_index_for_child(&mut self, relay_parent: &Hash) -> Option<&SessionIndex> { + self.session_index_for_child.get(relay_parent).map(|v| &v.0) } - pub(crate) fn cache_child_session_index(&mut self, relay_parent: Hash, index: SessionIndex) { - self.child_session_index.insert(relay_parent, ResidentSizeOf(index)); + pub(crate) fn cache_session_index_for_child( + &mut self, + relay_parent: Hash, + index: SessionIndex, + ) { + self.session_index_for_child.insert(relay_parent, ResidentSizeOf(index)); } pub(crate) fn validation_code( diff --git a/node/core/runtime-api/src/lib.rs b/node/core/runtime-api/src/lib.rs index b7a077ef54c9..e1e23d162061 100644 --- a/node/core/runtime-api/src/lib.rs +++ b/node/core/runtime-api/src/lib.rs @@ -135,7 +135,7 @@ where .requests_cache .cache_check_validation_outputs((relay_parent, para_id, commitments), b), SessionIndexForChild(relay_parent, session_index) => - self.requests_cache.cache_child_session_index(relay_parent, session_index), + self.requests_cache.cache_session_index_for_child(relay_parent, session_index), ValidationCode(relay_parent, para_id, assumption, code) => self .requests_cache .cache_validation_code((relay_parent, para_id, assumption), code), @@ -224,7 +224,7 @@ where Request::CheckValidationOutputs(para, commitments, sender) => query!(check_validation_outputs(para, commitments), sender) .map(|sender| Request::CheckValidationOutputs(para, commitments, sender)), - Request::SessionIndexForChild(sender) => query!(child_session_index(), sender) + Request::SessionIndexForChild(sender) => query!(session_index_for_child(), sender) .map(|sender| Request::SessionIndexForChild(sender)), Request::ValidationCode(para, assumption, sender) => query!(validation_code(para, assumption), sender) @@ -430,7 +430,7 @@ where sender ), Request::SessionIndexForChild(sender) => - query!(SessionIndexForChild, child_session_index(), ver = 1, sender), + query!(SessionIndexForChild, session_index_for_child(), ver = 1, sender), Request::ValidationCode(para, assumption, sender) => query!(ValidationCode, validation_code(para, assumption), ver = 1, sender), Request::ValidationCodeByHash(validation_code_hash, sender) => query!( diff --git a/node/core/runtime-api/src/tests.rs b/node/core/runtime-api/src/tests.rs index 588ae34cc090..d9e5700ff691 100644 --- a/node/core/runtime-api/src/tests.rs +++ b/node/core/runtime-api/src/tests.rs @@ -43,7 +43,7 @@ struct MockRuntimeApi { availability_cores: Vec, availability_cores_wait: Arc>, validation_data: HashMap, - child_session_index: SessionIndex, + session_index_for_child: SessionIndex, session_info: HashMap, validation_code: HashMap, validation_code_by_hash: HashMap, @@ -121,8 +121,8 @@ sp_api::mock_impl_runtime_apis! { ) } - fn child_session_index(&self) -> SessionIndex { - self.child_session_index.clone() + fn session_index_for_child(&self) -> SessionIndex { + self.session_index_for_child.clone() } fn session_info(&self, index: SessionIndex) -> Option { @@ -483,7 +483,7 @@ fn requests_check_validation_outputs() { } #[test] -fn requests_child_session_index() { +fn requests_session_index_for_child() { let (ctx, mut ctx_handle) = make_subsystem_context(TaskExecutor::new()); let runtime_api = Arc::new(MockRuntimeApi::default()); let relay_parent = [1; 32].into(); @@ -500,7 +500,7 @@ fn requests_child_session_index() { }) .await; - assert_eq!(rx.await.unwrap().unwrap(), runtime_api.child_session_index); + assert_eq!(rx.await.unwrap().unwrap(), runtime_api.session_index_for_child); ctx_handle.send(FromOverseer::Signal(OverseerSignal::Conclude)).await; }; diff --git a/node/malus/src/variants/back_garbage_candidate.rs b/node/malus/src/variants/back_garbage_candidate.rs index b27cf36770a2..06938f2a4fea 100644 --- a/node/malus/src/variants/back_garbage_candidate.rs +++ b/node/malus/src/variants/back_garbage_candidate.rs @@ -141,7 +141,7 @@ where Some("malus"), Box::pin(async move { let relay_parent = candidate_descriptor.relay_parent; - let session_index = util::request_child_session_index( + let session_index = util::request_session_index_for_child( relay_parent, &mut subsystem_sender, ) diff --git a/node/network/availability-distribution/src/requester/session_cache.rs b/node/network/availability-distribution/src/requester/session_cache.rs index a09af708d48b..a0d32e0626fb 100644 --- a/node/network/availability-distribution/src/requester/session_cache.rs +++ b/node/network/availability-distribution/src/requester/session_cache.rs @@ -105,7 +105,7 @@ impl SessionCache { Context: SubsystemContext, F: FnOnce(&SessionInfo) -> R, { - let session_index = runtime.get_child_session_index(ctx.sender(), parent).await?; + let session_index = runtime.get_session_index_for_child(ctx.sender(), parent).await?; if let Some(o_info) = self.session_info_cache.get(&session_index) { tracing::trace!(target: LOG_TARGET, session_index, "Got session from lru"); diff --git a/node/network/collator-protocol/src/collator_side/mod.rs b/node/network/collator-protocol/src/collator_side/mod.rs index a15c31ee538b..58d6898bc2ac 100644 --- a/node/network/collator-protocol/src/collator_side/mod.rs +++ b/node/network/collator-protocol/src/collator_side/mod.rs @@ -434,7 +434,7 @@ where Context: SubsystemContext, Context: overseer::SubsystemContext, { - let session_index = runtime.get_child_session_index(ctx.sender(), relay_parent).await?; + let session_index = runtime.get_session_index_for_child(ctx.sender(), relay_parent).await?; let info = &runtime .get_session_info_by_index(ctx.sender(), relay_parent, session_index) .await? diff --git a/node/network/dispute-distribution/src/sender/mod.rs b/node/network/dispute-distribution/src/sender/mod.rs index dbdc3ccb41b1..16ae0d2abcb3 100644 --- a/node/network/dispute-distribution/src/sender/mod.rs +++ b/node/network/dispute-distribution/src/sender/mod.rs @@ -329,7 +329,7 @@ async fn get_active_session_indeces( ) -> Result> { let mut indeces = HashMap::new(); for head in active_heads { - let session_index = runtime.get_child_session_index(ctx.sender(), *head).await?; + let session_index = runtime.get_session_index_for_child(ctx.sender(), *head).await?; indeces.insert(session_index, *head); } Ok(indeces) diff --git a/node/network/gossip-support/src/lib.rs b/node/network/gossip-support/src/lib.rs index 9086ceb45cc5..a12fbf82d236 100644 --- a/node/network/gossip-support/src/lib.rs +++ b/node/network/gossip-support/src/lib.rs @@ -200,7 +200,7 @@ where { for leaf in leaves { let current_index = - util::request_child_session_index(leaf, ctx.sender()).await.await??; + util::request_session_index_for_child(leaf, ctx.sender()).await.await??; let since_failure = self.last_failure.map(|i| i.elapsed()).unwrap_or_default(); let force_request = since_failure >= BACKOFF_DURATION; let leaf_session = Some((current_index, leaf)); diff --git a/node/network/statement-distribution/src/lib.rs b/node/network/statement-distribution/src/lib.rs index a5f01e7b3b01..502e2557cef9 100644 --- a/node/network/statement-distribution/src/lib.rs +++ b/node/network/statement-distribution/src/lib.rs @@ -1799,7 +1799,7 @@ impl StatementDistributionSubsystem { ); let session_index = - runtime.get_child_session_index(ctx.sender(), relay_parent).await?; + runtime.get_session_index_for_child(ctx.sender(), relay_parent).await?; let info = runtime .get_session_info_by_index(ctx.sender(), relay_parent, session_index) .await?; diff --git a/node/subsystem-util/src/lib.rs b/node/subsystem-util/src/lib.rs index 5aa9c0bdc3ec..bf120c945f02 100644 --- a/node/subsystem-util/src/lib.rs +++ b/node/subsystem-util/src/lib.rs @@ -209,7 +209,7 @@ specialize_requests! { fn request_availability_cores() -> Vec; AvailabilityCores; fn request_persisted_validation_data(para_id: ParaId, assumption: OccupiedCoreAssumption) -> Option; PersistedValidationData; fn request_assumed_validation_data(para_id: ParaId, expected_persisted_validation_data_hash: Hash) -> Option<(PersistedValidationData, ValidationCodeHash)>; AssumedValidationData; - fn request_child_session_index() -> SessionIndex; SessionIndexForChild; + fn request_session_index_for_child() -> SessionIndex; SessionIndexForChild; fn request_validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) -> Option; ValidationCode; fn request_validation_code_by_hash(validation_code_hash: ValidationCodeHash) -> Option; ValidationCodeByHash; fn request_candidate_pending_availability(para_id: ParaId) -> Option; CandidatePendingAvailability; @@ -326,12 +326,12 @@ impl Validator { keystore: SyncCryptoStorePtr, sender: &mut impl SubsystemSender, ) -> Result { - // Note: request_validators and request_child_session_index do not and cannot + // Note: request_validators and request_session_index_for_child do not and cannot // run concurrently: they both have a mutable handle to the same sender. // However, each of them returns a oneshot::Receiver, and those are resolved concurrently. let (validators, session_index) = futures::try_join!( request_validators(parent, sender).await, - request_child_session_index(parent, sender).await, + request_session_index_for_child(parent, sender).await, )?; let signing_context = SigningContext { session_index: session_index?, parent_hash: parent }; diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index 1df917704198..faacadb35574 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -102,7 +102,7 @@ impl RollingSessionWindow { window_size: SessionWindowSize, block_hash: Hash, ) -> Result { - let session_index = get_child_session_index(ctx, block_hash).await?; + let session_index = get_session_index_for_child(ctx, block_hash).await?; let window_start = session_index.saturating_sub(window_size.get() - 1); @@ -160,7 +160,7 @@ impl RollingSessionWindow { ctx: &mut (impl SubsystemContext + overseer::SubsystemContext), block_hash: Hash, ) -> Result { - let session_index = get_child_session_index(ctx, block_hash).await?; + let session_index = get_session_index_for_child(ctx, block_hash).await?; let old_window_start = self.earliest_session; @@ -212,7 +212,7 @@ impl RollingSessionWindow { } } -async fn get_child_session_index( +async fn get_session_index_for_child( ctx: &mut (impl SubsystemContext + overseer::SubsystemContext), block_hash: Hash, ) -> Result { diff --git a/node/subsystem-util/src/runtime/mod.rs b/node/subsystem-util/src/runtime/mod.rs index d5aa60040951..4440ec52176e 100644 --- a/node/subsystem-util/src/runtime/mod.rs +++ b/node/subsystem-util/src/runtime/mod.rs @@ -36,7 +36,7 @@ use polkadot_primitives::{ }; use crate::{ - request_availability_cores, request_candidate_events, request_child_session_index, + request_availability_cores, request_candidate_events, request_session_index_for_child, request_session_info, request_validation_code_by_hash, request_validator_groups, }; @@ -119,7 +119,7 @@ impl RuntimeInfo { /// Retrieve the current session index.- fix the name and comment. /// Add comment about why using the child session index. - pub async fn get_child_session_index( + pub async fn get_session_index_for_child( &mut self, sender: &mut Sender, parent: Hash, @@ -130,7 +130,8 @@ impl RuntimeInfo { match self.session_index_cache.get(&parent) { Some(index) => Ok(*index), None => { - let index = recv_runtime(request_child_session_index(parent, sender).await).await?; + let index = + recv_runtime(request_session_index_for_child(parent, sender).await).await?; self.session_index_cache.put(parent, index); Ok(index) }, @@ -146,7 +147,7 @@ impl RuntimeInfo { where Sender: SubsystemSender, { - let session_index = self.get_child_session_index(sender, parent).await?; + let session_index = self.get_session_index_for_child(sender, parent).await?; self.get_session_info_by_index(sender, parent, session_index).await } @@ -195,7 +196,7 @@ impl RuntimeInfo { Payload: EncodeAs + Clone, RealPayload: Encode + Clone, { - let session_index = self.get_child_session_index(sender, parent).await?; + let session_index = self.get_session_index_for_child(sender, parent).await?; let info = self.get_session_info_by_index(sender, parent, session_index).await?; Ok(check_signature(session_index, &info.session_info, parent, signed)) } diff --git a/primitives/src/v2/mod.rs b/primitives/src/v2/mod.rs index a45c46c4b295..065d5cc3c057 100644 --- a/primitives/src/v2/mod.rs +++ b/primitives/src/v2/mod.rs @@ -171,7 +171,7 @@ sp_api::decl_runtime_apis! { /// Returns the session index expected at a child of the block. /// /// This can be used to instantiate a `SigningContext`. - fn child_session_index() -> v1::SessionIndex; + fn session_index_for_child() -> v1::SessionIndex; /// Old method to fetch v1 session info. #[changed_in(2)] diff --git a/roadmap/implementers-guide/src/node/approval/approval-voting.md b/roadmap/implementers-guide/src/node/approval/approval-voting.md index 6a11cbc77178..adb95e1f6389 100644 --- a/roadmap/implementers-guide/src/node/approval/approval-voting.md +++ b/roadmap/implementers-guide/src/node/approval/approval-voting.md @@ -177,7 +177,7 @@ On receiving an `OverseerSignal::ActiveLeavesUpdate(update)`: * We update the `StoredBlockRange` and the `BlockNumber` maps. * We use the `RuntimeApiSubsystem` to determine information about these blocks. It is generally safe to assume that runtime state is available for recent, unfinalized blocks. In the case that it isn't, it means that we are catching up to the head of the chain and needn't worry about assignments to those blocks anyway, as the security assumption of the protocol tolerates nodes being temporarily offline or out-of-date. * We fetch the set of candidates included by each block by dispatching a `RuntimeApiRequest::CandidateEvents` and checking the `CandidateIncluded` events. - * We fetch the session of the block by dispatching a `child_session_index` request with the parent-hash of the block. + * We fetch the session of the block by dispatching a `session_index_for_child` request with the parent-hash of the block. * If the `session index - APPROVAL_SESSIONS > state.earliest_session`, then bump `state.earliest_sessions` to that amount and prune earlier sessions. * If the session isn't in our `state.session_info`, load the session info for it and for all sessions since the earliest-session, including the earliest-session, if that is missing. And it can be, just after pruning, if we've done a big jump forward, as is the case when we've just finished chain synchronization. * If any of the runtime API calls fail, we just warn and skip the block. diff --git a/roadmap/implementers-guide/src/runtime-api/session-index.md b/roadmap/implementers-guide/src/runtime-api/session-index.md index 587e8b7918ea..1baf6a167dbb 100644 --- a/roadmap/implementers-guide/src/runtime-api/session-index.md +++ b/roadmap/implementers-guide/src/runtime-api/session-index.md @@ -8,5 +8,5 @@ This session index can be used to derive a [`SigningContext`](../types/candidate ```rust /// Returns the session index expected at a child of the block. -fn child_session_index(at: Block) -> SessionIndex; +fn session_index_for_child(at: Block) -> SessionIndex; ``` diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index d93f1f022bf5..c0b6e2d27c4e 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -3038,8 +3038,8 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::check_validation_outputs::(para_id, outputs) } - fn child_session_index() -> SessionIndex { - parachains_runtime_api_impl::child_session_index::() + fn session_index_for_child() -> SessionIndex { + parachains_runtime_api_impl::session_index_for_child::() } fn validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) diff --git a/runtime/parachains/src/runtime_api_impl/v1.rs b/runtime/parachains/src/runtime_api_impl/v1.rs index da9e8e5e1404..994a720bc590 100644 --- a/runtime/parachains/src/runtime_api_impl/v1.rs +++ b/runtime/parachains/src/runtime_api_impl/v1.rs @@ -264,8 +264,8 @@ pub fn check_validation_outputs( >::check_validation_outputs_for_runtime_api(para_id, outputs) } -/// Implementation for the `child_session_index` function of the runtime API. -pub fn child_session_index() -> SessionIndex { +/// Implementation for the `session_index_for_child` function of the runtime API. +pub fn session_index_for_child() -> SessionIndex { // Just returns the session index from `inclusion`. Runtime APIs follow // initialization so the initializer will have applied any pending session change // which is expected at the child of the block whose context the runtime API was invoked @@ -281,7 +281,7 @@ pub fn child_session_index() -> SessionIndex { /// Gets next, current and some historical authority ids using `session_info` module. pub fn relevant_authority_ids( ) -> Vec { - let current_session_index = child_session_index::(); + let current_session_index = session_index_for_child::(); let earliest_stored_session = >::earliest_stored_session(); // Due to `max_validators`, the `SessionInfo` stores only the validators who are actively diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index 3d664bc918f1..984fe1609352 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -1792,8 +1792,8 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::check_validation_outputs::(para_id, outputs) } - fn child_session_index() -> SessionIndex { - parachains_runtime_api_impl::child_session_index::() + fn session_index_for_child() -> SessionIndex { + parachains_runtime_api_impl::session_index_for_child::() } fn validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 1b420c6818ec..8208fa548eae 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -1188,8 +1188,8 @@ sp_api::impl_runtime_apis! { runtime_api_impl::check_validation_outputs::(para_id, outputs) } - fn child_session_index() -> SessionIndex { - runtime_api_impl::child_session_index::() + fn session_index_for_child() -> SessionIndex { + runtime_api_impl::session_index_for_child::() } fn validation_code(para_id: Id, assumption: OccupiedCoreAssumption) diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index 0120bb085673..eed721ad8896 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -835,8 +835,8 @@ sp_api::impl_runtime_apis! { runtime_impl::check_validation_outputs::(para_id, outputs) } - fn child_session_index() -> SessionIndex { - runtime_impl::child_session_index::() + fn session_index_for_child() -> SessionIndex { + runtime_impl::session_index_for_child::() } fn validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index fcc0e7a7266d..318018dd6a5e 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1220,8 +1220,8 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::check_validation_outputs::(para_id, outputs) } - fn child_session_index() -> SessionIndex { - parachains_runtime_api_impl::child_session_index::() + fn session_index_for_child() -> SessionIndex { + parachains_runtime_api_impl::session_index_for_child::() } fn validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) From a814a2666381aa4a856727259101da4f1a21b336 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 18 Jan 2022 12:58:52 +0000 Subject: [PATCH 04/10] fix comments Signed-off-by: Andrei Sandu --- node/subsystem-util/src/rolling_session_window.rs | 5 +++++ node/subsystem-util/src/runtime/mod.rs | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index faacadb35574..f36e3b8edd2c 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -212,6 +212,11 @@ impl RollingSessionWindow { } } +// Returns the session index expected at any child of the `parent` block. +// +// Note: This internal `fn` is a duplicate of the one exported by `subsystem-util`, +// that is because `RollingSessionWindow` already implements it's own caching mechanism. +// It would not make sense to have 2 layers of caching. async fn get_session_index_for_child( ctx: &mut (impl SubsystemContext + overseer::SubsystemContext), block_hash: Hash, diff --git a/node/subsystem-util/src/runtime/mod.rs b/node/subsystem-util/src/runtime/mod.rs index 4440ec52176e..c3ee4368b2f4 100644 --- a/node/subsystem-util/src/runtime/mod.rs +++ b/node/subsystem-util/src/runtime/mod.rs @@ -117,8 +117,8 @@ impl RuntimeInfo { } } - /// Retrieve the current session index.- fix the name and comment. - /// Add comment about why using the child session index. + /// Returns the session index expected at any child of the `parent` block. + /// This does not return the session index for the `parent` block. pub async fn get_session_index_for_child( &mut self, sender: &mut Sender, From 20363b27f93d072530cc3006c816730ccde12d4b Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Tue, 18 Jan 2022 20:11:48 +0000 Subject: [PATCH 05/10] renaming and more comments Signed-off-by: Andrei Sandu --- .../src/requester/session_cache.rs | 6 ++++-- .../network/dispute-distribution/src/sender/mod.rs | 5 +++-- .../dispute-distribution/src/sender/send_task.rs | 6 ++++-- node/network/statement-distribution/src/lib.rs | 5 +++-- node/subsystem-util/src/runtime/mod.rs | 14 +++++++------- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/node/network/availability-distribution/src/requester/session_cache.rs b/node/network/availability-distribution/src/requester/session_cache.rs index a0d32e0626fb..10fda8cd9c6a 100644 --- a/node/network/availability-distribution/src/requester/session_cache.rs +++ b/node/network/availability-distribution/src/requester/session_cache.rs @@ -177,13 +177,15 @@ impl SessionCache { &self, ctx: &mut Context, runtime: &mut RuntimeInfo, - parent: Hash, + relay_parent: Hash, session_index: SessionIndex, ) -> Result, Error> where Context: SubsystemContext, { - let info = runtime.get_session_info_by_index(ctx.sender(), parent, session_index).await?; + let info = runtime + .get_session_info_by_index(ctx.sender(), relay_parent, session_index) + .await?; let discovery_keys = info.session_info.discovery_keys.clone(); let mut validator_groups = info.session_info.validator_groups.clone(); diff --git a/node/network/dispute-distribution/src/sender/mod.rs b/node/network/dispute-distribution/src/sender/mod.rs index 16ae0d2abcb3..8ee4e576b2b1 100644 --- a/node/network/dispute-distribution/src/sender/mod.rs +++ b/node/network/dispute-distribution/src/sender/mod.rs @@ -309,7 +309,7 @@ impl DisputeSender { ctx: &mut Context, runtime: &mut RuntimeInfo, ) -> Result { - let new_sessions = get_active_session_indeces(ctx, runtime, &self.active_heads).await?; + let new_sessions = get_active_session_indices(ctx, runtime, &self.active_heads).await?; let new_sessions_raw: HashSet<_> = new_sessions.keys().collect(); let old_sessions_raw: HashSet<_> = self.active_sessions.keys().collect(); let updated = new_sessions_raw != old_sessions_raw; @@ -322,12 +322,13 @@ impl DisputeSender { /// Retrieve the currently active sessions. /// /// List is all indices of all active sessions together with the head that was used for the query. -async fn get_active_session_indeces( +async fn get_active_session_indices( ctx: &mut Context, runtime: &mut RuntimeInfo, active_heads: &Vec, ) -> Result> { let mut indeces = HashMap::new(); + // Iterate all heads we track as active and fetch the child' session indices. for head in active_heads { let session_index = runtime.get_session_index_for_child(ctx.sender(), *head).await?; indeces.insert(session_index, *head); diff --git a/node/network/dispute-distribution/src/sender/send_task.rs b/node/network/dispute-distribution/src/sender/send_task.rs index 8a71a0cacbfe..ed4aab74939a 100644 --- a/node/network/dispute-distribution/src/sender/send_task.rs +++ b/node/network/dispute-distribution/src/sender/send_task.rs @@ -204,7 +204,8 @@ impl SendTask { active_sessions: &HashMap, ) -> Result> { let ref_head = self.request.0.candidate_receipt.descriptor.relay_parent; - // Parachain validators: + // Retrieve all authorities which participated in the parachain consensus of the session + // in which the candidate was backed. let info = runtime .get_session_info_by_index(ctx.sender(), ref_head, self.request.0.session_index) .await?; @@ -219,7 +220,8 @@ impl SendTask { .map(|(_, v)| v.clone()) .collect(); - // Current authorities: + // Retrieve all authorities for the current session as indicated by the active + // heads we are tracking. for (session_index, head) in active_sessions.iter() { let info = runtime.get_session_info_by_index(ctx.sender(), *head, *session_index).await?; diff --git a/node/network/statement-distribution/src/lib.rs b/node/network/statement-distribution/src/lib.rs index 502e2557cef9..06a1d36bcc1e 100644 --- a/node/network/statement-distribution/src/lib.rs +++ b/node/network/statement-distribution/src/lib.rs @@ -624,9 +624,9 @@ struct ActiveHeadData { statements: IndexMap, /// Large statements we are waiting for with associated meta data. waiting_large_statements: HashMap, - /// The validators at this head. + /// The parachain validators at the head's child session index. validators: Vec, - /// The session index this head is at. + /// The session index this head's child is at. session_index: sp_staking::SessionIndex, /// How many `Seconded` statements we've seen per validator. seconded_counts: HashMap, @@ -1798,6 +1798,7 @@ impl StatementDistributionSubsystem { "New active leaf", ); + // Retrieve the parachain validators at the child of the head we track. let session_index = runtime.get_session_index_for_child(ctx.sender(), relay_parent).await?; let info = runtime diff --git a/node/subsystem-util/src/runtime/mod.rs b/node/subsystem-util/src/runtime/mod.rs index c3ee4368b2f4..d7afac0b58c2 100644 --- a/node/subsystem-util/src/runtime/mod.rs +++ b/node/subsystem-util/src/runtime/mod.rs @@ -142,14 +142,14 @@ impl RuntimeInfo { pub async fn get_session_info<'a, Sender>( &'a mut self, sender: &mut Sender, - parent: Hash, + relay_parent: Hash, ) -> Result<&'a ExtendedSessionInfo> where Sender: SubsystemSender, { - let session_index = self.get_session_index_for_child(sender, parent).await?; + let session_index = self.get_session_index_for_child(sender, relay_parent).await?; - self.get_session_info_by_index(sender, parent, session_index).await + self.get_session_info_by_index(sender, relay_parent, session_index).await } /// Get `ExtendedSessionInfo` by session index. @@ -186,7 +186,7 @@ impl RuntimeInfo { pub async fn check_signature( &mut self, sender: &mut Sender, - parent: Hash, + relay_parent: Hash, signed: UncheckedSigned, ) -> Result< std::result::Result, UncheckedSigned>, @@ -196,9 +196,9 @@ impl RuntimeInfo { Payload: EncodeAs + Clone, RealPayload: Encode + Clone, { - let session_index = self.get_session_index_for_child(sender, parent).await?; - let info = self.get_session_info_by_index(sender, parent, session_index).await?; - Ok(check_signature(session_index, &info.session_info, parent, signed)) + let session_index = self.get_session_index_for_child(sender, relay_parent).await?; + let info = self.get_session_info_by_index(sender, relay_parent, session_index).await?; + Ok(check_signature(session_index, &info.session_info, relay_parent, signed)) } /// Build `ValidatorInfo` for the current session. From 3494d266d0247499c971b48c53d3ac889c5ecc61 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 19 Jan 2022 17:00:38 +0000 Subject: [PATCH 06/10] review feedback Signed-off-by: Andrei Sandu --- .../availability-distribution/src/requester/mod.rs | 9 +++------ node/network/dispute-distribution/src/sender/mod.rs | 6 ++++-- node/network/statement-distribution/src/lib.rs | 2 +- node/subsystem-types/src/lib.rs | 2 +- node/subsystem-util/src/rolling_session_window.rs | 6 +++--- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/node/network/availability-distribution/src/requester/mod.rs b/node/network/availability-distribution/src/requester/mod.rs index d78dcf4b936c..fea5e7aae737 100644 --- a/node/network/availability-distribution/src/requester/mod.rs +++ b/node/network/availability-distribution/src/requester/mod.rs @@ -98,15 +98,12 @@ impl Requester { tracing::trace!(target: LOG_TARGET, ?update, "Update fetching heads"); let ActiveLeavesUpdate { activated, deactivated } = update; // Stale leaves happen after a reversion - we don't want to re-run availability there. - if let Some(leaf) = activated.and_then(|h| match h.status { - LeafStatus::Stale => None, - LeafStatus::Fresh => Some(h), - }) { + if let Some(leaf) = activated.filter(|leaf| leaf.status == LeafStatus::Fresh) { + // Order important! We need to handle activated, prior to deactivated, otherwise we might + // cancel still needed jobs. self.start_requesting_chunks(ctx, runtime, leaf).await?; } - // Order important! We need to handle activated, prior to deactivated, otherwise we might - // cancel still needed jobs. self.stop_requesting_chunks(deactivated.into_iter()); Ok(()) } diff --git a/node/network/dispute-distribution/src/sender/mod.rs b/node/network/dispute-distribution/src/sender/mod.rs index 8ee4e576b2b1..ee2e15b57648 100644 --- a/node/network/dispute-distribution/src/sender/mod.rs +++ b/node/network/dispute-distribution/src/sender/mod.rs @@ -194,12 +194,14 @@ impl DisputeSender { // storage. We will iterate `active_sessions` to find a suitable head. We assume that there is at // least one active head which, by `session_index`, is at least as recent as the `dispute` passed in. // We need to avoid picking an older one from a session that might not yet exist in storage. - // Related to https://github.com/paritytech/polkadot/issues/4730. + // Related to . let ref_head = self .active_sessions .iter() .find_map(|(active_session_index, head_hash)| { - // Pick the first one that is at least as recent as the dispute. + // There might be more than one session index that is at least as recent as the dispute + // so we just pick the first one. Keep in mind we are talking about the session index for the + // child of block identified by `head_hash` and not the session index for the block. if active_session_index >= &session_index { Some(head_hash) } else { diff --git a/node/network/statement-distribution/src/lib.rs b/node/network/statement-distribution/src/lib.rs index 06a1d36bcc1e..2c522a340a2a 100644 --- a/node/network/statement-distribution/src/lib.rs +++ b/node/network/statement-distribution/src/lib.rs @@ -626,7 +626,7 @@ struct ActiveHeadData { waiting_large_statements: HashMap, /// The parachain validators at the head's child session index. validators: Vec, - /// The session index this head's child is at. + /// The current session index of this fork. session_index: sp_staking::SessionIndex, /// How many `Seconded` statements we've seen per validator. seconded_counts: HashMap, diff --git a/node/subsystem-types/src/lib.rs b/node/subsystem-types/src/lib.rs index dca7d56a98b4..e2150e14abe5 100644 --- a/node/subsystem-types/src/lib.rs +++ b/node/subsystem-types/src/lib.rs @@ -40,7 +40,7 @@ pub use polkadot_node_jaeger as jaeger; const ACTIVE_LEAVES_SMALLVEC_CAPACITY: usize = 8; /// The status of an activated leaf. -#[derive(Debug, Clone)] +#[derive(Clone, Debug, PartialEq)] pub enum LeafStatus { /// A leaf is fresh when it's the first time the leaf has been encountered. /// Most leaves should be fresh. diff --git a/node/subsystem-util/src/rolling_session_window.rs b/node/subsystem-util/src/rolling_session_window.rs index f36e3b8edd2c..9c3753c26245 100644 --- a/node/subsystem-util/src/rolling_session_window.rs +++ b/node/subsystem-util/src/rolling_session_window.rs @@ -214,9 +214,9 @@ impl RollingSessionWindow { // Returns the session index expected at any child of the `parent` block. // -// Note: This internal `fn` is a duplicate of the one exported by `subsystem-util`, -// that is because `RollingSessionWindow` already implements it's own caching mechanism. -// It would not make sense to have 2 layers of caching. +// Note: We could use `RuntimeInfo::get_session_index_for_child` here but it's +// cleaner to just call the runtime API directly without needing to create an instance +// of `RuntimeInfo`. async fn get_session_index_for_child( ctx: &mut (impl SubsystemContext + overseer::SubsystemContext), block_hash: Hash, From 4f3aced36f5cf4c5299628d60ae18e4961cf1a15 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 26 Jan 2022 10:26:09 +0000 Subject: [PATCH 07/10] Run Fetch task in correct session Signed-off-by: Andrei Sandu --- .../src/requester/mod.rs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/node/network/availability-distribution/src/requester/mod.rs b/node/network/availability-distribution/src/requester/mod.rs index 63da4f7eee14..93db2d9b7a96 100644 --- a/node/network/availability-distribution/src/requester/mod.rs +++ b/node/network/availability-distribution/src/requester/mod.rs @@ -206,17 +206,21 @@ impl Requester { let tx = self.tx.clone(); let metrics = self.metrics.clone(); + // We need to run the fetching task in the context of the leaf session, + // so we need to pass in the parent of the leaf to `with_session_info()`. + let leaf_parent = + get_block_ancestors(ctx, leaf, 1).await?.into_iter().next().unwrap_or(leaf); let task_cfg = self .session_cache .with_session_info( ctx, runtime, - // We use leaf here, as relay_parent must be in the same session as the - // leaf. This is guaranteed by runtime which ensures that cores are cleared - // at session boundaries. At the same time, only leaves are guaranteed to - // be fetchable by the state trie. - leaf, - |info| FetchTaskConfig::new(leaf, &core, tx, metrics, info), + // We need to use the leaf parent here, such that we fetch the correct backing group + // when the candidate was backed at the session boudnary. + // The state trie still contains the block as the chances of this parent to be + // already finalized are negligible. + leaf_parent, + |info| FetchTaskConfig::new(leaf_parent, &core, tx, metrics, info), ) .await?; @@ -274,7 +278,7 @@ where // `head` is the child of the first block in `ancestors`, request its session index. let head_session_index = match ancestors_iter.next() { - Some(parent) => runtime.get_session_index(ctx.sender(), *parent).await?, + Some(parent) => runtime.get_session_index_for_child(ctx.sender(), *parent).await?, None => { // No first element, i.e. empty. return Ok(ancestors) @@ -285,7 +289,7 @@ where // The first parent is skipped. for parent in ancestors_iter { // Parent is the i-th ancestor, request session index for its child -- (i-1)th element. - let session_index = runtime.get_session_index(ctx.sender(), *parent).await?; + let session_index = runtime.get_session_index_for_child(ctx.sender(), *parent).await?; if session_index == head_session_index { session_ancestry_len += 1; } else { From f9e81e63ff198e72ceaac986ebe6bd9bb73eb7b9 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 26 Jan 2022 11:10:12 +0000 Subject: [PATCH 08/10] Log warning when ancestors unavailable Signed-off-by: Andrei Sandu --- .../availability-distribution/src/requester/mod.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/node/network/availability-distribution/src/requester/mod.rs b/node/network/availability-distribution/src/requester/mod.rs index 93db2d9b7a96..4124102cfbdb 100644 --- a/node/network/availability-distribution/src/requester/mod.rs +++ b/node/network/availability-distribution/src/requester/mod.rs @@ -209,7 +209,16 @@ impl Requester { // We need to run the fetching task in the context of the leaf session, // so we need to pass in the parent of the leaf to `with_session_info()`. let leaf_parent = - get_block_ancestors(ctx, leaf, 1).await?.into_iter().next().unwrap_or(leaf); + get_block_ancestors(ctx, leaf, 1).await?.into_iter().next().unwrap_or_else( + || { + tracing::warn!( + target: LOG_TARGET, + leaf = ?leaf, + "Failed to fetch ancestors for leaf" + ); + leaf + }, + ); let task_cfg = self .session_cache .with_session_info( From 320d84f25125800df3e21401b41c4b2ad78ddf65 Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 26 Jan 2022 12:10:51 +0000 Subject: [PATCH 09/10] Fixes Signed-off-by: Andrei Sandu --- .../src/requester/mod.rs | 39 ++++++++----------- .../src/requester/tests.rs | 5 ++- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/node/network/availability-distribution/src/requester/mod.rs b/node/network/availability-distribution/src/requester/mod.rs index 4124102cfbdb..6a9a86321b12 100644 --- a/node/network/availability-distribution/src/requester/mod.rs +++ b/node/network/availability-distribution/src/requester/mod.rs @@ -206,34 +206,29 @@ impl Requester { let tx = self.tx.clone(); let metrics = self.metrics.clone(); - // We need to run the fetching task in the context of the leaf session, - // so we need to pass in the parent of the leaf to `with_session_info()`. - let leaf_parent = - get_block_ancestors(ctx, leaf, 1).await?.into_iter().next().unwrap_or_else( - || { - tracing::warn!( - target: LOG_TARGET, - leaf = ?leaf, - "Failed to fetch ancestors for leaf" - ); - leaf - }, - ); let task_cfg = self .session_cache .with_session_info( ctx, runtime, - // We need to use the leaf parent here, such that we fetch the correct backing group - // when the candidate was backed at the session boudnary. - // The state trie still contains the block as the chances of this parent to be - // already finalized are negligible. - leaf_parent, - |info| FetchTaskConfig::new(leaf_parent, &core, tx, metrics, info), + // We use leaf here, the relay_parent must be in the same session as the + // leaf. This is guaranteed by runtime which ensures that cores are cleared + // at session boundaries. At the same time, only leaves are guaranteed to + // be fetchable by the state trie. + leaf, + |info| FetchTaskConfig::new(leaf, &core, tx, metrics, info), ) - .await?; - - if let Some(task_cfg) = task_cfg { + .await + .map_err(|err| { + tracing::warn!( + target: LOG_TARGET, + error = ?err, + "Failed to spawn a fetch task" + ); + err + }); + + if let Ok(Some(task_cfg)) = task_cfg { e.insert(FetchTask::start(task_cfg, ctx).await?); } // Not a validator, nothing to do. diff --git a/node/network/availability-distribution/src/requester/tests.rs b/node/network/availability-distribution/src/requester/tests.rs index f44589ff9b88..9e4a70601109 100644 --- a/node/network/availability-distribution/src/requester/tests.rs +++ b/node/network/availability-distribution/src/requester/tests.rs @@ -22,8 +22,11 @@ use polkadot_node_network_protocol::jaeger; use polkadot_node_primitives::{BlockData, ErasureChunk, PoV, SpawnNamed}; use polkadot_node_subsystem_util::runtime::RuntimeInfo; use polkadot_primitives::v1::{ - BlockNumber, CoreState, GroupIndex, Hash, Id, ScheduledCore, SessionIndex, SessionInfo, + BlockNumber, CoreState, GroupIndex, Hash, Id, ScheduledCore, SessionIndex, }; + +use polkadot_primitives::v2::SessionInfo; + use polkadot_subsystem::{ messages::{ AllMessages, AvailabilityDistributionMessage, AvailabilityStoreMessage, ChainApiMessage, From 2046a439e0ff83a9699d138ef7b17465b1bc5a4c Mon Sep 17 00:00:00 2001 From: Andrei Sandu Date: Wed, 26 Jan 2022 13:18:13 +0000 Subject: [PATCH 10/10] fix Signed-off-by: Andrei Sandu --- node/network/availability-distribution/src/requester/tests.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/node/network/availability-distribution/src/requester/tests.rs b/node/network/availability-distribution/src/requester/tests.rs index 1e5c2f5c1fd1..3f6d284d9b92 100644 --- a/node/network/availability-distribution/src/requester/tests.rs +++ b/node/network/availability-distribution/src/requester/tests.rs @@ -26,8 +26,6 @@ use polkadot_primitives::{ v2::SessionInfo, }; -use polkadot_primitives::v2::SessionInfo; - use polkadot_subsystem::{ messages::{ AllMessages, AvailabilityDistributionMessage, AvailabilityStoreMessage, ChainApiMessage,