Skip to content

Commit 1690c66

Browse files
G8XSUG8XSU
committed
Minimize reads to counterparty_claimable_outpoints
Reads related to current/previous commitment_tx should be explicit and reads related to older commitment_txs should be minimized.
1 parent 1890e80 commit 1690c66

File tree

1 file changed

+29
-26
lines changed

1 file changed

+29
-26
lines changed

‎lightning/src/chain/channelmonitor.rs‎

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -2430,8 +2430,8 @@ macro_rules! fail_unbroadcast_htlcs {
24302430
debug_assert_eq!($commitment_tx_confirmed.txid(), $commitment_txid_confirmed);
24312431

24322432
macro_rules! check_htlc_fails {
2433-
($txid: expr, $commitment_tx: expr) => {
2434-
if let Some(ref latest_outpoints) = $self.counterparty_claimable_outpoints.get($txid) {
2433+
($txid: expr, $commitment_tx: expr, $per_commitment_outpoints: expr) => {
2434+
if let Some(ref latest_outpoints) = $per_commitment_outpoints {
24352435
for &(ref htlc, ref source_option) in latest_outpoints.iter() {
24362436
if let &Some(ref source) = source_option {
24372437
// Check if the HTLC is present in the commitment transaction that was
@@ -2491,10 +2491,10 @@ macro_rules! fail_unbroadcast_htlcs {
24912491
}
24922492
}
24932493
if let Some(ref txid) = $self.current_counterparty_commitment_txid {
2494-
check_htlc_fails!(txid, "current");
2494+
check_htlc_fails!(txid, "current", $self.counterparty_claimable_outpoints.get(txid));
24952495
}
24962496
if let Some(ref txid) = $self.prev_counterparty_commitment_txid {
2497-
check_htlc_fails!(txid, "previous");
2497+
check_htlc_fails!(txid, "previous", $self.counterparty_claimable_outpoints.get(txid));
24982498
}
24992499
} }
25002500
}
@@ -2763,15 +2763,15 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
27632763
// If the channel is force closed, try to claim the output from this preimage.
27642764
// First check if a counterparty commitment transaction has been broadcasted:
27652765
macro_rules! claim_htlcs {
2766-
($commitment_number: expr, $txid: expr) => {
2767-
let (htlc_claim_reqs, _) = self.get_counterparty_output_claim_info($commitment_number, $txid, None);
2766+
($commitment_number: expr, $txid: expr, $htlcs: expr) => {
2767+
let (htlc_claim_reqs, _) = self.get_counterparty_output_claim_info($commitment_number, $txid, None, $htlcs);
27682768
self.onchain_tx_handler.update_claims_view_from_requests(htlc_claim_reqs, self.best_block.height, self.best_block.height, broadcaster, fee_estimator, logger);
27692769
}
27702770
}
27712771
if let Some(txid) = self.current_counterparty_commitment_txid {
27722772
if txid == confirmed_spend_txid {
27732773
if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
2774-
claim_htlcs!(*commitment_number, txid);
2774+
claim_htlcs!(*commitment_number, txid, self.counterparty_claimable_outpoints.get(&txid));
27752775
} else {
27762776
debug_assert!(false);
27772777
log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
@@ -2782,7 +2782,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
27822782
if let Some(txid) = self.prev_counterparty_commitment_txid {
27832783
if txid == confirmed_spend_txid {
27842784
if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
2785-
claim_htlcs!(*commitment_number, txid);
2785+
claim_htlcs!(*commitment_number, txid, self.counterparty_claimable_outpoints.get(&txid));
27862786
} else {
27872787
debug_assert!(false);
27882788
log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
@@ -3279,8 +3279,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
32793279
}
32803280

32813281
// Then, try to find revoked htlc outputs
3282-
if let Some(ref per_commitment_data) = per_commitment_option {
3283-
for (_, &(ref htlc, _)) in per_commitment_data.iter().enumerate() {
3282+
if let Some(per_commitment_claimable_data) = per_commitment_option {
3283+
for (htlc, _) in per_commitment_claimable_data {
32843284
if let Some(transaction_output_index) = htlc.transaction_output_index {
32853285
if transaction_output_index as usize >= tx.output.len() ||
32863286
tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 {
@@ -3304,9 +3304,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33043304
}
33053305
self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number);
33063306

3307-
if let Some(per_commitment_data) = per_commitment_option {
3307+
if let Some(per_commitment_claimable_data) = per_commitment_option {
33083308
fail_unbroadcast_htlcs!(self, "revoked_counterparty", commitment_txid, tx, height,
3309-
block_hash, per_commitment_data.iter().map(|(htlc, htlc_source)|
3309+
block_hash, per_commitment_claimable_data.iter().map(|(htlc, htlc_source)|
33103310
(htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
33113311
), logger);
33123312
} else {
@@ -3319,7 +3319,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33193319
block_hash, [].iter().map(|reference| *reference), logger);
33203320
}
33213321
}
3322-
} else if let Some(per_commitment_data) = per_commitment_option {
3322+
} else if let Some(per_commitment_claimable_data) = per_commitment_option {
33233323
// While this isn't useful yet, there is a potential race where if a counterparty
33243324
// revokes a state at the same time as the commitment transaction for that state is
33253325
// confirmed, and the watchtower receives the block before the user, the user could
@@ -3334,12 +3334,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33343334

33353335
log_info!(logger, "Got broadcast of non-revoked counterparty commitment transaction {}", commitment_txid);
33363336
fail_unbroadcast_htlcs!(self, "counterparty", commitment_txid, tx, height, block_hash,
3337-
per_commitment_data.iter().map(|(htlc, htlc_source)|
3337+
per_commitment_claimable_data.iter().map(|(htlc, htlc_source)|
33383338
(htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref()))
33393339
), logger);
3340-
33413340
let (htlc_claim_reqs, counterparty_output_info) =
3342-
self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx));
3341+
self.get_counterparty_output_claim_info(commitment_number, commitment_txid, Some(tx), per_commitment_option);
33433342
to_counterparty_output_info = counterparty_output_info;
33443343
for req in htlc_claim_reqs {
33453344
claimable_outpoints.push(req);
@@ -3350,12 +3349,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33503349
}
33513350

33523351
/// Returns the HTLC claim package templates and the counterparty output info
3353-
fn get_counterparty_output_claim_info(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>)
3352+
fn get_counterparty_output_claim_info(&self, commitment_number: u64, commitment_txid: Txid, tx: Option<&Transaction>, per_commitment_option: Option<&Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>>)
33543353
-> (Vec<PackageTemplate>, CommitmentTxCounterpartyOutputInfo) {
33553354
let mut claimable_outpoints = Vec::new();
33563355
let mut to_counterparty_output_info: CommitmentTxCounterpartyOutputInfo = None;
33573356

3358-
let htlc_outputs = match self.counterparty_claimable_outpoints.get(&commitment_txid) {
3357+
let per_commitment_claimable_data = match per_commitment_option {
33593358
Some(outputs) => outputs,
33603359
None => return (claimable_outpoints, to_counterparty_output_info),
33613360
};
@@ -3394,7 +3393,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33943393
}
33953394
}
33963395

3397-
for (_, &(ref htlc, _)) in htlc_outputs.iter().enumerate() {
3396+
for &(ref htlc, _) in per_commitment_claimable_data.iter() {
33983397
if let Some(transaction_output_index) = htlc.transaction_output_index {
33993398
if let Some(transaction) = tx {
34003399
if transaction_output_index as usize >= transaction.output.len() ||
@@ -3584,6 +3583,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
35843583
if counterparty_commitment_txid == confirmed_commitment_txid {
35853584
continue;
35863585
}
3586+
// If we have generated claims for counterparty_commitment_txid earlier, we can rely on always
3587+
// having claim related htlcs for counterparty_commitment_txid in counterparty_claimable_outpoints.
35873588
for (htlc, _) in self.counterparty_claimable_outpoints.get(counterparty_commitment_txid).unwrap_or(&vec![]) {
35883589
log_trace!(logger, "Canceling claims for previously confirmed counterparty commitment {}",
35893590
counterparty_commitment_txid);
@@ -4219,9 +4220,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
42194220
}
42204221

42214222
macro_rules! check_htlc_valid_counterparty {
4222-
($counterparty_txid: expr, $htlc_output: expr) => {
4223-
if let Some(txid) = $counterparty_txid {
4224-
for &(ref pending_htlc, ref pending_source) in self.counterparty_claimable_outpoints.get(&txid).unwrap() {
4223+
($htlc_output: expr, $per_commitment_data: expr) => {
4224+
for &(ref pending_htlc, ref pending_source) in $per_commitment_data {
42254225
if pending_htlc.payment_hash == $htlc_output.payment_hash && pending_htlc.amount_msat == $htlc_output.amount_msat {
42264226
if let &Some(ref source) = pending_source {
42274227
log_claim!("revoked counterparty commitment tx", false, pending_htlc, true);
@@ -4230,7 +4230,6 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
42304230
}
42314231
}
42324232
}
4233-
}
42344233
}
42354234
}
42364235

@@ -4247,9 +4246,13 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
42474246
// resolve the source HTLC with the original sender.
42484247
payment_data = Some(((*source).clone(), htlc_output.payment_hash, htlc_output.amount_msat));
42494248
} else if !$holder_tx {
4250-
check_htlc_valid_counterparty!(self.current_counterparty_commitment_txid, htlc_output);
4249+
if let Some(current_counterparty_commitment_txid) = &self.current_counterparty_commitment_txid {
4250+
check_htlc_valid_counterparty!(htlc_output, self.counterparty_claimable_outpoints.get(current_counterparty_commitment_txid).unwrap());
4251+
}
42514252
if payment_data.is_none() {
4252-
check_htlc_valid_counterparty!(self.prev_counterparty_commitment_txid, htlc_output);
4253+
if let Some(prev_counterparty_commitment_txid) = &self.prev_counterparty_commitment_txid {
4254+
check_htlc_valid_counterparty!(htlc_output, self.counterparty_claimable_outpoints.get(prev_counterparty_commitment_txid).unwrap());
4255+
}
42534256
}
42544257
}
42554258
if payment_data.is_none() {
@@ -4287,7 +4290,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
42874290
}
42884291
}
42894292
if let Some(ref htlc_outputs) = self.counterparty_claimable_outpoints.get(&input.previous_output.txid) {
4290-
scan_commitment!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, (b.as_ref().clone()).map(|boxed| &**boxed))),
4293+
scan_commitment!(htlc_outputs.iter().map(|&(ref a, ref b)| (a, b.as_ref().map(|boxed| &**boxed))),
42914294
"counterparty commitment tx", false);
42924295
}
42934296

0 commit comments

Comments
 (0)