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

Update to work with async keystore – Companion PR for #7000 #1740

Merged
55 commits merged into from
Oct 9, 2020
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
605bed8
Fix keystore types
Sep 9, 2020
55f59c3
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Sep 21, 2020
e60ba32
Use SyncCryptoStorePtr
Sep 21, 2020
7a49293
Borrow keystore
Sep 21, 2020
1072116
Fix unused imports
Sep 21, 2020
c86f635
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Sep 21, 2020
6fc0496
Fix polkadot service
Sep 21, 2020
950860e
Fix bitfield-distribution tests
Sep 21, 2020
95b9c82
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Sep 21, 2020
5c16b3b
Fix indentation
Sep 21, 2020
b6a15e5
Fix backing tests
Sep 22, 2020
8aed67d
Fix tests
Sep 22, 2020
2a932af
Fix provisioner tests
Sep 22, 2020
75e54e6
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Sep 22, 2020
4812ec7
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Sep 22, 2020
59866d2
Removed SyncCryptoStorePtr
Sep 23, 2020
0ddfa8c
Fix services
Sep 23, 2020
f3882b4
Address PR feedback
Sep 23, 2020
644af04
Address PR feedback - 2
Sep 23, 2020
b180b62
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Sep 24, 2020
6094a36
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Sep 28, 2020
bc26850
Update CryptoStorePtr imports to be from sp_keystore
Sep 28, 2020
a27f7c1
Typo
Sep 28, 2020
8028961
Fix CryptoStore import
Sep 28, 2020
08eb205
Document the reason behind using filesystem keystore
Sep 28, 2020
34768ca
Remove VALIDATORS
Sep 28, 2020
23d4845
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Oct 1, 2020
14d683a
Fix duplicate dependency
Oct 1, 2020
9aeda15
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Oct 2, 2020
40e0043
Mark sp-keystore as optional
Oct 7, 2020
a1cf9e4
Fix availability distribution
Oct 7, 2020
3e2da96
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Oct 7, 2020
65680f1
Fix call to sign_with
Oct 7, 2020
af6c665
Fix keystore usage
Oct 7, 2020
d56de6c
Remove tokio and fix parachains Cargo config
Oct 7, 2020
509fa4e
Typos
Oct 7, 2020
e4092ef
Fix keystore dereferencing
Oct 7, 2020
75d50a6
Fix CryptoStore import
Oct 7, 2020
b683b00
Fix provisioner
Oct 7, 2020
f9725cf
Fix node backing
Oct 7, 2020
eceb309
Update services
Oct 7, 2020
a8a7465
Cleanup dependencies
Oct 7, 2020
3b5e1a8
Use sync_keystore
Oct 7, 2020
74f2a34
Fix node service
Oct 7, 2020
e21cb88
Fix node service - 2
Oct 7, 2020
e799871
Fix node service - 3
Oct 7, 2020
8441c63
Rename CryptoStorePtr to SyncCryptoStorePtr
Oct 8, 2020
4423c88
Merge remote-tracking branch 'upstream/master' into keystore-fixes
Oct 8, 2020
590cc51
"Update Substrate"
Oct 8, 2020
e5e9174
Apply suggestions from code review
bkchr Oct 8, 2020
9ce9ecc
Update node/core/backing/Cargo.toml
bkchr Oct 8, 2020
d2787bc
Update primitives/src/v0.rs
rakanalh Oct 8, 2020
fa43595
Fix wasm build
Oct 9, 2020
738914e
Merge branch 'keystore-fixes' of github.com:rakanalh/polkadot into ke…
Oct 9, 2020
306f3cb
Update Cargo.lock
Oct 9, 2020
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
249 changes: 193 additions & 56 deletions Cargo.lock

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions node/core/backing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ edition = "2018"

[dependencies]
futures = "0.3.5"
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
rakanalh marked this conversation as resolved.
Show resolved Hide resolved
sp-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
sc-client-api = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-blockchain = { git = "https://github.com/paritytech/substrate", branch = "master" }
Expand All @@ -22,7 +23,9 @@ log = "0.4.8"

[dev-dependencies]
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-application-crypto = { git = "https://github.com/paritytech/substrate", branch = "master" }
sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" }
futures = { version = "0.3.5", features = ["thread-pool"] }
tokio = "0.2.22"
rakanalh marked this conversation as resolved.
Show resolved Hide resolved
assert_matches = "1.3.0"
polkadot-node-subsystem-test-helpers = { path = "../../subsystem-test-helpers" }
112 changes: 75 additions & 37 deletions node/core/backing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use futures::{
Future, FutureExt, SinkExt, StreamExt,
};

use keystore::KeyStorePtr;
use sp_core::traits::CryptoStorePtr;
use polkadot_primitives::v1::{
CommittedCandidateReceipt, BackedCandidate, Id as ParaId, ValidatorId,
ValidatorIndex, SigningContext, PoV,
Expand Down Expand Up @@ -101,6 +101,7 @@ struct CandidateBackingJob {
seconded: Option<Hash>,
/// We have already reported misbehaviors for these validators.
reported_misbehavior_for: HashSet<ValidatorIndex>,
keystore: CryptoStorePtr,
table: Table<TableContext>,
table_context: TableContext,
metrics: Metrics,
Expand Down Expand Up @@ -328,9 +329,12 @@ impl CandidateBackingJob {
};

let issued_statement = statement.is_some();
if let Some(signed_statement) = statement.and_then(|s| self.sign_statement(s)) {
self.import_statement(&signed_statement).await?;
self.distribute_signed_statement(signed_statement).await?;

if let Some(statement) = statement {
if let Some(signed_statement) = self.sign_statement(statement).await {
self.import_statement(&signed_statement).await?;
self.distribute_signed_statement(signed_statement).await?;
}
}

Ok(issued_statement)
Expand Down Expand Up @@ -530,7 +534,7 @@ impl CandidateBackingJob {

self.issued_statements.insert(candidate_hash);

if let Some(signed_statement) = self.sign_statement(statement) {
if let Some(signed_statement) = self.sign_statement(statement).await {
self.distribute_signed_statement(signed_statement).await?;
}

Expand All @@ -553,8 +557,10 @@ impl CandidateBackingJob {
Ok(())
}

fn sign_statement(&self, statement: Statement) -> Option<SignedFullStatement> {
let signed = self.table_context.validator.as_ref()?.sign(statement);
async fn sign_statement(&self, statement: Statement) -> Option<SignedFullStatement> {
let signed = self.table_context
.validator.as_ref()?
.sign(self.keystore.clone(), statement).await.ok()?;
bkchr marked this conversation as resolved.
Show resolved Hide resolved
self.metrics.on_statement_signed();
Some(signed)
}
Expand Down Expand Up @@ -694,14 +700,14 @@ impl util::JobTrait for CandidateBackingJob {
type ToJob = ToJob;
type FromJob = FromJob;
type Error = Error;
type RunArgs = KeyStorePtr;
type RunArgs = CryptoStorePtr;
type Metrics = Metrics;

const NAME: &'static str = "CandidateBackingJob";

fn run(
parent: Hash,
keystore: KeyStorePtr,
keystore: CryptoStorePtr,
metrics: Metrics,
rx_to: mpsc::Receiver<Self::ToJob>,
mut tx_from: mpsc::Sender<Self::FromJob>,
Expand Down Expand Up @@ -748,7 +754,7 @@ impl util::JobTrait for CandidateBackingJob {
&validators,
signing_context,
keystore.clone(),
) {
).await {
Ok(v) => v,
Err(util::Error::NotAValidator) => { return Ok(()) },
Err(e) => {
Expand Down Expand Up @@ -802,6 +808,7 @@ impl util::JobTrait for CandidateBackingJob {
issued_statements: HashSet::new(),
seconded: None,
reported_misbehavior_for: HashSet::new(),
keystore,
table: Table::default(),
table_context,
metrics,
Expand Down Expand Up @@ -859,33 +866,36 @@ impl metrics::Metrics for Metrics {
}
}

delegated_subsystem!(CandidateBackingJob(KeyStorePtr, Metrics) <- ToJob as CandidateBackingSubsystem);
delegated_subsystem!(CandidateBackingJob(CryptoStorePtr, Metrics) <- ToJob as CandidateBackingSubsystem);

#[cfg(test)]
mod tests {
use super::*;
use assert_matches::assert_matches;
use futures::{executor, future, Future};
use futures::{future, Future};
use polkadot_primitives::v1::{
ScheduledCore, BlockData, CandidateCommitments,
PersistedValidationData, ValidationData, TransientValidationData, HeadData,
ValidatorPair, ValidityAttestation, GroupRotationInfo,
ValidityAttestation, GroupRotationInfo,
};
use polkadot_subsystem::{
messages::RuntimeApiRequest,
ActiveLeavesUpdate, FromOverseer, OverseerSignal,
};
use polkadot_node_primitives::InvalidCandidate;
use sp_keyring::Sr25519Keyring;
use sp_core::traits::CryptoStore;
use sp_application_crypto::AppKey;
use std::collections::HashMap;
use tokio::runtime::Runtime;

fn validator_pubkeys(val_ids: &[Sr25519Keyring]) -> Vec<ValidatorId> {
val_ids.iter().map(|v| v.public().into()).collect()
}

struct TestState {
chain_ids: Vec<ParaId>,
keystore: KeyStorePtr,
keystore: CryptoStorePtr,
validators: Vec<Sr25519Keyring>,
validator_public: Vec<ValidatorId>,
validation_data: ValidationData,
Expand All @@ -912,9 +922,9 @@ mod tests {
Sr25519Keyring::Ferdie,
];

let keystore = keystore::Store::new_in_memory();
let keystore = Arc::new(keystore::LocalKeystore::in_memory());
bkchr marked this conversation as resolved.
Show resolved Hide resolved
// Make sure `Alice` key is in the keystore, so this mocked node will be a parachain validator.
keystore.write().insert_ephemeral_from_seed::<ValidatorPair>(&validators[0].to_seed())
futures::executor::block_on(keystore.sr25519_generate_new(ValidatorId::ID, Some(&validators[0].to_seed())))
.expect("Insert key into keystore");

let validator_public = validator_pubkeys(&validators);
Expand Down Expand Up @@ -985,7 +995,7 @@ mod tests {
virtual_overseer: polkadot_node_subsystem_test_helpers::TestSubsystemContextHandle<CandidateBackingMessage>,
}

fn test_harness<T: Future<Output=()>>(keystore: KeyStorePtr, test: impl FnOnce(TestHarness) -> T) {
fn test_harness<T: Future<Output=()>>(keystore: CryptoStorePtr, test: impl FnOnce(TestHarness) -> T) {
let pool = sp_core::testing::TaskExecutor::new();

let (context, virtual_overseer) = polkadot_node_subsystem_test_helpers::make_subsystem_context(pool.clone());
Expand All @@ -998,8 +1008,8 @@ mod tests {

futures::pin_mut!(test_fut);
futures::pin_mut!(subsystem);

executor::block_on(future::select(test_fut, subsystem));
let mut rt = Runtime::new().unwrap();
rt.block_on(future::select(test_fut, subsystem));
}

fn make_erasure_root(test: &TestState, pov: PoV) -> Hash {
Expand Down Expand Up @@ -1203,20 +1213,27 @@ mod tests {
}.build();

let candidate_a_hash = candidate_a.hash();

let public0 = test_state.keystore.sr25519_generate_new(
ValidatorId::ID, Some(&test_state.validators[0].to_seed())
).await.expect("Insert key into keystore");
let public2 = test_state.keystore.sr25519_generate_new(
ValidatorId::ID, Some(&test_state.validators[2].to_seed())
).await.expect("Insert key into keystore");
let signed_a = SignedFullStatement::sign(
&test_state.keystore,
Statement::Seconded(candidate_a.clone()),
&test_state.signing_context,
2,
&test_state.validators[2].pair().into(),
);
&public2.into(),
).await.expect("should be signed");

let signed_b = SignedFullStatement::sign(
&test_state.keystore,
Statement::Valid(candidate_a_hash),
&test_state.signing_context,
0,
&test_state.validators[0].pair().into(),
);
&public0.into(),
).await.expect("should be signed");

let statement = CandidateBackingMessage::Statement(test_state.relay_parent, signed_a.clone());

Expand Down Expand Up @@ -1330,27 +1347,35 @@ mod tests {
}.build();

let candidate_a_hash = candidate_a.hash();

let public0 = test_state.keystore.sr25519_generate_new(
ValidatorId::ID, Some(&test_state.validators[0].to_seed())
).await.expect("Insert key into keystore");
let public2 = test_state.keystore.sr25519_generate_new(
ValidatorId::ID, Some(&test_state.validators[2].to_seed())
).await.expect("Insert key into keystore");
let signed_a = SignedFullStatement::sign(
&test_state.keystore,
Statement::Seconded(candidate_a.clone()),
&test_state.signing_context,
2,
&test_state.validators[2].pair().into(),
);
&public2.into(),
).await.expect("should be signed");

let signed_b = SignedFullStatement::sign(
&test_state.keystore,
Statement::Valid(candidate_a_hash),
&test_state.signing_context,
0,
&test_state.validators[0].pair().into(),
);
&public0.into(),
).await.expect("should be signed");

let signed_c = SignedFullStatement::sign(
&test_state.keystore,
Statement::Invalid(candidate_a_hash),
&test_state.signing_context,
0,
&test_state.validators[0].pair().into(),
);
&public0.into(),
).await.expect("should be signed");

let statement = CandidateBackingMessage::Statement(test_state.relay_parent, signed_a.clone());

Expand Down Expand Up @@ -1600,12 +1625,17 @@ mod tests {

let candidate_hash = candidate.hash();

let validator2 = test_state.keystore.sr25519_generate_new(
ValidatorId::ID, Some(&test_state.validators[2].to_seed())
).await.expect("Insert key into keystore");

let signed_a = SignedFullStatement::sign(
&test_state.keystore,
Statement::Seconded(candidate.clone()),
&test_state.signing_context,
2,
&test_state.validators[2].pair().into(),
);
&validator2.into(),
).await.expect("should be signed");

// Send in a `Statement` with a candidate.
let statement = CandidateBackingMessage::Statement(
Expand Down Expand Up @@ -1733,12 +1763,16 @@ mod tests {
..Default::default()
}.build();

let public2 = test_state.keystore.sr25519_generate_new(
ValidatorId::ID, Some(&test_state.validators[2].to_seed())
).await.expect("Insert key into keystore");
let signed_a = SignedFullStatement::sign(
&test_state.keystore,
Statement::Seconded(candidate.clone()),
&test_state.signing_context,
2,
&test_state.validators[2].pair().into(),
);
&public2.into(),
).await.expect("should be signed");

// Send in a `Statement` with a candidate.
let statement = CandidateBackingMessage::Statement(
Expand Down Expand Up @@ -1869,12 +1903,16 @@ mod tests {
..Default::default()
}.build();

let public2 = test_state.keystore.sr25519_generate_new(
ValidatorId::ID, Some(&test_state.validators[2].to_seed())
).await.expect("Insert key into keystore");
let seconding = SignedFullStatement::sign(
&test_state.keystore,
Statement::Seconded(candidate_a.clone()),
&test_state.signing_context,
2,
&test_state.validators[2].pair().into(),
);
&public2.into(),
).await.expect("should be signed");

let statement = CandidateBackingMessage::Statement(
test_state.relay_parent,
Expand Down
2 changes: 1 addition & 1 deletion node/core/bitfield-signing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ log = "0.4.8"
polkadot-primitives = { path = "../../../primitives" }
polkadot-node-subsystem = { path = "../../subsystem" }
polkadot-node-subsystem-util = { path = "../../subsystem-util" }
keystore = { package = "sc-keystore", git = "https://github.com/paritytech/substrate", branch = "master" }
sp-core = { package = "sp-core", git = "https://github.com/paritytech/substrate", branch = "master" }
rakanalh marked this conversation as resolved.
Show resolved Hide resolved
wasm-timer = "0.2.4"
14 changes: 10 additions & 4 deletions node/core/bitfield-signing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use futures::{
prelude::*,
stream, Future,
};
use keystore::KeyStorePtr;
use sp_core::traits::{Error as KeystoreError, CryptoStorePtr};
use polkadot_node_subsystem::{
messages::{
self, AllMessages, AvailabilityStoreMessage, BitfieldDistributionMessage,
Expand Down Expand Up @@ -132,6 +132,9 @@ pub enum Error {
/// the runtime API failed to return what we wanted
#[from]
Runtime(RuntimeApiError),
/// the keystore failed to process signing request
#[from]
Keystore(KeystoreError),
}

// if there is a candidate pending availability, query the Availability Store
Expand Down Expand Up @@ -289,7 +292,7 @@ impl JobTrait for BitfieldSigningJob {
type ToJob = ToJob;
type FromJob = FromJob;
type Error = Error;
type RunArgs = KeyStorePtr;
type RunArgs = CryptoStorePtr;
type Metrics = Metrics;

const NAME: &'static str = "BitfieldSigningJob";
Expand All @@ -308,7 +311,7 @@ impl JobTrait for BitfieldSigningJob {

// now do all the work we can before we need to wait for the availability store
// if we're not a validator, we can just succeed effortlessly
let validator = match Validator::new(relay_parent, keystore, sender.clone()).await {
let validator = match Validator::new(relay_parent, keystore.clone(), sender.clone()).await {
Ok(validator) => validator,
Err(util::Error::NotAValidator) => return Ok(()),
Err(err) => return Err(Error::Util(err)),
Expand All @@ -329,7 +332,10 @@ impl JobTrait for BitfieldSigningJob {
Ok(bitfield) => bitfield,
};

let signed_bitfield = validator.sign(bitfield);
let signed_bitfield = validator
.sign(keystore.clone(), bitfield)
.await
.map_err(|e| Error::Keystore(e))?;
metrics.on_bitfield_signed();

// make an anonymous scope to contain some use statements to simplify creating the outbound message
Expand Down
5 changes: 4 additions & 1 deletion node/core/provisioner/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@ polkadot-node-subsystem-util = { path = "../../subsystem-util" }
[dev-dependencies]
lazy_static = "1.4"
sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }
tokio = "0.2"
sp-application-crypto = { git = "https://github.com/paritytech/substrate", branch = "master" }
sc-keystore = { git = "https://github.com/paritytech/substrate", branch = "master" }
tokio = { version = "0.2.22", features = ["time"] }
ordian marked this conversation as resolved.
Show resolved Hide resolved
tempfile = "3.1.0"
Loading