Skip to content

Commit

Permalink
Provide built counterparty commitment transactions to ChannelMonitor
Browse files Browse the repository at this point in the history 
`LatestCounterpartyCommitmentTXInfo` provides `ChannelMonitor` with the
data it needs to build counterparty commitment transactions.
`ChannelMonitor` then rebuilds commitment transactions from these pieces
of data when requested.

This commit adds a new variant to `ChannelMonitorUpdateStep` called
`LatestCounterpartyCommitmentTX`, which will provide fully built
commitment transactions to `ChannelMonitor`. When the `ChannelMonitor`
is asked for the counterparty commitment transactions, it will not
rebuild them, but just clone them.

As a result, this new variant eliminates any calls to the upcoming
`TxBuilder` trait in `ChannelMonitor`, which gets rid of a generic
parameter on `ChannelMonitor`. This commit also avoids any footguns that
might come with passing around disparate pieces of data to re-build
commitment transactions from scratch.

We also add a `htlc_outputs` field to the variant to include the dust
HTLCs, as well as all the HTLC sources. This means that this new variant
will store non-dust HTLCs both in the `htlc_outputs` field, and in the
`commitment_tx` field. This is ok for now as the number of HTLCs
per-commitment nowadays is very low.

We add code to handle this new variant, but refrain from
immediately setting it. This will allow us to atomically switch from
`LatestCounterpartyCommitmentTXInfo` to `LatestCounterpartyCommitmentTX`
in the future while still allowing downgrades.
  • Loading branch information
@tankyleo
tankyleo committed Mar 8, 2025
1 parent 4c43a5b commit 6e70599
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 39 deletions.
88 changes: 64 additions & 24 deletions lightning/src/chain/channelmonitor.rs
View file
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 @@ -1493,17 +1511,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
/// This is used to provide the counterparty commitment information 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,
) 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)
},
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,21 @@ 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(|(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 +3483,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 +3505,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 +5080,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 +5101,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 +5205,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
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

0 comments on commit 6e70599

Please sign in to comment.