Skip to content

Commit

Permalink
Merge #686: bitcoind_test: Fix clippy
Browse files Browse the repository at this point in the history 
b9a207b Use is_err instead of redundant pattern matching (Tobin C. Harding)
ef868cc Use unwrap_or_else instead of expect (Tobin C. Harding)
a1a5467 Remove clone call from Copy type (Tobin C. Harding)
f939367 Remove useless conversion (Tobin C. Harding)
c20be39 Remove explicit reference (Tobin C. Harding)
7ed6680 Do not mutate local variable (Tobin C. Harding)
e361cfc Remove unneeded return statement (Tobin C. Harding)
f9907b0 Use += operator (Tobin C. Harding)
5e0df21 Use single quotes (Tobin C. Harding)
7218e5e Remove redundant field names (Tobin C. Harding)
6a2577f Remove useless let binding (Tobin C. Harding)
ccc808a Use iterator instead of array access (Tobin C. Harding)
badeb44 Remove unnecessary cast (Tobin C. Harding)

Pull request description:

  All the clippy patches rom #682, only touches `bitcoind_test`.

ACKs for top commit:
  apoelstra:
    utACK b9a207b

Tree-SHA512: ae9451601bc5232f0ce194be1b7c6c1c31c8118ea87951f3eac9c6c4349c7ff963516c8cc34c2a63c9f1b99f03c781e687c74b46dc24624bbfa81a0decb513ef
  • Loading branch information
@apoelstra
apoelstra committed May 16, 2024
2 parents 351ffa8 + b9a207b commit b507ff6
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 57 deletions.
34 changes: 16 additions & 18 deletions bitcoind-tests/tests/setup/test_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ fn setup_keys(
let mut x_only_keypairs = vec![];
let mut x_only_pks = vec![];

for i in 0..n {
let keypair = bitcoin::secp256k1::Keypair::from_secret_key(&secp_sign, &sks[i]);
for sk in &sks {
let keypair = bitcoin::secp256k1::Keypair::from_secret_key(&secp_sign, &sk);
let (xpk, _parity) = XOnlyPublicKey::from_keypair(&keypair);
x_only_keypairs.push(keypair);
x_only_pks.push(xpk);
Expand All @@ -99,13 +99,13 @@ impl TestData {
// generate a fixed data for n keys
pub(crate) fn new_fixed_data(n: usize) -> Self {
let (sks, pks, x_only_keypairs, x_only_pks) = setup_keys(n);
let sha256_pre = [0x12 as u8; 32];
let sha256_pre = [0x12_u8; 32];
let sha256 = sha256::Hash::hash(&sha256_pre);
let hash256_pre = [0x34 as u8; 32];
let hash256_pre = [0x34_u8; 32];
let hash256 = hash256::Hash::hash(&hash256_pre);
let hash160_pre = [0x56 as u8; 32];
let hash160_pre = [0x56_u8; 32];
let hash160 = hash160::Hash::hash(&hash160_pre);
let ripemd160_pre = [0x78 as u8; 32];
let ripemd160_pre = [0x78_u8; 32];
let ripemd160 = ripemd160::Hash::hash(&ripemd160_pre);

let pubdata = PubData { pks, sha256, hash256, ripemd160, hash160, x_only_pks };
Expand Down Expand Up @@ -148,8 +148,7 @@ pub fn parse_insane_ms<Ctx: ScriptContext>(
let ms =
Miniscript::<String, Ctx>::from_str_insane(&ms).expect("only parsing valid minsicripts");
let mut translator = StrTranslatorLoose(0, pubdata);
let ms = ms.translate_pk(&mut translator).unwrap();
ms
ms.translate_pk(&mut translator).unwrap()
}

// Translate Str to DescriptorPublicKey
Expand All @@ -158,15 +157,15 @@ struct StrDescPubKeyTranslator<'a>(usize, &'a PubData);

impl<'a> Translator<String, DescriptorPublicKey, ()> for StrDescPubKeyTranslator<'a> {
fn pk(&mut self, pk_str: &String) -> Result<DescriptorPublicKey, ()> {
let avail = !pk_str.ends_with("!");
let avail = !pk_str.ends_with('!');
if avail {
self.0 = self.0 + 1;
if pk_str.starts_with("K") {
self.0 += 1;
if pk_str.starts_with('K') {
Ok(DescriptorPublicKey::Single(SinglePub {
origin: None,
key: SinglePubKey::FullKey(self.1.pks[self.0]),
}))
} else if pk_str.starts_with("X") {
} else if pk_str.starts_with('X') {
Ok(DescriptorPublicKey::Single(SinglePub {
origin: None,
key: SinglePubKey::XOnly(self.1.x_only_pks[self.0]),
Expand Down Expand Up @@ -211,15 +210,15 @@ struct StrTranslatorLoose<'a>(usize, &'a PubData);

impl<'a> Translator<String, DescriptorPublicKey, ()> for StrTranslatorLoose<'a> {
fn pk(&mut self, pk_str: &String) -> Result<DescriptorPublicKey, ()> {
let avail = !pk_str.ends_with("!");
let avail = !pk_str.ends_with('!');
if avail {
self.0 = self.0 + 1;
if pk_str.starts_with("K") {
self.0 += 1;
if pk_str.starts_with('K') {
Ok(DescriptorPublicKey::Single(SinglePub {
origin: None,
key: SinglePubKey::FullKey(self.1.pks[self.0]),
}))
} else if pk_str.starts_with("X") {
} else if pk_str.starts_with('X') {
Ok(DescriptorPublicKey::Single(SinglePub {
origin: None,
key: SinglePubKey::XOnly(self.1.x_only_pks[self.0]),
Expand Down Expand Up @@ -291,6 +290,5 @@ fn subs_hash_frag(ms: &str, pubdata: &PubData) -> String {
let ms = ms.replace("hash256(H!)", &format!("hash256({})", rand_hash32.to_lower_hex_string()));
let ms =
ms.replace("ripemd160(H!)", &format!("ripemd160({})", rand_hash20.to_lower_hex_string()));
let ms = ms.replace("hash160(H!)", &format!("hash160({})", rand_hash20.to_lower_hex_string()));
ms
ms.replace("hash160(H!)", &format!("hash160({})", rand_hash20.to_lower_hex_string()))
}
32 changes: 17 additions & 15 deletions bitcoind-tests/tests/test_cpp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,7 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) {
let mut psbt = Psbt {
unsigned_tx: Transaction {
version: transaction::Version::TWO,
lock_time: absolute::LockTime::from_time(1_603_866_330)
.expect("valid timestamp")
.into(), // 10/28/2020 @ 6:25am (UTC)
lock_time: absolute::LockTime::from_time(1_603_866_330).expect("valid timestamp"), // 10/28/2020 @ 6:25am (UTC)
input: vec![],
output: vec![],
},
Expand All @@ -119,13 +117,15 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) {
outputs: vec![],
};
// figure out the outpoint from the txid
let (outpoint, witness_utxo) = get_vout(&cl, txid, btc(1.0));
let mut txin = TxIn::default();
txin.previous_output = outpoint;
// set the sequence to a non-final number for the locktime transactions to be
// processed correctly.
// We waited 50 blocks, keep 49 for safety
txin.sequence = Sequence::from_height(49);
let (outpoint, witness_utxo) = get_vout(cl, txid, btc(1.0));
let txin = TxIn {
previous_output: outpoint,
// set the sequence to a non-final number for the locktime transactions to be
// processed correctly.
// We waited 50 blocks, keep 49 for safety
sequence: Sequence::from_height(49),
..Default::default()
};
psbt.unsigned_tx.input.push(txin);
// Get a new script pubkey from the node so that
// the node wallet tracks the receiving transaction
Expand All @@ -138,11 +138,13 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) {
value: Amount::from_sat(99_999_000),
script_pubkey: addr.script_pubkey(),
});
let mut input = psbt::Input::default();
input.witness_utxo = Some(witness_utxo);
input.witness_script = Some(desc.explicit_script().unwrap());
let input = psbt::Input {
witness_utxo: Some(witness_utxo),
witness_script: Some(desc.explicit_script().unwrap()),
..Default::default()
};
psbt.inputs.push(input);
psbt.update_input_with_descriptor(0, &desc).unwrap();
psbt.update_input_with_descriptor(0, desc).unwrap();
psbt.outputs.push(psbt::Output::default());
psbts.push(psbt);
}
Expand Down Expand Up @@ -213,7 +215,7 @@ pub fn test_from_cpp_ms(cl: &Client, testdata: &TestData) {
// Check whether the node accepts the transactions
let txid = cl
.send_raw_transaction(&tx)
.expect(&format!("{} send tx failed for ms {}", i, ms));
.unwrap_or_else(|_| panic!("{} send tx failed for ms {}", i, ms));
spend_txids.push(txid);
}
}
Expand Down
47 changes: 23 additions & 24 deletions bitcoind-tests/tests/test_desc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ pub fn test_desc_satisfy(
.unwrap();
assert_eq!(blocks.len(), 1);

let definite_desc = test_util::parse_test_desc(&descriptor, &testdata.pubdata)
let definite_desc = test_util::parse_test_desc(descriptor, &testdata.pubdata)
.map_err(|_| DescError::DescParseError)?
.at_derivation_index(0)
.unwrap();
Expand All @@ -104,9 +104,7 @@ pub fn test_desc_satisfy(
let mut psbt = Psbt {
unsigned_tx: Transaction {
version: transaction::Version::TWO,
lock_time: absolute::LockTime::from_time(1_603_866_330)
.expect("valid timestamp")
.into(), // 10/28/2020 @ 6:25am (UTC)
lock_time: absolute::LockTime::from_time(1_603_866_330).expect("valid timestamp"), // 10/28/2020 @ 6:25am (UTC)
input: vec![],
output: vec![],
},
Expand All @@ -118,13 +116,15 @@ pub fn test_desc_satisfy(
outputs: vec![],
};
// figure out the outpoint from the txid
let (outpoint, witness_utxo) = get_vout(&cl, txid, btc(1.0), derived_desc.script_pubkey());
let mut txin = TxIn::default();
txin.previous_output = outpoint;
// set the sequence to a non-final number for the locktime transactions to be
// processed correctly.
// We waited 2 blocks, keep 1 for safety
txin.sequence = Sequence::from_height(1);
let (outpoint, witness_utxo) = get_vout(cl, txid, btc(1.0), derived_desc.script_pubkey());
let txin = TxIn {
previous_output: outpoint,
// set the sequence to a non-final number for the locktime transactions to be
// processed correctly.
// We waited 2 blocks, keep 1 for safety
sequence: Sequence::from_height(1),
..Default::default()
};
psbt.unsigned_tx.input.push(txin);
// Get a new script pubkey from the node so that
// the node wallet tracks the receiving transaction
Expand Down Expand Up @@ -160,7 +160,7 @@ pub fn test_desc_satisfy(
let internal_key_present = x_only_pks
.iter()
.position(|&x| x.to_public_key() == *tr.internal_key());
let internal_keypair = internal_key_present.map(|idx| xonly_keypairs[idx].clone());
let internal_keypair = internal_key_present.map(|idx| xonly_keypairs[idx]);
let prevouts = [witness_utxo];
let prevouts = sighash::Prevouts::All(&prevouts);

Expand All @@ -177,8 +177,7 @@ pub fn test_desc_satisfy(
rand::thread_rng().fill_bytes(&mut aux_rand);
let schnorr_sig =
secp.sign_schnorr_with_aux_rand(&msg, &internal_keypair, &aux_rand);
psbt.inputs[0].tap_key_sig =
Some(taproot::Signature { sig: schnorr_sig, hash_ty: hash_ty });
psbt.inputs[0].tap_key_sig = Some(taproot::Signature { sig: schnorr_sig, hash_ty });
} else {
// No internal key
}
Expand All @@ -189,7 +188,7 @@ pub fn test_desc_satisfy(
let leaf_hash = TapLeafHash::from_script(&ms.encode(), LeafVersion::TapScript);
ms.iter_pk().filter_map(move |pk| {
let i = x_only_pks.iter().position(|&x| x.to_public_key() == pk);
i.map(|idx| (xonly_keypairs[idx].clone(), leaf_hash))
i.map(|idx| (xonly_keypairs[idx], leaf_hash))
})
})
.collect();
Expand All @@ -205,14 +204,14 @@ pub fn test_desc_satisfy(
x_only_pks[xonly_keypairs.iter().position(|&x| x == keypair).unwrap()];
psbt.inputs[0]
.tap_script_sigs
.insert((x_only_pk, leaf_hash), taproot::Signature { sig, hash_ty: hash_ty });
.insert((x_only_pk, leaf_hash), taproot::Signature { sig, hash_ty });
}
}
_ => {
// Non-tr descriptors
// Ecdsa sigs
let sks_reqd = match derived_desc {
Descriptor::Bare(bare) => find_sks_ms(&bare.as_inner(), testdata),
Descriptor::Bare(bare) => find_sks_ms(bare.as_inner(), testdata),
Descriptor::Pkh(pk) => find_sk_single_key(*pk.as_inner(), testdata),
Descriptor::Wpkh(pk) => find_sk_single_key(*pk.as_inner(), testdata),
Descriptor::Sh(sh) => match sh.as_inner() {
Expand All @@ -221,7 +220,7 @@ pub fn test_desc_satisfy(
let ms = Miniscript::from_ast(smv.sorted_node()).unwrap();
find_sks_ms(&ms, testdata)
}
miniscript::descriptor::WshInner::Ms(ref ms) => find_sks_ms(&ms, testdata),
miniscript::descriptor::WshInner::Ms(ref ms) => find_sks_ms(ms, testdata),
},
miniscript::descriptor::ShInner::Wpkh(pk) => {
find_sk_single_key(*pk.as_inner(), testdata)
Expand All @@ -230,14 +229,14 @@ pub fn test_desc_satisfy(
let ms = Miniscript::from_ast(smv.sorted_node()).unwrap();
find_sks_ms(&ms, testdata)
}
miniscript::descriptor::ShInner::Ms(ms) => find_sks_ms(&ms, testdata),
miniscript::descriptor::ShInner::Ms(ms) => find_sks_ms(ms, testdata),
},
Descriptor::Wsh(wsh) => match wsh.as_inner() {
miniscript::descriptor::WshInner::SortedMulti(ref smv) => {
let ms = Miniscript::from_ast(smv.sorted_node()).unwrap();
find_sks_ms(&ms, testdata)
}
miniscript::descriptor::WshInner::Ms(ref ms) => find_sks_ms(&ms, testdata),
miniscript::descriptor::WshInner::Ms(ref ms) => find_sks_ms(ms, testdata),
},
Descriptor::Tr(_tr) => unreachable!("Tr checked earlier"),
};
Expand All @@ -256,7 +255,7 @@ pub fn test_desc_satisfy(
assert!(secp.verify_ecdsa(&msg, &sig, &pk.inner).is_ok());
psbt.inputs[0]
.partial_sigs
.insert(pk, ecdsa::Signature { sig, hash_ty: hash_ty });
.insert(pk, ecdsa::Signature { sig, hash_ty });
}
}
}
Expand All @@ -277,7 +276,7 @@ pub fn test_desc_satisfy(
println!("Testing descriptor: {}", definite_desc);
// Finalize the transaction using psbt
// Let miniscript do it's magic!
if let Err(_) = psbt.finalize_mut(&secp) {
if psbt.finalize_mut(&secp).is_err() {
return Err(DescError::PsbtFinalizeError);
}
let tx = psbt.extract(&secp).expect("Extraction error");
Expand All @@ -287,7 +286,7 @@ pub fn test_desc_satisfy(
// Check whether the node accepts the transactions
let txid = cl
.send_raw_transaction(&tx)
.expect(&format!("send tx failed for desc {}", definite_desc));
.unwrap_or_else(|_| panic!("send tx failed for desc {}", definite_desc));

// Finally mine the blocks and await confirmations
let _blocks = cl
Expand All @@ -298,7 +297,7 @@ pub fn test_desc_satisfy(
// Assert that the confirmations are > 0.
let num_conf = cl.get_transaction(&txid, None).unwrap().info.confirmations;
assert!(num_conf > 0);
return Ok(tx.input[0].witness.clone());
Ok(tx.input[0].witness.clone())
}

// Find all secret corresponding to the known public keys in ms
Expand Down

0 comments on commit b507ff6

Please sign in to comment.