From 8c4899f2cc85204e0e7d5cf5df380782ed215088 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Mon, 24 Jun 2024 12:54:36 +0000 Subject: [PATCH] bitcoin: remove all direct use of hashing/engines in unit tests This is a continuation of the previous commit, but separated to make review a little easier. This one replaces test vectors that were previously computed by hashing garbage into Txids and various other hash types with new test vectors which are directly-computed garbage converted to hashes with from_byte_array. In one case (src/hash_types.rs) this results in changing a bunch of fixed test vectors. This is okay; this test is supposed to check the direction of string serialization, which is unaffected by this commit (or any commit in this PR). The existing test vectors, because they hash the empty string, result in different bytes depending on the underlying hash algo (sha256, sha256d, sha256t, etc). The new ones just use the same fixed test vector for all of them. This commit also updates a doctest in crypto/sighash.rs which demonstrates how to manually feed sighash data into a hash engine and correctly handle the sighash single bug. Because you can no longer directly get a sighash object from an engine, this particular example should maybe be rewritten to just encode to a Vec rather than a hash engine, explaining that maybe you'd do this when implementing a HWW, to verify the exact data being hashed. Or something. Unrelatedly, you can check that there are no API changes in this commit or the last several. The next commit will remove GeneralHash impls and that's when you'll see changes. --- bitcoin/src/bip152.rs | 7 +-- bitcoin/src/blockdata/script/tests.rs | 6 +-- bitcoin/src/crypto/sighash.rs | 6 ++- bitcoin/src/hash_types.rs | 66 ++++++++++++++++++--------- 4 files changed, 56 insertions(+), 29 deletions(-) diff --git a/bitcoin/src/bip152.rs b/bitcoin/src/bip152.rs index df432fc00..f8341ea9f 100644 --- a/bitcoin/src/bip152.rs +++ b/bitcoin/src/bip152.rs @@ -383,11 +383,12 @@ mod test { }; fn dummy_tx(nonce: &[u8]) -> Transaction { + let dummy_txid = Txid::from_byte_array(hashes::sha256::Hash::hash(nonce).to_byte_array()); Transaction { version: transaction::Version::ONE, lock_time: absolute::LockTime::from_consensus(2), input: vec![TxIn { - previous_output: OutPoint::new(Txid::hash(nonce), 0), + previous_output: OutPoint::new(dummy_txid, 0), script_sig: ScriptBuf::new(), sequence: Sequence(1), witness: Witness::new(), @@ -400,8 +401,8 @@ mod test { Block { header: block::Header { version: block::Version::ONE, - prev_blockhash: BlockHash::hash(&[0]), - merkle_root: TxMerkleNode::hash(&[1]), + prev_blockhash: BlockHash::from_byte_array([0x99; 32]), + merkle_root: TxMerkleNode::from_byte_array([0x77; 32]), time: 2, bits: CompactTarget::from_consensus(3), nonce: 4, diff --git a/bitcoin/src/blockdata/script/tests.rs b/bitcoin/src/blockdata/script/tests.rs index 82fdfb982..beaa3d1e3 100644 --- a/bitcoin/src/blockdata/script/tests.rs +++ b/bitcoin/src/blockdata/script/tests.rs @@ -6,7 +6,7 @@ use hex_lit::hex; use super::*; use crate::consensus::encode::{deserialize, serialize}; -use crate::crypto::key::{PubkeyHash, PublicKey, WPubkeyHash, XOnlyPublicKey}; +use crate::crypto::key::{PublicKey, XOnlyPublicKey}; use crate::FeeRate; #[test] @@ -203,10 +203,10 @@ fn script_generators() { .unwrap(); assert!(ScriptBuf::new_p2pk(pubkey).is_p2pk()); - let pubkey_hash = PubkeyHash::hash(&pubkey.inner.serialize()); + let pubkey_hash = pubkey.pubkey_hash(); assert!(ScriptBuf::new_p2pkh(pubkey_hash).is_p2pkh()); - let wpubkey_hash = WPubkeyHash::hash(&pubkey.inner.serialize()); + let wpubkey_hash = pubkey.wpubkey_hash().unwrap(); assert!(ScriptBuf::new_p2wpkh(wpubkey_hash).is_p2wpkh()); let script = Builder::new().push_opcode(OP_NUMEQUAL).push_verify().into_script(); diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index 1919e24ad..7d5fb0154 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -1366,10 +1366,10 @@ impl EncodeSigningDataResult { /// /// ```rust /// # use bitcoin::consensus::deserialize; - /// # use bitcoin::hashes::{Hash, hex::FromHex}; + /// # use bitcoin::hashes::{sha256d, Hash, hex::FromHex}; /// # use bitcoin::sighash::{LegacySighash, SighashCache}; /// # use bitcoin::Transaction; - /// # let mut writer = LegacySighash::engine(); + /// # let mut writer = sha256d::Hash::engine(); /// # let input_index = 0; /// # let script_pubkey = bitcoin::ScriptBuf::new(); /// # let sighash_u32 = 0u32; @@ -1381,6 +1381,8 @@ impl EncodeSigningDataResult { /// .is_sighash_single_bug() /// .expect("writer can't fail") { /// // use a hash value of "1", instead of computing the actual hash due to SIGHASH_SINGLE bug + /// } else { + /// // use the hash from `writer` /// } /// ``` pub fn is_sighash_single_bug(self) -> Result { diff --git a/bitcoin/src/hash_types.rs b/bitcoin/src/hash_types.rs index 58c008a7f..bcd2fc611 100644 --- a/bitcoin/src/hash_types.rs +++ b/bitcoin/src/hash_types.rs @@ -18,65 +18,89 @@ mod tests { WScriptHash, XKeyIdentifier, }; + #[rustfmt::skip] + /// sha256d of the empty string + const DUMMY32: [u8; 32] = [ + 0x5d, 0xf6, 0xe0, 0xe2, 0x76, 0x13, 0x59, 0xd3, + 0x0a, 0x82, 0x75, 0x05, 0x8e, 0x29, 0x9f, 0xcc, + 0x03, 0x81, 0x53, 0x45, 0x45, 0xf5, 0x5c, 0xf4, + 0x3e, 0x41, 0x98, 0x3f, 0x5d, 0x4c, 0x94, 0x56, + ]; + /// hash160 of the empty string + #[rustfmt::skip] + const DUMMY20: [u8; 20] = [ + 0xb4, 0x72, 0xa2, 0x66, 0xd0, 0xbd, 0x89, 0xc1, 0x37, 0x06, + 0xa4, 0x13, 0x2c, 0xcf, 0xb1, 0x6f, 0x7c, 0x3b, 0x9f, 0xcb, + ]; + #[test] fn hash_display() { assert_eq!( - Txid::hash(&[]).to_string(), + Txid::from_byte_array(DUMMY32).to_string(), "56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d", ); assert_eq!( - Wtxid::hash(&[]).to_string(), + Wtxid::from_byte_array(DUMMY32).to_string(), "56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d", ); assert_eq!( - BlockHash::hash(&[]).to_string(), + BlockHash::from_byte_array(DUMMY32).to_string(), "56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d", ); assert_eq!( - LegacySighash::hash(&[]).to_string(), + LegacySighash::from_byte_array(DUMMY32).to_string(), "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456", ); assert_eq!( - SegwitV0Sighash::hash(&[]).to_string(), + SegwitV0Sighash::from_byte_array(DUMMY32).to_string(), "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456", ); assert_eq!( - TapSighash::hash(&[]).to_string(), - "dabc11914abcd8072900042a2681e52f8dba99ce82e224f97b5fdb7cd4b9c803", - ); - - assert_eq!(PubkeyHash::hash(&[]).to_string(), "b472a266d0bd89c13706a4132ccfb16f7c3b9fcb",); - assert_eq!(ScriptHash::hash(&[]).to_string(), "b472a266d0bd89c13706a4132ccfb16f7c3b9fcb",); - assert_eq!(WPubkeyHash::hash(&[]).to_string(), "b472a266d0bd89c13706a4132ccfb16f7c3b9fcb",); - assert_eq!( - WScriptHash::hash(&[]).to_string(), - "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + TapSighash::from_byte_array(DUMMY32).to_string(), + "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456", ); assert_eq!( - TxMerkleNode::hash(&[]).to_string(), + PubkeyHash::from_byte_array(DUMMY20).to_string(), + "b472a266d0bd89c13706a4132ccfb16f7c3b9fcb", + ); + assert_eq!( + ScriptHash::from_byte_array(DUMMY20).to_string(), + "b472a266d0bd89c13706a4132ccfb16f7c3b9fcb", + ); + assert_eq!( + WPubkeyHash::from_byte_array(DUMMY20).to_string(), + "b472a266d0bd89c13706a4132ccfb16f7c3b9fcb", + ); + assert_eq!( + WScriptHash::from_byte_array(DUMMY32).to_string(), + "5df6e0e2761359d30a8275058e299fcc0381534545f55cf43e41983f5d4c9456", + ); + + assert_eq!( + TxMerkleNode::from_byte_array(DUMMY32).to_string(), "56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d", ); assert_eq!( - WitnessMerkleNode::hash(&[]).to_string(), + WitnessMerkleNode::from_byte_array(DUMMY32).to_string(), "56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d", ); assert_eq!( - WitnessCommitment::hash(&[]).to_string(), + WitnessCommitment::from_byte_array(DUMMY32).to_string(), "56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d", ); assert_eq!( - XKeyIdentifier::hash(&[]).to_string(), + XKeyIdentifier::from_byte_array(DUMMY20).to_string(), "b472a266d0bd89c13706a4132ccfb16f7c3b9fcb", ); assert_eq!( - FilterHash::hash(&[]).to_string(), + FilterHash::from_byte_array(DUMMY32).to_string(), "56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d", ); assert_eq!( - FilterHeader::hash(&[]).to_string(), + FilterHeader::from_byte_array(DUMMY32).to_string(), "56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d", ); }