-
Notifications
You must be signed in to change notification settings - Fork 421
Track funding tx channelmonitor #4109
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.
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
Open
martinsaposnic
wants to merge
7
commits into
lightningdevkit:main
Choose a base branch
from
martinsaposnic:track-funding-tx-channelmonitor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+709
−23
Open
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
8376197
Prefactor: drop #[rustfmt::skip] on broadcast_latest_holder_commitmen…
martinsaposnic cdc4ebf
Add manual-funding broadcast tracking to ChannelMonitor
martinsaposnic b725130
Set funding_seen_onchain=true in filter_block
martinsaposnic c665c95
Gate holder broadcast queueing on funding confirmation
martinsaposnic 1f9ee40
Defer claimable tracking until funding tx confirms
martinsaposnic ef38903
Queue holder commit once funding tx confirms
martinsaposnic 400730f
Test manual broadcast tracking and holder commit flow
martinsaposnic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2327,6 +2327,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> { | |
/// close channel with their commitment transaction after a substantial amount of time. Best | ||
/// may be to contact the other node operator out-of-band to coordinate other options available | ||
/// to you. | ||
/// | ||
/// Note: For channels using manual funding broadcast (see | ||
/// [`crate::ln::channelmanager::ChannelManager::funding_transaction_generated_manual_broadcast`]), | ||
/// automatic broadcasts are suppressed until the funding transaction has been observed on-chain. | ||
/// Calling this method overrides that suppression and queues the latest holder commitment | ||
/// transaction for broadcast even if the funding has not yet been seen on-chain. This may result | ||
/// in unconfirmable transactions being broadcast or [`Event::BumpTransaction`] notifications for | ||
/// transactions that cannot be confirmed until the funding transaction is visible. | ||
/// | ||
/// [`Event::BumpTransaction`]: crate::events::Event::BumpTransaction | ||
pub fn broadcast_latest_holder_commitment_txn<B: Deref, F: Deref, L: Deref>( | ||
&self, broadcaster: &B, fee_estimator: &F, logger: &L, | ||
) where | ||
|
@@ -2337,10 +2347,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> { | |
let mut inner = self.inner.lock().unwrap(); | ||
let fee_estimator = LowerBoundedFeeEstimator::new(&**fee_estimator); | ||
let logger = WithChannelMonitor::from_impl(logger, &*inner, None); | ||
|
||
inner.queue_latest_holder_commitment_txn_for_broadcast( | ||
broadcaster, | ||
&fee_estimator, | ||
&logger, | ||
false, | ||
); | ||
} | ||
|
||
|
@@ -3958,8 +3970,15 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> { | |
} | ||
|
||
#[rustfmt::skip] | ||
/// Note: For channels where the funding transaction is being manually managed (see | ||
/// [`crate::ln::channelmanager::ChannelManager::funding_transaction_generated_manual_broadcast`]), | ||
/// this method returns without queuing any transactions until the funding transaction has been | ||
/// observed on-chain, unless `require_funding_seen` is `false`. This prevents attempting to | ||
/// broadcast unconfirmable holder commitment transactions before the funding is visible. | ||
/// See also | ||
/// [`crate::chain::channelmonitor::ChannelMonitor::broadcast_latest_holder_commitment_txn`]. | ||
pub(crate) fn queue_latest_holder_commitment_txn_for_broadcast<B: Deref, F: Deref, L: Deref>( | ||
&mut self, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &WithChannelMonitor<L> | ||
&mut self, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &WithChannelMonitor<L>, require_funding_seen: bool, | ||
) | ||
where | ||
B::Target: BroadcasterInterface, | ||
|
@@ -3971,6 +3990,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> { | |
message: "ChannelMonitor-initiated commitment transaction broadcast".to_owned(), | ||
}; | ||
let (claimable_outpoints, _) = self.generate_claimable_outpoints_and_watch_outputs(Some(reason)); | ||
// In manual-broadcast mode, if `require_funding_seen` is true and we have not yet observed | ||
// the funding transaction on-chain, do not queue any transactions. | ||
if require_funding_seen && self.is_manual_broadcast && !self.funding_seen_onchain { | ||
log_info!(logger, "Not broadcasting holder commitment for manual-broadcast channel before funding appears on-chain"); | ||
return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we want to set |
||
} | ||
let conf_target = self.closure_conf_target(); | ||
self.onchain_tx_handler.update_claims_view_from_requests( | ||
claimable_outpoints, self.best_block.height, self.best_block.height, broadcaster, | ||
|
@@ -4285,7 +4310,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> { | |
log_trace!(logger, "Avoiding commitment broadcast, already detected confirmed spend onchain"); | ||
continue; | ||
} | ||
self.queue_latest_holder_commitment_txn_for_broadcast(broadcaster, &bounded_fee_estimator, logger); | ||
self.queue_latest_holder_commitment_txn_for_broadcast(broadcaster, &bounded_fee_estimator, logger, true); | ||
} else if !self.holder_tx_signed { | ||
log_error!(logger, "WARNING: You have a potentially-unsafe holder commitment transaction available to broadcast"); | ||
log_error!(logger, " in channel monitor for channel {}!", &self.channel_id()); | ||
|
@@ -5751,7 +5776,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> { | |
// Only attempt to broadcast the new commitment after the `block_disconnected` call above so that | ||
// it doesn't get removed from the set of pending claims. | ||
if should_broadcast_commitment { | ||
self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, &bounded_fee_estimator, logger); | ||
self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, &bounded_fee_estimator, logger, true); | ||
} | ||
|
||
self.best_block = fork_point; | ||
|
@@ -5812,7 +5837,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> { | |
// Only attempt to broadcast the new commitment after the `transaction_unconfirmed` call above so | ||
// that it doesn't get removed from the set of pending claims. | ||
if should_broadcast_commitment { | ||
self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, fee_estimator, logger); | ||
self.queue_latest_holder_commitment_txn_for_broadcast(&broadcaster, fee_estimator, logger, true); | ||
} | ||
} | ||
|
||
|
@@ -6945,7 +6970,7 @@ mod tests { | |
let monitor = ChannelMonitor::new( | ||
Secp256k1::new(), keys, Some(shutdown_script.into_inner()), 0, &ScriptBuf::new(), | ||
&channel_parameters, true, 0, HolderCommitmentTransaction::dummy(0, funding_outpoint, Vec::new()), | ||
best_block, dummy_key, channel_id, | ||
best_block, dummy_key, channel_id, false, | ||
); | ||
|
||
let nondust_htlcs = preimages_slice_to_htlcs!(preimages[0..10]); | ||
|
@@ -7205,7 +7230,7 @@ mod tests { | |
let monitor = ChannelMonitor::new( | ||
Secp256k1::new(), keys, Some(shutdown_script.into_inner()), 0, &ScriptBuf::new(), | ||
&channel_parameters, true, 0, HolderCommitmentTransaction::dummy(0, funding_outpoint, Vec::new()), | ||
best_block, dummy_key, channel_id, | ||
best_block, dummy_key, channel_id, false | ||
); | ||
|
||
let chan_id = monitor.inner.lock().unwrap().channel_id(); | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Instead of having this huge doc-link, we'd usually just link to
ChannelMonitor::broadcast_latest_holder_commitment_txn
and create a corresponding footnote below.