Consider dust threshold for fee rate determination

Previously, the `feerate_bump` method did not enforce the dust
threshold, which could result in us thinking we had raised the fee
rate without actually having done so. Instead,
`compute_package_output` blindly accepted the updated fee rate while
enforcing a non-dust output value, resulting in repeated broadcast
attempts of an identical transaction.

Author: arik-so

lightning/src/chain/onchaintx.rs

@@ -215,6 +215,7 @@ pub(crate) enum OnchainClaim {
 }
 
 /// Represents the different feerate strategies a pending request can use when generating a claim.
+#[derive(Debug)]
 pub(crate) enum FeerateStrategy {
 	/// We must reuse the most recently used feerate, if any.
 	RetryPrevious,

lightning/src/chain/package.rs

@@ -1117,10 +1117,10 @@ impl PackageTemplate {
 		// 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, feerate_strategy,
-				conf_target, fee_estimator, logger,
+				predicted_weight, input_amounts, dust_limit_sats, self.feerate_previous,
+				feerate_strategy, conf_target, fee_estimator, logger,
 			) {
-				return Some((cmp::max(input_amounts as i64 - new_fee as i64, dust_limit_sats as i64) as u64, feerate));
+				return Some((input_amounts.saturating_sub(new_fee), feerate));
 			}
 		} else {
 			if let Some((new_fee, feerate)) = compute_fee_from_spent_amounts(input_amounts, predicted_weight, conf_target, fee_estimator, logger) {
@@ -1270,16 +1270,20 @@ fn compute_fee_from_spent_amounts<F: Deref, L: Logger>(
 /// respect BIP125 rules 3) and 4) and if required adjust the new fee to meet the RBF policy
 /// requirement.
 fn feerate_bump<F: Deref, L: Logger>(
-	predicted_weight: u64, input_amounts: u64, previous_feerate: u64, feerate_strategy: &FeerateStrategy,
-	conf_target: ConfirmationTarget, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
+	predicted_weight: u64, input_amounts: u64, dust_limit_sats: u64, previous_feerate: u64,
+	feerate_strategy: &FeerateStrategy, conf_target: ConfirmationTarget,
+	fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &L,
 ) -> Option<(u64, u64)>
 where
 	F::Target: FeeEstimator,
 {
+	let previous_fee = previous_feerate * predicted_weight / 1000;
+
 	// If old feerate inferior to actual one given back by Fee Estimator, use it to compute new fee...
 	let (new_fee, new_feerate) = if let Some((new_fee, new_feerate)) =
 		compute_fee_from_spent_amounts(input_amounts, predicted_weight, conf_target, fee_estimator, logger)
 	{
+		log_debug!(logger, "Initiating fee rate bump from {} s/kWU ({} s) to {} s/kWU ({} s) using {:?} strategy", previous_feerate, previous_fee, new_feerate, new_fee, feerate_strategy);
 		match feerate_strategy {
 			FeerateStrategy::RetryPrevious => {
 				let previous_fee = previous_feerate * predicted_weight / 1000;
@@ -1297,15 +1301,12 @@ where
 				// ...else just increase the previous feerate by 25% (because that's a nice number)
 				let bumped_feerate = previous_feerate + (previous_feerate / 4);
 				let bumped_fee = bumped_feerate * predicted_weight / 1000;
-				if input_amounts <= bumped_fee {
-					log_warn!(logger, "Can't 25% bump new claiming tx, amount {} is too small", input_amounts);
-					return None;
-				}
+
 				(bumped_fee, bumped_feerate)
 			},
 		}
 	} else {
-		log_warn!(logger, "Can't new-estimation bump new claiming tx, amount {} is too small", input_amounts);
+		log_warn!(logger, "Can't bump new claiming tx, input amount {} is too small", input_amounts);
 		return None;
 	};
 
@@ -1316,17 +1317,26 @@ where
 		return Some((new_fee, new_feerate));
 	}
 
-	let previous_fee = previous_feerate * predicted_weight / 1000;
 	let min_relay_fee = INCREMENTAL_RELAY_FEE_SAT_PER_1000_WEIGHT * predicted_weight / 1000;
 	// BIP 125 Opt-in Full Replace-by-Fee Signaling
 	// 	* 3. The replacement transaction pays an absolute fee of at least the sum paid by the original transactions.
 	//	* 4. The replacement transaction must also pay for its own bandwidth at or above the rate set by the node's minimum relay fee setting.
-	let new_fee = if new_fee < previous_fee + min_relay_fee {
-		new_fee + previous_fee + min_relay_fee - new_fee
-	} else {
-		new_fee
-	};
-	Some((new_fee, new_fee * 1000 / predicted_weight))
+	let naive_new_fee = new_fee;
+	let new_fee = cmp::max(new_fee, previous_fee + min_relay_fee);
+
+	if new_fee > naive_new_fee {
+		log_debug!(logger, "Naive fee bump of {}s does not meet min relay fee requirements of {}s", naive_new_fee - previous_fee, min_relay_fee);
+	}
+
+	let remaining_output_amount = input_amounts.saturating_sub(new_fee);
+	if remaining_output_amount < dust_limit_sats {
+		log_warn!(logger, "Can't bump new claiming tx, output amount {} would end up below dust threshold {}", remaining_output_amount, dust_limit_sats);
+		return None;
+	}
+
+	let new_feerate = new_fee * 1000 / predicted_weight;
+	log_debug!(logger, "Fee rate bumped by {}s from {} s/KWU ({} s) to {} s/KWU ({} s)", new_fee - previous_fee, previous_feerate, previous_fee, new_feerate, new_fee);
+	Some((new_fee, new_feerate))
 }
 
 #[cfg(test)]