Merge pull request #16 from tcharding/01-13-extract

v2: Implement Extractor and Finalizer roles

Cargo.toml

@@ -23,15 +23,15 @@ no-std = ["bitcoin/no-std", "core2"]
 serde = ["actual-serde", "bitcoin/serde"]
 base64 = ["bitcoin/base64"]
 
-# TODO: There is curently no way to turn on miniscript/serde and base64
 miniscript-std = ["std", "miniscript/std"]
 miniscript-no-std = ["no-std", "miniscript/no-std"]
 
 [dependencies]
 bitcoin = { version = "0.31.0", default-features = false, features = [] }
 
-# Consider using "miniscript-std" or "miniscript-no-std"
+# Currenty miniscript only works in with "std" enabled.
 miniscript = { version = "11.0.0", default-features = false, optional = true }
+
 # Do NOT use this as a feature! Use the `serde` feature instead.
 actual-serde = { package = "serde", version = "1.0.103", default-features = false, features = [ "derive", "alloc" ], optional = true }
 # There is no reason to use this dependency directly, it is activated by the "no-std" feature.

examples/v2.rs

@@ -19,8 +19,7 @@ use psbt::bitcoin::{
     script, Address, Amount, Network, OutPoint, PublicKey, ScriptBuf, Sequence, TxOut, Txid,
 };
 use psbt::v2::{
-    self, Constructor, Input, InputBuilder, Modifiable, Output, OutputBuilder, Psbt, Signer,
-    Updater,
+    self, Constructor, InputBuilder, Modifiable, Output, OutputBuilder, Psbt, Signer, Updater,
 };
 
 pub const DUMMY_UTXO_AMOUNT: Amount = Amount::from_sat(20_000_000);
@@ -63,7 +62,9 @@ fn main() -> anyhow::Result<()> {
         .build();
 
     // If no lock time is required we can just create the `Input` directly.
-    let input_b = Input::new(previous_output_b);
+    let input_b = InputBuilder::new(previous_output_b)
+        // .segwit_fund(txout); TODO: Add funding utxo.
+        .build();
 
     // Build Alice's change output.
     let change = TxOut { value: change_value_a, script_pubkey: change_address_a.script_pubkey() };

src/v0/extractor.rs

@@ -60,8 +60,23 @@ impl Psbt {
     /// [`extract_tx_fee_rate_limit`]: Psbt::extract_tx_fee_rate_limit
     pub fn extract_tx_unchecked_fee_rate(self) -> Transaction { self.internal_extract_tx() }
 
+    // TODO: This is incomplete, it does not do the checks specified in the bip.
     #[inline]
     fn internal_extract_tx(self) -> Transaction {
+        // The Transaction Extractor must only accept a PSBT. It checks whether all inputs have
+        // complete scriptSigs and scriptWitnesses by checking for the presence of 0x07 Finalized
+        // scriptSig and 0x08 Finalized scriptWitness typed records. If they do, the Transaction
+        // Extractor should construct complete scriptSigs and scriptWitnesses and encode them into
+        // network serialized transactions. Otherwise the Extractor must not modify the PSBT. The
+        // Extractor should produce a fully valid, network serialized transaction if all inputs are
+        // complete.
+
+        // The Transaction Extractor does not need to know how to interpret scripts in order to
+        // extract the network serialized transaction. However it may be able to in order to
+        // validate the network serialized transaction at the same time.
+
+        // A single entity is likely to be both a Transaction Extractor and an Input Finalizer.
+
         let mut tx: Transaction = self.global.unsigned_tx;
 
         for (vin, psbtin) in tx.input.iter_mut().zip(self.inputs.into_iter()) {

src/v2/error.rs

@@ -4,11 +4,11 @@
 
 use core::fmt;
 
-use bitcoin::{sighash, FeeRate, Transaction};
+use bitcoin::sighash::{self, EcdsaSighashType, NonStandardSighashTypeError};
+use bitcoin::PublicKey;
 
 use crate::error::{write_err, FundingUtxoError};
 use crate::v2::map::{global, input, output};
-use crate::v2::Psbt;
 
 /// Error while deserializing a PSBT.
 ///
@@ -102,59 +102,6 @@ impl std::error::Error for IndexOutOfBoundsError {
     }
 }
 
-/// This error is returned when extracting a [`Transaction`] from a PSBT..
-#[derive(Debug, Clone, PartialEq, Eq)]
-#[non_exhaustive]
-pub enum ExtractTxError {
-    /// The [`FeeRate`] is too high
-    AbsurdFeeRate {
-        /// The [`FeeRate`]
-        fee_rate: FeeRate,
-        /// The extracted [`Transaction`] (use this to ignore the error)
-        tx: Transaction,
-    },
-    /// One or more of the inputs lacks value information (witness_utxo or non_witness_utxo)
-    MissingInputValue {
-        /// The extracted [`Transaction`] (use this to ignore the error)
-        tx: Transaction,
-    },
-    /// Input value is less than Output Value, and the [`Transaction`] would be invalid.
-    SendingTooMuch {
-        /// The original `Psbt` is returned untouched.
-        psbt: Psbt,
-    },
-}
-
-impl fmt::Display for ExtractTxError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        use ExtractTxError::*;
-
-        match *self {
-            AbsurdFeeRate { fee_rate, .. } =>
-                write!(f, "An absurdly high fee rate of {}", fee_rate),
-            MissingInputValue { .. } => write!(
-                f,
-                "One of the inputs lacked value information (witness_utxo or non_witness_utxo)"
-            ),
-            SendingTooMuch { .. } => write!(
-                f,
-                "Transaction would be invalid due to output value being greater than input value."
-            ),
-        }
-    }
-}
-
-#[cfg(feature = "std")]
-impl std::error::Error for ExtractTxError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        use ExtractTxError::*;
-
-        match *self {
-            AbsurdFeeRate { .. } | MissingInputValue { .. } | SendingTooMuch { .. } => None,
-        }
-    }
-}
-
 /// Errors encountered while calculating the sighash message.
 #[derive(Debug, Clone, PartialEq, Eq)]
 #[non_exhaustive]
@@ -302,9 +249,7 @@ impl fmt::Display for InputsNotModifiableError {
 }
 
 #[cfg(feature = "std")]
-impl std::error::Error for InputsNotModifiableError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
-}
+impl std::error::Error for InputsNotModifiableError {}
 
 /// Error when passing an PSBT with outputs not modifiable to an output adding `Constructor`.
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -365,3 +310,66 @@ impl fmt::Display for DetermineLockTimeError {
 impl std::error::Error for DetermineLockTimeError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None }
 }
+
+// TODO: Consider creating a type that has input_index and E and simplify all these similar error types?
+/// Error checking the partials sigs have correct sighash types.
+#[derive(Debug)]
+pub enum PartialSigsSighashTypeError {
+    /// Non-standard sighash type found in `input.sighash_type` field.
+    NonStandardInputSighashType {
+        /// The input index with the non-standard sighash type.
+        input_index: usize,
+        /// The non-standard sighash type error.
+        error: NonStandardSighashTypeError,
+    },
+    /// Non-standard sighash type found in `input.partial_sigs`.
+    NonStandardPartialSigsSighashType {
+        /// The input index with the non-standard sighash type.
+        input_index: usize,
+        /// The non-standard sighash type error.
+        error: NonStandardSighashTypeError,
+    },
+    /// Wrong sighash flag in partial signature.
+    WrongSighashFlag {
+        /// The input index with the wrong sighash flag.
+        input_index: usize,
+        /// The sighash type we got.
+        got: EcdsaSighashType,
+        /// The sighash type we require.
+        required: EcdsaSighashType,
+        /// The associated pubkey (key into the `input.partial_sigs` map).
+        pubkey: PublicKey,
+    },
+}
+
+impl fmt::Display for PartialSigsSighashTypeError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use PartialSigsSighashTypeError::*;
+
+        match *self {
+            NonStandardInputSighashType { input_index, ref error } =>
+                write_err!(f, "non-standard sighash type for input {} in sighash_type field", input_index; error),
+            NonStandardPartialSigsSighashType { input_index, ref error } =>
+                write_err!(f, "non-standard sighash type for input {} in partial_sigs", input_index; error),
+            WrongSighashFlag { input_index, got, required, pubkey } => write!(
+                f,
+                "wrong sighash flag for input {} (got: {}, required: {}) pubkey: {}",
+                input_index, got, required, pubkey
+            ),
+        }
+    }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for PartialSigsSighashTypeError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        use PartialSigsSighashTypeError::*;
+
+        // TODO: Is this correct for a struct error fields?
+        match *self {
+            NonStandardInputSighashType { input_index: _, ref error } => Some(error),
+            NonStandardPartialSigsSighashType { input_index: _, ref error } => Some(error),
+            WrongSighashFlag { .. } => None,
+        }
+    }
+}