Skip to content
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

Serialized signture improvements #658

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions src/ecdsa/serialized_signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
//! unable to run on platforms without allocator. We implement a special type to encapsulate
//! serialized signatures and since it's a bit more complicated it has its own module.

use core::borrow::Borrow;
use core::convert::TryFrom;
use core::{fmt, ops};

pub use into_iter::IntoIter;
Expand Down Expand Up @@ -41,11 +43,30 @@ impl PartialEq for SerializedSignature {
fn eq(&self, other: &SerializedSignature) -> bool { **self == **other }
}

impl PartialEq<[u8]> for SerializedSignature {
#[inline]
fn eq(&self, other: &[u8]) -> bool { **self == *other }
}

impl PartialEq<SerializedSignature> for [u8] {
#[inline]
fn eq(&self, other: &SerializedSignature) -> bool { *self == **other }
}

impl core::hash::Hash for SerializedSignature {
fn hash<H: core::hash::Hasher>(&self, state: &mut H) { (**self).hash(state) }
}

impl AsRef<[u8]> for SerializedSignature {
#[inline]
fn as_ref(&self) -> &[u8] { self }
}

impl Borrow<[u8]> for SerializedSignature {
#[inline]
fn borrow(&self) -> &[u8] { self }
}

impl ops::Deref for SerializedSignature {
type Target = [u8];

Expand All @@ -71,6 +92,28 @@ impl<'a> IntoIterator for &'a SerializedSignature {
fn into_iter(self) -> Self::IntoIter { self.iter() }
}

impl From<Signature> for SerializedSignature {
fn from(value: Signature) -> Self { Self::from_signature(&value) }
}

impl<'a> From<&'a Signature> for SerializedSignature {
fn from(value: &'a Signature) -> Self { Self::from_signature(value) }
}

impl TryFrom<SerializedSignature> for Signature {
type Error = Error;

fn try_from(value: SerializedSignature) -> Result<Self, Self::Error> { value.to_signature() }
}

impl<'a> TryFrom<&'a SerializedSignature> for Signature {
type Error = Error;

fn try_from(value: &'a SerializedSignature) -> Result<Self, Self::Error> {
value.to_signature()
}
}

impl SerializedSignature {
/// Creates `SerializedSignature` from data and length.
///
Expand All @@ -84,6 +127,7 @@ impl SerializedSignature {
}

/// Get the capacity of the underlying data buffer.
#[deprecated = "This always returns 72"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is true why is the buffer size const called MAX_LEN? Also, if it is always 72 why do we even have the len field?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FTR I didn't look up the rules of sigs right now, and I don't knew them off the top of my head, I'm just looking at the code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is related to #659, reviewing that PR made me wonder why we do equality/ord comparison on the bytes not covered by the len field.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't confuse self.data.len() (which is MAX_LEN because of array) with self.len :)

why we do equality/ord comparison on the bytes not covered by the len field.

I'm not sure what you mean. The array only contains valid data up to len so we compare only those. The rest may be considered padding. (If we wanted to be crazy efficient we would use MaybeUninit<u8> instead.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ignore me, I don't know what code I was looking at yesterday, the **self == **other seemed totally new to me just now, I was baffled for a minute till I saw the Deref impl.

#[inline]
pub fn capacity(&self) -> usize { self.data.len() }

Expand All @@ -106,6 +150,7 @@ impl SerializedSignature {
pub fn from_signature(sig: &Signature) -> SerializedSignature { sig.serialize_der() }

/// Check if the space is zero.
#[deprecated = "This always returns false"]
#[inline]
pub fn is_empty(&self) -> bool { self.len() == 0 }
}
Expand Down
Loading