Skip to content

Commit 2d8e724

Browse files
committed
Remove unnecessary allocation in send_commitment_no_state_update
The logging for loop in `send_commitment_no_state_update` only iterates over the non-dust HTLCs. Thus we do not need to create a `Vec<HTLCOutputInCommitment>` that includes both dust and non-dust HTLCs, we can grab the `Vec<HTLCOutputInCommitment>` that holds only non-dust HTLCs directly from `CommitmentTransaction`.
1 parent 0635dbf commit 2d8e724

File tree

1 file changed

+11
-21
lines changed

1 file changed

+11
-21
lines changed

lightning/src/ln/channel.rs

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6838,7 +6838,7 @@ impl<SP: Deref> FundedChannel<SP> where
68386838
log_trace!(logger, "Regenerating latest commitment update in channel {} with{} {} update_adds, {} update_fulfills, {} update_fails, and {} update_fail_malformeds",
68396839
&self.context.channel_id(), if update_fee.is_some() { " update_fee," } else { "" },
68406840
update_add_htlcs.len(), update_fulfill_htlcs.len(), update_fail_htlcs.len(), update_fail_malformed_htlcs.len());
6841-
let commitment_signed = if let Ok(update) = self.send_commitment_no_state_update(logger).map(|(cu, _)| cu) {
6841+
let commitment_signed = if let Ok(update) = self.send_commitment_no_state_update(logger) {
68426842
if self.context.signer_pending_commitment_update {
68436843
log_trace!(logger, "Commitment update generated: clearing signer_pending_commitment_update");
68446844
self.context.signer_pending_commitment_update = false;
@@ -8750,7 +8750,7 @@ impl<SP: Deref> FundedChannel<SP> where
87508750

87518751
/// Only fails in case of signer rejection. Used for channel_reestablish commitment_signed
87528752
/// generation when we shouldn't change HTLC/channel state.
8753-
fn send_commitment_no_state_update<L: Deref>(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger {
8753+
fn send_commitment_no_state_update<L: Deref>(&self, logger: &L) -> Result<msgs::CommitmentSigned, ChannelError> where L::Target: Logger {
87548754
// Get the fee tests from `build_commitment_no_state_update`
87558755
#[cfg(any(test, fuzzing))]
87568756
self.build_commitment_no_state_update(logger);
@@ -8763,11 +8763,6 @@ impl<SP: Deref> FundedChannel<SP> where
87638763
let (signature, htlc_signatures);
87648764

87658765
{
8766-
let mut htlcs = Vec::with_capacity(commitment_stats.htlcs_included.len());
8767-
for &(ref htlc, _) in commitment_stats.htlcs_included.iter() {
8768-
htlcs.push(htlc);
8769-
}
8770-
87718766
let res = ecdsa.sign_counterparty_commitment(
87728767
&self.funding.channel_transaction_parameters,
87738768
&commitment_stats.tx,
@@ -8784,7 +8779,8 @@ impl<SP: Deref> FundedChannel<SP> where
87848779
log_bytes!(signature.serialize_compact()[..]), &self.context.channel_id());
87858780

87868781
let counterparty_keys = commitment_stats.tx.trust().keys();
8787-
for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) {
8782+
debug_assert_eq!(htlc_signatures.len(), commitment_stats.tx.htlcs().len());
8783+
for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(commitment_stats.tx.htlcs()) {
87888784
log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {} in channel {}",
87898785
encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, self.funding.get_holder_selected_contest_delay(), htlc, &self.context.channel_type, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)),
87908786
encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &self.context.channel_type, &counterparty_keys)),
@@ -8793,14 +8789,14 @@ impl<SP: Deref> FundedChannel<SP> where
87938789
}
87948790
}
87958791

8796-
Ok((msgs::CommitmentSigned {
8792+
Ok(msgs::CommitmentSigned {
87978793
channel_id: self.context.channel_id,
87988794
signature,
87998795
htlc_signatures,
88008796
batch: None,
88018797
#[cfg(taproot)]
88028798
partial_signature_with_nonce: None,
8803-
}, (counterparty_commitment_txid, commitment_stats.htlcs_included)))
8799+
})
88048800
},
88058801
// TODO (taproot|arik)
88068802
#[cfg(taproot)]
@@ -11873,14 +11869,8 @@ mod tests {
1187311869
( $counterparty_sig_hex: expr, $sig_hex: expr, $tx_hex: expr, $opt_anchors: expr, {
1187411870
$( { $htlc_idx: expr, $counterparty_htlc_sig_hex: expr, $htlc_sig_hex: expr, $htlc_tx_hex: expr } ), *
1187511871
} ) => { {
11876-
let (commitment_tx, htlcs): (_, Vec<HTLCOutputInCommitment>) = {
11877-
let mut commitment_stats = chan.context.build_commitment_transaction(&chan.funding, 0xffffffffffff - 42, &per_commitment_point, true, false, &logger);
11878-
11879-
let htlcs = commitment_stats.htlcs_included.drain(..)
11880-
.filter_map(|(htlc, _)| if htlc.transaction_output_index.is_some() { Some(htlc) } else { None })
11881-
.collect();
11882-
(commitment_stats.tx, htlcs)
11883-
};
11872+
let commitment_stats = chan.context.build_commitment_transaction(&chan.funding, 0xffffffffffff - 42, &per_commitment_point, true, false, &logger);
11873+
let commitment_tx = commitment_stats.tx;
1188411874
let trusted_tx = commitment_tx.trust();
1188511875
let unsigned_tx = trusted_tx.built_transaction();
1188611876
let redeemscript = chan.funding.get_funding_redeemscript();
@@ -11895,10 +11885,10 @@ mod tests {
1189511885
counterparty_htlc_sigs.clear(); // Don't warn about excess mut for no-HTLC calls
1189611886
$({
1189711887
let remote_signature = Signature::from_der(&<Vec<u8>>::from_hex($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
11898-
per_htlc.push((htlcs[$htlc_idx].clone(), Some(remote_signature)));
11888+
per_htlc.push((commitment_tx.htlcs()[$htlc_idx].clone(), Some(remote_signature)));
1189911889
counterparty_htlc_sigs.push(remote_signature);
1190011890
})*
11901-
assert_eq!(htlcs.len(), per_htlc.len());
11891+
assert_eq!(commitment_tx.htlcs().len(), per_htlc.len());
1190211892

1190311893
let holder_commitment_tx = HolderCommitmentTransaction::new(
1190411894
commitment_tx.clone(),
@@ -11921,7 +11911,7 @@ mod tests {
1192111911
log_trace!(logger, "verifying htlc {}", $htlc_idx);
1192211912
let remote_signature = Signature::from_der(&<Vec<u8>>::from_hex($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
1192311913

11924-
let ref htlc = htlcs[$htlc_idx];
11914+
let ref htlc = commitment_tx.htlcs()[$htlc_idx];
1192511915
let keys = commitment_tx.trust().keys();
1192611916
let mut htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw,
1192711917
chan.funding.get_counterparty_selected_contest_delay().unwrap(),

0 commit comments

Comments
 (0)