From ce962ad5c583a0c1d1a786b9f229097e361dec09 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 11 May 2023 15:39:13 -0700 Subject: [PATCH] Change package ID computation for HTLC claims on anchor channels While the previous way of computing the identifier was safe, it wouldn't have been in certain scenarios if we considered splitting aggregated packages. While this type of splitting has yet to be implemented, it may come in the near future. To ensure we're prepared to handle such, we opt to instead commit to all of the HTLCs to claim in the request. --- lightning/src/chain/onchaintx.rs | 33 +++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 45968c57..3dd229f3 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -17,9 +17,12 @@ use bitcoin::PackedLockTime; use bitcoin::blockdata::transaction::Transaction; use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint; use bitcoin::blockdata::script::Script; - +use bitcoin::hashes::Hash; +#[cfg(anchors)] +use bitcoin::hashes::HashEngine; +#[cfg(anchors)] +use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hash_types::{Txid, BlockHash}; - use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; use bitcoin::secp256k1; @@ -48,7 +51,6 @@ use core::ops::Deref; use core::mem::replace; #[cfg(anchors)] use core::mem::swap; -use bitcoin::hashes::Hash; const MAX_ALLOC_SIZE: usize = 64*1024; @@ -774,19 +776,24 @@ impl OnchainTxHandler OnchainClaim::Event(claim_event) => { log_info!(logger, "Yielding onchain event to spend inputs {:?}", req.outpoints()); let package_id = match claim_event { - ClaimEvent::BumpCommitment { ref commitment_tx, .. } => commitment_tx.txid().into_inner(), + ClaimEvent::BumpCommitment { ref commitment_tx, .. } => + // For commitment claims, we can just use their txid as it should + // already be unique. + commitment_tx.txid().into_inner(), ClaimEvent::BumpHTLC { ref htlcs, .. } => { - // Use the same construction as a lightning channel id to generate - // the package id for this request based on the first HTLC. It - // doesn't matter what we use as long as it's unique per request. - let mut package_id = [0; 32]; - package_id[..].copy_from_slice(&htlcs[0].commitment_txid[..]); - let htlc_output_index = htlcs[0].htlc.transaction_output_index.unwrap(); - package_id[30] ^= ((htlc_output_index >> 8) & 0xff) as u8; - package_id[31] ^= ((htlc_output_index >> 0) & 0xff) as u8; - package_id + // For HTLC claims, commit to the entire set of HTLC outputs to + // claim, which will always be unique per request. Once a claim ID + // is generated, it is assigned and remains unchanged, even if the + // underlying set of HTLCs changes. + let mut engine = Sha256::engine(); + for htlc in htlcs { + engine.input(&htlc.commitment_txid.into_inner()); + engine.input(&htlc.htlc.transaction_output_index.unwrap().to_be_bytes()); + } + Sha256::from_engine(engine).into_inner() }, }; + debug_assert!(self.pending_claim_requests.get(&package_id).is_none()); debug_assert_eq!(self.pending_claim_events.iter().filter(|entry| entry.0 == package_id).count(), 0); self.pending_claim_events.push((package_id, claim_event)); package_id -- 2.30.2