Skip to content

Commit 040b3cd

Browse files
send warning when we receive a old commitment transaction
During a `channel_reestablish` now we send a warning message when we receive a old commitment transaction from the peer. In addition, this commit include the update of functional test to make sure that the receiver will generate warn messages. Signed-off-by: Vincenzo Palazzo <[email protected]>
1 parent 171dfee commit 040b3cd

File tree

2 files changed

+50
-16
lines changed

2 files changed

+50
-16
lines changed

lightning/src/ln/channel.rs

+9
Original file line numberDiff line numberDiff line change
@@ -3737,6 +3737,15 @@ impl<Signer: Sign> Channel<Signer> {
37373737
}
37383738
}
37393739

3740+
// Before we change the state of the channel, we check if the peer is sending a very old
3741+
// commitment transaction number, if yes we send a warning message.
3742+
let our_commitment_transaction = INITIAL_COMMITMENT_NUMBER - self.cur_holder_commitment_transaction_number - 1;
3743+
if msg.next_remote_commitment_number + 1 < our_commitment_transaction {
3744+
return Err(
3745+
ChannelError::Warn(format!("Peer attempted to reestablish channel with a very old local commitment transaction: {} (received) vs {} (expected)", msg.next_remote_commitment_number, our_commitment_transaction))
3746+
);
3747+
}
3748+
37403749
// Go ahead and unmark PeerDisconnected as various calls we may make check for it (and all
37413750
// remaining cases either succeed or ErrorMessage-fail).
37423751
self.channel_state &= !(ChannelState::PeerDisconnected as u32);

lightning/src/ln/functional_tests.rs

+41-16
Original file line numberDiff line numberDiff line change
@@ -7345,7 +7345,7 @@ fn test_data_loss_protect() {
73457345
logger = test_utils::TestLogger::with_id(format!("node {}", 0));
73467346
let mut chain_monitor = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(&mut io::Cursor::new(previous_chain_monitor_state.0), keys_manager).unwrap().1;
73477347
chain_source = test_utils::TestChainSource::new(Network::Testnet);
7348-
tx_broadcaster = test_utils::TestBroadcaster{txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new()))};
7348+
tx_broadcaster = test_utils::TestBroadcaster { txn_broadcasted: Mutex::new(Vec::new()), blocks: Arc::new(Mutex::new(Vec::new())) };
73497349
fee_estimator = test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) };
73507350
persister = test_utils::TestPersister::new();
73517351
monitor = test_utils::TestChainMonitor::new(Some(&chain_source), &tx_broadcaster, &logger, &fee_estimator, &persister, keys_manager);
@@ -7388,8 +7388,7 @@ fn test_data_loss_protect() {
73887388
if let MessageSendEvent::SendChannelReestablish { ref node_id, ref msg } = msg {
73897389
assert_eq!(*node_id, nodes[1].node.get_our_node_id());
73907390
reestablish_1.push(msg.clone());
7391-
} else if let MessageSendEvent::BroadcastChannelUpdate { .. } = msg {
7392-
} else if let MessageSendEvent::HandleError { ref action, .. } = msg {
7391+
} else if let MessageSendEvent::BroadcastChannelUpdate { .. } = msg {} else if let MessageSendEvent::HandleError { ref action, .. } = msg {
73937392
match action {
73947393
&ErrorAction::SendErrorMessage { ref msg } => {
73957394
assert_eq!(msg.data, "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can't do any automated broadcasting");
@@ -7402,22 +7401,48 @@ fn test_data_loss_protect() {
74027401
}
74037402

74047403
// Check we close channel detecting A is fallen-behind
7404+
// Check that we sent the warning message when we detected that A has fallen behind,
7405+
// and give the possibility for A to recover from the warning.
74057406
nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]);
7406-
check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "Peer attempted to reestablish channel with a very old local commitment transaction".to_string() });
7407-
assert_eq!(check_closed_broadcast!(nodes[1], true).unwrap().data, "Peer attempted to reestablish channel with a very old local commitment transaction");
7408-
check_added_monitors!(nodes[1], 1);
7407+
let warn_msg = "Peer attempted to reestablish channel with a very old local commitment transaction".to_owned();
7408+
assert!(check_warn_msg!(nodes[1], nodes[0].node.get_our_node_id(), chan.2).contains(&warn_msg));
74097409

74107410
// Check A is able to claim to_remote output
7411-
let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
7412-
assert_eq!(node_txn.len(), 1);
7413-
check_spends!(node_txn[0], chan.3);
7414-
assert_eq!(node_txn[0].output.len(), 2);
7415-
mine_transaction(&nodes[0], &node_txn[0]);
7416-
connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
7417-
check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can\'t do any automated broadcasting".to_string() });
7418-
let spend_txn = check_spendable_outputs!(nodes[0], node_cfgs[0].keys_manager);
7419-
assert_eq!(spend_txn.len(), 1);
7420-
check_spends!(spend_txn[0], node_txn[0]);
7411+
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
7412+
// The node B should not broadcast the transaction to force close the channel!
7413+
assert!(node_txn.is_empty());
7414+
// B should now detect that there is something wrong and should force close the channel.
7415+
let exp_err = "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can\'t do any automated broadcasting";
7416+
check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: exp_err.to_string() });
7417+
7418+
// after the warning message sent by B, we should not able to
7419+
// use the channel, or reconnect with success to the channel.
7420+
assert!(nodes[0].node.list_usable_channels().is_empty());
7421+
nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
7422+
nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
7423+
let retry_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
7424+
7425+
nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &retry_reestablish[0]);
7426+
let mut err_msgs_0 = Vec::with_capacity(1);
7427+
for msg in nodes[0].node.get_and_clear_pending_msg_events() {
7428+
if let MessageSendEvent::HandleError { ref action, .. } = msg {
7429+
match action {
7430+
&ErrorAction::SendErrorMessage { ref msg } => {
7431+
assert_eq!(msg.data, "Failed to find corresponding channel");
7432+
err_msgs_0.push(msg.clone());
7433+
},
7434+
_ => panic!("Unexpected event!"),
7435+
}
7436+
} else {
7437+
panic!("Unexpected event!");
7438+
}
7439+
}
7440+
assert_eq!(err_msgs_0.len(), 1);
7441+
nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), &err_msgs_0[0]);
7442+
assert!(nodes[1].node.list_usable_channels().is_empty());
7443+
check_added_monitors!(nodes[1], 1);
7444+
check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyForceClosed { peer_msg: "Failed to find corresponding channel".to_owned() });
7445+
check_closed_broadcast!(nodes[1], false);
74217446
}
74227447

74237448
#[test]

0 commit comments

Comments
 (0)