From e28010915f4d784ba06a1947ddadaeb5addf6082 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 12 Jul 2023 10:48:55 +0200 Subject: [PATCH 1/5] lianad: update rust-miniscript (and rust-bitcoin) dependencies Rust-bitcoin, that we use through rust-miniscript, has seen plenty of breaking changes in the latest version. I've tried to keep the necessary changes here minimal, still it had to be a single commit to keep it hygienic. But i'll try to summarize the main things here. Tobin also wrote a guide about the release at https://rust-bitcoin.org/blog/release-0.30.0/. The most verbose change in this commit is probably due to the `Address` type overhaul. It's overengineered if you ask me but hey here we are. I tried to keep network validation in commands, and otherwise passing around unchecked addresses (to avoid having to pass around a global state between our various components). Another non-obvious change was changes in hash types upstream and the removal of `ToHex`, forcing us to get the hex representation of a txid through its `Display` implementation. It is however displayed backward in this case ("little-endian" if you will), and we need a regular hex encoding for some queries to the database. We needed to make sure we didn't implement any silent bug here. The rest (Script type changes, PSBT serialization updates, ..) is probably self-explanatory. --- Cargo.lock | 46 ++++++--- Cargo.toml | 2 +- src/bitcoin/d/mod.rs | 5 +- src/bitcoin/mod.rs | 4 +- src/bitcoin/poller/looper.rs | 23 +++-- src/commands/mod.rs | 176 ++++++++++++++++++---------------- src/commands/utils.rs | 48 ++++++---- src/database/mod.rs | 13 +-- src/database/sqlite/mod.rs | 67 ++++++++----- src/database/sqlite/schema.rs | 10 +- src/descriptors/analysis.rs | 2 +- src/descriptors/keys.rs | 3 +- src/descriptors/mod.rs | 16 ++-- src/jsonrpc/api.rs | 7 +- src/jsonrpc/mod.rs | 2 +- src/lib.rs | 4 + src/random.rs | 4 +- src/signer.rs | 22 ++--- src/testutils.rs | 4 +- 19 files changed, 263 insertions(+), 195 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 337e954a8..9824ccc9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -70,29 +70,45 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "93f2635620bf0b9d4576eb7bb9a38a55df78bd1205d26fa994b25911a69f212f" dependencies = [ - "bitcoin_hashes", + "bitcoin_hashes 0.11.0", "serde", "unicode-normalization", ] [[package]] name = "bitcoin" -version = "0.29.2" +version = "0.30.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0694ea59225b0c5f3cb405ff3f670e4828358ed26aec49dc352f730f0cb1a8a3" +checksum = "b36f4c848f6bd9ff208128f08751135846cc23ae57d66ab10a22efff1c675f3c" dependencies = [ + "base64", "bech32", - "bitcoin_hashes", + "bitcoin-private", + "bitcoin_hashes 0.12.0", + "hex_lit", "secp256k1", "serde", ] +[[package]] +name = "bitcoin-private" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73290177011694f38ec25e165d0387ab7ea749a4b81cd4c80dae5988229f7a57" + [[package]] name = "bitcoin_hashes" version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90064b8dee6815a6470d60bad07bbbaee885c0e12d04177138fa3291a01b7bc4" + +[[package]] +name = "bitcoin_hashes" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d7066118b13d4b20b23645932dfb3a81ce7e29f95726c2036fa33cd7b092501" dependencies = [ + "bitcoin-private", "serde", ] @@ -196,6 +212,12 @@ dependencies = [ "hashbrown", ] +[[package]] +name = "hex_lit" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3011d1213f159867b13cfd6ac92d2cd5f1345762c63be3554e84092d85a50bbd" + [[package]] name = "itoa" version = "1.0.4" @@ -269,10 +291,12 @@ checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" [[package]] name = "miniscript" -version = "9.0.0" -source = "git+https://github.com/darosior/rust-miniscript?branch=multipath_descriptors_on_9.0#3104519501ce6ad15b36dcec759936f4d3bd3980" +version = "10.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1eb102b66b2127a872dbcc73095b7b47aeb9d92f7b03c2b2298253ffc82c7594" dependencies = [ "bitcoin", + "bitcoin-private", "serde", ] @@ -391,20 +415,20 @@ checksum = "4501abdff3ae82a1c1b477a17252eb69cee9e66eb915c1abaa4f44d873df9f09" [[package]] name = "secp256k1" -version = "0.24.0" +version = "0.27.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7649a0b3ffb32636e60c7ce0d70511eda9c52c658cd0634e194d5a19943aeff" +checksum = "25996b82292a7a57ed3508f052cfff8640d38d32018784acd714758b43da9c8f" dependencies = [ - "bitcoin_hashes", + "bitcoin_hashes 0.12.0", "secp256k1-sys", "serde", ] [[package]] name = "secp256k1-sys" -version = "0.6.1" +version = "0.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83080e2c2fc1006e625be82e5d1eb6a43b7fd9578b617fcc55814daf286bba4b" +checksum = "70a129b9e9efbfb223753b9163c4ab3b13cff7fd9c7f010fbac25ab4099fa07e" dependencies = [ "cc", ] diff --git a/Cargo.toml b/Cargo.toml index 155524d7c..e308ae944 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,7 @@ daemon = ["libc"] [dependencies] # For managing transactions (it re-exports the bitcoin crate) -miniscript = { git = "https://github.com/darosior/rust-miniscript", branch = "multipath_descriptors_on_9.0", features = ["serde", "compiler"] } +miniscript = { version = "10.0", features = ["serde", "compiler", "base64"] } # Don't reinvent the wheel dirs = "5.0" diff --git a/src/bitcoin/d/mod.rs b/src/bitcoin/d/mod.rs index 8ed12a0a4..798694ffb 100644 --- a/src/bitcoin/d/mod.rs +++ b/src/bitcoin/d/mod.rs @@ -27,7 +27,7 @@ use jsonrpc::{ }; use miniscript::{ - bitcoin::{self, hashes::hex::FromHex}, + bitcoin::{self, address, hashes::hex::FromHex}, descriptor, }; @@ -651,6 +651,7 @@ impl BitcoinD { bitcoin::Network::Testnet => "test", bitcoin::Network::Regtest => "regtest", bitcoin::Network::Signet => "signet", + _ => "Unknown network, undefined at the time of writing", }; if bitcoind_net != bip70_net { return Err(BitcoindError::NetworkMismatch( @@ -1072,7 +1073,7 @@ pub struct LSBlockEntry { pub outpoint: bitcoin::OutPoint, pub amount: bitcoin::Amount, pub block_height: Option, - pub address: bitcoin::Address, + pub address: bitcoin::Address, pub parent_descs: Vec>, } diff --git a/src/bitcoin/mod.rs b/src/bitcoin/mod.rs index 2c9e5e7b4..a3b9e01a4 100644 --- a/src/bitcoin/mod.rs +++ b/src/bitcoin/mod.rs @@ -12,7 +12,7 @@ use crate::{ use std::{fmt, sync}; -use miniscript::bitcoin; +use miniscript::bitcoin::{self, address}; /// Information about a block #[derive(Debug, Clone, Eq, PartialEq, Copy)] @@ -408,5 +408,5 @@ pub struct UTxO { pub outpoint: bitcoin::OutPoint, pub amount: bitcoin::Amount, pub block_height: Option, - pub address: bitcoin::Address, + pub address: bitcoin::Address, } diff --git a/src/bitcoin/poller/looper.rs b/src/bitcoin/poller/looper.rs index 04038f2de..c3f246d83 100644 --- a/src/bitcoin/poller/looper.rs +++ b/src/bitcoin/poller/looper.rs @@ -31,16 +31,28 @@ fn update_coins( descs: &[descriptors::SinglePathLianaDesc], secp: &secp256k1::Secp256k1, ) -> UpdatedCoins { + let network = db_conn.network(); let curr_coins = db_conn.coins(CoinType::All); log::debug!("Current coins: {:?}", curr_coins); // Start by fetching newly received coins. let mut received = Vec::new(); for utxo in bit.received_coins(previous_tip, descs) { + let UTxO { + outpoint, + amount, + address, + .. + } = utxo; // We can only really treat them if we know the derivation index that was used. - if let Some((derivation_index, is_change)) = - db_conn.derivation_index_by_address(&utxo.address) - { + let address = match address.require_network(network) { + Ok(addr) => addr, + Err(e) => { + log::error!("Invalid network for address: {}", e); + continue; + } + }; + if let Some((derivation_index, is_change)) = db_conn.derivation_index_by_address(&address) { // First of if we are receiving coins that are beyond our next derivation index, // adjust it. if derivation_index > db_conn.receive_index() { @@ -52,9 +64,6 @@ fn update_coins( // Now record this coin as a newly received one. if !curr_coins.contains_key(&utxo.outpoint) { - let UTxO { - outpoint, amount, .. - } = utxo; let coin = Coin { outpoint, amount, @@ -71,7 +80,7 @@ fn update_coins( log::error!( "Could not get derivation index for coin '{}' (address: '{}')", &utxo.outpoint, - &utxo.address + &address ); } } diff --git a/src/commands/mod.rs b/src/commands/mod.rs index f125bf709..fba9961a8 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -11,8 +11,8 @@ use crate::{ }; use utils::{ - deser_amount_from_sats, deser_base64, deser_hex, ser_amount, ser_base64, ser_hex, - to_base64_string, + deser_addr_assume_checked, deser_amount_from_sats, deser_fromstr, deser_hex, ser_amount, + ser_hex, ser_to_string, }; use std::{ @@ -23,8 +23,9 @@ use std::{ use miniscript::{ bitcoin::{ - self, - util::psbt::{Input as PsbtIn, Output as PsbtOut, PartiallySignedTransaction as Psbt}, + self, address, + locktime::absolute, + psbt::{Input as PsbtIn, Output as PsbtOut, PartiallySignedTransaction as Psbt}, }, psbt::PsbtExt, }; @@ -49,7 +50,7 @@ pub enum CommandError { InvalidFeerate(/* sats/vb */ u64), UnknownOutpoint(bitcoin::OutPoint), AlreadySpent(bitcoin::OutPoint), - AddressNetwork(bitcoin::Address, /* Expected */ bitcoin::Network), + Address(bitcoin::address::Error), InvalidOutputValue(bitcoin::Amount), InsufficientFunds( /* in value */ bitcoin::Amount, @@ -77,10 +78,9 @@ impl fmt::Display for CommandError { Self::InvalidFeerate(sats_vb) => write!(f, "Invalid feerate: {} sats/vb.", sats_vb), Self::AlreadySpent(op) => write!(f, "Coin at '{}' is already spent.", op), Self::UnknownOutpoint(op) => write!(f, "Unknown outpoint '{}'.", op), - Self::AddressNetwork(addr, expected) => write!( + Self::Address(e) => write!( f, - "Invalid network for address '{}'. Our network is '{}' but address is for '{}'.", - addr, expected, addr.network + "Address error: {}", e ), Self::InvalidOutputValue(amount) => write!(f, "Invalid output value '{}'.", amount), Self::InsufficientFunds(in_val, out_val, feerate) => if let Some(out_val) = out_val { @@ -115,7 +115,7 @@ impl fmt::Display for CommandError { Self::SanityCheckFailure(psbt) => write!( f, "BUG! Please report this. Failed sanity checks for PSBT '{}'.", - to_base64_string(psbt) + psbt ), Self::UnknownSpend(txid) => write!(f, "Unknown spend transaction '{}'.", txid), Self::SpendFinalization(e) => { @@ -141,7 +141,7 @@ impl std::error::Error for CommandError {} // Sanity check the value of a transaction output. fn check_output_value(value: bitcoin::Amount) -> Result<(), CommandError> { // NOTE: the network parameter isn't used upstream - if value.to_sat() > bitcoin::blockdata::constants::max_money(bitcoin::Network::Bitcoin) + if value.to_sat() > bitcoin::blockdata::constants::MAX_MONEY || value.to_sat() < DUST_OUTPUT_SATS { Err(CommandError::InvalidOutputValue(value)) @@ -235,19 +235,14 @@ impl DaemonControl { } // Check whether this address is valid for the network we are operating on. - fn validate_address(&self, addr: &bitcoin::Address) -> Result<(), CommandError> { - // NOTE: signet uses testnet addresses - if addr.network == self.config.bitcoin_config.network - || (addr.network == bitcoin::Network::Testnet - && self.config.bitcoin_config.network == bitcoin::Network::Signet) - { - return Ok(()); - } - - Err(CommandError::AddressNetwork( - addr.clone(), - self.config.bitcoin_config.network, - )) + fn validate_address( + &self, + addr: bitcoin::Address, + ) -> Result { + // NOTE: signet uses testnet addresses, and legacy addresses on regtest use testnet + // encoding. + addr.require_network(self.config.bitcoin_config.network) + .map_err(CommandError::Address) } } @@ -287,7 +282,7 @@ impl DaemonControl { .receive_descriptor() .derive(index, &self.secp) .address(self.config.bitcoin_config.network); - GetAddressResult { address } + GetAddressResult::new(address) } /// Get a list of all known coins. @@ -323,7 +318,7 @@ impl DaemonControl { pub fn create_spend( &self, - destinations: &HashMap, + destinations: &HashMap, u64>, coins_outpoints: &[bitcoin::OutPoint], feerate_vb: u64, ) -> Result { @@ -396,7 +391,7 @@ impl DaemonControl { let mut txouts = Vec::with_capacity(destinations.len()); let mut psbt_outs = Vec::with_capacity(destinations.len()); for (address, value_sat) in destinations { - self.validate_address(address)?; + let address = self.validate_address(address.clone())?; let amount = bitcoin::Amount::from_sat(*value_sat); check_output_value(amount)?; @@ -409,7 +404,7 @@ impl DaemonControl { // If it's an address of ours, signal it as change to signing devices by adding the // BIP32 derivation path to the PSBT output. let bip32_derivation = - if let Some((index, is_change)) = db_conn.derivation_index_by_address(address) { + if let Some((index, is_change)) = db_conn.derivation_index_by_address(&address) { let desc = if is_change { self.config.main_descriptor.change_descriptor() } else { @@ -430,7 +425,7 @@ impl DaemonControl { // isn't much less than what was asked (and obviously that fees aren't negative). let mut tx = bitcoin::Transaction { version: 2, - lock_time: bitcoin::PackedLockTime(0), // TODO: randomized anti fee sniping + lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), // TODO: randomized anti fee sniping input: txins, output: txouts, }; @@ -691,21 +686,21 @@ impl DaemonControl { /// Note that not all coins may be spendable through a single recovery path at the same time. pub fn create_recovery( &self, - address: bitcoin::Address, + address: bitcoin::Address, feerate_vb: u64, timelock: Option, ) -> Result { if feerate_vb < 1 { return Err(CommandError::InvalidFeerate(feerate_vb)); } - self.validate_address(&address)?; + let address = self.validate_address(address)?; let mut db_conn = self.db.connection(); // The transaction template. We'll fill-in the inputs afterward. let mut psbt = Psbt { unsigned_tx: bitcoin::Transaction { version: 2, - lock_time: bitcoin::PackedLockTime(0), // TODO: anti-fee sniping + lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), // TODO: anti-fee sniping input: Vec::new(), output: vec![bitcoin::TxOut { script_pubkey: address.script_pubkey(), @@ -813,7 +808,18 @@ pub struct GetInfoResult { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct GetAddressResult { - pub address: bitcoin::Address, + #[serde(deserialize_with = "deser_addr_assume_checked")] + address: bitcoin::Address, +} + +impl GetAddressResult { + pub fn new(address: bitcoin::Address) -> Self { + Self { address } + } + + pub fn address(&self) -> &bitcoin::Address { + &self.address + } } #[derive(Debug, Clone, Copy, Serialize, Deserialize)] @@ -843,13 +849,13 @@ pub struct ListCoinsResult { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct CreateSpendResult { - #[serde(serialize_with = "ser_base64", deserialize_with = "deser_base64")] + #[serde(serialize_with = "ser_to_string", deserialize_with = "deser_fromstr")] pub psbt: Psbt, } #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ListSpendEntry { - #[serde(serialize_with = "ser_base64", deserialize_with = "deser_base64")] + #[serde(serialize_with = "ser_to_string", deserialize_with = "deser_fromstr")] pub psbt: Psbt, pub updated_at: Option, } @@ -874,7 +880,7 @@ pub struct TransactionInfo { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct CreateRecoveryResult { - #[serde(serialize_with = "ser_base64", deserialize_with = "deser_base64")] + #[serde(serialize_with = "ser_to_string", deserialize_with = "deser_fromstr")] pub psbt: Psbt, } @@ -884,14 +890,13 @@ mod tests { use crate::{bitcoin::Block, database::BlockInfo, testutils::*}; use bitcoin::{ + bip32::{self, ChildNumber}, blockdata::transaction::{TxIn, TxOut}, - util::bip32::ChildNumber, - OutPoint, PackedLockTime, Script, Sequence, Transaction, Txid, Witness, + locktime::absolute, + OutPoint, ScriptBuf, Sequence, Transaction, Txid, Witness, }; use std::str::FromStr; - use bitcoin::util::bip32; - #[test] fn getinfo() { let ms = DummyLiana::new(DummyBitcoind::new(), DummyDatabase::new()); @@ -913,6 +918,7 @@ mod tests { "bc1q9ksrc647hx8zp2cewl8p5f487dgux3777yees8rjcx46t4daqzzqt7yga8" ) .unwrap() + .assume_checked() ); // We won't get the same twice. let addr2 = control.get_new_address().address; @@ -933,7 +939,7 @@ mod tests { ( bitcoin::Transaction { version: 2, - lock_time: bitcoin::PackedLockTime(0), + lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), input: vec![], output: vec![], }, @@ -947,10 +953,11 @@ mod tests { let dummy_addr = bitcoin::Address::from_str("bc1qnsexk3gnuyayu92fc3tczvc7k62u22a22ua2kv").unwrap(); let dummy_value = 10_000; - let mut destinations: HashMap = [(dummy_addr.clone(), dummy_value)] - .iter() - .cloned() - .collect(); + let mut destinations: HashMap, u64> = + [(dummy_addr.clone(), dummy_value)] + .iter() + .cloned() + .collect(); assert_eq!( control.create_spend(&destinations, &[], 1), Err(CommandError::NoOutpoint) @@ -982,7 +989,10 @@ mod tests { assert_eq!(tx.input.len(), 1); assert_eq!(tx.input[0].previous_output, dummy_op); assert_eq!(tx.output.len(), 2); - assert_eq!(tx.output[0].script_pubkey, dummy_addr.script_pubkey()); + assert_eq!( + tx.output[0].script_pubkey, + dummy_addr.payload.script_pubkey() + ); assert_eq!(tx.output[0].value, dummy_value); // Transaction is 1 in (P2WSH satisfaction), 2 outs. At 1sat/vb, it's 171 sats fees. @@ -1025,22 +1035,19 @@ mod tests { ); // If we ask to create an output for an address from another network, it will fail. - let invalid_addr = bitcoin::Address { - network: bitcoin::Network::Testnet, - payload: dummy_addr.payload.clone(), - }; - let invalid_destinations: HashMap = + let invalid_addr = + bitcoin::Address::new(bitcoin::Network::Testnet, dummy_addr.payload.clone()); + let invalid_destinations: HashMap, u64> = [(invalid_addr.clone(), dummy_value)] .iter() .cloned() .collect(); - assert_eq!( + assert!(matches!( control.create_spend(&invalid_destinations, &[dummy_op], 1), - Err(CommandError::AddressNetwork( - invalid_addr, - bitcoin::Network::Bitcoin + Err(CommandError::Address( + address::Error::NetworkValidation { .. } )) - ); + )); // If we ask for a large, but valid, output we won't get a change output. 95_000 because we // won't create an output lower than 5k sats. @@ -1050,7 +1057,10 @@ mod tests { assert_eq!(tx.input.len(), 1); assert_eq!(tx.input[0].previous_output, dummy_op); assert_eq!(tx.output.len(), 1); - assert_eq!(tx.output[0].script_pubkey, dummy_addr.script_pubkey()); + assert_eq!( + tx.output[0].script_pubkey, + dummy_addr.payload.script_pubkey() + ); assert_eq!(tx.output[0].value, 95_000); // Now if we mark the coin as spent, we won't create another Spend transaction containing @@ -1104,7 +1114,7 @@ mod tests { let mut dummy_bitcoind = DummyBitcoind::new(); let dummy_tx = bitcoin::Transaction { version: 2, - lock_time: bitcoin::PackedLockTime(0), + lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), input: vec![], output: vec![], }; @@ -1145,17 +1155,17 @@ mod tests { bitcoin::Address::from_str("bc1q39srgatmkp6k2ne3l52yhkjprdvunvspqydmkx").unwrap(); let dummy_value_a = 50_000; let dummy_value_b = 60_000; - let destinations_a: HashMap = + let destinations_a: HashMap, u64> = [(dummy_addr_a.clone(), dummy_value_a)] .iter() .cloned() .collect(); - let destinations_b: HashMap = + let destinations_b: HashMap, u64> = [(dummy_addr_b.clone(), dummy_value_b)] .iter() .cloned() .collect(); - let destinations_c: HashMap = + let destinations_c: HashMap, u64> = [(dummy_addr_a, dummy_value_a), (dummy_addr_b, dummy_value_b)] .iter() .cloned() @@ -1185,7 +1195,7 @@ mod tests { assert_eq!(db_conn.spend_tx(&txid_c).unwrap(), psbt_c); // As well as update them, with or without new signatures - let sig = bitcoin::EcdsaSig::from_str("304402204004fcdbb9c0d0cbf585f58cee34dccb012efbd8fc2b0d5e97760045ae35803802201a0bd7ec2383e0b93748abc9946c8e17a8312e314dab85982aeba650e738cbf401").unwrap(); + let sig = bitcoin::ecdsa::Signature::from_str("304402204004fcdbb9c0d0cbf585f58cee34dccb012efbd8fc2b0d5e97760045ae35803802201a0bd7ec2383e0b93748abc9946c8e17a8312e314dab85982aeba650e738cbf401").unwrap(); psbt_a.inputs[0].partial_sigs.insert( bitcoin::PublicKey::from_str( "023a664c5617412f0b292665b1fd9d766456a7a3b1614c7e7c5f411200ff1958ef", @@ -1224,68 +1234,68 @@ mod tests { let deposit1: Transaction = Transaction { version: 1, - lock_time: PackedLockTime(1), + lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), previous_output: outpoint, - script_sig: Script::new(), + script_sig: ScriptBuf::new(), sequence: Sequence(0), }], output: vec![TxOut { - script_pubkey: Script::new(), + script_pubkey: ScriptBuf::new(), value: 100_000_000, }], }; let deposit2: Transaction = Transaction { version: 1, - lock_time: PackedLockTime(1), + lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), previous_output: outpoint, - script_sig: Script::new(), + script_sig: ScriptBuf::new(), sequence: Sequence(0), }], output: vec![TxOut { - script_pubkey: Script::new(), + script_pubkey: ScriptBuf::new(), value: 2000, }], }; let deposit3: Transaction = Transaction { version: 1, - lock_time: PackedLockTime(1), + lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), previous_output: outpoint, - script_sig: Script::new(), + script_sig: ScriptBuf::new(), sequence: Sequence(0), }], output: vec![TxOut { - script_pubkey: Script::new(), + script_pubkey: ScriptBuf::new(), value: 3000, }], }; let spend_tx: Transaction = Transaction { version: 1, - lock_time: PackedLockTime(1), + lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), previous_output: OutPoint { txid: deposit1.txid(), vout: 0, }, - script_sig: Script::new(), + script_sig: ScriptBuf::new(), sequence: Sequence(0), }], output: vec![ TxOut { - script_pubkey: Script::new(), + script_pubkey: ScriptBuf::new(), value: 4000, }, TxOut { - script_pubkey: Script::new(), + script_pubkey: ScriptBuf::new(), value: 100_000_000 - 4000 - 1000, }, ], @@ -1447,45 +1457,45 @@ mod tests { let tx1: Transaction = Transaction { version: 1, - lock_time: PackedLockTime(1), + lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), previous_output: outpoint, - script_sig: Script::new(), + script_sig: ScriptBuf::new(), sequence: Sequence(0), }], output: vec![TxOut { - script_pubkey: Script::new(), + script_pubkey: ScriptBuf::new(), value: 100_000_000, }], }; let tx2: Transaction = Transaction { version: 1, - lock_time: PackedLockTime(1), + lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), previous_output: outpoint, - script_sig: Script::new(), + script_sig: ScriptBuf::new(), sequence: Sequence(0), }], output: vec![TxOut { - script_pubkey: Script::new(), + script_pubkey: ScriptBuf::new(), value: 2000, }], }; let tx3: Transaction = Transaction { version: 1, - lock_time: PackedLockTime(1), + lock_time: absolute::LockTime::Blocks(absolute::Height::from_consensus(1).unwrap()), input: vec![TxIn { witness: Witness::new(), previous_output: outpoint, - script_sig: Script::new(), + script_sig: ScriptBuf::new(), sequence: Sequence(0), }], output: vec![TxOut { - script_pubkey: Script::new(), + script_pubkey: ScriptBuf::new(), value: 3000, }], }; diff --git a/src/commands/utils.rs b/src/commands/utils.rs index 85ac95eab..ce0e26441 100644 --- a/src/commands/utils.rs +++ b/src/commands/utils.rs @@ -1,40 +1,48 @@ +use std::str::FromStr; + use miniscript::bitcoin::{self, consensus, hashes::hex::FromHex}; use serde::{de, Deserialize, Deserializer, Serializer}; -/// Serialize an amount as sats -pub fn ser_amount(amount: &bitcoin::Amount, s: S) -> Result { - s.serialize_u64(amount.to_sat()) -} - -/// Deserialize an amount from sats -pub fn deser_amount_from_sats<'de, D>(deserializer: D) -> Result +pub fn deser_fromstr<'de, D, T>(deserializer: D) -> Result where D: Deserializer<'de>, + T: FromStr, + ::Err: std::fmt::Display, { - let a = u64::deserialize(deserializer)?; - Ok(bitcoin::Amount::from_sat(a)) + let string = String::deserialize(deserializer)?; + T::from_str(&string).map_err(de::Error::custom) } -pub fn to_base64_string(t: T) -> String { - base64::encode(consensus::serialize(&t)) +pub fn ser_to_string( + field: T, + s: S, +) -> Result { + s.serialize_str(&field.to_string()) } -pub fn ser_base64(t: T, s: S) -> Result +/// Deserialize an address from string, assuming the network was checked. +pub fn deser_addr_assume_checked<'de, D>(deserializer: D) -> Result where - S: Serializer, - T: consensus::Encodable, + D: Deserializer<'de>, { - s.serialize_str(&to_base64_string(t)) + let string = String::deserialize(deserializer)?; + bitcoin::Address::from_str(&string) + .map(|addr| addr.assume_checked()) + .map_err(de::Error::custom) } -pub fn deser_base64<'de, D, T>(d: D) -> Result +/// Serialize an amount as sats +pub fn ser_amount(amount: &bitcoin::Amount, s: S) -> Result { + s.serialize_u64(amount.to_sat()) +} + +/// Deserialize an amount from sats +pub fn deser_amount_from_sats<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, - T: consensus::Decodable, { - let s = String::deserialize(d)?; - let s = base64::decode(s).map_err(de::Error::custom)?; - consensus::deserialize(&s).map_err(de::Error::custom) + let a = u64::deserialize(deserializer)?; + Ok(bitcoin::Amount::from_sat(a)) } pub fn ser_hex(t: T, s: S) -> Result diff --git a/src/database/mod.rs b/src/database/mod.rs index dd08ede01..23daf8c23 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -14,10 +14,7 @@ use crate::{ use std::{collections::HashMap, sync}; -use miniscript::bitcoin::{ - self, secp256k1, - util::{bip32, psbt::PartiallySignedTransaction as Psbt}, -}; +use miniscript::bitcoin::{self, bip32, psbt::PartiallySignedTransaction as Psbt, secp256k1}; pub trait DatabaseInterface: Send { fn connection(&self) -> Box; @@ -220,8 +217,12 @@ impl DatabaseConnection for SqliteConn { &mut self, address: &bitcoin::Address, ) -> Option<(bip32::ChildNumber, bool)> { - self.db_address(address) - .map(|db_addr| (db_addr.derivation_index, address == &db_addr.change_address)) + self.db_address(address).map(|db_addr| { + ( + db_addr.derivation_index, + address == &db_addr.change_address.assume_checked(), + ) + }) } fn coins_by_outpoints( diff --git a/src/database/sqlite/mod.rs b/src/database/sqlite/mod.rs index 523fc7a26..2e63ec124 100644 --- a/src/database/sqlite/mod.rs +++ b/src/database/sqlite/mod.rs @@ -28,11 +28,11 @@ use crate::{ use std::{cmp, convert::TryInto, fmt, io, path}; use miniscript::bitcoin::{ - self, + self, bip32, consensus::encode, - hashes::hex::ToHex, + hashes::{sha256, Hash}, + psbt::PartiallySignedTransaction as Psbt, secp256k1, - util::{bip32, psbt::PartiallySignedTransaction as Psbt}, }; const DB_VERSION: i64 = 1; @@ -84,6 +84,24 @@ impl From for SqliteDbError { } } +// In Bitcoin land, txids are usually displayed in reverse byte order. This is what rust-bitcoin +// implements as `fmt::Display` for `bitcoin::Txid`. However, we store them as raw bytes in the +// database and it so happens we sometimes have to look for a txid in hex, in which case we want +// the "frontward" hex serialization. This is a hack to implement it. +#[derive(Debug, Clone, Copy)] +struct FrontwardHexTxid(bitcoin::Txid); + +impl fmt::Display for FrontwardHexTxid { + fn fmt(&self, f: &mut fmt::Formatter) -> std::fmt::Result { + write!( + f, + "{:x}", + // sha256 isn't displayed in reverse byte order (contrary to sha256d). + sha256::Hash::from_byte_array(self.0.to_byte_array()) + ) + } +} + #[derive(Debug, Clone)] pub struct FreshDbOptions { pub(self) bitcoind_network: bitcoin::Network, @@ -217,7 +235,7 @@ impl SqliteConn { db_tx .execute( "UPDATE tip SET blockheight = (?1), blockhash = (?2)", - rusqlite::params![tip.height, tip.hash.to_vec()], + rusqlite::params![tip.height, tip.hash[..].to_vec()], ) .map(|_| ()) }) @@ -369,7 +387,7 @@ impl SqliteConn { VALUES (?1, ?2, ?3, ?4, ?5, ?6)", rusqlite::params![ WALLET_ID, - coin.outpoint.txid.to_vec(), + coin.outpoint.txid[..].to_vec(), coin.outpoint.vout, coin.amount.to_sat(), deriv_index, @@ -388,7 +406,7 @@ impl SqliteConn { for outpoint in outpoints { db_tx.execute( "DELETE FROM coins WHERE txid = ?1 AND vout = ?2", - rusqlite::params![outpoint.txid.to_vec(), outpoint.vout,], + rusqlite::params![outpoint.txid[..].to_vec(), outpoint.vout,], )?; } @@ -406,7 +424,7 @@ impl SqliteConn { for (outpoint, height, time) in outpoints { db_tx.execute( "UPDATE coins SET blockheight = ?1, blocktime = ?2 WHERE txid = ?3 AND vout = ?4", - rusqlite::params![height, time, outpoint.txid.to_vec(), outpoint.vout,], + rusqlite::params![height, time, outpoint.txid[..].to_vec(), outpoint.vout,], )?; } @@ -424,7 +442,11 @@ impl SqliteConn { for (outpoint, spend_txid) in outpoints { db_tx.execute( "UPDATE coins SET spend_txid = ?1 WHERE txid = ?2 AND vout = ?3", - rusqlite::params![spend_txid.to_vec(), outpoint.txid.to_vec(), outpoint.vout,], + rusqlite::params![ + spend_txid[..].to_vec(), + outpoint.txid[..].to_vec(), + outpoint.vout, + ], )?; } @@ -444,10 +466,10 @@ impl SqliteConn { db_tx.execute( "UPDATE coins SET spend_txid = ?1, spend_block_height = ?2, spend_block_time = ?3 WHERE txid = ?4 AND vout = ?5", rusqlite::params![ - spend_txid.to_vec(), + spend_txid[..].to_vec(), height, time, - outpoint.txid.to_vec(), + outpoint.txid[..].to_vec(), outpoint.vout, ], )?; @@ -473,10 +495,11 @@ impl SqliteConn { // SELECT * FROM coins WHERE (txid, vout) IN ((txidA, voutA), (txidB, voutB)); let mut query = "SELECT * FROM coins WHERE (txid, vout) IN (VALUES ".to_string(); for (i, outpoint) in outpoints.iter().enumerate() { - // NOTE: the txid is not stored as little-endian. Convert it to vec first. + // NOTE: SQLite doesn't know Satoshi decided txids would be displayed as little-endian + // hex. query += &format!( "(x'{}', {})", - &outpoint.txid.to_vec().to_hex(), + FrontwardHexTxid(outpoint.txid), outpoint.vout ); if i != outpoints.len() - 1 { @@ -495,7 +518,7 @@ impl SqliteConn { db_query( &mut self.conn, "SELECT * FROM spend_transactions WHERE txid = ?1", - rusqlite::params![txid.to_vec()], + rusqlite::params![txid[..].to_vec()], |row| row.try_into(), ) .expect("Db must not fail") @@ -504,14 +527,13 @@ impl SqliteConn { /// Insert a new Spend transaction or replace an existing one. pub fn store_spend(&mut self, psbt: &Psbt) { - let txid = psbt.unsigned_tx.txid().to_vec(); - let psbt = encode::serialize(psbt); + let txid = &psbt.unsigned_tx.txid()[..].to_vec(); db_exec(&mut self.conn, |db_tx| { db_tx.execute( "INSERT into spend_transactions (psbt, txid, updated_at) VALUES (?1, ?2, ?3) \ ON CONFLICT DO UPDATE SET psbt=excluded.psbt", - rusqlite::params![psbt, txid, curr_timestamp()], + rusqlite::params![psbt.serialize(), txid, curr_timestamp()], )?; Ok(()) }) @@ -564,7 +586,7 @@ impl SqliteConn { db_exec(&mut self.conn, |db_tx| { db_tx.execute( "DELETE FROM spend_transactions WHERE txid = ?1", - rusqlite::params![txid.to_vec()], + rusqlite::params![txid[..].to_vec()], )?; Ok(()) }) @@ -593,7 +615,7 @@ impl SqliteConn { )?; db_tx.execute( "UPDATE tip SET blockheight = (?1), blockhash = (?2)", - rusqlite::params![new_tip.height, new_tip.hash.to_vec()], + rusqlite::params![new_tip.height, new_tip.hash[..].to_vec()], )?; Ok(()) }) @@ -612,7 +634,7 @@ mod tests { str::FromStr, }; - use bitcoin::{hashes::Hash, util::bip32}; + use bitcoin::{bip32, hashes::Hash}; // The database schema used by the first versions of Liana (database version 0). Used to test // migrations starting from the first version. @@ -687,7 +709,7 @@ CREATE TABLE spend_transactions ( "; fn psbt_from_str(psbt_str: &str) -> Psbt { - bitcoin::consensus::deserialize(&base64::decode(psbt_str).unwrap()).unwrap() + Psbt::from_str(psbt_str).unwrap() } fn dummy_options() -> FreshDbOptions { @@ -1449,14 +1471,13 @@ CREATE TABLE spend_transactions ( // The helper that was used to store Spend transaction in previous versions of the software // when there was no associated timestamp. fn store_spend_old(conn: &mut rusqlite::Connection, psbt: &Psbt) { - let txid = psbt.unsigned_tx.txid().to_vec(); - let psbt = encode::serialize(psbt); + let txid = &psbt.unsigned_tx.txid()[..].to_vec(); db_exec(conn, |db_tx| { db_tx.execute( "INSERT into spend_transactions (psbt, txid) VALUES (?1, ?2) \ ON CONFLICT DO UPDATE SET psbt=excluded.psbt", - rusqlite::params![psbt, txid], + rusqlite::params![psbt.serialize(), txid], )?; Ok(()) }) diff --git a/src/database/sqlite/schema.rs b/src/database/sqlite/schema.rs index 597d81142..93e101e77 100644 --- a/src/database/sqlite/schema.rs +++ b/src/database/sqlite/schema.rs @@ -3,9 +3,7 @@ use crate::descriptors::LianaDescriptor; use std::{convert::TryFrom, str::FromStr}; use miniscript::bitcoin::{ - self, - consensus::encode, - util::{bip32, psbt::PartiallySignedTransaction as Psbt}, + self, address, bip32, consensus::encode, psbt::PartiallySignedTransaction as Psbt, }; pub const SCHEMA: &str = "\ @@ -219,8 +217,8 @@ impl TryFrom<&rusqlite::Row<'_>> for DbCoin { #[derive(Debug, Clone, PartialEq, Eq)] pub struct DbAddress { - pub receive_address: bitcoin::Address, - pub change_address: bitcoin::Address, + pub receive_address: bitcoin::Address, + pub change_address: bitcoin::Address, pub derivation_index: bip32::ChildNumber, } @@ -264,7 +262,7 @@ impl TryFrom<&rusqlite::Row<'_>> for DbSpendTransaction { let id: i64 = row.get(0)?; let psbt: Vec = row.get(1)?; - let psbt: Psbt = encode::deserialize(&psbt).expect("We only store valid PSBTs"); + let psbt = Psbt::deserialize(&psbt).expect("We only store valid PSBTs"); let txid: Vec = row.get(2)?; let txid: bitcoin::Txid = encode::deserialize(&txid).expect("We only store valid txids"); diff --git a/src/descriptors/analysis.rs b/src/descriptors/analysis.rs index f80193369..4f4dad2b4 100644 --- a/src/descriptors/analysis.rs +++ b/src/descriptors/analysis.rs @@ -1,5 +1,5 @@ use miniscript::{ - bitcoin::{util::bip32, Sequence}, + bitcoin::{bip32, Sequence}, descriptor, policy::{compiler, Concrete as ConcretePolicy, Liftable, Semantic as SemanticPolicy}, ScriptContext, diff --git a/src/descriptors/keys.rs b/src/descriptors/keys.rs index 29e1d13ea..56150bb37 100644 --- a/src/descriptors/keys.rs +++ b/src/descriptors/keys.rs @@ -1,8 +1,7 @@ use miniscript::{ bitcoin::{ - self, + self, bip32, hashes::{hash160, ripemd160, sha256}, - util::bip32, }, hash256, MiniscriptKey, ToPublicKey, }; diff --git a/src/descriptors/mod.rs b/src/descriptors/mod.rs index 947586da6..f8436e79d 100644 --- a/src/descriptors/mod.rs +++ b/src/descriptors/mod.rs @@ -1,10 +1,8 @@ use miniscript::{ bitcoin::{ - self, secp256k1, - util::{ - bip32, - psbt::{Input as PsbtIn, Psbt}, - }, + self, bip32, + psbt::{Input as PsbtIn, Psbt}, + secp256k1, }, descriptor, translate_hash_clone, ForEachKey, TranslatePk, Translator, }; @@ -367,11 +365,11 @@ impl DerivedSinglePathLianaDesc { .expect("A P2WSH always has an address") } - pub fn script_pubkey(&self) -> bitcoin::Script { + pub fn script_pubkey(&self) -> bitcoin::ScriptBuf { self.0.script_pubkey() } - pub fn witness_script(&self) -> bitcoin::Script { + pub fn witness_script(&self) -> bitcoin::ScriptBuf { self.0.explicit_script().expect("Not a Taproot descriptor") } @@ -733,7 +731,7 @@ mod tests { } fn psbt_from_str(psbt_str: &str) -> Psbt { - bitcoin::consensus::deserialize(&base64::decode(psbt_str).unwrap()).unwrap() + Psbt::from_str(psbt_str).unwrap() } #[test] @@ -973,7 +971,7 @@ mod tests { "0282574238aea21ec72ffffd8d8a981a30004b5794c8094ff394fec79509a5834f", ) .unwrap(); - let dummy_sig = bitcoin::EcdsaSig::from_str ("30440220264d47ed3fd613e4ac34303c59a0e558d41e487a68af5c5d4bb790f6ccf218ab02203213fe4d51729f9852a28f7d22b2ecb2b096eaf07ad44638af77e4bdbdd4462901").unwrap(); + let dummy_sig = bitcoin::ecdsa::Signature::from_str("30440220264d47ed3fd613e4ac34303c59a0e558d41e487a68af5c5d4bb790f6ccf218ab02203213fe4d51729f9852a28f7d22b2ecb2b096eaf07ad44638af77e4bdbdd4462901").unwrap(); let dummy_der_path = bip32::DerivationPath::from_str("m/0/1").unwrap(); let fingerprint = prim_path.thresh_origins().1.into_iter().next().unwrap().0; psbt.inputs[0] diff --git a/src/jsonrpc/api.rs b/src/jsonrpc/api.rs index 3e67659d0..7d54c85f4 100644 --- a/src/jsonrpc/api.rs +++ b/src/jsonrpc/api.rs @@ -5,7 +5,7 @@ use crate::{ use std::{collections::HashMap, convert::TryInto, str::FromStr}; -use miniscript::bitcoin::{self, consensus, util::psbt::PartiallySignedTransaction as Psbt}; +use miniscript::bitcoin::{self, psbt::PartiallySignedTransaction as Psbt}; fn create_spend(control: &DaemonControl, params: Params) -> Result { let destinations = params @@ -19,7 +19,7 @@ fn create_spend(control: &DaemonControl, params: Params) -> Result>>() + .collect::, u64>>>() }) .ok_or_else(|| Error::invalid_params("Invalid 'destinations' parameter."))?; let outpoints = params @@ -51,8 +51,7 @@ fn update_spend(control: &DaemonControl, params: Params) -> Result for Error { | commands::CommandError::UnknownOutpoint(..) | commands::CommandError::InvalidFeerate(..) | commands::CommandError::AlreadySpent(..) - | commands::CommandError::AddressNetwork(..) + | commands::CommandError::Address(..) | commands::CommandError::InvalidOutputValue(..) | commands::CommandError::InsufficientFunds(..) | commands::CommandError::InsaneFees(..) diff --git a/src/lib.rs b/src/lib.rs index a08c256a5..abb520191 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -222,6 +222,10 @@ fn maybe_delete_watchonly_wallet( miniscript::bitcoin::Network::Testnet | miniscript::bitcoin::Network::Signet | miniscript::bitcoin::Network::Regtest => parent_dir.join("wallets").join(wallet_name), + net => panic!( + "Unsupported network '{}', unknown at the time of writing.", + net + ), }; if wallet_path.exists() { diff --git a/src/random.rs b/src/random.rs index 700cc4024..9c5723c8b 100644 --- a/src/random.rs +++ b/src/random.rs @@ -67,7 +67,7 @@ fn additional_data() -> Result<[u8; 32], RandomnessError> { engine.input(&pid.to_be_bytes()); // TODO: get some more contextual information - Ok(*sha256::Hash::from_engine(engine).as_inner()) + Ok(sha256::Hash::from_engine(engine).to_byte_array()) } /// Get 32 random bytes. This is mainly based on OS-provided randomness (`getrandom` or @@ -86,7 +86,7 @@ pub fn random_bytes() -> Result<[u8; 32], RandomnessError> { engine.input(&additional_data()?); // TODO: add more sources of randomness - Ok(*sha256::Hash::from_engine(engine).as_inner()) + Ok(sha256::Hash::from_engine(engine).to_byte_array()) } #[cfg(test)] diff --git a/src/signer.rs b/src/signer.rs index 76650a9be..ebce7510a 100644 --- a/src/signer.rs +++ b/src/signer.rs @@ -15,14 +15,11 @@ use std::{ use miniscript::bitcoin::{ self, + bip32::{self, Error as Bip32Error}, + ecdsa, hashes::Hash, - secp256k1, - util::{ - bip32::{self, Error as Bip32Error}, - ecdsa, - psbt::Psbt, - sighash, - }, + psbt::Psbt, + secp256k1, sighash, }; /// An error related to using a signer. @@ -78,7 +75,7 @@ fn create_dir(path: &path::Path) -> io::Result<()> { // TODO: permissions on Windows.. #[cfg(not(unix))] - return { fs::create_dir_all(path) }; + fs::create_dir_all(path) } // Create a file with no permission for the group and other users, and only read permissions for @@ -259,7 +256,7 @@ impl HotSigner { let sighash = sighash_cache .segwit_signature_hash(i, witscript, value, sig_type) .map_err(|_| SignerError::InsanePsbt)?; - let sighash = secp256k1::Message::from_slice(sighash.as_hash().as_inner()) + let sighash = secp256k1::Message::from_slice(sighash.as_byte_array()) .expect("Sighash is always 32 bytes."); // Then provide a signature for all the keys they asked for. @@ -277,7 +274,7 @@ impl HotSigner { let sig = secp.sign_ecdsa_low_r(&sighash, &privkey.inner); psbt.inputs[i].partial_sigs.insert( pubkey, - ecdsa::EcdsaSig { + ecdsa::Signature { sig, hash_ty: sig_type, }, @@ -301,7 +298,7 @@ mod tests { use super::*; use crate::{descriptors, testutils::*}; use miniscript::{ - bitcoin::util::psbt::Input as PsbtIn, + bitcoin::{locktime::absolute, psbt::Input as PsbtIn}, descriptor::{DerivPaths, DescriptorMultiXKey, DescriptorPublicKey, Wildcard}, }; use std::collections::{BTreeMap, HashSet}; @@ -434,7 +431,7 @@ mod tests { let mut dummy_psbt = Psbt { unsigned_tx: bitcoin::Transaction { version: 2, - lock_time: bitcoin::PackedLockTime(0), + lock_time: absolute::LockTime::Blocks(absolute::Height::ZERO), input: vec![bitcoin::TxIn { sequence: bitcoin::Sequence::ENABLE_RBF_NO_LOCKTIME, previous_output: bitcoin::OutPoint::from_str( @@ -449,6 +446,7 @@ mod tests { "bc1qvklensptw5lk7d470ds60pcpsr0psdpgyvwepv", ) .unwrap() + .payload .script_pubkey(), }], }, diff --git a/src/testutils.rs b/src/testutils.rs index ee5b0e3b8..4f7391834 100644 --- a/src/testutils.rs +++ b/src/testutils.rs @@ -9,9 +9,7 @@ use std::{collections::HashMap, env, fs, io, path, process, str::FromStr, sync, use miniscript::{ bitcoin::{ - self, secp256k1, - util::{bip32, psbt::PartiallySignedTransaction as Psbt}, - Transaction, Txid, + self, bip32, psbt::PartiallySignedTransaction as Psbt, secp256k1, Transaction, Txid, }, descriptor, }; From 0ac4d80ddbdb61ef3ba9d7e6eb7b76114dfcf961 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 12 Jul 2023 17:24:51 +0200 Subject: [PATCH 2/5] commands: fix two clippy lints For some reason it started warning about them after the upgrade of rust-miniscript. --- src/commands/mod.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index fba9961a8..2c65fae15 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -192,7 +192,7 @@ fn sanity_check_psbt( let value_out: u64 = tx.output.iter().map(|o| o.value).sum(); let abs_fee = value_in .checked_sub(value_out) - .ok_or_else(|| CommandError::InsaneFees(InsaneFeeInfo::NegativeFee))?; + .ok_or(CommandError::InsaneFees(InsaneFeeInfo::NegativeFee))?; if abs_fee > MAX_FEE { return Err(CommandError::InsaneFees(InsaneFeeInfo::TooHighFee(abs_fee))); } @@ -201,7 +201,7 @@ fn sanity_check_psbt( let tx_vb = (tx.vsize() + spent_desc.max_sat_vbytes() * tx.input.len()) as u64; let feerate_sats_vb = abs_fee .checked_div(tx_vb) - .ok_or_else(|| CommandError::InsaneFees(InsaneFeeInfo::InvalidFeerate))?; + .ok_or(CommandError::InsaneFees(InsaneFeeInfo::InvalidFeerate))?; if !(1..=MAX_FEERATE).contains(&feerate_sats_vb) { return Err(CommandError::InsaneFees(InsaneFeeInfo::TooHighFeerate( feerate_sats_vb, @@ -1038,10 +1038,7 @@ mod tests { let invalid_addr = bitcoin::Address::new(bitcoin::Network::Testnet, dummy_addr.payload.clone()); let invalid_destinations: HashMap, u64> = - [(invalid_addr.clone(), dummy_value)] - .iter() - .cloned() - .collect(); + [(invalid_addr, dummy_value)].iter().cloned().collect(); assert!(matches!( control.create_spend(&invalid_destinations, &[dummy_op], 1), Err(CommandError::Address( From b9753b48d03ec7d4398764ec6a91e04388bdb85e Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 12 Jul 2023 17:27:52 +0200 Subject: [PATCH 3/5] descriptors: update the satisfaction size estimation The latest rust-miniscript version deprecated the helper we were using, in favour of one that gives the maximum size difference of a transaction input before and after satisfaction. The new helper differs in that it does not account for the empty ScriptSig byte (which uncovered we were actually double-counting it), and assumes the non-satisfied transaction input is already part of a Segwit transaction (which we rectified). This uncovered a mistake in the computation of the witness script size in the unit test. We also get rid of the needless wu_to_vb() standalone function. --- src/commands/mod.rs | 8 +++---- src/descriptors/mod.rs | 48 ++++++++++++++++++++++++++---------------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 2c65fae15..e5ca6322f 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -732,7 +732,7 @@ impl DaemonControl { // that is fed to the transaction while doing so, to compute the fees afterward. let mut in_value = bitcoin::Amount::from_sat(0); let txin_sat_vb = self.config.main_descriptor.max_sat_vbytes(); - let mut sat_vb = 0; + let mut sat_vb = 1; // Start at 1 for the segwit marker size, rounded up. let mut spent_txs = HashMap::new(); for coin in sweepable_coins { in_value += coin.amount; @@ -995,12 +995,12 @@ mod tests { ); assert_eq!(tx.output[0].value, dummy_value); - // Transaction is 1 in (P2WSH satisfaction), 2 outs. At 1sat/vb, it's 171 sats fees. + // 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_829); + assert_eq!(tx.output[1].value, 89_830); let res = control.create_spend(&destinations, &[dummy_op], 2).unwrap(); let tx = res.psbt.unsigned_tx; - assert_eq!(tx.output[1].value, 89_658); + assert_eq!(tx.output[1].value, 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). diff --git a/src/descriptors/mod.rs b/src/descriptors/mod.rs index f8436e79d..e61e4950d 100644 --- a/src/descriptors/mod.rs +++ b/src/descriptors/mod.rs @@ -19,13 +19,6 @@ pub use analysis::*; const WITNESS_FACTOR: usize = 4; -// Convert a size in weight units to a size in virtual bytes, rounding up. -fn wu_to_vb(vb: usize) -> usize { - (vb + WITNESS_FACTOR - 1) - .checked_div(WITNESS_FACTOR) - .expect("Non 0") -} - #[derive(Debug)] pub enum LianaDescError { Miniscript(miniscript::Error), @@ -187,18 +180,30 @@ impl LianaDescriptor { .0 } - /// Get the maximum size in WU of a satisfaction for this descriptor. + /// Get the maximum size difference of a transaction input spending a Script derived from this + /// descriptor before and after satisfaction. The returned value is in weight units. + /// Callers are expected to account for the Segwit marker (2 WU). This takes into account the + /// size of the witness stack length varint. pub fn max_sat_weight(&self) -> usize { + // We add one to account for the witness stack size, as the `max_weight_to_satisfy` method + // computes the difference in size for a satisfied input that was *already* in a + // transaction that spent one or more Segwit coins (and thus already have 1 WU accounted + // for the emtpy witness). But this method is used to account between a completely "nude" + // transaction (and therefore no Segwit marker nor empty witness in inputs) and a satisfied + // transaction. self.multi_desc - .max_satisfaction_weight() - .expect("Cannot fail for P2WSH") + .max_weight_to_satisfy() + .expect("Always satisfiable") + + 1 } - /// Get the maximum size in vbytes (rounded up) of a satisfaction for this descriptor. + /// Get the maximum size difference of a transaction input spending a Script derived from this + /// descriptor before and after satisfaction. The returned value is in (rounded up) virtual + /// bytes. + /// Callers are expected to account for the Segwit marker (2 WU). This takes into account the + /// size of the witness stack length varint. pub fn max_sat_vbytes(&self) -> usize { - self.multi_desc - .max_satisfaction_weight() - .expect("Cannot fail for P2WSH") + self.max_sat_weight() .checked_add(WITNESS_FACTOR - 1) .unwrap() .checked_div(WITNESS_FACTOR) @@ -209,7 +214,7 @@ impl LianaDescriptor { /// a coin with this Script. pub fn spender_input_size(&self) -> usize { // txid + vout + nSequence + empty scriptSig + witness - 32 + 4 + 4 + 1 + wu_to_vb(self.max_sat_weight()) + 32 + 4 + 4 + 1 + self.max_sat_vbytes() } /// Get some information about a PSBT input spending Liana coins. @@ -413,6 +418,13 @@ mod tests { descriptor::DescriptorPublicKey::from_str(&xpub_str).unwrap() } + // Convert a size in weight units to a size in virtual bytes, rounding up. + fn wu_to_vb(vb: usize) -> usize { + (vb + WITNESS_FACTOR - 1) + .checked_div(WITNESS_FACTOR) + .expect("Non 0") + } + #[test] fn descriptor_creation() { let owner_key = PathInfo::Single(descriptor::DescriptorPublicKey::from_str("[abcdef01]xpub6Eze7yAT3Y1wGrnzedCNVYDXUqa9NmHVWck5emBaTbXtURbe1NWZbK9bsz1TiVE7Cz341PMTfYgFw1KdLWdzcM1UMFTcdQfCYhhXZ2HJvTW/<0;1>/*").unwrap()); @@ -603,7 +615,7 @@ mod tests { #[test] fn inheritance_descriptor_sat_size() { let desc = LianaDescriptor::from_str("wsh(or_d(pk([92162c45]tpubD6NzVbkrYhZ4WzTf9SsD6h7AH7oQEippXK2KP8qvhMMqFoNeN5YFVi7vRyeRSDGtgd2bPyMxUNmHui8t5yCgszxPPxMafu1VVzDpg9aruYW/<0;1>/*),and_v(v:pkh([abcdef01]tpubD6NzVbkrYhZ4Wdgu2yfdmrce5g4fiH1ZLmKhewsnNKupbi4sxjH1ZVAorkBLWSkhsjhg8kiq8C4BrBjMy3SjAKDyDdbuvUa1ToAHbiR98js/<0;1>/*),older(2))))#ravw7jw5").unwrap(); - assert_eq!(desc.max_sat_vbytes(), (1 + 69 + 1 + 34 + 73 + 3) / 4); // See the stack details below. + assert_eq!(desc.max_sat_vbytes(), (1 + 66 + 1 + 34 + 73 + 3) / 4); // See the stack details below. // Maximum input size is (txid + vout + scriptsig + nSequence + max_sat). // Where max_sat is: @@ -614,11 +626,11 @@ mod tests { // - Push a signature for the recovery key // NOTE: The specific value is asserted because this was tested against a regtest // transaction. - let stack = vec![vec![0; 68], vec![0; 0], vec![0; 33], vec![0; 72]]; + 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() + stack .iter() - .map(|item| bitcoin::VarInt(stack.len() as u64).len() + item.len()) + .map(|item| bitcoin::VarInt(item.len() as u64).len() + item.len()) .sum::(); assert_eq!( desc.spender_input_size(), From 96e4cb53537a59f4b2e11011f77e7635ee4a1dd9 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 12 Jul 2023 18:25:10 +0200 Subject: [PATCH 4/5] Update proc-macro2 to fix a nightly compilation bug --- Cargo.lock | 4 ++-- gui/Cargo.lock | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9824ccc9e..d3194da07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -332,9 +332,9 @@ checksum = "1df8c4ec4b0627e53bdf214615ad287367e482558cf84b109250b37464dc03ae" [[package]] name = "proc-macro2" -version = "1.0.47" +version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ea3d908b0e36316caf9e9e2c4625cdde190a7e6f440d794667ed17a1855e725" +checksum = "78803b62cbf1f46fde80d7c0e803111524b9877184cfe7c3033659490ac7a7da" dependencies = [ "unicode-ident", ] diff --git a/gui/Cargo.lock b/gui/Cargo.lock index 0ba3aaa92..fa70fc3ed 100644 --- a/gui/Cargo.lock +++ b/gui/Cargo.lock @@ -2788,9 +2788,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.56" +version = "1.0.64" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b63bdb0cd06f1f4dedf69b254734f9b45af66e4a031e42a7480257d9898b435" +checksum = "78803b62cbf1f46fde80d7c0e803111524b9877184cfe7c3033659490ac7a7da" dependencies = [ "unicode-ident", ] From a19f2c1536cab6f6a3e85eedcba9be4b907dc33b Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 13 Jul 2023 09:31:35 +0200 Subject: [PATCH 5/5] daemon: drop the base64 dependency rust-bitcoin now takes care of that. --- Cargo.lock | 1 - Cargo.toml | 3 --- 2 files changed, 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d3194da07..b58d267a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -241,7 +241,6 @@ name = "liana" version = "1.0.0" dependencies = [ "backtrace", - "base64", "bip39", "dirs", "fern", diff --git a/Cargo.toml b/Cargo.toml index e308ae944..fac64f57f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,9 +54,6 @@ jsonrpc = "0.12" # Used for daemonization libc = { version = "0.2", optional = true } -# Used for PSBTs -base64 = "0.13" - # Used for generating mnemonics getrandom = "0.2"