Bump default CSV delay on counterparty states to 7 days of blocks

Author: Antoine Riard

lightning/src/ln/functional_tests.rs

@@ -2395,11 +2395,11 @@ fn test_justice_tx_htlc_timeout() {
 	let mut alice_config = UserConfig::default();
 	alice_config.channel_handshake_config.announced_channel = true;
 	alice_config.channel_handshake_limits.force_announced_channel_preference = false;
-	alice_config.channel_handshake_config.our_to_self_delay = 6 * 24 * 5;
+	alice_config.channel_handshake_config.our_to_self_delay = 1008 + 288;
 	let mut bob_config = UserConfig::default();
 	bob_config.channel_handshake_config.announced_channel = true;
 	bob_config.channel_handshake_limits.force_announced_channel_preference = false;
-	bob_config.channel_handshake_config.our_to_self_delay = 6 * 24 * 3;
+	bob_config.channel_handshake_config.our_to_self_delay = 1008 + 144;
 	let user_cfgs = [Some(alice_config), Some(bob_config)];
 	let mut chanmon_cfgs = create_chanmon_cfgs(2);
 	chanmon_cfgs[0].keys_manager.disable_revocation_policy_check = true;
@@ -2458,11 +2458,11 @@ fn test_justice_tx_htlc_success() {
 	let mut alice_config = UserConfig::default();
 	alice_config.channel_handshake_config.announced_channel = true;
 	alice_config.channel_handshake_limits.force_announced_channel_preference = false;
-	alice_config.channel_handshake_config.our_to_self_delay = 6 * 24 * 5;
+	alice_config.channel_handshake_config.our_to_self_delay = 1008 + 288;
 	let mut bob_config = UserConfig::default();
 	bob_config.channel_handshake_config.announced_channel = true;
 	bob_config.channel_handshake_limits.force_announced_channel_preference = false;
-	bob_config.channel_handshake_config.our_to_self_delay = 6 * 24 * 3;
+	bob_config.channel_handshake_config.our_to_self_delay = 1008 + 144;
 	let user_cfgs = [Some(alice_config), Some(bob_config)];
 	let mut chanmon_cfgs = create_chanmon_cfgs(2);
 	chanmon_cfgs[0].keys_manager.disable_revocation_policy_check = true;
@@ -4299,13 +4299,13 @@ fn test_claim_sizeable_push_msat() {
 	assert_eq!(node_txn[0].output.len(), 2); // We can't force trimming of to_remote output as channel_reserve_satoshis block us to do so at channel opening
 
 	mine_transaction(&nodes[1], &node_txn[0]);
-	connect_blocks(&nodes[1], BREAKDOWN_TIMEOUT as u32 - 1);
+	connect_blocks(&nodes[1], (BREAKDOWN_TIMEOUT * 7) as u32 - 1);
 
 	let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
 	assert_eq!(spend_txn.len(), 1);
 	assert_eq!(spend_txn[0].input.len(), 1);
 	check_spends!(spend_txn[0], node_txn[0]);
-	assert_eq!(spend_txn[0].input[0].sequence.0, BREAKDOWN_TIMEOUT as u32);
+	assert_eq!(spend_txn[0].input[0].sequence.0, (BREAKDOWN_TIMEOUT * 7) as u32);
 }
 
 #[test]
@@ -4951,14 +4951,14 @@ fn test_dynamic_spendable_outputs_local_htlc_success_tx() {
 	};
 
 	mine_transaction(&nodes[1], &node_tx);
-	connect_blocks(&nodes[1], BREAKDOWN_TIMEOUT as u32 - 1);
+	connect_blocks(&nodes[1], (BREAKDOWN_TIMEOUT * 7) as u32 - 1);
 
 	// Verify that B is able to spend its own HTLC-Success tx thanks to spendable output event given back by its ChannelMonitor
 	let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager);
 	assert_eq!(spend_txn.len(), 1);
 	assert_eq!(spend_txn[0].input.len(), 1);
 	check_spends!(spend_txn[0], node_tx);
-	assert_eq!(spend_txn[0].input[0].sequence.0, BREAKDOWN_TIMEOUT as u32);
+	assert_eq!(spend_txn[0].input[0].sequence.0, (BREAKDOWN_TIMEOUT * 7) as u32);
 }
 
 fn do_test_fail_backwards_unrevoked_remote_announce(deliver_last_raa: bool, announce_latest: bool) {
@@ -5299,7 +5299,7 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 	};
 
 	mine_transaction(&nodes[0], &htlc_timeout);
-	connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
+	connect_blocks(&nodes[0], (BREAKDOWN_TIMEOUT * 7) as u32 - 1);
 	expect_payment_failed!(nodes[0], our_payment_hash, false);
 
 	// Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
@@ -5308,11 +5308,11 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() {
 	check_spends!(spend_txn[0], local_txn[0]);
 	assert_eq!(spend_txn[1].input.len(), 1);
 	check_spends!(spend_txn[1], htlc_timeout);
-	assert_eq!(spend_txn[1].input[0].sequence.0, BREAKDOWN_TIMEOUT as u32);
+	assert_eq!(spend_txn[1].input[0].sequence.0, (BREAKDOWN_TIMEOUT * 7) as u32);
 	assert_eq!(spend_txn[2].input.len(), 2);
 	check_spends!(spend_txn[2], local_txn[0], htlc_timeout);
-	assert!(spend_txn[2].input[0].sequence.0 == BREAKDOWN_TIMEOUT as u32 ||
-	        spend_txn[2].input[1].sequence.0 == BREAKDOWN_TIMEOUT as u32);
+	assert!(spend_txn[2].input[0].sequence.0 == (BREAKDOWN_TIMEOUT * 7) as u32 ||
+	        spend_txn[2].input[1].sequence.0 == (BREAKDOWN_TIMEOUT * 7) as u32);
 }
 
 #[test]
@@ -5385,7 +5385,7 @@ fn test_key_derivation_params() {
 	};
 
 	mine_transaction(&nodes[0], &htlc_timeout);
-	connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
+	connect_blocks(&nodes[0], (BREAKDOWN_TIMEOUT * 7) as u32 - 1);
 	expect_payment_failed!(nodes[0], our_payment_hash, false);
 
 	// Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor
@@ -5395,11 +5395,11 @@ fn test_key_derivation_params() {
 	check_spends!(spend_txn[0], local_txn_1[0]);
 	assert_eq!(spend_txn[1].input.len(), 1);
 	check_spends!(spend_txn[1], htlc_timeout);
-	assert_eq!(spend_txn[1].input[0].sequence.0, BREAKDOWN_TIMEOUT as u32);
+	assert_eq!(spend_txn[1].input[0].sequence.0, (BREAKDOWN_TIMEOUT * 7) as u32);
 	assert_eq!(spend_txn[2].input.len(), 2);
 	check_spends!(spend_txn[2], local_txn_1[0], htlc_timeout);
-	assert!(spend_txn[2].input[0].sequence.0 == BREAKDOWN_TIMEOUT as u32 ||
-	        spend_txn[2].input[1].sequence.0 == BREAKDOWN_TIMEOUT as u32);
+	assert!(spend_txn[2].input[0].sequence.0 == (BREAKDOWN_TIMEOUT * 7) as u32 ||
+	        spend_txn[2].input[1].sequence.0 == (BREAKDOWN_TIMEOUT * 7) as u32);
 }
 
 #[test]
@@ -5637,8 +5637,8 @@ fn bolt2_open_channel_sending_node_checks_part2() {
 	assert!(node0_to_1_send_open_channel.channel_flags<=1);
 
 	// BOLT #2 spec: Sending node should set to_self_delay sufficient to ensure the sender can irreversibly spend a commitment transaction output, in case of misbehaviour by the receiver.
-	assert!(BREAKDOWN_TIMEOUT>0);
-	assert!(node0_to_1_send_open_channel.to_self_delay==BREAKDOWN_TIMEOUT);
+	assert!((BREAKDOWN_TIMEOUT*7)>0);
+	assert!(node0_to_1_send_open_channel.to_self_delay==(BREAKDOWN_TIMEOUT*7));
 
 	// BOLT #2 spec: Sending node must ensure the chain_hash value identifies the chain it wishes to open the channel within.
 	let chain_hash=genesis_block(Network::Testnet).header.block_hash();
@@ -9245,7 +9245,7 @@ fn do_test_tx_confirmed_skipping_blocks_immediate_broadcast(test_height_before_t
 
 	let conf_height = nodes[1].best_block_info().1;
 	if !test_height_before_timelock {
-		connect_blocks(&nodes[1], 24 * 6);
+		connect_blocks(&nodes[1], (BREAKDOWN_TIMEOUT*7)  as u32);
 	}
 	nodes[1].chain_monitor.chain_monitor.transactions_confirmed(
 		&nodes[1].get_block_header(conf_height), &[(0, &node_txn[0])], conf_height);

lightning/src/ln/monitor_tests.rs

@@ -398,7 +398,7 @@ fn do_test_claim_value_force_close(prev_commitment_tx: bool) {
 
 	// Broadcast the closing transaction (which has both pending HTLCs in it) and get B's
 	// broadcasted HTLC claim transaction with preimage.
-	let node_b_commitment_claimable = nodes[1].best_block_info().1 + BREAKDOWN_TIMEOUT as u32;
+	let node_b_commitment_claimable = nodes[1].best_block_info().1 + (BREAKDOWN_TIMEOUT * 7) as u32;
 	mine_transaction(&nodes[0], &remote_txn[0]);
 	mine_transaction(&nodes[1], &remote_txn[0]);
 
@@ -513,7 +513,7 @@ fn do_test_claim_value_force_close(prev_commitment_tx: bool) {
 	// Node B will no longer consider the HTLC "contentious" after the HTLC claim transaction
 	// confirms, and consider it simply "awaiting confirmations". Note that it has to wait for the
 	// standard revocable transaction CSV delay before receiving a `SpendableOutputs`.
-	let node_b_htlc_claimable = nodes[1].best_block_info().1 + BREAKDOWN_TIMEOUT as u32;
+	let node_b_htlc_claimable = nodes[1].best_block_info().1 + (BREAKDOWN_TIMEOUT * 7) as u32;
 	mine_transaction(&nodes[1], &b_broadcast_txn[0]);
 
 	assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations {
@@ -633,7 +633,7 @@ fn test_balances_on_local_commitment_htlcs() {
 
 	// First confirm the commitment transaction on nodes[0], which should leave us with three
 	// claimable balances.
-	let node_a_commitment_claimable = nodes[0].best_block_info().1 + BREAKDOWN_TIMEOUT as u32;
+	let node_a_commitment_claimable = nodes[0].best_block_info().1 + (BREAKDOWN_TIMEOUT * 7) as u32;
 	mine_transaction(&nodes[0], &as_txn[0]);
 	check_added_monitors!(nodes[0], 1);
 	check_closed_broadcast!(nodes[0], true);
@@ -679,7 +679,7 @@ fn test_balances_on_local_commitment_htlcs() {
 
 	// Now confirm nodes[0]'s HTLC-Timeout transaction, which changes the claimable balance to an
 	// "awaiting confirmations" one.
-	let node_a_htlc_claimable = nodes[0].best_block_info().1 + BREAKDOWN_TIMEOUT as u32;
+	let node_a_htlc_claimable = nodes[0].best_block_info().1 + (BREAKDOWN_TIMEOUT * 7) as u32;
 	mine_transaction(&nodes[0], &as_txn[1]);
 	// Note that prior to the fix in the commit which introduced this test, this (and the next
 	// balance) check failed. With this check removed, the code panicked in the `connect_blocks`
@@ -813,7 +813,7 @@ fn test_no_preimage_inbound_htlc_balances() {
 
 	// Now close the channel by confirming A's commitment transaction on both nodes, checking the
 	// claimable balances remain the same except for the non-HTLC balance changing variant.
-	let node_a_commitment_claimable = nodes[0].best_block_info().1 + BREAKDOWN_TIMEOUT as u32;
+	let node_a_commitment_claimable = nodes[0].best_block_info().1 + (BREAKDOWN_TIMEOUT * 7) as u32;
 	let as_pre_spend_claims = sorted_vec(vec![Balance::ClaimableAwaitingConfirmations {
 			claimable_amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate *
 				(channel::commitment_tx_base_weight(opt_anchors) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000,
@@ -885,7 +885,7 @@ fn test_no_preimage_inbound_htlc_balances() {
 	// Now confirm the two HTLC timeout transactions for A, checking that the inbound HTLC resolves
 	// after ANTI_REORG_DELAY confirmations and the other takes BREAKDOWN_TIMEOUT confirmations.
 	mine_transaction(&nodes[0], &as_htlc_timeout_claim[0]);
-	let as_timeout_claimable_height = nodes[0].best_block_info().1 + (BREAKDOWN_TIMEOUT as u32) - 1;
+	let as_timeout_claimable_height = nodes[0].best_block_info().1 + (BREAKDOWN_TIMEOUT * 7) as u32 - 1;
 	assert_eq!(sorted_vec(vec![Balance::ClaimableAwaitingConfirmations {
 			claimable_amount_satoshis: 1_000_000 - 500_000 - 10_000 - chan_feerate *
 				(channel::commitment_tx_base_weight(opt_anchors) + 2 * channel::COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000,
@@ -1998,7 +1998,7 @@ fn test_yield_anchors_events() {
 
 	assert!(nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty());
 
-	connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32);
+	connect_blocks(&nodes[0], (BREAKDOWN_TIMEOUT * 7) as u32);
 
 	let holder_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
 	assert_eq!(holder_events.len(), 3);

lightning/src/ln/payment_tests.rs

@@ -706,7 +706,7 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool, co
 	connect_block(&nodes[0], &Block { header, txdata: vec![node_txn[1].clone()]});
 
 	if confirm_commitment_tx {
-		connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32 - 1);
+		connect_blocks(&nodes[0], (BREAKDOWN_TIMEOUT * 7) as u32 - 1);
 	}
 
 	header.prev_blockhash = nodes[0].best_block_hash();

lightning/src/util/config.rs

@@ -45,8 +45,11 @@ pub struct ChannelHandshakeConfig {
 	/// case of an honest unilateral channel close, which implicitly decrease the economic value of
 	/// our channel.
 	///
-	/// Default value: [`BREAKDOWN_TIMEOUT`], we enforce it as a minimum at channel opening so you
-	/// can tweak config to ask for more security, not less.
+	/// Default value: [`BREAKDOWN_TIMEOUT`], we enforce [`BREAKDOWN_TIMEOUT`] * 7 as a minimum at
+	/// channel opening so you can tweak config to ask for less security than the default of 7 days
+	/// of block. When setting this value, consider how long it may take to upgrade node(s) after
+	/// a bug was discovered a patch releaaed. While not all potential sources of error can be
+	/// recovered, some classes of bugs may allow this much time to react.
 	pub our_to_self_delay: u16,
 	/// Set to the smallest value HTLC we will accept to process.
 	///
@@ -189,7 +192,7 @@ impl Default for ChannelHandshakeConfig {
 	fn default() -> ChannelHandshakeConfig {
 		ChannelHandshakeConfig {
 			minimum_depth: 6,
-			our_to_self_delay: BREAKDOWN_TIMEOUT,
+			our_to_self_delay: BREAKDOWN_TIMEOUT * 7,
 			our_htlc_minimum_msat: 1,
 			max_inbound_htlc_value_in_flight_percent_of_channel: 10,
 			negotiate_scid_privacy: false,