From 688a5d9312848c73776419702765a62bf915279e Mon Sep 17 00:00:00 2001 From: Chris Sosnin <48099298+slumber@users.noreply.github.com> Date: Fri, 22 Oct 2021 15:49:26 +0300 Subject: [PATCH] Introduce new Runtime API endpoint for fetching the validation data (#3728) * Introduce new Runtime API endpoint `persisted_validation_data_with_code_hash` that will be used by the candidate validation subsystem in order to decrease amount of runtime API requests. * Node-side part of new runtime API request * Define code hash getter via macro * Rename new endpoint to `assumed_validation_data` * Docs for runtime API impl of new endpoint * AssumedValidationData specialized request function * fmt --- node/core/runtime-api/src/cache.rs | 27 ++++++++ node/core/runtime-api/src/lib.rs | 30 +++++++++ node/core/runtime-api/src/tests.rs | 63 +++++++++++++++++++ node/subsystem-types/src/messages.rs | 7 +++ node/subsystem-util/src/lib.rs | 3 +- primitives/src/v1/mod.rs | 8 +++ runtime/kusama/src/lib.rs | 10 +++ runtime/parachains/src/paras.rs | 3 +- runtime/parachains/src/runtime_api_impl/v1.rs | 46 ++++++++++++-- runtime/polkadot/src/lib.rs | 10 +++ runtime/rococo/src/lib.rs | 10 +++ runtime/test-runtime/src/lib.rs | 10 +++ runtime/westend/src/lib.rs | 10 +++ 13 files changed, 230 insertions(+), 7 deletions(-) diff --git a/node/core/runtime-api/src/cache.rs b/node/core/runtime-api/src/cache.rs index 9c3aeaa8d819..88b579402e64 100644 --- a/node/core/runtime-api/src/cache.rs +++ b/node/core/runtime-api/src/cache.rs @@ -33,6 +33,7 @@ const VALIDATORS_CACHE_SIZE: usize = 64 * 1024; const VALIDATOR_GROUPS_CACHE_SIZE: usize = 64 * 1024; 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 VALIDATION_CODE_CACHE_SIZE: usize = 10 * 1024 * 1024; @@ -80,6 +81,10 @@ pub(crate) struct RequestResultCache { (Hash, ParaId, OccupiedCoreAssumption), ResidentSizeOf>, >, + assumed_validation_data: MemoryLruCache< + (ParaId, Hash), + ResidentSizeOf>, + >, check_validation_outputs: MemoryLruCache<(Hash, ParaId, CandidateCommitments), ResidentSizeOf>, session_index_for_child: MemoryLruCache>, @@ -111,6 +116,7 @@ impl Default for RequestResultCache { validator_groups: MemoryLruCache::new(VALIDATOR_GROUPS_CACHE_SIZE), availability_cores: MemoryLruCache::new(AVAILABILITY_CORES_CACHE_SIZE), 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), validation_code: MemoryLruCache::new(VALIDATION_CODE_CACHE_SIZE), @@ -190,6 +196,21 @@ impl RequestResultCache { self.persisted_validation_data.insert(key, ResidentSizeOf(data)); } + pub(crate) fn assumed_validation_data( + &mut self, + key: (Hash, ParaId, Hash), + ) -> Option<&Option<(PersistedValidationData, ValidationCodeHash)>> { + self.assumed_validation_data.get(&(key.1, key.2)).map(|v| &v.0) + } + + pub(crate) fn cache_assumed_validation_data( + &mut self, + key: (ParaId, Hash), + data: Option<(PersistedValidationData, ValidationCodeHash)>, + ) { + self.assumed_validation_data.insert(key, ResidentSizeOf(data)); + } + pub(crate) fn check_validation_outputs( &mut self, key: (Hash, ParaId, CandidateCommitments), @@ -347,6 +368,12 @@ pub(crate) enum RequestResult { ValidatorGroups(Hash, (Vec>, GroupRotationInfo)), AvailabilityCores(Hash, Vec), PersistedValidationData(Hash, ParaId, OccupiedCoreAssumption, Option), + AssumedValidationData( + Hash, + ParaId, + Hash, + Option<(PersistedValidationData, ValidationCodeHash)>, + ), CheckValidationOutputs(Hash, ParaId, CandidateCommitments, bool), SessionIndexForChild(Hash, SessionIndex), ValidationCode(Hash, ParaId, OccupiedCoreAssumption, Option), diff --git a/node/core/runtime-api/src/lib.rs b/node/core/runtime-api/src/lib.rs index a24192c2c54f..4aadae4cfa5f 100644 --- a/node/core/runtime-api/src/lib.rs +++ b/node/core/runtime-api/src/lib.rs @@ -119,6 +119,15 @@ where PersistedValidationData(relay_parent, para_id, assumption, data) => self .requests_cache .cache_persisted_validation_data((relay_parent, para_id, assumption), data), + AssumedValidationData( + _relay_parent, + para_id, + expected_persisted_validation_data_hash, + data, + ) => self.requests_cache.cache_assumed_validation_data( + (para_id, expected_persisted_validation_data_hash), + data, + ), CheckValidationOutputs(relay_parent, para_id, commitments, b) => self .requests_cache .cache_check_validation_outputs((relay_parent, para_id, commitments), b), @@ -186,6 +195,21 @@ where Request::PersistedValidationData(para, assumption, sender) => query!(persisted_validation_data(para, assumption), sender) .map(|sender| Request::PersistedValidationData(para, assumption, sender)), + Request::AssumedValidationData( + para, + expected_persisted_validation_data_hash, + sender, + ) => query!( + assumed_validation_data(para, expected_persisted_validation_data_hash), + sender + ) + .map(|sender| { + Request::AssumedValidationData( + para, + expected_persisted_validation_data_hash, + sender, + ) + }), Request::CheckValidationOutputs(para, commitments, sender) => query!(check_validation_outputs(para, commitments), sender) .map(|sender| Request::CheckValidationOutputs(para, commitments, sender)), @@ -330,6 +354,12 @@ where query!(AvailabilityCores, availability_cores(), sender), Request::PersistedValidationData(para, assumption, sender) => query!(PersistedValidationData, persisted_validation_data(para, assumption), sender), + Request::AssumedValidationData(para, expected_persisted_validation_data_hash, sender) => + query!( + AssumedValidationData, + assumed_validation_data(para, expected_persisted_validation_data_hash), + sender + ), Request::CheckValidationOutputs(para, commitments, sender) => query!(CheckValidationOutputs, check_validation_outputs(para, commitments), sender), Request::SessionIndexForChild(sender) => diff --git a/node/core/runtime-api/src/tests.rs b/node/core/runtime-api/src/tests.rs index 983aa7d89e20..ab2156551949 100644 --- a/node/core/runtime-api/src/tests.rs +++ b/node/core/runtime-api/src/tests.rs @@ -90,6 +90,17 @@ sp_api::mock_impl_runtime_apis! { self.validation_data.get(¶).cloned() } + fn assumed_validation_data( + para_id: ParaId, + expected_persisted_validation_data_hash: Hash, + ) -> Option<(PersistedValidationData, ValidationCodeHash)> { + self.validation_data + .get(¶_id) + .cloned() + .filter(|data| data.hash() == expected_persisted_validation_data_hash) + .zip(self.validation_code.get(¶_id).map(|code| code.hash())) + } + fn check_validation_outputs( &self, para_id: ParaId, @@ -345,6 +356,58 @@ fn requests_persisted_validation_data() { futures::executor::block_on(future::join(subsystem_task, test_task)); } +#[test] +fn requests_assumed_validation_data() { + let (ctx, mut ctx_handle) = test_helpers::make_subsystem_context(TaskExecutor::new()); + let relay_parent = [1; 32].into(); + let para_a = 5.into(); + let para_b = 6.into(); + let spawner = sp_core::testing::TaskExecutor::new(); + + let validation_code = ValidationCode(vec![1, 2, 3]); + let expected_data_hash = ::default().hash(); + let expected_code_hash = validation_code.hash(); + + let mut runtime_api = MockRuntimeApi::default(); + runtime_api.validation_data.insert(para_a, Default::default()); + runtime_api.validation_code.insert(para_a, validation_code); + runtime_api.validation_data.insert(para_b, Default::default()); + let runtime_api = Arc::new(runtime_api); + + let subsystem = RuntimeApiSubsystem::new(runtime_api.clone(), Metrics(None), spawner); + let subsystem_task = run(ctx, subsystem).map(|x| x.unwrap()); + let test_task = async move { + let (tx, rx) = oneshot::channel(); + + ctx_handle + .send(FromOverseer::Communication { + msg: RuntimeApiMessage::Request( + relay_parent, + Request::AssumedValidationData(para_a, expected_data_hash, tx), + ), + }) + .await; + + assert_eq!(rx.await.unwrap().unwrap(), Some((Default::default(), expected_code_hash))); + + let (tx, rx) = oneshot::channel(); + ctx_handle + .send(FromOverseer::Communication { + msg: RuntimeApiMessage::Request( + relay_parent, + Request::AssumedValidationData(para_a, Hash::zero(), tx), + ), + }) + .await; + + assert_eq!(rx.await.unwrap().unwrap(), None); + + ctx_handle.send(FromOverseer::Signal(OverseerSignal::Conclude)).await; + }; + + futures::executor::block_on(future::join(subsystem_task, test_task)); +} + #[test] fn requests_check_validation_outputs() { let (ctx, mut ctx_handle) = test_helpers::make_subsystem_context(TaskExecutor::new()); diff --git a/node/subsystem-types/src/messages.rs b/node/subsystem-types/src/messages.rs index f9c739f9dbdf..c89ccd118904 100644 --- a/node/subsystem-types/src/messages.rs +++ b/node/subsystem-types/src/messages.rs @@ -611,6 +611,13 @@ pub enum RuntimeApiRequest { OccupiedCoreAssumption, RuntimeApiSender>, ), + /// Get the persisted validation data for a particular para along with the current validation code + /// hash, matching the data hash against an expected one. + AssumedValidationData( + ParaId, + Hash, + RuntimeApiSender>, + ), /// Sends back `true` if the validation outputs pass all acceptance criteria checks. CheckValidationOutputs( ParaId, diff --git a/node/subsystem-util/src/lib.rs b/node/subsystem-util/src/lib.rs index 4e4e194e03dc..7ef0fccffc4e 100644 --- a/node/subsystem-util/src/lib.rs +++ b/node/subsystem-util/src/lib.rs @@ -53,7 +53,7 @@ use polkadot_primitives::v1::{ AuthorityDiscoveryId, CandidateEvent, CommittedCandidateReceipt, CoreState, EncodeAs, GroupIndex, GroupRotationInfo, Hash, Id as ParaId, OccupiedCoreAssumption, PersistedValidationData, SessionIndex, SessionInfo, Signed, SigningContext, ValidationCode, - ValidatorId, ValidatorIndex, + ValidationCodeHash, ValidatorId, ValidatorIndex, }; use sp_application_crypto::AppKey; use sp_core::{traits::SpawnNamed, Public}; @@ -206,6 +206,7 @@ specialize_requests! { fn request_validator_groups() -> (Vec>, GroupRotationInfo); ValidatorGroups; 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_validation_code(para_id: ParaId, assumption: OccupiedCoreAssumption) -> Option; ValidationCode; fn request_candidate_pending_availability(para_id: ParaId) -> Option; CandidatePendingAvailability; diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index 0b7a98b854ea..8360dcd7d0f3 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -1000,6 +1000,14 @@ sp_api::decl_runtime_apis! { fn persisted_validation_data(para_id: Id, assumption: OccupiedCoreAssumption) -> Option>; + /// Returns the persisted validation data for the given `ParaId` along with the corresponding + /// validation code hash. Instead of accepting assumption about the para, matches the validation + /// data hash against an expected one and yields `None` if they're not equal. + fn assumed_validation_data( + para_id: Id, + expected_persisted_validation_data_hash: Hash, + ) -> Option<(PersistedValidationData, ValidationCodeHash)>; + /// Checks if the given validation outputs pass the acceptance criteria. fn check_validation_outputs(para_id: Id, outputs: CandidateCommitments) -> bool; diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 0cbc9df4f159..ffd94a92c02d 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -1638,6 +1638,16 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::persisted_validation_data::(para_id, assumption) } + fn assumed_validation_data( + para_id: ParaId, + expected_persisted_validation_data_hash: Hash, + ) -> Option<(PersistedValidationData, ValidationCodeHash)> { + parachains_runtime_api_impl::assumed_validation_data::( + para_id, + expected_persisted_validation_data_hash, + ) + } + fn check_validation_outputs( para_id: ParaId, outputs: primitives::v1::CandidateCommitments, diff --git a/runtime/parachains/src/paras.rs b/runtime/parachains/src/paras.rs index a087a709c802..f192d712cccd 100644 --- a/runtime/parachains/src/paras.rs +++ b/runtime/parachains/src/paras.rs @@ -365,6 +365,7 @@ pub mod pallet { /// /// Corresponding code can be retrieved with [`CodeByHash`]. #[pallet::storage] + #[pallet::getter(fn current_code_hash)] pub(super) type CurrentCodeHash = StorageMap<_, Twox64Concat, ParaId, ValidationCodeHash>; @@ -623,7 +624,7 @@ impl Pallet { /// The validation code of live para. pub(crate) fn current_code(para_id: &ParaId) -> Option { - CurrentCodeHash::::get(para_id).and_then(|code_hash| { + Self::current_code_hash(para_id).and_then(|code_hash| { let code = CodeByHash::::get(&code_hash); if code.is_none() { log::error!( diff --git a/runtime/parachains/src/runtime_api_impl/v1.rs b/runtime/parachains/src/runtime_api_impl/v1.rs index 5909ea55290b..d544ec4ed646 100644 --- a/runtime/parachains/src/runtime_api_impl/v1.rs +++ b/runtime/parachains/src/runtime_api_impl/v1.rs @@ -23,7 +23,7 @@ use crate::{ }; use primitives::v1::{ AuthorityDiscoveryId, CandidateEvent, CommittedCandidateReceipt, CoreIndex, CoreOccupied, - CoreState, GroupIndex, GroupRotationInfo, Id as ParaId, InboundDownwardMessage, + CoreState, GroupIndex, GroupRotationInfo, Hash, Id as ParaId, InboundDownwardMessage, InboundHrmpMessage, OccupiedCore, OccupiedCoreAssumption, PersistedValidationData, ScheduledCore, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, @@ -173,6 +173,16 @@ pub fn availability_cores() -> Vec( +) -> (::BlockNumber, ::Hash) { + use parity_scale_codec::Decode as _; + let relay_parent_number = >::block_number(); + let relay_parent_storage_root = T::Hash::decode(&mut &sp_io::storage::root()[..]) + .expect("storage root must decode to the Hash type; qed"); + (relay_parent_number, relay_parent_storage_root) +} + fn with_assumption( para_id: ParaId, assumption: OccupiedCoreAssumption, @@ -203,10 +213,7 @@ pub fn persisted_validation_data( para_id: ParaId, assumption: OccupiedCoreAssumption, ) -> Option> { - use parity_scale_codec::Decode as _; - let relay_parent_number = >::block_number(); - let relay_parent_storage_root = T::Hash::decode(&mut &sp_io::storage::root()[..]) - .expect("storage root must decode to the Hash type; qed"); + let (relay_parent_number, relay_parent_storage_root) = current_relay_parent::(); with_assumption::(para_id, assumption, || { crate::util::make_persisted_validation_data::( para_id, @@ -216,6 +223,35 @@ pub fn persisted_validation_data( }) } +/// Implementation for the `assumed_validation_data` function of the runtime API. +pub fn assumed_validation_data( + para_id: ParaId, + expected_persisted_validation_data_hash: Hash, +) -> Option<(PersistedValidationData, ValidationCodeHash)> { + let (relay_parent_number, relay_parent_storage_root) = current_relay_parent::(); + // This closure obtains the `persisted_validation_data` for the given `para_id` and matches + // its hash against an expected one. + let make_validation_data = || { + crate::util::make_persisted_validation_data::( + para_id, + relay_parent_number, + relay_parent_storage_root, + ) + .filter(|validation_data| validation_data.hash() == expected_persisted_validation_data_hash) + }; + + let persisted_validation_data = make_validation_data().or_else(|| { + // Try again with force enacting the core. This check only makes sense if + // the core is occupied. + >::pending_availability(para_id).and_then(|_| { + >::force_enact(para_id); + make_validation_data() + }) + }); + // If we were successful, also query current validation code hash. + persisted_validation_data.zip(>::current_code_hash(¶_id)) +} + /// Implementation for the `check_validation_outputs` function of the runtime API. pub fn check_validation_outputs( para_id: ParaId, diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index ec5885686750..94acb97954dd 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -1536,6 +1536,16 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::persisted_validation_data::(para_id, assumption) } + fn assumed_validation_data( + para_id: ParaId, + expected_persisted_validation_data_hash: Hash, + ) -> Option<(PersistedValidationData, ValidationCodeHash)> { + parachains_runtime_api_impl::assumed_validation_data::( + para_id, + expected_persisted_validation_data_hash, + ) + } + fn check_validation_outputs( para_id: ParaId, outputs: primitives::v1::CandidateCommitments, diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 35f086100137..7798a7767f63 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -1240,6 +1240,16 @@ sp_api::impl_runtime_apis! { runtime_api_impl::persisted_validation_data::(para_id, assumption) } + fn assumed_validation_data( + para_id: ParaId, + expected_persisted_validation_data_hash: Hash, + ) -> Option<(PersistedValidationData, ValidationCodeHash)> { + runtime_api_impl::assumed_validation_data::( + para_id, + expected_persisted_validation_data_hash, + ) + } + fn check_validation_outputs( para_id: Id, outputs: primitives::v1::CandidateCommitments, diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index 3fb1923f9aa0..f92f2d476e33 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -803,6 +803,16 @@ sp_api::impl_runtime_apis! { runtime_impl::persisted_validation_data::(para_id, assumption) } + fn assumed_validation_data( + para_id: ParaId, + expected_persisted_validation_data_hash: Hash, + ) -> Option<(PersistedValidationData, ValidationCodeHash)> { + runtime_impl::assumed_validation_data::( + para_id, + expected_persisted_validation_data_hash, + ) + } + fn check_validation_outputs( para_id: ParaId, outputs: primitives::v1::CandidateCommitments, diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index 4132fceeda65..633ec591508a 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1218,6 +1218,16 @@ sp_api::impl_runtime_apis! { parachains_runtime_api_impl::persisted_validation_data::(para_id, assumption) } + fn assumed_validation_data( + para_id: ParaId, + expected_persisted_validation_data_hash: Hash, + ) -> Option<(PersistedValidationData, ValidationCodeHash)> { + parachains_runtime_api_impl::assumed_validation_data::( + para_id, + expected_persisted_validation_data_hash, + ) + } + fn check_validation_outputs( para_id: ParaId, outputs: primitives::v1::CandidateCommitments,