SecretStore: publish key only if requested through contract

ethcore/native_contracts/res/secretstore_service.json

@@ -1,6 +1,8 @@
 [
 	{"constant":true,"inputs":[],"name":"serverKeyGenerationRequestsCount","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},
-	{"constant":true,"inputs":[{"name":"authority","type":"address"},{"name":"index","type":"uint256"}],"name":"getServerKeyGenerationRequest","outputs":[{"name":"","type":"bytes32"},{"name":"","type":"uint256"},{"name":"","type":"bool"}],"payable":false,"stateMutability":"view","type":"function"},
+	{"constant":true,"inputs":[{"name":"index","type":"uint256"}],"name":"getServerKeyId","outputs":[{"name":"","type":"bytes32"}],"payable":false,"stateMutability":"view","type":"function"},
 	{"constant":false,"inputs":[{"name":"serverKeyId","type":"bytes32"},{"name":"serverKeyPublic","type":"bytes"},{"name":"v","type":"uint8"},{"name":"r","type":"bytes32"},{"name":"s","type":"bytes32"}],"name":"serverKeyGenerated","outputs":[],"payable":false,"stateMutability":"nonpayable","type":"function"},
+	{"constant":true,"inputs":[{"name":"serverKeyId","type":"bytes32"}],"name":"getServerKeyThreshold","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},
+	{"constant":true,"inputs":[{"name":"serverKeyId","type":"bytes32"},{"name":"authority","type":"address"}],"name":"getServerKeyConfirmationStatus","outputs":[{"name":"","type":"bool"}],"payable":false,"stateMutability":"view","type":"function"},
 	{"anonymous":false,"inputs":[{"indexed":true,"name":"serverKeyId","type":"bytes32"},{"indexed":true,"name":"threshold","type":"uint256"}],"name":"ServerKeyRequested","type":"event"}
 ]

secret_store/src/listener/service_contract.rs

@@ -187,10 +187,30 @@ impl ServiceContract for OnChainServiceContract {
 	}
 
 	fn publish_server_key(&self, server_key_id: &ServerKeyId, server_key: &Public) -> Result<(), String> {
+		// only publish if contract address is set && client is online
+		let contract = self.contract.read();
+		if contract.address == Default::default() {
+			// it is not an error, because key could be generated even without contract
+			return Ok(());
+		}
+		let client = match self.client.upgrade() {
+			Some(client) => client,
+			None => return Err("client is required to publish key".into()),
+		};
+
+		// only publish key if contract waits for publication
+		// failing is ok here - it could be that enough confirmations have been recevied
+		// or key has been requested using HTTP API
+		let do_call = |a, d| future::done(client.call_contract(BlockId::Latest, a, d));
+		let self_address = public_to_address(self.self_key_pair.public());
+		if contract.get_server_key_confirmation_status(&do_call, server_key_id.clone(), self_address).wait().unwrap_or(false) {
+			return Ok(());
+		}
+
+		// prepare transaction data
 		let server_key_hash = keccak(server_key);
 		let signed_server_key = self.self_key_pair.sign(&server_key_hash).map_err(|e| format!("{}", e))?;
 		let signed_server_key: Signature = signed_server_key.into_electrum().into();
-		let contract = self.contract.read();
 		let transaction_data = contract.encode_server_key_generated_input(server_key_id.clone(),
 			server_key.to_vec(),
 			signed_server_key.v(),
@@ -198,13 +218,12 @@ impl ServiceContract for OnChainServiceContract {
 			signed_server_key.s().into()
 		)?;
 
+		// send transaction
 		if contract.address != Default::default() {
-			if let Some(client) = self.client.upgrade() {
+			client.transact_contract(
-				client.transact_contract(
+				contract.address.clone(),
-					contract.address.clone(),
+				transaction_data
-					transaction_data
+			).map_err(|e| format!("{}", e))?;
-				).map_err(|e| format!("{}", e))?;
-			} // else we will read this in the next refresh cycle
 		}
 
 		Ok(())
@@ -218,24 +237,27 @@ impl Iterator for PendingRequestsIterator {
 		if self.index >= self.length {
 			return None;
 		}
+
+		let index = self.index.clone();
 		self.index = self.index + 1.into();
 
+		let self_address = public_to_address(self.self_key_pair.public());
 		let do_call = |a, d| future::done(self.client.call_contract(BlockId::Latest, a, d));
-		let key_generation_request = self.contract.get_server_key_generation_request(&do_call,
+		self.contract.get_server_key_id(&do_call, index).wait()
-			public_to_address(self.self_key_pair.public()),
+			.and_then(|server_key_id|
-			(self.index - 1.into()).clone().into()).wait();
+				self.contract.get_server_key_threshold(&do_call, server_key_id.clone()).wait()
-		let (server_key_id, threshold, is_confirmed) = match key_generation_request {
+					.map(|threshold| (server_key_id, threshold)))
-			Ok((server_key_id, threshold, is_confirmed)) => {
+			.and_then(|(server_key_id, threshold)|
-				(server_key_id, threshold, is_confirmed)
+				self.contract.get_server_key_confirmation_status(&do_call, server_key_id.clone(), self_address).wait()
-			},
+					.map(|is_confirmed| (server_key_id, threshold, is_confirmed)))
-			Err(error) => {
+			.map(|(server_key_id, threshold, is_confirmed)|
-				warn!(target: "secretstore", "{}: call to get_server_key_generation_request failed: {}",
+				Some((is_confirmed, ServiceTask::GenerateServerKey(server_key_id, threshold.into()))))
+			.map_err(|error| {
+				warn!(target: "secretstore", "{}: reading service contract request failed: {}",
 					self.self_key_pair.public(), error);
-				return None;
+				()
-			},
+			})
-		};
+			.unwrap_or(None)
-
-		Some((is_confirmed, ServiceTask::GenerateServerKey(server_key_id, threshold.into())))
 	}
 }
 

secret_store/src/listener/service_contract_listener.rs

@@ -336,9 +336,10 @@ impl ChainNotify for ServiceContractListener {
 			return;
 		}
 
+		let reason = "enacted.len() != 0; qed";
 		self.process_service_contract_events(
-			enacted.first().expect("TODO").clone(),
+			enacted.first().expect(reason).clone(),
-			enacted.last().expect("TODO").clone());
+			enacted.last().expect(reason).clone());
 
 		// schedule retry if received enough blocks since last retry
 		// it maybe inaccurate when switching syncing/synced states, but that's ok
@@ -351,8 +352,6 @@ impl ChainNotify for ServiceContractListener {
 
 impl ClusterSessionsListener<GenerationSession> for ServiceContractListener {
 	fn on_session_removed(&self, session: Arc<GenerationSession>) {
-		// TODO: only start if session started via the contract
-
 		// only publish when the session is started by another node
 		// when it is started by this node, it is published from process_service_task
 		if !is_processed_by_this_key_server(&*self.data.key_server_set, &*self.data.self_key_pair, &session.id()) {