Stop passing current height to `PackageTemplate::build_package`

Now that we don't store the confirmation height of the inputs
being spent, passing the current height to
`PackageTemplate::build_package` is useless - we only use it to set
the height at which we should next bump the fee, but we just want
it to be "next block", so we might as well use `0` and avoid the
extra argument. Further, in one case we were already passing `0`,
so passing the argument is just confusing as we can't rely on it
being set.

Note that this does remove an assertion that we never merge
packages that were crated at different heights, and in the future
we may wish to do that (as there's no specific reason not to), but
we do not currently change the behavior.

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index 86f0d3de5ed226666db6d7185bc11e72675a7663..21034f83b5f9029fc6d2857d77f33c547486c8eb 100644 (file)
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -3002,7 +3002,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                // Assume that the broadcasted commitment transaction confirmed in the current best
                                // block. Even if not, its a reasonable metric for the bump criteria on the HTLC
                                // transactions.
-                               let (claim_reqs, _) = self.get_broadcasted_holder_claims(&holder_commitment_tx, self.best_block.height);
+                               let (claim_reqs, _) = self.get_broadcasted_holder_claims(&holder_commitment_tx);
                                let conf_target = self.closure_conf_target();
                                self.onchain_tx_handler.update_claims_view_from_requests(claim_reqs, self.best_block.height, self.best_block.height, broadcaster, conf_target, fee_estimator, logger);
                        }
@@ -3018,7 +3018,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                let commitment_package = PackageTemplate::build_package(
                        self.funding_info.0.txid.clone(), self.funding_info.0.index as u32,
                        PackageSolvingData::HolderFundingOutput(funding_outp),
-                       self.best_block.height, self.best_block.height
+                       self.best_block.height,
                );
                let mut claimable_outpoints = vec![commitment_package];
                let event = MonitorEvent::HolderForceClosedWithInfo {
@@ -3041,7 +3041,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        // assuming it gets confirmed in the next block. Sadly, we have code which considers
                        // "not yet confirmed" things as discardable, so we cannot do that here.
                        let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(
-                               &self.current_holder_commitment_tx, self.best_block.height
+                               &self.current_holder_commitment_tx,
                        );
                        let unsigned_commitment_tx = self.onchain_tx_handler.get_unsigned_holder_commitment_tx();
                        let new_outputs = self.get_broadcasted_holder_watch_outputs(
@@ -3459,7 +3459,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        for (idx, outp) in tx.output.iter().enumerate() {
                                if outp.script_pubkey == revokeable_p2wsh {
                                        let revk_outp = RevokedOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, outp.value, self.counterparty_commitment_params.on_counterparty_tx_csv, self.onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx());
-                                       let justice_package = PackageTemplate::build_package(commitment_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp), height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, height);
+                                       let justice_package = PackageTemplate::build_package(
+                                               commitment_txid, idx as u32,
+                                               PackageSolvingData::RevokedOutput(revk_outp),
+                                               height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32,
+                                       );
                                        claimable_outpoints.push(justice_package);
                                        to_counterparty_output_info =
                                                Some((idx.try_into().expect("Txn can't have more than 2^32 outputs"), outp.value));
@@ -3476,7 +3480,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                                        return (claimable_outpoints, to_counterparty_output_info);
                                                }
                                                let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), &self.onchain_tx_handler.channel_transaction_parameters.channel_type_features);
-                                               let justice_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp), htlc.cltv_expiry, height);
+                                               let justice_package = PackageTemplate::build_package(
+                                                       commitment_txid,
+                                                       transaction_output_index,
+                                                       PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp),
+                                                       htlc.cltv_expiry,
+                                               );
                                                claimable_outpoints.push(justice_package);
                                        }
                                }
@@ -3598,7 +3607,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                                                self.counterparty_commitment_params.counterparty_htlc_base_key,
                                                                htlc.clone(), self.onchain_tx_handler.channel_type_features().clone()))
                                        };
-                                       let counterparty_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, counterparty_htlc_outp, htlc.cltv_expiry, 0);
+                                       let counterparty_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, counterparty_htlc_outp, htlc.cltv_expiry);
                                        claimable_outpoints.push(counterparty_package);
                                }
                        }
@@ -3642,7 +3651,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                );
                                let justice_package = PackageTemplate::build_package(
                                        htlc_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp),
-                                       height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, height
+                                       height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32,
                                );
                                claimable_outpoints.push(justice_package);
                                if outputs_to_watch.is_none() {
@@ -3657,7 +3666,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
        // Returns (1) `PackageTemplate`s that can be given to the OnchainTxHandler, so that the handler can
        // broadcast transactions claiming holder HTLC commitment outputs and (2) a holder revokable
        // script so we can detect whether a holder transaction has been seen on-chain.
-       fn get_broadcasted_holder_claims(&self, holder_tx: &HolderSignedTx, conf_height: u32) -> (Vec<PackageTemplate>, Option<(ScriptBuf, PublicKey, RevocationKey)>) {
+       fn get_broadcasted_holder_claims(&self, holder_tx: &HolderSignedTx) -> (Vec<PackageTemplate>, Option<(ScriptBuf, PublicKey, RevocationKey)>) {
                let mut claim_requests = Vec::with_capacity(holder_tx.htlc_outputs.len());
 
                let redeemscript = chan_utils::get_revokeable_redeemscript(&holder_tx.revocation_key, self.on_holder_tx_csv, &holder_tx.delayed_payment_key);
@@ -3685,7 +3694,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                                let htlc_package = PackageTemplate::build_package(
                                        holder_tx.txid, transaction_output_index,
                                        PackageSolvingData::HolderHTLCOutput(htlc_output),
-                                       htlc.cltv_expiry, conf_height
+                                       htlc.cltv_expiry,
                                );
                                claim_requests.push(htlc_package);
                        }
@@ -3728,7 +3737,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                if self.current_holder_commitment_tx.txid == commitment_txid {
                        is_holder_tx = true;
                        log_info!(logger, "Got broadcast of latest holder commitment tx {}, searching for available HTLCs to claim", commitment_txid);
-                       let res = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, height);
+                       let res = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx);
                        let mut to_watch = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, tx);
                        append_onchain_update!(res, to_watch);
                        fail_unbroadcast_htlcs!(self, "latest holder", commitment_txid, tx, height,
@@ -3738,7 +3747,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        if holder_tx.txid == commitment_txid {
                                is_holder_tx = true;
                                log_info!(logger, "Got broadcast of previous holder commitment tx {}, searching for available HTLCs to claim", commitment_txid);
-                               let res = self.get_broadcasted_holder_claims(holder_tx, height);
+                               let res = self.get_broadcasted_holder_claims(holder_tx);
                                let mut to_watch = self.get_broadcasted_holder_watch_outputs(holder_tx, tx);
                                append_onchain_update!(res, to_watch);
                                fail_unbroadcast_htlcs!(self, "previous holder", commitment_txid, tx, height, block_hash,

diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs
index 95d21210f7c8c522f0ed9d78acfff579e775e891..2dcaa88002c79a5c68d7779fc9ba0d65edff00f7 100644 (file)
--- a/lightning/src/chain/package.rs
+++ b/lightning/src/chain/package.rs
@@ -1029,7 +1029,7 @@ impl PackageTemplate {
                }).is_some()
        }
 
-       pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, soonest_conf_deadline: u32, first_bump: u32) -> Self {
+       pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, soonest_conf_deadline: u32) -> Self {
                let (malleability, aggregable) = PackageSolvingData::map_output_type_flags(&input_solving_data);
                let inputs = vec![(BitcoinOutPoint { txid, vout }, input_solving_data)];
                PackageTemplate {
@@ -1038,7 +1038,7 @@ impl PackageTemplate {
                        soonest_conf_deadline,
                        aggregable,
                        feerate_previous: 0,
-                       height_timer: first_bump,
+                       height_timer: 0,
                }
        }
 }
@@ -1253,18 +1253,6 @@ mod tests {
                }
        }
 
-       #[test]
-       #[should_panic]
-       fn test_package_differing_heights() {
-               let txid = Txid::from_str("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
-               let secp_ctx = Secp256k1::new();
-               let revk_outp = dumb_revk_output!(secp_ctx, false);
-
-               let mut package_one_hundred = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, 100);
-               let package_two_hundred = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, 200);
-               package_one_hundred.merge_package(package_two_hundred);
-       }
-
        #[test]
        #[should_panic]
        fn test_package_untractable_merge_to() {
@@ -1273,8 +1261,8 @@ mod tests {
                let revk_outp = dumb_revk_output!(secp_ctx, false);
                let htlc_outp = dumb_htlc_output!();
 
-               let mut untractable_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, 100);
-               let malleable_package = PackageTemplate::build_package(txid, 1, htlc_outp.clone(), 1000, 100);
+               let mut untractable_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000);
+               let malleable_package = PackageTemplate::build_package(txid, 1, htlc_outp.clone(), 1000);
                untractable_package.merge_package(malleable_package);
        }
 
@@ -1286,8 +1274,8 @@ mod tests {
                let htlc_outp = dumb_htlc_output!();
                let revk_outp = dumb_revk_output!(secp_ctx, false);
 
-               let mut malleable_package = PackageTemplate::build_package(txid, 0, htlc_outp.clone(), 1000, 100);
-               let untractable_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, 100);
+               let mut malleable_package = PackageTemplate::build_package(txid, 0, htlc_outp.clone(), 1000);
+               let untractable_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000);
                malleable_package.merge_package(untractable_package);
        }
 
@@ -1299,8 +1287,8 @@ mod tests {
                let revk_outp = dumb_revk_output!(secp_ctx, false);
                let revk_outp_counterparty_balance = dumb_revk_output!(secp_ctx, true);
 
-               let mut noaggregation_package = PackageTemplate::build_package(txid, 0, revk_outp_counterparty_balance.clone(), 1000, 100);
-               let aggregation_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, 100);
+               let mut noaggregation_package = PackageTemplate::build_package(txid, 0, revk_outp_counterparty_balance.clone(), 1000);
+               let aggregation_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000);
                noaggregation_package.merge_package(aggregation_package);
        }
 
@@ -1312,8 +1300,8 @@ mod tests {
                let revk_outp = dumb_revk_output!(secp_ctx, false);
                let revk_outp_counterparty_balance = dumb_revk_output!(secp_ctx, true);
 
-               let mut aggregation_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, 100);
-               let noaggregation_package = PackageTemplate::build_package(txid, 1, revk_outp_counterparty_balance.clone(), 1000, 100);
+               let mut aggregation_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000);
+               let noaggregation_package = PackageTemplate::build_package(txid, 1, revk_outp_counterparty_balance.clone(), 1000);
                aggregation_package.merge_package(noaggregation_package);
        }
 
@@ -1324,9 +1312,9 @@ mod tests {
                let secp_ctx = Secp256k1::new();
                let revk_outp = dumb_revk_output!(secp_ctx, false);
 
-               let mut empty_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, 100);
+               let mut empty_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000);
                empty_package.inputs = vec![];
-               let package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, 100);
+               let package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000);
                empty_package.merge_package(package);
        }
 
@@ -1338,8 +1326,8 @@ mod tests {
                let revk_outp = dumb_revk_output!(secp_ctx, false);
                let counterparty_outp = dumb_counterparty_output!(secp_ctx, 0, ChannelTypeFeatures::only_static_remote_key());
 
-               let mut revoked_package = PackageTemplate::build_package(txid, 0, revk_outp, 1000, 100);
-               let counterparty_package = PackageTemplate::build_package(txid, 1, counterparty_outp, 1000, 100);
+               let mut revoked_package = PackageTemplate::build_package(txid, 0, revk_outp, 1000);
+               let counterparty_package = PackageTemplate::build_package(txid, 1, counterparty_outp, 1000);
                revoked_package.merge_package(counterparty_package);
        }
 
@@ -1351,9 +1339,9 @@ mod tests {
                let revk_outp_two = dumb_revk_output!(secp_ctx, false);
                let revk_outp_three = dumb_revk_output!(secp_ctx, false);
 
-               let mut package_one = PackageTemplate::build_package(txid, 0, revk_outp_one, 1000, 100);
-               let package_two = PackageTemplate::build_package(txid, 1, revk_outp_two, 1000, 100);
-               let package_three = PackageTemplate::build_package(txid, 2, revk_outp_three, 1000, 100);
+               let mut package_one = PackageTemplate::build_package(txid, 0, revk_outp_one, 1000);
+               let package_two = PackageTemplate::build_package(txid, 1, revk_outp_two, 1000);
+               let package_three = PackageTemplate::build_package(txid, 2, revk_outp_three, 1000);
 
                package_one.merge_package(package_two);
                package_one.merge_package(package_three);
@@ -1375,7 +1363,7 @@ mod tests {
                let txid = Txid::from_str("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
                let htlc_outp_one = dumb_htlc_output!();
 
-               let mut package_one = PackageTemplate::build_package(txid, 0, htlc_outp_one, 1000, 100);
+               let mut package_one = PackageTemplate::build_package(txid, 0, htlc_outp_one, 1000);
                let ret_split = package_one.split_package(&BitcoinOutPoint { txid, vout: 0});
                assert!(ret_split.is_none());
        }
@@ -1386,8 +1374,8 @@ mod tests {
                let secp_ctx = Secp256k1::new();
                let revk_outp = dumb_revk_output!(secp_ctx, false);
 
-               let mut package = PackageTemplate::build_package(txid, 0, revk_outp, 1000, 100);
-               assert_eq!(package.timer(), 100);
+               let mut package = PackageTemplate::build_package(txid, 0, revk_outp, 1000);
+               assert_eq!(package.timer(), 0);
                package.set_timer(101);
                assert_eq!(package.timer(), 101);
        }
@@ -1398,7 +1386,7 @@ mod tests {
                let secp_ctx = Secp256k1::new();
                let counterparty_outp = dumb_counterparty_output!(secp_ctx, 1_000_000, ChannelTypeFeatures::only_static_remote_key());
 
-               let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000, 100);
+               let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000);
                assert_eq!(package.package_amount(), 1000);
        }
 
@@ -1412,14 +1400,14 @@ mod tests {
 
                {
                        let revk_outp = dumb_revk_output!(secp_ctx, false);
-                       let package = PackageTemplate::build_package(txid, 0, revk_outp, 0, 100);
+                       let package = PackageTemplate::build_package(txid, 0, revk_outp, 0);
                        assert_eq!(package.package_weight(&ScriptBuf::new()),  weight_sans_output + WEIGHT_REVOKED_OUTPUT);
                }
 
                {
                        for channel_type_features in [ChannelTypeFeatures::only_static_remote_key(), ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()].iter() {
                                let counterparty_outp = dumb_counterparty_output!(secp_ctx, 1_000_000, channel_type_features.clone());
-                               let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000, 100);
+                               let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000);
                                assert_eq!(package.package_weight(&ScriptBuf::new()), weight_sans_output + weight_received_htlc(channel_type_features));
                        }
                }
@@ -1427,7 +1415,7 @@ mod tests {
                {
                        for channel_type_features in [ChannelTypeFeatures::only_static_remote_key(), ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()].iter() {
                                let counterparty_outp = dumb_counterparty_offered_output!(secp_ctx, 1_000_000, channel_type_features.clone());
-                               let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000, 100);
+                               let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000);
                                assert_eq!(package.package_weight(&ScriptBuf::new()), weight_sans_output + weight_offered_htlc(channel_type_features));
                        }
                }