From 7c6854fe026b18fdee05706d3a36ead7a7796926 Mon Sep 17 00:00:00 2001 From: Martin Habovstiak Date: Mon, 6 Feb 2023 11:40:02 +0100 Subject: [PATCH] Use `Borrow` instead of `Deref` in `SighashCache` The requirement for a type dereferencing to `Transaction` prevented storing the cache in long-lived data without resorting to silly wrappers. Since `Borrow` is implemented both for `T` and for smart pointers it's a more flexible bound which this change implements. While this is technically breaking, all usual non-generic code will continue to work beause smart pointers generally have `Borrow` implemented. --- bitcoin/src/psbt/mod.rs | 5 ++-- bitcoin/src/sighash.rs | 52 ++++++++++++++++++++--------------------- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs index ebca2936..a760e994 100644 --- a/bitcoin/src/psbt/mod.rs +++ b/bitcoin/src/psbt/mod.rs @@ -11,7 +11,6 @@ use std::collections::{HashMap, HashSet}; use core::{fmt, cmp}; -use core::ops::Deref; use secp256k1::{Message, Secp256k1, Signing}; use bitcoin_internals::write_err; @@ -266,7 +265,7 @@ impl PartiallySignedTransaction { ) -> Result, SignError> where C: Signing, - T: Deref, + T: Borrow, K: GetKey, { let msg_sighash_ty_res = self.sighash_ecdsa(input_index, cache); @@ -309,7 +308,7 @@ impl PartiallySignedTransaction { /// Uses the [`EcdsaSighashType`] from this input if one is specified. If no sighash type is /// specified uses [`EcdsaSighashType::All`]. This function does not support scripts that /// contain `OP_CODESEPARATOR`. - pub fn sighash_ecdsa>( + pub fn sighash_ecdsa>( &self, input_index: usize, cache: &mut SighashCache, diff --git a/bitcoin/src/sighash.rs b/bitcoin/src/sighash.rs index 39e4a1a3..b1e842cb 100644 --- a/bitcoin/src/sighash.rs +++ b/bitcoin/src/sighash.rs @@ -8,8 +8,7 @@ //! and legacy (before Bip143). //! -use core::borrow::Borrow; -use core::ops::{Deref, DerefMut}; +use core::borrow::{Borrow, BorrowMut}; use core::{fmt, str}; use crate::{io, Script, ScriptBuf, Transaction, TxIn, TxOut, Sequence, Sighash}; @@ -32,9 +31,9 @@ pub(crate) const UINT256_ONE: [u8; 32] = [ /// Efficiently calculates signature hash message for legacy, segwit and taproot inputs. #[derive(Debug)] -pub struct SighashCache> { +pub struct SighashCache> { /// Access to transaction required for transaction introspection. Moreover, type - /// `T: Deref` allows us to use borrowed and mutable borrowed types, + /// `T: Borrow` allows us to use borrowed and mutable borrowed types, /// the latter in particular is necessary for [`SighashCache::witness_mut`]. tx: T, @@ -490,7 +489,7 @@ impl fmt::Display for SighashTypeParseError { impl_std_error!(SighashTypeParseError); -impl> SighashCache { +impl> SighashCache { /// Constructs a new `SighashCache` from an unsigned transaction. /// /// The sighash components are computed in a lazy manner when required. For the generated @@ -511,7 +510,7 @@ impl> SighashCache { leaf_hash_code_separator: Option<(TapLeafHash, u32)>, sighash_type: SchnorrSighashType, ) -> Result<(), Error> { - prevouts.check_all(&self.tx)?; + prevouts.check_all(self.tx.borrow())?; let (sighash, anyone_can_pay) = sighash_type.split_anyonecanpay_flag(); @@ -524,10 +523,10 @@ impl> SighashCache { // * Transaction Data: // nVersion (4): the nVersion of the transaction. - self.tx.version.consensus_encode(&mut writer)?; + self.tx.borrow().version.consensus_encode(&mut writer)?; // nLockTime (4): the nLockTime of the transaction. - self.tx.lock_time.consensus_encode(&mut writer)?; + self.tx.borrow().lock_time.consensus_encode(&mut writer)?; // If the hash_type & 0x80 does not equal SIGHASH_ANYONECANPAY: // sha_prevouts (32): the SHA256 of the serialization of all input outpoints. @@ -565,9 +564,9 @@ impl> SighashCache { // scriptPubKey (35): scriptPubKey of the previous output spent by this input, serialized as script inside CTxOut. Its size is always 35 bytes. // nSequence (4): nSequence of this input. if anyone_can_pay { - let txin = &self.tx.input.get(input_index).ok_or(Error::IndexOutOfInputsBounds { + let txin = &self.tx.borrow().input.get(input_index).ok_or(Error::IndexOutOfInputsBounds { index: input_index, - inputs_size: self.tx.input.len(), + inputs_size: self.tx.borrow().input.len(), })?; let previous_output = prevouts.get(input_index)?; txin.previous_output.consensus_encode(&mut writer)?; @@ -594,11 +593,12 @@ impl> SighashCache { if sighash == SchnorrSighashType::Single { let mut enc = sha256::Hash::engine(); self.tx + .borrow() .output .get(input_index) .ok_or(Error::SingleWithoutCorrespondingOutput { index: input_index, - outputs_size: self.tx.output.len(), + outputs_size: self.tx.borrow().output.len(), })? .consensus_encode(&mut enc)?; let hash = sha256::Hash::from_engine(enc); @@ -695,7 +695,7 @@ impl> SighashCache { let (sighash, anyone_can_pay) = sighash_type.split_anyonecanpay_flag(); - self.tx.version.consensus_encode(&mut writer)?; + self.tx.borrow().version.consensus_encode(&mut writer)?; if !anyone_can_pay { self.segwit_cache().prevouts.consensus_encode(&mut writer)?; @@ -713,9 +713,9 @@ impl> SighashCache { } { - let txin = &self.tx.input.get(input_index).ok_or(Error::IndexOutOfInputsBounds { + let txin = &self.tx.borrow().input.get(input_index).ok_or(Error::IndexOutOfInputsBounds { index: input_index, - inputs_size: self.tx.input.len(), + inputs_size: self.tx.borrow().input.len(), })?; txin.previous_output.consensus_encode(&mut writer)?; @@ -726,16 +726,16 @@ impl> SighashCache { if sighash != EcdsaSighashType::Single && sighash != EcdsaSighashType::None { self.segwit_cache().outputs.consensus_encode(&mut writer)?; - } else if sighash == EcdsaSighashType::Single && input_index < self.tx.output.len() { + } else if sighash == EcdsaSighashType::Single && input_index < self.tx.borrow().output.len() { let mut single_enc = Sighash::engine(); - self.tx.output[input_index].consensus_encode(&mut single_enc)?; + self.tx.borrow().output[input_index].consensus_encode(&mut single_enc)?; let hash = Sighash::from_engine(single_enc); writer.write_all(&hash[..])?; } else { writer.write_all(&zero_hash[..])?; } - self.tx.lock_time.consensus_encode(&mut writer)?; + self.tx.borrow().lock_time.consensus_encode(&mut writer)?; sighash_type.to_u32().consensus_encode(&mut writer)?; Ok(()) } @@ -788,15 +788,15 @@ impl> SighashCache { script_pubkey: &Script, sighash_type: U, ) -> EncodeSigningDataResult { - if input_index >= self.tx.input.len() { + if input_index >= self.tx.borrow().input.len() { return EncodeSigningDataResult::WriteResult(Err(Error::IndexOutOfInputsBounds { index: input_index, - inputs_size: self.tx.input.len(), + inputs_size: self.tx.borrow().input.len(), })); } let sighash_type: u32 = sighash_type.into(); - if is_invalid_use_of_sighash_single(sighash_type, input_index, self.tx.output.len()) { + if is_invalid_use_of_sighash_single(sighash_type, input_index, self.tx.borrow().output.len()) { // We cannot correctly handle the SIGHASH_SINGLE bug here because usage of this function // will result in the data written to the writer being hashed, however the correct // handling of the SIGHASH_SINGLE bug is to return the 'one array' - either implement @@ -882,7 +882,7 @@ impl> SighashCache { EncodeSigningDataResult::WriteResult( encode_signing_data_to_inner( - &self.tx, + self.tx.borrow(), writer, input_index, script_pubkey, @@ -930,12 +930,12 @@ impl> SighashCache { #[inline] fn common_cache(&mut self) -> &CommonCache { - Self::common_cache_minimal_borrow(&mut self.common_cache, &self.tx) + Self::common_cache_minimal_borrow(&mut self.common_cache, self.tx.borrow()) } fn common_cache_minimal_borrow<'a>( common_cache: &'a mut Option, - tx: &R, + tx: &Transaction, ) -> &'a CommonCache { common_cache.get_or_insert_with(|| { let mut enc_prevouts = sha256::Hash::engine(); @@ -960,7 +960,7 @@ impl> SighashCache { fn segwit_cache(&mut self) -> &SegwitCache { let common_cache = &mut self.common_cache; - let tx = &self.tx; + let tx = self.tx.borrow(); self.segwit_cache.get_or_insert_with(|| { let common_cache = Self::common_cache_minimal_borrow(common_cache, tx); SegwitCache { @@ -987,7 +987,7 @@ impl> SighashCache { } } -impl> SighashCache { +impl> SighashCache { /// When the `SighashCache` is initialized with a mutable reference to a transaction instead of /// a regular reference, this method is available to allow modification to the witnesses. /// @@ -1008,7 +1008,7 @@ impl> SighashCache { /// } /// ``` pub fn witness_mut(&mut self, input_index: usize) -> Option<&mut Witness> { - self.tx.input.get_mut(input_index).map(|i| &mut i.witness) + self.tx.borrow_mut().input.get_mut(input_index).map(|i| &mut i.witness) } }