diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index eef870a3f58..2330e5c29f3 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -216,12 +216,14 @@ impl pallet_aura::Trait for Runtime { } parameter_types! { + pub const FinalityVotesCachingInterval: Option = Some(16); pub const KovanAuraConfiguration: pallet_bridge_eth_poa::AuraConfiguration = kovan::kovan_aura_configuration(); pub const KovanValidatorsConfiguration: pallet_bridge_eth_poa::ValidatorsConfiguration = kovan::kovan_validators_configuration(); } impl pallet_bridge_eth_poa::Trait for Runtime { type AuraConfiguration = KovanAuraConfiguration; + type FinalityVotesCachingInterval = FinalityVotesCachingInterval; type ValidatorsConfiguration = KovanValidatorsConfiguration; type OnHeadersSubmitted = (); } @@ -495,7 +497,8 @@ impl_runtime_apis! { impl sp_bridge_eth_poa::EthereumHeadersApi for Runtime { fn best_block() -> (u64, sp_bridge_eth_poa::H256) { - BridgeEthPoA::best_block() + let best_block = BridgeEthPoA::best_block(); + (best_block.number, best_block.hash) } fn is_import_requires_receipts(header: sp_bridge_eth_poa::Header) -> bool { diff --git a/modules/ethereum/src/finality.rs b/modules/ethereum/src/finality.rs index 70d79fb20d0..0fb5f1f7774 100644 --- a/modules/ethereum/src/finality.rs +++ b/modules/ethereum/src/finality.rs @@ -16,55 +16,108 @@ use crate::error::Error; use crate::Storage; -use primitives::{public_to_address, Address, Header, SealedEmptyStep, H256}; +use codec::{Decode, Encode}; +use primitives::{public_to_address, Address, Header, HeaderId, SealedEmptyStep, H256}; use sp_io::crypto::secp256k1_ecdsa_recover; -use sp_std::prelude::*; -use sp_std::{ - collections::{ - btree_map::{BTreeMap, Entry}, - btree_set::BTreeSet, - vec_deque::VecDeque, - }, - iter::from_fn, +use sp_runtime::RuntimeDebug; +use sp_std::collections::{ + btree_map::{BTreeMap, Entry}, + btree_set::BTreeSet, + vec_deque::VecDeque, }; +use sp_std::prelude::*; + +/// Cached finality votes for given block. +#[derive(RuntimeDebug, Default)] +#[cfg_attr(test, derive(PartialEq))] +pub struct CachedFinalityVotes { + /// Header ancestors that were read while we have been searching for + /// cached votes entry. Newest header has index 0. + pub unaccounted_ancestry: VecDeque<(HeaderId, Option, Header)>, + /// Cached finality votes, if they have been found. The associated + /// header is not included into `unaccounted_ancestry`. + pub votes: Option>, +} + +/// Finality effects. +#[derive(RuntimeDebug)] +#[cfg_attr(test, derive(PartialEq))] +pub struct FinalityEffects { + /// Finalized headers. + pub finalized_headers: Vec<(HeaderId, Option)>, + /// Finality votes used in computation. + pub votes: FinalityVotes, +} + +/// Finality votes for given block. +#[derive(RuntimeDebug, Decode, Encode)] +#[cfg_attr(test, derive(Clone, PartialEq))] +pub struct FinalityVotes { + /// Number of votes per each validator. + pub votes: BTreeMap, + /// Ancestry blocks with oldest ancestors at the beginning and newest at the + /// end of the queue. + pub ancestry: VecDeque>, +} + +/// Information about block ancestor that is used in computations. +#[derive(RuntimeDebug, Decode, Encode)] +#[cfg_attr(test, derive(Clone, Default, PartialEq))] +pub struct FinalityAncestor { + /// Bock id. + pub id: HeaderId, + /// Block submitter. + pub submitter: Option, + /// Validators that have signed this block and empty steps on top + /// of this block. + pub signers: BTreeSet
, +} /// Tries to finalize blocks when given block is imported. /// /// Returns numbers and hashes of finalized blocks in ascending order. pub fn finalize_blocks( storage: &S, - best_finalized_hash: &H256, - header_validators: (&H256, &[Address]), - hash: &H256, + best_finalized: HeaderId, + header_validators: (HeaderId, &[Address]), + id: HeaderId, submitter: Option<&S::Submitter>, header: &Header, two_thirds_majority_transition: u64, -) -> Result)>, Error> { +) -> Result, Error> { // compute count of voters for every unfinalized block in ancestry let validators = header_validators.1.iter().collect(); - let (mut votes, mut headers) = prepare_votes( - storage, - best_finalized_hash, - &header_validators.0, + let votes = prepare_votes( + storage.cached_finality_votes(&header.parent_hash, |hash| { + *hash == header_validators.0.hash || *hash == best_finalized.hash + }), + best_finalized.number, &validators, - hash, + id, header, - submitter, - two_thirds_majority_transition, + submitter.cloned(), )?; // now let's iterate in reverse order && find just finalized blocks - let mut newly_finalized = Vec::new(); - while let Some((oldest_hash, oldest_number, submitter, signers)) = headers.pop_front() { - if !is_finalized(&validators, &votes, oldest_number >= two_thirds_majority_transition) { + let mut finalized_headers = Vec::new(); + let mut current_votes = votes.votes.clone(); + for ancestor in &votes.ancestry { + if !is_finalized( + &validators, + ¤t_votes, + ancestor.id.number >= two_thirds_majority_transition, + ) { break; } - remove_signers_votes(&signers, &mut votes); - newly_finalized.push((oldest_number, oldest_hash, submitter)); + remove_signers_votes(&ancestor.signers, &mut current_votes); + finalized_headers.push((ancestor.id, ancestor.submitter.clone())); } - Ok(newly_finalized) + Ok(FinalityEffects { + finalized_headers, + votes, + }) } /// Returns true if there are enough votes to treat this header as finalized. @@ -78,67 +131,66 @@ fn is_finalized( } /// Prepare 'votes' of header and its ancestors' signers. -fn prepare_votes( - storage: &S, - best_finalized_hash: &H256, - validators_begin: &H256, +fn prepare_votes( + mut cached_votes: CachedFinalityVotes, + best_finalized_number: u64, validators: &BTreeSet<&Address>, - hash: &H256, + id: HeaderId, header: &Header, - submitter: Option<&S::Submitter>, - two_thirds_majority_transition: u64, -) -> Result< - ( - BTreeMap, - VecDeque<(H256, u64, Option, BTreeSet
)>, - ), - Error, -> { + submitter: Option, +) -> Result, Error> { // this fn can only work with single validators set if !validators.contains(&header.author) { return Err(Error::NotValidator); } - // prepare iterator of signers of all ancestors of the header - // we only take ancestors that are not yet pruned and those signed by - // the same set of validators - let mut parent_empty_step_signers = empty_steps_signers(header); - let ancestry = ancestry(storage, header.parent_hash) - .map(|(hash, header, submitter)| { - let mut signers = BTreeSet::new(); - sp_std::mem::swap(&mut signers, &mut parent_empty_step_signers); - signers.insert(header.author); - - let empty_step_signers = empty_steps_signers(&header); - let res = (hash, header.number, submitter, signers); - parent_empty_step_signers = empty_step_signers; - res - }) - .take_while(|&(hash, _, _, _)| hash != *validators_begin && hash != *best_finalized_hash); - - // now let's iterate built iterator and compute number of validators - // 'voted' for each header - // we stop when finalized block is met (because we only interested in - // just finalized blocks) - let mut votes = BTreeMap::new(); - let mut headers = VecDeque::new(); - for (hash, number, submitter, signers) in ancestry { - add_signers_votes(validators, &signers, &mut votes)?; - if is_finalized(validators, &votes, number >= two_thirds_majority_transition) { - remove_signers_votes(&signers, &mut votes); + // now we have votes that were valid when some block B has been inserted + // things may have changed a bit, but we do not need to read anything else + // from the db, because we have ancestry + // so the only thing we need to do is: + // 1) remove votes from blocks that have been finalized after B has been inserted; + // 2) add votes from B descendants + let mut votes = cached_votes.votes.unwrap_or_default(); + + // remove votes from finalized blocks + while let Some(old_ancestor) = votes.ancestry.pop_front() { + if old_ancestor.id.number > best_finalized_number { + votes.ancestry.push_front(old_ancestor); break; } - headers.push_front((hash, number, submitter, signers)); + remove_signers_votes(&old_ancestor.signers, &mut votes.votes); + } + + // add votes from new blocks + let mut parent_empty_step_signers = empty_steps_signers(header); + let mut unaccounted_ancestry = VecDeque::new(); + while let Some((ancestor_id, ancestor_submitter, ancestor)) = cached_votes.unaccounted_ancestry.pop_front() { + let mut signers = empty_steps_signers(&ancestor); + sp_std::mem::swap(&mut signers, &mut parent_empty_step_signers); + signers.insert(ancestor.author); + + add_signers_votes(validators, &signers, &mut votes.votes)?; + + unaccounted_ancestry.push_front(FinalityAncestor { + id: ancestor_id, + submitter: ancestor_submitter, + signers, + }); } + votes.ancestry.extend(unaccounted_ancestry); - // update votes with last header vote + // add votes from block itself let mut header_signers = BTreeSet::new(); header_signers.insert(header.author); - *votes.entry(header.author).or_insert(0) += 1; - headers.push_back((*hash, header.number, submitter.cloned(), header_signers)); + *votes.votes.entry(header.author).or_insert(0) += 1; + votes.ancestry.push_back(FinalityAncestor { + id, + submitter, + signers: header_signers, + }); - Ok((votes, headers)) + Ok(votes) } /// Increase count of 'votes' for every passed signer. @@ -193,28 +245,21 @@ fn empty_step_signer(empty_step: &SealedEmptyStep, parent_hash: &H256) -> Option .map(|public| public_to_address(&public)) } -/// Return iterator of given header ancestors. -pub(crate) fn ancestry<'a, S: Storage>( - storage: &'a S, - mut parent_hash: H256, -) -> impl Iterator)> + 'a { - from_fn(move || { - let (header, submitter) = storage.header(&parent_hash)?; - if header.number == 0 { - return None; +impl Default for FinalityVotes { + fn default() -> Self { + FinalityVotes { + votes: BTreeMap::new(), + ancestry: VecDeque::new(), } - - let hash = parent_hash.clone(); - parent_hash = header.parent_hash.clone(); - Some((hash, header, submitter)) - }) + } } #[cfg(test)] mod tests { use super::*; - use crate::mock::{custom_test_ext, genesis, validator, validators_addresses, TestRuntime}; - use crate::{BridgeStorage, HeaderToImport}; + use crate::mock::{custom_test_ext, genesis, insert_header, validator, validators_addresses, TestRuntime}; + use crate::{BridgeStorage, FinalityCache, HeaderToImport}; + use frame_support::StorageMap; #[test] fn verifies_header_author() { @@ -222,9 +267,9 @@ mod tests { assert_eq!( finalize_blocks( &BridgeStorage::::new(), - &Default::default(), - (&Default::default(), &[]), - &Default::default(), + Default::default(), + (Default::default(), &[]), + Default::default(), None, &Header::default(), 0, @@ -235,7 +280,7 @@ mod tests { } #[test] - fn prepares_votes() { + fn finalize_blocks_works() { custom_test_ext(genesis(), validators_addresses(5)).execute_with(|| { // let's say we have 5 validators (we need 'votes' from 3 validators to achieve // finality) @@ -244,30 +289,32 @@ mod tests { // when header#1 is inserted, nothing is finalized (1 vote) let header1 = Header { author: validator(0).address().as_fixed_bytes().into(), - parent_hash: genesis().hash(), + parent_hash: genesis().compute_hash(), number: 1, ..Default::default() }; - let hash1 = header1.hash(); + let id1 = header1.compute_id(); let mut header_to_import = HeaderToImport { - context: storage.import_context(None, &genesis().hash()).unwrap(), + context: storage.import_context(None, &genesis().compute_hash()).unwrap(), is_best: true, - hash: hash1, + id: id1, header: header1, total_difficulty: 0.into(), enacted_change: None, scheduled_change: None, + finality_votes: Default::default(), }; assert_eq!( finalize_blocks( &storage, - &Default::default(), - (&Default::default(), &validators_addresses(5)), - &hash1, + Default::default(), + (Default::default(), &validators_addresses(5)), + id1, None, &header_to_import.header, u64::max_value(), - ), + ) + .map(|eff| eff.finalized_headers), Ok(Vec::new()), ); storage.insert_header(header_to_import.clone()); @@ -275,22 +322,23 @@ mod tests { // when header#2 is inserted, nothing is finalized (2 votes) header_to_import.header = Header { author: validator(1).address().as_fixed_bytes().into(), - parent_hash: hash1, + parent_hash: id1.hash, number: 2, ..Default::default() }; - header_to_import.hash = header_to_import.header.hash(); - let hash2 = header_to_import.header.hash(); + header_to_import.id = header_to_import.header.compute_id(); + let id2 = header_to_import.header.compute_id(); assert_eq!( finalize_blocks( &storage, - &Default::default(), - (&Default::default(), &validators_addresses(5)), - &hash2, + Default::default(), + (Default::default(), &validators_addresses(5)), + id2, None, &header_to_import.header, u64::max_value(), - ), + ) + .map(|eff| eff.finalized_headers), Ok(Vec::new()), ); storage.insert_header(header_to_import.clone()); @@ -298,25 +346,192 @@ mod tests { // when header#3 is inserted, header#1 is finalized (3 votes) header_to_import.header = Header { author: validator(2).address().as_fixed_bytes().into(), - parent_hash: hash2, + parent_hash: id2.hash, number: 3, ..Default::default() }; - header_to_import.hash = header_to_import.header.hash(); - let hash3 = header_to_import.header.hash(); + header_to_import.id = header_to_import.header.compute_id(); + let id3 = header_to_import.header.compute_id(); assert_eq!( finalize_blocks( &storage, - &Default::default(), - (&Default::default(), &validators_addresses(5)), - &hash3, + Default::default(), + (Default::default(), &validators_addresses(5)), + id3, None, &header_to_import.header, u64::max_value(), - ), - Ok(vec![(1, hash1, None)]), + ) + .map(|eff| eff.finalized_headers), + Ok(vec![(id1, None)]), ); storage.insert_header(header_to_import); }); } + + #[test] + fn cached_votes_are_updated_with_ancestry() { + // we're inserting header#5 + // cached votes are from header#3 + // header#4 has finalized header#1 and header#2 + // => when inserting header#5, we need to: + // 1) remove votes from header#1 and header#2 + // 2) add votes from header#4 and header#5 + let validators = validators_addresses(5); + let headers = (1..6) + .map(|number| Header { + number: number, + author: validators[number as usize - 1], + ..Default::default() + }) + .collect::>(); + let ancestry = headers + .iter() + .map(|header| FinalityAncestor { + id: header.compute_id(), + signers: vec![header.author].into_iter().collect(), + ..Default::default() + }) + .collect::>(); + let header5 = headers[4].clone(); + assert_eq!( + prepare_votes::<()>( + CachedFinalityVotes { + unaccounted_ancestry: vec![(headers[3].compute_id(), None, headers[3].clone()),] + .into_iter() + .collect(), + votes: Some(FinalityVotes { + votes: vec![(validators[0], 1), (validators[1], 1), (validators[2], 1),] + .into_iter() + .collect(), + ancestry: ancestry[..3].iter().cloned().collect(), + }), + }, + 2, + &validators.iter().collect(), + header5.compute_id(), + &header5, + None, + ) + .unwrap(), + FinalityVotes { + votes: vec![(validators[2], 1), (validators[3], 1), (validators[4], 1),] + .into_iter() + .collect(), + ancestry: ancestry[2..].iter().cloned().collect(), + }, + ); + } + + #[test] + fn prepare_votes_respects_finality_cache() { + let validators_addresses = validators_addresses(5); + custom_test_ext(genesis(), validators_addresses.clone()).execute_with(move || { + // we need signatures of 3 validators to finalize block + let mut storage = BridgeStorage::::new(); + + // headers 1..3 are signed by validator#0 + // headers 4..6 are signed by validator#1 + // headers 7..9 are signed by validator#2 + let mut hashes = Vec::new(); + let mut headers = Vec::new(); + let mut ancestry = Vec::new(); + let mut parent_hash = genesis().compute_hash(); + for i in 1..10 { + let header = Header { + author: validator((i - 1) / 3).address().as_fixed_bytes().into(), + parent_hash, + number: i as _, + ..Default::default() + }; + let id = header.compute_id(); + insert_header(&mut storage, header.clone()); + hashes.push(id.hash); + ancestry.push(FinalityAncestor { + id: header.compute_id(), + submitter: None, + signers: vec![header.author].into_iter().collect(), + }); + headers.push(header); + parent_hash = id.hash; + } + + // when we're inserting header#7 and last finalized header is 0: + // check that votes at #7 are computed correctly without cache + let expected_votes_at_7 = FinalityVotes { + votes: vec![ + (validators_addresses[0].clone(), 3), + (validators_addresses[1].clone(), 3), + (validators_addresses[2].clone(), 1), + ] + .into_iter() + .collect(), + ancestry: ancestry[..7].iter().cloned().collect(), + }; + let id7 = headers[6].compute_id(); + assert_eq!( + prepare_votes( + storage.cached_finality_votes(&hashes.get(5).unwrap(), |_| false,), + 0, + &validators_addresses.iter().collect(), + id7, + headers.get(6).unwrap(), + None, + ) + .unwrap(), + expected_votes_at_7, + ); + + // cached votes at #5 + let expected_votes_at_5 = FinalityVotes { + votes: vec![ + (validators_addresses[0].clone(), 3), + (validators_addresses[1].clone(), 2), + ] + .into_iter() + .collect(), + ancestry: ancestry[..5].iter().cloned().collect(), + }; + FinalityCache::::insert(hashes[4], expected_votes_at_5); + + // when we're inserting header#7 and last finalized header is 0: + // check that votes at #7 are computed correctly with cache + assert_eq!( + prepare_votes( + storage.cached_finality_votes(&hashes.get(5).unwrap(), |_| false,), + 0, + &validators_addresses.iter().collect(), + id7, + headers.get(6).unwrap(), + None, + ) + .unwrap(), + expected_votes_at_7, + ); + + // when we're inserting header#7 and last finalized header is 3: + // check that votes at #7 are computed correctly with cache + let expected_votes_at_7 = FinalityVotes { + votes: vec![ + (validators_addresses[1].clone(), 3), + (validators_addresses[2].clone(), 1), + ] + .into_iter() + .collect(), + ancestry: ancestry[3..7].iter().cloned().collect(), + }; + assert_eq!( + prepare_votes( + storage.cached_finality_votes(&hashes.get(5).unwrap(), |hash| *hash == hashes[2],), + 3, + &validators_addresses.iter().collect(), + id7, + headers.get(6).unwrap(), + None, + ) + .unwrap(), + expected_votes_at_7, + ); + }); + } } diff --git a/modules/ethereum/src/import.rs b/modules/ethereum/src/import.rs index 87025e543fa..a59bfc750b4 100644 --- a/modules/ethereum/src/import.rs +++ b/modules/ethereum/src/import.rs @@ -19,7 +19,7 @@ use crate::finality::finalize_blocks; use crate::validators::{Validators, ValidatorsConfiguration}; use crate::verification::{is_importable_header, verify_aura_header}; use crate::{AuraConfiguration, ChangeToEnact, Storage}; -use primitives::{Header, Receipt, H256}; +use primitives::{Header, HeaderId, Receipt}; use sp_std::{collections::btree_map::BTreeMap, prelude::*}; /// Maximal number of headers behind best blocks that we are aiming to store. When there @@ -64,7 +64,7 @@ pub fn import_headers( match import_result { Ok((_, finalized)) => { - for (_, _, submitter) in finalized { + for (_, submitter) in finalized { if let Some(submitter) = submitter { *finalized_headers.entry(submitter).or_default() += 1; } @@ -84,7 +84,7 @@ pub fn import_headers( /// Transactions receipts must be provided if `header_import_requires_receipts()` /// has returned true. /// -/// Returns imported block hash. +/// Returns imported block id and list of all finalized headers. pub fn import_header( storage: &mut S, aura_config: &AuraConfiguration, @@ -93,9 +93,9 @@ pub fn import_header( submitter: Option, header: Header, receipts: Option>, -) -> Result<(H256, Vec<(u64, H256, Option)>), Error> { +) -> Result<(HeaderId, Vec<(HeaderId, Option)>), Error> { // first check that we are able to import this header at all - let (hash, prev_finalized_hash) = is_importable_header(storage, &header)?; + let (header_id, finalized_id) = is_importable_header(storage, &header)?; // verify header let import_context = verify_aura_header(storage, aura_config, submitter, &header)?; @@ -108,9 +108,9 @@ pub fn import_header( let validators_set = import_context.validators_set(); let finalized_blocks = finalize_blocks( storage, - &prev_finalized_hash, - (&validators_set.enact_block, &validators_set.validators), - &hash, + finalized_id, + (validators_set.enact_block, &validators_set.validators), + header_id, import_context.submitter(), &header, aura_config.two_thirds_majority_transition, @@ -120,35 +120,36 @@ pub fn import_header( signal_block: None, validators, }) - .or_else(|| validators.finalize_validators_change(storage, &finalized_blocks)); + .or_else(|| validators.finalize_validators_change(storage, &finalized_blocks.finalized_headers)); // NOTE: we can't return Err() from anywhere below this line // (because otherwise we'll have inconsistent storage if transaction will fail) // and finally insert the block - let (_, _, best_total_difficulty) = storage.best_block(); + let (_, best_total_difficulty) = storage.best_block(); let total_difficulty = import_context.total_difficulty() + header.difficulty; let is_best = total_difficulty > best_total_difficulty; let header_number = header.number; storage.insert_header(import_context.into_import_header( is_best, - hash, + header_id, header, total_difficulty, enacted_change, scheduled_change, + finalized_blocks.votes, )); // now mark finalized headers && prune old headers storage.finalize_headers( - finalized_blocks.last().map(|(number, hash, _)| (*number, *hash)), + finalized_blocks.finalized_headers.last().map(|(id, _)| *id), match is_best { true => header_number.checked_sub(prune_depth), false => None, }, ); - Ok((hash, finalized_blocks)) + Ok((header_id, finalized_blocks.finalized_headers)) } /// Returns true if transactions receipts are required to import given header. @@ -178,7 +179,13 @@ mod tests { fn rejects_finalized_block_competitors() { custom_test_ext(genesis(), validators_addresses(3)).execute_with(|| { let mut storage = BridgeStorage::::new(); - storage.finalize_headers(Some((100, Default::default())), None); + storage.finalize_headers( + Some(HeaderId { + number: 100, + ..Default::default() + }), + None, + ); assert_eq!( import_header( &mut storage, @@ -239,7 +246,7 @@ mod tests { let validators = validators(3); let mut storage = BridgeStorage::::new(); let header = block_i(1, &validators); - let hash = header.hash(); + let hash = header.compute_hash(); assert_eq!( import_header( &mut storage, @@ -273,10 +280,10 @@ mod tests { // header [0..11] are finalizing blocks [0; 9] // => since we want to keep 10 finalized blocks, we aren't pruning anything - let mut latest_block_hash = Default::default(); + let mut latest_block_id = Default::default(); for i in 1..11 { let header = block_i(i, &validators); - let (rolling_last_block_hash, finalized_blocks) = import_header( + let (rolling_last_block_id, finalized_blocks) = import_header( &mut storage, &test_aura_config(), &validators_config, @@ -289,15 +296,15 @@ mod tests { match i { 2..=10 => assert_eq!( finalized_blocks, - vec![(i - 1, block_i(i - 1, &validators).hash(), Some(100))], + vec![(block_i(i - 1, &validators).compute_id(), Some(100))], "At {}", i, ), _ => assert_eq!(finalized_blocks, vec![], "At {}", i), } - latest_block_hash = rolling_last_block_hash; + latest_block_id = rolling_last_block_id; } - assert!(storage.header(&genesis().hash()).is_some()); + assert!(storage.header(&genesis().compute_hash()).is_some()); // header 11 finalizes headers [10] AND schedules change // => we prune header#0 @@ -307,7 +314,7 @@ mod tests { .parse() .unwrap(); }); - let (rolling_last_block_hash, finalized_blocks) = import_header( + let (rolling_last_block_id, finalized_blocks) = import_header( &mut storage, &test_aura_config(), &validators_config, @@ -315,23 +322,26 @@ mod tests { Some(101), header11.clone(), Some(vec![crate::validators::tests::validators_change_recept( - latest_block_hash, + latest_block_id.hash, )]), ) .unwrap(); - assert_eq!(finalized_blocks, vec![(10, block_i(10, &validators).hash(), Some(100))],); - assert!(storage.header(&genesis().hash()).is_none()); - latest_block_hash = rolling_last_block_hash; + assert_eq!( + finalized_blocks, + vec![(block_i(10, &validators).compute_id(), Some(100))], + ); + assert!(storage.header(&genesis().compute_hash()).is_none()); + latest_block_id = rolling_last_block_id; // and now let's say validators 1 && 2 went offline // => in the range 12-25 no blocks are finalized, but we still continue to prune old headers // until header#11 is met. we can't prune #11, because it schedules change let mut step = 56; - let mut expected_blocks = vec![(11, header11.hash(), Some(101))]; + let mut expected_blocks = vec![(header11.compute_id(), Some(101))]; for i in 12..25 { let header = Header { number: i as _, - parent_hash: latest_block_hash, + parent_hash: latest_block_id.hash, gas_limit: 0x2000.into(), author: validator(2).address(), seal: vec![vec![step].into(), vec![].into()], @@ -339,8 +349,8 @@ mod tests { ..Default::default() }; let header = signed_header(&validators, header, step as _); - expected_blocks.push((i, header.hash(), Some(102))); - let (rolling_last_block_hash, finalized_blocks) = import_header( + expected_blocks.push((header.compute_id(), Some(102))); + let (rolling_last_block_id, finalized_blocks) = import_header( &mut storage, &test_aura_config(), &validators_config, @@ -351,7 +361,7 @@ mod tests { ) .unwrap(); assert_eq!(finalized_blocks, vec![],); - latest_block_hash = rolling_last_block_hash; + latest_block_id = rolling_last_block_id; step += 3; } assert_eq!( @@ -367,7 +377,7 @@ mod tests { step -= 2; let header = Header { number: 25, - parent_hash: latest_block_hash, + parent_hash: latest_block_id.hash, gas_limit: 0x2000.into(), author: validator(0).address(), seal: vec![vec![step].into(), vec![].into()], diff --git a/modules/ethereum/src/lib.rs b/modules/ethereum/src/lib.rs index 3f662547d19..8b927bd54ef 100644 --- a/modules/ethereum/src/lib.rs +++ b/modules/ethereum/src/lib.rs @@ -16,9 +16,10 @@ #![cfg_attr(not(feature = "std"), no_std)] +use crate::finality::{CachedFinalityVotes, FinalityVotes}; use codec::{Decode, Encode}; use frame_support::{decl_module, decl_storage, traits::Get}; -use primitives::{Address, Header, RawTransaction, Receipt, H256, U256}; +use primitives::{Address, Header, HeaderId, RawTransaction, Receipt, H256, U256}; use sp_runtime::{ transaction_validity::{ InvalidTransaction, TransactionLongevity, TransactionPriority, TransactionSource, TransactionValidity, @@ -93,7 +94,7 @@ pub struct StoredHeader { /// Hash of the last block which has **SCHEDULED** validators set change. /// Note that signal doesn't mean that the set has been (or ever will be) enacted. /// Note that the header may already be pruned. - pub last_signal_block: Option, + pub last_signal_block: Option, } /// Validators set as it is stored in the runtime storage. @@ -103,9 +104,9 @@ pub struct ValidatorsSet { /// Validators of this set. pub validators: Vec
, /// Hash of the block where this set has been signalled. None if this is the first set. - pub signal_block: Option, + pub signal_block: Option, /// Hash of the block where this set has been enacted. - pub enact_block: H256, + pub enact_block: HeaderId, } /// Validators set change as it is stored in the runtime storage. @@ -115,7 +116,7 @@ pub struct ScheduledChange { /// Validators of this set. pub validators: Vec
, /// Hash of the block which has emitted previous validators change signal. - pub prev_signal_block: Option, + pub prev_signal_block: Option, } /// Header that we're importing. @@ -126,8 +127,8 @@ pub struct HeaderToImport { pub context: ImportContext, /// Should we consider this header as best? pub is_best: bool, - /// The hash of the header. - pub hash: H256, + /// The id of the header. + pub id: HeaderId, /// The header itself. pub header: Header, /// Total chain difficulty at the header. @@ -137,15 +138,17 @@ pub struct HeaderToImport { pub enacted_change: Option, /// Validators set scheduled change, if happened at the header. pub scheduled_change: Option>, + /// Finality votes at this header. + pub finality_votes: FinalityVotes, } /// Header that we're importing. #[derive(RuntimeDebug)] #[cfg_attr(test, derive(Clone, PartialEq))] pub struct ChangeToEnact { - /// The hash of the header where change has been scheduled. + /// The id of the header where change has been scheduled. /// None if it is a first set within current `ValidatorsSource`. - pub signal_block: Option, + pub signal_block: Option, /// Validators set that is enacted. pub validators: Vec
, } @@ -179,7 +182,7 @@ pub struct ImportContext { parent_scheduled_change: Option, validators_set_id: u64, validators_set: ValidatorsSet, - last_signal_block: Option, + last_signal_block: Option, } impl ImportContext { @@ -215,10 +218,13 @@ impl ImportContext { /// Returns reference to the latest block which has signalled change of validators set. /// This may point to parent if parent has signalled change. - pub fn last_signal_block(&self) -> Option<&H256> { + pub fn last_signal_block(&self) -> Option { match self.parent_scheduled_change { - Some(_) => Some(&self.parent_hash), - None => self.last_signal_block.as_ref(), + Some(_) => Some(HeaderId { + number: self.parent_header.number, + hash: self.parent_hash, + }), + None => self.last_signal_block, } } @@ -226,20 +232,22 @@ impl ImportContext { pub fn into_import_header( self, is_best: bool, - hash: H256, + id: HeaderId, header: Header, total_difficulty: U256, enacted_change: Option, scheduled_change: Option>, + finality_votes: FinalityVotes, ) -> HeaderToImport { HeaderToImport { context: self, is_best, - hash, + id, header, total_difficulty, enacted_change, scheduled_change, + finality_votes, } } } @@ -251,14 +259,22 @@ pub trait Storage { /// Header submitter identifier. type Submitter: Clone + Ord; - /// Get best known block. - fn best_block(&self) -> (u64, H256, U256); + /// Get best known block and total chain difficulty. + fn best_block(&self) -> (HeaderId, U256); /// Get last finalized block. - fn finalized_block(&self) -> (u64, H256); + fn finalized_block(&self) -> HeaderId; /// Get imported header by its hash. /// /// Returns header and its submitter (if known). fn header(&self, hash: &H256) -> Option<(Header, Option)>; + /// Returns latest cached finality votes (if any) for block ancestors, starting + /// from `parent_hash` block and stopping at genesis block, or block where `stop_at` + /// returns true. + fn cached_finality_votes( + &self, + parent_hash: &H256, + stop_at: impl Fn(&H256) -> bool, + ) -> CachedFinalityVotes; /// Get header import context by parent header hash. fn import_context( &self, @@ -275,7 +291,7 @@ pub trait Storage { /// It is the storage duty to ensure that unfinalized headers that have /// scheduled changes won't be pruned until they or their competitors /// are finalized. - fn finalize_headers(&mut self, finalized: Option<(u64, H256)>, prune_end: Option); + fn finalize_headers(&mut self, finalized: Option, prune_end: Option); } /// Decides whether the session should be ended. @@ -302,10 +318,17 @@ impl OnHeadersSubmitted for () { fn on_valid_headers_finalized(_submitter: AccountId, _finalized: u64) {} } -/// The module configuration trait +/// The module configuration trait. pub trait Trait: frame_system::Trait { /// Aura configuration. type AuraConfiguration: Get; + /// Interval (in blocks) for for finality votes caching. + /// If None, cache is disabled. + /// + /// Ideally, this should either be None (when we are sure that there won't + /// be any significant finalization delays), or something that is bit larger + /// than average finalization delay. + type FinalityVotesCachingInterval: Get>; /// Validators configuration. type ValidatorsConfiguration: Get; /// Handler for headers submission result. @@ -377,15 +400,17 @@ decl_module! { decl_storage! { trait Store for Module as Bridge { /// Best known block. - BestBlock: (u64, H256, U256); + BestBlock: (HeaderId, U256); /// Best finalized block. - FinalizedBlock: (u64, H256); + FinalizedBlock: HeaderId; /// Range of blocks that we want to prune. BlocksToPrune: PruningRange; /// Map of imported headers by hash. Headers: map hasher(identity) H256 => Option>; /// Map of imported header hashes by number. HeadersByNumber: map hasher(blake2_128_concat) u64 => Option>; + /// Map of cached finality data by header hash. + FinalityCache: map hasher(identity) H256 => Option>; /// The ID of next validator set. NextValidatorsSetId: u64; /// Map of validators sets by their id. @@ -412,9 +437,13 @@ decl_storage! { "Initial validators set can't be empty", ); - let initial_hash = config.initial_header.hash(); - BestBlock::put((config.initial_header.number, initial_hash, config.initial_difficulty)); - FinalizedBlock::put((config.initial_header.number, initial_hash)); + let initial_hash = config.initial_header.compute_hash(); + let initial_id = HeaderId { + number: config.initial_header.number, + hash: initial_hash, + }; + BestBlock::put((initial_id, config.initial_difficulty)); + FinalizedBlock::put(initial_id); BlocksToPrune::put(PruningRange { oldest_unpruned_block: config.initial_header.number, oldest_block_to_keep: config.initial_header.number, @@ -431,7 +460,7 @@ decl_storage! { ValidatorsSets::insert(0, ValidatorsSet { validators: config.initial_validators.clone(), signal_block: None, - enact_block: initial_hash, + enact_block: initial_id, }); ValidatorsSetsRc::insert(0, 1); }) @@ -442,9 +471,8 @@ impl Module { /// Returns number and hash of the best block known to the bridge module. /// The caller should only submit `import_header` transaction that makes /// (or leads to making) other header the best one. - pub fn best_block() -> (u64, H256) { - let (number, hash, _) = BridgeStorage::::new().best_block(); - (number, hash) + pub fn best_block() -> HeaderId { + BridgeStorage::::new().best_block().0 } /// Returns true if the import of given block requires transactions receipts. @@ -580,6 +608,7 @@ impl BridgeStorage { while let Some(hash) = blocks_at_number.pop() { let header = Headers::::take(&hash); ScheduledChanges::remove(hash); + FinalityCache::::remove(hash); if let Some(header) = header { ValidatorsSetsRc::mutate(header.next_validators_set_id, |rc| match *rc { Some(rc) if rc > 1 => Some(rc - 1), @@ -599,11 +628,11 @@ impl BridgeStorage { impl Storage for BridgeStorage { type Submitter = T::AccountId; - fn best_block(&self) -> (u64, H256, U256) { + fn best_block(&self) -> (HeaderId, U256) { BestBlock::get() } - fn finalized_block(&self) -> (u64, H256) { + fn finalized_block(&self) -> HeaderId { FinalizedBlock::get() } @@ -611,6 +640,41 @@ impl Storage for BridgeStorage { Headers::::get(hash).map(|header| (header.header, header.submitter)) } + fn cached_finality_votes( + &self, + parent_hash: &H256, + stop_at: impl Fn(&H256) -> bool, + ) -> CachedFinalityVotes { + let mut votes = CachedFinalityVotes::default(); + let mut current_hash = *parent_hash; + loop { + if stop_at(¤t_hash) { + return votes; + } + + let cached_votes = FinalityCache::::get(¤t_hash); + if let Some(cached_votes) = cached_votes { + votes.votes = Some(cached_votes); + return votes; + } + + let header = match Headers::::get(¤t_hash) { + Some(header) if header.header.number != 0 => header, + _ => return votes, + }; + let parent_hash = header.header.parent_hash; + let current_id = HeaderId { + number: header.header.number, + hash: current_hash, + }; + votes + .unaccounted_ancestry + .push_back((current_id, header.submitter, header.header)); + + current_hash = parent_hash; + } + } + fn import_context( &self, submitter: Option, @@ -639,11 +703,11 @@ impl Storage for BridgeStorage { fn insert_header(&mut self, header: HeaderToImport) { if header.is_best { - BestBlock::put((header.header.number, header.hash, header.total_difficulty)); + BestBlock::put((header.id, header.total_difficulty)); } if let Some(scheduled_change) = header.scheduled_change { ScheduledChanges::insert( - &header.hash, + &header.id.hash, ScheduledChange { validators: scheduled_change, prev_signal_block: header.context.last_signal_block, @@ -661,7 +725,7 @@ impl Storage for BridgeStorage { next_validators_set_id, ValidatorsSet { validators: enacted_change.validators, - enact_block: header.hash, + enact_block: header.id, signal_block: enacted_change.signal_block, }, ); @@ -677,17 +741,25 @@ impl Storage for BridgeStorage { } }; + let finality_votes_caching_interval = T::FinalityVotesCachingInterval::get(); + if let Some(finality_votes_caching_interval) = finality_votes_caching_interval { + let cache_entry_required = header.id.number != 0 && header.id.number % finality_votes_caching_interval == 0; + if cache_entry_required { + FinalityCache::::insert(header.id.hash, header.finality_votes); + } + } + frame_support::debug::trace!( target: "runtime", "Inserting PoA header: ({}, {})", header.header.number, - header.hash, + header.id.hash, ); - let last_signal_block = header.context.last_signal_block().cloned(); - HeadersByNumber::append(header.header.number, header.hash); + let last_signal_block = header.context.last_signal_block(); + HeadersByNumber::append(header.id.number, header.id.hash); Headers::::insert( - &header.hash, + &header.id.hash, StoredHeader { submitter: header.context.submitter, header: header.header, @@ -698,18 +770,18 @@ impl Storage for BridgeStorage { ); } - fn finalize_headers(&mut self, finalized: Option<(u64, H256)>, prune_end: Option) { + fn finalize_headers(&mut self, finalized: Option, prune_end: Option) { // remember just finalized block let finalized_number = finalized .as_ref() - .map(|f| f.0) - .unwrap_or_else(|| FinalizedBlock::get().0); + .map(|f| f.number) + .unwrap_or_else(|| FinalizedBlock::get().number); if let Some(finalized) = finalized { frame_support::debug::trace!( target: "runtime", "Finalizing PoA header: ({}, {})", - finalized.0, - finalized.1, + finalized.number, + finalized.hash, ); FinalizedBlock::put(finalized); @@ -735,21 +807,21 @@ pub fn verify_transaction_finalized( Some((header, _)) => header, None => return false, }; - let (finalized_number, finalized_hash) = storage.finalized_block(); + let finalized = storage.finalized_block(); // if header is not yet finalized => return - if header.number > finalized_number { + if header.number > finalized.number { return false; } // check if header is actually finalized - let is_finalized = match header.number < finalized_number { - true => finality::ancestry(storage, finalized_hash) - .skip_while(|(_, ancestor, _)| ancestor.number > header.number) - .filter(|&(ancestor_hash, _, _)| ancestor_hash == block) + let is_finalized = match header.number < finalized.number { + true => ancestry(storage, finalized.hash) + .skip_while(|(_, ancestor)| ancestor.number > header.number) + .filter(|&(ancestor_hash, _)| ancestor_hash == block) .next() .is_some(), - false => block == finalized_hash, + false => block == finalized.hash, }; if !is_finalized { return false; @@ -765,14 +837,49 @@ fn pool_configuration() -> PoolConfiguration { } } +/// Return iterator of given header ancestors. +fn ancestry<'a, S: Storage>(storage: &'a S, mut parent_hash: H256) -> impl Iterator + 'a { + sp_std::iter::from_fn(move || { + let (header, _) = storage.header(&parent_hash)?; + if header.number == 0 { + return None; + } + + let hash = parent_hash; + parent_hash = header.parent_hash; + Some((hash, header)) + }) +} + #[cfg(test)] pub(crate) mod tests { use super::*; + use crate::finality::FinalityAncestor; use crate::mock::{ - custom_block_i, custom_test_ext, genesis, insert_header, validators, validators_addresses, TestRuntime, + block_i, custom_block_i, custom_test_ext, genesis, insert_header, validators, validators_addresses, TestRuntime, }; use primitives::compute_merkle_root; + fn example_tx() -> Vec { + vec![42] + } + + fn example_header() -> Header { + let mut header = Header::default(); + header.number = 2; + header.transactions_root = compute_merkle_root(vec![example_tx()].into_iter()); + header.parent_hash = example_header_parent().compute_hash(); + header + } + + fn example_header_parent() -> Header { + let mut header = Header::default(); + header.number = 1; + header.transactions_root = compute_merkle_root(vec![example_tx()].into_iter()); + header.parent_hash = genesis().compute_hash(); + header + } + fn with_headers_to_prune(f: impl Fn(BridgeStorage) -> T) -> T { custom_test_ext(genesis(), validators_addresses(3)).execute_with(|| { let validators = validators(3); @@ -782,7 +889,7 @@ pub(crate) mod tests { let header = custom_block_i(i, &validators, |header| { header.gas_limit = header.gas_limit + U256::from(j); }); - let hash = header.hash(); + let hash = header.compute_hash(); headers_by_number.push(hash); Headers::::insert( hash, @@ -935,24 +1042,97 @@ pub(crate) mod tests { }); } - fn example_tx() -> Vec { - vec![42] - } + #[test] + fn finality_votes_are_cached() { + custom_test_ext(genesis(), validators_addresses(3)).execute_with(|| { + let mut storage = BridgeStorage::::new(); + let interval = ::FinalityVotesCachingInterval::get().unwrap(); - fn example_header() -> Header { - let mut header = Header::default(); - header.number = 2; - header.transactions_root = compute_merkle_root(vec![example_tx()].into_iter()); - header.parent_hash = example_header_parent().hash(); - header + // for all headers with number < interval, cache entry is not created + let validators = validators(3); + for i in 1..interval { + let header = block_i(i, &validators); + let id = header.compute_id(); + insert_header(&mut storage, header); + assert_eq!(FinalityCache::::get(&id.hash), None); + } + + // for header with number = interval, cache entry is created + let header_with_entry = block_i(interval, &validators); + let header_with_entry_hash = header_with_entry.compute_hash(); + insert_header(&mut storage, header_with_entry); + assert_eq!( + FinalityCache::::get(&header_with_entry_hash), + Some(Default::default()), + ); + + // when we later prune this header, cache entry is removed + BlocksToPrune::put(PruningRange { + oldest_unpruned_block: interval - 1, + oldest_block_to_keep: interval - 1, + }); + storage.finalize_headers(None, Some(interval + 1)); + assert_eq!(FinalityCache::::get(&header_with_entry_hash), None); + }); } - fn example_header_parent() -> Header { - let mut header = Header::default(); - header.number = 1; - header.transactions_root = compute_merkle_root(vec![example_tx()].into_iter()); - header.parent_hash = genesis().hash(); - header + #[test] + fn cached_finality_votes_finds_entry() { + custom_test_ext(genesis(), validators_addresses(3)).execute_with(|| { + // insert 5 headers + let validators = validators(3); + let mut storage = BridgeStorage::::new(); + let mut headers = Vec::new(); + for i in 1..5 { + let header = block_i(i, &validators); + headers.push(header.clone()); + insert_header(&mut storage, header); + } + + // when inserting header#6, entry isn't found + let hash5 = headers.last().unwrap().compute_hash(); + assert_eq!( + storage.cached_finality_votes(&hash5, |_| false), + CachedFinalityVotes { + unaccounted_ancestry: headers + .iter() + .map(|header| (header.compute_id(), None, header.clone(),)) + .rev() + .collect(), + votes: None, + }, + ); + + // let's now create entry at #3 + let hash3 = headers[2].compute_hash(); + let votes_at_3 = FinalityVotes { + votes: vec![([42; 20].into(), 21)].into_iter().collect(), + ancestry: vec![FinalityAncestor { + id: HeaderId { + number: 100, + hash: Default::default(), + }, + ..Default::default() + }] + .into_iter() + .collect(), + }; + FinalityCache::::insert(hash3, votes_at_3.clone()); + + // searching at #6 again => entry is found + assert_eq!( + storage.cached_finality_votes(&hash5, |_| false), + CachedFinalityVotes { + unaccounted_ancestry: headers + .iter() + .skip(3) + .map(|header| (header.compute_id(), None, header.clone(),)) + .rev() + .collect(), + votes: Some(votes_at_3), + }, + ); + }); } #[test] @@ -960,7 +1140,7 @@ pub(crate) mod tests { custom_test_ext(example_header(), validators_addresses(3)).execute_with(|| { let storage = BridgeStorage::::new(); assert_eq!( - verify_transaction_finalized(&storage, example_header().hash(), 0, &vec![example_tx()],), + verify_transaction_finalized(&storage, example_header().compute_hash(), 0, &vec![example_tx()],), true, ); }); @@ -972,9 +1152,9 @@ pub(crate) mod tests { let mut storage = BridgeStorage::::new(); insert_header(&mut storage, example_header_parent()); insert_header(&mut storage, example_header()); - storage.finalize_headers(Some((example_header().number, example_header().hash())), None); + storage.finalize_headers(Some(example_header().compute_id()), None); assert_eq!( - verify_transaction_finalized(&storage, example_header_parent().hash(), 0, &vec![example_tx()],), + verify_transaction_finalized(&storage, example_header_parent().compute_hash(), 0, &vec![example_tx()],), true, ); }); @@ -985,7 +1165,7 @@ pub(crate) mod tests { custom_test_ext(example_header(), validators_addresses(3)).execute_with(|| { let storage = BridgeStorage::::new(); assert_eq!( - verify_transaction_finalized(&storage, example_header().hash(), 1, &vec![],), + verify_transaction_finalized(&storage, example_header().compute_hash(), 1, &vec![],), false, ); }); @@ -996,7 +1176,7 @@ pub(crate) mod tests { custom_test_ext(genesis(), validators_addresses(3)).execute_with(|| { let storage = BridgeStorage::::new(); assert_eq!( - verify_transaction_finalized(&storage, example_header().hash(), 1, &vec![],), + verify_transaction_finalized(&storage, example_header().compute_hash(), 1, &vec![],), false, ); }); @@ -1009,7 +1189,7 @@ pub(crate) mod tests { insert_header(&mut storage, example_header_parent()); insert_header(&mut storage, example_header()); assert_eq!( - verify_transaction_finalized(&storage, example_header().hash(), 0, &vec![example_tx()],), + verify_transaction_finalized(&storage, example_header().compute_hash(), 0, &vec![example_tx()],), false, ); }); @@ -1020,13 +1200,13 @@ pub(crate) mod tests { custom_test_ext(genesis(), validators_addresses(3)).execute_with(|| { let mut finalized_header_sibling = example_header(); finalized_header_sibling.timestamp = 1; - let finalized_header_sibling_hash = finalized_header_sibling.hash(); + let finalized_header_sibling_hash = finalized_header_sibling.compute_hash(); let mut storage = BridgeStorage::::new(); insert_header(&mut storage, example_header_parent()); insert_header(&mut storage, example_header()); insert_header(&mut storage, finalized_header_sibling); - storage.finalize_headers(Some((example_header().number, example_header().hash())), None); + storage.finalize_headers(Some(example_header().compute_id()), None); assert_eq!( verify_transaction_finalized(&storage, finalized_header_sibling_hash, 0, &vec![example_tx()],), false, @@ -1039,13 +1219,13 @@ pub(crate) mod tests { custom_test_ext(genesis(), validators_addresses(3)).execute_with(|| { let mut finalized_header_uncle = example_header_parent(); finalized_header_uncle.timestamp = 1; - let finalized_header_uncle_hash = finalized_header_uncle.hash(); + let finalized_header_uncle_hash = finalized_header_uncle.compute_hash(); let mut storage = BridgeStorage::::new(); insert_header(&mut storage, example_header_parent()); insert_header(&mut storage, finalized_header_uncle); insert_header(&mut storage, example_header()); - storage.finalize_headers(Some((example_header().number, example_header().hash())), None); + storage.finalize_headers(Some(example_header().compute_id()), None); assert_eq!( verify_transaction_finalized(&storage, finalized_header_uncle_hash, 0, &vec![example_tx()],), false, @@ -1058,7 +1238,12 @@ pub(crate) mod tests { custom_test_ext(example_header(), validators_addresses(3)).execute_with(|| { let storage = BridgeStorage::::new(); assert_eq!( - verify_transaction_finalized(&storage, example_header().hash(), 0, &vec![example_tx(), example_tx(),],), + verify_transaction_finalized( + &storage, + example_header().compute_hash(), + 0, + &vec![example_tx(), example_tx(),], + ), false, ); }); diff --git a/modules/ethereum/src/mock.rs b/modules/ethereum/src/mock.rs index 44213813fe3..a4f9e0fe4bb 100644 --- a/modules/ethereum/src/mock.rs +++ b/modules/ethereum/src/mock.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity Bridges Common. If not, see . +use crate::finality::FinalityVotes; use crate::validators::{ValidatorsConfiguration, ValidatorsSource}; use crate::{AuraConfiguration, GenesisConfig, HeaderToImport, HeadersByNumber, Storage, Trait}; use frame_support::StorageMap; @@ -70,12 +71,14 @@ impl frame_system::Trait for TestRuntime { } parameter_types! { + pub const TestFinalityVotesCachingInterval: Option = Some(16); pub const TestAuraConfiguration: AuraConfiguration = test_aura_config(); pub const TestValidatorsConfiguration: ValidatorsConfiguration = test_validators_config(); } impl Trait for TestRuntime { type AuraConfiguration = TestAuraConfiguration; + type FinalityVotesCachingInterval = TestFinalityVotesCachingInterval; type ValidatorsConfiguration = TestValidatorsConfiguration; type OnHeadersSubmitted = (); } @@ -171,10 +174,11 @@ pub fn insert_header(storage: &mut S, header: Header) { storage.insert_header(HeaderToImport { context: storage.import_context(None, &header.parent_hash).unwrap(), is_best: true, - hash: header.hash(), + id: header.compute_id(), header, total_difficulty: 0.into(), enacted_change: None, scheduled_change: None, + finality_votes: FinalityVotes::default(), }); } diff --git a/modules/ethereum/src/validators.rs b/modules/ethereum/src/validators.rs index 8b1c5698fc7..70aa246c90e 100644 --- a/modules/ethereum/src/validators.rs +++ b/modules/ethereum/src/validators.rs @@ -16,7 +16,7 @@ use crate::error::Error; use crate::{ChangeToEnact, Storage}; -use primitives::{Address, Header, LogEntry, Receipt, H256, U256}; +use primitives::{Address, Header, HeaderId, LogEntry, Receipt, U256}; use sp_std::prelude::*; /// The hash of InitiateChange event of the validators set contract. @@ -181,18 +181,45 @@ impl<'a> Validators<'a> { /// Finalize changes when blocks are finalized. pub fn finalize_validators_change( &self, - storage: &mut S, - finalized_blocks: &[(u64, H256, Option)], + storage: &S, + finalized_blocks: &[(HeaderId, Option)], ) -> Option { - for (_, finalized_hash, _) in finalized_blocks.iter().rev() { - if let Some(changes) = storage.scheduled_change(finalized_hash) { - return Some(ChangeToEnact { - signal_block: Some(*finalized_hash), - validators: changes.validators, - }); - } - } - None + // if we haven't finalized any blocks, no changes may be finalized + let newest_finalized_id = match finalized_blocks.last().map(|(id, _)| id) { + Some(last_finalized_id) => last_finalized_id, + None => return None, + }; + let oldest_finalized_id = finalized_blocks + .first() + .map(|(id, _)| id) + .expect("finalized_blocks is not empty; qed"); + + // try to directly go to the header that has scheduled last change + // + // if we're unable to create import context for some block, it means + // that the header has already been pruned => it and its ancestors had + // no scheduled changes + // + // if we're unable to find scheduled changes for some block, it means + // that these changes have been finalized already + storage + .import_context(None, &newest_finalized_id.hash) + .and_then(|context| context.last_signal_block()) + .and_then(|signal_block| { + if signal_block.number >= oldest_finalized_id.number { + Some(signal_block) + } else { + None + } + }) + .and_then(|signal_block| { + storage + .scheduled_change(&signal_block.hash) + .map(|change| ChangeToEnact { + signal_block: Some(signal_block), + validators: change.validators, + }) + }) } /// Returns source of validators that should author the header. @@ -254,7 +281,10 @@ pub fn step_validator(header_validators: &[Address], header_step: u64) -> Addres #[cfg(test)] pub(crate) mod tests { use super::*; - use primitives::TransactionOutcome; + use crate::mock::{custom_test_ext, genesis, validators_addresses, TestRuntime}; + use crate::{BridgeStorage, Headers, ScheduledChange, ScheduledChanges, StoredHeader}; + use frame_support::StorageMap; + use primitives::{TransactionOutcome, H256}; pub(crate) fn validators_change_recept(parent_hash: H256) -> Receipt { Receipt { @@ -393,4 +423,72 @@ pub(crate) mod tests { Err(Error::TransactionsReceiptsMismatch), ); } + + fn try_finalize_with_scheduled_change(scheduled_at: Option) -> Option { + custom_test_ext(genesis(), validators_addresses(3)).execute_with(|| { + let config = ValidatorsConfiguration::Single(ValidatorsSource::Contract(Default::default(), Vec::new())); + let validators = Validators::new(&config); + let storage = BridgeStorage::::new(); + + // when we're finailizing blocks 10...100 + let id10 = HeaderId { + number: 10, + hash: [10; 32].into(), + }; + let id100 = HeaderId { + number: 100, + hash: [100; 32].into(), + }; + let finalized_blocks = vec![(id10, None), (id100, None)]; + let header100 = StoredHeader:: { + submitter: None, + header: Header { + number: 100, + ..Default::default() + }, + total_difficulty: 0.into(), + next_validators_set_id: 0, + last_signal_block: scheduled_at, + }; + let scheduled_change = ScheduledChange { + validators: validators_addresses(1), + prev_signal_block: None, + }; + Headers::::insert(id100.hash, header100); + if let Some(scheduled_at) = scheduled_at { + ScheduledChanges::insert(scheduled_at.hash, scheduled_change); + } + + validators.finalize_validators_change(&storage, &finalized_blocks) + }) + } + + #[test] + fn finalize_validators_change_finalizes_scheduled_change() { + let id50 = HeaderId { + number: 50, + ..Default::default() + }; + assert_eq!( + try_finalize_with_scheduled_change(Some(id50)), + Some(ChangeToEnact { + signal_block: Some(id50), + validators: validators_addresses(1), + }), + ); + } + + #[test] + fn finalize_validators_change_does_not_finalize_when_changes_are_not_scheduled() { + assert_eq!(try_finalize_with_scheduled_change(None), None,); + } + + #[test] + fn finalize_validators_change_does_not_finalize_changes_when_they_are_outside_of_range() { + let id5 = HeaderId { + number: 5, + ..Default::default() + }; + assert_eq!(try_finalize_with_scheduled_change(Some(id5)), None,); + } } diff --git a/modules/ethereum/src/verification.rs b/modules/ethereum/src/verification.rs index ca8d9af6d17..dee912e26a1 100644 --- a/modules/ethereum/src/verification.rs +++ b/modules/ethereum/src/verification.rs @@ -18,26 +18,26 @@ use crate::error::Error; use crate::validators::{step_validator, Validators, ValidatorsConfiguration}; use crate::{AuraConfiguration, ImportContext, PoolConfiguration, ScheduledChange, Storage}; use codec::Encode; -use primitives::{public_to_address, Address, Header, Receipt, SealedEmptyStep, H256, H520, U128, U256}; +use primitives::{public_to_address, Address, Header, HeaderId, Receipt, SealedEmptyStep, H256, H520, U128, U256}; use sp_io::crypto::secp256k1_ecdsa_recover; use sp_std::{vec, vec::Vec}; /// Pre-check to see if should try and import this header. /// Returns error if we should not try to import this block. -/// Returns hash of the header and number of the last finalized block otherwise. -pub fn is_importable_header(storage: &S, header: &Header) -> Result<(H256, H256), Error> { +/// Returns ID of passed header and best finalized header. +pub fn is_importable_header(storage: &S, header: &Header) -> Result<(HeaderId, HeaderId), Error> { // we never import any header that competes with finalized header - let (finalized_block_number, finalized_block_hash) = storage.finalized_block(); - if header.number <= finalized_block_number { + let finalized_id = storage.finalized_block(); + if header.number <= finalized_id.number { return Err(Error::AncientHeader); } // we never import any header with known hash - let hash = header.hash(); - if storage.header(&hash).is_some() { + let id = header.compute_id(); + if storage.header(&id.hash).is_some() { return Err(Error::KnownHeader); } - Ok((hash, finalized_block_hash)) + Ok((id, finalized_id)) } /// Try accept unsigned aura header into transaction pool. @@ -50,7 +50,7 @@ pub fn accept_aura_header_into_pool( receipts: Option<&Vec>, ) -> Result<(Vec>, Vec>), Error> { // check if we can verify further - let (hash, _) = is_importable_header(storage, header)?; + let (header_id, _) = is_importable_header(storage, header)?; // we can always do contextless checks contextless_checks(config, header)?; @@ -69,8 +69,8 @@ pub fn accept_aura_header_into_pool( // => if we see header with number > maximal ever seen header number + LIMIT, // => we consider this transaction invalid, but only at this moment (we do not want to ban it) // => let's mark it as Unknown transaction - let (best_number, best_hash, _) = storage.best_block(); - let difference = header.number.saturating_sub(best_number); + let (best_id, _) = storage.best_block(); + let difference = header.number.saturating_sub(best_id.number); if difference > pool_config.max_future_number_difference { return Err(Error::UnsignedTooFarInTheFuture); } @@ -87,7 +87,7 @@ pub fn accept_aura_header_into_pool( // previous headers here // => we can at least 'verify' that headers comprise a chain by providing and requiring // tag (header.number, header.hash) - let provides_header_number_and_hash_tag = (header.number, hash).encode(); + let provides_header_number_and_hash_tag = header_id.encode(); // depending on whether parent header is available, we either perform full or 'shortened' check let context = storage.import_context(None, &header.parent_hash); @@ -109,7 +109,7 @@ pub fn accept_aura_header_into_pool( // PoA chain AND that the header is produced either by previous, or next // scheduled validators set change let header_step = header.step().ok_or(Error::MissingStep)?; - let best_context = storage.import_context(None, &best_hash).expect( + let best_context = storage.import_context(None, &best_id.hash).expect( "import context is None only when header is missing from the storage;\ best header is always in the storage; qed", ); @@ -124,7 +124,11 @@ pub fn accept_aura_header_into_pool( // since our parent is missing from the storage, we **DO** require it // to be in the transaction pool // (- 1 can't underflow because there's always best block in the header) - let requires_header_number_and_hash_tag = (header.number - 1, header.parent_hash).encode(); + let requires_header_number_and_hash_tag = HeaderId { + number: header.number - 1, + hash: header.parent_hash, + } + .encode(); ( vec![requires_header_number_and_hash_tag], vec![provides_number_and_authority_tag, provides_header_number_and_hash_tag], @@ -313,7 +317,7 @@ fn find_next_validators_signal(storage: &S, context: &ImportContext< // if parent schedules validators set change, then it may be our set // else we'll start with last known change - let mut current_set_signal_block = context.last_signal_block().cloned(); + let mut current_set_signal_block = context.last_signal_block(); let mut next_scheduled_set: Option = None; loop { @@ -325,7 +329,7 @@ fn find_next_validators_signal(storage: &S, context: &ImportContext< return next_scheduled_set.map(|scheduled_set| scheduled_set.validators) } None => return next_scheduled_set.map(|scheduled_set| scheduled_set.validators), - Some(current_set_signal_block) => storage.scheduled_change(¤t_set_signal_block).expect( + Some(current_set_signal_block) => storage.scheduled_change(¤t_set_signal_block.hash).expect( "header that is associated with this change is not pruned;\ scheduled changes are only removed when header is pruned; qed", ), @@ -386,12 +390,12 @@ mod tests { let block1 = block_i(1, &validators); insert_header(&mut storage, block1); let block2 = block_i(2, &validators); - let block2_hash = block2.hash(); + let block2_id = block2.compute_id(); insert_header(&mut storage, block2); let block3 = block_i(3, &validators); insert_header(&mut storage, block3); - FinalizedBlock::put((2, block2_hash)); + FinalizedBlock::put(block2_id); let validators_config = ValidatorsConfiguration::Single(ValidatorsSource::Contract(Default::default(), Vec::new())); @@ -415,7 +419,10 @@ mod tests { ValidatorsSet { validators: finalized_set, signal_block: None, - enact_block: HeadersByNumber::get(&0).unwrap()[0].clone(), + enact_block: HeaderId { + number: 0, + hash: HeadersByNumber::get(&0).unwrap()[0], + }, }, ); @@ -423,7 +430,10 @@ mod tests { let mut header = Headers::::get(&header_hash).unwrap(); header.next_validators_set_id = set_id; if let Some(signalled_set) = signalled_set { - header.last_signal_block = Some(header.header.parent_hash); + header.last_signal_block = Some(HeaderId { + number: header.header.number - 1, + hash: header.header.parent_hash, + }); ScheduledChanges::insert( header.header.parent_hash, ScheduledChange { @@ -554,7 +564,7 @@ mod tests { assert_eq!(default_verify(&header), Err(Error::MissingParentBlock)); // when parent is in the storage - header.parent_hash = genesis().hash(); + header.parent_hash = genesis().compute_hash(); assert_ne!(default_verify(&header), Err(Error::MissingParentBlock)); } @@ -564,7 +574,7 @@ mod tests { let mut header = Header { seal: vec![vec![].into(), vec![].into()], gas_limit: test_aura_config().min_gas_limit, - parent_hash: genesis().hash(), + parent_hash: genesis().compute_hash(), ..Default::default() }; assert_eq!(default_verify(&header), Err(Error::MissingStep)); @@ -601,16 +611,16 @@ mod tests { seal: vec![ vec![45].into(), vec![142].into(), - SealedEmptyStep::rlp_of(&[sealed_empty_step(&validators, &genesis().hash(), 42)]), + SealedEmptyStep::rlp_of(&[sealed_empty_step(&validators, &genesis().compute_hash(), 42)]), ], gas_limit: test_aura_config().min_gas_limit, - parent_hash: genesis().hash(), + parent_hash: genesis().compute_hash(), ..Default::default() }; assert_eq!(verify_with_config(&config, &header), Err(Error::InsufficientProof)); // when empty step signature check fails - let mut wrong_sealed_empty_step = sealed_empty_step(&validators, &genesis().hash(), 43); + let mut wrong_sealed_empty_step = sealed_empty_step(&validators, &genesis().compute_hash(), 43); wrong_sealed_empty_step.signature = Default::default(); header.seal[2] = SealedEmptyStep::rlp_of(&[wrong_sealed_empty_step]); assert_eq!(verify_with_config(&config, &header), Err(Error::InsufficientProof)); @@ -618,15 +628,15 @@ mod tests { // when we are accepting strict empty steps and they come not in order config.strict_empty_steps_transition = 0; header.seal[2] = SealedEmptyStep::rlp_of(&[ - sealed_empty_step(&validators, &genesis().hash(), 44), - sealed_empty_step(&validators, &genesis().hash(), 43), + sealed_empty_step(&validators, &genesis().compute_hash(), 44), + sealed_empty_step(&validators, &genesis().compute_hash(), 43), ]); assert_eq!(verify_with_config(&config, &header), Err(Error::InsufficientProof)); // when empty steps are OK header.seal[2] = SealedEmptyStep::rlp_of(&[ - sealed_empty_step(&validators, &genesis().hash(), 43), - sealed_empty_step(&validators, &genesis().hash(), 44), + sealed_empty_step(&validators, &genesis().compute_hash(), 43), + sealed_empty_step(&validators, &genesis().compute_hash(), 44), ]); assert_ne!(verify_with_config(&config, &header), Err(Error::InsufficientProof)); } @@ -640,7 +650,7 @@ mod tests { let mut header = Header { seal: vec![vec![43].into(), vec![].into()], gas_limit: test_aura_config().min_gas_limit, - parent_hash: genesis().hash(), + parent_hash: genesis().compute_hash(), ..Default::default() }; assert_eq!(verify_with_config(&config, &header), Err(Error::InvalidDifficulty)); @@ -659,7 +669,7 @@ mod tests { author: validators[1].address().as_fixed_bytes().into(), seal: vec![vec![43].into(), vec![].into()], gas_limit: test_aura_config().min_gas_limit, - parent_hash: genesis().hash(), + parent_hash: genesis().compute_hash(), ..Default::default() }, 43, @@ -787,7 +797,7 @@ mod tests { assert_eq!( default_accept_into_pool(|validators| { let header = block_i(4, &validators); - hash = Some(header.hash()); + hash = Some(header.compute_hash()); (header, None) }), Ok(( @@ -819,7 +829,7 @@ mod tests { }, 47, ); - hash = Some(header.hash()); + hash = Some(header.compute_hash()); (header, None) }), Ok(( @@ -879,7 +889,7 @@ mod tests { }, 47, ); - hash = Some(header.hash()); + hash = Some(header.compute_hash()); (header, None) }), @@ -919,7 +929,7 @@ mod tests { .parse() .unwrap(); }); - hash = Some(header.hash()); + hash = Some(header.compute_hash()); (header, Some(vec![validators_change_recept(Default::default())])) }), Ok(( diff --git a/primitives/ethereum-poa/src/lib.rs b/primitives/ethereum-poa/src/lib.rs index 969af63e155..da04c13faa7 100644 --- a/primitives/ethereum-poa/src/lib.rs +++ b/primitives/ethereum-poa/src/lib.rs @@ -49,6 +49,15 @@ pub type RawTransaction = Vec; /// An ethereum address. pub type Address = H160; +/// Complete header id. +#[derive(Encode, Decode, Default, RuntimeDebug, PartialEq, Clone, Copy)] +pub struct HeaderId { + /// Header number. + pub number: u64, + /// Header hash. + pub hash: H256, +} + /// An Aura header. #[derive(Clone, Encode, Decode, PartialEq, RuntimeDebug)] #[cfg_attr(feature = "std", derive(Default, Serialize, Deserialize))] @@ -156,8 +165,16 @@ pub struct SealedEmptyStep { } impl Header { - /// Get the hash of this header (keccak of the RLP with seal). - pub fn hash(&self) -> H256 { + /// Compute id of this header. + pub fn compute_id(&self) -> HeaderId { + HeaderId { + number: self.number, + hash: self.compute_hash(), + } + } + + /// Compute hash of this header (keccak of the RLP with seal). + pub fn compute_hash(&self) -> H256 { keccak_256(&self.rlp(true)).into() } @@ -175,7 +192,7 @@ impl Header { pub fn seal_hash(&self, include_empty_steps: bool) -> Option { Some(match include_empty_steps { true => { - let mut message = self.hash().as_bytes().to_vec(); + let mut message = self.compute_hash().as_bytes().to_vec(); message.extend_from_slice(self.seal.get(2)?); keccak_256(&message).into() }