Skip to content

Commit 0ab955f

Browse files
committed
Add debug_asserts to ensure we don't use an invalid package_amount
This somewhat cleans up the public API of PackageSolvingData to make it harder to get an invalid amount and use it, adding further debug assertion to check it at test-time.
1 parent d0fc7a3 commit 0ab955f

File tree

2 files changed

+11
-8
lines changed

2 files changed

+11
-8
lines changed

lightning/src/chain/onchaintx.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,9 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
344344
// Compute new height timer to decide when we need to regenerate a new bumped version of the claim tx (if we
345345
// didn't receive confirmation of it before, or not enough reorg-safe depth on top of it).
346346
let new_timer = Some(cached_request.get_height_timer(height));
347-
let amt = cached_request.package_amount();
348347
if cached_request.is_malleable() {
349348
let predicted_weight = cached_request.package_weight(&self.destination_script);
350-
if let Some((output_value, new_feerate)) = cached_request.compute_package_output(predicted_weight, amt, fee_estimator, logger) {
349+
if let Some((output_value, new_feerate)) = cached_request.compute_package_output(predicted_weight, fee_estimator, logger) {
351350
assert!(new_feerate != 0);
352351

353352
let transaction = cached_request.finalize_package(self, output_value, self.destination_script.clone(), logger).unwrap();
@@ -359,8 +358,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
359358
// Note: Currently, amounts of holder outputs spending witnesses aren't used
360359
// as we can't malleate spending package to increase their feerate. This
361360
// should change with the remaining anchor output patchset.
362-
debug_assert!(amt == 0);
363-
if let Some(transaction) = cached_request.finalize_package(self, amt, self.destination_script.clone(), logger) {
361+
if let Some(transaction) = cached_request.finalize_package(self, 0, self.destination_script.clone(), logger) {
364362
return Some((None, 0, transaction));
365363
}
366364
}

lightning/src/chain/package.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,8 @@ impl PackageSolvingData {
274274
// Note: Currently, amounts of holder outputs spending witnesses aren't used
275275
// as we can't malleate spending package to increase their feerate. This
276276
// should change with the remaining anchor output patchset.
277-
PackageSolvingData::HolderHTLCOutput(..) => { 0 },
278-
PackageSolvingData::HolderFundingOutput(..) => { 0 },
277+
PackageSolvingData::HolderHTLCOutput(..) => { debug_assert!(false); 0 },
278+
PackageSolvingData::HolderFundingOutput(..) => { debug_assert!(false); 0 },
279279
};
280280
amt
281281
}
@@ -577,7 +577,9 @@ impl PackageTemplate {
577577
}
578578
self.height_timer = cmp::min(self.height_timer, merge_from.height_timer);
579579
}
580-
pub(crate) fn package_amount(&self) -> u64 {
580+
/// Gets the amount of all outptus being spent by this package, only valid for malleable
581+
/// packages.
582+
fn package_amount(&self) -> u64 {
581583
let mut amounts = 0;
582584
for (_, outp) in self.inputs.iter() {
583585
amounts += outp.amount();
@@ -628,6 +630,7 @@ impl PackageTemplate {
628630
return Some(bumped_tx);
629631
},
630632
PackageMalleability::Untractable => {
633+
debug_assert_eq!(value, 0, "value is ignored for non-malleable packages, should be zero to ensure callsites are correct");
631634
if let Some((outpoint, outp)) = self.inputs.first() {
632635
if let Some(final_tx) = outp.get_finalized_tx(outpoint, onchain_handler) {
633636
log_trace!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);
@@ -653,10 +656,12 @@ impl PackageTemplate {
653656
current_height + LOW_FREQUENCY_BUMP_INTERVAL
654657
}
655658
/// Returns value in satoshis to be included as package outgoing output amount and feerate with which package finalization should be done.
656-
pub(crate) fn compute_package_output<F: Deref, L: Deref>(&self, predicted_weight: usize, input_amounts: u64, fee_estimator: &F, logger: &L) -> Option<(u64, u64)>
659+
pub(crate) fn compute_package_output<F: Deref, L: Deref>(&self, predicted_weight: usize, fee_estimator: &F, logger: &L) -> Option<(u64, u64)>
657660
where F::Target: FeeEstimator,
658661
L::Target: Logger,
659662
{
663+
debug_assert!(self.malleability == PackageMalleability::Malleable, "The package output is fixed for non-malleable packages");
664+
let input_amounts = self.package_amount();
660665
// If old feerate is 0, first iteration of this claim, use normal fee calculation
661666
if self.feerate_previous != 0 {
662667
if let Some((new_fee, feerate)) = feerate_bump(predicted_weight, input_amounts, self.feerate_previous, fee_estimator, logger) {

0 commit comments

Comments
 (0)