From 6874ce91e23eb2b2a17e1c8cd9bb2d3efa13eb85 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 31 May 2022 09:58:15 +1000 Subject: [PATCH 1/3] Remove as_inner `self` and the referenced type returned by `as_inner` are both `Copy` types. There is no need to provide an reference getter method to a `Copy` type since implementing `Copy` implies that copying is cheap. --- src/util/address.rs | 2 +- src/util/schnorr.rs | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/util/address.rs b/src/util/address.rs index afa4c714..124024c3 100644 --- a/src/util/address.rs +++ b/src/util/address.rs @@ -473,7 +473,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(), } } diff --git a/src/util/schnorr.rs b/src/util/schnorr.rs index 8d679120..9b514e13 100644 --- a/src/util/schnorr.rs +++ b/src/util/schnorr.rs @@ -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. From 8ffa32315db05c911a269ab8a502587248abcb40 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Wed, 19 Jan 2022 13:41:45 +1100 Subject: [PATCH 2/3] Use fn name to_ instead of into_ Rust convention is to use `to_` for conversion methods that convert from an owned type to an owned `Copy` type. `into_` is for owned to owned non-`Copy` types. Re-name and deprecate conversion methods that use `into_` for `Copy` types to use `to_`. --- src/blockdata/opcodes.rs | 17 ++++++++++++++- src/blockdata/script.rs | 46 ++++++++++++++++++++-------------------- src/util/address.rs | 20 ++++++++++++----- src/util/schnorr.rs | 7 ++++++ 4 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/blockdata/opcodes.rs b/src/blockdata/opcodes.rs index ef1b054b..33e626f0 100644 --- a/src/blockdata/opcodes.rs +++ b/src/blockdata/opcodes.rs @@ -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); diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index 839a7c29..ec688838 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -484,20 +484,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. @@ -505,12 +505,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 } @@ -530,8 +530,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 } @@ -541,7 +541,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. @@ -549,7 +549,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. @@ -557,13 +557,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 } } @@ -878,7 +878,7 @@ impl Builder { // We can special-case -1, 1-16 if data == -1 || (data >= 1 && data <= 16) { 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) } @@ -902,16 +902,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); @@ -941,7 +941,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 } diff --git a/src/util/address.rs b/src/util/address.rs index 124024c3..5697dbed 100644 --- a/src/util/address.rs +++ b/src/util/address.rs @@ -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 { - 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 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 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) } } } diff --git a/src/util/schnorr.rs b/src/util/schnorr.rs index 9b514e13..a71695fb 100644 --- a/src/util/schnorr.rs +++ b/src/util/schnorr.rs @@ -187,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 for crate::XOnlyPublicKey { From 5fbb211085d81d4b4a08b46ee89c63afed9fcc90 Mon Sep 17 00:00:00 2001 From: Tobin Harding Date: Wed, 19 Jan 2022 14:05:01 +1100 Subject: [PATCH 3/3] Use fn name to_ instead of as_ Rust convention is to use `to_` for conversion methods that convert from an owned type to an owned `Copy` type. `as_` is for borrowed to borrowed types. Re-name and deprecate conversion methods that use `as_` for owned to owned `Copy` types to use `to_`. --- src/blockdata/script.rs | 2 +- src/network/address.rs | 2 +- src/network/constants.rs | 6 ++++ src/util/amount.rs | 69 ++++++++++++++++++++++++++++++---------- 4 files changed, 60 insertions(+), 19 deletions(-) diff --git a/src/blockdata/script.rs b/src/blockdata/script.rs index ec688838..73ec1e18 100644 --- a/src/blockdata/script.rs +++ b/src/blockdata/script.rs @@ -644,7 +644,7 @@ impl Script { #[cfg(feature="bitcoinconsensus")] #[cfg_attr(docsrs, doc(cfg(feature = "bitcoinconsensus")))] pub fn verify_with_flags>(&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. diff --git a/src/network/address.rs b/src/network/address.rs index 9338b3d7..d7e1a92c 100644 --- a/src/network/address.rs +++ b/src/network/address.rs @@ -267,7 +267,7 @@ impl Encodable for AddrV2Message { fn consensus_encode(&self, mut e: W) -> Result { 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. diff --git a/src/network/constants.rs b/src/network/constants.rs index 8ccc22cc..554b1da9 100644 --- a/src/network/constants.rs +++ b/src/network/constants.rs @@ -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 } } diff --git a/src/util/amount.rs b/src/util/amount.rs index 18f2e72d..7e05748d 100644 --- a/src/util/amount.rs +++ b/src/util/amount.rs @@ -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 { - 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(self, s: S) -> Result { - u64::serialize(&self.as_sat(), s) + u64::serialize(&self.to_sat(), s) } fn des_sat<'d, D: Deserializer<'d>>(d: D) -> Result { Ok(Amount::from_sat(u64::deserialize(d)?)) @@ -1282,16 +1317,16 @@ pub mod serde { "u" } fn ser_sat_opt(self, s: S) -> Result { - s.serialize_some(&self.as_sat()) + s.serialize_some(&self.to_sat()) } fn ser_btc_opt(self, s: S) -> Result { - s.serialize_some(&self.as_btc()) + s.serialize_some(&self.to_btc()) } } impl SerdeAmount for SignedAmount { fn ser_sat(self, s: S) -> Result { - i64::serialize(&self.as_sat(), s) + i64::serialize(&self.to_sat(), s) } fn des_sat<'d, D: Deserializer<'d>>(d: D) -> Result { Ok(SignedAmount::from_sat(i64::deserialize(d)?)) @@ -1310,10 +1345,10 @@ pub mod serde { "i" } fn ser_sat_opt(self, s: S) -> Result { - s.serialize_some(&self.as_sat()) + s.serialize_some(&self.to_sat()) } fn ser_btc_opt(self, s: S) -> Result { - s.serialize_some(&self.as_btc()) + s.serialize_some(&self.to_btc()) } }