From b688c6e83e722a9da36880edb051d72635d5765b Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Tue, 21 Dec 2021 15:15:30 +0100 Subject: [PATCH] pvf-precheck: hook up runtime API (#4542) This commit hooks up the API provided by #4457 to the runtime API subsystem. In a following PR this API will be consumed by the PVF pre-checking subsystem. Co-authored-by: Chris Sosnin Co-authored-by: Chris Sosnin --- Cargo.lock | 1 + node/core/runtime-api/Cargo.toml | 1 + node/core/runtime-api/src/cache.rs | 26 ++++++- node/core/runtime-api/src/lib.rs | 20 +++++ node/core/runtime-api/src/tests.rs | 108 ++++++++++++++++++++++++++- node/subsystem-types/src/messages.rs | 10 ++- primitives/src/v1/mod.rs | 7 ++ runtime/kusama/src/lib.rs | 11 +++ runtime/polkadot/src/lib.rs | 11 +++ runtime/rococo/src/lib.rs | 12 ++- runtime/test-runtime/src/lib.rs | 11 +++ runtime/westend/src/lib.rs | 11 +++ 12 files changed, 220 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fc77b190d022..fe63ae17da10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6378,6 +6378,7 @@ dependencies = [ "sp-authority-discovery", "sp-consensus-babe", "sp-core", + "sp-keyring", "tracing", ] diff --git a/node/core/runtime-api/Cargo.toml b/node/core/runtime-api/Cargo.toml index f10b5dc2c457..434be9ba63ef 100644 --- a/node/core/runtime-api/Cargo.toml +++ b/node/core/runtime-api/Cargo.toml @@ -21,6 +21,7 @@ polkadot-node-subsystem-util = { path = "../../subsystem-util" } [dev-dependencies] sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" } +sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" } futures = { version = "0.3.19", features = ["thread-pool"] } polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" } polkadot-node-primitives = { path = "../../primitives" } diff --git a/node/core/runtime-api/src/cache.rs b/node/core/runtime-api/src/cache.rs index 88b579402e64..023646611601 100644 --- a/node/core/runtime-api/src/cache.rs +++ b/node/core/runtime-api/src/cache.rs @@ -24,8 +24,8 @@ use polkadot_primitives::v1::{ AuthorityDiscoveryId, BlockNumber, CandidateCommitments, CandidateEvent, CommittedCandidateReceipt, CoreState, GroupRotationInfo, Hash, Id as ParaId, InboundDownwardMessage, InboundHrmpMessage, OccupiedCoreAssumption, PersistedValidationData, - ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode, ValidationCodeHash, - ValidatorId, ValidatorIndex, + PvfCheckStatement, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode, + ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature, }; const AUTHORITIES_CACHE_SIZE: usize = 128 * 1024; @@ -44,6 +44,7 @@ const DMQ_CONTENTS_CACHE_SIZE: usize = 64 * 1024; const INBOUND_HRMP_CHANNELS_CACHE_SIZE: usize = 64 * 1024; const CURRENT_BABE_EPOCH_CACHE_SIZE: usize = 64 * 1024; const ON_CHAIN_VOTES_CACHE_SIZE: usize = 3 * 1024; +const PVFS_REQUIRE_PRECHECK_SIZE: usize = 1024; struct ResidentSizeOf(T); @@ -106,6 +107,7 @@ pub(crate) struct RequestResultCache { >, current_babe_epoch: MemoryLruCache>, on_chain_votes: MemoryLruCache>>, + pvfs_require_precheck: MemoryLruCache>>, } impl Default for RequestResultCache { @@ -130,6 +132,7 @@ impl Default for RequestResultCache { inbound_hrmp_channels_contents: MemoryLruCache::new(INBOUND_HRMP_CHANNELS_CACHE_SIZE), current_babe_epoch: MemoryLruCache::new(CURRENT_BABE_EPOCH_CACHE_SIZE), on_chain_votes: MemoryLruCache::new(ON_CHAIN_VOTES_CACHE_SIZE), + pvfs_require_precheck: MemoryLruCache::new(PVFS_REQUIRE_PRECHECK_SIZE), } } } @@ -360,9 +363,25 @@ impl RequestResultCache { ) { self.on_chain_votes.insert(relay_parent, ResidentSizeOf(scraped)); } + + pub(crate) fn pvfs_require_precheck( + &mut self, + relay_parent: &Hash, + ) -> Option<&Vec> { + self.pvfs_require_precheck.get(relay_parent).map(|v| &v.0) + } + + pub(crate) fn cache_pvfs_require_precheck( + &mut self, + relay_parent: Hash, + pvfs: Vec, + ) { + self.pvfs_require_precheck.insert(relay_parent, ResidentSizeOf(pvfs)) + } } pub(crate) enum RequestResult { + // The structure of each variant is (relay_parent, [params,]*, result) Authorities(Hash, Vec), Validators(Hash, Vec), ValidatorGroups(Hash, (Vec>, GroupRotationInfo)), @@ -389,4 +408,7 @@ pub(crate) enum RequestResult { ), CurrentBabeEpoch(Hash, Epoch), FetchOnChainVotes(Hash, Option), + PvfsRequirePrecheck(Hash, Vec), + // This is a request with side-effects and no result, hence (). + SubmitPvfCheckStatement(Hash, PvfCheckStatement, ValidatorSignature, ()), } diff --git a/node/core/runtime-api/src/lib.rs b/node/core/runtime-api/src/lib.rs index b4d625a727d6..a5cc27cc9ac7 100644 --- a/node/core/runtime-api/src/lib.rs +++ b/node/core/runtime-api/src/lib.rs @@ -154,6 +154,9 @@ where self.requests_cache.cache_current_babe_epoch(relay_parent, epoch), FetchOnChainVotes(relay_parent, scraped) => self.requests_cache.cache_on_chain_votes(relay_parent, scraped), + PvfsRequirePrecheck(relay_parent, pvfs) => + self.requests_cache.cache_pvfs_require_precheck(relay_parent, pvfs), + SubmitPvfCheckStatement(_, _, _, ()) => {}, } } @@ -237,6 +240,12 @@ where query!(current_babe_epoch(), sender).map(|sender| Request::CurrentBabeEpoch(sender)), Request::FetchOnChainVotes(sender) => query!(on_chain_votes(), sender).map(|sender| Request::FetchOnChainVotes(sender)), + Request::PvfsRequirePrecheck(sender) => query!(pvfs_require_precheck(), sender) + .map(|sender| Request::PvfsRequirePrecheck(sender)), + request @ Request::SubmitPvfCheckStatement(_, _, _) => { + // This request is side-effecting and thus cannot be cached. + Some(request) + }, } } @@ -441,6 +450,17 @@ where query!(CurrentBabeEpoch, current_epoch(), ver = 1, sender), Request::FetchOnChainVotes(sender) => query!(FetchOnChainVotes, on_chain_votes(), ver = 1, sender), + Request::SubmitPvfCheckStatement(stmt, signature, sender) => { + query!( + SubmitPvfCheckStatement, + submit_pvf_check_statement(stmt, signature), + ver = 2, + sender + ) + }, + Request::PvfsRequirePrecheck(sender) => { + query!(PvfsRequirePrecheck, pvfs_require_precheck(), ver = 2, sender) + }, } } diff --git a/node/core/runtime-api/src/tests.rs b/node/core/runtime-api/src/tests.rs index 7b50cc5b145f..eeb56b844b1f 100644 --- a/node/core/runtime-api/src/tests.rs +++ b/node/core/runtime-api/src/tests.rs @@ -23,8 +23,8 @@ use polkadot_node_subsystem_test_helpers::make_subsystem_context; use polkadot_primitives::v1::{ AuthorityDiscoveryId, CandidateEvent, CommittedCandidateReceipt, CoreState, GroupRotationInfo, Id as ParaId, InboundDownwardMessage, InboundHrmpMessage, OccupiedCoreAssumption, - PersistedValidationData, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode, - ValidationCodeHash, ValidatorId, ValidatorIndex, + PersistedValidationData, PvfCheckStatement, ScrapedOnChainVotes, SessionIndex, SessionInfo, + ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature, }; use sp_core::testing::TaskExecutor; use std::{ @@ -51,6 +51,8 @@ struct MockRuntimeApi { hrmp_channels: HashMap>>, babe_epoch: Option, on_chain_votes: Option, + submitted_pvf_check_statement: Arc>>, + pvfs_require_precheck: Vec, } impl ProvideRuntimeApi for MockRuntimeApi { @@ -166,6 +168,18 @@ sp_api::mock_impl_runtime_apis! { fn on_chain_votes(&self) -> Option { self.on_chain_votes.clone() } + + fn submit_pvf_check_statement(stmt: PvfCheckStatement, signature: ValidatorSignature) { + self + .submitted_pvf_check_statement + .lock() + .expect("poisoned mutext") + .push((stmt, signature)); + } + + fn pvfs_require_precheck() -> Vec { + self.pvfs_require_precheck.clone() + } } impl BabeApi for MockRuntimeApi { @@ -877,3 +891,93 @@ fn request_babe_epoch() { futures::executor::block_on(future::join(subsystem_task, test_task)); } + +#[test] +fn requests_submit_pvf_check_statement() { + let (ctx, mut ctx_handle) = make_subsystem_context(TaskExecutor::new()); + let spawner = sp_core::testing::TaskExecutor::new(); + + let runtime_api = Arc::new(MockRuntimeApi::default()); + let subsystem = RuntimeApiSubsystem::new(runtime_api.clone(), Metrics(None), spawner); + let subsystem_task = run(ctx, subsystem).map(|x| x.unwrap()); + + let relay_parent = [1; 32].into(); + let test_task = async move { + let (stmt, sig) = fake_statement(); + + // Send the same statement twice. + // + // Here we just want to ensure that those requests do not go through the cache. + let (tx, rx) = oneshot::channel(); + ctx_handle + .send(FromOverseer::Communication { + msg: RuntimeApiMessage::Request( + relay_parent, + Request::SubmitPvfCheckStatement(stmt.clone(), sig.clone(), tx), + ), + }) + .await; + assert_eq!(rx.await.unwrap().unwrap(), ()); + let (tx, rx) = oneshot::channel(); + ctx_handle + .send(FromOverseer::Communication { + msg: RuntimeApiMessage::Request( + relay_parent, + Request::SubmitPvfCheckStatement(stmt.clone(), sig.clone(), tx), + ), + }) + .await; + assert_eq!(rx.await.unwrap().unwrap(), ()); + + assert_eq!( + &*runtime_api.submitted_pvf_check_statement.lock().expect("poisened mutex"), + &[(stmt.clone(), sig.clone()), (stmt.clone(), sig.clone())] + ); + + ctx_handle.send(FromOverseer::Signal(OverseerSignal::Conclude)).await; + }; + + futures::executor::block_on(future::join(subsystem_task, test_task)); + + fn fake_statement() -> (PvfCheckStatement, ValidatorSignature) { + let stmt = PvfCheckStatement { + accept: true, + subject: [1; 32].into(), + session_index: 1, + validator_index: 1.into(), + }; + let sig = sp_keyring::Sr25519Keyring::Alice.sign(&stmt.signing_payload()).into(); + (stmt, sig) + } +} + +#[test] +fn requests_pvfs_require_precheck() { + let (ctx, mut ctx_handle) = make_subsystem_context(TaskExecutor::new()); + let spawner = sp_core::testing::TaskExecutor::new(); + + let runtime_api = Arc::new({ + let mut runtime_api = MockRuntimeApi::default(); + runtime_api.pvfs_require_precheck = vec![[1; 32].into(), [2; 32].into()]; + runtime_api + }); + + let subsystem = RuntimeApiSubsystem::new(runtime_api.clone(), Metrics(None), spawner); + let subsystem_task = run(ctx, subsystem).map(|x| x.unwrap()); + + let relay_parent = [1; 32].into(); + let test_task = async move { + let (tx, rx) = oneshot::channel(); + + ctx_handle + .send(FromOverseer::Communication { + msg: RuntimeApiMessage::Request(relay_parent, Request::PvfsRequirePrecheck(tx)), + }) + .await; + + assert_eq!(rx.await.unwrap().unwrap(), vec![[1; 32].into(), [2; 32].into()]); + ctx_handle.send(FromOverseer::Signal(OverseerSignal::Conclude)).await; + }; + + futures::executor::block_on(future::join(subsystem_task, test_task)); +} diff --git a/node/subsystem-types/src/messages.rs b/node/subsystem-types/src/messages.rs index 0aa6b1abb46b..c88c8d635235 100644 --- a/node/subsystem-types/src/messages.rs +++ b/node/subsystem-types/src/messages.rs @@ -43,9 +43,9 @@ use polkadot_primitives::v1::{ CandidateHash, CandidateIndex, CandidateReceipt, CollatorId, CommittedCandidateReceipt, CoreState, GroupIndex, GroupRotationInfo, Hash, Header as BlockHeader, Id as ParaId, InboundDownwardMessage, InboundHrmpMessage, MultiDisputeStatementSet, OccupiedCoreAssumption, - PersistedValidationData, SessionIndex, SessionInfo, SignedAvailabilityBitfield, - SignedAvailabilityBitfields, ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, - ValidatorSignature, + PersistedValidationData, PvfCheckStatement, SessionIndex, SessionInfo, + SignedAvailabilityBitfield, SignedAvailabilityBitfields, ValidationCode, ValidationCodeHash, + ValidatorId, ValidatorIndex, ValidatorSignature, }; use polkadot_statement_table::v1::Misbehavior; use std::{ @@ -666,6 +666,10 @@ pub enum RuntimeApiRequest { CurrentBabeEpoch(RuntimeApiSender), /// Get all disputes in relation to a relay parent. FetchOnChainVotes(RuntimeApiSender>), + /// Submits a PVF pre-checking statement into the transaction pool. + SubmitPvfCheckStatement(PvfCheckStatement, ValidatorSignature, RuntimeApiSender<()>), + /// Returns code hashes of PVFs that require pre-checking by validators in the active set. + PvfsRequirePrecheck(RuntimeApiSender>), } /// A message to the Runtime API subsystem. diff --git a/primitives/src/v1/mod.rs b/primitives/src/v1/mod.rs index 23149fb45797..003dc681d66d 100644 --- a/primitives/src/v1/mod.rs +++ b/primitives/src/v1/mod.rs @@ -986,6 +986,7 @@ impl ApprovalVote { sp_api::decl_runtime_apis! { /// The API for querying the state of parachains on-chain. + #[api_version(2)] pub trait ParachainHost { /// Get the current validators. fn validators() -> Vec; @@ -1054,6 +1055,12 @@ sp_api::decl_runtime_apis! { /// Scrape dispute relevant from on-chain, backing votes and resolved disputes. fn on_chain_votes() -> Option>; + + /// Submits a PVF pre-checking statement into the transaction pool. + fn submit_pvf_check_statement(stmt: PvfCheckStatement, signature: ValidatorSignature); + + /// Returns code hashes of PVFs that require pre-checking by validators in the active set. + fn pvfs_require_precheck() -> Vec; } } diff --git a/runtime/kusama/src/lib.rs b/runtime/kusama/src/lib.rs index 2bb9ba978689..8c7f156f3240 100644 --- a/runtime/kusama/src/lib.rs +++ b/runtime/kusama/src/lib.rs @@ -1814,6 +1814,17 @@ sp_api::impl_runtime_apis! { fn on_chain_votes() -> Option> { parachains_runtime_api_impl::on_chain_votes::() } + + fn submit_pvf_check_statement( + stmt: primitives::v1::PvfCheckStatement, + signature: primitives::v1::ValidatorSignature, + ) { + parachains_runtime_api_impl::submit_pvf_check_statement::(stmt, signature) + } + + fn pvfs_require_precheck() -> Vec { + parachains_runtime_api_impl::pvfs_require_precheck::() + } } impl beefy_primitives::BeefyApi for Runtime { diff --git a/runtime/polkadot/src/lib.rs b/runtime/polkadot/src/lib.rs index ed0471428bf9..091879aaee39 100644 --- a/runtime/polkadot/src/lib.rs +++ b/runtime/polkadot/src/lib.rs @@ -1781,6 +1781,17 @@ sp_api::impl_runtime_apis! { fn on_chain_votes() -> Option> { parachains_runtime_api_impl::on_chain_votes::() } + + fn submit_pvf_check_statement( + stmt: primitives::v1::PvfCheckStatement, + signature: primitives::v1::ValidatorSignature, + ) { + parachains_runtime_api_impl::submit_pvf_check_statement::(stmt, signature) + } + + fn pvfs_require_precheck() -> Vec { + parachains_runtime_api_impl::pvfs_require_precheck::() + } } impl beefy_primitives::BeefyApi for Runtime { diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 3b5a83c51b0d..2e1b61aafeb9 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -38,9 +38,9 @@ use parity_scale_codec::{Decode, Encode, MaxEncodedLen}; use primitives::v1::{ AccountId, AccountIndex, Balance, BlockNumber, CandidateEvent, CommittedCandidateReceipt, CoreState, GroupRotationInfo, Hash, Id, InboundDownwardMessage, InboundHrmpMessage, Moment, - Nonce, OccupiedCoreAssumption, PersistedValidationData, ScrapedOnChainVotes, + Nonce, OccupiedCoreAssumption, PersistedValidationData, PvfCheckStatement, ScrapedOnChainVotes, SessionInfo as SessionInfoData, Signature, ValidationCode, ValidationCodeHash, ValidatorId, - ValidatorIndex, + ValidatorIndex, ValidatorSignature, }; use runtime_common::{ assigned_slots, auctions, crowdloan, impls::ToAuthor, paras_registrar, paras_sudo_wrapper, @@ -1372,6 +1372,14 @@ sp_api::impl_runtime_apis! { fn on_chain_votes() -> Option> { runtime_api_impl::on_chain_votes::() } + + fn submit_pvf_check_statement(stmt: PvfCheckStatement, signature: ValidatorSignature) { + runtime_api_impl::submit_pvf_check_statement::(stmt, signature) + } + + fn pvfs_require_precheck() -> Vec { + runtime_api_impl::pvfs_require_precheck::() + } } impl fg_primitives::GrandpaApi for Runtime { diff --git a/runtime/test-runtime/src/lib.rs b/runtime/test-runtime/src/lib.rs index 09bd3be61f18..d0f3654df1b4 100644 --- a/runtime/test-runtime/src/lib.rs +++ b/runtime/test-runtime/src/lib.rs @@ -873,6 +873,17 @@ sp_api::impl_runtime_apis! { fn on_chain_votes() -> Option> { runtime_impl::on_chain_votes::() } + + fn submit_pvf_check_statement( + stmt: primitives::v1::PvfCheckStatement, + signature: primitives::v1::ValidatorSignature, + ) { + runtime_impl::submit_pvf_check_statement::(stmt, signature) + } + + fn pvfs_require_precheck() -> Vec { + runtime_impl::pvfs_require_precheck::() + } } impl beefy_primitives::BeefyApi for Runtime { diff --git a/runtime/westend/src/lib.rs b/runtime/westend/src/lib.rs index a98e69dc55d7..e68b42491e5c 100644 --- a/runtime/westend/src/lib.rs +++ b/runtime/westend/src/lib.rs @@ -1385,6 +1385,17 @@ sp_api::impl_runtime_apis! { fn on_chain_votes() -> Option> { parachains_runtime_api_impl::on_chain_votes::() } + + fn submit_pvf_check_statement( + stmt: primitives::v1::PvfCheckStatement, + signature: primitives::v1::ValidatorSignature, + ) { + parachains_runtime_api_impl::submit_pvf_check_statement::(stmt, signature) + } + + fn pvfs_require_precheck() -> Vec { + parachains_runtime_api_impl::pvfs_require_precheck::() + } } impl beefy_primitives::BeefyApi for Runtime {