Skip to content
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

OutputSweeper: Delay pruning until monitors have likely been archived #3559

Merged
merged 3 commits into from
Jan 27, 2025
Merged

OutputSweeper: Delay pruning until monitors have likely been archived #3559

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c03c8a4
Introduce `SpendableOutputDescriptor::outpoint` accessor
tnull Jan 24, 2025
d8b50f0
Prefactor: Make monior archival delay a `pub const`
tnull Jan 24, 2025
84412cc
`OutputSweeper`: Delay pruning until monitors have likely been archived
tnull Jan 24, 2025
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
16 changes: 10 additions & 6 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,10 @@ pub(crate) const LATENCY_GRACE_PERIOD_BLOCKS: u32 = 3;
// solved by a previous claim tx. What we want to avoid is reorg evicting our claim tx and us not
// keep bumping another claim tx to solve the outpoint.
pub const ANTI_REORG_DELAY: u32 = 6;
/// Number of blocks we wait before assuming a [`ChannelMonitor`] to be fully resolved and
/// considering it be safely archived.
// 4032 blocks are roughly four weeks
pub const ARCHIVAL_DELAY_BLOCKS: u32 = 4032;
/// Number of blocks before confirmation at which we fail back an un-relayed HTLC or at which we
/// refuse to accept a new HTLC.
///
Expand Down Expand Up @@ -2015,10 +2019,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
///
/// This function returns a tuple of two booleans, the first indicating whether the monitor is
/// fully resolved, and the second whether the monitor needs persistence to ensure it is
/// reliably marked as resolved within 4032 blocks.
/// reliably marked as resolved within [`ARCHIVAL_DELAY_BLOCKS`] blocks.
///
/// The first boolean is true only if [`Self::get_claimable_balances`] has been empty for at least
/// 4032 blocks as an additional protection against any bugs resulting in spuriously empty balance sets.
/// The first boolean is true only if [`Self::get_claimable_balances`] has been empty for at
/// least [`ARCHIVAL_DELAY_BLOCKS`] blocks as an additional protection against any bugs
/// resulting in spuriously empty balance sets.
pub fn check_and_update_full_resolution_status<L: Logger>(&self, logger: &L) -> (bool, bool) {
let mut is_all_funds_claimed = self.get_claimable_balances().is_empty();
let current_height = self.current_best_block().height;
Expand All @@ -2034,11 +2039,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
// 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, 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)
(current_height >= balances_empty_height + ARCHIVAL_DELAY_BLOCKS, false)
},
(Some(_), false, _)|(Some(_), _, false) => {
// previously assumed we claimed all funds, but we have new funds to claim or
Expand All @@ -2058,7 +2062,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
// 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.get_funding_txo().0, ARCHIVAL_DELAY_BLOCKS);
inner.balances_empty_height = Some(current_height);
(false, true)
},
Expand Down
18 changes: 9 additions & 9 deletions lightning/src/ln/monitor_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
//! Further functional tests which test blockchain reorganizations.

use crate::sign::{ecdsa::EcdsaChannelSigner, OutputSpender, SpendableOutputDescriptor};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, ARCHIVAL_DELAY_BLOCKS,LATENCY_GRACE_PERIOD_BLOCKS, Balance, BalanceSource, ChannelMonitorUpdateStep};
use crate::chain::transaction::OutPoint;
use crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator, compute_feerate_sat_per_1000_weight};
use crate::events::bump_transaction::{BumpTransactionEvent, WalletSource};
Expand Down Expand Up @@ -246,31 +246,31 @@ fn archive_fully_resolved_monitors() {

// 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.
// Thus, calling `archive_fully_resolved_channel_monitors` and waiting `ARCHIVAL_DELAY_BLOCKS`
// 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);
connect_blocks(&nodes[0], ARCHIVAL_DELAY_BLOCKS);
nodes[0].chain_monitor.chain_monitor.archive_fully_resolved_channel_monitors();
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.
// ...however, nodes[1]'s `ChannelMonitor` is ready to be archived, and will be in exactly
// `ARCHIVAL_DELAY_BLOCKS` 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);
connect_blocks(&nodes[1], ARCHIVAL_DELAY_BLOCKS - 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[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.
// to be archived `ARCHIVAL_DELAY_BLOCKS` 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);
connect_blocks(&nodes[0], ARCHIVAL_DELAY_BLOCKS - 1);
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);
Expand Down