From 6556dd585d8defc301a3f59a833e5e761ce0c48a Mon Sep 17 00:00:00 2001 From: hashmap Date: Fri, 24 Apr 2020 11:18:26 +0200 Subject: [PATCH] Pass byte slice to to_hex (#3307) Currently we pass a Vec. This requires an extra allocation and copy of all elements if a caller doesn't have a Vec already, which is at least 95% of cases. Another, a smaller issue, we have a function util::to_hex and some structs implement to_hex() on top of it, so we have a mix of it in the code. This PR introduces a trait and a blanket impl for AsRef<[u8]> which brings a uniform API (obj.to_hex()). One unfortunate case is arrays of size bigger than 32 - Rust doesn't implement AsRef for them so it requires an ugly hack (&array[..]).to_hex(). --- api/src/types.rs | 50 ++++++++++++++++------------- core/src/core/hash.rs | 11 ++----- core/src/core/id.rs | 13 ++++---- core/src/core/merkle_proof.rs | 4 +-- core/src/core/transaction.rs | 18 ++++++----- core/src/genesis.rs | 1 + core/src/libtx/secp_ser.rs | 24 +++++++------- core/src/pow/cuckatoo.rs | 4 +-- core/tests/block.rs | 5 ++- etc/gen_gen/src/bin/gen_gen.rs | 4 +-- keychain/src/mnemonic.rs | 4 +-- keychain/src/types.rs | 10 +----- servers/src/common/hooks.rs | 1 + servers/src/mining/stratumserver.rs | 4 +-- util/src/hex.rs | 31 ++++++++++++++---- 15 files changed, 98 insertions(+), 86 deletions(-) diff --git a/api/src/types.rs b/api/src/types.rs index 65d2699d6..37848f478 100644 --- a/api/src/types.rs +++ b/api/src/types.rs @@ -20,8 +20,8 @@ use crate::core::core::merkle_proof::MerkleProof; use crate::core::core::{KernelFeatures, TxKernel}; use crate::core::{core, ser}; use crate::p2p; -use crate::util; use crate::util::secp::pedersen; +use crate::util::{self, ToHex}; use serde; use serde::de::MapAccess; use serde::ser::SerializeStruct; @@ -61,8 +61,8 @@ impl Tip { pub fn from_tip(tip: chain::Tip) -> Tip { Tip { height: tip.height, - last_block_pushed: util::to_hex(tip.last_block_h.to_vec()), - prev_block_to_last: util::to_hex(tip.prev_block_h.to_vec()), + last_block_pushed: tip.last_block_h.to_hex(), + prev_block_to_last: tip.prev_block_h.to_hex(), total_difficulty: tip.total_difficulty.to_num(), } } @@ -142,9 +142,7 @@ impl TxHashSetNode { let mut return_vec = Vec::new(); let last_n = chain.get_last_n_output(distance); for x in last_n { - return_vec.push(TxHashSetNode { - hash: util::to_hex(x.0.to_vec()), - }); + return_vec.push(TxHashSetNode { hash: x.0.to_hex() }); } return_vec } @@ -154,7 +152,7 @@ impl TxHashSetNode { let last_n = head.get_last_n_rangeproof(distance); for elem in last_n { return_vec.push(TxHashSetNode { - hash: util::to_hex(elem.0.to_vec()), + hash: elem.0.to_hex(), }); } return_vec @@ -165,7 +163,7 @@ impl TxHashSetNode { let last_n = head.get_last_n_kernel(distance); for elem in last_n { return_vec.push(TxHashSetNode { - hash: util::to_hex(elem.0.to_vec()), + hash: elem.0.to_hex(), }); } return_vec @@ -213,12 +211,18 @@ impl PrintableCommitment { } } +impl AsRef<[u8]> for PrintableCommitment { + fn as_ref(&self) -> &[u8] { + &self.commit.0 + } +} + impl serde::ser::Serialize for PrintableCommitment { fn serialize(&self, serializer: S) -> Result where S: serde::ser::Serializer, { - serializer.serialize_str(&util::to_hex(self.to_vec())) + serializer.serialize_str(&self.to_hex()) } } @@ -297,7 +301,7 @@ impl OutputPrintable { }; let proof = if include_proof { - Some(util::to_hex(output.proof.proof.to_vec())) + Some(output.proof_bytes().to_hex()) } else { None }; @@ -320,7 +324,7 @@ impl OutputPrintable { commit: output.commit, spent, proof, - proof_hash: util::to_hex(output.proof.hash().to_vec()), + proof_hash: output.proof.hash().to_hex(), block_height, merkle_proof, mmr_index: output_pos, @@ -357,7 +361,7 @@ impl serde::ser::Serialize for OutputPrintable { { let mut state = serializer.serialize_struct("OutputPrintable", 7)?; state.serialize_field("output_type", &self.output_type)?; - state.serialize_field("commit", &util::to_hex(self.commit.0.to_vec()))?; + state.serialize_field("commit", &self.commit.to_hex())?; state.serialize_field("spent", &self.spent)?; state.serialize_field("proof", &self.proof)?; state.serialize_field("proof_hash", &self.proof_hash)?; @@ -512,8 +516,8 @@ impl TxKernelPrintable { features, fee, lock_height, - excess: util::to_hex(k.excess.0.to_vec()), - excess_sig: util::to_hex(k.excess_sig.to_raw_data().to_vec()), + excess: k.excess.to_hex(), + excess_sig: (&k.excess_sig.to_raw_data()[..]).to_hex(), } } } @@ -532,9 +536,9 @@ pub struct BlockHeaderInfo { impl BlockHeaderInfo { pub fn from_header(header: &core::BlockHeader) -> BlockHeaderInfo { BlockHeaderInfo { - hash: util::to_hex(header.hash().to_vec()), + hash: header.hash().to_hex(), height: header.height, - previous: util::to_hex(header.prev_hash.to_vec()), + previous: header.prev_hash.to_hex(), } } } @@ -576,15 +580,15 @@ pub struct BlockHeaderPrintable { impl BlockHeaderPrintable { pub fn from_header(header: &core::BlockHeader) -> BlockHeaderPrintable { BlockHeaderPrintable { - hash: util::to_hex(header.hash().to_vec()), + hash: header.hash().to_hex(), version: header.version.into(), height: header.height, - previous: util::to_hex(header.prev_hash.to_vec()), - prev_root: util::to_hex(header.prev_root.to_vec()), + previous: header.prev_hash.to_hex(), + prev_root: header.prev_root.to_hex(), timestamp: header.timestamp.to_rfc3339(), - output_root: util::to_hex(header.output_root.to_vec()), - range_proof_root: util::to_hex(header.range_proof_root.to_vec()), - kernel_root: util::to_hex(header.kernel_root.to_vec()), + output_root: header.output_root.to_hex(), + range_proof_root: header.range_proof_root.to_hex(), + kernel_root: header.kernel_root.to_hex(), nonce: header.pow.nonce, edge_bits: header.pow.edge_bits(), cuckoo_solution: header.pow.proof.nonces.clone(), @@ -618,7 +622,7 @@ impl BlockPrintable { let inputs = block .inputs() .iter() - .map(|x| util::to_hex(x.commitment().0.to_vec())) + .map(|x| x.commitment().to_hex()) .collect(); let outputs = block .outputs() diff --git a/core/src/core/hash.rs b/core/src/core/hash.rs index fd886c407..34c17fc55 100644 --- a/core/src/core/hash.rs +++ b/core/src/core/hash.rs @@ -20,10 +20,8 @@ use crate::ser::{self, Error, ProtocolVersion, Readable, Reader, Writeable, Writer}; use blake2::blake2b::Blake2b; use byteorder::{BigEndian, ByteOrder}; -use std::cmp::min; -use std::convert::AsRef; -use std::{fmt, ops}; -use util; +use std::{cmp::min, convert::AsRef, fmt, ops}; +use util::ToHex; /// A hash consisting of all zeroes, used as a sentinel. No known preimage. pub const ZERO_HASH: Hash = Hash([0; 32]); @@ -73,11 +71,6 @@ impl Hash { &self.0 } - /// Convert a hash to hex string format. - pub fn to_hex(&self) -> String { - util::to_hex(self.to_vec()) - } - /// Convert hex string back to hash. pub fn from_hex(hex: &str) -> Result { let bytes = util::from_hex(hex) diff --git a/core/src/core/id.rs b/core/src/core/id.rs index 1bc827530..af97709f4 100644 --- a/core/src/core/id.rs +++ b/core/src/core/id.rs @@ -19,7 +19,7 @@ use crate::ser::{self, Readable, Reader, Writeable, Writer}; use byteorder::{ByteOrder, LittleEndian}; use siphasher::sip::SipHasher24; use std::cmp::{min, Ordering}; -use util; +use util::ToHex; /// The size of a short id used to identify inputs|outputs|kernels (6 bytes) pub const SHORT_ID_SIZE: usize = 6; @@ -84,6 +84,12 @@ impl ::std::fmt::Debug for ShortId { } } +impl AsRef<[u8]> for ShortId { + fn as_ref(&self) -> &[u8] { + self.0.as_ref() + } +} + impl Readable for ShortId { fn read(reader: &mut dyn Reader) -> Result { let v = reader.read_fixed_bytes(SHORT_ID_SIZE)?; @@ -108,11 +114,6 @@ impl ShortId { ShortId(hash) } - /// Hex string representation of a short_id - pub fn to_hex(&self) -> String { - util::to_hex(self.0.to_vec()) - } - /// Reconstructs a switch commit hash from a hex string. pub fn from_hex(hex: &str) -> Result { let bytes = util::from_hex(hex) diff --git a/core/src/core/merkle_proof.rs b/core/src/core/merkle_proof.rs index 61d32852a..813a87d7f 100644 --- a/core/src/core/merkle_proof.rs +++ b/core/src/core/merkle_proof.rs @@ -18,7 +18,7 @@ use crate::core::hash::Hash; use crate::core::pmmr; use crate::ser; use crate::ser::{PMMRIndexHashable, Readable, Reader, Writeable, Writer}; -use util; +use util::ToHex; /// Merkle proof errors. #[derive(Clone, Debug, PartialEq)] @@ -79,7 +79,7 @@ impl MerkleProof { pub fn to_hex(&self) -> String { let mut vec = Vec::new(); ser::serialize_default(&mut vec, &self).expect("serialization failed"); - util::to_hex(vec) + vec.to_hex() } /// Convert hex string representation back to a Merkle proof instance diff --git a/core/src/core/transaction.rs b/core/src/core/transaction.rs index b7ee94cbe..2da44cb5a 100644 --- a/core/src/core/transaction.rs +++ b/core/src/core/transaction.rs @@ -30,11 +30,11 @@ use std::cmp::{max, min}; use std::convert::TryInto; use std::sync::Arc; use std::{error, fmt}; -use util; use util::secp; use util::secp::pedersen::{Commitment, RangeProof}; use util::static_secp_instance; use util::RwLock; +use util::ToHex; /// Various tx kernel variants. #[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] @@ -1469,6 +1469,11 @@ impl Output { self.proof } + /// Get range proof as byte slice + pub fn proof_bytes(&self) -> &[u8] { + &self.proof.proof[..] + } + /// Validates the range proof using the commitment pub fn verify_proof(&self) -> Result<(), Error> { let secp = static_secp_instance(); @@ -1523,14 +1528,11 @@ impl OutputIdentifier { commit: self.commit, } } +} - /// convert an output_identifier to hex string format. - pub fn to_hex(&self) -> String { - format!( - "{:b}{}", - self.features as u8, - util::to_hex(self.commit.0.to_vec()), - ) +impl ToHex for OutputIdentifier { + fn to_hex(&self) -> String { + format!("{:b}{}", self.features as u8, self.commit.to_hex()) } } diff --git a/core/src/genesis.rs b/core/src/genesis.rs index 3101f0c83..2dd6cb80a 100644 --- a/core/src/genesis.rs +++ b/core/src/genesis.rs @@ -273,6 +273,7 @@ mod test { use super::*; use crate::core::hash::Hashed; use crate::ser::{self, ProtocolVersion}; + use util::ToHex; #[test] fn floonet_genesis_hash() { diff --git a/core/src/libtx/secp_ser.rs b/core/src/libtx/secp_ser.rs index c5ce075d8..704f24e0b 100644 --- a/core/src/libtx/secp_ser.rs +++ b/core/src/libtx/secp_ser.rs @@ -17,13 +17,13 @@ use keychain::BlindingFactor; use serde::{Deserialize, Deserializer, Serializer}; use util::secp::pedersen::{Commitment, RangeProof}; -use util::{from_hex, to_hex}; +use util::{from_hex, ToHex}; /// Serializes a secp PublicKey to and from hex pub mod pubkey_serde { use serde::{Deserialize, Deserializer, Serializer}; use util::secp::key::PublicKey; - use util::{from_hex, static_secp_instance, to_hex}; + use util::{from_hex, static_secp_instance, ToHex}; /// pub fn serialize(key: &PublicKey, serializer: S) -> Result @@ -32,7 +32,7 @@ pub mod pubkey_serde { { let static_secp = static_secp_instance(); let static_secp = static_secp.lock(); - serializer.serialize_str(&to_hex(key.serialize_vec(&static_secp, true).to_vec())) + serializer.serialize_str(&key.serialize_vec(&static_secp, true).to_hex()) } /// @@ -56,7 +56,7 @@ pub mod pubkey_serde { pub mod option_sig_serde { use crate::serde::{Deserialize, Deserializer, Serializer}; use serde::de::Error; - use util::{from_hex, secp, static_secp_instance, to_hex}; + use util::{from_hex, secp, static_secp_instance, ToHex}; /// pub fn serialize(sig: &Option, serializer: S) -> Result @@ -67,7 +67,7 @@ pub mod option_sig_serde { let static_secp = static_secp.lock(); match sig { Some(sig) => { - serializer.serialize_str(&to_hex(sig.serialize_compact(&static_secp).to_vec())) + serializer.serialize_str(&(&sig.serialize_compact(&static_secp)[..]).to_hex()) } None => serializer.serialize_none(), } @@ -99,7 +99,7 @@ pub mod option_sig_serde { pub mod option_seckey_serde { use crate::serde::{Deserialize, Deserializer, Serializer}; use serde::de::Error; - use util::{from_hex, secp, static_secp_instance, to_hex}; + use util::{from_hex, secp, static_secp_instance, ToHex}; /// pub fn serialize( @@ -110,7 +110,7 @@ pub mod option_seckey_serde { S: Serializer, { match key { - Some(key) => serializer.serialize_str(&to_hex(key.0.to_vec())), + Some(key) => serializer.serialize_str(&key.0.to_hex()), None => serializer.serialize_none(), } } @@ -141,7 +141,7 @@ pub mod option_seckey_serde { pub mod sig_serde { use crate::serde::{Deserialize, Deserializer, Serializer}; use serde::de::Error; - use util::{from_hex, secp, static_secp_instance, to_hex}; + use util::{from_hex, secp, static_secp_instance, ToHex}; /// pub fn serialize(sig: &secp::Signature, serializer: S) -> Result @@ -150,7 +150,7 @@ pub mod sig_serde { { let static_secp = static_secp_instance(); let static_secp = static_secp.lock(); - serializer.serialize_str(&to_hex(sig.serialize_compact(&static_secp).to_vec())) + serializer.serialize_str(&(&sig.serialize_compact(&static_secp)[..]).to_hex()) } /// @@ -176,7 +176,7 @@ pub mod option_commitment_serde { use crate::serde::{Deserialize, Deserializer, Serializer}; use serde::de::Error; use util::secp::pedersen::Commitment; - use util::{from_hex, to_hex}; + use util::{from_hex, ToHex}; /// pub fn serialize(commit: &Option, serializer: S) -> Result @@ -184,7 +184,7 @@ pub mod option_commitment_serde { S: Serializer, { match commit { - Some(c) => serializer.serialize_str(&to_hex(c.0.to_vec())), + Some(c) => serializer.serialize_str(&c.to_hex()), None => serializer.serialize_none(), } } @@ -242,7 +242,7 @@ where T: AsRef<[u8]>, S: Serializer, { - serializer.serialize_str(&to_hex(bytes.as_ref().to_vec())) + serializer.serialize_str(&bytes.to_hex()) } /// Used to ensure u64s are serialised in json diff --git a/core/src/pow/cuckatoo.rs b/core/src/pow/cuckatoo.rs index a7c688b7a..d5611f2b3 100644 --- a/core/src/pow/cuckatoo.rs +++ b/core/src/pow/cuckatoo.rs @@ -19,7 +19,7 @@ use crate::pow::{PoWContext, Proof}; use byteorder::{BigEndian, WriteBytesExt}; use croaring::Bitmap; use std::mem; -use util; +use util::ToHex; struct Graph where @@ -224,7 +224,7 @@ where pub fn sipkey_hex(&self, index: usize) -> Result { let mut rdr = vec![]; rdr.write_u64::(self.params.siphash_keys[index])?; - Ok(util::to_hex(rdr)) + Ok(rdr.to_hex()) } /// Return number of bytes used by the graph diff --git a/core/tests/block.rs b/core/tests/block.rs index c7b567bc7..afd9aa66c 100644 --- a/core/tests/block.rs +++ b/core/tests/block.rs @@ -32,8 +32,7 @@ use grin_core as core; use grin_core::global::ChainTypes; use keychain::{BlindingFactor, ExtKeychain, Keychain}; use std::sync::Arc; -use util::secp; -use util::RwLock; +use util::{secp, RwLock, ToHex}; fn verifier_cache() -> Arc> { Arc::new(RwLock::new(LruVerifierCache::new())) @@ -576,7 +575,7 @@ fn validate_header_proof() { b.header.write_pre_pow(&mut writer).unwrap(); b.header.pow.write_pre_pow(&mut writer).unwrap(); } - let pre_pow = util::to_hex(header_buf); + let pre_pow = header_buf.to_hex(); let reconstructed = BlockHeader::from_pre_pow_and_proof( pre_pow, diff --git a/etc/gen_gen/src/bin/gen_gen.rs b/etc/gen_gen/src/bin/gen_gen.rs index 84753d164..fdbfd9711 100644 --- a/etc/gen_gen/src/bin/gen_gen.rs +++ b/etc/gen_gen/src/bin/gen_gen.rs @@ -29,7 +29,7 @@ use grin_chain as chain; use grin_core as core; use grin_miner_plugin as plugin; use grin_store as store; -use grin_util as util; +use grin_util::{self as util, ToHex}; use grin_wallet as wallet; use grin_core::core::hash::Hashed; @@ -216,7 +216,7 @@ fn update_genesis_rs(gen: &core::core::Block) { "excess".to_string(), format!( "Commitment::from_vec(util::from_hex({:x?}.to_string()).unwrap())", - util::to_hex(gen.kernels()[0].excess.0.to_vec()) + gen.kernels()[0].excess.to_hex()) ), )); replacements.push(( diff --git a/keychain/src/mnemonic.rs b/keychain/src/mnemonic.rs index 5cf1b5d3e..e1e2c852a 100644 --- a/keychain/src/mnemonic.rs +++ b/keychain/src/mnemonic.rs @@ -172,7 +172,7 @@ where #[cfg(test)] mod tests { use super::{from_entropy, to_entropy, to_seed}; - use crate::util::{from_hex, to_hex}; + use crate::util::{from_hex, ToHex}; use rand::{thread_rng, Rng}; struct Test<'a> { @@ -311,7 +311,7 @@ mod tests { let tests = tests(); for t in tests.iter() { assert_eq!( - to_hex(to_seed(t.mnemonic, "TREZOR").unwrap().to_vec()), + (&to_seed(t.mnemonic, "TREZOR").unwrap()[..]).to_hex(), t.seed.to_string() ); assert_eq!( diff --git a/keychain/src/types.rs b/keychain/src/types.rs index 8a5f20a71..947e7ddd9 100644 --- a/keychain/src/types.rs +++ b/keychain/src/types.rs @@ -27,12 +27,12 @@ use crate::blake2::blake2b::blake2b; use crate::extkey_bip32::{self, ChildNumber}; use serde::{de, ser}; //TODO: Convert errors to use ErrorKind -use crate::util; use crate::util::secp::constants::SECRET_KEY_SIZE; use crate::util::secp::key::{PublicKey, SecretKey}; use crate::util::secp::pedersen::Commitment; use crate::util::secp::{self, Message, Secp256k1, Signature}; use crate::util::static_secp_instance; +use crate::util::ToHex; use zeroize::Zeroize; use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt}; @@ -195,10 +195,6 @@ impl Identifier { Ok(Identifier::from_bytes(&bytes)) } - pub fn to_hex(&self) -> String { - util::to_hex(self.0.to_vec()) - } - pub fn to_bip_32_string(&self) -> String { let p = ExtKeychainPath::from_identifier(&self); let mut retval = String::from("m"); @@ -288,10 +284,6 @@ impl BlindingFactor { BlindingFactor::from_secret_key(secp::key::ZERO_KEY) } - pub fn to_hex(&self) -> String { - util::to_hex(self.0.to_vec()) - } - pub fn from_hex(hex: &str) -> Result { let bytes = util::from_hex(hex).unwrap(); Ok(BlindingFactor::from_slice(&bytes)) diff --git a/servers/src/common/hooks.rs b/servers/src/common/hooks.rs index a64ade459..bf01537ec 100644 --- a/servers/src/common/hooks.rs +++ b/servers/src/common/hooks.rs @@ -25,6 +25,7 @@ use crate::core::core; use crate::core::core::hash::Hashed; use crate::p2p::types::PeerAddr; use futures::TryFutureExt; +use grin_util::ToHex; use hyper::client::HttpConnector; use hyper::header::HeaderValue; use hyper::Client; diff --git a/servers/src/mining/stratumserver.rs b/servers/src/mining/stratumserver.rs index cec415ff1..e87883124 100644 --- a/servers/src/mining/stratumserver.rs +++ b/servers/src/mining/stratumserver.rs @@ -42,7 +42,7 @@ use crate::core::{pow, ser}; use crate::keychain; use crate::mining::mine_block; use crate::pool; -use crate::util; +use crate::util::ToHex; type Tx = mpsc::UnboundedSender; @@ -342,7 +342,7 @@ impl Handler { bh.write_pre_pow(&mut writer).unwrap(); bh.pow.write_pre_pow(&mut writer).unwrap(); } - let pre_pow = util::to_hex(header_buf); + let pre_pow = header_buf.to_hex(); let job_template = JobTemplate { height: bh.height, job_id: (self.current_state.read().current_block_versions.len() - 1) as u64, diff --git a/util/src/hex.rs b/util/src/hex.rs index 0e76086ea..6a5ff438b 100644 --- a/util/src/hex.rs +++ b/util/src/hex.rs @@ -19,14 +19,26 @@ use std::fmt::Write; /// Encode the provided bytes into a hex string -pub fn to_hex(bytes: Vec) -> String { - let mut s = String::new(); +fn to_hex(bytes: &[u8]) -> String { + let mut s = String::with_capacity(bytes.len() * 2); for byte in bytes { - write!(&mut s, "{:02x}", byte).expect("Unable to write"); + write!(&mut s, "{:02x}", byte).expect("Unable to write hex"); } s } +/// Convert to hex +pub trait ToHex { + /// convert to hex + fn to_hex(&self) -> String; +} + +impl> ToHex for T { + fn to_hex(&self) -> String { + to_hex(self.as_ref()) + } +} + /// Decode a hex string into bytes. pub fn from_hex(hex: &str) -> Result, String> { let hex = hex.trim().trim_start_matches("0x"); @@ -46,9 +58,16 @@ mod test { #[test] fn test_to_hex() { - assert_eq!(to_hex(vec![0, 0, 0, 0]), "00000000"); - assert_eq!(to_hex(vec![10, 11, 12, 13]), "0a0b0c0d"); - assert_eq!(to_hex(vec![0, 0, 0, 255]), "000000ff"); + assert_eq!(vec![0, 0, 0, 0].to_hex(), "00000000"); + assert_eq!(vec![10, 11, 12, 13].to_hex(), "0a0b0c0d"); + assert_eq!([0, 0, 0, 255].to_hex(), "000000ff"); + } + + #[test] + fn test_to_hex_trait() { + assert_eq!(vec![0, 0, 0, 0].to_hex(), "00000000"); + assert_eq!(vec![10, 11, 12, 13].to_hex(), "0a0b0c0d"); + assert_eq!([0, 0, 0, 255].to_hex(), "000000ff"); } #[test]