Merge pull request #7331 from paritytech/secretstore_resurrect_error404

SecretStore: return error 404 when there's no key shares for given key on all nodes

secret_store/src/key_server_cluster/admin_sessions/key_version_negotiation_session.rs

@@ -232,7 +232,7 @@ impl<T> SessionImpl<T> where T: SessionTransport {
 		// try to complete session
 		Self::try_complete(&self.core, &mut *data);
 		if no_confirmations_required && data.state != SessionState::Finished {
-			return Err(Error::ConsensusUnreachable);
+			return Err(Error::MissingKeyShare);
 		} else if data.state == SessionState::Finished {
 			return Ok(());
 		}
@@ -438,6 +438,8 @@ impl FastestResultComputer {
 impl SessionResultComputer for FastestResultComputer {
 	fn compute_result(&self, threshold: Option<usize>, confirmations: &BTreeSet<NodeId>, versions: &BTreeMap<H256, BTreeSet<NodeId>>) -> Option<Result<(H256, NodeId), Error>> {
 		match self.threshold.or(threshold) {
+			// if there's no versions at all && we're not waiting for confirmations anymore
+			_ if confirmations.is_empty() && versions.is_empty() => Some(Err(Error::MissingKeyShare)),
 			// if we have key share on this node
 			Some(threshold) => {
 				// select version this node have, with enough participants
@@ -473,6 +475,9 @@ impl SessionResultComputer for LargestSupportResultComputer {
 		if !confirmations.is_empty() {
 			return None;
 		}
+		if versions.is_empty() {
+			return Some(Err(Error::MissingKeyShare));
+		}
 
 		versions.iter()
 			.max_by_key(|&(_, ref n)| n.len())
@@ -491,7 +496,8 @@ mod tests {
 	use key_server_cluster::cluster::tests::DummyCluster;
 	use key_server_cluster::admin_sessions::ShareChangeSessionMeta;
 	use key_server_cluster::message::{Message, KeyVersionNegotiationMessage, RequestKeyVersions, KeyVersions};
-	use super::{SessionImpl, SessionTransport, SessionParams, FastestResultComputer, SessionState};
+	use super::{SessionImpl, SessionTransport, SessionParams, FastestResultComputer, LargestSupportResultComputer,
+		SessionResultComputer, SessionState};
 
 	struct DummyTransport {
 		cluster: Arc<DummyCluster>,
@@ -707,4 +713,19 @@ mod tests {
 		// we can't be sure that node has given key version because previous ShareAdd session could fail
 		assert!(ml.session(0).data.lock().state != SessionState::Finished);
 	}
+
+	#[test]
+	fn fastest_computer_returns_missing_share_if_no_versions_returned() {
+		let computer = FastestResultComputer {
+			self_node_id: Default::default(),
+			threshold: None,
+		};
+		assert_eq!(computer.compute_result(Some(10), &Default::default(), &Default::default()), Some(Err(Error::MissingKeyShare)));
+	}
+
+	#[test]
+	fn largest_computer_returns_missing_share_if_no_versions_returned() {
+		let computer = LargestSupportResultComputer;
+		assert_eq!(computer.compute_result(Some(10), &Default::default(), &Default::default()), Some(Err(Error::MissingKeyShare)));
+	}
 }

secret_store/src/key_server_cluster/cluster.rs

@@ -452,8 +452,8 @@ impl ClusterCore {
 			let is_master_node = meta.self_node_id == meta.master_node_id;
 			if is_master_node && session.is_finished() {
 				data.sessions.negotiation_sessions.remove(&session.id());
-				if let Ok((version, master)) = session.wait() {
+				match session.wait() {
-					match session.continue_action() {
+					Ok((version, master)) => match session.continue_action() {
 						Some(ContinueAction::Decrypt(session, is_shadow_decryption)) => {
 							let initialization_error = if data.self_key_pair.public() == &master {
 								session.initialize(version, is_shadow_decryption)
@@ -479,19 +479,18 @@ impl ClusterCore {
 							}
 						},
 						None => (),
-					}
+					},
-				} else {
+					Err(error) => match session.continue_action() {
-					match session.continue_action() {
 						Some(ContinueAction::Decrypt(session, _)) => {
 							data.sessions.decryption_sessions.remove(&session.id());
-							session.on_session_error(&meta.self_node_id, Error::ConsensusUnreachable);
+							session.on_session_error(&meta.self_node_id, error);
 						},
 						Some(ContinueAction::Sign(session, _)) => {
 							data.sessions.signing_sessions.remove(&session.id());
-							session.on_session_error(&meta.self_node_id, Error::ConsensusUnreachable);
+							session.on_session_error(&meta.self_node_id, error);
 						},
 						None => (),
-					}
+					},
 				}
 			}
 		}

secret_store/src/types/all.rs

@@ -150,6 +150,7 @@ impl From<key_server_cluster::Error> for Error {
 	fn from(err: key_server_cluster::Error) -> Self {
 		match err {
 			key_server_cluster::Error::AccessDenied => Error::AccessDenied,
+			key_server_cluster::Error::MissingKeyShare => Error::DocumentNotFound,
 			_ => Error::Internal(err.into()),
 		}
 	}