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

Async keystore + Authority-Discovery async/await #7000

Merged
139 commits merged into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from 83 commits
Commits
Show all changes
139 commits
Select commit Hold shift + click to select a range
6ca9f1b
Asyncify sign_with
May 6, 2020
139e31b
Asyncify generate/get keys
May 7, 2020
0a9ff21
Complete BareCryptoStore asyncification
May 7, 2020
2d4e2c9
Cleanup
May 7, 2020
dd001bb
Rebase
May 18, 2020
1949f7c
Add Proxy
May 18, 2020
88d48e6
Inject keystore proxy into extensions
May 20, 2020
a6f6ffc
Implement some methods
May 20, 2020
d7714e9
Await on send
May 20, 2020
73a32cf
Cleanup
May 20, 2020
b540f7a
Send result over the oneshot channel sender
May 22, 2020
391fd8a
Process one future at a time
May 25, 2020
0c352da
Fix cargo stuff
Aug 11, 2020
0bf4779
Asyncify sr25519_vrf_sign
Jun 12, 2020
700dcbf
Cherry-pick and fix changes
Aug 21, 2020
de8d0a9
Introduce SyncCryptoStore
Aug 21, 2020
ec5cc86
SQUASH ME WITH THE first commit
Aug 21, 2020
6fd1dfd
Implement into SyncCryptoStore
Aug 21, 2020
8c69a47
Implement BareCryptoStore for KeystoreProxyAdapter
Aug 21, 2020
0f4a993
authority-discovery
Aug 21, 2020
f199295
AURA
Aug 21, 2020
f1b9a50
BABE
Aug 21, 2020
a7dfb5b
finality-grandpa
Aug 21, 2020
8be5932
offchain-workers
Aug 21, 2020
05734ca
benchmarking-cli
Aug 21, 2020
e333cc6
sp_io
Aug 21, 2020
c25fc7b
test-utils
Aug 21, 2020
fcb9fff
application-crypto
Aug 21, 2020
8393835
Extensions and RPC
Aug 21, 2020
d428bc2
Client Service
Aug 21, 2020
1833dcf
bin
Aug 21, 2020
a51c40b
Update cargo.lock
Aug 21, 2020
437fb10
Implement BareCryptoStore on proxy directly
Aug 26, 2020
ce83c89
Simplify proxy setup
Aug 26, 2020
5632e8b
Fix authority-discover
Aug 26, 2020
49b58cb
Pass async keystore to authority-discovery
Aug 28, 2020
774c7d9
Fix tests
Aug 28, 2020
c3d976a
Use async keystore in authority-discovery
Aug 28, 2020
0a97a02
Rename BareCryptoStore to CryptoStore
Aug 28, 2020
e95ff86
WIP
Aug 31, 2020
bae8aee
Remote mutable borrow in CryptoStore trait
Aug 31, 2020
09c61ab
Implement Keystore with backends
Aug 31, 2020
4533975
Remove Proxy implementation
Aug 31, 2020
d0884ef
Fix service builder and keystore user-crates
Aug 31, 2020
54a0573
Fix tests
Aug 31, 2020
a0b73d2
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Aug 31, 2020
3165841
Rework authority-discovery after refactoring
Sep 1, 2020
0eed201
futures::select!
Sep 1, 2020
9e2a0ac
Fix multiple mut borrows in authority-discovery
Sep 1, 2020
312cc5b
Merge fixes
Sep 1, 2020
b0cc694
Require sync
Sep 1, 2020
6bb48a2
Restore Cargo.lock
Sep 1, 2020
387da2e
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 1, 2020
cdf43e6
PR feedback - round 1
Sep 1, 2020
fd00d9b
Remove Keystore and use LocalKeystore directly
Sep 1, 2020
68965ce
Join
Sep 1, 2020
b0a6fce
Remove sync requirement
Sep 1, 2020
da4c039
Fix keystore tests
Sep 1, 2020
3fca456
Fix tests
Sep 2, 2020
325fe0d
client/authority-discovery: Remove event stream dynamic dispatching
mxinden Sep 2, 2020
0be4d79
Make it compile
Sep 2, 2020
792292c
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 2, 2020
4d8654f
Fix submit_transaction
Sep 2, 2020
4f6a5e5
Fix block_on issue
Sep 4, 2020
54304e4
Use await in async context
Sep 4, 2020
a710060
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 4, 2020
b471f58
Fix manual seal keystore
Sep 4, 2020
e1f18cc
Fix authoring_blocks test
Sep 7, 2020
0ec4fde
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 7, 2020
230103a
fix aura authoring_blocks
Sep 7, 2020
100a09c
Try to fix tests for auth-discovery
Sep 8, 2020
8509a1b
client/authority-discovery: Fix lookup_throttling test
mxinden Sep 8, 2020
e4bb4b9
client/authority-discovery: Fix triggers_dht_get_query test
mxinden Sep 8, 2020
873afc0
Fix epoch_authorship_works
Sep 8, 2020
e67e615
client/authority-discovery: Remove timing assumption in unit test
mxinden Sep 8, 2020
7c4a12d
client/authority-discovery: Revert changes to termination test
mxinden Sep 8, 2020
bdf2e27
PR feedback
Sep 9, 2020
e9ef22f
Remove deadcode and mark test code
Sep 9, 2020
e53226b
Fix test_sync
Sep 14, 2020
3c1a606
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 14, 2020
d739ddf
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 15, 2020
bcfa470
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 17, 2020
cc0cf4e
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 20, 2020
9897f53
Use the correct keyring type
Sep 21, 2020
8124deb
Return when from_service stream is closed
Sep 21, 2020
ac63821
Convert SyncCryptoStore to a trait
Sep 21, 2020
64b4d9c
Fix line width
Sep 21, 2020
3ad796b
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 21, 2020
ac45a38
Fix line width - take 2
Sep 21, 2020
c26f7f6
Remove unused import
Sep 21, 2020
84f307f
Fix keystore instantiation
Sep 21, 2020
221e1f6
PR feedback
Sep 21, 2020
ca8e6a8
Merge branch 'master' into pr/7000
shawntabrizi Sep 21, 2020
9aa1152
Remove KeystoreContainer
Sep 22, 2020
d9cf952
Merge branch 'async-keystore-auth-discovery' of github.com:rakanalh/s…
Sep 22, 2020
814eef0
Revert "Remove KeystoreContainer"
Sep 22, 2020
edb4ecb
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 22, 2020
576943e
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 22, 2020
b6b08a5
Take a ref of keystore
Sep 22, 2020
d44cfe8
Move keystore to dev-dependencies
Sep 22, 2020
adbba45
Address some PR feedback
Sep 22, 2020
89ccff5
Missed one
Sep 22, 2020
07134dd
Pass keystore reference - take 2
Sep 23, 2020
db70636
client/finality-grandpa: Use `Arc<dyn CryptoStore>` instead of SyncXXX
mxinden Sep 22, 2020
ad69c2c
Remove SyncCryptoStorePtr
Sep 23, 2020
453d944
Remove KeystoreContainer & SyncCryptoStorePtr
Sep 23, 2020
331807e
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 23, 2020
4622550
PR feedback
Sep 23, 2020
2885dd8
*: Use CryptoStorePtr whereever possible
mxinden Sep 23, 2020
757061c
*: Define SyncCryptoStore as a pure extension trait of CryptoStore
mxinden Sep 23, 2020
99e005f
Follow up to SyncCryptoStore extension trait
Sep 23, 2020
2bc499e
Adjust docs for SyncCryptoStore as Ben suggested
Sep 23, 2020
a71f01f
Cleanup unnecessary requirements
Sep 23, 2020
2c74c4d
sp-keystore
Sep 24, 2020
a56121e
Use async_std::task::block_on in keystore
Sep 24, 2020
667e878
Fix block_on std requirement
Sep 24, 2020
c6df3de
Update primitives/keystore/src/lib.rs
rakanalh Sep 24, 2020
015866d
Fix wasm build
Sep 24, 2020
c789610
Merge branch 'async-keystore-auth-discovery' of github.com:rakanalh/s…
Sep 24, 2020
31d4176
Remove unused var
Sep 24, 2020
3e666fe
Fix wasm compilation - take 2
Sep 24, 2020
f5acb5d
Revert async-std in keystore
Sep 24, 2020
360d730
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Sep 28, 2020
4c74e9d
Fix indent
Oct 1, 2020
b5a66db
Fix version and copyright
Oct 1, 2020
8d5f685
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Oct 1, 2020
9e170f2
Cleanup feature = "std"
Oct 1, 2020
a176da9
Auth Discovery: Ignore if from_service is cloed
Oct 2, 2020
a5fa388
Max's suggestion
Oct 4, 2020
9a4ede4
Revert async-std usage for block_on
Oct 5, 2020
6346a13
Address PR feedback
Oct 6, 2020
086d702
Fix example offchain worker build
Oct 7, 2020
72fb7ad
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Oct 7, 2020
a5f4bd1
Address PR feedback
Oct 8, 2020
eaae1ce
Merge remote-tracking branch 'upstream/master' into async-keystore-au…
Oct 8, 2020
ae6d14f
Update Cargo.lock
Oct 8, 2020
a8e96f8
Move unused methods to test helper functions
Oct 8, 2020
757a5d8
Restore accidentally deleted cargo.lock files
Oct 8, 2020
d2a5de4
Fix unused imports
Oct 8, 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
14 changes: 14 additions & 0 deletions Cargo.lock

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

20 changes: 10 additions & 10 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_params, 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_params,
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_params,
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_params.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_params.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_params.sync_keystore())
} else {
None
};
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.sync_keystore(),
backend,
network,
network_status_sinks,
Expand Down
58 changes: 32 additions & 26 deletions bin/node/cli/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ use sc_network::{Event, NetworkService};
use sp_runtime::traits::Block as BlockT;
use futures::prelude::*;
use sc_client_api::{ExecutorProvider, RemoteBackend};
use sp_core::traits::BareCryptoStorePtr;
use node_executor::Executor;

type FullClient = sc_service::TFullClient<Block, RuntimeApi, Executor>;
Expand Down Expand Up @@ -64,7 +63,7 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
),
)
>, ServiceError> {
let (client, backend, keystore, task_manager) =
let (client, backend, keystore_params, task_manager) =
sc_service::new_full_parts::<Block, RuntimeApi, Executor>(&config)?;
let client = Arc::new(client);

Expand Down Expand Up @@ -122,7 +121,7 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
let client = client.clone();
let pool = transaction_pool.clone();
let select_chain = select_chain.clone();
let keystore = keystore.clone();
let keystore = keystore_params.sync_keystore();

let rpc_extensions_builder = move |deny_unsafe, subscription_executor| {
let deps = node_rpc::FullDeps {
Expand Down Expand Up @@ -151,8 +150,8 @@ pub fn new_partial(config: &Configuration) -> Result<sc_service::PartialComponen
};

Ok(sc_service::PartialComponents {
client, backend, task_manager, keystore, select_chain, import_queue, transaction_pool,
inherent_data_providers,
client, backend, task_manager, keystore_params,
select_chain, import_queue, transaction_pool, inherent_data_providers,
other: (rpc_extensions_builder, import_setup, rpc_setup)
})
}
Expand All @@ -175,8 +174,8 @@ pub fn new_full_base(
)
) -> Result<NewFullBase, 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_params,
select_chain, transaction_pool, inherent_data_providers,
other: (rpc_extensions_builder, import_setup, rpc_setup),
} = new_partial(&config)?;

Expand Down Expand Up @@ -212,7 +211,7 @@ pub fn new_full_base(
config,
backend: backend.clone(),
client: client.clone(),
keystore: keystore.clone(),
keystore: keystore_params.sync_keystore(),
network: network.clone(),
rpc_extensions_builder: Box::new(rpc_extensions_builder),
transaction_pool: transaction_pool.clone(),
Expand All @@ -239,7 +238,7 @@ pub fn new_full_base(
sp_consensus::CanAuthorWithNativeVersion::new(client.executor().clone());

let babe_config = sc_consensus_babe::BabeParams {
keystore: keystore.clone(),
keystore: keystore_params.sync_keystore(),
client: client.clone(),
select_chain,
env: proposer,
Expand All @@ -261,7 +260,7 @@ pub fn new_full_base(
sc_service::config::Role::Authority { ref sentry_nodes } => (
sentry_nodes.clone(),
sc_authority_discovery::Role::Authority (
keystore.clone(),
keystore_params.keystore(),
),
),
sc_service::config::Role::Sentry {..} => (
Expand All @@ -275,23 +274,23 @@ pub fn new_full_base(
.filter_map(|e| async move { match e {
Event::Dht(e) => Some(e),
_ => None,
}}).boxed();
}});
let (authority_discovery_worker, _service) = sc_authority_discovery::new_worker_and_service(
client.clone(),
network.clone(),
sentries,
dht_event_stream,
Box::pin(dht_event_stream),
authority_discovery_role,
prometheus_registry.clone(),
);

task_manager.spawn_handle().spawn("authority-discovery-worker", authority_discovery_worker);
task_manager.spawn_handle().spawn("authority-discovery-worker", authority_discovery_worker.run());
}

// 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 BareCryptoStorePtr)
Some(keystore_params.sync_keystore())
} else {
None
};
Expand Down Expand Up @@ -358,7 +357,7 @@ pub fn new_light_base(config: Configuration) -> Result<(
Arc<NetworkService<Block, <Block as BlockT>::Hash>>,
Arc<sc_transaction_pool::LightPool<Block, LightClient, sc_network::config::OnDemand<Block>>>
), ServiceError> {
let (client, backend, keystore, mut task_manager, on_demand) =
let (client, backend, keystore_params, mut task_manager, on_demand) =
sc_service::new_light_parts::<Block, RuntimeApi, Executor>(&config)?;

let select_chain = sc_consensus::LongestChain::new(backend.clone());
Expand Down Expand Up @@ -440,7 +439,8 @@ pub fn new_light_base(config: Configuration) -> Result<(
rpc_extensions_builder: Box::new(sc_service::NoopRpcExtensionBuilder(rpc_extensions)),
client: client.clone(),
transaction_pool: transaction_pool.clone(),
config, keystore, backend, network_status_sinks, system_rpc_tx,
keystore: keystore_params.sync_keystore(),
config, backend, network_status_sinks, system_rpc_tx,
network: network.clone(),
telemetry_connection_sinks: sc_service::TelemetryConnectionSinks::default(),
task_manager: &mut task_manager,
Expand All @@ -458,7 +458,7 @@ pub fn new_light(config: Configuration) -> Result<TaskManager, ServiceError> {

#[cfg(test)]
mod tests {
use std::{sync::Arc, borrow::Cow, any::Any};
use std::{sync::Arc, borrow::Cow, any::Any, convert::TryInto};
use sc_consensus_babe::{CompatibleDigestItem, BabeIntermediate, INTERMEDIATE_KEY};
use sc_consensus_epochs::descendent_query;
use sp_consensus::{
Expand All @@ -469,7 +469,7 @@ mod tests {
use node_runtime::{BalancesCall, Call, UncheckedExtrinsic, Address};
use node_runtime::constants::{currency::CENTS, time::SLOT_DURATION};
use codec::Encode;
use sp_core::{crypto::Pair as CryptoPair, H256};
use sp_core::{crypto::Pair as CryptoPair, H256, traits::SyncCryptoStore, Public};
use sp_runtime::{
generic::{BlockId, Era, Digest, SignedPayload},
traits::{Block as BlockT, Header as HeaderT},
Expand All @@ -480,9 +480,10 @@ mod tests {
use sp_keyring::AccountKeyring;
use sc_service_test::TestNetNode;
use crate::service::{new_full_base, new_light_base, NewFullBase};
use sp_runtime::traits::IdentifyAccount;
use sp_runtime::{key_types::BABE, traits::IdentifyAccount, RuntimeAppPublic};
use sp_transaction_pool::{MaintainedTransactionPool, ChainEvent};
use sc_client_api::BlockBackend;
use sc_keystore::LocalKeystore;

type AccountPublic = <Signature as Verify>::Signer;

Expand All @@ -492,10 +493,11 @@ mod tests {
#[ignore]
fn test_sync() {
let keystore_path = tempfile::tempdir().expect("Creates keystore path");
let keystore = sc_keystore::Store::open(keystore_path.path(), None)
.expect("Creates keystore");
let alice = keystore.write().insert_ephemeral_from_seed::<sc_consensus_babe::AuthorityPair>("//Alice")
.expect("Creates authority pair");
let keystore: Arc<SyncCryptoStore> = Arc::new(LocalKeystore::open(keystore_path.path(), None)
.expect("Creates keystore")
.into());
let alice: sp_consensus_babe::AuthorityId = keystore.sr25519_generate_new(BABE, Some("//Alice"))
.expect("Creates authority pair").into();

let chain_spec = crate::chain_spec::tests::integration_test_config_with_single_authority();

Expand Down Expand Up @@ -574,7 +576,7 @@ mod tests {
slot_num,
&parent_header,
&*service.client(),
&keystore,
keystore.clone(),
&babe_link,
) {
break babe_pre_digest;
Expand All @@ -600,9 +602,13 @@ mod tests {
// sign the pre-sealed hash of the block and then
// add it to a digest item.
let to_sign = pre_hash.encode();
let signature = alice.sign(&to_sign[..]);
let signature = keystore
.sign_with(sp_consensus_babe::AuthorityId::ID, &alice.to_public_crypto_pair(), &to_sign)
.unwrap()
.try_into()
.unwrap();
let item = <DigestItem as CompatibleDigestItem>::babe_seal(
signature.into(),
signature,
);
slot_num += 1;

Expand Down
1 change: 1 addition & 0 deletions bin/node/executor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ sp-io = { version = "2.0.0-rc6", path = "../../../primitives/io" }
sp-state-machine = { version = "0.8.0-rc6", path = "../../../primitives/state-machine" }
sp-trie = { version = "2.0.0-rc6", path = "../../../primitives/trie" }
trie-root = "0.16.0"
futures = "0.3.4"
rakanalh marked this conversation as resolved.
Show resolved Hide resolved
frame-benchmarking = { version = "2.0.0-rc6", path = "../../../frame/benchmarking" }

[dev-dependencies]
Expand Down
Loading