Merge #278: Clean up hashes and related types ahead of bitcoin_hashes 1.0

fa3ea85 address: introduce constructors for (W)ScriptHash (Andrew Poelstra)
b553acb re-export WPubkeyHash and PubkeyHash rather than defining our own (Andrew Poelstra)
5c22659 dynafed: use newtypes for the various merkle roots in dynafed params (Andrew Poelstra)
8153c4a issuance: also back AssetId by [u8; 32] rather than Midstate (Andrew Poelstra)
b985342 issuance: move AssetId boilerplate into macro (Andrew Poelstra)
da8942a issuance: use newtype rather than bare sha256::Midstate for asset entropy (Andrew Poelstra)
5f54fef hash_types: add serde regression tests for all 32-byte hashes (Andrew Poelstra)
90183f4 pset: remove a bunch of other ununused error variants (Andrew Poelstra)
7ec80fe pset: remove unused FromSliceError (Andrew Poelstra)
da967f8 pset: eliminate a use of Hash::from_slice (Andrew Poelstra)
0c006d7 sighash: strongly type leaf_version field in SighashCache (Andrew Poelstra)
dc701a5 taproot: use TapNodeHash for internal hashes in Taptrees (Andrew Poelstra)
dbfbaf8 serde: modernize serde imports (Andrew Poelstra)

Pull request description:

  I am going to PR soon to update to bitcoin-hashes 1.0. There are several big changes versus the old version of bitcoin-hashes that we were using:
  
  * Hash wrapper types (like `Txid`, `Blockhash`, etc) no longer have general hashing methods like `hash` that let you hash up arbitrary data; the intention of these types is that they only be constructable from byte slices or by hashing the right kind of data
  * We no longer directly support construction from byte slices; instead you must use arrays. stdlib has a `TryFrom<&[u8; N]> for &[u8]` impl which is just a pointer cast, so you can get the old behavior by using this `TryFrom`
  * The `sha256::Midstate` type now carries a length with it, so it behaves something like a deconstructed hash engine rather than being a "hash" which is computed from the sha256 compression function; you can no longer effectively wrap a midstate the way you would wrap a sha256 hash, so instead we directly wrap `[u8; 32]`s for the types that did this
  * Many changes to make the macros more ergonomic, standard trait impls more consistent and complete, etc
  
  This PR makes a bunch of prepatory changes to make this transition smaller. In particular, it
  
  * eliminate all the bare `sha256::Midstate` uses by introducing newtypes for each midstate (asset entropy, dynafed elided params, etc)
  * replace slices with arrays in many places throughout the codebase, eliminating a ton of unreachable panic paths
  * **makes `Address::p2wpkh` and `Address::p2shwpkh` panic if you give them uncompressed keys** rather than producing unspendable addresses; later we will clean this up further by type-separating compressed keys, but for now ISTM that a panic is better than silently creating black-hole addresses
  * does a whole pile of code cleanups


ACKs for top commit:
  stringhandler:
    utACK fa3ea85


Tree-SHA512: c051ec3a8578072e612164d39cf07dddbbf50b14a0d9d1ce25f0a57473b9d8d54d897b5f4ecc3ee9687e4f67cb08d4b025a08271a234d7fa5e0060b41fcaa354

Author: apoelstra

Cargo.toml

@@ -19,10 +19,10 @@ default = ["json-contract"]
 
 json-contract = ["serde_json"]
 "serde" = [
+    "dep:serde",
     "bitcoin/serde",
     "bitcoin/serde",
     "secp256k1-zkp/serde",
-    "actual-serde",
 ]
 base64 = ["bitcoin/base64"]
 
@@ -33,10 +33,7 @@ secp256k1-zkp = { version = "0.11.0", features = ["global-context", "hashes"] }
 
 # Used for ContractHash::from_json_contract.
 serde_json = { version = "1.0", optional = true }
-
-actual-serde = { package = "serde", version = "1.0.103", features = [
-    "derive",
-], optional = true }
+serde = { version = "1.0.103", features = [ "derive" ], optional = true }
 hex = { package = "hex-conservative", version = "1.1.0" }
 
 

src/address.rs

@@ -36,7 +36,7 @@ use crate::schnorr::{TapTweak, TweakedPublicKey, UntweakedPublicKey};
 use crate::taproot::TapNodeHash;
 
 use crate::{opcodes, script};
-use crate::{PubkeyHash, ScriptHash, WPubkeyHash, WScriptHash};
+use crate::{PubkeyHash, ScriptHash, WScriptHash};
 
 /// Encoding error
 #[derive(Debug, PartialEq)]
@@ -236,13 +236,9 @@ impl Address {
         blinder: Option<secp256k1_zkp::PublicKey>,
         params: &'static AddressParams,
     ) -> Address {
-        let mut hash_engine = PubkeyHash::engine();
-        pk.write_into(&mut hash_engine)
-            .expect("engines don't error");
-
         Address {
             params,
-            payload: Payload::PubkeyHash(PubkeyHash::from_engine(hash_engine)),
+            payload: Payload::PubkeyHash(pk.pubkey_hash()),
             blinding_pubkey: blinder,
         }
     }
@@ -257,50 +253,51 @@ impl Address {
     ) -> Address {
         Address {
             params,
-            payload: Payload::ScriptHash(ScriptHash::hash(&script[..])),
+            payload: Payload::ScriptHash(ScriptHash::hash_script(script)),
             blinding_pubkey: blinder,
         }
     }
 
     /// Create a witness pay to public key address from a public key
     /// This is the native segwit address type for an output redeemable with a single signature
+    ///
+    /// # Panics
+    ///
+    /// Panics if the provided public key is not compressed.
     pub fn p2wpkh(
         pk: &PublicKey,
         blinder: Option<secp256k1_zkp::PublicKey>,
         params: &'static AddressParams,
     ) -> Address {
-        let mut hash_engine = WPubkeyHash::engine();
-        pk.write_into(&mut hash_engine)
-            .expect("engines don't error");
-
         Address {
             params,
             payload: Payload::WitnessProgram {
                 version: Fe32::Q,
-                program: WPubkeyHash::from_engine(hash_engine)[..].to_vec(),
+                program: pk.wpubkey_hash().expect("public key must be compressed").as_byte_array().to_vec(),
             },
             blinding_pubkey: blinder,
         }
     }
 
     /// Create a pay to script address that embeds a witness pay to public key
     /// This is a segwit address type that looks familiar (as p2sh) to legacy clients
+    ///
+    /// # Panics
+    ///
+    /// Panics if the provided public key is not compressed.
     pub fn p2shwpkh(
         pk: &PublicKey,
         blinder: Option<secp256k1_zkp::PublicKey>,
         params: &'static AddressParams,
     ) -> Address {
-        let mut hash_engine = ScriptHash::engine();
-        pk.write_into(&mut hash_engine)
-            .expect("engines don't error");
-
+        let pkh = pk.wpubkey_hash().expect("public key must be compressed");
         let builder = script::Builder::new()
             .push_int(0)
-            .push_slice(&ScriptHash::from_engine(hash_engine)[..]);
+            .push_slice(pkh.as_ref());
 
         Address {
             params,
-            payload: Payload::ScriptHash(ScriptHash::hash(builder.into_script().as_bytes())),
+            payload: Payload::ScriptHash(ScriptHash::hash_script(&builder.into_script())),
             blinding_pubkey: blinder,
         }
     }
@@ -315,7 +312,7 @@ impl Address {
             params,
             payload: Payload::WitnessProgram {
                 version: Fe32::Q,
-                program: WScriptHash::hash(&script[..])[..].to_vec(),
+                program: WScriptHash::hash_script(script).as_byte_array().to_vec(),
             },
             blinding_pubkey: blinder,
         }

src/blind.rs

@@ -15,6 +15,7 @@
 //! # Transactions Blinding
 //!
 
+use core::convert::TryFrom;
 use std::{self, collections::BTreeMap, fmt};
 
 use secp256k1_zkp::{
@@ -26,7 +27,6 @@ use secp256k1_zkp::{Generator, RangeProof, Secp256k1, Signing, SurjectionProof};
 
 use crate::{AddressParams, Script, TxIn};
 
-use crate::hashes;
 use crate::{
     confidential::{Asset, AssetBlindingFactor, Nonce, Value, ValueBlindingFactor},
     Address, AssetId, Transaction, TxOut, TxOutWitness,
@@ -224,8 +224,7 @@ impl RangeProofMessage {
 /// Information about Transaction Input Asset
 #[cfg_attr(
     feature = "serde",
-    derive(Serialize, Deserialize),
-    serde(crate = "actual_serde")
+    derive(serde::Serialize, serde::Deserialize),
 )]
 #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
 pub struct TxOutSecrets {
@@ -763,7 +762,8 @@ impl TxOut {
         )?;
 
         let (asset, asset_bf) = opening.message.as_ref().split_at(32);
-        let asset = AssetId::from_slice(asset)?;
+        let asset = <[u8; 32]>::try_from(asset).map_err(UnblindError::MalformedAssetId)?;
+        let asset = AssetId::from_byte_array(asset);
         let asset_bf = AssetBlindingFactor::from_slice(&asset_bf[..32])?;
 
         let value = opening.value;
@@ -788,7 +788,7 @@ pub enum UnblindError {
     /// Transaction output does not have a rangeproof.
     MissingRangeproof,
     /// Malformed asset ID.
-    MalformedAssetId(hashes::FromSliceError),
+    MalformedAssetId(core::array::TryFromSliceError),
     /// Error originated in `secp256k1_zkp`.
     Upstream(secp256k1_zkp::Error),
 }
@@ -823,12 +823,6 @@ impl From<secp256k1_zkp::Error> for UnblindError {
     }
 }
 
-impl From<hashes::FromSliceError> for UnblindError {
-    fn from(from: hashes::FromSliceError) -> Self {
-        UnblindError::MalformedAssetId(from)
-    }
-}
-
 impl TxIn {
     /// Blind issuances for this [`TxIn`]. Asset amount and token amount must be
     /// set in [`AssetIssuance`](crate::AssetIssuance) field for this input
@@ -1516,7 +1510,7 @@ mod tests {
 
     #[test]
     fn blind_value_proof_test() {
-        let id = AssetId::from_slice(&[1u8; 32]).unwrap();
+        let id = AssetId::from_byte_array([1u8; 32]);
         let abf = AssetBlindingFactor::new(&mut thread_rng());
         let asset = confidential::Asset::new_confidential(SECP256K1, id, abf);
 
@@ -1542,7 +1536,7 @@ mod tests {
 
     #[test]
     fn blind_asset_proof_test() {
-        let id = AssetId::from_slice(&[1u8; 32]).unwrap();
+        let id = AssetId::from_byte_array([1u8; 32]);
         let abf = AssetBlindingFactor::new(&mut thread_rng());
         let asset = confidential::Asset::new_confidential(SECP256K1, id, abf);
 

src/block.rs

@@ -21,11 +21,16 @@ use std::io;
 #[cfg(feature = "serde")] use std::fmt;
 
 use crate::dynafed;
-use crate::hashes::{Hash, sha256};
+use crate::hashes::Hash;
 use crate::Transaction;
 use crate::encode::{self, serialize, Decodable, Encodable, VarInt};
 use crate::{BlockHash, Script, TxMerkleNode};
 
+impl_sha256_midstate_wrapper! {
+    /// The Merkle root of a set of dynafed parameters.
+    pub struct DynafedRoot([u8; 32]);
+}
+
 /// Data related to block signatures
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]
 pub enum ExtData {
@@ -268,15 +273,15 @@ impl BlockHeader {
     }
 
     /// Calculate the root of the dynafed params. Returns [None] when not dynafed.
-    pub fn calculate_dynafed_params_root(&self) -> Option<sha256::Midstate> {
+    pub fn calculate_dynafed_params_root(&self) -> Option<crate::DynafedRoot> {
         match self.ext {
             ExtData::Proof { .. } => None,
             ExtData::Dynafed { ref current, ref proposed, .. } => {
                 let leaves = [
                     current.calculate_root().to_byte_array(),
                     proposed.calculate_root().to_byte_array(),
                 ];
-                Some(crate::fast_merkle_root::fast_merkle_root(&leaves[..]))
+                Some(DynafedRoot::from_midstate(crate::fast_merkle_root::fast_merkle_root(&leaves[..])))
             }
         }
     }

src/confidential.rs

@@ -1102,7 +1102,6 @@ impl<'de> Deserialize<'de> for ValueBlindingFactor {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::hashes::sha256;
 
     #[cfg(feature = "serde")]
     use std::str::FromStr;
@@ -1144,7 +1143,7 @@ mod tests {
 
         let assets = [
             Asset::Null,
-            Asset::Explicit(AssetId::from_inner(sha256::Midstate::from_byte_array([0; 32]))),
+            Asset::Explicit(AssetId::from_byte_array([0; 32])),
             Asset::from_commitment(&[
                 0x0a, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
                 1, 1, 1, 1, 1, 1,

src/dynafed.rs

@@ -23,9 +23,19 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
 use serde::ser::{SerializeSeq, SerializeStruct};
 
 use crate::encode::{self, Encodable, Decodable};
-use crate::hashes::{Hash, sha256, sha256d};
+use crate::hashes::{Hash, sha256d};
 use crate::Script;
 
+impl_sha256_midstate_wrapper! {
+    /// The Merkle root of a set of dynafed parameters.
+    pub struct ParamsRoot([u8; 32]);
+}
+
+impl_sha256_midstate_wrapper! {
+    /// A hash of elided dynafed parameter data.
+    pub struct ElidedRoot([u8; 32]);
+}
+
 /// ad-hoc struct to fmt in hex
 #[cfg(feature = "serde")]
 struct HexBytes<'a>(&'a [u8]);
@@ -109,7 +119,7 @@ impl FullParams {
     /// Return the `extra root` of this params.
     /// The extra root commits to the consensus parameters unrelated to
     /// blocksigning: `fedpeg_program`, `fedpegscript` and `extension_space`.
-    fn extra_root(&self) -> sha256::Midstate {
+    fn extra_root(&self) -> ElidedRoot {
         fn serialize_hash<E: Encodable>(obj: &E) -> sha256d::Hash {
             let mut engine = sha256d::Hash::engine();
             obj.consensus_encode(&mut engine).expect("engines don't error");
@@ -121,11 +131,11 @@ impl FullParams {
             serialize_hash(&self.fedpegscript).to_byte_array(),
             serialize_hash(&self.extension_space).to_byte_array(),
         ];
-        crate::fast_merkle_root::fast_merkle_root(&leaves[..])
+        ElidedRoot::from_midstate(crate::fast_merkle_root::fast_merkle_root(&leaves[..]))
     }
 
     /// Calculate the root of this [`FullParams`].
-    pub fn calculate_root(&self) -> sha256::Midstate {
+    pub fn calculate_root(&self) -> ParamsRoot {
         fn serialize_hash<E: Encodable>(obj: &E) -> sha256d::Hash {
             let mut engine = sha256d::Hash::engine();
             obj.consensus_encode(&mut engine).expect("engines don't error");
@@ -142,7 +152,7 @@ impl FullParams {
             compact_root.to_byte_array(),
             self.extra_root().to_byte_array(),
         ];
-        crate::fast_merkle_root::fast_merkle_root(&leaves[..])
+        ParamsRoot::from_midstate(crate::fast_merkle_root::fast_merkle_root(&leaves[..]))
     }
 
     /// Turns parameters into compact parameters.
@@ -223,7 +233,7 @@ pub enum Params {
         /// Maximum, in bytes, of the size of a blocksigning witness
         signblock_witness_limit: u32,
         /// Merkle root of extra data
-        elided_root: sha256::Midstate,
+        elided_root: ElidedRoot,
     },
     /// Full dynamic federations parameters
     Full(FullParams),
@@ -319,7 +329,7 @@ impl Params {
     }
 
     /// Get the `elided_root`. Is [None] for non-[`Params::Compact`] params.
-    pub fn elided_root(&self) -> Option<&sha256::Midstate> {
+    pub fn elided_root(&self) -> Option<&ElidedRoot> {
         match *self {
             Params::Null => None,
             Params::Compact { ref elided_root, ..} => Some(elided_root),
@@ -330,24 +340,24 @@ impl Params {
     /// Return the `extra root` of this params.
     /// The extra root commits to the consensus parameters unrelated to
     /// blocksigning: `fedpeg_program`, `fedpegscript` and `extension_space`.
-    fn extra_root(&self) -> sha256::Midstate {
+    fn extra_root(&self) -> ElidedRoot {
         match *self {
-            Params::Null => sha256::Midstate::from_byte_array([0u8; 32]),
+            Params::Null => ElidedRoot::from_byte_array([0u8; 32]),
             Params::Compact { ref elided_root, .. } => *elided_root,
             Params::Full(ref f) => f.extra_root(),
         }
     }
 
     /// Calculate the root of this [Params].
-    pub fn calculate_root(&self) -> sha256::Midstate {
+    pub fn calculate_root(&self) -> ParamsRoot {
         fn serialize_hash<E: Encodable>(obj: &E) -> sha256d::Hash {
             let mut engine = sha256d::Hash::engine();
             obj.consensus_encode(&mut engine).expect("engines don't error");
             sha256d::Hash::from_engine(engine)
         }
 
         if self.is_null() {
-            return sha256::Midstate::from_byte_array([0u8; 32]);
+            return ParamsRoot::from_byte_array([0u8; 32]);
         }
 
         let leaves = [
@@ -360,7 +370,7 @@ impl Params {
             compact_root.to_byte_array(),
             self.extra_root().to_byte_array(),
         ];
-        crate::fast_merkle_root::fast_merkle_root(&leaves[..])
+        ParamsRoot::from_midstate(crate::fast_merkle_root::fast_merkle_root(&leaves[..]))
     }
 
     /// Get the full params when this params are full.
@@ -626,7 +636,7 @@ impl Decodable for Params {
             1 => Ok(Params::Compact {
                 signblockscript: Decodable::consensus_decode(&mut d)?,
                 signblock_witness_limit: Decodable::consensus_decode(&mut d)?,
-                elided_root: sha256::Midstate::from_byte_array(Decodable::consensus_decode(&mut d)?),
+                elided_root: ElidedRoot::from_byte_array(Decodable::consensus_decode(&mut d)?),
             }),
             2 => Ok(Params::Full(Decodable::consensus_decode(&mut d)?)),
             _ => Err(encode::Error::ParseFailed(
@@ -640,7 +650,6 @@ impl Decodable for Params {
 mod tests {
     use std::fmt::{self, Write};
 
-    use crate::hashes::sha256;
     use crate::{BlockHash, TxMerkleNode};
 
     use super::*;
@@ -683,7 +692,7 @@ mod tests {
         let compact_entry = Params::Compact {
             signblockscript: signblockscript.clone(),
             signblock_witness_limit: signblock_wl,
-            elided_root: sha256::Midstate::from_byte_array([0; 32]),
+            elided_root: ElidedRoot::from_byte_array([0; 32]),
         };
         assert_eq!(
             format!("{:x}", compact_entry.calculate_root()),
@@ -751,7 +760,7 @@ mod tests {
         let compact = params.into_compact().unwrap();
         assert_eq!(
             to_debug_string(&compact),
-            "Compact { signblockscript: 0102, signblock_witness_limit: 3, elided_root: 0xc3058c822b22a13bb7c47cf50d3f3c7817e7d9075ff55a7d16c85b9673e7e553 }",
+            "Compact { signblockscript: 0102, signblock_witness_limit: 3, elided_root: c3058c822b22a13bb7c47cf50d3f3c7817e7d9075ff55a7d16c85b9673e7e553 }",
         );
         assert_eq!(compact.calculate_root(), full.calculate_root());
         assert_eq!(compact.elided_root(), Some(&extra_root));