Skip to content

Commit

Permalink
Merge #1004: Clear Clippy warnings
Browse files Browse the repository at this point in the history
a6efe98 Use write_all to write whole buffer (Tobin C. Harding)
51c60b8 Allow no is_empty method for VarInt (Tobin C. Harding)
841f1f5 Implement Default for TaprootBuilder (Tobin C. Harding)
f81d4aa Remove unnecessary call to clone (Tobin C. Harding)
27649ba Use copied instead of map to copy (Tobin C. Harding)
62ccc91 Use iter().flatten().any() instead of if let Some (Tobin C. Harding)
4b28a1b Remove unneeded return statement (Tobin C. Harding)
16cac3c Derive Default for Witness (Tobin C. Harding)
c751898 Remove unnecessary closure (Tobin C. Harding)
dfff853 Ignore bytes written for sighash_single bug output (Tobin C. Harding)
14c72e7 Use contains combinator instead of manual range (Tobin C. Harding)
b7d6c3e Remove additional reference (Tobin C. Harding)
1940b00 Implement From instead of Into (Tobin C. Harding)
fcd0f4d Use struct field init shorthand (Tobin C. Harding)
641960f Use rustfmt::skip (Tobin C. Harding)
3cd00e5 Remove unnecessary whitespace (Tobin C. Harding)

Pull request description:

  Clear all current Clippy warnings, codebase wide. Possibly contentious patches include:

  - [commit](fcd0f4d): `fcd0f4d Use struct field init shorthand`
  - [commit](14c72e7): `14c72e7 Use contains combinator instead of manual range`
  - [commit](3b3c378): `3b3c378 Use iter().flatten() instead of if let Some`

  ## Notes

  Please note commit `dfff8535 Ignore bytes written for sighash_single bug output` touches the same lines of code as commit `a6efe982 Use write_all to write whole buffer`.

ACKs for top commit:
  apoelstra:
    ACK a6efe98
  Kixunil:
    ACK a6efe98

Tree-SHA512: 5351a82fd3deadb8e53911c43b5a60a9517d5c57014f5fa833b79b32c0a4606ada0bcd28e06ce35d47aa74115c7cf70c27a1ba9c561a3424ac85a4f69774014d
  • Loading branch information
apoelstra committed Jun 1, 2022
2 parents 58a62c0 + a6efe98 commit 8f81fc5
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 64 deletions.
8 changes: 4 additions & 4 deletions src/blockdata/script.rs
Expand Up @@ -470,9 +470,9 @@ impl Script {
/// the current script, assuming that the script is a Tapscript.
#[inline]
pub fn to_v1_p2tr<C: Verification>(&self, secp: &Secp256k1<C>, internal_key: UntweakedPublicKey) -> Script {
let leaf_hash = TapLeafHash::from_script(&self, LeafVersion::TapScript);
let leaf_hash = TapLeafHash::from_script(self, LeafVersion::TapScript);
let merkle_root = TapBranchHash::from_inner(leaf_hash.into_inner());
Script::new_v1_p2tr(&secp, internal_key, Some(merkle_root))
Script::new_v1_p2tr(secp, internal_key, Some(merkle_root))
}

/// Returns witness version of the script, if any, assuming the script is a `scriptPubkey`.
Expand Down Expand Up @@ -525,7 +525,7 @@ impl Script {
// special meaning. The value of the first push is called the "version byte". The following
// byte vector pushed is called the "witness program".
let script_len = self.0.len();
if script_len < 4 || script_len > 42 {
if !(4..=42).contains(&script_len) {
return false
}
let ver_opcode = opcodes::All::from(self.0[0]); // Version 0 or PUSHNUM_1-PUSHNUM_16
Expand Down Expand Up @@ -877,7 +877,7 @@ impl Builder {
/// dedicated opcodes to push some small integers.
pub fn push_int(self, data: i64) -> Builder {
// We can special-case -1, 1-16
if data == -1 || (data >= 1 && data <= 16) {
if data == -1 || (1..=16).contains(&data) {
let opcode = opcodes::All::from(
(data - 1 + opcodes::OP_TRUE.into_u8() as i64) as u8
);
Expand Down
2 changes: 1 addition & 1 deletion src/blockdata/transaction.rs
Expand Up @@ -353,7 +353,7 @@ impl Transaction {
// will result in the data written to the writer being hashed, however the correct
// handling of the SIGHASH_SINGLE bug is to return the 'one array' - either implement
// this behaviour manually or use `signature_hash()`.
writer.write(b"[not a transaction] SIGHASH_SINGLE bug")?;
writer.write_all(b"[not a transaction] SIGHASH_SINGLE bug")?;
return Ok(())
}

Expand Down
21 changes: 4 additions & 17 deletions src/blockdata/witness.rs
Expand Up @@ -22,7 +22,7 @@ use serde;
/// For serialization and deserialization performance it is stored internally as a single `Vec`,
/// saving some allocations.
///
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
#[derive(Clone, Default, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
pub struct Witness {
/// contains the witness Vec<Vec<u8>> serialization without the initial varint indicating the
/// number of elements (which is stored in `witness_elements`)
Expand Down Expand Up @@ -67,12 +67,12 @@ impl Decodable for Witness {
let element_size = element_size_varint.0 as usize;
let required_len = cursor
.checked_add(element_size)
.ok_or_else(|| self::Error::OversizedVectorAllocation {
.ok_or(self::Error::OversizedVectorAllocation {
requested: usize::max_value(),
max: MAX_VEC_SIZE,
})?
.checked_add(element_size_varint_len)
.ok_or_else(|| self::Error::OversizedVectorAllocation {
.ok_or(self::Error::OversizedVectorAllocation {
requested: usize::max_value(),
max: MAX_VEC_SIZE,
})?;
Expand Down Expand Up @@ -218,7 +218,7 @@ impl Witness {
pub fn push_bitcoin_signature(&mut self, signature: &ecdsa::SerializedSignature, hash_type: EcdsaSighashType) {
// Note that a maximal length ECDSA signature is 72 bytes, plus the sighash type makes 73
let mut sig = [0; 73];
sig[..signature.len()].copy_from_slice(&signature);
sig[..signature.len()].copy_from_slice(signature);
sig[signature.len()] = hash_type as u8;
self.push(&sig[..signature.len() + 1]);
}
Expand Down Expand Up @@ -249,19 +249,6 @@ impl Witness {
}
}

impl Default for Witness {
fn default() -> Self {
// from https://doc.rust-lang.org/std/vec/struct.Vec.html#method.new
// The vector will not allocate until elements are pushed onto it.
Witness {
content: Vec::new(),
witness_elements: 0,
last: 0,
second_to_last: 0,
}
}
}

impl<'a> Iterator for Iter<'a> {
type Item = &'a [u8];

Expand Down
1 change: 1 addition & 0 deletions src/consensus/encode.rs
Expand Up @@ -400,6 +400,7 @@ impl_int_encodable!(i16, read_i16, emit_i16);
impl_int_encodable!(i32, read_i32, emit_i32);
impl_int_encodable!(i64, read_i64, emit_i64);

#[allow(clippy::len_without_is_empty)] // VarInt has on concept of 'is_empty'.
impl VarInt {
/// Gets the length of this VarInt when encoded.
/// Returns 1 for 0..=0xFC, 3 for 0xFD..=(2^16-1), 5 for 0x10000..=(2^32-1),
Expand Down
6 changes: 3 additions & 3 deletions src/network/constants.rs
Expand Up @@ -238,9 +238,9 @@ impl From<u64> for ServiceFlags {
}
}

impl Into<u64> for ServiceFlags {
fn into(self) -> u64 {
self.0
impl From<ServiceFlags> for u64 {
fn from(flags: ServiceFlags) -> Self {
flags.0
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/util/address.rs
Expand Up @@ -621,7 +621,7 @@ impl Address {
network: Network
) -> Address {
Address {
network: network,
network,
payload: Payload::p2tr(secp, internal_key, merkle_root),
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/util/bip32.rs
Expand Up @@ -283,9 +283,9 @@ impl From<Vec<ChildNumber>> for DerivationPath {
}
}

impl Into<Vec<ChildNumber>> for DerivationPath {
fn into(self) -> Vec<ChildNumber> {
self.0
impl From<DerivationPath> for Vec<ChildNumber> {
fn from(path: DerivationPath) -> Self {
path.0
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/util/ecdsa.rs
Expand Up @@ -58,7 +58,7 @@ impl EcdsaSig {
pub fn to_vec(&self) -> Vec<u8> {
// TODO: add support to serialize to a writer to SerializedSig
self.sig.serialize_der()
.iter().map(|x| *x)
.iter().copied()
.chain(iter::once(self.hash_ty as u8))
.collect()
}
Expand Down
2 changes: 1 addition & 1 deletion src/util/key.rs
Expand Up @@ -228,7 +228,7 @@ impl FromStr for PublicKey {
match s.len() {
66 => PublicKey::from_slice(&<[u8; 33]>::from_hex(s)?),
130 => PublicKey::from_slice(&<[u8; 65]>::from_hex(s)?),
len => return Err(Error::Hex(hex::Error::InvalidLength(66, len)))
len => Err(Error::Hex(hex::Error::InvalidLength(66, len))),
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/util/psbt/macros.rs
Expand Up @@ -92,7 +92,7 @@ macro_rules! impl_psbtmap_consensus_enc_dec_oding {
};
}

#[cfg_attr(rustfmt, rustfmt_skip)]
#[rustfmt::skip]
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() {
Expand Down Expand Up @@ -122,8 +122,7 @@ macro_rules! impl_psbt_insert_pair {
};
}


#[cfg_attr(rustfmt, rustfmt_skip)]
#[rustfmt::skip]
macro_rules! impl_psbt_get_pair {
($rv:ident.push($slf:ident.$unkeyed_name:ident, $unkeyed_typeval:ident)) => {
if let Some(ref $unkeyed_name) = $slf.$unkeyed_name {
Expand Down
12 changes: 6 additions & 6 deletions src/util/psbt/serialize.rs
Expand Up @@ -125,7 +125,7 @@ impl Deserialize for EcdsaSig {
// also has a field sighash_u32 (See BIP141). For example, when signing with non-standard
// 0x05, the sighash message would have the last field as 0x05u32 while, the verification
// would use check the signature assuming sighash_u32 as `0x01`.
EcdsaSig::from_slice(&bytes)
EcdsaSig::from_slice(bytes)
.map_err(|e| match e {
EcdsaSigError::EmptySignature => {
encode::Error::ParseFailed("Empty partial signature data")
Expand All @@ -145,7 +145,7 @@ impl Deserialize for EcdsaSig {

impl Serialize for KeySource {
fn serialize(&self) -> Vec<u8> {
let mut rv: Vec<u8> = Vec::with_capacity(key_source_len(&self));
let mut rv: Vec<u8> = Vec::with_capacity(key_source_len(self));

rv.append(&mut self.0.to_bytes().to_vec());

Expand Down Expand Up @@ -207,7 +207,7 @@ impl Deserialize for PsbtSighashType {
// Taproot related ser/deser
impl Serialize for XOnlyPublicKey {
fn serialize(&self) -> Vec<u8> {
XOnlyPublicKey::serialize(&self).to_vec()
XOnlyPublicKey::serialize(self).to_vec()
}
}

Expand All @@ -226,7 +226,7 @@ impl Serialize for schnorr::SchnorrSig {

impl Deserialize for schnorr::SchnorrSig {
fn deserialize(bytes: &[u8]) -> Result<Self, encode::Error> {
schnorr::SchnorrSig::from_slice(&bytes)
schnorr::SchnorrSig::from_slice(bytes)
.map_err(|e| match e {
schnorr::SchnorrSigError::InvalidSighashType(flag) => {
encode::Error::from(psbt::Error::NonStandardSighashType(flag as u32))
Expand Down Expand Up @@ -264,7 +264,7 @@ impl Deserialize for (XOnlyPublicKey, TapLeafHash) {

impl Serialize for ControlBlock {
fn serialize(&self) -> Vec<u8> {
ControlBlock::serialize(&self)
ControlBlock::serialize(self)
}
}

Expand Down Expand Up @@ -311,7 +311,7 @@ impl Serialize for (Vec<TapLeafHash>, KeySource) {

impl Deserialize for (Vec<TapLeafHash>, KeySource) {
fn deserialize(bytes: &[u8]) -> Result<Self, encode::Error> {
let (leafhash_vec, consumed) = deserialize_partial::<Vec<TapLeafHash>>(&bytes)?;
let (leafhash_vec, consumed) = deserialize_partial::<Vec<TapLeafHash>>(bytes)?;
let key_source = KeySource::deserialize(&bytes[consumed..])?;
Ok((leafhash_vec, key_source))
}
Expand Down
10 changes: 5 additions & 5 deletions src/util/schnorr.rs
Expand Up @@ -109,10 +109,10 @@ impl TapTweak for UntweakedPublicKey {
/// The tweaked key and its parity.
fn tap_tweak<C: Verification>(self, secp: &Secp256k1<C>, merkle_root: Option<TapBranchHash>) -> (TweakedPublicKey, secp256k1::Parity) {
let tweak_value = TapTweakHash::from_key_and_tweak(self, merkle_root).into_inner();
let mut output_key = self.clone();
let parity = output_key.tweak_add_assign(&secp, &tweak_value).expect("Tap tweak failed");
let mut output_key = self;
let parity = output_key.tweak_add_assign(secp, &tweak_value).expect("Tap tweak failed");

debug_assert!(self.tweak_add_check(&secp, &output_key, parity, tweak_value));
debug_assert!(self.tweak_add_check(secp, &output_key, parity, tweak_value));
(TweakedPublicKey(output_key), parity)
}

Expand Down Expand Up @@ -140,7 +140,7 @@ impl TapTweak for UntweakedKeyPair {
fn tap_tweak<C: Verification>(mut self, secp: &Secp256k1<C>, merkle_root: Option<TapBranchHash>) -> TweakedKeyPair {
let pubkey = crate::XOnlyPublicKey::from_keypair(&self);
let tweak_value = TapTweakHash::from_key_and_tweak(pubkey, merkle_root).into_inner();
self.tweak_add_assign(&secp, &tweak_value).expect("Tap tweak failed");
self.tweak_add_assign(secp, &tweak_value).expect("Tap tweak failed");
TweakedKeyPair(self)
}

Expand Down Expand Up @@ -229,7 +229,7 @@ impl SchnorrSig {
// default type
let sig = secp256k1::schnorr::Signature::from_slice(sl)
.map_err(SchnorrSigError::Secp256k1)?;
return Ok( SchnorrSig { sig, hash_ty : SchnorrSighashType::Default });
Ok(SchnorrSig { sig, hash_ty: SchnorrSighashType::Default })
},
65 => {
let (hash_ty, sig) = sl.split_last().expect("Slice len checked == 65");
Expand Down
6 changes: 3 additions & 3 deletions src/util/sighash.rs
Expand Up @@ -440,7 +440,7 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
.tx
.input
.get(input_index)
.ok_or_else(|| Error::IndexOutOfInputsBounds {
.ok_or(Error::IndexOutOfInputsBounds {
index: input_index,
inputs_size: self.tx.input.len(),
})?;
Expand Down Expand Up @@ -473,7 +473,7 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
self.tx
.output
.get(input_index)
.ok_or_else(|| Error::SingleWithoutCorrespondingOutput {
.ok_or(Error::SingleWithoutCorrespondingOutput {
index: input_index,
outputs_size: self.tx.output.len(),
})?
Expand Down Expand Up @@ -597,7 +597,7 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
.tx
.input
.get(input_index)
.ok_or_else(|| Error::IndexOutOfInputsBounds {
.ok_or(Error::IndexOutOfInputsBounds {
index: input_index,
inputs_size: self.tx.input.len(),
})?;
Expand Down
31 changes: 15 additions & 16 deletions src/util/taproot.rs
Expand Up @@ -217,10 +217,10 @@ impl TaprootSpendInfo {
) -> Self {
let (output_key, parity) = internal_key.tap_tweak(secp, merkle_root);
Self {
internal_key: internal_key,
merkle_root: merkle_root,
internal_key,
merkle_root,
output_key_parity: parity,
output_key: output_key,
output_key,
script_map: BTreeMap::new(),
}
}
Expand Down Expand Up @@ -442,14 +442,7 @@ impl TaprootBuilder {

/// Checks if the builder has hidden nodes.
pub fn has_hidden_nodes(&self) -> bool {
for node in &self.branch {
if let Some(node) = node {
if node.has_hidden_nodes {
return true
}
}
}
false
self.branch.iter().flatten().any(|node| node.has_hidden_nodes)
}

/// Creates a [`TaprootSpendInfo`] with the given internal key.
Expand Down Expand Up @@ -521,6 +514,12 @@ impl TaprootBuilder {
}
}

impl Default for TaprootBuilder {
fn default() -> Self {
Self::new()
}
}

/// Represents the node information in taproot tree.
///
/// Helper type used in merkle tree construction allowing one to build sparse merkle trees. The node
Expand All @@ -544,7 +543,7 @@ impl NodeInfo {
/// Creates a new [`NodeInfo`] with omitted/hidden info.
pub fn new_hidden_node(hash: sha256::Hash) -> Self {
Self {
hash: hash,
hash,
leaves: vec![],
has_hidden_nodes: true
}
Expand Down Expand Up @@ -596,8 +595,8 @@ impl ScriptLeaf {
/// Creates an new [`ScriptLeaf`] from `script` and `ver` and no merkle branch.
fn new(script: Script, ver: LeafVersion) -> Self {
Self {
script: script,
ver: ver,
script,
ver,
merkle_branch: TaprootMerkleBranch(vec![]),
}
}
Expand Down Expand Up @@ -682,7 +681,7 @@ impl TaprootMerkleBranch {

/// Serializes `self` as bytes.
pub fn serialize(&self) -> Vec<u8> {
self.0.iter().map(|e| e.as_inner()).flatten().map(|x| *x).collect::<Vec<u8>>()
self.0.iter().flat_map(|e| e.as_inner()).copied().collect::<Vec<u8>>()
}

/// Appends elements to proof.
Expand Down Expand Up @@ -802,7 +801,7 @@ impl ControlBlock {
) -> bool {
// compute the script hash
// Initially the curr_hash is the leaf hash
let leaf_hash = TapLeafHash::from_script(&script, self.leaf_version);
let leaf_hash = TapLeafHash::from_script(script, self.leaf_version);
let mut curr_hash = TapBranchHash::from_inner(leaf_hash.into_inner());
// Verify the proof
for elem in self.merkle_branch.as_inner() {
Expand Down

0 comments on commit 8f81fc5

Please sign in to comment.