Fix fork choice (#10837)
* Fix fork choice: `is_from_route_finalized` check before switching to parent * Add tests for `tree_route` with finalization * Fix Cargo dependencies * Add comment on `tree_route` for finalization. Refactor a test. * Fix compilation error * Checkout Cargo.lock from master
This commit is contained in:
		
							parent
							
								
									44cc442d12
								
							
						
					
					
						commit
						073d242d1e
					
				
							
								
								
									
										2
									
								
								Cargo.lock
									
									
									
										generated
									
									
									
								
							
							
						
						
									
										2
									
								
								Cargo.lock
									
									
									
										generated
									
									
									
								
							@ -962,12 +962,14 @@ dependencies = [
 | 
			
		||||
 "parity-bytes 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 | 
			
		||||
 "parity-util-mem 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 | 
			
		||||
 "parking_lot 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
 | 
			
		||||
 "rand 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)",
 | 
			
		||||
 "rayon 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 | 
			
		||||
 "rlp 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
 | 
			
		||||
 "rlp_compress 0.1.0",
 | 
			
		||||
 "rlp_derive 0.1.0",
 | 
			
		||||
 "rustc-hex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
 | 
			
		||||
 "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
 | 
			
		||||
 "triehash-ethereum 0.2.0",
 | 
			
		||||
]
 | 
			
		||||
 | 
			
		||||
[[package]]
 | 
			
		||||
 | 
			
		||||
@ -13,21 +13,23 @@ blooms-db = { path = "../../util/blooms-db" }
 | 
			
		||||
common-types = { path = "../types" }
 | 
			
		||||
ethcore-db = { path = "../db" }
 | 
			
		||||
ethereum-types = "0.6.0"
 | 
			
		||||
keccak-hash = "0.2.0"
 | 
			
		||||
parity-util-mem = "0.1"
 | 
			
		||||
itertools = "0.5"
 | 
			
		||||
kvdb = "0.1"
 | 
			
		||||
log = "0.4"
 | 
			
		||||
parity-bytes = "0.1"
 | 
			
		||||
parking_lot = "0.8"
 | 
			
		||||
rand = "0.6"
 | 
			
		||||
rayon = "1.0"
 | 
			
		||||
rlp = "0.4.0"
 | 
			
		||||
rlp_compress = { path = "../../util/rlp-compress" }
 | 
			
		||||
rlp_derive = { path = "../../util/rlp-derive" }
 | 
			
		||||
triehash-ethereum = { version = "0.2",  path = "../../util/triehash-ethereum" }
 | 
			
		||||
 | 
			
		||||
[dev-dependencies]
 | 
			
		||||
env_logger = "0.5"
 | 
			
		||||
ethkey = { path = "../../accounts/ethkey" }
 | 
			
		||||
keccak-hash = "0.2.0"
 | 
			
		||||
rustc-hex = "1.0"
 | 
			
		||||
tempdir = "0.3"
 | 
			
		||||
kvdb-memorydb = "0.1"
 | 
			
		||||
 | 
			
		||||
@ -710,6 +710,10 @@ impl BlockChain {
 | 
			
		||||
	///
 | 
			
		||||
	/// If the tree route verges into pruned or unknown blocks,
 | 
			
		||||
	/// `None` is returned.
 | 
			
		||||
	///
 | 
			
		||||
	/// `is_from_route_finalized` returns whether the `from` part of the
 | 
			
		||||
	/// route contains a finalized block. This only holds if the two parts (from
 | 
			
		||||
	/// and to) are on different branches, ie. on 2 different forks.
 | 
			
		||||
	pub fn tree_route(&self, from: H256, to: H256) -> Option<TreeRoute> {
 | 
			
		||||
		let mut from_branch = vec![];
 | 
			
		||||
		let mut is_from_route_finalized = false;
 | 
			
		||||
@ -723,9 +727,9 @@ impl BlockChain {
 | 
			
		||||
		// reset from && to to the same level
 | 
			
		||||
		while from_details.number > to_details.number {
 | 
			
		||||
			from_branch.push(current_from);
 | 
			
		||||
			is_from_route_finalized = is_from_route_finalized || from_details.is_finalized;
 | 
			
		||||
			current_from = from_details.parent.clone();
 | 
			
		||||
			from_details = self.block_details(&from_details.parent)?;
 | 
			
		||||
			is_from_route_finalized = is_from_route_finalized || from_details.is_finalized;
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
		while to_details.number > from_details.number {
 | 
			
		||||
@ -739,9 +743,9 @@ impl BlockChain {
 | 
			
		||||
		// move to shared parent
 | 
			
		||||
		while current_from != current_to {
 | 
			
		||||
			from_branch.push(current_from);
 | 
			
		||||
			is_from_route_finalized = is_from_route_finalized || from_details.is_finalized;
 | 
			
		||||
			current_from = from_details.parent.clone();
 | 
			
		||||
			from_details = self.block_details(&from_details.parent)?;
 | 
			
		||||
			is_from_route_finalized = is_from_route_finalized || from_details.is_finalized;
 | 
			
		||||
 | 
			
		||||
			to_branch.push(current_to);
 | 
			
		||||
			current_to = to_details.parent.clone();
 | 
			
		||||
@ -2503,4 +2507,74 @@ mod tests {
 | 
			
		||||
			assert_eq!(bc.epoch_transition_for(fork_hash).unwrap().block_number, 0);
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	#[test]
 | 
			
		||||
	fn tree_rout_with_finalization() {
 | 
			
		||||
		let genesis = BlockBuilder::genesis();
 | 
			
		||||
		let a = genesis.add_block();
 | 
			
		||||
		// First branch
 | 
			
		||||
		let a1 = a.add_block_with_random_transactions();
 | 
			
		||||
		let a2 = a1.add_block_with_random_transactions();
 | 
			
		||||
		let a3 = a2.add_block_with_random_transactions();
 | 
			
		||||
		// Second branch
 | 
			
		||||
		let b1 = a.add_block_with_random_transactions();
 | 
			
		||||
		let b2 = b1.add_block_with_random_transactions();
 | 
			
		||||
 | 
			
		||||
		let a_hash = a.last().hash();
 | 
			
		||||
		let a1_hash = a1.last().hash();
 | 
			
		||||
		let a2_hash = a2.last().hash();
 | 
			
		||||
		let a3_hash = a3.last().hash();
 | 
			
		||||
		let b2_hash = b2.last().hash();
 | 
			
		||||
 | 
			
		||||
		let bootstrap_chain = |blocks: Vec<&BlockBuilder>| {
 | 
			
		||||
			let db = new_db();
 | 
			
		||||
			let bc = new_chain(genesis.last().encoded(), db.clone());
 | 
			
		||||
			let mut batch = db.key_value().transaction();
 | 
			
		||||
			for block in blocks {
 | 
			
		||||
				insert_block_batch(&mut batch, &bc, block.last().encoded(), vec![]);
 | 
			
		||||
				bc.commit();
 | 
			
		||||
			}
 | 
			
		||||
			db.key_value().write(batch).unwrap();
 | 
			
		||||
			(db, bc)
 | 
			
		||||
		};
 | 
			
		||||
 | 
			
		||||
		let mark_finalized = |block_hash: H256, db: &Arc<dyn BlockChainDB>, bc: &BlockChain| {
 | 
			
		||||
			let mut batch = db.key_value().transaction();
 | 
			
		||||
			bc.mark_finalized(&mut batch, block_hash).unwrap();
 | 
			
		||||
			bc.commit();
 | 
			
		||||
			db.key_value().write(batch).unwrap();
 | 
			
		||||
		};
 | 
			
		||||
 | 
			
		||||
		// Case 1: fork, with finalized common ancestor
 | 
			
		||||
		{
 | 
			
		||||
			let (db, bc) = bootstrap_chain(vec![&a, &a1, &a2, &a3, &b1, &b2]);
 | 
			
		||||
			assert_eq!(bc.best_block_hash(), a3_hash);
 | 
			
		||||
			assert_eq!(bc.block_hash(2).unwrap(), a1_hash);
 | 
			
		||||
 | 
			
		||||
			mark_finalized(a_hash, &db, &bc);
 | 
			
		||||
			assert!(!bc.tree_route(a3_hash, b2_hash).unwrap().is_from_route_finalized);
 | 
			
		||||
			assert!(!bc.tree_route(b2_hash, a3_hash).unwrap().is_from_route_finalized);
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
		// Case 2: fork with a finalized block on a branch
 | 
			
		||||
		{
 | 
			
		||||
			let (db, bc) = bootstrap_chain(vec![&a, &a1, &a2, &a3, &b1, &b2]);
 | 
			
		||||
			assert_eq!(bc.best_block_hash(), a3_hash);
 | 
			
		||||
			assert_eq!(bc.block_hash(2).unwrap(), a1_hash);
 | 
			
		||||
 | 
			
		||||
			mark_finalized(a2_hash, &db, &bc);
 | 
			
		||||
			assert!(bc.tree_route(a3_hash, b2_hash).unwrap().is_from_route_finalized);
 | 
			
		||||
			assert!(!bc.tree_route(b2_hash, a3_hash).unwrap().is_from_route_finalized);
 | 
			
		||||
		}
 | 
			
		||||
 | 
			
		||||
		// Case 3: no-fork, with a finalized block
 | 
			
		||||
		{
 | 
			
		||||
			let (db, bc) = bootstrap_chain(vec![&a, &a1, &a2]);
 | 
			
		||||
			assert_eq!(bc.best_block_hash(), a2_hash);
 | 
			
		||||
 | 
			
		||||
			mark_finalized(a1_hash, &db, &bc);
 | 
			
		||||
			assert!(!bc.tree_route(a1_hash, a2_hash).unwrap().is_from_route_finalized);
 | 
			
		||||
			assert!(!bc.tree_route(a2_hash, a1_hash).unwrap().is_from_route_finalized);
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
@ -21,11 +21,13 @@ use ethereum_types::{U256, H256, Bloom};
 | 
			
		||||
 | 
			
		||||
use common_types::encoded;
 | 
			
		||||
use common_types::header::Header;
 | 
			
		||||
use common_types::transaction::SignedTransaction;
 | 
			
		||||
use common_types::transaction::{SignedTransaction, Transaction, Action};
 | 
			
		||||
use common_types::view;
 | 
			
		||||
use common_types::views::BlockView;
 | 
			
		||||
use keccak_hash::keccak;
 | 
			
		||||
use rlp::encode;
 | 
			
		||||
use rlp_derive::RlpEncodable;
 | 
			
		||||
use triehash_ethereum::ordered_trie_root;
 | 
			
		||||
 | 
			
		||||
/// Helper structure, used for encoding blocks.
 | 
			
		||||
#[derive(Default, Clone, RlpEncodable)]
 | 
			
		||||
@ -136,6 +138,29 @@ impl BlockBuilder {
 | 
			
		||||
		})
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	/// Add a block with randomly generated transactions.
 | 
			
		||||
	#[inline]
 | 
			
		||||
	pub fn add_block_with_random_transactions(&self) -> Self {
 | 
			
		||||
		// Maximum of ~50 transactions
 | 
			
		||||
		let count = rand::random::<u8>() as usize / 5;
 | 
			
		||||
		let transactions = std::iter::repeat_with(|| {
 | 
			
		||||
			let data_len = rand::random::<u8>();
 | 
			
		||||
			let data = std::iter::repeat_with(|| rand::random::<u8>())
 | 
			
		||||
				.take(data_len as usize)
 | 
			
		||||
				.collect::<Vec<_>>();
 | 
			
		||||
			Transaction {
 | 
			
		||||
				nonce: 0.into(),
 | 
			
		||||
				gas_price: 0.into(),
 | 
			
		||||
				gas: 100_000.into(),
 | 
			
		||||
				action: Action::Create,
 | 
			
		||||
				value: 100.into(),
 | 
			
		||||
				data,
 | 
			
		||||
			}.sign(&keccak("").into(), None)
 | 
			
		||||
		}).take(count);
 | 
			
		||||
 | 
			
		||||
		self.add_block_with_transactions(transactions)
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	/// Add a block with given transactions.
 | 
			
		||||
	#[inline]
 | 
			
		||||
	pub fn add_block_with_transactions<T>(&self, transactions: T) -> Self
 | 
			
		||||
@ -166,11 +191,15 @@ impl BlockBuilder {
 | 
			
		||||
			let mut block = Block::default();
 | 
			
		||||
			let metadata = get_metadata();
 | 
			
		||||
			let block_number = parent_number + 1;
 | 
			
		||||
			let transactions = metadata.transactions;
 | 
			
		||||
			let transactions_root = ordered_trie_root(transactions.iter().map(rlp::encode));
 | 
			
		||||
 | 
			
		||||
			block.header.set_parent_hash(parent_hash);
 | 
			
		||||
			block.header.set_number(block_number);
 | 
			
		||||
			block.header.set_log_bloom(metadata.bloom);
 | 
			
		||||
			block.header.set_difficulty(metadata.difficulty);
 | 
			
		||||
			block.transactions = metadata.transactions;
 | 
			
		||||
			block.header.set_transactions_root(transactions_root);
 | 
			
		||||
			block.transactions = transactions;
 | 
			
		||||
 | 
			
		||||
			parent_hash = block.hash();
 | 
			
		||||
			parent_number = block_number;
 | 
			
		||||
 | 
			
		||||
@ -288,7 +288,7 @@ impl Provider {
 | 
			
		||||
		let mut state_buf = [0u8; 64];
 | 
			
		||||
		state_buf[..32].clone_from_slice(state_hash.as_bytes());
 | 
			
		||||
		state_buf[32..].clone_from_slice(nonce_h256.as_bytes());
 | 
			
		||||
		keccak(&state_buf.as_ref())
 | 
			
		||||
		keccak(AsRef::<[u8]>::as_ref(&state_buf[..]))
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	fn pool_client<'a>(&'a self, nonce_cache: &'a NonceCache, local_accounts: &'a HashSet<Address>) -> miner::pool_client::PoolClient<'a, Client> {
 | 
			
		||||
 | 
			
		||||
@ -463,7 +463,7 @@ mod tests {
 | 
			
		||||
 | 
			
		||||
	#[test]
 | 
			
		||||
	fn should_disallow_unsigned_transactions() {
 | 
			
		||||
		let rlp = "ea80843b9aca0083015f90948921ebb5f79e9e3920abe571004d0b1d5119c154865af3107a400080038080".into();
 | 
			
		||||
		let rlp = "ea80843b9aca0083015f90948921ebb5f79e9e3920abe571004d0b1d5119c154865af3107a400080038080";
 | 
			
		||||
		let transaction: UnverifiedTransaction = ::rlp::decode(&::rustc_hex::FromHex::from_hex(rlp).unwrap()).unwrap();
 | 
			
		||||
		let spec = ::ethereum::new_ropsten_test();
 | 
			
		||||
		let ethparams = get_default_ethash_extensions();
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user