From aab63c339d86eef2e76abbc23e937e27349c57a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Silva?= Date: Thu, 15 Feb 2018 00:39:29 +0000 Subject: [PATCH] Aura: Broadcast empty step messages instead of creating empty blocks (#7605) * aura: broadcast empty step message instead of sealing empty block * aura: add empty_step messages to seal * aura: include parent_hash in empty step message * aura: verify received empty step messages * aura: verify empty step messages in block * aura: fix dead lock on empty_steps * aura: fix EmptyStep Encodable * aura: take number of empty steps into account in chain score * aura: use empty step signers for finality * aura: add empty "empty step" messages to seal when reading from spec * aura: fix EmptyStep rlp encoding * aura: use Vec instead of Bytes * aura: fix block empty step verification * Update .gitlab-ci.yml fix lint * aura: fix accumulation of empty step signatures for finality * aura: include empty steps in seal signature * aura: configurable max number of empty steps * engine: pass block header to seal_fields method This is necessary to make the number of seal fields dynamic, e.g. activating a transition on a certain block number that changes the seal. * aura: add transition to enable empty step messages * aura: clear old empty step messages on verify_block_external * aura: ignore empty step messages from the future * aura: report skipped primaries when empty steps are not enabled * aura: fix tests * aura: report misbehavior * aura: add tests for rolling finality with multiple signatures * engine: fix validator set test In this test the block validation wasn't failing because the block was in the future (expected failure) but was instead failing because the author of the block isn't the expected authority. Since we added reporting of blocks produced by the wrong authority this test started failing. * aura: reward all the authors of empty step messages * aura: fix reward attribution for new blocks * aura: add tests for empty steps broadcasting and inclusion in blocks * aura: reduce size of empty step messages in seal * aura: add test for empty step inclusion in blocks * aura: add test for rewarding of empty steps * aura: add test for empty steps validation * aura: fix rlp encoding of sealed empty step * aura: fix grumbles --- ethcore/light/src/client/mod.rs | 4 + ethcore/res/authority_round_empty_steps.json | 51 ++ ethcore/src/block.rs | 6 +- ethcore/src/client/client.rs | 4 + ethcore/src/client/test_client.rs | 4 + ethcore/src/client/traits.rs | 3 + .../src/engines/authority_round/finality.rs | 92 +- ethcore/src/engines/authority_round/mod.rs | 834 ++++++++++++++++-- ethcore/src/engines/basic_authority.rs | 2 +- ethcore/src/engines/mod.rs | 14 +- ethcore/src/engines/tendermint/mod.rs | 22 +- ethcore/src/engines/validator_set/contract.rs | 2 +- ethcore/src/ethereum/ethash.rs | 14 +- ethcore/src/spec/spec.rs | 7 + ethcore/src/tests/helpers.rs | 14 +- ethcore/src/verification/verification.rs | 5 +- json/src/spec/authority_round.rs | 6 + 17 files changed, 953 insertions(+), 131 deletions(-) create mode 100644 ethcore/res/authority_round_empty_steps.json diff --git a/ethcore/light/src/client/mod.rs b/ethcore/light/src/client/mod.rs index 3f8f22b0a..32ddf93fb 100644 --- a/ethcore/light/src/client/mod.rs +++ b/ethcore/light/src/client/mod.rs @@ -630,4 +630,8 @@ impl ::ethcore::client::EngineClient for Client { fn block_number(&self, id: BlockId) -> Option { self.block_header(id).map(|hdr| hdr.number()) } + + fn block_header(&self, id: BlockId) -> Option { + Client::block_header(self, id) + } } diff --git a/ethcore/res/authority_round_empty_steps.json b/ethcore/res/authority_round_empty_steps.json new file mode 100644 index 000000000..b884e8e15 --- /dev/null +++ b/ethcore/res/authority_round_empty_steps.json @@ -0,0 +1,51 @@ +{ + "name": "TestAuthorityRoundEmptySteps", + "engine": { + "authorityRound": { + "params": { + "stepDuration": 1, + "startStep": 2, + "validators": { + "list": [ + "0x7d577a597b2742b498cb5cf0c26cdcd726d39e6e", + "0x82a978b3f5962a5b0957d9ee9eef472ee55b42f1" + ] + }, + "blockReward": "10", + "immediateTransitions": true, + "emptyStepsTransition": "1", + "maximumEmptySteps": "2" + } + } + }, + "params": { + "gasLimitBoundDivisor": "0x0400", + "accountStartNonce": "0x0", + "maximumExtraDataSize": "0x20", + "minGasLimit": "0x1388", + "networkID" : "0x69" + }, + "genesis": { + "seal": { + "authorityRound": { + "step": "0x0", + "signature": "0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" + } + }, + "difficulty": "0x20000", + "author": "0x0000000000000000000000000000000000000000", + "timestamp": "0x00", + "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000", + "extraData": "0x", + "gasLimit": "0x222222" + }, + "accounts": { + "0000000000000000000000000000000000000001": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ecrecover", "pricing": { "linear": { "base": 3000, "word": 0 } } } }, + "0000000000000000000000000000000000000002": { "balance": "1", "nonce": "1048576", "builtin": { "name": "sha256", "pricing": { "linear": { "base": 60, "word": 12 } } } }, + "0000000000000000000000000000000000000003": { "balance": "1", "nonce": "1048576", "builtin": { "name": "ripemd160", "pricing": { "linear": { "base": 600, "word": 120 } } } }, + "0000000000000000000000000000000000000004": { "balance": "1", "nonce": "1048576", "builtin": { "name": "identity", "pricing": { "linear": { "base": 15, "word": 3 } } } }, + "9cce34f7ab185c7aba1b7c8140d620b4bda941d6": { "balance": "1606938044258990275541962092341162602522202993782792835301376", "nonce": "1048576" }, + "7d577a597b2742b498cb5cf0c26cdcd726d39e6e": { "balance": "1000000000" }, + "82a978b3f5962a5b0957d9ee9eef472ee55b42f1": { "balance": "1000000000" } + } +} diff --git a/ethcore/src/block.rs b/ethcore/src/block.rs index c4c82a17b..329a1c328 100644 --- a/ethcore/src/block.rs +++ b/ethcore/src/block.rs @@ -543,9 +543,11 @@ impl LockedBlock { /// /// NOTE: This does not check the validity of `seal` with the engine. pub fn seal(self, engine: &EthEngine, seal: Vec) -> Result { + let expected_seal_fields = engine.seal_fields(self.header()); let mut s = self; - if seal.len() != engine.seal_fields() { - return Err(BlockError::InvalidSealArity(Mismatch{expected: engine.seal_fields(), found: seal.len()})); + if seal.len() != expected_seal_fields { + return Err(BlockError::InvalidSealArity( + Mismatch { expected: expected_seal_fields, found: seal.len() })); } s.block.header.set_seal(seal); Ok(SealedBlock { block: s.block, uncle_bytes: s.uncle_bytes }) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 0cce7d4bd..08faf286f 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -2002,6 +2002,10 @@ impl super::traits::EngineClient for Client { fn block_number(&self, id: BlockId) -> Option { BlockChainClient::block_number(self, id) } + + fn block_header(&self, id: BlockId) -> Option<::encoded::Header> { + BlockChainClient::block_header(self, id) + } } impl ProvingBlockChainClient for Client { diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 241b54acd..0fec09014 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -834,4 +834,8 @@ impl super::traits::EngineClient for TestBlockChainClient { fn block_number(&self, id: BlockId) -> Option { BlockChainClient::block_number(self, id) } + + fn block_header(&self, id: BlockId) -> Option<::encoded::Header> { + BlockChainClient::block_header(self, id) + } } diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index 3b6b8c555..eecf23a4b 100644 --- a/ethcore/src/client/traits.rs +++ b/ethcore/src/client/traits.rs @@ -339,6 +339,9 @@ pub trait EngineClient: Sync + Send { /// Get a block number by ID. fn block_number(&self, id: BlockId) -> Option; + + /// Get raw block header data by block id. + fn block_header(&self, id: BlockId) -> Option; } /// Extended client interface for providing proofs of the state. diff --git a/ethcore/src/engines/authority_round/finality.rs b/ethcore/src/engines/authority_round/finality.rs index fbfaba0f0..61f1c1822 100644 --- a/ethcore/src/engines/authority_round/finality.rs +++ b/ethcore/src/engines/authority_round/finality.rs @@ -30,7 +30,7 @@ pub struct UnknownValidator; /// Rolling finality checker for authority round consensus. /// Stores a chain of unfinalized hashes that can be pushed onto. pub struct RollingFinality { - headers: VecDeque<(H256, Address)>, + headers: VecDeque<(H256, Vec
)>, signers: SimpleList, sign_count: HashMap, last_pushed: Option, @@ -52,28 +52,31 @@ impl RollingFinality { /// /// Fails if any provided signature isn't part of the signers set. pub fn build_ancestry_subchain(&mut self, iterable: I) -> Result<(), UnknownValidator> - where I: IntoIterator + where I: IntoIterator)> { self.clear(); - for (hash, signer) in iterable { - if !self.signers.contains(&signer) { return Err(UnknownValidator) } + for (hash, signers) in iterable { + if signers.iter().any(|s| !self.signers.contains(s)) { return Err(UnknownValidator) } if self.last_pushed.is_none() { self.last_pushed = Some(hash) } // break when we've got our first finalized block. { let current_signed = self.sign_count.len(); - let would_be_finalized = (current_signed + 1) * 2 > self.signers.len(); - let entry = self.sign_count.entry(signer); - if let (true, &Entry::Vacant(_)) = (would_be_finalized, &entry) { + let new_signers = signers.iter().filter(|s| !self.sign_count.contains_key(s)).count(); + let would_be_finalized = (current_signed + new_signers) * 2 > self.signers.len(); + + if would_be_finalized { trace!(target: "finality", "Encountered already finalized block {}", hash); break } - *entry.or_insert(0) += 1; + for signer in signers.iter() { + *self.sign_count.entry(*signer).or_insert(0) += 1; + } } - self.headers.push_front((hash, signer)); + self.headers.push_front((hash, signers)); } trace!(target: "finality", "Rolling finality state: {:?}", self.headers); @@ -103,30 +106,35 @@ impl RollingFinality { /// Fails if `signer` isn't a member of the active validator set. /// Returns a list of all newly finalized headers. // TODO: optimize with smallvec. - pub fn push_hash(&mut self, head: H256, signer: Address) -> Result, UnknownValidator> { - if !self.signers.contains(&signer) { return Err(UnknownValidator) } + pub fn push_hash(&mut self, head: H256, signers: Vec
) -> Result, UnknownValidator> { + if signers.iter().any(|s| !self.signers.contains(s)) { return Err(UnknownValidator) } - self.headers.push_back((head, signer)); - *self.sign_count.entry(signer).or_insert(0) += 1; + for signer in signers.iter() { + *self.sign_count.entry(*signer).or_insert(0) += 1; + } + + self.headers.push_back((head, signers)); let mut newly_finalized = Vec::new(); while self.sign_count.len() * 2 > self.signers.len() { - let (hash, signer) = self.headers.pop_front() + let (hash, signers) = self.headers.pop_front() .expect("headers length always greater than sign count length; qed"); newly_finalized.push(hash); - match self.sign_count.entry(signer) { - Entry::Occupied(mut entry) => { - // decrement count for this signer and purge on zero. - *entry.get_mut() -= 1; + for signer in signers { + match self.sign_count.entry(signer) { + Entry::Occupied(mut entry) => { + // decrement count for this signer and purge on zero. + *entry.get_mut() -= 1; - if *entry.get() == 0 { - entry.remove(); + if *entry.get() == 0 { + entry.remove(); + } } + Entry::Vacant(_) => panic!("all hashes in `header` should have entries in `sign_count` for their signers; qed"), } - Entry::Vacant(_) => panic!("all hashes in `header` should have an entry in `sign_count` for their signer; qed"), } } @@ -137,7 +145,7 @@ impl RollingFinality { } } -pub struct Iter<'a>(::std::collections::vec_deque::Iter<'a, (H256, Address)>); +pub struct Iter<'a>(::std::collections::vec_deque::Iter<'a, (H256, Vec
)>); impl<'a> Iterator for Iter<'a> { type Item = H256; @@ -153,10 +161,10 @@ mod tests { use super::RollingFinality; #[test] - fn rejects_unknown_signer() { - let signers = (0..3).map(|_| Address::random()).collect(); - let mut finality = RollingFinality::blank(signers); - assert!(finality.push_hash(H256::random(), Address::random()).is_err()); + fn rejects_unknown_signers() { + let signers = (0..3).map(|_| Address::random()).collect::>(); + let mut finality = RollingFinality::blank(signers.clone()); + assert!(finality.push_hash(H256::random(), vec![signers[0], Address::random()]).is_err()); } #[test] @@ -169,19 +177,29 @@ mod tests { // 3 / 6 signers is < 51% so no finality. for (i, hash) in hashes.iter().take(6).cloned().enumerate() { let i = i % 3; - assert!(finality.push_hash(hash, signers[i]).unwrap().len() == 0); + assert!(finality.push_hash(hash, vec![signers[i]]).unwrap().len() == 0); } // after pushing a block signed by a fourth validator, the first four // blocks of the unverified chain become verified. - assert_eq!(finality.push_hash(hashes[6], signers[4]).unwrap(), + assert_eq!(finality.push_hash(hashes[6], vec![signers[4]]).unwrap(), vec![hashes[0], hashes[1], hashes[2], hashes[3]]); } + #[test] + fn finalize_multiple_signers() { + let signers: Vec<_> = (0..6).map(|_| Address::random()).collect(); + let mut finality = RollingFinality::blank(signers.clone()); + let hash = H256::random(); + + // after pushing a block signed by four validators, it becomes verified right away. + assert_eq!(finality.push_hash(hash, signers[0..4].to_vec()).unwrap(), vec![hash]); + } + #[test] fn from_ancestry() { let signers: Vec<_> = (0..6).map(|_| Address::random()).collect(); - let hashes: Vec<_> = (0..12).map(|i| (H256::random(), signers[i % 6])).collect(); + let hashes: Vec<_> = (0..12).map(|i| (H256::random(), vec![signers[i % 6]])).collect(); let mut finality = RollingFinality::blank(signers.clone()); finality.build_ancestry_subchain(hashes.iter().rev().cloned()).unwrap(); @@ -189,4 +207,20 @@ mod tests { assert_eq!(finality.unfinalized_hashes().count(), 3); assert_eq!(finality.subchain_head(), Some(hashes[11].0)); } + + #[test] + fn from_ancestry_multiple_signers() { + let signers: Vec<_> = (0..6).map(|_| Address::random()).collect(); + let hashes: Vec<_> = (0..12).map(|i| { + (H256::random(), vec![signers[i % 6], signers[(i + 1) % 6], signers[(i + 2) % 6]]) + }).collect(); + + let mut finality = RollingFinality::blank(signers.clone()); + finality.build_ancestry_subchain(hashes.iter().rev().cloned()).unwrap(); + + // only the last hash has < 51% of authorities' signatures + assert_eq!(finality.unfinalized_hashes().count(), 1); + assert_eq!(finality.unfinalized_hashes().next(), Some(hashes[11].0)); + assert_eq!(finality.subchain_head(), Some(hashes[11].0)); + } } diff --git a/ethcore/src/engines/authority_round/mod.rs b/ethcore/src/engines/authority_round/mod.rs index 913a5123d..4200f6b36 100644 --- a/ethcore/src/engines/authority_round/mod.rs +++ b/ethcore/src/engines/authority_round/mod.rs @@ -16,10 +16,12 @@ //! A blockchain engine that supports a non-instant BFT proof-of-authority. +use std::fmt; use std::sync::atomic::{AtomicUsize, AtomicBool, Ordering as AtomicOrdering}; use std::sync::{Weak, Arc}; use std::time::{UNIX_EPOCH, Duration}; use std::collections::{BTreeMap, HashSet}; +use std::iter::FromIterator; use account_provider::AccountProvider; use block::*; @@ -28,6 +30,7 @@ use engines::{Engine, Seal, EngineError, ConstructedVerifier}; use error::{Error, BlockError}; use ethjson; use machine::{AuxiliaryData, Call, EthereumMachine}; +use hash::keccak; use header::{Header, BlockNumber}; use super::signer::EngineSigner; @@ -35,10 +38,10 @@ use super::validator_set::{ValidatorSet, SimpleList, new_validator_set}; use self::finality::RollingFinality; -use ethkey::{verify_address, Signature}; +use ethkey::{public_to_address, recover, verify_address, Signature}; use io::{IoContext, IoHandler, TimerToken, IoService}; use itertools::{self, Itertools}; -use rlp::{UntrustedRlp, encode}; +use rlp::{encode, Decodable, DecoderError, Encodable, RlpStream, UntrustedRlp}; use ethereum_types::{H256, H520, Address, U128, U256}; use parking_lot::{Mutex, RwLock}; use unexpected::{Mismatch, OutOfBounds}; @@ -69,6 +72,10 @@ pub struct AuthorityRoundParams { pub maximum_uncle_count_transition: u64, /// Number of accepted uncles. pub maximum_uncle_count: usize, + /// Empty step messages transition block. + pub empty_steps_transition: u64, + /// Number of accepted empty steps. + pub maximum_empty_steps: usize, } const U16_MAX: usize = ::std::u16::MAX as usize; @@ -90,6 +97,8 @@ impl From for AuthorityRoundParams { block_reward: p.block_reward.map_or_else(Default::default, Into::into), maximum_uncle_count_transition: p.maximum_uncle_count_transition.map_or(0, Into::into), maximum_uncle_count: p.maximum_uncle_count.map_or(0, Into::into), + empty_steps_transition: p.empty_steps_transition.map_or(u64::max_value(), |n| ::std::cmp::max(n.into(), 1)), + maximum_empty_steps: p.maximum_empty_steps.map_or(0, Into::into), } } } @@ -172,8 +181,8 @@ impl Step { } // Chain scoring: total weight is sqrt(U256::max_value())*height - step -fn calculate_score(parent_step: U256, current_step: U256) -> U256 { - U256::from(U128::max_value()) + parent_step - current_step +fn calculate_score(parent_step: U256, current_step: U256, current_empty_steps: U256) -> U256 { + U256::from(U128::max_value()) + parent_step - current_step + current_empty_steps } struct EpochManager { @@ -261,6 +270,110 @@ impl EpochManager { } } +/// A message broadcast by authorities when it's their turn to seal a block but there are no +/// transactions. Other authorities accumulate these messages and later include them in the seal as +/// proof. +#[derive(Clone, Debug)] +struct EmptyStep { + signature: H520, + step: usize, + parent_hash: H256, +} + +impl EmptyStep { + fn from_sealed(sealed_empty_step: SealedEmptyStep, parent_hash: &H256) -> EmptyStep { + let signature = sealed_empty_step.signature; + let step = sealed_empty_step.step; + let parent_hash = parent_hash.clone(); + EmptyStep { signature, step, parent_hash } + } + + fn verify(&self, validators: &ValidatorSet) -> Result { + let message = keccak(empty_step_rlp(self.step, &self.parent_hash)); + let correct_proposer = step_proposer(validators, &self.parent_hash, self.step); + + verify_address(&correct_proposer, &self.signature.into(), &message) + .map_err(|e| e.into()) + } + + fn author(&self) -> Result { + let message = keccak(empty_step_rlp(self.step, &self.parent_hash)); + let public = recover(&self.signature.into(), &message)?; + Ok(public_to_address(&public)) + } + + fn sealed(&self) -> SealedEmptyStep { + let signature = self.signature; + let step = self.step; + SealedEmptyStep { signature, step } + } +} + +impl fmt::Display for EmptyStep { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + write!(f, "({}, {}, {})", self.signature, self.step, self.parent_hash) + } +} + +impl Encodable for EmptyStep { + fn rlp_append(&self, s: &mut RlpStream) { + let empty_step_rlp = empty_step_rlp(self.step, &self.parent_hash); + s.begin_list(2) + .append(&self.signature) + .append_raw(&empty_step_rlp, 1); + } +} + +impl Decodable for EmptyStep { + fn decode(rlp: &UntrustedRlp) -> Result { + let signature = rlp.val_at(0)?; + let empty_step_rlp = rlp.at(1)?; + + let step = empty_step_rlp.val_at(0)?; + let parent_hash = empty_step_rlp.val_at(1)?; + + Ok(EmptyStep { signature, step, parent_hash }) + } +} + +pub fn empty_step_full_rlp(signature: &H520, empty_step_rlp: &[u8]) -> Vec { + let mut s = RlpStream::new_list(2); + s.append(signature).append_raw(empty_step_rlp, 1); + s.out() +} + +pub fn empty_step_rlp(step: usize, parent_hash: &H256) -> Vec { + let mut s = RlpStream::new_list(2); + s.append(&step).append(parent_hash); + s.out() +} + +/// An empty step message that is included in a seal, the only difference is that it doesn't include +/// the `parent_hash` in order to save space. The included signature is of the original empty step +/// message, which can be reconstructed by using the parent hash of the block in which this sealed +/// empty message is included. +struct SealedEmptyStep { + signature: H520, + step: usize, +} + +impl Encodable for SealedEmptyStep { + fn rlp_append(&self, s: &mut RlpStream) { + s.begin_list(2) + .append(&self.signature) + .append(&self.step); + } +} + +impl Decodable for SealedEmptyStep { + fn decode(rlp: &UntrustedRlp) -> Result { + let signature = rlp.val_at(0)?; + let step = rlp.val_at(1)?; + + Ok(SealedEmptyStep { signature, step }) + } +} + /// Engine using `AuthorityRound` proof-of-authority BFT consensus. pub struct AuthorityRound { transition_service: IoService<()>, @@ -271,11 +384,14 @@ pub struct AuthorityRound { validators: Box, validate_score_transition: u64, validate_step_transition: u64, + empty_steps: Mutex>, epoch_manager: Mutex, immediate_transitions: bool, block_reward: U256, maximum_uncle_count_transition: u64, maximum_uncle_count: usize, + empty_steps_transition: u64, + maximum_empty_steps: usize, machine: EthereumMachine, } @@ -283,15 +399,16 @@ pub struct AuthorityRound { struct EpochVerifier { step: Arc, subchain_validators: SimpleList, + empty_steps_transition: u64, } impl super::EpochVerifier for EpochVerifier { fn verify_light(&self, header: &Header) -> Result<(), Error> { // Validate the timestamp - verify_timestamp(&*self.step, header_step(header)?)?; + verify_timestamp(&*self.step, header_step(header, self.empty_steps_transition)?)?; // always check the seal since it's fast. // nothing heavier to do. - verify_external(header, &self.subchain_validators) + verify_external(header, &self.subchain_validators, self.empty_steps_transition) } fn check_finality_proof(&self, proof: &[u8]) -> Option> { @@ -309,28 +426,103 @@ impl super::EpochVerifier for EpochVerifier { let headers: Vec
= otry!(UntrustedRlp::new(proof).as_list().ok()); - for header in &headers { - // ensure all headers have correct number of seal fields so we can `verify_external` - // without panic. - // - // `verify_external` checks that signature is correct and author == signer. - if header.seal().len() != 2 { return None } - otry!(verify_external(header, &self.subchain_validators).ok()); + { + let mut push_header = |parent_header: &Header, header: Option<&Header>| { + // ensure all headers have correct number of seal fields so we can `verify_external` + // and get `empty_steps` without panic. + if parent_header.seal().len() != header_expected_seal_fields(parent_header, self.empty_steps_transition) { + return None + } + if header.iter().any(|h| h.seal().len() != header_expected_seal_fields(h, self.empty_steps_transition)) { + return None + } - let newly_finalized = otry!(finality_checker.push_hash(header.hash(), header.author().clone()).ok()); - finalized.extend(newly_finalized); + // `verify_external` checks that signature is correct and author == signer. + otry!(verify_external(parent_header, &self.subchain_validators, self.empty_steps_transition).ok()); + + let mut signers = match header { + Some(header) => otry!(header_empty_steps_signers(header, self.empty_steps_transition).ok()), + _ => Vec::new(), + }; + signers.push(parent_header.author().clone()); + + let newly_finalized = otry!(finality_checker.push_hash(parent_header.hash(), signers).ok()); + finalized.extend(newly_finalized); + + Some(()) + }; + + for window in headers.windows(2) { + otry!(push_header(&window[0], Some(&window[1]))); + } + + if let Some(last) = headers.last() { + otry!(push_header(last, None)); + } } if finalized.is_empty() { None } else { Some(finalized) } } } -fn header_step(header: &Header) -> Result { - UntrustedRlp::new(&header.seal().get(0).expect("was either checked with verify_block_basic or is genesis; has 2 fields; qed (Make sure the spec file has a correct genesis seal)")).as_val() +fn header_seal_hash(header: &Header, empty_steps_rlp: Option<&[u8]>) -> H256 { + match empty_steps_rlp { + Some(empty_steps_rlp) => { + let mut message = header.bare_hash().to_vec(); + message.extend_from_slice(empty_steps_rlp); + keccak(message) + }, + None => { + header.bare_hash() + }, + } } -fn header_signature(header: &Header) -> Result { - UntrustedRlp::new(&header.seal().get(1).expect("was checked with verify_block_basic; has 2 fields; qed")).as_val::().map(Into::into) +fn header_expected_seal_fields(header: &Header, empty_steps_transition: u64) -> usize { + if header.number() >= empty_steps_transition { + 3 + } else { + 2 + } +} + +fn header_step(header: &Header, empty_steps_transition: u64) -> Result { + let expected_seal_fields = header_expected_seal_fields(header, empty_steps_transition); + UntrustedRlp::new(&header.seal().get(0).expect( + &format!("was either checked with verify_block_basic or is genesis; has {} fields; qed (Make sure the spec file has a correct genesis seal)", expected_seal_fields))).as_val() +} + +fn header_signature(header: &Header, empty_steps_transition: u64) -> Result { + let expected_seal_fields = header_expected_seal_fields(header, empty_steps_transition); + UntrustedRlp::new(&header.seal().get(1).expect( + &format!("was checked with verify_block_basic; has {} fields; qed", expected_seal_fields))).as_val::().map(Into::into) +} + +// extracts the raw empty steps vec from the header seal. should only be called when there are 3 fields in the seal +// (i.e. header.number() >= self.empty_steps_transition) +fn header_empty_steps_raw(header: &Header) -> &[u8] { + header.seal().get(2).expect("was checked with verify_block_basic; has 3 fields; qed") +} + +// extracts the empty steps from the header seal. should only be called when there are 3 fields in the seal +// (i.e. header.number() >= self.empty_steps_transition). +fn header_empty_steps(header: &Header) -> Result, ::rlp::DecoderError> { + let empty_steps = UntrustedRlp::new(header_empty_steps_raw(header)).as_list::()?; + Ok(empty_steps.into_iter().map(|s| EmptyStep::from_sealed(s, header.parent_hash())).collect()) +} + +// gets the signers of empty step messages for the given header, does not include repeated signers +fn header_empty_steps_signers(header: &Header, empty_steps_transition: u64) -> Result, Error> { + if header.number() >= empty_steps_transition { + let mut signers = HashSet::new(); + for empty_step in header_empty_steps(header)? { + signers.insert(empty_step.author()?); + } + + Ok(Vec::from_iter(signers.into_iter())) + } else { + Ok(Vec::new()) + } } fn step_proposer(validators: &ValidatorSet, bh: &H256, step: usize) -> Address { @@ -359,16 +551,25 @@ fn verify_timestamp(step: &Step, header_step: usize) -> Result<(), BlockError> { } } -fn verify_external(header: &Header, validators: &ValidatorSet) -> Result<(), Error> { - let header_step = header_step(header)?; +fn verify_external(header: &Header, validators: &ValidatorSet, empty_steps_transition: u64) -> Result<(), Error> { + let header_step = header_step(header, empty_steps_transition)?; - let proposer_signature = header_signature(header)?; + let proposer_signature = header_signature(header, empty_steps_transition)?; let correct_proposer = validators.get(header.parent_hash(), header_step); - let is_invalid_proposer = *header.author() != correct_proposer || - !verify_address(&correct_proposer, &proposer_signature, &header.bare_hash())?; + let is_invalid_proposer = *header.author() != correct_proposer || { + let empty_steps_rlp = if header.number() >= empty_steps_transition { + Some(header_empty_steps_raw(header)) + } else { + None + }; + + let header_seal_hash = header_seal_hash(header, empty_steps_rlp); + !verify_address(&correct_proposer, &proposer_signature, &header_seal_hash)? + }; if is_invalid_proposer { trace!(target: "engine", "verify_block_external: bad proposer for step: {}", header_step); + validators.report_benign(header.author(), header.number(), header.number()); Err(EngineError::NotProposer(Mismatch { expected: correct_proposer, found: header.author().clone() }))? } else { Ok(()) @@ -423,11 +624,14 @@ impl AuthorityRound { validators: our_params.validators, validate_score_transition: our_params.validate_score_transition, validate_step_transition: our_params.validate_step_transition, + empty_steps: Mutex::new(Vec::new()), epoch_manager: Mutex::new(EpochManager::blank()), immediate_transitions: our_params.immediate_transitions, block_reward: our_params.block_reward, maximum_uncle_count_transition: our_params.maximum_uncle_count_transition, maximum_uncle_count: our_params.maximum_uncle_count, + empty_steps_transition: our_params.empty_steps_transition, + maximum_empty_steps: our_params.maximum_empty_steps, machine: machine, }); @@ -438,6 +642,52 @@ impl AuthorityRound { } Ok(engine) } + + fn empty_steps(&self, from_step: U256, to_step: U256, parent_hash: H256) -> Vec { + self.empty_steps.lock().iter().filter(|e| { + U256::from(e.step) > from_step && + U256::from(e.step) < to_step && + e.parent_hash == parent_hash + }).cloned().collect() + } + + + fn clear_empty_steps(&self, step: U256) { + // clear old `empty_steps` messages + self.empty_steps.lock().retain(|e| U256::from(e.step) > step); + } + + fn handle_empty_step_message(&self, empty_step: EmptyStep) { + let mut empty_steps = self.empty_steps.lock(); + empty_steps.push(empty_step); + } + + fn generate_empty_step(&self, parent_hash: &H256) { + let step = self.step.load(); + let empty_step_rlp = empty_step_rlp(step, parent_hash); + + if let Ok(signature) = self.sign(keccak(&empty_step_rlp)).map(Into::into) { + let message_rlp = empty_step_full_rlp(&signature, &empty_step_rlp); + + let parent_hash = *parent_hash; + let empty_step = EmptyStep { signature, step, parent_hash }; + + trace!(target: "engine", "broadcasting empty step message: {:?}", empty_step); + self.broadcast_message(message_rlp); + self.handle_empty_step_message(empty_step); + + } else { + warn!(target: "engine", "generate_empty_step: FAIL: accounts secret key unavailable"); + } + } + + fn broadcast_message(&self, message: Vec) { + if let Some(ref weak) = *self.client.read() { + if let Some(c) = weak.upgrade() { + c.broadcast_consensus_message(message); + } + } + } } fn unix_now() -> Duration { @@ -482,8 +732,11 @@ impl Engine for AuthorityRound { fn machine(&self) -> &EthereumMachine { &self.machine } - /// Two fields - consensus step and the corresponding proposer signature. - fn seal_fields(&self) -> usize { 2 } + /// Three fields - consensus step and the corresponding proposer signature, and a list of empty + /// step messages (which should be empty if no steps are skipped) + fn seal_fields(&self, header: &Header) -> usize { + header_expected_seal_fields(header, self.empty_steps_transition) + } fn step(&self) { self.step.increment(); @@ -497,10 +750,30 @@ impl Engine for AuthorityRound { /// Additional engine-specific information for the user/developer concerning `header`. fn extra_info(&self, header: &Header) -> BTreeMap { - map![ - "step".into() => header_step(header).as_ref().map(ToString::to_string).unwrap_or("".into()), - "signature".into() => header_signature(header).as_ref().map(ToString::to_string).unwrap_or("".into()) - ] + let step = header_step(header, self.empty_steps_transition).as_ref().map(ToString::to_string).unwrap_or("".into()); + let signature = header_signature(header, self.empty_steps_transition).as_ref().map(ToString::to_string).unwrap_or("".into()); + + let mut info = map![ + "step".into() => step, + "signature".into() => signature + ]; + + if header.number() >= self.empty_steps_transition { + let empty_steps = + if let Ok(empty_steps) = header_empty_steps(header).as_ref() { + format!("[{}]", + empty_steps.iter().fold( + "".to_string(), + |acc, e| if acc.len() > 0 { acc + ","} else { acc } + &e.to_string())) + + } else { + "".into() + }; + + info.insert("emptySteps".into(), empty_steps); + } + + info } fn maximum_uncle_count(&self, block: BlockNumber) -> usize { @@ -513,8 +786,16 @@ impl Engine for AuthorityRound { } fn populate_from_parent(&self, header: &mut Header, parent: &Header) { - let parent_step = header_step(parent).expect("Header has been verified; qed"); - let score = calculate_score(parent_step.into(), self.step.load().into()); + let parent_step = header_step(parent, self.empty_steps_transition).expect("Header has been verified; qed"); + let current_step = self.step.load(); + + let current_empty_steps_len = if header.number() >= self.empty_steps_transition { + self.empty_steps(parent_step.into(), current_step.into(), parent.hash()).len() + } else { + 0 + }; + + let score = calculate_score(parent_step.into(), current_step.into(), current_empty_steps_len.into()); header.set_difficulty(score); } @@ -523,6 +804,28 @@ impl Engine for AuthorityRound { Some(self.signer.read().is_some()) } + fn handle_message(&self, rlp: &[u8]) -> Result<(), EngineError> { + fn fmt_err(x: T) -> EngineError { + EngineError::MalformedMessage(format!("{:?}", x)) + } + + let rlp = UntrustedRlp::new(rlp); + let empty_step: EmptyStep = rlp.as_val().map_err(fmt_err)?;; + + if empty_step.verify(&*self.validators).unwrap_or(false) { + if self.step.check_future(empty_step.step).is_ok() { + trace!(target: "engine", "handle_message: received empty step message {:?}", empty_step); + self.handle_empty_step_message(empty_step); + } else { + trace!(target: "engine", "handle_message: empty step message from the future {:?}", empty_step); + } + } else { + trace!(target: "engine", "handle_message: received invalid step message {:?}", empty_step); + }; + + Ok(()) + } + /// Attempt to seal the block internally. /// /// This operation is synchronous and may (quite reasonably) not be available, in which case @@ -533,15 +836,22 @@ impl Engine for AuthorityRound { if !self.can_propose.load(AtomicOrdering::SeqCst) { return Seal::None; } let header = block.header(); - let parent_step: U256 = header_step(parent) + let parent_step: U256 = header_step(parent, self.empty_steps_transition) .expect("Header has been verified; qed").into(); let step = self.step.load(); - let expected_diff = calculate_score(parent_step, step.into()); + // filter messages from old and future steps and different parents + let empty_steps = if header.number() >= self.empty_steps_transition { + self.empty_steps(parent_step.into(), step.into(), *header.parent_hash()) + } else { + Vec::new() + }; + + let expected_diff = calculate_score(parent_step, step.into(), empty_steps.len().into()); if header.difficulty() != &expected_diff { - debug!(target: "engine", "Aborting seal generation. The step has changed in the meantime. {:?} != {:?}", + debug!(target: "engine", "Aborting seal generation. The step or empty_steps have changed in the meantime. {:?} != {:?}", header.difficulty(), expected_diff); return Seal::None; } @@ -584,12 +894,42 @@ impl Engine for AuthorityRound { return Seal::None; } - if let Ok(signature) = self.sign(header.bare_hash()) { + // if there are no transactions to include in the block, we don't seal and instead broadcast a signed + // `EmptyStep(step, parent_hash)` message. If we exceed the maximum amount of `empty_step` rounds we proceed + // with the seal. + if header.number() >= self.empty_steps_transition && + block.transactions().is_empty() && + empty_steps.len() < self.maximum_empty_steps { + + self.generate_empty_step(header.parent_hash()); + return Seal::None; + } + + let empty_steps_rlp = if header.number() >= self.empty_steps_transition { + let empty_steps: Vec<_> = empty_steps.iter().map(|e| e.sealed()).collect(); + Some(::rlp::encode_list(&empty_steps).into_vec()) + } else { + None + }; + + if let Ok(signature) = self.sign(header_seal_hash(header, empty_steps_rlp.as_ref().map(|e| &**e))) { trace!(target: "engine", "generate_seal: Issuing a block for step {}.", step); // only issue the seal if we were the first to reach the compare_and_swap. if self.can_propose.compare_and_swap(true, false, AtomicOrdering::SeqCst) { - return Seal::Regular(vec![encode(&step).into_vec(), encode(&(&H520::from(signature) as &[u8])).into_vec()]); + + self.clear_empty_steps(parent_step); + + let mut fields = vec![ + encode(&step).into_vec(), + encode(&(&H520::from(signature) as &[u8])).into_vec(), + ]; + + if let Some(empty_steps_rlp) = empty_steps_rlp { + fields.push(empty_steps_rlp); + } + + return Seal::Regular(fields); } } else { warn!(target: "engine", "generate_seal: FAIL: Accounts secret key unavailable."); @@ -635,8 +975,37 @@ impl Engine for AuthorityRound { /// Apply the block reward on finalisation of the block. fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error> { - // TODO: move to "machine::WithBalances" trait. - ::engines::common::bestow_block_reward(block, self.block_reward) + if block.header().number() >= self.empty_steps_transition { + let empty_steps = + if block.header().seal().is_empty() { + // this is a new block, calculate rewards based on the empty steps messages we have accumulated + let client = match self.client.read().as_ref().and_then(|weak| weak.upgrade()) { + Some(client) => client, + None => { + debug!(target: "engine", "Unable to close block: missing client ref."); + return Err(EngineError::RequiresClient.into()) + }, + }; + + let parent = client.block_header(::client::BlockId::Hash(*block.header().parent_hash())) + .expect("hash is from parent; parent header must exist; qed") + .decode(); + + let parent_step = header_step(&parent, self.empty_steps_transition)?; + let current_step = self.step.load(); + self.empty_steps(parent_step.into(), current_step.into(), parent.hash()) + } else { + // we're verifying a block, extract empty steps from the seal + header_empty_steps(block.header())? + }; + + for empty_step in empty_steps { + ::engines::common::bestow_block_reward(block, self.block_reward, empty_step.author()?)?; + } + } + + let author = *block.header().author(); + ::engines::common::bestow_block_reward(block, self.block_reward, author) } /// Check the number of seal fields. @@ -651,7 +1020,7 @@ impl Engine for AuthorityRound { // If yes then probably benign reporting needs to be moved further in the verification. let set_number = header.number(); - match verify_timestamp(&*self.step, header_step(header)?) { + match verify_timestamp(&*self.step, header_step(header, self.empty_steps_transition)?) { Err(BlockError::InvalidSeal) => { self.validators.report_benign(header.author(), set_number, header.number()); Err(BlockError::InvalidSeal.into()) @@ -663,8 +1032,8 @@ impl Engine for AuthorityRound { /// Do the step and gas limit validation. fn verify_block_family(&self, header: &Header, parent: &Header) -> Result<(), Error> { - let step = header_step(header)?; - let parent_step = header_step(parent)?; + let step = header_step(header, self.empty_steps_transition)?; + let parent_step = header_step(parent, self.empty_steps_transition)?; // TODO [ToDr] Should this go from epoch manager? let set_number = header.number(); @@ -677,19 +1046,50 @@ impl Engine for AuthorityRound { Err(EngineError::DoubleVote(header.author().clone()))?; } - // Report skipped primaries. - if let (true, Some(me)) = (step > parent_step + 1, self.signer.read().address()) { - debug!(target: "engine", "Author {} built block with step gap. current step: {}, parent step: {}", - header.author(), step, parent_step); - let mut reported = HashSet::new(); - for s in parent_step + 1..step { - let skipped_primary = step_proposer(&*self.validators, &parent.hash(), s); - // Do not report this signer. - if skipped_primary != me { - self.validators.report_benign(&skipped_primary, set_number, header.number()); + // If empty step messages are enabled we will validate the messages in the seal, missing messages are not + // reported as there's no way to tell whether the empty step message was never sent or simply not included. + if header.number() >= self.empty_steps_transition { + let validate_empty_steps = || -> Result<(), Error> { + let empty_steps = header_empty_steps(header)?; + for empty_step in empty_steps { + if empty_step.step <= parent_step || empty_step.step >= step { + Err(EngineError::InsufficientProof( + format!("empty step proof for invalid step: {:?}", empty_step.step)))?; + } + + if empty_step.parent_hash != *header.parent_hash() { + Err(EngineError::InsufficientProof( + format!("empty step proof for invalid parent hash: {:?}", empty_step.parent_hash)))?; + } + + if !empty_step.verify(&*self.validators).unwrap_or(false) { + Err(EngineError::InsufficientProof( + format!("invalid empty step proof: {:?}", empty_step)))?; + } } - // Stop reporting once validators start repeating. - if !reported.insert(skipped_primary) { break; } + Ok(()) + }; + + if let err @ Err(_) = validate_empty_steps() { + self.validators.report_benign(header.author(), set_number, header.number()); + return err; + } + + } else { + // Report skipped primaries. + if let (true, Some(me)) = (step > parent_step + 1, self.signer.read().address()) { + debug!(target: "engine", "Author {} built block with step gap. current step: {}, parent step: {}", + header.author(), step, parent_step); + let mut reported = HashSet::new(); + for s in parent_step + 1..step { + let skipped_primary = step_proposer(&*self.validators, &parent.hash(), s); + // Do not report this signer. + if skipped_primary != me { + self.validators.report_benign(&skipped_primary, set_number, header.number()); + // Stop reporting once validators start repeating. + if !reported.insert(skipped_primary) { break; } + } + } } } @@ -725,7 +1125,12 @@ impl Engine for AuthorityRound { // verify signature against fixed list, but reports should go to the // contract itself. - verify_external(header, validators) + let res = verify_external(header, validators, self.empty_steps_transition); + if res.is_ok() { + let header_step = header_step(header, self.empty_steps_transition)?; + self.clear_empty_steps(header_step.into()); + } + res } fn genesis_epoch_data(&self, header: &Header, call: &Call) -> Result, String> { @@ -780,20 +1185,39 @@ impl Engine for AuthorityRound { chain_head.hash(), chain_head.parent_hash()); let mut hash = chain_head.parent_hash().clone(); + let mut parent_empty_steps_signers = match header_empty_steps_signers(&chain_head, self.empty_steps_transition) { + Ok(empty_step_signers) => empty_step_signers, + Err(_) => { + warn!(target: "finality", "Failed to get empty step signatures from block {}", chain_head.hash()); + return None; + } + }; + let epoch_transition_hash = epoch_manager.epoch_transition_hash; // walk the chain within current epoch backwards. - // author == ec_recover(sig) known since - // the blocks are in the DB. + // author == ec_recover(sig) known since the blocks are in the DB. + // the empty steps messages in a header signal approval of the parent header. let ancestry_iter = itertools::repeat_call(move || { chain(hash).and_then(|header| { if header.number() == 0 { return None } - let res = (hash, header.author().clone()); - trace!(target: "finality", "Ancestry iteration: yielding {:?}", res); + let mut signers = vec![header.author().clone()]; + signers.extend(parent_empty_steps_signers.drain(..)); - hash = header.parent_hash().clone(); - Some(res) + if let Ok(empty_step_signers) = header_empty_steps_signers(&header, self.empty_steps_transition) { + let res = (hash, signers); + trace!(target: "finality", "Ancestry iteration: yielding {:?}", res); + + hash = header.parent_hash().clone(); + parent_empty_steps_signers = empty_step_signers; + + Some(res) + + } else { + warn!(target: "finality", "Failed to get empty step signatures from block {}", header.hash()); + None + } }) }) .while_some() @@ -806,7 +1230,7 @@ impl Engine for AuthorityRound { } { - if let Ok(finalized) = epoch_manager.finality_checker.push_hash(chain_head.hash(), *chain_head.author()) { + if let Ok(finalized) = epoch_manager.finality_checker.push_hash(chain_head.hash(), vec![chain_head.author().clone()]) { let mut finalized = finalized.into_iter(); while let Some(finalized_hash) = finalized.next() { if let Some(pending) = transition_store(finalized_hash) { @@ -860,6 +1284,7 @@ impl Engine for AuthorityRound { let verifier = Box::new(EpochVerifier { step: self.step.clone(), subchain_validators: list, + empty_steps_transition: self.empty_steps_transition, }); match finalize { @@ -898,16 +1323,18 @@ mod tests { use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering as AtomicOrdering}; use hash::keccak; - use ethereum_types::{H520, U256}; + use ethereum_types::{Address, H520, H256, U256}; use header::Header; use rlp::encode; use block::*; use tests::helpers::*; use account_provider::AccountProvider; use spec::Spec; - use engines::{Seal, Engine}; + use transaction::{Action, Transaction}; + use engines::{Seal, Engine, EngineError, EthEngine}; use engines::validator_set::TestSet; - use super::{AuthorityRoundParams, AuthorityRound}; + use error::Error; + use super::{AuthorityRoundParams, AuthorityRound, EmptyStep, SealedEmptyStep}; #[test] fn has_valid_metadata() { @@ -1085,6 +1512,8 @@ mod tests { immediate_transitions: true, maximum_uncle_count_transition: 0, maximum_uncle_count: 0, + empty_steps_transition: u64::max_value(), + maximum_empty_steps: 0, block_reward: Default::default(), }; @@ -1125,6 +1554,8 @@ mod tests { immediate_transitions: true, maximum_uncle_count_transition: 1, maximum_uncle_count: 0, + empty_steps_transition: u64::max_value(), + maximum_empty_steps: 0, block_reward: Default::default(), }; @@ -1177,6 +1608,8 @@ mod tests { immediate_transitions: true, maximum_uncle_count_transition: 0, maximum_uncle_count: 0, + empty_steps_transition: u64::max_value(), + maximum_empty_steps: 0, block_reward: Default::default(), }; @@ -1185,5 +1618,274 @@ mod tests { let machine = ::machine::EthereumMachine::regular(c_params, Default::default()); AuthorityRound::new(params, machine).unwrap(); } -} + fn setup_empty_steps() -> (Spec, Arc, Vec
) { + let spec = Spec::new_test_round_empty_steps(); + let tap = Arc::new(AccountProvider::transient_provider()); + + let addr1 = tap.insert_account(keccak("1").into(), "1").unwrap(); + let addr2 = tap.insert_account(keccak("0").into(), "0").unwrap(); + + let accounts = vec![addr1, addr2]; + + (spec, tap, accounts) + } + + fn empty_step(engine: &EthEngine, step: usize, parent_hash: &H256) -> EmptyStep { + let empty_step_rlp = super::empty_step_rlp(step, parent_hash); + let signature = engine.sign(keccak(&empty_step_rlp)).unwrap().into(); + let parent_hash = parent_hash.clone(); + EmptyStep { step, signature, parent_hash } + } + + fn sealed_empty_step(engine: &EthEngine, step: usize, parent_hash: &H256) -> SealedEmptyStep { + let empty_step_rlp = super::empty_step_rlp(step, parent_hash); + let signature = engine.sign(keccak(&empty_step_rlp)).unwrap().into(); + SealedEmptyStep { signature, step } + } + + #[test] + fn broadcast_empty_step_message() { + let (spec, tap, accounts) = setup_empty_steps(); + + let addr1 = accounts[0]; + + let engine = &*spec.engine; + let genesis_header = spec.genesis_header(); + let db1 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap(); + + let last_hashes = Arc::new(vec![genesis_header.hash()]); + let b1 = OpenBlock::new(engine, Default::default(), false, db1, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap(); + let b1 = b1.close_and_lock(); + + let client = generate_dummy_client(0); + let notify = Arc::new(TestNotify::default()); + client.add_notify(notify.clone()); + engine.register_client(Arc::downgrade(&client) as _); + + engine.set_signer(tap.clone(), addr1, "1".into()); + + // the block is empty so we don't seal and instead broadcast an empty step message + assert_eq!(engine.generate_seal(b1.block(), &genesis_header), Seal::None); + + // spec starts with step 2 + let empty_step_rlp = encode(&empty_step(engine, 2, &genesis_header.hash())).into_vec(); + + // we've received the message + assert!(notify.messages.read().contains(&empty_step_rlp)); + } + + #[test] + fn seal_with_empty_steps() { + let (spec, tap, accounts) = setup_empty_steps(); + + let addr1 = accounts[0]; + let addr2 = accounts[1]; + + let engine = &*spec.engine; + let genesis_header = spec.genesis_header(); + let db1 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap(); + let db2 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap(); + + let last_hashes = Arc::new(vec![genesis_header.hash()]); + + // step 2 + let b1 = OpenBlock::new(engine, Default::default(), false, db1, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap(); + let b1 = b1.close_and_lock(); + + // since the block is empty it isn't sealed and we generate empty steps + engine.set_signer(tap.clone(), addr1, "1".into()); + assert_eq!(engine.generate_seal(b1.block(), &genesis_header), Seal::None); + engine.step(); + + // step 3 + let mut b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes.clone(), addr2, (3141562.into(), 31415620.into()), vec![], false).unwrap(); + b2.push_transaction(Transaction { + action: Action::Create, + nonce: U256::from(0), + gas_price: U256::from(3000), + gas: U256::from(53_000), + value: U256::from(1), + data: vec![], + }.fake_sign(addr2), None).unwrap(); + let b2 = b2.close_and_lock(); + + // we will now seal a block with 1tx and include the accumulated empty step message + engine.set_signer(tap.clone(), addr2, "0".into()); + if let Seal::Regular(seal) = engine.generate_seal(b2.block(), &genesis_header) { + engine.set_signer(tap.clone(), addr1, "1".into()); + let empty_step2 = sealed_empty_step(engine, 2, &genesis_header.hash()); + let empty_steps = ::rlp::encode_list(&vec![empty_step2]); + + assert_eq!(seal[0], encode(&3usize).into_vec()); + assert_eq!(seal[2], empty_steps.into_vec()); + } + } + + #[test] + fn seal_empty_block_with_empty_steps() { + let (spec, tap, accounts) = setup_empty_steps(); + + let addr1 = accounts[0]; + let addr2 = accounts[1]; + + let engine = &*spec.engine; + let genesis_header = spec.genesis_header(); + let db1 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap(); + let db2 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap(); + let db3 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap(); + + let last_hashes = Arc::new(vec![genesis_header.hash()]); + + // step 2 + let b1 = OpenBlock::new(engine, Default::default(), false, db1, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap(); + let b1 = b1.close_and_lock(); + + // since the block is empty it isn't sealed and we generate empty steps + engine.set_signer(tap.clone(), addr1, "1".into()); + assert_eq!(engine.generate_seal(b1.block(), &genesis_header), Seal::None); + engine.step(); + + // step 3 + let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes.clone(), addr2, (3141562.into(), 31415620.into()), vec![], false).unwrap(); + let b2 = b2.close_and_lock(); + engine.set_signer(tap.clone(), addr2, "0".into()); + assert_eq!(engine.generate_seal(b2.block(), &genesis_header), Seal::None); + engine.step(); + + // step 4 + // the spec sets the maximum_empty_steps to 2 so we will now seal an empty block and include the empty step messages + let b3 = OpenBlock::new(engine, Default::default(), false, db3, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap(); + let b3 = b3.close_and_lock(); + + engine.set_signer(tap.clone(), addr1, "1".into()); + if let Seal::Regular(seal) = engine.generate_seal(b3.block(), &genesis_header) { + let empty_step2 = sealed_empty_step(engine, 2, &genesis_header.hash()); + engine.set_signer(tap.clone(), addr2, "0".into()); + let empty_step3 = sealed_empty_step(engine, 3, &genesis_header.hash()); + + let empty_steps = ::rlp::encode_list(&vec![empty_step2, empty_step3]); + + assert_eq!(seal[0], encode(&4usize).into_vec()); + assert_eq!(seal[2], empty_steps.into_vec()); + } + } + + #[test] + fn reward_empty_steps() { + let (spec, tap, accounts) = setup_empty_steps(); + + let addr1 = accounts[0]; + + let engine = &*spec.engine; + let genesis_header = spec.genesis_header(); + let db1 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap(); + let db2 = spec.ensure_db_good(get_temp_state_db(), &Default::default()).unwrap(); + + let last_hashes = Arc::new(vec![genesis_header.hash()]); + + let client = generate_dummy_client_with_spec_and_accounts(Spec::new_test_round_empty_steps, None); + engine.register_client(Arc::downgrade(&client) as _); + + // step 2 + let b1 = OpenBlock::new(engine, Default::default(), false, db1, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap(); + let b1 = b1.close_and_lock(); + + // since the block is empty it isn't sealed and we generate empty steps + engine.set_signer(tap.clone(), addr1, "1".into()); + assert_eq!(engine.generate_seal(b1.block(), &genesis_header), Seal::None); + engine.step(); + + // step 3 + // the signer of the accumulated empty step message should be rewarded + let b2 = OpenBlock::new(engine, Default::default(), false, db2, &genesis_header, last_hashes.clone(), addr1, (3141562.into(), 31415620.into()), vec![], false).unwrap(); + let addr1_balance = b2.block().fields().state.balance(&addr1).unwrap(); + + // after closing the block `addr1` should be reward twice, one for the included empty step message and another for block creation + let b2 = b2.close_and_lock(); + + // the spec sets the block reward to 10 + assert_eq!(b2.block().fields().state.balance(&addr1).unwrap(), addr1_balance + (10 * 2).into()) + } + + #[test] + fn verify_seal_empty_steps() { + let (spec, tap, accounts) = setup_empty_steps(); + let addr1 = accounts[0]; + let addr2 = accounts[1]; + let engine = &*spec.engine; + + let mut parent_header: Header = Header::default(); + parent_header.set_seal(vec![encode(&0usize).into_vec()]); + parent_header.set_gas_limit("222222".parse::().unwrap()); + + let mut header: Header = Header::default(); + header.set_parent_hash(parent_header.hash()); + header.set_number(1); + header.set_gas_limit("222222".parse::().unwrap()); + header.set_author(addr1); + + let signature = tap.sign(addr1, Some("1".into()), header.bare_hash()).unwrap(); + + // empty step with invalid step + let empty_steps = vec![SealedEmptyStep { signature: 0.into(), step: 2 }]; + header.set_seal(vec![ + encode(&2usize).into_vec(), + encode(&(&*signature as &[u8])).into_vec(), + ::rlp::encode_list(&empty_steps).into_vec(), + ]); + + assert!(match engine.verify_block_family(&header, &parent_header) { + Err(Error::Engine(EngineError::InsufficientProof(ref s))) + if s.contains("invalid step") => true, + _ => false, + }); + + // empty step with invalid signature + let empty_steps = vec![SealedEmptyStep { signature: 0.into(), step: 1 }]; + header.set_seal(vec![ + encode(&2usize).into_vec(), + encode(&(&*signature as &[u8])).into_vec(), + ::rlp::encode_list(&empty_steps).into_vec(), + ]); + + assert!(match engine.verify_block_family(&header, &parent_header) { + Err(Error::Engine(EngineError::InsufficientProof(ref s))) + if s.contains("invalid empty step proof") => true, + _ => false, + }); + + // empty step with valid signature from incorrect proposer for step + engine.set_signer(tap.clone(), addr1, "1".into()); + let empty_steps = vec![sealed_empty_step(engine, 1, &parent_header.hash())]; + header.set_seal(vec![ + encode(&2usize).into_vec(), + encode(&(&*signature as &[u8])).into_vec(), + ::rlp::encode_list(&empty_steps).into_vec(), + ]); + + assert!(match engine.verify_block_family(&header, &parent_header) { + Err(Error::Engine(EngineError::InsufficientProof(ref s))) + if s.contains("invalid empty step proof") => true, + _ => false, + }); + + // valid empty steps + engine.set_signer(tap.clone(), addr1, "1".into()); + let empty_step2 = sealed_empty_step(engine, 2, &parent_header.hash()); + engine.set_signer(tap.clone(), addr2, "0".into()); + let empty_step3 = sealed_empty_step(engine, 3, &parent_header.hash()); + + let empty_steps = vec![empty_step2, empty_step3]; + header.set_seal(vec![ + encode(&4usize).into_vec(), + encode(&(&*signature as &[u8])).into_vec(), + ::rlp::encode_list(&empty_steps).into_vec(), + ]); + + assert!(match engine.verify_block_family(&header, &parent_header) { + Ok(_) => true, + _ => false, + }); + } +} diff --git a/ethcore/src/engines/basic_authority.rs b/ethcore/src/engines/basic_authority.rs index 82c1ac845..c6bb0a115 100644 --- a/ethcore/src/engines/basic_authority.rs +++ b/ethcore/src/engines/basic_authority.rs @@ -97,7 +97,7 @@ impl Engine for BasicAuthority { fn machine(&self) -> &EthereumMachine { &self.machine } // One field - the signature - fn seal_fields(&self) -> usize { 1 } + fn seal_fields(&self, _header: &Header) -> usize { 1 } fn seals_internally(&self) -> Option { Some(self.signer.read().is_some()) diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index 9185a9300..7964c55a4 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -181,7 +181,7 @@ pub trait Engine: Sync + Send { fn machine(&self) -> &M; /// The number of additional header fields required for this engine. - fn seal_fields(&self) -> usize { 0 } + fn seal_fields(&self, _header: &M::Header) -> usize { 0 } /// Additional engine-specific information for the user/developer concerning `header`. fn extra_info(&self, _header: &M::Header) -> BTreeMap { BTreeMap::new() } @@ -406,27 +406,27 @@ pub mod common { use trace::{Tracer, ExecutiveTracer, RewardType}; use state::CleanupMode; - use ethereum_types::U256; + use ethereum_types::{Address, U256}; /// Give reward and trace. - pub fn bestow_block_reward(block: &mut ExecutedBlock, reward: U256) -> Result<(), Error> { + pub fn bestow_block_reward(block: &mut ExecutedBlock, reward: U256, receiver: Address) -> Result<(), Error> { let fields = block.fields_mut(); + // Bestow block reward - let res = fields.state.add_balance(fields.header.author(), &reward, CleanupMode::NoEmpty) + let res = fields.state.add_balance(&receiver, &reward, CleanupMode::NoEmpty) .map_err(::error::Error::from) .and_then(|_| fields.state.commit()); - let block_author = fields.header.author().clone(); fields.traces.as_mut().map(move |traces| { let mut tracer = ExecutiveTracer::default(); - tracer.trace_reward(block_author, reward, RewardType::Block); + tracer.trace_reward(receiver, reward, RewardType::Block); traces.push(tracer.drain()) }); - // Commit state so that we can actually figure out the state root. if let Err(ref e) = res { warn!("Encountered error on bestowing reward: {}", e); } + res } } diff --git a/ethcore/src/engines/tendermint/mod.rs b/ethcore/src/engines/tendermint/mod.rs index fd32294f4..338eddc7a 100644 --- a/ethcore/src/engines/tendermint/mod.rs +++ b/ethcore/src/engines/tendermint/mod.rs @@ -441,7 +441,7 @@ impl Engine for Tendermint { fn name(&self) -> &str { "Tendermint" } /// (consensus view, proposal signature, authority signatures) - fn seal_fields(&self) -> usize { 3 } + fn seal_fields(&self, _header: &Header) -> usize { 3 } fn machine(&self) -> &EthereumMachine { &self.machine } @@ -561,7 +561,8 @@ impl Engine for Tendermint { /// Apply the block reward on finalisation of the block. fn on_close_block(&self, block: &mut ExecutedBlock) -> Result<(), Error>{ - ::engines::common::bestow_block_reward(block, self.block_reward) + let author = *block.header().author(); + ::engines::common::bestow_block_reward(block, self.block_reward, author) } fn verify_local_seal(&self, _header: &Header) -> Result<(), Error> { @@ -570,7 +571,8 @@ impl Engine for Tendermint { fn verify_block_basic(&self, header: &Header) -> Result<(), Error> { let seal_length = header.seal().len(); - if seal_length == self.seal_fields() { + let expected_seal_fields = self.seal_fields(header); + if seal_length == expected_seal_fields { // Either proposal or commit. if (header.seal()[1] == ::rlp::NULL_RLP) != (header.seal()[2] == ::rlp::EMPTY_LIST_RLP) { @@ -581,7 +583,7 @@ impl Engine for Tendermint { } } else { Err(BlockError::InvalidSealArity( - Mismatch { expected: self.seal_fields(), found: seal_length } + Mismatch { expected: expected_seal_fields, found: seal_length } ).into()) } } @@ -777,7 +779,6 @@ mod tests { use block::*; use error::{Error, BlockError}; use header::Header; - use client::ChainNotify; use miner::MinerService; use tests::helpers::*; use account_provider::AccountProvider; @@ -837,17 +838,6 @@ mod tests { addr } - #[derive(Default)] - struct TestNotify { - messages: RwLock>, - } - - impl ChainNotify for TestNotify { - fn broadcast(&self, data: Vec) { - self.messages.write().push(data); - } - } - #[test] fn has_valid_metadata() { let engine = Spec::new_test_tendermint().engine; diff --git a/ethcore/src/engines/validator_set/contract.rs b/ethcore/src/engines/validator_set/contract.rs index 545b2eaae..20c43345c 100644 --- a/ethcore/src/engines/validator_set/contract.rs +++ b/ethcore/src/engines/validator_set/contract.rs @@ -174,7 +174,7 @@ mod tests { // Check a block that is a bit in future, reject it but don't report the validator. let mut header = Header::default(); - let seal = vec![encode(&5u8).into_vec(), encode(&(&H520::default() as &[u8])).into_vec()]; + let seal = vec![encode(&4u8).into_vec(), encode(&(&H520::default() as &[u8])).into_vec()]; header.set_seal(seal); header.set_author(v1); header.set_number(2); diff --git a/ethcore/src/ethereum/ethash.rs b/ethcore/src/ethereum/ethash.rs index 4d3bc46f9..a9ee79f30 100644 --- a/ethcore/src/ethereum/ethash.rs +++ b/ethcore/src/ethereum/ethash.rs @@ -169,11 +169,11 @@ impl Engine for Arc { fn machine(&self) -> &EthereumMachine { &self.machine } // Two fields - nonce and mix. - fn seal_fields(&self) -> usize { 2 } + fn seal_fields(&self, _header: &Header) -> usize { 2 } /// Additional engine-specific information for the user/developer concerning `header`. fn extra_info(&self, header: &Header) -> BTreeMap { - if header.seal().len() == self.seal_fields() { + if header.seal().len() == self.seal_fields(header) { map![ "nonce".to_owned() => format!("0x{:x}", header.nonce()), "mixHash".to_owned() => format!("0x{:x}", header.mix_hash()) @@ -265,9 +265,10 @@ impl Engine for Arc { fn verify_block_basic(&self, header: &Header) -> Result<(), Error> { // check the seal fields. - if header.seal().len() != self.seal_fields() { + let expected_seal_fields = self.seal_fields(header); + if header.seal().len() != expected_seal_fields { return Err(From::from(BlockError::InvalidSealArity( - Mismatch { expected: self.seal_fields(), found: header.seal().len() } + Mismatch { expected: expected_seal_fields, found: header.seal().len() } ))); } UntrustedRlp::new(&header.seal()[0]).as_val::()?; @@ -296,9 +297,10 @@ impl Engine for Arc { } fn verify_block_unordered(&self, header: &Header) -> Result<(), Error> { - if header.seal().len() != self.seal_fields() { + let expected_seal_fields = self.seal_fields(header); + if header.seal().len() != expected_seal_fields { return Err(From::from(BlockError::InvalidSealArity( - Mismatch { expected: self.seal_fields(), found: header.seal().len() } + Mismatch { expected: expected_seal_fields, found: header.seal().len() } ))); } let result = self.pow.compute_light(header.number() as u64, &header.bare_hash().0, header.nonce().low_u64()); diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index 303f08eb5..92e35de54 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -762,6 +762,13 @@ impl Spec { load_bundled!("authority_round") } + /// Create a new Spec with AuthorityRound consensus which does internal sealing (not + /// requiring work) with empty step messages enabled. + /// Accounts with secrets keccak("0") and keccak("1") are the validators. + pub fn new_test_round_empty_steps() -> Self { + load_bundled!("authority_round_empty_steps") + } + /// Create a new Spec with Tendermint consensus which does internal sealing (not requiring /// work). /// Account keccak("0") and keccak("1") are a authorities. diff --git a/ethcore/src/tests/helpers.rs b/ethcore/src/tests/helpers.rs index 626aa8349..9a88b0b3d 100644 --- a/ethcore/src/tests/helpers.rs +++ b/ethcore/src/tests/helpers.rs @@ -19,7 +19,7 @@ use ethereum_types::{H256, U256}; use block::{OpenBlock, Drain}; use blockchain::{BlockChain, Config as BlockChainConfig}; use bytes::Bytes; -use client::{BlockChainClient, Client, ClientConfig}; +use client::{BlockChainClient, ChainNotify, Client, ClientConfig}; use ethereum::ethash::EthashParams; use ethkey::KeyPair; use evm::Factory as EvmFactory; @@ -29,6 +29,7 @@ use header::Header; use io::*; use machine::EthashExtensions; use miner::Miner; +use parking_lot::RwLock; use rlp::{self, RlpStream}; use spec::*; use state_db::StateDB; @@ -388,3 +389,14 @@ pub fn get_default_ethash_params() -> EthashParams { expip2_duration_limit: 30, } } + +#[derive(Default)] +pub struct TestNotify { + pub messages: RwLock>, +} + +impl ChainNotify for TestNotify { + fn broadcast(&self, data: Vec) { + self.messages.write().push(data); + } +} diff --git a/ethcore/src/verification/verification.rs b/ethcore/src/verification/verification.rs index c9b33ce46..6a7c2c788 100644 --- a/ethcore/src/verification/verification.rs +++ b/ethcore/src/verification/verification.rs @@ -242,9 +242,10 @@ pub fn verify_block_final(expected: &Header, got: &Header) -> Result<(), Error> /// Check basic header parameters. pub fn verify_header_params(header: &Header, engine: &EthEngine, is_full: bool) -> Result<(), Error> { - if header.seal().len() != engine.seal_fields() { + let expected_seal_fields = engine.seal_fields(header); + if header.seal().len() != expected_seal_fields { return Err(From::from(BlockError::InvalidSealArity( - Mismatch { expected: engine.seal_fields(), found: header.seal().len() } + Mismatch { expected: expected_seal_fields, found: header.seal().len() } ))); } diff --git a/json/src/spec/authority_round.rs b/json/src/spec/authority_round.rs index f84d51201..4dea4f098 100644 --- a/json/src/spec/authority_round.rs +++ b/json/src/spec/authority_round.rs @@ -49,6 +49,12 @@ pub struct AuthorityRoundParams { /// Maximum number of accepted uncles. #[serde(rename="maximumUncleCount")] pub maximum_uncle_count: Option, + /// Block at which empty step messages should start. + #[serde(rename="emptyStepsTransition")] + pub empty_steps_transition: Option, + /// Maximum number of accepted empty steps. + #[serde(rename="maximumEmptySteps")] + pub maximum_empty_steps: Option, } /// Authority engine deserialization.