Skip to content

Commit

Permalink
descriptors: update the satisfaction size estimation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
darosior committed Jul 12, 2023
1 parent 96909c7 commit 677e995
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 22 deletions.
8 changes: 4 additions & 4 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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).
Expand Down
48 changes: 30 additions & 18 deletions src/descriptors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand All @@ -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.
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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:
Expand All @@ -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::<usize>();
assert_eq!(
desc.spender_input_size(),
Expand Down

0 comments on commit 677e995

Please sign in to comment.