Skip to content

[Custom Transactions] Provide Built Counterparty Commitment Transactions To ChannelMonitor #3654

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 11, 2025
Merged

[Custom Transactions] Provide Built Counterparty Commitment Transactions To ChannelMonitor #3654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
107 changes: 79 additions & 28 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,12 @@ pub(crate) enum ChannelMonitorUpdateStep {
to_broadcaster_value_sat: Option<u64>,
to_countersignatory_value_sat: Option<u64>,
},
LatestCounterpartyCommitmentTX {
// The dust and non-dust htlcs for that commitment
htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
// Contains only the non-dust htlcs
commitment_tx: CommitmentTransaction,
},
PaymentPreimage {
payment_preimage: PaymentPreimage,
/// If this preimage was from an inbound payment claim, information about the claim should
Expand Down Expand Up @@ -571,6 +577,7 @@ impl ChannelMonitorUpdateStep {
match self {
ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } => "LatestHolderCommitmentTXInfo",
ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } => "LatestCounterpartyCommitmentTXInfo",
ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } => "LatestCounterpartyCommitmentTX",
ChannelMonitorUpdateStep::PaymentPreimage { .. } => "PaymentPreimage",
ChannelMonitorUpdateStep::CommitmentSecret { .. } => "CommitmentSecret",
ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed",
Expand Down Expand Up @@ -609,6 +616,10 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
(5, ShutdownScript) => {
(0, scriptpubkey, required),
},
(6, LatestCounterpartyCommitmentTX) => {
(0, htlc_outputs, required_vec),
(2, commitment_tx, required),
},
);

/// Indicates whether the balance is derived from a cooperative close, a force-close
Expand Down Expand Up @@ -1019,6 +1030,11 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
/// Ordering of tuple data: (their_per_commitment_point, feerate_per_kw, to_broadcaster_sats,
/// to_countersignatory_sats)
initial_counterparty_commitment_info: Option<(PublicKey, u32, u64, u64)>,
/// Initial counterparty commitment transaction
///
/// We previously used the field above to re-build the counterparty commitment transaction,
/// we now provide the transaction outright.
initial_counterparty_commitment_tx: Option<CommitmentTransaction>,

/// The first block height at which we had no remaining claimable balances.
balances_empty_height: Option<u32>,
Expand Down Expand Up @@ -1242,6 +1258,7 @@ impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
(23, self.holder_pays_commitment_tx_fee, option),
(25, self.payment_preimages, required),
(27, self.first_confirmed_funding_txo, required),
(29, self.initial_counterparty_commitment_tx, option),
});

Ok(())
Expand Down Expand Up @@ -1454,6 +1471,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
best_block,
counterparty_node_id: counterparty_node_id,
initial_counterparty_commitment_info: None,
initial_counterparty_commitment_tx: None,
balances_empty_height: None,

failed_back_htlc_ids: new_hash_set(),
Expand Down Expand Up @@ -1486,24 +1504,19 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
}

/// A variant of `Self::provide_latest_counterparty_commitment_tx` used to provide
/// additional information to the monitor to store in order to recreate the initial
/// counterparty commitment transaction during persistence (mainly for use in third-party
/// watchtowers).
/// the counterparty commitment transaction to the monitor so that the transaction
/// can be retrieved during the initial persistence of the monitor (mainly for use in
/// third-party watchtowers).
///
/// This is used to provide the counterparty commitment information directly to the monitor
/// This is used to provide the counterparty commitment transaction directly to the monitor
/// before the initial persistence of a new channel.
pub(crate) fn provide_initial_counterparty_commitment_tx<L: Deref>(
&self, txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
commitment_number: u64, their_cur_per_commitment_point: PublicKey, feerate_per_kw: u32,
to_broadcaster_value_sat: u64, to_countersignatory_value_sat: u64, logger: &L,
)
where L::Target: Logger
&self, commitment_tx: CommitmentTransaction, logger: &L,
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's update the docs accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks ! Addressed below.

) where L::Target: Logger
{
let mut inner = self.inner.lock().unwrap();
let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
inner.provide_initial_counterparty_commitment_tx(txid,
htlc_outputs, commitment_number, their_cur_per_commitment_point, feerate_per_kw,
to_broadcaster_value_sat, to_countersignatory_value_sat, &logger);
inner.provide_initial_counterparty_commitment_tx(commitment_tx, &logger);
}

/// Informs this monitor of the latest counterparty (ie non-broadcastable) commitment transaction.
Expand Down Expand Up @@ -2872,20 +2885,21 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}

fn provide_initial_counterparty_commitment_tx<L: Deref>(
&mut self, txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
commitment_number: u64, their_per_commitment_point: PublicKey, feerate_per_kw: u32,
to_broadcaster_value: u64, to_countersignatory_value: u64, logger: &WithChannelMonitor<L>,
&mut self, commitment_tx: CommitmentTransaction, logger: &WithChannelMonitor<L>,
) where L::Target: Logger {
self.initial_counterparty_commitment_info = Some((their_per_commitment_point.clone(),
feerate_per_kw, to_broadcaster_value, to_countersignatory_value));
// We populate this field for downgrades
self.initial_counterparty_commitment_info = Some((commitment_tx.per_commitment_point(),
commitment_tx.feerate_per_kw(), commitment_tx.to_broadcaster_value_sat(), commitment_tx.to_countersignatory_value_sat()));

#[cfg(debug_assertions)] {
let rebuilt_commitment_tx = self.initial_counterparty_commitment_tx().unwrap();
debug_assert_eq!(rebuilt_commitment_tx.trust().txid(), txid);
debug_assert_eq!(rebuilt_commitment_tx.trust().txid(), commitment_tx.trust().txid());
}

self.provide_latest_counterparty_commitment_tx(txid, htlc_outputs, commitment_number,
their_per_commitment_point, logger);
self.provide_latest_counterparty_commitment_tx(commitment_tx.trust().txid(), Vec::new(), commitment_tx.commitment_number(),
commitment_tx.per_commitment_point(), logger);
// Soon, we will only populate this field
self.initial_counterparty_commitment_tx = Some(commitment_tx);
}

fn provide_latest_counterparty_commitment_tx<L: Deref>(
Expand Down Expand Up @@ -3223,10 +3237,17 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
if self.lockdown_from_offchain { panic!(); }
self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone());
}
// Soon we will drop the `LatestCounterpartyCommitmentTXInfo` variant in favor of `LatestCounterpartyCommitmentTX`.
// For now we just add the code to handle the new updates.
// Next step: in channel, switch channel monitor updates to use the `LatestCounterpartyCommitmentTX` variant.
ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => {
log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
self.provide_latest_counterparty_commitment_tx(*commitment_txid, htlc_outputs.clone(), *commitment_number, *their_per_commitment_point, logger)
},
ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { htlc_outputs, commitment_tx } => {
log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
self.provide_latest_counterparty_commitment_tx(commitment_tx.trust().txid(), htlc_outputs.clone(), commitment_tx.commitment_number(), commitment_tx.per_commitment_point(), logger)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we handle having both the legacy and new version? I guess i was hoping we wouldn't have to wait a release or two for this stuff (which we have to do if we're trying to bridge versions here).

Copy link
Contributor

Choose a reason for hiding this comment

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

We could -- I think it was done that way first in #3606 but I was concerned about the extra storage cost. We're planning to use this new variant (along with a similar one for latest holder commitment) for spliced channels as a way to move to the new format sooner since you can't downgrade with a pending splice anyway.

Copy link
Contributor Author

@tankyleo tankyleo Mar 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think it was done that way first in #3606 but I was concerned about the extra storage cost.

Right yes we previously added another field to the legacy variant that contained the full transaction. This made most of the fields in the legacy variant redundant so we went for a new variant.

Then to alleviate the storage cost we decided against immediately setting this new variant.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea, okay, that all makes sense, just annoying that we have to wait a release or two to ship things. Alas.

},
ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage, payment_info } => {
log_trace!(logger, "Updating ChannelMonitor with payment preimage");
self.provide_payment_preimage(&PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array()), &payment_preimage, payment_info, broadcaster, &bounded_fee_estimator, logger)
Expand Down Expand Up @@ -3289,6 +3310,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
match update {
ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. }
|ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. }
|ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. }
|ChannelMonitorUpdateStep::ShutdownScript { .. }
|ChannelMonitorUpdateStep::CommitmentSecret { .. } =>
is_pre_close_update = true,
Expand Down Expand Up @@ -3419,14 +3441,32 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}

fn initial_counterparty_commitment_tx(&mut self) -> Option<CommitmentTransaction> {
let (their_per_commitment_point, feerate_per_kw, to_broadcaster_value,
to_countersignatory_value) = self.initial_counterparty_commitment_info?;
let htlc_outputs = vec![];

let commitment_tx = self.build_counterparty_commitment_tx(INITIAL_COMMITMENT_NUMBER,
&their_per_commitment_point, to_broadcaster_value, to_countersignatory_value,
feerate_per_kw, htlc_outputs);
Some(commitment_tx)
self.initial_counterparty_commitment_tx.clone().or_else(|| {
// This provides forward compatibility; an old monitor will not contain the full
// transaction; only enough information to rebuild it
self.initial_counterparty_commitment_info.map(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we ever return None now? It seems before we couldn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before we could return None at this call self.initial_counterparty_commitment_info?;

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, for reason I was unaware that ? worked with Option return types.

|(
their_per_commitment_point,
feerate_per_kw,
to_broadcaster_value,
to_countersignatory_value,
)| {
let htlc_outputs = vec![];

let commitment_tx = self.build_counterparty_commitment_tx(
INITIAL_COMMITMENT_NUMBER,
&their_per_commitment_point,
to_broadcaster_value,
to_countersignatory_value,
feerate_per_kw,
htlc_outputs,
);
// Take the opportunity to populate this recently introduced field
self.initial_counterparty_commitment_tx = Some(commitment_tx.clone());
commitment_tx
},
)
})
}

fn build_counterparty_commitment_tx(
Expand Down Expand Up @@ -3454,6 +3494,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {

fn counterparty_commitment_txs_from_update(&self, update: &ChannelMonitorUpdate) -> Vec<CommitmentTransaction> {
update.updates.iter().filter_map(|update| {
// Soon we will drop the first branch here in favor of the second.
// In preparation, we just add the second branch without deleting the first.
// Next step: in channel, switch channel monitor updates to use the `LatestCounterpartyCommitmentTX` variant.
match update {
&ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid,
ref htlc_outputs, commitment_number, their_per_commitment_point,
Expand All @@ -3473,6 +3516,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {

Some(commitment_tx)
},
&ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX {
htlc_outputs: _, ref commitment_tx,
} => {
Some(commitment_tx.clone())
},
_ => None,
}
}).collect()
Expand Down Expand Up @@ -5043,6 +5091,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
let mut spendable_txids_confirmed = Some(Vec::new());
let mut counterparty_fulfilled_htlcs = Some(new_hash_map());
let mut initial_counterparty_commitment_info = None;
let mut initial_counterparty_commitment_tx = None;
let mut balances_empty_height = None;
let mut channel_id = None;
let mut holder_pays_commitment_tx_fee = None;
Expand All @@ -5063,6 +5112,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
(23, holder_pays_commitment_tx_fee, option),
(25, payment_preimages_with_info, option),
(27, first_confirmed_funding_txo, (default_value, funding_info.0)),
(29, initial_counterparty_commitment_tx, option),
});
if let Some(payment_preimages_with_info) = payment_preimages_with_info {
if payment_preimages_with_info.len() != payment_preimages.len() {
Expand Down Expand Up @@ -5166,6 +5216,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
best_block,
counterparty_node_id: counterparty_node_id.unwrap(),
initial_counterparty_commitment_info,
initial_counterparty_commitment_tx,
balances_empty_height,
failed_back_htlc_ids: new_hash_set(),
})))
Expand Down
22 changes: 7 additions & 15 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2062,7 +2062,7 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide

fn initial_commitment_signed<L: Deref>(
&mut self, channel_id: ChannelId, counterparty_signature: Signature, holder_commitment_point: &mut HolderCommitmentPoint,
counterparty_commitment_number: u64, best_block: BestBlock, signer_provider: &SP, logger: &L,
best_block: BestBlock, signer_provider: &SP, logger: &L,
) -> Result<(ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>, CommitmentTransaction), ChannelError>
where
L::Target: Logger
Expand Down Expand Up @@ -2140,14 +2140,7 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
funding_redeemscript.clone(), funding.get_value_satoshis(),
obscure_factor,
holder_commitment_tx, best_block, context.counterparty_node_id, context.channel_id());
channel_monitor.provide_initial_counterparty_commitment_tx(
counterparty_initial_bitcoin_tx.txid, Vec::new(),
counterparty_commitment_number,
context.counterparty_cur_commitment_point.unwrap(),
counterparty_initial_commitment_tx.feerate_per_kw(),
counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
counterparty_initial_commitment_tx.to_countersignatory_value_sat(),
logger);
channel_monitor.provide_initial_counterparty_commitment_tx(counterparty_initial_commitment_tx.clone(), logger);

self.context_mut().cur_counterparty_commitment_transaction_number -= 1;

Expand Down Expand Up @@ -5681,8 +5674,7 @@ impl<SP: Deref> FundedChannel<SP> where
self.context.assert_no_commitment_advancement(holder_commitment_point.transaction_number(), "initial commitment_signed");

let (channel_monitor, _) = self.initial_commitment_signed(
self.context.channel_id(), msg.signature, holder_commitment_point,
self.context.cur_counterparty_commitment_transaction_number, best_block, signer_provider, logger)?;
self.context.channel_id(), msg.signature, holder_commitment_point, best_block, signer_provider, logger)?;
self.holder_commitment_point = *holder_commitment_point;

log_info!(logger, "Received initial commitment_signed from peer for channel {}", &self.context.channel_id());
Expand Down Expand Up @@ -8743,6 +8735,8 @@ impl<SP: Deref> FundedChannel<SP> where
self.context.latest_monitor_update_id += 1;
let monitor_update = ChannelMonitorUpdate {
update_id: self.context.latest_monitor_update_id,
// Soon, we will switch this to `LatestCounterpartyCommitmentTX`,
// and provide the full commit tx instead of the information needed to rebuild it.
updates: vec![ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
commitment_txid: counterparty_commitment_txid,
htlc_outputs: htlcs.clone(),
Expand Down Expand Up @@ -9465,8 +9459,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {

let (channel_monitor, _) = match self.initial_commitment_signed(
self.context.channel_id(), msg.signature,
&mut holder_commitment_point, self.context.cur_counterparty_commitment_transaction_number,
best_block, signer_provider, logger
&mut holder_commitment_point, best_block, signer_provider, logger
) {
Ok(channel_monitor) => channel_monitor,
Err(err) => return Err((self, err)),
Expand Down Expand Up @@ -9735,8 +9728,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {

let (channel_monitor, counterparty_initial_commitment_tx) = match self.initial_commitment_signed(
ChannelId::v1_from_funding_outpoint(funding_txo), msg.signature,
&mut holder_commitment_point, self.context.cur_counterparty_commitment_transaction_number,
best_block, signer_provider, logger
&mut holder_commitment_point, best_block, signer_provider, logger
) {
Ok(channel_monitor) => channel_monitor,
Err(err) => return Err((self, err)),
Expand Down
Loading