Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use cached session index to obtain executor params #1190

Merged
merged 12 commits into from
Sep 1, 2023
Merged
60 changes: 59 additions & 1 deletion polkadot/node/core/approval-voting/src/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,9 @@ 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::{
ExecutorParams, Id as ParaId, IndexedVec, SessionInfo, ValidatorId, ValidatorIndex,
};
pub(crate) use sp_consensus_babe::{
digests::{CompatibleDigestItem, PreDigest, SecondaryVRFPreDigest},
AllowedSlots, BabeEpochConfiguration, Epoch as BabeEpoch,
Expand Down Expand Up @@ -835,6 +837,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 +968,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 +1202,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 +1418,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 polkadot/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`?
s0me0ne-unkn0wn marked this conversation as resolved.
Show resolved Hide resolved
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 polkadot/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 polkadot/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 {
s0me0ne-unkn0wn marked this conversation as resolved.
Show resolved Hide resolved
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