Rlp decode returns Result (openethereum#8527)

rlp::decode returns Result

Make a best effort to handle decoding errors gracefully throughout the code, using `expect` where the value is guaranteed to be valid (and in other places where it makes sense).

ethcore/light/src/client/header_chain.rs

@@ -228,15 +228,15 @@ impl HeaderChain {
 		let decoded_header = spec.genesis_header();
 
 		let chain = if let Some(current) = db.get(col, CURRENT_KEY)? {
-			let curr : BestAndLatest = ::rlp::decode(&current);
+			let curr : BestAndLatest = ::rlp::decode(&current).expect("decoding db value failed");
 
 			let mut cur_number = curr.latest_num;
 			let mut candidates = BTreeMap::new();
 
 			// load all era entries, referenced headers within them,
 			// and live epoch proofs.
 			while let Some(entry) = db.get(col, era_key(cur_number).as_bytes())? {
-				let entry: Entry = ::rlp::decode(&entry);
+				let entry: Entry = ::rlp::decode(&entry).expect("decoding db value failed");
 				trace!(target: "chain", "loaded header chain entry for era {} with {} candidates",
 					cur_number, entry.candidates.len());
 
@@ -524,7 +524,10 @@ impl HeaderChain {
 											None
 										}
 										Ok(None) => panic!("stored candidates always have corresponding headers; qed"),
-										Ok(Some(header)) => Some((epoch_transition, ::rlp::decode(&header))),
+										Ok(Some(header)) => Some((
+											epoch_transition,
+											::rlp::decode(&header).expect("decoding value from db failed")
+										)),
 									};
 								}
 							}
@@ -591,7 +594,7 @@ impl HeaderChain {
 													in an inconsistent state", h_num);
 								ErrorKind::Database(msg.into())
 							})?;
-						::rlp::decode(&bytes)
+						::rlp::decode(&bytes).expect("decoding db value failed")
 					};
 
 					let total_difficulty = entry.candidates.iter()
@@ -604,9 +607,9 @@ impl HeaderChain {
 						.total_difficulty;
 
 					break Ok(Some(SpecHardcodedSync {
-						header: header,
-						total_difficulty: total_difficulty,
-						chts: chts,
+						header,
+						total_difficulty,
+						chts,
 					}));
 				},
 				None => {
@@ -742,7 +745,7 @@ impl HeaderChain {
 	/// so including it within a CHT would be redundant.
 	pub fn cht_root(&self, n: usize) -> Option<H256> {
 		match self.db.get(self.col, cht_key(n as u64).as_bytes()) {
-			Ok(val) => val.map(|x| ::rlp::decode(&x)),
+			Ok(db_fetch) => db_fetch.map(|bytes| ::rlp::decode(&bytes).expect("decoding value from db failed")),
 			Err(e) => {
 				warn!(target: "chain", "Error reading from database: {}", e);
 				None
@@ -793,7 +796,7 @@ impl HeaderChain {
 	pub fn pending_transition(&self, hash: H256) -> Option<PendingEpochTransition> {
 		let key = pending_transition_key(hash);
 		match self.db.get(self.col, &*key) {
-			Ok(val) => val.map(|x| ::rlp::decode(&x)),
+			Ok(db_fetch) => db_fetch.map(|bytes| ::rlp::decode(&bytes).expect("decoding value from db failed")),
 			Err(e) => {
 				warn!(target: "chain", "Error reading from database: {}", e);
 				None
@@ -1192,7 +1195,7 @@ mod tests {
 
 		let cache = Arc::new(Mutex::new(Cache::new(Default::default(), Duration::from_secs(6 * 3600))));
 
-		let chain = HeaderChain::new(db.clone(), None, &spec, cache, HardcodedSync::Allow).unwrap();
+		let chain = HeaderChain::new(db.clone(), None, &spec, cache, HardcodedSync::Allow).expect("failed to instantiate a new HeaderChain");
 
 		let mut parent_hash = genesis_header.hash();
 		let mut rolling_timestamp = genesis_header.timestamp();
@@ -1211,14 +1214,14 @@ mod tests {
 			parent_hash = header.hash();
 
 			let mut tx = db.transaction();
-			let pending = chain.insert(&mut tx, header, None).unwrap();
+			let pending = chain.insert(&mut tx, header, None).expect("failed inserting a transaction");
 			db.write(tx).unwrap();
 			chain.apply_pending(pending);
 
 			rolling_timestamp += 10;
 		}
 
-		let hardcoded_sync = chain.read_hardcoded_sync().unwrap().unwrap();
+		let hardcoded_sync = chain.read_hardcoded_sync().expect("failed reading hardcoded sync").expect("failed unwrapping hardcoded sync");
 		assert_eq!(hardcoded_sync.chts.len(), 3);
 		assert_eq!(hardcoded_sync.total_difficulty, total_difficulty);
 		let decoded: Header = hardcoded_sync.header.decode();

ethcore/light/src/net/request_credits.rs

@@ -407,7 +407,7 @@ mod tests {
 		let costs = CostTable::default();
 		let serialized = ::rlp::encode(&costs);
 
-		let new_costs: CostTable = ::rlp::decode(&*serialized);
+		let new_costs: CostTable = ::rlp::decode(&*serialized).unwrap();
 
 		assert_eq!(costs, new_costs);
 	}

ethcore/light/src/types/request/mod.rs

@@ -1642,7 +1642,7 @@ mod tests {
 	{
 		// check as single value.
 		let bytes = ::rlp::encode(&val);
-		let new_val: T = ::rlp::decode(&bytes);
+		let new_val: T = ::rlp::decode(&bytes).unwrap();
 		assert_eq!(val, new_val);
 
 		// check as list containing single value.

ethcore/src/blockchain/blockchain.rs

@@ -438,7 +438,7 @@ impl<'a> Iterator for EpochTransitionIter<'a> {
 				return None
 			}
 
-			let transitions: EpochTransitions = ::rlp::decode(&val[..]);
+			let transitions: EpochTransitions = ::rlp::decode(&val[..]).expect("decode error: the db is corrupted or the data structure has changed");
 
 			// if there are multiple candidates, at most one will be on the
 			// canon chain.
@@ -462,7 +462,7 @@ impl<'a> Iterator for EpochTransitionIter<'a> {
 impl BlockChain {
 	/// Create new instance of blockchain from given Genesis.
 	pub fn new(config: Config, genesis: &[u8], db: Arc<KeyValueDB>) -> BlockChain {
-		// 400 is the avarage size of the key
+		// 400 is the average size of the key
 		let cache_man = CacheManager::new(config.pref_cache_size, config.max_cache_size, 400);
 
 		let mut bc = BlockChain {

ethcore/src/db.rs

@@ -218,15 +218,12 @@ impl Writable for DBTransaction {
 }
 
 impl<KVDB: KeyValueDB + ?Sized> Readable for KVDB {
-	fn read<T, R>(&self, col: Option<u32>, key: &Key<T, Target = R>) -> Option<T> where T: rlp::Decodable, R: Deref<Target = [u8]> {
-		let result = self.get(col, &key.key());
+	fn read<T, R>(&self, col: Option<u32>, key: &Key<T, Target = R>) -> Option<T>
+		where T: rlp::Decodable, R: Deref<Target = [u8]> {
+		self.get(col, &key.key())
+			.expect(&format!("db get failed, key: {:?}", &key.key() as &[u8]))
+			.map(|v| rlp::decode(&v).expect("decode db value failed") )
 
-		match result {
-			Ok(option) => option.map(|v| rlp::decode(&v)),
-			Err(err) => {
-				panic!("db get failed, key: {:?}, err: {:?}", &key.key() as &[u8], err);
-			}
-		}
 	}
 
 	fn exists<T, R>(&self, col: Option<u32>, key: &Key<T, Target = R>) -> bool where R: Deref<Target = [u8]> {

ethcore/src/encoded.rs

@@ -24,13 +24,12 @@
 //! decoded object where parts like the hash can be saved.
 
 use block::Block as FullBlock;
-use header::{BlockNumber, Header as FullHeader};
-use transaction::UnverifiedTransaction;
-
+use ethereum_types::{H256, Bloom, U256, Address};
 use hash::keccak;
+use header::{BlockNumber, Header as FullHeader};
 use heapsize::HeapSizeOf;
-use ethereum_types::{H256, Bloom, U256, Address};
 use rlp::{Rlp, RlpStream};
+use transaction::UnverifiedTransaction;
 use views::{self, BlockView, HeaderView, BodyView};
 
 /// Owning header view.
@@ -48,7 +47,7 @@ impl Header {
 	pub fn new(encoded: Vec<u8>) -> Self { Header(encoded) }
 
 	/// Upgrade this encoded view to a fully owned `Header` object.
-	pub fn decode(&self) -> FullHeader { ::rlp::decode(&self.0) }
+	pub fn decode(&self) -> FullHeader { ::rlp::decode(&self.0).expect("decoding failure") }
 
 	/// Get a borrowed header view onto the data.
 	#[inline]
@@ -205,7 +204,7 @@ impl Block {
 	pub fn header_view(&self) -> HeaderView { self.view().header_view() }
 
 	/// Decode to a full block.
-	pub fn decode(&self) -> FullBlock { ::rlp::decode(&self.0) }
+	pub fn decode(&self) -> FullBlock { ::rlp::decode(&self.0).expect("decoding failure") }
 
 	/// Decode the header.
 	pub fn decode_header(&self) -> FullHeader { self.view().rlp().val_at(0) }

ethcore/src/engines/tendermint/mod.rs

@@ -143,8 +143,10 @@ impl <F> super::EpochVerifier<EthereumMachine> for EpochVerifier<F>
 	}
 
 	fn check_finality_proof(&self, proof: &[u8]) -> Option<Vec<H256>> {
-		let header: Header = ::rlp::decode(proof);
-		self.verify_light(&header).ok().map(|_| vec![header.hash()])
+		match ::rlp::decode(proof) {
+			Ok(header) => self.verify_light(&header).ok().map(|_| vec![header.hash()]),
+			Err(_) => None // REVIEW: log perhaps? Not sure what the policy is.
+		}
 	}
 }
 

ethcore/src/header.rs

@@ -398,7 +398,7 @@ mod tests {
 		let nonce = "88ab4e252a7e8c2a23".from_hex().unwrap();
 		let nonce_decoded = "ab4e252a7e8c2a23".from_hex().unwrap();
 
-		let header: Header = rlp::decode(&header_rlp);
+		let header: Header = rlp::decode(&header_rlp).expect("error decoding header");
 		let seal_fields = header.seal.clone();
 		assert_eq!(seal_fields.len(), 2);
 		assert_eq!(seal_fields[0], mix_hash);
@@ -415,7 +415,7 @@ mod tests {
 		// that's rlp of block header created with ethash engine.
 		let header_rlp = "f901f9a0d405da4e66f1445d455195229624e133f5baafe72b5cf7b3c36c12c8146e98b7a01dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347948888f1f195afa192cfee860698584c030f4c9db1a05fb2b4bfdef7b314451cb138a534d225c922fc0e5fbe25e451142732c3e25c25a088d2ec6b9860aae1a2c3b299f72b6a5d70d7f7ba4722c78f2c49ba96273c2158a007c6fdfa8eea7e86b81f5b0fc0f78f90cc19f4aa60d323151e0cac660199e9a1b90100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008302008003832fefba82524d84568e932a80a0a0349d8c3df71f1a48a9df7d03fd5f14aeee7d91332c009ecaff0a71ead405bd88ab4e252a7e8c2a23".from_hex().unwrap();
 
-		let header: Header = rlp::decode(&header_rlp);
+		let header: Header = rlp::decode(&header_rlp).expect("error decoding header");
 		let encoded_header = rlp::encode(&header).into_vec();
 
 		assert_eq!(header_rlp, encoded_header);

ethcore/src/snapshot/account.rs

@@ -236,7 +236,7 @@ mod tests {
 		};
 
 		let thin_rlp = ::rlp::encode(&account);
-		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp), account);
+		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
 
 		let fat_rlps = to_fat_rlps(&keccak(&addr), &account, &AccountDB::new(db.as_hashdb(), &addr), &mut Default::default(), usize::max_value(), usize::max_value()).unwrap();
 		let fat_rlp = Rlp::new(&fat_rlps[0]).at(1).unwrap();
@@ -261,7 +261,7 @@ mod tests {
 		};
 
 		let thin_rlp = ::rlp::encode(&account);
-		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp), account);
+		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
 
 		let fat_rlp = to_fat_rlps(&keccak(&addr), &account, &AccountDB::new(db.as_hashdb(), &addr), &mut Default::default(), usize::max_value(), usize::max_value()).unwrap();
 		let fat_rlp = Rlp::new(&fat_rlp[0]).at(1).unwrap();
@@ -286,7 +286,7 @@ mod tests {
 		};
 
 		let thin_rlp = ::rlp::encode(&account);
-		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp), account);
+		assert_eq!(::rlp::decode::<BasicAccount>(&thin_rlp).unwrap(), account);
 
 		let fat_rlps = to_fat_rlps(&keccak(addr), &account, &AccountDB::new(db.as_hashdb(), &addr), &mut Default::default(), 500, 1000).unwrap();
 		let mut root = KECCAK_NULL_RLP;

ethcore/src/snapshot/mod.rs

@@ -281,7 +281,7 @@ pub fn chunk_state<'a>(db: &HashDB, root: &H256, writer: &Mutex<SnapshotWriter +
 	// account_key here is the address' hash.
 	for item in account_trie.iter()? {
 		let (account_key, account_data) = item?;
-		let account = ::rlp::decode(&*account_data);
+		let account = ::rlp::decode(&*account_data)?;
 		let account_key_hash = H256::from_slice(&account_key);
 
 		let account_db = AccountDB::from_hash(db, account_key_hash);
@@ -467,10 +467,10 @@ fn rebuild_accounts(
 		*out = (hash, thin_rlp);
 	}
 	if let Some(&(ref hash, ref rlp)) = out_chunk.iter().last() {
-		known_storage_roots.insert(*hash, ::rlp::decode::<BasicAccount>(rlp).storage_root);
+		known_storage_roots.insert(*hash, ::rlp::decode::<BasicAccount>(rlp)?.storage_root);
 	}
 	if let Some(&(ref hash, ref rlp)) = out_chunk.iter().next() {
-		known_storage_roots.insert(*hash, ::rlp::decode::<BasicAccount>(rlp).storage_root);
+		known_storage_roots.insert(*hash, ::rlp::decode::<BasicAccount>(rlp)?.storage_root);
 	}
 	Ok(status)
 }

ethcore/src/snapshot/tests/helpers.rs

@@ -75,7 +75,7 @@ impl StateProducer {
 
 		// sweep once to alter storage tries.
 		for &mut (ref mut address_hash, ref mut account_data) in &mut accounts_to_modify {
-			let mut account: BasicAccount = ::rlp::decode(&*account_data);
+			let mut account: BasicAccount = ::rlp::decode(&*account_data).expect("error decoding basic account");
 			let acct_db = AccountDBMut::from_hash(db, *address_hash);
 			fill_storage(acct_db, &mut account.storage_root, &mut self.storage_seed);
 			*account_data = DBValue::from_vec(::rlp::encode(&account).into_vec());

ethcore/src/snapshot/tests/state.rs

@@ -114,7 +114,7 @@ fn get_code_from_prev_chunk() {
 	// first one will have code inlined,
 	// second will just have its hash.
 	let thin_rlp = acc_stream.out();
-	let acc: BasicAccount = ::rlp::decode(&thin_rlp);
+	let acc: BasicAccount = ::rlp::decode(&thin_rlp).expect("error decoding basic account");
 
 	let mut make_chunk = |acc, hash| {
 		let mut db = MemoryDB::new();

ethcore/src/state/account.rs

@@ -21,6 +21,7 @@ use std::sync::Arc;
 use std::collections::{HashMap, BTreeMap};
 use hash::{KECCAK_EMPTY, KECCAK_NULL_RLP, keccak};
 use ethereum_types::{H256, U256, Address};
+use error::Error;
 use hashdb::HashDB;
 use kvdb::DBValue;
 use bytes::{Bytes, ToPretty};
@@ -144,9 +145,10 @@ impl Account {
 	}
 
 	/// Create a new account from RLP.
-	pub fn from_rlp(rlp: &[u8]) -> Account {
-		let basic: BasicAccount = ::rlp::decode(rlp);
-		basic.into()
+	pub fn from_rlp(rlp: &[u8]) -> Result<Account, Error> {
+		::rlp::decode::<BasicAccount>(rlp)
+			.map(|ba| ba.into())
+			.map_err(|e| e.into())
 	}
 
 	/// Create a new contract account.
@@ -202,8 +204,8 @@ impl Account {
 			return Ok(value);
 		}
 		let db = SecTrieDB::new(db, &self.storage_root)?;
-
-		let item: U256 = db.get_with(key, ::rlp::decode)?.unwrap_or_else(U256::zero);
+		let panicky_decoder = |bytes:&[u8]| ::rlp::decode(&bytes).expect("decoding db value failed");
+		let item: U256 = db.get_with(key, panicky_decoder)?.unwrap_or_else(U256::zero);
 		let value: H256 = item.into();
 		self.storage_cache.borrow_mut().insert(key.clone(), value.clone());
 		Ok(value)
@@ -478,7 +480,8 @@ impl Account {
 
 		let trie = TrieDB::new(db, &self.storage_root)?;
 		let item: U256 = {
-			let query = (&mut recorder, ::rlp::decode);
+			let panicky_decoder = |bytes:&[u8]| ::rlp::decode(bytes).expect("decoding db value failed");
+			let query = (&mut recorder, panicky_decoder);
 			trie.get_with(&storage_key, query)?.unwrap_or_else(U256::zero)
 		};
 
@@ -528,7 +531,7 @@ mod tests {
 			a.rlp()
 		};
 
-		let a = Account::from_rlp(&rlp);
+		let a = Account::from_rlp(&rlp).expect("decoding db value failed");
 		assert_eq!(*a.storage_root().unwrap(), "c57e1afb758b07f8d2c8f13a3b6e44fa5ff94ab266facc5a4fd3f062426e50b2".into());
 		assert_eq!(a.storage_at(&db.immutable(), &0x00u64.into()).unwrap(), 0x1234u64.into());
 		assert_eq!(a.storage_at(&db.immutable(), &0x01u64.into()).unwrap(), H256::default());
@@ -546,10 +549,10 @@ mod tests {
 			a.rlp()
 		};
 
-		let mut a = Account::from_rlp(&rlp);
+		let mut a = Account::from_rlp(&rlp).expect("decoding db value failed");
 		assert!(a.cache_code(&db.immutable()).is_some());
 
-		let mut a = Account::from_rlp(&rlp);
+		let mut a = Account::from_rlp(&rlp).expect("decoding db value failed");
 		assert_eq!(a.note_code(vec![0x55, 0x44, 0xffu8]), Ok(()));
 	}
 
@@ -609,7 +612,7 @@ mod tests {
 	#[test]
 	fn rlpio() {
 		let a = Account::new(69u8.into(), 0u8.into(), HashMap::new(), Bytes::new());
-		let b = Account::from_rlp(&a.rlp());
+		let b = Account::from_rlp(&a.rlp()).unwrap();
 		assert_eq!(a.balance(), b.balance());
 		assert_eq!(a.nonce(), b.nonce());
 		assert_eq!(a.code_hash(), b.code_hash());