Merge rust-bitcoin/rust-bitcoin#1625: Improve `SighashCache` API

b3188bbac3 Add `Transaction` accessors to `SighashCache` (Martin Habovstiak)
7c6854fe02 Use `Borrow` instead of `Deref` in `SighashCache` (Martin Habovstiak)

Pull request description:

  This changes the bound from `Deref<Target = Transaction>` to `Borrow<Transaction>` (with respective `mut` changes) and adds accessors.

  Closes #1423 (PSBT stuff will be separate issue).

ACKs for top commit:
  tcharding:
    ACK b3188bbac3
  apoelstra:
    ACK b3188bbac3

Tree-SHA512: 9db2c5890b26e9eefd483d697b42e84b1d7d3b8676fc39b4f39075c149e12697aa538828a757f9187578a958d72a592bb913f8f5788c93feb273db5370979d99
This commit is contained in:
Andrew Poelstra 2023-02-07 00:54:02 +00:00
commit 0aba8e56f6
No known key found for this signature in database
GPG Key ID: C588D63CE41B97C1
2 changed files with 38 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}; use crate::{io, Script, ScriptBuf, Transaction, TxIn, TxOut, Sequence};
@ -33,9 +32,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,
@ -491,7 +490,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
@ -501,6 +500,16 @@ impl<R: Deref<Target = Transaction>> SighashCache<R> {
SighashCache { tx, common_cache: None, taproot_cache: None, segwit_cache: None } SighashCache { tx, common_cache: None, taproot_cache: None, segwit_cache: None }
} }
/// Returns the reference to the cached transaction.
pub fn transaction(&self) -> &Transaction {
self.tx.borrow()
}
/// Destroys the cache and recovers the stored transaction.
pub fn into_transaction(self) -> R {
self.tx
}
/// Encodes the BIP341 signing data for any flag type into a given object implementing a /// Encodes the BIP341 signing data for any flag type into a given object implementing a
/// [`io::Write`] trait. /// [`io::Write`] trait.
pub fn taproot_encode_signing_data_to<Write: io::Write, T: Borrow<TxOut>>( pub fn taproot_encode_signing_data_to<Write: io::Write, T: Borrow<TxOut>>(
@ -512,7 +521,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();
@ -525,10 +534,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.
@ -566,9 +575,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)?;
@ -595,11 +604,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);
@ -696,7 +706,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)?;
@ -714,9 +724,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)?;
@ -727,16 +737,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(())
} }
@ -789,15 +799,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
@ -883,7 +893,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,
@ -931,12 +941,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();
@ -961,7 +971,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 {
@ -988,7 +998,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.
/// ///
@ -1009,7 +1019,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)
} }
} }