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

pvf-precheck: hook up runtime API #4542

Merged
merged 1 commit into from
Dec 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions node/core/runtime-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
26 changes: 24 additions & 2 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,
ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode, ValidationCodeHash,
ValidatorId, ValidatorIndex,
PvfCheckStatement, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode,
ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature,
};

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

Expand Down Expand Up @@ -106,6 +107,7 @@ 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 @@ -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),
}
}
}
Expand Down Expand Up @@ -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<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 @@ -389,4 +408,7 @@ 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: 20 additions & 0 deletions node/core/runtime-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_, _, _, ()) => {},
}
}

Expand Down Expand Up @@ -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)
},
}
}

Expand Down Expand Up @@ -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)
},
}
}

Expand Down
108 changes: 106 additions & 2 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, ScrapedOnChainVotes, SessionIndex, SessionInfo, ValidationCode,
ValidationCodeHash, ValidatorId, ValidatorIndex,
PersistedValidationData, PvfCheckStatement, ScrapedOnChainVotes, SessionIndex, SessionInfo,
ValidationCode, ValidationCodeHash, ValidatorId, ValidatorIndex, ValidatorSignature,
};
use sp_core::testing::TaskExecutor;
use std::{
Expand All @@ -51,6 +51,8 @@ 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 @@ -166,6 +168,18 @@ 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 @@ -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));
}
10 changes: 7 additions & 3 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, 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::{
Expand Down Expand Up @@ -666,6 +666,10 @@ 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: 7 additions & 0 deletions primitives/src/v1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<H: Encode + Decode = Hash, N: Encode + Decode = BlockNumber> {
/// Get the current validators.
fn validators() -> Vec<ValidatorId>;
Expand Down Expand Up @@ -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<ScrapedOnChainVotes<H>>;

/// Submits a PVF pre-checking statement into the transaction pool.
fn submit_pvf_check_statement(stmt: PvfCheckStatement, signature: ValidatorSignature);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other kind of statements we imported via inherents. How come we are using the transaction pool here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was suggested by Rob here. We never really discussed exactly this part, mostly because I agree with this. On one hand, we can afford routing those through the chain: we do not expect a lot of those and they are not too heavy. On the other hand, it greatly simplifies the whole construction. The next PR is for subsystem and you'll see how simple it is.

That being said, in future we can reconsider and implement it via a network protocol if such need arises.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, I fully understand this - maybe a quick call tomorrow?

Copy link
Contributor Author

@pepyakin pepyakin Dec 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure thing.


/// 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: 11 additions & 0 deletions runtime/kusama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1814,6 +1814,17 @@ 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: 11 additions & 0 deletions runtime/polkadot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1781,6 +1781,17 @@ 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: 10 additions & 2 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, 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,
Expand Down Expand Up @@ -1372,6 +1372,14 @@ 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: 11 additions & 0 deletions runtime/test-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,17 @@ 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