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

Commit

Permalink
Async keystore + Authority-Discovery async/await (#7000)
Browse files Browse the repository at this point in the history
* Asyncify sign_with

* Asyncify generate/get keys

* Complete BareCryptoStore asyncification

* Cleanup

* Rebase

* Add Proxy

* Inject keystore proxy into extensions

* Implement some methods

* Await on send

* Cleanup

* Send result over the oneshot channel sender

* Process one future at a time

* Fix cargo stuff

* Asyncify sr25519_vrf_sign

* Cherry-pick and fix changes

* Introduce SyncCryptoStore

* SQUASH ME WITH THE first commit

* Implement into SyncCryptoStore

* Implement BareCryptoStore for KeystoreProxyAdapter

* authority-discovery

* AURA

* BABE

* finality-grandpa

* offchain-workers

* benchmarking-cli

* sp_io

* test-utils

* application-crypto

* Extensions and RPC

* Client Service

* bin

* Update cargo.lock

* Implement BareCryptoStore on proxy directly

* Simplify proxy setup

* Fix authority-discover

* Pass async keystore to authority-discovery

* Fix tests

* Use async keystore in authority-discovery

* Rename BareCryptoStore to CryptoStore

* WIP

* Remote mutable borrow in CryptoStore trait

* Implement Keystore with backends

* Remove Proxy implementation

* Fix service builder and keystore user-crates

* Fix tests

* Rework authority-discovery after refactoring

* futures::select!

* Fix multiple mut borrows in authority-discovery

* Merge fixes

* Require sync

* Restore Cargo.lock

* PR feedback - round 1

* Remove Keystore and use LocalKeystore directly

Also renamed KeystoreParams to KeystoreContainer

* Join

* Remove sync requirement

* Fix keystore tests

* Fix tests

* client/authority-discovery: Remove event stream dynamic dispatching

With authority-discovery moving from a poll based future to an `async`
future Rust has difficulties propagating the `Sync` trade through the
generated state machine.

Instead of using dynamic dispatching, use a trait parameter to specify
the DHT event stream.

* Make it compile

* Fix submit_transaction

* Fix block_on issue

* Use await in async context

* Fix manual seal keystore

* Fix authoring_blocks test

* fix aura authoring_blocks

* Try to fix tests for auth-discovery

* client/authority-discovery: Fix lookup_throttling test

* client/authority-discovery: Fix triggers_dht_get_query test

* Fix epoch_authorship_works

* client/authority-discovery: Remove timing assumption in unit test

* client/authority-discovery: Revert changes to termination test

* PR feedback

* Remove deadcode and mark test code

* Fix test_sync

* Use the correct keyring type

* Return when from_service stream is closed

* Convert SyncCryptoStore to a trait

* Fix line width

* Fix line width - take 2

* Remove unused import

* Fix keystore instantiation

* PR feedback

* Remove KeystoreContainer

* Revert "Remove KeystoreContainer"

This reverts commit 9aa1152.

* Take a ref of keystore

* Move keystore to dev-dependencies

* Address some PR feedback

* Missed one

* Pass keystore reference - take 2

* client/finality-grandpa: Use `Arc<dyn CryptoStore>` instead of SyncXXX

Instead of using `SyncCryptoStorePtr` within `client/finality-grandpa`,
which is a type alias for `Arc<dyn SyncCryptoStore>`, use `Arc<dyn
CryptoStore>`. Benefits are:

1. No additional mental overhead of a `SyncCryptoStorePtr`.

2. Ability for new code to use the asynchronous methods of `CryptoStore`
instead of the synchronous `SyncCryptoStore` methods within
`client/finality-granpa` without the need for larger refactorings.

Note: This commit uses `Arc<dyn CryptoStore>` instead of
`CryptoStorePtr`, as I find the type signature more descriptive. This is
subjective and in no way required.

* Remove SyncCryptoStorePtr

* Remove KeystoreContainer & SyncCryptoStorePtr

* PR feedback

* *: Use CryptoStorePtr whereever possible

* *: Define SyncCryptoStore as a pure extension trait of CryptoStore

* Follow up to SyncCryptoStore extension trait

* Adjust docs for SyncCryptoStore as Ben suggested

* Cleanup unnecessary requirements

* sp-keystore

* Use async_std::task::block_on in keystore

* Fix block_on std requirement

* Update primitives/keystore/src/lib.rs

Co-authored-by: Max Inden <mail@max-inden.de>

* Fix wasm build

* Remove unused var

* Fix wasm compilation - take 2

* Revert async-std in keystore

* Fix indent

* Fix version and copyright

* Cleanup feature = "std"

* Auth Discovery: Ignore if from_service is cloed

* Max's suggestion

* Revert async-std usage for block_on

* Address PR feedback

* Fix example offchain worker build

* Address PR feedback

* Update Cargo.lock

* Move unused methods to test helper functions

* Restore accidentally deleted cargo.lock files

* Fix unused imports

Co-authored-by: Max Inden <mail@max-inden.de>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
  • Loading branch information
3 people authored Oct 8, 2020
1 parent ffe1540 commit a845ff3
Show file tree
Hide file tree
Showing 70 changed files with 2,395 additions and 1,763 deletions.
44 changes: 42 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ members = [
"primitives/finality-grandpa",
"primitives/inherents",
"primitives/keyring",
"primitives/keystore",
"primitives/offchain",
"primitives/panic-handler",
"primitives/npos-elections",
Expand Down
22 changes: 11 additions & 11 deletions bin/node-template/node/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
>, ServiceError> {
let inherent_data_providers = sp_inherents::InherentDataProviders::new();

let (client, backend, keystore, task_manager) =
let (client, backend, keystore_container, task_manager) =
sc_service::new_full_parts::<Block, RuntimeApi, Executor>(&config)?;
let client = Arc::new(client);

Expand Down Expand Up @@ -73,17 +73,17 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
)?;

Ok(sc_service::PartialComponents {
client, backend, task_manager, import_queue, keystore, select_chain, transaction_pool,
inherent_data_providers,
client, backend, task_manager, import_queue, keystore_container,
select_chain, transaction_pool,inherent_data_providers,
other: (aura_block_import, grandpa_link),
})
}

/// Builds a new service for a full client.
pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
let sc_service::PartialComponents {
client, backend, mut task_manager, import_queue, keystore, select_chain, transaction_pool,
inherent_data_providers,
client, backend, mut task_manager, import_queue, keystore_container,
select_chain, transaction_pool, inherent_data_providers,
other: (block_import, grandpa_link),
} = new_partial(&config)?;

Expand Down Expand Up @@ -134,11 +134,11 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
sc_service::spawn_tasks(sc_service::SpawnTasksParams {
network: network.clone(),
client: client.clone(),
keystore: keystore.clone(),
keystore: keystore_container.sync_keystore(),
task_manager: &mut task_manager,
transaction_pool: transaction_pool.clone(),
telemetry_connection_sinks: telemetry_connection_sinks.clone(),
rpc_extensions_builder: rpc_extensions_builder,
rpc_extensions_builder,
on_demand: None,
remote_blockchain: None,
backend, network_status_sinks, system_rpc_tx, config,
Expand All @@ -163,7 +163,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
network.clone(),
inherent_data_providers.clone(),
force_authoring,
keystore.clone(),
keystore_container.sync_keystore(),
can_author_with,
)?;

Expand All @@ -175,7 +175,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
// if the node isn't actively participating in consensus then it doesn't
// need a keystore, regardless of which protocol we use below.
let keystore = if role.is_authority() {
Some(keystore as sp_core::traits::BareCryptoStorePtr)
Some(keystore_container.sync_keystore())
} else {
None
};
Expand Down Expand Up @@ -228,7 +228,7 @@ pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {

/// Builds a new service for a light client.
pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> {
let (client, backend, keystore, mut task_manager, on_demand) =
let (client, backend, keystore_container, mut task_manager, on_demand) =
sc_service::new_light_parts::<Block, RuntimeApi, Executor>(&config)?;

let transaction_pool = Arc::new(sc_transaction_pool::BasicPool::new_light(
Expand Down Expand Up @@ -290,7 +290,7 @@ pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> {
telemetry_connection_sinks: sc_service::TelemetryConnectionSinks::default(),
config,
client,
keystore,
keystore: keystore_container.sync_keystore(),
backend,
network,
network_status_sinks,
Expand Down
1 change: 1 addition & 0 deletions bin/node/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ sp-timestamp = { version = "2.0.0", default-features = false, path = "../../../p
sp-finality-tracker = { version = "2.0.0", default-features = false, path = "../../../primitives/finality-tracker" }
sp-inherents = { version = "2.0.0", path = "../../../primitives/inherents" }
sp-keyring = { version = "2.0.0", path = "../../../primitives/keyring" }
sp-keystore = { version = "0.8.0", path = "../../../primitives/keystore" }
sp-io = { version = "2.0.0", path = "../../../primitives/io" }
sp-consensus = { version = "0.8.0", path = "../../../primitives/consensus/common" }
sp-transaction-pool = { version = "2.0.0", path = "../../../primitives/transaction-pool" }
Expand Down
Loading

0 comments on commit a845ff3

Please sign in to comment.