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 `ChannelMonitor` is
asked for 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 avoids adding a generic
parameter on `ChannelMonitor`. This commit also stomps any bugs that
might come from 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 29a8b9f
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 29a8b9f

Please sign in to comment.