Skip to content

Commit

Permalink
Merge #458: Removed Default from SerializedSignature
Browse files Browse the repository at this point in the history
b18f5d0 Removed `Default` from `SerializedSignature` (Martin Habovstiak)

Pull request description:

  `Default` was pointless, so it was replaced with internal
  `from_raw_parts` method which also checks the length.

  This commit also documents changes to `SerializedSignature`.

  Closes #454

ACKs for top commit:
  tcharding:
    utACK b18f5d0
  apoelstra:
    ACK b18f5d0

Tree-SHA512: 5ee32160721d4d22cfe7c5dcc433bf013fc78a350e86b3d8d42c207fec7f2bf11c47fce77269ae816567be77602fdc86231d86e2c62aa2d327540056ab445842
  • Loading branch information
apoelstra committed Jun 22, 2022
2 parents 7975be5 + b18f5d0 commit 568f16a
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 21 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ The major change in this version is the increase of the Minimum Supported Rust V
* Key tweaking methods renamed and refactored to use a more [functional-style](https://github.com/rust-bitcoin/rust-secp256k1/pull/406), they now accept a [new Scalar](https://github.com/rust-bitcoin/rust-secp256k1/pull/445) type instead of raw slices
* Update [`rand` dependency to 0.8](https://github.com/rust-bitcoin/rust-secp256k1/pull/331)
* `KeyPair::from_secret_key` [borrows SecretKey](https://github.com/rust-bitcoin/rust-secp256k1/pull/430) instead of taking ownership
* `SerializedSignature` no longer implements `Default`

## New features/APIs

Expand All @@ -18,6 +19,7 @@ The major change in this version is the increase of the Minimum Supported Rust V
* [Implemented `TryFrom` for `Parity`](https://github.com/rust-bitcoin/rust-secp256k1/pull/409)
* The [alloc feature](https://github.com/rust-bitcoin/rust-secp256k1/pull/331) can be used on targets with allocators without a standard library
* `SharedSecret` can be created from a slice, parsed from a hex string, or [(de)serialized using serde](https://github.com/rust-bitcoin/rust-secp256k1/pull/418)
* `SerializedSignature` implements `IntoIterator` (both owned and shared reference)
* We now [derive `std::hash::Hash` for `Signature`](https://github.com/rust-bitcoin/rust-secp256k1/pull/441)

## Other improvements
Expand Down
10 changes: 4 additions & 6 deletions src/ecdsa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,20 +157,18 @@ impl Signature {
#[inline]
/// Serializes the signature in DER format
pub fn serialize_der(&self) -> SerializedSignature {
let mut ret = SerializedSignature::default();
let mut len: usize = ret.capacity();
let mut data = [0u8; serialized_signature::MAX_LEN];
let mut len: usize = serialized_signature::MAX_LEN;
unsafe {
let err = ffi::secp256k1_ecdsa_signature_serialize_der(
ffi::secp256k1_context_no_precomp,
ret.get_data_mut_ptr(),
data.as_mut_ptr(),
&mut len,
self.as_c_ptr(),
);
debug_assert!(err == 1);
assert!(len <= serialized_signature::MAX_LEN, "libsecp256k1 set length to {} but the maximum is {}", len, serialized_signature::MAX_LEN);
ret.set_len(len);
SerializedSignature::from_raw_parts(data, len)
}
ret
}

#[inline]
Expand Down
28 changes: 13 additions & 15 deletions src/ecdsa/serialized_signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,6 @@ impl fmt::Display for SerializedSignature {
}
}

impl Default for SerializedSignature {
#[inline]
fn default() -> SerializedSignature {
SerializedSignature {
data: [0u8; MAX_LEN],
len: 0,
}
}
}

impl PartialEq for SerializedSignature {
#[inline]
fn eq(&self, other: &SerializedSignature) -> bool {
Expand Down Expand Up @@ -91,10 +81,18 @@ impl<'a> IntoIterator for &'a SerializedSignature {
}

impl SerializedSignature {
/// Get a pointer to the underlying data with the specified capacity.
/// Creates `SerializedSignature` from data and length.
///
/// ## Panics
///
/// If `len` > `MAX_LEN`
#[inline]
pub(crate) fn get_data_mut_ptr(&mut self) -> *mut u8 {
self.data.as_mut_ptr()
pub(crate) fn from_raw_parts(data: [u8; MAX_LEN], len: usize) -> Self {
assert!(len <= MAX_LEN, "attempt to set length to {} but the maximum is {}", len, MAX_LEN);
SerializedSignature {
data,
len,
}
}

/// Get the capacity of the underlying data buffer.
Expand All @@ -111,7 +109,7 @@ impl SerializedSignature {

/// Set the length of the object.
#[inline]
pub(crate) fn set_len(&mut self, len: usize) {
pub(crate) fn set_len_unchecked(&mut self, len: usize) {
self.len = len;
}

Expand Down Expand Up @@ -218,7 +216,7 @@ mod into_iter {
// reach this
let new_len = self.signature.len() - 1;
let byte = self.signature[new_len];
self.signature.set_len(new_len);
self.signature.set_len_unchecked(new_len);
Some(byte)
}
}
Expand Down

0 comments on commit 568f16a

Please sign in to comment.