From 16eb81e1f7dd5c3285daf1d64016786c58730339 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Mon, 5 Aug 2019 14:35:51 -0400 Subject: [PATCH 1/4] Replaced slow vec initialization, and dual calls to hashmap --- src/consensus/encode.rs | 4 ++-- src/network/message_blockdata.rs | 2 +- src/util/psbt/macros.rs | 12 ++++++------ src/util/psbt/map/global.rs | 23 ++++++++--------------- src/util/psbt/map/input.rs | 9 +++------ src/util/psbt/map/output.rs | 10 ++++------ 6 files changed, 24 insertions(+), 36 deletions(-) diff --git a/src/consensus/encode.rs b/src/consensus/encode.rs index 9bd8fb45..4f81c42f 100644 --- a/src/consensus/encode.rs +++ b/src/consensus/encode.rs @@ -543,8 +543,8 @@ impl Decodable for [u16; 8] { #[inline] fn consensus_decode(mut d: D) -> Result { let mut res = [0; 8]; - for i in 0..8 { - res[i] = Decodable::consensus_decode(&mut d)?; + for item in &mut res { + *item = Decodable::consensus_decode(&mut d)?; } Ok(res) } diff --git a/src/network/message_blockdata.rs b/src/network/message_blockdata.rs index 3326fb52..e2795ef0 100644 --- a/src/network/message_blockdata.rs +++ b/src/network/message_blockdata.rs @@ -111,7 +111,7 @@ impl GetBlocksMessage { pub fn new(locator_hashes: Vec, stop_hash: BlockHash) -> GetBlocksMessage { GetBlocksMessage { version: constants::PROTOCOL_VERSION, - locator_hashes: locator_hashes.clone(), + locator_hashes: locator_hashes, stop_hash: stop_hash } } diff --git a/src/util/psbt/macros.rs b/src/util/psbt/macros.rs index 7844b7ca..80f6678d 100644 --- a/src/util/psbt/macros.rs +++ b/src/util/psbt/macros.rs @@ -119,12 +119,12 @@ macro_rules! impl_psbt_insert_pair { if !$raw_key.key.is_empty() { let key_val: $keyed_key_type = ::util::psbt::serialize::Deserialize::deserialize(&$raw_key.key)?; - if $slf.$keyed_name.contains_key(&key_val) { - return Err(::util::psbt::Error::DuplicateKey($raw_key).into()); - } else { - let val: $keyed_value_type = ::util::psbt::serialize::Deserialize::deserialize(&$raw_value)?; - - $slf.$keyed_name.insert(key_val, val); + match $slf.$keyed_name.entry(key_val) { + ::std::collections::btree_map::Entry::Vacant(empty_key) => { + let val: $keyed_value_type = ::util::psbt::serialize::Deserialize::deserialize(&$raw_value)?; + empty_key.insert(val); + } + ::std::collections::btree_map::Entry::Occupied(_) => return Err(::util::psbt::Error::DuplicateKey($raw_key).into()), } } else { return Err(::util::psbt::Error::InvalidKey($raw_key).into()); diff --git a/src/util/psbt/map/global.rs b/src/util/psbt/map/global.rs index b34b4114..f6b411d4 100644 --- a/src/util/psbt/map/global.rs +++ b/src/util/psbt/map/global.rs @@ -13,6 +13,7 @@ // use std::collections::BTreeMap; +use std::collections::btree_map::Entry; use std::io::{self, Cursor}; use blockdata::transaction::Transaction; @@ -60,15 +61,10 @@ impl Map for Global { } = pair; match raw_key.type_value { - 0u8 => { - return Err(Error::DuplicateKey(raw_key).into()); - } - _ => { - if self.unknown.contains_key(&raw_key) { - return Err(Error::DuplicateKey(raw_key).into()); - } else { - self.unknown.insert(raw_key, raw_value); - } + 0u8 => return Err(Error::DuplicateKey(raw_key).into()), + _ => match self.unknown.entry(raw_key) { + Entry::Vacant(empty_key) => {empty_key.insert(raw_value);}, + Entry::Occupied(k) => return Err(Error::DuplicateKey(k.key().clone()).into()), } } @@ -158,12 +154,9 @@ impl Decodable for Global { return Err(Error::InvalidKey(pair.key).into()) } } - _ => { - if unknowns.contains_key(&pair.key) { - return Err(Error::DuplicateKey(pair.key).into()); - } else { - unknowns.insert(pair.key, pair.value); - } + _ => match unknowns.entry(pair.key) { + Entry::Vacant(empty_key) => {empty_key.insert(pair.value);}, + Entry::Occupied(k) => return Err(Error::DuplicateKey(k.key().clone()).into()), } } } diff --git a/src/util/psbt/map/input.rs b/src/util/psbt/map/input.rs index 32f19bc5..05a24432 100644 --- a/src/util/psbt/map/input.rs +++ b/src/util/psbt/map/input.rs @@ -112,12 +112,9 @@ impl Map for Input { self.hd_keypaths <= | } } - _ => { - if self.unknown.contains_key(&raw_key) { - return Err(Error::DuplicateKey(raw_key).into()); - } else { - self.unknown.insert(raw_key, raw_value); - } + _ => match self.unknown.entry(raw_key) { + ::std::collections::btree_map::Entry::Vacant(empty_key) => {empty_key.insert(raw_value);}, + ::std::collections::btree_map::Entry::Occupied(k) => return Err(Error::DuplicateKey(k.key().clone()).into()), } } diff --git a/src/util/psbt/map/output.rs b/src/util/psbt/map/output.rs index 8219e812..7e32a421 100644 --- a/src/util/psbt/map/output.rs +++ b/src/util/psbt/map/output.rs @@ -13,6 +13,7 @@ // use std::collections::BTreeMap; +use std::collections::btree_map::Entry; use blockdata::script::Script; use consensus::encode; @@ -61,12 +62,9 @@ impl Map for Output { self.hd_keypaths <= | } } - _ => { - if self.unknown.contains_key(&raw_key) { - return Err(Error::DuplicateKey(raw_key).into()); - } else { - self.unknown.insert(raw_key, raw_value); - } + _ => match self.unknown.entry(raw_key) { + Entry::Vacant(empty_key) => {empty_key.insert(raw_value);}, + Entry::Occupied(k) => return Err(Error::DuplicateKey(k.key().clone()).into()), } } From 3f2d4287061c7b42a9609b21369a7acdab679091 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Mon, 5 Aug 2019 14:52:34 -0400 Subject: [PATCH 2/4] Remove needless references --- src/blockdata/opcodes.rs | 34 +++++++++++++++++----------------- src/blockdata/transaction.rs | 6 +++--- src/network/constants.rs | 13 ++++++------- src/util/amount.rs | 32 ++++++++++++++++---------------- src/util/bip158.rs | 4 ++-- src/util/bip32.rs | 16 ++++++++-------- src/util/psbt/mod.rs | 4 ++-- 7 files changed, 54 insertions(+), 55 deletions(-) diff --git a/src/blockdata/opcodes.rs b/src/blockdata/opcodes.rs index 724b16c2..697eee54 100644 --- a/src/blockdata/opcodes.rs +++ b/src/blockdata/opcodes.rs @@ -658,29 +658,29 @@ impl fmt::Debug for All { impl All { /// Classifies an Opcode into a broad class #[inline] - pub fn classify(&self) -> Class { + pub fn classify(self) -> Class { // 17 opcodes - if *self == all::OP_VERIF || *self == all::OP_VERNOTIF || - *self == all::OP_CAT || *self == all::OP_SUBSTR || - *self == all::OP_LEFT || *self == all::OP_RIGHT || - *self == all::OP_INVERT || *self == all::OP_AND || - *self == all::OP_OR || *self == all::OP_XOR || - *self == all::OP_2MUL || *self == all::OP_2DIV || - *self == all::OP_MUL || *self == all::OP_DIV || *self == all::OP_MOD || - *self == all::OP_LSHIFT || *self == all::OP_RSHIFT { + if self == all::OP_VERIF || self == all::OP_VERNOTIF || + self == all::OP_CAT || self == all::OP_SUBSTR || + self == all::OP_LEFT || self == all::OP_RIGHT || + self == all::OP_INVERT || self == all::OP_AND || + self == all::OP_OR || self == all::OP_XOR || + self == all::OP_2MUL || self == all::OP_2DIV || + self == all::OP_MUL || self == all::OP_DIV || self == all::OP_MOD || + self == all::OP_LSHIFT || self == all::OP_RSHIFT { Class::IllegalOp // 11 opcodes - } else if *self == all::OP_NOP || + } else if self == all::OP_NOP || (all::OP_NOP1.code <= self.code && self.code <= all::OP_NOP10.code) { Class::NoOp // 75 opcodes - } else if *self == all::OP_RESERVED || *self == all::OP_VER || *self == all::OP_RETURN || - *self == all::OP_RESERVED1 || *self == all::OP_RESERVED2 || + } else if self == all::OP_RESERVED || self == all::OP_VER || self == all::OP_RETURN || + self == all::OP_RESERVED1 || self == all::OP_RESERVED2 || self.code >= all::OP_RETURN_186.code { Class::ReturnOp // 1 opcode - } else if *self == all::OP_PUSHNUM_NEG1 { + } else if self == all::OP_PUSHNUM_NEG1 { Class::PushNum(-1) // 16 opcodes } else if all::OP_PUSHNUM_1.code <= self.code && @@ -691,13 +691,13 @@ impl All { Class::PushBytes(self.code as u32) // 60 opcodes } else { - Class::Ordinary(Ordinary::try_from_all(*self).unwrap()) + Class::Ordinary(Ordinary::try_from_all(self).unwrap()) } } /// Encode as a byte #[inline] - pub fn into_u8(&self) -> u8 { + pub fn into_u8(self) -> u8 { self.code } } @@ -809,8 +809,8 @@ ordinary_opcode! { impl Ordinary { /// Encode as a byte #[inline] - pub fn into_u8(&self) -> u8 { - *self as u8 + pub fn into_u8(self) -> u8 { + self as u8 } } diff --git a/src/blockdata/transaction.rs b/src/blockdata/transaction.rs index 6e83f97b..99b87f22 100644 --- a/src/blockdata/transaction.rs +++ b/src/blockdata/transaction.rs @@ -598,8 +598,8 @@ pub enum SigHashType { impl SigHashType { /// Break the sighash flag into the "real" sighash flag and the ANYONECANPAY boolean - pub(crate) fn split_anyonecanpay_flag(&self) -> (SigHashType, bool) { - match *self { + pub(crate) fn split_anyonecanpay_flag(self) -> (SigHashType, bool) { + match self { SigHashType::All => (SigHashType::All, false), SigHashType::None => (SigHashType::None, false), SigHashType::Single => (SigHashType::Single, false), @@ -626,7 +626,7 @@ impl SigHashType { } /// Converts to a u32 - pub fn as_u32(&self) -> u32 { *self as u32 } + pub fn as_u32(self) -> u32 { self as u32 } } diff --git a/src/network/constants.rs b/src/network/constants.rs index 8724398a..0540981a 100644 --- a/src/network/constants.rs +++ b/src/network/constants.rs @@ -89,9 +89,9 @@ impl Network { /// let network = Network::Bitcoin; /// assert_eq!(network.magic(), 0xD9B4BEF9); /// ``` - pub fn magic(&self) -> u32 { + pub fn magic(self) -> u32 { // Note: any new entries here must be added to `from_magic` above - match *self { + match self { Network::Bitcoin => 0xD9B4BEF9, Network::Testnet => 0x0709110B, Network::Regtest => 0xDAB5BFFA, @@ -154,12 +154,12 @@ impl ServiceFlags { } /// Check whether [ServiceFlags] are included in this one. - pub fn has(&self, flags: ServiceFlags) -> bool { + pub fn has(self, flags: ServiceFlags) -> bool { (self.0 | flags.0) == self.0 } /// Get the integer representation of this [ServiceFlags]. - pub fn as_u64(&self) -> u64 { + pub fn as_u64(self) -> u64 { self.0 } } @@ -178,11 +178,10 @@ impl fmt::UpperHex for ServiceFlags { impl fmt::Display for ServiceFlags { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if *self == ServiceFlags::NONE { + let mut flags = *self; + if flags == ServiceFlags::NONE { return write!(f, "ServiceFlags(NONE)"); } - - let mut flags = self.clone(); let mut first = true; macro_rules! write_flag { ($f:ident) => { diff --git a/src/util/amount.rs b/src/util/amount.rs index 1bbb6ec6..31cb31f6 100644 --- a/src/util/amount.rs +++ b/src/util/amount.rs @@ -340,7 +340,7 @@ impl Amount { /// Express this [Amount] as a floating-point value in the given denomination. /// /// Please be aware of the risk of using floating-point numbers. - pub fn to_float_in(&self, denom: Denomination) -> f64 { + pub fn to_float_in(self, denom: Denomination) -> f64 { f64::from_str(&self.to_string_in(denom)).unwrap() } @@ -349,7 +349,7 @@ impl Amount { /// Equivalent to `to_float_in(Denomination::Bitcoin)`. /// /// Please be aware of the risk of using floating-point numbers. - pub fn as_btc(&self) -> f64 { + pub fn as_btc(self) -> f64 { self.to_float_in(Denomination::Bitcoin) } @@ -370,14 +370,14 @@ impl Amount { /// Format the value of this [Amount] in the given denomination. /// /// Does not include the denomination. - pub fn fmt_value_in(&self, f: &mut fmt::Write, denom: Denomination) -> fmt::Result { + pub fn fmt_value_in(self, f: &mut fmt::Write, denom: Denomination) -> fmt::Result { fmt_satoshi_in(self.as_sat(), false, f, denom) } /// Get a string number of this [Amount] in the given denomination. /// /// Does not include the denomination. - pub fn to_string_in(&self, denom: Denomination) -> String { + pub fn to_string_in(self, denom: Denomination) -> String { let mut buf = String::new(); self.fmt_value_in(&mut buf, denom).unwrap(); buf @@ -385,7 +385,7 @@ impl Amount { /// Get a formatted string of this [Amount] in the given denomination, /// suffixed with the abbreviation for the denomination. - pub fn to_string_with_denomination(&self, denom: Denomination) -> String { + pub fn to_string_with_denomination(self, denom: Denomination) -> String { let mut buf = String::new(); self.fmt_value_in(&mut buf, denom).unwrap(); write!(buf, " {}", denom).unwrap(); @@ -637,7 +637,7 @@ impl SignedAmount { /// Express this [SignedAmount] as a floating-point value in the given denomination. /// /// Please be aware of the risk of using floating-point numbers. - pub fn to_float_in(&self, denom: Denomination) -> f64 { + pub fn to_float_in(self, denom: Denomination) -> f64 { f64::from_str(&self.to_string_in(denom)).unwrap() } @@ -646,7 +646,7 @@ impl SignedAmount { /// Equivalent to `to_float_in(Denomination::Bitcoin)`. /// /// Please be aware of the risk of using floating-point numbers. - pub fn as_btc(&self) -> f64 { + pub fn as_btc(self) -> f64 { self.to_float_in(Denomination::Bitcoin) } @@ -667,7 +667,7 @@ impl SignedAmount { /// Format the value of this [SignedAmount] in the given denomination. /// /// Does not include the denomination. - pub fn fmt_value_in(&self, f: &mut fmt::Write, denom: Denomination) -> fmt::Result { + pub fn fmt_value_in(self, f: &mut fmt::Write, denom: Denomination) -> fmt::Result { let sats = self.as_sat().checked_abs().map(|a: i64| a as u64).unwrap_or_else(|| { // We could also hard code this into `9223372036854775808` u64::max_value() - self.as_sat() as u64 +1 @@ -678,7 +678,7 @@ impl SignedAmount { /// Get a string number of this [SignedAmount] in the given denomination. /// /// Does not include the denomination. - pub fn to_string_in(&self, denom: Denomination) -> String { + pub fn to_string_in(self, denom: Denomination) -> String { let mut buf = String::new(); self.fmt_value_in(&mut buf, denom).unwrap(); buf @@ -686,7 +686,7 @@ impl SignedAmount { /// Get a formatted string of this [SignedAmount] in the given denomination, /// suffixed with the abbreviation for the denomination. - pub fn to_string_with_denomination(&self, denom: Denomination) -> String { + pub fn to_string_with_denomination(self, denom: Denomination) -> String { let mut buf = String::new(); self.fmt_value_in(&mut buf, denom).unwrap(); write!(buf, " {}", denom).unwrap(); @@ -1313,12 +1313,12 @@ mod tests { use super::Denomination as D; let amt = Amount::from_sat(42); let denom = Amount::to_string_with_denomination; - assert_eq!(Amount::from_str(&denom(&amt, D::Bitcoin)), Ok(amt)); - assert_eq!(Amount::from_str(&denom(&amt, D::MilliBitcoin)), Ok(amt)); - assert_eq!(Amount::from_str(&denom(&amt, D::MicroBitcoin)), Ok(amt)); - assert_eq!(Amount::from_str(&denom(&amt, D::Bit)), Ok(amt)); - assert_eq!(Amount::from_str(&denom(&amt, D::Satoshi)), Ok(amt)); - assert_eq!(Amount::from_str(&denom(&amt, D::MilliSatoshi)), Ok(amt)); + assert_eq!(Amount::from_str(&denom(amt, D::Bitcoin)), Ok(amt)); + assert_eq!(Amount::from_str(&denom(amt, D::MilliBitcoin)), Ok(amt)); + assert_eq!(Amount::from_str(&denom(amt, D::MicroBitcoin)), Ok(amt)); + assert_eq!(Amount::from_str(&denom(amt, D::Bit)), Ok(amt)); + assert_eq!(Amount::from_str(&denom(amt, D::Satoshi)), Ok(amt)); + assert_eq!(Amount::from_str(&denom(amt, D::MilliSatoshi)), Ok(amt)); assert_eq!(Amount::from_str("42 satoshi BTC"), Err(ParseAmountError::InvalidFormat)); assert_eq!(SignedAmount::from_str("-42 satoshi BTC"), Err(ParseAmountError::InvalidFormat)); diff --git a/src/util/bip158.rs b/src/util/bip158.rs index b7154bfe..d196a11d 100644 --- a/src/util/bip158.rs +++ b/src/util/bip158.rs @@ -241,7 +241,7 @@ impl GCSFilterReader { pub fn match_any(&self, reader: &mut io::Read, query: &mut Iterator) -> Result { let mut decoder = reader; let n_elements: VarInt = Decodable::consensus_decode(&mut decoder).unwrap_or(VarInt(0)); - let ref mut reader = decoder; + let reader = &mut decoder; // map hashes to [0, n_elements << grp] let nm = n_elements.0 * self.m; let mut mapped = query.map(|e| map_to_range(self.filter.hash(e), nm)).collect::>(); @@ -281,7 +281,7 @@ impl GCSFilterReader { pub fn match_all(&self, reader: &mut io::Read, query: &mut Iterator) -> Result { let mut decoder = reader; let n_elements: VarInt = Decodable::consensus_decode(&mut decoder).unwrap_or(VarInt(0)); - let ref mut reader = decoder; + let reader = &mut decoder; // map hashes to [0, n_elements << grp] let nm = n_elements.0 * self.m; let mut mapped = query.map(|e| map_to_range(self.filter.hash(e), nm)).collect::>(); diff --git a/src/util/bip32.rs b/src/util/bip32.rs index 470e59e1..d66167ec 100644 --- a/src/util/bip32.rs +++ b/src/util/bip32.rs @@ -124,15 +124,15 @@ impl ChildNumber { /// Returns `true` if the child number is a [`Normal`] value. /// /// [`Normal`]: #variant.Normal - pub fn is_normal(&self) -> bool { + pub fn is_normal(self) -> bool { !self.is_hardened() } /// Returns `true` if the child number is a [`Hardened`] value. /// /// [`Hardened`]: #variant.Hardened - pub fn is_hardened(&self) -> bool { - match *self { + pub fn is_hardened(self) -> bool { + match self { ChildNumber::Hardened {..} => true, ChildNumber::Normal {..} => false, } @@ -548,7 +548,7 @@ impl ExtendedPubKey { i: ChildNumber, ) -> Result { let (sk, chain_code) = self.ckd_pub_tweak(i)?; - let mut pk = self.public_key.clone(); + let mut pk = self.public_key; pk.key.add_exp_assign(secp, &sk[..]).map_err(Error::Ecdsa)?; Ok(ExtendedPubKey { @@ -604,9 +604,9 @@ impl FromStr for ExtendedPrivKey { let cn_int: u32 = endian::slice_to_u32_be(&data[9..13]); let child_number: ChildNumber = ChildNumber::from(cn_int); - let network = if &data[0..4] == [0x04u8, 0x88, 0xAD, 0xE4] { + let network = if data[0..4] == [0x04u8, 0x88, 0xAD, 0xE4] { Network::Bitcoin - } else if &data[0..4] == [0x04u8, 0x35, 0x83, 0x94] { + } else if data[0..4] == [0x04u8, 0x35, 0x83, 0x94] { Network::Testnet } else { return Err(base58::Error::InvalidVersion((&data[0..4]).to_vec())); @@ -661,9 +661,9 @@ impl FromStr for ExtendedPubKey { let child_number: ChildNumber = ChildNumber::from(cn_int); Ok(ExtendedPubKey { - network: if &data[0..4] == [0x04u8, 0x88, 0xB2, 0x1E] { + network: if data[0..4] == [0x04u8, 0x88, 0xB2, 0x1E] { Network::Bitcoin - } else if &data[0..4] == [0x04u8, 0x35, 0x87, 0xCF] { + } else if data[0..4] == [0x04u8, 0x35, 0x87, 0xCF] { Network::Testnet } else { return Err(base58::Error::InvalidVersion((&data[0..4]).to_vec())); diff --git a/src/util/psbt/mod.rs b/src/util/psbt/mod.rs index d281f1ce..985039f8 100644 --- a/src/util/psbt/mod.rs +++ b/src/util/psbt/mod.rs @@ -67,8 +67,8 @@ impl PartiallySignedTransaction { let mut tx: Transaction = self.global.unsigned_tx; for (vin, psbtin) in tx.input.iter_mut().zip(self.inputs.into_iter()) { - vin.script_sig = psbtin.final_script_sig.unwrap_or_else(|| Script::new()); - vin.witness = psbtin.final_script_witness.unwrap_or_else(|| Vec::new()); + vin.script_sig = psbtin.final_script_sig.unwrap_or_else(Script::new); + vin.witness = psbtin.final_script_witness.unwrap_or_else(Vec::new); } tx From a473d01b1710bb556e4c8513ed72c939b8abf307 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Mon, 5 Aug 2019 15:41:07 -0400 Subject: [PATCH 3/4] Made some idiomatic changes --- src/blockdata/block.rs | 2 +- src/consensus/encode.rs | 6 ++--- src/network/address.rs | 6 ++--- src/network/message.rs | 4 +-- src/util/address.rs | 30 ++++++++++------------ src/util/amount.rs | 55 ++++++++++++++++++++++------------------- src/util/base58.rs | 2 +- src/util/bip158.rs | 46 +++++++++++++++++----------------- src/util/bip32.rs | 20 ++++++--------- src/util/hash.rs | 2 +- src/util/merkleblock.rs | 11 +-------- src/util/misc.rs | 4 +-- src/util/psbt/macros.rs | 2 +- 13 files changed, 87 insertions(+), 103 deletions(-) diff --git a/src/blockdata/block.rs b/src/blockdata/block.rs index 059ab9ae..c081609b 100644 --- a/src/blockdata/block.rs +++ b/src/blockdata/block.rs @@ -78,7 +78,7 @@ impl Block { if self.txdata.iter().all(|t| t.input.iter().all(|i| i.witness.is_empty())) { return true; } - if self.txdata.len() > 0 { + if !self.txdata.is_empty() { let coinbase = &self.txdata[0]; if coinbase.is_coin_base() { // commitment is in the last output that starts with below magic diff --git a/src/consensus/encode.rs b/src/consensus/encode.rs index 4f81c42f..e3a38396 100644 --- a/src/consensus/encode.rs +++ b/src/consensus/encode.rs @@ -162,7 +162,7 @@ pub fn serialize_hex(data: &T) -> String { /// Deserialize an object from a vector, will error if said deserialization /// doesn't consume the entire vector. -pub fn deserialize<'a, T: Decodable>(data: &'a [u8]) -> Result { +pub fn deserialize(data: &[u8]) -> Result { let (rv, consumed) = deserialize_partial(data)?; // Fail if data are not consumed entirely. @@ -175,8 +175,8 @@ pub fn deserialize<'a, T: Decodable>(data: &'a [u8]) -> Result { /// Deserialize an object from a vector, but will not report an error if said deserialization /// doesn't consume the entire vector. -pub fn deserialize_partial<'a, T: Decodable>( - data: &'a [u8], +pub fn deserialize_partial( + data: &[u8], ) -> Result<(T, usize), Error> { let mut decoder = Cursor::new(data); let rv = Decodable::consensus_decode(&mut decoder)?; diff --git a/src/network/address.rs b/src/network/address.rs index f9dcbf0b..6edc5bb5 100644 --- a/src/network/address.rs +++ b/src/network/address.rs @@ -41,9 +41,9 @@ const ONION : [u16; 3] = [0xFD87, 0xD87E, 0xEB43]; impl Address { /// Create an address message for a socket pub fn new (socket :&SocketAddr, services: ServiceFlags) -> Address { - let (address, port) = match socket { - &SocketAddr::V4(ref addr) => (addr.ip().to_ipv6_mapped().segments(), addr.port()), - &SocketAddr::V6(ref addr) => (addr.ip().segments(), addr.port()) + let (address, port) = match *socket { + SocketAddr::V4(addr) => (addr.ip().to_ipv6_mapped().segments(), addr.port()), + SocketAddr::V6(addr) => (addr.ip().segments(), addr.port()) }; Address { address: address, port: port, services: services } } diff --git a/src/network/message.rs b/src/network/message.rs index 5710023c..5147252b 100644 --- a/src/network/message.rs +++ b/src/network/message.rs @@ -72,9 +72,7 @@ impl Encodable for CommandString { if strbytes.len() > 12 { return Err(encode::Error::UnrecognizedNetworkCommand(self.0.clone().into_owned())); } - for x in 0..strbytes.len() { - rawbytes[x] = strbytes[x]; - } + rawbytes[..strbytes.len()].clone_from_slice(&strbytes[..]); rawbytes.consensus_encode(s) } } diff --git a/src/util/address.rs b/src/util/address.rs index 7715736f..dcb418d5 100644 --- a/src/util/address.rs +++ b/src/util/address.rs @@ -18,26 +18,22 @@ //! # Example: creating a new address from a randomly-generated key pair //! //! ```rust -//! extern crate secp256k1; -//! extern crate bitcoin; //! //! use bitcoin::network::constants::Network; //! use bitcoin::util::address::Address; //! use bitcoin::util::key; -//! use secp256k1::Secp256k1; -//! use secp256k1::rand::thread_rng; +//! use bitcoin::secp256k1::Secp256k1; +//! use bitcoin::secp256k1::rand::thread_rng; //! -//! fn main() { -//! // Generate random key pair -//! let s = Secp256k1::new(); -//! let public_key = key::PublicKey { -//! compressed: true, -//! key: s.generate_keypair(&mut thread_rng()).1, -//! }; +//! // Generate random key pair +//! let s = Secp256k1::new(); +//! let public_key = key::PublicKey { +//! compressed: true, +//! key: s.generate_keypair(&mut thread_rng()).1, +//! }; //! -//! // Generate pay-to-pubkey-hash address -//! let address = Address::p2pkh(&public_key, Network::Bitcoin); -//! } +//! // Generate pay-to-pubkey-hash address +//! let address = Address::p2pkh(&public_key, Network::Bitcoin); //! ``` use std::fmt::{self, Display, Formatter}; @@ -219,7 +215,7 @@ impl Payload { assert!(ver.to_u8() <= 16); let mut verop = ver.to_u8(); if verop > 0 { - verop = 0x50 + verop; + verop += 0x50; } script::Builder::new().push_opcode(verop.into()).push_slice(&prog) } @@ -406,7 +402,7 @@ impl Display for Address { /// Returns the same slice when no prefix is found. fn find_bech32_prefix(bech32: &str) -> &str { // Split at the last occurrence of the separator character '1'. - match bech32.rfind("1") { + match bech32.rfind('1') { None => bech32, Some(sep) => bech32.split_at(sep).0, } @@ -427,7 +423,7 @@ impl FromStr for Address { if let Some(network) = bech32_network { // decode as bech32 let (_, payload) = bech32::decode(s)?; - if payload.len() == 0 { + if payload.is_empty() { return Err(Error::EmptyBech32Payload); } diff --git a/src/util/amount.rs b/src/util/amount.rs index 31cb31f6..1fea2b15 100644 --- a/src/util/amount.rs +++ b/src/util/amount.rs @@ -19,6 +19,7 @@ use std::error; use std::fmt::{self, Write}; use std::ops; use std::str::FromStr; +use std::cmp::Ordering; /// A set of denominations in which amounts can be expressed. #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] @@ -131,7 +132,7 @@ impl error::Error for ParseAmountError { fn is_too_precise(s: &str, precision: usize) -> bool { - s.contains(".") || precision >= s.len() || s.chars().rev().take(precision).any(|d| d != '0') + s.contains('.') || precision >= s.len() || s.chars().rev().take(precision).any(|d| d != '0') } /// Parse decimal string in the given denomination into a satoshi value and a @@ -140,14 +141,14 @@ fn parse_signed_to_satoshi( mut s: &str, denom: Denomination, ) -> Result<(bool, u64), ParseAmountError> { - if s.len() == 0 { + if s.is_empty() { return Err(ParseAmountError::InvalidFormat); } if s.len() > 50 { return Err(ParseAmountError::InputTooLarge); } - let is_negative = s.chars().next().unwrap() == '-'; + let is_negative = s.starts_with('-'); if is_negative { if s.len() == 1 { return Err(ParseAmountError::InvalidFormat); @@ -229,27 +230,29 @@ fn fmt_satoshi_in( f.write_str("-")?; } - if denom.precision() > 0 { - // add zeroes in the end - let width = denom.precision() as usize; - write!(f, "{}{:0width$}", satoshi, 0, width = width)?; - } else if denom.precision() < 0 { - // need to inject a comma in the number - let nb_decimals = denom.precision().abs() as usize; - let real = format!("{:0width$}", satoshi, width = nb_decimals); - if real.len() == nb_decimals { - write!(f, "0.{}", &real[real.len() - nb_decimals..])?; - } else { - write!( - f, - "{}.{}", - &real[0..(real.len() - nb_decimals)], - &real[real.len() - nb_decimals..] - )?; + let precision = denom.precision(); + match precision.cmp(&0) { + Ordering::Greater => { + // add zeroes in the end + let width = precision as usize; + write!(f, "{}{:0width$}", satoshi, 0, width = width)?; } - } else { - // denom.precision() == 0 - write!(f, "{}", satoshi)?; + Ordering::Less => { + // need to inject a comma in the number + let nb_decimals = precision.abs() as usize; + let real = format!("{:0width$}", satoshi, width = nb_decimals); + if real.len() == nb_decimals { + write!(f, "0.{}", &real[real.len() - nb_decimals..])?; + } else { + write!( + f, + "{}.{}", + &real[0..(real.len() - nb_decimals)], + &real[real.len() - nb_decimals..] + )?; + } + } + Ordering::Equal => write!(f, "{}", satoshi)?, } Ok(()) } @@ -327,7 +330,7 @@ impl Amount { /// If you want to parse only the amount without the denomination, /// use [from_str_in]. pub fn from_str_with_denomination(s: &str) -> Result { - let mut split = s.splitn(3, " "); + let mut split = s.splitn(3, ' '); let amt_str = split.next().unwrap(); let denom_str = split.next().ok_or(ParseAmountError::InvalidFormat)?; if split.next().is_some() { @@ -614,7 +617,7 @@ impl SignedAmount { return Err(ParseAmountError::TooBig); } Ok(match negative { - true => SignedAmount(-1 * satoshi as i64), + true => SignedAmount(-(satoshi as i64)), false => SignedAmount(satoshi as i64), }) } @@ -624,7 +627,7 @@ impl SignedAmount { /// If you want to parse only the amount without the denomination, /// use [from_str_in]. pub fn from_str_with_denomination(s: &str) -> Result { - let mut split = s.splitn(3, " "); + let mut split = s.splitn(3, ' '); let amt_str = split.next().unwrap(); let denom_str = split.next().ok_or(ParseAmountError::InvalidFormat)?; if split.next().is_some() { diff --git a/src/util/base58.rs b/src/util/base58.rs index 68a0a2b7..2134b5c6 100644 --- a/src/util/base58.rs +++ b/src/util/base58.rs @@ -103,7 +103,7 @@ impl SmallVec { } } -static BASE58_CHARS: &'static [u8] = b"123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz"; +static BASE58_CHARS: &[u8] = b"123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz"; static BASE58_DIGITS: [Option; 128] = [ None, None, None, None, None, None, None, None, // 0-7 diff --git a/src/util/bip158.rs b/src/util/bip158.rs index d196a11d..4cb896a5 100644 --- a/src/util/bip158.rs +++ b/src/util/bip158.rs @@ -49,6 +49,8 @@ use std::collections::HashSet; use std::error; use std::fmt::{Display, Formatter}; use std::io::Cursor; +use std::cmp::Ordering; + use hashes::{Hash, siphash24}; use hash_types::{BlockHash, FilterHash}; @@ -260,17 +262,17 @@ impl GCSFilterReader { let mut remaining = n_elements.0 - 1; for p in mapped { loop { - if data == p { - return Ok(true); - } else if data < p { - if remaining > 0 { - data += self.filter.golomb_rice_decode(&mut reader)?; - remaining -= 1; - } else { - return Ok(false); + match data.cmp(&p) { + Ordering::Equal => return Ok(true), + Ordering::Less => { + if remaining > 0 { + data += self.filter.golomb_rice_decode(&mut reader)?; + remaining -= 1; + } else { + return Ok(false); + } } - } else { - break; + Ordering::Greater => break, } } } @@ -301,17 +303,17 @@ impl GCSFilterReader { let mut remaining = n_elements.0 - 1; for p in mapped { loop { - if data == p { - break; - } else if data < p { - if remaining > 0 { - data += self.filter.golomb_rice_decode(&mut reader)?; - remaining -= 1; - } else { - return Ok(false); - } - } else { - return Ok(false); + match data.cmp(&p) { + Ordering::Equal => break, + Ordering::Less => { + if remaining > 0 { + data += self.filter.golomb_rice_decode(&mut reader)?; + remaining -= 1; + } else { + return Ok(false); + } + }, + Ordering::Greater => return Ok(false), } } } @@ -423,7 +425,7 @@ impl GCSFilter { q += 1; } let r = reader.read(self.p)?; - return Ok((q << self.p) + r); + Ok((q << self.p) + r) } /// Hash an arbitrary slice with siphash using parameters of this filter diff --git a/src/util/bip32.rs b/src/util/bip32.rs index d66167ec..be3e189c 100644 --- a/src/util/bip32.rs +++ b/src/util/bip32.rs @@ -179,13 +179,11 @@ impl FromStr for ChildNumber { type Err = Error; fn from_str(inp: &str) -> Result { - Ok(match inp.chars().last().map_or(false, |l| l == '\'' || l == 'h') { - true => ChildNumber::from_hardened_idx( - inp[0..inp.len() - 1].parse().map_err(|_| Error::InvalidChildNumberFormat)? - )?, - false => ChildNumber::from_normal_idx( - inp.parse().map_err(|_| Error::InvalidChildNumberFormat)? - )?, + let is_hardened = inp.chars().last().map_or(false, |l| l == '\'' || l == 'h'); + Ok(if is_hardened { + ChildNumber::from_hardened_idx(inp[0..inp.len() - 1].parse().map_err(|_| Error::InvalidChildNumberFormat)?)? + } else { + ChildNumber::from_normal_idx(inp.parse().map_err(|_| Error::InvalidChildNumberFormat)?)? }) } } @@ -258,7 +256,7 @@ impl FromStr for DerivationPath { type Err = Error; fn from_str(path: &str) -> Result { - let mut parts = path.split("/"); + let mut parts = path.split('/'); // First parts must be `m`. if parts.next().unwrap() != "m" { return Err(Error::InvalidDerivationPathFormat); @@ -292,11 +290,7 @@ impl<'a> Iterator for DerivationPathIterator<'a> { type Item = DerivationPath; fn next(&mut self) -> Option { - if self.next_child.is_none() { - return None; - } - - let ret = self.next_child.unwrap(); + let ret = self.next_child?; self.next_child = ret.increment().ok(); Some(self.base.child(ret)) } diff --git a/src/util/hash.rs b/src/util/hash.rs index ba7b6d20..fa089135 100644 --- a/src/util/hash.rs +++ b/src/util/hash.rs @@ -30,7 +30,7 @@ pub fn bitcoin_merkle_root_inline(data: &mut [T]) -> T ::Engine: io::Write, { // Base case - if data.len() < 1 { + if data.is_empty() { return Default::default(); } if data.len() < 2 { diff --git a/src/util/merkleblock.rs b/src/util/merkleblock.rs index 2c097d9b..d4ce3973 100644 --- a/src/util/merkleblock.rs +++ b/src/util/merkleblock.rs @@ -25,12 +25,10 @@ //! # Examples //! //! ```rust -//! extern crate bitcoin; //! use bitcoin::hash_types::Txid; //! use bitcoin::hashes::hex::FromHex; //! use bitcoin::{Block, MerkleBlock}; //! -//! # fn main() { //! // Get the proof from a bitcoind by running in the terminal: //! // $ TXID="5a4ebf66822b0b2d56bd9dc64ece0bc38ee7844a23ff1d7320a88c5fdb2ad3e2" //! // $ bitcoin-cli gettxoutproof [\"$TXID\"] @@ -52,7 +50,6 @@ //! ); //! assert_eq!(1, index.len()); //! assert_eq!(1, index[0]); -//! # } //! ``` use std::collections::HashSet; @@ -133,12 +130,10 @@ impl PartialMerkleTree { /// # Examples /// /// ```rust - /// extern crate bitcoin; /// use bitcoin::hash_types::Txid; /// use bitcoin::hashes::hex::FromHex; /// use bitcoin::util::merkleblock::PartialMerkleTree; /// - /// # fn main() { /// // Block 80000 /// let txids: Vec = [ /// "c06fbab289f723c6261d3030ddb6be121f7d2508d77862bb1e484f5cd7f92b25", @@ -152,7 +147,6 @@ impl PartialMerkleTree { /// let matches = vec![false, true]; /// let tree = PartialMerkleTree::from_txids(&txids, &matches); /// assert!(tree.extract_matches(&mut vec![], &mut vec![]).is_ok()); - /// # } /// ``` pub fn from_txids(txids: &[Txid], matches: &[bool]) -> Self { // We can never have zero txs in a merkle block, we always need the coinbase tx @@ -271,7 +265,7 @@ impl PartialMerkleTree { if height == 0 || !parent_of_match { // If at height 0, or nothing interesting below, store hash and stop let hash = self.calc_hash(height, pos, txids); - self.hashes.push(hash.into()); + self.hashes.push(hash); } else { // Otherwise, don't store any hash, but descend into the subtrees self.traverse_and_build(height - 1, pos * 2, txids, matches); @@ -407,12 +401,10 @@ impl MerkleBlock { /// # Examples /// /// ```rust - /// extern crate bitcoin; /// use bitcoin::hash_types::Txid; /// use bitcoin::hashes::hex::FromHex; /// use bitcoin::{Block, MerkleBlock}; /// - /// # fn main() { /// // Block 80000 /// let block_bytes = Vec::from_hex("01000000ba8b9cda965dd8e536670f9ddec10e53aab14b20bacad2\ /// 7b9137190000000000190760b278fe7b8565fda3b968b918d5fd997f993b23674c0af3b6fde300b38f33\ @@ -437,7 +429,6 @@ impl MerkleBlock { /// let mut index: Vec = vec![]; /// assert!(mb.extract_matches(&mut matches, &mut index).is_ok()); /// assert_eq!(txid, matches[0]); - /// # } /// ``` pub fn from_block(block: &Block, match_txids: &HashSet) -> Self { let header = block.header; diff --git a/src/util/misc.rs b/src/util/misc.rs index f5c2a545..2ee4fb0c 100644 --- a/src/util/misc.rs +++ b/src/util/misc.rs @@ -20,14 +20,14 @@ use hashes::{sha256d, Hash}; use blockdata::opcodes; use consensus::encode; -static MSG_SIGN_PREFIX: &'static [u8] = b"\x18Bitcoin Signed Message:\n"; +static MSG_SIGN_PREFIX: &[u8] = b"\x18Bitcoin Signed Message:\n"; /// Search for `needle` in the vector `haystack` and remove every /// instance of it, returning the number of instances removed. /// Loops through the vector opcode by opcode, skipping pushed data. pub fn script_find_and_remove(haystack: &mut Vec, needle: &[u8]) -> usize { if needle.len() > haystack.len() { return 0; } - if needle.len() == 0 { return 0; } + if needle.is_empty() { return 0; } let mut top = haystack.len() - needle.len(); let mut n_deleted = 0; diff --git a/src/util/psbt/macros.rs b/src/util/psbt/macros.rs index 80f6678d..5109b94f 100644 --- a/src/util/psbt/macros.rs +++ b/src/util/psbt/macros.rs @@ -104,7 +104,7 @@ macro_rules! impl_psbtmap_consensus_enc_dec_oding { macro_rules! impl_psbt_insert_pair { ($slf:ident.$unkeyed_name:ident <= <$raw_key:ident: _>|<$raw_value:ident: $unkeyed_value_type:ty>) => { if $raw_key.key.is_empty() { - if let None = $slf.$unkeyed_name { + if $slf.$unkeyed_name.is_none() { let val: $unkeyed_value_type = ::util::psbt::serialize::Deserialize::deserialize(&$raw_value)?; $slf.$unkeyed_name = Some(val) From 2cc88a99aa45c04c2112f9ba23ef99f1f5297986 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Mon, 5 Aug 2019 15:44:32 -0400 Subject: [PATCH 4/4] Removed PartialEq,PartialOrd impls, shouldn't be manually impl when Hash is derived --- src/util/amount.rs | 42 ++---------------------------------------- 1 file changed, 2 insertions(+), 40 deletions(-) diff --git a/src/util/amount.rs b/src/util/amount.rs index 1fea2b15..f945b190 100644 --- a/src/util/amount.rs +++ b/src/util/amount.rs @@ -274,7 +274,7 @@ fn fmt_satoshi_in( /// zero is considered an underflow and will cause a panic if you're not using /// the checked arithmetic methods. /// -#[derive(Copy, Clone, Hash)] +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Amount(u64); impl Amount { @@ -445,25 +445,6 @@ impl default::Default for Amount { } } -impl PartialEq for Amount { - fn eq(&self, other: &Amount) -> bool { - PartialEq::eq(&self.0, &other.0) - } -} -impl Eq for Amount {} - -impl PartialOrd for Amount { - fn partial_cmp(&self, other: &Amount) -> Option<::std::cmp::Ordering> { - PartialOrd::partial_cmp(&self.0, &other.0) - } -} - -impl Ord for Amount { - fn cmp(&self, other: &Amount) -> ::std::cmp::Ordering { - Ord::cmp(&self.0, &other.0) - } -} - impl fmt::Debug for Amount { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "Amount({} satoshi)", self.as_sat()) @@ -571,7 +552,7 @@ impl FromStr for Amount { /// start with `checked_`. The operations from [std::ops] that [Amount] /// implements will panic when overflow or underflow occurs. /// -#[derive(Copy, Clone, Hash)] +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct SignedAmount(i64); impl SignedAmount { @@ -789,25 +770,6 @@ impl default::Default for SignedAmount { } } -impl PartialEq for SignedAmount { - fn eq(&self, other: &SignedAmount) -> bool { - PartialEq::eq(&self.0, &other.0) - } -} -impl Eq for SignedAmount {} - -impl PartialOrd for SignedAmount { - fn partial_cmp(&self, other: &SignedAmount) -> Option<::std::cmp::Ordering> { - PartialOrd::partial_cmp(&self.0, &other.0) - } -} - -impl Ord for SignedAmount { - fn cmp(&self, other: &SignedAmount) -> ::std::cmp::Ordering { - Ord::cmp(&self.0, &other.0) - } -} - impl fmt::Debug for SignedAmount { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "SignedAmount({} satoshi)", self.as_sat())