From: Matt Corallo <git@bluematt.me>
Date: Wed, 7 Dec 2022 00:30:43 +0000 (+0000)
Subject: DRY the comparison blocks in `update_claims_view_from_matched_txn`
X-Git-Tag: v0.0.113~7^2~4
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=60f3c0a206bfdc851400bce7753db688b01b094e;p=rust-lightning

DRY the comparison blocks in `update_claims_view_from_matched_txn`

In `update_claims_view_from_matched_txn` we have two different
tx-equivalence checks which do the same thing - both check that the
tx which appeared on chain spent all of the outpoints which we
intended to spend in a given package. While one is more effecient
than the other (but only usable in a subset of cases), the
difference between O(N) and O(N^2) when N is 1-5 is trivial.

Still, it is possible we hit this code with just shy of 900 HTLC
outputs in a channel, and a transaction with a ton of inputs.

While having to spin through a few million entries if our
counterparty wastes a full block isn't really a big deal, we go
ahead and use a sorted vec and binary searches because its trivial.
---

diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
index c2ab8751..b7534946 100644
--- a/lightning/src/chain/onchaintx.rs
+++ b/lightning/src/chain/onchaintx.rs
@@ -733,31 +733,13 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 						// outpoints to know if transaction is the original claim or a bumped one issued
 						// by us.
 						let mut are_sets_equal = true;
-						if !request.requires_external_funding() || !request.is_malleable() {
-							// If the claim does not require external funds to be allocated through
-							// additional inputs we can simply check the inputs in order as they
-							// cannot change under us.
-							if request.outpoints().len() != tx.input.len() {
+						let mut tx_inputs = tx.input.iter().map(|input| &input.previous_output).collect::<Vec<_>>();
+						tx_inputs.sort_unstable();
+						for request_input in request.outpoints() {
+							if tx_inputs.binary_search(&request_input).is_err() {
 								are_sets_equal = false;
-							} else {
-								for (claim_inp, tx_inp) in request.outpoints().iter().zip(tx.input.iter()) {
-									if **claim_inp != tx_inp.previous_output {
-										are_sets_equal = false;
-									}
-								}
-							}
-						} else {
-							// Otherwise, we'll do a linear search for each input (we don't expect
-							// large input sets to exist) to ensure the request's input set is fully
-							// spent to be resilient against the external claim reordering inputs.
-							let mut spends_all_inputs = true;
-							for request_input in request.outpoints() {
-								if tx.input.iter().find(|input| input.previous_output == *request_input).is_none() {
-									spends_all_inputs = false;
-									break;
-								}
+								break;
 							}
-							are_sets_equal = spends_all_inputs;
 						}
 
 						macro_rules! clean_claim_request_after_safety_delay {