set dir readonly false
[rust-lightning] / lightning / src / chain / keysinterface.rs
index 1cc41b0f8fa6882f498997eb2840f6ac605f9bdc..da5a99ddbb76a92bd7aaeef65ab0b574b136bbf1 100644 (file)
@@ -33,7 +33,7 @@ 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, ChannelTransactionParameters, CommitmentTransaction};
 use ln::msgs::UnsignedChannelAnnouncement;
 
 use std::sync::atomic::{AtomicUsize, Ordering};
@@ -45,11 +45,11 @@ use ln::msgs::DecodeError;
 /// spend on-chain. The information needed to do this is provided in this enum, including the
 /// outpoint describing which txid and output index is available, the full output which exists at
 /// that txid/index, and any keys or other information required to sign.
-#[derive(Clone, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 pub enum SpendableOutputDescriptor {
-       /// An output to a script which was provided via KeysInterface, thus you should already know
-       /// how to spend it. No keys are provided as rust-lightning was never given any keys - only the
-       /// script_pubkey as it appears in the output.
+       /// An output to a script which was provided via KeysInterface directly, either from
+       /// `get_destination_script()` or `get_shutdown_pubkey()`, thus you should already know how to
+       /// spend it. No secret keys are provided as rust-lightning was never given any key.
        /// These may include outputs from a transaction punishing our counterparty or claiming an HTLC
        /// on-chain using the payment preimage or after it has timed out.
        StaticOutput {
@@ -79,7 +79,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
@@ -98,11 +98,14 @@ pub enum SpendableOutputDescriptor {
                to_self_delay: u16,
                /// The output which is referenced by the given outpoint
                output: TxOut,
-               /// The channel keys state used to proceed to derivation of signing key. Must
-               /// be pass to KeysInterface::derive_channel_keys.
-               key_derivation_params: (u64, u64),
                /// The revocation_pubkey used to derive witnessScript
-               revocation_pubkey: PublicKey
+               revocation_pubkey: PublicKey,
+               /// Arbitrary identification information returned by a call to
+               /// `ChannelKeys::channel_keys_id()`. This may be useful in re-deriving keys used in
+               /// the channel to spend the output.
+               channel_keys_id: [u8; 32],
+               /// The value of the channel which this output originated from, possibly indirectly.
+               channel_value_satoshis: u64,
        },
        /// An output to a P2WPKH, spendable exclusively by our payment key (ie the private key which
        /// corresponds to the public key in ChannelKeys::pubkeys().payment_point).
@@ -116,9 +119,12 @@ pub enum SpendableOutputDescriptor {
                outpoint: OutPoint,
                /// The output which is reference by the given outpoint
                output: TxOut,
-               /// The channel keys state used to proceed to derivation of signing key. Must
-               /// be pass to KeysInterface::derive_channel_keys.
-               key_derivation_params: (u64, u64),
+               /// Arbitrary identification information returned by a call to
+               /// `ChannelKeys::channel_keys_id()`. This may be useful in re-deriving keys used in
+               /// the channel to spend the output.
+               channel_keys_id: [u8; 32],
+               /// The value of the channel which this transactions spends.
+               channel_value_satoshis: u64,
        }
 }
 
@@ -130,22 +136,22 @@ impl Writeable for SpendableOutputDescriptor {
                                outpoint.write(writer)?;
                                output.write(writer)?;
                        },
-                       &SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref key_derivation_params, ref revocation_pubkey } => {
+                       &SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref revocation_pubkey, ref channel_keys_id, channel_value_satoshis } => {
                                1u8.write(writer)?;
                                outpoint.write(writer)?;
                                per_commitment_point.write(writer)?;
                                to_self_delay.write(writer)?;
                                output.write(writer)?;
-                               key_derivation_params.0.write(writer)?;
-                               key_derivation_params.1.write(writer)?;
                                revocation_pubkey.write(writer)?;
+                               channel_keys_id.write(writer)?;
+                               channel_value_satoshis.write(writer)?;
                        },
-                       &SpendableOutputDescriptor::StaticOutputCounterpartyPayment { ref outpoint, ref output, ref key_derivation_params } => {
+                       &SpendableOutputDescriptor::StaticOutputCounterpartyPayment { ref outpoint, ref output, ref channel_keys_id, channel_value_satoshis } => {
                                2u8.write(writer)?;
                                outpoint.write(writer)?;
                                output.write(writer)?;
-                               key_derivation_params.0.write(writer)?;
-                               key_derivation_params.1.write(writer)?;
+                               channel_keys_id.write(writer)?;
+                               channel_value_satoshis.write(writer)?;
                        },
                }
                Ok(())
@@ -164,13 +170,15 @@ impl Readable for SpendableOutputDescriptor {
                                per_commitment_point: Readable::read(reader)?,
                                to_self_delay: Readable::read(reader)?,
                                output: Readable::read(reader)?,
-                               key_derivation_params: (Readable::read(reader)?, Readable::read(reader)?),
                                revocation_pubkey: Readable::read(reader)?,
+                               channel_keys_id: Readable::read(reader)?,
+                               channel_value_satoshis: Readable::read(reader)?,
                        }),
                        2u8 => Ok(SpendableOutputDescriptor::StaticOutputCounterpartyPayment {
                                outpoint: Readable::read(reader)?,
                                output: Readable::read(reader)?,
-                               key_derivation_params: (Readable::read(reader)?, Readable::read(reader)?),
+                               channel_keys_id: Readable::read(reader)?,
+                               channel_value_satoshis: Readable::read(reader)?,
                        }),
                        _ => Err(DecodeError::InvalidValue),
                }
@@ -204,7 +212,7 @@ impl Readable for SpendableOutputDescriptor {
 // routine).
 // 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 {
+pub trait ChannelKeys : Send+Clone + Writeable {
        /// Gets the per-commitment point for a specific commitment number
        ///
        /// Note that the commitment number starts at (1 << 48) - 1 and counts backwards.
@@ -221,50 +229,40 @@ pub trait ChannelKeys : Send+Clone {
        fn release_commitment_secret(&self, idx: u64) -> [u8; 32];
        /// Gets the holder's channel public keys and basepoints
        fn pubkeys(&self) -> &ChannelPublicKeys;
-       /// Gets arbitrary identifiers describing the set of keys which are provided back to you in
-       /// some SpendableOutputDescriptor types. These should be sufficient to identify this
+       /// Gets an arbitrary identifier describing the set of keys which are provided back to you in
+       /// some SpendableOutputDescriptor types. This should be sufficient to identify this
        /// ChannelKeys object uniquely and lookup or re-derive its keys.
-       fn key_derivation_params(&self) -> (u64, u64);
+       fn channel_keys_id(&self) -> [u8; 32];
 
        /// Create a signature for a counterparty's commitment transaction and associated HTLC transactions.
        ///
        /// Note that if signing fails or is rejected, the channel will be force-closed.
        //
        // TODO: Document the things someone using this interface should enforce before signing.
-       // TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
-       // making the callee generate it via some util function we expose)!
-       fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, feerate_per_kw: u32, commitment_tx: &Transaction, keys: &PreCalculatedTxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
-
-       /// Create a signature for a holder's commitment transaction. This will only ever be called with
-       /// the same holder_commitment_tx (or a copy thereof), though there are currently no guarantees
-       /// that it will not be called multiple times.
+       fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
+
+       /// Create a signatures for a holder's commitment transaction and its claiming HTLC transactions.
+       /// This will only ever be called with a non-revoked commitment_tx.  This will be called with the
+       /// latest commitment_tx when we initiate a force-close.
+       /// This will be called with the previous latest, just to get claiming HTLC signatures, if we are
+       /// reacting to a ChannelMonitor replica that decided to broadcast before it had been updated to
+       /// the latest.
+       /// This may be called multiple times for the same transaction.
+       ///
        /// An external signer implementation should check that the commitment has not been revoked.
+       ///
+       /// May return Err if key derivation fails.  Callers, such as ChannelMonitor, will panic in such a case.
        //
        // TODO: Document the things someone using this interface should enforce before signing.
-       // 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, ()>;
+       // TODO: Key derivation failure should panic rather than Err
+       fn sign_holder_commitment_and_htlcs<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, 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
        /// get called once.
        #[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, ()>;
-
-       /// Create a signature for each HTLC transaction spending a holder's commitment transaction.
-       ///
-       /// Unlike sign_holder_commitment, this may be called multiple times with *different*
-       /// holder_commitment_tx values. While this will never be called with a revoked
-       /// holder_commitment_tx, it is possible that it is called with the second-latest
-       /// holder_commitment_tx (only if we haven't yet revoked it) if some watchtower/secondary
-       /// ChannelMonitor decided to broadcast before it had been updated to the latest.
-       ///
-       /// Either an Err should be returned, or a Vec with one entry for each HTLC which exists in
-       /// holder_commitment_tx. For those HTLCs which have transaction_output_index set to None
-       /// (implying they were considered dust at the time the commitment transaction was negotiated),
-       /// a corresponding None should be included in the return value. All other positions in the
-       /// return value must contain a signature.
-       fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()>;
+       fn unsafe_sign_holder_commitment_and_htlcs<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
 
        /// Create a signature for the given input in a transaction spending an HTLC or commitment
        /// transaction output when our counterparty broadcasts an old state.
@@ -319,13 +317,17 @@ 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.
+       ///
+       /// channel_parameters.is_populated() MUST be true.
        ///
        /// 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_parameters: &ChannelTransactionParameters);
 }
 
 /// A trait to describe an object which can get user secrets and key material.
@@ -333,42 +335,47 @@ pub trait KeysInterface: Send + Sync {
        /// A type which implements ChannelKeys which will be returned by get_channel_keys.
        type ChanKeySigner : ChannelKeys;
 
-       /// Get node secret key (aka node_id or network_key)
+       /// Get node secret key (aka node_id or network_key).
+       ///
+       /// This method must return the same value each time it is called.
        fn get_node_secret(&self) -> SecretKey;
-       /// Get destination redeemScript to encumber static protocol exit points.
+       /// Get a script pubkey which we send funds to when claiming on-chain contestable outputs.
+       ///
+       /// This method should return a different value each time it is called, to avoid linking
+       /// on-chain funds across channels as controlled to the same user.
        fn get_destination_script(&self) -> Script;
-       /// Get shutdown_pubkey to use as PublicKey at channel closure
+       /// Get a public key which we will send funds to (in the form of a P2WPKH output) when closing
+       /// a channel.
+       ///
+       /// This method should return a different value each time it is called, to avoid linking
+       /// on-chain funds across channels as controlled to the same user.
        fn get_shutdown_pubkey(&self) -> PublicKey;
        /// Get a new set of ChannelKeys for per-channel secrets. These MUST be unique even if you
        /// restarted with some stale data!
+       ///
+       /// This method must return a different value each time it is called.
        fn get_channel_keys(&self, inbound: bool, channel_value_satoshis: u64) -> Self::ChanKeySigner;
        /// Gets a unique, cryptographically-secure, random 32 byte value. This is used for encrypting
        /// onion packets and for temporary channel IDs. There is no requirement that these be
        /// persisted anywhere, though they must be unique across restarts.
+       ///
+       /// This method must return a different value each time it is called.
        fn get_secure_random_bytes(&self) -> [u8; 32];
-}
 
-#[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 {
-       /// Counterparty public keys and base points
-       counterparty_channel_pubkeys: ChannelPublicKeys,
-       /// 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.
-       counterparty_selected_contest_delay: u16,
-       /// 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.
-       holder_selected_contest_delay: u16,
+       /// Reads a `ChanKeySigner` for this `KeysInterface` from the given input stream.
+       /// This is only called during deserialization of other objects which contain
+       /// `ChannelKeys`-implementing objects (ie `ChannelMonitor`s and `ChannelManager`s).
+       /// The bytes are exactly those which `<Self::ChanKeySigner as Writeable>::write()` writes, and
+       /// contain no versioning scheme. You may wish to include your own version prefix and ensure
+       /// you've read all of the provided bytes to ensure no corruption occurred.
+       fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::ChanKeySigner, DecodeError>;
 }
 
 #[derive(Clone)]
 /// A simple implementation of ChannelKeys that just keeps the private keys in memory.
+///
+/// This implementation performs no policy checks and is insufficient by itself as
+/// a secure external signer.
 pub struct InMemoryChannelKeys {
        /// Private key of anchor tx
        pub funding_key: SecretKey,
@@ -385,11 +392,11 @@ 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>,
+       channel_parameters: Option<ChannelTransactionParameters>,
        /// The total value of this channel
        channel_value_satoshis: u64,
        /// Key derivation parameters
-       key_derivation_params: (u64, u64),
+       channel_keys_id: [u8; 32],
 }
 
 impl InMemoryChannelKeys {
@@ -403,7 +410,7 @@ impl InMemoryChannelKeys {
                htlc_base_key: SecretKey,
                commitment_seed: [u8; 32],
                channel_value_satoshis: u64,
-               key_derivation_params: (u64, u64)) -> InMemoryChannelKeys {
+               channel_keys_id: [u8; 32]) -> InMemoryChannelKeys {
                let holder_channel_pubkeys =
                        InMemoryChannelKeys::make_holder_keys(secp_ctx, &funding_key, &revocation_base_key,
                                                             &payment_key, &delayed_payment_base_key,
@@ -417,8 +424,8 @@ impl InMemoryChannelKeys {
                        commitment_seed,
                        channel_value_satoshis,
                        holder_channel_pubkeys,
-                       accepted_channel_data: None,
-                       key_derivation_params,
+                       channel_parameters: None,
+                       channel_keys_id,
                }
        }
 
@@ -439,21 +446,36 @@ impl InMemoryChannelKeys {
        }
 
        /// Counterparty pubkeys.
-       /// Will panic if on_accept wasn't called.
-       pub fn counterparty_pubkeys(&self) -> &ChannelPublicKeys { &self.accepted_channel_data.as_ref().unwrap().counterparty_channel_pubkeys }
+       /// Will panic if ready_channel wasn't called.
+       pub fn counterparty_pubkeys(&self) -> &ChannelPublicKeys { &self.get_channel_parameters().counterparty_parameters.as_ref().unwrap().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.
-       pub fn counterparty_selected_contest_delay(&self) -> u16 { self.accepted_channel_data.as_ref().unwrap().counterparty_selected_contest_delay }
+       /// broadcast a transaction.
+       /// Will panic if ready_channel wasn't called.
+       pub fn counterparty_selected_contest_delay(&self) -> u16 { self.get_channel_parameters().counterparty_parameters.as_ref().unwrap().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.
-       pub fn holder_selected_contest_delay(&self) -> u16 { self.accepted_channel_data.as_ref().unwrap().holder_selected_contest_delay }
+       /// Will panic if ready_channel wasn't called.
+       pub fn holder_selected_contest_delay(&self) -> u16 { self.get_channel_parameters().holder_selected_contest_delay }
+
+       /// Whether the holder is the initiator
+       /// Will panic if ready_channel wasn't called.
+       pub fn is_outbound(&self) -> bool { self.get_channel_parameters().is_outbound_from_holder }
+
+       /// Funding outpoint
+       /// Will panic if ready_channel wasn't called.
+       pub fn funding_outpoint(&self) -> &OutPoint { self.get_channel_parameters().funding_outpoint.as_ref().unwrap() }
+
+       /// Obtain a ChannelTransactionParameters for this channel, to be used when verifying or
+       /// building transactions.
+       ///
+       /// Will panic if ready_channel wasn't called.
+       pub fn get_channel_parameters(&self) -> &ChannelTransactionParameters {
+               self.channel_parameters.as_ref().unwrap()
+       }
 }
 
 impl ChannelKeys for InMemoryChannelKeys {
@@ -467,58 +489,53 @@ impl ChannelKeys for InMemoryChannelKeys {
        }
 
        fn pubkeys(&self) -> &ChannelPublicKeys { &self.holder_channel_pubkeys }
-       fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
+       fn channel_keys_id(&self) -> [u8; 32] { self.channel_keys_id }
 
-       fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, feerate_per_kw: u32, commitment_tx: &Transaction, pre_keys: &PreCalculatedTxCreationKeys, htlcs: &[&HTLCOutputInCommitment], secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
-               if commitment_tx.input.len() != 1 { return Err(()); }
-               let keys = pre_keys.trust_key_derivation();
+       fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
+               let trusted_tx = commitment_tx.trust();
+               let keys = trusted_tx.keys();
 
                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
-               let accepted_data = self.accepted_channel_data.as_ref().expect("must accept before signing");
-               let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &accepted_data.counterparty_channel_pubkeys.funding_pubkey);
-
-               let commitment_sighash = hash_to_message!(&bip143::SigHashCache::new(commitment_tx).signature_hash(0, &channel_funding_redeemscript, self.channel_value_satoshis, SigHashType::All)[..]);
-               let commitment_sig = secp_ctx.sign(&commitment_sighash, &self.funding_key);
-
-               let commitment_txid = commitment_tx.txid();
-
-               let mut htlc_sigs = Vec::with_capacity(htlcs.len());
-               for ref htlc in htlcs {
-                       if let Some(_) = htlc.transaction_output_index {
-                               let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, feerate_per_kw, accepted_data.holder_selected_contest_delay, htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
-                               let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
-                               let htlc_sighash = hash_to_message!(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, htlc.amount_msat / 1000, SigHashType::All)[..]);
-                               let our_htlc_key = match chan_utils::derive_private_key(&secp_ctx, &keys.per_commitment_point, &self.htlc_base_key) {
-                                       Ok(s) => s,
-                                       Err(_) => return Err(()),
-                               };
-                               htlc_sigs.push(secp_ctx.sign(&htlc_sighash, &our_htlc_key));
-                       }
+               let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
+
+               let built_tx = trusted_tx.built_transaction();
+               let commitment_sig = built_tx.sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+               let commitment_txid = built_tx.txid;
+
+               let mut htlc_sigs = Vec::with_capacity(commitment_tx.htlcs().len());
+               for htlc in commitment_tx.htlcs() {
+                       let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_tx.feerate_per_kw(), self.holder_selected_contest_delay(), htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
+                       let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
+                       let htlc_sighash = hash_to_message!(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, htlc.amount_msat / 1000, SigHashType::All)[..]);
+                       let holder_htlc_key = match chan_utils::derive_private_key(&secp_ctx, &keys.per_commitment_point, &self.htlc_base_key) {
+                               Ok(s) => s,
+                               Err(_) => return Err(()),
+                       };
+                       htlc_sigs.push(secp_ctx.sign(&htlc_sighash, &holder_htlc_key));
                }
 
                Ok((commitment_sig, htlc_sigs))
        }
 
-       fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
+       fn sign_holder_commitment_and_htlcs<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
-               let counterparty_channel_data = self.accepted_channel_data.as_ref().expect("must accept before signing");
-               let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_channel_data.counterparty_channel_pubkeys.funding_pubkey);
-
-               Ok(holder_commitment_tx.get_holder_sig(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
+               let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
+               let trusted_tx = commitment_tx.trust();
+               let sig = trusted_tx.built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+               let channel_parameters = self.get_channel_parameters();
+               let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?;
+               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, ()> {
+       fn unsafe_sign_holder_commitment_and_htlcs<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
-               let counterparty_channel_pubkeys = &self.accepted_channel_data.as_ref().expect("must accept before signing").counterparty_channel_pubkeys;
-               let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_channel_pubkeys.funding_pubkey);
-
-               Ok(holder_commitment_tx.get_holder_sig(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
-       }
-
-       fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
-               let counterparty_selected_contest_delay = self.accepted_channel_data.as_ref().unwrap().counterparty_selected_contest_delay;
-               holder_commitment_tx.get_htlc_sigs(&self.htlc_base_key, counterparty_selected_contest_delay, secp_ctx)
+               let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
+               let trusted_tx = commitment_tx.trust();
+               let sig = trusted_tx.built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+               let channel_parameters = self.get_channel_parameters();
+               let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?;
+               Ok((sig, htlc_sigs))
        }
 
        fn sign_justice_transaction<T: secp256k1::Signing + secp256k1::Verification>(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &Option<HTLCOutputInCommitment>, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
@@ -575,8 +592,7 @@ impl ChannelKeys for InMemoryChannelKeys {
                if closing_tx.output.len() > 2 { return Err(()); }
 
                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
-               let counterparty_channel_data = self.accepted_channel_data.as_ref().expect("must accept before signing");
-               let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_channel_data.counterparty_channel_pubkeys.funding_pubkey);
+               let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
 
                let sighash = hash_to_message!(&bip143::SigHashCache::new(closing_tx)
                        .signature_hash(0, &channel_funding_redeemscript, self.channel_value_satoshis, SigHashType::All)[..]);
@@ -588,19 +604,13 @@ 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 {
-                       counterparty_channel_pubkeys: channel_pubkeys.clone(),
-                       counterparty_selected_contest_delay,
-                       holder_selected_contest_delay,
-               });
+       fn ready_channel(&mut self, channel_parameters: &ChannelTransactionParameters) {
+               assert!(self.channel_parameters.is_none(), "Acceptance already noted");
+               assert!(channel_parameters.is_populated(), "Channel parameters must be fully populated");
+               self.channel_parameters = Some(channel_parameters.clone());
        }
 }
 
-impl_writeable!(AcceptedChannelData, 0,
- { counterparty_channel_pubkeys, counterparty_selected_contest_delay, holder_selected_contest_delay });
-
 impl Writeable for InMemoryChannelKeys {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
                self.funding_key.write(writer)?;
@@ -609,10 +619,9 @@ impl Writeable for InMemoryChannelKeys {
                self.delayed_payment_base_key.write(writer)?;
                self.htlc_base_key.write(writer)?;
                self.commitment_seed.write(writer)?;
-               self.accepted_channel_data.write(writer)?;
+               self.channel_parameters.write(writer)?;
                self.channel_value_satoshis.write(writer)?;
-               self.key_derivation_params.0.write(writer)?;
-               self.key_derivation_params.1.write(writer)?;
+               self.channel_keys_id.write(writer)?;
 
                Ok(())
        }
@@ -633,8 +642,7 @@ impl Readable for InMemoryChannelKeys {
                        InMemoryChannelKeys::make_holder_keys(&secp_ctx, &funding_key, &revocation_base_key,
                                                             &payment_key, &delayed_payment_base_key,
                                                             &htlc_base_key);
-               let params_1 = Readable::read(reader)?;
-               let params_2 = Readable::read(reader)?;
+               let keys_id = Readable::read(reader)?;
 
                Ok(InMemoryChannelKeys {
                        funding_key,
@@ -645,8 +653,8 @@ impl Readable for InMemoryChannelKeys {
                        commitment_seed,
                        channel_value_satoshis,
                        holder_channel_pubkeys,
-                       accepted_channel_data: counterparty_channel_data,
-                       key_derivation_params: (params_1, params_2),
+                       channel_parameters: counterparty_channel_data,
+                       channel_keys_id: keys_id,
                })
        }
 }
@@ -742,19 +750,19 @@ impl KeysManager {
        /// Derive an old set of ChannelKeys for per-channel secrets based on a key derivation
        /// parameters.
        /// Key derivation parameters are accessible through a per-channel secrets
-       /// ChannelKeys::key_derivation_params and is provided inside DynamicOuputP2WSH in case of
+       /// ChannelKeys::channel_keys_id and is provided inside DynamicOuputP2WSH in case of
        /// onchain output detection for which a corresponding delayed_payment_key must be derived.
-       pub fn derive_channel_keys(&self, channel_value_satoshis: u64, params_1: u64, params_2: u64) -> InMemoryChannelKeys {
-               let chan_id = ((params_1 & 0xFFFF_FFFF_0000_0000) >> 32) as u32;
+       pub fn derive_channel_keys(&self, channel_value_satoshis: u64, params: &[u8; 32]) -> InMemoryChannelKeys {
+               let chan_id = byte_utils::slice_to_be64(&params[0..8]);
+               assert!(chan_id <= std::u32::MAX as u64); // Otherwise the params field wasn't created by us
                let mut unique_start = Sha256::engine();
-               unique_start.input(&byte_utils::be64_to_array(params_2));
-               unique_start.input(&byte_utils::be32_to_array(params_1 as u32));
+               unique_start.input(params);
                unique_start.input(&self.seed);
 
                // We only seriously intend to rely on the channel_master_key for true secure
                // entropy, everything else just ensures uniqueness. We rely on the unique_start (ie
                // starting_time provided in the constructor) to be unique.
-               let child_privkey = self.channel_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(chan_id).expect("key space exhausted")).expect("Your RNG is busted");
+               let child_privkey = self.channel_master_key.ckd_priv(&self.secp_ctx, ChildNumber::from_hardened_idx(chan_id as u32).expect("key space exhausted")).expect("Your RNG is busted");
                unique_start.input(&child_privkey.private_key.key[..]);
 
                let seed = Sha256::from_engine(unique_start).into_inner();
@@ -789,7 +797,7 @@ impl KeysManager {
                        htlc_base_key,
                        commitment_seed,
                        channel_value_satoshis,
-                       (params_1, params_2),
+                       params.clone()
                )
        }
 }
@@ -811,8 +819,12 @@ impl KeysInterface for KeysManager {
 
        fn get_channel_keys(&self, _inbound: bool, channel_value_satoshis: u64) -> Self::ChanKeySigner {
                let child_ix = self.channel_child_index.fetch_add(1, Ordering::AcqRel);
-               let ix_and_nanos: u64 = (child_ix as u64) << 32 | (self.starting_time_nanos as u64);
-               self.derive_channel_keys(channel_value_satoshis, ix_and_nanos, self.starting_time_secs)
+               assert!(child_ix <= std::u32::MAX as usize);
+               let mut id = [0; 32];
+               id[0..8].copy_from_slice(&byte_utils::be64_to_array(child_ix as u64));
+               id[8..16].copy_from_slice(&byte_utils::be64_to_array(self.starting_time_nanos as u64));
+               id[16..24].copy_from_slice(&byte_utils::be64_to_array(self.starting_time_secs));
+               self.derive_channel_keys(channel_value_satoshis, &id)
        }
 
        fn get_secure_random_bytes(&self) -> [u8; 32] {
@@ -825,4 +837,8 @@ impl KeysInterface for KeysManager {
                sha.input(b"Unique Secure Random Bytes Salt");
                Sha256::from_engine(sha).into_inner()
        }
+
+       fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::ChanKeySigner, DecodeError> {
+               InMemoryChannelKeys::read(&mut std::io::Cursor::new(reader))
+       }
 }