eth_sign improvements backport (#4473)

* Fix postsign (#4347)

* Fix whitespace.

* Fix post sign.

* Fix message.

* Fix tests.

* Rest of the problems.

* All hail the linter and its omniscience.

* ...and its divine omniscience.

* Grumbles and wording.

* Make signing compatible with geth. (#4468)

js/src/api/rpc/parity/parity.js

@@ -313,6 +313,11 @@ export default class Parity {
       .execute('parity_postTransaction', inOptions(options));
   }
 
+  postSign (from, message) {
+    return this._transport
+      .execute('parity_postSign', from, message);
+  }
+
   registryAddress () {
     return this._transport
       .execute('parity_registryAddress')

js/src/views/Signer/components/RequestPending/requestPending.js

@@ -61,7 +61,7 @@ export default class RequestPending extends Component {
           address={ sign.address }
           className={ className }
           focus={ focus }
-          hash={ sign.hash }
+          data={ sign.data }
           id={ id }
           isFinished={ false }
           isSending={ isSending }

js/src/views/Signer/components/SignRequest/signRequest.css

@@ -27,8 +27,20 @@
   min-height: $pendingHeight;
 }
 
+.signData {
+    border: 0.25em solid red;
+    padding: 0.5em;
+    margin-left: -2em;
+    overflow: auto;
+    max-height: 6em;
+}
+
+.signData > p {
+    color: white;
+}
+
 .signDetails {
-  flex: 1;
+  flex: 10;
 }
 
 .address, .info {

js/src/views/Signer/components/SignRequest/signRequest.js

@@ -19,16 +19,30 @@ import { observer } from 'mobx-react';
 
 import Account from '../Account';
 import TransactionPendingForm from '../TransactionPendingForm';
-import TxHashLink from '../TxHashLink';
 
 import styles from './signRequest.css';
 
+function isAscii (data) {
+  for (var i = 2; i < data.length; i += 2) {
+    let n = parseInt(data.substr(i, 2), 16);
+
+    if (n < 32 || n >= 128) {
+      return false;
+    }
+  }
+  return true;
+}
+
 @observer
 export default class SignRequest extends Component {
+  static contextTypes = {
+    api: PropTypes.object
+  };
+
   static propTypes = {
     id: PropTypes.object.isRequired,
     address: PropTypes.string.isRequired,
-    hash: PropTypes.string.isRequired,
+    data: PropTypes.string.isRequired,
     isFinished: PropTypes.bool.isRequired,
     isTest: PropTypes.bool.isRequired,
     store: PropTypes.object.isRequired,
@@ -62,8 +76,23 @@ export default class SignRequest extends Component {
     );
   }
 
+  renderAsciiDetails (ascii) {
+    return (
+      <div className={ styles.signData }>
+        <p>{ascii}</p>
+      </div>
+    );
+  }
+
+  renderBinaryDetails (data) {
+    return (<div className={ styles.signData }>
+      <p>(Unknown binary data)</p>
+    </div>);
+  }
+
   renderDetails () {
-    const { address, hash, isTest, store } = this.props;
+    const { api } = this.context;
+    const { address, isTest, store, data } = this.props;
     const balance = store.balances[address];
 
     if (!balance) {
@@ -78,9 +107,14 @@ export default class SignRequest extends Component {
             balance={ balance }
             isTest={ isTest } />
         </div>
-        <div className={ styles.info } title={ hash }>
+        <div className={ styles.info } title={ api.util.sha3(data) }>
-          <p>Dapp is requesting to sign arbitrary transaction using this account.</p>
+          <p>A request to sign data using your account:</p>
-          <p><strong>Confirm the transaction only if you trust the app.</strong></p>
+          {
+            isAscii(data)
+              ? this.renderAsciiDetails(api.util.hexToAscii(data))
+              : this.renderBinaryDetails(data)
+          }
+          <p><strong>WARNING: This consequences of doing this may be grave. Confirm the request only if you are sure.</strong></p>
         </div>
       </div>
     );
@@ -91,18 +125,9 @@ export default class SignRequest extends Component {
 
     if (isFinished) {
       if (status === 'confirmed') {
-        const { hash, isTest } = this.props;
-
         return (
           <div className={ styles.actions }>
             <span className={ styles.isConfirmed }>Confirmed</span>
-            <div>
-              Transaction hash:
-              <TxHashLink
-                isTest={ isTest }
-                txHash={ hash }
-                className={ styles.txHash } />
-            </div>
           </div>
         );
       }

js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormConfirm/transactionPendingFormConfirm.js

@@ -147,7 +147,7 @@ class TransactionPendingFormConfirm extends Component {
               label={
                 isSending
                   ? 'Confirming...'
-                  : 'Confirm Transaction'
+                  : 'Confirm Request'
               }
               onTouchTap={ this.onConfirm }
               primary />
@@ -251,7 +251,7 @@ function mapStateToProps (_, initProps) {
 
   return (state) => {
     const { accounts } = state.personal;
-    let gotAddress = Object.keys(accounts).find(a => a.toLowerCase() === address.toLowerCase());
+    const gotAddress = Object.keys(accounts).find(a => a.toLowerCase() === address.toLowerCase());
     const account = gotAddress ? accounts[gotAddress] : {};
 
     return { account };

js/src/views/Signer/components/TransactionPendingForm/TransactionPendingFormReject/transactionPendingFormReject.js

@@ -33,14 +33,14 @@ export default class TransactionPendingFormReject extends Component {
     return (
       <div>
         <div className={ styles.rejectText }>
-          Are you sure you want to reject transaction? <br />
+          Are you sure you want to reject request? <br />
           <strong>This cannot be undone</strong>
         </div>
         <RaisedButton
           onTouchTap={ onReject }
           className={ styles.rejectButton }
           fullWidth
-          label={ 'Reject Transaction' }
+          label={ 'Reject Request' }
         />
       </div>
     );

js/src/views/Signer/components/TransactionPendingForm/transactionPendingForm.js

@@ -76,7 +76,7 @@ export default class TransactionPendingForm extends Component {
     let html;
 
     if (!isRejectOpen) {
-      html = <span>reject transaction</span>;
+      html = <span>reject request</span>;
     } else {
       html = <span><BackIcon />{ "I've changed my mind" }</span>;
     }

rpc/src/v1/helpers/dispatch.rs

@@ -19,6 +19,7 @@ use std::ops::Deref;
 use rlp;
 use util::{Address, H520, H256, U256, Uint, Bytes};
 use util::bytes::ToPretty;
+use util::sha3::Hashable;
 
 use ethkey::Signature;
 use ethcore::miner::MinerService;
@@ -108,8 +109,12 @@ pub fn execute<C, M>(client: &C, miner: &M, accounts: &AccountProvider, payload:
 					.map(ConfirmationResponse::SignTransaction)
 				)
 		},
-		ConfirmationPayload::Signature(address, hash) => {
+		ConfirmationPayload::Signature(address, mut data) => {
-			signature(accounts, address, hash, pass)
+			let mut message_data =
+				format!("\x19Ethereum Signed Message:\n{}", data.len())
+				.into_bytes();
+			message_data.append(&mut data);
+			signature(accounts, address, message_data.sha3(), pass)
 				.map(|result| result
 					.map(|rsv| {
 						let mut vrs = [0u8; 65];
@@ -250,8 +255,8 @@ pub fn from_rpc<C, M>(payload: RpcConfirmationPayload, client: &C, miner: &M) ->
 		RpcConfirmationPayload::Decrypt(RpcDecryptRequest { address, msg }) => {
 			ConfirmationPayload::Decrypt(address.into(), msg.into())
 		},
-		RpcConfirmationPayload::Signature(RpcSignRequest { address, hash }) => {
+		RpcConfirmationPayload::Signature(RpcSignRequest { address, data }) => {
-			ConfirmationPayload::Signature(address.into(), hash.into())
+			ConfirmationPayload::Signature(address.into(), data.into())
 		},
 	}
 }

rpc/src/v1/helpers/requests.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/>.
 
-use util::{Address, U256, Bytes, H256};
+use util::{Address, U256, Bytes};
 use v1::types::TransactionCondition;
 
 /// Transaction request coming from RPC
@@ -110,7 +110,7 @@ pub enum ConfirmationPayload {
 	/// Sign Transaction
 	SignTransaction(FilledTransactionRequest),
 	/// Sign request
-	Signature(Address, H256),
+	Signature(Address, Bytes),
 	/// Decrypt request
 	Decrypt(Address, Bytes),
 }

rpc/src/v1/impls/signing.rs

@@ -18,7 +18,7 @@
 
 use std::sync::{Arc, Weak};
 use transient_hashmap::TransientHashMap;
-use util::{U256, Mutex, Hashable};
+use util::{U256, Mutex};
 
 use ethcore::account_provider::AccountProvider;
 use ethcore::miner::MinerService;
@@ -122,9 +122,9 @@ impl<C: 'static, M: 'static> ParitySigning for SigningQueueClient<C, M> where
 	C: MiningBlockChainClient,
 	M: MinerService,
 {
-	fn post_sign(&self, address: RpcH160, hash: RpcH256) -> Result<RpcEither<RpcU256, RpcConfirmationResponse>, Error> {
+	fn post_sign(&self, address: RpcH160, data: RpcBytes) -> Result<RpcEither<RpcU256, RpcConfirmationResponse>, Error> {
 		self.active()?;
-		self.dispatch(RpcConfirmationPayload::Signature((address, hash).into()))
+		self.dispatch(RpcConfirmationPayload::Signature((address, data).into()))
 			.map(|result| match result {
 				DispatchResult::Value(v) => RpcEither::Or(v),
 				DispatchResult::Promise(promise) => {
@@ -183,8 +183,8 @@ impl<C: 'static, M: 'static> EthSigning for SigningQueueClient<C, M> where
 	M: MinerService,
 {
 	fn sign(&self, ready: Ready<RpcH520>, address: RpcH160, data: RpcBytes) {
-		let hash = data.0.sha3().into();
+		let res = self.active().and_then(|_| self.dispatch(RpcConfirmationPayload::Signature((address, data).into())));
-		let res = self.active().and_then(|_| self.dispatch(RpcConfirmationPayload::Signature((address, hash).into())));
+
 		self.handle_dispatch(res, |response| {
 			match response {
 				Ok(RpcConfirmationResponse::Signature(signature)) => ready.ready(Ok(signature)),

rpc/src/v1/impls/signing_unsafe.rs

@@ -17,7 +17,6 @@
 //! Unsafe Signing RPC implementation.
 
 use std::sync::{Arc, Weak};
-use util::Hashable;
 
 use ethcore::account_provider::AccountProvider;
 use ethcore::miner::MinerService;
@@ -86,8 +85,7 @@ impl<C: 'static, M: 'static> EthSigning for SigningUnsafeClient<C, M> where
 	M: MinerService,
 {
 	fn sign(&self, ready: Ready<RpcH520>, address: RpcH160, data: RpcBytes) {
-		let hash = data.0.sha3().into();
+		let result = match self.handle(RpcConfirmationPayload::Signature((address, data).into())) {
-		let result = match self.handle(RpcConfirmationPayload::Signature((address, hash).into())) {
 			Ok(RpcConfirmationResponse::Signature(signature)) => Ok(signature),
 			Err(e) => Err(e),
 			e => Err(errors::internal("Unexpected result", e)),
@@ -127,7 +125,7 @@ impl<C: 'static, M: 'static> ParitySigning for SigningUnsafeClient<C, M> where
 		ready.ready(result);
 	}
 
-	fn post_sign(&self, _: RpcH160, _: RpcH256) -> Result<RpcEither<RpcU256, RpcConfirmationResponse>, Error> {
+	fn post_sign(&self, _: RpcH160, _: RpcBytes) -> Result<RpcEither<RpcU256, RpcConfirmationResponse>, Error> {
 		// We don't support this in non-signer mode.
 		Err(errors::signer_disabled())
 	}

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

@@ -309,7 +309,7 @@ fn rpc_eth_sign() {
 		],
 		"id": 1
 	}"#;
-	let res = r#"{"jsonrpc":"2.0","result":"0x1b5100b2be0aafd86271c8f49891262920bfbfeaeccb2ef1d0b2053aefc3ddb399483eb3c902ecf4add3156461a61f59e924a65eb5e6cdbab0a158d45db5f87cdf","id":1}"#;
+	let res = r#"{"jsonrpc":"2.0","result":"0x1ba2870db1d0c26ef93c7b72d2a0830fa6b841e0593f7186bc6c7cc317af8cf3a42fda03bd589a49949aa05db83300cdb553116274518dbe9d90c65d0213f4af49","id":1}"#;
 
 	assert_eq!(tester.io.handle_request_sync(&req), Some(res.into()));
 }

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

@@ -85,14 +85,14 @@ fn should_return_list_of_items_to_confirm() {
 		nonce: None,
 		condition: None,
 	})).unwrap();
-	tester.signer.add_request(ConfirmationPayload::Signature(1.into(), 5.into())).unwrap();
+	tester.signer.add_request(ConfirmationPayload::Signature(1.into(), vec![5].into())).unwrap();
 
 	// when
 	let request = r#"{"jsonrpc":"2.0","method":"signer_requestsToConfirm","params":[],"id":1}"#;
 	let response = concat!(
 		r#"{"jsonrpc":"2.0","result":["#,
 		r#"{"id":"0x1","payload":{"sendTransaction":{"condition":null,"data":"0x","from":"0x0000000000000000000000000000000000000001","gas":"0x989680","gasPrice":"0x2710","nonce":null,"to":"0xd46e8dd67c5d32be8058bb8eb970870f07244567","value":"0x1"}}},"#,
-		r#"{"id":"0x2","payload":{"sign":{"address":"0x0000000000000000000000000000000000000001","hash":"0x0000000000000000000000000000000000000000000000000000000000000005"}}}"#,
+		r#"{"id":"0x2","payload":{"sign":{"address":"0x0000000000000000000000000000000000000001","data":"0x05"}}}"#,
 		r#"],"id":1}"#
 	);
 
@@ -156,7 +156,7 @@ fn should_not_remove_transaction_if_password_is_invalid() {
 fn should_not_remove_sign_if_password_is_invalid() {
 	// given
 	let tester = signer_tester();
-	tester.signer.add_request(ConfirmationPayload::Signature(0.into(), 5.into())).unwrap();
+	tester.signer.add_request(ConfirmationPayload::Signature(0.into(), vec![5].into())).unwrap();
 	assert_eq!(tester.signer.requests().len(), 1);
 
 	// when

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

@@ -201,7 +201,7 @@ fn should_sign_if_account_is_unlocked() {
 		],
 		"id": 1
 	}"#;
-	let response = r#"{"jsonrpc":"2.0","result":"0x1bb3062482b0687e9c97c7609ea60c1649959dbb334f71b3d5cacd496e0848ba8137bc765756627722389c6c39bc77700ccdc8916916a0eb03bcf5191d4f74dc65","id":1}"#;
+	let response = r#"{"jsonrpc":"2.0","result":"0x1bdb53b32e56cf3e9735377b7664d6de5a03e125b1bf8ec55715d253668b4238503b4ac931fe6af90add73e72a585e952665376b2b9afc5b6b239b7df74c734e12","id":1}"#;
 	assert_eq!(tester.io.handle_request_sync(&request), Some(response.to_owned()));
 	assert_eq!(tester.signer.requests().len(), 0);
 }

rpc/src/v1/traits/parity_signing.rs

@@ -18,7 +18,7 @@
 use jsonrpc_core::Error;
 use jsonrpc_macros::Ready;
 
-use v1::types::{U256, H160, H256, Bytes, ConfirmationResponse, TransactionRequest, Either};
+use v1::types::{U256, H160, Bytes, ConfirmationResponse, TransactionRequest, Either};
 
 build_rpc_trait! {
 	/// Signing methods implementation.
@@ -26,7 +26,7 @@ build_rpc_trait! {
 		/// Posts sign request asynchronously.
 		/// Will return a confirmation ID for later use with check_transaction.
 		#[rpc(name = "parity_postSign")]
-		fn post_sign(&self, H160, H256) -> Result<Either<U256, ConfirmationResponse>, Error>;
+		fn post_sign(&self, H160, Bytes) -> Result<Either<U256, ConfirmationResponse>, Error>;
 
 		/// Posts transaction asynchronously.
 		/// Will return a transaction ID for later use with check_transaction.

rpc/src/v1/types/confirmations.rs

@@ -19,6 +19,7 @@
 use std::fmt;
 use serde::{Serialize, Serializer};
 use util::log::Colour;
+use util::bytes::ToPretty;
 
 use v1::types::{U256, TransactionRequest, RichRawTransaction, H160, H256, H520, Bytes, TransactionCondition};
 use v1::helpers;
@@ -64,14 +65,14 @@ pub struct SignRequest {
 	/// Address
 	pub address: H160,
 	/// Hash to sign
-	pub hash: H256,
+	pub data: Bytes,
 }
 
-impl From<(H160, H256)> for SignRequest {
+impl From<(H160, Bytes)> for SignRequest {
-	fn from(tuple: (H160, H256)) -> Self {
+	fn from(tuple: (H160, Bytes)) -> Self {
 		SignRequest {
 			address: tuple.0,
-			hash: tuple.1,
+			data: tuple.1,
 		}
 	}
 }
@@ -80,8 +81,8 @@ impl fmt::Display for SignRequest {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
 		write!(
 			f,
-			"sign 0x{:?} with {}",
+			"sign 0x{} with {}",
-			self.hash,
+			self.data.0.pretty(),
 			Colour::White.bold().paint(format!("0x{:?}", self.address)),
 		)
 	}
@@ -172,9 +173,9 @@ 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::Signature(address, hash) => ConfirmationPayload::Signature(SignRequest {
+			helpers::ConfirmationPayload::Signature(address, data) => ConfirmationPayload::Signature(SignRequest {
 				address: address.into(),
-				hash: hash.into(),
+				data: data.into(),
 			}),
 			helpers::ConfirmationPayload::Decrypt(address, msg) => ConfirmationPayload::Decrypt(DecryptRequest {
 				address: address.into(),
@@ -247,12 +248,12 @@ mod tests {
 		// given
 		let request = helpers::ConfirmationRequest {
 			id: 15.into(),
-			payload: helpers::ConfirmationPayload::Signature(1.into(), 5.into()),
+			payload: helpers::ConfirmationPayload::Signature(1.into(), vec![5].into()),
 		};
 
 		// when
 		let res = serde_json::to_string(&ConfirmationRequest::from(request));
-		let expected = r#"{"id":"0xf","payload":{"sign":{"address":"0x0000000000000000000000000000000000000001","hash":"0x0000000000000000000000000000000000000000000000000000000000000005"}}}"#;
+		let expected = r#"{"id":"0xf","payload":{"sign":{"address":"0x0000000000000000000000000000000000000001","data":"0x05"}}}"#;
 
 		// then
 		assert_eq!(res.unwrap(), expected.to_owned());