diff --git a/Cargo.lock b/Cargo.lock index 02b8fc58..7d057c72 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -56,6 +56,12 @@ version = "0.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" +[[package]] +name = "base64" +version = "0.21.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35636a1494ede3b646cc98f74f8e62c773a38a659ebc777a2cf26b9b74171df9" + [[package]] name = "bdk_coin_select" version = "0.1.1" @@ -64,9 +70,9 @@ checksum = "c0320167c3655e83f0415d52f39618902e449186ffc7dfb090f922f79675c316" [[package]] name = "bech32" -version = "0.9.1" +version = "0.10.0-beta" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d86b93f97252c47b41663388e6d155714a9d0c398b99f1005cbc5f978b29f445" +checksum = "98f7eed2b2781a6f0b5c903471d48e15f56fb4e1165df8a9a2337fd1a59d45ea" [[package]] name = "bip39" @@ -81,19 +87,29 @@ dependencies = [ [[package]] name = "bitcoin" -version = "0.30.2" +version = "0.31.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1945a5048598e4189e239d3f809b19bdad4845c4b2ba400d304d2dcf26d2c462" +checksum = "5973a027b341b462105675962214dfe3c938ad9afd395d84b28602608bdcec7b" dependencies = [ - "base64", + "base64 0.21.5", "bech32", - "bitcoin-private", - "bitcoin_hashes 0.12.0", + "bitcoin-internals", + "bitcoin_hashes 0.13.0", + "hex-conservative", "hex_lit", "secp256k1", "serde", ] +[[package]] +name = "bitcoin-internals" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9425c3bf7089c983facbae04de54513cce73b41c7f9ff8c845b54e7bc64ebbfb" +dependencies = [ + "serde", +] + [[package]] name = "bitcoin-private" version = "0.1.0" @@ -113,6 +129,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d7066118b13d4b20b23645932dfb3a81ce7e29f95726c2036fa33cd7b092501" dependencies = [ "bitcoin-private", +] + +[[package]] +name = "bitcoin_hashes" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1930a4dabfebb8d7d9992db18ebe3ae2876f0a305fab206fd168df931ede293b" +dependencies = [ + "bitcoin-internals", + "hex-conservative", "serde", ] @@ -221,6 +247,12 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "hex-conservative" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30ed443af458ccb6d81c1e7e661545f94d3176752fb1df2f543b902a1e0f51e2" + [[package]] name = "hex_lit" version = "0.1.1" @@ -239,7 +271,7 @@ version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a26d9104d516092f092d97448787505881fdb6518293b2d6500bf9c180c839dd" dependencies = [ - "base64", + "base64 0.13.1", "minreq", "serde", "serde_json", @@ -308,12 +340,13 @@ checksum = "523dc4f511e55ab87b694dc30d0f820d60906ef06413f93d4d7a1385599cc149" [[package]] name = "miniscript" -version = "10.0.0" +version = "11.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1eb102b66b2127a872dbcc73095b7b47aeb9d92f7b03c2b2298253ffc82c7594" +checksum = "86a23dd3ad145a980e231185d114399f25a0a307d2cd918010ddda6334323df9" dependencies = [ + "bech32", "bitcoin", - "bitcoin-private", + "bitcoin-internals", "serde", ] @@ -448,9 +481,9 @@ checksum = "f98d2aa92eebf49b69786be48e4477826b256916e84a57ff2a4f21923b48eb4c" [[package]] name = "secp256k1" -version = "0.27.0" +version = "0.28.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25996b82292a7a57ed3508f052cfff8640d38d32018784acd714758b43da9c8f" +checksum = "2acea373acb8c21ecb5a23741452acd2593ed44ee3d343e72baaa143bc89d0d5" dependencies = [ "bitcoin_hashes 0.12.0", "secp256k1-sys", @@ -459,9 +492,9 @@ dependencies = [ [[package]] name = "secp256k1-sys" -version = "0.8.1" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "70a129b9e9efbfb223753b9163c4ab3b13cff7fd9c7f010fbac25ab4099fa07e" +checksum = "4dd97a086ec737e30053fd5c46f097465d25bb81dd3608825f65298c4c98be83" dependencies = [ "cc", ] diff --git a/Cargo.toml b/Cargo.toml index cd63dd83..21c21949 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ nonblocking_shutdown = [] [dependencies] # For managing transactions (it re-exports the bitcoin crate) -miniscript = { version = "10.0", features = ["serde", "compiler", "base64"] } +miniscript = { version = "11.0", features = ["serde", "compiler", "base64"] } # Coin selection algorithms for spend transaction creation. bdk_coin_select = { version = "0.1.0" } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 12343d37..83d8ec2c 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -29,7 +29,7 @@ use std::{ }; use miniscript::{ - bitcoin::{self, address, bip32, psbt::PartiallySignedTransaction as Psbt}, + bitcoin::{self, address, bip32, psbt::Psbt}, psbt::PsbtExt, }; use serde::{Deserialize, Serialize}; @@ -609,7 +609,8 @@ impl DaemonControl { // Then, broadcast it (or try to, we never know if we are not going to hit an // error at broadcast time). - let final_tx = spend_psbt.extract_tx(); + // These checks are already performed at Spend creation time. TODO: a belt-and-suspenders is still worth it though. + let final_tx = spend_psbt.extract_tx_unchecked_fee_rate(); self.bitcoin .broadcast_tx(&final_tx) .map_err(CommandError::TxBroadcast) @@ -729,7 +730,7 @@ impl DaemonControl { .expect("address already used in finalized transaction"); ( address.clone(), - bitcoin::Amount::from_sat(txo.value), + txo.value, db_conn.derivation_index_by_address(&address), ) }) @@ -1138,9 +1139,9 @@ mod tests { use bitcoin::{ bip32::{self, ChildNumber}, - blockdata::transaction::{TxIn, TxOut}, + blockdata::transaction::{TxIn, TxOut, Version as TxVersion}, locktime::absolute, - OutPoint, ScriptBuf, Sequence, Transaction, Txid, Witness, + Amount, OutPoint, ScriptBuf, Sequence, Transaction, Txid, Witness, }; use std::{collections::BTreeMap, str::FromStr}; @@ -1300,7 +1301,7 @@ mod tests { dummy_op.txid, ( bitcoin::Transaction { - version: 2, + version: TxVersion::TWO, lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), input: vec![], output: vec![], @@ -1371,18 +1372,24 @@ mod tests { assert_eq!(tx.output.len(), 2); assert_eq!( tx.output[0].script_pubkey, - dummy_addr.payload.script_pubkey() + dummy_addr.payload().script_pubkey() ); - assert_eq!(tx.output[0].value, dummy_value); + assert_eq!(tx.output[0].value.to_sat(), dummy_value); + + // NOTE: if you are wondering about the usefulness of these tests asserting arbitrary fixed + // values, that's a belt-and-suspenders check to make sure size and fee calculations do not + // change unexpectedly. For instance this specific test caught how a change in + // rust-bitcoin's serialization of transactions with no input silently affected our fee + // calculation. // Transaction is 1 in (P2WSH satisfaction), 2 outs. At 1sat/vb, it's 170 sats fees. // At 2sats/vb, it's twice that. - assert_eq!(tx.output[1].value, 89_830); + assert_eq!(tx.output[1].value.to_sat(), 89_830); let res = control .create_spend(&destinations, &[dummy_op], 2, None) .unwrap(); let tx = res.psbt.unsigned_tx; - assert_eq!(tx.output[1].value, 89_660); + assert_eq!(tx.output[1].value.to_sat(), 89_660); // A feerate of 555 won't trigger the sanity checks (they were previously not taking the // satisfaction size into account and overestimating the feerate). @@ -1414,7 +1421,7 @@ mod tests { // If we ask to create an output for an address from another network, it will fail. let invalid_addr = - bitcoin::Address::new(bitcoin::Network::Testnet, dummy_addr.payload.clone()); + bitcoin::Address::new(bitcoin::Network::Testnet, dummy_addr.payload().clone()); let invalid_destinations: HashMap, u64> = [(invalid_addr, dummy_value)].iter().cloned().collect(); assert!(matches!( @@ -1436,9 +1443,9 @@ mod tests { assert_eq!(tx.output.len(), 1); assert_eq!( tx.output[0].script_pubkey, - dummy_addr.payload.script_pubkey() + dummy_addr.payload().script_pubkey() ); - assert_eq!(tx.output[0].value, 95_000); + assert_eq!(tx.output[0].value.to_sat(), 95_000); // Now if we mark the coin as spent, we won't create another Spend transaction containing // it. @@ -1555,9 +1562,9 @@ mod tests { assert_eq!(tx_auto.output.len(), 2); assert_eq!( tx_auto.output[0].script_pubkey, - dummy_addr.payload.script_pubkey() + dummy_addr.payload().script_pubkey() ); - assert_eq!(tx_auto.output[0].value, 80_000); + assert_eq!(tx_auto.output[0].value, Amount::from_sat(80_000)); // Create a second transaction using manual coin selection. let res_manual = control @@ -1648,7 +1655,7 @@ mod tests { .unwrap(); let mut dummy_bitcoind = DummyBitcoind::new(); let dummy_tx = bitcoin::Transaction { - version: 2, + version: TxVersion::TWO, lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), input: vec![], output: vec![], @@ -1770,7 +1777,7 @@ mod tests { let mut dummy_bitcoind = DummyBitcoind::new(); // Transaction spends outpoint a. let dummy_tx_a = bitcoin::Transaction { - version: 2, + version: TxVersion::TWO, lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), input: vec![bitcoin::TxIn { previous_output: dummy_op_a, @@ -1845,7 +1852,7 @@ mod tests { ); let deposit1: Transaction = Transaction { - version: 1, + version: TxVersion::ONE, lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), @@ -1855,12 +1862,12 @@ mod tests { }], output: vec![TxOut { script_pubkey: ScriptBuf::new(), - value: 100_000_000, + value: Amount::from_sat(100_000_000), }], }; let deposit2: Transaction = Transaction { - version: 1, + version: TxVersion::ONE, lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), @@ -1870,12 +1877,12 @@ mod tests { }], output: vec![TxOut { script_pubkey: ScriptBuf::new(), - value: 2000, + value: Amount::from_sat(2000), }], }; let deposit3: Transaction = Transaction { - version: 1, + version: TxVersion::ONE, lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), @@ -1885,12 +1892,12 @@ mod tests { }], output: vec![TxOut { script_pubkey: ScriptBuf::new(), - value: 3000, + value: Amount::from_sat(3000), }], }; let spend_tx: Transaction = Transaction { - version: 1, + version: TxVersion::ONE, lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), @@ -1904,11 +1911,11 @@ mod tests { output: vec![ TxOut { script_pubkey: ScriptBuf::new(), - value: 4000, + value: Amount::from_sat(4000), }, TxOut { script_pubkey: ScriptBuf::new(), - value: 100_000_000 - 4000 - 1000, + value: Amount::from_sat(100_000_000 - 4000 - 1000), }, ], }; @@ -2072,7 +2079,7 @@ mod tests { ); let tx1: Transaction = Transaction { - version: 1, + version: TxVersion::ONE, lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), @@ -2082,12 +2089,12 @@ mod tests { }], output: vec![TxOut { script_pubkey: ScriptBuf::new(), - value: 100_000_000, + value: Amount::from_sat(100_000_000), }], }; let tx2: Transaction = Transaction { - version: 1, + version: TxVersion::ONE, lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), @@ -2097,12 +2104,12 @@ mod tests { }], output: vec![TxOut { script_pubkey: ScriptBuf::new(), - value: 2000, + value: Amount::from_sat(2000), }], }; let tx3: Transaction = Transaction { - version: 1, + version: TxVersion::ONE, lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), @@ -2112,7 +2119,7 @@ mod tests { }], output: vec![TxOut { script_pubkey: ScriptBuf::new(), - value: 3000, + value: Amount::from_sat(3000), }], }; diff --git a/src/database/mod.rs b/src/database/mod.rs index 375d8ad0..9c2be7c6 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -20,7 +20,7 @@ use std::{ sync, }; -use miniscript::bitcoin::{self, bip32, psbt::PartiallySignedTransaction as Psbt, secp256k1}; +use miniscript::bitcoin::{self, bip32, psbt::Psbt, secp256k1}; pub trait DatabaseInterface: Send { fn connection(&self) -> Box; diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index b51b6a30..3654cc4b 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -39,7 +39,7 @@ use miniscript::bitcoin::{ self, bip32, consensus::encode, hashes::{sha256, Hash}, - psbt::PartiallySignedTransaction as Psbt, + psbt::Psbt, secp256k1, }; diff --git a/src/database/sqlite/schema.rs b/src/database/sqlite/schema.rs index 7ebb280b..6add975a 100644 --- a/src/database/sqlite/schema.rs +++ b/src/database/sqlite/schema.rs @@ -2,9 +2,7 @@ use crate::descriptors::LianaDescriptor; use std::{convert::TryFrom, str::FromStr}; -use miniscript::bitcoin::{ - self, address, bip32, consensus::encode, psbt::PartiallySignedTransaction as Psbt, -}; +use miniscript::bitcoin::{self, address, bip32, consensus::encode, psbt::Psbt}; pub const SCHEMA: &str = "\ CREATE TABLE version ( diff --git a/src/descriptors/analysis.rs b/src/descriptors/analysis.rs index b106a068..499a4d7e 100644 --- a/src/descriptors/analysis.rs +++ b/src/descriptors/analysis.rs @@ -72,7 +72,7 @@ fn is_single_key_or_multisig(policy: &SemanticPolicy, + keys_set: HashSet<(bip32::Xpub, descriptor::DerivPaths)>, } impl DescKeyChecker { @@ -347,7 +347,9 @@ impl PathInfo { PathInfo::Single(key) => ConcretePolicy::Key(key), PathInfo::Multi(thresh, keys) => ConcretePolicy::Threshold( thresh, - keys.into_iter().map(ConcretePolicy::Key).collect(), + keys.into_iter() + .map(|key| ConcretePolicy::Key(key).into()) + .collect(), ), } } @@ -531,9 +533,9 @@ impl LianaPolicy { .fold(primary_keys, |tl_policy, (timelock, path_info)| { let timelock = ConcretePolicy::Older(Sequence::from_height(timelock)); let keys = path_info.into_ms_policy(); - let recovery_branch = ConcretePolicy::And(vec![keys, timelock]); + let recovery_branch = ConcretePolicy::And(vec![keys.into(), timelock.into()]); // We assume the larger the timelock the less likely a branch would be used. - ConcretePolicy::Or(vec![(99, tl_policy), (1, recovery_branch)]) + ConcretePolicy::Or(vec![(99, tl_policy.into()), (1, recovery_branch.into())]) }); tl_policy diff --git a/src/descriptors/mod.rs b/src/descriptors/mod.rs index b822ac82..06f46b79 100644 --- a/src/descriptors/mod.rs +++ b/src/descriptors/mod.rs @@ -786,10 +786,10 @@ mod tests { // NOTE: The specific value is asserted because this was tested against a regtest // transaction. let stack = vec![vec![0; 65], vec![0; 0], vec![0; 33], vec![0; 72]]; - let witness_size = bitcoin::VarInt(stack.len() as u64).len() + let witness_size = bitcoin::VarInt(stack.len() as u64).size() + stack .iter() - .map(|item| bitcoin::VarInt(item.len() as u64).len() + item.len()) + .map(|item| bitcoin::VarInt(item.len() as u64).size() + item.len()) .sum::(); assert_eq!( desc.spender_input_size(), diff --git a/src/jsonrpc/api.rs b/src/jsonrpc/api.rs index 403563cf..f99ee9ef 100644 --- a/src/jsonrpc/api.rs +++ b/src/jsonrpc/api.rs @@ -10,7 +10,7 @@ use std::{ str::FromStr, }; -use miniscript::bitcoin::{self, psbt::PartiallySignedTransaction as Psbt}; +use miniscript::bitcoin::{self, psbt::Psbt}; fn create_spend(control: &DaemonControl, params: Params) -> Result { let destinations = params diff --git a/src/signer.rs b/src/signer.rs index 495a6bb5..e074e233 100644 --- a/src/signer.rs +++ b/src/signer.rs @@ -58,7 +58,7 @@ pub const MNEMONICS_FOLDER_NAME: &str = "mnemonics"; /// A signer that keeps the key on the laptop. Based on BIP39. pub struct HotSigner { mnemonic: bip39::Mnemonic, - master_xpriv: bip32::ExtendedPrivKey, + master_xpriv: bip32::Xpriv, } // TODO: instead of copying them here we could have a util module with those helpers. @@ -103,8 +103,8 @@ impl HotSigner { network: bitcoin::Network, mnemonic: bip39::Mnemonic, ) -> Result { - let master_xpriv = bip32::ExtendedPrivKey::new_master(network, &mnemonic.to_seed("")) - .map_err(SignerError::Bip32)?; + let master_xpriv = + bip32::Xpriv::new_master(network, &mnemonic.to_seed("")).map_err(SignerError::Bip32)?; Ok(Self { mnemonic, master_xpriv, @@ -212,7 +212,7 @@ impl HotSigner { &self, der_path: &bip32::DerivationPath, secp: &secp256k1::Secp256k1, - ) -> bip32::ExtendedPrivKey { + ) -> bip32::Xpriv { self.master_xpriv .derive_priv(secp, der_path) .expect("Never fails") @@ -223,9 +223,9 @@ impl HotSigner { &self, der_path: &bip32::DerivationPath, secp: &secp256k1::Secp256k1, - ) -> bip32::ExtendedPubKey { + ) -> bip32::Xpub { let xpriv = self.xpriv_at(der_path, secp); - bip32::ExtendedPubKey::from_priv(secp, &xpriv) + bip32::Xpub::from_priv(secp, &xpriv) } /// Sign all inputs of the given PSBT. @@ -254,9 +254,9 @@ impl HotSigner { .value; let sig_type = sighash::EcdsaSighashType::All; let sighash = sighash_cache - .segwit_signature_hash(i, witscript, value, sig_type) + .p2wsh_signature_hash(i, witscript, value, sig_type) .map_err(|_| SignerError::InsanePsbt)?; - let sighash = secp256k1::Message::from_slice(sighash.as_byte_array()) + let sighash = secp256k1::Message::from_digest_slice(sighash.as_byte_array()) .expect("Sighash is always 32 bytes."); // Then provide a signature for all the keys they asked for. @@ -297,7 +297,7 @@ mod tests { use super::*; use crate::{descriptors, testutils::*}; use miniscript::{ - bitcoin::{locktime::absolute, psbt::Input as PsbtIn}, + bitcoin::{locktime::absolute, psbt::Input as PsbtIn, Amount}, descriptor::{DerivPaths, DescriptorMultiXKey, DescriptorPublicKey, Wildcard}, }; use std::collections::{BTreeMap, HashSet}; @@ -429,7 +429,7 @@ mod tests { let spent_coin_desc = desc.receive_descriptor().derive(42.into(), &secp); let mut dummy_psbt = Psbt { unsigned_tx: bitcoin::Transaction { - version: 2, + version: bitcoin::transaction::Version::TWO, lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), input: vec![bitcoin::TxIn { sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME, @@ -440,12 +440,12 @@ mod tests { ..bitcoin::TxIn::default() }], output: vec![bitcoin::TxOut { - value: 18_420, + value: Amount::from_sat(18_420), script_pubkey: bitcoin::Address::from_str( "bc1qvklensptw5lk7d470ds60pcpsr0psdpgyvwepv", ) .unwrap() - .payload + .payload() .script_pubkey(), }], }, @@ -457,7 +457,7 @@ mod tests { witness_script: Some(spent_coin_desc.witness_script()), bip32_derivation: spent_coin_desc.bip32_derivations(), witness_utxo: Some(bitcoin::TxOut { - value: 19_000, + value: Amount::from_sat(19_000), script_pubkey: spent_coin_desc.script_pubkey(), }), ..PsbtIn::default() @@ -479,10 +479,7 @@ mod tests { // We can add another external output to the transaction, we can still sign without issue. // The output can be insane, we don't check it. It doesn't even need an accompanying PSBT // output. - dummy_psbt - .unsigned_tx - .output - .push(bitcoin::TxOut::default()); + dummy_psbt.unsigned_tx.output.push(bitcoin::TxOut::NULL); let psbt = dummy_psbt.clone(); assert!(psbt.inputs[0].partial_sigs.is_empty()); let psbt = prim_signer_a.sign_psbt(psbt, &secp).unwrap(); @@ -499,7 +496,7 @@ mod tests { witness_script: Some(other_spent_coin_desc.witness_script()), bip32_derivation: other_spent_coin_desc.bip32_derivations(), witness_utxo: Some(bitcoin::TxOut { - value: 19_000, + value: Amount::from_sat(19_000), script_pubkey: other_spent_coin_desc.script_pubkey(), }), ..PsbtIn::default() diff --git a/src/spend.rs b/src/spend.rs index 9a308521..091839f7 100644 --- a/src/spend.rs +++ b/src/spend.rs @@ -23,7 +23,7 @@ pub const DUST_OUTPUT_SATS: u64 = 5_000; pub const LONG_TERM_FEERATE_VB: f32 = 10.0; /// Assume that paying more than 1BTC in fee is a bug. -pub const MAX_FEE: u64 = bitcoin::blockdata::constants::COIN_VALUE; +pub const MAX_FEE: bitcoin::Amount = bitcoin::Amount::ONE_BTC; /// Assume that paying more than 1000sat/vb in feerate is a bug. pub const MAX_FEERATE: u64 = 1_000; @@ -53,7 +53,7 @@ impl fmt::Display for SpendCreationError { Self::InvalidOutputValue(amount) => write!(f, "Invalid output value '{}'.", amount), Self::InsaneFees(info) => write!( f, - "We assume transactions with a fee larger than {} sats or a feerate larger than {} sats/vb are a mistake. \ + "We assume transactions with a fee larger than {} or a feerate larger than {} sats/vb are a mistake. \ The created transaction {}.", MAX_FEE, MAX_FEERATE, @@ -81,10 +81,7 @@ impl std::error::Error for SpendCreationError {} // Sanity check the value of a transaction output. fn check_output_value(value: bitcoin::Amount) -> Result<(), SpendCreationError> { - // NOTE: the network parameter isn't used upstream - if value.to_sat() > bitcoin::blockdata::constants::MAX_MONEY - || value.to_sat() < DUST_OUTPUT_SATS - { + if value > bitcoin::Amount::MAX_MONEY || value.to_sat() < DUST_OUTPUT_SATS { Err(SpendCreationError::InvalidOutputValue(value)) } else { Ok(()) @@ -118,15 +115,16 @@ fn sanity_check_psbt( .witness_utxo .as_ref() .ok_or_else(|| SpendCreationError::SanityCheckFailure(psbt.clone()))? - .value; + .value + .to_sat(); } // Compute the output value and check the absolute fee isn't insane. - let value_out: u64 = tx.output.iter().map(|o| o.value).sum(); + let value_out: u64 = tx.output.iter().map(|o| o.value.to_sat()).sum(); let abs_fee = value_in .checked_sub(value_out) .ok_or(SpendCreationError::InsaneFees(InsaneFeeInfo::NegativeFee))?; - if abs_fee > MAX_FEE { + if abs_fee > MAX_FEE.to_sat() { return Err(SpendCreationError::InsaneFees(InsaneFeeInfo::TooHighFee( abs_fee, ))); @@ -147,7 +145,7 @@ fn sanity_check_psbt( // Check for dust outputs for txo in psbt.unsigned_tx.output.iter() { - if txo.value < txo.script_pubkey.dust_value().to_sat() { + if txo.value < txo.script_pubkey.dust_value() { return Err(SpendCreationError::SanityCheckFailure(psbt.clone())); } } @@ -257,16 +255,25 @@ fn select_coins_for_spend( max_sat_weight: u32, must_have_change: bool, ) -> Result<(Vec, bitcoin::Amount), InsufficientFunds> { - let out_value_nochange = base_tx.output.iter().map(|o| o.value).sum(); + let out_value_nochange = base_tx.output.iter().map(|o| o.value.to_sat()).sum(); // Create the coin selector from the given candidates. NOTE: the coin selector keeps track // of the original ordering of candidates so we can select any mandatory candidates using their // original indices. - let base_weight: u32 = base_tx + let mut base_weight: u32 = base_tx .weight() .to_wu() .try_into() .expect("Transaction weight must fit in u32"); + // Starting with version 0.31, rust-bitcoin now accounts for the segwit marker when serializing + // transactions with no input. But BDK's coin selector does add the segwit marker cost to the + // transaction size upon selecting the first segwit coin. To avoid accounting twice for it, + // drop it from the base weight (but only when it was added). + // NOTE: make sure to reconsider this when updating rust-bitcoin!! Behaviour may change again + // who knows. + if base_tx.input.is_empty() { + base_weight = base_weight.saturating_sub(2); + } let max_input_weight = TXIN_BASE_WEIGHT + max_sat_weight; let candidates: Vec = candidate_coins .iter() @@ -293,12 +300,15 @@ fn select_coins_for_spend( let long_term_feerate = FeeRate::from_sat_per_vb(LONG_TERM_FEERATE_VB); let drain_weights = DrainWeights { output_weight: { + // We don't reuse the above base_weight.since 2 WU may have been substracted from it. + // See comment above for details. + let nochange_weight = base_tx.weight().to_wu(); let mut tx_with_change = base_tx; tx_with_change.output.push(change_txo); tx_with_change .weight() .to_wu() - .checked_sub(base_weight.into()) + .checked_sub(nochange_weight) .expect("base_weight can't be larger") .try_into() .expect("tx size must always fit in u32") @@ -481,7 +491,7 @@ pub fn create_spend( // Create transaction with no inputs and no outputs. let mut tx = bitcoin::Transaction { - version: 2, + version: bitcoin::transaction::Version::TWO, lock_time: LockTime::Blocks(Height::ZERO), // TODO: randomized anti fee sniping input: Vec::with_capacity(candidate_coins.iter().filter(|c| c.must_select).count()), output: Vec::with_capacity(destinations.len()), @@ -493,7 +503,7 @@ pub fn create_spend( check_output_value(*amount)?; tx.output.push(bitcoin::TxOut { - value: amount.to_sat(), + value: *amount, script_pubkey: address.addr.script_pubkey(), }); // If it's an address of ours, signal it as change to signing devices by adding the @@ -520,7 +530,7 @@ pub fn create_spend( // We need to get the size of a potential change output to select coins / determine whether // we should include one, so get the change address and create a dummy txo for this purpose. let mut change_txo = bitcoin::TxOut { - value: std::u64::MAX, + value: bitcoin::Amount::MAX, script_pubkey: change_addr.addr.script_pubkey(), }; // Now select the coins necessary using the provided candidates and determine whether @@ -575,7 +585,7 @@ pub fn create_spend( }; // TODO: shuffle once we have Taproot - change_txo.value = change_amount.to_sat(); + change_txo.value = change_amount; tx.output.push(change_txo); psbt_outs.push(PsbtOut { bip32_derivation, @@ -600,7 +610,7 @@ pub fn create_spend( let coin_desc = derived_desc(secp, main_descriptor, cand); let witness_script = Some(coin_desc.witness_script()); let witness_utxo = Some(bitcoin::TxOut { - value: cand.amount.to_sat(), + value: cand.amount, script_pubkey: coin_desc.script_pubkey(), }); let non_witness_utxo = tx_getter.get_tx(&cand.outpoint.txid); diff --git a/src/testutils.rs b/src/testutils.rs index 3bd663b0..c175c156 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -13,9 +13,7 @@ use std::{ }; use miniscript::{ - bitcoin::{ - self, bip32, psbt::PartiallySignedTransaction as Psbt, secp256k1, Transaction, Txid, - }, + bitcoin::{self, bip32, psbt::Psbt, secp256k1, Transaction, Txid}, descriptor, };