milksad
/
rust-bitcoin-unsafe-fast
14
0
Fork 0
Code Issues Pull Requests Activity

Merge rust-bitcoin/rust-bitcoin#798: Audit conversion methods 

5fbb211085 Use fn name to_ instead of as_ (Tobin Harding)
8ffa32315d Use fn name to_ instead of into_ (Tobin Harding)
6874ce91e2 Remove as_inner (Tobin C. Harding)

Pull request description:

  Rust has naming conventions surrounding conversion functions

  We have a handful of methods that are not following convention. This PR is done as three patches, separated by incorrect function name (`into_` or `as_`) and by whether or not the original method needs deprecating. Can be squashed if folks prefer.

  From the docs: https://rust-lang.github.io/api-guidelines/naming.html

  <h2><a class="header" href="https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv" id="ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv">Ad-hoc conversions follow <code>as_</code>, <code>to_</code>, <code>into_</code> conventions (C-CONV)</a></h2>
  <p>Conversions should be provided as methods, with names prefixed as follows:</p>

  Prefix | Cost | Ownership
  -- | -- | --
  as_ | Free | borrowed -> borrowed
  to_ | Expensive | borrowed -> borrowed
  | | | borrowed -> owned (non-Copy types)
  | | | owned -> owned (Copy types)
  into_ | Variable | owned -> owned (non-Copy types)

  EDIT: I did actually audit all uses of `to_` when I first did this, I did this by grepping for `fn to_` and checking the output against the table.

ACKs for top commit:
  apoelstra:
    ACK 5fbb211085
  Kixunil:
    ACK 5fbb211085

Tree-SHA512: f750b2d1a10bc1d4bb030d8528a582701cc3d615aa8a8ab391324dae639544bb3629a19b372784e1e274a8ddcc613c621c7aae21a3ea54fde356a6aa5e611ac0
This commit is contained in:
Andrew Poelstra 2022-06-01 16:19:17 +00:00
parent 8f81fc5aa7 5fbb211085
commit 95548afccf
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
7 changed files with 122 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/blockdata/opcodes.rs
View File

@ -736,7 +736,14 @@ impl All {
/// Encode as a byte
#[inline]
#[deprecated(since = "0.29.0", note = "use to_u8 instead")]
pub fn into_u8(self) -> u8 {
self.to_u8()
}
/// Encodes [`All`] as a byte.
#[inline]
pub fn to_u8(self) -> u8 {
self.code
}
}
@ -859,9 +866,17 @@ ordinary_opcode! {
impl Ordinary {
/// Encode as a byte
#[inline]
#[deprecated(since = "0.29.0", note = "use to_u8 instead")]
pub fn into_u8(self) -> u8 {
self.to_u8()
}
/// Encodes [`All`] as a byte.
#[inline]
pub fn to_u8(self) -> u8 {
self as u8
}
}
#[cfg(test)]
@ -872,7 +887,7 @@ mod tests {
macro_rules! roundtrip {
($unique:expr, $op:ident) => {
assert_eq!(all::$op, All::from(all::$op.into_u8()));
assert_eq!(all::$op, All::from(all::$op.to_u8()));
let s1 = format!("{}", all::$op);
let s2 = format!("{:?}", all::$op);

48
src/blockdata/script.rs
View File

@ -485,20 +485,20 @@ impl Script {
#[inline]
pub fn is_p2sh(&self) -> bool {
self.0.len() == 23
&& self.0[0] == opcodes::all::OP_HASH160.into_u8()
&& self.0[1] == opcodes::all::OP_PUSHBYTES_20.into_u8()
&& self.0[22] == opcodes::all::OP_EQUAL.into_u8()
&& self.0[0] == opcodes::all::OP_HASH160.to_u8()
&& self.0[1] == opcodes::all::OP_PUSHBYTES_20.to_u8()
&& self.0[22] == opcodes::all::OP_EQUAL.to_u8()
}
/// Checks whether a script pubkey is a P2PKH output.
#[inline]
pub fn is_p2pkh(&self) -> bool {
self.0.len() == 25
&& self.0[0] == opcodes::all::OP_DUP.into_u8()
&& self.0[1] == opcodes::all::OP_HASH160.into_u8()
&& self.0[2] == opcodes::all::OP_PUSHBYTES_20.into_u8()
&& self.0[23] == opcodes::all::OP_EQUALVERIFY.into_u8()
&& self.0[24] == opcodes::all::OP_CHECKSIG.into_u8()
&& self.0[0] == opcodes::all::OP_DUP.to_u8()
&& self.0[1] == opcodes::all::OP_HASH160.to_u8()
&& self.0[2] == opcodes::all::OP_PUSHBYTES_20.to_u8()
&& self.0[23] == opcodes::all::OP_EQUALVERIFY.to_u8()
&& self.0[24] == opcodes::all::OP_CHECKSIG.to_u8()
}
/// Checks whether a script pubkey is a P2PK output.
@ -506,12 +506,12 @@ impl Script {
pub fn is_p2pk(&self) -> bool {
match self.len() {
67 => {
self.0[0] == opcodes::all::OP_PUSHBYTES_65.into_u8()
&& self.0[66] == opcodes::all::OP_CHECKSIG.into_u8()
self.0[0] == opcodes::all::OP_PUSHBYTES_65.to_u8()
&& self.0[66] == opcodes::all::OP_CHECKSIG.to_u8()
}
35 => {
self.0[0] == opcodes::all::OP_PUSHBYTES_33.into_u8()
&& self.0[34] == opcodes::all::OP_CHECKSIG.into_u8()
self.0[0] == opcodes::all::OP_PUSHBYTES_33.to_u8()
&& self.0[34] == opcodes::all::OP_CHECKSIG.to_u8()
}
_ => false
}
@ -531,8 +531,8 @@ impl Script {
let ver_opcode = opcodes::All::from(self.0[0]); // Version 0 or PUSHNUM_1-PUSHNUM_16
let push_opbyte = self.0[1]; // Second byte push opcode 2-40 bytes
WitnessVersion::from_opcode(ver_opcode).is_ok()
&& push_opbyte >= opcodes::all::OP_PUSHBYTES_2.into_u8()
&& push_opbyte <= opcodes::all::OP_PUSHBYTES_40.into_u8()
&& push_opbyte >= opcodes::all::OP_PUSHBYTES_2.to_u8()
&& push_opbyte <= opcodes::all::OP_PUSHBYTES_40.to_u8()
// Check that the rest of the script has the correct size
&& script_len - 2 == push_opbyte as usize
}
@ -542,7 +542,7 @@ impl Script {
pub fn is_v0_p2wsh(&self) -> bool {
self.0.len() == 34
&& self.witness_version() == Some(WitnessVersion::V0)
&& self.0[1] == opcodes::all::OP_PUSHBYTES_32.into_u8()
&& self.0[1] == opcodes::all::OP_PUSHBYTES_32.to_u8()
}
/// Checks whether a script pubkey is a P2WPKH output.
@ -550,7 +550,7 @@ impl Script {
pub fn is_v0_p2wpkh(&self) -> bool {
self.0.len() == 22
&& self.witness_version() == Some(WitnessVersion::V0)
&& self.0[1] == opcodes::all::OP_PUSHBYTES_20.into_u8()
&& self.0[1] == opcodes::all::OP_PUSHBYTES_20.to_u8()
}
/// Checks whether a script pubkey is a P2TR output.
@ -558,13 +558,13 @@ impl Script {
pub fn is_v1_p2tr(&self) -> bool {
self.0.len() == 34
&& self.witness_version() == Some(WitnessVersion::V1)
&& self.0[1] == opcodes::all::OP_PUSHBYTES_32.into_u8()
&& self.0[1] == opcodes::all::OP_PUSHBYTES_32.to_u8()
}
/// Check if this is an OP_RETURN output.
pub fn is_op_return (&self) -> bool {
match self.0.first() {
Some(b) => *b == opcodes::all::OP_RETURN.into_u8(),
Some(b) => *b == opcodes::all::OP_RETURN.to_u8(),
None => false
}
}
@ -645,7 +645,7 @@ impl Script {
#[cfg(feature="bitcoinconsensus")]
#[cfg_attr(docsrs, doc(cfg(feature = "bitcoinconsensus")))]
pub fn verify_with_flags<F: Into<u32>>(&self, index: usize, amount: crate::Amount, spending: &[u8], flags: F) -> Result<(), Error> {
Ok(bitcoinconsensus::verify_with_flags (&self.0[..], amount.as_sat(), spending, index, flags.into())?)
Ok(bitcoinconsensus::verify_with_flags (&self.0[..], amount.to_sat(), spending, index, flags.into())?)
}
/// Writes the assembly decoding of the script bytes to the formatter.
@ -879,7 +879,7 @@ impl Builder {
// We can special-case -1, 1-16
if data == -1 || (1..=16).contains(&data) {
let opcode = opcodes::All::from(
(data - 1 + opcodes::OP_TRUE.into_u8() as i64) as u8
(data - 1 + opcodes::OP_TRUE.to_u8() as i64) as u8
);
self.push_opcode(opcode)
}
@ -903,16 +903,16 @@ impl Builder {
match data.len() as u64 {
n if n < opcodes::Ordinary::OP_PUSHDATA1 as u64 => { self.0.push(n as u8); },
n if n < 0x100 => {
self.0.push(opcodes::Ordinary::OP_PUSHDATA1.into_u8());
self.0.push(opcodes::Ordinary::OP_PUSHDATA1.to_u8());
self.0.push(n as u8);
},
n if n < 0x10000 => {
self.0.push(opcodes::Ordinary::OP_PUSHDATA2.into_u8());
self.0.push(opcodes::Ordinary::OP_PUSHDATA2.to_u8());
self.0.push((n % 0x100) as u8);
self.0.push((n / 0x100) as u8);
},
n if n < 0x100000000 => {
self.0.push(opcodes::Ordinary::OP_PUSHDATA4.into_u8());
self.0.push(opcodes::Ordinary::OP_PUSHDATA4.to_u8());
self.0.push((n % 0x100) as u8);
self.0.push(((n / 0x100) % 0x100) as u8);
self.0.push(((n / 0x10000) % 0x100) as u8);
@ -942,7 +942,7 @@ impl Builder {
/// Adds a single opcode to the script.
pub fn push_opcode(mut self, data: opcodes::All) -> Builder {
self.0.push(data.into_u8());
self.0.push(data.to_u8());
self.1 = Some(data);
self
}

2
src/network/address.rs
View File

@ -267,7 +267,7 @@ impl Encodable for AddrV2Message {
fn consensus_encode<W: io::Write>(&self, mut e: W) -> Result<usize, io::Error> {
let mut len = 0;
len += self.time.consensus_encode(&mut e)?;
len += VarInt(self.services.as_u64()).consensus_encode(&mut e)?;
len += VarInt(self.services.to_u64()).consensus_encode(&mut e)?;
len += self.addr.consensus_encode(&mut e)?;
// consensus_encode always encodes in LE, and we want to encode in BE.

6
src/network/constants.rs
View File

@ -178,7 +178,13 @@ impl ServiceFlags {
}
/// Get the integer representation of this [ServiceFlags].
#[deprecated(since = "0.29.0", note = "use to_u64 instead")]
pub fn as_u64(self) -> u64 {
self.to_u64()
}
/// Gets the integer representation of this [`ServiceFlags`].
pub fn to_u64(self) -> u64 {
self.0
}
}

22
src/util/address.rs
View File

@ -296,10 +296,10 @@ impl WitnessVersion {
/// If the opcode does not correspond to any witness version, errors with
/// [`Error::MalformedWitnessVersion`].
pub fn from_opcode(opcode: opcodes::All) -> Result<Self, Error> {
match opcode.into_u8() {
match opcode.to_u8() {
0 => Ok(WitnessVersion::V0),
version if version >= opcodes::all::OP_PUSHNUM_1.into_u8() && version <= opcodes::all::OP_PUSHNUM_16.into_u8() =>
WitnessVersion::from_num(version - opcodes::all::OP_PUSHNUM_1.into_u8() + 1),
version if version >= opcodes::all::OP_PUSHNUM_1.to_u8() && version <= opcodes::all::OP_PUSHNUM_16.to_u8() =>
WitnessVersion::from_num(version - opcodes::all::OP_PUSHNUM_1.to_u8() + 1),
_ => Err(Error::MalformedWitnessVersion)
}
}
@ -326,7 +326,17 @@ impl WitnessVersion {
/// NB: this is not the same as an integer representation of the opcode signifying witness
/// version in bitcoin script. Thus, there is no function to directly convert witness version
/// into a byte since the conversion requires context (bitcoin script or just a version number).
#[deprecated(since = "0.29.0", note = "use to_num instead")]
pub fn into_num(self) -> u8 {
self.to_num()
}
/// Returns integer version number representation for a given [`WitnessVersion`] value.
///
/// NB: this is not the same as an integer representation of the opcode signifying witness
/// version in bitcoin script. Thus, there is no function to directly convert witness version
/// into a byte since the conversion requires context (bitcoin script or just a version number).
pub fn to_num(self) -> u8 {
self as u8
}
@ -342,7 +352,7 @@ impl WitnessVersion {
impl From<WitnessVersion> for ::bech32::u5 {
/// Converts [`WitnessVersion`] instance into corresponding Bech32(m) u5-value ([`bech32::u5`]).
fn from(version: WitnessVersion) -> Self {
::bech32::u5::try_from_u8(version.into_num()).expect("WitnessVersion must be 0..=16")
::bech32::u5::try_from_u8(version.to_num()).expect("WitnessVersion must be 0..=16")
}
}
@ -351,7 +361,7 @@ impl From<WitnessVersion> for opcodes::All {
fn from(version: WitnessVersion) -> opcodes::All {
match version {
WitnessVersion::V0 => opcodes::all::OP_PUSHBYTES_0,
no => opcodes::All::from(opcodes::all::OP_PUSHNUM_1.into_u8() + no.into_num() - 1)
no => opcodes::All::from(opcodes::all::OP_PUSHNUM_1.to_u8() + no.to_num() - 1)
}
}
}
@ -473,7 +483,7 @@ impl Payload {
pub fn p2tr_tweaked(output_key: TweakedPublicKey) -> Payload {
Payload::WitnessProgram {
version: WitnessVersion::V1,
program: output_key.as_inner().serialize().to_vec(),
program: output_key.to_inner().serialize().to_vec(),
}
}

69
src/util/amount.rs
View File

@ -484,7 +484,13 @@ impl Amount {
}
/// Get the number of satoshis in this [Amount].
#[deprecated(since = "0.29.0", note = "use to_sat instead")]
pub fn as_sat(self) -> u64 {
self.to_sat()
}
/// Gets the number of satoshis in this [`Amount`].
pub fn to_sat(self) -> u64 {
self.0
}
@ -543,9 +549,22 @@ impl Amount {
/// Express this [Amount] as a floating-point value in Bitcoin.
///
/// Equivalent to `to_float_in(Denomination::Bitcoin)`.
#[deprecated(since = "0.29.0", note = "use to_btc instead")]
pub fn as_btc(self) -> f64 {
self.to_btc()
}
/// Express this [`Amount`] as a floating-point value in Bitcoin.
///
/// Please be aware of the risk of using floating-point numbers.
pub fn as_btc(self) -> f64 {
///
/// # Examples
/// ```
/// # use bitcoin::{Amount, Denomination};
/// let amount = Amount::from_sat(100_000);
/// assert_eq!(amount.to_btc(), amount.to_float_in(Denomination::Bitcoin))
/// ```
pub fn to_btc(self) -> f64 {
self.to_float_in(Denomination::Bitcoin)
}
@ -566,7 +585,7 @@ impl Amount {
/// Create an object that implements [`fmt::Display`] using specified denomination.
pub fn display_in(self, denomination: Denomination) -> Display {
Display {
sats_abs: self.as_sat(),
sats_abs: self.to_sat(),
is_negative: false,
style: DisplayStyle::FixedDenomination { denomination, show_denomination: false, },
}
@ -578,7 +597,7 @@ impl Amount {
/// avoid confusion the denomination is always shown.
pub fn display_dynamic(self) -> Display {
Display {
sats_abs: self.as_sat(),
sats_abs: self.to_sat(),
is_negative: false,
style: DisplayStyle::DynamicDenomination,
}
@ -588,7 +607,7 @@ impl Amount {
///
/// Does not include the denomination.
pub fn fmt_value_in(self, f: &mut dyn fmt::Write, denom: Denomination) -> fmt::Result {
fmt_satoshi_in(self.as_sat(), false, f, denom, false, FormatOptions::default())
fmt_satoshi_in(self.to_sat(), false, f, denom, false, FormatOptions::default())
}
/// Get a string number of this [Amount] in the given denomination.
@ -645,10 +664,10 @@ impl Amount {
/// Convert to a signed amount.
pub fn to_signed(self) -> Result<SignedAmount, ParseAmountError> {
if self.as_sat() > SignedAmount::max_value().as_sat() as u64 {
if self.to_sat() > SignedAmount::max_value().to_sat() as u64 {
Err(ParseAmountError::TooBig)
} else {
Ok(SignedAmount::from_sat(self.as_sat() as i64))
Ok(SignedAmount::from_sat(self.to_sat() as i64))
}
}
}
@ -661,7 +680,7 @@ impl default::Default for Amount {
impl fmt::Debug for Amount {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "Amount({:.8} BTC)", self.as_btc())
write!(f, "Amount({:.8} BTC)", self.to_btc())
}
}
@ -799,7 +818,7 @@ impl fmt::Display for Display {
let format_options = FormatOptions::from_formatter(f);
match &self.style {
DisplayStyle::FixedDenomination { show_denomination, denomination } => fmt_satoshi_in(self.sats_abs, self.is_negative, f, *denomination, *show_denomination, format_options),
DisplayStyle::DynamicDenomination if self.sats_abs >= Amount::ONE_BTC.as_sat() => {
DisplayStyle::DynamicDenomination if self.sats_abs >= Amount::ONE_BTC.to_sat() => {
fmt_satoshi_in(self.sats_abs, self.is_negative, f, Denomination::Bitcoin, true, format_options)
},
DisplayStyle::DynamicDenomination => {
@ -848,7 +867,13 @@ impl SignedAmount {
}
/// Get the number of satoshis in this [SignedAmount].
#[deprecated(since = "0.29.0", note = "use to_sat instead")]
pub fn as_sat(self) -> i64 {
self.to_sat()
}
/// Gets the number of satoshis in this [`SignedAmount`].
pub fn to_sat(self) -> i64 {
self.0
}
@ -909,7 +934,17 @@ impl SignedAmount {
/// Equivalent to `to_float_in(Denomination::Bitcoin)`.
///
/// Please be aware of the risk of using floating-point numbers.
#[deprecated(since = "0.29.0", note = "use to_btc instead")]
pub fn as_btc(self) -> f64 {
self.to_btc()
}
/// Express this [`SignedAmount`] as a floating-point value in Bitcoin.
///
/// Equivalent to `to_float_in(Denomination::Bitcoin)`.
///
/// Please be aware of the risk of using floating-point numbers.
pub fn to_btc(self) -> f64 {
self.to_float_in(Denomination::Bitcoin)
}
@ -931,7 +966,7 @@ impl SignedAmount {
///
/// This is the implementation of `unsigned_abs()` copied from `core` to support older MSRV.
fn to_sat_abs(self) -> u64 {
self.as_sat().wrapping_abs() as u64
self.to_sat().wrapping_abs() as u64
}
/// Create an object that implements [`fmt::Display`] using specified denomination.
@ -1063,7 +1098,7 @@ impl SignedAmount {
if self.is_negative() {
Err(ParseAmountError::Negative)
} else {
Ok(Amount::from_sat(self.as_sat() as u64))
Ok(Amount::from_sat(self.to_sat() as u64))
}
}
}
@ -1076,7 +1111,7 @@ impl default::Default for SignedAmount {
impl fmt::Debug for SignedAmount {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "SignedAmount({:.8} BTC)", self.as_btc())
write!(f, "SignedAmount({:.8} BTC)", self.to_btc())
}
}
@ -1263,7 +1298,7 @@ pub mod serde {
impl SerdeAmount for Amount {
fn ser_sat<S: Serializer>(self, s: S) -> Result<S::Ok, S::Error> {
u64::serialize(&self.as_sat(), s)
u64::serialize(&self.to_sat(), s)
}
fn des_sat<'d, D: Deserializer<'d>>(d: D) -> Result<Self, D::Error> {
Ok(Amount::from_sat(u64::deserialize(d)?))
@ -1282,16 +1317,16 @@ pub mod serde {
"u"
}
fn ser_sat_opt<S: Serializer>(self, s: S) -> Result<S::Ok, S::Error> {
s.serialize_some(&self.as_sat())
s.serialize_some(&self.to_sat())
}
fn ser_btc_opt<S: Serializer>(self, s: S) -> Result<S::Ok, S::Error> {
s.serialize_some(&self.as_btc())
s.serialize_some(&self.to_btc())
}
}
impl SerdeAmount for SignedAmount {
fn ser_sat<S: Serializer>(self, s: S) -> Result<S::Ok, S::Error> {
i64::serialize(&self.as_sat(), s)
i64::serialize(&self.to_sat(), s)
}
fn des_sat<'d, D: Deserializer<'d>>(d: D) -> Result<Self, D::Error> {
Ok(SignedAmount::from_sat(i64::deserialize(d)?))
@ -1310,10 +1345,10 @@ pub mod serde {
"i"
}
fn ser_sat_opt<S: Serializer>(self, s: S) -> Result<S::Ok, S::Error> {
s.serialize_some(&self.as_sat())
s.serialize_some(&self.to_sat())
}
fn ser_btc_opt<S: Serializer>(self, s: S) -> Result<S::Ok, S::Error> {
s.serialize_some(&self.as_btc())
s.serialize_some(&self.to_btc())
}
}

12
src/util/schnorr.rs
View File

@ -165,11 +165,6 @@ impl TweakedPublicKey {
self.0
}
/// Returns a reference to underlying public key.
pub fn as_inner(&self) -> &crate::XOnlyPublicKey {
&self.0
}
/// Serialize the key as a byte-encoded pair of values. In compressed form
/// the y-coordinate is represented by only a single bit, as x determines
/// it up to one bit.
@ -192,9 +187,16 @@ impl TweakedKeyPair {
/// Returns the underlying key pair
#[inline]
#[deprecated(since = "0.29.0", note = "use to_inner instead")]
pub fn into_inner(self) -> crate::KeyPair {
self.0
}
/// Returns the underlying key pair.
#[inline]
pub fn to_inner(self) -> crate::KeyPair {
self.0
}
}
impl From<TweakedPublicKey> for crate::XOnlyPublicKey {