Skip to content

Remove data dependency on OnchainTxHandler from onchain claims #3690

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

Prev Previous commit
Next Next commit
Track ChannelTransactionParameters in RevokedHTLCOutput
The `ChannelMonitor` and `OnchainTxHandler` have historically been tied
together, often tracking some of the same state twice. As we introduce
support for splices in the `ChannelMonitor`, we'd like to avoid leaking
some of those details to the `OnchainTxHandler`. Ultimately, the
`OnchainTxHandler` should stand on its own and support claiming funds
from multiple `ChannelMonitor`s, allowing us to save on fees by batching
aggregatable claims across multiple in-flight closing channels.

Once splices are supported, we may run into cases where we are
attempting to claim an output from a specific `FundingScope`, while also
having an additional pending `FundingScope` for a splice. If the pending
splice confirms over the output claim, we need to cancel the claim and
re-offer it with the set of relevant parameters in the new
`FundingScope`.

This commit tracks the `ChannelTransactionParameters` for the specific
`FundingScope` the `RevokedHTLCOutput` claim originated from. This
allows us to remove the dependency on `OnchainTxHandler` when obtaining
the current `ChannelTransactionParameters` and ensures any alternative
state due to splicing does not leak into the `OnchainTxHandler`.
  • Loading branch information
wpaulino committed Apr 7, 2025
commit 97348c7f64cb98c789b49086f6543dae7eeb58e6
7 changes: 2 additions & 5 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3762,11 +3762,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
return (claimable_outpoints, to_counterparty_output_info);
}
let revk_htlc_outp = RevokedHTLCOutput::build(
per_commitment_point,
self.counterparty_commitment_params.counterparty_delayed_payment_base_key,
self.counterparty_commitment_params.counterparty_htlc_base_key,
per_commitment_key, htlc.amount_msat / 1000, htlc.clone(),
self.channel_type_features(),
per_commitment_point, per_commitment_key, htlc.clone(),
self.funding.channel_parameters.clone(),
);
let counterparty_spendable_height = if htlc.offered {
htlc.cltv_expiry
Expand Down
37 changes: 29 additions & 8 deletions lightning/src/chain/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,19 +181,32 @@ pub(crate) struct RevokedHTLCOutput {
weight: u64,
amount: u64,
htlc: HTLCOutputInCommitment,
channel_parameters: Option<ChannelTransactionParameters>,
}

impl RevokedHTLCOutput {
pub(crate) fn build(per_commitment_point: PublicKey, counterparty_delayed_payment_base_key: DelayedPaymentBasepoint, counterparty_htlc_base_key: HtlcBasepoint, per_commitment_key: SecretKey, amount: u64, htlc: HTLCOutputInCommitment, channel_type_features: &ChannelTypeFeatures) -> Self {
let weight = if htlc.offered { weight_revoked_offered_htlc(channel_type_features) } else { weight_revoked_received_htlc(channel_type_features) };
pub(crate) fn build(
per_commitment_point: PublicKey, per_commitment_key: SecretKey,
htlc: HTLCOutputInCommitment, channel_parameters: ChannelTransactionParameters,
) -> Self {
let weight = if htlc.offered {
weight_revoked_offered_htlc(&channel_parameters.channel_type_features)
} else {
weight_revoked_received_htlc(&channel_parameters.channel_type_features)
};
let directed_params = channel_parameters.as_counterparty_broadcastable();
let counterparty_keys = directed_params.broadcaster_pubkeys();
let counterparty_delayed_payment_base_key = counterparty_keys.delayed_payment_basepoint;
let counterparty_htlc_base_key = counterparty_keys.htlc_basepoint;
RevokedHTLCOutput {
per_commitment_point,
counterparty_delayed_payment_base_key,
counterparty_htlc_base_key,
per_commitment_key,
weight,
amount,
htlc
amount: htlc.amount_msat / 1000,
htlc,
channel_parameters: Some(channel_parameters),
}
}
}
Expand All @@ -206,6 +219,7 @@ impl_writeable_tlv_based!(RevokedHTLCOutput, {
(8, weight, required),
(10, amount, required),
(12, htlc, required),
(13, channel_parameters, (option: ReadableArgs, None)), // Added in 0.2.
});

/// A struct to describe a HTLC output on a counterparty commitment transaction.
Expand Down Expand Up @@ -789,8 +803,8 @@ impl PackageSolvingData {
} else { return false; }
},
PackageSolvingData::RevokedHTLCOutput(ref outp) => {
let directed_parameters =
&onchain_handler.channel_transaction_parameters.as_counterparty_broadcastable();
let channel_parameters = outp.channel_parameters.as_ref().unwrap_or(channel_parameters);
let directed_parameters = channel_parameters.as_counterparty_broadcastable();
debug_assert_eq!(
directed_parameters.broadcaster_pubkeys().delayed_payment_basepoint,
outp.counterparty_delayed_payment_base_key,
Expand All @@ -803,7 +817,9 @@ impl PackageSolvingData {
&outp.per_commitment_point, directed_parameters.broadcaster_pubkeys(),
directed_parameters.countersignatory_pubkeys(), &onchain_handler.secp_ctx,
);
let witness_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&outp.htlc, &onchain_handler.channel_type_features(), &chan_keys.broadcaster_htlc_key, &chan_keys.countersignatory_htlc_key, &chan_keys.revocation_key);
let witness_script = chan_utils::get_htlc_redeemscript(
&outp.htlc, &channel_parameters.channel_type_features, &chan_keys
);
//TODO: should we panic on signer failure ?
if let Ok(sig) = onchain_handler.signer.sign_justice_revoked_htlc(channel_parameters, &bumped_tx, i, outp.amount, &outp.per_commitment_key, &outp.htlc, &onchain_handler.secp_ctx) {
let mut ser_sig = sig.serialize_der().to_vec();
Expand Down Expand Up @@ -1624,7 +1640,12 @@ mod tests {
let dumb_point = PublicKey::from_secret_key(&secp_ctx, &dumb_scalar);
let hash = PaymentHash([1; 32]);
let htlc = HTLCOutputInCommitment { offered: false, amount_msat: 1_000_000, cltv_expiry: 0, payment_hash: hash, transaction_output_index: None };
PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput::build(dumb_point, DelayedPaymentBasepoint::from(dumb_point), HtlcBasepoint::from(dumb_point), dumb_scalar, 1_000_000 / 1_000, htlc, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies()))
let mut channel_parameters = ChannelTransactionParameters::test_dummy(0);
channel_parameters.channel_type_features =
ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies();
PackageSolvingData::RevokedHTLCOutput(RevokedHTLCOutput::build(
dumb_point, dumb_scalar, htlc, channel_parameters
))
}
}
}
Expand Down