Skip to content

Commit

Permalink
Use cached session index to obtain executor params (#1190)
Browse files Browse the repository at this point in the history 
* Import changes from archieved repo

* Revert erroneous changes

* Fix more tests

* Resolve discussions

* Fix MORE tests

* approval-voting: launch_approval better interface (#1355)

---------

Co-authored-by: Javier Viola <javier@parity.io>
Co-authored-by: ordian <noreply@reusable.software>
Co-authored-by: ordian <write@reusable.software>
  • Loading branch information
@s0me0ne-unkn0wn @pepoviola @ordian
4 people authored Sep 1, 2023
1 parent ccdf636 commit a2b6470
Show file tree
Hide file tree
Showing 29 changed files with 809 additions and 321 deletions.
60 changes: 59 additions & 1 deletion polkadot/node/core/approval-voting/src/import.rs
View file
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
58 changes: 40 additions & 18 deletions polkadot/node/core/approval-voting/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ use polkadot_node_subsystem_util::{
self,
database::Database,
metrics::{self, prometheus},
runtime::{Config as RuntimeInfoConfig, RuntimeInfo},
runtime::{Config as RuntimeInfoConfig, ExtendedSessionInfo, RuntimeInfo},
TimeoutExt,
};
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 @@ -643,32 +643,46 @@ impl CurrentlyCheckingSet {
}
}

async fn get_session_info<'a, Sender>(
async fn get_extended_session_info<'a, Sender>(
runtime_info: &'a mut RuntimeInfo,
sender: &mut Sender,
relay_parent: Hash,
session_index: SessionIndex,
) -> Option<&'a SessionInfo>
) -> Option<&'a ExtendedSessionInfo>
where
Sender: SubsystemSender<RuntimeApiMessage>,
{
match runtime_info
.get_session_info_by_index(sender, relay_parent, session_index)
.await
{
Ok(extended_info) => Some(&extended_info.session_info),
Ok(extended_info) => Some(&extended_info),
Err(_) => {
gum::debug!(
target: LOG_TARGET,
session = session_index,
?relay_parent,
"Can't obtain SessionInfo"
"Can't obtain SessionInfo or ExecutorParams"
);
None
},
}
}

async fn get_session_info<'a, Sender>(
runtime_info: &'a mut RuntimeInfo,
sender: &mut Sender,
relay_parent: Hash,
session_index: SessionIndex,
) -> Option<&'a SessionInfo>
where
Sender: SubsystemSender<RuntimeApiMessage>,
{
get_extended_session_info(runtime_info, sender, relay_parent, session_index)
.await
.map(|extended_info| &extended_info.session_info)
}

struct State {
keystore: Arc<LocalKeystore>,
slot_duration_millis: u64,
Expand Down Expand Up @@ -746,6 +760,7 @@ enum Action {
relay_block_hash: Hash,
candidate_index: CandidateIndex,
session: SessionIndex,
executor_params: ExecutorParams,
candidate: CandidateReceipt,
backing_group: GroupIndex,
},
Expand Down Expand Up @@ -968,6 +983,7 @@ async fn handle_actions<Context>(
relay_block_hash,
candidate_index,
session,
executor_params,
candidate,
backing_group,
} => {
Expand Down Expand Up @@ -1008,6 +1024,7 @@ async fn handle_actions<Context>(
},
None => {
let ctx = &mut *ctx;

currently_checking_set
.insert_relay_block_hash(
candidate_hash,
Expand All @@ -1022,6 +1039,7 @@ async fn handle_actions<Context>(
validator_index,
block_hash,
backing_group,
executor_params,
&launch_approval_span,
)
.await
Expand Down Expand Up @@ -2328,17 +2346,18 @@ async fn process_wakeup<Context>(
_ => return Ok(Vec::new()),
};

let session_info = match get_session_info(
session_info_provider,
ctx.sender(),
block_entry.parent_hash(),
block_entry.session(),
)
.await
{
Some(i) => i,
None => return Ok(Vec::new()),
};
let ExtendedSessionInfo { ref session_info, ref executor_params, .. } =
match get_extended_session_info(
session_info_provider,
ctx.sender(),
block_entry.parent_hash(),
block_entry.session(),
)
.await
{
Some(i) => i,
None => return Ok(Vec::new()),
};

let block_tick = slot_number_to_tick(state.slot_duration_millis, block_entry.slot());
let no_show_duration = slot_number_to_tick(
Expand Down Expand Up @@ -2425,6 +2444,7 @@ async fn process_wakeup<Context>(
relay_block_hash: relay_block,
candidate_index: i as _,
session: block_entry.session(),
executor_params: executor_params.clone(),
candidate: candidate_receipt,
backing_group,
});
Expand Down Expand Up @@ -2466,6 +2486,7 @@ async fn launch_approval<Context>(
validator_index: ValidatorIndex,
block_hash: Hash,
backing_group: GroupIndex,
executor_params: ExecutorParams,
span: &jaeger::Span,
) -> SubsystemResult<RemoteHandle<ApprovalState>> {
let (a_tx, a_rx) = oneshot::channel();
Expand Down Expand Up @@ -2611,6 +2632,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
View file
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
18 changes: 13 additions & 5 deletions polkadot/node/core/backing/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,18 @@ 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::{
self, prospective_parachains_mode, request_min_backing_votes, 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 @@ -555,6 +555,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 @@ -564,6 +565,7 @@ async fn request_candidate_validation(
code,
candidate_receipt,
pov,
executor_params,
PvfExecTimeoutKind::Backing,
tx,
))
Expand Down Expand Up @@ -630,6 +632,11 @@ 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)),
};

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

0 comments on commit a2b6470

Please sign in to comment.