From 3f33370e7dde69888926af4736de0da9d618f445 Mon Sep 17 00:00:00 2001 From: Fredrik Harrysson Date: Mon, 12 Mar 2018 11:06:48 +0100 Subject: [PATCH] Echo back the message hash of a ping in the pong request (#8042) * Echo back the message hash of a ping in the pong request * Fixed broken link in README (#8012) * Fixed broken link in README * Updated wiki link * [hardware wallet] sleeping -> pollling (#8018) * Use polling, enable missing doc warnings & docs * make try_connect_polling() a free function * `Client` refactoring (#7038) * Improves `BestBlock` comment * Improves `TraceDB` comment * Improves `journaldb::Algorithm` comment. Probably the whole enum should be renamed to `Strategy` or something alike. * Comments some of the `Client`'s fields * Deglobs client imports * Fixes comments * Extracts `import_lock` to `Importer` struct * Extracts `verifier` to `Importer` struct * Extracts `block_queue` to `Importer` struct * Extracts `miner` to `Importer` struct * Extracts `ancient_verifier` to `Importer` struct * Extracts `rng` to `Importer` struct * Extracts `import_old_block` to `Importer` struct * Adds `Nonce` trait * Adds `Balance` trait * Adds `ChainInfo` trait * Fixes imports for tests using `chain_info` method * Adds `BlockInfo` trait * Adds more `ChainInfo` imports * Adds `BlockInfo` imports * Adds `ReopenBlock` trait * Adds `PrepareOpenBlock` trait * Fixes import in tests * Adds `CallContract` trait * Fixes imports in tests using `call_contract` method * Adds `TransactionInfo` trait * Adds `RegistryInfo` trait * Fixes imports in tests using `registry_address` method * Adds `ScheduleInfo` trait * Adds `ImportSealedBlock` trait * Fixes imports in test using `import_sealed_block` method * Adds `BroadcastProposalBlock` trait * Migrates `Miner` to static dispatch * Fixes tests * Moves `calculate_enacted_retracted` to `Importer` * Moves import-related methods to `Importer` * Removes redundant `import_old_block` wrapper * Extracts `import_block*` into separate trait * Fixes tests * Handles `Pending` in `LightFetch` * Handles `Pending` in filters * Handles `Pending` in `ParityClient` * Handles `Pending` in `EthClient` * Removes `BlockId::Pending`, partly refactors dependent code * Adds `StateInfo` trait * Exports `StateOrBlock` and `BlockChain` types from `client` module * Refactors `balance` RPC using generic API * Refactors `storage_at` RPC using generic API * Makes `MinerService::pending_state`'s return type dynamic * Adds `StateOrBlock` and `BlockChain` types * Adds impl of `client::BlockChain` for `Client` * Exports `StateInfo` trait from `client` module * Missing `self` use To be fixed up to "Adds impl of `client::BlockChain` for `Client`" * Adds `number_to_id` and refactors dependent RPC methods * Refactors `code_at` using generic API * Adds `StateClient` trait * Refactors RPC to use `StateClient` trait * Reverts `client::BlockChain` trait stuff, refactors methods to accept `StateOrBlock` * Refactors TestClient * Adds helper function `block_number_to_id` * Uses `block_number_to_id` instead of local function * Handles `Pending` in `list_accounts` and `list_storage_keys` * Attempt to use associated types for state instead of trait objects * Simplifies `state_at_beginning` * Extracts `call` and `call_many` into separate trait * Refactors `build_last_hashes` to accept reference * Exports `Call` type from the module * Refactors `call` and `call_many` to accept state and header * Exports `state_at` in `StateClient` * Exports `pending_block_header` from `MinerService` * Refactors RPC `call` method using new API * Adds missing parentheses * Refactors `parity::call` to use new call API * Update .gitlab-ci.yml fix gitlab lint * Fixes error handling * Refactors `traces::call` and `call_many` to use new call API * Refactors `call_contract` * Refactors `block_header` * Refactors internal RPC method `block` * Moves `estimate_gas` to `Call` trait, refactors parameters * Refactors `estimate_gas` in RPC * Refactors `uncle` * Refactors RPC `transaction` * Covers missing branches * Makes it all compile, fixes compiler grumbles * Adds casts in `blockchain` module * Fixes `PendingBlock` tests, work on `MinerService` * Adds test stubs for StateClient and EngineInfo * Makes `state_db` public * Adds missing impls for `TestBlockChainClient` * Adds trait documentation * Adds missing docs to the `state_db` module * Fixes trivial compilation errors * Moves `code_hash` method to a `BlockInfo` trait * Refactors `Verifier` to be generic over client * Refactors `TransactionFilter` to be generic over client * Refactors `Miner` and `Client` to reflect changes in verifier and txfilter API * Moves `ServiceTransactionChecker` back to `ethcore` * Fixes trait bounds in `Miner` API * Fixes `Client` * Fixes lifetime bound in `FullFamilyParams` * Adds comments to `FullFamilyParams` * Fixes imports in `ethcore` * Fixes BlockNumber handling in `code_at` and `replay_block_transactions` * fix compile issues * First step to redundant trait merge * Fixes compilation error in RPC tests * Adds mock `State` as a stub for `TestClient` * Handles `StateOrBlock::State` in `TestBlockChainClient::balance` * Fixes `transaction_count` RPC * Fixes `transaction_count` * Moves `service_transaction.json` to the `contracts` subfolder * Fixes compilation errors in tests * Refactors client to use `AccountData` * Refactors client to use `BlockChain` * Refactors miner to use aggregate traits * Adds `SealedBlockImporter` trait * Refactors miner to use `SealedBlockImporter` trait * Removes unused imports * Simplifies `RegistryInfo::registry_address` * Fixes indentation * Removes commented out trait bound * Bump master to 1.11.0 (#8021) * Bump master to 1.11.0 * Bump price-info * Bump mac installer version * Fix gitlab builds * Add MCIP-6 Byzyantium transition to Musicoin spec (#7841) * Add test chain spec for musicoin byzantium testnet * Add MCIP-6 Byzyantium transition to Musicoin spec * Update mcip6_byz.json * ethcore: update musicoin byzantium block number * ethcore: update musicoin byzantium block number * ethcore: update musicoin bootnodes * Update musicoin.json * Update musicoin.json * More bootnodes. * prelude to the block module cleanup (#8025) * prelude to block cleanup * fixed tests * fix cache & snapcraft CI build (#8052) after successful testing it is necessary to port in a ```beta``` and ```stable``` * Update refs to shell (#8051) * Abstract devp2p (#8048) * Rename ethcore-network to ethcore-network-devp2p * Fix typo * Extract generic traits into util/network * Simplify util/network * Fix devp2p tests * Remove old feature * Fix RPC tests * Change port because testing environment didn't like those ports --- util/network-devp2p/src/discovery.rs | 31 +++++++++++++++++++++------ util/network-devp2p/src/node_table.rs | 1 + 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/util/network-devp2p/src/discovery.rs b/util/network-devp2p/src/discovery.rs index 8f51ed8d2..5e22e31d5 100644 --- a/util/network-devp2p/src/discovery.rs +++ b/util/network-devp2p/src/discovery.rs @@ -179,6 +179,7 @@ impl Discovery { } } + /// Removes the timeout of a given NodeId if it can be found in one of the discovery buckets fn clear_ping(&mut self, id: &NodeId) { let bucket = &mut self.node_buckets[Discovery::distance(&self.id_hash, &keccak(id)) as usize]; if let Some(node) = bucket.nodes.iter_mut().find(|n| &n.address.id == id) { @@ -186,6 +187,7 @@ impl Discovery { } } + /// Starts the discovery process at round 0 fn start(&mut self) { trace!(target: "discovery", "Starting discovery"); self.discovery_round = 0; @@ -379,7 +381,7 @@ impl Discovery { let packet_id = signed[0]; let rlp = UntrustedRlp::new(&signed[1..]); match packet_id { - PACKET_PING => self.on_ping(&rlp, &node_id, &from), + PACKET_PING => self.on_ping(&rlp, &node_id, &from, &hash_signed), PACKET_PONG => self.on_pong(&rlp, &node_id, &from), PACKET_FIND_NODE => self.on_find_node(&rlp, &node_id, &from), PACKET_NEIGHBOURS => self.on_neighbours(&rlp, &node_id, &from), @@ -390,6 +392,7 @@ impl Discovery { } } + /// Validate that given timestamp is in within one second of now or in the future fn check_timestamp(&self, timestamp: u64) -> Result<(), Error> { if self.check_timestamps && timestamp < time::get_time().sec as u64{ debug!(target: "discovery", "Expired packet"); @@ -402,7 +405,7 @@ impl Discovery { entry.endpoint.is_allowed(&self.ip_filter) && entry.id != self.id } - fn on_ping(&mut self, rlp: &UntrustedRlp, node: &NodeId, from: &SocketAddr) -> Result, Error> { + fn on_ping(&mut self, rlp: &UntrustedRlp, node: &NodeId, from: &SocketAddr, echo_hash: &[u8]) -> Result, Error> { trace!(target: "discovery", "Got Ping from {:?}", &from); let source = NodeEndpoint::from_rlp(&rlp.at(1)?)?; let dest = NodeEndpoint::from_rlp(&rlp.at(2)?)?; @@ -418,10 +421,9 @@ impl Discovery { self.update_node(entry.clone()); added_map.insert(node.clone(), entry); } - let hash = keccak(rlp.as_raw()); let mut response = RlpStream::new_list(2); dest.to_rlp_list(&mut response); - response.append(&hash); + response.append(&echo_hash); self.send_packet(PACKET_PONG, from, &response.drain()); Ok(Some(TableUpdates { added: added_map, removed: HashSet::new() })) @@ -429,7 +431,7 @@ impl Discovery { fn on_pong(&mut self, rlp: &UntrustedRlp, node: &NodeId, from: &SocketAddr) -> Result, Error> { trace!(target: "discovery", "Got Pong from {:?}", &from); - // TODO: validate pong packet + // TODO: validate pong packet in rlp.val_at(1) let dest = NodeEndpoint::from_rlp(&rlp.at(0)?)?; let timestamp: u64 = rlp.val_at(2)?; self.check_timestamp(timestamp)?; @@ -439,8 +441,6 @@ impl Discovery { entry.endpoint.address = from.clone(); } self.clear_ping(node); - let mut added_map = HashMap::new(); - added_map.insert(node.clone(), entry); Ok(None) } @@ -707,4 +707,21 @@ mod tests { assert!(discovery.on_packet(&packet, from.clone()).is_ok()); } + #[test] + fn test_ping() { + let key1 = Random.generate().unwrap(); + let key2 = Random.generate().unwrap(); + let ep1 = NodeEndpoint { address: SocketAddr::from_str("127.0.0.1:40344").unwrap(), udp_port: 40344 }; + let ep2 = NodeEndpoint { address: SocketAddr::from_str("127.0.0.1:40345").unwrap(), udp_port: 40345 }; + let mut discovery1 = Discovery::new(&key1, ep1.address.clone(), ep1.clone(), 0, IpFilter::default()); + let mut discovery2 = Discovery::new(&key2, ep2.address.clone(), ep2.clone(), 0, IpFilter::default()); + + discovery1.ping(&ep2); + let ping_data = discovery1.send_queue.pop_front().unwrap(); + discovery2.on_packet(&ping_data.payload, ep1.address.clone()).ok(); + let pong_data = discovery2.send_queue.pop_front().unwrap(); + let data = &pong_data.payload[(32 + 65)..]; + let rlp = UntrustedRlp::new(&data[1..]); + assert_eq!(ping_data.payload[0..32], rlp.val_at::>(1).unwrap()[..]) + } } diff --git a/util/network-devp2p/src/node_table.rs b/util/network-devp2p/src/node_table.rs index c1626e304..c3927c694 100644 --- a/util/network-devp2p/src/node_table.rs +++ b/util/network-devp2p/src/node_table.rs @@ -101,6 +101,7 @@ impl NodeEndpoint { self.to_rlp(rlp); } + /// Validates that the port is not 0 and address IP is specified pub fn is_valid(&self) -> bool { self.udp_port != 0 && self.address.port() != 0 && match self.address {