Merge rust-bitcoin/rust-bitcoin#3978: Delete `TxOut::NULL`

53bcdefba5 api: Run just check-api (Tobin C. Harding)
dd2df2bf10 Delete `TxOut::NULL` (Martin Habovstiak)
a9ffb1571c Stop using `TxOut::NULL` in tests (Martin Habovstiak)
313406d6ab Optimize `encode_signing_data_to_inner` (Martin Habovstiak)

Pull request description:

  This removes `TxOut::NULL` from our API and replaces the very few occurrences in our code. The PR has three commits so that the first one provably doesn't break anything, and the second one could be dropped if anyone complains about not deprecating but I really hope that won't happen.

  As a side effect this also improves signing performance.

  Closes #3975

ACKs for top commit:
  tcharding:
    ACK 53bcdefba5
  apoelstra:
    ACK 53bcdefba5f60878cc7049d6e157e21e985bb72c; successfully ran local tests; thanks!! No need to deprecate. Nobody was using this thing except maybe as a test dummy value

Tree-SHA512: c603512c2df6b43f6bbc107e5a40f64fa6a5f48a96e192cf2304179e3fd76925a8b3c5f03cd64f4c684ee086fd6abdcebfcf9e5785f0c95b5df9b173f8050e7a

api/primitives/all-features.txt

@@ -46,7 +46,6 @@ impl bitcoin_primitives::taproot::TapTweakHash
 impl bitcoin_primitives::transaction::OutPoint
 impl bitcoin_primitives::transaction::Transaction
 impl bitcoin_primitives::transaction::TxIn
-impl bitcoin_primitives::transaction::TxOut
 impl bitcoin_primitives::transaction::Txid
 impl bitcoin_primitives::transaction::Version
 impl bitcoin_primitives::transaction::Wtxid
@@ -1258,7 +1257,6 @@ pub const bitcoin_primitives::transaction::OutPoint::COINBASE_PREVOUT: Self
 pub const bitcoin_primitives::transaction::OutPoint::SIZE: usize
 pub const bitcoin_primitives::transaction::Transaction::MAX_STANDARD_WEIGHT: bitcoin_units::weight::Weight
 pub const bitcoin_primitives::transaction::TxIn::EMPTY_COINBASE: bitcoin_primitives::transaction::TxIn
-pub const bitcoin_primitives::transaction::TxOut::NULL: Self
 pub const bitcoin_primitives::transaction::Txid::COINBASE_PREVOUT: Self
 pub const bitcoin_primitives::transaction::Txid::DISPLAY_BACKWARD: bool
 pub const bitcoin_primitives::transaction::Version::ONE: Self

api/primitives/alloc-only.txt

@@ -46,7 +46,6 @@ impl bitcoin_primitives::taproot::TapTweakHash
 impl bitcoin_primitives::transaction::OutPoint
 impl bitcoin_primitives::transaction::Transaction
 impl bitcoin_primitives::transaction::TxIn
-impl bitcoin_primitives::transaction::TxOut
 impl bitcoin_primitives::transaction::Txid
 impl bitcoin_primitives::transaction::Version
 impl bitcoin_primitives::transaction::Wtxid
@@ -1184,7 +1183,6 @@ pub const bitcoin_primitives::transaction::OutPoint::COINBASE_PREVOUT: Self
 pub const bitcoin_primitives::transaction::OutPoint::SIZE: usize
 pub const bitcoin_primitives::transaction::Transaction::MAX_STANDARD_WEIGHT: bitcoin_units::weight::Weight
 pub const bitcoin_primitives::transaction::TxIn::EMPTY_COINBASE: bitcoin_primitives::transaction::TxIn
-pub const bitcoin_primitives::transaction::TxOut::NULL: Self
 pub const bitcoin_primitives::transaction::Txid::COINBASE_PREVOUT: Self
 pub const bitcoin_primitives::transaction::Txid::DISPLAY_BACKWARD: bool
 pub const bitcoin_primitives::transaction::Version::ONE: Self

bitcoin/src/crypto/sighash.rs

@@ -22,11 +22,11 @@ use io::Write;
 
 use crate::address::script_pubkey::ScriptExt as _;
 use crate::consensus::{encode, Encodable};
-use crate::prelude::{Borrow, BorrowMut, String, ToOwned, Vec};
+use crate::prelude::{Borrow, BorrowMut, String, ToOwned};
 use crate::taproot::{LeafVersion, TapLeafHash, TapLeafTag, TAPROOT_ANNEX_PREFIX};
 use crate::transaction::TransactionExt as _;
 use crate::witness::Witness;
-use crate::{transaction, Amount, Script, ScriptBuf, Sequence, Transaction, TxIn, TxOut};
+use crate::{transaction, Amount, Script, Sequence, Transaction, TxOut};
 
 /// Used for signature hash for invalid use of SIGHASH_SINGLE.
 #[rustfmt::skip]
@@ -959,63 +959,59 @@ impl<R: Borrow<Transaction>> SighashCache<R> {
             script_pubkey: &Script,
             sighash_type: u32,
         ) -> Result<(), io::Error> {
+            use crate::consensus::encode::WriteExt;
+
             let (sighash, anyone_can_pay) =
                 EcdsaSighashType::from_consensus(sighash_type).split_anyonecanpay_flag();
 
-            // Build tx to sign
+            self_.version.consensus_encode(writer)?;
-            let mut tx = Transaction {
-                version: self_.version,
-                lock_time: self_.lock_time,
-                input: vec![],
-                output: vec![],
-            };
             // Add all inputs necessary..
             if anyone_can_pay {
-                tx.input = vec![TxIn {
+                writer.emit_compact_size(1u8)?;
-                    previous_output: self_.input[input_index].previous_output,
+                self_.input[input_index].previous_output.consensus_encode(writer)?;
-                    script_sig: script_pubkey.to_owned(),
+                script_pubkey.consensus_encode(writer)?;
-                    sequence: self_.input[input_index].sequence,
+                self_.input[input_index].sequence.consensus_encode(writer)?;
-                    witness: Witness::default(),
-                }];
             } else {
-                tx.input = Vec::with_capacity(self_.input.len());
+                writer.emit_compact_size(self_.input.len())?;
                 for (n, input) in self_.input.iter().enumerate() {
-                    tx.input.push(TxIn {
+                    input.previous_output.consensus_encode(writer)?;
-                        previous_output: input.previous_output,
+                    if n == input_index {
-                        script_sig: if n == input_index {
+                        script_pubkey.consensus_encode(writer)?;
-                            script_pubkey.to_owned()
                     } else {
-                            ScriptBuf::new()
+                        Script::new().consensus_encode(writer)?;
-                        },
+                    }
-                        sequence: if n != input_index
+                    if n != input_index
                         && (sighash == EcdsaSighashType::Single
                             || sighash == EcdsaSighashType::None)
                     {
-                            Sequence::ZERO
+                        Sequence::ZERO.consensus_encode(writer)?;
                     } else {
-                            input.sequence
+                        input.sequence.consensus_encode(writer)?;
-                        },
+                    }
-                        witness: Witness::default(),
-                    });
                 }
             }
             // ..then all outputs
-            tx.output = match sighash {
+            match sighash {
-                EcdsaSighashType::All => self_.output.clone(),
+                EcdsaSighashType::All => {
+                    self_.output.consensus_encode(writer)?;
+                },
                 EcdsaSighashType::Single => {
-                    let output_iter = self_
+                    // sign all outputs up to and including this one, but erase
-                        .output
+                    // all of them except for this one
-                        .iter()
+                    let count = input_index.min(self_.output.len() - 1);
-                        .take(input_index + 1) // sign all outputs up to and including this one, but erase
+                    writer.emit_compact_size(count + 1)?;
-                        .enumerate() // all of them except for this one
+                    for _ in 0..count {
-                        .map(|(n, out)| if n == input_index { out.clone() } else { TxOut::NULL });
+                        // consensus encoding of the "NULL txout" - max amount, empty script_pubkey
-                    output_iter.collect()
+                        writer.write_all(&[0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00])?;
                     }
-                EcdsaSighashType::None => vec![],
+                    self_.output[count].consensus_encode(writer)?;
+                },
+                EcdsaSighashType::None => {
+                    writer.emit_compact_size(0u8)?;
+                },
                 _ => unreachable!(),
             };
-            // hash the result
+            self_.lock_time.consensus_encode(writer)?;
-            tx.consensus_encode(writer)?;
             sighash_type.to_le_bytes().consensus_encode(writer)?;
             Ok(())
         }
@@ -1529,10 +1525,13 @@ mod tests {
     use super::*;
     use crate::consensus::deserialize;
     use crate::locktime::absolute;
-    use crate::script::ScriptBufExt as _;
+    use crate::script::{ScriptBuf, ScriptBufExt as _};
+    use crate::TxIn;
 
     extern crate serde_json;
 
+    const DUMMY_TXOUT: TxOut = TxOut { value: Amount::MIN, script_pubkey: ScriptBuf::new() };
+
     #[test]
     fn sighash_single_bug() {
         const SIGHASH_SINGLE: u32 = 3;
@@ -1542,7 +1541,7 @@ mod tests {
             version: transaction::Version::ONE,
             lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn::EMPTY_COINBASE, TxIn::EMPTY_COINBASE],
-            output: vec![TxOut::NULL],
+            output: vec![DUMMY_TXOUT],
         };
         let script = ScriptBuf::new();
         let cache = SighashCache::new(&tx);
@@ -1743,13 +1742,13 @@ mod tests {
             c.taproot_signature_hash(0, &empty_prevouts, None, None, TapSighashType::All),
             Err(TaprootError::PrevoutsSize(PrevoutsSizeError))
         );
-        let two = [TxOut::NULL, TxOut::NULL];
+        let two = [DUMMY_TXOUT, DUMMY_TXOUT];
         let too_many_prevouts = Prevouts::All(&two);
         assert_eq!(
             c.taproot_signature_hash(0, &too_many_prevouts, None, None, TapSighashType::All),
             Err(TaprootError::PrevoutsSize(PrevoutsSizeError))
         );
-        let tx_out = TxOut::NULL;
+        let tx_out = DUMMY_TXOUT;
         let prevout = Prevouts::One(1, &tx_out);
         assert_eq!(
             c.taproot_signature_hash(0, &prevout, None, None, TapSighashType::All),

bitcoin/src/psbt/mod.rs

@@ -2279,7 +2279,7 @@ mod tests {
             version: transaction::Version::TWO,
             lock_time: absolute::LockTime::ZERO,
             input: vec![TxIn::EMPTY_COINBASE, TxIn::EMPTY_COINBASE],
-            output: vec![TxOut::NULL],
+            output: vec![TxOut { value: Amount::from_sat(0), script_pubkey: ScriptBuf::new() }],
         };
         let mut psbt = Psbt::from_unsigned_tx(unsigned_tx).unwrap();
 

primitives/src/transaction.rs

@@ -350,13 +350,6 @@ pub struct TxOut {
     pub script_pubkey: ScriptBuf,
 }
 
-#[cfg(feature = "alloc")]
-impl TxOut {
-    /// This is used as a "null txout" in consensus signing code.
-    pub const NULL: Self =
-        TxOut { value: Amount::from_sat(0xffffffffffffffff), script_pubkey: ScriptBuf::new() };
-}
-
 /// A reference to a transaction output.
 ///
 /// ### Bitcoin Core References