Backporting (#3198)

* sendRawTransaction invalid RLP error * Returning proper error for estimate_gas * Exposing engine extra info in block RPC (#3169) * Exposing extra info in RPC * Proper serialization and client trait API * Fixing possible race condition in ethcore_hashContent (#3191) * Remove dapp logos (GHH points to dapp-assets) (#3192)
2016-11-04 18:59:49 +01:00
parent 522401296d
commit 33cc10549a
24 changed files with 195 additions and 80 deletions
--- a/rpc/src/v1/helpers/errors.rs
+++ b/rpc/src/v1/helpers/errors.rs
@@ -21,6 +21,7 @@ macro_rules! rpc_unimplemented {
 }

 use std::fmt;
+use rlp::DecoderError;
 use ethcore::error::{Error as EthcoreError, CallError};
 use ethcore::account_provider::{Error as AccountError};
 use fetch::FetchError;
@@ -271,6 +272,14 @@ pub fn from_transaction_error(error: EthcoreError) -> Error {
 	}
 }

+pub fn from_rlp_error(error: DecoderError) -> Error {
+	Error {
+		code: ErrorCode::InvalidParams,
+		message: "Invalid RLP.".into(),
+		data: Some(Value::String(format!("{:?}", error))),
+	}
+}
+
 pub fn from_call_error(error: CallError) -> Error {
 	match error {
 		CallError::StatePruned => state_pruned(),
--- a/rpc/src/v1/impls/eth.rs
+++ b/rpc/src/v1/impls/eth.rs
@@ -44,7 +44,7 @@ use ethcore::snapshot::SnapshotService;
 use self::ethash::SeedHashCompute;
 use v1::traits::Eth;
 use v1::types::{
-	Block, BlockTransactions, BlockNumber, Bytes, SyncStatus, SyncInfo,
+	RichBlock, Block, BlockTransactions, BlockNumber, Bytes, SyncStatus, SyncInfo,
 	Transaction, CallRequest, Index, Filter, Log, Receipt, Work,
 	H64 as RpcH64, H256 as RpcH256, H160 as RpcH160, U256 as RpcU256,
 };
@@ -53,6 +53,8 @@ use v1::helpers::dispatch::{default_gas_price, dispatch_transaction};
 use v1::helpers::block_import::is_major_importing;
 use v1::helpers::auto_args::Trailing;

+const EXTRA_INFO_PROOF: &'static str = "Object exists in in blockchain (fetched earlier), extra_info is always available if object exists; qed";
+
 /// Eth RPC options
 pub struct EthClientOptions {
 	/// Returns receipt from pending blocks
@@ -117,36 +119,39 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> EthClient<C, SN, S, M, EM> where
 		}
 	}

-	fn block(&self, id: BlockID, include_txs: bool) -> Result<Option<Block>, Error> {
+	fn block(&self, id: BlockID, include_txs: bool) -> Result<Option<RichBlock>, Error> {
 		let client = take_weak!(self.client);
 		match (client.block(id.clone()), client.block_total_difficulty(id)) {
 			(Some(bytes), Some(total_difficulty)) => {
 				let block_view = BlockView::new(&bytes);
 				let view = block_view.header_view();
-				let block = Block {
-					hash: Some(view.sha3().into()),
-					size: Some(bytes.len().into()),
-					parent_hash: view.parent_hash().into(),
-					uncles_hash: view.uncles_hash().into(),
-					author: view.author().into(),
-					miner: view.author().into(),
-					state_root: view.state_root().into(),
-					transactions_root: view.transactions_root().into(),
-					receipts_root: view.receipts_root().into(),
-					number: Some(view.number().into()),
-					gas_used: view.gas_used().into(),
-					gas_limit: view.gas_limit().into(),
-					logs_bloom: view.log_bloom().into(),
-					timestamp: view.timestamp().into(),
-					difficulty: view.difficulty().into(),
-					total_difficulty: total_difficulty.into(),
-					seal_fields: view.seal().into_iter().map(|f| rlp::decode(&f)).map(Bytes::new).collect(),
-					uncles: block_view.uncle_hashes().into_iter().map(Into::into).collect(),
-					transactions: match include_txs {
-						true => BlockTransactions::Full(block_view.localized_transactions().into_iter().map(Into::into).collect()),
-						false => BlockTransactions::Hashes(block_view.transaction_hashes().into_iter().map(Into::into).collect()),
+				let block = RichBlock {
+					block: Block {
+						hash: Some(view.sha3().into()),
+						size: Some(bytes.len().into()),
+						parent_hash: view.parent_hash().into(),
+						uncles_hash: view.uncles_hash().into(),
+						author: view.author().into(),
+						miner: view.author().into(),
+						state_root: view.state_root().into(),
+						transactions_root: view.transactions_root().into(),
+						receipts_root: view.receipts_root().into(),
+						number: Some(view.number().into()),
+						gas_used: view.gas_used().into(),
+						gas_limit: view.gas_limit().into(),
+						logs_bloom: view.log_bloom().into(),
+						timestamp: view.timestamp().into(),
+						difficulty: view.difficulty().into(),
+						total_difficulty: total_difficulty.into(),
+						seal_fields: view.seal().into_iter().map(|f| rlp::decode(&f)).map(Bytes::new).collect(),
+						uncles: block_view.uncle_hashes().into_iter().map(Into::into).collect(),
+						transactions: match include_txs {
+							true => BlockTransactions::Full(block_view.localized_transactions().into_iter().map(Into::into).collect()),
+							false => BlockTransactions::Hashes(block_view.transaction_hashes().into_iter().map(Into::into).collect()),
+						},
+						extra_data: Bytes::new(view.extra_data()),
 					},
-					extra_data: Bytes::new(view.extra_data())
+					extra_info: client.block_extra_info(id.clone()).expect(EXTRA_INFO_PROOF),
 				};
 				Ok(Some(block))
 			},
@@ -161,7 +166,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> EthClient<C, SN, S, M, EM> where
 		}
 	}

-	fn uncle(&self, id: UncleID) -> Result<Option<Block>, Error> {
+	fn uncle(&self, id: UncleID) -> Result<Option<RichBlock>, Error> {
 		let client = take_weak!(self.client);
 		let uncle: BlockHeader = match client.uncle(id) {
 			Some(rlp) => rlp::decode(&rlp),
@@ -172,27 +177,30 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> EthClient<C, SN, S, M, EM> where
 			None => { return Ok(None); }
 		};

-		let block = Block {
-			hash: Some(uncle.hash().into()),
-			size: None,
-			parent_hash: uncle.parent_hash().clone().into(),
-			uncles_hash: uncle.uncles_hash().clone().into(),
-			author: uncle.author().clone().into(),
-			miner: uncle.author().clone().into(),
-			state_root: uncle.state_root().clone().into(),
-			transactions_root: uncle.transactions_root().clone().into(),
-			number: Some(uncle.number().into()),
-			gas_used: uncle.gas_used().clone().into(),
-			gas_limit: uncle.gas_limit().clone().into(),
-			logs_bloom: uncle.log_bloom().clone().into(),
-			timestamp: uncle.timestamp().into(),
-			difficulty: uncle.difficulty().clone().into(),
-			total_difficulty: (uncle.difficulty().clone() + parent_difficulty).into(),
-			receipts_root: uncle.receipts_root().clone().into(),
-			extra_data: uncle.extra_data().clone().into(),
-			seal_fields: uncle.seal().clone().into_iter().map(|f| rlp::decode(&f)).map(Bytes::new).collect(),
-			uncles: vec![],
-			transactions: BlockTransactions::Hashes(vec![]),
+		let block = RichBlock {
+			block: Block {
+				hash: Some(uncle.hash().into()),
+				size: None,
+				parent_hash: uncle.parent_hash().clone().into(),
+				uncles_hash: uncle.uncles_hash().clone().into(),
+				author: uncle.author().clone().into(),
+				miner: uncle.author().clone().into(),
+				state_root: uncle.state_root().clone().into(),
+				transactions_root: uncle.transactions_root().clone().into(),
+				number: Some(uncle.number().into()),
+				gas_used: uncle.gas_used().clone().into(),
+				gas_limit: uncle.gas_limit().clone().into(),
+				logs_bloom: uncle.log_bloom().clone().into(),
+				timestamp: uncle.timestamp().into(),
+				difficulty: uncle.difficulty().clone().into(),
+				total_difficulty: (uncle.difficulty().clone() + parent_difficulty).into(),
+				receipts_root: uncle.receipts_root().clone().into(),
+				extra_data: uncle.extra_data().clone().into(),
+				seal_fields: uncle.seal().clone().into_iter().map(|f| rlp::decode(&f)).map(Bytes::new).collect(),
+				uncles: vec![],
+				transactions: BlockTransactions::Hashes(vec![]),
+			},
+			extra_info: client.uncle_extra_info(id).expect(EXTRA_INFO_PROOF),
 		};
 		Ok(Some(block))
 	}
@@ -435,13 +443,13 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 		}
 	}

-	fn block_by_hash(&self, hash: RpcH256, include_txs: bool) -> Result<Option<Block>, Error> {
+	fn block_by_hash(&self, hash: RpcH256, include_txs: bool) -> Result<Option<RichBlock>, Error> {
 		try!(self.active());

 		self.block(BlockID::Hash(hash.into()), include_txs)
 	}

-	fn block_by_number(&self, num: BlockNumber, include_txs: bool) -> Result<Option<Block>, Error> {
+	fn block_by_number(&self, num: BlockNumber, include_txs: bool) -> Result<Option<RichBlock>, Error> {
 		try!(self.active());

 		self.block(num.into(), include_txs)
@@ -483,13 +491,13 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 		}
 	}

-	fn uncle_by_block_hash_and_index(&self, hash: RpcH256, index: Index) -> Result<Option<Block>, Error> {
+	fn uncle_by_block_hash_and_index(&self, hash: RpcH256, index: Index) -> Result<Option<RichBlock>, Error> {
 		try!(self.active());

 		self.uncle(UncleID { block: BlockID::Hash(hash.into()), position: index.value() })
 	}

-	fn uncle_by_block_number_and_index(&self, num: BlockNumber, index: Index) -> Result<Option<Block>, Error> {
+	fn uncle_by_block_number_and_index(&self, num: BlockNumber, index: Index) -> Result<Option<RichBlock>, Error> {
 		try!(self.active());

 		self.uncle(UncleID { block: num.into(), position: index.value() })
@@ -603,7 +611,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 		let raw_transaction = raw.to_vec();
 		match UntrustedRlp::new(&raw_transaction).as_val() {
 			Ok(signed_transaction) => dispatch_transaction(&*take_weak!(self.client), &*take_weak!(self.miner), signed_transaction),
-			Err(_) => Ok(RpcH256::from(H256::from(0))),
+			Err(e) => Err(errors::from_rlp_error(e)),
 		}
 	}

@@ -613,15 +621,15 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where
 		let request = CallRequest::into(request);
 		let signed = try!(self.sign_call(request));

-		let r = match num.0 {
+		let result = match num.0 {
 			BlockNumber::Pending => take_weak!(self.miner).call(&*take_weak!(self.client), &signed, Default::default()),
 			num => take_weak!(self.client).call(&signed, num.into(), Default::default()),
 		};

-		match r {
-			Ok(b) => Ok(Bytes(b.output)),
-			Err(e) => Err(errors::from_call_error(e)),
-		}
+
+		result
+			.map(|b| b.output.into())
+			.map_err(errors::from_call_error)
 	}

 	fn estimate_gas(&self, request: CallRequest, num: Trailing<BlockNumber>) -> Result<RpcU256, Error> {
@@ -629,12 +637,14 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM> Eth for EthClient<C, SN, S, M, EM> where

 		let request = CallRequest::into(request);
 		let signed = try!(self.sign_call(request));
-		let r = match num.0 {
+		let result = match num.0 {
 			BlockNumber::Pending => take_weak!(self.miner).call(&*take_weak!(self.client), &signed, Default::default()),
 			num => take_weak!(self.client).call(&signed, num.into(), Default::default()),
 		};

-		Ok(RpcU256::from(r.map(|res| res.gas_used + res.refunded).unwrap_or(From::from(0))))
+		result
+			.map(|res| (res.gas_used + res.refunded).into())
+			.map_err(errors::from_call_error)
 	}

 	fn compile_lll(&self, _: String) -> Result<Bytes, Error> {
--- a/rpc/src/v1/impls/ethcore.rs
+++ b/rpc/src/v1/impls/ethcore.rs
@@ -302,7 +302,9 @@ impl<C, M, S: ?Sized, F> Ethcore for EthcoreClient<C, M, S, F> where
 							.map(Into::into);

 					// Receive ready and invoke with result.
-					let ready: Ready<H256> = rx.try_recv().expect("When on_done is invoked ready object is always sent.");
+					let ready: Ready<H256> = rx.recv().expect(
+						"recv() fails when `tx` has been dropped, if this closure is invoked `tx` is not dropped (`res == Ok()`); qed"
+					);
 					ready.ready(result);
 				}));

@@ -310,7 +312,9 @@ impl<C, M, S: ?Sized, F> Ethcore for EthcoreClient<C, M, S, F> where
 				if let Err(e) = res {
 					ready.ready(Err(errors::from_fetch_error(e)));
 				} else {
-					tx.send(ready).expect("Rx end is sent to on_done closure.");
+					tx.send(ready).expect(
+						"send() fails when `rx` end is dropped, if `res == Ok()`: `rx` is moved to the closure; qed"
+					);
 				}
 			}
 		}
--- a/rpc/src/v1/tests/mocked/eth.rs
+++ b/rpc/src/v1/tests/mocked/eth.rs
@@ -805,6 +805,23 @@ fn rpc_eth_send_transaction_error() {
 	assert_eq!(tester.io.handle_request_sync(&request), Some(response.into()));
 }

+#[test]
+fn rpc_eth_send_raw_transaction_error() {
+	let tester = EthTester::default();
+
+	let req = r#"{
+		"jsonrpc": "2.0",
+		"method": "eth_sendRawTransaction",
+		"params": [
+			"0x0123"
+		],
+		"id": 1
+	}"#;
+	let res = r#"{"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid RLP.","data":"RlpIncorrectListLen"},"id":1}"#.into();
+
+	assert_eq!(tester.io.handle_request_sync(&req), Some(res));
+}
+
 #[test]
 fn rpc_eth_send_raw_transaction() {
 	let tester = EthTester::default();
--- a/rpc/src/v1/traits/eth.rs
+++ b/rpc/src/v1/traits/eth.rs
@@ -17,7 +17,7 @@
 //! Eth rpc interface.
 use jsonrpc_core::*;

-use v1::types::{Block, BlockNumber, Bytes, CallRequest, Filter, FilterChanges, Index};
+use v1::types::{RichBlock, BlockNumber, Bytes, CallRequest, Filter, FilterChanges, Index};
 use v1::types::{Log, Receipt, SyncStatus, Transaction, Work};
 use v1::types::{H64, H160, H256, U256};

@@ -68,11 +68,11 @@ build_rpc_trait! {

 		/// Returns block with given hash.
 		#[rpc(name = "eth_getBlockByHash")]
-		fn block_by_hash(&self, H256, bool) -> Result<Option<Block>, Error>;
+		fn block_by_hash(&self, H256, bool) -> Result<Option<RichBlock>, Error>;

 		/// Returns block with given number.
 		#[rpc(name = "eth_getBlockByNumber")]
-		fn block_by_number(&self, BlockNumber, bool) -> Result<Option<Block>, Error>;
+		fn block_by_number(&self, BlockNumber, bool) -> Result<Option<RichBlock>, Error>;

 		/// Returns the number of transactions sent from given address at given time (block number).
 		#[rpc(name = "eth_getTransactionCount")]
@@ -128,11 +128,11 @@ build_rpc_trait! {

 		/// Returns an uncles at given block and index.
 		#[rpc(name = "eth_getUncleByBlockHashAndIndex")]
-		fn uncle_by_block_hash_and_index(&self, H256, Index) -> Result<Option<Block>, Error>;
+		fn uncle_by_block_hash_and_index(&self, H256, Index) -> Result<Option<RichBlock>, Error>;

 		/// Returns an uncles at given block and index.
 		#[rpc(name = "eth_getUncleByBlockNumberAndIndex")]
-		fn uncle_by_block_number_and_index(&self, BlockNumber, Index) -> Result<Option<Block>, Error>;
+		fn uncle_by_block_number_and_index(&self, BlockNumber, Index) -> Result<Option<RichBlock>, Error>;

 		/// Returns available compilers.
 		#[rpc(name = "eth_getCompilers")]
--- a/rpc/src/v1/types/block.rs
+++ b/rpc/src/v1/types/block.rs
@@ -14,6 +14,8 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.

+use std::ops::Deref;
+use std::collections::BTreeMap;
 use serde::{Serialize, Serializer};
 use v1::types::{Bytes, Transaction, H160, H256, H2048, U256};

@@ -93,11 +95,45 @@ pub struct Block {
 	pub size: Option<U256>,
 }

+/// Block representation with additional info
+#[derive(Debug)]
+pub struct RichBlock {
+	/// Standard block
+	pub block: Block,
+	/// Engine-specific fields with additional description.
+	/// Should be included directly to serialized block object.
+	#[serde(skip_serializing)]
+	pub extra_info: BTreeMap<String, String>,
+}
+
+impl Deref for RichBlock {
+	type Target = Block;
+	fn deref(&self) -> &Self::Target {
+		&self.block
+	}
+}
+
+impl Serialize for RichBlock {
+	fn serialize<S>(&self, serializer: &mut S) -> Result<(), S::Error> where S: Serializer {
+		use serde_json::{to_value, Value};
+
+		let serialized = (to_value(&self.block), to_value(&self.extra_info));
+		if let (Value::Object(mut block), Value::Object(extras)) = serialized {
+			// join two objects
+			block.extend(extras);
+			// and serialize
+			try!(block.serialize(serializer));
+		}
+		Ok(())
+	}
+}
+
 #[cfg(test)]
 mod tests {
+	use std::collections::BTreeMap;
 	use serde_json;
-	use v1::types::{Transaction, H160, H256, H2048, Bytes, U256};
-	use super::{Block, BlockTransactions};
+	use v1::types::{Transaction, H64, H160, H256, H2048, Bytes, U256};
+	use super::{Block, RichBlock, BlockTransactions};

 	#[test]
 	fn test_serialize_block_transactions() {
@@ -134,8 +170,17 @@ mod tests {
 			transactions: BlockTransactions::Hashes(vec![].into()),
 			size: Some(69.into()),
 		};
+		let serialized_block = serde_json::to_string(&block).unwrap();
+		let rich_block = RichBlock {
+			block: block,
+			extra_info: map![
+				"mixHash".into() => format!("0x{:?}", H256::default()),
+				"nonce".into() => format!("0x{:?}", H64::default())
+			],
+		};
+		let serialized_rich_block = serde_json::to_string(&rich_block).unwrap();

-		let serialized = serde_json::to_string(&block).unwrap();
-		assert_eq!(serialized, r#"{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","parentHash":"0x0000000000000000000000000000000000000000000000000000000000000000","sha3Uncles":"0x0000000000000000000000000000000000000000000000000000000000000000","author":"0x0000000000000000000000000000000000000000","miner":"0x0000000000000000000000000000000000000000","stateRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","transactionsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","receiptsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","number":"0x0","gasUsed":"0x0","gasLimit":"0x0","extraData":"0x","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","timestamp":"0x0","difficulty":"0x0","totalDifficulty":"0x0","sealFields":["0x","0x"],"uncles":[],"transactions":[],"size":"0x45"}"#);
+		assert_eq!(serialized_block, r#"{"hash":"0x0000000000000000000000000000000000000000000000000000000000000000","parentHash":"0x0000000000000000000000000000000000000000000000000000000000000000","sha3Uncles":"0x0000000000000000000000000000000000000000000000000000000000000000","author":"0x0000000000000000000000000000000000000000","miner":"0x0000000000000000000000000000000000000000","stateRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","transactionsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","receiptsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","number":"0x0","gasUsed":"0x0","gasLimit":"0x0","extraData":"0x","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","timestamp":"0x0","difficulty":"0x0","totalDifficulty":"0x0","sealFields":["0x","0x"],"uncles":[],"transactions":[],"size":"0x45"}"#);
+		assert_eq!(serialized_rich_block, r#"{"author":"0x0000000000000000000000000000000000000000","difficulty":"0x0","extraData":"0x","gasLimit":"0x0","gasUsed":"0x0","hash":"0x0000000000000000000000000000000000000000000000000000000000000000","logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","miner":"0x0000000000000000000000000000000000000000","mixHash":"0x0000000000000000000000000000000000000000000000000000000000000000","nonce":"0x0000000000000000","number":"0x0","parentHash":"0x0000000000000000000000000000000000000000000000000000000000000000","receiptsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","sealFields":["0x","0x"],"sha3Uncles":"0x0000000000000000000000000000000000000000000000000000000000000000","size":"0x45","stateRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","timestamp":"0x0","totalDifficulty":"0x0","transactions":[],"transactionsRoot":"0x0000000000000000000000000000000000000000000000000000000000000000","uncles":[]}"#);
 	}
 }
--- a/rpc/src/v1/types/mod.rs.in
+++ b/rpc/src/v1/types/mod.rs.in
@@ -35,7 +35,7 @@ mod work;
 mod histogram;

 pub use self::bytes::Bytes;
-pub use self::block::{Block, BlockTransactions};
+pub use self::block::{RichBlock, Block, BlockTransactions};
 pub use self::block_number::BlockNumber;
 pub use self::call_request::CallRequest;
 pub use self::confirmations::{ConfirmationPayload, ConfirmationRequest, TransactionModification};