-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ScriptPubKey human-readable ser/deser (for json) #232
Changes from 1 commit
d06ad17
744d4af
bb3499e
ee9685a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -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<ScriptPublicKey>; | ||||||
|
||||||
/// 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<S>(&self, serializer: S) -> Result<S::Ok, S::Error> | ||||||
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")) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's safe to use unsafe from_utf8_unchecked, because hex encoding will never produce non-utf8. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replaced There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||||||
} else { | ||||||
ScriptPublicKeyInternal { version: self.version, script: &self.script }.serialize(serializer) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
it can be done without There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's keep it as-is |
||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
impl<'de: 'a, 'a> Deserialize<'de> for ScriptPublicKey { | ||||||
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> | ||||||
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 = <std::string::String as Deserialize>::deserialize(deserializer)?; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
It needs to be tested, I think it can prevent String allocation There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||||||
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<Self, Self::Err> { | ||||||
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<Transaction>; | |||||
mod tests { | ||||||
use super::*; | ||||||
|
||||||
#[test] | ||||||
fn test_spk_serde_json() { | ||||||
let vec = (0..SCRIPT_VECTOR_SIZE as u8).collect::<Vec<_>>(); | ||||||
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::<ScriptPublicKey>(&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 | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add this renaming? It was my fault. I think without renaming it, xml and other structure-name sensitive formats can work incorrectly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed