From be7e16a4f84993db746981c8aa87e910891eb144 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Wed, 20 Nov 2019 17:08:27 +0200 Subject: [PATCH 1/4] Breaking change: `web3::api::Accounts` can now sign arbitrary signed data. Previously, it could only sign std::String and std::str (UTF-8) data. This reflects the functionality in the Go implementation of message signing: https://github.com/bas-vk/go-ethereum/blob/e61035c5a3630e4f6fd0fb3e5346a4eed8cedc80/internal/ethapi/api.go#L352 as well as what is available in the `ethereum-tx-sign` crate: https://docs.rs/ethereum-tx-sign/2.0.0/ethereum_tx_sign/struct.RawTransaction.html (note the `data` field is a Vec) --- src/api/accounts.rs | 19 ++++++++++++------- src/types/recovery.rs | 27 ++++++++++++++++++++------- src/types/signed.rs | 4 ++-- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/api/accounts.rs b/src/api/accounts.rs index 4252f2ed..b40cd101 100644 --- a/src/api/accounts.rs +++ b/src/api/accounts.rs @@ -54,12 +54,14 @@ impl Accounts { /// using keccak256. pub fn hash_message(&self, message: S) -> H256 where - S: AsRef, + S: AsRef<[u8]>, { let message = message.as_ref(); - let eth_message = format!("\x19Ethereum Signed Message:\n{}{}", message.len(), message); - eth_message.as_bytes().keccak256().into() + let mut eth_message = Vec::from(format!("\x19Ethereum Signed Message:\n{}", message.len()).as_bytes()); + eth_message.extend_from_slice(message); + + eth_message.keccak256().into() } /// Sign arbitrary string data. @@ -72,10 +74,10 @@ impl Accounts { /// such as `ethsign`. pub fn sign(&self, message: S, key: &SecretKey) -> Result where - S: AsRef, + S: AsRef<[u8]>, { - let message = message.as_ref().to_string(); - let message_hash = self.hash_message(&message); + let message = message.as_ref(); + let message_hash = self.hash_message(message); let signature = key.sign(&message_hash[..]).map_err(EthsignError::from)?; // convert to 'Electrum' notation @@ -89,6 +91,9 @@ impl Accounts { bytes }); + // We perform this allocation only after all previous fallible action have completed successfully. + let message = message.to_owned(); + Ok(SignedData { message, message_hash, @@ -109,7 +114,7 @@ impl Accounts { { let recovery = recovery.into(); let message_hash = match recovery.message { - RecoveryMessage::String(ref message) => self.hash_message(message), + RecoveryMessage::Bytes(ref message) => self.hash_message(message), RecoveryMessage::Hash(hash) => hash, }; let signature = recovery.as_signature(); diff --git a/src/types/recovery.rs b/src/types/recovery.rs index 1c5b3e8d..35a50a79 100644 --- a/src/types/recovery.rs +++ b/src/types/recovery.rs @@ -105,26 +105,39 @@ impl<'a> From<&'a EthtxSignedTransaction<'a>> for Recovery { /// Recovery message data. /// -/// The message data can either be a string message that is first hashed +/// The message data can either be a binary message that is first hashed /// according to EIP-191 and then recovered based on the signature or a /// precomputed hash. #[derive(Clone, Debug, PartialEq)] pub enum RecoveryMessage { - /// Message string - String(String), + /// Message bytes + Bytes(Vec), /// Message hash Hash(H256), } -impl<'a> From<&'a str> for RecoveryMessage { - fn from(s: &'a str) -> Self { +impl From<&[u8]> for RecoveryMessage { + fn from(s: &[u8]) -> Self { s.to_owned().into() } } + +impl From> for RecoveryMessage { + fn from(s: Vec) -> Self { + RecoveryMessage::Bytes(s) + } +} + +impl From<&str> for RecoveryMessage { + fn from(s: &str) -> Self { + s.as_bytes().to_owned().into() + } +} + impl From for RecoveryMessage { fn from(s: String) -> Self { - RecoveryMessage::String(s) + RecoveryMessage::Bytes(s.into_bytes()) } } @@ -170,7 +183,7 @@ mod tests { .unwrap(); let signed = SignedData { - message: message.to_string(), + message: message.as_bytes().to_owned(), message_hash: "1da44b586eb0729ff70a73c326926f6ed5a25f5b056e7f47fbc6e58d86871655" .parse() .unwrap(), diff --git a/src/types/signed.rs b/src/types/signed.rs index 2b63e565..9b3a03c0 100644 --- a/src/types/signed.rs +++ b/src/types/signed.rs @@ -4,8 +4,8 @@ use serde::{Deserialize, Serialize}; /// Struct representing signed data returned from `Accounts::sign` method. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] pub struct SignedData { - /// The original method that was signed. - pub message: String, + /// The original message that was signed. + pub message: Vec, /// The keccak256 hash of the signed data. #[serde(rename = "messageHash")] pub message_hash: H256, From 7f64d6014455d9ba1567d2b61e5beba13a8ef4b0 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Wed, 20 Nov 2019 17:13:42 +0200 Subject: [PATCH 2/4] fixed some typo --- src/api/accounts.rs | 2 +- src/types/signed.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/api/accounts.rs b/src/api/accounts.rs index b40cd101..77c4cf36 100644 --- a/src/api/accounts.rs +++ b/src/api/accounts.rs @@ -91,7 +91,7 @@ impl Accounts { bytes }); - // We perform this allocation only after all previous fallible action have completed successfully. + // We perform this allocation only after all previous fallible actions have completed successfully. let message = message.to_owned(); Ok(SignedData { diff --git a/src/types/signed.rs b/src/types/signed.rs index 9b3a03c0..87b30e91 100644 --- a/src/types/signed.rs +++ b/src/types/signed.rs @@ -22,7 +22,7 @@ pub struct SignedData { /// Transaction data for signing. /// /// The `Accounts::sign_transaction` method will fill optional fields with sane -/// defaults when they are ommited. Specifically the signing account's current +/// defaults when they are omitted. Specifically the signing account's current /// transaction count will be used for the `nonce`, the estimated recommended /// gas price will be used for `gas_price`, and the current network ID will be /// used for the `chain_id`. From fde7404b1a8b5b65d1616ca0d981c0be7c1c4820 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Wed, 20 Nov 2019 17:23:46 +0200 Subject: [PATCH 3/4] renamed RecoveryMessage::Bytes to RecoveryMessage::Data --- src/api/accounts.rs | 2 +- src/types/recovery.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/api/accounts.rs b/src/api/accounts.rs index 77c4cf36..581952bf 100644 --- a/src/api/accounts.rs +++ b/src/api/accounts.rs @@ -114,7 +114,7 @@ impl Accounts { { let recovery = recovery.into(); let message_hash = match recovery.message { - RecoveryMessage::Bytes(ref message) => self.hash_message(message), + RecoveryMessage::Data(ref message) => self.hash_message(message), RecoveryMessage::Hash(hash) => hash, }; let signature = recovery.as_signature(); diff --git a/src/types/recovery.rs b/src/types/recovery.rs index 35a50a79..de1d3d0b 100644 --- a/src/types/recovery.rs +++ b/src/types/recovery.rs @@ -111,7 +111,7 @@ impl<'a> From<&'a EthtxSignedTransaction<'a>> for Recovery { #[derive(Clone, Debug, PartialEq)] pub enum RecoveryMessage { /// Message bytes - Bytes(Vec), + Data(Vec), /// Message hash Hash(H256), } @@ -125,7 +125,7 @@ impl From<&[u8]> for RecoveryMessage { impl From> for RecoveryMessage { fn from(s: Vec) -> Self { - RecoveryMessage::Bytes(s) + RecoveryMessage::Data(s) } } @@ -137,7 +137,7 @@ impl From<&str> for RecoveryMessage { impl From for RecoveryMessage { fn from(s: String) -> Self { - RecoveryMessage::Bytes(s.into_bytes()) + RecoveryMessage::Data(s.into_bytes()) } } From 0fe4df67a8acce094da28cfe746ea20ef1a02384 Mon Sep 17 00:00:00 2001 From: Reuven Podmazo Date: Thu, 21 Nov 2019 15:39:16 +0200 Subject: [PATCH 4/4] formatted code according to rustfmt --- src/types/recovery.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/types/recovery.rs b/src/types/recovery.rs index de1d3d0b..2f1e9757 100644 --- a/src/types/recovery.rs +++ b/src/types/recovery.rs @@ -122,7 +122,6 @@ impl From<&[u8]> for RecoveryMessage { } } - impl From> for RecoveryMessage { fn from(s: Vec) -> Self { RecoveryMessage::Data(s)