Skip to content

Commit f021734

Browse files
tankyleotankyleo
committed
Delete another allocation in the build of commitment transactions
1 parent 6ba5280 commit f021734

File tree

1 file changed

+135
-85
lines changed

1 file changed

+135
-85
lines changed

‎lightning/src/ln/chan_utils.rs

+135-85
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,6 @@ use bitcoin::{secp256k1, Sequence, Witness};
4141

4242
use crate::io;
4343
use core::cmp;
44-
use core::iter::IntoIterator;
45-
use crate::util::transaction_utils::sort_outputs;
4644
use crate::ln::channel::{INITIAL_COMMITMENT_NUMBER, ANCHOR_OUTPUT_VALUE_SATOSHI};
4745
use core::ops::Deref;
4846
use crate::chain;
@@ -1561,8 +1559,28 @@ impl CommitmentTransaction {
15611559
fn rebuild_transaction(&self, keys: &TxCreationKeys, channel_parameters: &DirectedChannelTransactionParameters) -> BuiltCommitmentTransaction {
15621560
let (obscured_commitment_transaction_number, txins) = Self::build_inputs(self.commitment_number, channel_parameters);
15631561

1564-
let txout_htlc_pairs: Vec<(TxOut, Option<&HTLCOutputInCommitment>)> = Self::build_txout_htlc_pairs(keys, self.to_broadcaster_value_sat, self.to_countersignatory_value_sat, &self.nondust_htlcs, channel_parameters);
1565-
let outputs = txout_htlc_pairs.into_iter().map(|(output, _)| output).collect();
1562+
// First rebuild the htlc outputs
1563+
let mut outputs = Self::build_htlc_outputs(keys, &self.nondust_htlcs, channel_parameters.channel_type_features());
1564+
1565+
// Then insert the max-4 non-htlc outputs
1566+
let insert_non_htlc_output = |non_htlc_output: TxOut| {
1567+
let idx = match outputs.binary_search_by(|output| output.value.cmp(&non_htlc_output.value).then(output.script_pubkey.cmp(&non_htlc_output.script_pubkey))) {
1568+
// For non-HTLC outputs, if they're copying our SPK we don't really care if we
1569+
// close the channel due to mismatches - they're doing something dumb
1570+
Ok(i) => i,
1571+
Err(i) => i,
1572+
};
1573+
outputs.insert(idx, non_htlc_output);
1574+
};
1575+
1576+
Self::insert_non_htlc_outputs(
1577+
keys,
1578+
self.to_broadcaster_value_sat,
1579+
self.to_countersignatory_value_sat,
1580+
channel_parameters,
1581+
!self.nondust_htlcs.is_empty(),
1582+
insert_non_htlc_output
1583+
);
15661584

15671585
let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs);
15681586
let txid = transaction.compute_txid();
@@ -1582,62 +1600,77 @@ impl CommitmentTransaction {
15821600
}
15831601
}
15841602

1585-
// This function populates all the HTLCs in the passed vector with their output indices,
1586-
// and returns that vector along with the vector of outputs for the transaction.
1587-
// Both the outputs and the HTLCs vectors returned are sorted in the order they appear in the
1588-
// commitment transaction.
1589-
fn build_outputs_and_htlcs(keys: &TxCreationKeys, to_broadcaster_value_sat: Amount, to_countersignatory_value_sat: Amount, mut nondust_htlcs: Vec<HTLCOutputInCommitment>, channel_parameters: &DirectedChannelTransactionParameters) -> (Vec<TxOut>, Vec<HTLCOutputInCommitment>) {
1590-
let txout_htlc_pairs: Vec<(TxOut, Option<&mut HTLCOutputInCommitment>)> = Self::build_txout_htlc_pairs(keys, to_broadcaster_value_sat, to_countersignatory_value_sat, &mut nondust_htlcs, channel_parameters);
1591-
let mut outputs: Vec<TxOut> = Vec::with_capacity(txout_htlc_pairs.len());
1592-
for (idx, (output, htlc)) in txout_htlc_pairs.into_iter().enumerate() {
1593-
if let Some(htlc) = htlc {
1594-
htlc.transaction_output_index = Some(idx as u32);
1595-
}
1596-
outputs.push(output);
1597-
}
1598-
nondust_htlcs.sort_unstable_by_key(|nondust_htlc| nondust_htlc.transaction_output_index.unwrap());
1603+
fn build_outputs_and_htlcs(
1604+
keys: &TxCreationKeys,
1605+
to_broadcaster_value_sat: Amount,
1606+
to_countersignatory_value_sat: Amount,
1607+
nondust_htlcs: Vec<HTLCOutputInCommitment>,
1608+
channel_parameters: &DirectedChannelTransactionParameters
1609+
) -> (Vec<TxOut>, Vec<HTLCOutputInCommitment>) {
1610+
// First build and sort the htlc outputs
1611+
let (mut outputs, mut nondust_htlcs) = Self::build_sorted_htlc_outputs(keys, nondust_htlcs, channel_parameters.channel_type_features());
1612+
let tx_has_htlc_outputs = !outputs.is_empty();
1613+
1614+
// Then insert the max-4 non-htlc outputs
1615+
let insert_non_htlc_output = |non_htlc_output: TxOut| {
1616+
let idx = match outputs.binary_search_by(|output| output.value.cmp(&non_htlc_output.value).then(output.script_pubkey.cmp(&non_htlc_output.script_pubkey))) {
1617+
// For non-HTLC outputs, if they're copying our SPK we don't really care if we
1618+
// close the channel due to mismatches - they're doing something dumb
1619+
Ok(i) => i,
1620+
Err(i) => i,
1621+
};
1622+
outputs.insert(idx, non_htlc_output);
1623+
1624+
// Increment the transaction output indices of all the HTLCs that come after the output we
1625+
// just inserted.
1626+
nondust_htlcs
1627+
.iter_mut()
1628+
.rev()
1629+
.map_while(|htlc| {
1630+
let i = htlc.transaction_output_index.as_mut().unwrap();
1631+
(*i >= idx as u32).then(|| i)
1632+
})
1633+
.for_each(|i| *i += 1);
1634+
};
1635+
1636+
Self::insert_non_htlc_outputs(
1637+
keys,
1638+
to_broadcaster_value_sat,
1639+
to_countersignatory_value_sat,
1640+
channel_parameters,
1641+
tx_has_htlc_outputs,
1642+
insert_non_htlc_output
1643+
);
1644+
15991645
(outputs, nondust_htlcs)
16001646
}
16011647

1602-
// Builds the set of outputs for the commitment transaction. Each HTLC output is paired with its corresponding data.
1603-
// There will be an output for all passed HTLCs, since they are all non-dust.
1604-
// Then depending on amounts, and features, this function assigns holder, counterparty, and anchor outputs.
1605-
//
1606-
// The set of outputs are sorted according to BIP-69 ordering, guaranteeing that HTLCs are
1607-
// returned in increasing output index order.
1608-
//
1609-
// This is used in two cases:
1610-
// - initial sorting of outputs / HTLCs in the constructor, via `build_outputs_and_htlcs`
1611-
// - building of a bitcoin transaction during a verify() call
1612-
//
1613-
// Note that the HTLC data in the second column of the returned table is not populated with the
1614-
// output indices of the HTLCs; this is done in `build_outputs_and_htlcs`.
1615-
fn build_txout_htlc_pairs<I, T>(keys: &TxCreationKeys, to_broadcaster_value_sat: Amount, to_countersignatory_value_sat: Amount, nondust_htlcs: T, channel_parameters: &DirectedChannelTransactionParameters) -> Vec<(TxOut, Option<I>)>
1616-
where
1617-
I: Deref<Target = HTLCOutputInCommitment>,
1618-
T: Deref<Target = Vec<HTLCOutputInCommitment>> + IntoIterator<Item = I>,
1648+
fn insert_non_htlc_outputs<F>(
1649+
keys: &TxCreationKeys,
1650+
to_broadcaster_value_sat: Amount,
1651+
to_countersignatory_value_sat: Amount,
1652+
channel_parameters: &DirectedChannelTransactionParameters,
1653+
tx_has_htlc_outputs: bool,
1654+
mut insert_non_htlc_output: F,
1655+
) where
1656+
F: FnMut(TxOut),
16191657
{
16201658
let countersignatory_payment_point = &channel_parameters.countersignatory_pubkeys().payment_point;
16211659
let countersignatory_funding_key = &channel_parameters.countersignatory_pubkeys().funding_pubkey;
16221660
let broadcaster_funding_key = &channel_parameters.broadcaster_pubkeys().funding_pubkey;
16231661
let channel_type = channel_parameters.channel_type_features();
16241662
let contest_delay = channel_parameters.contest_delay();
16251663

1626-
let mut txout_htlc_pairs: Vec<(TxOut, Option<I>)> = Vec::with_capacity(nondust_htlcs.len() + 4);
1627-
16281664
if to_countersignatory_value_sat > Amount::ZERO {
16291665
let script = if channel_type.supports_anchors_zero_fee_htlc_tx() {
16301666
get_to_countersigner_keyed_anchor_redeemscript(countersignatory_payment_point).to_p2wsh()
16311667
} else {
16321668
ScriptBuf::new_p2wpkh(&Hash160::hash(&countersignatory_payment_point.serialize()).into())
16331669
};
1634-
txout_htlc_pairs.push((
1635-
TxOut {
1636-
script_pubkey: script.clone(),
1637-
value: to_countersignatory_value_sat,
1638-
},
1639-
None,
1640-
))
1670+
insert_non_htlc_output(TxOut {
1671+
script_pubkey: script,
1672+
value: to_countersignatory_value_sat,
1673+
});
16411674
}
16421675

16431676
if to_broadcaster_value_sat > Amount::ZERO {
@@ -1646,65 +1679,82 @@ impl CommitmentTransaction {
16461679
contest_delay,
16471680
&keys.broadcaster_delayed_payment_key,
16481681
);
1649-
txout_htlc_pairs.push((
1650-
TxOut {
1651-
script_pubkey: redeem_script.to_p2wsh(),
1652-
value: to_broadcaster_value_sat,
1653-
},
1654-
None,
1655-
));
1682+
insert_non_htlc_output(TxOut {
1683+
script_pubkey: redeem_script.to_p2wsh(),
1684+
value: to_broadcaster_value_sat,
1685+
});
16561686
}
16571687

16581688
if channel_type.supports_anchors_zero_fee_htlc_tx() {
1659-
if to_broadcaster_value_sat > Amount::ZERO || !nondust_htlcs.is_empty() {
1689+
if to_broadcaster_value_sat > Amount::ZERO || tx_has_htlc_outputs {
16601690
let anchor_script = get_keyed_anchor_redeemscript(broadcaster_funding_key);
1661-
txout_htlc_pairs.push((
1662-
TxOut {
1663-
script_pubkey: anchor_script.to_p2wsh(),
1664-
value: Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI),
1665-
},
1666-
None,
1667-
));
1691+
insert_non_htlc_output(TxOut {
1692+
script_pubkey: anchor_script.to_p2wsh(),
1693+
value: Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI),
1694+
});
16681695
}
16691696

1670-
if to_countersignatory_value_sat > Amount::ZERO || !nondust_htlcs.is_empty() {
1697+
if to_countersignatory_value_sat > Amount::ZERO || tx_has_htlc_outputs {
16711698
let anchor_script = get_keyed_anchor_redeemscript(countersignatory_funding_key);
1672-
txout_htlc_pairs.push((
1673-
TxOut {
1674-
script_pubkey: anchor_script.to_p2wsh(),
1675-
value: Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI),
1676-
},
1677-
None,
1678-
));
1699+
insert_non_htlc_output(TxOut {
1700+
script_pubkey: anchor_script.to_p2wsh(),
1701+
value: Amount::from_sat(ANCHOR_OUTPUT_VALUE_SATOSHI),
1702+
});
16791703
}
16801704
}
1705+
}
1706+
1707+
fn build_htlc_outputs(keys: &TxCreationKeys, nondust_htlcs: &Vec<HTLCOutputInCommitment>, channel_type: &ChannelTypeFeatures) -> Vec<TxOut> {
1708+
// Allocate memory for the 4 possible non-htlc outputs
1709+
let mut txouts = Vec::with_capacity(nondust_htlcs.len() + 4);
16811710

16821711
for htlc in nondust_htlcs {
1683-
let script = get_htlc_redeemscript(&htlc, channel_type, keys);
1712+
let script = get_htlc_redeemscript(htlc, channel_type, keys);
16841713
let txout = TxOut {
16851714
script_pubkey: script.to_p2wsh(),
16861715
value: htlc.to_bitcoin_amount(),
16871716
};
1688-
txout_htlc_pairs.push((txout, Some(htlc)));
1717+
txouts.push(txout);
16891718
}
16901719

1691-
// Sort output in BIP-69 order (amount, scriptPubkey). Tie-breaks based on HTLC
1692-
// CLTV expiration height.
1693-
sort_outputs(&mut txout_htlc_pairs, |htlc_output_a, htlc_output_b| {
1694-
if let Some(htlc_output_a) = htlc_output_a {
1695-
if let Some(htlc_output_b) = htlc_output_b {
1696-
htlc_output_a.cltv_expiry.cmp(&htlc_output_b.cltv_expiry)
1697-
// Note that due to hash collisions, we have to have a fallback comparison
1698-
// here for fuzzing mode (otherwise at least chanmon_fail_consistency
1699-
// may fail)!
1700-
.then(htlc_output_a.payment_hash.cmp(&htlc_output_b.payment_hash))
1701-
// For non-HTLC outputs, if they're copying our SPK we don't really care if we
1702-
// close the channel due to mismatches - they're doing something dumb:
1703-
} else { cmp::Ordering::Equal }
1704-
} else { cmp::Ordering::Equal }
1705-
});
1720+
txouts
1721+
}
1722+
1723+
fn build_sorted_htlc_outputs(
1724+
keys: &TxCreationKeys,
1725+
mut nondust_htlcs: Vec<HTLCOutputInCommitment>,
1726+
channel_type: &ChannelTypeFeatures
1727+
) -> (Vec<TxOut>, Vec<HTLCOutputInCommitment>) {
1728+
let mut txouts = Self::build_htlc_outputs(keys, &nondust_htlcs, channel_type);
1729+
1730+
// CLN uses dumb sort... so we do too
1731+
for i in 0..txouts.len() {
1732+
// Find the minimum element
1733+
let mut min = i;
1734+
for j in i + 1..txouts.len() {
1735+
if txouts[j].value.cmp(&txouts[min].value)
1736+
.then(txouts[j].script_pubkey.cmp(&txouts[min].script_pubkey))
1737+
.then(nondust_htlcs[j].cltv_expiry.cmp(&nondust_htlcs[min].cltv_expiry))
1738+
// Note that due to hash collisions, we have to have a fallback comparison
1739+
// here for fuzzing mode (otherwise at least chanmon_fail_consistency
1740+
// may fail)!
1741+
.then(nondust_htlcs[j].payment_hash.cmp(&nondust_htlcs[min].payment_hash))
1742+
.is_lt()
1743+
{
1744+
min = j;
1745+
}
1746+
}
1747+
1748+
// Then swap the minimum element into the first position
1749+
txouts.swap(i, min);
1750+
nondust_htlcs.swap(i, min);
1751+
1752+
// Initialize the transaction output index; we will update it when we
1753+
// add the non-htlc transaction outputs.
1754+
nondust_htlcs[i].transaction_output_index = Some(i as u32);
1755+
}
17061756

1707-
txout_htlc_pairs
1757+
(txouts, nondust_htlcs)
17081758
}
17091759

17101760
fn build_inputs(commitment_number: u64, channel_parameters: &DirectedChannelTransactionParameters) -> (u64, Vec<TxIn>) {

0 commit comments

Comments
 (0)