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.
This commit is contained in:
Martin Habovstiak 2023-02-06 11:40:02 +01:00
parent 732dd038ff
commit 7c6854fe02
2 changed files with 28 additions and 29 deletions

View File

@ -11,7 +11,6 @@
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
use core::{fmt, cmp}; use core::{fmt, cmp};
use core::ops::Deref;
use secp256k1::{Message, Secp256k1, Signing}; use secp256k1::{Message, Secp256k1, Signing};
use bitcoin_internals::write_err; use bitcoin_internals::write_err;
@ -266,7 +265,7 @@ impl PartiallySignedTransaction {
) -> Result<Vec<PublicKey>, SignError> ) -> Result<Vec<PublicKey>, SignError>
where where
C: Signing, C: Signing,
T: Deref<Target=Transaction>, T: Borrow<Transaction>,
K: GetKey, K: GetKey,
{ {
let msg_sighash_ty_res = self.sighash_ecdsa(input_index, cache); 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 /// 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 /// specified uses [`EcdsaSighashType::All`]. This function does not support scripts that
/// contain `OP_CODESEPARATOR`. /// contain `OP_CODESEPARATOR`.
pub fn sighash_ecdsa<T: Deref<Target=Transaction>>( pub fn sighash_ecdsa<T: Borrow<Transaction>>(
&self, &self,
input_index: usize, input_index: usize,
cache: &mut SighashCache<T>, cache: &mut SighashCache<T>,

View File

@ -8,8 +8,7 @@
//! and legacy (before Bip143). //! and legacy (before Bip143).
//! //!
use core::borrow::Borrow; use core::borrow::{Borrow, BorrowMut};
use core::ops::{Deref, DerefMut};
use core::{fmt, str}; use core::{fmt, str};
use crate::{io, Script, ScriptBuf, Transaction, TxIn, TxOut, Sequence, Sighash}; 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. /// Efficiently calculates signature hash message for legacy, segwit and taproot inputs.
#[derive(Debug)] #[derive(Debug)]
pub struct SighashCache<T: Deref<Target = Transaction>> { pub struct SighashCache<T: Borrow<Transaction>> {
/// Access to transaction required for transaction introspection. Moreover, type /// Access to transaction required for transaction introspection. Moreover, type
/// `T: Deref<Target=Transaction>` allows us to use borrowed and mutable borrowed types, /// `T: Borrow<Transaction>` allows us to use borrowed and mutable borrowed types,
/// the latter in particular is necessary for [`SighashCache::witness_mut`]. /// the latter in particular is necessary for [`SighashCache::witness_mut`].
tx: T, tx: T,
@ -490,7 +489,7 @@ impl fmt::Display for SighashTypeParseError {
impl_std_error!(SighashTypeParseError); impl_std_error!(SighashTypeParseError);
impl<R: Deref<Target = Transaction>> SighashCache<R> { impl<R: Borrow<Transaction>> SighashCache<R> {
/// Constructs a new `SighashCache` from an unsigned transaction. /// Constructs a new `SighashCache` from an unsigned transaction.
/// ///
/// The sighash components are computed in a lazy manner when required. For the generated /// The sighash components are computed in a lazy manner when required. For the generated
@ -511,7 +510,7 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
leaf_hash_code_separator: Option<(TapLeafHash, u32)>, leaf_hash_code_separator: Option<(TapLeafHash, u32)>,
sighash_type: SchnorrSighashType, sighash_type: SchnorrSighashType,
) -> Result<(), Error> { ) -> Result<(), Error> {
prevouts.check_all(&self.tx)?; prevouts.check_all(self.tx.borrow())?;
let (sighash, anyone_can_pay) = sighash_type.split_anyonecanpay_flag(); let (sighash, anyone_can_pay) = sighash_type.split_anyonecanpay_flag();
@ -524,10 +523,10 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
// * Transaction Data: // * Transaction Data:
// nVersion (4): the nVersion of the transaction. // 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. // 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: // If the hash_type & 0x80 does not equal SIGHASH_ANYONECANPAY:
// sha_prevouts (32): the SHA256 of the serialization of all input outpoints. // sha_prevouts (32): the SHA256 of the serialization of all input outpoints.
@ -565,9 +564,9 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
// scriptPubKey (35): scriptPubKey of the previous output spent by this input, serialized as script inside CTxOut. Its size is always 35 bytes. // 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. // nSequence (4): nSequence of this input.
if anyone_can_pay { 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, index: input_index,
inputs_size: self.tx.input.len(), inputs_size: self.tx.borrow().input.len(),
})?; })?;
let previous_output = prevouts.get(input_index)?; let previous_output = prevouts.get(input_index)?;
txin.previous_output.consensus_encode(&mut writer)?; txin.previous_output.consensus_encode(&mut writer)?;
@ -594,11 +593,12 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
if sighash == SchnorrSighashType::Single { if sighash == SchnorrSighashType::Single {
let mut enc = sha256::Hash::engine(); let mut enc = sha256::Hash::engine();
self.tx self.tx
.borrow()
.output .output
.get(input_index) .get(input_index)
.ok_or(Error::SingleWithoutCorrespondingOutput { .ok_or(Error::SingleWithoutCorrespondingOutput {
index: input_index, index: input_index,
outputs_size: self.tx.output.len(), outputs_size: self.tx.borrow().output.len(),
})? })?
.consensus_encode(&mut enc)?; .consensus_encode(&mut enc)?;
let hash = sha256::Hash::from_engine(enc); let hash = sha256::Hash::from_engine(enc);
@ -695,7 +695,7 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
let (sighash, anyone_can_pay) = sighash_type.split_anyonecanpay_flag(); 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 { if !anyone_can_pay {
self.segwit_cache().prevouts.consensus_encode(&mut writer)?; self.segwit_cache().prevouts.consensus_encode(&mut writer)?;
@ -713,9 +713,9 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
} }
{ {
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, index: input_index,
inputs_size: self.tx.input.len(), inputs_size: self.tx.borrow().input.len(),
})?; })?;
txin.previous_output.consensus_encode(&mut writer)?; txin.previous_output.consensus_encode(&mut writer)?;
@ -726,16 +726,16 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
if sighash != EcdsaSighashType::Single && sighash != EcdsaSighashType::None { if sighash != EcdsaSighashType::Single && sighash != EcdsaSighashType::None {
self.segwit_cache().outputs.consensus_encode(&mut writer)?; 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(); 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); let hash = Sighash::from_engine(single_enc);
writer.write_all(&hash[..])?; writer.write_all(&hash[..])?;
} else { } else {
writer.write_all(&zero_hash[..])?; 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)?; sighash_type.to_u32().consensus_encode(&mut writer)?;
Ok(()) Ok(())
} }
@ -788,15 +788,15 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
script_pubkey: &Script, script_pubkey: &Script,
sighash_type: U, sighash_type: U,
) -> EncodeSigningDataResult<Error> { ) -> EncodeSigningDataResult<Error> {
if input_index >= self.tx.input.len() { if input_index >= self.tx.borrow().input.len() {
return EncodeSigningDataResult::WriteResult(Err(Error::IndexOutOfInputsBounds { return EncodeSigningDataResult::WriteResult(Err(Error::IndexOutOfInputsBounds {
index: input_index, index: input_index,
inputs_size: self.tx.input.len(), inputs_size: self.tx.borrow().input.len(),
})); }));
} }
let sighash_type: u32 = sighash_type.into(); 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 // 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 // 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 // handling of the SIGHASH_SINGLE bug is to return the 'one array' - either implement
@ -882,7 +882,7 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
EncodeSigningDataResult::WriteResult( EncodeSigningDataResult::WriteResult(
encode_signing_data_to_inner( encode_signing_data_to_inner(
&self.tx, self.tx.borrow(),
writer, writer,
input_index, input_index,
script_pubkey, script_pubkey,
@ -930,12 +930,12 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
#[inline] #[inline]
fn common_cache(&mut self) -> &CommonCache { 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>( fn common_cache_minimal_borrow<'a>(
common_cache: &'a mut Option<CommonCache>, common_cache: &'a mut Option<CommonCache>,
tx: &R, tx: &Transaction,
) -> &'a CommonCache { ) -> &'a CommonCache {
common_cache.get_or_insert_with(|| { common_cache.get_or_insert_with(|| {
let mut enc_prevouts = sha256::Hash::engine(); let mut enc_prevouts = sha256::Hash::engine();
@ -960,7 +960,7 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
fn segwit_cache(&mut self) -> &SegwitCache { fn segwit_cache(&mut self) -> &SegwitCache {
let common_cache = &mut self.common_cache; let common_cache = &mut self.common_cache;
let tx = &self.tx; let tx = self.tx.borrow();
self.segwit_cache.get_or_insert_with(|| { self.segwit_cache.get_or_insert_with(|| {
let common_cache = Self::common_cache_minimal_borrow(common_cache, tx); let common_cache = Self::common_cache_minimal_borrow(common_cache, tx);
SegwitCache { SegwitCache {
@ -987,7 +987,7 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
} }
} }
impl<R: DerefMut<Target = Transaction>> SighashCache<R> { impl<R: BorrowMut<Transaction>> SighashCache<R> {
/// When the `SighashCache` is initialized with a mutable reference to a transaction instead of /// 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. /// a regular reference, this method is available to allow modification to the witnesses.
/// ///
@ -1008,7 +1008,7 @@ impl<R: DerefMut<Target = Transaction>> SighashCache<R> {
/// } /// }
/// ``` /// ```
pub fn witness_mut(&mut self, input_index: usize) -> Option<&mut Witness> { 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)
} }
} }