Skip to content
Merged
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ check-cfg = [
"cfg(ldk_test_vectors)",
"cfg(taproot)",
"cfg(require_route_graph_test)",
"cfg(splicing)",
"cfg(simple_close)",
"cfg(peer_storage)",
]
2 changes: 0 additions & 2 deletions ci/ci-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,6 @@ fi
echo -e "\n\nTest cfg-flag builds"
RUSTFLAGS="--cfg=taproot" cargo test --verbose --color always -p lightning
[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
RUSTFLAGS="--cfg=splicing" cargo test --verbose --color always -p lightning
[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
RUSTFLAGS="--cfg=async_payments" cargo test --verbose --color always -p lightning
[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
RUSTFLAGS="--cfg=simple_close" cargo test --verbose --color always -p lightning
Expand Down
3 changes: 0 additions & 3 deletions lightning-net-tokio/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,11 +742,8 @@ mod tests {
fn handle_open_channel_v2(&self, _their_node_id: PublicKey, _msg: &OpenChannelV2) {}
fn handle_accept_channel_v2(&self, _their_node_id: PublicKey, _msg: &AcceptChannelV2) {}
fn handle_stfu(&self, _their_node_id: PublicKey, _msg: &Stfu) {}
#[cfg(splicing)]
fn handle_splice_init(&self, _their_node_id: PublicKey, _msg: &SpliceInit) {}
#[cfg(splicing)]
fn handle_splice_ack(&self, _their_node_id: PublicKey, _msg: &SpliceAck) {}
#[cfg(splicing)]
fn handle_splice_locked(&self, _their_node_id: PublicKey, _msg: &SpliceLocked) {}
fn handle_tx_add_input(&self, _their_node_id: PublicKey, _msg: &TxAddInput) {}
fn handle_tx_add_output(&self, _their_node_id: PublicKey, _msg: &TxAddOutput) {}
Expand Down
46 changes: 30 additions & 16 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3399,26 +3399,35 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {

// Prune HTLCs from the previous counterparty commitment tx so we don't generate failure/fulfill
// events for now-revoked/fulfilled HTLCs.
if let Some(txid) = self.funding.prev_counterparty_commitment_txid.take() {
if self.funding.current_counterparty_commitment_txid.unwrap() != txid {
let cur_claimables = self.funding.counterparty_claimable_outpoints.get(
&self.funding.current_counterparty_commitment_txid.unwrap()).unwrap();
for (_, ref source_opt) in self.funding.counterparty_claimable_outpoints.get(&txid).unwrap() {
if let Some(source) = source_opt {
if !cur_claimables.iter()
.any(|(_, cur_source_opt)| cur_source_opt == source_opt)
{
self.counterparty_fulfilled_htlcs.remove(&SentHTLCId::from_source(source));
let mut removed_fulfilled_htlcs = false;
let prune_htlc_sources = |funding: &mut FundingScope| {
if let Some(txid) = funding.prev_counterparty_commitment_txid.take() {
if funding.current_counterparty_commitment_txid.unwrap() != txid {
let cur_claimables = funding.counterparty_claimable_outpoints.get(
&funding.current_counterparty_commitment_txid.unwrap()).unwrap();
// We only need to remove fulfilled HTLCs once for the first `FundingScope` we
// come across since all `FundingScope`s share the same set of HTLC sources.
if !removed_fulfilled_htlcs {
for (_, ref source_opt) in funding.counterparty_claimable_outpoints.get(&txid).unwrap() {
if let Some(source) = source_opt {
if !cur_claimables.iter()
.any(|(_, cur_source_opt)| cur_source_opt == source_opt)
{
self.counterparty_fulfilled_htlcs.remove(&SentHTLCId::from_source(source));
}
}
}
removed_fulfilled_htlcs = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just run the loop below at this point and return rather than needing this variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just a small optimization to not remove HTLCs that have already been removed since we have duplicate data across FundingScopes. The loop below does need to run for every FundingScope though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, but that is already inside the closure, which is called on each FundingScope.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, nevermind. Didn't see that removed_fulfilled_htlcs is used across all calls.

}
for &mut (_, ref mut source_opt) in funding.counterparty_claimable_outpoints.get_mut(&txid).unwrap() {
*source_opt = None;
}
} else {
assert!(cfg!(fuzzing), "Commitment txids are unique outside of fuzzing, where hashes can collide");
}
for &mut (_, ref mut source_opt) in self.funding.counterparty_claimable_outpoints.get_mut(&txid).unwrap() {
*source_opt = None;
}
} else {
assert!(cfg!(fuzzing), "Commitment txids are unique outside of fuzzing, where hashes can collide");
}
}
};
core::iter::once(&mut self.funding).chain(&mut self.pending_funding).for_each(prune_htlc_sources);

if !self.payment_preimages.is_empty() {
let min_idx = self.get_min_seen_secret();
Expand Down Expand Up @@ -4049,6 +4058,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {

mem::swap(&mut self.funding, &mut new_funding);
self.onchain_tx_handler.update_after_renegotiated_funding_locked(
self.funding.channel_parameters.clone(),
self.funding.current_holder_commitment_tx.clone(),
self.funding.prev_holder_commitment_tx.clone(),
);
Expand Down Expand Up @@ -5164,6 +5174,10 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
let txid = tx.compute_txid();
log_trace!(logger, "Transaction {} confirmed in block {}", txid , block_hash);
// If a transaction has already been confirmed, ensure we don't bother processing it duplicatively.
if self.alternative_funding_confirmed.map(|(alternative_funding_txid, _)| alternative_funding_txid == txid).unwrap_or(false) {
log_debug!(logger, "Skipping redundant processing of funding-spend tx {} as it was previously confirmed", txid);
continue 'tx_iter;
}
if Some(txid) == self.funding_spend_confirmed {
log_debug!(logger, "Skipping redundant processing of funding-spend tx {} as it was previously confirmed", txid);
continue 'tx_iter;
Expand Down
9 changes: 6 additions & 3 deletions lightning/src/chain/onchaintx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1236,11 +1236,14 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
self.prev_holder_commitment = Some(replace(&mut self.holder_commitment, tx));
}

/// Replaces the current/prev holder commitment transactions spending the currently confirmed
/// funding outpoint with those spending the new funding outpoint.
/// Replaces all the data pertaining to the currently locked funding transaction after a new
/// funding transaction has been renegotiated and locked.
pub(crate) fn update_after_renegotiated_funding_locked(
&mut self, current: HolderCommitmentTransaction, prev: Option<HolderCommitmentTransaction>,
&mut self, channel_parameters: ChannelTransactionParameters,
current: HolderCommitmentTransaction, prev: Option<HolderCommitmentTransaction>,
) {
self.channel_value_satoshis = channel_parameters.channel_value_satoshis;
self.channel_transaction_parameters = channel_parameters;
self.holder_commitment = current;
self.prev_holder_commitment = prev;
}
Expand Down
Loading
Loading