Skip to content

Commit df84dd3

Browse files
devrandomdevrandom
committed
ChannelKeys - separate commitment revocation from getting the per-commitment point
1 parent bb369b5 commit df84dd3

File tree

4 files changed

+37
-33
lines changed

4 files changed

+37
-33
lines changed

‎lightning/src/chain/keysinterface.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,12 @@ impl Readable for SpendableOutputDescriptor {
195195
// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
196196
// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
197197
pub trait ChannelKeys : Send+Clone {
198-
/// Gets the commitment seed for a specific commitment number
198+
/// Gets the per-commitment point for a specific commitment number
199199
/// Note that the commitment number starts at (1 << 48) - 1 and counts backwards
200-
fn commitment_secret(&self, idx: u64) -> [u8; 32];
200+
fn get_per_commitment_point<T: secp256k1::Signing + secp256k1::Verification>(&self, idx: u64, secp_ctx: &Secp256k1<T>) -> PublicKey;
201+
/// Gets the commitment seed for a specific commitment number, thereby revoking the commitment
202+
/// Note that the commitment number starts at (1 << 48) - 1 and counts backwards
203+
fn revoke_commitment(&self, idx: u64) -> [u8; 32];
201204
/// Gets the local channel public keys and basepoints
202205
fn pubkeys(&self) -> &ChannelPublicKeys;
203206
/// Gets arbitrary identifiers describing the set of keys which are provided back to you in
@@ -405,7 +408,12 @@ impl InMemoryChannelKeys {
405408
}
406409

407410
impl ChannelKeys for InMemoryChannelKeys {
408-
fn commitment_secret(&self, idx: u64) -> [u8; 32] {
411+
fn get_per_commitment_point<T: secp256k1::Signing + secp256k1::Verification>(&self, idx: u64, secp_ctx: &Secp256k1<T>) -> PublicKey {
412+
let commitment_secret = SecretKey::from_slice(&chan_utils::build_commitment_secret(&self.commitment_seed, idx)).unwrap();
413+
PublicKey::from_secret_key(secp_ctx, &commitment_secret)
414+
}
415+
416+
fn revoke_commitment(&self, idx: u64) -> [u8; 32] {
409417
chan_utils::build_commitment_secret(&self.commitment_seed, idx)
410418
}
411419

‎lightning/src/ln/channel.rs

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -780,13 +780,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
780780
Ok(chan)
781781
}
782782

783-
// Utilities to derive keys:
784-
785-
fn build_local_commitment_secret(&self, idx: u64) -> SecretKey {
786-
let res = self.local_keys.commitment_secret(idx);
787-
SecretKey::from_slice(&res).unwrap()
788-
}
789-
790783
// Utilities to build transactions:
791784

792785
fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
@@ -1118,7 +1111,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
11181111
/// The result is a transaction which we can revoke ownership of (ie a "local" transaction)
11191112
/// TODO Some magic rust shit to compile-time check this?
11201113
fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
1121-
let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
1114+
let per_commitment_point = self.local_keys.get_per_commitment_point(commitment_number, &self.secp_ctx);
11221115
let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
11231116
let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
11241117
let their_pubkeys = self.their_pubkeys.as_ref().unwrap();
@@ -2020,8 +2013,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
20202013
}
20212014
}
20222015

2023-
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number - 1));
2024-
let per_commitment_secret = self.local_keys.commitment_secret(self.cur_local_commitment_transaction_number + 1);
2016+
let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number - 1, &self.secp_ctx);
2017+
let per_commitment_secret = self.local_keys.revoke_commitment(self.cur_local_commitment_transaction_number + 1);
20252018

20262019
// Update state now that we've passed all the can-fail calls...
20272020
let mut need_our_commitment = false;
@@ -2606,8 +2599,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
26062599
let funding_locked = if self.monitor_pending_funding_locked {
26072600
assert!(!self.channel_outbound, "Funding transaction broadcast without FundingBroadcastSafe!");
26082601
self.monitor_pending_funding_locked = false;
2609-
let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
2610-
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
2602+
let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
26112603
Some(msgs::FundingLocked {
26122604
channel_id: self.channel_id(),
26132605
next_per_commitment_point: next_per_commitment_point,
@@ -2659,8 +2651,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
26592651
}
26602652

26612653
fn get_last_revoke_and_ack(&self) -> msgs::RevokeAndACK {
2662-
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(self.cur_local_commitment_transaction_number));
2663-
let per_commitment_secret = self.local_keys.commitment_secret(self.cur_local_commitment_transaction_number + 2);
2654+
let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
2655+
let per_commitment_secret = self.local_keys.revoke_commitment(self.cur_local_commitment_transaction_number + 2);
26642656
msgs::RevokeAndACK {
26652657
channel_id: self.channel_id,
26662658
per_commitment_secret,
@@ -2743,7 +2735,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
27432735
if msg.next_remote_commitment_number > 0 {
27442736
match msg.data_loss_protect {
27452737
OptionalField::Present(ref data_loss) => {
2746-
if self.local_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1) != data_loss.your_last_per_commitment_secret {
2738+
let expected_point = self.local_keys.get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1, &self.secp_ctx);
2739+
let given_secret = SecretKey::from_slice(&data_loss.your_last_per_commitment_secret)
2740+
.map_err(|_| ChannelError::Close("Peer sent a garbage channel_reestablish with unparseable secret key"))?;
2741+
if expected_point != PublicKey::from_secret_key(&self.secp_ctx, &given_secret) {
27472742
return Err(ChannelError::Close("Peer sent a garbage channel_reestablish with secret key not matching the commitment height provided"));
27482743
}
27492744
if msg.next_remote_commitment_number > INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number {
@@ -2779,8 +2774,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
27792774
}
27802775

27812776
// We have OurFundingLocked set!
2782-
let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
2783-
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
2777+
let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
27842778
return Ok((Some(msgs::FundingLocked {
27852779
channel_id: self.channel_id(),
27862780
next_per_commitment_point: next_per_commitment_point,
@@ -2810,8 +2804,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
28102804

28112805
let resend_funding_locked = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number == 1 {
28122806
// We should never have to worry about MonitorUpdateFailed resending FundingLocked
2813-
let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
2814-
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
2807+
let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
28152808
Some(msgs::FundingLocked {
28162809
channel_id: self.channel_id(),
28172810
next_per_commitment_point: next_per_commitment_point,
@@ -3397,8 +3390,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
33973390
//a protocol oversight, but I assume I'm just missing something.
33983391
if need_commitment_update {
33993392
if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
3400-
let next_per_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
3401-
let next_per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &next_per_commitment_secret);
3393+
let next_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
34023394
return Ok((Some(msgs::FundingLocked {
34033395
channel_id: self.channel_id,
34043396
next_per_commitment_point: next_per_commitment_point,
@@ -3449,7 +3441,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
34493441
panic!("Tried to send an open_channel for a channel that has already advanced");
34503442
}
34513443

3452-
let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
3444+
let first_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
34533445
let local_keys = self.local_keys.pubkeys();
34543446

34553447
msgs::OpenChannel {
@@ -3469,7 +3461,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
34693461
payment_point: local_keys.payment_point,
34703462
delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
34713463
htlc_basepoint: local_keys.htlc_basepoint,
3472-
first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
3464+
first_per_commitment_point,
34733465
channel_flags: if self.config.announced_channel {1} else {0},
34743466
shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
34753467
}
@@ -3486,7 +3478,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
34863478
panic!("Tried to send an accept_channel for a channel that has already advanced");
34873479
}
34883480

3489-
let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
3481+
let first_per_commitment_point = self.local_keys.get_per_commitment_point(self.cur_local_commitment_transaction_number, &self.secp_ctx);
34903482
let local_keys = self.local_keys.pubkeys();
34913483

34923484
msgs::AcceptChannel {
@@ -3503,7 +3495,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
35033495
payment_point: local_keys.payment_point,
35043496
delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
35053497
htlc_basepoint: local_keys.htlc_basepoint,
3506-
first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
3498+
first_per_commitment_point,
35073499
shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
35083500
}
35093501
}

‎lightning/src/ln/functional_tests.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,15 +1609,15 @@ fn test_fee_spike_violation_fails_htlc() {
16091609
let chan_keys = local_chan.get_local_keys();
16101610
let pubkeys = chan_keys.pubkeys();
16111611
(pubkeys.revocation_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
1612-
chan_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER), chan_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER - 2))
1612+
chan_keys.revoke_commitment(INITIAL_COMMITMENT_NUMBER), chan_keys.revoke_commitment(INITIAL_COMMITMENT_NUMBER - 2))
16131613
};
16141614
let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_payment_point, remote_secret1) = {
16151615
let chan_lock = nodes[1].node.channel_state.lock().unwrap();
16161616
let remote_chan = chan_lock.by_id.get(&chan.2).unwrap();
16171617
let chan_keys = remote_chan.get_local_keys();
16181618
let pubkeys = chan_keys.pubkeys();
16191619
(pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
1620-
chan_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER - 1))
1620+
chan_keys.revoke_commitment(INITIAL_COMMITMENT_NUMBER - 1))
16211621
};
16221622

16231623
// Assemble the set of keys we can use for signatures for our commitment_signed message.
@@ -8129,8 +8129,8 @@ fn test_counterparty_raa_skip_no_crash() {
81298129
let local_keys = &guard.by_id.get_mut(&channel_id).unwrap().local_keys;
81308130
const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
81318131
let next_per_commitment_point = PublicKey::from_secret_key(&Secp256k1::new(),
8132-
&SecretKey::from_slice(&local_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER - 2)).unwrap());
8133-
let per_commitment_secret = local_keys.commitment_secret(INITIAL_COMMITMENT_NUMBER);
8132+
&SecretKey::from_slice(&local_keys.revoke_commitment(INITIAL_COMMITMENT_NUMBER - 2)).unwrap());
8133+
let per_commitment_secret = local_keys.revoke_commitment(INITIAL_COMMITMENT_NUMBER);
81348134

81358135
nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(),
81368136
&msgs::RevokeAndACK { channel_id, per_commitment_secret, next_per_commitment_point });

‎lightning/src/util/enforcing_trait_impls.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ impl EnforcingChannelKeys {
4848
}
4949

5050
impl ChannelKeys for EnforcingChannelKeys {
51-
fn commitment_secret(&self, idx: u64) -> [u8; 32] { self.inner.commitment_secret(idx) }
51+
fn get_per_commitment_point<T: secp256k1::Signing + secp256k1::Verification>(&self, idx: u64, secp_ctx: &Secp256k1<T>) -> PublicKey {
52+
self.inner.get_per_commitment_point(idx, secp_ctx)
53+
}
54+
55+
fn revoke_commitment(&self, idx: u64) -> [u8; 32] { self.inner.revoke_commitment(idx) }
5256
fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() }
5357
fn key_derivation_params(&self) -> (u64, u64) { self.inner.key_derivation_params() }
5458

0 commit comments

Comments
 (0)