Skip to content

Commit 2b121df

Browse files
committed
Rename MonitorEvent::CommitmentTxConfirmed to HolderForceClosed
The `MonitorEvent::CommitmentTxConfirmed` has always been a result of us force-closing the channel, not the counterparty doing so. Thus, it was always a bit of a misnoder. Worse, it carried over into the channel's `ClosureReason` in the event API. Here we simply rename it and use the proper `ClosureReason`.
1 parent 5a36a83 commit 2b121df

File tree

5 files changed

+19
-19
lines changed

5 files changed

+19
-19
lines changed

lightning/src/chain/channelmonitor.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ pub enum MonitorEvent {
134134
HTLCEvent(HTLCUpdate),
135135

136136
/// A monitor event that the Channel's commitment transaction was confirmed.
137-
CommitmentTxConfirmed(OutPoint),
137+
HolderForceClosed(OutPoint),
138138

139139
/// Indicates a [`ChannelMonitor`] update has completed. See
140140
/// [`ChannelMonitorUpdateStatus::InProgress`] for more information on how this is used.
@@ -158,7 +158,7 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorEvent,
158158
},
159159
;
160160
(2, HTLCEvent),
161-
(4, CommitmentTxConfirmed),
161+
(4, HolderForceClosed),
162162
);
163163

164164
/// Simple structure sent back by `chain::Watch` when an HTLC from a forward channel is detected on
@@ -1028,7 +1028,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signe
10281028

10291029
writer.write_all(&(self.pending_monitor_events.iter().filter(|ev| match ev {
10301030
MonitorEvent::HTLCEvent(_) => true,
1031-
MonitorEvent::CommitmentTxConfirmed(_) => true,
1031+
MonitorEvent::HolderForceClosed(_) => true,
10321032
_ => false,
10331033
}).count() as u64).to_be_bytes())?;
10341034
for event in self.pending_monitor_events.iter() {
@@ -1037,7 +1037,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signe
10371037
0u8.write(writer)?;
10381038
upd.write(writer)?;
10391039
},
1040-
MonitorEvent::CommitmentTxConfirmed(_) => 1u8.write(writer)?,
1040+
MonitorEvent::HolderForceClosed(_) => 1u8.write(writer)?,
10411041
_ => {}, // Covered in the TLV writes below
10421042
}
10431043
}
@@ -2521,7 +2521,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
25212521
txs.push(tx);
25222522
}
25232523
broadcaster.broadcast_transactions(&txs);
2524-
self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
2524+
self.pending_monitor_events.push(MonitorEvent::HolderForceClosed(self.funding_info.0));
25252525
}
25262526

25272527
pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: F, logger: &L) -> Result<(), ()>
@@ -2634,7 +2634,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
26342634
log_error!(logger, " in channel monitor for channel {}!", &self.funding_info.0.to_channel_id());
26352635
log_error!(logger, " Read the docs for ChannelMonitor::get_latest_holder_commitment_txn and take manual action!");
26362636
} else {
2637-
// If we generated a MonitorEvent::CommitmentTxConfirmed, the ChannelManager
2637+
// If we generated a MonitorEvent::HolderForceClosed, the ChannelManager
26382638
// will still give us a ChannelForceClosed event with !should_broadcast, but we
26392639
// shouldn't print the scary warning above.
26402640
log_info!(logger, "Channel off-chain state closed after we broadcasted our latest commitment transaction.");
@@ -3479,7 +3479,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
34793479
let funding_outp = HolderFundingOutput::build(self.funding_redeemscript.clone(), self.channel_value_satoshis, self.onchain_tx_handler.channel_type_features().clone());
34803480
let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), self.best_block.height(), self.best_block.height());
34813481
claimable_outpoints.push(commitment_package);
3482-
self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
3482+
self.pending_monitor_events.push(MonitorEvent::HolderForceClosed(self.funding_info.0));
34833483
let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);
34843484
self.holder_tx_signed = true;
34853485
// We can't broadcast our HTLC transactions while the commitment transaction is
@@ -4240,7 +4240,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
42404240
for _ in 0..pending_monitor_events_len {
42414241
let ev = match <u8 as Readable>::read(reader)? {
42424242
0 => MonitorEvent::HTLCEvent(Readable::read(reader)?),
4243-
1 => MonitorEvent::CommitmentTxConfirmed(funding_info.0),
4243+
1 => MonitorEvent::HolderForceClosed(funding_info.0),
42444244
_ => return Err(DecodeError::InvalidValue)
42454245
};
42464246
pending_monitor_events.as_mut().unwrap().push(ev);

lightning/src/ln/channelmanager.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6393,7 +6393,7 @@ where
63936393
self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);
63946394
}
63956395
},
6396-
MonitorEvent::CommitmentTxConfirmed(funding_outpoint) => {
6396+
MonitorEvent::HolderForceClosed(funding_outpoint) => {
63976397
let counterparty_node_id_opt = match counterparty_node_id {
63986398
Some(cp_id) => Some(cp_id),
63996399
None => {
@@ -6417,7 +6417,7 @@ where
64176417
msg: update
64186418
});
64196419
}
6420-
self.issue_channel_close_events(&chan.context, ClosureReason::CommitmentTxConfirmed);
6420+
self.issue_channel_close_events(&chan.context, ClosureReason::HolderForceClosed);
64216421
pending_msg_events.push(events::MessageSendEvent::HandleError {
64226422
node_id: chan.context.get_counterparty_node_id(),
64236423
action: msgs::ErrorAction::SendErrorMessage {

lightning/src/ln/functional_tests.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2389,6 +2389,7 @@ fn channel_monitor_network_test() {
23892389
}
23902390
check_added_monitors!(nodes[4], 1);
23912391
test_txn_broadcast(&nodes[4], &chan_4, None, HTLCType::SUCCESS);
2392+
check_closed_event!(nodes[4], 1, ClosureReason::HolderForceClosed, [nodes[3].node.get_our_node_id()], 100000);
23922393

23932394
mine_transaction(&nodes[4], &node_txn[0]);
23942395
check_preimage_claim(&nodes[4], &node_txn);
@@ -2401,8 +2402,7 @@ fn channel_monitor_network_test() {
24012402

24022403
assert_eq!(nodes[3].chain_monitor.chain_monitor.watch_channel(OutPoint { txid: chan_3.3.txid(), index: 0 }, chan_3_mon),
24032404
Ok(ChannelMonitorUpdateStatus::Completed));
2404-
check_closed_event!(nodes[3], 1, ClosureReason::CommitmentTxConfirmed, [nodes[4].node.get_our_node_id()], 100000);
2405-
check_closed_event!(nodes[4], 1, ClosureReason::CommitmentTxConfirmed, [nodes[3].node.get_our_node_id()], 100000);
2405+
check_closed_event!(nodes[3], 1, ClosureReason::HolderForceClosed, [nodes[4].node.get_our_node_id()], 100000);
24062406
}
24072407

24082408
#[test]
@@ -5574,7 +5574,7 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) {
55745574
test_txn_broadcast(&nodes[1], &chan, None, if use_dust { HTLCType::NONE } else { HTLCType::SUCCESS });
55755575
check_closed_broadcast!(nodes[1], true);
55765576
check_added_monitors!(nodes[1], 1);
5577-
check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 100000);
5577+
check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000);
55785578
}
55795579

55805580
fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
@@ -5605,7 +5605,7 @@ fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
56055605
test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
56065606
check_closed_broadcast!(nodes[0], true);
56075607
check_added_monitors!(nodes[0], 1);
5608-
check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed, [nodes[1].node.get_our_node_id()], 100000);
5608+
check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000);
56095609
}
56105610

56115611
fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no_close: bool) {
@@ -5651,7 +5651,7 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no
56515651
test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
56525652
check_closed_broadcast!(nodes[0], true);
56535653
check_added_monitors!(nodes[0], 1);
5654-
check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed, [nodes[1].node.get_our_node_id()], 100000);
5654+
check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000);
56555655
} else {
56565656
expect_payment_failed!(nodes[0], our_payment_hash, true);
56575657
}
@@ -8509,7 +8509,7 @@ fn test_concurrent_monitor_claim() {
85098509
let height = HTLC_TIMEOUT_BROADCAST + 1;
85108510
connect_blocks(&nodes[0], height - nodes[0].best_block_info().1);
85118511
check_closed_broadcast(&nodes[0], 1, true);
8512-
check_closed_event!(&nodes[0], 1, ClosureReason::CommitmentTxConfirmed, false,
8512+
check_closed_event!(&nodes[0], 1, ClosureReason::HolderForceClosed, false,
85138513
[nodes[1].node.get_our_node_id()], 100000);
85148514
watchtower_alice.chain_monitor.block_connected(&create_dummy_block(BlockHash::all_zeros(), 42, vec![bob_state_y.clone()]), height);
85158515
check_added_monitors(&nodes[0], 1);

lightning/src/ln/monitor_tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,14 +1046,14 @@ fn do_test_revoked_counterparty_commitment_balances(confirm_htlc_spend_first: bo
10461046
assert!(failed_payments.is_empty());
10471047
if let Event::PendingHTLCsForwardable { .. } = events[0] {} else { panic!(); }
10481048
match &events[1] {
1049-
Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => {},
1049+
Event::ChannelClosed { reason: ClosureReason::HolderForceClosed, .. } => {},
10501050
_ => panic!(),
10511051
}
10521052

10531053
connect_blocks(&nodes[1], htlc_cltv_timeout + 1 - 10);
10541054
check_closed_broadcast!(nodes[1], true);
10551055
check_added_monitors!(nodes[1], 1);
1056-
check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 1000000);
1056+
check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 1000000);
10571057

10581058
// Prior to channel closure, B considers the preimage HTLC as its own, and otherwise only
10591059
// lists the two on-chain timeout-able HTLCs as claimable balances.

lightning/src/ln/reorg_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ fn test_set_outpoints_partial_claiming() {
462462
// Connect blocks on node B
463463
connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
464464
check_closed_broadcast!(nodes[1], true);
465-
check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed, [nodes[0].node.get_our_node_id()], 1000000);
465+
check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 1000000);
466466
check_added_monitors!(nodes[1], 1);
467467
// Verify node B broadcast 2 HTLC-timeout txn
468468
let partial_claim_tx = {

0 commit comments

Comments
 (0)