Merge rust-bitcoin/rust-bitcoin#3468: Move `block::Header` to `primitives`

ff64fa3c46 Move block::Header to primitives (Tobin C. Harding)
65925117a0 Manually implement block::Header::block_hash (Tobin C. Harding)
b02ab25342 Add regression test for block_hash function (Tobin C. Harding)
5614694b69 block: Remove wildcard re-export (Tobin C. Harding)

Pull request description:

  - Patch 1: Trivial preparation.
  - Patch 2 and 3: Reduce the API surface in a disappointing way due to the orphan rule.
  - Patch 4: Do the move.

ACKs for top commit:
  apoelstra:
    ACK ff64fa3c46 successfully ran local tests; nice!

Tree-SHA512: 8d93de4a12c9f71f9dfbdc023fd8defb78f7d3da995490af8f83a927c2ca6338dd236bc08691311dc7345183fb6d0be61eedce3e4424d4060be2de934facd60a

bitcoin/src/blockdata/block.rs

@@ -20,103 +20,54 @@ use crate::consensus::{encode, Decodable, Encodable};
 use crate::internal_macros::{impl_consensus_encoding, impl_hashencode};
 use crate::merkle_tree::{MerkleNode as _, TxMerkleNode, WitnessMerkleNode};
 use crate::network::Params;
-use crate::pow::{CompactTarget, Target, Work};
+use crate::pow::{Target, Work};
 use crate::prelude::Vec;
 use crate::script::{self, ScriptExt as _};
 use crate::transaction::{Transaction, Wtxid};
 
 #[rustfmt::skip]                // Keep public re-exports separate.
 #[doc(inline)]
-pub use primitives::block::*;
+pub use primitives::block::{Version, BlockHash, Header, WitnessCommitment};
 
 impl_hashencode!(BlockHash);
 
-/// Bitcoin block header.
-///
-/// Contains all the block's information except the actual transactions, but
-/// including a root of a [Merkle tree] committing to all transactions in the block.
-///
-/// [Merkle tree]: https://en.wikipedia.org/wiki/Merkle_tree
-///
-/// ### Bitcoin Core References
-///
-/// * [CBlockHeader definition](https://github.com/bitcoin/bitcoin/blob/345457b542b6a980ccfbc868af0970a6f91d1b82/src/primitives/block.h#L20)
-#[derive(Copy, PartialEq, Eq, Clone, PartialOrd, Ord, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub struct Header {
-    /// Block version, now repurposed for soft fork signalling.
-    pub version: Version,
-    /// Reference to the previous block in the chain.
-    pub prev_blockhash: BlockHash,
-    /// The root hash of the Merkle tree of transactions in the block.
-    pub merkle_root: TxMerkleNode,
-    /// The timestamp of the block, as claimed by the miner.
-    pub time: u32,
-    /// The target value below which the blockhash must lie.
-    pub bits: CompactTarget,
-    /// The nonce, selected to obtain a low enough blockhash.
-    pub nonce: u32,
-}
-
 impl_consensus_encoding!(Header, version, prev_blockhash, merkle_root, time, bits, nonce);
 
-impl Header {
+crate::internal_macros::define_extension_trait! {
-    /// The number of bytes that the block header contributes to the size of a block.
+    /// Extension functionality for the [`Header`] type.
-    // Serialized length of fields (version, prev_blockhash, merkle_root, time, bits, nonce)
+    pub trait HeaderExt impl for Header {
-    pub const SIZE: usize = 4 + 32 + 32 + 4 + 4 + 4; // 80
+        /// Computes the target (range [0, T] inclusive) that a blockhash must land in to be valid.
+        fn target(&self) -> Target { self.bits.into() }
 
-    /// Returns the block hash.
+        /// Computes the popular "difficulty" measure for mining.
-    pub fn block_hash(&self) -> BlockHash {
+        ///
-        let mut engine = sha256d::Hash::engine();
+        /// Difficulty represents how difficult the current target makes it to find a block, relative to
-        self.consensus_encode(&mut engine).expect("engines don't error");
+        /// how difficult it would be at the highest possible target (highest target == lowest difficulty).
-        BlockHash::from_byte_array(sha256d::Hash::from_engine(engine).to_byte_array())
+        fn difficulty(&self, params: impl AsRef<Params>) -> u128 {
-    }
+            self.target().difficulty(params)
-
-    /// Computes the target (range [0, T] inclusive) that a blockhash must land in to be valid.
-    pub fn target(&self) -> Target { self.bits.into() }
-
-    /// Computes the popular "difficulty" measure for mining.
-    ///
-    /// Difficulty represents how difficult the current target makes it to find a block, relative to
-    /// how difficult it would be at the highest possible target (highest target == lowest difficulty).
-    pub fn difficulty(&self, params: impl AsRef<Params>) -> u128 {
-        self.target().difficulty(params)
-    }
-
-    /// Computes the popular "difficulty" measure for mining and returns a float value of f64.
-    pub fn difficulty_float(&self, params: impl AsRef<Params>) -> f64 {
-        self.target().difficulty_float(params)
-    }
-
-    /// Checks that the proof-of-work for the block is valid, returning the block hash.
-    pub fn validate_pow(&self, required_target: Target) -> Result<BlockHash, ValidationError> {
-        let target = self.target();
-        if target != required_target {
-            return Err(ValidationError::BadTarget);
         }
-        let block_hash = self.block_hash();
+
-        if target.is_met_by(block_hash) {
+        /// Computes the popular "difficulty" measure for mining and returns a float value of f64.
-            Ok(block_hash)
+        fn difficulty_float(&self, params: impl AsRef<Params>) -> f64 {
-        } else {
+            self.target().difficulty_float(params)
-            Err(ValidationError::BadProofOfWork)
         }
-    }
 
-    /// Returns the total work of the block.
+        /// Checks that the proof-of-work for the block is valid, returning the block hash.
-    pub fn work(&self) -> Work { self.target().to_work() }
+        fn validate_pow(&self, required_target: Target) -> Result<BlockHash, ValidationError> {
-}
+            let target = self.target();
+            if target != required_target {
+                return Err(ValidationError::BadTarget);
+            }
+            let block_hash = self.block_hash();
+            if target.is_met_by(block_hash) {
+                Ok(block_hash)
+            } else {
+                Err(ValidationError::BadProofOfWork)
+            }
+        }
 
-impl fmt::Debug for Header {
+        /// Returns the total work of the block.
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        fn work(&self) -> Work { self.target().to_work() }
-        f.debug_struct("Header")
-            .field("block_hash", &self.block_hash())
-            .field("version", &self.version)
-            .field("prev_blockhash", &self.prev_blockhash)
-            .field("merkle_root", &self.merkle_root)
-            .field("time", &self.time)
-            .field("bits", &self.bits)
-            .field("nonce", &self.nonce)
-            .finish()
     }
 }
 
@@ -308,14 +259,6 @@ impl Block {
     }
 }
 
-impl From<Header> for BlockHash {
-    fn from(header: Header) -> BlockHash { header.block_hash() }
-}
-
-impl From<&Header> for BlockHash {
-    fn from(header: &Header) -> BlockHash { header.block_hash() }
-}
-
 impl From<Block> for BlockHash {
     fn from(block: Block) -> BlockHash { block.block_hash() }
 }
@@ -400,20 +343,6 @@ impl std::error::Error for ValidationError {
     }
 }
 
-#[cfg(feature = "arbitrary")]
-impl<'a> Arbitrary<'a> for Header {
-    fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
-        Ok(Header {
-            version: Version::arbitrary(u)?,
-            prev_blockhash: BlockHash::from_byte_array(u.arbitrary()?),
-            merkle_root: TxMerkleNode::from_byte_array(u.arbitrary()?),
-            time: u.arbitrary()?,
-            bits: CompactTarget::from_consensus(u.arbitrary()?),
-            nonce: u.arbitrary()?,
-        })
-    }
-}
-
 #[cfg(feature = "arbitrary")]
 impl<'a> Arbitrary<'a> for Block {
     fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
@@ -428,7 +357,7 @@ mod tests {
 
     use super::*;
     use crate::consensus::encode::{deserialize, serialize};
-    use crate::{Network, TestnetVersion};
+    use crate::{CompactTarget, Network, TestnetVersion};
 
     #[test]
     fn test_coinbase_and_bip34() {
@@ -572,16 +501,27 @@ mod tests {
         }
     }
 
+    fn header() -> Header {
+        let header = hex!("010000004ddccd549d28f385ab457e98d1b11ce80bfea2c5ab93015ade4973e400000000bf4473e53794beae34e64fccc471dace6ae544180816f89591894e0f417a914cd74d6e49ffff001d323b3a7b");
+        deserialize(&header).expect("can't deserialize correct block header")
+    }
+
     #[test]
     fn compact_roundrtip_test() {
-        let some_header = hex!("010000004ddccd549d28f385ab457e98d1b11ce80bfea2c5ab93015ade4973e400000000bf4473e53794beae34e64fccc471dace6ae544180816f89591894e0f417a914cd74d6e49ffff001d323b3a7b");
+        let header = header();
-
-        let header: Header =
-            deserialize(&some_header).expect("can't deserialize correct block header");
-
         assert_eq!(header.bits, header.target().to_compact_lossy());
     }
 
+    #[test]
+    fn header_block_hash_regression() {
+        let header = header();
+        let block_hash = "00000000b0c5a240b2a61d2e75692224efd4cbecdf6eaf4cc2cf477ca7c270e7";
+
+        let want = block_hash.parse::<BlockHash>().unwrap();
+        let got = header.block_hash();
+        assert_eq!(got, want)
+    }
+
     #[test]
     fn soft_fork_signalling() {
         for i in 0..31 {

bitcoin/src/pow.rs

@@ -254,7 +254,7 @@ impl Target {
     /// Panics if `self` is zero (divide by zero).
     ///
     /// [max]: Target::max
-    /// [target]: crate::block::Header::target
+    /// [target]: crate::block::HeaderExt::target
     #[cfg_attr(all(test, mutate), mutate)]
     pub fn difficulty(&self, params: impl AsRef<Params>) -> u128 {
         // Panic here may be eaiser to debug than during the actual division.

primitives/src/block.rs

@@ -7,9 +7,83 @@
 //! module describes structures and functions needed to describe
 //! these blocks and the blockchain.
 
+use core::fmt;
+
 #[cfg(feature = "arbitrary")]
 use arbitrary::{Arbitrary, Unstructured};
-use hashes::sha256d;
+use hashes::{sha256d, HashEngine as _};
+
+use crate::merkle_tree::TxMerkleNode;
+use crate::pow::CompactTarget;
+
+/// Bitcoin block header.
+///
+/// Contains all the block's information except the actual transactions, but
+/// including a root of a [Merkle tree] committing to all transactions in the block.
+///
+/// [Merkle tree]: https://en.wikipedia.org/wiki/Merkle_tree
+///
+/// ### Bitcoin Core References
+///
+/// * [CBlockHeader definition](https://github.com/bitcoin/bitcoin/blob/345457b542b6a980ccfbc868af0970a6f91d1b82/src/primitives/block.h#L20)
+#[derive(Copy, PartialEq, Eq, Clone, PartialOrd, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+pub struct Header {
+    /// Block version, now repurposed for soft fork signalling.
+    pub version: Version,
+    /// Reference to the previous block in the chain.
+    pub prev_blockhash: BlockHash,
+    /// The root hash of the Merkle tree of transactions in the block.
+    pub merkle_root: TxMerkleNode,
+    /// The timestamp of the block, as claimed by the miner.
+    pub time: u32,
+    /// The target value below which the blockhash must lie.
+    pub bits: CompactTarget,
+    /// The nonce, selected to obtain a low enough blockhash.
+    pub nonce: u32,
+}
+
+impl Header {
+    /// The number of bytes that the block header contributes to the size of a block.
+    // Serialized length of fields (version, prev_blockhash, merkle_root, time, bits, nonce)
+    pub const SIZE: usize = 4 + 32 + 32 + 4 + 4 + 4; // 80
+
+    /// Returns the block hash.
+    // This is the same as `Encodable` but done manually because `Encodable` isn't in `primitives`.
+    pub fn block_hash(&self) -> BlockHash {
+        let mut engine = sha256d::Hash::engine();
+        engine.input(&self.version.to_consensus().to_le_bytes());
+        engine.input(self.prev_blockhash.as_byte_array());
+        engine.input(self.merkle_root.as_byte_array());
+        engine.input(&self.time.to_le_bytes());
+        engine.input(&self.bits.to_consensus().to_le_bytes());
+        engine.input(&self.nonce.to_le_bytes());
+
+        BlockHash::from_byte_array(sha256d::Hash::from_engine(engine).to_byte_array())
+    }
+}
+
+impl fmt::Debug for Header {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct("Header")
+            .field("block_hash", &self.block_hash())
+            .field("version", &self.version)
+            .field("prev_blockhash", &self.prev_blockhash)
+            .field("merkle_root", &self.merkle_root)
+            .field("time", &self.time)
+            .field("bits", &self.bits)
+            .field("nonce", &self.nonce)
+            .finish()
+    }
+}
+
+impl From<Header> for BlockHash {
+    fn from(header: Header) -> BlockHash { header.block_hash() }
+}
+
+impl From<&Header> for BlockHash {
+    fn from(header: &Header) -> BlockHash { header.block_hash() }
+}
 
 /// Bitcoin block version number.
 ///
@@ -93,6 +167,20 @@ impl BlockHash {
     pub const GENESIS_PREVIOUS_BLOCK_HASH: Self = Self::from_byte_array([0; 32]);
 }
 
+#[cfg(feature = "arbitrary")]
+impl<'a> Arbitrary<'a> for Header {
+    fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
+        Ok(Header {
+            version: Version::arbitrary(u)?,
+            prev_blockhash: BlockHash::from_byte_array(u.arbitrary()?),
+            merkle_root: TxMerkleNode::from_byte_array(u.arbitrary()?),
+            time: u.arbitrary()?,
+            bits: CompactTarget::from_consensus(u.arbitrary()?),
+            nonce: u.arbitrary()?,
+        })
+    }
+}
+
 #[cfg(feature = "arbitrary")]
 impl<'a> Arbitrary<'a> for Version {
     fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {