Skip to content

Ensure monitors are not archived if they have a preimage we need #3450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 11, 2024
Merged

Ensure monitors are not archived if they have a preimage we need #3450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 23 additions & 12 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1993,7 +1993,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
}

/// Checks if the monitor is fully resolved. Resolved monitor is one that has claimed all of
/// its outputs and balances (i.e. [`Self::get_claimable_balances`] returns an empty set).
/// its outputs and balances (i.e. [`Self::get_claimable_balances`] returns an empty set) and
/// which does not have any payment preimages for HTLCs which are still pending on other
/// channels.
///
/// Additionally may update state to track when the balances set became empty.
///
/// This function returns a tuple of two booleans, the first indicating whether the monitor is
Expand All @@ -2012,33 +2015,41 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
is_all_funds_claimed = false;
}

// As long as HTLCs remain unresolved, they'll be present as a `Balance`. After that point,
// if they contained a preimage, an event will appear in `pending_monitor_events` which,
// once processed, implies the preimage exists in the corresponding inbound channel.
let preimages_not_needed_elsewhere = inner.pending_monitor_events.is_empty();

const BLOCKS_THRESHOLD: u32 = 4032; // ~four weeks
match (inner.balances_empty_height, is_all_funds_claimed) {
(Some(balances_empty_height), true) => {
match (inner.balances_empty_height, is_all_funds_claimed, preimages_not_needed_elsewhere) {
(Some(balances_empty_height), true, true) => {
// Claimed all funds, check if reached the blocks threshold.
(current_height >= balances_empty_height + BLOCKS_THRESHOLD, false)
},
(Some(_), false) => {
// previously assumed we claimed all funds, but we have new funds to claim.
// Should not happen in practice.
debug_assert!(false, "Thought we were done claiming funds, but claimable_balances now has entries");
(Some(_), false, _)|(Some(_), _, false) => {
// previously assumed we claimed all funds, but we have new funds to claim or
// preimages are suddenly needed (because of a duplicate-hash HTLC).
// This should never happen as once the `Balance`s and preimages are clear, we
// should never create new ones.
debug_assert!(false,
"Thought we were done claiming funds, but claimable_balances now has entries");
log_error!(logger,
"WARNING: LDK thought it was done claiming all the available funds in the ChannelMonitor for channel {}, but later decided it had more to claim. This is potentially an important bug in LDK, please report it at https://github.com/lightningdevkit/rust-lightning/issues/new",
inner.get_funding_txo().0);
inner.balances_empty_height = None;
(false, true)
},
(None, true) => {
// Claimed all funds but `balances_empty_height` is None. It is set to the
// current block height.
(None, true, true) => {
// Claimed all funds and preimages can be deleted, but `balances_empty_height` is
// None. It is set to the current block height.
log_debug!(logger,
"ChannelMonitor funded at {} is now fully resolved. It will become archivable in {} blocks",
inner.get_funding_txo().0, BLOCKS_THRESHOLD);
inner.balances_empty_height = Some(current_height);
(false, true)
},
(None, false) => {
// Have funds to claim.
(None, false, _)|(None, _, false) => {
// Have funds to claim or our preimages are still needed.
(false, false)
},
}
Expand Down
136 changes: 110 additions & 26 deletions lightning/src/ln/monitor_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ fn revoked_output_htlc_resolution_timing() {

#[test]
fn archive_fully_resolved_monitors() {
// Test we can archive fully resolved channel monitor.
// Test we archive fully resolved channel monitors at the right time.
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let mut user_config = test_default_channel_config();
Expand All @@ -171,46 +171,130 @@ fn archive_fully_resolved_monitors() {
let (_, _, chan_id, funding_tx) =
create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 1_000_000);

nodes[0].node.close_channel(&chan_id, &nodes[1].node.get_our_node_id()).unwrap();
let node_0_shutdown = get_event_msg!(nodes[0], MessageSendEvent::SendShutdown, nodes[1].node.get_our_node_id());
nodes[1].node.handle_shutdown(nodes[0].node.get_our_node_id(), &node_0_shutdown);
let node_1_shutdown = get_event_msg!(nodes[1], MessageSendEvent::SendShutdown, nodes[0].node.get_our_node_id());
nodes[0].node.handle_shutdown(nodes[1].node.get_our_node_id(), &node_1_shutdown);
let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 10_000_000);

let node_0_closing_signed = get_event_msg!(nodes[0], MessageSendEvent::SendClosingSigned, nodes[1].node.get_our_node_id());
nodes[1].node.handle_closing_signed(nodes[0].node.get_our_node_id(), &node_0_closing_signed);
let node_1_closing_signed = get_event_msg!(nodes[1], MessageSendEvent::SendClosingSigned, nodes[0].node.get_our_node_id());
nodes[0].node.handle_closing_signed(nodes[1].node.get_our_node_id(), &node_1_closing_signed);
let (_, node_0_2nd_closing_signed) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
nodes[1].node.handle_closing_signed(nodes[0].node.get_our_node_id(), &node_0_2nd_closing_signed.unwrap());
let (_, _) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
nodes[0].node.force_close_broadcasting_latest_txn(&chan_id, &nodes[1].node.get_our_node_id(), "".to_owned()).unwrap();
check_added_monitors!(nodes[0], 1);
check_closed_broadcast!(nodes[0], true);
check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true) }, [nodes[1].node.get_our_node_id()], 1_000_000);

let shutdown_tx = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
let commitment_tx = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
assert_eq!(commitment_tx.len(), 1);

mine_transaction(&nodes[0], &shutdown_tx[0]);
mine_transaction(&nodes[1], &shutdown_tx[0]);
mine_transaction(&nodes[0], &commitment_tx[0]);
mine_transaction(&nodes[1], &commitment_tx[0]);
let reason = ClosureReason::CommitmentTxConfirmed;
check_closed_event!(nodes[1], 1, reason, [nodes[0].node.get_our_node_id()], 1_000_000);
check_closed_broadcast(&nodes[1], 1, true);
check_added_monitors(&nodes[1], 1);

connect_blocks(&nodes[0], 6);
connect_blocks(&nodes[0], BREAKDOWN_TIMEOUT as u32);
connect_blocks(&nodes[1], 6);

check_closed_event!(nodes[0], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[1].node.get_our_node_id()], 1000000);
check_closed_event!(nodes[1], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[0].node.get_our_node_id()], 1000000);
// After the commitment transaction reaches enough confirmations for nodes[0] to claim its
// balance, both nodes should still have a pending `Balance` as the HTLC has not yet resolved.
assert!(!nodes[0].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
assert!(!nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());

let spendable_event = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
assert_eq!(spendable_event.len(), 1);
if let Event::SpendableOutputs { .. } = spendable_event[0] {} else { panic!(); }

// Until the `Balance` set of both monitors goes empty, calling
// `archive_fully_resolved_channel_monitors` will do nothing (though we don't bother to observe
// that except later by checking that the monitors are archived at the exact correct block
// height).
nodes[0].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[0].chain_monitor.chain_monitor.list_monitors().len(), 1);
// First archive should set balances_empty_height to current block height
nodes[1].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[1].chain_monitor.chain_monitor.list_monitors().len(), 1);

nodes[1].node.claim_funds(payment_preimage);
check_added_monitors(&nodes[1], 1);
expect_payment_claimed!(nodes[1], payment_hash, 10_000_000);
let htlc_claim_tx = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
assert_eq!(htlc_claim_tx.len(), 1);

mine_transaction(&nodes[0], &htlc_claim_tx[0]);
mine_transaction(&nodes[1], &htlc_claim_tx[0]);

// Both nodes should retain the pending `Balance` until the HTLC resolution transaction has six
// confirmations
assert!(!nodes[0].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
assert!(!nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());

// Until the `Balance` set of both monitors goes empty, calling
// `archive_fully_resolved_channel_monitors` will do nothing (though we don't bother to observe
// that except later by checking that the monitors are archived at the exact correct block
// height).
nodes[0].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[0].chain_monitor.chain_monitor.list_monitors().len(), 1);
nodes[1].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[1].chain_monitor.chain_monitor.list_monitors().len(), 1);

connect_blocks(&nodes[0], 5);
connect_blocks(&nodes[1], 5);

assert!(nodes[0].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());
assert!(nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).is_empty());

// At this point, both nodes have no more `Balance`s, but nodes[0]'s `ChannelMonitor` still
// hasn't had the `MonitorEvent` that contains the preimage claimed by the `ChannelManager`.
// Thus, calling `archive_fully_resolved_channel_monitors` and waiting 4032 blocks will not
// result in the `ChannelMonitor` being archived.
nodes[0].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[0].chain_monitor.chain_monitor.list_monitors().len(), 1);
connect_blocks(&nodes[0], 4032);
// Second call after 4032 blocks, should archive the monitor
nodes[0].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
// Should have no monitors left
assert_eq!(nodes[0].chain_monitor.chain_monitor.list_monitors().len(), 1);

// ...however, nodes[1]'s `ChannelMonitor` is ready to be archived, and will be in exactly 4032
// blocks.
nodes[1].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[1].chain_monitor.chain_monitor.list_monitors().len(), 1);
connect_blocks(&nodes[1], 4031);
nodes[1].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[1].chain_monitor.chain_monitor.list_monitors().len(), 1);
connect_blocks(&nodes[1], 1);
nodes[1].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[1].chain_monitor.chain_monitor.list_monitors().len(), 0);

// Finally, we process the pending `MonitorEvent` from nodes[0], allowing the `ChannelMonitor`
// to be archived 4032 blocks later.
expect_payment_sent(&nodes[0], payment_preimage, None, true, false);
nodes[0].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[0].chain_monitor.chain_monitor.list_monitors().len(), 1);
connect_blocks(&nodes[0], 4031);
nodes[0].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[0].chain_monitor.chain_monitor.list_monitors().len(), 1);
connect_blocks(&nodes[0], 1);
nodes[0].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
assert_eq!(nodes[0].chain_monitor.chain_monitor.list_monitors().len(), 0);

// Remove the corresponding outputs and transactions the chain source is
// watching. This is to make sure the `Drop` function assertions pass.
nodes.get_mut(0).unwrap().chain_source.remove_watched_txn_and_outputs(
OutPoint { txid: funding_tx.compute_txid(), index: 0 },
funding_tx.output[0].script_pubkey.clone()
);
for node in nodes {
node.chain_source.remove_watched_txn_and_outputs(
OutPoint { txid: funding_tx.compute_txid(), index: 0 },
funding_tx.output[0].script_pubkey.clone()
);
node.chain_source.remove_watched_txn_and_outputs(
OutPoint { txid: commitment_tx[0].compute_txid(), index: 0 },
commitment_tx[0].output[0].script_pubkey.clone()
);
node.chain_source.remove_watched_txn_and_outputs(
OutPoint { txid: commitment_tx[0].compute_txid(), index: 1 },
commitment_tx[0].output[1].script_pubkey.clone()
);
node.chain_source.remove_watched_txn_and_outputs(
OutPoint { txid: commitment_tx[0].compute_txid(), index: 2 },
commitment_tx[0].output[2].script_pubkey.clone()
);
node.chain_source.remove_watched_txn_and_outputs(
OutPoint { txid: htlc_claim_tx[0].compute_txid(), index: 0 },
htlc_claim_tx[0].output[0].script_pubkey.clone()
);
}
}

fn do_chanmon_claim_value_coop_close(anchors: bool) {
Expand Down
Loading