Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
125 changes: 104 additions & 21 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1202,6 +1202,19 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
funding: FundingScope,
pending_funding: Vec<FundingScope>,

/// True if this channel was configured for manual funding broadcasts. Monitors written by
/// versions prior to LDK 0.2 load with `false` until a new update persists it.
is_manual_broadcast: bool,
/// True once we've observed either funding transaction on-chain. Older monitors prior to LDK 0.2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: there are not two funding txn, so this is worded a bit confusingly. We could say "we've observed a funding transaction" if we're referring to dual-funding RBF or "we've observed the funding transaction" otherwise.

/// assume this is `true` when absent during upgrade so holder broadcasts aren't gated unexpectedly.
/// In manual-broadcast channels we also use this to trigger deferred holder
/// broadcasts once the funding transaction finally appears on-chain.
///
/// Note: This tracks whether the funding transaction was ever broadcast, not whether it is
/// currently confirmed. It is never reset, even if the funding transaction is unconfirmed due
/// to a reorg.
funding_seen_onchain: bool,

latest_update_id: u64,
commitment_transaction_number_obscure_factor: u64,

Expand Down Expand Up @@ -1740,6 +1753,8 @@ pub(crate) fn write_chanmon_internal<Signer: EcdsaChannelSigner, W: Writer>(
(32, channel_monitor.pending_funding, optional_vec),
(33, channel_monitor.htlcs_resolved_to_user, required),
(34, channel_monitor.alternative_funding_confirmed, option),
(35, channel_monitor.is_manual_broadcast, required),
(37, channel_monitor.funding_seen_onchain, required),
});

Ok(())
Expand Down Expand Up @@ -1868,6 +1883,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
commitment_transaction_number_obscure_factor: u64,
initial_holder_commitment_tx: HolderCommitmentTransaction, best_block: BestBlock,
counterparty_node_id: PublicKey, channel_id: ChannelId,
is_manual_broadcast: bool,
) -> ChannelMonitor<Signer> {

assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
Expand Down Expand Up @@ -1914,6 +1930,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
},
pending_funding: vec![],

is_manual_broadcast,
funding_seen_onchain: false,

latest_update_id: 0,
commitment_transaction_number_obscure_factor,

Expand Down Expand Up @@ -2327,19 +2346,33 @@ 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.
#[rustfmt::skip]
///
/// 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
&self, broadcaster: &B, fee_estimator: &F, logger: &L,
) where
B::Target: BroadcasterInterface,
F::Target: FeeEstimator,
L::Target: Logger
L::Target: Logger,
{
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);

inner.queue_latest_holder_commitment_txn_for_broadcast(
broadcaster,
&fee_estimator,
&logger,
false,
);
}

/// Unsafe test-only version of `broadcast_latest_holder_commitment_txn` used by our test framework
Expand Down Expand Up @@ -3952,12 +3985,26 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
claimable_outpoints.append(&mut new_outpoints);
}
(claimable_outpoints, watch_outputs)
// In manual-broadcast mode, if we have not yet observed the funding transaction on-chain,
// return empty vectors.
if self.is_manual_broadcast && !self.funding_seen_onchain {
return (Vec::new(), Vec::new());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to avoid the allocations above by just moving this up a bit

} else {
(claimable_outpoints, watch_outputs)
}
}

#[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 [`ChannelMonitor::broadcast_latest_holder_commitment_txn`].
///
/// [`ChannelMonitor::broadcast_latest_holder_commitment_txn`]: 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,
Expand All @@ -3969,6 +4016,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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to set holder_tx_signed and push the monitor event here. Basically, we should always call generate_claimable_outpoints_and_watch_outputs but we can skip actually passing them to the onchain_tx_handler, similar to what you did elsewhere.

}
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,
Expand Down Expand Up @@ -4291,7 +4344,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());
Expand Down Expand Up @@ -5310,7 +5363,21 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
F::Target: FeeEstimator,
L::Target: Logger,
{
let funding_seen_before = self.funding_seen_onchain;
let txn_matched = self.filter_block(txdata);

if !self.funding_seen_onchain {
for &(_, tx) in txdata.iter() {
let txid = tx.compute_txid();
if txid == self.funding.funding_txid() ||
self.pending_funding.iter().any(|f| f.funding_txid() == txid)
{
self.funding_seen_onchain = true;
break;
}
}
}

for tx in &txn_matched {
let mut output_val = Amount::ZERO;
for out in tx.output.iter() {
Expand All @@ -5331,6 +5398,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {

let mut watch_outputs = Vec::new();
let mut claimable_outpoints = Vec::new();

if self.is_manual_broadcast && !funding_seen_before && self.funding_seen_onchain && self.holder_tx_signed
{
should_broadcast_commitment = true;
}
'tx_iter: for tx in &txn_matched {
let txid = tx.compute_txid();
log_trace!(logger, "Transaction {} confirmed in block {}", txid , block_hash);
Expand Down Expand Up @@ -5582,13 +5654,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
log_trace!(logger, "Processing {} matched transactions for block at height {}.", txn_matched.len(), conf_height);
debug_assert!(self.best_block.height >= conf_height);

let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);
if let Some(payment_hash) = should_broadcast {
let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
let (mut new_outpoints, mut new_outputs) =
self.generate_claimable_outpoints_and_watch_outputs(Some(reason));
claimable_outpoints.append(&mut new_outpoints);
watch_outputs.append(&mut new_outputs);
// Only generate claims if we haven't already done so (e.g., in transactions_confirmed).
if claimable_outpoints.is_empty() {
let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);
if let Some(payment_hash) = should_broadcast {
let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) };
let (mut new_outpoints, mut new_outputs) =
self.generate_claimable_outpoints_and_watch_outputs(Some(reason));
claimable_outpoints.append(&mut new_outpoints);
watch_outputs.append(&mut new_outputs);
}
}

// Find which on-chain events have reached their confirmation threshold.
Expand Down Expand Up @@ -5826,7 +5901,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;
Expand Down Expand Up @@ -5887,14 +5962,14 @@ 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);
}
}

/// Filters a block's `txdata` for transactions spending watched outputs or for any child
/// transactions thereof.
#[rustfmt::skip]
fn filter_block<'a>(&self, txdata: &TransactionData<'a>) -> Vec<&'a Transaction> {
fn filter_block<'a>(&mut self, txdata: &TransactionData<'a>) -> Vec<&'a Transaction> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this hunk can be dropped now, I think.

let mut matched_txn = new_hash_set();
txdata.iter().filter(|&&(_, tx)| {
let mut matches = self.spends_watched_output(tx);
Expand Down Expand Up @@ -6560,6 +6635,8 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
let mut channel_parameters = None;
let mut pending_funding = None;
let mut alternative_funding_confirmed = None;
let mut is_manual_broadcast = RequiredWrapper(None);
let mut funding_seen_onchain = RequiredWrapper(None);
read_tlv_fields!(reader, {
(1, funding_spend_confirmed, option),
(3, htlcs_resolved_on_chain, optional_vec),
Expand All @@ -6580,6 +6657,8 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
(32, pending_funding, optional_vec),
(33, htlcs_resolved_to_user, option),
(34, alternative_funding_confirmed, option),
(35, is_manual_broadcast, (default_value, false)),
(37, funding_seen_onchain, (default_value, true)),
});
// Note that `payment_preimages_with_info` was added (and is always written) in LDK 0.1, so
// we can use it to determine if this monitor was last written by LDK 0.1 or later.
Expand Down Expand Up @@ -6693,6 +6772,10 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
prev_holder_commitment_tx,
},
pending_funding: pending_funding.unwrap_or(vec![]),
is_manual_broadcast: is_manual_broadcast.0.unwrap(),
// Older monitors prior to LDK 0.2 assume this is `true` when absent
// during upgrade so holder broadcasts aren't gated unexpectedly.
funding_seen_onchain: funding_seen_onchain.0.unwrap(),

latest_update_id,
commitment_transaction_number_obscure_factor,
Expand Down Expand Up @@ -7029,7 +7112,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]);
Expand Down Expand Up @@ -7290,7 +7373,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();
Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3196,6 +3196,7 @@ where
funding.get_holder_selected_contest_delay(), &context.destination_script,
&funding.channel_transaction_parameters, funding.is_outbound(), obscure_factor,
holder_commitment_tx, best_block, context.counterparty_node_id, context.channel_id(),
context.is_manual_broadcast,
);
channel_monitor.provide_initial_counterparty_commitment_tx(
counterparty_initial_commitment_tx.clone(),
Expand Down
60 changes: 60 additions & 0 deletions lightning/src/ln/functional_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1804,6 +1804,66 @@ pub fn create_chan_between_nodes_with_value_a<'a, 'b, 'c: 'd, 'd>(
(msgs, chan_id, tx)
}

pub fn create_channel_manual_funding<'a, 'b, 'c: 'd, 'd>(
nodes: &'a Vec<Node<'b, 'c, 'd>>, initiator: usize, counterparty: usize, channel_value: u64,
push_msat: u64,
) -> (ChannelId, Transaction, OutPoint) {
let node_a = &nodes[initiator];
let node_b = &nodes[counterparty];
let node_a_id = node_a.node.get_our_node_id();
let node_b_id = node_b.node.get_our_node_id();

let temp_channel_id = exchange_open_accept_chan(node_a, node_b, channel_value, push_msat);

let (funding_temp_id, funding_tx, funding_outpoint) =
create_funding_transaction(node_a, &node_b_id, channel_value, 42);
assert_eq!(temp_channel_id, funding_temp_id);

node_a
.node
.funding_transaction_generated_manual_broadcast(
funding_temp_id,
node_b_id,
funding_tx.clone(),
)
.unwrap();
check_added_monitors!(node_a, 0);

let funding_created = get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b_id);
node_b.node.handle_funding_created(node_a_id, &funding_created);
check_added_monitors!(node_b, 1);
let channel_id_b = expect_channel_pending_event(node_b, &node_a_id);

let funding_signed = get_event_msg!(node_b, MessageSendEvent::SendFundingSigned, node_a_id);
node_a.node.handle_funding_signed(node_b_id, &funding_signed);
check_added_monitors!(node_a, 1);

let events = node_a.node.get_and_clear_pending_events();
assert_eq!(events.len(), 2);
let funding_txid = funding_tx.compute_txid();
let mut channel_id = None;
for event in events {
match event {
Event::FundingTxBroadcastSafe { funding_txo, counterparty_node_id, .. } => {
assert_eq!(counterparty_node_id, node_b_id);
assert_eq!(funding_txo.txid, funding_txid);
assert_eq!(funding_txo.vout, u32::from(funding_outpoint.index));
},
Event::ChannelPending { channel_id: pending_id, counterparty_node_id, .. } => {
assert_eq!(counterparty_node_id, node_b_id);
channel_id = Some(pending_id);
},
_ => panic!("Unexpected event"),
}
}
let channel_id = channel_id.expect("channel pending event missing");
assert_eq!(channel_id, channel_id_b);

assert!(node_a.tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty());

(channel_id, funding_tx, funding_outpoint)
}

pub fn create_chan_between_nodes_with_value_b<'a, 'b, 'c>(
node_a: &Node<'a, 'b, 'c>, node_b: &Node<'a, 'b, 'c>,
as_funding_msgs: &(msgs::ChannelReady, msgs::AnnouncementSignatures),
Expand Down
Loading
Loading