Merge pull request #1904 from TheBlueMatt/2022-12-1825-followups
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 12 Dec 2022 17:58:21 +0000 (17:58 +0000)
committerGitHub <noreply@github.com>
Mon, 12 Dec 2022 17:58:21 +0000 (17:58 +0000)
Trivial Followups to #1825

lightning/src/chain/channelmonitor.rs
lightning/src/chain/onchaintx.rs
lightning/src/ln/chan_utils.rs
lightning/src/util/events.rs

index 5c1e102aefc8e2cc44350acc052f99ffdab5e224..a41d853311ce924f388eaa5efbbb2ceb79489a64 100644 (file)
@@ -3032,10 +3032,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                if let Some(new_outputs) = new_outputs_option {
                                                        watch_outputs.push(new_outputs);
                                                }
-                                               // Since there may be multiple HTLCs (all from the same commitment) being
-                                               // claimed by the counterparty within the same transaction, and
-                                               // `check_spend_counterparty_htlc` already checks for all of them, we can
-                                               // safely break from our loop.
+                                               // Since there may be multiple HTLCs for this channel (all spending the
+                                               // same commitment tx) being claimed by the counterparty within the same
+                                               // transaction, and `check_spend_counterparty_htlc` already checks all the
+                                               // ones relevant to this channel, we can safely break from our loop.
                                                break;
                                        }
                                }
index 5bf3a8fd4823369902fba99752ef4b546aebaad3..9bfe6fa9af21b1547e5aa8bf2183062cde66f4cd 100644 (file)
@@ -79,7 +79,7 @@ enum OnchainEvent {
        /// Outpoint under claim process by our own tx, once this one get enough confirmations, we remove it from
        /// bump-txn candidate buffer.
        Claim {
-               claim_request: Txid,
+               package_id: PackageID,
        },
        /// Claim tx aggregate multiple claimable outpoints. One of the outpoint may be claimed by a counterparty party tx.
        /// In this case, we need to drop the outpoint and regenerate a new claim tx. By safety, we keep tracking
@@ -123,7 +123,7 @@ impl MaybeReadable for OnchainEventEntry {
 
 impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
        (0, Claim) => {
-               (0, claim_request, required),
+               (0, package_id, required),
        },
        (1, ContentiousOutpoint) => {
                (0, package, required),
@@ -480,8 +480,8 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
                                // We check for outpoint spends within claims individually rather than as a set
                                // since requests can have outpoints split off.
                                if !self.onchain_events_awaiting_threshold_conf.iter()
-                                       .any(|event_entry| if let OnchainEvent::Claim { claim_request } = event_entry.event {
-                                               first_claim_txid_height.0 == claim_request.into_inner()
+                                       .any(|event_entry| if let OnchainEvent::Claim { package_id } = event_entry.event {
+                                               first_claim_txid_height.0 == package_id
                                        } else {
                                                // The onchain event is not a claim, keep seeking until we find one.
                                                false
@@ -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 {
@@ -766,7 +748,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
                                                                        txid: tx.txid(),
                                                                        height: conf_height,
                                                                        block_hash: Some(conf_hash),
-                                                                       event: OnchainEvent::Claim { claim_request: Txid::from_inner(first_claim_txid_height.0) }
+                                                                       event: OnchainEvent::Claim { package_id: first_claim_txid_height.0 }
                                                                };
                                                                if !self.onchain_events_awaiting_threshold_conf.contains(&entry) {
                                                                        self.onchain_events_awaiting_threshold_conf.push(entry);
@@ -821,13 +803,13 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
                for entry in onchain_events_awaiting_threshold_conf {
                        if entry.has_reached_confirmation_threshold(cur_height) {
                                match entry.event {
-                                       OnchainEvent::Claim { claim_request } => {
-                                               let package_id = claim_request.into_inner();
+                                       OnchainEvent::Claim { package_id } => {
                                                // We may remove a whole set of claim outpoints here, as these one may have
                                                // been aggregated in a single tx and claimed so atomically
                                                if let Some(request) = self.pending_claim_requests.remove(&package_id) {
                                                        for outpoint in request.outpoints() {
-                                                               log_debug!(logger, "Removing claim tracking for {} due to maturation of claim tx {}.", outpoint, claim_request);
+                                                               log_debug!(logger, "Removing claim tracking for {} due to maturation of claim package {}.",
+                                                                       outpoint, log_bytes!(package_id));
                                                                self.claimable_outpoints.remove(&outpoint);
                                                                #[cfg(anchors)]
                                                                self.pending_claim_events.remove(&package_id);
@@ -1065,7 +1047,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 
        #[cfg(anchors)]
        pub(crate) fn generate_external_htlc_claim(
-               &mut self, outp: &::bitcoin::OutPoint, preimage: &Option<PaymentPreimage>
+               &self, outp: &::bitcoin::OutPoint, preimage: &Option<PaymentPreimage>
        ) -> Option<ExternalHTLCClaim> {
                let find_htlc = |holder_commitment: &HolderCommitmentTransaction| -> Option<ExternalHTLCClaim> {
                        let trusted_tx = holder_commitment.trust();
index 729425d70d5bd705bfe65e345567794c8381e64e..408f1cd7e477ca3eb0e4acde601cde9eb0c908f1 100644 (file)
@@ -739,17 +739,12 @@ pub fn build_htlc_input_witness(
        } else {
                EcdsaSighashType::All
        };
-       let mut remote_sig = remote_sig.serialize_der().to_vec();
-       remote_sig.push(remote_sighash_type as u8);
-
-       let mut local_sig = local_sig.serialize_der().to_vec();
-       local_sig.push(EcdsaSighashType::All as u8);
 
        let mut witness = Witness::new();
        // First push the multisig dummy, note that due to BIP147 (NULLDUMMY) it must be a zero-length element.
        witness.push(vec![]);
-       witness.push(remote_sig);
-       witness.push(local_sig);
+       witness.push_bitcoin_signature(&remote_sig.serialize_der(), remote_sighash_type);
+       witness.push_bitcoin_signature(&local_sig.serialize_der(), EcdsaSighashType::All);
        if let Some(preimage) = preimage {
                witness.push(preimage.0.to_vec());
        } else {
@@ -801,9 +796,10 @@ pub(crate) fn get_anchor_output<'a>(commitment_tx: &'a Transaction, funding_pubk
 /// Returns the witness required to satisfy and spend an anchor input.
 pub fn build_anchor_input_witness(funding_key: &PublicKey, funding_sig: &Signature) -> Witness {
        let anchor_redeem_script = chan_utils::get_anchor_redeemscript(funding_key);
-       let mut funding_sig = funding_sig.serialize_der().to_vec();
-       funding_sig.push(EcdsaSighashType::All as u8);
-       Witness::from_vec(vec![funding_sig, anchor_redeem_script.to_bytes()])
+       let mut ret = Witness::new();
+       ret.push_bitcoin_signature(&funding_sig.serialize_der(), EcdsaSighashType::All);
+       ret.push(anchor_redeem_script.as_bytes());
+       ret
 }
 
 /// Per-channel data used to build transactions in conjunction with the per-commitment data (CommitmentTransaction).
@@ -1037,17 +1033,13 @@ impl HolderCommitmentTransaction {
                // First push the multisig dummy, note that due to BIP147 (NULLDUMMY) it must be a zero-length element.
                let mut tx = self.inner.built.transaction.clone();
                tx.input[0].witness.push(Vec::new());
-               let mut ser_holder_sig = holder_sig.serialize_der().to_vec();
-               ser_holder_sig.push(EcdsaSighashType::All as u8);
-               let mut ser_cp_sig = self.counterparty_sig.serialize_der().to_vec();
-               ser_cp_sig.push(EcdsaSighashType::All as u8);
 
                if self.holder_sig_first {
-                       tx.input[0].witness.push(ser_holder_sig);
-                       tx.input[0].witness.push(ser_cp_sig);
+                       tx.input[0].witness.push_bitcoin_signature(&holder_sig.serialize_der(), EcdsaSighashType::All);
+                       tx.input[0].witness.push_bitcoin_signature(&self.counterparty_sig.serialize_der(), EcdsaSighashType::All);
                } else {
-                       tx.input[0].witness.push(ser_cp_sig);
-                       tx.input[0].witness.push(ser_holder_sig);
+                       tx.input[0].witness.push_bitcoin_signature(&self.counterparty_sig.serialize_der(), EcdsaSighashType::All);
+                       tx.input[0].witness.push_bitcoin_signature(&holder_sig.serialize_der(), EcdsaSighashType::All);
                }
 
                tx.input[0].witness.push(funding_redeemscript.as_bytes().to_vec());
index 2c7e5413dafff9d9f027eab116d0967faae6ed2e..375174f6f6afa86c556c6804586adfa52020844a 100644 (file)
@@ -1125,6 +1125,7 @@ impl Writeable for Event {
                                        BumpTransactionEvent::ChannelClose { .. } => {}
                                        BumpTransactionEvent::HTLCResolution { .. } => {}
                                }
+                               write_tlv_fields!(writer, {}); // Write a length field for forwards compat
                        }
                        &Event::ChannelReady { ref channel_id, ref user_channel_id, ref counterparty_node_id, ref channel_type } => {
                                29u8.write(writer)?;