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..a53b94ad2c98d1f5f40cb15a93ccf6831976365b 100644 (file)
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -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, self.best_block.height,
                        );
                        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, _conf_height: u32) -> (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);
                        }

diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs
index 1b570d18e4153583a5b1b86cd7b48db29bf546d5..47fa48b63738c9d3b7e9dae7a09e7fb1ad4df496 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));
                        }
                }