Skip to content

Commit 92d8e53

Browse files
TheBlueMattTheBlueMatt
committed
Make HTLCOutputInCommitment::transaction_output_index an Option
We really shouldn't have split out the with-source HTLCs from the in-transaction HTLCs when we added back-failing, and will need almost all of the info in HTLCOutputInCommitment for each HTLC to fix would_broadcast_at_height, so this is a first step at recombining them.
1 parent 0baf07e commit 92d8e53

File tree

3 files changed

+193
-181
lines changed

3 files changed

+193
-181
lines changed

‎src/ln/chan_utils.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ pub struct HTLCOutputInCommitment {
147147
pub amount_msat: u64,
148148
pub cltv_expiry: u32,
149149
pub payment_hash: PaymentHash,
150-
pub transaction_output_index: u32,
150+
pub transaction_output_index: Option<u32>,
151151
}
152152

153153
#[inline]
@@ -222,12 +222,13 @@ pub fn get_htlc_redeemscript(htlc: &HTLCOutputInCommitment, keys: &TxCreationKey
222222
get_htlc_redeemscript_with_explicit_keys(htlc, &keys.a_htlc_key, &keys.b_htlc_key, &keys.revocation_key)
223223
}
224224

225+
/// panics if htlc.transaction_output_index.is_none()!
225226
pub fn build_htlc_transaction(prev_hash: &Sha256dHash, feerate_per_kw: u64, to_self_delay: u16, htlc: &HTLCOutputInCommitment, a_delayed_payment_key: &PublicKey, revocation_key: &PublicKey) -> Transaction {
226227
let mut txins: Vec<TxIn> = Vec::new();
227228
txins.push(TxIn {
228229
previous_output: OutPoint {
229230
txid: prev_hash.clone(),
230-
vout: htlc.transaction_output_index,
231+
vout: htlc.transaction_output_index.expect("Can't build an HTLC transaction for a dust output"),
231232
},
232233
script_sig: Script::new(),
233234
sequence: 0,

‎src/ln/channel.rs

Lines changed: 44 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -132,18 +132,6 @@ struct OutboundHTLCOutput {
132132
fail_reason: Option<HTLCFailReason>,
133133
}
134134

135-
macro_rules! get_htlc_in_commitment {
136-
($htlc: expr, $offered: expr) => {
137-
HTLCOutputInCommitment {
138-
offered: $offered,
139-
amount_msat: $htlc.amount_msat,
140-
cltv_expiry: $htlc.cltv_expiry,
141-
payment_hash: $htlc.payment_hash,
142-
transaction_output_index: 0
143-
}
144-
}
145-
}
146-
147135
/// See AwaitingRemoteRevoke ChannelState for more info
148136
enum HTLCUpdateAwaitingACK {
149137
AddHTLC {
@@ -775,38 +763,46 @@ impl Channel {
775763
};
776764

777765
let mut txouts: Vec<(TxOut, Option<(HTLCOutputInCommitment, Option<&HTLCSource>)>)> = Vec::with_capacity(self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len() + 2);
778-
let mut unincluded_htlc_sources: Vec<(PaymentHash, &HTLCSource, Option<u32>)> = Vec::new();
766+
let mut unincluded_htlc_sources: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new();
779767

780768
let dust_limit_satoshis = if local { self.our_dust_limit_satoshis } else { self.their_dust_limit_satoshis };
781769
let mut remote_htlc_total_msat = 0;
782770
let mut local_htlc_total_msat = 0;
783771
let mut value_to_self_msat_offset = 0;
784772

773+
macro_rules! get_htlc_in_commitment {
774+
($htlc: expr, $offered: expr) => {
775+
HTLCOutputInCommitment {
776+
offered: $offered,
777+
amount_msat: $htlc.amount_msat,
778+
cltv_expiry: $htlc.cltv_expiry,
779+
payment_hash: $htlc.payment_hash,
780+
transaction_output_index: None
781+
}
782+
}
783+
}
784+
785785
macro_rules! add_htlc_output {
786786
($htlc: expr, $outbound: expr, $source: expr) => {
787787
if $outbound == local { // "offered HTLC output"
788+
let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
788789
if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
789-
let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
790790
txouts.push((TxOut {
791791
script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
792792
value: $htlc.amount_msat / 1000
793793
}, Some((htlc_in_tx, $source))));
794794
} else {
795-
if let Some(source) = $source {
796-
unincluded_htlc_sources.push(($htlc.payment_hash, source, None));
797-
}
795+
unincluded_htlc_sources.push((htlc_in_tx, $source));
798796
}
799797
} else {
798+
let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
800799
if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) {
801-
let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
802800
txouts.push((TxOut { // "received HTLC output"
803801
script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
804802
value: $htlc.amount_msat / 1000
805803
}, Some((htlc_in_tx, $source))));
806804
} else {
807-
if let Some(source) = $source {
808-
unincluded_htlc_sources.push(($htlc.payment_hash, source, None));
809-
}
805+
unincluded_htlc_sources.push((htlc_in_tx, $source));
810806
}
811807
}
812808
}
@@ -924,14 +920,18 @@ impl Channel {
924920
for (idx, out) in txouts.drain(..).enumerate() {
925921
outputs.push(out.0);
926922
if let Some((mut htlc, source_option)) = out.1 {
927-
htlc.transaction_output_index = idx as u32;
923+
htlc.transaction_output_index = Some(idx as u32);
928924
if let Some(source) = source_option {
929925
htlc_sources.push((htlc.payment_hash, source, Some(idx as u32)));
930926
}
931927
htlcs_included.push(htlc);
932928
}
933929
}
934-
htlc_sources.append(&mut unincluded_htlc_sources);
930+
for (htlc, source_option) in unincluded_htlc_sources.drain(..) {
931+
if let Some(source) = source_option {
932+
htlc_sources.push((htlc.payment_hash, source, None));
933+
}
934+
}
935935

936936
(Transaction {
937937
version: 2,
@@ -1720,18 +1720,20 @@ impl Channel {
17201720

17211721
let mut htlcs_and_sigs = Vec::with_capacity(local_commitment_tx.1.len());
17221722
for (idx, htlc) in local_commitment_tx.1.drain(..).enumerate() {
1723-
let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
1724-
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
1725-
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
1726-
secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
1727-
let htlc_sig = if htlc.offered {
1728-
let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, &htlc, &local_keys)?;
1729-
new_local_commitment_txn.push(htlc_tx);
1730-
htlc_sig
1731-
} else {
1732-
self.create_htlc_tx_signature(&htlc_tx, &htlc, &local_keys)?.1
1733-
};
1734-
htlcs_and_sigs.push((htlc, msg.htlc_signatures[idx], htlc_sig));
1723+
if let Some(_) = htlc.transaction_output_index {
1724+
let mut htlc_tx = self.build_htlc_transaction(&local_commitment_txid, &htlc, true, &local_keys, feerate_per_kw);
1725+
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys);
1726+
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
1727+
secp_check!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key), "Invalid HTLC tx signature from peer");
1728+
let htlc_sig = if htlc.offered {
1729+
let htlc_sig = self.sign_htlc_transaction(&mut htlc_tx, &msg.htlc_signatures[idx], &None, &htlc, &local_keys)?;
1730+
new_local_commitment_txn.push(htlc_tx);
1731+
htlc_sig
1732+
} else {
1733+
self.create_htlc_tx_signature(&htlc_tx, &htlc, &local_keys)?.1
1734+
};
1735+
htlcs_and_sigs.push((htlc, msg.htlc_signatures[idx], htlc_sig));
1736+
}
17351737
}
17361738

17371739
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number - 1));
@@ -3295,11 +3297,13 @@ impl Channel {
32953297
let mut htlc_sigs = Vec::new();
32963298

32973299
for ref htlc in remote_commitment_tx.1.iter() {
3298-
let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys, feerate_per_kw);
3299-
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys);
3300-
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
3301-
let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
3302-
htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key));
3300+
if let Some(_) = htlc.transaction_output_index {
3301+
let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys, feerate_per_kw);
3302+
let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys);
3303+
let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap();
3304+
let our_htlc_key = secp_check!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key), "Derived invalid key, peer is maliciously selecting parameters");
3305+
htlc_sigs.push(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key));
3306+
}
33033307
}
33043308

33053309
Ok((msgs::CommitmentSigned {

0 commit comments

Comments
 (0)