Add POSDAO transition and malice report queue. (openethereum#11245)

* Add POSDAO transition; call emitInitiateChange.

* Retry failed malice reports.

* Make malice reports with zero gas price.

* Address review comments.

* Extract ReportQueue from ValidatorSafeContract.

* Add shouldValidatorReport to validator set contract.

* Rename queue_report to enqueue_report

* Increment nonce between randomness and POSDAO transactions.

* Refactor the test ValidatorSet contract

* Address review comments, docs

* Update ethcore/res/validator_contract.sol

Co-Authored-By: David <dvdplm@gmail.com>

* Update ethcore/res/validator_contract.sol

Co-Authored-By: David <dvdplm@gmail.com>

Co-authored-by: varasev <33550681+varasev@users.noreply.github.com>
Co-authored-by: David <dvdplm@gmail.com>

ethcore/client-traits/src/lib.rs

@@ -444,9 +444,9 @@ impl TransactionRequest {
 		self
 	}
 
-	/// Sets a gas price. If this is not specified, a sensible default is used.
-	pub fn gas_price(mut self, gas_price: U256) -> TransactionRequest {
-		self.gas_price = Some(gas_price);
+	/// Sets a gas price. If this is not specified or `None`, a sensible default is used.
+	pub fn gas_price<T: Into<Option<U256>>>(mut self, gas_price: T) -> TransactionRequest {
+		self.gas_price = gas_price.into();
 		self
 	}
 

ethcore/engines/authority-round/src/lib.rs

@@ -60,6 +60,7 @@ use itertools::{self, Itertools};
 use rand::rngs::OsRng;
 use rlp::{encode, Decodable, DecoderError, Encodable, RlpStream, Rlp};
 use ethereum_types::{H256, H520, Address, U128, U256};
+use parity_bytes::Bytes;
 use parking_lot::{Mutex, RwLock};
 use time_utils::CheckedSystemTime;
 use common_types::{
@@ -80,7 +81,7 @@ use common_types::{
 	transaction::SignedTransaction,
 };
 use unexpected::{Mismatch, OutOfBounds};
-use validator_set::{ValidatorSet, SimpleList, new_validator_set};
+use validator_set::{ValidatorSet, SimpleList, new_validator_set_posdao};
 
 mod finality;
 mod randomness;
@@ -128,6 +129,9 @@ pub struct AuthorityRoundParams {
 	/// The addresses of contracts that determine the block gas limit with their associated block
 	/// numbers.
 	pub block_gas_limit_contract_transitions: BTreeMap<u64, Address>,
+	/// If set, this is the block number at which the consensus engine switches from AuRa to AuRa
+	/// with POSDAO modifications.
+	pub posdao_transition: Option<BlockNumber>,
 }
 
 const U16_MAX: usize = ::std::u16::MAX as usize;
@@ -195,7 +199,7 @@ impl From<ethjson::spec::AuthorityRoundParams> for AuthorityRoundParams {
 			.collect();
 		AuthorityRoundParams {
 			step_durations,
-			validators: new_validator_set(p.validators),
+			validators: new_validator_set_posdao(p.validators, p.posdao_transition.map(Into::into)),
 			start_step: p.start_step.map(Into::into),
 			validate_score_transition: p.validate_score_transition.map_or(0, Into::into),
 			validate_step_transition: p.validate_step_transition.map_or(0, Into::into),
@@ -210,6 +214,7 @@ impl From<ethjson::spec::AuthorityRoundParams> for AuthorityRoundParams {
 			strict_empty_steps_transition: p.strict_empty_steps_transition.map_or(0, Into::into),
 			randomness_contract_address,
 			block_gas_limit_contract_transitions,
+			posdao_transition: p.posdao_transition.map(Into::into),
 		}
 	}
 }
@@ -598,6 +603,10 @@ pub struct AuthorityRound {
 	block_gas_limit_contract_transitions: BTreeMap<u64, Address>,
 	/// Memoized gas limit overrides, by block hash.
 	gas_limit_override_cache: Mutex<LruCache<H256, Option<U256>>>,
+	/// The block number at which the consensus engine switches from AuRa to AuRa with POSDAO
+	/// modifications. For details about POSDAO, see the whitepaper:
+	/// https://www.xdaichain.com/for-validators/posdao-whitepaper
+	posdao_transition: Option<BlockNumber>,
 }
 
 // header-chain validator.
@@ -893,6 +902,7 @@ impl AuthorityRound {
 				randomness_contract_address: our_params.randomness_contract_address,
 				block_gas_limit_contract_transitions: our_params.block_gas_limit_contract_transitions,
 				gas_limit_override_cache: Mutex::new(LruCache::new(GAS_LIMIT_OVERRIDE_CACHE_CAPACITY)),
+				posdao_transition: our_params.posdao_transition,
 			});
 
 		// Do not initialize timeouts for tests.
@@ -1131,6 +1141,53 @@ impl AuthorityRound {
 			EngineError::RequiresClient
 		})
 	}
+
+	fn run_posdao(&self, block: &ExecutedBlock, nonce: Option<U256>) -> Result<Vec<SignedTransaction>, Error> {
+		// Skip the rest of the function unless there has been a transition to POSDAO AuRa.
+		if self.posdao_transition.map_or(true, |posdao_block| block.header.number() < posdao_block) {
+			trace!(target: "engine", "Skipping POSDAO calls to validator set contracts");
+			return Ok(Vec::new());
+		}
+
+		let opt_signer = self.signer.read();
+		let signer = match opt_signer.as_ref() {
+			Some(signer) => signer,
+			None => return Ok(Vec::new()), // We are not a validator, so we shouldn't call the contracts.
+		};
+		let our_addr = signer.address();
+		let client = self.upgrade_client_or("Unable to prepare block")?;
+		let full_client = client.as_full_client().ok_or_else(|| {
+			EngineError::FailedSystemCall("Failed to upgrade to BlockchainClient.".to_string())
+		})?;
+
+		// Makes a constant contract call.
+		let mut call = |to: Address, data: Bytes| {
+			full_client.call_contract(BlockId::Latest, to, data).map_err(|e| format!("{}", e))
+		};
+
+		// Our current account nonce. The transactions must have consecutive nonces, starting with this one.
+		let mut tx_nonce = if let Some(tx_nonce) = nonce {
+			tx_nonce
+		} else {
+			block.state.nonce(&our_addr)?
+		};
+		let mut transactions = Vec::new();
+
+		// Creates and signs a transaction with the given contract call.
+		let mut make_transaction = |to: Address, data: Bytes| -> Result<SignedTransaction, Error> {
+			let tx_request = TransactionRequest::call(to, data).gas_price(U256::zero()).nonce(tx_nonce);
+			tx_nonce += U256::one(); // Increment the nonce for the next transaction.
+			Ok(full_client.create_transaction(tx_request)?)
+		};
+
+		// Genesis is never a new block, but might as well check.
+		let first = block.header.number() == 0;
+		for (addr, data) in self.validators.generate_engine_transactions(first, &block.header, &mut call)? {
+			transactions.push(make_transaction(addr, data)?);
+		}
+
+		Ok(transactions)
+	}
 }
 
 fn unix_now() -> Duration {
@@ -1519,7 +1576,10 @@ impl Engine for AuthorityRound {
 	}
 
 	fn generate_engine_transactions(&self, block: &ExecutedBlock) -> Result<Vec<SignedTransaction>, Error> {
-		self.run_randomness_phase(block)
+		let mut transactions = self.run_randomness_phase(block)?;
+		let nonce = transactions.last().map(|tx| tx.nonce + U256::one());
+		transactions.extend(self.run_posdao(block, nonce)?);
+		Ok(transactions)
 	}
 
 	/// Check the number of seal fields.
@@ -1976,6 +2036,7 @@ mod tests {
 			two_thirds_majority_transition: 0,
 			randomness_contract_address: BTreeMap::new(),
 			block_gas_limit_contract_transitions: BTreeMap::new(),
+			posdao_transition: Some(0),
 		};
 
 		// mutate aura params

ethcore/engines/validator-set/res/validator_report.json

@@ -1,4 +1,5 @@
 [
 	{"constant":false,"inputs":[{"name":"validator","type":"address"},{"name":"blockNumber","type":"uint256"},{"name":"proof","type":"bytes"}],"name":"reportMalicious","outputs":[],"payable":false,"type":"function"},
-	{"constant":false,"inputs":[{"name":"validator","type":"address"},{"name":"blockNumber","type":"uint256"}],"name":"reportBenign","outputs":[],"payable":false,"type":"function"}
+	{"constant":false,"inputs":[{"name":"validator","type":"address"},{"name":"blockNumber","type":"uint256"}],"name":"reportBenign","outputs":[],"payable":false,"type":"function"},
+	{"constant": true, "inputs": [ { "name": "validator", "type": "address" }, { "name": "blockNum", "type": "uint256" } ], "name": "maliceReportedForBlock", "outputs": [ { "name": "", "type": "address[]" } ], "payable": false, "stateMutability": "view", "type": "function" }
 ]

ethcore/engines/validator-set/res/validator_set.json

@@ -1,5 +1,55 @@
 [
 	{"constant":false,"inputs":[],"name":"finalizeChange","outputs":[],"payable":false,"type":"function"},
 	{"constant":true,"inputs":[],"name":"getValidators","outputs":[{"name":"validators","type":"address[]"}],"payable":false,"type":"function"},
-	{"anonymous":false,"inputs":[{"indexed":true,"name":"_parent_hash","type":"bytes32"},{"indexed":false,"name":"_new_set","type":"address[]"}],"name":"InitiateChange","type":"event"}
+	{"anonymous":false,"inputs":[{"indexed":true,"name":"_parent_hash","type":"bytes32"},{"indexed":false,"name":"_new_set","type":"address[]"}],"name":"InitiateChange","type":"event"},
+	{
+		"constant": true,
+		"inputs": [],
+		"name": "emitInitiateChangeCallable",
+		"outputs": [
+			{
+				"name": "",
+				"type": "bool"
+			}
+		],
+		"payable": false,
+		"stateMutability": "view",
+		"type": "function"
+	},
+	{
+		"constant": false,
+		"inputs": [],
+		"name": "emitInitiateChange",
+		"outputs": [],
+		"payable": false,
+		"stateMutability": "nonpayable",
+		"type": "function"
+	},
+	{
+	  "constant": true,
+	  "inputs": [
+		{
+		  "name": "_reportingValidator",
+		  "type": "address"
+		},
+		{
+		  "name": "_maliciousValidator",
+		  "type": "address"
+		},
+		{
+			"name": "_blockNumber",
+			"type": "uint256"
+		}
+	  ],
+	  "name": "shouldValidatorReport",
+	  "outputs": [
+		{
+		  "name": "",
+		  "type": "bool"
+		}
+	  ],
+	  "payable": false,
+	  "stateMutability": "view",
+	  "type": "function"
+	}
 ]

ethcore/engines/validator-set/src/contract.rs

@@ -21,8 +21,8 @@ use std::sync::Weak;
 
 use parity_bytes::Bytes;
 use ethabi_contract::use_contract;
-use ethereum_types::{H256, Address};
-use log::{warn, trace};
+use ethereum_types::{H256, U256, Address};
+use log::{info, warn, trace};
 use machine::Machine;
 use parking_lot::RwLock;
 use common_types::{
@@ -31,6 +31,7 @@ use common_types::{
 	header::Header,
 	errors::EthcoreError,
 	engines::machine::{Call, AuxiliaryData},
+	transaction,
 };
 
 use client_traits::{EngineClient, TransactionRequest};
@@ -48,31 +49,63 @@ pub struct ValidatorContract {
 	contract_address: Address,
 	validators: ValidatorSafeContract,
 	client: RwLock<Option<Weak<dyn EngineClient>>>, // TODO [keorn]: remove
+	posdao_transition: Option<BlockNumber>,
 }
 
 impl ValidatorContract {
-	pub fn new(contract_address: Address) -> Self {
+	pub fn new(contract_address: Address, posdao_transition: Option<BlockNumber>) -> Self {
 		ValidatorContract {
 			contract_address,
-			validators: ValidatorSafeContract::new(contract_address),
+			validators: ValidatorSafeContract::new(contract_address, posdao_transition),
 			client: RwLock::new(None),
+			posdao_transition,
 		}
 	}
 }
 
 impl ValidatorContract {
-	fn transact(&self, data: Bytes) -> Result<(), String> {
-		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(TransactionRequest::call(self.contract_address, data))
-					.map_err(|e| format!("Transaction import error: {}", e))?;
-				Ok(())
-			},
-			None => Err("No full client!".into()),
+	fn transact(&self, data: Bytes, gas_price: Option<U256>, client: &dyn EngineClient) -> Result<(), String> {
+		let full_client = client.as_full_client().ok_or("No full client!")?;
+		let tx_request = TransactionRequest::call(self.contract_address, data).gas_price(gas_price);
+		match full_client.transact(tx_request) {
+			Ok(()) | Err(transaction::Error::AlreadyImported) => Ok(()),
+			Err(e) => Err(e.to_string())?,
+		}
+	}
+
+	fn do_report_malicious(&self, address: &Address, block: BlockNumber, proof: Bytes) -> Result<(), EthcoreError> {
+		let client = self.client.read().as_ref().and_then(Weak::upgrade).ok_or("No client!")?;
+		let latest = client.block_header(BlockId::Latest).ok_or("No latest block!")?;
+		if !self.contains(&latest.parent_hash(), address) {
+			warn!(target: "engine", "Not reporting {} on block {}: Not a validator", address, block);
+			return Ok(());
+		}
+		let data = validator_report::functions::report_malicious::encode_input(*address, block, proof);
+		self.validators.enqueue_report(*address, block, data.clone());
+		let gas_price = self.report_gas_price(latest.number());
+		self.transact(data, gas_price, &*client)?;
+		warn!(target: "engine", "Reported malicious validator {} at block {}", address, block);
+		Ok(())
+	}
+
+	fn do_report_benign(&self, address: &Address, block: BlockNumber) -> Result<(), EthcoreError> {
+		let client = self.client.read().as_ref().and_then(Weak::upgrade).ok_or("No client!")?;
+		let latest = client.block_header(BlockId::Latest).ok_or("No latest block!")?;
+		let data = validator_report::functions::report_benign::encode_input(*address, block);
+		let gas_price = self.report_gas_price(latest.number());
+		self.transact(data, gas_price, &*client)?;
+		warn!(target: "engine", "Benign report for validator {} at block {}", address, block);
+		Ok(())
+	}
+
+	/// Returns the gas price for report transactions.
+	///
+	/// After `posdaoTransition`, this is zero. Otherwise it is the default (`None`).
+	fn report_gas_price(&self, block: BlockNumber) -> Option<U256> {
+		if self.posdao_transition? <= block {
+			Some(0.into())
+		} else {
+			None
 		}
 	}
 }
@@ -82,6 +115,16 @@ impl ValidatorSet for ValidatorContract {
 		self.validators.default_caller(id)
 	}
 
+	fn generate_engine_transactions(&self, first: bool, header: &Header, call: &mut SystemCall)
+		-> Result<Vec<(Address, Bytes)>, EthcoreError>
+	{
+		self.validators.generate_engine_transactions(first, header, call)
+	}
+
+	fn on_close_block(&self, header: &Header, address: &Address) -> Result<(), EthcoreError> {
+		self.validators.on_close_block(header, address)
+	}
+
 	fn on_epoch_begin(&self, first: bool, header: &Header, call: &mut SystemCall) -> Result<(), EthcoreError> {
 		self.validators.on_epoch_begin(first, header, call)
 	}
@@ -120,19 +163,15 @@ 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) {
-			Ok(_) => warn!(target: "engine", "Reported malicious validator {}", address),
-			Err(s) => warn!(target: "engine", "Validator {} could not be reported {}", address, s),
+		if let Err(s) = self.do_report_malicious(address, block, proof) {
+			warn!(target: "engine", "Validator {} could not be reported ({}) on block {}", address, s, block);
 		}
 	}
 
 	fn report_benign(&self, address: &Address, _set_block: BlockNumber, block: BlockNumber) {
 		trace!(target: "engine", "validator set recording benign misbehaviour at block #{} by {:#x}", block, address);
-		let data = validator_report::functions::report_benign::encode_input(*address, block);
-		match self.transact(data) {
-			Ok(_) => warn!(target: "engine", "Reported benign validator misbehaviour {}", address),
-			Err(s) => warn!(target: "engine", "Validator {} could not be reported {}", address, s),
+		if let Err(s) = self.do_report_benign(address, block) {
+			warn!(target: "engine", "Validator {} could not be reported ({}) on block {}", address, s, block);
 		}
 	}
 
@@ -150,6 +189,7 @@ mod tests {
 	use call_contract::CallContract;
 	use common_types::{header::Header, ids::BlockId};
 	use client_traits::{BlockChainClient, ChainInfo, BlockInfo, TransactionRequest};
+	use ethabi::FunctionOutputDecoder;
 	use ethcore::{
 		miner::{self, MinerService},
 		test_helpers::generate_dummy_client_with_spec,
@@ -167,7 +207,8 @@ mod tests {
 	#[test]
 	fn fetches_validators() {
 		let client = generate_dummy_client_with_spec(spec::new_validator_contract);
-		let vc = Arc::new(ValidatorContract::new("0000000000000000000000000000000000000005".parse::<Address>().unwrap()));
+		let addr: Address = "0000000000000000000000000000000000000005".parse().unwrap();
+		let vc = Arc::new(ValidatorContract::new(addr, None));
 		vc.register_client(Arc::downgrade(&client) as _);
 		let last_hash = client.best_block_header().hash();
 		assert!(vc.contains(&last_hash, &"7d577a597b2742b498cb5cf0c26cdcd726d39e6e".parse::<Address>().unwrap()));
@@ -198,6 +239,8 @@ mod tests {
 		assert!(client.engine().verify_block_external(&header).is_err());
 		client.engine().step();
 		assert_eq!(client.chain_info().best_block_number, 0);
+		// `reportBenign` when the designated proposer releases block from the future (bad clock).
+		assert!(client.engine().verify_block_basic(&header).is_err());
 
 		// Now create one that is more in future. That one should be rejected and validator should be reported.
 		let mut header = Header::default();
@@ -211,7 +254,7 @@ mod tests {
 		// Seal a block.
 		client.engine().step();
 		assert_eq!(client.chain_info().best_block_number, 1);
-		// Check if the unresponsive validator is `disliked`.
+		// Check if the unresponsive validator is `disliked`. "d8f2e0bf" accesses the field `disliked`..
 		assert_eq!(
 			client.call_contract(BlockId::Latest, validator_contract, "d8f2e0bf".from_hex().unwrap()).unwrap().to_hex(),
 			"0000000000000000000000007d577a597b2742b498cb5cf0c26cdcd726d39e6e"
@@ -223,6 +266,9 @@ mod tests {
 		client.engine().step();
 		client.engine().step();
 		assert_eq!(client.chain_info().best_block_number, 2);
+		let (data, decoder) = super::validator_report::functions::malice_reported_for_block::call(v1, 1);
+		let reported_enc = client.call_contract(BlockId::Latest, validator_contract, data).expect("call failed");
+		assert_ne!(Vec::<Address>::new(), decoder.decode(&reported_enc).expect("decoding failed"));
 
 		// Check if misbehaving validator was removed.
 		client.transact(TransactionRequest::call(Default::default(), Default::default())).unwrap();