Merge pull request #5597 from paritytech/ethsign

Fix eth_sign signature encoding.

ethkey/src/signature.rs

@@ -45,27 +45,23 @@ impl Signature {
 		self.0[64]
 	}
 
-	/// Encode the signature into VRS array (V altered to be in "Electrum" notation).
-	pub fn into_vrs(self) -> [u8; 65] {
-		let mut vrs = [0u8; 65];
-		vrs[0] = self.v() + 27;
-		vrs[1..33].copy_from_slice(self.r());
-		vrs[33..65].copy_from_slice(self.s());
-		vrs
+	/// Encode the signature into RSV array (V altered to be in "Electrum" notation).
+	pub fn into_electrum(mut self) -> [u8; 65] {
+		self.0[64] += 27;
+		self.0
 	}
 
-	/// Parse bytes as a signature encoded as VRS (V in "Electrum" notation).
+	/// Parse bytes as a signature encoded as RSV (V in "Electrum" notation).
 	/// May return empty (invalid) signature if given data has invalid length.
-	pub fn from_vrs(data: &[u8]) -> Self {
+	pub fn from_electrum(data: &[u8]) -> Self {
 		if data.len() != 65 || data[0] < 27 {
 			// fallback to empty (invalid) signature
 			return Signature::default();
 		}
 
 		let mut sig = [0u8; 65];
-		sig[0..32].copy_from_slice(&data[1..33]);
-		sig[32..64].copy_from_slice(&data[33..65]);
-		sig[64] = data[0] - 27;
+		sig.copy_from_slice(data);
+		sig[64] -= 27;
 		Signature(sig)
 	}
 
@@ -255,8 +251,8 @@ mod tests {
 		let signature = sign(keypair.secret(), &message).unwrap();
 
 		// when
-		let vrs = signature.clone().into_vrs();
-		let from_vrs = Signature::from_vrs(&vrs);
+		let vrs = signature.clone().into_electrum();
+		let from_vrs = Signature::from_electrum(&vrs);
 
 		// then
 		assert_eq!(signature, from_vrs);

rpc/src/v1/helpers/dispatch.rs

@@ -511,7 +511,7 @@ pub fn execute<D: Dispatcher + 'static>(
 			let hash = eth_data_hash(data);
 			let res = signature(&accounts, address, hash, pass)
 				.map(|result| result
-					.map(|rsv| H520(rsv.into_vrs()))
+					.map(|rsv| H520(rsv.into_electrum()))
 					.map(RpcH520::from)
 					.map(ConfirmationResponse::Signature)
 				);

rpc/src/v1/impls/signer.rs

@@ -194,7 +194,7 @@ impl<D: Dispatcher + 'static> Signer for SignerClient<D> {
 				},
 				ConfirmationPayload::EthSignMessage(address, data) => {
 					let expected_hash = eth_data_hash(data);
-					let signature = ethkey::Signature::from_vrs(&bytes.0);
+					let signature = ethkey::Signature::from_electrum(&bytes.0);
 					match ethkey::verify_address(&address, &signature, &expected_hash) {
 						Ok(true) => Ok(ConfirmationResponse::Signature(bytes.0.as_slice().into())),
 						Ok(false) => Err(errors::invalid_params("Sender address does not match the signature.", ())),

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

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

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

@@ -492,7 +492,7 @@ fn should_confirm_data_sign_with_signature() {
 	assert_eq!(tester.signer.requests().len(), 1);
 
 	let data_hash = eth_data_hash(vec![1, 2, 3, 4].into());
-	let signature = H520(tester.accounts.sign(address, Some("test".into()), data_hash).unwrap().into_vrs());
+	let signature = H520(tester.accounts.sign(address, Some("test".into()), data_hash).unwrap().into_electrum());
 	let signature = format!("0x{:?}", signature);
 
 	// when

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

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