From 4f4a0c5b38ff3e1c6358cb1068e4cd7af8dafa29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com>
Date: Mon, 22 Feb 2021 15:24:09 +0100
Subject: [PATCH] Make keystore return `None` when a key doesn't exist (#8163)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Make keystore return `None` when a key doesn't exist
* Fixes
* More fixes
* Update comment
* Update primitives/keystore/src/lib.rs
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
* Update client/keystore/src/local.rs
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
* Address comments
* Update client/keystore/src/local.rs
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
---
substrate/bin/node/cli/src/service.rs | 4 +-
.../client/authority-discovery/src/worker.rs | 6 +-
.../authority-discovery/src/worker/tests.rs | 6 +-
substrate/client/consensus/aura/src/lib.rs | 7 +-
.../client/consensus/babe/src/authorship.rs | 4 +-
substrate/client/consensus/babe/src/lib.rs | 3 +
substrate/client/keystore/src/lib.rs | 4 -
substrate/client/keystore/src/local.rs | 130 ++++++++++++------
.../primitives/finality-grandpa/src/lib.rs | 2 +-
substrate/primitives/io/src/lib.rs | 9 +-
substrate/primitives/keystore/src/lib.rs | 69 +++++-----
substrate/primitives/keystore/src/testing.rs | 48 ++++---
12 files changed, 174 insertions(+), 118 deletions(-)
diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs
index db4ed3f4f1d..312a0226fc3 100644
--- a/substrate/bin/node/cli/src/service.rs
+++ b/substrate/bin/node/cli/src/service.rs
@@ -625,9 +625,7 @@ mod tests {
sp_consensus_babe::AuthorityId::ID,
&alice.to_public_crypto_pair(),
&to_sign,
- ).unwrap()
- .try_into()
- .unwrap();
+ ).unwrap().unwrap().try_into().unwrap();
let item = <DigestItem as CompatibleDigestItem>::babe_seal(
signature,
);
diff --git a/substrate/client/authority-discovery/src/worker.rs b/substrate/client/authority-discovery/src/worker.rs
index dac7a97746b..b1fb89669bf 100644
--- a/substrate/client/authority-discovery/src/worker.rs
+++ b/substrate/client/authority-discovery/src/worker.rs
@@ -296,10 +296,10 @@ where
for (sign_result, key) in signatures.into_iter().zip(keys) {
let mut signed_addresses = vec![];
- // sign_with_all returns Result<Signature, Error> signature
- // is generated for a public key that is supported.
// Verify that all signatures exist for all provided keys.
- let signature = sign_result.map_err(|_| Error::MissingSignature(key.clone()))?;
+ let signature = sign_result.ok()
+ .flatten()
+ .ok_or_else(|| Error::MissingSignature(key.clone()))?;
schema::SignedAuthorityAddresses {
addresses: serialized_addresses.clone(),
signature,
diff --git a/substrate/client/authority-discovery/src/worker/tests.rs b/substrate/client/authority-discovery/src/worker/tests.rs
index a994e08691b..04f597aa26b 100644
--- a/substrate/client/authority-discovery/src/worker/tests.rs
+++ b/substrate/client/authority-discovery/src/worker/tests.rs
@@ -187,7 +187,7 @@ async fn build_dht_event(
serialized_addresses.as_slice(),
)
.await
- .map_err(|_| Error::Signing)
+ .unwrap()
.unwrap();
let mut signed_addresses = vec![];
@@ -195,9 +195,7 @@ async fn build_dht_event(
addresses: serialized_addresses.clone(),
signature,
}
- .encode(&mut signed_addresses)
- .map_err(Error::EncodingProto)
- .unwrap();
+ .encode(&mut signed_addresses).unwrap();
let key = hash_authority_id(&public_key.to_raw_vec());
let value = signed_addresses;
diff --git a/substrate/client/consensus/aura/src/lib.rs b/substrate/client/consensus/aura/src/lib.rs
index 0702ccd7f13..47ce364cb66 100644
--- a/substrate/client/consensus/aura/src/lib.rs
+++ b/substrate/client/consensus/aura/src/lib.rs
@@ -255,8 +255,8 @@ where
let expected_author = slot_author::<P>(slot, epoch_data);
expected_author.and_then(|p| {
if SyncCryptoStore::has_keys(
- &*self.keystore,
- &[(p.to_raw_vec(), sp_application_crypto::key_types::AURA)],
+ &*self.keystore,
+ &[(p.to_raw_vec(), sp_application_crypto::key_types::AURA)],
) {
Some(p.clone())
} else {
@@ -299,6 +299,9 @@ where
header_hash.as_ref()
).map_err(|e| sp_consensus::Error::CannotSign(
public.clone(), e.to_string(),
+ ))?
+ .ok_or_else(|| sp_consensus::Error::CannotSign(
+ public.clone(), "Could not find key in keystore.".into(),
))?;
let signature = signature.clone().try_into()
.map_err(|_| sp_consensus::Error::InvalidSignature(
diff --git a/substrate/client/consensus/babe/src/authorship.rs b/substrate/client/consensus/babe/src/authorship.rs
index 1120f660613..cf75a4a43f2 100644
--- a/substrate/client/consensus/babe/src/authorship.rs
+++ b/substrate/client/consensus/babe/src/authorship.rs
@@ -159,7 +159,7 @@ fn claim_secondary_slot(
authority_id.as_ref(),
transcript_data,
);
- if let Ok(signature) = result {
+ if let Ok(Some(signature)) = result {
Some(PreDigest::SecondaryVRF(SecondaryVRFPreDigest {
slot,
vrf_output: VRFOutput(signature.output),
@@ -265,7 +265,7 @@ fn claim_primary_slot(
authority_id.as_ref(),
transcript_data,
);
- if let Ok(signature) = result {
+ if let Ok(Some(signature)) = result {
let public = PublicKey::from_bytes(&authority_id.to_raw_vec()).ok()?;
let inout = match signature.output.attach_input_hash(&public, transcript) {
Ok(inout) => inout,
diff --git a/substrate/client/consensus/babe/src/lib.rs b/substrate/client/consensus/babe/src/lib.rs
index a6530dea08d..a8e533d2a83 100644
--- a/substrate/client/consensus/babe/src/lib.rs
+++ b/substrate/client/consensus/babe/src/lib.rs
@@ -649,6 +649,9 @@ where
)
.map_err(|e| sp_consensus::Error::CannotSign(
public.clone(), e.to_string(),
+ ))?
+ .ok_or_else(|| sp_consensus::Error::CannotSign(
+ public.clone(), "Could not find key in keystore.".into(),
))?;
let signature: AuthoritySignature = signature.clone().try_into()
.map_err(|_| sp_consensus::Error::InvalidSignature(
diff --git a/substrate/client/keystore/src/lib.rs b/substrate/client/keystore/src/lib.rs
index 9cad56efacf..38ab640d2e3 100644
--- a/substrate/client/keystore/src/lib.rs
+++ b/substrate/client/keystore/src/lib.rs
@@ -46,9 +46,6 @@ pub enum Error {
/// Public key type is not supported
#[display(fmt="Key crypto type is not supported")]
KeyNotSupported(KeyTypeId),
- /// Pair not found for public key and KeyTypeId
- #[display(fmt="Pair not found for {} public key", "_0")]
- PairNotFound(String),
/// Keystore unavailable
#[display(fmt="Keystore unavailable")]
Unavailable,
@@ -61,7 +58,6 @@ impl From<Error> for TraitError {
fn from(error: Error) -> Self {
match error {
Error::KeyNotSupported(id) => TraitError::KeyNotSupported(id),
- Error::PairNotFound(e) => TraitError::PairNotFound(e),
Error::InvalidSeed | Error::InvalidPhrase | Error::InvalidPassword => {
TraitError::ValidationError(error.to_string())
},
diff --git a/substrate/client/keystore/src/local.rs b/substrate/client/keystore/src/local.rs
index 866a50ae4c9..482ef407601 100644
--- a/substrate/client/keystore/src/local.rs
+++ b/substrate/client/keystore/src/local.rs
@@ -60,9 +60,9 @@ impl LocalKeystore {
/// Get a key pair for the given public key.
///
- /// This function is only available for a local keystore. If your application plans to work with
- /// remote keystores, you do not want to depend on it.
- pub fn key_pair<Pair: AppPair>(&self, public: &<Pair as AppKey>::Public) -> Result<Pair> {
+ /// Returns `Ok(None)` if the key doesn't exist, `Ok(Some(_))` if the key exists and
+ /// `Err(_)` when something failed.
+ pub fn key_pair<Pair: AppPair>(&self, public: &<Pair as AppKey>::Public) -> Result<Option<Pair>> {
self.0.read().key_pair::<Pair>(public)
}
}
@@ -130,7 +130,7 @@ impl CryptoStore for LocalKeystore {
id: KeyTypeId,
key: &CryptoTypePublicPair,
msg: &[u8],
- ) -> std::result::Result<Vec<u8>, TraitError> {
+ ) -> std::result::Result<Option<Vec<u8>>, TraitError> {
SyncCryptoStore::sign_with(self, id, key, msg)
}
@@ -139,7 +139,7 @@ impl CryptoStore for LocalKeystore {
key_type: KeyTypeId,
public: &sr25519::Public,
transcript_data: VRFTranscriptData,
- ) -> std::result::Result<VRFSignature, TraitError> {
+ ) -> std::result::Result<Option<VRFSignature>, TraitError> {
SyncCryptoStore::sr25519_vrf_sign(self, key_type, public, transcript_data)
}
}
@@ -175,28 +175,28 @@ impl SyncCryptoStore for LocalKeystore {
id: KeyTypeId,
key: &CryptoTypePublicPair,
msg: &[u8],
- ) -> std::result::Result<Vec<u8>, TraitError> {
+ ) -> std::result::Result<Option<Vec<u8>>, TraitError> {
match key.0 {
ed25519::CRYPTO_ID => {
let pub_key = ed25519::Public::from_slice(key.1.as_slice());
- let key_pair: ed25519::Pair = self.0.read()
+ let key_pair = self.0.read()
.key_pair_by_type::<ed25519::Pair>(&pub_key, id)
.map_err(|e| TraitError::from(e))?;
- Ok(key_pair.sign(msg).encode())
+ key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
}
sr25519::CRYPTO_ID => {
let pub_key = sr25519::Public::from_slice(key.1.as_slice());
- let key_pair: sr25519::Pair = self.0.read()
+ let key_pair = self.0.read()
.key_pair_by_type::<sr25519::Pair>(&pub_key, id)
.map_err(|e| TraitError::from(e))?;
- Ok(key_pair.sign(msg).encode())
+ key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
},
ecdsa::CRYPTO_ID => {
let pub_key = ecdsa::Public::from_slice(key.1.as_slice());
- let key_pair: ecdsa::Pair = self.0.read()
+ let key_pair = self.0.read()
.key_pair_by_type::<ecdsa::Pair>(&pub_key, id)
.map_err(|e| TraitError::from(e))?;
- Ok(key_pair.sign(msg).encode())
+ key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
}
_ => Err(TraitError::KeyNotSupported(id))
}
@@ -232,7 +232,7 @@ impl SyncCryptoStore for LocalKeystore {
.map(|k| ed25519::Public::from_slice(k.as_slice()))
.collect()
})
- .unwrap_or_default()
+ .unwrap_or_default()
}
fn ed25519_generate_new(
@@ -278,7 +278,8 @@ impl SyncCryptoStore for LocalKeystore {
}
fn has_keys(&self, public_keys: &[(Vec<u8>, KeyTypeId)]) -> bool {
- public_keys.iter().all(|(p, t)| self.0.read().key_phrase_by_type(&p, *t).is_ok())
+ public_keys.iter()
+ .all(|(p, t)| self.0.read().key_phrase_by_type(&p, *t).ok().flatten().is_some())
}
fn sr25519_vrf_sign(
@@ -286,16 +287,19 @@ impl SyncCryptoStore for LocalKeystore {
key_type: KeyTypeId,
public: &Sr25519Public,
transcript_data: VRFTranscriptData,
- ) -> std::result::Result<VRFSignature, TraitError> {
+ ) -> std::result::Result<Option<VRFSignature>, TraitError> {
let transcript = make_transcript(transcript_data);
- let pair = self.0.read().key_pair_by_type::<Sr25519Pair>(public, key_type)
- .map_err(|e| TraitError::PairNotFound(e.to_string()))?;
+ let pair = self.0.read().key_pair_by_type::<Sr25519Pair>(public, key_type)?;
- let (inout, proof, _) = pair.as_ref().vrf_sign(transcript);
- Ok(VRFSignature {
- output: inout.to_output(),
- proof,
- })
+ if let Some(pair) = pair {
+ let (inout, proof, _) = pair.as_ref().vrf_sign(transcript);
+ Ok(Some(VRFSignature {
+ output: inout.to_output(),
+ proof,
+ }))
+ } else {
+ Ok(None)
+ }
}
}
@@ -411,36 +415,53 @@ impl KeystoreInner {
}
/// Get the key phrase for a given public key and key type.
- fn key_phrase_by_type(&self, public: &[u8], key_type: KeyTypeId) -> Result<String> {
+ fn key_phrase_by_type(&self, public: &[u8], key_type: KeyTypeId) -> Result<Option<String>> {
if let Some(phrase) = self.get_additional_pair(public, key_type) {
- return Ok(phrase.clone())
+ return Ok(Some(phrase.clone()))
}
- let path = self.key_file_path(public, key_type).ok_or_else(|| Error::Unavailable)?;
- let file = File::open(path)?;
+ let path = if let Some(path) = self.key_file_path(public, key_type) {
+ path
+ } else {
+ return Ok(None);
+ };
+
+ if path.exists() {
+ let file = File::open(path)?;
- serde_json::from_reader(&file).map_err(Into::into)
+ serde_json::from_reader(&file).map_err(Into::into).map(Some)
+ } else {
+ Ok(None)
+ }
}
/// Get a key pair for the given public key and key type.
- fn key_pair_by_type<Pair: PairT>(&self,
+ fn key_pair_by_type<Pair: PairT>(
+ &self,
public: &Pair::Public,
key_type: KeyTypeId,
- ) -> Result<Pair> {
- let phrase = self.key_phrase_by_type(public.as_slice(), key_type)?;
+ ) -> Result<Option<Pair>> {
+ let phrase = if let Some(p) = self.key_phrase_by_type(public.as_slice(), key_type)? {
+ p
+ } else {
+ return Ok(None)
+ };
+
let pair = Pair::from_string(
&phrase,
self.password(),
).map_err(|_| Error::InvalidPhrase)?;
if &pair.public() == public {
- Ok(pair)
+ Ok(Some(pair))
} else {
Err(Error::InvalidPassword)
}
}
- /// Returns the file path for the given public key and key type.
+ /// Get the file path for the given public key and key type.
+ ///
+ /// Returns `None` if the keystore only exists in-memory and there isn't any path to provide.
fn key_file_path(&self, public: &[u8], key_type: KeyTypeId) -> Option<PathBuf> {
let mut buf = self.path.as_ref()?.clone();
let key_type = hex::encode(key_type.0);
@@ -481,8 +502,12 @@ impl KeystoreInner {
}
/// Get a key pair for the given public key.
- pub fn key_pair<Pair: AppPair>(&self, public: &<Pair as AppKey>::Public) -> Result<Pair> {
- self.key_pair_by_type::<Pair::Generic>(IsWrappedBy::from_ref(public), Pair::ID).map(Into::into)
+ ///
+ /// Returns `Ok(None)` if the key doesn't exist, `Ok(Some(_))` if the key exists or `Err(_)` when
+ /// something failed.
+ pub fn key_pair<Pair: AppPair>(&self, public: &<Pair as AppKey>::Public) -> Result<Option<Pair>> {
+ self.key_pair_by_type::<Pair::Generic>(IsWrappedBy::from_ref(public), Pair::ID)
+ .map(|v| v.map(Into::into))
}
}
@@ -531,13 +556,40 @@ mod tests {
assert!(store.public_keys::<ed25519::AppPublic>().unwrap().is_empty());
let key: ed25519::AppPair = store.generate().unwrap();
- let key2: ed25519::AppPair = store.key_pair(&key.public()).unwrap();
+ let key2: ed25519::AppPair = store.key_pair(&key.public()).unwrap().unwrap();
assert_eq!(key.public(), key2.public());
assert_eq!(store.public_keys::<ed25519::AppPublic>().unwrap()[0], key.public());
}
+ #[test]
+ fn has_keys_works() {
+ let temp_dir = TempDir::new().unwrap();
+ let store = LocalKeystore::open(temp_dir.path(), None).unwrap();
+
+ let key: ed25519::AppPair = store.0.write().generate().unwrap();
+ let key2 = ed25519::Pair::generate().0;
+
+ assert!(
+ !SyncCryptoStore::has_keys(&store, &[(key2.public().to_vec(), ed25519::AppPublic::ID)])
+ );
+
+ assert!(
+ !SyncCryptoStore::has_keys(
+ &store,
+ &[
+ (key2.public().to_vec(), ed25519::AppPublic::ID),
+ (key.public().to_raw_vec(), ed25519::AppPublic::ID),
+ ],
+ )
+ );
+
+ assert!(
+ SyncCryptoStore::has_keys(&store, &[(key.public().to_raw_vec(), ed25519::AppPublic::ID)])
+ );
+ }
+
#[test]
fn test_insert_ephemeral_from_seed() {
let temp_dir = TempDir::new().unwrap();
@@ -554,7 +606,7 @@ mod tests {
drop(store);
let store = KeystoreInner::open(temp_dir.path(), None).unwrap();
// Keys generated from seed should not be persisted!
- assert!(store.key_pair::<ed25519::AppPair>(&pair.public()).is_err());
+ assert!(store.key_pair::<ed25519::AppPair>(&pair.public()).unwrap().is_none());
}
#[test]
@@ -569,7 +621,7 @@ mod tests {
let pair: ed25519::AppPair = store.generate().unwrap();
assert_eq!(
pair.public(),
- store.key_pair::<ed25519::AppPair>(&pair.public()).unwrap().public(),
+ store.key_pair::<ed25519::AppPair>(&pair.public()).unwrap().unwrap().public(),
);
// Without the password the key should not be retrievable
@@ -582,7 +634,7 @@ mod tests {
).unwrap();
assert_eq!(
pair.public(),
- store.key_pair::<ed25519::AppPair>(&pair.public()).unwrap().public(),
+ store.key_pair::<ed25519::AppPair>(&pair.public()).unwrap().unwrap().public(),
);
}
@@ -626,7 +678,7 @@ mod tests {
let store_key_pair = store.key_pair_by_type::<sr25519::AppPair>(
&key_pair.public(),
SR25519,
- ).expect("Gets key pair from keystore");
+ ).expect("Gets key pair from keystore").unwrap();
assert_eq!(key_pair.public(), store_key_pair.public());
}
diff --git a/substrate/primitives/finality-grandpa/src/lib.rs b/substrate/primitives/finality-grandpa/src/lib.rs
index 383e4fe3713..5b393bd1d80 100644
--- a/substrate/primitives/finality-grandpa/src/lib.rs
+++ b/substrate/primitives/finality-grandpa/src/lib.rs
@@ -400,7 +400,7 @@ where
AuthorityId::ID,
&public.to_public_crypto_pair(),
&encoded[..],
- ).ok()?.try_into().ok()?;
+ ).ok().flatten()?.try_into().ok()?;
Some(grandpa::SignedMessage {
message,
diff --git a/substrate/primitives/io/src/lib.rs b/substrate/primitives/io/src/lib.rs
index 397dd3c2171..c0db1120dc4 100644
--- a/substrate/primitives/io/src/lib.rs
+++ b/substrate/primitives/io/src/lib.rs
@@ -474,8 +474,9 @@ pub trait Crypto {
let keystore = &***self.extension::<KeystoreExt>()
.expect("No `keystore` associated for the current context!");
SyncCryptoStore::sign_with(keystore, id, &pub_key.into(), msg)
- .map(|sig| ed25519::Signature::from_slice(sig.as_slice()))
.ok()
+ .flatten()
+ .map(|sig| ed25519::Signature::from_slice(sig.as_slice()))
}
/// Verify `ed25519` signature.
@@ -600,8 +601,9 @@ pub trait Crypto {
let keystore = &***self.extension::<KeystoreExt>()
.expect("No `keystore` associated for the current context!");
SyncCryptoStore::sign_with(keystore, id, &pub_key.into(), msg)
- .map(|sig| sr25519::Signature::from_slice(sig.as_slice()))
.ok()
+ .flatten()
+ .map(|sig| sr25519::Signature::from_slice(sig.as_slice()))
}
/// Verify an `sr25519` signature.
@@ -646,8 +648,9 @@ pub trait Crypto {
let keystore = &***self.extension::<KeystoreExt>()
.expect("No `keystore` associated for the current context!");
SyncCryptoStore::sign_with(keystore, id, &pub_key.into(), msg)
- .map(|sig| ecdsa::Signature::from_slice(sig.as_slice()))
.ok()
+ .flatten()
+ .map(|sig| ecdsa::Signature::from_slice(sig.as_slice()))
}
/// Verify `ecdsa` signature.
diff --git a/substrate/primitives/keystore/src/lib.rs b/substrate/primitives/keystore/src/lib.rs
index f42f6dd7122..2fda3a48c5d 100644
--- a/substrate/primitives/keystore/src/lib.rs
+++ b/substrate/primitives/keystore/src/lib.rs
@@ -34,9 +34,6 @@ pub enum Error {
/// Public key type is not supported
#[display(fmt="Key not supported: {:?}", _0)]
KeyNotSupported(KeyTypeId),
- /// Pair not found for public key and KeyTypeId
- #[display(fmt="Pair was not found: {}", _0)]
- PairNotFound(String),
/// Validation error
#[display(fmt="Validation error: {}", _0)]
ValidationError(String),
@@ -125,37 +122,39 @@ pub trait CryptoStore: Send + Sync {
/// Signs a message with the private key that matches
/// the public key passed.
///
- /// Returns the SCALE encoded signature if key is found & supported,
- /// an error otherwise.
+ /// Returns the SCALE encoded signature if key is found and supported, `None` if the key doesn't
+ /// exist or an error when something failed.
async fn sign_with(
&self,
id: KeyTypeId,
key: &CryptoTypePublicPair,
msg: &[u8],
- ) -> Result<Vec<u8>, Error>;
+ ) -> Result<Option<Vec<u8>>, Error>;
/// Sign with any key
///
/// Given a list of public keys, find the first supported key and
/// sign the provided message with that key.
///
- /// Returns a tuple of the used key and the SCALE encoded signature.
+ /// Returns a tuple of the used key and the SCALE encoded signature or `None` if no key could
+ /// be found to sign.
async fn sign_with_any(
&self,
id: KeyTypeId,
keys: Vec<CryptoTypePublicPair>,
msg: &[u8]
- ) -> Result<(CryptoTypePublicPair, Vec<u8>), Error> {
+ ) -> Result<Option<(CryptoTypePublicPair, Vec<u8>)>, Error> {
if keys.len() == 1 {
- return self.sign_with(id, &keys[0], msg).await.map(|s| (keys[0].clone(), s));
+ return Ok(self.sign_with(id, &keys[0], msg).await?.map(|s| (keys[0].clone(), s)));
} else {
for k in self.supported_keys(id, keys).await? {
- if let Ok(sign) = self.sign_with(id, &k, msg).await {
- return Ok((k, sign));
+ if let Ok(Some(sign)) = self.sign_with(id, &k, msg).await {
+ return Ok(Some((k, sign)));
}
}
}
- Err(Error::KeyNotSupported(id))
+
+ Ok(None)
}
/// Sign with all keys
@@ -164,13 +163,13 @@ pub trait CryptoStore: Send + Sync {
/// each key given that the key is supported.
///
/// Returns a list of `Result`s each representing the SCALE encoded
- /// signature of each key or a Error for non-supported keys.
+ /// signature of each key, `None` if the key doesn't exist or a error when something failed.
async fn sign_with_all(
&self,
id: KeyTypeId,
keys: Vec<CryptoTypePublicPair>,
msg: &[u8],
- ) -> Result<Vec<Result<Vec<u8>, Error>>, ()> {
+ ) -> Result<Vec<Result<Option<Vec<u8>>, Error>>, ()> {
let futs = keys.iter()
.map(|k| self.sign_with(id, k, msg));
@@ -187,16 +186,14 @@ pub trait CryptoStore: Send + Sync {
/// Namely, VRFOutput and VRFProof which are returned
/// inside the `VRFSignature` container struct.
///
- /// This function will return an error in the cases where
- /// the public key and key type provided do not match a private
- /// key in the keystore. Or, in the context of remote signing
- /// an error could be a network one.
+ /// This function will return `None` if the given `key_type` and `public` combination
+ /// doesn't exist in the keystore or an `Err` when something failed.
async fn sr25519_vrf_sign(
&self,
key_type: KeyTypeId,
public: &sr25519::Public,
transcript_data: VRFTranscriptData,
- ) -> Result<VRFSignature, Error>;
+ ) -> Result<Option<VRFSignature>, Error>;
}
/// Sync version of the CryptoStore
@@ -285,37 +282,41 @@ pub trait SyncCryptoStore: CryptoStore + Send + Sync {
/// Signs a message with the private key that matches
/// the public key passed.
///
- /// Returns the SCALE encoded signature if key is found & supported,
- /// an error otherwise.
+ /// Returns the SCALE encoded signature if key is found and supported, `None` if the key doesn't
+ /// exist or an error when something failed.
fn sign_with(
&self,
id: KeyTypeId,
key: &CryptoTypePublicPair,
msg: &[u8],
- ) -> Result<Vec<u8>, Error>;
+ ) -> Result<Option<Vec<u8>>, Error>;
/// Sign with any key
///
/// Given a list of public keys, find the first supported key and
/// sign the provided message with that key.
///
- /// Returns a tuple of the used key and the SCALE encoded signature.
+ /// Returns a tuple of the used key and the SCALE encoded signature or `None` if no key could
+ /// be found to sign.
fn sign_with_any(
&self,
id: KeyTypeId,
keys: Vec<CryptoTypePublicPair>,
msg: &[u8]
- ) -> Result<(CryptoTypePublicPair, Vec<u8>), Error> {
+ ) -> Result<Option<(CryptoTypePublicPair, Vec<u8>)>, Error> {
if keys.len() == 1 {
- return SyncCryptoStore::sign_with(self, id, &keys[0], msg).map(|s| (keys[0].clone(), s));
+ return Ok(
+ SyncCryptoStore::sign_with(self, id, &keys[0], msg)?.map(|s| (keys[0].clone(), s)),
+ )
} else {
for k in SyncCryptoStore::supported_keys(self, id, keys)? {
- if let Ok(sign) = SyncCryptoStore::sign_with(self, id, &k, msg) {
- return Ok((k, sign));
+ if let Ok(Some(sign)) = SyncCryptoStore::sign_with(self, id, &k, msg) {
+ return Ok(Some((k, sign)));
}
}
}
- Err(Error::KeyNotSupported(id))
+
+ Ok(None)
}
/// Sign with all keys
@@ -324,13 +325,13 @@ pub trait SyncCryptoStore: CryptoStore + Send + Sync {
/// each key given that the key is supported.
///
/// Returns a list of `Result`s each representing the SCALE encoded
- /// signature of each key or a Error for non-supported keys.
+ /// signature of each key, `None` if the key doesn't exist or an error when something failed.
fn sign_with_all(
&self,
id: KeyTypeId,
keys: Vec<CryptoTypePublicPair>,
msg: &[u8],
- ) -> Result<Vec<Result<Vec<u8>, Error>>, ()>{
+ ) -> Result<Vec<Result<Option<Vec<u8>>, Error>>, ()> {
Ok(keys.iter().map(|k| SyncCryptoStore::sign_with(self, id, k, msg)).collect())
}
@@ -344,16 +345,14 @@ pub trait SyncCryptoStore: CryptoStore + Send + Sync {
/// Namely, VRFOutput and VRFProof which are returned
/// inside the `VRFSignature` container struct.
///
- /// This function will return an error in the cases where
- /// the public key and key type provided do not match a private
- /// key in the keystore. Or, in the context of remote signing
- /// an error could be a network one.
+ /// This function will return `None` if the given `key_type` and `public` combination
+ /// doesn't exist in the keystore or an `Err` when something failed.
fn sr25519_vrf_sign(
&self,
key_type: KeyTypeId,
public: &sr25519::Public,
transcript_data: VRFTranscriptData,
- ) -> Result<VRFSignature, Error>;
+ ) -> Result<Option<VRFSignature>, Error>;
}
/// A pointer to a keystore.
diff --git a/substrate/primitives/keystore/src/testing.rs b/substrate/primitives/keystore/src/testing.rs
index 702e2bbc857..caee7178e09 100644
--- a/substrate/primitives/keystore/src/testing.rs
+++ b/substrate/primitives/keystore/src/testing.rs
@@ -132,7 +132,7 @@ impl CryptoStore for KeyStore {
id: KeyTypeId,
key: &CryptoTypePublicPair,
msg: &[u8],
- ) -> Result<Vec<u8>, Error> {
+ ) -> Result<Option<Vec<u8>>, Error> {
SyncCryptoStore::sign_with(self, id, key, msg)
}
@@ -141,7 +141,7 @@ impl CryptoStore for KeyStore {
key_type: KeyTypeId,
public: &sr25519::Public,
transcript_data: VRFTranscriptData,
- ) -> Result<VRFSignature, Error> {
+ ) -> Result<Option<VRFSignature>, Error> {
SyncCryptoStore::sr25519_vrf_sign(self, key_type, public, transcript_data)
}
}
@@ -280,27 +280,27 @@ impl SyncCryptoStore for KeyStore {
id: KeyTypeId,
key: &CryptoTypePublicPair,
msg: &[u8],
- ) -> Result<Vec<u8>, Error> {
+ ) -> Result<Option<Vec<u8>>, Error> {
use codec::Encode;
match key.0 {
ed25519::CRYPTO_ID => {
- let key_pair: ed25519::Pair = self
- .ed25519_key_pair(id, &ed25519::Public::from_slice(key.1.as_slice()))
- .ok_or_else(|| Error::PairNotFound("ed25519".to_owned()))?;
- return Ok(key_pair.sign(msg).encode());
+ let key_pair = self
+ .ed25519_key_pair(id, &ed25519::Public::from_slice(key.1.as_slice()));
+
+ key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
}
sr25519::CRYPTO_ID => {
- let key_pair: sr25519::Pair = self
- .sr25519_key_pair(id, &sr25519::Public::from_slice(key.1.as_slice()))
- .ok_or_else(|| Error::PairNotFound("sr25519".to_owned()))?;
- return Ok(key_pair.sign(msg).encode());
+ let key_pair = self
+ .sr25519_key_pair(id, &sr25519::Public::from_slice(key.1.as_slice()));
+
+ key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
}
ecdsa::CRYPTO_ID => {
- let key_pair: ecdsa::Pair = self
- .ecdsa_key_pair(id, &ecdsa::Public::from_slice(key.1.as_slice()))
- .ok_or_else(|| Error::PairNotFound("ecdsa".to_owned()))?;
- return Ok(key_pair.sign(msg).encode());
+ let key_pair = self
+ .ecdsa_key_pair(id, &ecdsa::Public::from_slice(key.1.as_slice()));
+
+ key_pair.map(|k| k.sign(msg).encode()).map(Ok).transpose()
}
_ => Err(Error::KeyNotSupported(id))
}
@@ -311,15 +311,19 @@ impl SyncCryptoStore for KeyStore {
key_type: KeyTypeId,
public: &sr25519::Public,
transcript_data: VRFTranscriptData,
- ) -> Result<VRFSignature, Error> {
+ ) -> Result<Option<VRFSignature>, Error> {
let transcript = make_transcript(transcript_data);
- let pair = self.sr25519_key_pair(key_type, public)
- .ok_or_else(|| Error::PairNotFound("Not found".to_owned()))?;
+ let pair = if let Some(k) = self.sr25519_key_pair(key_type, public) {
+ k
+ } else {
+ return Ok(None)
+ };
+
let (inout, proof, _) = pair.as_ref().vrf_sign(transcript);
- Ok(VRFSignature {
+ Ok(Some(VRFSignature {
output: inout.to_output(),
proof,
- })
+ }))
}
}
@@ -394,7 +398,7 @@ mod tests {
&key_pair.public(),
transcript_data.clone(),
);
- assert!(result.is_err());
+ assert!(result.unwrap().is_none());
SyncCryptoStore::insert_unknown(
&store,
@@ -410,6 +414,6 @@ mod tests {
transcript_data,
);
- assert!(result.is_ok());
+ assert!(result.unwrap().is_some());
}
}
--
GitLab