From 7f73d5f7db40cc5cd9ebd2b58560b9bcb85f116d Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 18 Feb 2021 19:48:39 +0100 Subject: [PATCH 1/4] doc: correct SigHashType doc comment Super nit, but a hashtype is not specific to a transaction but a signature. Signed-off-by: Antoine Poinsot --- src/blockdata/transaction.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/blockdata/transaction.rs b/src/blockdata/transaction.rs index f5147a7873..27f61142d9 100644 --- a/src/blockdata/transaction.rs +++ b/src/blockdata/transaction.rs @@ -608,7 +608,7 @@ impl Decodable for Transaction { } } -/// Hashtype of a transaction, encoded in the last byte of a signature +/// 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 { From 466f161e0bb27a2d04a0b5321449b9916aa6afaa Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 18 Feb 2021 23:32:35 +0100 Subject: [PATCH 2/4] transaction: document why we mask sighash types with 0x9f Signed-off-by: Antoine Poinsot Co-Authored-by: sanket1729 --- src/blockdata/transaction.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/blockdata/transaction.rs b/src/blockdata/transaction.rs index 27f61142d9..8448c991c5 100644 --- a/src/blockdata/transaction.rs +++ b/src/blockdata/transaction.rs @@ -675,7 +675,12 @@ impl SigHashType { /// Reads a 4-byte uint32 as a sighash type pub fn from_u32(n: u32) -> SigHashType { - match n & 0x9f { + // 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, From bf98d9fd60ef198b89678c3a6fe352c369f5e611 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 18 Feb 2021 22:05:34 +0100 Subject: [PATCH 3/4] transaction: add a method to err on non-standard types to SigHashType Right now, any sighash type could be parsed without error, which matches consensus rules. However most of them would be invalid by standardness, so it's a bit footgun-y (even more so for pre-signed transactions protocols for which standardness is critical). This adds `from_u32_standard()`, which takes care to error if we are passed an invalid-by-current-policy-rules SIGHASH type. Signed-off-by: Antoine Poinsot --- src/blockdata/transaction.rs | 44 ++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/src/blockdata/transaction.rs b/src/blockdata/transaction.rs index 8448c991c5..2faa3d7cfd 100644 --- a/src/blockdata/transaction.rs +++ b/src/blockdata/transaction.rs @@ -608,6 +608,19 @@ impl Decodable for Transaction { } } +/// 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; + +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)] @@ -673,7 +686,10 @@ impl SigHashType { } } - /// Reads a 4-byte uint32 as a sighash type + /// 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]. pub fn from_u32(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. @@ -694,6 +710,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 { + 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) + } + } + /// Converts to a u32 pub fn as_u32(self) -> u32 { self as u32 } } @@ -706,7 +737,7 @@ impl From 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; @@ -999,6 +1030,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 From e36f3a38e44a43bf5de5ced11c810f1ec9ac8211 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Fri, 19 Feb 2021 11:22:26 +0100 Subject: [PATCH 4/4] transaction: deprecate SigHashType::from_u32 in favor of from_u32_consensus Signed-off-by: Antoine Poinsot --- src/blockdata/transaction.rs | 8 +++++++- src/util/bip143.rs | 2 +- src/util/psbt/serialize.rs | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/blockdata/transaction.rs b/src/blockdata/transaction.rs index 2faa3d7cfd..acf509874a 100644 --- a/src/blockdata/transaction.rs +++ b/src/blockdata/transaction.rs @@ -686,11 +686,17 @@ impl SigHashType { } } + /// 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 { + 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]. - pub fn from_u32(n: u32) -> SigHashType { + 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. diff --git a/src/util/bip143.rs b/src/util/bip143.rs index de49bfbe09..d12477db75 100644 --- a/src/util/bip143.rs +++ b/src/util/bip143.rs @@ -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); } diff --git a/src/util/psbt/serialize.rs b/src/util/psbt/serialize.rs index 7344c73f5a..5e530a4596 100644 --- a/src/util/psbt/serialize.rs +++ b/src/util/psbt/serialize.rs @@ -132,7 +132,7 @@ impl Serialize for SigHashType { impl Deserialize for SigHashType { fn deserialize(bytes: &[u8]) -> Result { 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)