Skip to content

Commit

Permalink
Use fixed width serde impls for keys
Browse files Browse the repository at this point in the history 
Currently we serialize keys 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 types.

Implement a tuple based visitor that encodes the keys as fixed width
data.

Do fixed width serde implementations for:

- `SecretKey`
- `PublicKey`
- `KeyPair`
- `XOnlyPublicKey`
  • Loading branch information
@tcharding
tcharding committed Mar 1, 2022
1 parent faa1539 commit 330a18e
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 26 deletions.
120 changes: 97 additions & 23 deletions src/key.rs
View file
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; constants::SECRET_KEY_SIZE * 2];
s.serialize_str(::to_hex(&self.0, &mut buf).expect("fixed-size hex serialization"))
} else {
s.serialize_bytes(&self[..])
let mut tuple = s.serialize_tuple(constants::SECRET_KEY_SIZE)?;
for byte in self.0.iter() {
tuple.serialize_element(byte)?;
}
tuple.end()
}
}
}
Expand All @@ -316,10 +323,11 @@ impl<'de> ::serde::Deserialize<'de> for SecretKey {
"a hex string representing 32 byte SecretKey"
))
} else {
d.deserialize_bytes(super::serde_util::BytesVisitor::new(
let visitor = super::serde_util::Tuple32Visitor::new(
"raw 32 bytes SecretKey",
SecretKey::from_slice
))
);
d.deserialize_tuple(constants::SECRET_KEY_SIZE, visitor)
}
}
}
Expand Down Expand Up @@ -618,7 +626,11 @@ impl ::serde::Serialize for PublicKey {
if s.is_human_readable() {
s.collect_str(self)
} else {
s.serialize_bytes(&self.serialize())
let mut tuple = s.serialize_tuple(constants::PUBLIC_KEY_SIZE)?;
for byte in self.serialize().iter() { // Serialize in compressed form.
tuple.serialize_element(&byte)?;
}
tuple.end()
}
}
}
Expand All @@ -632,10 +644,11 @@ impl<'de> ::serde::Deserialize<'de> for PublicKey {
"an ASCII hex string representing a public key"
))
} else {
d.deserialize_bytes(super::serde_util::BytesVisitor::new(
"a bytestring representing a public key",
let visitor = super::serde_util::Tuple33Visitor::new(
"33 bytes compressed public key",
PublicKey::from_slice
))
);
d.deserialize_tuple(constants::PUBLIC_KEY_SIZE, visitor)
}
}
}
Expand Down Expand Up @@ -929,7 +942,11 @@ impl ::serde::Serialize for KeyPair {
s.serialize_str(::to_hex(&self.secret_bytes(), &mut buf)
.expect("fixed-size hex serialization"))
} else {
s.serialize_bytes(&self.0[..])
let mut tuple = s.serialize_tuple(constants::SECRET_KEY_SIZE)?;
for byte in self.secret_bytes().iter() {
tuple.serialize_element(&byte)?;
}
tuple.end()
}
}
}
Expand All @@ -943,13 +960,14 @@ impl<'de> ::serde::Deserialize<'de> for KeyPair {
"a hex string representing 32 byte KeyPair"
))
} else {
d.deserialize_bytes(super::serde_util::BytesVisitor::new(
let visitor = super::serde_util::Tuple32Visitor::new(
"raw 32 bytes KeyPair",
|data| unsafe {
let ctx = Secp256k1::from_raw_all(ffi::secp256k1_context_no_precomp as *mut ffi::Context);
KeyPair::from_seckey_slice(&ctx, data)
}
))
);
d.deserialize_tuple(constants::SECRET_KEY_SIZE, visitor)
}
}
}
Expand Down Expand Up @@ -1370,7 +1388,11 @@ impl ::serde::Serialize for XOnlyPublicKey {
if s.is_human_readable() {
s.collect_str(self)
} else {
s.serialize_bytes(&self.serialize())
let mut tuple = s.serialize_tuple(constants::SCHNORRSIG_PUBLIC_KEY_SIZE)?;
for byte in self.serialize().iter() {
tuple.serialize_element(&byte)?;
}
tuple.end()
}
}
}
Expand All @@ -1384,10 +1406,11 @@ impl<'de> ::serde::Deserialize<'de> for XOnlyPublicKey {
"a hex string representing 32 byte schnorr public key"
))
} else {
d.deserialize_bytes(super::serde_util::BytesVisitor::new(
let visitor = super::serde_util::Tuple32Visitor::new(
"raw 32 bytes schnorr public key",
XOnlyPublicKey::from_slice
))
);
d.deserialize_tuple(constants::SCHNORRSIG_PUBLIC_KEY_SIZE, visitor)
}
}
}
Expand Down Expand Up @@ -1928,6 +1951,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 @@ -1950,22 +1974,32 @@ 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)]);
assert_tokens(&pk.readable(), &[Token::String(PK_STR)]);

}

#[test]
Expand Down Expand Up @@ -2044,12 +2078,52 @@ mod test {

let sk = KeyPairWrapper(KeyPair::from_seckey_slice(&::SECP256K1, &SK_BYTES).unwrap());

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)]);
}

#[test]
#[cfg(not(fuzzing))]
#[cfg(all(feature = "global-context", feature = "serde"))]
fn test_serde_x_only_pubkey() {
use serde_test::{Configure, Token, assert_tokens};
use key::KeyPair;

static SK_BYTES: [u8; 32] = [
1, 1, 1, 1, 1, 1, 1, 1,
0, 1, 2, 3, 4, 5, 6, 7,
0xff, 0xff, 0, 0, 0xff, 0xff, 0, 0,
99, 99, 99, 99, 99, 99, 99, 99
];

static PK_STR: &'static str = "\
18845781f631c48f1c9709e23092067d06837f30aa0cd0544ac887fe91ddd166\
";

let kp = KeyPair::from_seckey_slice(&::SECP256K1, &SK_BYTES).unwrap();
let pk = XOnlyPublicKey::from_keypair(&kp);

assert_tokens(&pk.compact(), &[
Token::Tuple{ len: 32 },
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)]);
assert_tokens(&pk.readable(), &[Token::String(PK_STR)]);
}
}
11 changes: 8 additions & 3 deletions src/schnorr.rs
View file
Expand Up @@ -595,9 +595,14 @@ mod tests {
assert_tokens(&sig.readable(), &[Token::Str(SIG_STR)]);
assert_tokens(&sig.readable(), &[Token::String(SIG_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: 32 },
Token::U8(24), Token::U8(132), Token::U8(87), Token::U8(129), Token::U8(246), Token::U8(49), Token::U8(196), Token::U8(143),
Token::U8(28), Token::U8(151), Token::U8(9), Token::U8(226), Token::U8(48), Token::U8(146), Token::U8(6), Token::U8(125),
Token::U8(6), Token::U8(131), Token::U8(127), Token::U8(48), Token::U8(170), Token::U8(12), Token::U8(208), Token::U8(84),
Token::U8(74), Token::U8(200), Token::U8(135), Token::U8(254), Token::U8(145), Token::U8(221), Token::U8(209), Token::U8(102),
Token::TupleEnd
]);

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

macro_rules! impl_tuple_visitor {
($thing:ident, $len:expr) => {
pub(crate) struct $thing<F> {
expectation: &'static str,
parse_fn: F,
}

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

impl<'de, F, T, E> de::Visitor<'de> for $thing<F>
where
F: FnOnce(&[u8]) -> Result<T, E>,
E: 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 bytes = [0u8; $len];

for (i, byte) in bytes.iter_mut().enumerate() {
if let Some(value) = seq.next_element()? {
*byte = value;
} else {
return Err(de::Error::invalid_length(i, &self));
}
}
(self.parse_fn)(&bytes).map_err(de::Error::custom)
}
}
}
}

impl_tuple_visitor!(Tuple32Visitor, 32);
impl_tuple_visitor!(Tuple33Visitor, 33);

0 comments on commit 330a18e

Please sign in to comment.