Read registry_address from block with REQUEST_CONFIRMATIONS_REQUIRED (#8309)

* Read registry_address from block with REQUEST_CONFIRMATIONS_REQUIRED

* Require confirmation blocks in key_server_set

* Add license preamble

* TODO item for constant confirmation required number

* Change license year in helpers.rs to 2015-2018

secret_store/src/helpers.rs

@@ -0,0 +1,29 @@
+// Copyright 2015-2018 Parity Technologies (UK) Ltd.
+// This file is part of Parity.
+
+// Parity is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Parity is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Parity.  If not, see <http://www.gnu.org/licenses/>.
+
+use ethcore::client::{Client, BlockChainClient, BlockId};
+use ethereum_types::H256;
+
+// TODO: Instead of a constant, make this based on consensus finality.
+/// Number of confirmations required before request can be processed.
+pub const REQUEST_CONFIRMATIONS_REQUIRED: u64 = 3;
+
+/// Get hash of the last block with at least n confirmations.
+pub fn get_confirmed_block_hash(client: &Client, confirmations: u64) -> Option<H256> {
+	client.block_number(BlockId::Latest)
+		.map(|b| b.saturating_sub(confirmations))
+		.and_then(|b| client.block_hash(BlockId::Number(b)))
+}

secret_store/src/key_server_set.rs

@@ -26,6 +26,7 @@ use ethereum_types::{H256, Address};
 use bytes::Bytes;
 use types::all::{Error, Public, NodeAddress, NodeId};
 use trusted_client::TrustedClient;
+use helpers::{get_confirmed_block_hash, REQUEST_CONFIRMATIONS_REQUIRED};
 use {NodeKeyPair};
 
 use_contract!(key_server, "KeyServerSet", "res/key_server_set.json");
@@ -325,7 +326,7 @@ impl CachedContract {
 
 	fn read_from_registry_if_required(&mut self, client: &Client, enacted: Vec<H256>, retracted: Vec<H256>) {
 		// read new contract from registry
-		let new_contract_addr = client.registry_address(KEY_SERVER_SET_CONTRACT_REGISTRY_NAME.to_owned(), BlockId::Latest);
+		let new_contract_addr = get_confirmed_block_hash(&*client, REQUEST_CONFIRMATIONS_REQUIRED).and_then(|block_hash| client.registry_address(KEY_SERVER_SET_CONTRACT_REGISTRY_NAME.to_owned(), BlockId::Hash(block_hash)));
 
 		// new contract installed => read nodes set from the contract
 		if self.contract_address.as_ref() != new_contract_addr.as_ref() {

secret_store/src/lib.rs

@@ -54,6 +54,7 @@ extern crate log;
 
 mod key_server_cluster;
 mod types;
+mod helpers;
 
 mod traits;
 mod acl_storage;

secret_store/src/listener/service_contract.rs

@@ -26,6 +26,7 @@ use ethereum_types::{H256, U256, Address};
 use listener::ApiMask;
 use listener::service_contract_listener::ServiceTask;
 use trusted_client::TrustedClient;
+use helpers::{get_confirmed_block_hash, REQUEST_CONFIRMATIONS_REQUIRED};
 use {ServerKeyId, NodeKeyPair, ContractAddress};
 
 use_contract!(service, "Service", "res/service.json");
@@ -52,9 +53,6 @@ const DOCUMENT_KEY_COMMON_PART_RETRIEVAL_REQUESTED_EVENT_NAME: &'static [u8] = &
 /// Document key personal part retrieval has been requested.
 const DOCUMENT_KEY_PERSONAL_PART_RETRIEVAL_REQUESTED_EVENT_NAME: &'static [u8] = &*b"DocumentKeyPersonalRetrievalRequested(bytes32,bytes)";
 
-/// Number of confirmations required before request can be processed.
-const REQUEST_CONFIRMATIONS_REQUIRED: u64 = 3;
-
 lazy_static! {
 	pub static ref SERVER_KEY_GENERATION_REQUESTED_EVENT_NAME_HASH: H256 = keccak(SERVER_KEY_GENERATION_REQUESTED_EVENT_NAME);
 	pub static ref SERVER_KEY_RETRIEVAL_REQUESTED_EVENT_NAME_HASH: H256 = keccak(SERVER_KEY_RETRIEVAL_REQUESTED_EVENT_NAME);
@@ -234,16 +232,16 @@ impl OnChainServiceContract {
 
 impl ServiceContract for OnChainServiceContract {
 	fn update(&self) -> bool {
-		// TODO [Sec]: registry_address currently reads from BlockId::Latest, instead of
-		// from block with REQUEST_CONFIRMATIONS_REQUIRED confirmations
 		if let &ContractAddress::Registry = &self.address {
 			if let Some(client) = self.client.get() {
-				// update contract address from registry
+				if let Some(block_hash) = get_confirmed_block_hash(&*client, REQUEST_CONFIRMATIONS_REQUIRED) {
-				let service_contract_addr = client.registry_address(self.name.clone(), BlockId::Latest).unwrap_or_default();
+					// update contract address from registry
-				if self.data.read().contract_address != service_contract_addr {
+					let service_contract_addr = client.registry_address(self.name.clone(), BlockId::Hash(block_hash)).unwrap_or_default();
-					trace!(target: "secretstore", "{}: installing {} service contract from address {}",
+					if self.data.read().contract_address != service_contract_addr {
-						self.self_key_pair.public(), self.name, service_contract_addr);
+						trace!(target: "secretstore", "{}: installing {} service contract from address {}",
-					self.data.write().contract_address = service_contract_addr;
+							   self.self_key_pair.public(), self.name, service_contract_addr);
+						self.data.write().contract_address = service_contract_addr;
+					}
 				}
 			}
 		}
@@ -460,13 +458,6 @@ pub fn mask_topics(mask: &ApiMask) -> Vec<H256> {
 	topics
 }
 
-/// Get hash of the last block with at least n confirmations.
-fn get_confirmed_block_hash(client: &Client, confirmations: u64) -> Option<H256> {
-	client.block_number(BlockId::Latest)
-		.map(|b| b.saturating_sub(confirmations))
-		.and_then(|b| client.block_hash(BlockId::Number(b)))
-}
-
 impl ServerKeyGenerationService {
 	/// Parse request log entry.
 	pub fn parse_log(origin: &Address, contract: &service::Service, raw_log: RawLog) -> Result<ServiceTask, String> {