Merge pull request #1011 from TheBlueMatt/2021-07-new-closing-fee

Clean up existing and add range-based closing_signed negotiation

Author: TheBlueMatt

fuzz/src/msg_targets/gen_target.sh

@@ -14,7 +14,6 @@ echo "mod utils;" > mod.rs
 GEN_TEST AcceptChannel test_msg ""
 GEN_TEST AnnouncementSignatures test_msg ""
 GEN_TEST ChannelReestablish test_msg ""
-GEN_TEST ClosingSigned test_msg ""
 GEN_TEST CommitmentSigned test_msg ""
 GEN_TEST DecodedOnionErrorPacket test_msg ""
 GEN_TEST FundingCreated test_msg ""
@@ -40,6 +39,7 @@ GEN_TEST UpdateAddHTLC test_msg_hole ", 85, 33"
 GEN_TEST ErrorMessage test_msg_hole ", 32, 2"
 GEN_TEST ChannelUpdate test_msg_hole ", 108, 1"
 
+GEN_TEST ClosingSigned test_msg_simple ""
 GEN_TEST Init test_msg_simple ""
 GEN_TEST OnionHopData test_msg_simple ""
 GEN_TEST Ping test_msg_simple ""

fuzz/src/msg_targets/mod.rs

@@ -2,7 +2,6 @@ mod utils;
 pub mod msg_accept_channel;
 pub mod msg_announcement_signatures;
 pub mod msg_channel_reestablish;
-pub mod msg_closing_signed;
 pub mod msg_commitment_signed;
 pub mod msg_decoded_onion_error_packet;
 pub mod msg_funding_created;
@@ -25,6 +24,7 @@ pub mod msg_gossip_timestamp_filter;
 pub mod msg_update_add_htlc;
 pub mod msg_error_message;
 pub mod msg_channel_update;
+pub mod msg_closing_signed;
 pub mod msg_init;
 pub mod msg_onion_hop_data;
 pub mod msg_ping;

fuzz/src/msg_targets/msg_closing_signed.rs

@@ -17,11 +17,11 @@ use utils::test_logger;
 
 #[inline]
 pub fn msg_closing_signed_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
-	test_msg!(msgs::ClosingSigned, data);
+	test_msg_simple!(msgs::ClosingSigned, data);
 }
 
 #[no_mangle]
 pub extern "C" fn msg_closing_signed_run(data: *const u8, datalen: usize) {
 	let data = unsafe { std::slice::from_raw_parts(data, datalen) };
-	test_msg!(msgs::ClosingSigned, data);
+	test_msg_simple!(msgs::ClosingSigned, data);
 }

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -142,7 +142,7 @@ fn test_monitor_and_persister_update_fail() {
 	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
 	nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &updates.update_fulfill_htlcs[0]);
 	if let Some(ref mut channel) = nodes[0].node.channel_state.lock().unwrap().by_id.get_mut(&chan.2) {
-		if let Ok((_, _, _, update)) = channel.commitment_signed(&updates.commitment_signed, &node_cfgs[0].fee_estimator, &node_cfgs[0].logger) {
+		if let Ok((_, _, update)) = channel.commitment_signed(&updates.commitment_signed, &node_cfgs[0].logger) {
 			// Check that even though the persister is returning a TemporaryFailure,
 			// because the update is bogus, ultimately the error that's returned
 			// should be a PermanentFailure.
@@ -2561,8 +2561,8 @@ fn test_reconnect_dup_htlc_claims() {
 
 #[test]
 fn test_temporary_error_during_shutdown() {
-	// Test that temporary failures when updating the monitor's shutdown script do not prevent
-	// cooperative close.
+	// Test that temporary failures when updating the monitor's shutdown script delay cooperative
+	// close.
 	let mut config = test_default_channel_config();
 	config.channel_options.commit_upfront_shutdown_pubkey = false;
 
@@ -2575,9 +2575,41 @@ fn test_temporary_error_during_shutdown() {
 
 	*nodes[0].chain_monitor.update_ret.lock().unwrap() = Some(Err(ChannelMonitorUpdateErr::TemporaryFailure));
 	*nodes[1].chain_monitor.update_ret.lock().unwrap() = Some(Err(ChannelMonitorUpdateErr::TemporaryFailure));
-	close_channel(&nodes[0], &nodes[1], &channel_id, funding_tx, false);
-	check_added_monitors!(nodes[0], 1);
+
+	nodes[0].node.close_channel(&channel_id).unwrap();
+	nodes[1].node.handle_shutdown(&nodes[0].node.get_our_node_id(), &InitFeatures::known(), &get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id()));
 	check_added_monitors!(nodes[1], 1);
+
+	nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id()));
+	check_added_monitors!(nodes[0], 1);
+
+	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+
+	*nodes[0].chain_monitor.update_ret.lock().unwrap() = None;
+	*nodes[1].chain_monitor.update_ret.lock().unwrap() = None;
+
+	let (outpoint, latest_update) = nodes[0].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
+	nodes[0].node.channel_monitor_updated(&outpoint, latest_update);
+	nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendClosingSigned, nodes[1].node.get_our_node_id()));
+
+	assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+	*nodes[1].chain_monitor.update_ret.lock().unwrap() = None;
+	let (outpoint, latest_update) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
+	nodes[1].node.channel_monitor_updated(&outpoint, latest_update);
+
+	nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendClosingSigned, nodes[0].node.get_our_node_id()));
+	let (_, closing_signed_a) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
+	let txn_a = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
+
+	nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &closing_signed_a.unwrap());
+	let (_, none_b) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
+	assert!(none_b.is_none());
+	let txn_b = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
+
+	assert_eq!(txn_a, txn_b);
+	assert_eq!(txn_a.len(), 1);
+	check_spends!(txn_a[0], funding_tx);
 }
 
 #[test]