From ec652ddc373b3cf1b0978a8a73b958a394c6244f Mon Sep 17 00:00:00 2001 From: "Demi M. Obenour" Date: Thu, 14 Feb 2019 19:29:58 -0500 Subject: [PATCH 1/2] Queue malice reports This is a squash of 18 commits: Queue failed reports for later insertion into blocks If we fail to send a transaction that reports a validator as malicious, store it on a stack. When we next seal a block, included the failed transactions. Insert our reports every time we prepare a block Implement queued reporting Prioritize the `emitInitiateChange` call Fix warnings These warnings cluttered build output, and made it hard to see actual warnings in new code. Implement (partial) cache purging. Once an entry that cannot be purged is found, further entries are skipped. Finish report queuing Use the correct reporting address Drop reports beyond MAX_QUEUED_REPORTS This helps prevent denial-of-service attacks. Drop queued report limits We now only keep 500 reports that have not been confirmed, or 100 reports that have been. Lower limits on queued reports Add a script to generate ABI files and use it to fix compilation error due to incomplete ABI json. Remove old validator reports Reports over 100 blocks old are useless anyway. Fix an unused variable warning in the test client Show a better error when the validator set contract is bad Fix backwards comparison between bad and current block numbers Drop reports for block numbers ahead of the current one Fix review comments from Andreas Remove a commented-out constant, and use the standard library `VecDeque::retain()` method instead of (inefficiently) reimplementing it ourselves. --- ethcore/light/src/client/mod.rs | 6 + ethcore/res/contracts/validator_set_aura.json | 25 +++- .../res/contracts/validator_set_aura.json.txt | 7 ++ ethcore/src/client/client.rs | 4 +- ethcore/src/client/test_client.rs | 10 +- ethcore/src/client/traits.rs | 14 ++- ethcore/src/engines/authority_round/mod.rs | 7 +- ethcore/src/engines/validator_set/contract.rs | 32 +++-- ethcore/src/engines/validator_set/mod.rs | 6 +- ethcore/src/engines/validator_set/multi.rs | 9 +- .../engines/validator_set/safe_contract.rs | 111 +++++++++++++++++- ethcore/types/src/views/view_rlp.rs | 1 + scripts/get-abi | 35 ++++++ 13 files changed, 233 insertions(+), 34 deletions(-) create mode 100644 ethcore/res/contracts/validator_set_aura.json.txt create mode 100755 scripts/get-abi diff --git a/ethcore/light/src/client/mod.rs b/ethcore/light/src/client/mod.rs index 8205ae2ab3b..623d82ac183 100644 --- a/ethcore/light/src/client/mod.rs +++ b/ethcore/light/src/client/mod.rs @@ -619,6 +619,12 @@ impl ::ethcore::client::ChainInfo for Client { } } +impl ::ethcore::client::Nonce for Client { + fn nonce(&self, _address: ðereum_types::H160, _blockid: ethcore::client::BlockId) -> std::option::Option { + panic!("we never call this function on a light client, so this is unreachable; qed") + } +} + impl ::ethcore::client::EngineClient for Client { fn update_sealing(&self) { } fn submit_seal(&self, _block_hash: H256, _seal: Vec>) { } diff --git a/ethcore/res/contracts/validator_set_aura.json b/ethcore/res/contracts/validator_set_aura.json index a42eee20a6c..3b678bff0e0 100644 --- a/ethcore/res/contracts/validator_set_aura.json +++ b/ethcore/res/contracts/validator_set_aura.json @@ -61,5 +61,28 @@ ], "name": "InitiateChange", "type": "event" + }, + { + "constant": true, + "inputs": [ + { + "name": "_maliciousMiningAddress", + "type": "address" + }, + { + "name": "_blockNumber", + "type": "uint256" + } + ], + "name": "maliceReportedForBlock", + "outputs": [ + { + "name": "", + "type": "address[]" + } + ], + "payable": false, + "stateMutability": "view", + "type": "function" } -] +] \ No newline at end of file diff --git a/ethcore/res/contracts/validator_set_aura.json.txt b/ethcore/res/contracts/validator_set_aura.json.txt new file mode 100644 index 00000000000..f912cf10e39 --- /dev/null +++ b/ethcore/res/contracts/validator_set_aura.json.txt @@ -0,0 +1,7 @@ +getValidators +initiateChange +emitInitiateChangeCallable +emitInitiateChange +maliceReportedForBlock +finalizeChange +InitiateChange diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 46343ee986f..212300a4a79 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -2187,10 +2187,10 @@ impl BlockChainClient for Client { Ok(SignedTransaction::new(transaction.with_signature(signature, chain_id))?) } - fn transact(&self, action: Action, data: Bytes, gas: Option, gas_price: Option) + fn transact(&self, action: Action, data: Bytes, gas: Option, gas_price: Option, nonce: Option) -> Result<(), transaction::Error> { - let signed = self.create_transaction(action, data, gas, gas_price, None)?; + let signed = self.create_transaction(action, data, gas, gas_price, nonce)?; self.importer.miner.import_own_transaction(self, signed.into()) } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 23695a62e58..77fd3d77606 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -896,8 +896,14 @@ impl BlockChainClient for TestBlockChainClient { Ok(SignedTransaction::new(transaction.with_signature(sig, chain_id)).unwrap()) } - fn transact(&self, action: Action, data: Bytes, gas: Option, gas_price: Option) - -> Result<(), transaction::Error> + fn transact( + &self, + action: Action, + data: Bytes, + gas: Option, + gas_price: Option, + _nonce: Option, + ) -> Result<(), transaction::Error> { let signed = self.create_transaction(action, data, gas, gas_price, None)?; self.miner.import_own_transaction(self, signed.into()) diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index b5916fa9835..db4b11accb3 100644 --- a/ethcore/src/client/traits.rs +++ b/ethcore/src/client/traits.rs @@ -376,11 +376,12 @@ pub trait BlockChainClient : Sync + Send + AccountData + BlockChain + CallContra fn pruning_info(&self) -> PruningInfo; /// Schedule state-altering transaction to be executed on the next pending block. - fn transact_contract(&self, address: Address, data: Bytes) -> Result<(), transaction::Error> { - self.transact(Action::Call(address), data, None, None) + fn transact_contract(&self, address: Address, data: Bytes, nonce: Option) -> Result<(), transaction::Error> { + self.transact(Action::Call(address), data, None, None, nonce) } - /// Returns a signed transaction. If gas limit, gas price, or nonce are not specified, the defaults are used. + /// Returns a signed transaction. If gas limit, gas price, or nonce are not + /// specified, the defaults are used. fn create_transaction( &self, action: Action, @@ -390,10 +391,11 @@ pub trait BlockChainClient : Sync + Send + AccountData + BlockChain + CallContra nonce: Option ) -> Result; - /// Schedule state-altering transaction to be executed on the next pending block with the given gas parameters. + /// Schedule state-altering transaction to be executed on the next pending + /// block with the given gas and nonce parameters. /// /// If they are `None`, sensible values are selected automatically. - fn transact(&self, action: Action, data: Bytes, gas: Option, gas_price: Option) + fn transact(&self, action: Action, data: Bytes, gas: Option, gas_price: Option, nonce: Option) -> Result<(), transaction::Error>; /// Get the address of the registry itself. @@ -441,7 +443,7 @@ pub trait BroadcastProposalBlock { pub trait SealedBlockImporter: ImportSealedBlock + BroadcastProposalBlock {} /// Client facilities used by internally sealing Engines. -pub trait EngineClient: Sync + Send + ChainInfo { +pub trait EngineClient: Sync + Send + ChainInfo + Nonce { /// Make a new block and seal it. fn update_sealing(&self); diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index 4409608a723..61e8198722d 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -1226,7 +1226,12 @@ impl Engine for AuthorityRound { }, }; - block_reward::apply_block_rewards(&rewards, block, &self.machine) + block_reward::apply_block_rewards(&rewards, block, &self.machine)?; + + match self.signer.read().as_ref() { + Some(signer) => self.validators.on_close_block(block.header(), &signer.address()), + None => Ok(()), // We are not a validator, so we can't report malicious validators. + } } /// Make calls to the randomness and validator set contracts. diff --git a/ethcore/src/engines/validator_set/contract.rs b/ethcore/src/engines/validator_set/contract.rs index 9defe203f96..5d71f473209 100644 --- a/ethcore/src/engines/validator_set/contract.rs +++ b/ethcore/src/engines/validator_set/contract.rs @@ -25,7 +25,7 @@ use machine::{AuxiliaryData, Call, EthereumMachine}; use parking_lot::RwLock; use types::BlockNumber; use types::header::Header; -use types::transaction::Action; +use types::transaction::{self, Action}; use client::EngineClient; @@ -38,7 +38,7 @@ use_contract!(validator_report, "res/contracts/validator_report.json"); pub struct ValidatorContract { contract_address: Address, validators: ValidatorSafeContract, - client: RwLock>>, // TODO [keorn]: remove + client: RwLock>>, // TODO [keorn]: remove } impl ValidatorContract { @@ -52,16 +52,17 @@ impl ValidatorContract { } impl ValidatorContract { - fn transact(&self, data: Bytes) -> Result<(), String> { + fn transact(&self, data: Bytes) -> Result<(), ::error::Error> { let client = self.client.read().as_ref() .and_then(Weak::upgrade) .ok_or_else(|| "No client!")?; match client.as_full_client() { Some(c) => { - c.transact(Action::Call(self.contract_address), data, None, Some(0.into())) - .map_err(|e| format!("Transaction import error: {}", e))?; - Ok(()) + match c.transact(Action::Call(self.contract_address), data, None, Some(0.into()), None) { + Ok(()) | Err(transaction::Error::AlreadyImported) => Ok(()), + Err(e) => Err(e)?, + } }, None => Err("No full client!".into()), } @@ -84,6 +85,10 @@ impl ValidatorSet for ValidatorContract { self.validators.on_epoch_begin(first, header, call) } + fn on_close_block(&self, header: &Header, address: &Address) -> Result<(), ::error::Error> { + self.validators.on_close_block(header, address) + } + fn genesis_epoch_data(&self, header: &Header, call: &Call) -> Result, String> { self.validators.genesis_epoch_data(header, call) } @@ -117,18 +122,21 @@ impl ValidatorSet for ValidatorContract { self.validators.count_with_caller(bh, caller) } - fn report_malicious(&self, address: &Address, _set_block: BlockNumber, block: BlockNumber, proof: Bytes) { + fn report_malicious(&self, address: &Address, set_block: BlockNumber, block: BlockNumber, proof: Bytes) { let data = validator_report::functions::report_malicious::encode_input(*address, block, proof); - match self.transact(data) { - Ok(_) => warn!(target: "engine", "Reported malicious validator {}", address), - Err(s) => warn!(target: "engine", "Validator {} could not be reported {}", address, s), + match self.transact(data.clone()) { + Ok(()) => warn!(target: "engine", "Reported malicious validator {} at block {}", address, set_block), + Err(s) => { + warn!(target: "engine", "Validator {} could not be reported ({}) on block {}", address, s, set_block); + self.validators.queue_report((*address, set_block, data)) + } } } fn report_benign(&self, address: &Address, _set_block: BlockNumber, block: BlockNumber) { let data = validator_report::functions::report_benign::encode_input(*address, block); match self.transact(data) { - Ok(_) => warn!(target: "engine", "Reported benign validator misbehaviour {}", address), + Ok(()) => warn!(target: "engine", "Reported benign validator misbehaviour {}", address), Err(s) => warn!(target: "engine", "Validator {} could not be reported {}", address, s), } } @@ -223,7 +231,7 @@ mod tests { assert_eq!(client.chain_info().best_block_number, 2); // Check if misbehaving validator was removed. - client.transact_contract(Default::default(), Default::default()).unwrap(); + client.transact_contract(Default::default(), Default::default(), Default::default()).unwrap(); client.engine().step(); client.engine().step(); assert_eq!(client.chain_info().best_block_number, 2); diff --git a/ethcore/src/engines/validator_set/mod.rs b/ethcore/src/engines/validator_set/mod.rs index 45a7d9ded67..bef39592601 100644 --- a/ethcore/src/engines/validator_set/mod.rs +++ b/ethcore/src/engines/validator_set/mod.rs @@ -92,7 +92,11 @@ pub trait ValidatorSet: Send + Sync + 'static { Ok(()) } - #[cfg(all())] + /// Called on the close of every block. + fn on_close_block(&self, _header: &Header, _address: &Address) -> Result<(), ::error::Error> { + Ok(()) + } + /// Called for each new block this node is creating. If this block is /// the first block of an epoch, this is called *after* on_epoch_begin(), /// but with the same parameters. diff --git a/ethcore/src/engines/validator_set/multi.rs b/ethcore/src/engines/validator_set/multi.rs index 213b504d24c..bb250359947 100644 --- a/ethcore/src/engines/validator_set/multi.rs +++ b/ethcore/src/engines/validator_set/multi.rs @@ -91,6 +91,9 @@ impl ValidatorSet for Multi { self.map_children(header, &mut |set: &dyn ValidatorSet, first| set.on_prepare_block(first, header, call)) } + fn on_close_block(&self, header: &Header, address: &Address) -> Result<(), ::error::Error> { + self.map_children(header, &mut |set: &dyn ValidatorSet, _first| set.on_close_block(header, address)) + } fn on_epoch_begin(&self, _first: bool, header: &Header, call: &mut SystemCall) -> Result<(), ::error::Error> { self.map_children(header, &mut |set: &dyn ValidatorSet, first| set.on_epoch_begin(first, header, call)) @@ -192,7 +195,7 @@ mod tests { // Wrong signer for the first block. let signer = Box::new((tap.clone(), v1, "".into())); client.miner().set_author(miner::Author::Sealer(signer)); - client.transact_contract(Default::default(), Default::default()).unwrap(); + client.transact_contract(Default::default(), Default::default(), Default::default()).unwrap(); ::client::EngineClient::update_sealing(&*client); assert_eq!(client.chain_info().best_block_number, 0); // Right signer for the first block. @@ -201,7 +204,7 @@ mod tests { ::client::EngineClient::update_sealing(&*client); assert_eq!(client.chain_info().best_block_number, 1); // This time v0 is wrong. - client.transact_contract(Default::default(), Default::default()).unwrap(); + client.transact_contract(Default::default(), Default::default(), Default::default()).unwrap(); ::client::EngineClient::update_sealing(&*client); assert_eq!(client.chain_info().best_block_number, 1); let signer = Box::new((tap.clone(), v1, "".into())); @@ -209,7 +212,7 @@ mod tests { ::client::EngineClient::update_sealing(&*client); assert_eq!(client.chain_info().best_block_number, 2); // v1 is still good. - client.transact_contract(Default::default(), Default::default()).unwrap(); + client.transact_contract(Default::default(), Default::default(), Default::default()).unwrap(); ::client::EngineClient::update_sealing(&*client); assert_eq!(client.chain_info().best_block_number, 3); diff --git a/ethcore/src/engines/validator_set/safe_contract.rs b/ethcore/src/engines/validator_set/safe_contract.rs index c4cc142a245..41e92704080 100644 --- a/ethcore/src/engines/validator_set/safe_contract.rs +++ b/ethcore/src/engines/validator_set/safe_contract.rs @@ -16,6 +16,7 @@ /// Validator set maintained in a contract, updated using `getValidators` method. +use std::collections::VecDeque; use std::sync::{Weak, Arc}; use bytes::Bytes; @@ -24,12 +25,13 @@ use ethereum_types::{H256, U256, Address, Bloom}; use hash::keccak; use kvdb::DBValue; use memory_cache::MemoryLruCache; -use parking_lot::RwLock; +use parking_lot::{RwLock, Mutex}; use rlp::{Rlp, RlpStream}; use types::header::Header; use types::ids::BlockId; use types::log_entry::LogEntry; use types::receipt::Receipt; +use types::transaction::{self, Action}; use unexpected::Mismatch; use client::EngineClient; @@ -39,6 +41,9 @@ use super::simple_list::SimpleList; use_contract!(validator_set, "res/contracts/validator_set_aura.json"); +/// The maximum number of reports to keep queued. +const MAX_QUEUED_REPORTS: usize = 10; + const MEMOIZE_CAPACITY: usize = 500; // TODO: ethabi should be able to generate this. @@ -76,6 +81,7 @@ pub struct ValidatorSafeContract { contract_address: Address, validators: RwLock>, client: RwLock>>, // TODO [keorn]: remove + queued_reports: Mutex)>>, } // first proof is just a state proof call of `getValidators` at header's state. @@ -191,11 +197,34 @@ fn prove_initial(contract_address: Address, header: &Header, caller: &Call) -> R } impl ValidatorSafeContract { + fn transact(&self, data: Bytes, nonce: U256) -> Result<(), ::error::Error> { + let client = self.client.read().as_ref() + .and_then(Weak::upgrade) + .ok_or_else(|| "No client!")?; + + match client.as_full_client() { + Some(c) => { + match c.transact(Action::Call(self.contract_address), data, None, Some(0.into()), Some(nonce)) { + Ok(()) | Err(transaction::Error::AlreadyImported) => Ok(()), + Err(e) => Err(e)?, + } + }, + None => Err("No full client!".into()), + } + } + + pub(crate) fn queue_report(&self, data: (Address, u64, Vec)) { + self.queued_reports + .lock() + .push_back(data) + } + pub fn new(contract_address: Address) -> Self { ValidatorSafeContract { contract_address, validators: RwLock::new(MemoryLruCache::new(MEMOIZE_CAPACITY)), client: RwLock::new(None), + queued_reports: Mutex::new(VecDeque::new()), } } @@ -295,17 +324,87 @@ impl ValidatorSet for ValidatorSafeContract { -> Result, ::error::Error> { let (data, decoder) = validator_set::functions::emit_initiate_change_callable::call(); - if !caller(self.contract_address, data) + let mut returned_transactions = if !caller(self.contract_address, data) .and_then(|x| decoder.decode(&x) .map_err(|x| format!("chain spec bug: could not decode: {:?}", x))) .map_err(::engines::EngineError::FailedSystemCall)? { trace!(target: "engine", "New block #{} issued ― no need to call emitInitiateChange()", header.number()); - return Ok(Vec::new()); + Vec::new() + } else { + trace!(target: "engine", "New block issued #{} ― calling emitInitiateChange()", header.number()); + let (data, _decoder) = validator_set::functions::emit_initiate_change::call(); + vec![(self.contract_address, data)] + }; + let queued_reports = self.queued_reports.lock(); + for (_address, _block, data) in queued_reports.iter().take(10) { + returned_transactions.push((self.contract_address, data.clone())) } + Ok(returned_transactions) + } - trace!(target: "engine", "New block issued #{} ― calling emitInitiateChange()", header.number()); - let (data, _decoder) = validator_set::functions::emit_initiate_change::call(); - Ok(vec![(self.contract_address, data)]) + fn on_close_block(&self, header: &Header, address: &Address) -> Result<(), ::error::Error> { + let client = self.client.read() + .as_ref() + .and_then(Weak::upgrade) + .ok_or_else(|| ::error::Error::from("No client!"))?; + let client = client + .as_full_client() + .ok_or_else(|| ::error::Error::from("No full client!"))?; + let nonce = client + .nonce(&self.contract_address, BlockId::Latest) + .ok_or_else(||"No nonce!")?; + let mut i = 0u64; + debug!(target: "engine", "Checking for cached reports"); + for (address, block, data) in self.queued_reports.lock().iter() { + debug!(target: "engine", "Trying to report"); + loop { + match self.transact(data.clone(), nonce + U256::from(i)) { + Ok(()) => break, + Err(err) => { + let msg = format!("{}", err); + if msg != "Transaction import error: Transaction error (No longer valid)" { + warn!(target: "engine", "Cannot report validator {} for misbehavior on block {}: {}", address, block, err); + break + } + } + } + i += 1 + }; + i += 1 + } + let mut queue = self.queued_reports.lock(); + queue.retain(|&(malicious_validator_address, block, ref _data)| { + debug!(target: "engine", "Checking if report can be removed from cache"); + let result = { + let current_block_number = header.number(); + if block > current_block_number { + return false // Report cannot be used, as it is for a block that isn’t in the current chain + } + if current_block_number > 100 && current_block_number - 100 > block { + return false // Report is too old and cannot be used + } + let (data, decoder) = validator_set::functions::malice_reported_for_block::call( + malicious_validator_address, block); + let result = client.call_contract(BlockId::Latest, self.contract_address, data) + .expect("this is a bug in the genesis block; either the validator set contract is missing, or it is invalid; this is a fatal error in the configuration of Parity, so we cannot recover; qed"); + decoder.decode(&result[..]) + .expect("this is a bug in the genesis block; either the validator set contract is missing, or it is invalid; this is a fatal error in the configuration of Parity, so we cannot recover; qed") + }; + debug!(target: "engine", "Got data from contract: {:?}", result); + if result.contains(&address) { + debug!(target: "engine", "Successfully removed report from report cache"); + return false + } + return true + }); + + while queue.len() > MAX_QUEUED_REPORTS { + warn!(target: "engine", "Removing report from report cache, even though it has not \ + been finalized"); + drop(queue.pop_front()) + } + + Ok(()) } fn on_epoch_begin(&self, _first: bool, _header: &Header, caller: &mut SystemCall) -> Result<(), ::error::Error> { diff --git a/ethcore/types/src/views/view_rlp.rs b/ethcore/types/src/views/view_rlp.rs index a6c789de989..9e668845e1d 100644 --- a/ethcore/types/src/views/view_rlp.rs +++ b/ethcore/types/src/views/view_rlp.rs @@ -127,6 +127,7 @@ impl<'a, 'view> Iterator for ViewRlpIterator<'a, 'view> { } } +/// Create a `ViewRlp` #[macro_export] macro_rules! view { ($view: ident, $bytes: expr) => { diff --git a/scripts/get-abi b/scripts/get-abi new file mode 100755 index 00000000000..16894e2f680 --- /dev/null +++ b/scripts/get-abi @@ -0,0 +1,35 @@ +#!/usr/bin/python3 -- +""" +Get the ABI of a contract JSON file (specified in ``sys.argv[1]``) and write it +to ``sys.argv[2]``. Both ``sys.argv[1]`` and ``sys.argv[2]`` must end in +.json. Additionally, a file named ``sys.argv[2] + '.txt'`` must exist. It +must have one line for each symbol in the ABI that will be placed in the output. + +The input file must be encoded in UTF-8, and the output file will be encoded in +UTF-8. +""" + +__version__ = '0.0.1' +__all__ = ['main'] +import re +_re = re.compile(r'\A[A-Za-z_][A-Za-z0-9_]*\n\Z') +def main(input_file, output_file, used_file): + import os, json + _type, _dict, _open, _set = type, dict, open, set + with _open(used_file, 'r', encoding='UTF-8') as used_stream: + used_functions = _set(i.strip() for i in used_stream if _re.match(i)) + with _open(input_file, 'r', encoding='UTF-8') as input_stream, \ + _open(output_file, 'w', encoding='UTF-8') as output_stream: + my_list = [i for i in json.load(input_stream)['abi'] + if _type(i) is _dict and i.get('name') in used_functions] + json.dump(my_list, output_stream, sort_keys=True, indent = '\t') +if __name__ == '__main__': + import sys, json + if len(sys.argv) != 3: + print('must have 2 arguments, not {}'.format(len(sys.argv) - 1), + file=sys.stderr) + sys.exit(1) + try: + main(sys.argv[1], sys.argv[2], sys.argv[2] + '.txt') + except (OSError, UnicodeError, json.decoder.JSONDecodeError) as e: + print('Failed to generate ABI file: {}'.format(e), file=sys.stderr) From e4cfd7d3e25817da1ef32942abe135a4ce828b73 Mon Sep 17 00:00:00 2001 From: Andreas Fackler Date: Thu, 25 Apr 2019 10:58:30 +0200 Subject: [PATCH 2/2] Queue reports even if transaction creation was successful. Having created a transaction doesn't guarantee that it will be included in a block. We should queue the transaction anyway. Also remove some unnecessary closures and match statements. --- ethcore/src/engines/validator_set/contract.rs | 22 +++++++------------ .../engines/validator_set/safe_contract.rs | 16 +++++--------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/ethcore/src/engines/validator_set/contract.rs b/ethcore/src/engines/validator_set/contract.rs index 5d71f473209..2045bd760f0 100644 --- a/ethcore/src/engines/validator_set/contract.rs +++ b/ethcore/src/engines/validator_set/contract.rs @@ -53,18 +53,12 @@ impl ValidatorContract { impl ValidatorContract { fn transact(&self, data: Bytes) -> Result<(), ::error::Error> { - let client = self.client.read().as_ref() - .and_then(Weak::upgrade) - .ok_or_else(|| "No client!")?; - - match client.as_full_client() { - Some(c) => { - match c.transact(Action::Call(self.contract_address), data, None, Some(0.into()), None) { - Ok(()) | Err(transaction::Error::AlreadyImported) => Ok(()), - Err(e) => Err(e)?, - } - }, - None => Err("No full client!".into()), + let client = self.client.read().as_ref().and_then(Weak::upgrade).ok_or("No client!")?; + let full_client = client.as_full_client().ok_or("No full client!")?; + + match full_client.transact(Action::Call(self.contract_address), data, None, Some(0.into()), None) { + Ok(()) | Err(transaction::Error::AlreadyImported) => Ok(()), + Err(e) => Err(e)?, } } } @@ -124,11 +118,11 @@ impl ValidatorSet for ValidatorContract { fn report_malicious(&self, address: &Address, set_block: BlockNumber, block: BlockNumber, proof: Bytes) { let data = validator_report::functions::report_malicious::encode_input(*address, block, proof); - match self.transact(data.clone()) { + self.validators.queue_report((*address, set_block, data.clone())); + match self.transact(data) { Ok(()) => warn!(target: "engine", "Reported malicious validator {} at block {}", address, set_block), Err(s) => { warn!(target: "engine", "Validator {} could not be reported ({}) on block {}", address, s, set_block); - self.validators.queue_report((*address, set_block, data)) } } } diff --git a/ethcore/src/engines/validator_set/safe_contract.rs b/ethcore/src/engines/validator_set/safe_contract.rs index 41e92704080..9358b4929e1 100644 --- a/ethcore/src/engines/validator_set/safe_contract.rs +++ b/ethcore/src/engines/validator_set/safe_contract.rs @@ -198,18 +198,12 @@ fn prove_initial(contract_address: Address, header: &Header, caller: &Call) -> R impl ValidatorSafeContract { fn transact(&self, data: Bytes, nonce: U256) -> Result<(), ::error::Error> { - let client = self.client.read().as_ref() - .and_then(Weak::upgrade) - .ok_or_else(|| "No client!")?; + let client = self.client.read().as_ref().and_then(Weak::upgrade).ok_or("No client!")?; + let full_client = client.as_full_client().ok_or("No full client!")?; - match client.as_full_client() { - Some(c) => { - match c.transact(Action::Call(self.contract_address), data, None, Some(0.into()), Some(nonce)) { - Ok(()) | Err(transaction::Error::AlreadyImported) => Ok(()), - Err(e) => Err(e)?, - } - }, - None => Err("No full client!".into()), + match full_client.transact(Action::Call(self.contract_address), data, None, Some(0.into()), Some(nonce)) { + Ok(()) | Err(transaction::Error::AlreadyImported) => Ok(()), + Err(e) => Err(e)?, } }