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

Commit

Permalink
[Companion #13615] Keystore overhaul (#6892)
Browse files Browse the repository at this point in the history
* Remove not required async calls

* Fixed missing renaming

* make_keystore can be sync

* More fixes

* Trivial nitpicks

* Cherry pick test fix from master

* Fixes after master merge

* update lockfile for {"substrate"}

---------

Co-authored-by: parity-processbot <>
  • Loading branch information
davxy committed Mar 17, 2023
1 parent 1f89e10 commit 97bd688
Show file tree
Hide file tree
Showing 38 changed files with 546 additions and 648 deletions.
367 changes: 183 additions & 184 deletions Cargo.lock

Large diffs are not rendered by default.

17 changes: 7 additions & 10 deletions node/core/approval-voting/src/criteria.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,17 +557,14 @@ mod tests {
use sp_application_crypto::sr25519;
use sp_core::crypto::Pair as PairT;
use sp_keyring::sr25519::Keyring as Sr25519Keyring;
use sp_keystore::CryptoStore;
use sp_keystore::Keystore;

// sets up a keystore with the given keyring accounts.
async fn make_keystore(accounts: &[Sr25519Keyring]) -> LocalKeystore {
fn make_keystore(accounts: &[Sr25519Keyring]) -> LocalKeystore {
let store = LocalKeystore::in_memory();

for s in accounts.iter().copied().map(|k| k.to_seed()) {
store
.sr25519_generate_new(ASSIGNMENT_KEY_TYPE_ID, Some(s.as_str()))
.await
.unwrap();
store.sr25519_generate_new(ASSIGNMENT_KEY_TYPE_ID, Some(s.as_str())).unwrap();
}

store
Expand Down Expand Up @@ -620,7 +617,7 @@ mod tests {

#[test]
fn assignments_produced_for_non_backing() {
let keystore = futures::executor::block_on(make_keystore(&[Sr25519Keyring::Alice]));
let keystore = make_keystore(&[Sr25519Keyring::Alice]);

let c_a = CandidateHash(Hash::repeat_byte(0));
let c_b = CandidateHash(Hash::repeat_byte(1));
Expand Down Expand Up @@ -655,7 +652,7 @@ mod tests {

#[test]
fn assign_to_nonzero_core() {
let keystore = futures::executor::block_on(make_keystore(&[Sr25519Keyring::Alice]));
let keystore = make_keystore(&[Sr25519Keyring::Alice]);

let c_a = CandidateHash(Hash::repeat_byte(0));
let c_b = CandidateHash(Hash::repeat_byte(1));
Expand Down Expand Up @@ -688,7 +685,7 @@ mod tests {

#[test]
fn succeeds_empty_for_0_cores() {
let keystore = futures::executor::block_on(make_keystore(&[Sr25519Keyring::Alice]));
let keystore = make_keystore(&[Sr25519Keyring::Alice]);

let relay_vrf_story = RelayVRFStory([42u8; 32]);
let assignments = compute_assignments(
Expand Down Expand Up @@ -728,7 +725,7 @@ mod tests {
rotation_offset: usize,
f: impl Fn(&mut MutatedAssignment) -> Option<bool>, // None = skip
) {
let keystore = futures::executor::block_on(make_keystore(&[Sr25519Keyring::Alice]));
let keystore = make_keystore(&[Sr25519Keyring::Alice]);

let group_for_core = |i| GroupIndex(((i + rotation_offset) % n_cores) as _);

Expand Down
12 changes: 4 additions & 8 deletions node/core/approval-voting/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use assert_matches::assert_matches;
use async_trait::async_trait;
use parking_lot::Mutex;
use sp_keyring::sr25519::Keyring as Sr25519Keyring;
use sp_keystore::SyncCryptoStore;
use sp_keystore::Keystore;
use std::{
pin::Pin,
sync::{
Expand Down Expand Up @@ -2312,12 +2312,8 @@ fn subsystem_validate_approvals_cache() {
let store = config.backend();

test_harness(config, |test_harness| async move {
let TestHarness {
mut virtual_overseer,
clock,
sync_oracle_handle: _sync_oracle_handle,
..
} = test_harness;
let TestHarness { mut virtual_overseer, clock, sync_oracle_handle: _sync_oracle_handle } =
test_harness;

assert_matches!(
overseer_recv(&mut virtual_overseer).await,
Expand Down Expand Up @@ -2399,7 +2395,7 @@ fn subsystem_validate_approvals_cache() {
}

/// Ensure that when two assignments are imported, only one triggers the Approval Checking work
pub async fn handle_double_assignment_import(
async fn handle_double_assignment_import(
virtual_overseer: &mut VirtualOverseer,
candidate_index: CandidateIndex,
) {
Expand Down
35 changes: 16 additions & 19 deletions node/core/backing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ use polkadot_primitives::{
CommittedCandidateReceipt, CoreIndex, CoreState, Hash, Id as ParaId, PvfExecTimeoutKind,
SigningContext, ValidatorId, ValidatorIndex, ValidatorSignature, ValidityAttestation,
};
use sp_keystore::SyncCryptoStorePtr;
use sp_keystore::KeystorePtr;
use statement_table::{
generic::AttestedCandidate as TableAttestedCandidate,
v2::{
Expand Down Expand Up @@ -118,13 +118,13 @@ impl ValidatedCandidateCommand {

/// The candidate backing subsystem.
pub struct CandidateBackingSubsystem {
keystore: SyncCryptoStorePtr,
keystore: KeystorePtr,
metrics: Metrics,
}

impl CandidateBackingSubsystem {
/// Create a new instance of the `CandidateBackingSubsystem`.
pub fn new(keystore: SyncCryptoStorePtr, metrics: Metrics) -> Self {
pub fn new(keystore: KeystorePtr, metrics: Metrics) -> Self {
Self { keystore, metrics }
}
}
Expand All @@ -149,7 +149,7 @@ where
#[overseer::contextbounds(CandidateBacking, prefix = self::overseer)]
async fn run<Context>(
mut ctx: Context,
keystore: SyncCryptoStorePtr,
keystore: KeystorePtr,
metrics: Metrics,
) -> FatalResult<()> {
let (background_validation_tx, mut background_validation_rx) = mpsc::channel(16);
Expand Down Expand Up @@ -178,7 +178,7 @@ async fn run<Context>(
#[overseer::contextbounds(CandidateBacking, prefix = self::overseer)]
async fn run_iteration<Context>(
ctx: &mut Context,
keystore: SyncCryptoStorePtr,
keystore: KeystorePtr,
metrics: &Metrics,
jobs: &mut HashMap<Hash, JobAndSpan<Context>>,
background_validation_tx: mpsc::Sender<(Hash, ValidatedCandidateCommand)>,
Expand Down Expand Up @@ -265,7 +265,7 @@ async fn handle_active_leaves_update<Context>(
ctx: &mut Context,
update: ActiveLeavesUpdate,
jobs: &mut HashMap<Hash, JobAndSpan<Context>>,
keystore: &SyncCryptoStorePtr,
keystore: &KeystorePtr,
background_validation_tx: &mpsc::Sender<(Hash, ValidatedCandidateCommand)>,
metrics: &Metrics,
) -> Result<(), Error> {
Expand Down Expand Up @@ -323,7 +323,7 @@ async fn handle_active_leaves_update<Context>(

let signing_context = SigningContext { parent_hash: parent, session_index };
let validator =
match Validator::construct(&validators, signing_context.clone(), keystore.clone()).await {
match Validator::construct(&validators, signing_context.clone(), keystore.clone()) {
Ok(v) => Some(v),
Err(util::Error::NotAValidator) => None,
Err(e) => {
Expand Down Expand Up @@ -426,7 +426,7 @@ struct CandidateBackingJob<Context> {
/// The candidates that are includable, by hash. Each entry here indicates
/// that we've sent the provisioner the backed candidate.
backed: HashSet<CandidateHash>,
keystore: SyncCryptoStorePtr,
keystore: KeystorePtr,
table: Table<TableContext>,
table_context: TableContext,
background_validation_tx: mpsc::Sender<(Hash, ValidatedCandidateCommand)>,
Expand Down Expand Up @@ -814,8 +814,7 @@ impl<Context> CandidateBackingJob<Context> {
commitments,
});
if let Some(stmt) = self
.sign_import_and_distribute_statement(ctx, statement, root_span)
.await?
.sign_import_and_distribute_statement(ctx, statement, root_span)?
{
// Break cycle - bounded as there is only one candidate to
// second per block.
Expand Down Expand Up @@ -843,8 +842,7 @@ impl<Context> CandidateBackingJob<Context> {
if !self.issued_statements.contains(&candidate_hash) {
if res.is_ok() {
let statement = Statement::Valid(candidate_hash);
self.sign_import_and_distribute_statement(ctx, statement, &root_span)
.await?;
self.sign_import_and_distribute_statement(ctx, statement, &root_span)?;
}
self.issued_statements.insert(candidate_hash);
}
Expand Down Expand Up @@ -966,14 +964,14 @@ impl<Context> CandidateBackingJob<Context> {
Ok(())
}

async fn sign_import_and_distribute_statement(
fn sign_import_and_distribute_statement(
&mut self,
ctx: &mut Context,
statement: Statement,
root_span: &jaeger::Span,
) -> Result<Option<SignedFullStatement>, Error> {
if let Some(signed_statement) = self.sign_statement(statement).await {
self.import_statement(ctx, &signed_statement, root_span).await?;
if let Some(signed_statement) = self.sign_statement(statement) {
self.import_statement(ctx, &signed_statement, root_span)?;
let smsg = StatementDistributionMessage::Share(self.parent, signed_statement.clone());
ctx.send_unbounded_message(smsg);

Expand Down Expand Up @@ -1001,7 +999,7 @@ impl<Context> CandidateBackingJob<Context> {
}

/// Import a statement into the statement table and return the summary of the import.
async fn import_statement(
fn import_statement(
&mut self,
ctx: &mut Context,
statement: &SignedFullStatement,
Expand Down Expand Up @@ -1222,7 +1220,7 @@ impl<Context> CandidateBackingJob<Context> {
ctx: &mut Context,
statement: SignedFullStatement,
) -> Result<(), Error> {
if let Some(summary) = self.import_statement(ctx, &statement, root_span).await? {
if let Some(summary) = self.import_statement(ctx, &statement, root_span)? {
if Some(summary.group_id) != self.assignment {
return Ok(())
}
Expand Down Expand Up @@ -1277,13 +1275,12 @@ impl<Context> CandidateBackingJob<Context> {
Ok(())
}

async fn sign_statement(&mut self, statement: Statement) -> Option<SignedFullStatement> {
fn sign_statement(&mut self, statement: Statement) -> Option<SignedFullStatement> {
let signed = self
.table_context
.validator
.as_ref()?
.sign(self.keystore.clone(), statement)
.await
.ok()
.flatten()?;
self.metrics.on_statement_signed();
Expand Down
Loading

0 comments on commit 97bd688

Please sign in to comment.