From e3f7b70c3805c4dba6720b39af27d2a1d67afc8c Mon Sep 17 00:00:00 2001 From: Andrew Jones Date: Thu, 29 Mar 2018 10:19:45 +0100 Subject: [PATCH] Replace all Rlp usages with UntrustedRlp except for ethcore views (#8233) * Replace Rlp with UntrustedRlp and unsafely unwrap All Rlp methods return Result<_,DecoderError> now, so for this first pass each will be marked with `expect("TODO")`. In the next pass we can categorise figure out how to handle each case. * Handle DecoderError for tendermint message * Unwrap rlp results in TestBlockcChainClient Rlp should be valid since created manually in tests * Replace `use rlp::*` with explicit imports * Remove rlp decode unwraps from light cli request * Structured rlp encoding for curr best and latest in header chain * Propogate decoder errors from send_packet * Fix body uncles rlp index * Use BodyView in sync and `expect` rlp errors * Revert bbf28f removing original Rlp for this phase This can be done again in the next phase, in order that we can leave the ethcore views unchanged * Restore legacy Rlp and UntrustedRlp Use legacy Rlp for ethcore views. Will redo replacing Rlp with UntrustedRlp in a subsequent PR * Fix tests * Replace boilerplate Encodable/Decodable with derive * Use BlockView instead of Rlp, remove unwrap * Remove rlp test_cli unwraps by using BlockView instead of Rlp directly * Remove unneccesary change to use borrowed hash * Construct sync block using new_from_header_and_body --- ethcore/light/src/client/header_chain.rs | 34 +++++++++++++++-------- ethcore/light/src/net/request_credits.rs | 2 +- ethcore/light/src/net/tests/mod.rs | 2 +- ethcore/light/src/on_demand/request.rs | 19 ++++--------- ethcore/src/blockchain/blockchain.rs | 2 +- ethcore/src/client/test_client.rs | 16 ++++++----- ethcore/src/encoded.rs | 17 +++++++++++- ethcore/src/engines/tendermint/message.rs | 4 +-- ethcore/src/header.rs | 2 +- ethcore/src/state/account.rs | 2 +- ethcore/src/trace/types/flat.rs | 2 +- ethcore/src/trace/types/trace.rs | 2 +- ethcore/src/views/block.rs | 10 +++++++ ethcore/src/views/body.rs | 11 +++++++- ethcore/transaction/src/transaction.rs | 1 - ethcore/types/src/receipt.rs | 2 +- ethcore/types/src/snapshot_manifest.rs | 2 +- sync/src/block_sync.rs | 2 +- sync/src/blocks.rs | 15 ++++------ sync/src/chain.rs | 2 +- util/network-devp2p/src/connection.rs | 2 +- util/network-devp2p/src/discovery.rs | 27 ++++++++++-------- util/network-devp2p/src/handshake.rs | 2 +- util/network-devp2p/src/host.rs | 3 +- util/network-devp2p/src/node_table.rs | 2 +- util/network-devp2p/src/session.rs | 2 +- 26 files changed, 113 insertions(+), 74 deletions(-) diff --git a/ethcore/light/src/client/header_chain.rs b/ethcore/light/src/client/header_chain.rs index 3bbd6cb95..84b7916ee 100644 --- a/ethcore/light/src/client/header_chain.rs +++ b/ethcore/light/src/client/header_chain.rs @@ -41,7 +41,7 @@ use ethcore::engines::epoch::{ PendingTransition as PendingEpochTransition }; -use rlp::{Encodable, Decodable, DecoderError, RlpStream, Rlp, UntrustedRlp}; +use rlp::{Encodable, Decodable, DecoderError, RlpStream, UntrustedRlp}; use heapsize::HeapSizeOf; use ethereum_types::{H256, H264, U256}; use plain_hasher::H256FastMap; @@ -74,6 +74,22 @@ pub struct BlockDescriptor { pub total_difficulty: U256, } +// best block data +#[derive(RlpEncodable, RlpDecodable)] +struct BestAndLatest { + best_num: u64, + latest_num: u64 +} + +impl BestAndLatest { + fn new(best_num: u64, latest_num: u64) -> Self { + BestAndLatest { + best_num, + latest_num, + } + } +} + // candidate block description. struct Candidate { hash: H256, @@ -212,12 +228,9 @@ impl HeaderChain { let decoded_header = spec.genesis_header(); let chain = if let Some(current) = db.get(col, CURRENT_KEY)? { - let (best_number, highest_number) = { - let rlp = Rlp::new(¤t); - (rlp.val_at(0), rlp.val_at(1)) - }; + let curr : BestAndLatest = ::rlp::decode(¤t); - let mut cur_number = highest_number; + let mut cur_number = curr.latest_num; let mut candidates = BTreeMap::new(); // load all era entries, referenced headers within them, @@ -245,7 +258,7 @@ impl HeaderChain { // fill best block block descriptor. let best_block = { - let era = match candidates.get(&best_number) { + let era = match candidates.get(&curr.best_num) { Some(era) => era, None => return Err(Error::Database("Database corrupt: highest block referenced but no data.".into())), }; @@ -253,7 +266,7 @@ impl HeaderChain { let best = &era.candidates[0]; BlockDescriptor { hash: best.hash, - number: best_number, + number: curr.best_num, total_difficulty: best.total_difficulty, } }; @@ -542,9 +555,8 @@ impl HeaderChain { // write the best and latest eras to the database. { let latest_num = *candidates.iter().rev().next().expect("at least one era just inserted; qed").0; - let mut stream = RlpStream::new_list(2); - stream.append(&best_num).append(&latest_num); - transaction.put(self.col, CURRENT_KEY, &stream.out()) + let curr = BestAndLatest::new(best_num, latest_num); + transaction.put(self.col, CURRENT_KEY, &::rlp::encode(&curr)) } Ok(pending) } diff --git a/ethcore/light/src/net/request_credits.rs b/ethcore/light/src/net/request_credits.rs index f97ce85bc..c35a29222 100644 --- a/ethcore/light/src/net/request_credits.rs +++ b/ethcore/light/src/net/request_credits.rs @@ -29,7 +29,7 @@ use request::{self, Request}; use super::error::Error; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, Decodable, Encodable, DecoderError}; use ethereum_types::U256; use std::time::{Duration, Instant}; diff --git a/ethcore/light/src/net/tests/mod.rs b/ethcore/light/src/net/tests/mod.rs index c3c792b78..4beb32833 100644 --- a/ethcore/light/src/net/tests/mod.rs +++ b/ethcore/light/src/net/tests/mod.rs @@ -31,7 +31,7 @@ use provider::Provider; use request; use request::*; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream}; use ethereum_types::{H256, U256, Address}; use std::sync::Arc; diff --git a/ethcore/light/src/on_demand/request.rs b/ethcore/light/src/on_demand/request.rs index e7303ad88..8b184cc1d 100644 --- a/ethcore/light/src/on_demand/request.rs +++ b/ethcore/light/src/on_demand/request.rs @@ -439,13 +439,7 @@ impl CheckedRequest { block_header .and_then(|hdr| cache.block_body(&block_hash).map(|b| (hdr, b))) .map(|(hdr, body)| { - let mut stream = RlpStream::new_list(3); - let body = body.rlp(); - stream.append_raw(&hdr.rlp().as_raw(), 1); - stream.append_raw(&body.at(0).as_raw(), 1); - stream.append_raw(&body.at(1).as_raw(), 1); - - Response::Body(encoded::Block::new(stream.out())) + Response::Body(encoded::Block::new_from_header_and_body(&hdr.view(), &body.view())) }) } CheckedRequest::Code(_, ref req) => { @@ -778,25 +772,22 @@ impl Body { pub fn check_response(&self, cache: &Mutex<::cache::Cache>, body: &encoded::Body) -> Result { // check the integrity of the the body against the header let header = self.0.as_ref()?; - let tx_root = ::triehash::ordered_trie_root(body.rlp().at(0).iter().map(|r| r.as_raw())); + let tx_root = ::triehash::ordered_trie_root(body.transactions_rlp().iter().map(|r| r.as_raw())); if tx_root != header.transactions_root() { return Err(Error::WrongTrieRoot(header.transactions_root(), tx_root)); } - let uncles_hash = keccak(body.rlp().at(1).as_raw()); + let uncles_hash = keccak(body.uncles_rlp().as_raw()); if uncles_hash != header.uncles_hash() { return Err(Error::WrongHash(header.uncles_hash(), uncles_hash)); } // concatenate the header and the body. - let mut stream = RlpStream::new_list(3); - stream.append_raw(header.rlp().as_raw(), 1); - stream.append_raw(body.rlp().at(0).as_raw(), 1); - stream.append_raw(body.rlp().at(1).as_raw(), 1); + let block = encoded::Block::new_from_header_and_body(&header.view(), &body.view()); cache.lock().insert_block_body(header.hash(), body.clone()); - Ok(encoded::Block::new(stream.out())) + Ok(block) } } diff --git a/ethcore/src/blockchain/blockchain.rs b/ethcore/src/blockchain/blockchain.rs index 09be1143f..86a1c522a 100644 --- a/ethcore/src/blockchain/blockchain.rs +++ b/ethcore/src/blockchain/blockchain.rs @@ -25,7 +25,7 @@ use heapsize::HeapSizeOf; use ethereum_types::{H256, Bloom, U256}; use parking_lot::{Mutex, RwLock}; use bytes::Bytes; -use rlp::*; +use rlp::{Rlp, RlpStream}; use rlp_compress::{compress, decompress, blocks_swapper}; use header::*; use transaction::*; diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index b806183a2..69d8c70d6 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -29,7 +29,7 @@ use journaldb; use kvdb::DBValue; use kvdb_memorydb; use bytes::Bytes; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream}; use ethkey::{Generator, Random}; use transaction::{self, Transaction, LocalizedTransaction, PendingTransaction, SignedTransaction, Action}; use blockchain::{TreeRoute, BlockReceipts}; @@ -66,6 +66,7 @@ use encoded; use engines::EthEngine; use trie; use state::StateInfo; +use views::BlockView; /// Test client. pub struct TestBlockChainClient { @@ -469,7 +470,7 @@ impl ChainInfo for TestBlockChainClient { impl BlockInfo for TestBlockChainClient { fn block_header(&self, id: BlockId) -> Option { self.block_hash(id) - .and_then(|hash| self.blocks.read().get(&hash).map(|r| Rlp::new(r).at(0).as_raw().to_vec())) + .and_then(|hash| self.blocks.read().get(&hash).map(|r| BlockView::new(r).header_rlp().as_raw().to_vec())) .map(encoded::Header::new) } @@ -510,7 +511,7 @@ impl RegistryInfo for TestBlockChainClient { impl ImportBlock for TestBlockChainClient { fn import_block(&self, b: Bytes) -> Result { - let header = Rlp::new(&b).val_at::(0); + let header = BlockView::new(&b).header(); let h = header.hash(); let number: usize = header.number() as usize; if number > self.blocks.read().len() { @@ -519,7 +520,7 @@ impl ImportBlock for TestBlockChainClient { if number > 0 { match self.blocks.read().get(header.parent_hash()) { Some(parent) => { - let parent = Rlp::new(parent).val_at::(0); + let parent = BlockView::new(parent).header(); if parent.number() != (header.number() - 1) { panic!("Unexpected block parent"); } @@ -544,7 +545,7 @@ impl ImportBlock for TestBlockChainClient { while n > 0 && self.numbers.read()[&n] != parent_hash { *self.numbers.write().get_mut(&n).unwrap() = parent_hash.clone(); n -= 1; - parent_hash = Rlp::new(&self.blocks.read()[&parent_hash]).val_at::(0).parent_hash().clone(); + parent_hash = BlockView::new(&self.blocks.read()[&parent_hash]).header().parent_hash().clone(); } } } @@ -683,9 +684,10 @@ impl BlockChainClient for TestBlockChainClient { fn block_body(&self, id: BlockId) -> Option { self.block_hash(id).and_then(|hash| self.blocks.read().get(&hash).map(|r| { + let block = BlockView::new(r); let mut stream = RlpStream::new_list(2); - stream.append_raw(Rlp::new(r).at(1).as_raw(), 1); - stream.append_raw(Rlp::new(r).at(2).as_raw(), 1); + stream.append_raw(block.transactions_rlp().as_raw(), 1); + stream.append_raw(block.uncles_rlp().as_raw(), 1); encoded::Body::new(stream.out()) })) } diff --git a/ethcore/src/encoded.rs b/ethcore/src/encoded.rs index 098a90a8a..5d3d31606 100644 --- a/ethcore/src/encoded.rs +++ b/ethcore/src/encoded.rs @@ -31,7 +31,7 @@ use views; use hash::keccak; use heapsize::HeapSizeOf; use ethereum_types::{H256, Bloom, U256, Address}; -use rlp::Rlp; +use rlp::{Rlp, RlpStream}; /// Owning header view. #[derive(Debug, Clone, PartialEq, Eq)] @@ -144,6 +144,9 @@ impl Body { // forwarders to borrowed view. impl Body { + /// Get raw rlp of transactions + pub fn transactions_rlp(&self) -> Rlp { self.view().transactions_rlp() } + /// Get a vector of all transactions. pub fn transactions(&self) -> Vec { self.view().transactions() } @@ -156,6 +159,9 @@ impl Body { /// The hash of each transaction in the block. pub fn transaction_hashes(&self) -> Vec { self.view().transaction_hashes() } + /// Get raw rlp of uncle headers + pub fn uncles_rlp(&self) -> Rlp { self.view().uncles_rlp() } + /// Decode uncle headers. pub fn uncles(&self) -> Vec { self.view().uncles() } @@ -181,6 +187,15 @@ impl Block { /// Create a new owning block view. The raw bytes passed in must be an rlp-encoded block. pub fn new(raw: Vec) -> Self { Block(raw) } + /// Create a new owning block view by concatenating the encoded header and body + pub fn new_from_header_and_body(header: &views::HeaderView, body: &views::BodyView) -> Self { + let mut stream = RlpStream::new_list(3); + stream.append_raw(header.rlp().as_raw(), 1); + stream.append_raw(body.transactions_rlp().as_raw(), 1); + stream.append_raw(body.uncles_rlp().as_raw(), 1); + Block::new(stream.out()) + } + /// Get a borrowed view of the whole block. #[inline] pub fn view(&self) -> views::BlockView { views::BlockView::new(&self.0) } diff --git a/ethcore/src/engines/tendermint/message.rs b/ethcore/src/engines/tendermint/message.rs index 1f71f41bc..eac08df9b 100644 --- a/ethcore/src/engines/tendermint/message.rs +++ b/ethcore/src/engines/tendermint/message.rs @@ -23,7 +23,7 @@ use bytes::Bytes; use super::{Height, View, BlockHash, Step}; use error::Error; use header::Header; -use rlp::{Rlp, UntrustedRlp, RlpStream, Encodable, Decodable, DecoderError}; +use rlp::{UntrustedRlp, RlpStream, Encodable, Decodable, DecoderError}; use ethkey::{recover, public_to_address}; use super::super::vote_collector::Message; @@ -100,7 +100,7 @@ impl ConsensusMessage { pub fn verify(&self) -> Result { let full_rlp = ::rlp::encode(self); - let block_info = Rlp::new(&full_rlp).at(1); + let block_info = UntrustedRlp::new(&full_rlp).at(1)?; let public_key = recover(&self.signature.into(), &keccak(block_info.as_raw()))?; Ok(public_to_address(&public_key)) } diff --git a/ethcore/src/header.rs b/ethcore/src/header.rs index f6f5090ef..7980da278 100644 --- a/ethcore/src/header.rs +++ b/ethcore/src/header.rs @@ -23,7 +23,7 @@ use hash::{KECCAK_NULL_RLP, KECCAK_EMPTY_LIST_RLP, keccak}; use heapsize::HeapSizeOf; use ethereum_types::{H256, U256, Address, Bloom}; use bytes::Bytes; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, Encodable, DecoderError, Decodable}; pub use types::BlockNumber; diff --git a/ethcore/src/state/account.rs b/ethcore/src/state/account.rs index f70b3eb6a..e08ad7b18 100755 --- a/ethcore/src/state/account.rs +++ b/ethcore/src/state/account.rs @@ -27,7 +27,7 @@ use bytes::{Bytes, ToPretty}; use trie; use trie::{SecTrieDB, Trie, TrieFactory, TrieError}; use pod_account::*; -use rlp::*; +use rlp::{RlpStream, encode}; use lru_cache::LruCache; use basic_account::BasicAccount; diff --git a/ethcore/src/trace/types/flat.rs b/ethcore/src/trace/types/flat.rs index e97f4d323..e3f66e170 100644 --- a/ethcore/src/trace/types/flat.rs +++ b/ethcore/src/trace/types/flat.rs @@ -17,7 +17,7 @@ //! Flat trace module use std::collections::VecDeque; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, Decodable, Encodable, DecoderError}; use heapsize::HeapSizeOf; use ethereum_types::Bloom; use super::trace::{Action, Res}; diff --git a/ethcore/src/trace/types/trace.rs b/ethcore/src/trace/types/trace.rs index 18fe329c4..e6db17603 100644 --- a/ethcore/src/trace/types/trace.rs +++ b/ethcore/src/trace/types/trace.rs @@ -18,7 +18,7 @@ use ethereum_types::{U256, Address, Bloom, BloomInput}; use bytes::Bytes; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, Encodable, DecoderError, Decodable}; use vm::ActionParams; use evm::CallType; diff --git a/ethcore/src/views/block.rs b/ethcore/src/views/block.rs index c60cc07c5..f6bb92873 100644 --- a/ethcore/src/views/block.rs +++ b/ethcore/src/views/block.rs @@ -91,6 +91,11 @@ impl<'a> BlockView<'a> { }).collect() } + /// Return the raw rlp for the transactions in the given block. + pub fn transactions_rlp(&self) -> Rlp<'a> { + self.rlp.at(1) + } + /// Return number of transactions in given block, without deserializing them. pub fn transactions_count(&self) -> usize { self.rlp.at(1).iter().count() @@ -125,6 +130,11 @@ impl<'a> BlockView<'a> { }) } + /// Returns raw rlp for the uncles in the given block + pub fn uncles_rlp(&self) -> Rlp<'a> { + self.rlp.at(2) + } + /// Return list of uncles of given block. pub fn uncles(&self) -> Vec
{ self.rlp.list_at(2) diff --git a/ethcore/src/views/body.rs b/ethcore/src/views/body.rs index 433f7c2b6..d23f1a1d7 100644 --- a/ethcore/src/views/body.rs +++ b/ethcore/src/views/body.rs @@ -68,11 +68,15 @@ impl<'a> BodyView<'a> { }).collect() } + /// Return the raw rlp for the transactions in the given block. + pub fn transactions_rlp(&self) -> Rlp<'a> { + self.rlp.at(0) + } + /// Return number of transactions in given block, without deserializing them. pub fn transactions_count(&self) -> usize { self.rlp.at(0).item_count() } - /// Return List of transactions in given block. pub fn transaction_views(&self) -> Vec> { self.rlp.at(0).iter().map(TransactionView::new_from_rlp).collect() @@ -99,6 +103,11 @@ impl<'a> BodyView<'a> { }) } + /// Returns raw rlp for the uncles in the given block + pub fn uncles_rlp(&self) -> Rlp<'a> { + self.rlp.at(1) + } + /// Return list of uncles of given block. pub fn uncles(&self) -> Vec
{ self.rlp.list_at(1) diff --git a/ethcore/transaction/src/transaction.rs b/ethcore/transaction/src/transaction.rs index f206c549a..d458e1f60 100644 --- a/ethcore/transaction/src/transaction.rs +++ b/ethcore/transaction/src/transaction.rs @@ -25,7 +25,6 @@ use evm::Schedule; use hash::keccak; use heapsize::HeapSizeOf; use rlp::{self, RlpStream, UntrustedRlp, DecoderError, Encodable}; -// use rlp::*; type Bytes = Vec; type BlockNumber = u64; diff --git a/ethcore/types/src/receipt.rs b/ethcore/types/src/receipt.rs index 29b2e909b..8aee99365 100644 --- a/ethcore/types/src/receipt.rs +++ b/ethcore/types/src/receipt.rs @@ -18,7 +18,7 @@ use ethereum_types::{H256, U256, Address, Bloom}; use heapsize::HeapSizeOf; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, Encodable, Decodable, DecoderError}; use {BlockNumber}; use log_entry::{LogEntry, LocalizedLogEntry}; diff --git a/ethcore/types/src/snapshot_manifest.rs b/ethcore/types/src/snapshot_manifest.rs index 72f83afeb..7f41f9994 100644 --- a/ethcore/types/src/snapshot_manifest.rs +++ b/ethcore/types/src/snapshot_manifest.rs @@ -17,7 +17,7 @@ //! Snapshot manifest type definition use ethereum_types::H256; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, DecoderError}; use bytes::Bytes; /// Manifest data. diff --git a/sync/src/block_sync.rs b/sync/src/block_sync.rs index 9e72c9d53..5cf81b2e8 100644 --- a/sync/src/block_sync.rs +++ b/sync/src/block_sync.rs @@ -22,7 +22,7 @@ use std::collections::{HashSet, VecDeque}; use std::cmp; use heapsize::HeapSizeOf; use ethereum_types::H256; -use rlp::*; +use rlp::UntrustedRlp; use ethcore::views::{BlockView}; use ethcore::header::{BlockNumber, Header as BlockHeader}; use ethcore::client::{BlockStatus, BlockId, BlockImportError}; diff --git a/sync/src/blocks.rs b/sync/src/blocks.rs index 65244a4e7..37aa579ef 100644 --- a/sync/src/blocks.rs +++ b/sync/src/blocks.rs @@ -22,8 +22,10 @@ use heapsize::HeapSizeOf; use ethereum_types::H256; use triehash::ordered_trie_root; use bytes::Bytes; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, DecoderError}; use network; +use ethcore::encoded::Block; +use ethcore::views::{HeaderView, BodyView}; use ethcore::header::Header as BlockHeader; known_heap_size!(0, HeaderId); @@ -290,15 +292,10 @@ impl BlockCollection { } for block in blocks { - let mut block_rlp = RlpStream::new_list(3); - block_rlp.append_raw(&block.header, 1); - { - let body = Rlp::new(block.body.as_ref().expect("blocks contains only full blocks; qed")); - block_rlp.append_raw(body.at(0).as_raw(), 1); - block_rlp.append_raw(body.at(1).as_raw(), 1); - } + let body = BodyView::new(block.body.as_ref().expect("blocks contains only full blocks; qed")); + let block_view = Block::new_from_header_and_body(&HeaderView::new(&block.header), &body); drained.push(BlockAndReceipts { - block: block_rlp.out(), + block: block_view.rlp().as_raw().to_vec(), receipts: block.receipts.clone(), }); } diff --git a/sync/src/chain.rs b/sync/src/chain.rs index a4d72ac96..6b5f370d3 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -97,7 +97,7 @@ use ethereum_types::{H256, U256}; use plain_hasher::H256FastMap; use parking_lot::RwLock; use bytes::Bytes; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, DecoderError, Encodable}; use network::{self, PeerId, PacketId}; use ethcore::header::{BlockNumber, Header as BlockHeader}; use ethcore::client::{BlockChainClient, BlockStatus, BlockId, BlockChainInfo, BlockImportError, BlockQueueInfo}; diff --git a/util/network-devp2p/src/connection.rs b/util/network-devp2p/src/connection.rs index ea3b99d41..4d775b215 100644 --- a/util/network-devp2p/src/connection.rs +++ b/util/network-devp2p/src/connection.rs @@ -23,7 +23,7 @@ use mio::deprecated::{Handler, EventLoop, TryRead, TryWrite}; use mio::tcp::*; use ethereum_types::{H128, H256, H512}; use ethcore_bytes::*; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream}; use std::io::{self, Cursor, Read, Write}; use io::{IoContext, StreamToken}; use handshake::Handshake; diff --git a/util/network-devp2p/src/discovery.rs b/util/network-devp2p/src/discovery.rs index 4a43ab3eb..fa32f89f9 100644 --- a/util/network-devp2p/src/discovery.rs +++ b/util/network-devp2p/src/discovery.rs @@ -25,7 +25,7 @@ use mio::deprecated::{Handler, EventLoop}; use mio::udp::*; use hash::keccak; use ethereum_types::{H256, H520}; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, encode_list}; use node_table::*; use network::{Error, ErrorKind}; use io::{StreamToken, IoContext}; @@ -216,7 +216,8 @@ impl Discovery { let nearest = nearest.filter(|x| !self.discovery_nodes.contains(&x.id)).take(ALPHA).collect::>(); for r in nearest { let rlp = encode_list(&(&[self.discovery_id.clone()][..])); - self.send_packet(PACKET_FIND_NODE, &r.endpoint.udp_address(), &rlp); + self.send_packet(PACKET_FIND_NODE, &r.endpoint.udp_address(), &rlp) + .unwrap_or_else(|e| warn!("Error sending node discovery packet for {:?}: {:?}", &r.endpoint, e)); self.discovery_nodes.insert(r.id.clone()); tried_count += 1; trace!(target: "discovery", "Sent FindNode to {:?}", &r.endpoint); @@ -251,16 +252,17 @@ impl Discovery { self.public_endpoint.to_rlp_list(&mut rlp); node.to_rlp_list(&mut rlp); trace!(target: "discovery", "Sent Ping to {:?}", &node); - self.send_packet(PACKET_PING, &node.udp_address(), &rlp.drain()); + self.send_packet(PACKET_PING, &node.udp_address(), &rlp.drain()) + .unwrap_or_else(|e| warn!("Error sending Ping packet: {:?}", e)) } - fn send_packet(&mut self, packet_id: u8, address: &SocketAddr, payload: &[u8]) { + fn send_packet(&mut self, packet_id: u8, address: &SocketAddr, payload: &[u8]) -> Result<(), Error> { let mut rlp = RlpStream::new(); rlp.append_raw(&[packet_id], 1); - let source = Rlp::new(payload); - rlp.begin_list(source.item_count() + 1); - for i in 0 .. source.item_count() { - rlp.append_raw(source.at(i).as_raw(), 1); + let source = UntrustedRlp::new(payload); + rlp.begin_list(source.item_count()? + 1); + for i in 0 .. source.item_count()? { + rlp.append_raw(source.at(i)?.as_raw(), 1); } let timestamp = 60 + SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or_default().as_secs() as u32; rlp.append(×tamp); @@ -269,9 +271,9 @@ impl Discovery { let hash = keccak(bytes.as_ref()); let signature = match sign(&self.secret, &hash) { Ok(s) => s, - Err(_) => { + Err(e) => { warn!("Error signing UDP packet"); - return; + return Err(Error::from(e)); } }; let mut packet = Bytes::with_capacity(bytes.len() + 32 + 65); @@ -281,6 +283,7 @@ impl Discovery { let signed_hash = keccak(&packet[32..]); packet[0..32].clone_from_slice(&signed_hash); self.send_to(packet, address.clone()); + Ok(()) } fn nearest_node_entries(target: &NodeId, buckets: &[NodeBucket]) -> Vec { @@ -425,7 +428,7 @@ impl Discovery { let mut response = RlpStream::new_list(2); dest.to_rlp_list(&mut response); response.append(&echo_hash); - self.send_packet(PACKET_PONG, from, &response.drain()); + self.send_packet(PACKET_PONG, from, &response.drain())?; Ok(Some(TableUpdates { added: added_map, removed: HashSet::new() })) } @@ -456,7 +459,7 @@ impl Discovery { } let mut packets = Discovery::prepare_neighbours_packets(&nearest); for p in packets.drain(..) { - self.send_packet(PACKET_NEIGHBOURS, from, &p); + self.send_packet(PACKET_NEIGHBOURS, from, &p)?; } trace!(target: "discovery", "Sent {} Neighbours to {:?}", nearest.len(), &from); Ok(None) diff --git a/util/network-devp2p/src/handshake.rs b/util/network-devp2p/src/handshake.rs index dbd31c5a8..020b65477 100644 --- a/util/network-devp2p/src/handshake.rs +++ b/util/network-devp2p/src/handshake.rs @@ -19,7 +19,7 @@ use hash::write_keccak; use mio::tcp::*; use ethereum_types::{H256, H520}; use ethcore_bytes::Bytes; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream}; use connection::{Connection}; use node_table::NodeId; use io::{IoContext, StreamToken}; diff --git a/util/network-devp2p/src/host.rs b/util/network-devp2p/src/host.rs index 87e9a0278..9df2d0b38 100644 --- a/util/network-devp2p/src/host.rs +++ b/util/network-devp2p/src/host.rs @@ -30,7 +30,8 @@ use mio::*; use mio::deprecated::{EventLoop}; use mio::tcp::*; use ethereum_types::H256; -use rlp::*; +use rlp::{RlpStream, Encodable}; + use session::{Session, SessionData}; use io::*; use PROTOCOL_VERSION; diff --git a/util/network-devp2p/src/node_table.rs b/util/network-devp2p/src/node_table.rs index 244f518f2..6be9ae12e 100644 --- a/util/network-devp2p/src/node_table.rs +++ b/util/network-devp2p/src/node_table.rs @@ -22,7 +22,7 @@ use std::path::PathBuf; use std::str::FromStr; use std::{fs, mem, slice}; use ethereum_types::H512; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, DecoderError}; use network::{Error, ErrorKind, AllowIP, IpFilter}; use discovery::{TableUpdates, NodeEntry}; use ip_utils::*; diff --git a/util/network-devp2p/src/session.rs b/util/network-devp2p/src/session.rs index e82df62ba..179d706d7 100644 --- a/util/network-devp2p/src/session.rs +++ b/util/network-devp2p/src/session.rs @@ -23,7 +23,7 @@ use mio::*; use mio::deprecated::{Handler, EventLoop}; use mio::tcp::*; use ethereum_types::H256; -use rlp::*; +use rlp::{UntrustedRlp, RlpStream, EMPTY_LIST_RLP}; use connection::{EncryptedConnection, Packet, Connection, MAX_PAYLOAD_SIZE}; use handshake::Handshake; use io::{IoContext, StreamToken};