Skip to content

Commit

Permalink
Allow holder commitment and HTLC signature requests to fail
Browse files Browse the repository at this point in the history 
As part of the ongoing async signer work, our holder signatures must
also be capable of being obtained asynchronously. We expose a new
`ChannelMonitor::signer_unblocked` method to retry pending onchain
claims by re-signing and rebroadcasting transactions. Unfortunately, we
cannot retry said claims without them being registered first, so if
we're not able to obtain the signature synchronously, we must return the
transaction as unsigned and ensure it is not broadcast.
  • Loading branch information
@wpaulino
wpaulino committed Feb 6, 2024
1 parent 67c140b commit 2ba1858
Show file tree
Hide file tree
Showing 10 changed files with 323 additions and 50 deletions.
21 changes: 21 additions & 0 deletions lightning/src/chain/chainmonitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,27 @@ where C::Target: chain::Filter,
)
}
}

/// Triggers rebroadcasts of pending claims from force-closed channels after a transaction
/// signature generation failure.
///
/// `monitor_opt` can be used as a filter to only trigger them for a specific channel monitor.
pub fn signer_unblocked(&self, monitor_opt: Option<OutPoint>) {
let monitors = self.monitors.read().unwrap();
if let Some(funding_txo) = monitor_opt {
if let Some(monitor_holder) = monitors.get(&funding_txo) {
monitor_holder.monitor.signer_unblocked(
&*self.broadcaster, &*self.fee_estimator, &self.logger
)
}
} else {
for (_, monitor_holder) in &*monitors {
monitor_holder.monitor.signer_unblocked(
&*self.broadcaster, &*self.fee_estimator, &self.logger
)
}
}
}
}

impl<ChannelSigner: WriteableEcdsaChannelSigner, C: Deref, T: Deref, F: Deref, L: Deref, P: Deref>
Expand Down
34 changes: 31 additions & 3 deletions lightning/src/chain/channelmonitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1769,6 +1769,25 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
);
}

/// Triggers rebroadcasts of pending claims from a force-closed channel after a transaction
/// signature generation failure.
pub fn signer_unblocked<B: Deref, F: Deref, L: Deref>(
&self, broadcaster: B, fee_estimator: F, logger: &L,
)
where
B::Target: BroadcasterInterface,
F::Target: FeeEstimator,
L::Target: Logger,
{
let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
let mut inner = self.inner.lock().unwrap();
let logger = WithChannelMonitor::from_impl(logger, &*inner);
let current_height = inner.best_block.height;
inner.onchain_tx_handler.rebroadcast_pending_claims(
current_height, FeerateStrategy::RetryPrevious, &broadcaster, &fee_estimator, &logger,
);
}

/// Returns the descriptors for relevant outputs (i.e., those that we can spend) within the
/// transaction if they exist and the transaction has at least [`ANTI_REORG_DELAY`]
/// confirmations. For [`SpendableOutputDescriptor::DelayedPaymentOutput`] descriptors to be
Expand Down Expand Up @@ -1811,6 +1830,12 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
pub fn set_counterparty_payment_script(&self, script: ScriptBuf) {
self.inner.lock().unwrap().counterparty_payment_script = script;
}

#[cfg(test)]
pub fn do_signer_call<F: FnMut(&Signer) -> ()>(&self, mut f: F) {
let inner = self.inner.lock().unwrap();
f(&inner.onchain_tx_handler.signer);
}
}

impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
Expand Down Expand Up @@ -3508,9 +3533,12 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
continue;
}
} else { None };
if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(
&::bitcoin::OutPoint { txid, vout }, &preimage) {
holder_transactions.push(htlc_tx);
if let Some(htlc_tx) = self.onchain_tx_handler.get_maybe_signed_htlc_tx(
&::bitcoin::OutPoint { txid, vout }, &preimage
) {
if !htlc_tx.input.iter().any(|input| input.witness.is_empty()) {
holder_transactions.push(htlc_tx);
}
}
}
}
Expand Down
67 changes: 42 additions & 25 deletions lightning/src/chain/onchaintx.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ pub(crate) enum OnchainClaim {

/// Represents the different feerates a pending request can use when generating a claim.
pub(crate) enum FeerateStrategy {
/// We must reuse the most recently used feerate, if any.
RetryPrevious,
/// We must pick the highest between the most recently used and the current feerate estimate.
HighestOfPreviousOrNew,
/// We must force a bump of the most recently used feerate, either by using the current feerate
Expand Down Expand Up @@ -506,9 +508,13 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
}
match claim {
OnchainClaim::Tx(tx) => {
let log_start = if bumped_feerate { "Broadcasting RBF-bumped" } else { "Rebroadcasting" };
log_info!(logger, "{} onchain {}", log_start, log_tx!(tx));
broadcaster.broadcast_transactions(&[&tx]);
if tx.input.iter().any(|input| input.witness.is_empty()) {
log_info!(logger, "Waiting for signature of unsigned onchain transaction {}", tx.txid());
} else {
let log_start = if bumped_feerate { "Broadcasting RBF-bumped" } else { "Rebroadcasting" };
log_info!(logger, "{} onchain {}", log_start, log_tx!(tx));
broadcaster.broadcast_transactions(&[&tx]);
}
},
OnchainClaim::Event(event) => {
let log_start = if bumped_feerate { "Yielding fee-bumped" } else { "Replaying" };
Expand Down Expand Up @@ -610,7 +616,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
) {
assert!(new_feerate != 0);

let transaction = cached_request.finalize_malleable_package(
let transaction = cached_request.maybe_finalize_malleable_package(
cur_height, self, output_value, self.destination_script.clone(), logger
).unwrap();
log_trace!(logger, "...with timer {} and feerate {}", new_timer, new_feerate);
Expand All @@ -623,7 +629,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
// which require external funding.
let mut inputs = cached_request.inputs();
debug_assert_eq!(inputs.len(), 1);
let tx = match cached_request.finalize_untractable_package(self, logger) {
let tx = match cached_request.maybe_finalize_untractable_package(self, logger) {
Some(tx) => tx,
None => return None,
};
Expand All @@ -645,8 +651,8 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
let commitment_tx_feerate_sat_per_1000_weight =
compute_feerate_sat_per_1000_weight(fee_sat, tx.weight().to_wu());
if commitment_tx_feerate_sat_per_1000_weight >= package_target_feerate_sat_per_1000_weight {
log_debug!(logger, "Pre-signed {} already has feerate {} sat/kW above required {} sat/kW",
log_tx!(tx), commitment_tx_feerate_sat_per_1000_weight,
log_debug!(logger, "Pre-signed commitment {} already has feerate {} sat/kW above required {} sat/kW",
tx.txid(), commitment_tx_feerate_sat_per_1000_weight,
package_target_feerate_sat_per_1000_weight);
return Some((new_timer, 0, OnchainClaim::Tx(tx.clone())));
}
Expand Down Expand Up @@ -785,8 +791,12 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
// `OnchainClaim`.
let claim_id = match claim {
OnchainClaim::Tx(tx) => {
log_info!(logger, "Broadcasting onchain {}", log_tx!(tx));
broadcaster.broadcast_transactions(&[&tx]);
if tx.input.iter().any(|input| input.witness.is_empty()) {
log_info!(logger, "Waiting for signature of unsigned onchain transaction {}", tx.txid());
} else {
log_info!(logger, "Broadcasting onchain {}", log_tx!(tx));
broadcaster.broadcast_transactions(&[&tx]);
}
ClaimId(tx.txid().to_byte_array())
},
OnchainClaim::Event(claim_event) => {
Expand Down Expand Up @@ -978,8 +988,13 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
) {
match bump_claim {
OnchainClaim::Tx(bump_tx) => {
log_info!(logger, "Broadcasting RBF-bumped onchain {}", log_tx!(bump_tx));
broadcaster.broadcast_transactions(&[&bump_tx]);
if bump_tx.input.iter().any(|input| input.witness.is_empty()) {
log_info!(logger, "Waiting for signature of RBF-bumped unsigned onchain transaction {}",
bump_tx.txid());
} else {
log_info!(logger, "Broadcasting RBF-bumped onchain {}", log_tx!(bump_tx));
broadcaster.broadcast_transactions(&[&bump_tx]);
}
},
OnchainClaim::Event(claim_event) => {
log_info!(logger, "Yielding RBF-bumped onchain event to spend inputs {:?}", request.outpoints());
Expand Down Expand Up @@ -1061,8 +1076,12 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
request.set_feerate(new_feerate);
match bump_claim {
OnchainClaim::Tx(bump_tx) => {
log_info!(logger, "Broadcasting onchain {}", log_tx!(bump_tx));
broadcaster.broadcast_transactions(&[&bump_tx]);
if bump_tx.input.iter().any(|input| input.witness.is_empty()) {
log_info!(logger, "Waiting for signature of unsigned onchain transaction {}", bump_tx.txid());
} else {
log_info!(logger, "Broadcasting onchain {}", log_tx!(bump_tx));
broadcaster.broadcast_transactions(&[&bump_tx]);
}
},
OnchainClaim::Event(claim_event) => {
log_info!(logger, "Yielding onchain event after reorg to spend inputs {:?}", request.outpoints());
Expand Down Expand Up @@ -1115,13 +1134,10 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
&self.holder_commitment.trust().built_transaction().transaction
}

//TODO: getting lastest holder transactions should be infallible and result in us "force-closing the channel", but we may
// have empty holder commitment transaction if a ChannelMonitor is asked to force-close just after OutboundV1Channel::get_funding_created,
// before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing
// to monitor before.
pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {
let sig = self.signer.sign_holder_commitment(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment");
self.holder_commitment.add_holder_sig(funding_redeemscript, sig)
pub(crate) fn get_maybe_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {
self.signer.sign_holder_commitment(&self.holder_commitment, &self.secp_ctx)
.map(|sig| self.holder_commitment.add_holder_sig(funding_redeemscript, sig))
.unwrap_or_else(|_| self.get_unsigned_holder_commitment_tx().clone())
}

#[cfg(any(test, feature="unsafe_revoked_tx_signing"))]
Expand All @@ -1130,7 +1146,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
self.holder_commitment.add_holder_sig(funding_redeemscript, sig)
}

pub(crate) fn get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option<PaymentPreimage>) -> Option<Transaction> {
pub(crate) fn get_maybe_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option<PaymentPreimage>) -> Option<Transaction> {
let get_signed_htlc_tx = |holder_commitment: &HolderCommitmentTransaction| {
let trusted_tx = holder_commitment.trust();
if trusted_tx.txid() != outp.txid {
Expand Down Expand Up @@ -1158,10 +1174,11 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
preimage: preimage.clone(),
counterparty_sig: counterparty_htlc_sig.clone(),
};
let htlc_sig = self.signer.sign_holder_htlc_transaction(&htlc_tx, 0, &htlc_descriptor, &self.secp_ctx).unwrap();
htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness(
htlc_idx, &counterparty_htlc_sig, &htlc_sig, preimage,
);
if let Ok(htlc_sig) = self.signer.sign_holder_htlc_transaction(&htlc_tx, 0, &htlc_descriptor, &self.secp_ctx) {
htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness(
htlc_idx, &counterparty_htlc_sig, &htlc_sig, preimage,
);
}
Some(htlc_tx)
};

Expand Down
21 changes: 12 additions & 9 deletions lightning/src/chain/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -633,14 +633,14 @@ impl PackageSolvingData {
}
true
}
fn get_finalized_tx<Signer: WriteableEcdsaChannelSigner>(&self, outpoint: &BitcoinOutPoint, onchain_handler: &mut OnchainTxHandler<Signer>) -> Option<Transaction> {
fn get_maybe_finalized_tx<Signer: WriteableEcdsaChannelSigner>(&self, outpoint: &BitcoinOutPoint, onchain_handler: &mut OnchainTxHandler<Signer>) -> Option<Transaction> {
match self {
PackageSolvingData::HolderHTLCOutput(ref outp) => {
debug_assert!(!outp.channel_type_features.supports_anchors_zero_fee_htlc_tx());
return onchain_handler.get_fully_signed_htlc_tx(outpoint, &outp.preimage);
onchain_handler.get_maybe_signed_htlc_tx(outpoint, &outp.preimage)
}
PackageSolvingData::HolderFundingOutput(ref outp) => {
return Some(onchain_handler.get_fully_signed_holder_tx(&outp.funding_redeemscript));
Some(onchain_handler.get_maybe_signed_holder_tx(&outp.funding_redeemscript))
}
_ => { panic!("API Error!"); }
}
Expand Down Expand Up @@ -908,7 +908,7 @@ impl PackageTemplate {
}
htlcs
}
pub(crate) fn finalize_malleable_package<L: Logger, Signer: WriteableEcdsaChannelSigner>(
pub(crate) fn maybe_finalize_malleable_package<L: Logger, Signer: WriteableEcdsaChannelSigner>(
&self, current_height: u32, onchain_handler: &mut OnchainTxHandler<Signer>, value: u64,
destination_script: ScriptBuf, logger: &L
) -> Option<Transaction> {
Expand All @@ -927,19 +927,17 @@ impl PackageTemplate {
}
for (i, (outpoint, out)) in self.inputs.iter().enumerate() {
log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);
if !out.finalize_input(&mut bumped_tx, i, onchain_handler) { return None; }
if !out.finalize_input(&mut bumped_tx, i, onchain_handler) { continue; }
}
log_debug!(logger, "Finalized transaction {} ready to broadcast", bumped_tx.txid());
Some(bumped_tx)
}
pub(crate) fn finalize_untractable_package<L: Logger, Signer: WriteableEcdsaChannelSigner>(
pub(crate) fn maybe_finalize_untractable_package<L: Logger, Signer: WriteableEcdsaChannelSigner>(
&self, onchain_handler: &mut OnchainTxHandler<Signer>, logger: &L,
) -> Option<Transaction> {
debug_assert!(!self.is_malleable());
if let Some((outpoint, outp)) = self.inputs.first() {
if let Some(final_tx) = outp.get_finalized_tx(outpoint, onchain_handler) {
if let Some(final_tx) = outp.get_maybe_finalized_tx(outpoint, onchain_handler) {
log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);
log_debug!(logger, "Finalized transaction {} ready to broadcast", final_tx.txid());
return Some(final_tx);
}
return None;
Expand Down Expand Up @@ -996,6 +994,7 @@ impl PackageTemplate {
if self.feerate_previous != 0 {
let previous_feerate = self.feerate_previous.try_into().unwrap_or(u32::max_value());
match feerate_strategy {
FeerateStrategy::RetryPrevious => previous_feerate,
FeerateStrategy::HighestOfPreviousOrNew => cmp::max(previous_feerate, feerate_estimate),
FeerateStrategy::ForceBump => if feerate_estimate > previous_feerate {
feerate_estimate
Expand Down Expand Up @@ -1141,6 +1140,10 @@ where
// If old feerate inferior to actual one given back by Fee Estimator, use it to compute new fee...
let (new_fee, new_feerate) = if let Some((new_fee, new_feerate)) = compute_fee_from_spent_amounts(input_amounts, predicted_weight, fee_estimator, logger) {
match feerate_strategy {
FeerateStrategy::RetryPrevious => {
let previous_fee = previous_feerate * predicted_weight / 1000;
(previous_fee, previous_feerate)
},
FeerateStrategy::HighestOfPreviousOrNew => if new_feerate > previous_feerate {
(new_fee, new_feerate)
} else {
Expand Down
Loading

0 comments on commit 2ba1858

Please sign in to comment.