From d7a7f034db6e7d84e2182ab2cd76cfc0a438723c Mon Sep 17 00:00:00 2001 From: Wei Tang Date: Fri, 6 Apr 2018 18:03:13 +0800 Subject: [PATCH] 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 | 29 +++++++++++++++++++ secret_store/src/key_server_set.rs | 3 +- secret_store/src/lib.rs | 1 + secret_store/src/listener/service_contract.rs | 27 ++++++----------- 4 files changed, 41 insertions(+), 19 deletions(-) create mode 100644 secret_store/src/helpers.rs diff --git a/secret_store/src/helpers.rs b/secret_store/src/helpers.rs new file mode 100644 index 000000000..3bc49116e --- /dev/null +++ b/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 . + +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 { + client.block_number(BlockId::Latest) + .map(|b| b.saturating_sub(confirmations)) + .and_then(|b| client.block_hash(BlockId::Number(b))) +} diff --git a/secret_store/src/key_server_set.rs b/secret_store/src/key_server_set.rs index f069368b0..899709a7d 100644 --- a/secret_store/src/key_server_set.rs +++ 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, retracted: Vec) { // 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() { diff --git a/secret_store/src/lib.rs b/secret_store/src/lib.rs index e796ff4bc..f08a26a90 100644 --- a/secret_store/src/lib.rs +++ b/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; diff --git a/secret_store/src/listener/service_contract.rs b/secret_store/src/listener/service_contract.rs index 7bb28ae05..4d6ac14c8 100644 --- a/secret_store/src/listener/service_contract.rs +++ b/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 - let service_contract_addr = client.registry_address(self.name.clone(), BlockId::Latest).unwrap_or_default(); - if self.data.read().contract_address != service_contract_addr { - trace!(target: "secretstore", "{}: installing {} service contract from address {}", - self.self_key_pair.public(), self.name, service_contract_addr); - self.data.write().contract_address = service_contract_addr; + if let Some(block_hash) = get_confirmed_block_hash(&*client, REQUEST_CONFIRMATIONS_REQUIRED) { + // update contract address from registry + let service_contract_addr = client.registry_address(self.name.clone(), BlockId::Hash(block_hash)).unwrap_or_default(); + if self.data.read().contract_address != service_contract_addr { + trace!(target: "secretstore", "{}: installing {} service contract from address {}", + 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 { topics } -/// Get hash of the last block with at least n confirmations. -fn get_confirmed_block_hash(client: &Client, confirmations: u64) -> Option { - 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 {