milksad
/
rust-bitcoin-unsafe-fast
14
0
Fork 0
Code Issues Pull Requests Activity

Improve Signature field names 

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`.

Includes a function argument rename as well, just to be uniform.

Fix: #2139
This commit is contained in:
Tobin C. Harding 2024-01-15 09:47:35 +11:00
parent 52b239ef70
commit 271b45299f
No known key found for this signature in database
GPG Key ID: 40BF9E4C269D6607
11 changed files with 78 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
bitcoin/examples/sighash.rs
View File

@ -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);
}
}

4
bitcoin/examples/sign-tx-segwit-v0.rs
View File

@ -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);

4
bitcoin/examples/sign-tx-taproot.rs
View File

@ -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.

24
bitcoin/examples/taproot-psbt.rs
View File

@ -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);

4
bitcoin/src/blockdata/witness.rs
View File

@ -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")

39
bitcoin/src/crypto/ecdsa.rs
View File

@ -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
pub sig: secp256k1::ecdsa::Signature,
/// The corresponding hash type
pub hash_ty: EcdsaSighashType,
/// The underlying ECDSA Signature.
pub signature: secp256k1::ecdsa::Signature,
/// The corresponding hash type.
pub sighash_type: EcdsaSighashType,
}
impl Signature {
/// Constructs an ECDSA Bitcoin signature for [`EcdsaSighashType::All`].
pub fn sighash_all(sig: secp256k1::ecdsa::Signature) -> Signature {
Signature { sig, hash_ty: EcdsaSighashType::All }
pub fn sighash_all(signature: secp256k1::ecdsa::Signature) -> Signature {
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 hash_ty = EcdsaSighashType::from_standard(*hash_ty as u32)?;
let sig = secp256k1::ecdsa::Signature::from_der(sig).map_err(Error::Secp256k1)?;
Ok(Signature { sig, hash_ty })
let (sighash_type, sig) = sl.split_last().ok_or(Error::EmptySignature)?;
let sighash_type = EcdsaSighashType::from_standard(*sighash_type as u32)?;
let signature = secp256k1::ecdsa::Signature::from_der(sig).map_err(Error::Secp256k1)?;
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.hash_ty as u8].as_hex(), f)
fmt::LowerHex::fmt(&self.signature.serialize_der().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)?,
hash_ty: EcdsaSighashType::from_standard(*sighash_byte as u32)?,
signature: secp256k1::ecdsa::Signature::from_der(signature)?,
sighash_type: EcdsaSighashType::from_standard(*sighash_byte as u32)?,
})
}
}

4
bitcoin/src/crypto/key.rs
View File

@ -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)?)
}
}

4
bitcoin/src/crypto/sighash.rs
View File

@ -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,

32
bitcoin/src/crypto/taproot.rs
View File

@ -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
pub sig: secp256k1::schnorr::Signature,
/// The corresponding hash type
pub hash_ty: TapSighashType,
/// The underlying schnorr signature.
pub signature: secp256k1::schnorr::Signature,
/// The corresponding hash type.
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)?;
Ok(Signature { sig, hash_ty: TapSighashType::Default })
let signature = secp256k1::schnorr::Signature::from_slice(sl)?;
Ok(Signature { signature, sighash_type: TapSighashType::Default })
}
65 => {
let (hash_ty, sig) = sl.split_last().expect("Slice len checked == 65");
let hash_ty = TapSighashType::from_consensus_u8(*hash_ty)?;
let sig = secp256k1::schnorr::Signature::from_slice(sig)?;
Ok(Signature { sig, hash_ty })
let (sighash_type, signature) = sl.split_last().expect("Slice len checked == 65");
let sighash_type = TapSighashType::from_consensus_u8(*sighash_type)?;
let signature = secp256k1::schnorr::Signature::from_slice(signature)?;
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();
if self.hash_ty == TapSighashType::Default {
let mut ser_sig = self.signature.as_ref().to_vec();
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)

6
bitcoin/src/psbt/mod.rs
View File

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

8
bitcoin/tests/serde.rs
View File

@ -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(),
hash_ty: EcdsaSighashType::All,
signature: secp256k1::ecdsa::Signature::from_str(s.trim()).unwrap(),
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(),
hash_ty: TapSighashType::All,
signature: secp256k1::schnorr::Signature::from_str(s.trim()).unwrap(),
sighash_type: TapSighashType::All,
};
let got = serialize(&sig).unwrap();