Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
95 changes: 76 additions & 19 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use crate::chain::package::{
use crate::chain::transaction::{OutPoint, TransactionData};
use crate::chain::{BlockLocator, WatchedOutput};
use crate::events::bump_transaction::{AnchorDescriptor, BumpTransactionEvent};
use crate::events::{ClosureReason, Event, EventHandler, ReplayEvent};
use crate::events::{ClosureReason, Event, EventHandler, FundingInfo, ReplayEvent};
use crate::ln::chan_utils::{
self, ChannelTransactionParameters, CommitmentTransaction, CounterpartyCommitmentSecrets,
HTLCClaim, HTLCOutputInCommitment, HolderCommitmentTransaction,
Expand All @@ -55,6 +55,7 @@ use crate::ln::channel_keys::{
RevocationKey,
};
use crate::ln::channelmanager::{HTLCSource, PaymentClaimDetails, SentHTLCId};
use crate::ln::funding::FundingContribution;
use crate::ln::msgs::DecodeError;
use crate::ln::types::ChannelId;
use crate::sign::{
Expand Down Expand Up @@ -688,6 +689,7 @@ pub(crate) enum ChannelMonitorUpdateStep {
channel_parameters: ChannelTransactionParameters,
holder_commitment_tx: HolderCommitmentTransaction,
counterparty_commitment_tx: CommitmentTransaction,
funding_contribution: Option<FundingContribution>,
},
RenegotiatedFundingLocked {
funding_txid: Txid,
Expand Down Expand Up @@ -773,6 +775,7 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
(1, channel_parameters, (required: ReadableArgs, None)),
(3, holder_commitment_tx, required),
(5, counterparty_commitment_tx, required),
(7, funding_contribution, option),
},
(12, RenegotiatedFundingLocked) => {
(1, funding_txid, required),
Expand Down Expand Up @@ -1166,6 +1169,9 @@ struct FundingScope {
// transaction for which we have deleted claim information on some watchtowers.
current_holder_commitment_tx: HolderCommitmentTransaction,
prev_holder_commitment_tx: Option<HolderCommitmentTransaction>,

/// Our funding contribution when negotiating the corresponding funding transaction.
contribution: Option<FundingContribution>,
}

impl FundingScope {
Expand All @@ -1185,6 +1191,14 @@ impl FundingScope {
fn channel_type_features(&self) -> &ChannelTypeFeatures {
&self.channel_parameters.channel_type_features
}

fn contributed_inputs(&self) -> impl Iterator<Item = bitcoin::OutPoint> + '_ {
self.contribution.iter().flat_map(|contribution| contribution.contributed_inputs())
}

fn contributed_outputs(&self) -> impl Iterator<Item = &bitcoin::Script> + '_ {
self.contribution.iter().flat_map(|contribution| contribution.contributed_outputs())
}
}

impl_writeable_tlv_based!(FundingScope, {
Expand All @@ -1194,6 +1208,7 @@ impl_writeable_tlv_based!(FundingScope, {
(7, current_holder_commitment_tx, required),
(9, prev_holder_commitment_tx, option),
(11, counterparty_claimable_outpoints, required),
(13, contribution, option),
});

#[derive(Clone, PartialEq)]
Expand Down Expand Up @@ -1756,6 +1771,7 @@ pub(crate) fn write_chanmon_internal<Signer: EcdsaChannelSigner, W: Writer>(
(35, channel_monitor.is_manual_broadcast, required),
(37, channel_monitor.funding_seen_onchain, required),
(39, channel_monitor.best_block.previous_blocks, required),
(41, channel_monitor.funding.contribution, option),
});

Ok(())
Expand Down Expand Up @@ -1905,6 +1921,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {

current_holder_commitment_tx: initial_holder_commitment_tx,
prev_holder_commitment_tx: None,

contribution: None,
},
pending_funding: vec![],

Expand Down Expand Up @@ -3959,6 +3977,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
&mut self, logger: &WithContext<L>, channel_parameters: &ChannelTransactionParameters,
alternative_holder_commitment_tx: &HolderCommitmentTransaction,
alternative_counterparty_commitment_tx: &CommitmentTransaction,
funding_contribution: &Option<FundingContribution>,
) -> Result<(), ()> {
let alternative_counterparty_commitment_txid =
alternative_counterparty_commitment_tx.trust().txid();
Expand Down Expand Up @@ -4025,6 +4044,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
counterparty_claimable_outpoints,
current_holder_commitment_tx: alternative_holder_commitment_tx.clone(),
prev_holder_commitment_tx: None,
contribution: funding_contribution.clone(),
};
let alternative_funding_outpoint = alternative_funding.funding_outpoint();

Expand Down Expand Up @@ -4081,6 +4101,37 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
Ok(())
}

fn queue_discard_funding_event(
&mut self, discarded_funding: impl Iterator<Item = FundingScope>,
) {
let (mut discarded_inputs, mut discarded_outputs) = (new_hash_set(), new_hash_set());
for funding in discarded_funding {
if funding.contribution.is_none() {
self.pending_events.push(Event::DiscardFunding {
channel_id: self.channel_id,
funding_info: FundingInfo::OutPoint { outpoint: funding.funding_outpoint() },
});
} else if let Some(contribution) = funding.contribution {
if let Some((inputs, outputs)) = contribution.into_unique_contributions(
self.funding.contributed_inputs(),
self.funding.contributed_outputs(),
) {
discarded_inputs.extend(inputs);
discarded_outputs.extend(outputs);
}
}
}
if !discarded_inputs.is_empty() || !discarded_outputs.is_empty() {
self.pending_events.push(Event::DiscardFunding {
channel_id: self.channel_id,
funding_info: FundingInfo::Contribution {
inputs: discarded_inputs.into_iter().collect(),
outputs: discarded_outputs.into_iter().collect(),
Comment thread
wpaulino marked this conversation as resolved.
},
});
Comment on lines +4125 to +4131
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm... in channelmanager.rs, we make a separate DiscardFunding event for each attempt. Should we do the same here? It would remove the non-determinism introduced by the hash set.

}
}

fn promote_funding(&mut self, new_funding_txid: Txid) -> Result<(), ()> {
let prev_funding_txid = self.funding.funding_txid();

Expand Down Expand Up @@ -4111,18 +4162,20 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
let no_further_updates_allowed = self.no_further_updates_allowed();

// The swap above places the previous `FundingScope` into `pending_funding`.
for funding in self.pending_funding.drain(..) {
let funding_txid = funding.funding_txid();
self.outputs_to_watch.remove(&funding_txid);
if no_further_updates_allowed && funding_txid != prev_funding_txid {
self.pending_events.push(Event::DiscardFunding {
channel_id: self.channel_id,
funding_info: crate::events::FundingInfo::OutPoint {
outpoint: funding.funding_outpoint(),
},
});
}
for funding in &self.pending_funding {
self.outputs_to_watch.remove(&funding.funding_txid());
}
let mut discarded_funding = Vec::new();
mem::swap(&mut self.pending_funding, &mut discarded_funding);
let discarded_funding = discarded_funding
.into_iter()
// The previous funding is filtered out since it was already locked, so nothing needs to
// be discarded.
.filter(|funding| {
no_further_updates_allowed && funding.funding_txid() != prev_funding_txid
});
self.queue_discard_funding_event(discarded_funding);

if let Some((alternative_funding_txid, _)) = self.alternative_funding_confirmed.take() {
// In exceedingly rare cases, it's possible there was a reorg that caused a potential funding to
// be locked in that this `ChannelMonitor` has not yet seen. Thus, we avoid a runtime assertion
Expand Down Expand Up @@ -4239,11 +4292,13 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
},
ChannelMonitorUpdateStep::RenegotiatedFunding {
channel_parameters, holder_commitment_tx, counterparty_commitment_tx,
funding_contribution,
} => {
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,
funding_contribution,
) {
ret = Err(());
}
Expand Down Expand Up @@ -5810,15 +5865,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
self.funding_spend_confirmed = Some(entry.txid);
self.confirmed_commitment_tx_counterparty_output = commitment_tx_to_counterparty_output;
if self.alternative_funding_confirmed.is_none() {
for funding in self.pending_funding.drain(..) {
// We saw a confirmed commitment for our currently locked funding, so
// discard all pending ones.
for funding in &self.pending_funding {
self.outputs_to_watch.remove(&funding.funding_txid());
self.pending_events.push(Event::DiscardFunding {
channel_id: self.channel_id,
funding_info: crate::events::FundingInfo::OutPoint {
outpoint: funding.funding_outpoint(),
},
});
}
let mut discarded_funding = Vec::new();
mem::swap(&mut self.pending_funding, &mut discarded_funding);
self.queue_discard_funding_event(discarded_funding.into_iter());
}
},
OnchainEvent::AlternativeFundingConfirmation {} => {
Expand Down Expand Up @@ -6696,6 +6750,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
let mut is_manual_broadcast = RequiredWrapper(None);
let mut funding_seen_onchain = RequiredWrapper(None);
let mut best_block_previous_blocks = None;
let mut current_funding_contribution = None;
read_tlv_fields!(reader, {
(1, funding_spend_confirmed, option),
(3, htlcs_resolved_on_chain, optional_vec),
Expand All @@ -6719,6 +6774,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
(35, is_manual_broadcast, (default_value, false)),
(37, funding_seen_onchain, (default_value, true)),
(39, best_block_previous_blocks, option), // Added and always set in 0.3
(41, current_funding_contribution, option),
});
if let Some(previous_blocks) = best_block_previous_blocks {
best_block.previous_blocks = previous_blocks;
Expand Down Expand Up @@ -6837,6 +6893,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP

current_holder_commitment_tx,
prev_holder_commitment_tx,
contribution: current_funding_contribution,
},
pending_funding: pending_funding.unwrap_or(vec![]),
is_manual_broadcast: is_manual_broadcast.0.unwrap(),
Expand Down
6 changes: 3 additions & 3 deletions lightning/src/events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use bitcoin::hashes::sha256::Hash as Sha256;
use bitcoin::hashes::Hash;
use bitcoin::script::ScriptBuf;
use bitcoin::secp256k1::PublicKey;
use bitcoin::{OutPoint, Transaction, TxOut};
use bitcoin::{OutPoint, Transaction};
use core::ops::Deref;

#[allow(unused_imports)]
Expand Down Expand Up @@ -81,8 +81,8 @@ pub enum FundingInfo {
Contribution {
/// UTXOs spent as inputs contributed to the funding transaction.
inputs: Vec<OutPoint>,
/// Outputs contributed to the funding transaction.
outputs: Vec<TxOut>,
/// Output scripts contributed to the funding transaction.
outputs: Vec<ScriptBuf>,
},
}

Expand Down
43 changes: 27 additions & 16 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3115,7 +3115,7 @@ impl PendingFunding {
self.contributions.iter().flat_map(|c| c.contributed_inputs())
}

fn contributed_outputs(&self) -> impl Iterator<Item = &TxOut> + '_ {
fn contributed_outputs(&self) -> impl Iterator<Item = &bitcoin::Script> + '_ {
self.contributions.iter().flat_map(|c| c.contributed_outputs())
}

Expand All @@ -3124,7 +3124,7 @@ impl PendingFunding {
self.contributions[..len.saturating_sub(1)].iter().flat_map(|c| c.contributed_inputs())
}

fn prior_contributed_outputs(&self) -> impl Iterator<Item = &TxOut> + '_ {
fn prior_contributed_outputs(&self) -> impl Iterator<Item = &bitcoin::Script> + '_ {
let len = self.contributions.len();
self.contributions[..len.saturating_sub(1)].iter().flat_map(|c| c.contributed_outputs())
}
Expand Down Expand Up @@ -3173,7 +3173,7 @@ pub(crate) enum QuiescentAction {

pub(super) enum QuiescentError {
DoNothing,
DiscardFunding { inputs: Vec<bitcoin::OutPoint>, outputs: Vec<bitcoin::TxOut> },
DiscardFunding { inputs: Vec<bitcoin::OutPoint>, outputs: Vec<ScriptBuf> },
FailSplice(SpliceFundingFailed),
}

Expand Down Expand Up @@ -6850,23 +6850,27 @@ impl FundingNegotiationContext {
}
}

fn into_contributed_inputs_and_outputs(self) -> (Vec<bitcoin::OutPoint>, Vec<TxOut>) {
fn into_contributed_inputs_and_outputs(self) -> (Vec<bitcoin::OutPoint>, Vec<ScriptBuf>) {
let contributed_inputs =
self.our_funding_inputs.into_iter().map(|input| input.utxo.outpoint).collect();
let contributed_outputs = self.our_funding_outputs;
let contributed_outputs =
self.our_funding_outputs.into_iter().map(|output| output.script_pubkey).collect();
(contributed_inputs, contributed_outputs)
}

fn contributed_inputs(&self) -> impl Iterator<Item = bitcoin::OutPoint> + '_ {
self.our_funding_inputs.iter().map(|input| input.utxo.outpoint)
}

fn contributed_outputs(&self) -> impl Iterator<Item = &TxOut> + '_ {
self.our_funding_outputs.iter()
fn contributed_outputs(&self) -> impl Iterator<Item = &bitcoin::Script> + '_ {
self.our_funding_outputs.iter().map(|output| output.script_pubkey.as_script())
}

fn to_contributed_inputs_and_outputs(&self) -> (Vec<bitcoin::OutPoint>, Vec<TxOut>) {
(self.contributed_inputs().collect(), self.contributed_outputs().cloned().collect())
fn to_contributed_inputs_and_outputs(&self) -> (Vec<bitcoin::OutPoint>, Vec<ScriptBuf>) {
(
self.contributed_inputs().collect(),
self.contributed_outputs().map(|script| script.into()).collect(),
)
}
}

Expand Down Expand Up @@ -7027,8 +7031,8 @@ pub struct SpliceFundingFailed {
/// UTXOs spent as inputs contributed to the splice transaction.
pub contributed_inputs: Vec<bitcoin::OutPoint>,

/// Outputs contributed to the splice transaction.
pub contributed_outputs: Vec<bitcoin::TxOut>,
/// Output scripts contributed to the splice transaction.
pub contributed_outputs: Vec<ScriptBuf>,
}

macro_rules! maybe_create_splice_funding_failed {
Expand Down Expand Up @@ -7068,7 +7072,7 @@ macro_rules! maybe_create_splice_funding_failed {
contributed_inputs.retain(|i| *i != input);
}
for output in pending_splice.prior_contributed_outputs() {
contributed_outputs.retain(|o| o.script_pubkey != output.script_pubkey);
contributed_outputs.retain(|i| i != output);
}
}

Expand Down Expand Up @@ -7119,7 +7123,7 @@ where
inputs.retain(|i| *i != input);
}
for output in pending_splice.contributed_outputs() {
outputs.retain(|o| o.script_pubkey != output.script_pubkey);
outputs.retain(|o| o.as_script() != output);
}
}
SpliceFundingFailed {
Expand Down Expand Up @@ -8314,12 +8318,12 @@ where
&mut self, msg: &msgs::CommitmentSigned, fee_estimator: &LowerBoundedFeeEstimator<F>,
logger: &L,
) -> Result<Option<ChannelMonitorUpdate>, ChannelError> {
debug_assert!(self
let signing_session = self
.context
.interactive_tx_signing_session
.as_ref()
.map(|signing_session| !signing_session.has_received_tx_signatures())
.unwrap_or(false));
.expect("Signing session must exist for negotiated pending splice");
debug_assert!(!signing_session.has_received_tx_signatures());
Comment on lines +8321 to +8326
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: This changes from a debug_assert (no-op in release builds) to expect (panics in release builds). The old code combined the existence check and state check into a single debug assertion; now existence is enforced unconditionally while the state check remains debug-only.

This is arguably safer (fail-fast instead of silent invariant violation), but since signing_session isn't used beyond the debug_assert on the next line, consider whether returning a ChannelError would be more robust than panicking — consistent with how this function handles other missing state (e.g., the commitment_point error path at line 8340).


let pending_splice_funding = self
.pending_splice
Expand Down Expand Up @@ -8371,6 +8375,12 @@ where
);
}

let funding_contribution = self
.pending_splice
.as_ref()
.and_then(|pending_splice| pending_splice.contributions.last())
.cloned();

log_info!(
logger,
"Received splice initial commitment_signed from peer with funding txid {}",
Expand All @@ -8384,6 +8394,7 @@ where
channel_parameters: pending_splice_funding.channel_transaction_parameters.clone(),
holder_commitment_tx,
counterparty_commitment_tx,
funding_contribution,
}],
channel_id: Some(self.context.channel_id()),
};
Expand Down
Loading
Loading