Skip to content

Produce a single monitor update per commitment number #3738

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Produce a single monitor update per commitment number #3738

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
85 changes: 65 additions & 20 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::types::features::ChannelTypeFeatures;
use crate::types::payment::{PaymentHash, PaymentPreimage};
use crate::ln::msgs::DecodeError;
use crate::ln::channel_keys::{DelayedPaymentKey, DelayedPaymentBasepoint, HtlcBasepoint, HtlcKey, RevocationKey, RevocationBasepoint};
use crate::ln::chan_utils::{self,CommitmentTransaction, CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCClaim, ChannelTransactionParameters, HolderCommitmentTransaction};
use crate::ln::chan_utils::{self,CommitmentTransaction, CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCClaim, ChannelTransactionParameters, HolderCommitmentTransaction, HTLCOutputData};
use crate::ln::channelmanager::{HTLCSource, SentHTLCId, PaymentClaimDetails};
use crate::chain;
use crate::chain::{BestBlock, WatchedOutput};
Expand Down Expand Up @@ -591,11 +591,18 @@ pub(crate) enum ChannelMonitorUpdateStep {
to_broadcaster_value_sat: Option<u64>,
to_countersignatory_value_sat: Option<u64>,
},
LatestCounterpartyCommitmentTX {
LatestHolderCommitmentTXs {
// The dust and non-dust htlcs for that commitment
htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
htlc_data: Vec<(HTLCOutputData, Option<Box<HTLCSource>>)>,
// Contains only the non-dust htlcs
commitment_txs: Vec<HolderCommitmentTransaction>,
claimed_htlcs: Vec<(SentHTLCId, PaymentPreimage)>,
},
LatestCounterpartyCommitmentTXs {
// The dust and non-dust htlcs for that commitment
htlc_data: Vec<(HTLCOutputData, Option<Box<HTLCSource>>)>,
// Contains only the non-dust htlcs
commitment_tx: CommitmentTransaction,
commitment_txs: Vec<CommitmentTransaction>,
},
PaymentPreimage {
payment_preimage: PaymentPreimage,
Expand Down Expand Up @@ -624,7 +631,8 @@ impl ChannelMonitorUpdateStep {
match self {
ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } => "LatestHolderCommitmentTXInfo",
ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } => "LatestCounterpartyCommitmentTXInfo",
ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } => "LatestCounterpartyCommitmentTX",
ChannelMonitorUpdateStep::LatestHolderCommitmentTXs { .. } => "LatestHolderCommitmentTXs",
ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXs { .. } => "LatestCounterpartyCommitmentTXs",
ChannelMonitorUpdateStep::PaymentPreimage { .. } => "PaymentPreimage",
ChannelMonitorUpdateStep::CommitmentSecret { .. } => "CommitmentSecret",
ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed",
Expand Down Expand Up @@ -663,9 +671,14 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
(5, ShutdownScript) => {
(0, scriptpubkey, required),
},
(6, LatestCounterpartyCommitmentTX) => {
(0, htlc_outputs, required_vec),
(2, commitment_tx, required),
(6, LatestHolderCommitmentTXs) => { // Added in 0.2
(1, htlc_data, required_vec),
(3, commitment_txs, required_vec),
(5, claimed_htlcs, required_vec),
},
(8, LatestCounterpartyCommitmentTXs) => { // Added in 0.2
(1, htlc_data, required_vec),
(3, commitment_txs, required_vec),
},
);

Expand Down Expand Up @@ -3084,6 +3097,19 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
}

// This is the sibling of `provide_latest_counterparty_commitment_tx`, but updated for a world
// in which the HTLC-source table passed from channel does NOT store dust-vs-nondust and
// index HTLC data.
fn provide_latest_counterparty_commitment_data<L: Deref>(
&mut self,
_htlc_data: Vec<(HTLCOutputData, Option<Box<HTLCSource>>)>,
_txs: &Vec<CommitmentTransaction>,
_logger: &WithChannelMonitor<L>,
) where L::Target: Logger {
// TODO(splicing, 0.2): Populate this monitor's data structures
todo!();
}

/// Informs this monitor of the latest holder (ie broadcastable) commitment transaction. The
/// monitor watches for timeouts and may broadcast it if we approach such a timeout. Thus, it
/// is important that any clones of this channel monitor (including remote clones) by kept
Expand Down Expand Up @@ -3175,6 +3201,19 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
}
}

// This is the sibling of `provide_latest_holder_commitment_tx`, but updated for a world
// in which the HTLC-source table passed from channel does NOT store dust-vs-nondust and
// index HTLC data.
fn provide_latest_holder_commitment_data(
&mut self,
_htlc_data: Vec<(HTLCOutputData, Option<Box<HTLCSource>>)>,
_holder_commitment_txs: Vec<HolderCommitmentTransaction>,
_claimed_htlcs: &[(SentHTLCId, PaymentPreimage)],
) {
// TODO(splicing, 0.2): Populate this monitor's data structures
todo!();
}

/// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all
/// commitment_tx_infos which contain the payment hash have been revoked.
///
Expand Down Expand Up @@ -3392,16 +3431,21 @@ 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`.
// Soon we will drop the `LatestCounterpartyCommitmentTXInfo` variant in favor of `LatestCounterpartyCommitmentTXs`.
// 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.
// Next step: in channel, switch channel monitor updates to use the `LatestCounterpartyCommitmentTXs` 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::LatestHolderCommitmentTXs { htlc_data, commitment_txs, claimed_htlcs } => {
log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction(s)");
if self.lockdown_from_offchain { panic!(); }
self.provide_latest_holder_commitment_data(htlc_data.clone(), commitment_txs.clone(), claimed_htlcs)
}
ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXs { htlc_data, commitment_txs } => {
log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction(s)");
self.provide_latest_counterparty_commitment_data(htlc_data.clone(), commitment_txs, logger)
},
ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage, payment_info } => {
log_trace!(logger, "Updating ChannelMonitor with payment preimage");
Expand Down Expand Up @@ -3465,7 +3509,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
match update {
ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. }
|ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. }
|ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. }
|ChannelMonitorUpdateStep::LatestHolderCommitmentTXs { .. }
|ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXs { .. }
|ChannelMonitorUpdateStep::ShutdownScript { .. }
|ChannelMonitorUpdateStep::CommitmentSecret { .. } =>
is_pre_close_update = true,
Expand Down Expand Up @@ -3620,7 +3665,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
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.
// Next step: in channel, switch channel monitor updates to use the `LatestCounterpartyCommitmentTXs` variant.
match update {
&ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid,
ref htlc_outputs, commitment_number, their_per_commitment_point,
Expand All @@ -3638,16 +3683,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {

debug_assert_eq!(commitment_tx.trust().txid(), commitment_txid);

Some(commitment_tx)
Some(vec![commitment_tx])
},
&ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX {
htlc_outputs: _, ref commitment_tx,
&ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXs {
htlc_data: _, ref commitment_txs,
} => {
Some(commitment_tx.clone())
Some(commitment_txs.clone())
},
_ => None,
}
}).collect()
}).flatten().collect()
}

fn sign_to_local_justice_tx(
Expand Down
19 changes: 19 additions & 0 deletions lightning/src/ln/chan_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,25 @@ pub fn get_counterparty_payment_script(channel_type_features: &ChannelTypeFeatur
}
}

/// The subset of the fields in `HTLCOutputInCommitment` that is constant across multiple
/// commitment transactions for a commitment number (ie. splices). Therefore, this struct does not
/// store information on whether the HTLC is dust or non-dust, and if non-dust, which index in the
/// commitment transaction the HTLC got assigned to.
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct HTLCOutputData {
pub(crate) offered: bool,
pub(crate) amount_msat: u64,
pub(crate) cltv_expiry: u32,
pub(crate) payment_hash: PaymentHash,
}

impl_writeable_tlv_based!(HTLCOutputData, {
(1, offered, required),
(3, amount_msat, required),
(5, cltv_expiry, required),
(7, payment_hash, required),
});

/// Information about an HTLC as it appears in a commitment transaction
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct HTLCOutputInCommitment {
Expand Down
43 changes: 22 additions & 21 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9013,31 +9013,32 @@ impl<SP: Deref> FundedChannel<SP> where
}
self.context.resend_order = RAACommitmentOrder::RevokeAndACKFirst;

let mut updates = Vec::with_capacity(self.pending_funding.len() + 1);
for funding in core::iter::once(&self.funding).chain(self.pending_funding.iter()) {
// Even in the pending splices case, we will send a single update for each commitment number.
// In that case, the update will contain multiple commitment transactions.
let mut updates = vec![];
if self.pending_funding.is_empty() {
let (htlcs_ref, counterparty_commitment_tx) =
self.build_commitment_no_state_update(funding, logger);
self.build_commitment_no_state_update(&self.funding, logger);
let htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)> =
htlcs_ref.into_iter().map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect();

if self.pending_funding.is_empty() {
// Soon, we will switch this to `LatestCounterpartyCommitmentTX`,
// and provide the full commit tx instead of the information needed to rebuild it.
updates.push(ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
commitment_txid: counterparty_commitment_tx.trust().txid(),
htlc_outputs,
commitment_number: self.context.cur_counterparty_commitment_transaction_number,
their_per_commitment_point: self.context.counterparty_cur_commitment_point.unwrap(),
feerate_per_kw: Some(counterparty_commitment_tx.feerate_per_kw()),
to_broadcaster_value_sat: Some(counterparty_commitment_tx.to_broadcaster_value_sat()),
to_countersignatory_value_sat: Some(counterparty_commitment_tx.to_countersignatory_value_sat()),
});
} else {
updates.push(ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX {
htlc_outputs,
commitment_tx: counterparty_commitment_tx,
});
}
// Soon, we will switch this to `LatestCounterpartyCommitmentTXs`,
// and provide the full commit tx instead of the information needed to rebuild it.
updates.push(ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
commitment_txid: counterparty_commitment_tx.trust().txid(),
htlc_outputs,
commitment_number: self.context.cur_counterparty_commitment_transaction_number,
their_per_commitment_point: self.context.counterparty_cur_commitment_point.unwrap(),
feerate_per_kw: Some(counterparty_commitment_tx.feerate_per_kw()),
to_broadcaster_value_sat: Some(counterparty_commitment_tx.to_broadcaster_value_sat()),
to_countersignatory_value_sat: Some(counterparty_commitment_tx.to_countersignatory_value_sat()),
});
} else {
// TODO(splicing): Build the HTLC-source table once per commitment number, and then
// build all the commitment transactions for that commitment number against that same
// table. Use `LatestCounterpartyCommitmentTXs` and `LatestHolderCommitmentTXs` to
// communicate these to `ChannelMonitor`.
todo!();
}

if self.context.announcement_sigs_state == AnnouncementSigsState::MessageSent {
Expand Down
Loading