From 9c37ae5d65b980a97ff8b19c7efb45768c1c36a9 Mon Sep 17 00:00:00 2001 From: Eason Date: Mon, 28 Aug 2023 15:15:41 +0800 Subject: [PATCH 1/3] refactor: change the inner type of `Hex` --- protocol/src/types/primitive.rs | 115 +++++++++++--------------------- 1 file changed, 38 insertions(+), 77 deletions(-) diff --git a/protocol/src/types/primitive.rs b/protocol/src/types/primitive.rs index aa262373a..185644de4 100644 --- a/protocol/src/types/primitive.rs +++ b/protocol/src/types/primitive.rs @@ -1,21 +1,23 @@ -use std::cmp::Ordering; -use std::{fmt, str::FromStr}; - pub use ethereum_types::{ BigEndianHash, Bloom, Public, Secret, Signature, H128, H160, H256, H512, H520, H64, U128, U256, U512, U64, }; use zeroize::Zeroizing; +use std::cmp::Ordering; +use std::{fmt, str::FromStr}; + +use bytes::BytesMut; +use faster_hex::withpfx_lowercase; use ophelia::{PublicKey, UncompressedPublicKey}; use overlord::DurationConfig; use rlp_derive::{RlpDecodable, RlpEncodable}; -use serde::{de, Deserialize, Serialize}; +use serde::{de, ser, Deserialize, Serialize}; use common_crypto::Secp256k1PublicKey; use common_hasher::keccak256; -use crate::codec::{hex_decode, hex_encode, serialize_uint}; +use crate::codec::{deserialize_address, hex_decode, hex_encode, serialize_uint}; use crate::types::{BlockNumber, Bytes, TypesError}; use crate::{ProtocolError, ProtocolResult}; @@ -72,21 +74,19 @@ impl Hasher { } #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] -pub struct Hex(String); +pub struct Hex(Bytes); impl Hex { pub fn empty() -> Self { - Hex(String::from(HEX_PREFIX)) + Hex(Bytes::default()) } pub fn is_empty(&self) -> bool { - self.0.len() == 2 + self.0.is_empty() } pub fn encode>(src: T) -> Self { - let mut s = HEX_PREFIX.to_string(); - s.push_str(&hex_encode(src)); - Hex(s) + Hex(BytesMut::from(src.as_ref()).freeze()) } pub fn decode(s: String) -> ProtocolResult { @@ -106,20 +106,19 @@ impl Hex { HEX_PREFIX.to_string() + &s }; - let _ = hex_decode(&s[2..])?; - Ok(Hex(s)) + Ok(Hex(hex_decode(&s[2..])?.into())) } pub fn as_string(&self) -> String { - self.0.to_owned() + HEX_PREFIX.to_string() + &hex_encode(self.0.as_ref()) } pub fn as_string_trim0x(&self) -> String { - (self.0[2..]).to_owned() + hex_encode(self.0.as_ref()) } pub fn as_bytes(&self) -> Bytes { - Bytes::from(hex_decode(&self.0[2..]).expect("impossible, already checked in from_string")) + self.0.clone() } fn is_prefixed(s: &str) -> bool { @@ -129,40 +128,16 @@ impl Hex { impl Default for Hex { fn default() -> Self { - Hex(String::from("0x0000000000000000")) + Hex(vec![0u8; 8].into()) } } impl Serialize for Hex { fn serialize(&self, serializer: S) -> Result where - S: serde::ser::Serializer, - { - serializer.serialize_str(&self.0) - } -} - -struct HexVisitor; - -impl<'de> de::Visitor<'de> for HexVisitor { - type Value = Hex; - - fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - formatter.write_str("Expect a hex string") - } - - fn visit_string(self, v: String) -> Result - where - E: de::Error, - { - Hex::from_string(v).map_err(|e| de::Error::custom(e.to_string())) - } - - fn visit_str(self, v: &str) -> Result - where - E: de::Error, + S: ser::Serializer, { - Hex::from_string(v.to_owned()).map_err(|e| de::Error::custom(e.to_string())) + withpfx_lowercase::serialize(&self.0, serializer) } } @@ -171,7 +146,7 @@ impl<'de> Deserialize<'de> for Hex { where D: de::Deserializer<'de>, { - deserializer.deserialize_string(HexVisitor) + Ok(Hex(withpfx_lowercase::deserialize(deserializer)?)) } } @@ -197,42 +172,18 @@ impl Serialize for Address { where S: serde::ser::Serializer, { - serializer.serialize_bytes(self.0.as_bytes()) + serializer.serialize_str(&self.eip55()) } } -// struct AddressVisitor; - -// impl<'de> de::Visitor<'de> for AddressVisitor { -// type Value = Address; - -// fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result { -// formatter.write_str("Expect a bech32 string") -// } - -// fn visit_string(self, v: String) -> Result -// where -// E: de::Error, -// { -// Address::from_str(&v).map_err(|e| de::Error::custom(e.to_string())) -// } - -// fn visit_str(self, v: &str) -> Result -// where -// E: de::Error, -// { -// Address::from_str(&v).map_err(|e| de::Error::custom(e.to_string())) -// } -// } - -// impl<'de> Deserialize<'de> for Address { -// fn deserialize(deserializer: D) -> Result -// where -// D: de::Deserializer<'de>, -// { -// deserializer.deserialize_string(AddressVisitor) -// } -// } +impl<'de> Deserialize<'de> for Address { + fn deserialize(deserializer: D) -> Result + where + D: de::Deserializer<'de>, + { + Ok(Address(deserialize_address(deserializer)?)) + } +} impl Address { pub fn from_pubkey_bytes>(bytes: B) -> ProtocolResult { @@ -545,6 +496,16 @@ mod tests { let _ = Hex::decode(hex).unwrap(); } + #[test] + fn test_hex_codec() { + let data = H256::random(); + let hex = Hex::encode(data.0); + assert_eq!( + serde_json::to_string(&hex).unwrap(), + serde_json::to_string(&data).unwrap() + ); + } + #[test] fn test_default_values() { let bytes = Hex::empty(); From 0526fd757d7634a94f39250a5eed85d6648dca1d Mon Sep 17 00:00:00 2001 From: Eason Date: Tue, 29 Aug 2023 10:29:59 +0800 Subject: [PATCH 2/3] follow review opinion --- core/api/src/jsonrpc/impl/node.rs | 6 +-- core/api/src/jsonrpc/web3_types.rs | 8 ++-- core/consensus/src/tests/mod.rs | 5 +-- core/consensus/src/util.rs | 6 +-- protocol/src/codec/mod.rs | 9 ++-- protocol/src/types/block.rs | 9 ++-- protocol/src/types/primitive.rs | 69 ++++++++++++++---------------- 7 files changed, 54 insertions(+), 58 deletions(-) diff --git a/core/api/src/jsonrpc/impl/node.rs b/core/api/src/jsonrpc/impl/node.rs index 81f76a792..a48adc850 100644 --- a/core/api/src/jsonrpc/impl/node.rs +++ b/core/api/src/jsonrpc/impl/node.rs @@ -2,8 +2,6 @@ use std::path::PathBuf; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; -use jsonrpsee::core::Error; - use core_consensus::SYNC_STATUS; use protocol::lazy::CHAIN_ID; use protocol::types::{Hash, Hasher, Hex, H160, H256, U256}; @@ -116,9 +114,7 @@ impl AxonNodeRpcServer for NodeRpcImpl { } fn sha3(&self, data: Hex) -> RpcResult { - let decode_data = - Hex::decode(data.as_string()).map_err(|e| Error::Custom(e.to_string()))?; - Ok(Hasher::digest(decode_data.as_ref())) + Ok(Hasher::digest(data.as_ref())) } } diff --git a/core/api/src/jsonrpc/web3_types.rs b/core/api/src/jsonrpc/web3_types.rs index 07b9898ab..b69d363e4 100644 --- a/core/api/src/jsonrpc/web3_types.rs +++ b/core/api/src/jsonrpc/web3_types.rs @@ -1,4 +1,4 @@ -use std::fmt; +use std::{fmt, str::FromStr}; use either::Either; use serde::de::{Error, MapAccess, Visitor}; @@ -455,13 +455,13 @@ impl<'a> Visitor<'a> for BlockIdVisitor { } "blockHash" => { let value: String = visitor.next_value()?; - let raw_value = Hex::decode(value.clone()) + let raw_value = Hex::from_str(&value) .map_err(|e| Error::custom(format!("Invalid hex code: {}", e)))?; if raw_value.len() != 32 { return Err(Error::custom(format!("Invalid block hash: {}", value))); } else { let mut v = [0u8; 32]; - v.copy_from_slice(&raw_value); + v.copy_from_slice(raw_value.as_ref()); block_hash = Some(v.into()); break; } @@ -899,7 +899,7 @@ mod tests { #[test] fn test_web3_transaction_json() { // https://etherscan.io/getRawTx?tx=0x07c7388b03ab8403deeaefc551efbc632f8531f04dc9993a274dbba9bbb98cbf - let tx = Hex::decode("0x02f902f801728405f5e1008509898edcf78302ffb8943fc91a3afd70395cd496c647d5a6cc9d4b2b7fad8802c68af0bb140000b902843593564c000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000006480c64700000000000000000000000000000000000000000000000000000000000000020b080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000002c68af0bb1400000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000002c68af0bb1400000000000000000000000000000000000000000004a715ce36374beaa635218d9700000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000c3681a720605bd6f8fe9a2fabff6a7cdecdc605dc080a0d253ee687ab2d9734a5073d64a0ba26bc3bc1cf4582005137bba05ef88616ea89e8ba79925267b17403fdf3ab47641b4aa52322dc385429cc92a7003c5d7c2".into()).unwrap(); + let tx = Hex::from_str("0x02f902f801728405f5e1008509898edcf78302ffb8943fc91a3afd70395cd496c647d5a6cc9d4b2b7fad8802c68af0bb140000b902843593564c000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000006480c64700000000000000000000000000000000000000000000000000000000000000020b080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000002c68af0bb1400000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000002c68af0bb1400000000000000000000000000000000000000000004a715ce36374beaa635218d9700000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2000000000000000000000000c3681a720605bd6f8fe9a2fabff6a7cdecdc605dc080a0d253ee687ab2d9734a5073d64a0ba26bc3bc1cf4582005137bba05ef88616ea89e8ba79925267b17403fdf3ab47641b4aa52322dc385429cc92a7003c5d7c2").unwrap(); let tx = UnverifiedTransaction::decode(tx).unwrap(); let tx = SignedTransaction::from_unverified(tx, None).unwrap(); let tx_json = serde_json::to_value(Web3Transaction::from(tx)).unwrap(); diff --git a/core/consensus/src/tests/mod.rs b/core/consensus/src/tests/mod.rs index bca3bf3b1..fd3aea22a 100644 --- a/core/consensus/src/tests/mod.rs +++ b/core/consensus/src/tests/mod.rs @@ -90,10 +90,7 @@ fn _get_random_bytes(len: usize) -> Bytes { } fn _mock_pub_key() -> Hex { - Hex::from_string( - "0x026c184a9016f6f71a234c86b141621f38b68c78602ab06768db4d83682c616004".to_owned(), - ) - .unwrap() + Hex::from_str("0x026c184a9016f6f71a234c86b141621f38b68c78602ab06768db4d83682c616004").unwrap() } fn _mock_validators(len: usize) -> Vec { diff --git a/core/consensus/src/util.rs b/core/consensus/src/util.rs index 1a4bdcedb..9364b2d0c 100644 --- a/core/consensus/src/util.rs +++ b/core/consensus/src/util.rs @@ -189,6 +189,8 @@ pub fn convert_hex_to_bls_pubkeys(hex: Hex) -> ProtocolResult { #[cfg(test)] mod tests { + use std::str::FromStr; + use super::*; use protocol::codec::hex_decode; @@ -253,8 +255,6 @@ mod tests { #[test] fn test_convert_from_hex() { let hex_str = "0xa694f4e48a5a173b61731998f8f1204342dc5c8eb1e32cdae37415c20d11ae035ddac4a39f105e9c2d4d3691024d385d"; - assert!( - convert_hex_to_bls_pubkeys(Hex::from_string(String::from(hex_str)).unwrap()).is_ok() - ); + assert!(convert_hex_to_bls_pubkeys(Hex::from_str(hex_str).unwrap()).is_ok()); } } diff --git a/protocol/src/codec/mod.rs b/protocol/src/codec/mod.rs index b9ca08462..d1d3562b8 100644 --- a/protocol/src/codec/mod.rs +++ b/protocol/src/codec/mod.rs @@ -4,6 +4,8 @@ pub mod executor; pub mod receipt; pub mod transaction; +use std::str::FromStr; + pub use transaction::truncate_slice; use ethers_core::utils::parse_checksummed; @@ -57,13 +59,14 @@ impl Decodable for Address { impl Encodable for Hex { fn rlp_append(&self, s: &mut RlpStream) { - s.begin_list(1).append(&self.as_string_trim0x()); + s.begin_list(1).append(&self.as_string()); } } impl Decodable for Hex { fn decode(r: &Rlp) -> Result { - Hex::from_string(r.val_at(0)?).map_err(|_| DecoderError::Custom("hex check")) + let s: String = r.val_at(0)?; + Hex::from_str(s.as_str()).map_err(|_| DecoderError::Custom("hex check")) } } @@ -188,7 +191,7 @@ mod tests { impl Hex { fn random() -> Self { let data = (0..128).map(|_| random()).collect::>(); - Self::from_string(hex_encode(data)).unwrap() + Hex::encode(data) } } diff --git a/protocol/src/types/block.rs b/protocol/src/types/block.rs index da28030c3..e0725fcd5 100644 --- a/protocol/src/types/block.rs +++ b/protocol/src/types/block.rs @@ -272,7 +272,10 @@ mod tests { Block, BlockVersion, ConsensusConfig, Header, Hex, Metadata, MetadataVersion, ProposeCount, RichBlock, ValidatorExtend, H160, }; - use std::time::{SystemTime, UNIX_EPOCH}; + use std::{ + str::FromStr, + time::{SystemTime, UNIX_EPOCH}, + }; pub fn time_now() -> u64 { SystemTime::now() @@ -328,8 +331,8 @@ mod tests { version: MetadataVersion::new(0, 1000000000), epoch: 0, verifier_list: vec![ValidatorExtend { - bls_pub_key: Hex::from_string("0x04102947214862a503c73904deb5818298a186d68c7907bb609583192a7de6331493835e5b8281f4d9ee705537c0e765580e06f86ddce5867812fceb42eecefd209f0eddd0389d6b7b0100f00fb119ef9ab23826c6ea09aadcc76fa6cea6a32724".to_string()).unwrap(), - pub_key: Hex::from_string("0x02ef0cb0d7bc6c18b4bea1f5908d9106522b35ab3c399369605d4242525bda7e60".to_string()).unwrap(), + bls_pub_key: Hex::from_str("0x04102947214862a503c73904deb5818298a186d68c7907bb609583192a7de6331493835e5b8281f4d9ee705537c0e765580e06f86ddce5867812fceb42eecefd209f0eddd0389d6b7b0100f00fb119ef9ab23826c6ea09aadcc76fa6cea6a32724").unwrap(), + pub_key: Hex::from_str("0x02ef0cb0d7bc6c18b4bea1f5908d9106522b35ab3c399369605d4242525bda7e60").unwrap(), address: H160::default(), propose_weight: 1, vote_weight: 1, diff --git a/protocol/src/types/primitive.rs b/protocol/src/types/primitive.rs index 185644de4..99fed9a14 100644 --- a/protocol/src/types/primitive.rs +++ b/protocol/src/types/primitive.rs @@ -7,7 +7,6 @@ use zeroize::Zeroizing; use std::cmp::Ordering; use std::{fmt, str::FromStr}; -use bytes::BytesMut; use faster_hex::withpfx_lowercase; use ophelia::{PublicKey, UncompressedPublicKey}; use overlord::DurationConfig; @@ -18,7 +17,7 @@ use common_crypto::Secp256k1PublicKey; use common_hasher::keccak256; use crate::codec::{deserialize_address, hex_decode, hex_encode, serialize_uint}; -use crate::types::{BlockNumber, Bytes, TypesError}; +use crate::types::{BlockNumber, Bytes, BytesMut, TypesError}; use crate::{ProtocolError, ProtocolResult}; pub type Hash = H256; @@ -26,7 +25,6 @@ pub type MerkleRoot = Hash; const ADDRESS_LEN: usize = 20; const HEX_PREFIX: &str = "0x"; -const HEX_PREFIX_UPPER: &str = "0X"; pub const NIL_DATA: H256 = H256([ 0xc5, 0xd2, 0x46, 0x01, 0x86, 0xf7, 0x23, 0x3c, 0x92, 0x7e, 0x7d, 0xb2, 0xdc, 0xc7, 0x03, 0xc0, @@ -85,28 +83,12 @@ impl Hex { self.0.is_empty() } - pub fn encode>(src: T) -> Self { - Hex(BytesMut::from(src.as_ref()).freeze()) - } - - pub fn decode(s: String) -> ProtocolResult { - let s = if Self::is_prefixed(s.as_str()) { - &s[2..] - } else { - s.as_str() - }; - - Ok(Bytes::from(hex_decode(s)?)) + pub fn len(&self) -> usize { + self.0.len() } - pub fn from_string(s: String) -> ProtocolResult { - let s = if Self::is_prefixed(s.as_str()) { - s - } else { - HEX_PREFIX.to_string() + &s - }; - - Ok(Hex(hex_decode(&s[2..])?.into())) + pub fn encode>(src: T) -> Self { + Hex(BytesMut::from(src.as_ref()).freeze()) } pub fn as_string(&self) -> String { @@ -122,7 +104,7 @@ impl Hex { } fn is_prefixed(s: &str) -> bool { - s.starts_with(HEX_PREFIX) || s.starts_with(HEX_PREFIX_UPPER) + s.starts_with(HEX_PREFIX) } } @@ -132,6 +114,30 @@ impl Default for Hex { } } +impl AsRef<[u8]> for Hex { + fn as_ref(&self) -> &[u8] { + &self.0 + } +} + +impl FromStr for Hex { + type Err = ProtocolError; + + fn from_str(s: &str) -> Result { + if !Self::is_prefixed(s) { + return Err(TypesError::HexPrefix.into()); + } + + Ok(Hex(hex_decode(&s[2..])?.into())) + } +} + +impl From for Bytes { + fn from(bytes: Hex) -> Self { + bytes.0 + } +} + impl Serialize for Hex { fn serialize(&self, serializer: S) -> Result where @@ -480,20 +486,11 @@ mod tests { #[test] fn test_hex_decode() { - let hex = String::from("0x"); - let res = Hex::from_string(hex.clone()).unwrap(); - assert!(res.is_empty()); - - let res = Hex::decode(hex).unwrap(); + let res = Hex::from_str("0x").unwrap(); assert!(res.is_empty()); - let hex = String::from("123456"); - let _ = Hex::from_string(hex.clone()).unwrap(); - let _ = Hex::decode(hex).unwrap(); - - let hex = String::from("0x123f"); - let _ = Hex::from_string(hex.clone()).unwrap(); - let _ = Hex::decode(hex).unwrap(); + assert!(Hex::from_str("123456").is_ok()); + assert!(Hex::from_str("0x123f").is_ok()); } #[test] From 010f6a4094b49eb3191db88d6b5c7f17c84b15c6 Mon Sep 17 00:00:00 2001 From: Eason Date: Tue, 29 Aug 2023 11:31:18 +0800 Subject: [PATCH 3/3] fix unit test --- protocol/src/codec/mod.rs | 5 +++-- protocol/src/types/primitive.rs | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/protocol/src/codec/mod.rs b/protocol/src/codec/mod.rs index d1d3562b8..e312d9db7 100644 --- a/protocol/src/codec/mod.rs +++ b/protocol/src/codec/mod.rs @@ -12,7 +12,7 @@ use ethers_core::utils::parse_checksummed; use rlp::{Decodable, DecoderError, Encodable, Rlp, RlpStream}; use serde::{Deserialize as _, Deserializer, Serializer}; -use crate::types::{Address, Bytes, DBBytes, Hex, Key256Bits, TypesError, H160, U256}; +use crate::types::{Address, Bytes, DBBytes, Hex, Key256Bits, TypesError, H160, U256, HEX_PREFIX}; use crate::ProtocolResult; static CHARS: &[u8] = b"0123456789abcdef"; @@ -59,13 +59,14 @@ impl Decodable for Address { impl Encodable for Hex { fn rlp_append(&self, s: &mut RlpStream) { - s.begin_list(1).append(&self.as_string()); + s.begin_list(1).append(&self.as_string_trim0x()); } } impl Decodable for Hex { fn decode(r: &Rlp) -> Result { let s: String = r.val_at(0)?; + let s = HEX_PREFIX.to_string() + &s; Hex::from_str(s.as_str()).map_err(|_| DecoderError::Custom("hex check")) } } diff --git a/protocol/src/types/primitive.rs b/protocol/src/types/primitive.rs index 99fed9a14..31b15b5ff 100644 --- a/protocol/src/types/primitive.rs +++ b/protocol/src/types/primitive.rs @@ -24,7 +24,7 @@ pub type Hash = H256; pub type MerkleRoot = Hash; const ADDRESS_LEN: usize = 20; -const HEX_PREFIX: &str = "0x"; +pub(crate) const HEX_PREFIX: &str = "0x"; pub const NIL_DATA: H256 = H256([ 0xc5, 0xd2, 0x46, 0x01, 0x86, 0xf7, 0x23, 0x3c, 0x92, 0x7e, 0x7d, 0xb2, 0xdc, 0xc7, 0x03, 0xc0, @@ -489,7 +489,7 @@ mod tests { let res = Hex::from_str("0x").unwrap(); assert!(res.is_empty()); - assert!(Hex::from_str("123456").is_ok()); + assert!(Hex::from_str("123456").is_err()); assert!(Hex::from_str("0x123f").is_ok()); }