← Watch feed
Move sighashtype FromStr tests to crypto
Public commit record
78/100 · Adequate
What the developer wrote
Authored by Mitchell Bagot
Move sighashtype FromStr tests to crypto
The EcdsaSighashType and TapSighashType FromStr tests are currently
located in bitcoin. This means testing regressions to the FromStr impl
requires running a significantly larger test set for no reason. These
should be located alongside the impls themselves in crypto.
Move sighashtype_fromstr_display tests to crypto::sighash.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification
What changed, and why it matters
This commit simply moves existing unit tests from one place in the codebase to another. It does not change any production code, behavior, or fix any bug. There is no security impact.
Recommended action
No security action needed. This is a routine test-maintenance refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit relocates the FromStr/Display unit tests for EcdsaSighashType and TapSighashType from bitcoin/src/blockdata/transaction.rs and bitcoin/src/crypto/sighash.rs into crypto/src/sighash.rs, where the underlying implementations live. The test logic is preserved verbatim. No implementation code is modified.
Changed components
bitcoin/src/blockdata/transaction.rsbitcoin/src/crypto/sighash.rscrypto/src/sighash.rsInspect captured patch +82 / −73
diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs
index ddbcf550..e5d2ddab 100644
--- a/bitcoin/src/blockdata/transaction.rs
+++ b/bitcoin/src/blockdata/transaction.rs
@@ -1308,7 +1308,6 @@ mod tests {
use crate::consensus::encode::{deserialize, serialize};
use crate::constants::WITNESS_SCALE_FACTOR;
use crate::script::ScriptSigBuf;
- use crate::sighash::EcdsaSighashType;
use crate::{hex, parse_int};
const SOME_TX: &str = "0100000001a15d57094aa7a21a28cb20b59aab8fc7d1149a3bdbcddba9c622e4f5f6a99ece010000006c493046022100f93bb0e7d8db7bd46e40132d1f8242026e045f03a0efe71bbb8e3f475e970d790221009337cd7f1f929f00cc6ff01f03729b069a7c21b59b1736ddfee5db5946c5da8c0121033b9b137ee87d5a812d6f506efdd37f0affa7ffc310711c06c7f3e097c9447c52ffffffff0100e1f505000000001976a9140389035a9225b3839e2bbf32d826a1e222031fd888ac00000000";
@@ -1628,40 +1627,6 @@ mod tests {
);
}
- #[test]
- fn sighashtype_fromstr_display() {
- let sighashtypes = [
- ("SIGHASH_ALL", EcdsaSighashType::All),
- ("SIGHASH_NONE", EcdsaSighashType::None),
- ("SIGHASH_SINGLE", EcdsaSighashType::Single),
- ("SIGHASH_ALL|SIGHASH_ANYONECANPAY", EcdsaSighashType::AllPlusAnyoneCanPay),
- ("SIGHASH_NONE|SIGHASH_ANYONECANPAY", EcdsaSighashType::NonePlusAnyoneCanPay),
- ("SIGHASH_SINGLE|SIGHASH_ANYONECANPAY", EcdsaSighashType::SinglePlusAnyoneCanPay),
- ];
- for (s, sht) in sighashtypes {
- assert_eq!(sht.to_string(), s);
- assert_eq!(s.parse::<EcdsaSighashType>().unwrap(), sht);
- }
- let sht_mistakes = [
- "SIGHASH_ALL | SIGHASH_ANYONECANPAY",
- "SIGHASH_NONE |SIGHASH_ANYONECANPAY",
- "SIGHASH_SINGLE| SIGHASH_ANYONECANPAY",
- "SIGHASH_ALL SIGHASH_ANYONECANPAY",
- "SIGHASH_NONE |",
- "SIGHASH_SIGNLE",
- "sighash_none",
- "Sighash_none",
- "SigHash_None",
- "SigHash_NONE",
- ];
- for s in sht_mistakes {
- assert_eq!(
- s.parse::<EcdsaSighashType>().unwrap_err().to_string(),
- format!("failed to parse '{}' as SIGHASH string", s)
- );
- }
- }
-
#[test]
fn huge_witness() {
let hex =
diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs
index c4bc5505..9d1976e6 100644
--- a/bitcoin/src/crypto/sighash.rs
+++ b/bitcoin/src/crypto/sighash.rs
@@ -1293,7 +1293,6 @@ where
mod tests {
#[cfg(feature = "serde")]
use alloc::string::String;
- use alloc::string::ToString;
use alloc::vec::Vec;
use hashes::HashEngine;
@@ -1803,43 +1802,6 @@ mod tests {
}
}
- #[test]
- fn sighashtype_fromstr_display() {
- let sighashtypes = [
- ("SIGHASH_DEFAULT", TapSighashType::Default),
- ("SIGHASH_ALL", TapSighashType::All),
- ("SIGHASH_NONE", TapSighashType::None),
- ("SIGHASH_SINGLE", TapSighashType::Single),
- ("SIGHASH_ALL|SIGHASH_ANYONECANPAY", TapSighashType::AllPlusAnyoneCanPay),
- ("SIGHASH_NONE|SIGHASH_ANYONECANPAY", TapSighashType::NonePlusAnyoneCanPay),
- ("SIGHASH_SINGLE|SIGHASH_ANYONECANPAY", TapSighashType::SinglePlusAnyoneCanPay),
- ];
- for (s, sht) in sighashtypes {
- assert_eq!(sht.to_string(), s);
- assert_eq!(s.parse::<TapSighashType>().unwrap(), sht);
- }
- let sht_mistakes = [
- "SIGHASH_ALL | SIGHASH_ANYONECANPAY",
- "SIGHASH_NONE |SIGHASH_ANYONECANPAY",
- "SIGHASH_SINGLE| SIGHASH_ANYONECANPAY",
- "SIGHASH_ALL SIGHASH_ANYONECANPAY",
- "SIGHASH_NONE |",
- "SIGHASH_SIGNLE",
- "DEFAULT",
- "ALL",
- "sighash_none",
- "Sighash_none",
- "SigHash_None",
- "SigHash_NONE",
- ];
- for s in sht_mistakes {
- assert_eq!(
- s.parse::<TapSighashType>().unwrap_err().to_string(),
- format!("failed to parse '{}' as SIGHASH string", s)
- );
- }
- }
-
#[test]
fn bip143_p2wpkh() {
let tx = deserialize::<Transaction>(
diff --git a/crypto/src/sighash.rs b/crypto/src/sighash.rs
index 1a5f6035..48202dbf 100644
--- a/crypto/src/sighash.rs
+++ b/crypto/src/sighash.rs
@@ -314,3 +314,85 @@ impl<'a> Arbitrary<'a> for TapSighashType {
}
}
}
+
+#[cfg(test)]
+mod tests {
+ #[cfg(feature = "alloc")]
+ use alloc::{format, string::ToString};
+
+ #[cfg(feature = "alloc")]
+ use super::*;
+
+ #[test]
+ #[cfg(feature = "alloc")]
+ fn tapsighashtype_fromstr_display() {
+ let sighashtypes = [
+ ("SIGHASH_DEFAULT", TapSighashType::Default),
+ ("SIGHASH_ALL", TapSighashType::All),
+ ("SIGHASH_NONE", TapSighashType::None),
+ ("SIGHASH_SINGLE", TapSighashType::Single),
+ ("SIGHASH_ALL|SIGHASH_ANYONECANPAY", TapSighashType::AllPlusAnyoneCanPay),
+ ("SIGHASH_NONE|SIGHASH_ANYONECANPAY", TapSighashType::NonePlusAnyoneCanPay),
+ ("SIGHASH_SINGLE|SIGHASH_ANYONECANPAY", TapSighashType::SinglePlusAnyoneCanPay),
+ ];
+ for (s, sht) in sighashtypes {
+ assert_eq!(sht.to_string(), s);
+ assert_eq!(s.parse::<TapSighashType>().unwrap(), sht);
+ }
+ let sht_mistakes = [
+ "SIGHASH_ALL | SIGHASH_ANYONECANPAY",
+ "SIGHASH_NONE |SIGHASH_ANYONECANPAY",
+ "SIGHASH_SINGLE| SIGHASH_ANYONECANPAY",
+ "SIGHASH_ALL SIGHASH_ANYONECANPAY",
+ "SIGHASH_NONE |",
+ "SIGHASH_SIGNLE",
+ "DEFAULT",
+ "ALL",
+ "sighash_none",
+ "Sighash_none",
+ "SigHash_None",
+ "SigHash_NONE",
+ ];
+ for s in sht_mistakes {
+ assert_eq!(
+ s.parse::<TapSighashType>().unwrap_err().to_string(),
+ format!("failed to parse '{}' as SIGHASH string", s)
+ );
+ }
+ }
+
+ #[test]
+ #[cfg(feature = "alloc")]
+ fn ecdsasighashtype_fromstr_display() {
+ let sighashtypes = [
+ ("SIGHASH_ALL", EcdsaSighashType::All),
+ ("SIGHASH_NONE", EcdsaSighashType::None),
+ ("SIGHASH_SINGLE", EcdsaSighashType::Single),
+ ("SIGHASH_ALL|SIGHASH_ANYONECANPAY", EcdsaSighashType::AllPlusAnyoneCanPay),
+ ("SIGHASH_NONE|SIGHASH_ANYONECANPAY", EcdsaSighashType::NonePlusAnyoneCanPay),
+ ("SIGHASH_SINGLE|SIGHASH_ANYONECANPAY", EcdsaSighashType::SinglePlusAnyoneCanPay),
+ ];
+ for (s, sht) in sighashtypes {
+ assert_eq!(sht.to_string(), s);
+ assert_eq!(s.parse::<EcdsaSighashType>().unwrap(), sht);
+ }
+ let sht_mistakes = [
+ "SIGHASH_ALL | SIGHASH_ANYONECANPAY",
+ "SIGHASH_NONE |SIGHASH_ANYONECANPAY",
+ "SIGHASH_SINGLE| SIGHASH_ANYONECANPAY",
+ "SIGHASH_ALL SIGHASH_ANYONECANPAY",
+ "SIGHASH_NONE |",
+ "SIGHASH_SIGNLE",
+ "sighash_none",
+ "Sighash_none",
+ "SigHash_None",
+ "SigHash_NONE",
+ ];
+ for s in sht_mistakes {
+ assert_eq!(
+ s.parse::<EcdsaSighashType>().unwrap_err().to_string(),
+ format!("failed to parse '{}' as SIGHASH string", s)
+ );
+ }
+ }
+}
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.