Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SigHashType: add a method to error on non-standard hashtypes #573

Merged
merged 4 commits into from Feb 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 55 additions & 4 deletions src/blockdata/transaction.rs
Expand Up @@ -608,7 +608,20 @@ impl Decodable for Transaction {
}
}

/// Hashtype of a transaction, encoded in the last byte of a signature
/// This type is consensus valid but an input including it would prevent the transaction from
/// being relayed on today's Bitcoin network.
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct NonStandardSigHashType;
darosior marked this conversation as resolved.
Show resolved Hide resolved

impl fmt::Display for NonStandardSigHashType {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "Non standard sighash type")
}
}

impl error::Error for NonStandardSigHashType {}

/// Hashtype of an input's signature, encoded in the last byte of the signature
/// Fixed values so they can be casted as integer types for encoding
#[derive(PartialEq, Eq, Debug, Copy, Clone)]
pub enum SigHashType {
Expand Down Expand Up @@ -673,9 +686,23 @@ impl SigHashType {
}
}

/// Reads a 4-byte uint32 as a sighash type
/// Reads a 4-byte uint32 as a sighash type.
#[deprecated(since="0.26.1", note="please use `from_u32_consensus` or `from_u32_standard` instead")]
pub fn from_u32(n: u32) -> SigHashType {
match n & 0x9f {
Self::from_u32_consensus(n)
}

/// Reads a 4-byte uint32 as a sighash type.
///
/// **Note**: this replicates consensus behaviour, for current standardness rules correctness
/// you probably want [from_u32_standard].
sgeisler marked this conversation as resolved.
Show resolved Hide resolved
pub fn from_u32_consensus(n: u32) -> SigHashType {
// In Bitcoin Core, the SignatureHash function will mask the (int32) value with
// 0x1f to (apparently) deactivate ACP when checking for SINGLE and NONE bits.
// We however want to be matching also against on ACP-masked ALL, SINGLE, and NONE.
// So here we re-activate ACP.
let mask = 0x1f | 0x80;
match n & mask {
// "real" sighashes
0x01 => SigHashType::All,
0x02 => SigHashType::None,
Expand All @@ -689,6 +716,21 @@ impl SigHashType {
}
}

/// Read a 4-byte uint32 as a standard sighash type, returning an error if the type
/// is non standard.
pub fn from_u32_standard(n: u32) -> Result<SigHashType, NonStandardSigHashType> {
match n {
// Standard sighashes, see https://github.com/bitcoin/bitcoin/blob/b805dbb0b9c90dadef0424e5b3bf86ac308e103e/src/script/interpreter.cpp#L189-L198
0x01 => Ok(SigHashType::All),
0x02 => Ok(SigHashType::None),
0x03 => Ok(SigHashType::Single),
0x81 => Ok(SigHashType::AllPlusAnyoneCanPay),
0x82 => Ok(SigHashType::NonePlusAnyoneCanPay),
0x83 => Ok(SigHashType::SinglePlusAnyoneCanPay),
_ => Err(NonStandardSigHashType)
Copy link
Contributor

@sgeisler sgeisler Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g. in the PSBT API we'd include the input that was considered invalid in the error, but I don't think we are consistent about it. It's probably the right thing to do for new code though. I'm sorry I only see this now.

}
}

/// Converts to a u32
pub fn as_u32(self) -> u32 { self as u32 }
}
Expand All @@ -701,7 +743,7 @@ impl From<SigHashType> for u32 {

#[cfg(test)]
mod tests {
use super::{OutPoint, ParseOutPointError, Transaction, TxIn};
use super::{OutPoint, ParseOutPointError, Transaction, TxIn, NonStandardSigHashType};

use std::str::FromStr;
use blockdata::constants::WITNESS_SCALE_FACTOR;
Expand Down Expand Up @@ -994,6 +1036,15 @@ mod tests {
}
}

#[test]
fn test_sighashtype_standard() {
let nonstandard_hashtype = 0x04;
// This type is not well defined, by consensus it becomes ALL
assert_eq!(SigHashType::from_u32(nonstandard_hashtype), SigHashType::All);
// But it's policy-invalid to use it!
assert_eq!(SigHashType::from_u32_standard(nonstandard_hashtype), Err(NonStandardSigHashType));
}

// These test vectors were stolen from libbtc, which is Copyright 2014 Jonas Schnelli MIT
// They were transformed by replacing {...} with run_test_sighash(...), then the ones containing
// OP_CODESEPARATOR in their pubkeys were removed
Expand Down
2 changes: 1 addition & 1 deletion src/util/bip143.rs
Expand Up @@ -292,7 +292,7 @@ mod tests {
let raw_expected = SigHash::from_hex(expected_result).unwrap();
let expected_result = SigHash::from_slice(&raw_expected[..]).unwrap();
let mut cache = SigHashCache::new(&tx);
let sighash_type = SigHashType::from_u32(hash_type);
let sighash_type = SigHashType::from_u32_consensus(hash_type);
let actual_result = cache.signature_hash(input_index, &script, value, sighash_type);
assert_eq!(actual_result, expected_result);
}
Expand Down
2 changes: 1 addition & 1 deletion src/util/psbt/serialize.rs
Expand Up @@ -132,7 +132,7 @@ impl Serialize for SigHashType {
impl Deserialize for SigHashType {
fn deserialize(bytes: &[u8]) -> Result<Self, encode::Error> {
let raw: u32 = encode::deserialize(bytes)?;
let rv: SigHashType = SigHashType::from_u32(raw);
let rv: SigHashType = SigHashType::from_u32_consensus(raw);

if rv.as_u32() == raw {
Ok(rv)
Expand Down