Skip to content

Commit a1a2f2a

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request lightningdevkit#2667 from wpaulino/random-htlc-holder-sigs-non-anchors
Use sign_holder_htlc_transaction to sign non-anchors holder HTLCs
2 parents fba99b7 + b06a652 commit a1a2f2a

File tree

8 files changed

+435
-334
lines changed

8 files changed

+435
-334
lines changed

‎lightning/src/chain/channelmonitor.rs‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,15 @@ use crate::chain;
4242
use crate::chain::{BestBlock, WatchedOutput};
4343
use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator, LowerBoundedFeeEstimator};
4444
use crate::chain::transaction::{OutPoint, TransactionData};
45-
use crate::sign::{SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, WriteableEcdsaChannelSigner, SignerProvider, EntropySource};
45+
use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, SpendableOutputDescriptor, StaticPaymentOutputDescriptor, DelayedPaymentOutputDescriptor, WriteableEcdsaChannelSigner, SignerProvider, EntropySource};
4646
use crate::chain::onchaintx::{ClaimEvent, OnchainTxHandler};
4747
use crate::chain::package::{CounterpartyOfferedHTLCOutput, CounterpartyReceivedHTLCOutput, HolderFundingOutput, HolderHTLCOutput, PackageSolvingData, PackageTemplate, RevokedOutput, RevokedHTLCOutput};
4848
use crate::chain::Filter;
4949
use crate::util::logger::Logger;
5050
use crate::util::ser::{Readable, ReadableArgs, RequiredWrapper, MaybeReadable, UpgradableRequired, Writer, Writeable, U48};
5151
use crate::util::byte_utils;
5252
use crate::events::{Event, EventHandler};
53-
use crate::events::bump_transaction::{ChannelDerivationParameters, AnchorDescriptor, HTLCDescriptor, BumpTransactionEvent};
53+
use crate::events::bump_transaction::{AnchorDescriptor, BumpTransactionEvent};
5454

5555
use crate::prelude::*;
5656
use core::{cmp, mem};
@@ -1172,9 +1172,10 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
11721172
(holder_commitment_tx, trusted_tx.commitment_number())
11731173
};
11741174

1175-
let onchain_tx_handler =
1176-
OnchainTxHandler::new(destination_script.clone(), keys,
1177-
channel_parameters.clone(), initial_holder_commitment_tx, secp_ctx);
1175+
let onchain_tx_handler = OnchainTxHandler::new(
1176+
channel_value_satoshis, channel_keys_id, destination_script.clone(), keys,
1177+
channel_parameters.clone(), initial_holder_commitment_tx, secp_ctx
1178+
);
11781179

11791180
let mut outputs_to_watch = HashMap::new();
11801181
outputs_to_watch.insert(funding_info.0.txid, vec![(funding_info.0.index as u32, funding_info.1.clone())]);
@@ -2835,6 +2836,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
28352836
per_commitment_point: self.onchain_tx_handler.signer.get_per_commitment_point(
28362837
htlc.per_commitment_number, &self.onchain_tx_handler.secp_ctx,
28372838
),
2839+
feerate_per_kw: 0,
28382840
htlc: htlc.htlc,
28392841
preimage: htlc.preimage,
28402842
counterparty_sig: htlc.counterparty_sig,
@@ -3354,7 +3356,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
33543356
continue;
33553357
}
33563358
} else { None };
3357-
if let Some(htlc_tx) = self.onchain_tx_handler.unsafe_get_fully_signed_htlc_tx(
3359+
if let Some(htlc_tx) = self.onchain_tx_handler.get_fully_signed_htlc_tx(
33583360
&::bitcoin::OutPoint { txid, vout }, &preimage) {
33593361
holder_transactions.push(htlc_tx);
33603362
}

‎lightning/src/chain/onchaintx.rs‎

Lines changed: 57 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,13 @@ use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature};
2323
use bitcoin::secp256k1;
2424

2525
use crate::chain::chaininterface::compute_feerate_sat_per_1000_weight;
26-
use crate::sign::{ChannelSigner, EntropySource, SignerProvider};
26+
use crate::sign::{ChannelDerivationParameters, HTLCDescriptor, ChannelSigner, EntropySource, SignerProvider, WriteableEcdsaChannelSigner};
2727
use crate::ln::msgs::DecodeError;
2828
use crate::ln::PaymentPreimage;
2929
use crate::ln::chan_utils::{self, ChannelTransactionParameters, HTLCOutputInCommitment, HolderCommitmentTransaction};
3030
use crate::chain::ClaimId;
3131
use crate::chain::chaininterface::{ConfirmationTarget, FeeEstimator, BroadcasterInterface, LowerBoundedFeeEstimator};
3232
use crate::chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER};
33-
use crate::sign::WriteableEcdsaChannelSigner;
3433
use crate::chain::package::{PackageSolvingData, PackageTemplate};
3534
use crate::util::logger::Logger;
3635
use crate::util::ser::{Readable, ReadableArgs, MaybeReadable, UpgradableRequired, Writer, Writeable, VecWriter};
@@ -215,14 +214,11 @@ pub(crate) enum OnchainClaim {
215214
/// do RBF bumping if possible.
216215
#[derive(Clone)]
217216
pub struct OnchainTxHandler<ChannelSigner: WriteableEcdsaChannelSigner> {
217+
channel_value_satoshis: u64,
218+
channel_keys_id: [u8; 32],
218219
destination_script: Script,
219220
holder_commitment: HolderCommitmentTransaction,
220-
// holder_htlc_sigs and prev_holder_htlc_sigs are in the order as they appear in the commitment
221-
// transaction outputs (hence the Option<>s inside the Vec). The first usize is the index in
222-
// the set of HTLCs in the HolderCommitmentTransaction.
223-
holder_htlc_sigs: Option<Vec<Option<(usize, Signature)>>>,
224221
prev_holder_commitment: Option<HolderCommitmentTransaction>,
225-
prev_holder_htlc_sigs: Option<Vec<Option<(usize, Signature)>>>,
226222

227223
pub(super) signer: ChannelSigner,
228224
pub(crate) channel_transaction_parameters: ChannelTransactionParameters,
@@ -276,11 +272,11 @@ pub struct OnchainTxHandler<ChannelSigner: WriteableEcdsaChannelSigner> {
276272
impl<ChannelSigner: WriteableEcdsaChannelSigner> PartialEq for OnchainTxHandler<ChannelSigner> {
277273
fn eq(&self, other: &Self) -> bool {
278274
// `signer`, `secp_ctx`, and `pending_claim_events` are excluded on purpose.
279-
self.destination_script == other.destination_script &&
275+
self.channel_value_satoshis == other.channel_value_satoshis &&
276+
self.channel_keys_id == other.channel_keys_id &&
277+
self.destination_script == other.destination_script &&
280278
self.holder_commitment == other.holder_commitment &&
281-
self.holder_htlc_sigs == other.holder_htlc_sigs &&
282279
self.prev_holder_commitment == other.prev_holder_commitment &&
283-
self.prev_holder_htlc_sigs == other.prev_holder_htlc_sigs &&
284280
self.channel_transaction_parameters == other.channel_transaction_parameters &&
285281
self.pending_claim_requests == other.pending_claim_requests &&
286282
self.claimable_outpoints == other.claimable_outpoints &&
@@ -298,9 +294,9 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
298294

299295
self.destination_script.write(writer)?;
300296
self.holder_commitment.write(writer)?;
301-
self.holder_htlc_sigs.write(writer)?;
297+
None::<Option<Vec<Option<(usize, Signature)>>>>.write(writer)?; // holder_htlc_sigs
302298
self.prev_holder_commitment.write(writer)?;
303-
self.prev_holder_htlc_sigs.write(writer)?;
299+
None::<Option<Vec<Option<(usize, Signature)>>>>.write(writer)?; // prev_holder_htlc_sigs
304300

305301
self.channel_transaction_parameters.write(writer)?;
306302

@@ -355,9 +351,9 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
355351
let destination_script = Readable::read(reader)?;
356352

357353
let holder_commitment = Readable::read(reader)?;
358-
let holder_htlc_sigs = Readable::read(reader)?;
354+
let _holder_htlc_sigs: Option<Vec<Option<(usize, Signature)>>> = Readable::read(reader)?;
359355
let prev_holder_commitment = Readable::read(reader)?;
360-
let prev_holder_htlc_sigs = Readable::read(reader)?;
356+
let _prev_holder_htlc_sigs: Option<Vec<Option<(usize, Signature)>>> = Readable::read(reader)?;
361357

362358
let channel_parameters = Readable::read(reader)?;
363359

@@ -418,11 +414,11 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
418414
secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes());
419415

420416
Ok(OnchainTxHandler {
417+
channel_value_satoshis,
418+
channel_keys_id,
421419
destination_script,
422420
holder_commitment,
423-
holder_htlc_sigs,
424421
prev_holder_commitment,
425-
prev_holder_htlc_sigs,
426422
signer,
427423
channel_transaction_parameters: channel_parameters,
428424
claimable_outpoints,
@@ -436,13 +432,17 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
436432
}
437433

438434
impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
439-
pub(crate) fn new(destination_script: Script, signer: ChannelSigner, channel_parameters: ChannelTransactionParameters, holder_commitment: HolderCommitmentTransaction, secp_ctx: Secp256k1<secp256k1::All>) -> Self {
435+
pub(crate) fn new(
436+
channel_value_satoshis: u64, channel_keys_id: [u8; 32], destination_script: Script,
437+
signer: ChannelSigner, channel_parameters: ChannelTransactionParameters,
438+
holder_commitment: HolderCommitmentTransaction, secp_ctx: Secp256k1<secp256k1::All>
439+
) -> Self {
440440
OnchainTxHandler {
441+
channel_value_satoshis,
442+
channel_keys_id,
441443
destination_script,
442444
holder_commitment,
443-
holder_htlc_sigs: None,
444445
prev_holder_commitment: None,
445-
prev_holder_htlc_sigs: None,
446446
signer,
447447
channel_transaction_parameters: channel_parameters,
448448
pending_claim_requests: HashMap::new(),
@@ -1088,39 +1088,6 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
10881088

10891089
pub(crate) fn provide_latest_holder_tx(&mut self, tx: HolderCommitmentTransaction) {
10901090
self.prev_holder_commitment = Some(replace(&mut self.holder_commitment, tx));
1091-
self.holder_htlc_sigs = None;
1092-
}
1093-
1094-
// Normally holder HTLCs are signed at the same time as the holder commitment tx. However,
1095-
// in some configurations, the holder commitment tx has been signed and broadcast by a
1096-
// ChannelMonitor replica, so we handle that case here.
1097-
fn sign_latest_holder_htlcs(&mut self) {
1098-
if self.holder_htlc_sigs.is_none() {
1099-
let (_sig, sigs) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment");
1100-
self.holder_htlc_sigs = Some(Self::extract_holder_sigs(&self.holder_commitment, sigs));
1101-
}
1102-
}
1103-
1104-
// Normally only the latest commitment tx and HTLCs need to be signed. However, in some
1105-
// configurations we may have updated our holder commitment but a replica of the ChannelMonitor
1106-
// broadcast the previous one before we sync with it. We handle that case here.
1107-
fn sign_prev_holder_htlcs(&mut self) {
1108-
if self.prev_holder_htlc_sigs.is_none() {
1109-
if let Some(ref holder_commitment) = self.prev_holder_commitment {
1110-
let (_sig, sigs) = self.signer.sign_holder_commitment_and_htlcs(holder_commitment, &self.secp_ctx).expect("sign previous holder commitment");
1111-
self.prev_holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs));
1112-
}
1113-
}
1114-
}
1115-
1116-
fn extract_holder_sigs(holder_commitment: &HolderCommitmentTransaction, sigs: Vec<Signature>) -> Vec<Option<(usize, Signature)>> {
1117-
let mut ret = Vec::new();
1118-
for (htlc_idx, (holder_sig, htlc)) in sigs.iter().zip(holder_commitment.htlcs().iter()).enumerate() {
1119-
let tx_idx = htlc.transaction_output_index.unwrap();
1120-
if ret.len() <= tx_idx as usize { ret.resize(tx_idx as usize + 1, None); }
1121-
ret[tx_idx as usize] = Some((htlc_idx, holder_sig.clone()));
1122-
}
1123-
ret
11241091
}
11251092

11261093
pub(crate) fn get_unsigned_holder_commitment_tx(&self) -> &Transaction {
@@ -1132,48 +1099,54 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
11321099
// before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing
11331100
// to monitor before.
11341101
pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {
1135-
let (sig, htlc_sigs) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment");
1136-
self.holder_htlc_sigs = Some(Self::extract_holder_sigs(&self.holder_commitment, htlc_sigs));
1102+
let sig = self.signer.sign_holder_commitment(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment");
11371103
self.holder_commitment.add_holder_sig(funding_redeemscript, sig)
11381104
}
11391105

11401106
#[cfg(any(test, feature="unsafe_revoked_tx_signing"))]
11411107
pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {
1142-
let (sig, htlc_sigs) = self.signer.unsafe_sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment");
1143-
self.holder_htlc_sigs = Some(Self::extract_holder_sigs(&self.holder_commitment, htlc_sigs));
1108+
let sig = self.signer.unsafe_sign_holder_commitment(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment");
11441109
self.holder_commitment.add_holder_sig(funding_redeemscript, sig)
11451110
}
11461111

11471112
pub(crate) fn get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option<PaymentPreimage>) -> Option<Transaction> {
1148-
let mut htlc_tx = None;
1149-
let commitment_txid = self.holder_commitment.trust().txid();
1150-
// Check if the HTLC spends from the current holder commitment
1151-
if commitment_txid == outp.txid {
1152-
self.sign_latest_holder_htlcs();
1153-
if let &Some(ref htlc_sigs) = &self.holder_htlc_sigs {
1154-
let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap();
1155-
let trusted_tx = self.holder_commitment.trust();
1156-
let counterparty_htlc_sig = self.holder_commitment.counterparty_htlc_sigs[*htlc_idx];
1157-
htlc_tx = Some(trusted_tx
1158-
.get_signed_htlc_tx(&self.channel_transaction_parameters.as_holder_broadcastable(), *htlc_idx, &counterparty_htlc_sig, htlc_sig, preimage));
1159-
}
1160-
}
1161-
// If the HTLC doesn't spend the current holder commitment, check if it spends the previous one
1162-
if htlc_tx.is_none() && self.prev_holder_commitment.is_some() {
1163-
let commitment_txid = self.prev_holder_commitment.as_ref().unwrap().trust().txid();
1164-
if commitment_txid == outp.txid {
1165-
self.sign_prev_holder_htlcs();
1166-
if let &Some(ref htlc_sigs) = &self.prev_holder_htlc_sigs {
1167-
let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap();
1168-
let holder_commitment = self.prev_holder_commitment.as_ref().unwrap();
1169-
let trusted_tx = holder_commitment.trust();
1170-
let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[*htlc_idx];
1171-
htlc_tx = Some(trusted_tx
1172-
.get_signed_htlc_tx(&self.channel_transaction_parameters.as_holder_broadcastable(), *htlc_idx, &counterparty_htlc_sig, htlc_sig, preimage));
1173-
}
1113+
let get_signed_htlc_tx = |holder_commitment: &HolderCommitmentTransaction| {
1114+
let trusted_tx = holder_commitment.trust();
1115+
if trusted_tx.txid() != outp.txid {
1116+
return None;
11741117
}
1175-
}
1176-
htlc_tx
1118+
let (htlc_idx, htlc) = trusted_tx.htlcs().iter().enumerate()
1119+
.find(|(_, htlc)| htlc.transaction_output_index.unwrap() == outp.vout)
1120+
.unwrap();
1121+
let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[htlc_idx];
1122+
let mut htlc_tx = trusted_tx.build_unsigned_htlc_tx(
1123+
&self.channel_transaction_parameters.as_holder_broadcastable(), htlc_idx, preimage,
1124+
);
1125+
1126+
let htlc_descriptor = HTLCDescriptor {
1127+
channel_derivation_parameters: ChannelDerivationParameters {
1128+
value_satoshis: self.channel_value_satoshis,
1129+
keys_id: self.channel_keys_id,
1130+
transaction_parameters: self.channel_transaction_parameters.clone(),
1131+
},
1132+
commitment_txid: trusted_tx.txid(),
1133+
per_commitment_number: trusted_tx.commitment_number(),
1134+
per_commitment_point: trusted_tx.per_commitment_point(),
1135+
feerate_per_kw: trusted_tx.feerate_per_kw(),
1136+
htlc: htlc.clone(),
1137+
preimage: preimage.clone(),
1138+
counterparty_sig: counterparty_htlc_sig.clone(),
1139+
};
1140+
let htlc_sig = self.signer.sign_holder_htlc_transaction(&htlc_tx, 0, &htlc_descriptor, &self.secp_ctx).unwrap();
1141+
htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness(
1142+
htlc_idx, &counterparty_htlc_sig, &htlc_sig, preimage,
1143+
);
1144+
Some(htlc_tx)
1145+
};
1146+
1147+
// Check if the HTLC spends from the current holder commitment first, or the previous.
1148+
get_signed_htlc_tx(&self.holder_commitment)
1149+
.or_else(|| self.prev_holder_commitment.as_ref().and_then(|prev_holder_commitment| get_signed_htlc_tx(prev_holder_commitment)))
11771150
}
11781151

11791152
pub(crate) fn generate_external_htlc_claim(
@@ -1209,18 +1182,4 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
12091182
pub(crate) fn channel_type_features(&self) -> &ChannelTypeFeatures {
12101183
&self.channel_transaction_parameters.channel_type_features
12111184
}
1212-
1213-
#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
1214-
pub(crate) fn unsafe_get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option<PaymentPreimage>) -> Option<Transaction> {
1215-
let latest_had_sigs = self.holder_htlc_sigs.is_some();
1216-
let prev_had_sigs = self.prev_holder_htlc_sigs.is_some();
1217-
let ret = self.get_fully_signed_htlc_tx(outp, preimage);
1218-
if !latest_had_sigs {
1219-
self.holder_htlc_sigs = None;
1220-
}
1221-
if !prev_had_sigs {
1222-
self.prev_holder_htlc_sigs = None;
1223-
}
1224-
ret
1225-
}
12261185
}

0 commit comments

Comments
 (0)