Merge rust-bitcoin/rust-bitcoin#2338: Improve `Signature` field names

271b45299f Improve Signature field names (Tobin C. Harding)

Pull request description:

  Applies to both `ecdsa::Signature` and `taproot::Signature`.

  Re-name the `Signature` fields with more descriptive names. The names used were decided upon in the issue discussion.

  Impove rustdocs while we are at it.

  Note, the change to `sign-tx-segwit-v0` is refactor only, the diff does not show it but we have a local variable already called `sighash_type` that is equal to `EcdsaSighashType::All`.

  Fix: #2139

ACKs for top commit:
  Kixunil:
    ACK 271b45299f
  apoelstra:
    ACK 271b45299f

Tree-SHA512: b27221e922206b2510b073c0f38678f97f0c946707e3e679eee181faa170348101198706f9ca5803a55c799b0b330cc263cc103cd9beefff4e5c58d8fea77b8d

bitcoin/examples/sighash.rs

@@ -44,7 +44,7 @@ fn compute_sighash_p2wpkh(raw_tx: &[u8], inp_idx: usize, value: u64) {
 
     let mut cache = sighash::SighashCache::new(&tx);
     let sighash = cache
-        .p2wpkh_signature_hash(inp_idx, &spk, Amount::from_sat(value), sig.hash_ty)
+        .p2wpkh_signature_hash(inp_idx, &spk, Amount::from_sat(value), sig.sighash_type)
         .expect("failed to compute sighash");
     println!("Segwit p2wpkh sighash: {:x}", sighash);
     let msg = secp256k1::Message::from_digest(sighash.to_byte_array());
@@ -93,9 +93,9 @@ fn compute_sighash_legacy(raw_tx: &[u8], inp_idx: usize, script_pubkey_bytes_opt
         let sig = ecdsa::Signature::from_slice(instr.unwrap().push_bytes().unwrap().as_bytes())
             .expect("failed to parse sig");
         let sighash = cache
-            .legacy_signature_hash(inp_idx, script_code, sig.hash_ty.to_u32())
+            .legacy_signature_hash(inp_idx, script_code, sig.sighash_type.to_u32())
             .expect("failed to compute sighash");
-        println!("Legacy sighash: {:x} (sighash flag {})", sighash, sig.hash_ty);
+        println!("Legacy sighash: {:x} (sighash flag {})", sighash, sig.sighash_type);
     }
 }
 
@@ -126,9 +126,14 @@ fn compute_sighash_p2wsh(raw_tx: &[u8], inp_idx: usize, value: u64) {
         assert!((70..=72).contains(&sig_len), "signature length {} out of bounds", sig_len);
         //here we assume that all sighash_flags are the same. Can they be different?
         let sighash = cache
-            .p2wsh_signature_hash(inp_idx, witness_script, Amount::from_sat(value), sig.hash_ty)
+            .p2wsh_signature_hash(
+                inp_idx,
+                witness_script,
+                Amount::from_sat(value),
+                sig.sighash_type,
+            )
             .expect("failed to compute sighash");
-        println!("Segwit p2wsh sighash: {:x} ({})", sighash, sig.hash_ty);
+        println!("Segwit p2wsh sighash: {:x} ({})", sighash, sig.sighash_type);
     }
 }
 

bitcoin/examples/sign-tx-segwit-v0.rs

@@ -66,10 +66,10 @@ fn main() {
 
     // Sign the sighash using the secp256k1 library (exported by rust-bitcoin).
     let msg = Message::from(sighash);
-    let sig = secp.sign_ecdsa(&msg, &sk);
+    let signature = secp.sign_ecdsa(&msg, &sk);
 
     // Update the witness stack.
-    let signature = bitcoin::ecdsa::Signature { sig, hash_ty: EcdsaSighashType::All };
+    let signature = bitcoin::ecdsa::Signature { signature, sighash_type };
     let pk = sk.public_key(&secp);
     *sighasher.witness_mut(input_index).unwrap() = Witness::p2wpkh(&signature, &pk);
 

bitcoin/examples/sign-tx-taproot.rs

@@ -72,10 +72,10 @@ fn main() {
     // Sign the sighash using the secp256k1 library (exported by rust-bitcoin).
     let tweaked: TweakedKeypair = keypair.tap_tweak(&secp, None);
     let msg = Message::from_digest(sighash.to_byte_array());
-    let sig = secp.sign_schnorr(&msg, &tweaked.to_inner());
+    let signature = secp.sign_schnorr(&msg, &tweaked.to_inner());
 
     // Update the witness stack.
-    let signature = bitcoin::taproot::Signature { sig, hash_ty: sighash_type };
+    let signature = bitcoin::taproot::Signature { signature, sighash_type };
     sighasher.witness_mut(input_index).unwrap().push(&signature.to_vec());
 
     // Get the signed transaction.

bitcoin/examples/taproot-psbt.rs

@@ -282,14 +282,14 @@ fn generate_bip86_key_spend_tx(
     let unsigned_tx = psbt.unsigned_tx.clone();
     psbt.inputs.iter_mut().enumerate().try_for_each::<_, Result<(), Box<dyn std::error::Error>>>(
         |(vout, input)| {
-            let hash_ty = input
+            let sighash_type = input
                 .sighash_type
                 .and_then(|psbt_sighash_type| psbt_sighash_type.taproot_hash_ty().ok())
                 .unwrap_or(TapSighashType::All);
             let hash = SighashCache::new(&unsigned_tx).taproot_key_spend_signature_hash(
                 vout,
                 &sighash::Prevouts::All(input_txouts.as_slice()),
-                hash_ty,
+                sighash_type,
             )?;
 
             let (_, (_, derivation_path)) = input
@@ -304,7 +304,7 @@ fn generate_bip86_key_spend_tx(
                 None,
                 input,
                 hash,
-                hash_ty,
+                sighash_type,
                 secp,
             );
 
@@ -512,7 +512,7 @@ impl BenefactorWallet {
             psbt.outputs = vec![Output::default()];
             psbt.unsigned_tx.lock_time = absolute::LockTime::ZERO;
 
-            let hash_ty = input
+            let sighash_type = input
                 .sighash_type
                 .and_then(|psbt_sighash_type| psbt_sighash_type.taproot_hash_ty().ok())
                 .unwrap_or(TapSighashType::All);
@@ -522,7 +522,7 @@ impl BenefactorWallet {
                     value: input_value,
                     script_pubkey: prevout_script_pubkey,
                 }]),
-                hash_ty,
+                sighash_type,
             )?;
 
             {
@@ -538,7 +538,7 @@ impl BenefactorWallet {
                     None,
                     input,
                     hash,
-                    hash_ty,
+                    sighash_type,
                     &self.secp,
                 );
             }
@@ -654,7 +654,7 @@ impl BeneficiaryWallet {
             let secret_key =
                 self.master_xpriv.derive_priv(&self.secp, &derivation_path)?.to_priv().inner;
             for lh in leaf_hashes {
-                let hash_ty = TapSighashType::All;
+                let sighash_type = TapSighashType::All;
                 let hash = SighashCache::new(&unsigned_tx).taproot_script_spend_signature_hash(
                     0,
                     &sighash::Prevouts::All(&[TxOut {
@@ -662,7 +662,7 @@ impl BeneficiaryWallet {
                         script_pubkey: input_script_pubkey.clone(),
                     }]),
                     *lh,
-                    hash_ty,
+                    sighash_type,
                 )?;
                 sign_psbt_taproot(
                     &secret_key,
@@ -670,7 +670,7 @@ impl BeneficiaryWallet {
                     Some(*lh),
                     &mut psbt.inputs[0],
                     hash,
-                    hash_ty,
+                    sighash_type,
                     &self.secp,
                 );
             }
@@ -730,7 +730,7 @@ fn sign_psbt_taproot(
     leaf_hash: Option<TapLeafHash>,
     psbt_input: &mut psbt::Input,
     hash: TapSighash,
-    hash_ty: TapSighashType,
+    sighash_type: TapSighashType,
     secp: &Secp256k1<secp256k1::All>,
 ) {
     let keypair = secp256k1::Keypair::from_seckey_slice(secp, secret_key.as_ref()).unwrap();
@@ -740,9 +740,9 @@ fn sign_psbt_taproot(
     };
 
     let msg = secp256k1::Message::from_digest(hash.to_byte_array());
-    let sig = secp.sign_schnorr(&msg, &keypair);
+    let signature = secp.sign_schnorr(&msg, &keypair);
 
-    let final_signature = taproot::Signature { sig, hash_ty };
+    let final_signature = taproot::Signature { signature, sighash_type };
 
     if let Some(lh) = leaf_hash {
         psbt_input.tap_script_sigs.insert((pubkey, lh), final_signature);

bitcoin/src/blockdata/witness.rs

@@ -637,9 +637,9 @@ mod test {
         // The very first signature in block 734,958
         let sig_bytes =
             hex!("304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b541");
-        let sig = secp256k1::ecdsa::Signature::from_der(&sig_bytes).unwrap();
+        let signature = secp256k1::ecdsa::Signature::from_der(&sig_bytes).unwrap();
         let mut witness = Witness::default();
-        let signature = crate::ecdsa::Signature { sig, hash_ty: EcdsaSighashType::All };
+        let signature = crate::ecdsa::Signature { signature, sighash_type: EcdsaSighashType::All };
         witness.push_ecdsa_signature(&signature);
         let expected_witness = vec![hex!(
             "304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b54101")

bitcoin/src/crypto/ecdsa.rs

@@ -22,24 +22,24 @@ const MAX_SIG_LEN: usize = 73;
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
 pub struct Signature {
-    /// The underlying ECDSA Signature
+    /// The underlying ECDSA Signature.
-    pub sig: secp256k1::ecdsa::Signature,
+    pub signature: secp256k1::ecdsa::Signature,
-    /// The corresponding hash type
+    /// The corresponding hash type.
-    pub hash_ty: EcdsaSighashType,
+    pub sighash_type: EcdsaSighashType,
 }
 
 impl Signature {
     /// Constructs an ECDSA Bitcoin signature for [`EcdsaSighashType::All`].
-    pub fn sighash_all(sig: secp256k1::ecdsa::Signature) -> Signature {
+    pub fn sighash_all(signature: secp256k1::ecdsa::Signature) -> Signature {
-        Signature { sig, hash_ty: EcdsaSighashType::All }
+        Signature { signature, sighash_type: EcdsaSighashType::All }
     }
 
     /// Deserializes from slice following the standardness rules for [`EcdsaSighashType`].
     pub fn from_slice(sl: &[u8]) -> Result<Self, Error> {
-        let (hash_ty, sig) = sl.split_last().ok_or(Error::EmptySignature)?;
+        let (sighash_type, sig) = sl.split_last().ok_or(Error::EmptySignature)?;
-        let hash_ty = EcdsaSighashType::from_standard(*hash_ty as u32)?;
+        let sighash_type = EcdsaSighashType::from_standard(*sighash_type as u32)?;
-        let sig = secp256k1::ecdsa::Signature::from_der(sig).map_err(Error::Secp256k1)?;
+        let signature = secp256k1::ecdsa::Signature::from_der(sig).map_err(Error::Secp256k1)?;
-        Ok(Signature { sig, hash_ty })
+        Ok(Signature { signature, sighash_type })
     }
 
     /// Serializes an ECDSA signature (inner secp256k1 signature in DER format).
@@ -47,9 +47,9 @@ impl Signature {
     /// This does **not** perform extra heap allocation.
     pub fn serialize(&self) -> SerializedSignature {
         let mut buf = [0u8; MAX_SIG_LEN];
-        let signature = self.sig.serialize_der();
+        let signature = self.signature.serialize_der();
         buf[..signature.len()].copy_from_slice(&signature);
-        buf[signature.len()] = self.hash_ty as u8;
+        buf[signature.len()] = self.sighash_type as u8;
         SerializedSignature { data: buf, len: signature.len() + 1 }
     }
 
@@ -59,14 +59,19 @@ impl Signature {
     /// [`serialize`](Self::serialize) method instead.
     pub fn to_vec(self) -> Vec<u8> {
         // TODO: add support to serialize to a writer to SerializedSig
-        self.sig.serialize_der().iter().copied().chain(iter::once(self.hash_ty as u8)).collect()
+        self.signature
+            .serialize_der()
+            .iter()
+            .copied()
+            .chain(iter::once(self.sighash_type as u8))
+            .collect()
     }
 }
 
 impl fmt::Display for Signature {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        fmt::LowerHex::fmt(&self.sig.serialize_der().as_hex(), f)?;
+        fmt::LowerHex::fmt(&self.signature.serialize_der().as_hex(), f)?;
-        fmt::LowerHex::fmt(&[self.hash_ty as u8].as_hex(), f)
+        fmt::LowerHex::fmt(&[self.sighash_type as u8].as_hex(), f)
     }
 }
 
@@ -77,8 +82,8 @@ impl FromStr for Signature {
         let bytes = Vec::from_hex(s)?;
         let (sighash_byte, signature) = bytes.split_last().ok_or(Error::EmptySignature)?;
         Ok(Signature {
-            sig: secp256k1::ecdsa::Signature::from_der(signature)?,
+            signature: secp256k1::ecdsa::Signature::from_der(signature)?,
-            hash_ty: EcdsaSighashType::from_standard(*sighash_byte as u32)?,
+            sighash_type: EcdsaSighashType::from_standard(*sighash_byte as u32)?,
         })
     }
 }

bitcoin/src/crypto/key.rs

@@ -202,7 +202,7 @@ impl PublicKey {
         msg: &secp256k1::Message,
         sig: &ecdsa::Signature,
     ) -> Result<(), Error> {
-        Ok(secp.verify_ecdsa(msg, &sig.sig, &self.inner)?)
+        Ok(secp.verify_ecdsa(msg, &sig.signature, &self.inner)?)
     }
 }
 
@@ -321,7 +321,7 @@ impl CompressedPublicKey {
         msg: &secp256k1::Message,
         sig: &ecdsa::Signature,
     ) -> Result<(), Error> {
-        Ok(secp.verify_ecdsa(msg, &sig.sig, &self.0)?)
+        Ok(secp.verify_ecdsa(msg, &sig.signature, &self.0)?)
     }
 }
 

bitcoin/src/crypto/sighash.rs

@@ -586,10 +586,10 @@ impl TapSighashType {
     }
 
     /// Constructs a [`TapSighashType`] from a raw `u8`.
-    pub fn from_consensus_u8(hash_ty: u8) -> Result<Self, InvalidSighashTypeError> {
+    pub fn from_consensus_u8(sighash_type: u8) -> Result<Self, InvalidSighashTypeError> {
         use TapSighashType::*;
 
-        Ok(match hash_ty {
+        Ok(match sighash_type {
             0x00 => Default,
             0x01 => All,
             0x02 => None,

bitcoin/src/crypto/taproot.rs

@@ -18,10 +18,10 @@ use crate::taproot::serialized_signature::{self, SerializedSignature};
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(crate = "actual_serde"))]
 pub struct Signature {
-    /// The underlying schnorr signature
+    /// The underlying schnorr signature.
-    pub sig: secp256k1::schnorr::Signature,
+    pub signature: secp256k1::schnorr::Signature,
-    /// The corresponding hash type
+    /// The corresponding hash type.
-    pub hash_ty: TapSighashType,
+    pub sighash_type: TapSighashType,
 }
 
 impl Signature {
@@ -30,14 +30,14 @@ impl Signature {
         match sl.len() {
             64 => {
                 // default type
-                let sig = secp256k1::schnorr::Signature::from_slice(sl)?;
+                let signature = secp256k1::schnorr::Signature::from_slice(sl)?;
-                Ok(Signature { sig, hash_ty: TapSighashType::Default })
+                Ok(Signature { signature, sighash_type: TapSighashType::Default })
             }
             65 => {
-                let (hash_ty, sig) = sl.split_last().expect("Slice len checked == 65");
+                let (sighash_type, signature) = sl.split_last().expect("Slice len checked == 65");
-                let hash_ty = TapSighashType::from_consensus_u8(*hash_ty)?;
+                let sighash_type = TapSighashType::from_consensus_u8(*sighash_type)?;
-                let sig = secp256k1::schnorr::Signature::from_slice(sig)?;
+                let signature = secp256k1::schnorr::Signature::from_slice(signature)?;
-                Ok(Signature { sig, hash_ty })
+                Ok(Signature { signature, sighash_type })
             }
             len => Err(SigFromSliceError::InvalidSignatureSize(len)),
         }
@@ -48,11 +48,11 @@ impl Signature {
     /// Note: this allocates on the heap, prefer [`serialize`](Self::serialize) if vec is not needed.
     pub fn to_vec(self) -> Vec<u8> {
         // TODO: add support to serialize to a writer to SerializedSig
-        let mut ser_sig = self.sig.as_ref().to_vec();
+        let mut ser_sig = self.signature.as_ref().to_vec();
-        if self.hash_ty == TapSighashType::Default {
+        if self.sighash_type == TapSighashType::Default {
             // default sighash type, don't add extra sighash byte
         } else {
-            ser_sig.push(self.hash_ty as u8);
+            ser_sig.push(self.sighash_type as u8);
         }
         ser_sig
     }
@@ -63,13 +63,13 @@ impl Signature {
     /// You can get a slice from it using deref coercions or turn it into an iterator.
     pub fn serialize(self) -> SerializedSignature {
         let mut buf = [0; serialized_signature::MAX_LEN];
-        let ser_sig = self.sig.serialize();
+        let ser_sig = self.signature.serialize();
         buf[..64].copy_from_slice(&ser_sig);
-        let len = if self.hash_ty == TapSighashType::Default {
+        let len = if self.sighash_type == TapSighashType::Default {
             // default sighash type, don't add extra sighash byte
             64
         } else {
-            buf[64] = self.hash_ty as u8;
+            buf[64] = self.sighash_type as u8;
             65
         };
         SerializedSignature::from_raw_parts(buf, len)

bitcoin/src/psbt/mod.rs

@@ -371,8 +371,10 @@ impl Psbt {
                 Ok((msg, sighash_ty)) => (msg, sighash_ty),
             };
 
-            let sig =
+            let sig = ecdsa::Signature {
-                ecdsa::Signature { sig: secp.sign_ecdsa(&msg, &sk.inner), hash_ty: sighash_ty };
+                signature: secp.sign_ecdsa(&msg, &sk.inner),
+                sighash_type: sighash_ty,
+            };
 
             let pk = sk.public_key(secp);
 

bitcoin/tests/serde.rs

@@ -174,8 +174,8 @@ fn serde_regression_extended_pub_key() {
 fn serde_regression_ecdsa_sig() {
     let s = include_str!("data/serde/ecdsa_sig_hex");
     let sig = ecdsa::Signature {
-        sig: secp256k1::ecdsa::Signature::from_str(s.trim()).unwrap(),
+        signature: secp256k1::ecdsa::Signature::from_str(s.trim()).unwrap(),
-        hash_ty: EcdsaSighashType::All,
+        sighash_type: EcdsaSighashType::All,
     };
 
     let got = serialize(&sig).unwrap();
@@ -348,8 +348,8 @@ fn serde_regression_proprietary_key() {
 fn serde_regression_taproot_sig() {
     let s = include_str!("data/serde/taproot_sig_hex");
     let sig = taproot::Signature {
-        sig: secp256k1::schnorr::Signature::from_str(s.trim()).unwrap(),
+        signature: secp256k1::schnorr::Signature::from_str(s.trim()).unwrap(),
-        hash_ty: TapSighashType::All,
+        sighash_type: TapSighashType::All,
     };
 
     let got = serialize(&sig).unwrap();