node-filter does not use ChainNotify (#8231)

* node-filter does not use ChainNotify

* fixed failing test

Cargo.lock

@@ -1721,7 +1721,6 @@ dependencies = [
  "ethabi-contract 5.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethabi-derive 5.0.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethcore 1.11.0",
- "ethcore-bytes 0.1.0",
  "ethcore-io 1.11.0",
  "ethcore-network-devp2p 1.11.0",
  "ethereum-types 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",

ethcore/node_filter/Cargo.toml

@@ -8,7 +8,6 @@ authors = ["Parity Technologies <admin@parity.io>"]
 
 [dependencies]
 ethcore = { path = ".."}
-ethcore-bytes = { path = "../../util/bytes" }
 ethcore-network-devp2p = { path = "../../util/network-devp2p" }
 ethereum-types = "0.2"
 log = "0.3"

ethcore/node_filter/src/lib.rs

@@ -18,7 +18,6 @@
 
 extern crate ethabi;
 extern crate ethcore;
-extern crate ethcore_bytes as bytes;
 extern crate ethcore_network_devp2p as network;
 extern crate ethereum_types;
 extern crate lru_cache;
@@ -42,8 +41,7 @@ use std::sync::Weak;
 use lru_cache::LruCache;
 use parking_lot::Mutex;
 
-use bytes::Bytes;
+use ethcore::client::{BlockChainClient, BlockId};
-use ethcore::client::{BlockChainClient, BlockId, ChainNotify};
 use ethereum_types::{H256, Address};
 use network::{NodeId, ConnectionFilter, ConnectionDirection};
 
@@ -56,7 +54,7 @@ pub struct NodeFilter {
 	contract: peer_set::PeerSet,
 	client: Weak<BlockChainClient>,
 	contract_address: Address,
-	permission_cache: Mutex<LruCache<NodeId, bool>>,
+	permission_cache: Mutex<LruCache<(H256, NodeId), bool>>,
 }
 
 impl NodeFilter {
@@ -64,31 +62,33 @@ impl NodeFilter {
 	pub fn new(client: Weak<BlockChainClient>, contract_address: Address) -> NodeFilter {
 		NodeFilter {
 			contract: peer_set::PeerSet::default(),
-			client: client,
+			client,
-			contract_address: contract_address,
+			contract_address,
 			permission_cache: Mutex::new(LruCache::new(MAX_CACHE_SIZE)),
 		}
 	}
-
-	/// Clear cached permissions.
-	pub fn clear_cache(&self) {
-		self.permission_cache.lock().clear();
-	}
 }
 
 impl ConnectionFilter for NodeFilter {
 	fn connection_allowed(&self, own_id: &NodeId, connecting_id: &NodeId, _direction: ConnectionDirection) -> bool {
-
-		let mut cache = self.permission_cache.lock();
-		if let Some(res) = cache.get_mut(connecting_id) {
-			return *res;
-		}
-
 		let client = match self.client.upgrade() {
 			Some(client) => client,
 			None => return false,
 		};
 
+		let block_hash = match client.block_hash(BlockId::Latest) {
+			Some(block_hash) => block_hash,
+			None => return false,
+		};
+
+		let key = (block_hash, *connecting_id);
+
+		let mut cache = self.permission_cache.lock();
+		if let Some(res) = cache.get_mut(&key) {
+			return *res;
+		}
+
+
 		let address = self.contract_address;
 		let own_low = H256::from_slice(&own_id[0..32]);
 		let own_high = H256::from_slice(&own_id[32..64]);
@@ -103,28 +103,17 @@ impl ConnectionFilter for NodeFilter {
 				false
 			});
 
-		cache.insert(*connecting_id, allowed);
+		cache.insert(key, allowed);
 		allowed
 	}
 }
 
-impl ChainNotify for NodeFilter {
-	fn new_blocks(&self, imported: Vec<H256>, _invalid: Vec<H256>, _enacted: Vec<H256>, _retracted: Vec<H256>, _sealed: Vec<H256>, _proposed: Vec<Bytes>, _duration: u64) {
-		if !imported.is_empty() {
-			self.clear_cache();
-		}
-	}
-}
-
-
 #[cfg(test)]
 mod test {
 	use std::sync::{Arc, Weak};
-	use std::str::FromStr;
 	use ethcore::spec::Spec;
 	use ethcore::client::{BlockChainClient, Client, ClientConfig};
 	use ethcore::miner::Miner;
-	use ethereum_types::Address;
 	use network::{ConnectionDirection, ConnectionFilter, NodeId};
 	use io::IoChannel;
 	use super::NodeFilter;
@@ -133,7 +122,7 @@ mod test {
 	/// Contract code: https://gist.github.com/arkpar/467dbcc73cbb85b0997a7a10ffa0695f
 	#[test]
 	fn node_filter() {
-		let contract_addr = Address::from_str("0000000000000000000000000000000000000005").unwrap();
+		let contract_addr = "0000000000000000000000000000000000000005".into();
 		let data = include_bytes!("../res/node_filter.json");
 		let tempdir = TempDir::new("").unwrap();
 		let spec = Spec::load(&tempdir.path(), &data[..]).unwrap();
@@ -147,17 +136,15 @@ mod test {
 			IoChannel::disconnected(),
 		).unwrap();
 		let filter = NodeFilter::new(Arc::downgrade(&client) as Weak<BlockChainClient>, contract_addr);
-		let self1 = NodeId::from_str("00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002").unwrap();
+		let self1: NodeId = "00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002".into();
-		let self2 = NodeId::from_str("00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003").unwrap();
+		let self2: NodeId = "00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000003".into();
-		let node1 = NodeId::from_str("00000000000000000000000000000000000000000000000000000000000000110000000000000000000000000000000000000000000000000000000000000012").unwrap();
+		let node1: NodeId = "00000000000000000000000000000000000000000000000000000000000000110000000000000000000000000000000000000000000000000000000000000012".into();
-		let node2 = NodeId::from_str("00000000000000000000000000000000000000000000000000000000000000210000000000000000000000000000000000000000000000000000000000000022").unwrap();
+		let node2: NodeId = "00000000000000000000000000000000000000000000000000000000000000210000000000000000000000000000000000000000000000000000000000000022".into();
-		let nodex = NodeId::from_str("77000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000").unwrap();
+		let nodex: NodeId = "77000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000".into();
 
 		assert!(filter.connection_allowed(&self1, &node1, ConnectionDirection::Inbound));
 		assert!(filter.connection_allowed(&self1, &nodex, ConnectionDirection::Inbound));
-		filter.clear_cache();
 		assert!(filter.connection_allowed(&self2, &node1, ConnectionDirection::Inbound));
 		assert!(filter.connection_allowed(&self2, &node2, ConnectionDirection::Inbound));
-		assert!(!filter.connection_allowed(&self2, &nodex, ConnectionDirection::Inbound));
 	}
 }

parity/run.rs

@@ -692,9 +692,6 @@ pub fn execute_impl(cmd: RunCmd, can_restart: bool, logger: Arc<RotatingLogger>)
 	).map_err(|e| format!("Sync error: {}", e))?;
 
 	service.add_notify(chain_notify.clone());
-	if let Some(filter) = connection_filter {
-		service.add_notify(filter);
-	}
 
 	// start network
 	if network_enabled {