Merge pull request #4362 from ethcore/mh-tendermintrlpfix

Fix RLP encoding for types recursively calling `RlpStream::append`

ethcore/src/blockchain/extras.rs

@@ -233,7 +233,7 @@ impl Decodable for BlockReceipts {
 
 impl Encodable for BlockReceipts {
 	fn rlp_append(&self, s: &mut RlpStream) {
-		s.append(&self.receipts);
+		Encodable::rlp_append(&self.receipts, s);
 	}
 }
 
@@ -242,3 +242,21 @@ impl HeapSizeOf for BlockReceipts {
 		self.receipts.heap_size_of_children()
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use rlp::*;
+	use super::BlockReceipts;
+
+	#[test]
+	fn encode_block_receipts() {
+		let br = BlockReceipts::new(Vec::new());
+
+		let mut s = RlpStream::new_list(2);
+		s.append(&br);
+		assert!(!s.is_finished(), "List shouldn't finished yet");
+		s.append(&br);
+		assert!(s.is_finished(), "List should be finished now");
+		s.out();
+	}
+}

ethcore/src/engines/tendermint/message.rs

@@ -20,7 +20,7 @@ use util::*;
 use super::{Height, View, BlockHash, Step};
 use error::Error;
 use header::Header;
-use rlp::{Rlp, UntrustedRlp, RlpStream, Stream, Encodable, Decodable, Decoder, DecoderError, View as RlpView};
+use rlp::{Rlp, UntrustedRlp, RlpStream, Stream, RlpEncodable, Encodable, Decodable, Decoder, DecoderError, View as RlpView};
 use ethkey::{recover, public_to_address};
 use super::super::vote_collector::Message;
 
@@ -162,7 +162,7 @@ impl Decodable for Step {
 
 impl Encodable for Step {
 	fn rlp_append(&self, s: &mut RlpStream) {
-		s.append(&self.number());
+		RlpEncodable::rlp_append(&self.number(), s);
 	}
 }
 
@@ -193,8 +193,7 @@ impl Encodable for ConsensusMessage {
 }
 
 pub fn message_info_rlp(vote_step: &VoteStep, block_hash: Option<BlockHash>) -> Bytes {
-	// TODO: figure out whats wrong with nested list encoding
+	let mut s = RlpStream::new_list(4);
-	let mut s = RlpStream::new_list(5);
 	s.append(&vote_step.height).append(&vote_step.view).append(&vote_step.step).append(&block_hash.unwrap_or_else(H256::zero));
 	s.out()
 }
@@ -215,10 +214,22 @@ mod tests {
 	use super::super::Step;
 	use super::*;
 
+	#[test]
+	fn encode_step() {
+		let step = Step::Precommit;
+
+		let mut s = RlpStream::new_list(2);
+		s.append(&step);
+		assert!(!s.is_finished(), "List shouldn't finished yet");
+		s.append(&step);
+		assert!(s.is_finished(), "List should be finished now");
+		s.out();
+	}
+
 	#[test]
 	fn encode_decode() {
 		let message = ConsensusMessage {
-			signature: H520::default(),	
+			signature: H520::default(),
 			vote_step: VoteStep {
 				height: 10,
 				view: 123,
@@ -231,7 +242,7 @@ mod tests {
 		assert_eq!(message, rlp.as_val());
 
 		let message = ConsensusMessage {
-			signature: H520::default(),	
+			signature: H520::default(),
 			vote_step: VoteStep {
 				height: 1314,
 				view: 0,

ethcore/src/types/executed.rs

@@ -46,7 +46,7 @@ impl Encodable for CallType {
 			CallType::CallCode => 2,
 			CallType::DelegateCall => 3,
 		};
-		s.append(&v);
+		Encodable::rlp_append(&v, s);
 	}
 }
 
@@ -212,12 +212,28 @@ impl fmt::Display for CallError {
 /// Transaction execution result.
 pub type ExecutionResult = Result<Executed, ExecutionError>;
 
-#[test]
+#[cfg(test)]
-fn should_encode_and_decode_call_type() {
+mod tests {
-	use rlp;
+	use rlp::*;
+	use super::CallType;
 
-	let original = CallType::Call;
+	#[test]
-	let encoded = rlp::encode(&original);
+	fn encode_call_type() {
-	let decoded = rlp::decode(&encoded);
+		let ct = CallType::Call;
-	assert_eq!(original, decoded);
+
+		let mut s = RlpStream::new_list(2);
+		s.append(&ct);
+		assert!(!s.is_finished(), "List shouldn't finished yet");
+		s.append(&ct);
+		assert!(s.is_finished(), "List should be finished now");
+		s.out();
+	}
+
+	#[test]
+	fn should_encode_and_decode_call_type() {
+		let original = CallType::Call;
+		let encoded = encode(&original);
+		let decoded = decode(&encoded);
+		assert_eq!(original, decoded);
+	}
 }

ethcore/src/types/trace_types/error.rs

@@ -17,7 +17,7 @@
 //! Trace errors.
 
 use std::fmt;
-use rlp::{Encodable, RlpStream, Decodable, Decoder, DecoderError, Stream, View};
+use rlp::{RlpEncodable, Encodable, RlpStream, Decodable, Decoder, DecoderError, View};
 use evm::Error as EvmError;
 
 /// Trace evm errors.
@@ -79,7 +79,7 @@ impl Encodable for Error {
 			OutOfStack => 4,
 			Internal => 5,
 		};
-		s.append(&value);
+		RlpEncodable::rlp_append(&value, s);
 	}
 }
 
@@ -98,3 +98,21 @@ impl Decodable for Error {
 		}
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use rlp::*;
+	use super::Error;
+
+	#[test]
+	fn encode_error() {
+		let err = Error::BadJumpDestination;
+
+		let mut s = RlpStream::new_list(2);
+		s.append(&err);
+		assert!(!s.is_finished(), "List shouldn't finished yet");
+		s.append(&err);
+		assert!(s.is_finished(), "List should be finished now");
+		s.out();
+	}
+}

ethcore/src/types/trace_types/flat.rs

@@ -103,7 +103,7 @@ impl FlatTransactionTraces {
 
 impl Encodable for FlatTransactionTraces {
 	fn rlp_append(&self, s: &mut RlpStream) {
-		s.append(&self.0);
+		Encodable::rlp_append(&self.0, s);
 	}
 }
 
@@ -144,7 +144,7 @@ impl FlatBlockTraces {
 
 impl Encodable for FlatBlockTraces {
 	fn rlp_append(&self, s: &mut RlpStream) {
-		s.append(&self.0);
+		Encodable::rlp_append(&self.0, s);
 	}
 }
 
@@ -162,10 +162,35 @@ impl Into<Vec<FlatTransactionTraces>> for FlatBlockTraces {
 
 #[cfg(test)]
 mod tests {
+	use rlp::*;
 	use super::{FlatBlockTraces, FlatTransactionTraces, FlatTrace};
 	use trace::trace::{Action, Res, CallResult, Call, Suicide};
 	use types::executed::CallType;
 
+	#[test]
+	fn encode_flat_transaction_traces() {
+		let ftt = FlatTransactionTraces::from(Vec::new());
+
+		let mut s = RlpStream::new_list(2);
+		s.append(&ftt);
+		assert!(!s.is_finished(), "List shouldn't finished yet");
+		s.append(&ftt);
+		assert!(s.is_finished(), "List should be finished now");
+		s.out();
+	}
+
+	#[test]
+	fn encode_flat_block_traces() {
+		let fbt = FlatBlockTraces::from(Vec::new());
+
+		let mut s = RlpStream::new_list(2);
+		s.append(&fbt);
+		assert!(!s.is_finished(), "List shouldn't finished yet");
+		s.append(&fbt);
+		assert!(s.is_finished(), "List should be finished now");
+		s.out();
+	}
+
 	#[test]
 	fn test_trace_serialization() {
 		// block #51921