Skip to content

Introduce RenegotiatedFunding monitor update variant #3822

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+373 −6
Open

Introduce RenegotiatedFunding monitor update variant #3822

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
18 changes: 18 additions & 0 deletions lightning/src/chain/chainmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1180,6 +1180,24 @@ where
panic!("{}", err_str);
},
}

// We may need to start monitoring for any alternative funding transactions.
if let Some(ref chain_source) = self.chain_source {
for (funding_outpoint, funding_script) in update.renegotiated_funding_data() {
log_trace!(
logger,
"Registering renegotiated funding outpoint {} with the filter to monitor confirmations and spends",
funding_outpoint
);
chain_source.register_tx(&funding_outpoint.txid, &funding_script);
chain_source.register_output(WatchedOutput {
block_hash: None,
outpoint: funding_outpoint,
script_pubkey: funding_script,
});
}
}

if update_res.is_err() {
ChannelMonitorUpdateStatus::InProgress
} else {
Expand Down
256 changes: 251 additions & 5 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,37 @@ pub struct ChannelMonitorUpdate {
pub channel_id: Option<ChannelId>,
}

impl ChannelMonitorUpdate {
fn internal_renegotiated_funding_data(
&self,
) -> impl Iterator<Item = (OutPoint, ScriptBuf)> + '_ {
self.updates.iter().filter_map(|update| match update {
ChannelMonitorUpdateStep::RenegotiatedFunding { channel_parameters, .. } => {
let funding_outpoint = channel_parameters
.funding_outpoint
.expect("Renegotiated funding must always have known outpoint");
let funding_script = channel_parameters.make_funding_redeemscript().to_p2wsh();
Some((funding_outpoint, funding_script))
},
_ => None,
})
}

/// Returns an iterator of new (funding outpoint, funding script) to monitor the chain for as
/// a result of a renegotiated funding transaction.
#[cfg(c_bindings)]
pub fn renegotiated_funding_data(&self) -> Vec<(OutPoint, ScriptBuf)> {
self.internal_renegotiated_funding_data().collect()
}

/// Returns an iterator of new (funding outpoint, funding script) to monitor the chain for as
/// a result of a renegotiated funding transaction.
#[cfg(not(c_bindings))]
pub fn renegotiated_funding_data(&self) -> impl Iterator<Item = (OutPoint, ScriptBuf)> + '_ {
self.internal_renegotiated_funding_data()
}
}

/// LDK prior to 0.1 used this constant as the [`ChannelMonitorUpdate::update_id`] for any
/// [`ChannelMonitorUpdate`]s which were generated after the channel was closed.
const LEGACY_CLOSED_CHANNEL_UPDATE_ID: u64 = u64::MAX;
Expand Down Expand Up @@ -640,6 +671,11 @@ pub(crate) enum ChannelMonitorUpdateStep {
ShutdownScript {
scriptpubkey: ScriptBuf,
},
RenegotiatedFunding {
channel_parameters: ChannelTransactionParameters,
holder_commitment_tx: HolderCommitmentTransaction,
counterparty_commitment_tx: CommitmentTransaction,
},
}

impl ChannelMonitorUpdateStep {
Expand All @@ -653,6 +689,7 @@ impl ChannelMonitorUpdateStep {
ChannelMonitorUpdateStep::CommitmentSecret { .. } => "CommitmentSecret",
ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed",
ChannelMonitorUpdateStep::ShutdownScript { .. } => "ShutdownScript",
ChannelMonitorUpdateStep::RenegotiatedFunding { .. } => "RenegotiatedFunding",
}
}
}
Expand Down Expand Up @@ -691,6 +728,11 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
(0, htlc_outputs, required_vec),
(2, commitment_tx, required),
},
(10, RenegotiatedFunding) => {
(1, channel_parameters, (required: ReadableArgs, None)),
(3, holder_commitment_tx, required),
(5, counterparty_commitment_tx, required),
},
);

/// Indicates whether the balance is derived from a cooperative close, a force-close
Expand Down Expand Up @@ -1024,9 +1066,69 @@ struct FundingScope {
prev_holder_commitment_tx: Option<HolderCommitmentTransaction>,
}

impl FundingScope {
fn funding_outpoint(&self) -> OutPoint {
let funding_outpoint = self.channel_parameters.funding_outpoint.as_ref();
*funding_outpoint.expect("Funding outpoint must be set for active monitor")
}

fn funding_txid(&self) -> Txid {
self.funding_outpoint().txid
}
}

impl Writeable for FundingScope {
fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
write_tlv_fields!(w, {
(1, self.channel_parameters, (required: ReadableArgs, None)),
(3, self.current_counterparty_commitment_txid, required),
(5, self.prev_counterparty_commitment_txid, option),
(7, self.current_holder_commitment_tx, required),
(9, self.prev_holder_commitment_tx, option),
(11, self.counterparty_claimable_outpoints, required),
});
Ok(())
}
}

impl Readable for FundingScope {
fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
let mut channel_parameters = RequiredWrapper(None);
let mut current_counterparty_commitment_txid = RequiredWrapper(None);
let mut prev_counterparty_commitment_txid = None;
let mut current_holder_commitment_tx = RequiredWrapper(None);
let mut prev_holder_commitment_tx = None;
let mut counterparty_claimable_outpoints = RequiredWrapper(None);

read_tlv_fields!(r, {
(1, channel_parameters, (required: ReadableArgs, None)),
(3, current_counterparty_commitment_txid, required),
(5, prev_counterparty_commitment_txid, option),
(7, current_holder_commitment_tx, required),
(9, prev_holder_commitment_tx, option),
(11, counterparty_claimable_outpoints, required),
});

let channel_parameters: ChannelTransactionParameters = channel_parameters.0.unwrap();
let redeem_script = channel_parameters.make_funding_redeemscript();

Ok(Self {
script_pubkey: redeem_script.to_p2wsh(),
redeem_script,
channel_parameters,
current_counterparty_commitment_txid: current_counterparty_commitment_txid.0.unwrap(),
prev_counterparty_commitment_txid,
current_holder_commitment_tx: current_holder_commitment_tx.0.unwrap(),
prev_holder_commitment_tx,
counterparty_claimable_outpoints: counterparty_claimable_outpoints.0.unwrap(),
})
}
}

#[derive(Clone, PartialEq)]
pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
funding: FundingScope,
pending_funding: Vec<FundingScope>,

latest_update_id: u64,
commitment_transaction_number_obscure_factor: u64,
Expand Down Expand Up @@ -1467,6 +1569,7 @@ impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
(27, self.first_confirmed_funding_txo, required),
(29, self.initial_counterparty_commitment_tx, option),
(31, self.funding.channel_parameters, required),
(32, self.pending_funding, optional_vec),
});

Ok(())
Expand Down Expand Up @@ -1636,6 +1739,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
current_holder_commitment_tx: initial_holder_commitment_tx,
prev_holder_commitment_tx: None,
},
pending_funding: vec![],

latest_update_id: 0,
commitment_transaction_number_obscure_factor,
Expand Down Expand Up @@ -1862,14 +1966,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
{
let lock = self.inner.lock().unwrap();
let logger = WithChannelMonitor::from_impl(logger, &*lock, None);
log_trace!(&logger, "Registering funding outpoint {}", &lock.get_funding_txo());
let funding_outpoint = lock.get_funding_txo();
filter.register_tx(&funding_outpoint.txid, &lock.funding.script_pubkey);
for funding in core::iter::once(&lock.funding).chain(&lock.pending_funding) {
let funding_outpoint = funding.funding_outpoint();
log_trace!(&logger, "Registering funding outpoint {} with the filter to monitor confirmations", &funding_outpoint);
filter.register_tx(&funding_outpoint.txid, &funding.script_pubkey);
}
for (txid, outputs) in lock.get_outputs_to_watch().iter() {
for (index, script_pubkey) in outputs.iter() {
assert!(*index <= u16::MAX as u32);
let outpoint = OutPoint { txid: *txid, index: *index as u16 };
log_trace!(logger, "Registering outpoint {} with the filter for monitoring spends", outpoint);
log_trace!(logger, "Registering outpoint {} with the filter to monitor spend", outpoint);
filter.register_output(WatchedOutput {
block_hash: None,
outpoint,
Expand Down Expand Up @@ -3453,6 +3559,128 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
);
}

fn renegotiated_funding<L: Deref>(
&mut self, logger: &WithChannelMonitor<L>,
channel_parameters: &ChannelTransactionParameters,
alternative_holder_commitment_tx: &HolderCommitmentTransaction,
alternative_counterparty_commitment_tx: &CommitmentTransaction,
) -> Result<(), ()>
where
L::Target: Logger,
{
let redeem_script = channel_parameters.make_funding_redeemscript();
let script_pubkey = redeem_script.to_p2wsh();
let alternative_counterparty_commitment_txid =
alternative_counterparty_commitment_tx.trust().txid();

// Both the current counterparty commitment and the alternative one share the same set of
// non-dust and dust HTLCs in the same order, though the index of each non-dust HTLC may be
// different.
//
// We clone all HTLCs and their sources to use in the alternative funding scope, and update
// each non-dust HTLC with their corresponding index in the alternative counterparty
// commitment.
let current_counterparty_commitment_htlcs =
if let Some(txid) = &self.funding.current_counterparty_commitment_txid {
self.funding.counterparty_claimable_outpoints.get(txid).unwrap()
} else {
debug_assert!(false);
log_error!(
logger,
"Received funding renegotiation while initial funding negotiation is still pending"
);
return Err(());
};
let mut htlcs_with_sources = current_counterparty_commitment_htlcs.clone();
let alternative_htlcs = alternative_counterparty_commitment_tx.nondust_htlcs();

let expected_non_dust_htlc_count = htlcs_with_sources
.iter()
// Non-dust HTLCs always come first, so the position of the first dust HTLC is equal to
// our non-dust HTLC count.
.position(|(htlc, _)| htlc.transaction_output_index.is_none())
.unwrap_or(htlcs_with_sources.len());
if alternative_htlcs.len() != expected_non_dust_htlc_count {
log_error!(
logger,
"Received alternative counterparty commitment with HTLC count mismatch"
);
return Err(());
}

for (alternative_htlc, (htlc, _)) in
alternative_htlcs.iter().zip(htlcs_with_sources.iter_mut())
{
debug_assert!(htlc.transaction_output_index.is_some());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same for these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The transaction_output_index.is_some() checks are more of an internal assertion, since there's a guarantee that CommitmentTransaction::nondust_htlcs returns HTLCs with their indices populated. The is_data_equal check should definitely return an error though.

debug_assert!(alternative_htlc.transaction_output_index.is_some());
if !alternative_htlc.is_data_equal(htlc) {
log_error!(
logger,
"Received alternative counterparty commitment with non-dust HTLC mismatch"
);
return Err(());
}
htlc.transaction_output_index = alternative_htlc.transaction_output_index;
}
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 further check that all alternative htlcs that were not considered have no output index?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The alternative HTLCs (coming from the alternative counterparty commitment transaction) are non-dust only. The other HTLC data is coming from counterparty_claimable_outpoints which has already gone through validation.


let mut counterparty_claimable_outpoints = new_hash_map();
counterparty_claimable_outpoints
.insert(alternative_counterparty_commitment_txid, htlcs_with_sources);

// TODO(splicing): Enforce any necessary RBF validity checks.
let alternative_funding = FundingScope {
script_pubkey: script_pubkey.clone(),
redeem_script,
channel_parameters: channel_parameters.clone(),
current_counterparty_commitment_txid: Some(alternative_counterparty_commitment_txid),
prev_counterparty_commitment_txid: None,
counterparty_claimable_outpoints,
current_holder_commitment_tx: alternative_holder_commitment_tx.clone(),
prev_holder_commitment_tx: None,
};
let alternative_funding_outpoint = alternative_funding.funding_outpoint();

if self
.pending_funding
.iter()
.any(|funding| funding.funding_txid() == alternative_funding_outpoint.txid)
{
log_error!(
logger,
"Renegotiated funding transaction with a duplicate funding txid {}",
alternative_funding_outpoint.txid
);
return Err(());
}

if let Some(parent_funding_txid) = channel_parameters.splice_parent_funding_txid.as_ref() {
// Only one splice can be negotiated at a time after we've exchanged `channel_ready`
// (implying our funding is confirmed) that spends our currently locked funding.
if !self.pending_funding.is_empty() {
log_error!(
logger,
"Negotiated splice while channel is pending channel_ready/splice_locked"
);
return Err(());
}
if *parent_funding_txid != self.funding.funding_txid() {
log_error!(
logger,
"Negotiated splice that does not spend currently locked funding transaction"
);
return Err(());
}
}

self.outputs_to_watch.insert(
alternative_funding_outpoint.txid,
vec![(alternative_funding_outpoint.index as u32, script_pubkey)],
);
self.pending_funding.push(alternative_funding);

Ok(())
}

#[rustfmt::skip]
fn update_monitor<B: Deref, F: Deref, L: Deref>(
&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &WithChannelMonitor<L>
Expand Down Expand Up @@ -3532,6 +3760,17 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
ret = Err(());
}
},
ChannelMonitorUpdateStep::RenegotiatedFunding {
channel_parameters, holder_commitment_tx, counterparty_commitment_tx,
} => {
log_trace!(logger, "Updating ChannelMonitor with alternative holder and counterparty commitment transactions for funding txid {}",
channel_parameters.funding_outpoint.unwrap().txid);
if let Err(_) = self.renegotiated_funding(
logger, channel_parameters, holder_commitment_tx, counterparty_commitment_tx,
) {
ret = Err(());
}
},
ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => {
log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast);
self.lockdown_from_offchain = true;
Expand Down Expand Up @@ -3583,7 +3822,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
|ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. }
|ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. }
|ChannelMonitorUpdateStep::ShutdownScript { .. }
|ChannelMonitorUpdateStep::CommitmentSecret { .. } =>
|ChannelMonitorUpdateStep::CommitmentSecret { .. }
|ChannelMonitorUpdateStep::RenegotiatedFunding { .. } =>
is_pre_close_update = true,
// After a channel is closed, we don't communicate with our peer about it, so the
// only things we will update is getting a new preimage (from a different channel)
Expand Down Expand Up @@ -3765,6 +4005,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
} => {
Some(commitment_tx.clone())
},
&ChannelMonitorUpdateStep::RenegotiatedFunding { ref counterparty_commitment_tx, .. } => {
Some(counterparty_commitment_tx.clone())
},
_ => None,
}
}).collect()
Expand Down Expand Up @@ -5438,6 +5681,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
let mut payment_preimages_with_info: Option<HashMap<_, _>> = None;
let mut first_confirmed_funding_txo = RequiredWrapper(None);
let mut channel_parameters = None;
let mut pending_funding = None;
read_tlv_fields!(reader, {
(1, funding_spend_confirmed, option),
(3, htlcs_resolved_on_chain, optional_vec),
Expand All @@ -5455,6 +5699,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
(27, first_confirmed_funding_txo, (default_value, outpoint)),
(29, initial_counterparty_commitment_tx, option),
(31, channel_parameters, (option: ReadableArgs, None)),
(32, pending_funding, optional_vec),
});
if let Some(payment_preimages_with_info) = payment_preimages_with_info {
if payment_preimages_with_info.len() != payment_preimages.len() {
Expand Down Expand Up @@ -5570,6 +5815,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
current_holder_commitment_tx,
prev_holder_commitment_tx,
},
pending_funding: pending_funding.unwrap_or(vec![]),

latest_update_id,
commitment_transaction_number_obscure_factor,
Expand Down
Loading
Loading