Drop ChannelKeys Private Key Methods#632

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.

Sign up for GitHub

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

Merged
TheBlueMatt merged 6 commits into from
Jun 6, 2020
Merged

Drop ChannelKeys Private Key Methods #632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b1d536e
Expose private keys from InMemoryChannelKeys publicly
TheBlueMatt May 28, 2020
9f7bcfb
Drop requirement that all ChannelKeys expose the funding privkey
TheBlueMatt May 28, 2020
1a574d2
Drop requirement that all ChannelKeys expose the payment_point
TheBlueMatt May 28, 2020
d9f5df9
Drop requirement that all ChannelKeys expose revocaion_basepoint
TheBlueMatt May 28, 2020
d77e40f
Drop requirement that ChannelKeys expose delayed_payment_basepoint
TheBlueMatt May 28, 2020
e5a7422
Drop requirement that all ChannelKeys expose htlc_basepoint
TheBlueMatt May 28, 2020
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
29 changes: 6 additions & 23 deletions lightning/src/chain/keysinterface.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,18 +195,6 @@ impl Readable for SpendableOutputDescriptor {
// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
pub trait ChannelKeys : Send+Clone {
/// Gets the private key for the anchor tx
fn funding_key<'a>(&'a self) -> &'a SecretKey;
/// Gets the local secret key for blinded revocation pubkey
fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
/// Gets the local secret key used in the to_remote output of remote commitment tx (ie the
/// output to us in transactions our counterparty broadcasts).
/// Also as part of obscured commitment number.
fn payment_key<'a>(&'a self) -> &'a SecretKey;
/// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output
fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
/// Gets the local htlc secret key used in commitment tx htlc outputs
fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
/// Gets the commitment seed
fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];

@ariardariardMay 29, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Future work, should we move commitment_seed out-of-memory and behind the interface by moving inside chan_utils::build_commitment_secret ? In case of undetected node compromise, access to the commitment seed let you derive revocation secret for future non-yet-existent local commitment transactions ?

@TheBlueMattTheBlueMattMay 29, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Yea, I think we have to. If you want an external signer to prevent the host from burning all the coins, you need some kind of exchange here, but we'd also need to get the ordering right, like, to call get_commitment_revocation_secret we'd have to first provide a copy of the new commitment transaction.

/// Gets the local channel public keys and basepoints
Expand DownExpand Up@@ -345,17 +333,17 @@ pub trait KeysInterface: Send + Sync {
/// A simple implementation of ChannelKeys that just keeps the private keys in memory.
pub struct InMemoryChannelKeys {
/// Private key of anchor tx
funding_key: SecretKey,
pub funding_key: SecretKey,

@arik-soarik-soJun 1, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think even for this concrete implementation, it would be better to not make the private key properties public. Perhaps add a test config constraint?

@TheBlueMattTheBlueMattJun 1, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

You need them to be able to sign transactions when handling SpendableOutput events, so there needs to be some way to get at them.

@valentinewallacevalentinewallaceJun 1, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't know the full context, but might it be better to expose a way to use them (e.g. expose a sign_tx method) rather than exposing the keys themselves?

@arik-soarik-soJun 1, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the signing only handles in impl for InMemoryChannelKeys, they may not have to be pub, right?

@TheBlueMattTheBlueMattJun 2, 2020

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

No, we dont require signing functions for SpendableOutputs events as those are purely external and can happen a month later. eg the use in the tests at https://github.com/rust-bitcoin/rust-lightning/blob/master/lightning/src/ln/functional_tests.rs#L4299

/// Local secret key for blinded revocation pubkey
revocation_base_key: SecretKey,
pub revocation_base_key: SecretKey,
/// Local secret key used for our balance in remote-broadcasted commitment transactions
payment_key: SecretKey,
pub payment_key: SecretKey,
/// Local secret key used in HTLC tx
delayed_payment_base_key: SecretKey,
pub delayed_payment_base_key: SecretKey,
/// Local htlc secret key used in commitment tx htlc outputs
htlc_base_key: SecretKey,
pub htlc_base_key: SecretKey,
/// Commitment seed
commitment_seed: [u8; 32],
pub commitment_seed: [u8; 32],
/// Local public keys and basepoints
pub(crate) local_channel_pubkeys: ChannelPublicKeys,
/// Remote public keys and base points
Expand DownExpand Up@@ -416,11 +404,6 @@ impl InMemoryChannelKeys {
}

impl ChannelKeys for InMemoryChannelKeys {
fn funding_key(&self) -> &SecretKey { &self.funding_key }
fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key }
fn payment_key(&self) -> &SecretKey { &self.payment_key }
fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key }
fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys }
fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
Expand Down
65 changes: 32 additions & 33 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -766,15 +766,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
let mut sha = Sha256::engine();
let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key());

let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize();
if self.channel_outbound {
sha.input(&our_payment_point.serialize());
sha.input(&self.local_keys.pubkeys().payment_point.serialize());
sha.input(their_payment_point);
} else {
sha.input(their_payment_point);
sha.input(&our_payment_point.serialize());
sha.input(&self.local_keys.pubkeys().payment_point.serialize());
}
let res = Sha256::from_engine(sha).into_inner();

Expand DownExpand Up@@ -1095,11 +1094,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
/// TODO Some magic rust shit to compile-time check this?
fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
}

#[inline]
Expand All@@ -1109,19 +1108,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
//TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
//may see payments to it!
let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key());
let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys"))
}

/// Gets the redeemscript for the funding transaction output (ie the funding transaction output
/// pays to get_funding_redeemscript().to_v0_p2wsh()).
/// Panics if called before accept_channel/new_from_req
pub fn get_funding_redeemscript(&self) -> Script {
let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey())
make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey())
}

/// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
Expand DownExpand Up@@ -1455,7 +1453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");

let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());

let remote_keys = self.build_remote_transaction_keys()?;
let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
Expand DownExpand Up@@ -1568,7 +1566,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
macro_rules! create_monitor {
() => { {
let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
&self.shutdown_pubkey, self.our_to_self_delay,
&self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
Expand DownExpand Up@@ -1899,7 +1897,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
let mut monitor_update = ChannelMonitorUpdate {
update_id: self.latest_monitor_update_id,
updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
htlc_outputs: htlcs_and_sigs
}]
};
Expand DownExpand Up@@ -2825,7 +2823,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

tx.input[0].witness.push(Vec::new()); // First is the multisig dummy

let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize();
let their_funding_key = self.their_funding_pubkey().serialize();
if our_funding_key[..] < their_funding_key[..] {
tx.input[0].witness.push(our_sig.serialize_der().to_vec());
Expand DownExpand Up@@ -3302,6 +3300,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}

let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
let local_keys = self.local_keys.pubkeys();

msgs::OpenChannel {
chain_hash: chain_hash,
Expand All@@ -3315,11 +3314,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
to_self_delay: self.our_to_self_delay,
max_accepted_htlcs: OUR_MAX_HTLCS,
funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
funding_pubkey: local_keys.funding_pubkey,
revocation_basepoint: local_keys.revocation_basepoint,
payment_point: local_keys.payment_point,
delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
htlc_basepoint: local_keys.htlc_basepoint,
first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
channel_flags: if self.config.announced_channel {1} else {0},
shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
Expand All@@ -3338,6 +3337,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}

let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
let local_keys = self.local_keys.pubkeys();

msgs::AcceptChannel {
temporary_channel_id: self.channel_id,
Expand All@@ -3348,11 +3348,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
minimum_depth: self.minimum_depth,
to_self_delay: self.our_to_self_delay,
max_accepted_htlcs: OUR_MAX_HTLCS,
funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
funding_pubkey: local_keys.funding_pubkey,
revocation_basepoint: local_keys.revocation_basepoint,
payment_point: local_keys.payment_point,
delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
htlc_basepoint: local_keys.htlc_basepoint,
first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
}
Expand DownExpand Up@@ -3431,16 +3431,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}

let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());

let msg = msgs::UnsignedChannelAnnouncement {
features: ChannelFeatures::known(),
chain_hash: chain_hash,
short_channel_id: self.get_short_channel_id().unwrap(),
node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() },
node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id },
bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() },
bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key },
bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() },
bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey },
excess_data: Vec::new(),
};

Expand DownExpand Up@@ -4442,7 +4441,7 @@ mod tests {
(0, 0)
);

assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..],
hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
let keys_provider = Keys { chan_keys: chan_keys.clone() };

Expand DownExpand Up@@ -4477,11 +4476,11 @@ mod tests {
// We can't just use build_local_transaction_keys here as the per_commitment_secret is not
// derived from a commitment_seed, so instead we copy it here and call
// build_commitment_transaction.
let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key());
let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();

chan.their_pubkeys = Some(their_pubkeys);

Expand DownExpand Up@@ -4512,7 +4511,7 @@ mod tests {
})*
assert_eq!(unsigned_tx.1.len(), per_htlc.len());

localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);

Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/channelmonitor.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1641,7 +1641,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
self.remote_payment_script = {
// Note that the Network here is ignored as we immediately drop the address for the
// script_pubkey version
let payment_hash160 = WPubkeyHash::hash(&PublicKey::from_secret_key(&self.secp_ctx, &self.keys.payment_key()).serialize());
let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
};

Expand Down
10 changes: 5 additions & 5 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3850,8 +3850,8 @@ fn test_invalid_channel_announcement() {
macro_rules! sign_msg {
($unsigned_msg: expr) => {
let msghash = Message::from_slice(&Sha256dHash::hash(&$unsigned_msg.encode()[..])[..]).unwrap();
let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key());
let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key());
let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key);
let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key);
let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].keys_manager.get_node_secret());
let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].keys_manager.get_node_secret());
chan_announcement = msgs::ChannelAnnouncement {
Expand DownExpand Up@@ -4293,10 +4293,10 @@ macro_rules! check_spendable_outputs {
};
let secp_ctx = Secp256k1::new();
let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
let remotepubkey = PublicKey::from_secret_key(&secp_ctx, &keys.payment_key());
let remotepubkey = keys.pubkeys().payment_point;
let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap();
let remotesig = secp_ctx.sign(&sighash, &keys.payment_key());
let remotesig = secp_ctx.sign(&sighash, &keys.inner.payment_key);
spend_tx.input[0].witness.push(remotesig.serialize_der().to_vec());
spend_tx.input[0].witness[0].push(SigHashType::All as u8);
spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec());
Expand All@@ -4321,7 +4321,7 @@ macro_rules! check_spendable_outputs {
};
let secp_ctx = Secp256k1::new();
let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) {
if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) {

let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey);
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Drop ChannelKeys Private Key Methods#632

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 6 commits into

from

Jun 6, 2020
 Merged


Drop ChannelKeys Private Key Methods
#632
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
b1d536e

Expose private keys from InMemoryChannelKeys publicly


TheBlueMatt May 28, 2020
9f7bcfb

Drop requirement that all ChannelKeys expose the funding privkey


TheBlueMatt May 28, 2020
1a574d2

Drop requirement that all ChannelKeys expose the payment_point


TheBlueMatt May 28, 2020
d9f5df9

Drop requirement that all ChannelKeys expose revocaion_basepoint


TheBlueMatt May 28, 2020
d77e40f

Drop requirement that ChannelKeys expose delayed_payment_basepoint


TheBlueMatt May 28, 2020
e5a7422

Drop requirement that all ChannelKeys expose htlc_basepoint


TheBlueMatt May 28, 2020

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


29 changes: 6 additions & 23 deletions
lightning/src/chain/keysinterface.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -195,18 +195,6 @@ impl Readable for SpendableOutputDescriptor {
// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
pub trait ChannelKeys : Send+Clone {
	/// Gets the private key for the anchor tx
	fn funding_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key for blinded revocation pubkey
	fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in the to_remote output of remote commitment tx (ie the
	/// output to us in transactions our counterparty broadcasts).
	/// Also as part of obscured commitment number.
	fn payment_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output
	fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local htlc secret key used in commitment tx htlc outputs
	fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the commitment seed
	fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];
@ariardariardMay 29, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Future work, should we move commitment_seed out-of-memory and behind the interface by moving inside chan_utils::build_commitment_secret ? In case of undetected node compromise, access to the commitment seed let you derive revocation secret for future non-yet-existent local commitment transactions ?
@TheBlueMattTheBlueMattMay 29, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Yea, I think we have to. If you want an external signer to prevent the host from burning all the coins, you need some kind of exchange here, but we'd also need to get the ordering right, like, to call get_commitment_revocation_secret we'd have to first provide a copy of the new commitment transaction.
	/// Gets the local channel public keys and basepoints
Expand DownExpand Up@@ -345,17 +333,17 @@ pub trait KeysInterface: Send + Sync {
/// A simple implementation of ChannelKeys that just keeps the private keys in memory.
pub struct InMemoryChannelKeys {
	/// Private key of anchor tx
	funding_key: SecretKey,
pub funding_key: SecretKey,
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I think even for this concrete implementation, it would be better to not make the private key properties public. Perhaps add a test config constraint?
@TheBlueMattTheBlueMattJun 1, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

You need them to be able to sign transactions when handling SpendableOutput events, so there needs to be some way to get at them.
@valentinewallacevalentinewallaceJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Don't know the full context, but might it be better to expose a way to use them (e.g. expose a sign_tx method) rather than exposing the keys themselves?
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If the signing only handles in impl for InMemoryChannelKeys, they may not have to be pub, right?
@TheBlueMattTheBlueMattJun 2, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

No, we dont require signing functions for SpendableOutputs events as those are purely external and can happen a month later. eg the use in the tests at https://github.com/rust-bitcoin/rust-lightning/blob/master/lightning/src/ln/functional_tests.rs#L4299
	/// Local secret key for blinded revocation pubkey
	revocation_base_key: SecretKey,
pub revocation_base_key: SecretKey,
	/// Local secret key used for our balance in remote-broadcasted commitment transactions
	payment_key: SecretKey,
pub payment_key: SecretKey,
	/// Local secret key used in HTLC tx
	delayed_payment_base_key: SecretKey,
pub delayed_payment_base_key: SecretKey,
	/// Local htlc secret key used in commitment tx htlc outputs
	htlc_base_key: SecretKey,
pub htlc_base_key: SecretKey,
	/// Commitment seed
	commitment_seed: [u8; 32],
pub commitment_seed: [u8; 32],
	/// Local public keys and basepoints
	pub(crate) local_channel_pubkeys: ChannelPublicKeys,
	/// Remote public keys and base points
Expand DownExpand Up@@ -416,11 +404,6 @@ impl InMemoryChannelKeys {
}

impl ChannelKeys for InMemoryChannelKeys {
	fn funding_key(&self) -> &SecretKey { &self.funding_key }
	fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key }
	fn payment_key(&self) -> &SecretKey { &self.payment_key }
	fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key }
	fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
	fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
	fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys }
	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
Expand Down

65 changes: 32 additions & 33 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -766,15 +766,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

	fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
 let mut sha = Sha256::engine();
 let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key());

 let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize();
 if self.channel_outbound {
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 sha.input(their_payment_point);
 } else {
 sha.input(their_payment_point);
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 }
 let res = Sha256::from_engine(sha).into_inner();

Expand DownExpand Up@@ -1095,11 +1094,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// TODO Some magic rust shit to compile-time check this?
	fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
 let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
 let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
	}

	#[inline]
Expand All@@ -1109,19 +1108,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
 //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
 //may see payments to it!
 let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys"))
	}

	/// Gets the redeemscript for the funding transaction output (ie the funding transaction output
	/// pays to get_funding_redeemscript().to_v0_p2wsh()).
	/// Panics if called before accept_channel/new_from_req
	pub fn get_funding_redeemscript(&self) -> Script {
 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
 make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey())
 make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey())
	}

	/// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
Expand DownExpand Up@@ -1455,7 +1453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
 secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");

 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());

 let remote_keys = self.build_remote_transaction_keys()?;
 let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
Expand DownExpand Up@@ -1568,7 +1566,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
 macro_rules! create_monitor {
 () => { {
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
 &self.shutdown_pubkey, self.our_to_self_delay,
 &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
Expand DownExpand Up@@ -1899,7 +1897,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let mut monitor_update = ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 htlc_outputs: htlcs_and_sigs
 }]
 };
Expand DownExpand Up@@ -2825,7 +2823,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 tx.input[0].witness.push(Vec::new()); // First is the multisig dummy

 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
 let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize();
 let their_funding_key = self.their_funding_pubkey().serialize();
 if our_funding_key[..] < their_funding_key[..] {
 tx.input[0].witness.push(our_sig.serialize_der().to_vec());
Expand DownExpand Up@@ -3302,6 +3300,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::OpenChannel {
 chain_hash: chain_hash,
Expand All@@ -3315,11 +3314,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 channel_flags: if self.config.announced_channel {1} else {0},
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
Expand All@@ -3338,6 +3337,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::AcceptChannel {
 temporary_channel_id: self.channel_id,
Expand All@@ -3348,11 +3348,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 minimum_depth: self.minimum_depth,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
 }
Expand DownExpand Up@@ -3431,16 +3431,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
 let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());

 let msg = msgs::UnsignedChannelAnnouncement {
 features: ChannelFeatures::known(),
 chain_hash: chain_hash,
 short_channel_id: self.get_short_channel_id().unwrap(),
 node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() },
 node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id },
 bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key },
 bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey },
 excess_data: Vec::new(),
 };

Expand DownExpand Up@@ -4442,7 +4441,7 @@ mod tests {
 (0, 0)
 );

 assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
 assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..],
 hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
 let keys_provider = Keys { chan_keys: chan_keys.clone() };

Expand DownExpand Up@@ -4477,11 +4476,11 @@ mod tests {
 // We can't just use build_local_transaction_keys here as the per_commitment_secret is not
 // derived from a commitment_seed, so instead we copy it here and call
 // build_commitment_transaction.
 let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key());
 let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
 let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
 let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
 let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
 let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();

 chan.their_pubkeys = Some(their_pubkeys);

Expand DownExpand Up@@ -4512,7 +4511,7 @@ mod tests {
 })*
 assert_eq!(unsigned_tx.1.len(), per_htlc.len());

 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
 assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1641,7 +1641,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 self.remote_payment_script = {
 // Note that the Network here is ignored as we immediately drop the address for the
 // script_pubkey version
 let payment_hash160 = WPubkeyHash::hash(&PublicKey::from_secret_key(&self.secp_ctx, &self.keys.payment_key()).serialize());
 let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
 Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
 };

Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3850,8 +3850,8 @@ fn test_invalid_channel_announcement() {
	macro_rules! sign_msg {
 ($unsigned_msg: expr) => {
 let msghash = Message::from_slice(&Sha256dHash::hash(&$unsigned_msg.encode()[..])[..]).unwrap();
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key());
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key());
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key);
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key);
 let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].keys_manager.get_node_secret());
 let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].keys_manager.get_node_secret());
 chan_announcement = msgs::ChannelAnnouncement {
Expand DownExpand Up@@ -4293,10 +4293,10 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 let remotepubkey = PublicKey::from_secret_key(&secp_ctx, &keys.payment_key());
 let remotepubkey = keys.pubkeys().payment_point;
 let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
 let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap();
 let remotesig = secp_ctx.sign(&sighash, &keys.payment_key());
 let remotesig = secp_ctx.sign(&sighash, &keys.inner.payment_key);
 spend_tx.input[0].witness.push(remotesig.serialize_der().to_vec());
 spend_tx.input[0].witness[0].push(SigHashType::All as u8);
 spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec());
Expand All@@ -4321,7 +4321,7 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) {
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) {

 let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
 let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Drop ChannelKeys Private Key Methods#632

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 6 commits into

from

Jun 6, 2020
 Merged


Drop ChannelKeys Private Key Methods
#632
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
b1d536e

Expose private keys from InMemoryChannelKeys publicly


TheBlueMatt May 28, 2020
9f7bcfb

Drop requirement that all ChannelKeys expose the funding privkey


TheBlueMatt May 28, 2020
1a574d2

Drop requirement that all ChannelKeys expose the payment_point


TheBlueMatt May 28, 2020
d9f5df9

Drop requirement that all ChannelKeys expose revocaion_basepoint


TheBlueMatt May 28, 2020
d77e40f

Drop requirement that ChannelKeys expose delayed_payment_basepoint


TheBlueMatt May 28, 2020
e5a7422

Drop requirement that all ChannelKeys expose htlc_basepoint


TheBlueMatt May 28, 2020

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


29 changes: 6 additions & 23 deletions
lightning/src/chain/keysinterface.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -195,18 +195,6 @@ impl Readable for SpendableOutputDescriptor {
// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
pub trait ChannelKeys : Send+Clone {
	/// Gets the private key for the anchor tx
	fn funding_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key for blinded revocation pubkey
	fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in the to_remote output of remote commitment tx (ie the
	/// output to us in transactions our counterparty broadcasts).
	/// Also as part of obscured commitment number.
	fn payment_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output
	fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local htlc secret key used in commitment tx htlc outputs
	fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the commitment seed
	fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];
@ariardariardMay 29, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Future work, should we move commitment_seed out-of-memory and behind the interface by moving inside chan_utils::build_commitment_secret ? In case of undetected node compromise, access to the commitment seed let you derive revocation secret for future non-yet-existent local commitment transactions ?
@TheBlueMattTheBlueMattMay 29, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Yea, I think we have to. If you want an external signer to prevent the host from burning all the coins, you need some kind of exchange here, but we'd also need to get the ordering right, like, to call get_commitment_revocation_secret we'd have to first provide a copy of the new commitment transaction.
	/// Gets the local channel public keys and basepoints
Expand DownExpand Up@@ -345,17 +333,17 @@ pub trait KeysInterface: Send + Sync {
/// A simple implementation of ChannelKeys that just keeps the private keys in memory.
pub struct InMemoryChannelKeys {
	/// Private key of anchor tx
	funding_key: SecretKey,
pub funding_key: SecretKey,
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I think even for this concrete implementation, it would be better to not make the private key properties public. Perhaps add a test config constraint?
@TheBlueMattTheBlueMattJun 1, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

You need them to be able to sign transactions when handling SpendableOutput events, so there needs to be some way to get at them.
@valentinewallacevalentinewallaceJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Don't know the full context, but might it be better to expose a way to use them (e.g. expose a sign_tx method) rather than exposing the keys themselves?
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If the signing only handles in impl for InMemoryChannelKeys, they may not have to be pub, right?
@TheBlueMattTheBlueMattJun 2, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

No, we dont require signing functions for SpendableOutputs events as those are purely external and can happen a month later. eg the use in the tests at https://github.com/rust-bitcoin/rust-lightning/blob/master/lightning/src/ln/functional_tests.rs#L4299
	/// Local secret key for blinded revocation pubkey
	revocation_base_key: SecretKey,
pub revocation_base_key: SecretKey,
	/// Local secret key used for our balance in remote-broadcasted commitment transactions
	payment_key: SecretKey,
pub payment_key: SecretKey,
	/// Local secret key used in HTLC tx
	delayed_payment_base_key: SecretKey,
pub delayed_payment_base_key: SecretKey,
	/// Local htlc secret key used in commitment tx htlc outputs
	htlc_base_key: SecretKey,
pub htlc_base_key: SecretKey,
	/// Commitment seed
	commitment_seed: [u8; 32],
pub commitment_seed: [u8; 32],
	/// Local public keys and basepoints
	pub(crate) local_channel_pubkeys: ChannelPublicKeys,
	/// Remote public keys and base points
Expand DownExpand Up@@ -416,11 +404,6 @@ impl InMemoryChannelKeys {
}

impl ChannelKeys for InMemoryChannelKeys {
	fn funding_key(&self) -> &SecretKey { &self.funding_key }
	fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key }
	fn payment_key(&self) -> &SecretKey { &self.payment_key }
	fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key }
	fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
	fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
	fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys }
	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
Expand Down

65 changes: 32 additions & 33 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -766,15 +766,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

	fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
 let mut sha = Sha256::engine();
 let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key());

 let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize();
 if self.channel_outbound {
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 sha.input(their_payment_point);
 } else {
 sha.input(their_payment_point);
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 }
 let res = Sha256::from_engine(sha).into_inner();

Expand DownExpand Up@@ -1095,11 +1094,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// TODO Some magic rust shit to compile-time check this?
	fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
 let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
 let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
	}

	#[inline]
Expand All@@ -1109,19 +1108,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
 //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
 //may see payments to it!
 let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys"))
	}

	/// Gets the redeemscript for the funding transaction output (ie the funding transaction output
	/// pays to get_funding_redeemscript().to_v0_p2wsh()).
	/// Panics if called before accept_channel/new_from_req
	pub fn get_funding_redeemscript(&self) -> Script {
 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
 make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey())
 make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey())
	}

	/// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
Expand DownExpand Up@@ -1455,7 +1453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
 secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");

 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());

 let remote_keys = self.build_remote_transaction_keys()?;
 let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
Expand DownExpand Up@@ -1568,7 +1566,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
 macro_rules! create_monitor {
 () => { {
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
 &self.shutdown_pubkey, self.our_to_self_delay,
 &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
Expand DownExpand Up@@ -1899,7 +1897,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let mut monitor_update = ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 htlc_outputs: htlcs_and_sigs
 }]
 };
Expand DownExpand Up@@ -2825,7 +2823,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 tx.input[0].witness.push(Vec::new()); // First is the multisig dummy

 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
 let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize();
 let their_funding_key = self.their_funding_pubkey().serialize();
 if our_funding_key[..] < their_funding_key[..] {
 tx.input[0].witness.push(our_sig.serialize_der().to_vec());
Expand DownExpand Up@@ -3302,6 +3300,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::OpenChannel {
 chain_hash: chain_hash,
Expand All@@ -3315,11 +3314,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 channel_flags: if self.config.announced_channel {1} else {0},
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
Expand All@@ -3338,6 +3337,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::AcceptChannel {
 temporary_channel_id: self.channel_id,
Expand All@@ -3348,11 +3348,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 minimum_depth: self.minimum_depth,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
 }
Expand DownExpand Up@@ -3431,16 +3431,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
 let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());

 let msg = msgs::UnsignedChannelAnnouncement {
 features: ChannelFeatures::known(),
 chain_hash: chain_hash,
 short_channel_id: self.get_short_channel_id().unwrap(),
 node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() },
 node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id },
 bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key },
 bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey },
 excess_data: Vec::new(),
 };

Expand DownExpand Up@@ -4442,7 +4441,7 @@ mod tests {
 (0, 0)
 );

 assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
 assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..],
 hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
 let keys_provider = Keys { chan_keys: chan_keys.clone() };

Expand DownExpand Up@@ -4477,11 +4476,11 @@ mod tests {
 // We can't just use build_local_transaction_keys here as the per_commitment_secret is not
 // derived from a commitment_seed, so instead we copy it here and call
 // build_commitment_transaction.
 let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key());
 let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
 let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
 let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
 let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
 let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();

 chan.their_pubkeys = Some(their_pubkeys);

Expand DownExpand Up@@ -4512,7 +4511,7 @@ mod tests {
 })*
 assert_eq!(unsigned_tx.1.len(), per_htlc.len());

 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
 assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1641,7 +1641,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 self.remote_payment_script = {
 // Note that the Network here is ignored as we immediately drop the address for the
 // script_pubkey version
 let payment_hash160 = WPubkeyHash::hash(&PublicKey::from_secret_key(&self.secp_ctx, &self.keys.payment_key()).serialize());
 let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
 Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
 };

Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3850,8 +3850,8 @@ fn test_invalid_channel_announcement() {
	macro_rules! sign_msg {
 ($unsigned_msg: expr) => {
 let msghash = Message::from_slice(&Sha256dHash::hash(&$unsigned_msg.encode()[..])[..]).unwrap();
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key());
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key());
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key);
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key);
 let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].keys_manager.get_node_secret());
 let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].keys_manager.get_node_secret());
 chan_announcement = msgs::ChannelAnnouncement {
Expand DownExpand Up@@ -4293,10 +4293,10 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 let remotepubkey = PublicKey::from_secret_key(&secp_ctx, &keys.payment_key());
 let remotepubkey = keys.pubkeys().payment_point;
 let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
 let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap();
 let remotesig = secp_ctx.sign(&sighash, &keys.payment_key());
 let remotesig = secp_ctx.sign(&sighash, &keys.inner.payment_key);
 spend_tx.input[0].witness.push(remotesig.serialize_der().to_vec());
 spend_tx.input[0].witness[0].push(SigHashType::All as u8);
 spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec());
Expand All@@ -4321,7 +4321,7 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) {
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) {

 let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
 let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Drop ChannelKeys Private Key Methods#632

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 6 commits into

from

Jun 6, 2020
 Merged


Drop ChannelKeys Private Key Methods
#632
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
b1d536e

Expose private keys from InMemoryChannelKeys publicly


TheBlueMatt May 28, 2020
9f7bcfb

Drop requirement that all ChannelKeys expose the funding privkey


TheBlueMatt May 28, 2020
1a574d2

Drop requirement that all ChannelKeys expose the payment_point


TheBlueMatt May 28, 2020
d9f5df9

Drop requirement that all ChannelKeys expose revocaion_basepoint


TheBlueMatt May 28, 2020
d77e40f

Drop requirement that ChannelKeys expose delayed_payment_basepoint


TheBlueMatt May 28, 2020
e5a7422

Drop requirement that all ChannelKeys expose htlc_basepoint


TheBlueMatt May 28, 2020

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


29 changes: 6 additions & 23 deletions
lightning/src/chain/keysinterface.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -195,18 +195,6 @@ impl Readable for SpendableOutputDescriptor {
// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
pub trait ChannelKeys : Send+Clone {
	/// Gets the private key for the anchor tx
	fn funding_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key for blinded revocation pubkey
	fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in the to_remote output of remote commitment tx (ie the
	/// output to us in transactions our counterparty broadcasts).
	/// Also as part of obscured commitment number.
	fn payment_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output
	fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local htlc secret key used in commitment tx htlc outputs
	fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the commitment seed
	fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];
@ariardariardMay 29, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Future work, should we move commitment_seed out-of-memory and behind the interface by moving inside chan_utils::build_commitment_secret ? In case of undetected node compromise, access to the commitment seed let you derive revocation secret for future non-yet-existent local commitment transactions ?
@TheBlueMattTheBlueMattMay 29, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Yea, I think we have to. If you want an external signer to prevent the host from burning all the coins, you need some kind of exchange here, but we'd also need to get the ordering right, like, to call get_commitment_revocation_secret we'd have to first provide a copy of the new commitment transaction.
	/// Gets the local channel public keys and basepoints
Expand DownExpand Up@@ -345,17 +333,17 @@ pub trait KeysInterface: Send + Sync {
/// A simple implementation of ChannelKeys that just keeps the private keys in memory.
pub struct InMemoryChannelKeys {
	/// Private key of anchor tx
	funding_key: SecretKey,
pub funding_key: SecretKey,
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I think even for this concrete implementation, it would be better to not make the private key properties public. Perhaps add a test config constraint?
@TheBlueMattTheBlueMattJun 1, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

You need them to be able to sign transactions when handling SpendableOutput events, so there needs to be some way to get at them.
@valentinewallacevalentinewallaceJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Don't know the full context, but might it be better to expose a way to use them (e.g. expose a sign_tx method) rather than exposing the keys themselves?
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If the signing only handles in impl for InMemoryChannelKeys, they may not have to be pub, right?
@TheBlueMattTheBlueMattJun 2, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

No, we dont require signing functions for SpendableOutputs events as those are purely external and can happen a month later. eg the use in the tests at https://github.com/rust-bitcoin/rust-lightning/blob/master/lightning/src/ln/functional_tests.rs#L4299
	/// Local secret key for blinded revocation pubkey
	revocation_base_key: SecretKey,
pub revocation_base_key: SecretKey,
	/// Local secret key used for our balance in remote-broadcasted commitment transactions
	payment_key: SecretKey,
pub payment_key: SecretKey,
	/// Local secret key used in HTLC tx
	delayed_payment_base_key: SecretKey,
pub delayed_payment_base_key: SecretKey,
	/// Local htlc secret key used in commitment tx htlc outputs
	htlc_base_key: SecretKey,
pub htlc_base_key: SecretKey,
	/// Commitment seed
	commitment_seed: [u8; 32],
pub commitment_seed: [u8; 32],
	/// Local public keys and basepoints
	pub(crate) local_channel_pubkeys: ChannelPublicKeys,
	/// Remote public keys and base points
Expand DownExpand Up@@ -416,11 +404,6 @@ impl InMemoryChannelKeys {
}

impl ChannelKeys for InMemoryChannelKeys {
	fn funding_key(&self) -> &SecretKey { &self.funding_key }
	fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key }
	fn payment_key(&self) -> &SecretKey { &self.payment_key }
	fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key }
	fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
	fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
	fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys }
	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
Expand Down

65 changes: 32 additions & 33 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -766,15 +766,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

	fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
 let mut sha = Sha256::engine();
 let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key());

 let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize();
 if self.channel_outbound {
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 sha.input(their_payment_point);
 } else {
 sha.input(their_payment_point);
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 }
 let res = Sha256::from_engine(sha).into_inner();

Expand DownExpand Up@@ -1095,11 +1094,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// TODO Some magic rust shit to compile-time check this?
	fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
 let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
 let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
	}

	#[inline]
Expand All@@ -1109,19 +1108,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
 //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
 //may see payments to it!
 let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys"))
	}

	/// Gets the redeemscript for the funding transaction output (ie the funding transaction output
	/// pays to get_funding_redeemscript().to_v0_p2wsh()).
	/// Panics if called before accept_channel/new_from_req
	pub fn get_funding_redeemscript(&self) -> Script {
 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
 make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey())
 make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey())
	}

	/// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
Expand DownExpand Up@@ -1455,7 +1453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
 secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");

 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());

 let remote_keys = self.build_remote_transaction_keys()?;
 let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
Expand DownExpand Up@@ -1568,7 +1566,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
 macro_rules! create_monitor {
 () => { {
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
 &self.shutdown_pubkey, self.our_to_self_delay,
 &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
Expand DownExpand Up@@ -1899,7 +1897,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let mut monitor_update = ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 htlc_outputs: htlcs_and_sigs
 }]
 };
Expand DownExpand Up@@ -2825,7 +2823,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 tx.input[0].witness.push(Vec::new()); // First is the multisig dummy

 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
 let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize();
 let their_funding_key = self.their_funding_pubkey().serialize();
 if our_funding_key[..] < their_funding_key[..] {
 tx.input[0].witness.push(our_sig.serialize_der().to_vec());
Expand DownExpand Up@@ -3302,6 +3300,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::OpenChannel {
 chain_hash: chain_hash,
Expand All@@ -3315,11 +3314,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 channel_flags: if self.config.announced_channel {1} else {0},
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
Expand All@@ -3338,6 +3337,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::AcceptChannel {
 temporary_channel_id: self.channel_id,
Expand All@@ -3348,11 +3348,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 minimum_depth: self.minimum_depth,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
 }
Expand DownExpand Up@@ -3431,16 +3431,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
 let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());

 let msg = msgs::UnsignedChannelAnnouncement {
 features: ChannelFeatures::known(),
 chain_hash: chain_hash,
 short_channel_id: self.get_short_channel_id().unwrap(),
 node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() },
 node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id },
 bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key },
 bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey },
 excess_data: Vec::new(),
 };

Expand DownExpand Up@@ -4442,7 +4441,7 @@ mod tests {
 (0, 0)
 );

 assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
 assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..],
 hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
 let keys_provider = Keys { chan_keys: chan_keys.clone() };

Expand DownExpand Up@@ -4477,11 +4476,11 @@ mod tests {
 // We can't just use build_local_transaction_keys here as the per_commitment_secret is not
 // derived from a commitment_seed, so instead we copy it here and call
 // build_commitment_transaction.
 let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key());
 let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
 let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
 let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
 let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
 let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();

 chan.their_pubkeys = Some(their_pubkeys);

Expand DownExpand Up@@ -4512,7 +4511,7 @@ mod tests {
 })*
 assert_eq!(unsigned_tx.1.len(), per_htlc.len());

 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
 assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1641,7 +1641,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 self.remote_payment_script = {
 // Note that the Network here is ignored as we immediately drop the address for the
 // script_pubkey version
 let payment_hash160 = WPubkeyHash::hash(&PublicKey::from_secret_key(&self.secp_ctx, &self.keys.payment_key()).serialize());
 let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
 Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
 };

Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3850,8 +3850,8 @@ fn test_invalid_channel_announcement() {
	macro_rules! sign_msg {
 ($unsigned_msg: expr) => {
 let msghash = Message::from_slice(&Sha256dHash::hash(&$unsigned_msg.encode()[..])[..]).unwrap();
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key());
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key());
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key);
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key);
 let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].keys_manager.get_node_secret());
 let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].keys_manager.get_node_secret());
 chan_announcement = msgs::ChannelAnnouncement {
Expand DownExpand Up@@ -4293,10 +4293,10 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 let remotepubkey = PublicKey::from_secret_key(&secp_ctx, &keys.payment_key());
 let remotepubkey = keys.pubkeys().payment_point;
 let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
 let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap();
 let remotesig = secp_ctx.sign(&sighash, &keys.payment_key());
 let remotesig = secp_ctx.sign(&sighash, &keys.inner.payment_key);
 spend_tx.input[0].witness.push(remotesig.serialize_der().to_vec());
 spend_tx.input[0].witness[0].push(SigHashType::All as u8);
 spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec());
Expand All@@ -4321,7 +4321,7 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) {
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) {

 let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
 let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Drop ChannelKeys Private Key Methods#632

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 6 commits into

from

Jun 6, 2020
 Merged


Drop ChannelKeys Private Key Methods
#632
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
b1d536e

Expose private keys from InMemoryChannelKeys publicly


TheBlueMatt May 28, 2020
9f7bcfb

Drop requirement that all ChannelKeys expose the funding privkey


TheBlueMatt May 28, 2020
1a574d2

Drop requirement that all ChannelKeys expose the payment_point


TheBlueMatt May 28, 2020
d9f5df9

Drop requirement that all ChannelKeys expose revocaion_basepoint


TheBlueMatt May 28, 2020
d77e40f

Drop requirement that ChannelKeys expose delayed_payment_basepoint


TheBlueMatt May 28, 2020
e5a7422

Drop requirement that all ChannelKeys expose htlc_basepoint


TheBlueMatt May 28, 2020

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


29 changes: 6 additions & 23 deletions
lightning/src/chain/keysinterface.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -195,18 +195,6 @@ impl Readable for SpendableOutputDescriptor {
// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
pub trait ChannelKeys : Send+Clone {
	/// Gets the private key for the anchor tx
	fn funding_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key for blinded revocation pubkey
	fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in the to_remote output of remote commitment tx (ie the
	/// output to us in transactions our counterparty broadcasts).
	/// Also as part of obscured commitment number.
	fn payment_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output
	fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local htlc secret key used in commitment tx htlc outputs
	fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the commitment seed
	fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];
@ariardariardMay 29, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Future work, should we move commitment_seed out-of-memory and behind the interface by moving inside chan_utils::build_commitment_secret ? In case of undetected node compromise, access to the commitment seed let you derive revocation secret for future non-yet-existent local commitment transactions ?
@TheBlueMattTheBlueMattMay 29, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Yea, I think we have to. If you want an external signer to prevent the host from burning all the coins, you need some kind of exchange here, but we'd also need to get the ordering right, like, to call get_commitment_revocation_secret we'd have to first provide a copy of the new commitment transaction.
	/// Gets the local channel public keys and basepoints
Expand DownExpand Up@@ -345,17 +333,17 @@ pub trait KeysInterface: Send + Sync {
/// A simple implementation of ChannelKeys that just keeps the private keys in memory.
pub struct InMemoryChannelKeys {
	/// Private key of anchor tx
	funding_key: SecretKey,
pub funding_key: SecretKey,
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I think even for this concrete implementation, it would be better to not make the private key properties public. Perhaps add a test config constraint?
@TheBlueMattTheBlueMattJun 1, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

You need them to be able to sign transactions when handling SpendableOutput events, so there needs to be some way to get at them.
@valentinewallacevalentinewallaceJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Don't know the full context, but might it be better to expose a way to use them (e.g. expose a sign_tx method) rather than exposing the keys themselves?
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If the signing only handles in impl for InMemoryChannelKeys, they may not have to be pub, right?
@TheBlueMattTheBlueMattJun 2, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

No, we dont require signing functions for SpendableOutputs events as those are purely external and can happen a month later. eg the use in the tests at https://github.com/rust-bitcoin/rust-lightning/blob/master/lightning/src/ln/functional_tests.rs#L4299
	/// Local secret key for blinded revocation pubkey
	revocation_base_key: SecretKey,
pub revocation_base_key: SecretKey,
	/// Local secret key used for our balance in remote-broadcasted commitment transactions
	payment_key: SecretKey,
pub payment_key: SecretKey,
	/// Local secret key used in HTLC tx
	delayed_payment_base_key: SecretKey,
pub delayed_payment_base_key: SecretKey,
	/// Local htlc secret key used in commitment tx htlc outputs
	htlc_base_key: SecretKey,
pub htlc_base_key: SecretKey,
	/// Commitment seed
	commitment_seed: [u8; 32],
pub commitment_seed: [u8; 32],
	/// Local public keys and basepoints
	pub(crate) local_channel_pubkeys: ChannelPublicKeys,
	/// Remote public keys and base points
Expand DownExpand Up@@ -416,11 +404,6 @@ impl InMemoryChannelKeys {
}

impl ChannelKeys for InMemoryChannelKeys {
	fn funding_key(&self) -> &SecretKey { &self.funding_key }
	fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key }
	fn payment_key(&self) -> &SecretKey { &self.payment_key }
	fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key }
	fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
	fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
	fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys }
	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
Expand Down

65 changes: 32 additions & 33 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -766,15 +766,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

	fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
 let mut sha = Sha256::engine();
 let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key());

 let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize();
 if self.channel_outbound {
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 sha.input(their_payment_point);
 } else {
 sha.input(their_payment_point);
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 }
 let res = Sha256::from_engine(sha).into_inner();

Expand DownExpand Up@@ -1095,11 +1094,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// TODO Some magic rust shit to compile-time check this?
	fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
 let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
 let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
	}

	#[inline]
Expand All@@ -1109,19 +1108,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
 //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
 //may see payments to it!
 let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys"))
	}

	/// Gets the redeemscript for the funding transaction output (ie the funding transaction output
	/// pays to get_funding_redeemscript().to_v0_p2wsh()).
	/// Panics if called before accept_channel/new_from_req
	pub fn get_funding_redeemscript(&self) -> Script {
 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
 make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey())
 make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey())
	}

	/// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
Expand DownExpand Up@@ -1455,7 +1453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
 secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");

 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());

 let remote_keys = self.build_remote_transaction_keys()?;
 let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
Expand DownExpand Up@@ -1568,7 +1566,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
 macro_rules! create_monitor {
 () => { {
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
 &self.shutdown_pubkey, self.our_to_self_delay,
 &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
Expand DownExpand Up@@ -1899,7 +1897,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let mut monitor_update = ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 htlc_outputs: htlcs_and_sigs
 }]
 };
Expand DownExpand Up@@ -2825,7 +2823,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 tx.input[0].witness.push(Vec::new()); // First is the multisig dummy

 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
 let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize();
 let their_funding_key = self.their_funding_pubkey().serialize();
 if our_funding_key[..] < their_funding_key[..] {
 tx.input[0].witness.push(our_sig.serialize_der().to_vec());
Expand DownExpand Up@@ -3302,6 +3300,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::OpenChannel {
 chain_hash: chain_hash,
Expand All@@ -3315,11 +3314,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 channel_flags: if self.config.announced_channel {1} else {0},
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
Expand All@@ -3338,6 +3337,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::AcceptChannel {
 temporary_channel_id: self.channel_id,
Expand All@@ -3348,11 +3348,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 minimum_depth: self.minimum_depth,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
 }
Expand DownExpand Up@@ -3431,16 +3431,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
 let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());

 let msg = msgs::UnsignedChannelAnnouncement {
 features: ChannelFeatures::known(),
 chain_hash: chain_hash,
 short_channel_id: self.get_short_channel_id().unwrap(),
 node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() },
 node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id },
 bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key },
 bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey },
 excess_data: Vec::new(),
 };

Expand DownExpand Up@@ -4442,7 +4441,7 @@ mod tests {
 (0, 0)
 );

 assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
 assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..],
 hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
 let keys_provider = Keys { chan_keys: chan_keys.clone() };

Expand DownExpand Up@@ -4477,11 +4476,11 @@ mod tests {
 // We can't just use build_local_transaction_keys here as the per_commitment_secret is not
 // derived from a commitment_seed, so instead we copy it here and call
 // build_commitment_transaction.
 let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key());
 let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
 let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
 let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
 let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
 let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();

 chan.their_pubkeys = Some(their_pubkeys);

Expand DownExpand Up@@ -4512,7 +4511,7 @@ mod tests {
 })*
 assert_eq!(unsigned_tx.1.len(), per_htlc.len());

 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
 assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1641,7 +1641,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 self.remote_payment_script = {
 // Note that the Network here is ignored as we immediately drop the address for the
 // script_pubkey version
 let payment_hash160 = WPubkeyHash::hash(&PublicKey::from_secret_key(&self.secp_ctx, &self.keys.payment_key()).serialize());
 let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
 Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
 };

Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3850,8 +3850,8 @@ fn test_invalid_channel_announcement() {
	macro_rules! sign_msg {
 ($unsigned_msg: expr) => {
 let msghash = Message::from_slice(&Sha256dHash::hash(&$unsigned_msg.encode()[..])[..]).unwrap();
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key());
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key());
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key);
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key);
 let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].keys_manager.get_node_secret());
 let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].keys_manager.get_node_secret());
 chan_announcement = msgs::ChannelAnnouncement {
Expand DownExpand Up@@ -4293,10 +4293,10 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 let remotepubkey = PublicKey::from_secret_key(&secp_ctx, &keys.payment_key());
 let remotepubkey = keys.pubkeys().payment_point;
 let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
 let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap();
 let remotesig = secp_ctx.sign(&sighash, &keys.payment_key());
 let remotesig = secp_ctx.sign(&sighash, &keys.inner.payment_key);
 spend_tx.input[0].witness.push(remotesig.serialize_der().to_vec());
 spend_tx.input[0].witness[0].push(SigHashType::All as u8);
 spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec());
Expand All@@ -4321,7 +4321,7 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) {
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) {

 let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
 let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Drop ChannelKeys Private Key Methods#632

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 6 commits into

from

Jun 6, 2020
 Merged


Drop ChannelKeys Private Key Methods
#632
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
b1d536e

Expose private keys from InMemoryChannelKeys publicly


TheBlueMatt May 28, 2020
9f7bcfb

Drop requirement that all ChannelKeys expose the funding privkey


TheBlueMatt May 28, 2020
1a574d2

Drop requirement that all ChannelKeys expose the payment_point


TheBlueMatt May 28, 2020
d9f5df9

Drop requirement that all ChannelKeys expose revocaion_basepoint


TheBlueMatt May 28, 2020
d77e40f

Drop requirement that ChannelKeys expose delayed_payment_basepoint


TheBlueMatt May 28, 2020
e5a7422

Drop requirement that all ChannelKeys expose htlc_basepoint


TheBlueMatt May 28, 2020

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


29 changes: 6 additions & 23 deletions
lightning/src/chain/keysinterface.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -195,18 +195,6 @@ impl Readable for SpendableOutputDescriptor {
// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
pub trait ChannelKeys : Send+Clone {
	/// Gets the private key for the anchor tx
	fn funding_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key for blinded revocation pubkey
	fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in the to_remote output of remote commitment tx (ie the
	/// output to us in transactions our counterparty broadcasts).
	/// Also as part of obscured commitment number.
	fn payment_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output
	fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local htlc secret key used in commitment tx htlc outputs
	fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the commitment seed
	fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];
@ariardariardMay 29, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Future work, should we move commitment_seed out-of-memory and behind the interface by moving inside chan_utils::build_commitment_secret ? In case of undetected node compromise, access to the commitment seed let you derive revocation secret for future non-yet-existent local commitment transactions ?
@TheBlueMattTheBlueMattMay 29, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Yea, I think we have to. If you want an external signer to prevent the host from burning all the coins, you need some kind of exchange here, but we'd also need to get the ordering right, like, to call get_commitment_revocation_secret we'd have to first provide a copy of the new commitment transaction.
	/// Gets the local channel public keys and basepoints
Expand DownExpand Up@@ -345,17 +333,17 @@ pub trait KeysInterface: Send + Sync {
/// A simple implementation of ChannelKeys that just keeps the private keys in memory.
pub struct InMemoryChannelKeys {
	/// Private key of anchor tx
	funding_key: SecretKey,
pub funding_key: SecretKey,
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I think even for this concrete implementation, it would be better to not make the private key properties public. Perhaps add a test config constraint?
@TheBlueMattTheBlueMattJun 1, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

You need them to be able to sign transactions when handling SpendableOutput events, so there needs to be some way to get at them.
@valentinewallacevalentinewallaceJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Don't know the full context, but might it be better to expose a way to use them (e.g. expose a sign_tx method) rather than exposing the keys themselves?
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If the signing only handles in impl for InMemoryChannelKeys, they may not have to be pub, right?
@TheBlueMattTheBlueMattJun 2, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

No, we dont require signing functions for SpendableOutputs events as those are purely external and can happen a month later. eg the use in the tests at https://github.com/rust-bitcoin/rust-lightning/blob/master/lightning/src/ln/functional_tests.rs#L4299
	/// Local secret key for blinded revocation pubkey
	revocation_base_key: SecretKey,
pub revocation_base_key: SecretKey,
	/// Local secret key used for our balance in remote-broadcasted commitment transactions
	payment_key: SecretKey,
pub payment_key: SecretKey,
	/// Local secret key used in HTLC tx
	delayed_payment_base_key: SecretKey,
pub delayed_payment_base_key: SecretKey,
	/// Local htlc secret key used in commitment tx htlc outputs
	htlc_base_key: SecretKey,
pub htlc_base_key: SecretKey,
	/// Commitment seed
	commitment_seed: [u8; 32],
pub commitment_seed: [u8; 32],
	/// Local public keys and basepoints
	pub(crate) local_channel_pubkeys: ChannelPublicKeys,
	/// Remote public keys and base points
Expand DownExpand Up@@ -416,11 +404,6 @@ impl InMemoryChannelKeys {
}

impl ChannelKeys for InMemoryChannelKeys {
	fn funding_key(&self) -> &SecretKey { &self.funding_key }
	fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key }
	fn payment_key(&self) -> &SecretKey { &self.payment_key }
	fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key }
	fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
	fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
	fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys }
	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
Expand Down

65 changes: 32 additions & 33 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -766,15 +766,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

	fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
 let mut sha = Sha256::engine();
 let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key());

 let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize();
 if self.channel_outbound {
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 sha.input(their_payment_point);
 } else {
 sha.input(their_payment_point);
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 }
 let res = Sha256::from_engine(sha).into_inner();

Expand DownExpand Up@@ -1095,11 +1094,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// TODO Some magic rust shit to compile-time check this?
	fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
 let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
 let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
	}

	#[inline]
Expand All@@ -1109,19 +1108,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
 //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
 //may see payments to it!
 let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys"))
	}

	/// Gets the redeemscript for the funding transaction output (ie the funding transaction output
	/// pays to get_funding_redeemscript().to_v0_p2wsh()).
	/// Panics if called before accept_channel/new_from_req
	pub fn get_funding_redeemscript(&self) -> Script {
 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
 make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey())
 make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey())
	}

	/// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
Expand DownExpand Up@@ -1455,7 +1453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
 secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");

 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());

 let remote_keys = self.build_remote_transaction_keys()?;
 let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
Expand DownExpand Up@@ -1568,7 +1566,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
 macro_rules! create_monitor {
 () => { {
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
 &self.shutdown_pubkey, self.our_to_self_delay,
 &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
Expand DownExpand Up@@ -1899,7 +1897,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let mut monitor_update = ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 htlc_outputs: htlcs_and_sigs
 }]
 };
Expand DownExpand Up@@ -2825,7 +2823,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 tx.input[0].witness.push(Vec::new()); // First is the multisig dummy

 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
 let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize();
 let their_funding_key = self.their_funding_pubkey().serialize();
 if our_funding_key[..] < their_funding_key[..] {
 tx.input[0].witness.push(our_sig.serialize_der().to_vec());
Expand DownExpand Up@@ -3302,6 +3300,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::OpenChannel {
 chain_hash: chain_hash,
Expand All@@ -3315,11 +3314,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 channel_flags: if self.config.announced_channel {1} else {0},
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
Expand All@@ -3338,6 +3337,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::AcceptChannel {
 temporary_channel_id: self.channel_id,
Expand All@@ -3348,11 +3348,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 minimum_depth: self.minimum_depth,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
 }
Expand DownExpand Up@@ -3431,16 +3431,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
 let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());

 let msg = msgs::UnsignedChannelAnnouncement {
 features: ChannelFeatures::known(),
 chain_hash: chain_hash,
 short_channel_id: self.get_short_channel_id().unwrap(),
 node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() },
 node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id },
 bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key },
 bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey },
 excess_data: Vec::new(),
 };

Expand DownExpand Up@@ -4442,7 +4441,7 @@ mod tests {
 (0, 0)
 );

 assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
 assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..],
 hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
 let keys_provider = Keys { chan_keys: chan_keys.clone() };

Expand DownExpand Up@@ -4477,11 +4476,11 @@ mod tests {
 // We can't just use build_local_transaction_keys here as the per_commitment_secret is not
 // derived from a commitment_seed, so instead we copy it here and call
 // build_commitment_transaction.
 let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key());
 let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
 let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
 let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
 let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
 let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();

 chan.their_pubkeys = Some(their_pubkeys);

Expand DownExpand Up@@ -4512,7 +4511,7 @@ mod tests {
 })*
 assert_eq!(unsigned_tx.1.len(), per_htlc.len());

 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
 assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1641,7 +1641,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 self.remote_payment_script = {
 // Note that the Network here is ignored as we immediately drop the address for the
 // script_pubkey version
 let payment_hash160 = WPubkeyHash::hash(&PublicKey::from_secret_key(&self.secp_ctx, &self.keys.payment_key()).serialize());
 let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
 Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
 };

Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3850,8 +3850,8 @@ fn test_invalid_channel_announcement() {
	macro_rules! sign_msg {
 ($unsigned_msg: expr) => {
 let msghash = Message::from_slice(&Sha256dHash::hash(&$unsigned_msg.encode()[..])[..]).unwrap();
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key());
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key());
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key);
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key);
 let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].keys_manager.get_node_secret());
 let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].keys_manager.get_node_secret());
 chan_announcement = msgs::ChannelAnnouncement {
Expand DownExpand Up@@ -4293,10 +4293,10 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 let remotepubkey = PublicKey::from_secret_key(&secp_ctx, &keys.payment_key());
 let remotepubkey = keys.pubkeys().payment_point;
 let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
 let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap();
 let remotesig = secp_ctx.sign(&sighash, &keys.payment_key());
 let remotesig = secp_ctx.sign(&sighash, &keys.inner.payment_key);
 spend_tx.input[0].witness.push(remotesig.serialize_der().to_vec());
 spend_tx.input[0].witness[0].push(SigHashType::All as u8);
 spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec());
Expand All@@ -4321,7 +4321,7 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) {
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) {

 let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
 let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Drop ChannelKeys Private Key Methods#632

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 6 commits into

from

Jun 6, 2020
 Merged


Drop ChannelKeys Private Key Methods
#632
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
b1d536e

Expose private keys from InMemoryChannelKeys publicly


TheBlueMatt May 28, 2020
9f7bcfb

Drop requirement that all ChannelKeys expose the funding privkey


TheBlueMatt May 28, 2020
1a574d2

Drop requirement that all ChannelKeys expose the payment_point


TheBlueMatt May 28, 2020
d9f5df9

Drop requirement that all ChannelKeys expose revocaion_basepoint


TheBlueMatt May 28, 2020
d77e40f

Drop requirement that ChannelKeys expose delayed_payment_basepoint


TheBlueMatt May 28, 2020
e5a7422

Drop requirement that all ChannelKeys expose htlc_basepoint


TheBlueMatt May 28, 2020

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


29 changes: 6 additions & 23 deletions
lightning/src/chain/keysinterface.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -195,18 +195,6 @@ impl Readable for SpendableOutputDescriptor {
// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
pub trait ChannelKeys : Send+Clone {
	/// Gets the private key for the anchor tx
	fn funding_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key for blinded revocation pubkey
	fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in the to_remote output of remote commitment tx (ie the
	/// output to us in transactions our counterparty broadcasts).
	/// Also as part of obscured commitment number.
	fn payment_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output
	fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local htlc secret key used in commitment tx htlc outputs
	fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the commitment seed
	fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];
@ariardariardMay 29, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Future work, should we move commitment_seed out-of-memory and behind the interface by moving inside chan_utils::build_commitment_secret ? In case of undetected node compromise, access to the commitment seed let you derive revocation secret for future non-yet-existent local commitment transactions ?
@TheBlueMattTheBlueMattMay 29, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Yea, I think we have to. If you want an external signer to prevent the host from burning all the coins, you need some kind of exchange here, but we'd also need to get the ordering right, like, to call get_commitment_revocation_secret we'd have to first provide a copy of the new commitment transaction.
	/// Gets the local channel public keys and basepoints
Expand DownExpand Up@@ -345,17 +333,17 @@ pub trait KeysInterface: Send + Sync {
/// A simple implementation of ChannelKeys that just keeps the private keys in memory.
pub struct InMemoryChannelKeys {
	/// Private key of anchor tx
	funding_key: SecretKey,
pub funding_key: SecretKey,
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I think even for this concrete implementation, it would be better to not make the private key properties public. Perhaps add a test config constraint?
@TheBlueMattTheBlueMattJun 1, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

You need them to be able to sign transactions when handling SpendableOutput events, so there needs to be some way to get at them.
@valentinewallacevalentinewallaceJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Don't know the full context, but might it be better to expose a way to use them (e.g. expose a sign_tx method) rather than exposing the keys themselves?
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If the signing only handles in impl for InMemoryChannelKeys, they may not have to be pub, right?
@TheBlueMattTheBlueMattJun 2, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

No, we dont require signing functions for SpendableOutputs events as those are purely external and can happen a month later. eg the use in the tests at https://github.com/rust-bitcoin/rust-lightning/blob/master/lightning/src/ln/functional_tests.rs#L4299
	/// Local secret key for blinded revocation pubkey
	revocation_base_key: SecretKey,
pub revocation_base_key: SecretKey,
	/// Local secret key used for our balance in remote-broadcasted commitment transactions
	payment_key: SecretKey,
pub payment_key: SecretKey,
	/// Local secret key used in HTLC tx
	delayed_payment_base_key: SecretKey,
pub delayed_payment_base_key: SecretKey,
	/// Local htlc secret key used in commitment tx htlc outputs
	htlc_base_key: SecretKey,
pub htlc_base_key: SecretKey,
	/// Commitment seed
	commitment_seed: [u8; 32],
pub commitment_seed: [u8; 32],
	/// Local public keys and basepoints
	pub(crate) local_channel_pubkeys: ChannelPublicKeys,
	/// Remote public keys and base points
Expand DownExpand Up@@ -416,11 +404,6 @@ impl InMemoryChannelKeys {
}

impl ChannelKeys for InMemoryChannelKeys {
	fn funding_key(&self) -> &SecretKey { &self.funding_key }
	fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key }
	fn payment_key(&self) -> &SecretKey { &self.payment_key }
	fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key }
	fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
	fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
	fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys }
	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
Expand Down

65 changes: 32 additions & 33 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -766,15 +766,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

	fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
 let mut sha = Sha256::engine();
 let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key());

 let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize();
 if self.channel_outbound {
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 sha.input(their_payment_point);
 } else {
 sha.input(their_payment_point);
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 }
 let res = Sha256::from_engine(sha).into_inner();

Expand DownExpand Up@@ -1095,11 +1094,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// TODO Some magic rust shit to compile-time check this?
	fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
 let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
 let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
	}

	#[inline]
Expand All@@ -1109,19 +1108,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
 //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
 //may see payments to it!
 let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys"))
	}

	/// Gets the redeemscript for the funding transaction output (ie the funding transaction output
	/// pays to get_funding_redeemscript().to_v0_p2wsh()).
	/// Panics if called before accept_channel/new_from_req
	pub fn get_funding_redeemscript(&self) -> Script {
 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
 make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey())
 make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey())
	}

	/// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
Expand DownExpand Up@@ -1455,7 +1453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
 secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");

 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());

 let remote_keys = self.build_remote_transaction_keys()?;
 let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
Expand DownExpand Up@@ -1568,7 +1566,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
 macro_rules! create_monitor {
 () => { {
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
 &self.shutdown_pubkey, self.our_to_self_delay,
 &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
Expand DownExpand Up@@ -1899,7 +1897,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let mut monitor_update = ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 htlc_outputs: htlcs_and_sigs
 }]
 };
Expand DownExpand Up@@ -2825,7 +2823,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 tx.input[0].witness.push(Vec::new()); // First is the multisig dummy

 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
 let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize();
 let their_funding_key = self.their_funding_pubkey().serialize();
 if our_funding_key[..] < their_funding_key[..] {
 tx.input[0].witness.push(our_sig.serialize_der().to_vec());
Expand DownExpand Up@@ -3302,6 +3300,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::OpenChannel {
 chain_hash: chain_hash,
Expand All@@ -3315,11 +3314,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 channel_flags: if self.config.announced_channel {1} else {0},
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
Expand All@@ -3338,6 +3337,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::AcceptChannel {
 temporary_channel_id: self.channel_id,
Expand All@@ -3348,11 +3348,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 minimum_depth: self.minimum_depth,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
 }
Expand DownExpand Up@@ -3431,16 +3431,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
 let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());

 let msg = msgs::UnsignedChannelAnnouncement {
 features: ChannelFeatures::known(),
 chain_hash: chain_hash,
 short_channel_id: self.get_short_channel_id().unwrap(),
 node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() },
 node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id },
 bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key },
 bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey },
 excess_data: Vec::new(),
 };

Expand DownExpand Up@@ -4442,7 +4441,7 @@ mod tests {
 (0, 0)
 );

 assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
 assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..],
 hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
 let keys_provider = Keys { chan_keys: chan_keys.clone() };

Expand DownExpand Up@@ -4477,11 +4476,11 @@ mod tests {
 // We can't just use build_local_transaction_keys here as the per_commitment_secret is not
 // derived from a commitment_seed, so instead we copy it here and call
 // build_commitment_transaction.
 let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key());
 let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
 let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
 let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
 let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
 let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();

 chan.their_pubkeys = Some(their_pubkeys);

Expand DownExpand Up@@ -4512,7 +4511,7 @@ mod tests {
 })*
 assert_eq!(unsigned_tx.1.len(), per_htlc.len());

 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
 assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1641,7 +1641,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 self.remote_payment_script = {
 // Note that the Network here is ignored as we immediately drop the address for the
 // script_pubkey version
 let payment_hash160 = WPubkeyHash::hash(&PublicKey::from_secret_key(&self.secp_ctx, &self.keys.payment_key()).serialize());
 let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
 Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
 };

Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3850,8 +3850,8 @@ fn test_invalid_channel_announcement() {
	macro_rules! sign_msg {
 ($unsigned_msg: expr) => {
 let msghash = Message::from_slice(&Sha256dHash::hash(&$unsigned_msg.encode()[..])[..]).unwrap();
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key());
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key());
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key);
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key);
 let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].keys_manager.get_node_secret());
 let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].keys_manager.get_node_secret());
 chan_announcement = msgs::ChannelAnnouncement {
Expand DownExpand Up@@ -4293,10 +4293,10 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 let remotepubkey = PublicKey::from_secret_key(&secp_ctx, &keys.payment_key());
 let remotepubkey = keys.pubkeys().payment_point;
 let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
 let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap();
 let remotesig = secp_ctx.sign(&sighash, &keys.payment_key());
 let remotesig = secp_ctx.sign(&sighash, &keys.inner.payment_key);
 spend_tx.input[0].witness.push(remotesig.serialize_der().to_vec());
 spend_tx.input[0].witness[0].push(SigHashType::All as u8);
 spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec());
Expand All@@ -4321,7 +4321,7 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) {
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) {

 let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
 let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey);
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Drop ChannelKeys Private Key Methods#632

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 6 commits into

from

Jun 6, 2020
 Merged


Drop ChannelKeys Private Key Methods
#632
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
b1d536e

Expose private keys from InMemoryChannelKeys publicly


TheBlueMatt May 28, 2020
9f7bcfb

Drop requirement that all ChannelKeys expose the funding privkey


TheBlueMatt May 28, 2020
1a574d2

Drop requirement that all ChannelKeys expose the payment_point


TheBlueMatt May 28, 2020
d9f5df9

Drop requirement that all ChannelKeys expose revocaion_basepoint


TheBlueMatt May 28, 2020
d77e40f

Drop requirement that ChannelKeys expose delayed_payment_basepoint


TheBlueMatt May 28, 2020
e5a7422

Drop requirement that all ChannelKeys expose htlc_basepoint


TheBlueMatt May 28, 2020

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


29 changes: 6 additions & 23 deletions
lightning/src/chain/keysinterface.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -195,18 +195,6 @@ impl Readable for SpendableOutputDescriptor {
// TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create
// ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors.
pub trait ChannelKeys : Send+Clone {
	/// Gets the private key for the anchor tx
	fn funding_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key for blinded revocation pubkey
	fn revocation_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in the to_remote output of remote commitment tx (ie the
	/// output to us in transactions our counterparty broadcasts).
	/// Also as part of obscured commitment number.
	fn payment_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output
	fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the local htlc secret key used in commitment tx htlc outputs
	fn htlc_base_key<'a>(&'a self) -> &'a SecretKey;
	/// Gets the commitment seed
	fn commitment_seed<'a>(&'a self) -> &'a [u8; 32];
@ariardariardMay 29, 2020

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Future work, should we move commitment_seed out-of-memory and behind the interface by moving inside chan_utils::build_commitment_secret ? In case of undetected node compromise, access to the commitment seed let you derive revocation secret for future non-yet-existent local commitment transactions ?
@TheBlueMattTheBlueMattMay 29, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

Yea, I think we have to. If you want an external signer to prevent the host from burning all the coins, you need some kind of exchange here, but we'd also need to get the ordering right, like, to call get_commitment_revocation_secret we'd have to first provide a copy of the new commitment transaction.
	/// Gets the local channel public keys and basepoints
Expand DownExpand Up@@ -345,17 +333,17 @@ pub trait KeysInterface: Send + Sync {
/// A simple implementation of ChannelKeys that just keeps the private keys in memory.
pub struct InMemoryChannelKeys {
	/// Private key of anchor tx
	funding_key: SecretKey,
pub funding_key: SecretKey,
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

I think even for this concrete implementation, it would be better to not make the private key properties public. Perhaps add a test config constraint?
@TheBlueMattTheBlueMattJun 1, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

You need them to be able to sign transactions when handling SpendableOutput events, so there needs to be some way to get at them.
@valentinewallacevalentinewallaceJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

Don't know the full context, but might it be better to expose a way to use them (e.g. expose a sign_tx method) rather than exposing the keys themselves?
@arik-soarik-soJun 1, 2020

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

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

If the signing only handles in impl for InMemoryChannelKeys, they may not have to be pub, right?
@TheBlueMattTheBlueMattJun 2, 2020

Copy link


Copy Markdown

CollaboratorAuthor
Choose a reason for hiding this comment

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

No, we dont require signing functions for SpendableOutputs events as those are purely external and can happen a month later. eg the use in the tests at https://github.com/rust-bitcoin/rust-lightning/blob/master/lightning/src/ln/functional_tests.rs#L4299
	/// Local secret key for blinded revocation pubkey
	revocation_base_key: SecretKey,
pub revocation_base_key: SecretKey,
	/// Local secret key used for our balance in remote-broadcasted commitment transactions
	payment_key: SecretKey,
pub payment_key: SecretKey,
	/// Local secret key used in HTLC tx
	delayed_payment_base_key: SecretKey,
pub delayed_payment_base_key: SecretKey,
	/// Local htlc secret key used in commitment tx htlc outputs
	htlc_base_key: SecretKey,
pub htlc_base_key: SecretKey,
	/// Commitment seed
	commitment_seed: [u8; 32],
pub commitment_seed: [u8; 32],
	/// Local public keys and basepoints
	pub(crate) local_channel_pubkeys: ChannelPublicKeys,
	/// Remote public keys and base points
Expand DownExpand Up@@ -416,11 +404,6 @@ impl InMemoryChannelKeys {
}

impl ChannelKeys for InMemoryChannelKeys {
	fn funding_key(&self) -> &SecretKey { &self.funding_key }
	fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key }
	fn payment_key(&self) -> &SecretKey { &self.payment_key }
	fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key }
	fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
	fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
	fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys }
	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
Expand Down

65 changes: 32 additions & 33 deletions
lightning/src/ln/channel.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -766,15 +766,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

	fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
 let mut sha = Sha256::engine();
 let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key());

 let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize();
 if self.channel_outbound {
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 sha.input(their_payment_point);
 } else {
 sha.input(their_payment_point);
 sha.input(&our_payment_point.serialize());
 sha.input(&self.local_keys.pubkeys().payment_point.serialize());
 }
 let res = Sha256::from_engine(sha).into_inner();

Expand DownExpand Up@@ -1095,11 +1094,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	/// TODO Some magic rust shit to compile-time check this?
	fn build_local_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
 let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number));
 let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys"))
	}

	#[inline]
Expand All@@ -1109,19 +1108,18 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
	fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
 //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
 //may see payments to it!
 let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key());
 let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key());
 let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint;
 let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint;
 let their_pubkeys = self.their_pubkeys.as_ref().unwrap();

 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys"))
 Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys"))
	}

	/// Gets the redeemscript for the funding transaction output (ie the funding transaction output
	/// pays to get_funding_redeemscript().to_v0_p2wsh()).
	/// Panics if called before accept_channel/new_from_req
	pub fn get_funding_redeemscript(&self) -> Script {
 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());
 make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey())
 make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey())
	}

	/// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
Expand DownExpand Up@@ -1455,7 +1453,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script));
 secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer");

 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());
 let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new());

 let remote_keys = self.build_remote_transaction_keys()?;
 let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0;
Expand DownExpand Up@@ -1568,7 +1566,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
 macro_rules! create_monitor {
 () => { {
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new());
 let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(),
 &self.shutdown_pubkey, self.our_to_self_delay,
 &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
Expand DownExpand Up@@ -1899,7 +1897,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 let mut monitor_update = ChannelMonitorUpdate {
 update_id: self.latest_monitor_update_id,
 updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo {
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source),
 htlc_outputs: htlcs_and_sigs
 }]
 };
Expand DownExpand Up@@ -2825,7 +2823,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {

 tx.input[0].witness.push(Vec::new()); // First is the multisig dummy

 let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize();
 let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize();
 let their_funding_key = self.their_funding_pubkey().serialize();
 if our_funding_key[..] < their_funding_key[..] {
 tx.input[0].witness.push(our_sig.serialize_der().to_vec());
Expand DownExpand Up@@ -3302,6 +3300,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::OpenChannel {
 chain_hash: chain_hash,
Expand All@@ -3315,11 +3314,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 channel_flags: if self.config.announced_channel {1} else {0},
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
Expand All@@ -3338,6 +3337,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number);
 let local_keys = self.local_keys.pubkeys();

 msgs::AcceptChannel {
 temporary_channel_id: self.channel_id,
Expand All@@ -3348,11 +3348,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 minimum_depth: self.minimum_depth,
 to_self_delay: self.our_to_self_delay,
 max_accepted_htlcs: OUR_MAX_HTLCS,
 funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()),
 revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()),
 payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()),
 delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()),
 htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()),
 funding_pubkey: local_keys.funding_pubkey,
 revocation_basepoint: local_keys.revocation_basepoint,
 payment_point: local_keys.payment_point,
 delayed_payment_basepoint: local_keys.delayed_payment_basepoint,
 htlc_basepoint: local_keys.htlc_basepoint,
 first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret),
 shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
 }
Expand DownExpand Up@@ -3431,16 +3431,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 }

 let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..];
 let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key());

 let msg = msgs::UnsignedChannelAnnouncement {
 features: ChannelFeatures::known(),
 chain_hash: chain_hash,
 short_channel_id: self.get_short_channel_id().unwrap(),
 node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() },
 node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id },
 bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key },
 bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() },
 bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey },
 excess_data: Vec::new(),
 };

Expand DownExpand Up@@ -4442,7 +4441,7 @@ mod tests {
 (0, 0)
 );

 assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..],
 assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..],
 hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]);
 let keys_provider = Keys { chan_keys: chan_keys.clone() };

Expand DownExpand Up@@ -4477,11 +4476,11 @@ mod tests {
 // We can't just use build_local_transaction_keys here as the per_commitment_secret is not
 // derived from a commitment_seed, so instead we copy it here and call
 // build_commitment_transaction.
 let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key());
 let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint;
 let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap();
 let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret);
 let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key());
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();
 let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint;
 let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap();

 chan.their_pubkeys = Some(their_pubkeys);

Expand DownExpand Up@@ -4512,7 +4511,7 @@ mod tests {
 })*
 assert_eq!(unsigned_tx.1.len(), per_htlc.len());

 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
 let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap();
 assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig);

Expand Down

2 changes: 1 addition & 1 deletion
lightning/src/ln/channelmonitor.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1641,7 +1641,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 self.remote_payment_script = {
 // Note that the Network here is ignored as we immediately drop the address for the
 // script_pubkey version
 let payment_hash160 = WPubkeyHash::hash(&PublicKey::from_secret_key(&self.secp_ctx, &self.keys.payment_key()).serialize());
 let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize());
 Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script()
 };

Expand Down

10 changes: 5 additions & 5 deletions
lightning/src/ln/functional_tests.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -3850,8 +3850,8 @@ fn test_invalid_channel_announcement() {
	macro_rules! sign_msg {
 ($unsigned_msg: expr) => {
 let msghash = Message::from_slice(&Sha256dHash::hash(&$unsigned_msg.encode()[..])[..]).unwrap();
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key());
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key());
 let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key);
 let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key);
 let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].keys_manager.get_node_secret());
 let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].keys_manager.get_node_secret());
 chan_announcement = msgs::ChannelAnnouncement {
Expand DownExpand Up@@ -4293,10 +4293,10 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 let remotepubkey = PublicKey::from_secret_key(&secp_ctx, &keys.payment_key());
 let remotepubkey = keys.pubkeys().payment_point;
 let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
 let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap();
 let remotesig = secp_ctx.sign(&sighash, &keys.payment_key());
 let remotesig = secp_ctx.sign(&sighash, &keys.inner.payment_key);
 spend_tx.input[0].witness.push(remotesig.serialize_der().to_vec());
 spend_tx.input[0].witness[0].push(SigHashType::All as u8);
 spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec());
Expand All@@ -4321,7 +4321,7 @@ macro_rules! check_spendable_outputs {
 };
 let secp_ctx = Secp256k1::new();
 let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1);
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) {
 if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) {

 let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
 let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey);
Expand Down
Loading