Fix validator contract syncing (#4789)

* make validator set aware of various states

* fix updater build

* clean up contract call

* failing sync test

* adjust tests

* nicer indent [ci skip]

* revert bound divisor

ethcore/res/authority_round.json

@@ -33,7 +33,7 @@
 		"timestamp": "0x00",
 		"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
 		"extraData": "0x",
-		"gasLimit": "0x2fefd8"
+		"gasLimit": "0x222222"
 	},
 	"accounts": {
 		"0000000000000000000000000000000000000001": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } },

ethcore/res/tendermint.json

@@ -38,7 +38,7 @@
 		"timestamp": "0x00",
 		"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
 		"extraData": "0x",
-		"gasLimit": "0x2fefd8"
+		"gasLimit": "0x222222"
 	},
 	"accounts": {
 		"0000000000000000000000000000000000000001": { "balance": "1", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } },

ethcore/res/validator_contract.json

@@ -27,7 +27,7 @@
 		"timestamp": "0x00",
 		"parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
 		"extraData": "0x",
-		"gasLimit": "0x2fefd8"
+		"gasLimit": "0x222222"
 	},
 	"accounts": {
 		"0000000000000000000000000000000000000001": { "balance": "1", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } },

ethcore/src/client/client.rs

@@ -253,7 +253,7 @@ impl Client {
 		if let Some(reg_addr) = client.additional_params().get("registrar").and_then(|s| Address::from_str(s).ok()) {
 			trace!(target: "client", "Found registrar at {}", reg_addr);
 			let weak = Arc::downgrade(&client);
-			let registrar = Registry::new(reg_addr, move |a, d| weak.upgrade().ok_or("No client!".into()).and_then(|c| c.call_contract(a, d)));
+			let registrar = Registry::new(reg_addr, move |a, d| weak.upgrade().ok_or("No client!".into()).and_then(|c| c.call_contract(BlockId::Latest, a, d)));
 			*client.registrar.lock() = Some(registrar);
 		}
 		Ok(client)
@@ -1428,7 +1428,7 @@ impl BlockChainClient for Client {
 		}
 	}
 
-	fn call_contract(&self, address: Address, data: Bytes) -> Result<Bytes, String> {
+	fn call_contract(&self, block_id: BlockId, address: Address, data: Bytes) -> Result<Bytes, String> {
 		let from = Address::default();
 		let transaction = Transaction {
 			nonce: self.latest_nonce(&from),
@@ -1439,7 +1439,7 @@ impl BlockChainClient for Client {
 			data: data,
 		}.fake_sign(from);
 
-		self.call(&transaction, BlockId::Latest, Default::default())
+		self.call(&transaction, block_id, Default::default())
 			.map_err(|e| format!("{:?}", e))
 			.map(|executed| {
 				executed.output

ethcore/src/client/test_client.rs

@@ -731,7 +731,7 @@ impl BlockChainClient for TestBlockChainClient {
 		}
 	}
 
-	fn call_contract(&self, _address: Address, _data: Bytes) -> Result<Bytes, String> { Ok(vec![]) }
+	fn call_contract(&self, _id: BlockId, _address: Address, _data: Bytes) -> Result<Bytes, String> { Ok(vec![]) }
 
 	fn transact_contract(&self, address: Address, data: Bytes) -> Result<TransactionImportResult, EthcoreError> {
 		let transaction = Transaction {

ethcore/src/client/traits.rs

@@ -255,7 +255,7 @@ pub trait BlockChainClient : Sync + Send {
 	fn pruning_info(&self) -> PruningInfo;
 
 	/// Like `call`, but with various defaults. Designed to be used for calling contracts.
-	fn call_contract(&self, address: Address, data: Bytes) -> Result<Bytes, String>;
+	fn call_contract(&self, id: BlockId, address: Address, data: Bytes) -> Result<Bytes, String>;
 
 	/// Import a transaction: used for misbehaviour reporting.
 	fn transact_contract(&self, address: Address, data: Bytes) -> Result<TransactionImportResult, EthcoreError>;

ethcore/src/engines/authority_round.rs

@@ -141,12 +141,12 @@ impl AuthorityRound {
 		}
 	}
 
-	fn step_proposer(&self, step: usize) -> Address {
-		self.validators.get(step)
+	fn step_proposer(&self, bh: &H256, step: usize) -> Address {
+		self.validators.get(bh, step)
 	}
 
-	fn is_step_proposer(&self, step: usize, address: &Address) -> bool {
-		self.step_proposer(step) == *address
+	fn is_step_proposer(&self, bh: &H256, step: usize, address: &Address) -> bool {
+		self.step_proposer(bh, step) == *address
 	}
 }
 
@@ -231,7 +231,7 @@ impl Engine for AuthorityRound {
 	}
 
 	fn seals_internally(&self) -> Option<bool> {
-		Some(self.validators.contains(&self.signer.address()))
+		Some(self.signer.address() != Address::default())
 	}
 
 	/// Attempt to seal the block internally.
@@ -242,7 +242,7 @@ impl Engine for AuthorityRound {
 		if self.proposed.load(AtomicOrdering::SeqCst) { return Seal::None; }
 		let header = block.header();
 		let step = self.step.load(AtomicOrdering::SeqCst);
-		if self.is_step_proposer(step, header.author()) {
+		if self.is_step_proposer(header.parent_hash(), step, header.author()) {
 			if let Ok(signature) = self.signer.sign(header.bare_hash()) {
 				trace!(target: "engine", "generate_seal: Issuing a block for step {}.", step);
 				self.proposed.store(true, AtomicOrdering::SeqCst);
@@ -281,32 +281,33 @@ impl Engine for AuthorityRound {
 		}
 	}
 
-	/// Check if the signature belongs to the correct proposer.
-	fn verify_block_unordered(&self, header: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
-		let header_step = header_step(header)?;
+	fn verify_block_unordered(&self, _header: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
+				Ok(())
+	}
+
+	/// Do the validator and gas limit validation.
+	fn verify_block_family(&self, header: &Header, parent: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
+		let step = header_step(header)?;
 		// Give one step slack if step is lagging, double vote is still not possible.
-		if header_step <= self.step.load(AtomicOrdering::SeqCst) + 1 {
+		if step <= self.step.load(AtomicOrdering::SeqCst) + 1 {
+			// Check if the signature belongs to a validator, can depend on parent state.
 			let proposer_signature = header_signature(header)?;
-			let correct_proposer = self.step_proposer(header_step);
-			if verify_address(&correct_proposer, &proposer_signature, &header.bare_hash())? {
-				Ok(())
-			} else {
-				trace!(target: "engine", "verify_block_unordered: bad proposer for step: {}", header_step);
+			let correct_proposer = self.step_proposer(header.parent_hash(), step);
+			if !verify_address(&correct_proposer, &proposer_signature, &header.bare_hash())? {
+				trace!(target: "engine", "verify_block_unordered: bad proposer for step: {}", step);
 				Err(EngineError::NotProposer(Mismatch { expected: correct_proposer, found: header.author().clone() }))?
 			}
 		} else {
 			trace!(target: "engine", "verify_block_unordered: block from the future");
 			self.validators.report_benign(header.author());
 			Err(BlockError::InvalidSeal)?
 		}
-	}
 
-	fn verify_block_family(&self, header: &Header, parent: &Header, _block: Option<&[u8]>) -> Result<(), Error> {
+		// Do not calculate difficulty for genesis blocks.
 		if header.number() == 0 {
 			return Err(From::from(BlockError::RidiculousNumber(OutOfBounds { min: Some(1), max: None, found: header.number() })));
 		}
 
-		let step = header_step(header)?;
 		// Check if parent is from a previous step.
 		if step == header_step(parent)? {
 			trace!(target: "engine", "Multiple blocks proposed for step {}.", step);
@@ -394,7 +395,7 @@ mod tests {
 		let mut header: Header = Header::default();
 		header.set_seal(vec![encode(&H520::default()).to_vec()]);
 
-		let verify_result = engine.verify_block_unordered(&header, None);
+		let verify_result = engine.verify_block_family(&header, &Default::default(), None);
 		assert!(verify_result.is_err());
 	}
 
@@ -432,10 +433,14 @@ mod tests {
 
 	#[test]
 	fn proposer_switching() {
-		let mut header: Header = Header::default();
 		let tap = AccountProvider::transient_provider();
 		let addr = tap.insert_account(Secret::from_slice(&"0".sha3()).unwrap(), "0").unwrap();
-
+		let mut parent_header: Header = Header::default();
+		parent_header.set_seal(vec![encode(&0usize).to_vec()]);
+		parent_header.set_gas_limit(U256::from_str("222222").unwrap());
+		let mut header: Header = Header::default();
+		header.set_number(1);
+		header.set_gas_limit(U256::from_str("222222").unwrap());
 		header.set_author(addr);
 
 		let engine = Spec::new_test_round().engine;
@@ -444,17 +449,22 @@ mod tests {
 		// Two validators.
 		// Spec starts with step 2.
 		header.set_seal(vec![encode(&2usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]);
-		assert!(engine.verify_block_seal(&header).is_err());
+		assert!(engine.verify_block_family(&header, &parent_header, None).is_err());
 		header.set_seal(vec![encode(&1usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]);
-		assert!(engine.verify_block_seal(&header).is_ok());
+		assert!(engine.verify_block_family(&header, &parent_header, None).is_ok());
 	}
 
 	#[test]
 	fn rejects_future_block() {
-		let mut header: Header = Header::default();
 		let tap = AccountProvider::transient_provider();
 		let addr = tap.insert_account(Secret::from_slice(&"0".sha3()).unwrap(), "0").unwrap();
 
+		let mut parent_header: Header = Header::default();
+		parent_header.set_seal(vec![encode(&0usize).to_vec()]);
+		parent_header.set_gas_limit(U256::from_str("222222").unwrap());
+		let mut header: Header = Header::default();
+		header.set_number(1);
+		header.set_gas_limit(U256::from_str("222222").unwrap());
 		header.set_author(addr);
 
 		let engine = Spec::new_test_round().engine;
@@ -463,8 +473,8 @@ mod tests {
 		// Two validators.
 		// Spec starts with step 2.
 		header.set_seal(vec![encode(&1usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]);
-		assert!(engine.verify_block_seal(&header).is_ok());
+		assert!(engine.verify_block_family(&header, &parent_header, None).is_ok());
 		header.set_seal(vec![encode(&5usize).to_vec(), encode(&(&*signature as &[u8])).to_vec()]);
-		assert!(engine.verify_block_seal(&header).is_err());
+		assert!(engine.verify_block_family(&header, &parent_header, None).is_err());
 	}
 }

ethcore/src/engines/basic_authority.rs

@@ -104,14 +104,14 @@ impl Engine for BasicAuthority {
 	}
 
 	fn seals_internally(&self) -> Option<bool> {
-		Some(self.validators.contains(&self.signer.address()))
+		Some(self.signer.address() != Address::default())
 	}
 
 	/// Attempt to seal the block internally.
 	fn generate_seal(&self, block: &ExecutedBlock) -> Seal {
 		let header = block.header();
 		let author = header.author();
-		if self.validators.contains(author) {
+		if self.validators.contains(header.parent_hash(), author) {
 			// account should be pernamently unlocked, otherwise sealing will fail
 			if let Ok(signature) = self.signer.sign(header.bare_hash()) {
 				return Seal::Regular(vec![::rlp::encode(&(&H520::from(signature) as &[u8])).to_vec()]);
@@ -133,20 +133,20 @@ impl Engine for BasicAuthority {
 		Ok(())
 	}
 
-	fn verify_block_unordered(&self, header: &Header, _block: Option<&[u8]>) -> result::Result<(), Error> {
-		use rlp::{UntrustedRlp, View};
+	fn verify_block_unordered(&self, _header: &Header, _block: Option<&[u8]>) -> result::Result<(), Error> {
+		Ok(())
+	}
 
-		// check the signature is legit.
+	fn verify_block_family(&self, header: &Header, parent: &Header, _block: Option<&[u8]>) -> result::Result<(), Error> {
+		use rlp::{UntrustedRlp, View};
+		// Check if the signature belongs to a validator, can depend on parent state.
 		let sig = UntrustedRlp::new(&header.seal()[0]).as_val::<H520>()?;
 		let signer = public_to_address(&recover(&sig.into(), &header.bare_hash())?);
-		if !self.validators.contains(&signer) {
+		if !self.validators.contains(header.parent_hash(), &signer) {
 			return Err(BlockError::InvalidSeal)?;
 		}
-		Ok(())
-	}
 
-	fn verify_block_family(&self, header: &Header, parent: &Header, _block: Option<&[u8]>) -> result::Result<(), Error> {
-		// we should not calculate difficulty for genesis blocks
+		// Do not calculate difficulty for genesis blocks.
 		if header.number() == 0 {
 			return Err(From::from(BlockError::RidiculousNumber(OutOfBounds { min: Some(1), max: None, found: header.number() })));
 		}
@@ -239,7 +239,7 @@ mod tests {
 		let mut header: Header = Header::default();
 		header.set_seal(vec![::rlp::encode(&H520::default()).to_vec()]);
 
-		let verify_result = engine.verify_block_unordered(&header, None);
+		let verify_result = engine.verify_block_family(&header, &Default::default(), None);
 		assert!(verify_result.is_err());
 	}