Skip to content

Commit f5e28f4

Browse files
wpaulinowpaulino
committed
Provide missing post-derivation signer parameters
Users may expect these to be provided manually after derivation in the event they need to perform any enforcement prior to signing.
1 parent dfe4e7f commit f5e28f4

File tree

3 files changed

+85
-86
lines changed

3 files changed

+85
-86
lines changed

‎lightning/src/chain/channelmonitor.rs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ 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::{AnchorDescriptor, HTLCDescriptor, BumpTransactionEvent};
53+
use crate::events::bump_transaction::{ChannelDerivationParameters, AnchorDescriptor, HTLCDescriptor, BumpTransactionEvent};
5454

5555
use crate::prelude::*;
5656
use core::{cmp, mem};
@@ -2611,8 +2611,11 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
26112611
commitment_tx,
26122612
commitment_tx_fee_satoshis,
26132613
anchor_descriptor: AnchorDescriptor {
2614-
channel_keys_id: self.channel_keys_id,
2615-
channel_value_satoshis: self.channel_value_satoshis,
2614+
channel_derivation_parameters: ChannelDerivationParameters {
2615+
keys_id: self.channel_keys_id,
2616+
value_satoshis: self.channel_value_satoshis,
2617+
transaction_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(),
2618+
},
26162619
outpoint: BitcoinOutPoint {
26172620
txid: commitment_txid,
26182621
vout: anchor_output_idx,
@@ -2627,9 +2630,11 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
26272630
let mut htlc_descriptors = Vec::with_capacity(htlcs.len());
26282631
for htlc in htlcs {
26292632
htlc_descriptors.push(HTLCDescriptor {
2630-
channel_keys_id: self.channel_keys_id,
2631-
channel_value_satoshis: self.channel_value_satoshis,
2632-
channel_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(),
2633+
channel_derivation_parameters: ChannelDerivationParameters {
2634+
keys_id: self.channel_keys_id,
2635+
value_satoshis: self.channel_value_satoshis,
2636+
transaction_parameters: self.onchain_tx_handler.channel_transaction_parameters.clone(),
2637+
},
26332638
commitment_txid: htlc.commitment_txid,
26342639
per_commitment_number: htlc.per_commitment_number,
26352640
per_commitment_point: self.onchain_tx_handler.signer.get_per_commitment_point(

‎lightning/src/events/bump_transaction.rs

Lines changed: 69 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
//!
1212
//! [`Event`]: crate::events::Event
1313
14+
use alloc::collections::BTreeMap;
1415
use core::convert::TryInto;
1516
use core::ops::Deref;
1617

@@ -50,45 +51,50 @@ const fn fee_for_weight(feerate_sat_per_1000_weight: u32, weight: u64) -> u64 {
5051
((feerate_sat_per_1000_weight as u64 * weight) + 1000 - 1) / 1000
5152
}
5253

54+
/// The parameters required to derive a channel signer via [`SignerProvider`].
55+
#[derive(Clone, Debug, PartialEq, Eq)]
56+
pub struct ChannelDerivationParameters {
57+
/// The value in satoshis of the channel we're attempting to spend the anchor output of.
58+
pub value_satoshis: u64,
59+
/// The unique identifier to re-derive the signer for the associated channel.
60+
pub keys_id: [u8; 32],
61+
/// The necessary channel parameters that need to be provided to the re-derived signer through
62+
/// [`ChannelSigner::provide_channel_parameters`].
63+
///
64+
/// [`ChannelSigner::provide_channel_parameters`]: crate::sign::ChannelSigner::provide_channel_parameters
65+
pub transaction_parameters: ChannelTransactionParameters,
66+
}
67+
5368
/// A descriptor used to sign for a commitment transaction's anchor output.
5469
#[derive(Clone, Debug, PartialEq, Eq)]
5570
pub struct AnchorDescriptor {
56-
/// A unique identifier used along with `channel_value_satoshis` to re-derive the
57-
/// [`InMemorySigner`] required to sign `input`.
58-
///
59-
/// [`InMemorySigner`]: crate::sign::InMemorySigner
60-
pub channel_keys_id: [u8; 32],
61-
/// The value in satoshis of the channel we're attempting to spend the anchor output of. This is
62-
/// used along with `channel_keys_id` to re-derive the [`InMemorySigner`] required to sign
63-
/// `input`.
64-
///
65-
/// [`InMemorySigner`]: crate::sign::InMemorySigner
66-
pub channel_value_satoshis: u64,
71+
/// The parameters required to derive the signer for the anchor input.
72+
pub channel_derivation_parameters: ChannelDerivationParameters,
6773
/// The transaction input's outpoint corresponding to the commitment transaction's anchor
6874
/// output.
6975
pub outpoint: OutPoint,
7076
}
7177

78+
impl AnchorDescriptor {
79+
/// Derives the channel signer required to sign the anchor input.
80+
pub fn derive_channel_signer<SP: Deref>(&self, signer_provider: &SP) -> <SP::Target as SignerProvider>::Signer
81+
where
82+
SP::Target: SignerProvider
83+
{
84+
let mut signer = signer_provider.derive_channel_signer(
85+
self.channel_derivation_parameters.value_satoshis,
86+
self.channel_derivation_parameters.keys_id,
87+
);
88+
signer.provide_channel_parameters(&self.channel_derivation_parameters.transaction_parameters);
89+
signer
90+
}
91+
}
92+
7293
/// A descriptor used to sign for a commitment transaction's HTLC output.
7394
#[derive(Clone, Debug, PartialEq, Eq)]
7495
pub struct HTLCDescriptor {
75-
/// A unique identifier used along with `channel_value_satoshis` to re-derive the
76-
/// [`InMemorySigner`] required to sign `input`.
77-
///
78-
/// [`InMemorySigner`]: crate::sign::InMemorySigner
79-
pub channel_keys_id: [u8; 32],
80-
/// The value in satoshis of the channel we're attempting to spend the anchor output of. This is
81-
/// used along with `channel_keys_id` to re-derive the [`InMemorySigner`] required to sign
82-
/// `input`.
83-
///
84-
/// [`InMemorySigner`]: crate::sign::InMemorySigner
85-
pub channel_value_satoshis: u64,
86-
/// The necessary channel parameters that need to be provided to the re-derived
87-
/// [`InMemorySigner`] through [`ChannelSigner::provide_channel_parameters`].
88-
///
89-
/// [`InMemorySigner`]: crate::sign::InMemorySigner
90-
/// [`ChannelSigner::provide_channel_parameters`]: crate::sign::ChannelSigner::provide_channel_parameters
91-
pub channel_parameters: ChannelTransactionParameters,
96+
/// The parameters required to derive the signer for the HTLC input.
97+
pub channel_derivation_parameters: ChannelDerivationParameters,
9298
/// The txid of the commitment transaction in which the HTLC output lives.
9399
pub commitment_txid: Txid,
94100
/// The number of the commitment transaction in which the HTLC output lives.
@@ -115,7 +121,7 @@ impl HTLCDescriptor {
115121
/// Returns the delayed output created as a result of spending the HTLC output in the commitment
116122
/// transaction.
117123
pub fn tx_output<C: secp256k1::Signing + secp256k1::Verification>(&self, secp: &Secp256k1<C>) -> TxOut {
118-
let channel_params = self.channel_parameters.as_holder_broadcastable();
124+
let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable();
119125
let broadcaster_keys = channel_params.broadcaster_pubkeys();
120126
let counterparty_keys = channel_params.countersignatory_pubkeys();
121127
let broadcaster_delayed_key = chan_utils::derive_public_key(
@@ -132,7 +138,7 @@ impl HTLCDescriptor {
132138

133139
/// Returns the witness script of the HTLC output in the commitment transaction.
134140
pub fn witness_script<C: secp256k1::Signing + secp256k1::Verification>(&self, secp: &Secp256k1<C>) -> Script {
135-
let channel_params = self.channel_parameters.as_holder_broadcastable();
141+
let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable();
136142
let broadcaster_keys = channel_params.broadcaster_pubkeys();
137143
let counterparty_keys = channel_params.countersignatory_pubkeys();
138144
let broadcaster_htlc_key = chan_utils::derive_public_key(
@@ -157,6 +163,19 @@ impl HTLCDescriptor {
157163
signature, &self.counterparty_sig, &self.preimage, witness_script, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() /* opt_anchors */
158164
)
159165
}
166+
167+
/// Derives the channel signer required to sign the HTLC input.
168+
pub fn derive_channel_signer<SP: Deref>(&self, signer_provider: &SP) -> <SP::Target as SignerProvider>::Signer
169+
where
170+
SP::Target: SignerProvider
171+
{
172+
let mut signer = signer_provider.derive_channel_signer(
173+
self.channel_derivation_parameters.value_satoshis,
174+
self.channel_derivation_parameters.keys_id,
175+
);
176+
signer.provide_channel_parameters(&self.channel_derivation_parameters.transaction_parameters);
177+
signer
178+
}
160179
}
161180

162181
/// Represents the different types of transactions, originating from LDK, to be bumped.
@@ -175,12 +194,11 @@ pub enum BumpTransactionEvent {
175194
/// broadcast first, as the child anchor transaction depends on it.
176195
///
177196
/// The consumer should be able to sign for any of the additional inputs included within the
178-
/// child anchor transaction. To sign its anchor input, an [`InMemorySigner`] should be
179-
/// re-derived through [`KeysManager::derive_channel_keys`] with the help of
180-
/// [`AnchorDescriptor::channel_keys_id`] and [`AnchorDescriptor::channel_value_satoshis`]. The
181-
/// anchor input signature can be computed with [`EcdsaChannelSigner::sign_holder_anchor_input`],
182-
/// which can then be provided to [`build_anchor_input_witness`] along with the `funding_pubkey`
183-
/// to obtain the full witness required to spend.
197+
/// child anchor transaction. To sign its anchor input, an [`EcdsaChannelSigner`] should be
198+
/// re-derived through [`AnchorDescriptor::derive_channel_signer`]. The anchor input signature
199+
/// can be computed with [`EcdsaChannelSigner::sign_holder_anchor_input`], which can then be
200+
/// provided to [`build_anchor_input_witness`] along with the `funding_pubkey` to obtain the
201+
/// full witness required to spend.
184202
///
185203
/// It is possible to receive more than one instance of this event if a valid child anchor
186204
/// transaction is never broadcast or is but not with a sufficient fee to be mined. Care should
@@ -199,8 +217,7 @@ pub enum BumpTransactionEvent {
199217
/// an empty `pending_htlcs`), confirmation of the commitment transaction can be considered to
200218
/// be not urgent.
201219
///
202-
/// [`InMemorySigner`]: crate::sign::InMemorySigner
203-
/// [`KeysManager::derive_channel_keys`]: crate::sign::KeysManager::derive_channel_keys
220+
/// [`EcdsaChannelSigner`]: crate::sign::EcdsaChannelSigner
204221
/// [`EcdsaChannelSigner::sign_holder_anchor_input`]: crate::sign::EcdsaChannelSigner::sign_holder_anchor_input
205222
/// [`build_anchor_input_witness`]: crate::ln::chan_utils::build_anchor_input_witness
206223
ChannelClose {
@@ -238,11 +255,11 @@ pub enum BumpTransactionEvent {
238255
/// broadcast by the consumer of the event.
239256
///
240257
/// The consumer should be able to sign for any of the non-HTLC inputs added to the resulting
241-
/// HTLC transaction. To sign HTLC inputs, an [`InMemorySigner`] should be re-derived through
242-
/// [`KeysManager::derive_channel_keys`] with the help of `channel_keys_id` and
243-
/// `channel_value_satoshis`. Each HTLC input's signature can be computed with
244-
/// [`EcdsaChannelSigner::sign_holder_htlc_transaction`], which can then be provided to
245-
/// [`HTLCDescriptor::tx_input_witness`] to obtain the fully signed witness required to spend.
258+
/// HTLC transaction. To sign HTLC inputs, an [`EcdsaChannelSigner`] should be re-derived
259+
/// through [`HTLCDescriptor::derive_channel_signer`]. Each HTLC input's signature can be
260+
/// computed with [`EcdsaChannelSigner::sign_holder_htlc_transaction`], which can then be
261+
/// provided to [`HTLCDescriptor::tx_input_witness`] to obtain the fully signed witness required
262+
/// to spend.
246263
///
247264
/// It is possible to receive more than one instance of this event if a valid HTLC transaction
248265
/// is never broadcast or is but not with a sufficient fee to be mined. Care should be taken by
@@ -254,8 +271,7 @@ pub enum BumpTransactionEvent {
254271
/// longer able to commit external confirmed funds to the HTLC transaction or the fee committed
255272
/// to the HTLC transaction is greater in value than the HTLCs being claimed.
256273
///
257-
/// [`InMemorySigner`]: crate::sign::InMemorySigner
258-
/// [`KeysManager::derive_channel_keys`]: crate::sign::KeysManager::derive_channel_keys
274+
/// [`EcdsaChannelSigner`]: crate::sign::EcdsaChannelSigner
259275
/// [`EcdsaChannelSigner::sign_holder_htlc_transaction`]: crate::sign::EcdsaChannelSigner::sign_holder_htlc_transaction
260276
/// [`HTLCDescriptor::tx_input_witness`]: HTLCDescriptor::tx_input_witness
261277
HTLCResolution {
@@ -667,9 +683,7 @@ where
667683
debug_assert_eq!(anchor_tx.output.len(), 1);
668684

669685
self.utxo_source.sign_tx(&mut anchor_tx)?;
670-
let signer = self.signer_provider.derive_channel_signer(
671-
anchor_descriptor.channel_value_satoshis, anchor_descriptor.channel_keys_id,
672-
);
686+
let signer = anchor_descriptor.derive_channel_signer(&self.signer_provider);
673687
let anchor_sig = signer.sign_holder_anchor_input(&anchor_tx, 0, &self.secp)?;
674688
anchor_tx.input[0].witness =
675689
chan_utils::build_anchor_input_witness(&signer.pubkeys().funding_pubkey, &anchor_sig);
@@ -683,25 +697,15 @@ where
683697
fn build_htlc_tx(
684698
&self, claim_id: ClaimId, target_feerate_sat_per_1000_weight: u32,
685699
htlc_descriptors: &[HTLCDescriptor], tx_lock_time: PackedLockTime,
686-
) -> Result<(Transaction, HashMap<[u8; 32], <SP::Target as SignerProvider>::Signer>), ()> {
700+
) -> Result<Transaction, ()> {
687701
let mut tx = Transaction {
688702
version: 2,
689703
lock_time: tx_lock_time,
690704
input: vec![],
691705
output: vec![],
692706
};
693-
// Unfortunately, we need to derive the signer for each HTLC ahead of time to obtain its
694-
// input.
695-
let mut signers = HashMap::new();
696707
let mut must_spend = Vec::with_capacity(htlc_descriptors.len());
697708
for htlc_descriptor in htlc_descriptors {
698-
signers.entry(htlc_descriptor.channel_keys_id)
699-
.or_insert_with(||
700-
self.signer_provider.derive_channel_signer(
701-
htlc_descriptor.channel_value_satoshis, htlc_descriptor.channel_keys_id,
702-
)
703-
);
704-
705709
let htlc_input = htlc_descriptor.unsigned_tx_input();
706710
must_spend.push(Input {
707711
outpoint: htlc_input.previous_output.clone(),
@@ -720,7 +724,7 @@ where
720724
claim_id, &must_spend, &tx.output, target_feerate_sat_per_1000_weight,
721725
)?;
722726
self.process_coin_selection(&mut tx, coin_selection);
723-
Ok((tx, signers))
727+
Ok(tx)
724728
}
725729

726730
/// Handles a [`BumpTransactionEvent::HTLCResolution`] event variant by producing a
@@ -729,16 +733,16 @@ where
729733
&self, claim_id: ClaimId, target_feerate_sat_per_1000_weight: u32,
730734
htlc_descriptors: &[HTLCDescriptor], tx_lock_time: PackedLockTime,
731735
) -> Result<(), ()> {
732-
let (mut htlc_tx, signers) = self.build_htlc_tx(
736+
let mut htlc_tx = self.build_htlc_tx(
733737
claim_id, target_feerate_sat_per_1000_weight, htlc_descriptors, tx_lock_time,
734738
)?;
735739

736740
self.utxo_source.sign_tx(&mut htlc_tx)?;
741+
let mut signers = BTreeMap::new();
737742
for (idx, htlc_descriptor) in htlc_descriptors.iter().enumerate() {
738-
let signer = signers.get(&htlc_descriptor.channel_keys_id).unwrap();
739-
let htlc_sig = signer.sign_holder_htlc_transaction(
740-
&htlc_tx, idx, htlc_descriptor, &self.secp
741-
)?;
743+
let signer = signers.entry(htlc_descriptor.channel_derivation_parameters.keys_id)
744+
.or_insert_with(|| htlc_descriptor.derive_channel_signer(&self.signer_provider));
745+
let htlc_sig = signer.sign_holder_htlc_transaction(&htlc_tx, idx, htlc_descriptor, &self.secp)?;
742746
let witness_script = htlc_descriptor.witness_script(&self.secp);
743747
htlc_tx.input[idx].witness = htlc_descriptor.tx_input_witness(&htlc_sig, &witness_script);
744748
}

‎lightning/src/ln/monitor_tests.rs

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1778,10 +1778,8 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) {
17781778
// feerate for the test, we just want to make sure the feerates we receive from
17791779
// the events never decrease.
17801780
tx.input.push(descriptor.unsigned_tx_input());
1781-
let signer = nodes[0].keys_manager.derive_channel_keys(
1782-
descriptor.channel_value_satoshis, &descriptor.channel_keys_id,
1783-
);
17841781
tx.output.push(descriptor.tx_output(&secp));
1782+
let signer = descriptor.derive_channel_signer(&nodes[0].keys_manager);
17851783
let our_sig = signer.sign_holder_htlc_transaction(&mut tx, 0, &descriptor, &secp).unwrap();
17861784
let witness_script = descriptor.witness_script(&secp);
17871785
tx.input[0].witness = descriptor.tx_input_witness(&our_sig, &witness_script);
@@ -1907,9 +1905,7 @@ fn test_yield_anchors_events() {
19071905
script_pubkey: Script::new_op_return(&[]),
19081906
}],
19091907
};
1910-
let signer = nodes[0].keys_manager.derive_channel_keys(
1911-
anchor_descriptor.channel_value_satoshis, &anchor_descriptor.channel_keys_id,
1912-
);
1908+
let signer = anchor_descriptor.derive_channel_signer(&nodes[0].keys_manager);
19131909
let funding_sig = signer.sign_holder_anchor_input(&mut anchor_tx, 0, &secp).unwrap();
19141910
anchor_tx.input[0].witness = chan_utils::build_anchor_input_witness(
19151911
&signer.pubkeys().funding_pubkey, &funding_sig
@@ -1955,9 +1951,7 @@ fn test_yield_anchors_events() {
19551951
}
19561952
]
19571953
};
1958-
let signer = nodes[0].keys_manager.derive_channel_keys(
1959-
htlc_descriptor.channel_value_satoshis, &htlc_descriptor.channel_keys_id
1960-
);
1954+
let signer = htlc_descriptor.derive_channel_signer(&nodes[0].keys_manager);
19611955
let our_sig = signer.sign_holder_htlc_transaction(&mut htlc_tx, 0, htlc_descriptor, &secp).unwrap();
19621956
let witness_script = htlc_descriptor.witness_script(&secp);
19631957
htlc_tx.input[0].witness = htlc_descriptor.tx_input_witness(&our_sig, &witness_script);
@@ -2118,9 +2112,7 @@ fn test_anchors_aggregated_revoked_htlc_tx() {
21182112
previous_output: anchor_descriptor.outpoint,
21192113
..Default::default()
21202114
});
2121-
let signer = nodes[1].keys_manager.derive_channel_keys(
2122-
anchor_descriptor.channel_value_satoshis, &anchor_descriptor.channel_keys_id,
2123-
);
2115+
let signer = anchor_descriptor.derive_channel_signer(&nodes[1].keys_manager);
21242116
signers.push(signer);
21252117
},
21262118
_ => panic!("Unexpected event"),
@@ -2234,9 +2226,7 @@ fn test_anchors_aggregated_revoked_htlc_tx() {
22342226
}
22352227
for (idx, htlc_descriptor) in descriptors.into_iter().enumerate() {
22362228
let htlc_input_idx = idx + 1;
2237-
let signer = nodes[1].keys_manager.derive_channel_keys(
2238-
htlc_descriptor.channel_value_satoshis, &htlc_descriptor.channel_keys_id
2239-
);
2229+
let signer = htlc_descriptor.derive_channel_signer(&nodes[1].keys_manager);
22402230
let our_sig = signer.sign_holder_htlc_transaction(&htlc_tx, htlc_input_idx, &htlc_descriptor, &secp).unwrap();
22412231
let witness_script = htlc_descriptor.witness_script(&secp);
22422232
htlc_tx.input[htlc_input_idx].witness = htlc_descriptor.tx_input_witness(&our_sig, &witness_script);

0 commit comments

Comments
 (0)