Skip to content

Commit 690ad18

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 72c42ee commit 690ad18

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.
@@ -118,7 +124,7 @@ impl HTLCDescriptor {
118124
/// Returns the delayed output created as a result of spending the HTLC output in the commitment
119125
/// transaction.
120126
pub fn tx_output<C: secp256k1::Signing + secp256k1::Verification>(&self, secp: &Secp256k1<C>) -> TxOut {
121-
let channel_params = self.channel_parameters.as_holder_broadcastable();
127+
let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable();
122128
let broadcaster_keys = channel_params.broadcaster_pubkeys();
123129
let counterparty_keys = channel_params.countersignatory_pubkeys();
124130
let broadcaster_delayed_key = chan_utils::derive_public_key(
@@ -135,7 +141,7 @@ impl HTLCDescriptor {
135141

136142
/// Returns the witness script of the HTLC output in the commitment transaction.
137143
pub fn witness_script<C: secp256k1::Signing + secp256k1::Verification>(&self, secp: &Secp256k1<C>) -> Script {
138-
let channel_params = self.channel_parameters.as_holder_broadcastable();
144+
let channel_params = self.channel_derivation_parameters.transaction_parameters.as_holder_broadcastable();
139145
let broadcaster_keys = channel_params.broadcaster_pubkeys();
140146
let counterparty_keys = channel_params.countersignatory_pubkeys();
141147
let broadcaster_htlc_key = chan_utils::derive_public_key(
@@ -160,6 +166,19 @@ impl HTLCDescriptor {
160166
signature, &self.counterparty_sig, &self.preimage, witness_script, &ChannelTypeFeatures::anchors_zero_htlc_fee_and_dependencies() /* opt_anchors */
161167
)
162168
}
169+
170+
/// Derives the channel signer required to sign the HTLC input.
171+
pub fn derive_channel_signer<SP: Deref>(&self, signer_provider: &SP) -> <SP::Target as SignerProvider>::Signer
172+
where
173+
SP::Target: SignerProvider
174+
{
175+
let mut signer = signer_provider.derive_channel_signer(
176+
self.channel_derivation_parameters.value_satoshis,
177+
self.channel_derivation_parameters.keys_id,
178+
);
179+
signer.provide_channel_parameters(&self.channel_derivation_parameters.transaction_parameters);
180+
signer
181+
}
163182
}
164183

165184
/// Represents the different types of transactions, originating from LDK, to be bumped.
@@ -178,12 +197,11 @@ pub enum BumpTransactionEvent {
178197
/// broadcast first, as the child anchor transaction depends on it.
179198
///
180199
/// The consumer should be able to sign for any of the additional inputs included within the
181-
/// child anchor transaction. To sign its anchor input, an [`InMemorySigner`] should be
182-
/// re-derived through [`KeysManager::derive_channel_keys`] with the help of
183-
/// [`AnchorDescriptor::channel_keys_id`] and [`AnchorDescriptor::channel_value_satoshis`]. The
184-
/// anchor input signature can be computed with [`EcdsaChannelSigner::sign_holder_anchor_input`],
185-
/// which can then be provided to [`build_anchor_input_witness`] along with the `funding_pubkey`
186-
/// to obtain the full witness required to spend.
200+
/// child anchor transaction. To sign its anchor input, an [`EcdsaChannelSigner`] should be
201+
/// re-derived through [`AnchorDescriptor::derive_channel_signer`]. The anchor input signature
202+
/// can be computed with [`EcdsaChannelSigner::sign_holder_anchor_input`], which can then be
203+
/// provided to [`build_anchor_input_witness`] along with the `funding_pubkey` to obtain the
204+
/// full witness required to spend.
187205
///
188206
/// It is possible to receive more than one instance of this event if a valid child anchor
189207
/// transaction is never broadcast or is but not with a sufficient fee to be mined. Care should
@@ -202,8 +220,7 @@ pub enum BumpTransactionEvent {
202220
/// an empty `pending_htlcs`), confirmation of the commitment transaction can be considered to
203221
/// be not urgent.
204222
///
205-
/// [`InMemorySigner`]: crate::sign::InMemorySigner
206-
/// [`KeysManager::derive_channel_keys`]: crate::sign::KeysManager::derive_channel_keys
223+
/// [`EcdsaChannelSigner`]: crate::sign::EcdsaChannelSigner
207224
/// [`EcdsaChannelSigner::sign_holder_anchor_input`]: crate::sign::EcdsaChannelSigner::sign_holder_anchor_input
208225
/// [`build_anchor_input_witness`]: crate::ln::chan_utils::build_anchor_input_witness
209226
ChannelClose {
@@ -241,11 +258,11 @@ pub enum BumpTransactionEvent {
241258
/// broadcast by the consumer of the event.
242259
///
243260
/// The consumer should be able to sign for any of the non-HTLC inputs added to the resulting
244-
/// HTLC transaction. To sign HTLC inputs, an [`InMemorySigner`] should be re-derived through
245-
/// [`KeysManager::derive_channel_keys`] with the help of `channel_keys_id` and
246-
/// `channel_value_satoshis`. Each HTLC input's signature can be computed with
247-
/// [`EcdsaChannelSigner::sign_holder_htlc_transaction`], which can then be provided to
248-
/// [`HTLCDescriptor::tx_input_witness`] to obtain the fully signed witness required to spend.
261+
/// HTLC transaction. To sign HTLC inputs, an [`EcdsaChannelSigner`] should be re-derived
262+
/// through [`HTLCDescriptor::derive_channel_signer`]. Each HTLC input's signature can be
263+
/// computed with [`EcdsaChannelSigner::sign_holder_htlc_transaction`], which can then be
264+
/// provided to [`HTLCDescriptor::tx_input_witness`] to obtain the fully signed witness required
265+
/// to spend.
249266
///
250267
/// It is possible to receive more than one instance of this event if a valid HTLC transaction
251268
/// is never broadcast or is but not with a sufficient fee to be mined. Care should be taken by
@@ -257,8 +274,7 @@ pub enum BumpTransactionEvent {
257274
/// longer able to commit external confirmed funds to the HTLC transaction or the fee committed
258275
/// to the HTLC transaction is greater in value than the HTLCs being claimed.
259276
///
260-
/// [`InMemorySigner`]: crate::sign::InMemorySigner
261-
/// [`KeysManager::derive_channel_keys`]: crate::sign::KeysManager::derive_channel_keys
277+
/// [`EcdsaChannelSigner`]: crate::sign::EcdsaChannelSigner
262278
/// [`EcdsaChannelSigner::sign_holder_htlc_transaction`]: crate::sign::EcdsaChannelSigner::sign_holder_htlc_transaction
263279
/// [`HTLCDescriptor::tx_input_witness`]: HTLCDescriptor::tx_input_witness
264280
HTLCResolution {
@@ -670,9 +686,7 @@ where
670686
debug_assert_eq!(anchor_tx.output.len(), 1);
671687

672688
self.utxo_source.sign_tx(&mut anchor_tx)?;
673-
let signer = self.signer_provider.derive_channel_signer(
674-
anchor_descriptor.channel_value_satoshis, anchor_descriptor.channel_keys_id,
675-
);
689+
let signer = anchor_descriptor.derive_channel_signer(&self.signer_provider);
676690
let anchor_sig = signer.sign_holder_anchor_input(&anchor_tx, 0, &self.secp)?;
677691
anchor_tx.input[0].witness =
678692
chan_utils::build_anchor_input_witness(&signer.pubkeys().funding_pubkey, &anchor_sig);
@@ -686,25 +700,15 @@ where
686700
fn build_htlc_tx(
687701
&self, claim_id: ClaimId, target_feerate_sat_per_1000_weight: u32,
688702
htlc_descriptors: &[HTLCDescriptor], tx_lock_time: PackedLockTime,
689-
) -> Result<(Transaction, HashMap<[u8; 32], <SP::Target as SignerProvider>::Signer>), ()> {
703+
) -> Result<Transaction, ()> {
690704
let mut tx = Transaction {
691705
version: 2,
692706
lock_time: tx_lock_time,
693707
input: vec![],
694708
output: vec![],
695709
};
696-
// Unfortunately, we need to derive the signer for each HTLC ahead of time to obtain its
697-
// input.
698-
let mut signers = HashMap::new();
699710
let mut must_spend = Vec::with_capacity(htlc_descriptors.len());
700711
for htlc_descriptor in htlc_descriptors {
701-
signers.entry(htlc_descriptor.channel_keys_id)
702-
.or_insert_with(||
703-
self.signer_provider.derive_channel_signer(
704-
htlc_descriptor.channel_value_satoshis, htlc_descriptor.channel_keys_id,
705-
)
706-
);
707-
708712
let htlc_input = htlc_descriptor.unsigned_tx_input();
709713
must_spend.push(Input {
710714
outpoint: htlc_input.previous_output.clone(),
@@ -723,7 +727,7 @@ where
723727
claim_id, &must_spend, &tx.output, target_feerate_sat_per_1000_weight,
724728
)?;
725729
self.process_coin_selection(&mut tx, coin_selection);
726-
Ok((tx, signers))
730+
Ok(tx)
727731
}
728732

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

739743
self.utxo_source.sign_tx(&mut htlc_tx)?;
744+
let mut signers = BTreeMap::new();
740745
for (idx, htlc_descriptor) in htlc_descriptors.iter().enumerate() {
741-
let signer = signers.get(&htlc_descriptor.channel_keys_id).unwrap();
742-
let htlc_sig = signer.sign_holder_htlc_transaction(
743-
&htlc_tx, idx, htlc_descriptor, &self.secp
744-
)?;
746+
let signer = signers.entry(htlc_descriptor.channel_derivation_parameters.keys_id)
747+
.or_insert_with(|| htlc_descriptor.derive_channel_signer(&self.signer_provider));
748+
let htlc_sig = signer.sign_holder_htlc_transaction(&htlc_tx, idx, htlc_descriptor, &self.secp)?;
745749
let witness_script = htlc_descriptor.witness_script(&self.secp);
746750
htlc_tx.input[idx].witness = htlc_descriptor.tx_input_witness(&htlc_sig, &witness_script);
747751
}

‎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)