X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=src%2Fln%2Fchannelmanager.rs;h=93c5be5604d2a0e1901ad7aa79a79c13ec764cd9;hb=6c1123cafd5e4821042e6f57efdf83af62e6fc6a;hp=c57486502a4ca77624b04d797087b68351411262;hpb=3f0bc24f1e0416c3c059dc9088d7667bdd3a283d;p=rust-lightning

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index c5748650..93c5be56 100644
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -2326,7 +2326,7 @@ mod tests {
 	use rand::{thread_rng,Rng};
 
 	use std::cell::RefCell;
-	use std::collections::HashMap;
+	use std::collections::{BTreeSet, HashMap};
 	use std::default::Default;
 	use std::rc::Rc;
 	use std::sync::{Arc, Mutex};
@@ -2648,6 +2648,17 @@ mod tests {
 		(chan_announcement.1, chan_announcement.2, chan_announcement.3, chan_announcement.4)
 	}
 
+	macro_rules! check_spends {
+		($tx: expr, $spends_tx: expr) => {
+			{
+				let mut funding_tx_map = HashMap::new();
+				let spends_tx = $spends_tx;
+				funding_tx_map.insert(spends_tx.txid(), spends_tx);
+				$tx.verify(&funding_tx_map).unwrap();
+			}
+		}
+	}
+
 	fn close_channel(outbound_node: &Node, inbound_node: &Node, channel_id: &[u8; 32], funding_tx: Transaction, close_inbound_first: bool) -> (msgs::ChannelUpdate, msgs::ChannelUpdate) {
 		let (node_a, broadcaster_a) = if close_inbound_first { (&inbound_node.node, &inbound_node.tx_broadcaster) } else { (&outbound_node.node, &outbound_node.tx_broadcaster) };
 		let (node_b, broadcaster_b) = if close_inbound_first { (&outbound_node.node, &outbound_node.tx_broadcaster) } else { (&inbound_node.node, &inbound_node.tx_broadcaster) };
@@ -2691,9 +2702,7 @@ mod tests {
 			tx_a = broadcaster_a.txn_broadcasted.lock().unwrap().remove(0);
 		}
 		assert_eq!(tx_a, tx_b);
-		let mut funding_tx_map = HashMap::new();
-		funding_tx_map.insert(funding_tx.txid(), funding_tx);
-		tx_a.verify(&funding_tx_map).unwrap();
+		check_spends!(tx_a, funding_tx);
 
 		let events_2 = node_a.get_and_clear_pending_events();
 		assert_eq!(events_2.len(), 1);
@@ -3153,6 +3162,168 @@ mod tests {
 		check_added_monitors!(nodes[1], 1);
 	}
 
+	#[test]
+	fn test_update_fee_unordered_raa() {
+		// Just the intro to the previous test followed by an out-of-order RAA (which caused a
+		// crash in an earlier version of the update_fee patch)
+		let mut nodes = create_network(2);
+		let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
+		let channel_id = chan.2;
+
+		macro_rules! get_feerate {
+			($node: expr) => {{
+				let chan_lock = $node.node.channel_state.lock().unwrap();
+				let chan = chan_lock.by_id.get(&channel_id).unwrap();
+				chan.get_feerate()
+			}}
+		}
+
+		// balancing
+		send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
+
+		// First nodes[0] generates an update_fee
+		nodes[0].node.update_fee(channel_id, get_feerate!(nodes[0]) + 20).unwrap();
+		check_added_monitors!(nodes[0], 1);
+
+		let events_0 = nodes[0].node.get_and_clear_pending_events();
+		assert_eq!(events_0.len(), 1);
+		let update_msg = match events_0[0] { // (1)
+			Event::UpdateHTLCs { updates: msgs::CommitmentUpdate { ref update_fee, .. }, .. } => {
+				update_fee.as_ref()
+			},
+			_ => panic!("Unexpected event"),
+		};
+
+		nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap()).unwrap();
+
+		// ...but before it's delivered, nodes[1] starts to send a payment back to nodes[0]...
+		let (_, our_payment_hash) = get_payment_preimage_hash!(nodes[0]);
+		nodes[1].node.send_payment(nodes[1].router.get_route(&nodes[0].node.get_our_node_id(), None, &Vec::new(), 40000, TEST_FINAL_CLTV).unwrap(), our_payment_hash).unwrap();
+		check_added_monitors!(nodes[1], 1);
+
+		let payment_event = {
+			let mut events_1 = nodes[1].node.get_and_clear_pending_events();
+			assert_eq!(events_1.len(), 1);
+			SendEvent::from_event(events_1.remove(0))
+		};
+		assert_eq!(payment_event.node_id, nodes[0].node.get_our_node_id());
+		assert_eq!(payment_event.msgs.len(), 1);
+
+		// ...now when the messages get delivered everyone should be happy
+		nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &payment_event.msgs[0]).unwrap();
+		let (as_revoke_msg, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &payment_event.commitment_msg).unwrap(); // (2)
+		assert!(as_commitment_signed.is_none()); // nodes[0] is awaiting nodes[1] revoke_and_ack
+		check_added_monitors!(nodes[0], 1);
+
+		assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_msg).unwrap().is_none()); // deliver (2)
+		check_added_monitors!(nodes[1], 1);
+
+		// We can't continue, sadly, because our (1) now has a bogus signature
+	}
+
+	#[test]
+	fn test_multi_flight_update_fee() {
+		let nodes = create_network(2);
+		let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
+		let channel_id = chan.2;
+
+		macro_rules! get_feerate {
+			($node: expr) => {{
+				let chan_lock = $node.node.channel_state.lock().unwrap();
+				let chan = chan_lock.by_id.get(&channel_id).unwrap();
+				chan.get_feerate()
+			}}
+		}
+
+		// A                                        B
+		// update_fee/commitment_signed          ->
+		//                                       .- send (1) RAA and (2) commitment_signed
+		// update_fee (never committed)          ->
+		// (3) update_fee                        ->
+		// We have to manually generate the above update_fee, it is allowed by the protocol but we
+		// don't track which updates correspond to which revoke_and_ack responses so we're in
+		// AwaitingRAA mode and will not generate the update_fee yet.
+		//                                       <- (1) RAA delivered
+		// (3) is generated and send (4) CS      -.
+		// Note that A cannot generate (4) prior to (1) being delivered as it otherwise doesn't
+		// know the per_commitment_point to use for it.
+		//                                       <- (2) commitment_signed delivered
+		// revoke_and_ack                        ->
+		//                                          B should send no response here
+		// (4) commitment_signed delivered       ->
+		//                                       <- RAA/commitment_signed delivered
+		// revoke_and_ack                        ->
+
+		// First nodes[0] generates an update_fee
+		let initial_feerate = get_feerate!(nodes[0]);
+		nodes[0].node.update_fee(channel_id, initial_feerate + 20).unwrap();
+		check_added_monitors!(nodes[0], 1);
+
+		let events_0 = nodes[0].node.get_and_clear_pending_events();
+		assert_eq!(events_0.len(), 1);
+		let (update_msg_1, commitment_signed_1) = match events_0[0] { // (1)
+			Event::UpdateHTLCs { updates: msgs::CommitmentUpdate { ref update_fee, ref commitment_signed, .. }, .. } => {
+				(update_fee.as_ref().unwrap(), commitment_signed)
+			},
+			_ => panic!("Unexpected event"),
+		};
+
+		// Deliver first update_fee/commitment_signed pair, generating (1) and (2):
+		nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg_1).unwrap();
+		let (bs_revoke_msg, bs_commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed_1).unwrap();
+		check_added_monitors!(nodes[1], 1);
+
+		// nodes[0] is awaiting a revoke from nodes[1] before it will create a new commitment
+		// transaction:
+		nodes[0].node.update_fee(channel_id, initial_feerate + 40).unwrap();
+		assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+
+		// Create the (3) update_fee message that nodes[0] will generate before it does...
+		let mut update_msg_2 = msgs::UpdateFee {
+			channel_id: update_msg_1.channel_id.clone(),
+			feerate_per_kw: (initial_feerate + 30) as u32,
+		};
+
+		nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), &update_msg_2).unwrap();
+
+		update_msg_2.feerate_per_kw = (initial_feerate + 40) as u32;
+		// Deliver (3)
+		nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), &update_msg_2).unwrap();
+
+		// Deliver (1), generating (3) and (4)
+		let as_second_update = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_revoke_msg).unwrap();
+		check_added_monitors!(nodes[0], 1);
+		assert!(as_second_update.as_ref().unwrap().update_add_htlcs.is_empty());
+		assert!(as_second_update.as_ref().unwrap().update_fulfill_htlcs.is_empty());
+		assert!(as_second_update.as_ref().unwrap().update_fail_htlcs.is_empty());
+		assert!(as_second_update.as_ref().unwrap().update_fail_malformed_htlcs.is_empty());
+		// Check that the update_fee newly generated matches what we delivered:
+		assert_eq!(as_second_update.as_ref().unwrap().update_fee.as_ref().unwrap().channel_id, update_msg_2.channel_id);
+		assert_eq!(as_second_update.as_ref().unwrap().update_fee.as_ref().unwrap().feerate_per_kw, update_msg_2.feerate_per_kw);
+
+		// Deliver (2) commitment_signed
+		let (as_revoke_msg, as_commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), bs_commitment_signed.as_ref().unwrap()).unwrap();
+		check_added_monitors!(nodes[0], 1);
+		assert!(as_commitment_signed.is_none());
+
+		assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_revoke_msg).unwrap().is_none());
+		check_added_monitors!(nodes[1], 1);
+
+		// Delever (4)
+		let (bs_second_revoke, bs_second_commitment) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &as_second_update.unwrap().commitment_signed).unwrap();
+		check_added_monitors!(nodes[1], 1);
+
+		assert!(nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &bs_second_revoke).unwrap().is_none());
+		check_added_monitors!(nodes[0], 1);
+
+		let (as_second_revoke, as_second_commitment) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &bs_second_commitment.unwrap()).unwrap();
+		assert!(as_second_commitment.is_none());
+		check_added_monitors!(nodes[0], 1);
+
+		assert!(nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &as_second_revoke).unwrap().is_none());
+		check_added_monitors!(nodes[1], 1);
+	}
+
 	#[test]
 	fn test_update_fee_vanilla() {
 		let nodes = create_network(2);
@@ -3317,6 +3488,21 @@ mod tests {
 			}}
 		}
 
+		// A                                        B
+		// (1) update_fee/commitment_signed      ->
+		//                                       <- (2) revoke_and_ack
+		//                                       .- send (3) commitment_signed
+		// (4) update_fee/commitment_signed      ->
+		//                                       .- send (5) revoke_and_ack (no CS as we're awaiting a revoke)
+		//                                       <- (3) commitment_signed delivered
+		// send (6) revoke_and_ack               -.
+		//                                       <- (5) deliver revoke_and_ack
+		// (6) deliver revoke_and_ack            ->
+		//                                       .- send (7) commitment_signed in response to (4)
+		//                                       <- (7) deliver commitment_signed
+		// revoke_and_ack                        ->
+
+		// Create and deliver (1)...
 		let feerate = get_feerate!(nodes[0]);
 		nodes[0].node.update_fee(channel_id, feerate+20).unwrap();
 
@@ -3330,15 +3516,18 @@ mod tests {
 		};
 		nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap()).unwrap();
 
+		// Generate (2) and (3):
 		let (revoke_msg, commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
 		let commitment_signed_0 = commitment_signed.unwrap();
 		check_added_monitors!(nodes[0], 1);
 		check_added_monitors!(nodes[1], 1);
 
+		// Deliver (2):
 		let resp_option = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
 		assert!(resp_option.is_none());
 		check_added_monitors!(nodes[0], 1);
 
+		// Create and deliver (4)...
 		nodes[0].node.update_fee(channel_id, feerate+30).unwrap();
 		let events_0 = nodes[0].node.get_and_clear_pending_events();
 		assert_eq!(events_0.len(), 1);
@@ -3351,21 +3540,27 @@ mod tests {
 		nodes[1].node.handle_update_fee(&nodes[0].node.get_our_node_id(), update_msg.unwrap()).unwrap();
 
 		let (revoke_msg, commitment_signed) = nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), commitment_signed).unwrap();
+		// ... creating (5)
 		assert!(commitment_signed.is_none());
 		check_added_monitors!(nodes[0], 1);
 		check_added_monitors!(nodes[1], 1);
+
+		// Handle (3), creating (6):
 		let (revoke_msg_0, commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed_0).unwrap();
 		assert!(commitment_signed.is_none());
 		check_added_monitors!(nodes[0], 1);
 
+		// Deliver (5):
 		let resp_option = nodes[0].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &revoke_msg).unwrap();
 		assert!(resp_option.is_none());
 		check_added_monitors!(nodes[0], 1);
 
+		// Deliver (6), creating (7):
 		let resp_option = nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(), &revoke_msg_0).unwrap();
 		let commitment_signed = resp_option.unwrap().commitment_signed;
 		check_added_monitors!(nodes[1], 1);
 
+		// Deliver (7)
 		let (revoke_msg, commitment_signed) = nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &commitment_signed).unwrap();
 		assert!(commitment_signed.is_none());
 		check_added_monitors!(nodes[0], 1);
@@ -3536,9 +3731,7 @@ mod tests {
 		let mut res = Vec::with_capacity(2);
 		node_txn.retain(|tx| {
 			if tx.input.len() == 1 && tx.input[0].previous_output.txid == chan.3.txid() {
-				let mut funding_tx_map = HashMap::new();
-				funding_tx_map.insert(chan.3.txid(), chan.3.clone());
-				tx.verify(&funding_tx_map).unwrap();
+				check_spends!(tx, chan.3.clone());
 				if commitment_tx.is_none() {
 					res.push(tx.clone());
 				}
@@ -3554,9 +3747,7 @@ mod tests {
 		if has_htlc_tx != HTLCType::NONE {
 			node_txn.retain(|tx| {
 				if tx.input.len() == 1 && tx.input[0].previous_output.txid == res[0].txid() {
-					let mut funding_tx_map = HashMap::new();
-					funding_tx_map.insert(res[0].txid(), res[0].clone());
-					tx.verify(&funding_tx_map).unwrap();
+					check_spends!(tx, res[0].clone());
 					if has_htlc_tx == HTLCType::TIMEOUT {
 						assert!(tx.lock_time != 0);
 					} else {
@@ -3580,9 +3771,7 @@ mod tests {
 		assert_eq!(node_txn.len(), 1);
 		node_txn.retain(|tx| {
 			if tx.input.len() == 1 && tx.input[0].previous_output.txid == revoked_tx.txid() {
-				let mut funding_tx_map = HashMap::new();
-				funding_tx_map.insert(revoked_tx.txid(), revoked_tx.clone());
-				tx.verify(&funding_tx_map).unwrap();
+				check_spends!(tx, revoked_tx.clone());
 				false
 			} else { true }
 		});
@@ -3598,10 +3787,7 @@ mod tests {
 
 		for tx in prev_txn {
 			if node_txn[0].input[0].previous_output.txid == tx.txid() {
-				let mut funding_tx_map = HashMap::new();
-				funding_tx_map.insert(tx.txid(), tx.clone());
-				node_txn[0].verify(&funding_tx_map).unwrap();
-
+				check_spends!(node_txn[0], tx.clone());
 				assert!(node_txn[0].input[0].witness[2].len() > 106); // must spend an htlc output
 				assert_eq!(tx.input.len(), 1); // must spend a commitment tx
 
@@ -3765,6 +3951,13 @@ mod tests {
 		let payment_preimage_3 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
 		// Get the will-be-revoked local txn from nodes[0]
 		let revoked_local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.iter().next().unwrap().1.last_local_commitment_txn.clone();
+		assert_eq!(revoked_local_txn.len(), 2); // First commitment tx, then HTLC tx
+		assert_eq!(revoked_local_txn[0].input.len(), 1);
+		assert_eq!(revoked_local_txn[0].input[0].previous_output.txid, chan_5.3.txid());
+		assert_eq!(revoked_local_txn[0].output.len(), 2); // Only HTLC and output back to 0 are present
+		assert_eq!(revoked_local_txn[1].input.len(), 1);
+		assert_eq!(revoked_local_txn[1].input[0].previous_output.txid, revoked_local_txn[0].txid());
+		assert_eq!(revoked_local_txn[1].input[0].witness.last().unwrap().len(), 133); // HTLC-Timeout
 		// Revoke the old state
 		claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage_3);
 
@@ -3775,11 +3968,9 @@ mod tests {
 				let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
 				assert_eq!(node_txn.len(), 3);
 				assert_eq!(node_txn.pop().unwrap(), node_txn[0]); // An outpoint registration will result in a 2nd block_connected
-				assert_eq!(node_txn[0].input.len(), 1);
+				assert_eq!(node_txn[0].input.len(), 2); // We should claim the revoked output and the HTLC output
 
-				let mut funding_tx_map = HashMap::new();
-				funding_tx_map.insert(revoked_local_txn[0].txid(), revoked_local_txn[0].clone());
-				node_txn[0].verify(&funding_tx_map).unwrap();
+				check_spends!(node_txn[0], revoked_local_txn[0].clone());
 				node_txn.swap_remove(0);
 			}
 			test_txn_broadcast(&nodes[1], &chan_5, None, HTLCType::NONE);
@@ -3795,6 +3986,173 @@ mod tests {
 		assert_eq!(nodes[1].node.list_channels().len(), 0);
 	}
 
+	#[test]
+	fn revoked_output_claim() {
+		// Simple test to ensure a node will claim a revoked output when a stale remote commitment
+		// transaction is broadcast by its counterparty
+		let nodes = create_network(2);
+		let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1);
+		// node[0] is gonna to revoke an old state thus node[1] should be able to claim the revoked output
+		let revoked_local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan_1.2).unwrap().last_local_commitment_txn.clone();
+		assert_eq!(revoked_local_txn.len(), 1);
+		// Only output is the full channel value back to nodes[0]:
+		assert_eq!(revoked_local_txn[0].output.len(), 1);
+		// Send a payment through, updating everyone's latest commitment txn
+		send_payment(&nodes[0], &vec!(&nodes[1])[..], 5000000);
+
+		// Inform nodes[1] that nodes[0] broadcast a stale tx
+		let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+		nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
+		let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
+		assert_eq!(node_txn.len(), 3); // nodes[1] will broadcast justice tx twice, and its own local state once
+
+		assert_eq!(node_txn[0], node_txn[2]);
+
+		check_spends!(node_txn[0], revoked_local_txn[0].clone());
+		check_spends!(node_txn[1], chan_1.3.clone());
+
+		// Inform nodes[0] that a watchtower cheated on its behalf, so it will force-close the chan
+		nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
+		get_announce_close_broadcast_events(&nodes, 0, 1);
+	}
+
+	#[test]
+	fn claim_htlc_outputs_shared_tx() {
+		// Node revoked old state, htlcs haven't time out yet, claim them in shared justice tx
+		let nodes = create_network(2);
+
+		// Create some new channel:
+		let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1);
+
+		// Rebalance the network to generate htlc in the two directions
+		send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
+		// node[0] is gonna to revoke an old state thus node[1] should be able to claim both offered/received HTLC outputs on top of commitment tx
+		let payment_preimage_1 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
+		let _payment_preimage_2 = route_payment(&nodes[1], &vec!(&nodes[0])[..], 3000000).0;
+
+		// Get the will-be-revoked local txn from node[0]
+		let revoked_local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan_1.2).unwrap().last_local_commitment_txn.clone();
+		assert_eq!(revoked_local_txn.len(), 2); // commitment tx + 1 HTLC-Timeout tx
+		assert_eq!(revoked_local_txn[0].input.len(), 1);
+		assert_eq!(revoked_local_txn[0].input[0].previous_output.txid, chan_1.3.txid());
+		assert_eq!(revoked_local_txn[1].input.len(), 1);
+		assert_eq!(revoked_local_txn[1].input[0].previous_output.txid, revoked_local_txn[0].txid());
+		assert_eq!(revoked_local_txn[1].input[0].witness.last().unwrap().len(), 133); // HTLC-Timeout
+		check_spends!(revoked_local_txn[1], revoked_local_txn[0].clone());
+
+		//Revoke the old state
+		claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage_1);
+
+		{
+			let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+
+			nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
+
+			nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1);
+			let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
+			assert_eq!(node_txn.len(), 4);
+
+			assert_eq!(node_txn[0].input.len(), 3); // Claim the revoked output + both revoked HTLC outputs
+			check_spends!(node_txn[0], revoked_local_txn[0].clone());
+
+			assert_eq!(node_txn[0], node_txn[3]); // justice tx is duplicated due to block re-scanning
+
+			let mut witness_lens = BTreeSet::new();
+			witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
+			witness_lens.insert(node_txn[0].input[1].witness.last().unwrap().len());
+			witness_lens.insert(node_txn[0].input[2].witness.last().unwrap().len());
+			assert_eq!(witness_lens.len(), 3);
+			assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
+			assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), 133); // revoked offered HTLC
+			assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), 138); // revoked received HTLC
+
+			// Next nodes[1] broadcasts its current local tx state:
+			assert_eq!(node_txn[1].input.len(), 1);
+			assert_eq!(node_txn[1].input[0].previous_output.txid, chan_1.3.txid()); //Spending funding tx unique txouput, tx broadcasted by ChannelManager
+
+			assert_eq!(node_txn[2].input.len(), 1);
+			let witness_script = node_txn[2].clone().input[0].witness.pop().unwrap();
+			assert_eq!(witness_script.len(), 133); //Spending an offered htlc output
+			assert_eq!(node_txn[2].input[0].previous_output.txid, node_txn[1].txid());
+			assert_ne!(node_txn[2].input[0].previous_output.txid, node_txn[0].input[0].previous_output.txid);
+			assert_ne!(node_txn[2].input[0].previous_output.txid, node_txn[0].input[1].previous_output.txid);
+		}
+		get_announce_close_broadcast_events(&nodes, 0, 1);
+		assert_eq!(nodes[0].node.list_channels().len(), 0);
+		assert_eq!(nodes[1].node.list_channels().len(), 0);
+	}
+
+	#[test]
+	fn claim_htlc_outputs_single_tx() {
+		// Node revoked old state, htlcs have timed out, claim each of them in separated justice tx
+		let nodes = create_network(2);
+
+		let chan_1 = create_announced_chan_between_nodes(&nodes, 0, 1);
+
+		// Rebalance the network to generate htlc in the two directions
+		send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
+		// node[0] is gonna to revoke an old state thus node[1] should be able to claim both offered/received HTLC outputs on top of commitment tx, but this
+		// time as two different claim transactions as we're gonna to timeout htlc with given a high current height
+		let payment_preimage_1 = route_payment(&nodes[0], &vec!(&nodes[1])[..], 3000000).0;
+		let _payment_preimage_2 = route_payment(&nodes[1], &vec!(&nodes[0])[..], 3000000).0;
+
+		// Get the will-be-revoked local txn from node[0]
+		let revoked_local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan_1.2).unwrap().last_local_commitment_txn.clone();
+
+		//Revoke the old state
+		claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage_1);
+
+		{
+			let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };
+
+			nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200);
+
+			nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200);
+			let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap();
+			assert_eq!(node_txn.len(), 12); // ChannelManager : 2, ChannelMontitor: 8 (1 standard revoked output, 2 revocation htlc tx, 1 local commitment tx + 1 htlc timeout tx) * 2 (block-rescan)
+
+			assert_eq!(node_txn[0], node_txn[7]);
+			assert_eq!(node_txn[1], node_txn[8]);
+			assert_eq!(node_txn[2], node_txn[9]);
+			assert_eq!(node_txn[3], node_txn[10]);
+			assert_eq!(node_txn[4], node_txn[11]);
+			assert_eq!(node_txn[3], node_txn[5]); //local commitment tx + htlc timeout tx broadcated by ChannelManger
+			assert_eq!(node_txn[4], node_txn[6]);
+
+			assert_eq!(node_txn[0].input.len(), 1);
+			assert_eq!(node_txn[1].input.len(), 1);
+			assert_eq!(node_txn[2].input.len(), 1);
+
+			let mut revoked_tx_map = HashMap::new();
+			revoked_tx_map.insert(revoked_local_txn[0].txid(), revoked_local_txn[0].clone());
+			node_txn[0].verify(&revoked_tx_map).unwrap();
+			node_txn[1].verify(&revoked_tx_map).unwrap();
+			node_txn[2].verify(&revoked_tx_map).unwrap();
+
+			let mut witness_lens = BTreeSet::new();
+			witness_lens.insert(node_txn[0].input[0].witness.last().unwrap().len());
+			witness_lens.insert(node_txn[1].input[0].witness.last().unwrap().len());
+			witness_lens.insert(node_txn[2].input[0].witness.last().unwrap().len());
+			assert_eq!(witness_lens.len(), 3);
+			assert_eq!(*witness_lens.iter().skip(0).next().unwrap(), 77); // revoked to_local
+			assert_eq!(*witness_lens.iter().skip(1).next().unwrap(), 133); // revoked offered HTLC
+			assert_eq!(*witness_lens.iter().skip(2).next().unwrap(), 138); // revoked received HTLC
+
+			assert_eq!(node_txn[3].input.len(), 1);
+			check_spends!(node_txn[3], chan_1.3.clone());
+
+			assert_eq!(node_txn[4].input.len(), 1);
+			let witness_script = node_txn[4].input[0].witness.last().unwrap();
+			assert_eq!(witness_script.len(), 133); //Spending an offered htlc output
+			assert_eq!(node_txn[4].input[0].previous_output.txid, node_txn[3].txid());
+			assert_ne!(node_txn[4].input[0].previous_output.txid, node_txn[0].input[0].previous_output.txid);
+			assert_ne!(node_txn[4].input[0].previous_output.txid, node_txn[1].input[0].previous_output.txid);
+		}
+		get_announce_close_broadcast_events(&nodes, 0, 1);
+		assert_eq!(nodes[0].node.list_channels().len(), 0);
+		assert_eq!(nodes[1].node.list_channels().len(), 0);
+	}
+
 	#[test]
 	fn test_htlc_ignore_latest_remote_commitment() {
 		// Test that HTLC transactions spending the latest remote commitment transaction are simply
@@ -3929,9 +4287,8 @@ mod tests {
 		assert_eq!(node_txn[0].input[0].previous_output.txid, tx.txid());
 		assert_eq!(node_txn[0].lock_time, 0); // Must be an HTLC-Success
 		assert_eq!(node_txn[0].input[0].witness.len(), 5); // Must be an HTLC-Success
-		let mut funding_tx_map = HashMap::new();
-		funding_tx_map.insert(tx.txid(), tx);
-		node_txn[0].verify(&funding_tx_map).unwrap();
+
+		check_spends!(node_txn[0], tx);
 	}
 
 	#[test]