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

Use cached session index to obtain executor params #7646

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
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
58 changes: 57 additions & 1 deletion node/core/approval-voting/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ pub(crate) mod tests {
use polkadot_node_subsystem::messages::{AllMessages, ApprovalVotingMessage};
use polkadot_node_subsystem_test_helpers::make_subsystem_context;
use polkadot_node_subsystem_util::database::Database;
use polkadot_primitives::{Id as ParaId, IndexedVec, SessionInfo, ValidatorId, ValidatorIndex};
use polkadot_primitives::{Id as ParaId, IndexedVec, SessionInfo, ValidatorId, ValidatorIndex, ExecutorParams};
pub(crate) use sp_consensus_babe::{
digests::{CompatibleDigestItem, PreDigest, SecondaryVRFPreDigest},
AllowedSlots, BabeEpochConfiguration, Epoch as BabeEpoch,
Expand Down Expand Up @@ -835,6 +835,20 @@ pub(crate) mod tests {
si_tx.send(Ok(Some(session_info.clone()))).unwrap();
}
);

assert_matches!(
handle.recv().await,
AllMessages::RuntimeApi(
RuntimeApiMessage::Request(
req_block_hash,
RuntimeApiRequest::SessionExecutorParams(idx, si_tx),
)
) => {
assert_eq!(session, idx);
assert_eq!(req_block_hash, hash);
si_tx.send(Ok(Some(ExecutorParams::default()))).unwrap();
}
);
});

futures::executor::block_on(futures::future::join(test_fut, aux_fut));
Expand Down Expand Up @@ -952,6 +966,20 @@ pub(crate) mod tests {
si_tx.send(Ok(Some(session_info.clone()))).unwrap();
}
);

assert_matches!(
handle.recv().await,
AllMessages::RuntimeApi(
RuntimeApiMessage::Request(
req_block_hash,
RuntimeApiRequest::SessionExecutorParams(idx, si_tx),
)
) => {
assert_eq!(session, idx);
assert_eq!(req_block_hash, hash);
si_tx.send(Ok(Some(ExecutorParams::default()))).unwrap();
}
);
});

futures::executor::block_on(futures::future::join(test_fut, aux_fut));
Expand Down Expand Up @@ -1172,6 +1200,20 @@ pub(crate) mod tests {
si_tx.send(Ok(Some(session_info.clone()))).unwrap();
}
);

assert_matches!(
handle.recv().await,
AllMessages::RuntimeApi(
RuntimeApiMessage::Request(
req_block_hash,
RuntimeApiRequest::SessionExecutorParams(idx, si_tx),
)
) => {
assert_eq!(session, idx);
assert_eq!(req_block_hash, hash);
si_tx.send(Ok(Some(ExecutorParams::default()))).unwrap();
}
);
});

futures::executor::block_on(futures::future::join(test_fut, aux_fut));
Expand Down Expand Up @@ -1374,6 +1416,20 @@ pub(crate) mod tests {
}
);

assert_matches!(
handle.recv().await,
AllMessages::RuntimeApi(
RuntimeApiMessage::Request(
req_block_hash,
RuntimeApiRequest::SessionExecutorParams(idx, si_tx),
)
) => {
assert_eq!(session, idx);
assert_eq!(req_block_hash, hash);
si_tx.send(Ok(Some(ExecutorParams::default()))).unwrap();
}
);

assert_matches!(
handle.recv().await,
AllMessages::ApprovalDistribution(ApprovalDistributionMessage::NewBlocks(
Expand Down
21 changes: 19 additions & 2 deletions node/core/approval-voting/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ use polkadot_node_subsystem_util::{
};
use polkadot_primitives::{
ApprovalVote, BlockNumber, CandidateHash, CandidateIndex, CandidateReceipt, DisputeStatement,
GroupIndex, Hash, PvfExecTimeoutKind, SessionIndex, SessionInfo, ValidDisputeStatementKind,
ValidatorId, ValidatorIndex, ValidatorPair, ValidatorSignature,
ExecutorParams, GroupIndex, Hash, PvfExecTimeoutKind, SessionIndex, SessionInfo,
ValidDisputeStatementKind, ValidatorId, ValidatorIndex, ValidatorPair, ValidatorSignature,
};
use sc_keystore::LocalKeystore;
use sp_application_crypto::Pair;
Expand Down Expand Up @@ -1009,6 +1009,15 @@ async fn handle_actions<Context>(
},
None => {
let ctx = &mut *ctx;
// FIXME: Should we request at `block_hash` or `relay_block_hash`?
let executor_params = match session_info_provider
.get_session_info_by_index(ctx.sender(), block_hash, session)
.await
{
Err(_) => None,
Ok(extended_session_info) =>
Some(extended_session_info.executor_params.clone()),
};
currently_checking_set
.insert_relay_block_hash(
candidate_hash,
Expand All @@ -1023,6 +1032,7 @@ async fn handle_actions<Context>(
validator_index,
block_hash,
backing_group,
executor_params,
&launch_approval_span,
)
.await
Expand Down Expand Up @@ -2467,6 +2477,7 @@ async fn launch_approval<Context>(
validator_index: ValidatorIndex,
block_hash: Hash,
backing_group: GroupIndex,
executor_params: Option<ExecutorParams>,
span: &jaeger::Span,
) -> SubsystemResult<RemoteHandle<ApprovalState>> {
let (a_tx, a_rx) = oneshot::channel();
Expand Down Expand Up @@ -2537,6 +2548,11 @@ async fn launch_approval<Context>(
// Force the move of the timer into the background task.
let _timer = timer;

let executor_params = match executor_params {
Some(ep) => ep,
None => return ApprovalState::failed(validator_index, candidate_hash),
};

let available_data = match a_rx.await {
Err(_) => return ApprovalState::failed(validator_index, candidate_hash),
Ok(Ok(a)) => a,
Expand Down Expand Up @@ -2612,6 +2628,7 @@ async fn launch_approval<Context>(
validation_code,
candidate.clone(),
available_data.pov,
executor_params,
PvfExecTimeoutKind::Approval,
val_tx,
))
Expand Down
15 changes: 14 additions & 1 deletion node/core/approval-voting/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,19 @@ async fn import_block(
si_tx.send(Ok(Some(session_info.clone()))).unwrap();
}
);
assert_matches!(
overseer_recv(overseer).await,
AllMessages::RuntimeApi(
RuntimeApiMessage::Request(
req_block_hash,
RuntimeApiRequest::SessionExecutorParams(_, si_tx),
)
) => {
// Make sure all SessionExecutorParams calls are not made for the leaf (but for its relay parent)
assert_ne!(req_block_hash, hashes[(number-1) as usize].0);
si_tx.send(Ok(Some(ExecutorParams::default()))).unwrap();
}
);
}

assert_matches!(
Expand Down Expand Up @@ -2376,7 +2389,7 @@ async fn handle_double_assignment_import(

assert_matches!(
overseer_recv(virtual_overseer).await,
AllMessages::CandidateValidation(CandidateValidationMessage::ValidateFromExhaustive(_, _, _, _, timeout, tx)) if timeout == PvfExecTimeoutKind::Approval => {
AllMessages::CandidateValidation(CandidateValidationMessage::ValidateFromExhaustive(_, _, _, _, _, timeout, tx)) if timeout == PvfExecTimeoutKind::Approval => {
tx.send(Ok(ValidationResult::Valid(Default::default(), Default::default())))
.unwrap();
}
Expand Down
19 changes: 14 additions & 5 deletions node/core/backing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,16 @@ use polkadot_node_subsystem::{
use polkadot_node_subsystem_util::{
self as util,
backing_implicit_view::{FetchError as ImplicitViewFetchError, View as ImplicitView},
request_from_runtime, request_session_index_for_child, request_validator_groups,
request_validators,
executor_params_at_relay_parent, request_from_runtime, request_session_index_for_child,
request_validator_groups, request_validators,
runtime::{prospective_parachains_mode, ProspectiveParachainsMode},
Validator,
};
use polkadot_primitives::{
BackedCandidate, CandidateCommitments, CandidateHash, CandidateReceipt,
CommittedCandidateReceipt, CoreIndex, CoreState, Hash, Id as ParaId, PersistedValidationData,
PvfExecTimeoutKind, SigningContext, ValidationCode, ValidatorId, ValidatorIndex,
ValidatorSignature, ValidityAttestation,
CommittedCandidateReceipt, CoreIndex, CoreState, ExecutorParams, Hash, Id as ParaId,
PersistedValidationData, PvfExecTimeoutKind, SigningContext, ValidationCode, ValidatorId,
ValidatorIndex, ValidatorSignature, ValidityAttestation,
};
use sp_keystore::KeystorePtr;
use statement_table::{
Expand Down Expand Up @@ -551,6 +551,7 @@ async fn request_candidate_validation(
code: ValidationCode,
candidate_receipt: CandidateReceipt,
pov: Arc<PoV>,
executor_params: ExecutorParams,
) -> Result<ValidationResult, Error> {
let (tx, rx) = oneshot::channel();

Expand All @@ -560,6 +561,7 @@ async fn request_candidate_validation(
code,
candidate_receipt,
pov,
executor_params,
PvfExecTimeoutKind::Backing,
tx,
))
Expand Down Expand Up @@ -626,6 +628,12 @@ async fn validate_and_make_available(
}
};

let executor_params = match executor_params_at_relay_parent(relay_parent, &mut sender).await {
Ok(ep) => ep,
Err(e) => return Err(Error::UtilError(e)), /* FIXME: Is it enough to just proparate
* `UtilError` here? */
};

let pov = match pov {
PoVData::Ready(pov) => pov,
PoVData::FetchFromValidator { from_validator, candidate_hash, pov_hash } =>
Expand Down Expand Up @@ -661,6 +669,7 @@ async fn validate_and_make_available(
validation_code,
candidate.clone(),
pov.clone(),
executor_params,
)
.await?
};
Expand Down
Loading
Loading