Add assertions 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.

diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
index 04de721f901a3a9e67ea01bca8ce2c6e3df3f362..9bbc2a4abd2e3e9a852c71e5e1d700d043fd4472 100644 (file)
--- a/lightning/src/chain/onchaintx.rs
+++ b/lightning/src/chain/onchaintx.rs
@@ -345,10 +345,9 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
                // Compute new height timer to decide when we need to regenerate a new bumped version of the claim tx (if we
                // didn't receive confirmation of it before, or not enough reorg-safe depth on top of it).
                let new_timer = Some(cached_request.get_height_timer(height));
-               let amt = cached_request.package_amount();
                if cached_request.is_malleable() {
                        let predicted_weight = cached_request.package_weight(&self.destination_script);
-                       if let Some((output_value, new_feerate)) = cached_request.compute_package_output(predicted_weight, amt, fee_estimator, logger) {
+                       if let Some((output_value, new_feerate)) = cached_request.compute_package_output(predicted_weight, fee_estimator, logger) {
                                assert!(new_feerate != 0);
 
                                let transaction = cached_request.finalize_package(self, output_value, self.destination_script.clone(), logger).unwrap();
@@ -360,8 +359,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
                        // Note: Currently, amounts of holder outputs spending witnesses aren't used
                        // as we can't malleate spending package to increase their feerate. This
                        // should change with the remaining anchor output patchset.
-                       debug_assert!(amt == 0);
-                       if let Some(transaction) = cached_request.finalize_package(self, amt, self.destination_script.clone(), logger) {
+                       if let Some(transaction) = cached_request.finalize_package(self, 0, self.destination_script.clone(), logger) {
                                return Some((None, 0, transaction));
                        }
                }

diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs
index 311937a121a0822eb01cea3bfbd4e8d6b851cd8f..9e92e2031bc1afe90c0d90d1c7f2a9322b298521 100644 (file)
--- a/lightning/src/chain/package.rs
+++ b/lightning/src/chain/package.rs
@@ -274,8 +274,8 @@ impl PackageSolvingData {
                        // Note: Currently, amounts of holder outputs spending witnesses aren't used
                        // as we can't malleate spending package to increase their feerate. This
                        // should change with the remaining anchor output patchset.
-                       PackageSolvingData::HolderHTLCOutput(..) => { 0 },
-                       PackageSolvingData::HolderFundingOutput(..) => { 0 },
+                       PackageSolvingData::HolderHTLCOutput(..) => { unreachable!() },
+                       PackageSolvingData::HolderFundingOutput(..) => { unreachable!() },
                };
                amt
        }
@@ -288,8 +288,8 @@ impl PackageSolvingData {
                        // Note: Currently, weights of holder outputs spending witnesses aren't used
                        // as we can't malleate spending package to increase their feerate. This
                        // should change with the remaining anchor output patchset.
-                       PackageSolvingData::HolderHTLCOutput(..) => { debug_assert!(false); 0 },
-                       PackageSolvingData::HolderFundingOutput(..) => { debug_assert!(false); 0 },
+                       PackageSolvingData::HolderHTLCOutput(..) => { unreachable!() },
+                       PackageSolvingData::HolderFundingOutput(..) => { unreachable!() },
                };
                weight
        }
@@ -577,7 +577,9 @@ impl PackageTemplate {
                }
                self.height_timer = cmp::min(self.height_timer, merge_from.height_timer);
        }
-       pub(crate) fn package_amount(&self) -> u64 {
+       /// Gets the amount of all outptus being spent by this package, only valid for malleable
+       /// packages.
+       fn package_amount(&self) -> u64 {
                let mut amounts = 0;
                for (_, outp) in self.inputs.iter() {
                        amounts += outp.amount();
@@ -628,6 +630,7 @@ impl PackageTemplate {
                                return Some(bumped_tx);
                        },
                        PackageMalleability::Untractable => {
+                               debug_assert_eq!(value, 0, "value is ignored for non-malleable packages, should be zero to ensure callsites are correct");
                                if let Some((outpoint, outp)) = self.inputs.first() {
                                        if let Some(final_tx) = outp.get_finalized_tx(outpoint, onchain_handler) {
                                                log_trace!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);
@@ -653,10 +656,12 @@ impl PackageTemplate {
                current_height + LOW_FREQUENCY_BUMP_INTERVAL
        }
        /// Returns value in satoshis to be included as package outgoing output amount and feerate with which package finalization should be done.
-       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)>
+       pub(crate) fn compute_package_output<F: Deref, L: Deref>(&self, predicted_weight: usize, fee_estimator: &F, logger: &L) -> Option<(u64, u64)>
                where F::Target: FeeEstimator,
                      L::Target: Logger,
        {
+               debug_assert!(self.malleability == PackageMalleability::Malleable, "The package output is fixed for non-malleable packages");
+               let input_amounts = self.package_amount();
                // If old feerate is 0, first iteration of this claim, use normal fee calculation
                if self.feerate_previous != 0 {
                        if let Some((new_fee, feerate)) = feerate_bump(predicted_weight, input_amounts, self.feerate_previous, fee_estimator, logger) {