From bf08ee44996dd898f741c755e844cec03d18b95b Mon Sep 17 00:00:00 2001 From: yancy Date: Thu, 19 Oct 2023 09:57:07 +0200 Subject: [PATCH 1/3] Replace helper macro with helper function Remove the macro hex_psbt and replace with a function. Using track_caller to accuretly report the test name and line number during a panic is used in place of a macro. --- bitcoin/tests/psbt.rs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/bitcoin/tests/psbt.rs b/bitcoin/tests/psbt.rs index 94d58c9d..f9d5abf8 100644 --- a/bitcoin/tests/psbt.rs +++ b/bitcoin/tests/psbt.rs @@ -26,10 +26,10 @@ macro_rules! hex_script { }; } -macro_rules! hex_psbt { - ($s:expr) => { - Psbt::deserialize(& as FromHex>::from_hex($s).unwrap()) - }; +#[track_caller] +fn hex_psbt(s: &str) -> Psbt { + let v: Vec = Vec::from_hex(s).expect("valid hex digits"); + Psbt::deserialize(&v).expect("valid magic and valid separators") } #[test] @@ -206,8 +206,7 @@ fn create_transaction() -> Transaction { fn create_psbt(tx: Transaction) -> Psbt { // String from BIP 174 test vector. let expected_psbt_hex = include_str!("data/create_psbt_hex"); - let expected_psbt = hex_psbt!(expected_psbt_hex).unwrap(); - + let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); let psbt = Psbt::from_unsigned_tx(tx).unwrap(); assert_eq!(psbt, expected_psbt); @@ -236,7 +235,7 @@ fn update_psbt(mut psbt: Psbt, fingerprint: Fingerprint) -> Psbt { ]; let expected_psbt_hex = include_str!("data/update_1_psbt_hex"); - let expected_psbt = hex_psbt!(expected_psbt_hex).unwrap(); + let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); let mut input_0 = psbt.inputs[0].clone(); @@ -293,7 +292,7 @@ fn bip32_derivation( /// Does the second update according to the BIP, returns the newly updated PSBT. Verifies against BIP 174 test vector. fn update_psbt_with_sighash_all(mut psbt: Psbt) -> Psbt { let expected_psbt_hex = include_str!("data/update_2_psbt_hex"); - let expected_psbt = hex_psbt!(expected_psbt_hex).unwrap(); + let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); let ty = PsbtSighashType::from_str("SIGHASH_ALL").unwrap(); @@ -333,7 +332,7 @@ fn parse_and_verify_keys( /// Does the first signing according to the BIP, returns the signed PSBT. Verifies against BIP 174 test vector. fn signer_one_sign(psbt: Psbt, key_map: BTreeMap) -> Psbt { let expected_psbt_hex = include_str!("data/sign_1_psbt_hex"); - let expected_psbt = hex_psbt!(expected_psbt_hex).unwrap(); + let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); let psbt = sign(psbt, key_map); @@ -344,7 +343,7 @@ fn signer_one_sign(psbt: Psbt, key_map: BTreeMap /// Does the second signing according to the BIP, returns the signed PSBT. Verifies against BIP 174 test vector. fn signer_two_sign(psbt: Psbt, key_map: BTreeMap) -> Psbt { let expected_psbt_hex = include_str!("data/sign_2_psbt_hex"); - let expected_psbt = hex_psbt!(expected_psbt_hex).unwrap(); + let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); let psbt = sign(psbt, key_map); @@ -355,7 +354,7 @@ fn signer_two_sign(psbt: Psbt, key_map: BTreeMap /// Does the combine according to the BIP, returns the combined PSBT. Verifies against BIP 174 test vector. fn combine(mut this: Psbt, that: Psbt) -> Psbt { let expected_psbt_hex = include_str!("data/combine_psbt_hex"); - let expected_psbt = hex_psbt!(expected_psbt_hex).unwrap(); + let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); this.combine(that).expect("failed to combine PSBTs"); @@ -367,7 +366,7 @@ fn combine(mut this: Psbt, that: Psbt) -> Psbt { /// test vector. fn finalize(psbt: Psbt) -> Psbt { let expected_psbt_hex = include_str!("data/finalize_psbt_hex"); - let expected_psbt = hex_psbt!(expected_psbt_hex).unwrap(); + let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); let psbt = finalize_psbt(psbt); @@ -394,7 +393,7 @@ fn combine_lexicographically() { let psbt_2_hex = include_str!("data/lex_psbt_2_hex"); let expected_psbt_hex = include_str!("data/lex_combine_psbt_hex"); - let expected_psbt = hex_psbt!(expected_psbt_hex).unwrap(); + let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); let v = Vec::from_hex(psbt_1_hex).unwrap(); let mut psbt_1 = Psbt::deserialize(&v).expect("failed to deserialize psbt 1"); From 7f26439e2036a9914c23b9f61a0f809534ef7566 Mon Sep 17 00:00:00 2001 From: yancy Date: Fri, 20 Oct 2023 09:41:01 +0200 Subject: [PATCH 2/3] Add track_caller to test helper functions The test helper files can panic when calling hex_psbt(). hex_psbt() will report the calling function, instead of the offending test. Adding track_caller to functions that call hex_psbt will report the line number of the failing test. --- bitcoin/tests/psbt.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bitcoin/tests/psbt.rs b/bitcoin/tests/psbt.rs index f9d5abf8..f25f46e2 100644 --- a/bitcoin/tests/psbt.rs +++ b/bitcoin/tests/psbt.rs @@ -203,6 +203,7 @@ fn create_transaction() -> Transaction { } /// Creates the initial PSBT, called by the Creator. Verifies against BIP 174 test vector. +#[track_caller] fn create_psbt(tx: Transaction) -> Psbt { // String from BIP 174 test vector. let expected_psbt_hex = include_str!("data/create_psbt_hex"); @@ -214,6 +215,7 @@ fn create_psbt(tx: Transaction) -> Psbt { } /// Updates `psbt` according to the BIP, returns the newly updated PSBT. Verifies against BIP 174 test vector. +#[track_caller] fn update_psbt(mut psbt: Psbt, fingerprint: Fingerprint) -> Psbt { // Strings from BIP 174 test vector. let previous_tx_0 = include_str!("data/previous_tx_0_hex"); @@ -290,6 +292,7 @@ fn bip32_derivation( } /// Does the second update according to the BIP, returns the newly updated PSBT. Verifies against BIP 174 test vector. +#[track_caller] fn update_psbt_with_sighash_all(mut psbt: Psbt) -> Psbt { let expected_psbt_hex = include_str!("data/update_2_psbt_hex"); let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); @@ -330,6 +333,7 @@ fn parse_and_verify_keys( } /// Does the first signing according to the BIP, returns the signed PSBT. Verifies against BIP 174 test vector. +#[track_caller] fn signer_one_sign(psbt: Psbt, key_map: BTreeMap) -> Psbt { let expected_psbt_hex = include_str!("data/sign_1_psbt_hex"); let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); @@ -341,6 +345,7 @@ fn signer_one_sign(psbt: Psbt, key_map: BTreeMap } /// Does the second signing according to the BIP, returns the signed PSBT. Verifies against BIP 174 test vector. +#[track_caller] fn signer_two_sign(psbt: Psbt, key_map: BTreeMap) -> Psbt { let expected_psbt_hex = include_str!("data/sign_2_psbt_hex"); let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); @@ -352,6 +357,7 @@ fn signer_two_sign(psbt: Psbt, key_map: BTreeMap } /// Does the combine according to the BIP, returns the combined PSBT. Verifies against BIP 174 test vector. +#[track_caller] fn combine(mut this: Psbt, that: Psbt) -> Psbt { let expected_psbt_hex = include_str!("data/combine_psbt_hex"); let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); @@ -364,6 +370,7 @@ fn combine(mut this: Psbt, that: Psbt) -> Psbt { /// Does the finalize step according to the BIP, returns the combined PSBT. Verifies against BIP 174 /// test vector. +#[track_caller] fn finalize(psbt: Psbt) -> Psbt { let expected_psbt_hex = include_str!("data/finalize_psbt_hex"); let expected_psbt: Psbt = hex_psbt(expected_psbt_hex); @@ -388,6 +395,7 @@ fn extract_transaction(psbt: Psbt) -> Transaction { } /// Combines two PSBTs lexicographically according to the BIP. Verifies against BIP 174 test vector. +#[track_caller] fn combine_lexicographically() { let psbt_1_hex = include_str!("data/lex_psbt_1_hex"); let psbt_2_hex = include_str!("data/lex_psbt_2_hex"); From b163d9b59aecb4506705d04457cd2c2111986251 Mon Sep 17 00:00:00 2001 From: yancy Date: Fri, 20 Oct 2023 09:52:59 +0200 Subject: [PATCH 3/3] Replace hex_script macro with a helper function Remove the macro hex_script and replace with a function. Using track_caller to accuretly report the test name and line number during a panic is used in place of a macro. --- bitcoin/tests/psbt.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/bitcoin/tests/psbt.rs b/bitcoin/tests/psbt.rs index f25f46e2..360be93d 100644 --- a/bitcoin/tests/psbt.rs +++ b/bitcoin/tests/psbt.rs @@ -20,18 +20,15 @@ use bitcoin::{ const NETWORK: Network = Network::Testnet; -macro_rules! hex_script { - ($s:expr) => { - ::from_hex($s).unwrap() - }; -} - #[track_caller] fn hex_psbt(s: &str) -> Psbt { let v: Vec = Vec::from_hex(s).expect("valid hex digits"); Psbt::deserialize(&v).expect("valid magic and valid separators") } +#[track_caller] +fn hex_script(s: &str) -> ScriptBuf { ScriptBuf::from_hex(s).expect("valid hex digits") } + #[test] fn bip174_psbt_workflow() { let secp = Secp256k1::new(); @@ -244,7 +241,7 @@ fn update_psbt(mut psbt: Psbt, fingerprint: Fingerprint) -> Psbt { let v = Vec::from_hex(previous_tx_1).unwrap(); let tx: Transaction = deserialize(&v).unwrap(); input_0.non_witness_utxo = Some(tx); - input_0.redeem_script = Some(hex_script!(redeem_script_0)); + input_0.redeem_script = Some(hex_script(redeem_script_0)); input_0.bip32_derivation = bip32_derivation(fingerprint, &pk_path, vec![0, 1]); let mut input_1 = psbt.inputs[1].clone(); @@ -253,8 +250,8 @@ fn update_psbt(mut psbt: Psbt, fingerprint: Fingerprint) -> Psbt { let tx: Transaction = deserialize(&v).unwrap(); input_1.witness_utxo = Some(tx.output[1].clone()); - input_1.redeem_script = Some(hex_script!(redeem_script_1)); - input_1.witness_script = Some(hex_script!(witness_script)); + input_1.redeem_script = Some(hex_script(redeem_script_1)); + input_1.witness_script = Some(hex_script(witness_script)); input_1.bip32_derivation = bip32_derivation(fingerprint, &pk_path, vec![2, 3]); psbt.inputs = vec![input_0, input_1];