Merge pull request #2235 from TheBlueMatt/2023-04-criterion
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Sat, 20 May 2023 23:02:44 +0000 (23:02 +0000)
committerGitHub <noreply@github.com>
Sat, 20 May 2023 23:02:44 +0000 (23:02 +0000)
Replace std's unmaintained bench with criterion

1  2 
.gitignore
lightning-persister/src/lib.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/sign/mod.rs

diff --combined .gitignore
index 28e55b41ce6430b1e15417d4debb0b86b10522e0,22969fae32880afae11eb83f20fdc8c2e68412e7..7a6dc4c793032bc2c1ddf8e3e465201df6cf3543
@@@ -8,7 -8,7 +8,8 @@@ lightning-c-bindings/a.ou
  Cargo.lock
  .idea
  lightning/target
- lightning/ldk-net_graph-*.bin
+ lightning/net_graph-*.bin
+ lightning-rapid-gossip-sync/res/full_graph.lngossip
  lightning-custom-message/target
 +lightning-transaction-sync/target
  no-std-check/target
index d1a1e4a299031a352564728200cf1829b66c2f89,f65ed111e399bcc459a4af87259134ba87c2fd2b..670a7369d8b92c7bb415125f3dc49186c4b2f3d3
@@@ -8,8 -8,7 +8,7 @@@
  
  #![cfg_attr(docsrs, feature(doc_auto_cfg))]
  
- #![cfg_attr(all(test, feature = "_bench_unstable"), feature(test))]
- #[cfg(all(test, feature = "_bench_unstable"))] extern crate test;
+ #[cfg(ldk_bench)] extern crate criterion;
  
  mod util;
  
@@@ -91,13 -90,13 +90,13 @@@ impl FilesystemPersister 
                                continue;
                        }
  
-                       let txid = Txid::from_hex(filename.split_at(64).0)
+                       let txid: Txid = Txid::from_hex(filename.split_at(64).0)
                                .map_err(|_| std::io::Error::new(
                                        std::io::ErrorKind::InvalidData,
                                        "Invalid tx ID in filename",
                                ))?;
  
-                       let index = filename.split_at(65).1.parse()
+                       let index: u16 = filename.split_at(65).1.parse()
                                .map_err(|_| std::io::Error::new(
                                        std::io::ErrorKind::InvalidData,
                                        "Invalid tx index in filename",
@@@ -136,8 -135,9 +135,8 @@@ mod tests 
        extern crate lightning;
        extern crate bitcoin;
        use crate::FilesystemPersister;
 -      use bitcoin::blockdata::block::{Block, BlockHeader};
        use bitcoin::hashes::hex::FromHex;
 -      use bitcoin::{Txid, TxMerkleNode};
 +      use bitcoin::Txid;
        use lightning::chain::ChannelMonitorUpdateStatus;
        use lightning::chain::chainmonitor::Persist;
        use lightning::chain::channelmonitor::CLOSED_CHANNEL_UPDATE_ID;
        use lightning::ln::functional_test_utils::*;
        use lightning::util::test_utils;
        use std::fs;
 -      use bitcoin::hashes::Hash;
        #[cfg(target_os = "windows")]
        use {
                lightning::get_event_msg,
                let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap();
                assert_eq!(node_txn.len(), 1);
  
 -              let header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 };
 -              connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[0].clone()]});
 +              connect_block(&nodes[1], &create_dummy_block(nodes[0].best_block_hash(), 42, vec![node_txn[0].clone(), node_txn[0].clone()]));
                check_closed_broadcast!(nodes[1], true);
                check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
                check_added_monitors!(nodes[1], 1);
        }
  }
  
- #[cfg(all(test, feature = "_bench_unstable"))]
+ #[cfg(ldk_bench)]
+ /// Benches
  pub mod bench {
-       use test::Bencher;
+       use criterion::Criterion;
  
-       #[bench]
-       fn bench_sends(bench: &mut Bencher) {
+       /// Bench!
+       pub fn bench_sends(bench: &mut Criterion) {
                let persister_a = super::FilesystemPersister::new("bench_filesystem_persister_a".to_string());
                let persister_b = super::FilesystemPersister::new("bench_filesystem_persister_b".to_string());
-               lightning::ln::channelmanager::bench::bench_two_sends(bench, persister_a, persister_b);
+               lightning::ln::channelmanager::bench::bench_two_sends(
+                       bench, "bench_filesystem_persisted_sends", persister_a, persister_b);
        }
  }
index f6cb81376e2490a205127760d5d6f14f0798bf0c,52cd2d8bc833a570a1125ea63cc83ec7284862e1..565eb83ef8d5cad0cc3e4846efa87e58d470a979
@@@ -2732,9 -2732,10 +2732,9 @@@ wher
                let onion_keys = onion_utils::construct_onion_keys(&self.secp_ctx, &path, &session_priv)
                        .map_err(|_| APIError::InvalidRoute{err: "Pubkey along hop was maliciously selected".to_owned()})?;
                let (onion_payloads, htlc_msat, htlc_cltv) = onion_utils::build_onion_payloads(path, total_value, recipient_onion, cur_height, keysend_preimage)?;
 -              if onion_utils::route_size_insane(&onion_payloads) {
 -                      return Err(APIError::InvalidRoute{err: "Route size too large considering onion data".to_owned()});
 -              }
 -              let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash);
 +
 +              let onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, prng_seed, payment_hash)
 +                      .map_err(|_| APIError::InvalidRoute { err: "Route size too large considering onion data".to_owned()})?;
  
                let err: Result<(), _> = loop {
                        let (counterparty_node_id, id) = match self.short_to_chan_info.read().unwrap().get(&path.hops.first().unwrap().short_channel_id) {
@@@ -9266,7 -9267,7 +9266,7 @@@ mod tests 
        }
  }
  
- #[cfg(all(any(test, feature = "_test_utils"), feature = "_bench_unstable"))]
+ #[cfg(ldk_bench)]
  pub mod bench {
        use crate::chain::Listen;
        use crate::chain::chainmonitor::{ChainMonitor, Persist};
  
        use crate::sync::{Arc, Mutex};
  
-       use test::Bencher;
+       use criterion::Criterion;
  
        type Manager<'a, P> = ChannelManager<
                &'a ChainMonitor<InMemorySigner, &'a test_utils::TestChainSource,
                fn chain_monitor(&self) -> Option<&test_utils::TestChainMonitor> { None }
        }
  
-       #[cfg(test)]
-       #[bench]
-       fn bench_sends(bench: &mut Bencher) {
-               bench_two_sends(bench, test_utils::TestPersister::new(), test_utils::TestPersister::new());
+       pub fn bench_sends(bench: &mut Criterion) {
+               bench_two_sends(bench, "bench_sends", test_utils::TestPersister::new(), test_utils::TestPersister::new());
        }
  
-       pub fn bench_two_sends<P: Persist<InMemorySigner>>(bench: &mut Bencher, persister_a: P, persister_b: P) {
+       pub fn bench_two_sends<P: Persist<InMemorySigner>>(bench: &mut Criterion, bench_name: &str, persister_a: P, persister_b: P) {
                // Do a simple benchmark of sending a payment back and forth between two nodes.
                // Note that this is unrealistic as each payment send will require at least two fsync
                // calls per node.
  
                assert_eq!(&tx_broadcaster.txn_broadcasted.lock().unwrap()[..], &[tx.clone()]);
  
 -              let block = Block {
 -                      header: BlockHeader { version: 0x20000000, prev_blockhash: BestBlock::from_network(network).block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 },
 -                      txdata: vec![tx],
 -              };
 +              let block = create_dummy_block(BestBlock::from_network(network).block_hash(), 42, vec![tx]);
                Listen::block_connected(&node_a, &block, 1);
                Listen::block_connected(&node_b, &block, 1);
  
                        }
                }
  
-               bench.iter(|| {
+               bench.bench_function(bench_name, |b| b.iter(|| {
                        send_payment!(node_a, node_b);
                        send_payment!(node_b, node_a);
-               });
+               }));
        }
  }
index fa8bdcc58be01ef57547cbf9beaf548be38ca474,56f08c7656a8c071f0a4a4bc4633e970c4130a0f..93841517ba75ecfbc154382c2340d238cda93daf
@@@ -85,14 -85,16 +85,14 @@@ pub fn confirm_transactions_at<'a, 'b, 
        if conf_height > first_connect_height {
                connect_blocks(node, conf_height - first_connect_height);
        }
 -      let mut block = Block {
 -              header: BlockHeader { version: 0x20000000, prev_blockhash: node.best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: conf_height, bits: 42, nonce: 42 },
 -              txdata: Vec::new(),
 -      };
 +      let mut txdata = Vec::new();
        for _ in 0..*node.network_chan_count.borrow() { // Make sure we don't end up with channels at the same short id by offsetting by chan_count
 -              block.txdata.push(Transaction { version: 0, lock_time: PackedLockTime::ZERO, input: Vec::new(), output: Vec::new() });
 +              txdata.push(Transaction { version: 0, lock_time: PackedLockTime::ZERO, input: Vec::new(), output: Vec::new() });
        }
        for tx in txn {
 -              block.txdata.push((*tx).clone());
 +              txdata.push((*tx).clone());
        }
 +      let block = create_dummy_block(node.best_block_hash(), conf_height, txdata);
        connect_block(node, &block);
        scid_utils::scid_from_parts(conf_height as u64, block.txdata.len() as u64 - 1, 0).unwrap()
  }
@@@ -189,31 -191,22 +189,31 @@@ impl ConnectStyle 
        }
  }
  
 +pub fn create_dummy_header(prev_blockhash: BlockHash, time: u32) -> BlockHeader {
 +      BlockHeader {
 +              version: 0x2000_0000,
 +              prev_blockhash,
 +              merkle_root: TxMerkleNode::all_zeros(),
 +              time,
 +              bits: 42,
 +              nonce: 42,
 +      }
 +}
 +
 +pub fn create_dummy_block(prev_blockhash: BlockHash, time: u32, txdata: Vec<Transaction>) -> Block {
 +      Block { header: create_dummy_header(prev_blockhash, time), txdata }
 +}
 +
  pub fn connect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, depth: u32) -> BlockHash {
        let skip_intermediaries = node.connect_style.borrow().skips_blocks();
  
        let height = node.best_block_info().1 + 1;
 -      let mut block = Block {
 -              header: BlockHeader { version: 0x2000000, prev_blockhash: node.best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: height, bits: 42, nonce: 42 },
 -              txdata: vec![],
 -      };
 +      let mut block = create_dummy_block(node.best_block_hash(), height, Vec::new());
        assert!(depth >= 1);
        for i in 1..depth {
                let prev_blockhash = block.header.block_hash();
                do_connect_block(node, block, skip_intermediaries);
 -              block = Block {
 -                      header: BlockHeader { version: 0x20000000, prev_blockhash, merkle_root: TxMerkleNode::all_zeros(), time: height + i, bits: 42, nonce: 42 },
 -                      txdata: vec![],
 -              };
 +              block = create_dummy_block(prev_blockhash, height + i, Vec::new());
        }
        let hash = block.header.block_hash();
        do_connect_block(node, block, false);
@@@ -1776,7 -1769,7 +1776,7 @@@ macro_rules! get_route_and_payment_has
  }
  
  #[macro_export]
- #[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))]
+ #[cfg(any(test, ldk_bench, feature = "_test_utils"))]
  macro_rules! expect_payment_claimable {
        ($node: expr, $expected_payment_hash: expr, $expected_payment_secret: expr, $expected_recv_value: expr) => {
                expect_payment_claimable!($node, $expected_payment_hash, $expected_payment_secret, $expected_recv_value, None, $node.node.get_our_node_id())
  }
  
  #[macro_export]
- #[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))]
+ #[cfg(any(test, ldk_bench, feature = "_test_utils"))]
  macro_rules! expect_payment_claimed {
        ($node: expr, $expected_payment_hash: expr, $expected_recv_value: expr) => {
                let events = $node.node.get_and_clear_pending_events();
@@@ -1920,7 -1913,7 +1920,7 @@@ macro_rules! expect_payment_forwarded 
        }
  }
  
- #[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))]
+ #[cfg(any(test, ldk_bench, feature = "_test_utils"))]
  pub fn expect_channel_pending_event<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, expected_counterparty_node_id: &PublicKey) {
        let events = node.node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
        }
  }
  
- #[cfg(any(test, feature = "_bench_unstable", feature = "_test_utils"))]
+ #[cfg(any(test, ldk_bench, feature = "_test_utils"))]
  pub fn expect_channel_ready_event<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, expected_counterparty_node_id: &PublicKey) {
        let events = node.node.get_and_clear_pending_events();
        assert_eq!(events.len(), 1);
index cd898f12b32e41ca7523f79843f4a1ceff3407be,bc4521fa861e502090b767ea0f62144684012b6d..226a2bab72b9c35d26bcac26d462a3d34bfe2a6c
@@@ -16,7 -16,6 +16,7 @@@ use bitcoin::blockdata::transaction::{T
  use bitcoin::blockdata::script::{Script, Builder};
  use bitcoin::blockdata::opcodes;
  use bitcoin::network::constants::Network;
 +use bitcoin::psbt::PartiallySignedTransaction;
  use bitcoin::util::bip32::{ExtendedPrivKey, ExtendedPubKey, ChildNumber};
  use bitcoin::util::sighash;
  
@@@ -219,126 -218,6 +219,126 @@@ impl_writeable_tlv_based_enum!(Spendabl
        (2, StaticPaymentOutput),
  );
  
 +impl SpendableOutputDescriptor {
 +      /// Turns this into a [`bitcoin::psbt::Input`] which can be used to create a
 +      /// [`PartiallySignedTransaction`] which spends the given descriptor.
 +      ///
 +      /// Note that this does not include any signatures, just the information required to
 +      /// construct the transaction and sign it.
 +      pub fn to_psbt_input(&self) -> bitcoin::psbt::Input {
 +              match self {
 +                      SpendableOutputDescriptor::StaticOutput { output, .. } => {
 +                              // Is a standard P2WPKH, no need for witness script
 +                              bitcoin::psbt::Input {
 +                                      witness_utxo: Some(output.clone()),
 +                                      ..Default::default()
 +                              }
 +                      },
 +                      SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) => {
 +                              // TODO we could add the witness script as well
 +                              bitcoin::psbt::Input {
 +                                      witness_utxo: Some(descriptor.output.clone()),
 +                                      ..Default::default()
 +                              }
 +                      },
 +                      SpendableOutputDescriptor::StaticPaymentOutput(descriptor) => {
 +                              // TODO we could add the witness script as well
 +                              bitcoin::psbt::Input {
 +                                      witness_utxo: Some(descriptor.output.clone()),
 +                                      ..Default::default()
 +                              }
 +                      },
 +              }
 +      }
 +
 +      /// Creates an unsigned [`PartiallySignedTransaction`] which spends the given descriptors to
 +      /// the given outputs, plus an output to the given change destination (if sufficient
 +      /// change value remains). The PSBT will have a feerate, at least, of the given value.
 +      ///
 +      /// The `locktime` argument is used to set the transaction's locktime. If `None`, the
 +      /// transaction will have a locktime of 0. It it recommended to set this to the current block
 +      /// height to avoid fee sniping, unless you have some specific reason to use a different
 +      /// locktime.
 +      ///
 +      /// Returns the PSBT and expected max transaction weight.
 +      ///
 +      /// Returns `Err(())` if the output value is greater than the input value minus required fee,
 +      /// if a descriptor was duplicated, or if an output descriptor `script_pubkey`
 +      /// does not match the one we can spend.
 +      ///
 +      /// We do not enforce that outputs meet the dust limit or that any output scripts are standard.
 +      pub fn create_spendable_outputs_psbt(descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>, change_destination_script: Script, feerate_sat_per_1000_weight: u32, locktime: Option<PackedLockTime>) -> Result<(PartiallySignedTransaction, usize), ()> {
 +              let mut input = Vec::with_capacity(descriptors.len());
 +              let mut input_value = 0;
 +              let mut witness_weight = 0;
 +              let mut output_set = HashSet::with_capacity(descriptors.len());
 +              for outp in descriptors {
 +                      match outp {
 +                              SpendableOutputDescriptor::StaticPaymentOutput(descriptor) => {
 +                                      if !output_set.insert(descriptor.outpoint) { return Err(()); }
 +                                      input.push(TxIn {
 +                                              previous_output: descriptor.outpoint.into_bitcoin_outpoint(),
 +                                              script_sig: Script::new(),
 +                                              sequence: Sequence::ZERO,
 +                                              witness: Witness::new(),
 +                                      });
 +                                      witness_weight += StaticPaymentOutputDescriptor::MAX_WITNESS_LENGTH;
 +                                      #[cfg(feature = "grind_signatures")]
 +                                      { witness_weight -= 1; } // Guarantees a low R signature
 +                                      input_value += descriptor.output.value;
 +                              },
 +                              SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) => {
 +                                      if !output_set.insert(descriptor.outpoint) { return Err(()); }
 +                                      input.push(TxIn {
 +                                              previous_output: descriptor.outpoint.into_bitcoin_outpoint(),
 +                                              script_sig: Script::new(),
 +                                              sequence: Sequence(descriptor.to_self_delay as u32),
 +                                              witness: Witness::new(),
 +                                      });
 +                                      witness_weight += DelayedPaymentOutputDescriptor::MAX_WITNESS_LENGTH;
 +                                      #[cfg(feature = "grind_signatures")]
 +                                      { witness_weight -= 1; } // Guarantees a low R signature
 +                                      input_value += descriptor.output.value;
 +                              },
 +                              SpendableOutputDescriptor::StaticOutput { ref outpoint, ref output } => {
 +                                      if !output_set.insert(*outpoint) { return Err(()); }
 +                                      input.push(TxIn {
 +                                              previous_output: outpoint.into_bitcoin_outpoint(),
 +                                              script_sig: Script::new(),
 +                                              sequence: Sequence::ZERO,
 +                                              witness: Witness::new(),
 +                                      });
 +                                      witness_weight += 1 + 73 + 34;
 +                                      #[cfg(feature = "grind_signatures")]
 +                                      { witness_weight -= 1; } // Guarantees a low R signature
 +                                      input_value += output.value;
 +                              }
 +                      }
 +                      if input_value > MAX_VALUE_MSAT / 1000 { return Err(()); }
 +              }
 +              let mut tx = Transaction {
 +                      version: 2,
 +                      lock_time: locktime.unwrap_or(PackedLockTime::ZERO),
 +                      input,
 +                      output: outputs,
 +              };
 +              let expected_max_weight =
 +                      transaction_utils::maybe_add_change_output(&mut tx, input_value, witness_weight, feerate_sat_per_1000_weight, change_destination_script)?;
 +
 +              let psbt_inputs = descriptors.iter().map(|d| d.to_psbt_input()).collect::<Vec<_>>();
 +              let psbt = PartiallySignedTransaction {
 +                      inputs: psbt_inputs,
 +                      outputs: vec![Default::default(); tx.output.len()],
 +                      unsigned_tx: tx,
 +                      xpub: Default::default(),
 +                      version: 0,
 +                      proprietary: Default::default(),
 +                      unknown: Default::default(),
 +              };
 +              Ok((psbt, expected_max_weight))
 +      }
 +}
 +
  /// A trait to handle Lightning channel key material without concretizing the channel type or
  /// the signature mechanism.
  pub trait ChannelSigner {
@@@ -1292,40 -1171,97 +1292,40 @@@ impl KeysManager 
                )
        }
  
 -      /// Creates a [`Transaction`] which spends the given descriptors to the given outputs, plus an
 -      /// output to the given change destination (if sufficient change value remains). The
 -      /// transaction will have a feerate, at least, of the given value.
 +      /// Signs the given [`PartiallySignedTransaction`] which spends the given [`SpendableOutputDescriptor`]s.
 +      /// The resulting inputs will be finalized and the PSBT will be ready for broadcast if there
 +      /// are no other inputs that need signing.
        ///
 -      /// Returns `Err(())` if the output value is greater than the input value minus required fee,
 -      /// if a descriptor was duplicated, or if an output descriptor `script_pubkey`
 -      /// does not match the one we can spend.
 -      ///
 -      /// We do not enforce that outputs meet the dust limit or that any output scripts are standard.
 +      /// Returns `Err(())` if the PSBT is missing a descriptor or if we fail to sign.
        ///
        /// May panic if the [`SpendableOutputDescriptor`]s were not generated by channels which used
        /// this [`KeysManager`] or one of the [`InMemorySigner`] created by this [`KeysManager`].
 -      pub fn spend_spendable_outputs<C: Signing>(&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>, change_destination_script: Script, feerate_sat_per_1000_weight: u32, secp_ctx: &Secp256k1<C>) -> Result<Transaction, ()> {
 -              let mut input = Vec::new();
 -              let mut input_value = 0;
 -              let mut witness_weight = 0;
 -              let mut output_set = HashSet::with_capacity(descriptors.len());
 -              for outp in descriptors {
 -                      match outp {
 -                              SpendableOutputDescriptor::StaticPaymentOutput(descriptor) => {
 -                                      input.push(TxIn {
 -                                              previous_output: descriptor.outpoint.into_bitcoin_outpoint(),
 -                                              script_sig: Script::new(),
 -                                              sequence: Sequence::ZERO,
 -                                              witness: Witness::new(),
 -                                      });
 -                                      witness_weight += StaticPaymentOutputDescriptor::MAX_WITNESS_LENGTH;
 -                                      #[cfg(feature = "grind_signatures")]
 -                                      { witness_weight -= 1; } // Guarantees a low R signature
 -                                      input_value += descriptor.output.value;
 -                                      if !output_set.insert(descriptor.outpoint) { return Err(()); }
 -                              },
 -                              SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) => {
 -                                      input.push(TxIn {
 -                                              previous_output: descriptor.outpoint.into_bitcoin_outpoint(),
 -                                              script_sig: Script::new(),
 -                                              sequence: Sequence(descriptor.to_self_delay as u32),
 -                                              witness: Witness::new(),
 -                                      });
 -                                      witness_weight += DelayedPaymentOutputDescriptor::MAX_WITNESS_LENGTH;
 -                                      #[cfg(feature = "grind_signatures")]
 -                                      { witness_weight -= 1; } // Guarantees a low R signature
 -                                      input_value += descriptor.output.value;
 -                                      if !output_set.insert(descriptor.outpoint) { return Err(()); }
 -                              },
 -                              SpendableOutputDescriptor::StaticOutput { ref outpoint, ref output } => {
 -                                      input.push(TxIn {
 -                                              previous_output: outpoint.into_bitcoin_outpoint(),
 -                                              script_sig: Script::new(),
 -                                              sequence: Sequence::ZERO,
 -                                              witness: Witness::new(),
 -                                      });
 -                                      witness_weight += 1 + 73 + 34;
 -                                      #[cfg(feature = "grind_signatures")]
 -                                      { witness_weight -= 1; } // Guarantees a low R signature
 -                                      input_value += output.value;
 -                                      if !output_set.insert(*outpoint) { return Err(()); }
 -                              }
 -                      }
 -                      if input_value > MAX_VALUE_MSAT / 1000 { return Err(()); }
 -              }
 -              let mut spend_tx = Transaction {
 -                      version: 2,
 -                      lock_time: PackedLockTime(0),
 -                      input,
 -                      output: outputs,
 -              };
 -              let expected_max_weight =
 -                      transaction_utils::maybe_add_change_output(&mut spend_tx, input_value, witness_weight, feerate_sat_per_1000_weight, change_destination_script)?;
 -
 +      pub fn sign_spendable_outputs_psbt<C: Signing>(&self, descriptors: &[&SpendableOutputDescriptor], psbt: &mut PartiallySignedTransaction, secp_ctx: &Secp256k1<C>) -> Result<(), ()> {
                let mut keys_cache: Option<(InMemorySigner, [u8; 32])> = None;
 -              let mut input_idx = 0;
                for outp in descriptors {
                        match outp {
                                SpendableOutputDescriptor::StaticPaymentOutput(descriptor) => {
 +                                      let input_idx = psbt.unsigned_tx.input.iter().position(|i| i.previous_output == descriptor.outpoint.into_bitcoin_outpoint()).ok_or(())?;
                                        if keys_cache.is_none() || keys_cache.as_ref().unwrap().1 != descriptor.channel_keys_id {
                                                keys_cache = Some((
                                                        self.derive_channel_keys(descriptor.channel_value_satoshis, &descriptor.channel_keys_id),
                                                        descriptor.channel_keys_id));
                                        }
 -                                      spend_tx.input[input_idx].witness = Witness::from_vec(keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&spend_tx, input_idx, &descriptor, &secp_ctx)?);
 +                                      let witness = Witness::from_vec(keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&psbt.unsigned_tx, input_idx, &descriptor, &secp_ctx)?);
 +                                      psbt.inputs[input_idx].final_script_witness = Some(witness);
                                },
                                SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) => {
 +                                      let input_idx = psbt.unsigned_tx.input.iter().position(|i| i.previous_output == descriptor.outpoint.into_bitcoin_outpoint()).ok_or(())?;
                                        if keys_cache.is_none() || keys_cache.as_ref().unwrap().1 != descriptor.channel_keys_id {
                                                keys_cache = Some((
                                                        self.derive_channel_keys(descriptor.channel_value_satoshis, &descriptor.channel_keys_id),
                                                        descriptor.channel_keys_id));
                                        }
 -                                      spend_tx.input[input_idx].witness = Witness::from_vec(keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&spend_tx, input_idx, &descriptor, &secp_ctx)?);
 +                                      let witness = Witness::from_vec(keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&psbt.unsigned_tx, input_idx, &descriptor, &secp_ctx)?);
 +                                      psbt.inputs[input_idx].final_script_witness = Some(witness);
                                },
 -                              SpendableOutputDescriptor::StaticOutput { ref output, .. } => {
 +                              SpendableOutputDescriptor::StaticOutput { ref outpoint, ref output } => {
 +                                      let input_idx = psbt.unsigned_tx.input.iter().position(|i| i.previous_output == outpoint.into_bitcoin_outpoint()).ok_or(())?;
                                        let derivation_idx = if output.script_pubkey == self.destination_script {
                                                1
                                        } else {
  
                                        if payment_script != output.script_pubkey { return Err(()); };
  
 -                                      let sighash = hash_to_message!(&sighash::SighashCache::new(&spend_tx).segwit_signature_hash(input_idx, &witness_script, output.value, EcdsaSighashType::All).unwrap()[..]);
 +                                      let sighash = hash_to_message!(&sighash::SighashCache::new(&psbt.unsigned_tx).segwit_signature_hash(input_idx, &witness_script, output.value, EcdsaSighashType::All).unwrap()[..]);
                                        let sig = sign_with_aux_rand(secp_ctx, &sighash, &secret.private_key, &self);
                                        let mut sig_ser = sig.serialize_der().to_vec();
                                        sig_ser.push(EcdsaSighashType::All as u8);
 -                                      spend_tx.input[input_idx].witness.push(sig_ser);
 -                                      spend_tx.input[input_idx].witness.push(pubkey.inner.serialize().to_vec());
 +                                      let witness = Witness::from_vec(vec![sig_ser, pubkey.inner.serialize().to_vec()]);
 +                                      psbt.inputs[input_idx].final_script_witness = Some(witness);
                                },
                        }
 -                      input_idx += 1;
                }
  
 +              Ok(())
 +      }
 +
 +      /// Creates a [`Transaction`] which spends the given descriptors to the given outputs, plus an
 +      /// output to the given change destination (if sufficient change value remains). The
 +      /// transaction will have a feerate, at least, of the given value.
 +      ///
 +      /// The `locktime` argument is used to set the transaction's locktime. If `None`, the
 +      /// transaction will have a locktime of 0. It it recommended to set this to the current block
 +      /// height to avoid fee sniping, unless you have some specific reason to use a different
 +      /// locktime.
 +      ///
 +      /// Returns `Err(())` if the output value is greater than the input value minus required fee,
 +      /// if a descriptor was duplicated, or if an output descriptor `script_pubkey`
 +      /// does not match the one we can spend.
 +      ///
 +      /// We do not enforce that outputs meet the dust limit or that any output scripts are standard.
 +      ///
 +      /// May panic if the [`SpendableOutputDescriptor`]s were not generated by channels which used
 +      /// this [`KeysManager`] or one of the [`InMemorySigner`] created by this [`KeysManager`].
 +      pub fn spend_spendable_outputs<C: Signing>(&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>, change_destination_script: Script, feerate_sat_per_1000_weight: u32, locktime: Option<PackedLockTime>, secp_ctx: &Secp256k1<C>) -> Result<Transaction, ()> {
 +              let (mut psbt, expected_max_weight) = SpendableOutputDescriptor::create_spendable_outputs_psbt(descriptors, outputs, change_destination_script, feerate_sat_per_1000_weight, locktime)?;
 +              self.sign_spendable_outputs_psbt(descriptors, &mut psbt, secp_ctx)?;
 +
 +              let spend_tx = psbt.extract_tx();
 +
                debug_assert!(expected_max_weight >= spend_tx.weight());
                // Note that witnesses with a signature vary somewhat in size, so allow
                // `expected_max_weight` to overshoot by up to 3 bytes per input.
@@@ -1601,8 -1512,8 +1601,8 @@@ impl PhantomKeysManager 
        }
  
        /// See [`KeysManager::spend_spendable_outputs`] for documentation on this method.
 -      pub fn spend_spendable_outputs<C: Signing>(&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>, change_destination_script: Script, feerate_sat_per_1000_weight: u32, secp_ctx: &Secp256k1<C>) -> Result<Transaction, ()> {
 -              self.inner.spend_spendable_outputs(descriptors, outputs, change_destination_script, feerate_sat_per_1000_weight, secp_ctx)
 +      pub fn spend_spendable_outputs<C: Signing>(&self, descriptors: &[&SpendableOutputDescriptor], outputs: Vec<TxOut>, change_destination_script: Script, feerate_sat_per_1000_weight: u32, locktime: Option<PackedLockTime>, secp_ctx: &Secp256k1<C>) -> Result<Transaction, ()> {
 +              self.inner.spend_spendable_outputs(descriptors, outputs, change_destination_script, feerate_sat_per_1000_weight, locktime, secp_ctx)
        }
  
        /// See [`KeysManager::derive_channel_keys`] for documentation on this method.
@@@ -1628,8 -1539,8 +1628,8 @@@ pub fn dyn_sign() 
        let _signer: Box<dyn EcdsaChannelSigner>;
  }
  
- #[cfg(all(test, feature = "_bench_unstable", not(feature = "no-std")))]
- mod benches {
+ #[cfg(ldk_bench)]
pub mod benches {
        use std::sync::{Arc, mpsc};
        use std::sync::mpsc::TryRecvError;
        use std::thread;
        use bitcoin::Network;
        use crate::sign::{EntropySource, KeysManager};
  
-       use test::Bencher;
+       use criterion::Criterion;
  
-       #[bench]
-       fn bench_get_secure_random_bytes(bench: &mut Bencher) {
+       pub fn bench_get_secure_random_bytes(bench: &mut Criterion) {
                let seed = [0u8; 32];
                let now = Duration::from_secs(genesis_block(Network::Testnet).header.time as u64);
                let keys_manager = Arc::new(KeysManager::new(&seed, now.as_secs(), now.subsec_micros()));
                        stops.push(stop_sender);
                }
  
-               bench.iter(|| {
-                       for _ in 1..100 {
-                               keys_manager.get_secure_random_bytes();
-                       }
-               });
+               bench.bench_function("get_secure_random_bytes", |b| b.iter(||
+                       keys_manager.get_secure_random_bytes()));
  
                for stop in stops {
                        let _ = stop.send(());
                        handle.join().unwrap();
                }
        }
  }