Skip to content

[Key derivation V2] Attach a version byte to channel_keys_id #3887

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ use crate::ln::channelmanager::{HTLCSource, PaymentClaimDetails, SentHTLCId};
use crate::ln::msgs::DecodeError;
use crate::ln::types::ChannelId;
use crate::sign::{
ecdsa::EcdsaChannelSigner, ChannelDerivationParameters, DelayedPaymentOutputDescriptor,
EntropySource, HTLCDescriptor, SignerProvider, SpendableOutputDescriptor,
StaticPaymentOutputDescriptor,
ecdsa::EcdsaChannelSigner, ChannelDerivationParameters, ChannelKeysId,
DelayedPaymentOutputDescriptor, EntropySource, HTLCDescriptor, SignerProvider,
SpendableOutputDescriptor, StaticPaymentOutputDescriptor,
};
use crate::types::features::ChannelTypeFeatures;
use crate::types::payment::{PaymentHash, PaymentPreimage};
Expand Down Expand Up @@ -1036,7 +1036,7 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
counterparty_payment_script: ScriptBuf,
shutdown_script: Option<ScriptBuf>,

channel_keys_id: [u8; 32],
channel_keys_id: ChannelKeysId,
holder_revocation_basepoint: RevocationBasepoint,
channel_id: ChannelId,
first_confirmed_funding_txo: OutPoint,
Expand Down Expand Up @@ -1297,7 +1297,7 @@ impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
None => ScriptBuf::new().write(writer)?,
}

self.channel_keys_id.write(writer)?;
self.channel_keys_id.id.write(writer)?;
self.holder_revocation_basepoint.write(writer)?;
let funding_outpoint = self.get_funding_txo();
writer.write_all(&funding_outpoint.txid[..])?;
Expand Down Expand Up @@ -1461,6 +1461,7 @@ impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
(15, self.counterparty_fulfilled_htlcs, required),
(17, self.initial_counterparty_commitment_info, option),
(19, self.channel_id, required),
(20, self.channel_keys_id.version, (no_write_default, 0)),
(21, self.balances_empty_height, option),
(23, self.holder_pays_commitment_tx_fee, option),
(25, self.payment_preimages, required),
Expand Down Expand Up @@ -5126,7 +5127,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
outpoint: OutPoint { txid: tx.compute_txid(), index: i as u16 },
output: outp.clone(),
channel_keys_id: Some(self.channel_keys_id),
channel_keys_id: Some(self.channel_keys_id.id),
channel_keys_version: self.channel_keys_id.version,
});
}
if let Some(ref broadcasted_holder_revokable_script) = self.broadcasted_holder_revokable_script {
Expand Down Expand Up @@ -5156,7 +5158,8 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
spendable_outputs.push(SpendableOutputDescriptor::StaticOutput {
outpoint: OutPoint { txid: tx.compute_txid(), index: i as u16 },
output: outp.clone(),
channel_keys_id: Some(self.channel_keys_id),
channel_keys_id: Some(self.channel_keys_id.id),
channel_keys_version: self.channel_keys_id.version,
});
}
}
Expand Down Expand Up @@ -5438,6 +5441,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
let mut payment_preimages_with_info: Option<HashMap<_, _>> = None;
let mut first_confirmed_funding_txo = RequiredWrapper(None);
let mut channel_parameters = None;
let mut channel_keys_version = RequiredWrapper(None);
read_tlv_fields!(reader, {
(1, funding_spend_confirmed, option),
(3, htlcs_resolved_on_chain, optional_vec),
Expand All @@ -5449,13 +5453,20 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
(15, counterparty_fulfilled_htlcs, option),
(17, initial_counterparty_commitment_info, option),
(19, channel_id, option),
(20, channel_keys_version, (default_value, 0)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, needs to be odd.

(21, balances_empty_height, option),
(23, holder_pays_commitment_tx_fee, option),
(25, payment_preimages_with_info, option),
(27, first_confirmed_funding_txo, (default_value, outpoint)),
(29, initial_counterparty_commitment_tx, option),
(31, channel_parameters, (option: ReadableArgs, None)),
});

let channel_keys_id = ChannelKeysId {
id: channel_keys_id,
version: channel_keys_version.0.unwrap(),
};

if let Some(payment_preimages_with_info) = payment_preimages_with_info {
if payment_preimages_with_info.len() != payment_preimages.len() {
return Err(DecodeError::InvalidValue);
Expand Down
19 changes: 14 additions & 5 deletions lightning/src/chain/onchaintx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::ln::chan_utils::{
self, ChannelTransactionParameters, HTLCOutputInCommitment, HolderCommitmentTransaction,
};
use crate::ln::msgs::DecodeError;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, HTLCDescriptor, SignerProvider};
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, HTLCDescriptor, SignerProvider, ChannelKeysId};
use crate::util::logger::Logger;
use crate::util::ser::{
MaybeReadable, Readable, ReadableArgs, UpgradableRequired, Writeable, Writer,
Expand Down Expand Up @@ -222,7 +222,7 @@ pub(crate) enum FeerateStrategy {
#[derive(Clone)]
pub struct OnchainTxHandler<ChannelSigner: EcdsaChannelSigner> {
channel_value_satoshis: u64, // Deprecated as of 0.2.
channel_keys_id: [u8; 32], // Deprecated as of 0.2.
channel_keys_id: ChannelKeysId, // Deprecated as of 0.2.
destination_script: ScriptBuf, // Deprecated as of 0.2.
holder_commitment: HolderCommitmentTransaction,
prev_holder_commitment: Option<HolderCommitmentTransaction>,
Expand Down Expand Up @@ -379,7 +379,6 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
bytes_read += bytes_to_read;
}

let signer = signer_provider.derive_channel_signer(channel_keys_id);

let pending_claim_requests_len: u64 = Readable::read(reader)?;
let mut pending_claim_requests = hash_map_with_capacity(cmp::min(pending_claim_requests_len as usize, MAX_ALLOC_SIZE / 128));
Expand Down Expand Up @@ -416,8 +415,18 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
}
}

// FIXME: Bump some serialization for this, or read this field elsewhere
let channel_keys_version: u8 = Readable::read(reader)?;

read_tlv_fields!(reader, {});

let channel_keys_id = ChannelKeysId {
id: channel_keys_id,
version: channel_keys_version,
};

let signer = signer_provider.derive_channel_signer(channel_keys_id);

let mut secp_ctx = Secp256k1::new();
secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes());

Expand All @@ -441,7 +450,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP

impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
pub(crate) fn new(
channel_value_satoshis: u64, channel_keys_id: [u8; 32], destination_script: ScriptBuf,
channel_value_satoshis: u64, channel_keys_id: ChannelKeysId, destination_script: ScriptBuf,
signer: ChannelSigner, channel_parameters: ChannelTransactionParameters,
holder_commitment: HolderCommitmentTransaction, secp_ctx: Secp256k1<secp256k1::All>,
) -> Self {
Expand Down Expand Up @@ -1222,7 +1231,7 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
}

// Deprecated as of 0.2, only use in cases where it was not previously available.
pub(crate) fn channel_keys_id(&self) -> [u8; 32] {
pub(crate) fn channel_keys_id(&self) -> ChannelKeysId {
self.channel_keys_id
}
}
Expand Down
20 changes: 15 additions & 5 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ use crate::ln::script::{self, ShutdownScript};
use crate::ln::types::ChannelId;
use crate::routing::gossip::NodeId;
use crate::sign::ecdsa::EcdsaChannelSigner;
use crate::sign::{ChannelSigner, EntropySource, NodeSigner, Recipient, SignerProvider};
use crate::sign::{
ChannelKeysId, ChannelSigner, EntropySource, NodeSigner, Recipient, SignerProvider,
};
use crate::types::features::{ChannelTypeFeatures, InitFeatures};
use crate::types::payment::{PaymentHash, PaymentPreimage};
use crate::util::config::{
Expand Down Expand Up @@ -2483,9 +2485,9 @@ where
/// The unique identifier used to re-derive the private key material for the channel through
/// [`SignerProvider::derive_channel_signer`].
#[cfg(not(any(test, feature = "_test_utils")))]
channel_keys_id: [u8; 32],
channel_keys_id: ChannelKeysId,
#[cfg(any(test, feature = "_test_utils"))]
pub channel_keys_id: [u8; 32],
pub channel_keys_id: ChannelKeysId,

/// If we can't release a [`ChannelMonitorUpdate`] until some external action completes, we
/// store it here and only release it to the `ChannelManager` once it asks for it.
Expand Down Expand Up @@ -3353,7 +3355,7 @@ where
outbound_scid_alias: u64,
temporary_channel_id_fn: Option<impl Fn(&ChannelPublicKeys) -> ChannelId>,
holder_selected_channel_reserve_satoshis: u64,
channel_keys_id: [u8; 32],
channel_keys_id: ChannelKeysId,
holder_signer: <SP::Target as SignerProvider>::EcdsaSigner,
_logger: L,
) -> Result<(FundingScope, ChannelContext<SP>), APIError>
Expand Down Expand Up @@ -12368,7 +12370,7 @@ where
(21, self.context.outbound_scid_alias, required),
(23, initial_channel_ready_event_emitted, option),
(25, user_id_high_opt, option),
(27, self.context.channel_keys_id, required),
(27, self.context.channel_keys_id.id, required),
(28, holder_max_accepted_htlcs, option),
(29, self.context.temporary_channel_id, option),
(31, channel_pending_event_emitted, option),
Expand All @@ -12389,6 +12391,7 @@ where
(58, self.interactive_tx_signing_session, option), // Added in 0.2
(59, self.funding.minimum_depth_override, option), // Added in 0.2
(60, self.context.historical_scids, optional_vec), // Added in 0.2
(62, self.context.channel_keys_id.version, (no_write_default, 0)), // Added in 0.2
});

Ok(())
Expand Down Expand Up @@ -12680,6 +12683,7 @@ where

let mut user_id_high_opt: Option<u64> = None;
let mut channel_keys_id = [0u8; 32];
let mut channel_keys_version = RequiredWrapper(None);
let mut temporary_channel_id: Option<ChannelId> = None;
let mut holder_max_accepted_htlcs: Option<u16> = None;

Expand Down Expand Up @@ -12753,8 +12757,14 @@ where
(58, interactive_tx_signing_session, option), // Added in 0.2
(59, minimum_depth_override, option), // Added in 0.2
(60, historical_scids, optional_vec), // Added in 0.2
(62, channel_keys_version, (default_value, 0)), // Added in 0.2
});

let channel_keys_id = ChannelKeysId {
id: channel_keys_id,
version: channel_keys_version.0.unwrap(),
};

let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);

let mut iter = preimages.into_iter();
Expand Down
Loading