Skip to content

Draft: phase-2 signer API #723

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

Closed
wants to merge 2 commits into from
Closed

Draft: phase-2 signer API #723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5982347
draft KeysInterface.sign_holder_commitment_phase2
devrandom Sep 29, 2020
05a8af6
pass in PreCalculatedTxCreationKeys
devrandom Oct 6, 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
168 changes: 149 additions & 19 deletions lightning/src/chain/keysinterface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@ use util::ser::{Writeable, Writer, Readable};

use chain::transaction::OutPoint;
use ln::chan_utils;
use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, PreCalculatedTxCreationKeys};
use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, PreCalculatedTxCreationKeys, derive_public_key, derive_public_revocation_key, TxCreationKeys};
use ln::msgs::UnsignedChannelAnnouncement;

use std::sync::atomic::{AtomicUsize, Ordering};
use std::io::Error;
use ln::msgs::DecodeError;
use ln::transaction::{CommitmentInfo, build_commitment_tx, get_commitment_transaction_number_obscure_factor};

/// When on-chain outputs are created by rust-lightning (which our counterparty is not able to
/// claim at any point in the future) an event is generated which you must track and be able to
Expand Down Expand Up @@ -79,7 +80,7 @@ pub enum SpendableOutputDescriptor {
///
/// To derive the revocation_pubkey provided here (which is used in the witness
/// script generation), you must pass the counterparty revocation_basepoint (which appears in the
/// call to ChannelKeys::on_accept) and the provided per_commitment point
/// call to ChannelKeys::ready_channel) and the provided per_commitment point
/// to chan_utils::derive_public_revocation_key.
///
/// The witness script which is hashed and included in the output script_pubkey may be
Expand Down Expand Up @@ -244,6 +245,18 @@ pub trait ChannelKeys : Send+Clone {
// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;

/// Create a signature for a holder's commitment transaction and attached HTLC transactions.
fn sign_holder_commitment_phase2<T: secp256k1::Signing + secp256k1::Verification>(
Copy link
Member Author

Choose a reason for hiding this comment

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

This replaces the previous function.

&self,
commitment_number: u64,
feerate_per_kw: u32,
to_holder_value_sat: u64,
to_counterparty_value_sat: u64,
htlcs: &Vec<HTLCOutputInCommitment>,
pre_keys: &PreCalculatedTxCreationKeys,
secp_ctx: &Secp256k1<T>,
) -> Result<(Signature, Vec<Signature>), ()>;

/// Same as sign_holder_commitment, but exists only for tests to get access to holder commitment
/// transactions which will be broadcasted later, after the channel has moved on to a newer
/// state. Thus, needs its own method as sign_holder_commitment may enforce that we only ever
Expand Down Expand Up @@ -319,13 +332,15 @@ pub trait ChannelKeys : Send+Clone {
/// protocol.
fn sign_channel_announcement<T: secp256k1::Signing>(&self, msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()>;

/// Set the counterparty channel basepoints and counterparty_selected/holder_selected_contest_delay.
/// This is done immediately on incoming channels and as soon as the channel is accepted on outgoing channels.
/// Set the counterparty static channel data, including basepoints,
/// counterparty_selected/holder_selected_contest_delay and funding outpoint.
/// This is done as soon as the funding outpoint is known. Since these are static channel data,
/// they MUST NOT be allowed to change to different values once set.
///
/// We bind holder_selected_contest_delay late here for API convenience.
///
/// Will be called before any signatures are applied.
fn on_accept(&mut self, channel_points: &ChannelPublicKeys, counterparty_selected_contest_delay: u16, holder_selected_contest_delay: u16);
fn ready_channel(&mut self, channel_points: &ChannelPublicKeys, counterparty_selected_contest_delay: u16, holder_selected_contest_delay: u16, is_outbound: bool, funding_outpoint: &OutPoint);
}

/// A trait to describe an object which can get user secrets and key material.
Expand All @@ -349,11 +364,11 @@ pub trait KeysInterface: Send + Sync {
}

#[derive(Clone)]
/// Holds late-bound channel data.
/// This data is available after the channel is known to be accepted, either
/// when receiving an open_channel for an inbound channel or when
/// receiving accept_channel for an outbound channel.
struct AcceptedChannelData {
/// Holds late-bound static channel data.
/// This data is available after the channel is readied, either
/// when receiving an funding_created for an inbound channel or when
/// creating a funding transaction for an outbound channel.
struct StaticChannelData {
/// Counterparty public keys and base points
counterparty_channel_pubkeys: ChannelPublicKeys,
/// The contest_delay value specified by our counterparty and applied on holder-broadcastable
Expand All @@ -365,6 +380,8 @@ struct AcceptedChannelData {
/// by our counterparty, ie the amount of time that they have to wait to recover their funds
/// if they broadcast a transaction.
holder_selected_contest_delay: u16,
/// Whether the holder is the initiator of this channel
is_outbound: bool,
}

#[derive(Clone)]
Expand All @@ -385,7 +402,9 @@ pub struct InMemoryChannelKeys {
/// Holder public keys and basepoints
pub(crate) holder_channel_pubkeys: ChannelPublicKeys,
/// Counterparty public keys and counterparty/holder selected_contest_delay, populated on channel acceptance
accepted_channel_data: Option<AcceptedChannelData>,
accepted_channel_data: Option<StaticChannelData>,
/// Funding outpoint, populated on channel funding creation
funding_outpoint: Option<OutPoint>,
/// The total value of this channel
channel_value_satoshis: u64,
/// Key derivation parameters
Expand Down Expand Up @@ -418,6 +437,7 @@ impl InMemoryChannelKeys {
channel_value_satoshis,
holder_channel_pubkeys,
accepted_channel_data: None,
funding_outpoint: None,
key_derivation_params,
}
}
Expand All @@ -439,21 +459,83 @@ impl InMemoryChannelKeys {
}

/// Counterparty pubkeys.
/// Will panic if on_accept wasn't called.
/// Will panic if ready_channel wasn't called.
pub fn counterparty_pubkeys(&self) -> &ChannelPublicKeys { &self.accepted_channel_data.as_ref().unwrap().counterparty_channel_pubkeys }

/// The contest_delay value specified by our counterparty and applied on holder-broadcastable
/// transactions, ie the amount of time that we have to wait to recover our funds if we
/// broadcast a transaction. You'll likely want to pass this to the
/// ln::chan_utils::build*_transaction functions when signing holder's transactions.
/// Will panic if on_accept wasn't called.
/// Will panic if ready_channel wasn't called.
pub fn counterparty_selected_contest_delay(&self) -> u16 { self.accepted_channel_data.as_ref().unwrap().counterparty_selected_contest_delay }

/// The contest_delay value specified by us and applied on transactions broadcastable
/// by our counterparty, ie the amount of time that they have to wait to recover their funds
/// if they broadcast a transaction.
/// Will panic if on_accept wasn't called.
/// Will panic if ready_channel wasn't called.
pub fn holder_selected_contest_delay(&self) -> u16 { self.accepted_channel_data.as_ref().unwrap().holder_selected_contest_delay }

/// Whether the holder is the initiator
pub fn is_outbound(&self) -> bool { self.accepted_channel_data.as_ref().unwrap().is_outbound }

/// Funding outpoint
/// Will panic if ready_channel wasn't called.
pub fn funding_outpoint(&self) -> &OutPoint { self.funding_outpoint.as_ref().unwrap() }

/// Prepare build information for a commitment tx that the holder can broadcast
pub fn build_holder_commitment_tx<T: secp256k1::Signing + secp256k1::Verification>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this need to be in the public interface, or are you imaging this is only a utility function in the InMemory version?

&self,
commitment_number: u64,
per_commitment_point: &PublicKey,
to_holder_value_sat: u64,
to_counterparty_value_sat: u64,
htlcs: &Vec<HTLCOutputInCommitment>,
keys: &TxCreationKeys,
secp_ctx: &Secp256k1<T>,
) -> Result<(bitcoin::Transaction, Vec<HTLCOutputInCommitment>, Vec<Script>), ()> {
let counterparty_points = self.counterparty_pubkeys();

let to_holder_delayed_pubkey = derive_public_key(
&secp_ctx,
&per_commitment_point,
&self.pubkeys().delayed_payment_basepoint,
).map_err(|_| ())?;

let revocation_pubkey = derive_public_revocation_key(
&secp_ctx,
&per_commitment_point,
&counterparty_points.revocation_basepoint,
).map_err(|_| ())?;

let info = CommitmentInfo {
is_counterparty_broadcaster: false,
to_countersigner_pubkey: counterparty_points.payment_point,
to_countersigner_value_sat: to_counterparty_value_sat,
revocation_pubkey,
to_broadcaster_delayed_pubkey: to_holder_delayed_pubkey,
to_broadcaster_value_sat: to_holder_value_sat,
to_self_delay: self.counterparty_selected_contest_delay(),
htlcs: htlcs.clone(),
};

let obscured_commitment_transaction_number =
self.get_commitment_transaction_number_obscure_factor() ^ commitment_number;
let (tx, htlcs, scripts) = build_commitment_tx(
&keys,
&info,
obscured_commitment_transaction_number,
self.funding_outpoint().into_bitcoin_outpoint(),
);
Ok((tx, htlcs, scripts))
}

fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
get_commitment_transaction_number_obscure_factor(
&self.pubkeys().payment_point,
&self.counterparty_pubkeys().payment_point,
self.is_outbound(),
)
}
}

impl ChannelKeys for InMemoryChannelKeys {
Expand Down Expand Up @@ -507,6 +589,48 @@ impl ChannelKeys for InMemoryChannelKeys {
Ok(holder_commitment_tx.get_holder_sig(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
}

fn sign_holder_commitment_phase2<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_number: u64, feerate_per_kw: u32, to_holder_value_sat: u64, to_counterparty_value_sat: u64, htlcs: &Vec<HTLCOutputInCommitment>, pre_keys: &PreCalculatedTxCreationKeys, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
let per_commitment_point = self.get_per_commitment_point(commitment_number, secp_ctx);
let keys = pre_keys.trust_key_derivation();
let (tx, htlcs, _) = self.build_holder_commitment_tx(
commitment_number,
&per_commitment_point,
to_holder_value_sat,
to_counterparty_value_sat,
htlcs,
keys,
secp_ctx,
)?;

let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
let sighash = hash_to_message!(&bip143::SigHashCache::new(&tx)
.signature_hash(0, &funding_redeemscript, self.channel_value_satoshis, SigHashType::All)[..]);
let sig = secp_ctx.sign(&sighash, &self.funding_key);

// We provide a dummy signature for the remote, since we don't require that sig
// to be passed in to this call. It would have been better if HolderCommitmentTransaction
// didn't require the remote sig.
let dummy_sig = Secp256k1::new().sign(
Copy link
Member Author

Choose a reason for hiding this comment

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

This awkwardness is due to HolderCommitmentTransaction holding counterparty signatures. Not sure what's the best thing here - perhaps separate the signature holding to another struct?

Copy link

Choose a reason for hiding this comment

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

I think we would like to get ride of HolderCommitmentTransaction. It's API can be enforced either by ChannelMonitor or an external signer implementation like InMemoryChannelKeys.

All dynamic state (balances, HTLCs, ...) would be stored in Channel and the rest assumed to be in signer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see why? HolderCommitmentTransaction, indeed, holds too much data today (specifically, the transaction itself), but communicating things like the counterparty signatures and the per htlc values has to be done. eg here the function signature has a ton of fields, that may be usefully just shoved into a struct, and HolderCommitmentTransaction is basically that. (I assume we agree here, just noting that we may end up wanting a struct either way).

&secp256k1::Message::from_slice(&[42; 32]).unwrap(),
&SecretKey::from_slice(&[42; 32]).unwrap(),
);
let htlcs_with_sig = htlcs.iter().map(|h| (h.clone(), Some(dummy_sig.clone()))).collect();
let commitment_tx = HolderCommitmentTransaction::new_missing_holder_sig(
tx,
dummy_sig,
&self.pubkeys().funding_pubkey,
&self.counterparty_pubkeys().funding_pubkey,
keys.clone(),
feerate_per_kw,
htlcs_with_sig,
);
let htlc_sigs_o = self.sign_holder_commitment_htlc_transactions(&commitment_tx, secp_ctx)?;
let htlc_sigs = htlc_sigs_o.iter().map(|o| o.unwrap()).collect();

Ok((sig, htlc_sigs))
}

#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
Expand Down Expand Up @@ -588,18 +712,21 @@ impl ChannelKeys for InMemoryChannelKeys {
Ok(secp_ctx.sign(&msghash, &self.funding_key))
}

fn on_accept(&mut self, channel_pubkeys: &ChannelPublicKeys, counterparty_selected_contest_delay: u16, holder_selected_contest_delay: u16) {
assert!(self.accepted_channel_data.is_none(), "Already accepted");
self.accepted_channel_data = Some(AcceptedChannelData {
fn ready_channel(&mut self, channel_pubkeys: &ChannelPublicKeys, counterparty_selected_contest_delay: u16, holder_selected_contest_delay: u16, is_outbound: bool, funding_outpoint: &OutPoint) {
assert!(self.accepted_channel_data.is_none(), "Acceptance already noted");
self.accepted_channel_data = Some(StaticChannelData {
counterparty_channel_pubkeys: channel_pubkeys.clone(),
counterparty_selected_contest_delay,
holder_selected_contest_delay,
is_outbound,
});
assert!(self.funding_outpoint.is_none(), "Funding creation already noted");
self.funding_outpoint = Some(funding_outpoint.clone());
}
}

impl_writeable!(AcceptedChannelData, 0,
{ counterparty_channel_pubkeys, counterparty_selected_contest_delay, holder_selected_contest_delay });
impl_writeable!(StaticChannelData, 0,
{ counterparty_channel_pubkeys, counterparty_selected_contest_delay, holder_selected_contest_delay, is_outbound });

impl Writeable for InMemoryChannelKeys {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
Expand All @@ -610,6 +737,7 @@ impl Writeable for InMemoryChannelKeys {
self.htlc_base_key.write(writer)?;
self.commitment_seed.write(writer)?;
self.accepted_channel_data.write(writer)?;
self.funding_outpoint.write(writer)?;
self.channel_value_satoshis.write(writer)?;
self.key_derivation_params.0.write(writer)?;
self.key_derivation_params.1.write(writer)?;
Expand All @@ -627,6 +755,7 @@ impl Readable for InMemoryChannelKeys {
let htlc_base_key = Readable::read(reader)?;
let commitment_seed = Readable::read(reader)?;
let counterparty_channel_data = Readable::read(reader)?;
let funding_outpoint = Readable::read(reader)?;
let channel_value_satoshis = Readable::read(reader)?;
let secp_ctx = Secp256k1::signing_only();
let holder_channel_pubkeys =
Expand All @@ -646,6 +775,7 @@ impl Readable for InMemoryChannelKeys {
channel_value_satoshis,
holder_channel_pubkeys,
accepted_channel_data: counterparty_channel_data,
funding_outpoint,
key_derivation_params: (params_1, params_2),
})
}
Expand Down
16 changes: 15 additions & 1 deletion lightning/src/ln/chan_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,8 @@ impl_writeable!(ChannelPublicKeys, 33*5, {


impl TxCreationKeys {
/// Create a new TxCreationKeys from channel base points and the per-commitment point
/// Create per-state keys from channel base points and the per-commitment point
/// Key set is asymmetric and can't be used as part of counter-signatory set of transactions.
pub fn derive_new<T: secp256k1::Signing + secp256k1::Verification>(secp_ctx: &Secp256k1<T>, per_commitment_point: &PublicKey, broadcaster_delayed_payment_base: &PublicKey, broadcaster_htlc_base: &PublicKey, countersignatory_revocation_base: &PublicKey, countersignatory_htlc_base: &PublicKey) -> Result<TxCreationKeys, SecpError> {
Ok(TxCreationKeys {
per_commitment_point: per_commitment_point.clone(),
Expand All @@ -382,6 +383,19 @@ impl TxCreationKeys {
broadcaster_delayed_payment_key: derive_public_key(&secp_ctx, &per_commitment_point, &broadcaster_delayed_payment_base)?,
})
}

/// Generate per-state keys from channel static keys.
/// Key set is asymmetric and can't be used as part of counter-signatory set of transactions.
pub fn make_tx_keys<T: secp256k1::Signing + secp256k1::Verification>(per_commitment_point: &PublicKey, broadcaster_keys: &ChannelPublicKeys, countersignatory_keys: &ChannelPublicKeys, secp_ctx: &Secp256k1<T>) -> Result<TxCreationKeys, SecpError> {
TxCreationKeys::derive_new(
&secp_ctx,
&per_commitment_point,
&broadcaster_keys.delayed_payment_basepoint,
&broadcaster_keys.htlc_basepoint,
&countersignatory_keys.revocation_basepoint,
&countersignatory_keys.htlc_basepoint,
)
}
}

/// A script either spendable by the revocation
Expand Down
10 changes: 5 additions & 5 deletions lightning/src/ln/channel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,15 +582,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
where K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
F::Target: FeeEstimator
{
let mut chan_keys = keys_provider.get_channel_keys(true, msg.funding_satoshis);
let chan_keys = keys_provider.get_channel_keys(true, msg.funding_satoshis);
let counterparty_pubkeys = ChannelPublicKeys {
funding_pubkey: msg.funding_pubkey,
revocation_basepoint: msg.revocation_basepoint,
payment_point: msg.payment_point,
delayed_payment_basepoint: msg.delayed_payment_basepoint,
htlc_basepoint: msg.htlc_basepoint
};
chan_keys.on_accept(&counterparty_pubkeys, msg.to_self_delay, config.own_channel_config.our_to_self_delay);
let mut local_config = (*config).channel_options.clone();

if config.own_channel_config.our_to_self_delay < BREAKDOWN_TIMEOUT {
Expand Down Expand Up @@ -1463,7 +1462,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
htlc_basepoint: msg.htlc_basepoint
};

self.holder_keys.on_accept(&counterparty_pubkeys, msg.to_self_delay, self.holder_selected_contest_delay);
self.counterparty_pubkeys = Some(counterparty_pubkeys);

self.counterparty_cur_commitment_point = Some(msg.first_per_commitment_point);
Expand Down Expand Up @@ -1518,6 +1516,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
}

let funding_txo = OutPoint{ txid: msg.funding_txid, index: msg.funding_output_index };
self.holder_keys.ready_channel(self.counterparty_pubkeys.as_ref().unwrap(), self.counterparty_selected_contest_delay, self.holder_selected_contest_delay, self.channel_outbound, &funding_txo);
self.funding_txo = Some(funding_txo.clone());

let (counterparty_initial_commitment_tx, initial_commitment_tx, signature) = match self.funding_created_signature(&msg.signature, logger) {
Expand Down Expand Up @@ -3545,6 +3544,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
}

self.holder_keys.ready_channel(self.counterparty_pubkeys.as_ref().unwrap(), self.counterparty_selected_contest_delay, self.holder_selected_contest_delay, self.channel_outbound, &funding_txo);
self.funding_txo = Some(funding_txo.clone());
let signature = match self.get_outbound_funding_created_signature(logger) {
Ok(res) => res,
Expand Down Expand Up @@ -4647,7 +4647,7 @@ mod tests {
chan.holder_dust_limit_satoshis = 546;

let funding_info = OutPoint{ txid: Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), index: 0 };
chan.funding_txo = Some(funding_info);
chan.funding_txo = Some(funding_info.clone());

let counterparty_pubkeys = ChannelPublicKeys {
funding_pubkey: public_from_secret_hex(&secp_ctx, "1552dfba4f6cf29a62a0af13c8d6981d36d0ef8d61ba10fb0fe90da7634d7e13"),
Expand All @@ -4656,7 +4656,7 @@ mod tests {
delayed_payment_basepoint: public_from_secret_hex(&secp_ctx, "1552dfba4f6cf29a62a0af13c8d6981d36d0ef8d61ba10fb0fe90da7634d7e13"),
htlc_basepoint: public_from_secret_hex(&secp_ctx, "4444444444444444444444444444444444444444444444444444444444444444")
};
chan_keys.on_accept(&counterparty_pubkeys, chan.counterparty_selected_contest_delay, chan.holder_selected_contest_delay);
chan_keys.ready_channel(&counterparty_pubkeys, chan.counterparty_selected_contest_delay, chan.holder_selected_contest_delay, false, &funding_info);

assert_eq!(counterparty_pubkeys.payment_point.serialize()[..],
hex::decode("032c0b7cf95324a07d05398b240174dc0c2be444d96b159aa6c7f7b1e668680991").unwrap()[..]);
Expand Down
1 change: 1 addition & 0 deletions lightning/src/ln/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ pub mod peer_handler;
pub mod chan_utils;
pub mod features;
pub(crate) mod onchaintx;
pub mod transaction;

#[cfg(feature = "fuzztarget")]
pub mod peer_channel_encryptor;
Expand Down
Loading