Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Revert "pvf-precheck: hook up runtime API (#4542)"
Browse files Browse the repository at this point in the history
This reverts commit c61d2ff.
  • Loading branch information
pepyakin committed Dec 22, 2021
1 parent 95a924f commit d19da26
Show file tree
Hide file tree
Showing 12 changed files with 9 additions and 220 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion node/core/runtime-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ 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" }
Expand Down
26 changes: 2 additions & 24 deletions node/core/runtime-api/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use polkadot_primitives::v1::{
AuthorityDiscoveryId, BlockNumber, CandidateCommitments, CandidateEvent,
CommittedCandidateReceipt, CoreState, GroupRotationInfo, Hash, Id as ParaId,
InboundDownwardMessage, InboundHrmpMessage, OccupiedCoreAssumption, PersistedValidationData,
PvfCheckStatement, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode,
ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature,
ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode, ValidationCodeHash,
ValidatorId, ValidatorIndex,
};

const AUTHORITIES_CACHE_SIZE: usize = 128 * 1024;
Expand All @@ -44,7 +44,6 @@ 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>(T);

Expand Down Expand Up @@ -107,7 +106,6 @@ pub(crate) struct RequestResultCache {
>,
current_babe_epoch: MemoryLruCache<Hash, DoesNotAllocate<Epoch>>,
on_chain_votes: MemoryLruCache<Hash, ResidentSizeOf<Option<ScrapedOnChainVotes>>>,
pvfs_require_precheck: MemoryLruCache<Hash, ResidentSizeOf<Vec<ValidationCodeHash>>>,
}

impl Default for RequestResultCache {
Expand All @@ -132,7 +130,6 @@ 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),
}
}
}
Expand Down Expand Up @@ -363,25 +360,9 @@ impl RequestResultCache {
) {
self.on_chain_votes.insert(relay_parent, ResidentSizeOf(scraped));
}

pub(crate) fn pvfs_require_precheck(
&mut self,
relay_parent: &Hash,
) -> Option<&Vec<ValidationCodeHash>> {
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<ValidationCodeHash>,
) {
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<AuthorityDiscoveryId>),
Validators(Hash, Vec<ValidatorId>),
ValidatorGroups(Hash, (Vec<Vec<ValidatorIndex>>, GroupRotationInfo)),
Expand All @@ -408,7 +389,4 @@ pub(crate) enum RequestResult {
),
CurrentBabeEpoch(Hash, Epoch),
FetchOnChainVotes(Hash, Option<ScrapedOnChainVotes>),
PvfsRequirePrecheck(Hash, Vec<ValidationCodeHash>),
// This is a request with side-effects and no result, hence ().
SubmitPvfCheckStatement(Hash, PvfCheckStatement, ValidatorSignature, ()),
}
20 changes: 0 additions & 20 deletions node/core/runtime-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,6 @@ 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(_, _, _, ()) => {},
}
}

Expand Down Expand Up @@ -240,12 +237,6 @@ 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)
},
}
}

Expand Down Expand Up @@ -450,17 +441,6 @@ 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)
},
}
}

Expand Down
108 changes: 2 additions & 106 deletions node/core/runtime-api/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, PvfCheckStatement, ScrapedOnChainVotes, SessionIndex, SessionInfo,
ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature,
PersistedValidationData, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode,
ValidationCodeHash, ValidatorId, ValidatorIndex,
};
use sp_core::testing::TaskExecutor;
use std::{
Expand All @@ -51,8 +51,6 @@ struct MockRuntimeApi {
hrmp_channels: HashMap<ParaId, BTreeMap<ParaId, Vec<InboundHrmpMessage>>>,
babe_epoch: Option<BabeEpoch>,
on_chain_votes: Option<ScrapedOnChainVotes>,
submitted_pvf_check_statement: Arc<Mutex<Vec<(PvfCheckStatement, ValidatorSignature)>>>,
pvfs_require_precheck: Vec<ValidationCodeHash>,
}

impl ProvideRuntimeApi<Block> for MockRuntimeApi {
Expand Down Expand Up @@ -168,18 +166,6 @@ sp_api::mock_impl_runtime_apis! {
fn on_chain_votes(&self) -> Option<ScrapedOnChainVotes> {
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<ValidationCodeHash> {
self.pvfs_require_precheck.clone()
}
}

impl BabeApi<Block> for MockRuntimeApi {
Expand Down Expand Up @@ -891,93 +877,3 @@ fn requests_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));
}
10 changes: 3 additions & 7 deletions node/subsystem-types/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, PvfCheckStatement, SessionIndex, SessionInfo,
SignedAvailabilityBitfield, SignedAvailabilityBitfields, ValidationCode, ValidationCodeHash,
ValidatorId, ValidatorIndex, ValidatorSignature,
PersistedValidationData, SessionIndex, SessionInfo, SignedAvailabilityBitfield,
SignedAvailabilityBitfields, ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex,
ValidatorSignature,
};
use polkadot_statement_table::v1::Misbehavior;
use std::{
Expand Down Expand Up @@ -666,10 +666,6 @@ pub enum RuntimeApiRequest {
CurrentBabeEpoch(RuntimeApiSender<BabeEpoch>),
/// Get all disputes in relation to a relay parent.
FetchOnChainVotes(RuntimeApiSender<Option<polkadot_primitives::v1::ScrapedOnChainVotes>>),
/// 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<Vec<ValidationCodeHash>>),
}

/// A message to the Runtime API subsystem.
Expand Down
7 changes: 0 additions & 7 deletions primitives/src/v1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,6 @@ impl ApprovalVote {

sp_api::decl_runtime_apis! {
/// The API for querying the state of parachains on-chain.
#[api_version(2)]
pub trait ParachainHost<H: Encode + Decode = Hash, N: Encode + Decode = BlockNumber> {
/// Get the current validators.
fn validators() -> Vec<ValidatorId>;
Expand Down Expand Up @@ -1055,12 +1054,6 @@ sp_api::decl_runtime_apis! {

/// Scrape dispute relevant from on-chain, backing votes and resolved disputes.
fn on_chain_votes() -> Option<ScrapedOnChainVotes<H>>;

/// 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<ValidationCodeHash>;
}
}

Expand Down
11 changes: 0 additions & 11 deletions runtime/kusama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1814,17 +1814,6 @@ sp_api::impl_runtime_apis! {
fn on_chain_votes() -> Option<ScrapedOnChainVotes<Hash>> {
parachains_runtime_api_impl::on_chain_votes::<Runtime>()
}

fn submit_pvf_check_statement(
stmt: primitives::v1::PvfCheckStatement,
signature: primitives::v1::ValidatorSignature,
) {
parachains_runtime_api_impl::submit_pvf_check_statement::<Runtime>(stmt, signature)
}

fn pvfs_require_precheck() -> Vec<ValidationCodeHash> {
parachains_runtime_api_impl::pvfs_require_precheck::<Runtime>()
}
}

impl beefy_primitives::BeefyApi<Block> for Runtime {
Expand Down
11 changes: 0 additions & 11 deletions runtime/polkadot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1781,17 +1781,6 @@ sp_api::impl_runtime_apis! {
fn on_chain_votes() -> Option<ScrapedOnChainVotes<Hash>> {
parachains_runtime_api_impl::on_chain_votes::<Runtime>()
}

fn submit_pvf_check_statement(
stmt: primitives::v1::PvfCheckStatement,
signature: primitives::v1::ValidatorSignature,
) {
parachains_runtime_api_impl::submit_pvf_check_statement::<Runtime>(stmt, signature)
}

fn pvfs_require_precheck() -> Vec<ValidationCodeHash> {
parachains_runtime_api_impl::pvfs_require_precheck::<Runtime>()
}
}

impl beefy_primitives::BeefyApi<Block> for Runtime {
Expand Down
12 changes: 2 additions & 10 deletions runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, PvfCheckStatement, ScrapedOnChainVotes,
Nonce, OccupiedCoreAssumption, PersistedValidationData, ScrapedOnChainVotes,
SessionInfo as SessionInfoData, Signature, ValidationCode, ValidationCodeHash, ValidatorId,
ValidatorIndex, ValidatorSignature,
ValidatorIndex,
};
use runtime_common::{
assigned_slots, auctions, crowdloan, impls::ToAuthor, paras_registrar, paras_sudo_wrapper,
Expand Down Expand Up @@ -1372,14 +1372,6 @@ sp_api::impl_runtime_apis! {
fn on_chain_votes() -> Option<ScrapedOnChainVotes<Hash>> {
runtime_api_impl::on_chain_votes::<Runtime>()
}

fn submit_pvf_check_statement(stmt: PvfCheckStatement, signature: ValidatorSignature) {
runtime_api_impl::submit_pvf_check_statement::<Runtime>(stmt, signature)
}

fn pvfs_require_precheck() -> Vec<ValidationCodeHash> {
runtime_api_impl::pvfs_require_precheck::<Runtime>()
}
}

impl fg_primitives::GrandpaApi<Block> for Runtime {
Expand Down
11 changes: 0 additions & 11 deletions runtime/test-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -873,17 +873,6 @@ sp_api::impl_runtime_apis! {
fn on_chain_votes() -> Option<ScrapedOnChainVotes<Hash>> {
runtime_impl::on_chain_votes::<Runtime>()
}

fn submit_pvf_check_statement(
stmt: primitives::v1::PvfCheckStatement,
signature: primitives::v1::ValidatorSignature,
) {
runtime_impl::submit_pvf_check_statement::<Runtime>(stmt, signature)
}

fn pvfs_require_precheck() -> Vec<ValidationCodeHash> {
runtime_impl::pvfs_require_precheck::<Runtime>()
}
}

impl beefy_primitives::BeefyApi<Block> for Runtime {
Expand Down
Loading

0 comments on commit d19da26

Please sign in to comment.