[Companion #13615] Keystore overhaul (#6892)

* 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 <>
This commit is contained in:
Davide Galassi
2023-03-17 13:09:15 +01:00
committed by GitHub
parent 4d904951fd
commit 46c36e5a4f
38 changed files with 546 additions and 648 deletions
+16 -19
View File
@@ -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::{
@@ -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 }
}
}
@@ -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);
@@ -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)>,
@@ -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> {
@@ -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) => {
@@ -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)>,
@@ -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.
@@ -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);
}
@@ -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);
@@ -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,
@@ -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(())
}
@@ -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();
+20 -55
View File
@@ -36,7 +36,7 @@ use polkadot_primitives::{
};
use sp_application_crypto::AppKey;
use sp_keyring::Sr25519Keyring;
use sp_keystore::{CryptoStore, SyncCryptoStore};
use sp_keystore::Keystore;
use sp_tracing as _;
use statement_table::v2::Misbehavior;
use std::collections::HashMap;
@@ -55,7 +55,7 @@ fn table_statement_to_primitive(statement: TableStatement) -> Statement {
struct TestState {
chain_ids: Vec<ParaId>,
keystore: SyncCryptoStorePtr,
keystore: KeystorePtr,
validators: Vec<Sr25519Keyring>,
validator_public: Vec<ValidatorId>,
validation_data: PersistedValidationData,
@@ -85,12 +85,8 @@ impl Default for TestState {
let keystore = Arc::new(sc_keystore::LocalKeystore::in_memory());
// Make sure `Alice` key is in the keystore, so this mocked node will be a parachain validator.
SyncCryptoStore::sr25519_generate_new(
&*keystore,
ValidatorId::ID,
Some(&validators[0].to_seed()),
)
.expect("Insert key into keystore");
Keystore::sr25519_generate_new(&*keystore, ValidatorId::ID, Some(&validators[0].to_seed()))
.expect("Insert key into keystore");
let validator_public = validator_pubkeys(&validators);
@@ -143,7 +139,7 @@ impl Default for TestState {
type VirtualOverseer = test_helpers::TestSubsystemContextHandle<CandidateBackingMessage>;
fn test_harness<T: Future<Output = VirtualOverseer>>(
keystore: SyncCryptoStorePtr,
keystore: KeystorePtr,
test: impl FnOnce(VirtualOverseer) -> T,
) {
let pool = sp_core::testing::TaskExecutor::new();
@@ -383,19 +379,17 @@ fn backing_works() {
let candidate_a_hash = candidate_a.hash();
let candidate_a_commitments_hash = candidate_a.commitments.hash();
let public1 = CryptoStore::sr25519_generate_new(
let public1 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[5].to_seed()),
)
.await
.expect("Insert key into keystore");
let public2 = CryptoStore::sr25519_generate_new(
let public2 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[2].to_seed()),
)
.await
.expect("Insert key into keystore");
let signed_a = SignedFullStatement::sign(
@@ -405,7 +399,6 @@ fn backing_works() {
ValidatorIndex(2),
&public2.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -417,7 +410,6 @@ fn backing_works() {
ValidatorIndex(5),
&public1.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -536,26 +528,23 @@ fn backing_works_while_validation_ongoing() {
let candidate_a_hash = candidate_a.hash();
let candidate_a_commitments_hash = candidate_a.commitments.hash();
let public1 = CryptoStore::sr25519_generate_new(
let public1 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[5].to_seed()),
)
.await
.expect("Insert key into keystore");
let public2 = CryptoStore::sr25519_generate_new(
let public2 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[2].to_seed()),
)
.await
.expect("Insert key into keystore");
let public3 = CryptoStore::sr25519_generate_new(
let public3 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[3].to_seed()),
)
.await
.expect("Insert key into keystore");
let signed_a = SignedFullStatement::sign(
@@ -565,7 +554,6 @@ fn backing_works_while_validation_ongoing() {
ValidatorIndex(2),
&public2.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -577,7 +565,6 @@ fn backing_works_while_validation_ongoing() {
ValidatorIndex(5),
&public1.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -589,7 +576,6 @@ fn backing_works_while_validation_ongoing() {
ValidatorIndex(3),
&public3.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -719,12 +705,11 @@ fn backing_misbehavior_works() {
let candidate_a_hash = candidate_a.hash();
let candidate_a_commitments_hash = candidate_a.commitments.hash();
let public2 = CryptoStore::sr25519_generate_new(
let public2 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[2].to_seed()),
)
.await
.expect("Insert key into keystore");
let seconded_2 = SignedFullStatement::sign(
&test_state.keystore,
@@ -733,7 +718,6 @@ fn backing_misbehavior_works() {
ValidatorIndex(2),
&public2.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -745,7 +729,6 @@ fn backing_misbehavior_works() {
ValidatorIndex(2),
&public2.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -1015,12 +998,11 @@ fn backing_second_after_first_fails_works() {
}
.build();
let validator2 = CryptoStore::sr25519_generate_new(
let validator2 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[2].to_seed()),
)
.await
.expect("Insert key into keystore");
let signed_a = SignedFullStatement::sign(
@@ -1030,7 +1012,6 @@ fn backing_second_after_first_fails_works() {
ValidatorIndex(2),
&validator2.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -1142,12 +1123,11 @@ fn backing_works_after_failed_validation() {
}
.build();
let public2 = CryptoStore::sr25519_generate_new(
let public2 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[2].to_seed()),
)
.await
.expect("Insert key into keystore");
let signed_a = SignedFullStatement::sign(
&test_state.keystore,
@@ -1156,7 +1136,6 @@ fn backing_works_after_failed_validation() {
ValidatorIndex(2),
&public2.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -1291,12 +1270,11 @@ fn validation_work_ignores_wrong_collator() {
}
.build();
let public2 = CryptoStore::sr25519_generate_new(
let public2 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[2].to_seed()),
)
.await
.expect("Insert key into keystore");
let seconding = SignedFullStatement::sign(
&test_state.keystore,
@@ -1305,7 +1283,6 @@ fn validation_work_ignores_wrong_collator() {
ValidatorIndex(2),
&public2.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -1417,26 +1394,23 @@ fn retry_works() {
}
.build();
let public2 = CryptoStore::sr25519_generate_new(
let public2 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[2].to_seed()),
)
.await
.expect("Insert key into keystore");
let public3 = CryptoStore::sr25519_generate_new(
let public3 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[3].to_seed()),
)
.await
.expect("Insert key into keystore");
let public5 = CryptoStore::sr25519_generate_new(
let public5 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[5].to_seed()),
)
.await
.expect("Insert key into keystore");
let signed_a = SignedFullStatement::sign(
&test_state.keystore,
@@ -1445,7 +1419,6 @@ fn retry_works() {
ValidatorIndex(2),
&public2.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -1456,7 +1429,6 @@ fn retry_works() {
ValidatorIndex(3),
&public3.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -1467,7 +1439,6 @@ fn retry_works() {
ValidatorIndex(5),
&public5.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -1574,26 +1545,23 @@ fn observes_backing_even_if_not_validator() {
.build();
let candidate_a_hash = candidate_a.hash();
let public0 = CryptoStore::sr25519_generate_new(
let public0 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[0].to_seed()),
)
.await
.expect("Insert key into keystore");
let public1 = CryptoStore::sr25519_generate_new(
let public1 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[5].to_seed()),
)
.await
.expect("Insert key into keystore");
let public2 = CryptoStore::sr25519_generate_new(
let public2 = Keystore::sr25519_generate_new(
&*test_state.keystore,
ValidatorId::ID,
Some(&test_state.validators[2].to_seed()),
)
.await
.expect("Insert key into keystore");
// Produce a 3-of-5 quorum on the candidate.
@@ -1605,7 +1573,6 @@ fn observes_backing_even_if_not_validator() {
ValidatorIndex(0),
&public0.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -1617,7 +1584,6 @@ fn observes_backing_even_if_not_validator() {
ValidatorIndex(5),
&public1.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");
@@ -1629,7 +1595,6 @@ fn observes_backing_even_if_not_validator() {
ValidatorIndex(2),
&public2.into(),
)
.await
.ok()
.flatten()
.expect("should be signed");