Fold sign_holder_commitment_htlc_transactions into sign_holder_commitment
[rust-lightning] / lightning / src / chain / keysinterface.rs
index 7c68256e02f69c246c018cd501440b6975c2adf7..80f734edbde3e0def17dcd23ceb903d564d969e2 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, LocalCommitmentTransaction, PreCalculatedTxCreationKeys};
+use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, ChannelTransactionParameters, CommitmentTransaction};
 use ln::msgs::UnsignedChannelAnnouncement;
 
 use std::sync::atomic::{AtomicUsize, Ordering};
@@ -45,7 +45,7 @@ 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
@@ -71,15 +71,15 @@ pub enum SpendableOutputDescriptor {
        /// it is an output from an old state which we broadcast (which should never happen).
        ///
        /// To derive the delayed_payment key which is used to sign for this input, you must pass the
-       /// local delayed_payment_base_key (ie the private key which corresponds to the pubkey in
+       /// holder delayed_payment_base_key (ie the private key which corresponds to the pubkey in
        /// ChannelKeys::pubkeys().delayed_payment_basepoint) and the provided per_commitment_point to
        /// chan_utils::derive_private_key. The public key can be generated without the secret key
        /// using chan_utils::derive_public_key and only the delayed_payment_basepoint which appears in
        /// ChannelKeys::pubkeys().
        ///
-       /// To derive the remote_revocation_pubkey provided here (which is used in the witness
-       /// script generation), you must pass the remote revocation_basepoint (which appears in the
-       /// call to ChannelKeys::on_accept) and the provided per_commitment point
+       /// 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::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
@@ -101,8 +101,8 @@ pub enum SpendableOutputDescriptor {
                /// 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 remote_revocation_pubkey used to derive witnessScript
-               remote_revocation_pubkey: PublicKey
+               /// The revocation_pubkey used to derive witnessScript
+               revocation_pubkey: PublicKey
        },
        /// 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).
@@ -111,7 +111,7 @@ pub enum SpendableOutputDescriptor {
        ///
        /// These are generally the result of our counterparty having broadcast the current state,
        /// allowing us to claim the non-HTLC-encumbered outputs immediately.
-       StaticOutputRemotePayment {
+       StaticOutputCounterpartyPayment {
                /// The outpoint which is spendable
                outpoint: OutPoint,
                /// The output which is reference by the given outpoint
@@ -130,7 +130,7 @@ 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 remote_revocation_pubkey } => {
+                       &SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref key_derivation_params, ref revocation_pubkey } => {
                                1u8.write(writer)?;
                                outpoint.write(writer)?;
                                per_commitment_point.write(writer)?;
@@ -138,9 +138,9 @@ impl Writeable for SpendableOutputDescriptor {
                                output.write(writer)?;
                                key_derivation_params.0.write(writer)?;
                                key_derivation_params.1.write(writer)?;
-                               remote_revocation_pubkey.write(writer)?;
+                               revocation_pubkey.write(writer)?;
                        },
-                       &SpendableOutputDescriptor::StaticOutputRemotePayment { ref outpoint, ref output, ref key_derivation_params } => {
+                       &SpendableOutputDescriptor::StaticOutputCounterpartyPayment { ref outpoint, ref output, ref key_derivation_params } => {
                                2u8.write(writer)?;
                                outpoint.write(writer)?;
                                output.write(writer)?;
@@ -165,9 +165,9 @@ impl Readable for SpendableOutputDescriptor {
                                to_self_delay: Readable::read(reader)?,
                                output: Readable::read(reader)?,
                                key_derivation_params: (Readable::read(reader)?, Readable::read(reader)?),
-                               remote_revocation_pubkey: Readable::read(reader)?,
+                               revocation_pubkey: Readable::read(reader)?,
                        }),
-                       2u8 => Ok(SpendableOutputDescriptor::StaticOutputRemotePayment {
+                       2u8 => Ok(SpendableOutputDescriptor::StaticOutputCounterpartyPayment {
                                outpoint: Readable::read(reader)?,
                                output: Readable::read(reader)?,
                                key_derivation_params: (Readable::read(reader)?, Readable::read(reader)?),
@@ -204,7 +204,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.
@@ -231,40 +231,30 @@ pub trait ChannelKeys : Send+Clone {
        /// 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: &LocalCommitmentTransaction, 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: &LocalCommitmentTransaction, 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: &LocalCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()>;
+       fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<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 +309,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/locally_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 locally_selected_contest_delay late here for API convenience.
+       /// 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, locally_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.
@@ -346,29 +340,21 @@ pub trait KeysInterface: Send + Sync {
        /// onion packets and for temporary channel IDs. There is no requirement that these be
        /// persisted anywhere, though they must be unique across restarts.
        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 locally-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.
-       locally_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,
@@ -384,8 +370,8 @@ pub struct InMemoryChannelKeys {
        pub commitment_seed: [u8; 32],
        /// Holder public keys and basepoints
        pub(crate) holder_channel_pubkeys: ChannelPublicKeys,
-       /// Counterparty public keys and counterparty/locally selected_contest_delay, populated on channel acceptance
-       accepted_channel_data: Option<AcceptedChannelData>,
+       /// Counterparty public keys and counterparty/holder selected_contest_delay, populated on channel acceptance
+       channel_parameters: Option<ChannelTransactionParameters>,
        /// The total value of this channel
        channel_value_satoshis: u64,
        /// Key derivation parameters
@@ -417,7 +403,7 @@ impl InMemoryChannelKeys {
                        commitment_seed,
                        channel_value_satoshis,
                        holder_channel_pubkeys,
-                       accepted_channel_data: None,
+                       channel_parameters: None,
                        key_derivation_params,
                }
        }
@@ -439,21 +425,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 locally-broadcastable
+       /// 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 locally_selected_contest_delay(&self) -> u16 { self.accepted_channel_data.as_ref().unwrap().locally_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 {
@@ -469,56 +470,47 @@ impl ChannelKeys for InMemoryChannelKeys {
        fn pubkeys(&self) -> &ChannelPublicKeys { &self.holder_channel_pubkeys }
        fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
 
-       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.locally_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: &LocalCommitmentTransaction, 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_local_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 sig = commitment_tx.trust().built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+               let channel_parameters = self.get_channel_parameters();
+               let trusted_tx = commitment_tx.trust();
+               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: &LocalCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
+       fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<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_local_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: &LocalCommitmentTransaction, 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 channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
+               Ok(commitment_tx.trust().built_transaction().sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
        }
 
        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, ()> {
@@ -546,7 +538,7 @@ impl ChannelKeys for InMemoryChannelKeys {
                                Ok(counterparty_delayedpubkey) => counterparty_delayedpubkey,
                                Err(_) => return Err(())
                        };
-                       chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.locally_selected_contest_delay(), &counterparty_delayedpubkey)
+                       chan_utils::get_revokeable_redeemscript(&revocation_pubkey, self.holder_selected_contest_delay(), &counterparty_delayedpubkey)
                };
                let mut sighash_parts = bip143::SigHashCache::new(justice_tx);
                let sighash = hash_to_message!(&sighash_parts.signature_hash(input, &witness_script, amount, SigHashType::All)[..]);
@@ -575,8 +567,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 +579,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, locally_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,
-                       locally_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, locally_selected_contest_delay });
-
 impl Writeable for InMemoryChannelKeys {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
                self.funding_key.write(writer)?;
@@ -609,7 +594,7 @@ 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)?;
@@ -645,7 +630,7 @@ impl Readable for InMemoryChannelKeys {
                        commitment_seed,
                        channel_value_satoshis,
                        holder_channel_pubkeys,
-                       accepted_channel_data: counterparty_channel_data,
+                       channel_parameters: counterparty_channel_data,
                        key_derivation_params: (params_1, params_2),
                })
        }
@@ -825,4 +810,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))
+       }
 }