From d06ad173590b827f8d6b4905fb8e70947214b00f Mon Sep 17 00:00:00 2001 From: Anton Yemelyanov Date: Wed, 26 Jul 2023 00:33:39 +0300 Subject: [PATCH 1/4] ScriptPubKey human-readable ser/deser (for json) --- Cargo.lock | 1 + consensus/core/Cargo.toml | 1 + consensus/core/src/tx.rs | 68 ++++++++++++++++++++++++++++++++------- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 17597a482..74e49bc72 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1759,6 +1759,7 @@ dependencies = [ "secp256k1", "serde", "serde_bytes", + "serde_json", "smallvec", "thiserror", ] diff --git a/consensus/core/Cargo.toml b/consensus/core/Cargo.toml index 10f7add3b..a9f069912 100644 --- a/consensus/core/Cargo.toml +++ b/consensus/core/Cargo.toml @@ -25,6 +25,7 @@ itertools.workspace = true cfg-if.workspace = true getrandom.workspace = true serde_bytes.workspace = true +serde_json.workspace = true secp256k1 = { version = "0.24", features = ["global-context", "rand-std"] } diff --git a/consensus/core/src/tx.rs b/consensus/core/src/tx.rs index 5ee8f06b0..f5827af70 100644 --- a/consensus/core/src/tx.rs +++ b/consensus/core/src/tx.rs @@ -1,7 +1,12 @@ use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; -use serde::{Deserialize, Deserializer, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, Serializer}; use smallvec::SmallVec; -use std::{collections::HashSet, fmt::Display, ops::Range}; +use std::{ + collections::HashSet, + fmt::Display, + ops::Range, + str::{self, FromStr}, +}; use crate::{ hashing, @@ -30,27 +35,57 @@ pub use smallvec::smallvec as scriptvec; pub type ScriptPublicKeys = HashSet; /// Represents a Kaspad ScriptPublicKey -#[derive(Default, Debug, PartialEq, Eq, Serialize, Clone, Hash)] -#[serde(rename_all = "camelCase")] +#[derive(Default, Debug, PartialEq, Eq, Clone, Hash)] pub struct ScriptPublicKey { version: ScriptPublicKeyVersion, script: ScriptVec, // Kept private to preserve read-only semantics } +#[derive(Default, Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Hash)] +#[serde(rename_all = "camelCase")] +struct ScriptPublicKeyInternal<'a> { + version: ScriptPublicKeyVersion, + script: &'a [u8], +} + +impl Serialize for ScriptPublicKey { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + if serializer.is_human_readable() { + let mut hex = [0u8; SCRIPT_VECTOR_SIZE * 2 + 4]; + faster_hex::hex_encode(&self.version.to_be_bytes(), &mut hex).expect("Unable to serialize ScriptPubKey version"); + faster_hex::hex_encode(&self.script, &mut hex[4..]).expect("Unable to serialize ScriptPubKey script"); + serializer.serialize_str(str::from_utf8(&hex).expect("hex is must be a valid UTF-8")) + } else { + ScriptPublicKeyInternal { version: self.version, script: &self.script }.serialize(serializer) + } + } +} + impl<'de: 'a, 'a> Deserialize<'de> for ScriptPublicKey { fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, { - #[derive(Default, Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Hash)] - #[serde(rename_all = "camelCase")] - struct ScriptPublicKeyInternal<'a> { - version: ScriptPublicKeyVersion, - script: &'a [u8], + if deserializer.is_human_readable() { + let s = ::deserialize(deserializer)?; + FromStr::from_str(&s).map_err(serde::de::Error::custom) + } else { + ScriptPublicKeyInternal::deserialize(deserializer) + .map(|ScriptPublicKeyInternal { script, version }| Self { version, script: SmallVec::from_slice(script) }) } + } +} - ScriptPublicKeyInternal::deserialize(deserializer) - .map(|ScriptPublicKeyInternal { script, version }| Self { version, script: SmallVec::from_slice(script) }) +impl FromStr for ScriptPublicKey { + type Err = faster_hex::Error; + fn from_str(hash_str: &str) -> Result { + let mut bytes = [0u8; SCRIPT_VECTOR_SIZE + 2]; + faster_hex::hex_decode(hash_str.as_bytes(), &mut bytes)?; + let version = u16::from_be_bytes(bytes[0..2].try_into().unwrap()); + Ok(Self { version, script: SmallVec::from_slice(&bytes[2..]) }) } } @@ -457,6 +492,17 @@ pub type SignableTransaction = MutableTransaction; mod tests { use super::*; + #[test] + fn test_spk_serde_json() { + let vec = (0..SCRIPT_VECTOR_SIZE as u8).collect::>(); + let spk = ScriptPublicKey::from_vec(0xc0de, vec.clone()); + let hex = serde_json::to_string(&spk).unwrap(); + assert_eq!("\"c0de000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20212223\"", hex); + let spk = serde_json::from_str::(&hex).unwrap(); + assert_eq!(spk.version, 0xc0de); + assert_eq!(spk.script.as_slice(), vec.as_slice()); + } + #[test] fn test_spk_borsh() { // Tests for ScriptPublicKey Borsh ser/deser since we manually implemented them From 744d4af5b1fac1c806fac6556c1a95ca622b733d Mon Sep 17 00:00:00 2001 From: Anton Yemelyanov Date: Wed, 26 Jul 2023 14:43:41 +0300 Subject: [PATCH 2/4] switch to from_utf8_unchecked() + serde rename ScriptPublicKey --- consensus/core/src/tx.rs | 7 ++++--- crypto/hashes/src/lib.rs | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/consensus/core/src/tx.rs b/consensus/core/src/tx.rs index f5827af70..d6e688f8e 100644 --- a/consensus/core/src/tx.rs +++ b/consensus/core/src/tx.rs @@ -43,6 +43,7 @@ pub struct ScriptPublicKey { #[derive(Default, Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Hash)] #[serde(rename_all = "camelCase")] +#[serde(rename = "ScriptPublicKey")] struct ScriptPublicKeyInternal<'a> { version: ScriptPublicKeyVersion, script: &'a [u8], @@ -57,7 +58,7 @@ impl Serialize for ScriptPublicKey { let mut hex = [0u8; SCRIPT_VECTOR_SIZE * 2 + 4]; faster_hex::hex_encode(&self.version.to_be_bytes(), &mut hex).expect("Unable to serialize ScriptPubKey version"); faster_hex::hex_encode(&self.script, &mut hex[4..]).expect("Unable to serialize ScriptPubKey script"); - serializer.serialize_str(str::from_utf8(&hex).expect("hex is must be a valid UTF-8")) + serializer.serialize_str(unsafe { str::from_utf8_unchecked(&hex) }) } else { ScriptPublicKeyInternal { version: self.version, script: &self.script }.serialize(serializer) } @@ -70,8 +71,8 @@ impl<'de: 'a, 'a> Deserialize<'de> for ScriptPublicKey { D: Deserializer<'de>, { if deserializer.is_human_readable() { - let s = ::deserialize(deserializer)?; - FromStr::from_str(&s).map_err(serde::de::Error::custom) + let s = <&str as Deserialize>::deserialize(deserializer)?; + FromStr::from_str(s).map_err(serde::de::Error::custom) } else { ScriptPublicKeyInternal::deserialize(deserializer) .map(|ScriptPublicKeyInternal { script, version }| Self { version, script: SmallVec::from_slice(script) }) diff --git a/crypto/hashes/src/lib.rs b/crypto/hashes/src/lib.rs index ea2f9a283..b0be56ea9 100644 --- a/crypto/hashes/src/lib.rs +++ b/crypto/hashes/src/lib.rs @@ -81,7 +81,7 @@ impl Display for Hash { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { let mut hex = [0u8; HASH_SIZE * 2]; faster_hex::hex_encode(&self.0, &mut hex).expect("The output is exactly twice the size of the input"); - f.write_str(str::from_utf8(&hex).expect("hex is always valid UTF-8")) + f.write_str(unsafe { str::from_utf8_unchecked(&hex) }) } } From bb3499e7d24f14ab6dd96ce7519baab4185b516d Mon Sep 17 00:00:00 2001 From: Anton Yemelyanov Date: Wed, 26 Jul 2023 14:48:33 +0300 Subject: [PATCH 3/4] replace serializer use of expect() with map_err(Error::custom) --- consensus/core/src/tx.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/core/src/tx.rs b/consensus/core/src/tx.rs index d6e688f8e..0f939d1cb 100644 --- a/consensus/core/src/tx.rs +++ b/consensus/core/src/tx.rs @@ -56,8 +56,8 @@ impl Serialize for ScriptPublicKey { { if serializer.is_human_readable() { let mut hex = [0u8; SCRIPT_VECTOR_SIZE * 2 + 4]; - faster_hex::hex_encode(&self.version.to_be_bytes(), &mut hex).expect("Unable to serialize ScriptPubKey version"); - faster_hex::hex_encode(&self.script, &mut hex[4..]).expect("Unable to serialize ScriptPubKey script"); + faster_hex::hex_encode(&self.version.to_be_bytes(), &mut hex).map_err(serde::ser::Error::custom)?; + faster_hex::hex_encode(&self.script, &mut hex[4..]).map_err(serde::ser::Error::custom)?; serializer.serialize_str(unsafe { str::from_utf8_unchecked(&hex) }) } else { ScriptPublicKeyInternal { version: self.version, script: &self.script }.serialize(serializer) From ee9685ac9090e0cd8c8b847427c30db99fde9df2 Mon Sep 17 00:00:00 2001 From: Anton Yemelyanov Date: Wed, 26 Jul 2023 20:17:54 +0300 Subject: [PATCH 4/4] fix incorrect usage of script length and handling of larger scripts --- consensus/core/src/tx.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/consensus/core/src/tx.rs b/consensus/core/src/tx.rs index 0f939d1cb..97252df43 100644 --- a/consensus/core/src/tx.rs +++ b/consensus/core/src/tx.rs @@ -55,7 +55,7 @@ impl Serialize for ScriptPublicKey { S: Serializer, { if serializer.is_human_readable() { - let mut hex = [0u8; SCRIPT_VECTOR_SIZE * 2 + 4]; + let mut hex = vec![0u8; self.script.len() * 2 + 4]; faster_hex::hex_encode(&self.version.to_be_bytes(), &mut hex).map_err(serde::ser::Error::custom)?; faster_hex::hex_encode(&self.script, &mut hex[4..]).map_err(serde::ser::Error::custom)?; serializer.serialize_str(unsafe { str::from_utf8_unchecked(&hex) }) @@ -82,9 +82,13 @@ impl<'de: 'a, 'a> Deserialize<'de> for ScriptPublicKey { impl FromStr for ScriptPublicKey { type Err = faster_hex::Error; - fn from_str(hash_str: &str) -> Result { - let mut bytes = [0u8; SCRIPT_VECTOR_SIZE + 2]; - faster_hex::hex_decode(hash_str.as_bytes(), &mut bytes)?; + fn from_str(hex_str: &str) -> Result { + let hex_len = hex_str.len(); + if hex_len < 4 { + return Err(faster_hex::Error::InvalidLength(hex_len)); + } + let mut bytes = vec![0u8; hex_len / 2]; + faster_hex::hex_decode(hex_str.as_bytes(), bytes.as_mut_slice())?; let version = u16::from_be_bytes(bytes[0..2].try_into().unwrap()); Ok(Self { version, script: SmallVec::from_slice(&bytes[2..]) }) } @@ -502,6 +506,11 @@ mod tests { let spk = serde_json::from_str::(&hex).unwrap(); assert_eq!(spk.version, 0xc0de); assert_eq!(spk.script.as_slice(), vec.as_slice()); + let result = "00".parse::(); + assert!(matches!(result, Err(faster_hex::Error::InvalidLength(2)))); + let result = "0000".parse::(); + let _empty = ScriptPublicKey { version: 0, script: ScriptVec::new() }; + assert!(matches!(result, Ok(_empty))); } #[test]