Skip to content

Commit 9fd53e9

Browse files
tankyleotankyleo
committed
this works
1 parent 7fefc6a commit 9fd53e9

File tree

2 files changed

+81
-96
lines changed

2 files changed

+81
-96
lines changed

‎lightning/src/chain/channelmonitor.rs

Lines changed: 78 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -319,33 +319,30 @@ fn write_legacy_holder_commitment_data<W: Writer>(
319319
let delayed_payment_key = &tx_keys.broadcaster_delayed_payment_key;
320320
let per_commitment_point = &tx_keys.per_commitment_point;
321321

322-
let mut nondust_htlcs = commitment_tx.nondust_htlcs().iter()
323-
.zip(commitment_tx.counterparty_htlc_sigs.iter());
324-
let mut sources = htlc_data.nondust_htlc_sources.iter();
325-
326-
// Use an iterator to write `htlc_outputs` to avoid allocations.
327-
let nondust_htlcs = core::iter::from_fn(move || {
328-
let (htlc, counterparty_htlc_sig) = if let Some(nondust_htlc) = nondust_htlcs.next() {
329-
nondust_htlc
330-
} else {
331-
debug_assert!(sources.next().is_none());
332-
return None;
333-
};
334-
335-
let mut source = None;
336-
if htlc.offered {
337-
source = sources.next();
338-
if source.is_none() {
339-
panic!("Every offered non-dust HTLC should have a corresponding source");
340-
}
322+
let mut table: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = htlc_data.htlcs_and_sources.iter().map(|(htlc, source)| (htlc.clone(), source.as_ref())).collect();
323+
let mut htlc_sig_sources = Vec::new();
324+
for (nondust_htlc, sig) in commitment_tx.nondust_htlcs().iter().zip(commitment_tx.counterparty_htlc_sigs.iter()) {
325+
let (htlc, source) = table
326+
.iter_mut()
327+
.filter(|(htlc, _source)| htlc.transaction_output_index.is_none())
328+
.find_map(|(htlc, source)| {
329+
if htlc.is_data_equal(nondust_htlc) {
330+
Some((htlc, source))
331+
} else {
332+
None
333+
}
334+
})
335+
.unwrap();
336+
htlc.transaction_output_index = Some(nondust_htlc.transaction_output_index.unwrap());
337+
htlc_sig_sources.push((htlc.clone(), Some(sig), source.clone()));
338+
}
339+
for (htlc, source) in table {
340+
if htlc.transaction_output_index.is_none() {
341+
htlc_sig_sources.push((htlc, None, source));
341342
}
342-
Some((htlc, Some(counterparty_htlc_sig), source))
343-
});
343+
}
344344

345-
// Dust HTLCs go last.
346-
let dust_htlcs = htlc_data.dust_htlcs.iter()
347-
.map(|(htlc, source)| (htlc, None::<&Signature>, source.as_ref()));
348-
let htlc_outputs = crate::util::ser::IterableOwned(nondust_htlcs.chain(dust_htlcs));
345+
let htlc_outputs = crate::util::ser::IterableOwned(htlc_sig_sources.into_iter());
349346

350347
write_tlv_fields!(writer, {
351348
(0, txid, required),
@@ -924,44 +921,21 @@ impl<Signer: EcdsaChannelSigner> Clone for ChannelMonitor<Signer> where Signer:
924921
struct HolderCommitmentHTLCData {
925922
// These must be sorted in increasing output index order to match the expected order of the
926923
// HTLCs in the `CommitmentTransaction`.
927-
nondust_htlc_sources: Vec<HTLCSource>,
928-
dust_htlcs: Vec<(HTLCOutputInCommitment, Option<HTLCSource>)>,
924+
htlcs_and_sources: Vec<(HTLCOutputInCommitment, Option<HTLCSource>)>,
929925
}
930926

931927
impl TryFrom<(&HolderCommitmentTransaction, &HolderSignedTx)> for HolderCommitmentHTLCData {
932928
type Error = ();
933929
fn try_from(value: (&HolderCommitmentTransaction, &HolderSignedTx)) -> Result<Self, Self::Error> {
934-
let holder_commitment_tx = value.0;
935930
let holder_signed_tx = value.1;
936931

937-
// HolderSignedTx tracks all HTLCs included in the commitment (dust included). For
938-
// `HolderCommitment`, we'll need to extract the dust HTLCs and their sources, and non-dust
939-
// HTLC sources, separately. All offered, non-dust HTLCs must have a source available.
940-
941-
let mut missing_nondust_source = false;
942-
let mut nondust_htlc_sources = Vec::with_capacity(holder_commitment_tx.nondust_htlcs().len());
943-
let dust_htlcs = holder_signed_tx.htlc_outputs.iter().filter_map(|(htlc, _, source)| {
944-
// Filter our non-dust HTLCs, while at the same time pushing their sources into
945-
// `nondust_htlc_sources`.
946-
if htlc.transaction_output_index.is_none() {
947-
return Some((htlc.clone(), source.clone()))
948-
}
949-
if htlc.offered {
950-
if let Some(source) = source {
951-
nondust_htlc_sources.push(source.clone());
952-
} else {
953-
missing_nondust_source = true;
954-
}
955-
}
956-
None
957-
}).collect();
958-
if missing_nondust_source {
959-
return Err(());
960-
}
961-
962932
Ok(Self {
963-
nondust_htlc_sources,
964-
dust_htlcs,
933+
htlcs_and_sources: holder_signed_tx.htlc_outputs.iter()
934+
.map(|(htlc, _, source)| {
935+
let mut htlc = htlc.clone();
936+
htlc.transaction_output_index = None;
937+
(htlc, source.clone())
938+
}).collect(),
965939
})
966940
}
967941
}
@@ -1171,20 +1145,28 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
11711145
// we'd be unable to mutate `self` while holding an immutable iterator (specifically, returned from
11721146
// a function) over `self`.
11731147
macro_rules! holder_commitment_htlcs {
1174-
($self: expr, CURRENT) => {
1148+
($self: expr, CURRENT) => {{
1149+
let nondust_htlc_count = $self.funding.current_holder_commitment_tx.nondust_htlcs().len();
1150+
let mut dust_iter = $self.current_holder_htlc_data.htlcs_and_sources.iter().map(|(htlc, _)| htlc);
1151+
if nondust_htlc_count > 0 {
1152+
dust_iter.nth(nondust_htlc_count - 1);
1153+
}
11751154
$self.funding.current_holder_commitment_tx.nondust_htlcs().iter()
1176-
.chain($self.current_holder_htlc_data.dust_htlcs.iter().map(|(htlc, _)| htlc))
1177-
};
1155+
.chain(dust_iter)
1156+
}};
11781157
($self: expr, CURRENT_WITH_SOURCES) => {{
11791158
holder_commitment_htlcs!(
11801159
&$self.funding.current_holder_commitment_tx, &$self.current_holder_htlc_data
11811160
)
11821161
}};
11831162
($self: expr, PREV) => {{
11841163
if let Some(tx) = &$self.funding.prev_holder_commitment_tx {
1185-
let dust_htlcs = $self.prev_holder_htlc_data.as_ref().unwrap().dust_htlcs.iter()
1186-
.map(|(htlc, _)| htlc);
1187-
Some(tx.nondust_htlcs().iter().chain(dust_htlcs))
1164+
let nondust_htlc_count = tx.nondust_htlcs().len();
1165+
let mut dust_iter = $self.prev_holder_htlc_data.as_ref().unwrap().htlcs_and_sources.iter().map(|(htlc, _)| htlc);
1166+
if nondust_htlc_count > 0 {
1167+
dust_iter.nth(nondust_htlc_count - 1);
1168+
}
1169+
Some(tx.nondust_htlcs().iter().chain(dust_iter))
11881170
} else {
11891171
None
11901172
}
@@ -1197,19 +1179,22 @@ macro_rules! holder_commitment_htlcs {
11971179
}
11981180
}};
11991181
($commitment_tx: expr, $htlc_data: expr) => {{
1200-
let mut sources = $htlc_data.nondust_htlc_sources.iter();
1201-
let nondust_htlcs = $commitment_tx.nondust_htlcs().iter().map(move |htlc| {
1202-
let mut source = None;
1203-
if htlc.offered && htlc.transaction_output_index.is_some() {
1204-
source = sources.next();
1205-
if source.is_none() {
1206-
panic!("Every offered non-dust HTLC should have a corresponding source");
1207-
}
1208-
}
1209-
(htlc, source)
1210-
});
1211-
let dust_htlcs = $htlc_data.dust_htlcs.iter().map(|(htlc, source)| (htlc, source.as_ref()));
1212-
nondust_htlcs.chain(dust_htlcs)
1182+
let mut table: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = $htlc_data.htlcs_and_sources.iter().map(|(htlc, source)| (htlc.clone(), source.as_ref())).collect();
1183+
for nondust_htlc in $commitment_tx.nondust_htlcs() {
1184+
let htlc = table
1185+
.iter_mut()
1186+
.filter(|(htlc, _source)| htlc.transaction_output_index.is_none())
1187+
.find_map(|(htlc, _source)| {
1188+
if htlc.is_data_equal(nondust_htlc) {
1189+
Some(htlc)
1190+
} else {
1191+
None
1192+
}
1193+
})
1194+
.unwrap();
1195+
htlc.transaction_output_index = Some(nondust_htlc.transaction_output_index.unwrap());
1196+
}
1197+
table.into_iter().map(|(htlc, source)| (htlc, source))
12131198
}};
12141199
}
12151200

@@ -2546,7 +2531,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
25462531
if htlc.transaction_output_index.is_some() {
25472532

25482533
if let Some(bal) = us.get_htlc_balance(
2549-
htlc, source, $holder_commitment, $counterparty_revoked_commitment, confirmed_txid
2534+
&htlc, source, $holder_commitment, $counterparty_revoked_commitment, confirmed_txid
25502535
) {
25512536
res.push(bal);
25522537
}
@@ -2885,7 +2870,7 @@ macro_rules! fail_unbroadcast_htlcs {
28852870
// broadcastable commitment transaction has the HTLC in it, but it
28862871
// cannot currently change after channel initialization, so we don't
28872872
// need to here.
2888-
let confirmed_htlcs_iter: &mut dyn Iterator<Item = (&HTLCOutputInCommitment, Option<&HTLCSource>)> = &mut $confirmed_htlcs_list;
2873+
let confirmed_htlcs_iter: &mut dyn Iterator<Item = (HTLCOutputInCommitment, Option<&HTLCSource>)> = &mut $confirmed_htlcs_list;
28892874

28902875
let mut matched_htlc = false;
28912876
for (ref broadcast_htlc, ref broadcast_source) in confirmed_htlcs_iter {
@@ -3135,7 +3120,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
31353120
// Backfill the non-dust HTLC sources.
31363121
debug_assert!(nondust_htlc_sources.is_empty());
31373122
nondust_htlc_sources.reserve_exact(holder_commitment_tx.nondust_htlcs().len());
3138-
let dust_htlcs = htlc_outputs.into_iter().filter_map(|(htlc, _, source)| {
3123+
let _dust_htlcs: Vec<_> = htlc_outputs.into_iter().filter_map(|(htlc, _, source)| {
31393124
// Filter our non-dust HTLCs, while at the same time pushing their sources into
31403125
// `nondust_htlc_sources`.
31413126
if htlc.transaction_output_index.is_none() {
@@ -3146,8 +3131,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
31463131
}
31473132
None
31483133
}).collect();
3149-
3150-
dust_htlcs
3134+
panic!("This path should not be exercised");
31513135
} else {
31523136
// If we don't have any non-dust HTLCs in htlc_outputs, assume they were all passed via
31533137
// `nondust_htlc_sources`, building up the final htlc_outputs by combining
@@ -3160,6 +3144,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
31603144
}
31613145
}
31623146

3147+
/*
31633148
debug_assert!(htlc_outputs.iter().all(|(htlc, _, _)| htlc.transaction_output_index.is_none()));
31643149
debug_assert!(htlc_outputs.iter().all(|(_, sig_opt, _)| sig_opt.is_none()));
31653150
debug_assert_eq!(holder_commitment_tx.trust().nondust_htlcs().len(), holder_commitment_tx.counterparty_htlc_sigs.len());
@@ -3172,17 +3157,21 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
31723157
}
31733158
}
31743159
assert!(sources.next().is_none(), "All HTLC sources should have been exhausted");
3160+
*/
31753161

31763162
// This only includes dust HTLCs as checked above.
3177-
htlc_outputs.into_iter().map(|(htlc, _, source)| (htlc, source)).collect()
3163+
htlc_outputs.into_iter().map(|(mut htlc, _, source)| {
3164+
htlc.transaction_output_index = None;
3165+
(htlc, source)
3166+
}).collect()
31783167
};
31793168

31803169
self.current_holder_commitment_number = holder_commitment_tx.trust().commitment_number();
31813170
self.onchain_tx_handler.provide_latest_holder_tx(holder_commitment_tx.clone());
31823171

31833172
mem::swap(&mut holder_commitment_tx, &mut self.funding.current_holder_commitment_tx);
31843173
self.funding.prev_holder_commitment_tx = Some(holder_commitment_tx);
3185-
let mut holder_htlc_data = HolderCommitmentHTLCData { nondust_htlc_sources, dust_htlcs };
3174+
let mut holder_htlc_data = HolderCommitmentHTLCData { htlcs_and_sources: dust_htlcs };
31863175
mem::swap(&mut holder_htlc_data, &mut self.current_holder_htlc_data);
31873176
self.prev_holder_htlc_data = Some(holder_htlc_data);
31883177

@@ -3812,7 +3801,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
38123801
if let Some(per_commitment_claimable_data) = per_commitment_option {
38133802
fail_unbroadcast_htlcs!(self, "revoked_counterparty", commitment_txid, tx, height,
38143803
block_hash, per_commitment_claimable_data.iter().map(|(htlc, htlc_source)|
3815-
(htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
3804+
(htlc.clone(), htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
38163805
), logger);
38173806
} else {
38183807
// Our fuzzers aren't constrained by pesky things like valid signatures, so can
@@ -3821,7 +3810,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
38213810
// fuzzing.
38223811
debug_assert!(cfg!(fuzzing), "We should have per-commitment option for any recognized old commitment txn");
38233812
fail_unbroadcast_htlcs!(self, "revoked counterparty", commitment_txid, tx, height,
3824-
block_hash, [].iter().map(|reference| *reference), logger);
3813+
block_hash, core::iter::empty(), logger);
38253814
}
38263815
}
38273816
} else if let Some(per_commitment_claimable_data) = per_commitment_option {
@@ -3837,7 +3826,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
38373826
log_info!(logger, "Got broadcast of non-revoked counterparty commitment transaction {}", commitment_txid);
38383827
fail_unbroadcast_htlcs!(self, "counterparty", commitment_txid, tx, height, block_hash,
38393828
per_commitment_claimable_data.iter().map(|(htlc, htlc_source)|
3840-
(htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
3829+
(htlc.clone(), htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
38413830
), logger);
38423831
let (htlc_claim_reqs, counterparty_output_info) =
38433832
self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx), per_commitment_option);
@@ -4532,8 +4521,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
45324521
} else { None }.into_iter().flatten();
45334522

45344523
let htlcs = holder_commitment_htlcs!(self, CURRENT_WITH_SOURCES)
4535-
.chain(current_counterparty_htlcs)
4536-
.chain(prev_counterparty_htlcs);
4524+
.chain(current_counterparty_htlcs.map(|(htlc, source)| (htlc.clone(), source)))
4525+
.chain(prev_counterparty_htlcs.map(|(htlc, source)| (htlc.clone(), source)));
45374526

45384527
let height = self.best_block.height;
45394528
for (htlc, source_opt) in htlcs {
@@ -5728,7 +5717,7 @@ mod tests {
57285717
let nondust_htlcs = dummy_commitment_tx.nondust_htlcs();
57295718

57305719
monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
5731-
nondust_htlcs.into_iter().map(|htlc| (htlc.clone(), Some(dummy_sig), Some(dummy_source.clone()))).collect());
5720+
nondust_htlcs.into_iter().map(|htlc| (htlc.clone(), None, Some(dummy_source.clone()))).collect());
57325721
monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()),
57335722
preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
57345723
monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()),
@@ -5768,7 +5757,7 @@ mod tests {
57685757
// These HTLCs now have their output indices assigned
57695758
let nondust_htlcs = dummy_commitment_tx.nondust_htlcs();
57705759
monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
5771-
nondust_htlcs.into_iter().map(|htlc| (htlc.clone(), Some(dummy_sig), Some(dummy_source.clone()))).collect());
5760+
nondust_htlcs.into_iter().map(|htlc| (htlc.clone(), None, Some(dummy_source.clone()))).collect());
57725761
secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
57735762
monitor.provide_secret(281474976710653, secret.clone()).unwrap();
57745763
assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 12);
@@ -5781,7 +5770,7 @@ mod tests {
57815770
// These HTLCs now have their output indices assigned
57825771
let nondust_htlcs = dummy_commitment_tx.nondust_htlcs();
57835772
monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(),
5784-
nondust_htlcs.into_iter().map(|htlc| (htlc.clone(), Some(dummy_sig), Some(dummy_source.clone()))).collect());
5773+
nondust_htlcs.into_iter().map(|htlc| (htlc.clone(), None, Some(dummy_source.clone()))).collect());
57855774
secret[0..32].clone_from_slice(&<Vec<u8>>::from_hex("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
57865775
monitor.provide_secret(281474976710652, secret.clone()).unwrap();
57875776
assert_eq!(monitor.inner.lock().unwrap().payment_preimages.len(), 5);

‎lightning/src/ln/channel.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3803,8 +3803,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38033803

38043804
let holder_keys = commitment_data.tx.trust().keys();
38053805
let mut nondust_htlc_sources = Vec::with_capacity(commitment_data.tx.nondust_htlcs().len());
3806-
let mut dust_htlcs = Vec::with_capacity(commitment_data.htlcs_included.len() - commitment_data.tx.nondust_htlcs().len());
3807-
for (idx, (htlc, mut source_opt)) in commitment_data.htlcs_included.into_iter().enumerate() {
3806+
for (idx, (htlc, mut source_opt)) in commitment_data.htlcs_included.clone().into_iter().enumerate() {
38083807
if let Some(_) = htlc.transaction_output_index {
38093808
let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_data.tx.feerate_per_kw(),
38103809
funding.get_counterparty_selected_contest_delay().unwrap(), &htlc, funding.get_channel_type(),
@@ -3826,10 +3825,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38263825
panic!("Missing outbound HTLC source");
38273826
}
38283827
}
3829-
} else {
3830-
dust_htlcs.push((htlc, None, source_opt.take().cloned()));
38313828
}
3832-
debug_assert!(source_opt.is_none(), "HTLCSource should have been put somewhere");
38333829
}
38343830

38353831
let holder_commitment_tx = HolderCommitmentTransaction::new(
@@ -3845,8 +3841,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
38453841

38463842
Ok(LatestHolderCommitmentTXInfo {
38473843
commitment_tx: holder_commitment_tx,
3848-
htlc_outputs: dust_htlcs,
3849-
nondust_htlc_sources,
3844+
htlc_outputs: commitment_data.htlcs_included.into_iter().map(|(htlc, source)| (htlc, None::<Signature>, source.cloned())).collect(),
3845+
nondust_htlc_sources: Vec::new(),
38503846
})
38513847
}
38523848

0 commit comments

Comments
 (0)