]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Add assertions to ensure we don't use an invalid package_amount
authorMatt Corallo <git@bluematt.me>
Wed, 26 May 2021 15:47:29 +0000 (15:47 +0000)
committerMatt Corallo <git@bluematt.me>
Fri, 28 May 2021 23:56:44 +0000 (23:56 +0000)
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.

lightning/src/chain/onchaintx.rs
lightning/src/chain/package.rs

index 04de721f901a3a9e67ea01bca8ce2c6e3df3f362..9bbc2a4abd2e3e9a852c71e5e1d700d043fd4472 100644 (file)
@@ -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));
                        }
                }
index 311937a121a0822eb01cea3bfbd4e8d6b851cd8f..9e92e2031bc1afe90c0d90d1c7f2a9322b298521 100644 (file)
@@ -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) {