Merge rust-bitcoin/rust-bitcoin#3544: Favour `to_vec` over `to_bytes`

a51768af3f key: Deprecate to_bytes (Tobin C. Harding)
3af3239ad0 script: Re-order functions (Tobin C. Harding)
db40297f87 script: deprecate to_bytes (Tobin C. Harding)
c5cd0db493 Revert the change to to_bytes (Tobin C. Harding)
dc2ca785d2 Add to_vec and deprecate to_bytes for array types (Tobin C. Harding)
a6b7ab32a8 Move impl_array_newtype to internal_macros (Tobin C. Harding)

Pull request description:

  Use `to_vec` and deprecate `to_bytes`, the opposite of what we did in #2585.

  For functions that return a `Vec` by first allocating use function name `to_vec`. This explicitly excludes:

  - Functions that return an array (`CompressedPublicKey::to_bytes`)
  - Functions that consume self and return a `Vec` without allocating (`ScriptBuf::into_bytes`)

  See #3025 for discussion and consensus.

  Close: #3025

ACKs for top commit:
  apoelstra:
    ACK a51768af3f3d4c8e138e1ded250800810bedc903; successfully ran local tests

Tree-SHA512: ee932c13ad2e09c2b76a7833b23c859df175aa307f56e673921f3ae8b5d865518c6f999749e3b627594457b3ca33301b777177ada3520cf006acc0f14e5dacf8
This commit is contained in:
merge-script 2024-11-01 00:55:15 +00:00
commit 4c8347a7ac
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
17 changed files with 194 additions and 187 deletions

View File

@ -690,7 +690,7 @@ impl BeneficiaryWallet {
script_witness.push(signature.to_vec());
}
for (control_block, (script, _)) in input.tap_scripts.iter() {
script_witness.push(script.to_bytes());
script_witness.push(script.to_vec());
script_witness.push(control_block.serialize());
}
input.final_script_witness = Some(script_witness);

View File

@ -9,11 +9,13 @@ use core::{convert, fmt, mem};
use std::error;
use hashes::{sha256, siphash24};
use internals::{impl_array_newtype, ToU64 as _};
use internals::ToU64 as _;
use io::{BufRead, Write};
use crate::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt};
use crate::internal_macros::{impl_array_newtype_stringify, impl_consensus_encoding};
use crate::internal_macros::{
impl_array_newtype, impl_array_newtype_stringify, impl_consensus_encoding,
};
use crate::prelude::Vec;
use crate::transaction::TxIdentifier;
use crate::{block, consensus, Block, BlockHash, Transaction};

View File

@ -10,11 +10,11 @@ use core::str::FromStr;
use core::{fmt, slice};
use hashes::{hash160, hash_newtype, sha512, GeneralHash, HashEngine, Hmac, HmacEngine};
use internals::{impl_array_newtype, write_err};
use internals::write_err;
use secp256k1::{Secp256k1, XOnlyPublicKey};
use crate::crypto::key::{CompressedPublicKey, Keypair, PrivateKey};
use crate::internal_macros::impl_array_newtype_stringify;
use crate::internal_macros::{impl_array_newtype, impl_array_newtype_stringify};
use crate::network::NetworkKind;
use crate::prelude::{String, Vec};

View File

@ -7,10 +7,9 @@
//! single transaction.
use hashes::sha256d;
use internals::impl_array_newtype;
use crate::block::{self, Block};
use crate::internal_macros::impl_array_newtype_stringify;
use crate::internal_macros::{impl_array_newtype, impl_array_newtype_stringify};
use crate::locktime::absolute;
use crate::network::{Network, Params};
use crate::opcodes::all::*;

View File

@ -54,7 +54,7 @@ fn p2pk_pubkey_bytes_valid_key_and_valid_script_returns_expected_key() {
let key = key_str.parse::<PublicKey>().unwrap();
let p2pk = Script::builder().push_key(key).push_opcode(OP_CHECKSIG).into_script();
let actual = p2pk.p2pk_pubkey_bytes().unwrap();
assert_eq!(actual.to_vec(), key.to_bytes());
assert_eq!(actual.to_vec(), key.to_vec());
}
#[test]
@ -109,7 +109,7 @@ fn p2pk_pubkey_bytes_compressed_key_returns_expected_key() {
let key = compressed_key_str.parse::<PublicKey>().unwrap();
let p2pk = Script::builder().push_key(key).push_opcode(OP_CHECKSIG).into_script();
let actual = p2pk.p2pk_pubkey_bytes().unwrap();
assert_eq!(actual.to_vec(), key.to_bytes());
assert_eq!(actual.to_vec(), key.to_vec());
}
#[test]

View File

@ -1604,7 +1604,7 @@ mod tests {
.is_err());
// test that we get a failure if we corrupt a signature
let mut witness = spending.input[1].witness.to_bytes();
let mut witness = spending.input[1].witness.to_vec();
witness[0][10] = 42;
spending.input[1].witness = Witness::from_slice(&witness);

View File

@ -112,10 +112,6 @@ impl Encodable for Witness {
crate::internal_macros::define_extension_trait! {
/// Extension functionality for the [`Witness`] type.
pub trait WitnessExt impl for Witness {
/// Convenience method to create an array of byte-arrays from this witness.
#[deprecated(since = "TBD", note = "use `to_bytes` instead")]
fn to_vec(&self) -> Vec<Vec<u8>> { self.to_bytes() }
/// Creates a witness required to spend a P2WPKH output.
///
/// The witness will be made up of the DER encoded signature + sighash_type followed by the
@ -269,7 +265,7 @@ mod test {
let expected_witness = vec![hex!(
"304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b54101")
];
assert_eq!(witness.to_bytes(), expected_witness);
assert_eq!(witness.to_vec(), expected_witness);
}
#[test]

View File

@ -61,7 +61,7 @@ impl Signature {
///
/// Note: this performs an extra heap allocation, you might prefer the
/// [`serialize`](Self::serialize) method instead.
pub fn to_bytes(self) -> Vec<u8> {
pub fn to_vec(self) -> Vec<u8> {
self.signature
.serialize_der()
.iter()
@ -70,10 +70,6 @@ impl Signature {
.collect()
}
/// Serializes an ECDSA signature (inner secp256k1 signature in DER format) into `Vec`.
#[deprecated(since = "TBD", note = "use `to_bytes` instead")]
pub fn to_vec(self) -> Vec<u8> { self.to_bytes() }
/// Serializes an ECDSA signature (inner secp256k1 signature in DER format) to a `writer`.
#[inline]
pub fn serialize_to_writer<W: Write + ?Sized>(&self, writer: &mut W) -> Result<(), io::Error> {
@ -345,6 +341,6 @@ mod tests {
let mut buf = vec![];
sig.serialize_to_writer(&mut buf).expect("write failed");
assert_eq!(sig.to_bytes(), buf)
assert_eq!(sig.to_vec(), buf)
}
}

View File

@ -110,7 +110,11 @@ impl PublicKey {
}
/// Serializes the public key to bytes.
pub fn to_bytes(self) -> Vec<u8> {
#[deprecated(since = "TBD", note = "use to_vec instead")]
pub fn to_bytes(self) -> Vec<u8> { self.to_vec() }
/// Serializes the public key to bytes.
pub fn to_vec(self) -> Vec<u8> {
let mut buf = Vec::new();
self.write_into(&mut buf).expect("vecs don't error");
buf
@ -442,8 +446,13 @@ impl PrivateKey {
}
}
/// Serializes the private key to bytes.
pub fn to_bytes(self) -> Vec<u8> { self.inner[..].to_vec() }
#[deprecated(since = "TBD", note = "use to_vec instead")]
pub fn to_bytes(self) -> Vec<u8> { self.to_vec() }
/// Serializes the private key to bytes.
pub fn to_vec(self) -> Vec<u8> { self.inner[..].to_vec() }
/// Deserializes a private key from a slice.
pub fn from_slice(

View File

@ -47,7 +47,7 @@ impl Signature {
/// Serializes the signature.
///
/// Note: this allocates on the heap, prefer [`serialize`](Self::serialize) if vec is not needed.
pub fn to_bytes(self) -> Vec<u8> {
pub fn to_vec(self) -> Vec<u8> {
let mut ser_sig = self.signature.as_ref().to_vec();
if self.sighash_type == TapSighashType::Default {
// default sighash type, don't add extra sighash byte
@ -57,10 +57,6 @@ impl Signature {
ser_sig
}
/// Serializes the signature.
#[deprecated(since = "TBD", note = "use `to_bytes` instead")]
pub fn to_vec(self) -> Vec<u8> { self.to_bytes() }
/// Serializes the signature to `writer`.
#[inline]
pub fn serialize_to_writer<W: Write + ?Sized>(&self, writer: &mut W) -> Result<(), io::Error> {

View File

@ -277,3 +277,126 @@ macro_rules! define_extension_trait {
};
}
pub(crate) use define_extension_trait;
/// Implements standard array methods for a given wrapper type.
macro_rules! impl_array_newtype {
($thing:ident, $ty:ty, $len:literal) => {
impl $thing {
/// Creates `Self` by wrapping `bytes`.
#[inline]
pub fn from_byte_array(bytes: [u8; $len]) -> Self { Self(bytes) }
/// Returns a reference the underlying byte array.
#[inline]
pub fn as_byte_array(&self) -> &[u8; $len] { &self.0 }
/// Returns the underlying byte array.
#[inline]
pub fn to_byte_array(self) -> [u8; $len] {
// We rely on `Copy` being implemented for $thing so conversion
// methods use the correct Rust naming conventions.
fn check_copy<T: Copy>() {}
check_copy::<$thing>();
self.0
}
/// Copies the underlying bytes into a new `Vec`.
#[inline]
pub fn to_vec(&self) -> alloc::vec::Vec<u8> { self.0.to_vec() }
/// Returns a slice of the underlying bytes.
#[inline]
pub fn as_bytes(&self) -> &[u8] { &self.0 }
/// Copies the underlying bytes into a new `Vec`.
#[inline]
#[deprecated(since = "TBD", note = "use to_vec instead")]
pub fn to_bytes(&self) -> alloc::vec::Vec<u8> { self.to_vec() }
/// Converts the object to a raw pointer.
#[inline]
pub fn as_ptr(&self) -> *const $ty {
let &$thing(ref dat) = self;
dat.as_ptr()
}
/// Converts the object to a mutable raw pointer.
#[inline]
pub fn as_mut_ptr(&mut self) -> *mut $ty {
let &mut $thing(ref mut dat) = self;
dat.as_mut_ptr()
}
/// Returns the length of the object as an array.
#[inline]
pub fn len(&self) -> usize { $len }
/// Returns whether the object, as an array, is empty. Always false.
#[inline]
pub fn is_empty(&self) -> bool { false }
}
impl<'a> core::convert::From<[$ty; $len]> for $thing {
fn from(data: [$ty; $len]) -> Self { $thing(data) }
}
impl<'a> core::convert::From<&'a [$ty; $len]> for $thing {
fn from(data: &'a [$ty; $len]) -> Self { $thing(*data) }
}
impl<'a> core::convert::TryFrom<&'a [$ty]> for $thing {
type Error = core::array::TryFromSliceError;
fn try_from(data: &'a [$ty]) -> core::result::Result<Self, Self::Error> {
use core::convert::TryInto;
Ok($thing(data.try_into()?))
}
}
impl AsRef<[$ty; $len]> for $thing {
fn as_ref(&self) -> &[$ty; $len] { &self.0 }
}
impl AsMut<[$ty; $len]> for $thing {
fn as_mut(&mut self) -> &mut [$ty; $len] { &mut self.0 }
}
impl AsRef<[$ty]> for $thing {
fn as_ref(&self) -> &[$ty] { &self.0 }
}
impl AsMut<[$ty]> for $thing {
fn as_mut(&mut self) -> &mut [$ty] { &mut self.0 }
}
impl core::borrow::Borrow<[$ty; $len]> for $thing {
fn borrow(&self) -> &[$ty; $len] { &self.0 }
}
impl core::borrow::BorrowMut<[$ty; $len]> for $thing {
fn borrow_mut(&mut self) -> &mut [$ty; $len] { &mut self.0 }
}
// The following two are valid because `[T; N]: Borrow<[T]>`
impl core::borrow::Borrow<[$ty]> for $thing {
fn borrow(&self) -> &[$ty] { &self.0 }
}
impl core::borrow::BorrowMut<[$ty]> for $thing {
fn borrow_mut(&mut self) -> &mut [$ty] { &mut self.0 }
}
impl<I> core::ops::Index<I> for $thing
where
[$ty]: core::ops::Index<I>,
{
type Output = <[$ty] as core::ops::Index<I>>::Output;
#[inline]
fn index(&self, index: I) -> &Self::Output { &self.0[index] }
}
};
}
pub(crate) use impl_array_newtype;

View File

@ -140,7 +140,7 @@ impl_psbt_hash_de_serialize!(sha256d::Hash);
impl_psbt_de_serialize!(Vec<TapLeafHash>);
impl Serialize for ScriptBuf {
fn serialize(&self) -> Vec<u8> { self.to_bytes() }
fn serialize(&self) -> Vec<u8> { self.to_vec() }
}
impl Deserialize for ScriptBuf {
@ -172,7 +172,7 @@ impl Deserialize for secp256k1::PublicKey {
}
impl Serialize for ecdsa::Signature {
fn serialize(&self) -> Vec<u8> { self.to_bytes() }
fn serialize(&self) -> Vec<u8> { self.to_vec() }
}
impl Deserialize for ecdsa::Signature {
@ -265,7 +265,7 @@ impl Deserialize for XOnlyPublicKey {
}
impl Serialize for taproot::Signature {
fn serialize(&self) -> Vec<u8> { self.to_bytes() }
fn serialize(&self) -> Vec<u8> { self.to_vec() }
}
impl Deserialize for taproot::Signature {

View File

@ -343,7 +343,7 @@ fn finalize_psbt_for_script_path_spend(mut psbt: Psbt) -> Psbt {
script_witness.push(signature.to_vec());
}
for (control_block, (script, _)) in input.tap_scripts.iter() {
script_witness.push(script.to_bytes());
script_witness.push(script.to_vec());
script_witness.push(control_block.serialize());
}
input.final_script_witness = Some(script_witness);

View File

@ -2,125 +2,6 @@
//! Various macros used by the Rust Bitcoin ecosystem.
/// Implements standard array methods for a given wrapper type.
#[macro_export]
macro_rules! impl_array_newtype {
($thing:ident, $ty:ty, $len:literal) => {
impl $thing {
/// Creates `Self` by wrapping `bytes`.
#[inline]
pub fn from_byte_array(bytes: [u8; $len]) -> Self { Self(bytes) }
/// Returns a reference the underlying byte array.
#[inline]
pub fn as_byte_array(&self) -> &[u8; $len] { &self.0 }
/// Returns the underlying byte array.
#[inline]
pub fn to_byte_array(self) -> [u8; $len] {
// We rely on `Copy` being implemented for $thing so conversion
// methods use the correct Rust naming conventions.
fn check_copy<T: Copy>() {}
check_copy::<$thing>();
self.0
}
/// Returns a slice of the underlying bytes.
#[inline]
pub fn as_bytes(&self) -> &[u8] { &self.0 }
/// Copies the underlying bytes into a new `Vec`.
#[cfg(feature = "alloc")]
#[inline]
pub fn to_bytes(&self) -> alloc::vec::Vec<u8> { self.0.to_vec() }
/// Converts the object to a raw pointer.
#[inline]
pub fn as_ptr(&self) -> *const $ty {
let &$thing(ref dat) = self;
dat.as_ptr()
}
/// Converts the object to a mutable raw pointer.
#[inline]
pub fn as_mut_ptr(&mut self) -> *mut $ty {
let &mut $thing(ref mut dat) = self;
dat.as_mut_ptr()
}
/// Returns the length of the object as an array.
#[inline]
pub fn len(&self) -> usize { $len }
/// Returns whether the object, as an array, is empty. Always false.
#[inline]
pub fn is_empty(&self) -> bool { false }
}
impl<'a> core::convert::From<[$ty; $len]> for $thing {
fn from(data: [$ty; $len]) -> Self { $thing(data) }
}
impl<'a> core::convert::From<&'a [$ty; $len]> for $thing {
fn from(data: &'a [$ty; $len]) -> Self { $thing(*data) }
}
impl<'a> core::convert::TryFrom<&'a [$ty]> for $thing {
type Error = core::array::TryFromSliceError;
fn try_from(data: &'a [$ty]) -> core::result::Result<Self, Self::Error> {
use core::convert::TryInto;
Ok($thing(data.try_into()?))
}
}
impl AsRef<[$ty; $len]> for $thing {
fn as_ref(&self) -> &[$ty; $len] { &self.0 }
}
impl AsMut<[$ty; $len]> for $thing {
fn as_mut(&mut self) -> &mut [$ty; $len] { &mut self.0 }
}
impl AsRef<[$ty]> for $thing {
fn as_ref(&self) -> &[$ty] { &self.0 }
}
impl AsMut<[$ty]> for $thing {
fn as_mut(&mut self) -> &mut [$ty] { &mut self.0 }
}
impl core::borrow::Borrow<[$ty; $len]> for $thing {
fn borrow(&self) -> &[$ty; $len] { &self.0 }
}
impl core::borrow::BorrowMut<[$ty; $len]> for $thing {
fn borrow_mut(&mut self) -> &mut [$ty; $len] { &mut self.0 }
}
// The following two are valid because `[T; N]: Borrow<[T]>`
impl core::borrow::Borrow<[$ty]> for $thing {
fn borrow(&self) -> &[$ty] { &self.0 }
}
impl core::borrow::BorrowMut<[$ty]> for $thing {
fn borrow_mut(&mut self) -> &mut [$ty] { &mut self.0 }
}
impl<I> core::ops::Index<I> for $thing
where
[$ty]: core::ops::Index<I>,
{
type Output = <[$ty] as core::ops::Index<I>>::Output;
#[inline]
fn index(&self, index: I) -> &Self::Output { &self.0[index] }
}
};
}
/// Implements `Debug` by calling through to `Display`.
#[macro_export]
macro_rules! debug_from_display {

View File

@ -97,6 +97,15 @@ impl Script {
#[inline]
pub fn as_mut_bytes(&mut self) -> &mut [u8] { &mut self.0 }
/// Returns a copy of the script data.
#[inline]
pub fn to_vec(&self) -> Vec<u8> { self.0.to_owned() }
/// Returns a copy of the script data.
#[inline]
#[deprecated(since = "TBD", note = "use to_vec instead")]
pub fn to_bytes(&self) -> Vec<u8> { self.to_vec() }
/// Returns the length in bytes of the script.
#[inline]
pub fn len(&self) -> usize { self.0.len() }
@ -105,10 +114,6 @@ impl Script {
#[inline]
pub fn is_empty(&self) -> bool { self.0.is_empty() }
/// Returns a copy of the script data.
#[inline]
pub fn to_bytes(&self) -> Vec<u8> { self.0.to_owned() }
/// Converts a [`Box<Script>`](Box) into a [`ScriptBuf`] without copying or allocating.
#[must_use = "`self` will be dropped if the result is not used"]
pub fn into_script_buf(self: Box<Self>) -> ScriptBuf {

View File

@ -26,6 +26,36 @@ impl ScriptBuf {
#[inline]
pub const fn new() -> Self { ScriptBuf(Vec::new()) }
/// Converts byte vector into script.
///
/// This method doesn't (re)allocate.
pub fn from_bytes(bytes: Vec<u8>) -> Self { ScriptBuf(bytes) }
/// Returns a reference to unsized script.
pub fn as_script(&self) -> &Script { Script::from_bytes(&self.0) }
/// Returns a mutable reference to unsized script.
pub fn as_mut_script(&mut self) -> &mut Script { Script::from_bytes_mut(&mut self.0) }
/// Converts the script into a byte vector.
///
/// This method doesn't (re)allocate.
pub fn into_bytes(self) -> Vec<u8> { self.0 }
/// Converts this `ScriptBuf` into a [boxed](Box) [`Script`].
///
/// This method reallocates if the capacity is greater than length of the script but should not
/// when they are equal. If you know beforehand that you need to create a script of exact size
/// use [`reserve_exact`](Self::reserve_exact) before adding data to the script so that the
/// reallocation can be avoided.
#[must_use = "`self` will be dropped if the result is not used"]
#[inline]
pub fn into_boxed_script(self) -> Box<Script> {
// Copied from PathBuf::into_boxed_path
let rw = Box::into_raw(self.0.into_boxed_slice()) as *mut Script;
unsafe { Box::from_raw(rw) }
}
/// Creates a new empty script with pre-allocated capacity.
pub fn with_capacity(capacity: usize) -> Self { ScriptBuf(Vec::with_capacity(capacity)) }
@ -55,36 +85,6 @@ impl ScriptBuf {
///
/// Panics if the new capacity exceeds `isize::MAX bytes`.
pub fn reserve_exact(&mut self, additional_len: usize) { self.0.reserve_exact(additional_len); }
/// Returns a reference to unsized script.
pub fn as_script(&self) -> &Script { Script::from_bytes(&self.0) }
/// Returns a mutable reference to unsized script.
pub fn as_mut_script(&mut self) -> &mut Script { Script::from_bytes_mut(&mut self.0) }
/// Converts byte vector into script.
///
/// This method doesn't (re)allocate.
pub fn from_bytes(bytes: Vec<u8>) -> Self { ScriptBuf(bytes) }
/// Converts the script into a byte vector.
///
/// This method doesn't (re)allocate.
pub fn into_bytes(self) -> Vec<u8> { self.0 }
/// Converts this `ScriptBuf` into a [boxed](Box) [`Script`].
///
/// This method reallocates if the capacity is greater than length of the script but should not
/// when they are equal. If you know beforehand that you need to create a script of exact size
/// use [`reserve_exact`](Self::reserve_exact) before adding data to the script so that the
/// reallocation can be avoided.
#[must_use = "`self` will be dropped if the result is not used"]
#[inline]
pub fn into_boxed_script(self) -> Box<Script> {
// Copied from PathBuf::into_boxed_path
let rw = Box::into_raw(self.0.into_boxed_slice()) as *mut Script;
unsafe { Box::from_raw(rw) }
}
}
#[cfg(feature = "arbitrary")]

View File

@ -17,7 +17,7 @@ use crate::prelude::Vec;
///
/// Can be logically seen as an array of bytestrings, i.e. `Vec<Vec<u8>>`, and it is serialized on the wire
/// in that format. You can convert between this type and `Vec<Vec<u8>>` by using [`Witness::from_slice`]
/// and [`Witness::to_bytes`].
/// and [`Witness::to_vec`].
///
/// For serialization and deserialization performance it is stored internally as a single `Vec`,
/// saving some allocations.
@ -97,7 +97,7 @@ impl Witness {
}
/// Convenience method to create an array of byte-arrays from this witness.
pub fn to_bytes(&self) -> Vec<Vec<u8>> { self.iter().map(|s| s.to_vec()).collect() }
pub fn to_vec(&self) -> Vec<Vec<u8>> { self.iter().map(|s| s.to_vec()).collect() }
/// Returns `true` if the witness contains no element.
pub fn is_empty(&self) -> bool { self.witness_elements == 0 }