onchaind: cap RBF penalty fee for testnet/regtest

On testnet I noticed if we can't reach bitcoind for some reason, we'll
keep RBFing our penalty tx ("it didn't go in, RBF harder!!").  Makes
no sense to grossly exceed the amount needed for next block, so simply
cap penalty at 2x "estimatesmartfee 2 CONSERVATIVE".

Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

lightningd/onchain_control.c

@@ -614,7 +614,7 @@ enum watch_result onchaind_funding_spent(struct channel *channel,
 	struct lightningd *ld = channel->peer->ld;
 	struct pubkey final_key;
 	int hsmfd;
-	u32 feerates[3];
+	u32 feerates[4];
 	enum state_change reason;
 
 	/* use REASON_ONCHAIN or closer's reason, if known */
@@ -730,6 +730,9 @@ enum watch_result onchaind_funding_spent(struct channel *channel,
 				feerates[i] = feerate_floor();
 		}
 	}
+	/* This is 10x highest bitcoind estimate (depending on dev-max-fee-multiplier),
+	 * so cap at 2x */
+	feerates[3] = feerate_max(ld, NULL) / 5;
 
 	log_debug(channel->log, "channel->static_remotekey_start[LOCAL] %"PRIu64,
 		  channel->static_remotekey_start[LOCAL]);
@@ -749,8 +752,8 @@ enum watch_result onchaind_funding_spent(struct channel *channel,
 				    * we specify theirs. */
 				  channel->channel_info.their_config.to_self_delay,
 				  channel->our_config.to_self_delay,
-				  /* delayed_to_us, htlc, and penalty. */
-				  feerates[0], feerates[1], feerates[2],
+				  /* delayed_to_us, htlc, penalty, and penalty_max. */
+				  feerates[0], feerates[1], feerates[2], feerates[3],
 				  channel->our_config.dust_limit,
 				  &our_last_txid,
 				  channel->shutdown_scriptpubkey[LOCAL],

onchaind/onchaind.c

@@ -43,7 +43,7 @@ static u32 delayed_to_us_feerate;
 static u32 htlc_feerate;
 
 /* The feerate for transactions spending from revoked transactions. */
-static u32 penalty_feerate;
+static u32 penalty_feerate, max_penalty_feerate;
 
 /* Min and max feerates we ever used */
 static u32 min_possible_feerate, max_possible_feerate;
@@ -878,10 +878,19 @@ compute_penalty_output_amount(struct amount_sat initial_amount,
 	struct amount_sat max_output_amount;
 	struct amount_sat output_amount;
 	struct amount_sat deducted_amount;
+	struct amount_sat min_output_amount, max_fee;
 
 	assert(depth <= max_depth);
 	assert(depth > 0);
 
+	/* We never pay more than max_penalty_feerate; at some point,
+	 * it's clearly not working. */
+	max_fee = amount_tx_fee(max_penalty_feerate, weight);
+	if (!amount_sat_sub(&min_output_amount, initial_amount, max_fee))
+		/* We may just donate the whole output as fee, meaning
+		 * we get zero amount. */
+		min_output_amount = AMOUNT_SAT(0);
+
 	/* The difference between initial_amount, and the fee suggested
 	 * by min_rbf_bump, is the largest allowed output amount.
 	 *
@@ -892,11 +901,7 @@ compute_penalty_output_amount(struct amount_sat initial_amount,
 	 */
 	if (!amount_sat_sub(&max_output_amount,
 			    initial_amount, min_rbf_bump(weight, depth - 1)))
-		/* If min_rbf_bump is larger than the initial_amount,
-		 * we should just donate the whole output as fee,
-		 * meaning we get 0 output amount.
-		 */
-		return AMOUNT_SAT(0);
+		return min_output_amount;
 
 	/* Map the depth / max_depth into a number between 0->1.  */
 	double x = (double) depth / (double) max_depth;
@@ -910,9 +915,14 @@ compute_penalty_output_amount(struct amount_sat initial_amount,
 
 	/* output_amount = initial_amount - deducted_amount.  */
 	if (!amount_sat_sub(&output_amount,
-			    initial_amount, deducted_amount))
-		/* If underflow, force to 0.  */
-		output_amount = AMOUNT_SAT(0);
+			    initial_amount, deducted_amount)) {
+		/* If underflow, force to min.  */
+		output_amount = min_output_amount;
+	}
+
+	/* If output below min, return min. */
+	if (amount_sat_less(output_amount, min_output_amount))
+		return min_output_amount;
 
 	/* If output exceeds max, return max.  */
 	if (amount_sat_less(max_output_amount, output_amount))
@@ -3908,6 +3918,7 @@ int main(int argc, char *argv[])
 				   &delayed_to_us_feerate,
 				   &htlc_feerate,
 				   &penalty_feerate,
+				   &max_penalty_feerate,
 				   &dust_limit,
 				   &our_broadcast_txid,
 				   &scriptpubkey[LOCAL],

onchaind/onchaind_wire.csv

@@ -23,6 +23,7 @@ msgdata,onchaind_init,remote_to_self_delay,u32,
 msgdata,onchaind_init,delayed_to_us_feerate,u32,
 msgdata,onchaind_init,htlc_feerate,u32,
 msgdata,onchaind_init,penalty_feerate,u32,
+msgdata,onchaind_init,max_penalty_feerate,u32,
 msgdata,onchaind_init,local_dust_limit_satoshi,amount_sat,
 # Gives an easy way to tell if it's our unilateral close or theirs...
 msgdata,onchaind_init,our_broadcast_txid,bitcoin_txid,

onchaind/test/run-grind_feerate-bug.c

@@ -49,7 +49,7 @@ bool fromwire_onchaind_dev_memleak(const void *p UNNEEDED)
 bool fromwire_onchaind_htlcs(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct htlc_stub **htlc UNNEEDED, bool **tell_if_missing UNNEEDED, bool **tell_immediately UNNEEDED)
 { fprintf(stderr, "fromwire_onchaind_htlcs called!\n"); abort(); }
 /* Generated stub for fromwire_onchaind_init */
-bool fromwire_onchaind_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct amount_msat *our_msat UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, u32 *ourwallet_index UNNEEDED, struct ext_key *ourwallet_ext_key UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *opener UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct tx_parts **tx_parts UNNEEDED, u32 *locktime UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, struct bitcoin_signature **htlc_signature UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, struct pubkey *local_funding_pubkey UNNEEDED, struct pubkey *remote_funding_pubkey UNNEEDED, u64 *local_static_remotekey_start UNNEEDED, u64 *remote_static_remotekey_start UNNEEDED, bool *option_anchor_outputs UNNEEDED, u32 *min_relay_feerate UNNEEDED)
+bool fromwire_onchaind_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct amount_msat *our_msat UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, u32 *max_penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, u32 *ourwallet_index UNNEEDED, struct ext_key *ourwallet_ext_key UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *opener UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct tx_parts **tx_parts UNNEEDED, u32 *locktime UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, struct bitcoin_signature **htlc_signature UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, struct pubkey *local_funding_pubkey UNNEEDED, struct pubkey *remote_funding_pubkey UNNEEDED, u64 *local_static_remotekey_start UNNEEDED, u64 *remote_static_remotekey_start UNNEEDED, bool *option_anchor_outputs UNNEEDED, u32 *min_relay_feerate UNNEEDED)
 { fprintf(stderr, "fromwire_onchaind_init called!\n"); abort(); }
 /* Generated stub for fromwire_onchaind_known_preimage */
 bool fromwire_onchaind_known_preimage(const void *p UNNEEDED, struct preimage *preimage UNNEEDED)

onchaind/test/run-grind_feerate.c

@@ -54,7 +54,7 @@ bool fromwire_onchaind_dev_memleak(const void *p UNNEEDED)
 bool fromwire_onchaind_htlcs(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct htlc_stub **htlc UNNEEDED, bool **tell_if_missing UNNEEDED, bool **tell_immediately UNNEEDED)
 { fprintf(stderr, "fromwire_onchaind_htlcs called!\n"); abort(); }
 /* Generated stub for fromwire_onchaind_init */
-bool fromwire_onchaind_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct amount_msat *our_msat UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, u32 *ourwallet_index UNNEEDED, struct ext_key *ourwallet_ext_key UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *opener UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct tx_parts **tx_parts UNNEEDED, u32 *locktime UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, struct bitcoin_signature **htlc_signature UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, struct pubkey *local_funding_pubkey UNNEEDED, struct pubkey *remote_funding_pubkey UNNEEDED, u64 *local_static_remotekey_start UNNEEDED, u64 *remote_static_remotekey_start UNNEEDED, bool *option_anchor_outputs UNNEEDED, u32 *min_relay_feerate UNNEEDED)
+bool fromwire_onchaind_init(const tal_t *ctx UNNEEDED, const void *p UNNEEDED, struct shachain *shachain UNNEEDED, const struct chainparams **chainparams UNNEEDED, struct amount_sat *funding_amount_satoshi UNNEEDED, struct amount_msat *our_msat UNNEEDED, struct pubkey *old_remote_per_commitment_point UNNEEDED, struct pubkey *remote_per_commitment_point UNNEEDED, u32 *local_to_self_delay UNNEEDED, u32 *remote_to_self_delay UNNEEDED, u32 *delayed_to_us_feerate UNNEEDED, u32 *htlc_feerate UNNEEDED, u32 *penalty_feerate UNNEEDED, u32 *max_penalty_feerate UNNEEDED, struct amount_sat *local_dust_limit_satoshi UNNEEDED, struct bitcoin_txid *our_broadcast_txid UNNEEDED, u8 **local_scriptpubkey UNNEEDED, u8 **remote_scriptpubkey UNNEEDED, u32 *ourwallet_index UNNEEDED, struct ext_key *ourwallet_ext_key UNNEEDED, struct pubkey *ourwallet_pubkey UNNEEDED, enum side *opener UNNEEDED, struct basepoints *local_basepoints UNNEEDED, struct basepoints *remote_basepoints UNNEEDED, struct tx_parts **tx_parts UNNEEDED, u32 *locktime UNNEEDED, u32 *tx_blockheight UNNEEDED, u32 *reasonable_depth UNNEEDED, struct bitcoin_signature **htlc_signature UNNEEDED, u32 *min_possible_feerate UNNEEDED, u32 *max_possible_feerate UNNEEDED, struct pubkey **possible_remote_per_commit_point UNNEEDED, struct pubkey *local_funding_pubkey UNNEEDED, struct pubkey *remote_funding_pubkey UNNEEDED, u64 *local_static_remotekey_start UNNEEDED, u64 *remote_static_remotekey_start UNNEEDED, bool *option_anchor_outputs UNNEEDED, u32 *min_relay_feerate UNNEEDED)
 { fprintf(stderr, "fromwire_onchaind_init called!\n"); abort(); }
 /* Generated stub for fromwire_onchaind_known_preimage */
 bool fromwire_onchaind_known_preimage(const void *p UNNEEDED, struct preimage *preimage UNNEEDED)

tests/test_closing.py

@@ -1668,7 +1668,9 @@ def test_penalty_rbf_burn(node_factory, bitcoind, executor, chainparams):
                                may_fail=True, allow_broken_log=True)
     l2 = node_factory.get_node(options={'dev-disable-commit-after': 1,
                                         'watchtime-blocks': to_self_delay,
-                                        'plugin': coin_mvt_plugin})
+                                        'plugin': coin_mvt_plugin},
+                               # Exporbitant feerates mean we don't have cap on RBF!
+                               feerates=(15000000, 11000, 7500, 3750))
 
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     l1.fundchannel(l2, 10**7)