Skip to content

Commit

Permalink
Use fixed width serde impls for secret/public key
Browse files Browse the repository at this point in the history
Currently we serialize `SecretKey` and `PublicKey` using the
`BytesVisitor`, this causes the serialized data to contain additional
metadata encoding the length (an extra 8 bytes). This extra data is
unnecessary since we know in advance the length of these two types (32
and 33 bytes respectively).

Implement a sequence based visitor that encodes the keys as fixed width
data.
  • Loading branch information
tcharding committed Feb 16, 2022
1 parent ef59aea commit 50ab92c
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 10 deletions.
43 changes: 33 additions & 10 deletions src/key.rs
Expand Up @@ -28,6 +28,9 @@ use Verification;
use constants;
use ffi::{self, CPtr};

#[cfg(feature = "serde")]
use serde::ser::SerializeTuple;

#[cfg(feature = "global-context")]
use {Message, ecdsa, SECP256K1};
#[cfg(all(feature = "global-context", feature = "rand-std"))]
Expand Down Expand Up @@ -302,7 +305,11 @@ impl ::serde::Serialize for SecretKey {
let mut buf = [0u8; 64];
s.serialize_str(::to_hex(&self.0, &mut buf).expect("fixed-size hex serialization"))
} else {
s.serialize_bytes(&self[..])
let mut seq = s.serialize_tuple(constants::SECRET_KEY_SIZE)?;
for byte in self.0.iter() {
seq.serialize_element(byte)?;
}
seq.end()
}
}
}
Expand All @@ -316,7 +323,7 @@ impl<'de> ::serde::Deserialize<'de> for SecretKey {
"a hex string representing 32 byte SecretKey"
))
} else {
d.deserialize_bytes(super::serde_util::BytesVisitor::new(
d.deserialize_seq(super::serde_util::SeqVisitor::new(
"raw 32 bytes SecretKey",
SecretKey::from_slice
))
Expand Down Expand Up @@ -618,7 +625,11 @@ impl ::serde::Serialize for PublicKey {
if s.is_human_readable() {
s.collect_str(self)
} else {
s.serialize_bytes(&self.serialize())
let mut seq = s.serialize_tuple(constants::PUBLIC_KEY_SIZE)?;
for byte in self.serialize() {
seq.serialize_element(&byte)?;
}
seq.end()
}
}
}
Expand All @@ -632,7 +643,7 @@ impl<'de> ::serde::Deserialize<'de> for PublicKey {
"an ASCII hex string representing a public key"
))
} else {
d.deserialize_bytes(super::serde_util::BytesVisitor::new(
d.deserialize_seq(super::serde_util::SeqVisitor::new(
"a bytestring representing a public key",
PublicKey::from_slice
))
Expand Down Expand Up @@ -1911,6 +1922,7 @@ mod test {
static SK_STR: &'static str = "\
01010101010101010001020304050607ffff0000ffff00006363636363636363\
";
#[cfg(fuzzing)]
static PK_BYTES: [u8; 33] = [
0x02,
0x18, 0x84, 0x57, 0x81, 0xf6, 0x31, 0xc4, 0x8f,
Expand All @@ -1933,17 +1945,28 @@ mod test {
#[cfg(fuzzing)]
let pk = PublicKey::from_slice(&PK_BYTES).expect("pk");

assert_tokens(&sk.compact(), &[Token::BorrowedBytes(&SK_BYTES[..])]);
assert_tokens(&sk.compact(), &[Token::Bytes(&SK_BYTES)]);
assert_tokens(&sk.compact(), &[Token::ByteBuf(&SK_BYTES)]);
assert_tokens(&sk.compact(), &[
Token::Tuple{ len: 32 },
Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1), Token::U8(1),
Token::U8(0), Token::U8(1), Token::U8(2), Token::U8(3), Token::U8(4), Token::U8(5), Token::U8(6), Token::U8(7),
Token::U8(0xff), Token::U8(0xff), Token::U8(0), Token::U8(0), Token::U8(0xff), Token::U8(0xff), Token::U8(0), Token::U8(0),
Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99), Token::U8(99),
Token::TupleEnd
]);

assert_tokens(&sk.readable(), &[Token::BorrowedStr(SK_STR)]);
assert_tokens(&sk.readable(), &[Token::Str(SK_STR)]);
assert_tokens(&sk.readable(), &[Token::String(SK_STR)]);

assert_tokens(&pk.compact(), &[Token::BorrowedBytes(&PK_BYTES[..])]);
assert_tokens(&pk.compact(), &[Token::Bytes(&PK_BYTES)]);
assert_tokens(&pk.compact(), &[Token::ByteBuf(&PK_BYTES)]);
assert_tokens(&pk.compact(), &[
Token::Tuple{ len: 33 },
Token::U8(0x02),
Token::U8(0x18), Token::U8(0x84), Token::U8(0x57), Token::U8(0x81), Token::U8(0xf6), Token::U8(0x31), Token::U8(0xc4), Token::U8(0x8f),
Token::U8(0x1c), Token::U8(0x97), Token::U8(0x09), Token::U8(0xe2), Token::U8(0x30), Token::U8(0x92), Token::U8(0x06), Token::U8(0x7d),
Token::U8(0x06), Token::U8(0x83), Token::U8(0x7f), Token::U8(0x30), Token::U8(0xaa), Token::U8(0x0c), Token::U8(0xd0), Token::U8(0x54),
Token::U8(0x4a), Token::U8(0xc8), Token::U8(0x87), Token::U8(0xfe), Token::U8(0x91), Token::U8(0xdd), Token::U8(0xd1), Token::U8(0x66),
Token::TupleEnd
]);

assert_tokens(&pk.readable(), &[Token::BorrowedStr(PK_STR)]);
assert_tokens(&pk.readable(), &[Token::Str(PK_STR)]);
Expand Down
41 changes: 41 additions & 0 deletions src/serde_util.rs
Expand Up @@ -67,3 +67,44 @@ where
(self.parse_fn)(v).map_err(E::custom)
}
}

pub struct SeqVisitor<F> {
expectation: &'static str,
parse_fn: F,
}

impl<F, T, Err> SeqVisitor<F>
where
F: FnOnce(&[u8]) -> Result<T, Err>,
Err: fmt::Display,
{
pub fn new(expectation: &'static str, parse_fn: F) -> Self {
SeqVisitor {
expectation,
parse_fn,
}
}
}

impl<'de, F, T, Err> de::Visitor<'de> for SeqVisitor<F>
where
F: FnOnce(&[u8]) -> Result<T, Err>,
Err: fmt::Display,
{
type Value = T;

fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
formatter.write_str(self.expectation)
}

fn visit_seq<V>(self, mut seq: V) -> Result<Self::Value, V::Error>
where
V: de::SeqAccess<'de>,
{
let mut v = Vec::new();
while let Some(value) = seq.next_element()? {
v.push(value)
}
(self.parse_fn)(&v).map_err(de::Error::custom)
}
}

0 comments on commit 50ab92c

Please sign in to comment.