added ConfirmationPayload::SignMessage for non-prefixed signatures, a…

…dded tests for sign191

rpc/src/v1/helpers/dispatch.rs

@@ -49,6 +49,7 @@ use v1::types::{
 	RichRawTransaction as RpcRichRawTransaction,
 	ConfirmationPayload as RpcConfirmationPayload,
 	ConfirmationResponse,
+	EthSignRequest as RpcEthSignRequest,
 	SignRequest as RpcSignRequest,
 	DecryptRequest as RpcDecryptRequest,
 };
@@ -693,6 +694,19 @@ pub fn execute<D: Dispatcher + 'static>(
 				);
 			Box::new(future::done(res))
 		},
+		ConfirmationPayload::SignMessage(address, data) => {
+			if accounts.is_hardware_address(&address) {
+				return Box::new(future::err(errors::account("Error signing message with hardware_wallet",
+					"Message signing is unsupported")));
+			}
+			let res = signature(&accounts, address, data, pass)
+				.map(|result| result
+					.map(|rsv| H520(rsv.into_electrum()))
+					.map(RpcH520::from)
+					.map(ConfirmationResponse::Signature)
+				);
+			Box::new(future::done(res))
+		},
 		ConfirmationPayload::Decrypt(address, data) => {
 			if accounts.is_hardware_address(&address) {
 				return Box::new(future::err(errors::unsupported("Decrypting via hardware wallets is not supported.", None)));
@@ -775,8 +789,11 @@ pub fn from_rpc<D>(payload: RpcConfirmationPayload, default_account: Address, di
 		RpcConfirmationPayload::Decrypt(RpcDecryptRequest { address, msg }) => {
 			Box::new(future::ok(ConfirmationPayload::Decrypt(address.into(), msg.into())))
 		},
-		RpcConfirmationPayload::EthSignMessage(RpcSignRequest { address, data }) => {
+		RpcConfirmationPayload::EthSignMessage(RpcEthSignRequest { address, data }) => {
 			Box::new(future::ok(ConfirmationPayload::EthSignMessage(address.into(), data.into())))
 		},
+		RpcConfirmationPayload::SignMessage(RpcSignRequest { address, data }) => {
+			Box::new(future::ok(ConfirmationPayload::SignMessage(address.into(), data.into())))
+		},
 	}
 }

rpc/src/v1/helpers/eip191.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-//! EIP-191 compliant decoding
+//! EIP-191 compliant decoding + hashing
 use v1::types::{EIP191Version, Bytes, WithValidator};
 use eip712::{hash_structured_data, EIP712};
 use serde_json::{Value, from_value};
@@ -25,26 +25,27 @@ use hash::keccak;
 use std::fmt::Display;
 use ethereum_types::H256;
 
-pub fn decode_message(version: EIP191Version, data: Value) -> Result<H256, Error> {
+/// deserializes and hashes the message depending on the version specifier
+pub fn hash_message(version: EIP191Version, message: Value) -> Result<H256, Error> {
 	let data = match version {
 		EIP191Version::StructuredData => {
-			let typed_data = from_value::<EIP712>(data)
+			let typed_data = from_value::<EIP712>(message)
 				.map_err(map_serde_err("StructuredData"))?;
 
 			hash_structured_data(typed_data)
 				.map_err(|err| errors::invalid_call_data(err.kind()))?
 		}
 
 		EIP191Version::WithValidator => {
-			let typed_data = from_value::<WithValidator>(data)
+			let data = from_value::<WithValidator>(message)
 				.map_err(map_serde_err("WithValidator"))?;
 			let prefix = b"\x19\x00";
-			let data = [&prefix[..], &typed_data.address.0[..], &typed_data.application_data.0[..]].concat();
+			let data = [&prefix[..], &data.validator.0[..], &data.application_data.0[..]].concat();
 			keccak(data)
 		}
 
 		EIP191Version::PersonalMessage => {
-			let bytes = from_value::<Bytes>(data)
+			let bytes = from_value::<Bytes>(message)
 				.map_err(map_serde_err("Bytes"))?;
 			eth_data_hash(bytes.0)
 		}

rpc/src/v1/helpers/requests.rs

@@ -18,6 +18,7 @@ use ethereum_types::{U256, Address};
 use bytes::Bytes;
 
 use v1::types::{Origin, TransactionCondition};
+use ethereum_types::H256;
 
 /// Transaction request coming from RPC
 #[derive(Debug, Clone, Default, Eq, PartialEq, Hash)]
@@ -117,6 +118,8 @@ pub enum ConfirmationPayload {
 	SignTransaction(FilledTransactionRequest),
 	/// Sign a message with an Ethereum specific security prefix.
 	EthSignMessage(Address, Bytes),
+	/// Sign a message
+	SignMessage(Address, H256),
 	/// Decrypt request
 	Decrypt(Address, Bytes),
 }
@@ -127,6 +130,7 @@ impl ConfirmationPayload {
 			ConfirmationPayload::SendTransaction(ref request) => request.from,
 			ConfirmationPayload::SignTransaction(ref request) => request.from,
 			ConfirmationPayload::EthSignMessage(ref address, _) => *address,
+			ConfirmationPayload::SignMessage(ref address, _) => *address,
 			ConfirmationPayload::Decrypt(ref address, _) => *address,
 		}
 	}

rpc/src/v1/impls/personal.rs

@@ -154,15 +154,11 @@ impl<D: Dispatcher + 'static> Personal for PersonalClient<D> {
 	}
 
 	fn sign_191(&self, version: EIP191Version, data: Value, account: RpcH160, password: String) -> BoxFuture<RpcH520> {
-		let data = match eip191::decode_message(version, data) {
-			Ok(d) => d,
-			Err(e) => return Box::new(future::err(e))
-		};
-
+		let data = try_bf!(eip191::hash_message(version, data));
 		let dispatcher = self.dispatcher.clone();
 		let accounts = self.accounts.clone();
 
-		let payload = RpcConfirmationPayload::EthSignMessage((account.clone(), RpcBytes(data.to_vec())).into());
+		let payload = RpcConfirmationPayload::SignMessage((account.clone(), data.into()).into());
 
 		Box::new(dispatch::from_rpc(payload, account.into(), &dispatcher)
 			.and_then(|payload| {

rpc/src/v1/impls/signer.rs

@@ -215,6 +215,14 @@ impl<D: Dispatcher + 'static> Signer for SignerClient<D> {
 						Err(err) => Err(errors::invalid_params("Invalid signature received.", err)),
 					}
 				},
+				ConfirmationPayload::SignMessage(address, hash) => {
+					let signature = ethkey::Signature::from_electrum(&bytes.0);
+					match ethkey::verify_address(&address, &signature, &hash) {
+						Ok(true) => Ok(ConfirmationResponse::Signature(bytes.0.as_slice().into())),
+						Ok(false) => Err(errors::invalid_params("Sender address does not match the signature.", ())),
+						Err(err) => Err(errors::invalid_params("Invalid signature received.", err)),
+					}
+				},
 				ConfirmationPayload::Decrypt(_address, _data) => {
 					// TODO [ToDr]: Decrypt can we verify if the answer is correct?
 					Ok(ConfirmationResponse::Decrypt(bytes))

rpc/src/v1/tests/mocked/personal.rs

@@ -25,12 +25,16 @@ use jsonrpc_core::IoHandler;
 use parking_lot::Mutex;
 use transaction::{Action, Transaction};
 use parity_runtime::Runtime;
+use hash::keccak;
 
 use v1::{PersonalClient, Personal, Metadata};
-use v1::helpers::nonce;
+use v1::helpers::{nonce, eip191};
 use v1::helpers::dispatch::{eth_data_hash, FullDispatcher};
 use v1::tests::helpers::TestMinerService;
-use v1::types::H520;
+use v1::types::{EIP191Version, WithValidator, H520};
+use rustc_hex::ToHex;
+use serde_json::to_value;
+use ethkey::Secret;
 
 struct PersonalTester {
 	_runtime: Runtime,
@@ -328,3 +332,89 @@ fn should_unlock_account_permanently() {
 	assert_eq!(tester.io.handle_request_sync(&request), Some(response.into()));
 	assert!(tester.accounts.sign(address, None, Default::default()).is_ok(), "Should unlock account.");
 }
+
+#[test]
+fn sign_eip191_with_validator() {
+	let tester = setup();
+	let address = tester.accounts.new_account(&"password123".into()).unwrap();
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "personal_sign191",
+		"params": [
+			"0x00",
+			{
+				"validator": ""#.to_owned() + &format!("0x{:x}", address) + r#"",
+				"applicationData": ""# + &format!("0x{:x}", keccak("hello world")) + r#""
+			},
+			""# + &format!("0x{:x}", address) + r#"",
+			"password123"
+		],
+		"id": 1
+	}"#;
+	let with_validator = to_value(WithValidator {
+		validator: address.into(),
+		application_data: keccak("hello world").to_vec().into()
+	}).unwrap();
+	let result = eip191::hash_message(EIP191Version::WithValidator, with_validator).unwrap();
+	let result = tester.accounts.sign(address, Some("password123".into()), result).unwrap().into_electrum();
+	let expected = r#"{"jsonrpc":"2.0","result":""#.to_owned() +  &format!("0x{}", result.to_hex()) + r#"","id":1}"#;
+	let response = tester.io.handle_request_sync(&request).unwrap();
+	assert_eq!(response, expected)
+}
+
+#[test]
+fn sign_eip191_structured_data() {
+	let tester = setup();
+	let secret: Secret = keccak("cow").into();
+	let address = tester.accounts.insert_account(secret, &"lol".into()).unwrap();
+	let request = r#"{
+		"jsonrpc": "2.0",
+		"method": "personal_sign191",
+		"params": [
+			"0x01",
+			{
+				"primaryType": "Mail",
+				"domain": {
+					"name": "Ether Mail",
+					"version": "1",
+					"chainId": "0x1",
+					"verifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC"
+				},
+				"message": {
+					"from": {
+						"name": "Cow",
+						"wallet": "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826"
+					},
+					"to": {
+						"name": "Bob",
+						"wallet": "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB"
+					},
+					"contents": "Hello, Bob!"
+				},
+				"types": {
+					"EIP712Domain": [
+						{ "name": "name", "type": "string" },
+						{ "name": "version", "type": "string" },
+						{ "name": "chainId", "type": "uint256" },
+						{ "name": "verifyingContract", "type": "address" }
+					],
+					"Person": [
+						{ "name": "name", "type": "string" },
+						{ "name": "wallet", "type": "address" }
+					],
+					"Mail": [
+						{ "name": "from", "type": "Person" },
+						{ "name": "to", "type": "Person" },
+						{ "name": "contents", "type": "string" }
+					]
+				}
+			},
+			""#.to_owned() + &format!("0x{:x}", address) + r#"",
+			"lol"
+		],
+		"id": 1
+	}"#;
+	let expected = r#"{"jsonrpc":"2.0","result":"0x4355c47d63924e8a72e509b65029052eb6c299d53a04e167c5775fd466751c9d07299936d304c153f6443dfa05f40ff007d72911b6f72307f996231605b915621c","id":1}"#;
+	let response = tester.io.handle_request_sync(&request).unwrap();
+	assert_eq!(response, expected)
+}

rpc/src/v1/types/confirmations.rs

@@ -59,23 +59,34 @@ impl fmt::Display for ConfirmationPayload {
 			ConfirmationPayload::SendTransaction(ref transaction) => write!(f, "{}", transaction),
 			ConfirmationPayload::SignTransaction(ref transaction) => write!(f, "(Sign only) {}", transaction),
 			ConfirmationPayload::EthSignMessage(ref sign) => write!(f, "{}", sign),
+			ConfirmationPayload::SignMessage(ref sign) => write!(f, "{}", sign),
 			ConfirmationPayload::Decrypt(ref decrypt) => write!(f, "{}", decrypt),
 		}
 	}
 }
 
+/// Ethereum-prefixed Sign request
+#[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
+pub struct EthSignRequest {
+	/// Address
+	pub address: H160,
+	/// Hash to sign
+	pub data: Bytes,
+}
+
 /// Sign request
 #[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct SignRequest {
 	/// Address
 	pub address: H160,
 	/// Hash to sign
-	pub data: Bytes,
+	pub data: H256,
 }
 
-impl From<(H160, Bytes)> for SignRequest {
-	fn from(tuple: (H160, Bytes)) -> Self {
+impl From<(H160, H256)> for SignRequest {
+	fn from(tuple: (H160, H256)) -> Self {
 		SignRequest {
 			address: tuple.0,
 			data: tuple.1,
@@ -94,6 +105,26 @@ impl fmt::Display for SignRequest {
 	}
 }
 
+impl From<(H160, Bytes)> for EthSignRequest {
+	fn from(tuple: (H160, Bytes)) -> Self {
+		EthSignRequest {
+			address: tuple.0,
+			data: tuple.1,
+		}
+	}
+}
+
+impl fmt::Display for EthSignRequest {
+	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+		write!(
+			f,
+			"sign 0x{} with {}",
+			self.data.0.pretty(),
+			Colour::White.bold().paint(format!("0x{:?}", self.address)),
+		)
+	}
+}
+
 /// Decrypt request
 #[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
@@ -169,7 +200,9 @@ pub enum ConfirmationPayload {
 	SignTransaction(TransactionRequest),
 	/// Signature
 	#[serde(rename = "sign")]
-	EthSignMessage(SignRequest),
+	EthSignMessage(EthSignRequest),
+	/// signature without prefix
+	SignMessage(SignRequest),
 	/// Decryption
 	Decrypt(DecryptRequest),
 }
@@ -179,7 +212,11 @@ impl From<helpers::ConfirmationPayload> for ConfirmationPayload {
 		match c {
 			helpers::ConfirmationPayload::SendTransaction(t) => ConfirmationPayload::SendTransaction(t.into()),
 			helpers::ConfirmationPayload::SignTransaction(t) => ConfirmationPayload::SignTransaction(t.into()),
-			helpers::ConfirmationPayload::EthSignMessage(address, data) => ConfirmationPayload::EthSignMessage(SignRequest {
+			helpers::ConfirmationPayload::EthSignMessage(address, data) => ConfirmationPayload::EthSignMessage(EthSignRequest {
+				address: address.into(),
+				data: data.into(),
+			}),
+			helpers::ConfirmationPayload::SignMessage(address, data) => ConfirmationPayload::SignMessage(SignRequest {
 				address: address.into(),
 				data: data.into(),
 			}),

rpc/src/v1/types/eip191.rs

@@ -19,16 +19,20 @@ use serde::{Deserialize, Deserializer};
 use serde::de;
 use v1::types::{H160, Bytes};
 
+/// EIP-191 version specifier
+#[derive(Debug)]
 pub enum EIP191Version {
 	StructuredData,
 	PersonalMessage,
 	WithValidator
 }
 
-#[derive(Deserialize)]
+/// EIP-191 version 0x0 struct
+#[derive(Deserialize, Serialize)]
+#[serde(rename_all = "camelCase")]
 pub struct WithValidator {
 	// address of intended validator
-	pub address: H160,
+	pub validator: H160,
 	// application specific data
 	pub application_data: Bytes
 }
@@ -40,8 +44,8 @@ impl<'de> Deserialize<'de> for EIP191Version {
 	{
 		let s = String::deserialize(deserializer)?;
 		let byte_version = match s.as_str() {
-			"0x00" | "0x0" | "0x" => EIP191Version::WithValidator,
-			"0x01" | "0x1" => EIP191Version::StructuredData,
+			"0x00" => EIP191Version::WithValidator,
+			"0x01" => EIP191Version::StructuredData,
 			"0x45" => EIP191Version::PersonalMessage,
 			other => return Err(de::Error::custom(format!("Invalid byte version '{}'", other))),
 		};

rpc/src/v1/types/mod.rs

@@ -54,7 +54,7 @@ pub use self::block_number::{BlockNumber, LightBlockNumber, block_number_to_id};
 pub use self::call_request::CallRequest;
 pub use self::confirmations::{
 	ConfirmationPayload, ConfirmationRequest, ConfirmationResponse, ConfirmationResponseWithToken,
-	TransactionModification, SignRequest, DecryptRequest, Either
+	TransactionModification, SignRequest, EthSignRequest, DecryptRequest, Either
 };
 pub use self::consensus_status::*;
 pub use self::derivation::{DeriveHash, DeriveHierarchical, Derive};