From c711c5b696b7c8353da29b3c451bf2ca00e8daa8 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 16 Feb 2023 16:09:54 +0100 Subject: [PATCH] commands: don't overestimate the feerate in tx creation sanity checks It was accounting for the transaction size without witnesses! --- src/commands/mod.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 496fd872..4fabc183 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -129,7 +129,10 @@ fn check_output_value(value: bitcoin::Amount) -> Result<(), CommandError> { // Apply some sanity checks on a created transaction's PSBT. // TODO: add more sanity checks from revault_tx -fn sanity_check_psbt(psbt: &Psbt) -> Result<(), CommandError> { +fn sanity_check_psbt( + spent_desc: &descriptors::MultipathDescriptor, + psbt: &Psbt, +) -> Result<(), CommandError> { let tx = &psbt.unsigned_tx; // Must have as many in/out in the PSBT and Bitcoin tx. @@ -161,7 +164,7 @@ fn sanity_check_psbt(psbt: &Psbt) -> Result<(), CommandError> { } // Check the feerate isn't insane. - let tx_vb = tx.vsize() as u64; + 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::SanityCheckFailure(psbt.clone()))?; @@ -466,7 +469,7 @@ impl DaemonControl { inputs: psbt_ins, outputs: psbt_outs, }; - sanity_check_psbt(&psbt)?; + sanity_check_psbt(&self.config.main_descriptor, &psbt)?; // TODO: maybe check for common standardness rules (max size, ..)? Ok(CreateSpendResult { psbt }) @@ -742,7 +745,7 @@ impl DaemonControl { })?; psbt.unsigned_tx.output[0].value = output_value.to_sat(); - sanity_check_psbt(&psbt)?; + sanity_check_psbt(&self.config.main_descriptor, &psbt)?; Ok(CreateRecoveryResult { psbt }) } @@ -950,6 +953,10 @@ mod tests { let tx = res.psbt.unsigned_tx; assert_eq!(tx.output[1].value, 89_658); + // A feerate of 555 won't trigger the sanity checks (they were previously not taking the + // satisfaction size into account and overestimating the feerate). + control.create_spend(&destinations, &[dummy_op], 555).unwrap(); + // If we ask for a too high feerate, or a too large/too small output, it'll fail. assert_eq!( control.create_spend(&destinations, &[dummy_op], 10_000),