Skip to content

Commit

Permalink
Feature: Psbt fee checks
Browse files Browse the repository at this point in the history
  • Loading branch information
junderw committed Sep 28, 2023
1 parent c31b933 commit dac627c
Show file tree
Hide file tree
Showing 6 changed files with 221 additions and 9 deletions.
2 changes: 1 addition & 1 deletion bitcoin/examples/ecdsa-psbt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ fn main() -> Result<()> {
let finalized = online.finalize_psbt(signed)?;

// You can use `bt sendrawtransaction` to broadcast the extracted transaction.
let tx = finalized.extract_tx();
let tx = finalized.extract_tx_unchecked_fee_rate();
tx.verify(|_| Some(previous_output())).expect("failed to verify transaction");

let hex = encode::serialize_hex(&tx);
Expand Down
6 changes: 3 additions & 3 deletions bitcoin/examples/taproot-psbt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ fn generate_bip86_key_spend_tx(
});

// EXTRACTOR
let tx = psbt.extract_tx();
let tx = psbt.extract_tx_unchecked_fee_rate();
tx.verify(|_| {
Some(TxOut {
value: from_amount,
Expand Down Expand Up @@ -553,7 +553,7 @@ impl BenefactorWallet {
});

// EXTRACTOR
let tx = psbt.extract_tx();
let tx = psbt.extract_tx_unchecked_fee_rate();
tx.verify(|_| {
Some(TxOut { value: input_value, script_pubkey: output_script_pubkey.clone() })
})
Expand Down Expand Up @@ -695,7 +695,7 @@ impl BeneficiaryWallet {
});

// EXTRACTOR
let tx = psbt.extract_tx();
let tx = psbt.extract_tx_unchecked_fee_rate();
tx.verify(|_| {
Some(TxOut { value: input_value, script_pubkey: input_script_pubkey.clone() })
})
Expand Down
2 changes: 1 addition & 1 deletion bitcoin/src/blockdata/fee_rate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl FeeRate {
impl fmt::Display for FeeRate {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
if f.alternate() {
write!(f, "{} sat/kwu", self.0)
write!(f, "{}.00 sat/vbyte", self.to_sat_per_vb_ceil())
} else {
fmt::Display::fmt(&self.0, f)
}
Expand Down
46 changes: 46 additions & 0 deletions bitcoin/src/psbt/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,52 @@ macro_rules! hex_psbt {
};
}

#[cfg(test)]
macro_rules! psbt_with_values {
($input:expr, $output:expr) => {
Psbt {
unsigned_tx: Transaction {
version: transaction::Version::TWO,
lock_time: absolute::LockTime::ZERO,
input: vec![TxIn {
previous_output: OutPoint {
txid: "f61b1742ca13176464adb3cb66050c00787bb3a4eead37e985f2df1e37718126"
.parse()
.unwrap(),
vout: 0,
},
script_sig: ScriptBuf::new(),
sequence: Sequence::ENABLE_LOCKTIME_NO_RBF,
witness: Witness::default(),
}],
output: vec![TxOut {
value: Amount::from_sat($output),
script_pubkey: ScriptBuf::from_hex(
"a9143545e6e33b832c47050f24d3eeb93c9c03948bc787",
)
.unwrap(),
}],
},
xpub: Default::default(),
version: 0,
proprietary: BTreeMap::new(),
unknown: BTreeMap::new(),

inputs: vec![Input {
witness_utxo: Some(TxOut {
value: Amount::from_sat($input),
script_pubkey: ScriptBuf::from_hex(
"a914339725ba21efd62ac753a9bcd067d6c7a6a39d0587",
)
.unwrap(),
}),
..Default::default()
}],
outputs: vec![],
}
};
}

macro_rules! combine {
($thing:ident, $slf:ident, $other:ident) => {
if let (&None, Some($thing)) = (&$slf.$thing, $other.$thing) {
Expand Down
172 changes: 169 additions & 3 deletions bitcoin/src/psbt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::crypto::ecdsa;
use crate::crypto::key::{PrivateKey, PublicKey};
use crate::prelude::*;
use crate::sighash::{self, EcdsaSighashType, SighashCache};
use crate::Amount;
use crate::{Amount, FeeRate};

#[macro_use]
mod macros;
Expand Down Expand Up @@ -121,8 +121,55 @@ impl Psbt {
Ok(psbt)
}

/// Extracts the `Transaction` from a PSBT by filling in the available signature information.
pub fn extract_tx(self) -> Transaction {
/// The default `max_fee_rate` value used for extracting transactions with [`extract_tx`]
///
/// As of 2023, even the biggest overpayers during the highest fee markets only paid around
/// 1000 sats/vByte. 25k sats/vByte is obviously a mistake at this point.
///
/// [`extract_tx`]: Psbt::extract_tx
pub const DEFAULT_MAX_FEE_RATE: FeeRate = FeeRate::from_sat_per_vb_unchecked(25_000);

/// An alias for [`extract_tx_fee_rate_limit`].
///
/// [`extract_tx_fee_rate_limit`]: Psbt::extract_tx_fee_rate_limit
pub fn extract_tx(self) -> Result<Transaction, ExtractTxError> {
self.internal_extract_tx_with_fee_rate_limit(Self::DEFAULT_MAX_FEE_RATE)
}

/// Extracts the [`Transaction`] from a [`Psbt`] by filling in the available signature information.
///
/// ## Errors
///
/// [`ExtractTxError`] variants will contain either the [`Psbt`] itself or the [`Transaction`]
/// that was extracted. These can be extracted from the Errors in order to recover.
/// See the error documentation for info on the variants. In general, it covers large fees.
pub fn extract_tx_fee_rate_limit(self) -> Result<Transaction, ExtractTxError> {
self.internal_extract_tx_with_fee_rate_limit(Self::DEFAULT_MAX_FEE_RATE)
}

/// Extracts the [`Transaction`] from a [`Psbt`] by filling in the available signature information.
///
/// ## Errors
///
/// See [`extract_tx`].
///
/// [`extract_tx`]: Psbt::extract_tx
pub fn extract_tx_with_fee_rate_limit(
self,
max_fee_rate: FeeRate,
) -> Result<Transaction, ExtractTxError> {
self.internal_extract_tx_with_fee_rate_limit(max_fee_rate)
}

/// Perform [`extract_tx_fee_rate_limit`] without the fee rate check.
///
/// This can result in a transaction with absurdly high fees. Use with caution.
///
/// [`extract_tx_fee_rate_limit`]: Psbt::extract_tx_fee_rate_limit
pub fn extract_tx_unchecked_fee_rate(self) -> Transaction { self.internal_extract_tx() }

#[inline]
fn internal_extract_tx(self) -> Transaction {
let mut tx: Transaction = self.unsigned_tx;

for (vin, psbtin) in tx.input.iter_mut().zip(self.inputs.into_iter()) {
Expand All @@ -133,6 +180,38 @@ impl Psbt {
tx
}

#[inline]
fn internal_extract_tx_with_fee_rate_limit(
self,
max_fee_rate: FeeRate,
) -> Result<Transaction, ExtractTxError> {
let fee = match self.fee() {
Ok(fee) => fee,
Err(Error::MissingUtxo) =>
return Err(ExtractTxError::MissingInputValue { tx: self.internal_extract_tx() }),
Err(Error::NegativeFee) => return Err(ExtractTxError::SendingTooMuch { psbt: self }),
Err(Error::FeeOverflow) =>
return Err(ExtractTxError::AbsurdFeeRate {
fee_rate: FeeRate::MAX,
tx: self.internal_extract_tx(),
}),
_ => unreachable!(),
};

// Note: Move prevents usage of &self from now on.
let tx = self.internal_extract_tx();

// Now that the extracted Transaction is made, decide how to return it.
let fee_rate =
FeeRate::from_sat_per_kwu(fee.to_sat().saturating_mul(1000) / tx.weight().to_wu());
// Prefer to return an AbsurdFeeRate error when both trigger.
if fee_rate > max_fee_rate {
return Err(ExtractTxError::AbsurdFeeRate { fee_rate, tx });
}

Ok(tx)
}

/// Combines this [`Psbt`] with `other` PSBT as described by BIP 174.
///
/// In accordance with BIP 174 this function is commutative i.e., `A.combine(B) == B.combine(A)`
Expand Down Expand Up @@ -771,6 +850,48 @@ impl From<IndexOutOfBoundsError> for SignError {
fn from(e: IndexOutOfBoundsError) -> Self { SignError::IndexOutOfBounds(e) }
}

#[derive(Debug, Clone, PartialEq, Eq)]
/// This error is returned when extracting a [`Transaction`] from a [`Psbt`].
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 {
match self {
ExtractTxError::AbsurdFeeRate { fee_rate, .. } =>
write!(f, "An absurdly high fee rate of {}", fee_rate),
ExtractTxError::MissingInputValue { .. } => write!(
f,
"One of the inputs lacked value information (witness_utxo or non_witness_utxo)"
),
ExtractTxError::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 {}

/// Input index out of bounds (actual index, maximum index allowed).
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum IndexOutOfBoundsError {
Expand Down Expand Up @@ -924,6 +1045,51 @@ mod tests {
assert!(!pk.compressed);
}

#[test]
fn psbt_high_fee_checks() {
let psbt = psbt_with_values!(5_000_000_000_000, 1000);
assert_eq!(
psbt.clone().extract_tx().map_err(|e| match e {
ExtractTxError::AbsurdFeeRate { fee_rate, .. } => fee_rate,
_ => panic!(""),
}),
Err(FeeRate::from_sat_per_kwu(15060240960843))
);
assert_eq!(
psbt.clone().extract_tx_fee_rate_limit().map_err(|e| match e {
ExtractTxError::AbsurdFeeRate { fee_rate, .. } => fee_rate,
_ => panic!(""),
}),
Err(FeeRate::from_sat_per_kwu(15060240960843))
);
assert_eq!(
psbt.clone()
.extract_tx_with_fee_rate_limit(FeeRate::from_sat_per_kwu(15060240960842))
.map_err(|e| match e {
ExtractTxError::AbsurdFeeRate { fee_rate, .. } => fee_rate,
_ => panic!(""),
}),
Err(FeeRate::from_sat_per_kwu(15060240960843))
);
assert!(psbt
.clone()
.extract_tx_with_fee_rate_limit(FeeRate::from_sat_per_kwu(15060240960843))
.is_ok());

// Testing that extract_tx will error at 25k sat/vbyte (6250000 sat/kwu)
assert_eq!(
psbt_with_values!(2076001, 1000).extract_tx().map_err(|e| match e {
ExtractTxError::AbsurdFeeRate { fee_rate, .. } => fee_rate,
_ => panic!(""),
}),
Err(FeeRate::from_sat_per_kwu(6250003)) // 6250000 is 25k sat/vbyte
);

// Lowering the input satoshis by 1 lowers the sat/kwu by 3
// Putting it exactly at 25k sat/vbyte
assert!(psbt_with_values!(2076000, 1000).extract_tx().is_ok());
}

#[test]
fn serialize_then_deserialize_output() {
let secp = &Secp256k1::new();
Expand Down
2 changes: 1 addition & 1 deletion bitcoin/tests/psbt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ fn finalize(psbt: Psbt) -> Psbt {
fn extract_transaction(psbt: Psbt) -> Transaction {
let expected_tx_hex = include_str!("data/extract_tx_hex");

let tx = psbt.extract_tx();
let tx = psbt.extract_tx_unchecked_fee_rate();

let got = serialize_hex(&tx);
assert_eq!(got, expected_tx_hex);
Expand Down

0 comments on commit dac627c

Please sign in to comment.