Skip to content

Commit 0dbf17b

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #2703 from wpaulino/retryable-commitment-broadcast
Refactor commitment broadcast to always go through OnchainTxHandler
2 parents 2b04f19 + 60bb39a commit 0dbf17b

File tree

7 files changed

+361
-136
lines changed

7 files changed

+361
-136
lines changed

‎lightning/src/chain/channelmonitor.rs‎

Lines changed: 112 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -2666,18 +2666,59 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
26662666
}
26672667
}
26682668

2669-
fn broadcast_latest_holder_commitment_txn<B: Deref, L: Deref>(&mut self, broadcaster: &B, logger: &WithChannelMonitor<L>)
2670-
where B::Target: BroadcasterInterface,
2671-
L::Target: Logger,
2672-
{
2673-
let commit_txs = self.get_latest_holder_commitment_txn(logger);
2674-
let mut txs = vec![];
2675-
for tx in commit_txs.iter() {
2676-
log_info!(logger, "Broadcasting local {}", log_tx!(tx));
2677-
txs.push(tx);
2678-
}
2679-
broadcaster.broadcast_transactions(&txs);
2669+
fn generate_claimable_outpoints_and_watch_outputs(&mut self) -> (Vec<PackageTemplate>, Vec<TransactionOutputs>) {
2670+
let funding_outp = HolderFundingOutput::build(
2671+
self.funding_redeemscript.clone(),
2672+
self.channel_value_satoshis,
2673+
self.onchain_tx_handler.channel_type_features().clone()
2674+
);
2675+
let commitment_package = PackageTemplate::build_package(
2676+
self.funding_info.0.txid.clone(), self.funding_info.0.index as u32,
2677+
PackageSolvingData::HolderFundingOutput(funding_outp),
2678+
self.best_block.height(), self.best_block.height()
2679+
);
2680+
let mut claimable_outpoints = vec![commitment_package];
26802681
self.pending_monitor_events.push(MonitorEvent::HolderForceClosed(self.funding_info.0));
2682+
// Although we aren't signing the transaction directly here, the transaction will be signed
2683+
// in the claim that is queued to OnchainTxHandler. We set holder_tx_signed here to reject
2684+
// new channel updates.
2685+
self.holder_tx_signed = true;
2686+
let mut watch_outputs = Vec::new();
2687+
// We can't broadcast our HTLC transactions while the commitment transaction is
2688+
// unconfirmed. We'll delay doing so until we detect the confirmed commitment in
2689+
// `transactions_confirmed`.
2690+
if !self.onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx() {
2691+
// Because we're broadcasting a commitment transaction, we should construct the package
2692+
// assuming it gets confirmed in the next block. Sadly, we have code which considers
2693+
// "not yet confirmed" things as discardable, so we cannot do that here.
2694+
let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(
2695+
&self.current_holder_commitment_tx, self.best_block.height()
2696+
);
2697+
let unsigned_commitment_tx = self.onchain_tx_handler.get_unsigned_holder_commitment_tx();
2698+
let new_outputs = self.get_broadcasted_holder_watch_outputs(
2699+
&self.current_holder_commitment_tx, &unsigned_commitment_tx
2700+
);
2701+
if !new_outputs.is_empty() {
2702+
watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs));
2703+
}
2704+
claimable_outpoints.append(&mut new_outpoints);
2705+
}
2706+
(claimable_outpoints, watch_outputs)
2707+
}
2708+
2709+
pub(crate) fn queue_latest_holder_commitment_txn_for_broadcast<B: Deref, F: Deref, L: Deref>(
2710+
&mut self, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator<F>, logger: &WithChannelMonitor<L>
2711+
)
2712+
where
2713+
B::Target: BroadcasterInterface,
2714+
F::Target: FeeEstimator,
2715+
L::Target: Logger,
2716+
{
2717+
let (claimable_outpoints, _) = self.generate_claimable_outpoints_and_watch_outputs();
2718+
self.onchain_tx_handler.update_claims_view_from_requests(
2719+
claimable_outpoints, self.best_block.height(), self.best_block.height(), broadcaster,
2720+
fee_estimator, logger
2721+
);
26812722
}
26822723

26832724
fn update_monitor<B: Deref, F: Deref, L: Deref>(
@@ -2767,26 +2808,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
27672808
log_trace!(logger, "Avoiding commitment broadcast, already detected confirmed spend onchain");
27682809
continue;
27692810
}
2770-
self.broadcast_latest_holder_commitment_txn(broadcaster, logger);
2771-
// If the channel supports anchor outputs, we'll need to emit an external
2772-
// event to be consumed such that a child transaction is broadcast with a
2773-
// high enough feerate for the parent commitment transaction to confirm.
2774-
if self.onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx() {
2775-
let funding_output = HolderFundingOutput::build(
2776-
self.funding_redeemscript.clone(), self.channel_value_satoshis,
2777-
self.onchain_tx_handler.channel_type_features().clone(),
2778-
);
2779-
let best_block_height = self.best_block.height();
2780-
let commitment_package = PackageTemplate::build_package(
2781-
self.funding_info.0.txid.clone(), self.funding_info.0.index as u32,
2782-
PackageSolvingData::HolderFundingOutput(funding_output),
2783-
best_block_height, best_block_height
2784-
);
2785-
self.onchain_tx_handler.update_claims_view_from_requests(
2786-
vec![commitment_package], best_block_height, best_block_height,
2787-
broadcaster, &bounded_fee_estimator, logger,
2788-
);
2789-
}
2811+
self.queue_latest_holder_commitment_txn_for_broadcast(broadcaster, &bounded_fee_estimator, logger);
27902812
} else if !self.holder_tx_signed {
27912813
log_error!(logger, "WARNING: You have a potentially-unsafe holder commitment transaction available to broadcast");
27922814
log_error!(logger, " in channel monitor for channel {}!", &self.funding_info.0.to_channel_id());
@@ -3363,6 +3385,58 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33633385
}
33643386
}
33653387

3388+
/// Cancels any existing pending claims for a commitment that previously confirmed and has now
3389+
/// been replaced by another.
3390+
pub fn cancel_prev_commitment_claims<L: Deref>(
3391+
&mut self, logger: &L, confirmed_commitment_txid: &Txid
3392+
) where L::Target: Logger {
3393+
for (counterparty_commitment_txid, _) in &self.counterparty_commitment_txn_on_chain {
3394+
// Cancel any pending claims for counterparty commitments we've seen confirm.
3395+
if counterparty_commitment_txid == confirmed_commitment_txid {
3396+
continue;
3397+
}
3398+
for (htlc, _) in self.counterparty_claimable_outpoints.get(counterparty_commitment_txid).unwrap_or(&vec![]) {
3399+
log_trace!(logger, "Canceling claims for previously confirmed counterparty commitment {}",
3400+
counterparty_commitment_txid);
3401+
let mut outpoint = BitcoinOutPoint { txid: *counterparty_commitment_txid, vout: 0 };
3402+
if let Some(vout) = htlc.transaction_output_index {
3403+
outpoint.vout = vout;
3404+
self.onchain_tx_handler.abandon_claim(&outpoint);
3405+
}
3406+
}
3407+
}
3408+
if self.holder_tx_signed {
3409+
// If we've signed, we may have broadcast either commitment (prev or current), and
3410+
// attempted to claim from it immediately without waiting for a confirmation.
3411+
if self.current_holder_commitment_tx.txid != *confirmed_commitment_txid {
3412+
log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
3413+
self.current_holder_commitment_tx.txid);
3414+
let mut outpoint = BitcoinOutPoint { txid: self.current_holder_commitment_tx.txid, vout: 0 };
3415+
for (htlc, _, _) in &self.current_holder_commitment_tx.htlc_outputs {
3416+
if let Some(vout) = htlc.transaction_output_index {
3417+
outpoint.vout = vout;
3418+
self.onchain_tx_handler.abandon_claim(&outpoint);
3419+
}
3420+
}
3421+
}
3422+
if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
3423+
if prev_holder_commitment_tx.txid != *confirmed_commitment_txid {
3424+
log_trace!(logger, "Canceling claims for previously broadcast holder commitment {}",
3425+
prev_holder_commitment_tx.txid);
3426+
let mut outpoint = BitcoinOutPoint { txid: prev_holder_commitment_tx.txid, vout: 0 };
3427+
for (htlc, _, _) in &prev_holder_commitment_tx.htlc_outputs {
3428+
if let Some(vout) = htlc.transaction_output_index {
3429+
outpoint.vout = vout;
3430+
self.onchain_tx_handler.abandon_claim(&outpoint);
3431+
}
3432+
}
3433+
}
3434+
}
3435+
} else {
3436+
// No previous claim.
3437+
}
3438+
}
3439+
33663440
fn get_latest_holder_commitment_txn<L: Deref>(
33673441
&mut self, logger: &WithChannelMonitor<L>,
33683442
) -> Vec<Transaction> where L::Target: Logger {
@@ -3578,6 +3652,10 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
35783652
commitment_tx_to_counterparty_output,
35793653
},
35803654
});
3655+
// Now that we've detected a confirmed commitment transaction, attempt to cancel
3656+
// pending claims for any commitments that were previously confirmed such that
3657+
// we don't continue claiming inputs that no longer exist.
3658+
self.cancel_prev_commitment_claims(&logger, &txid);
35813659
}
35823660
}
35833661
if tx.input.len() >= 1 {
@@ -3643,29 +3721,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
36433721

36443722
let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);
36453723
if should_broadcast {
3646-
let funding_outp = HolderFundingOutput::build(self.funding_redeemscript.clone(), self.channel_value_satoshis, self.onchain_tx_handler.channel_type_features().clone());
3647-
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());
3648-
claimable_outpoints.push(commitment_package);
3649-
self.pending_monitor_events.push(MonitorEvent::HolderForceClosed(self.funding_info.0));
3650-
// Although we aren't signing the transaction directly here, the transaction will be signed
3651-
// in the claim that is queued to OnchainTxHandler. We set holder_tx_signed here to reject
3652-
// new channel updates.
3653-
self.holder_tx_signed = true;
3654-
// We can't broadcast our HTLC transactions while the commitment transaction is
3655-
// unconfirmed. We'll delay doing so until we detect the confirmed commitment in
3656-
// `transactions_confirmed`.
3657-
if !self.onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx() {
3658-
// Because we're broadcasting a commitment transaction, we should construct the package
3659-
// assuming it gets confirmed in the next block. Sadly, we have code which considers
3660-
// "not yet confirmed" things as discardable, so we cannot do that here.
3661-
let (mut new_outpoints, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height());
3662-
let unsigned_commitment_tx = self.onchain_tx_handler.get_unsigned_holder_commitment_tx();
3663-
let new_outputs = self.get_broadcasted_holder_watch_outputs(&self.current_holder_commitment_tx, &unsigned_commitment_tx);
3664-
if !new_outputs.is_empty() {
3665-
watch_outputs.push((self.current_holder_commitment_tx.txid.clone(), new_outputs));
3666-
}
3667-
claimable_outpoints.append(&mut new_outpoints);
3668-
}
3724+
let (mut new_outpoints, mut new_outputs) = self.generate_claimable_outpoints_and_watch_outputs();
3725+
claimable_outpoints.append(&mut new_outpoints);
3726+
watch_outputs.append(&mut new_outputs);
36693727
}
36703728

36713729
// Find which on-chain events have reached their confirmation threshold.

‎lightning/src/chain/onchaintx.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,25 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
676676
None
677677
}
678678

679+
pub fn abandon_claim(&mut self, outpoint: &BitcoinOutPoint) {
680+
let claim_id = self.claimable_outpoints.get(outpoint).map(|(claim_id, _)| *claim_id)
681+
.or_else(|| {
682+
self.pending_claim_requests.iter()
683+
.find(|(_, claim)| claim.outpoints().iter().any(|claim_outpoint| *claim_outpoint == outpoint))
684+
.map(|(claim_id, _)| *claim_id)
685+
});
686+
if let Some(claim_id) = claim_id {
687+
if let Some(claim) = self.pending_claim_requests.remove(&claim_id) {
688+
for outpoint in claim.outpoints() {
689+
self.claimable_outpoints.remove(&outpoint);
690+
}
691+
}
692+
} else {
693+
self.locktimed_packages.values_mut().for_each(|claims|
694+
claims.retain(|claim| !claim.outpoints().iter().any(|claim_outpoint| *claim_outpoint == outpoint)));
695+
}
696+
}
697+
679698
/// Upon channelmonitor.block_connected(..) or upon provision of a preimage on the forward link
680699
/// for this channel, provide new relevant on-chain transactions and/or new claim requests.
681700
/// Together with `update_claims_view_from_matched_txn` this used to be named

0 commit comments

Comments
 (0)