Introduce CommitmentTransaction, ChannelTransactionParameters
authorDevrandom <c1.devrandom@niftybox.net>
Thu, 15 Oct 2020 11:45:18 +0000 (13:45 +0200)
committerDevrandom <c1.devrandom@niftybox.net>
Wed, 30 Dec 2020 21:40:18 +0000 (13:40 -0800)
CommitmentTransaction maintains the per-commitment transaction fields needed to construct the associated bitcoin transactions (commitment, HTLC).  It replaces passing around of Bitcoin transactions.  The ChannelKeys API is modified accordingly.

By regenerating the transaction when implementing a validating external signer, this allows a higher level of assurance that all relevant aspects of the transactions were checked for policy violations.

ChannelTransactionParameters replaces passing around of individual per-channel fields that are needed to construct Bitcoin transactions.

Eliminate ChannelStaticData in favor of ChannelTransactionParameters.

Use counterparty txid instead of tx in channelmonitor update.

lightning/Cargo.toml
lightning/src/chain/channelmonitor.rs
lightning/src/chain/keysinterface.rs
lightning/src/ln/chan_utils.rs
lightning/src/ln/channel.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/onchaintx.rs
lightning/src/util/enforcing_trait_impls.rs
lightning/src/util/ser.rs

index c1f364394dd98a3bf194fec80fc29773e62d0ad1..2d7d95dd8fadde57328f87c04c89e10a6622833e 100644 (file)
@@ -12,6 +12,7 @@ Still missing tons of error-handling. See GitHub issues for suggested projects i
 
 [features]
 fuzztarget = ["bitcoin/fuzztarget"]
+# Internal test utilities exposed to other repo crates
 _test_utils = ["hex", "regex"]
 # Unlog messages superior at targeted level.
 max_level_off = []
index c3382caaea4f8f199b4bf40843788db682c5e6bf..d85ac51d4d98e7f85bb1b72b00490d41390434c1 100644 (file)
@@ -27,7 +27,6 @@ use bitcoin::blockdata::transaction::{TxOut,Transaction};
 use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
 use bitcoin::blockdata::script::{Script, Builder};
 use bitcoin::blockdata::opcodes;
-use bitcoin::consensus::encode;
 
 use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
@@ -39,7 +38,7 @@ use bitcoin::secp256k1;
 
 use ln::msgs::DecodeError;
 use ln::chan_utils;
-use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction, HTLCType};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCType, ChannelTransactionParameters, HolderCommitmentTransaction};
 use ln::channelmanager::{HTLCSource, PaymentPreimage, PaymentHash};
 use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
 use chain::chaininterface::{BroadcasterInterface, FeeEstimator};
@@ -252,6 +251,7 @@ pub(crate) const ANTI_REORG_DELAY: u32 = 6;
 /// end up force-closing the channel on us to claim it.
 pub(crate) const HTLC_FAIL_BACK_BUFFER: u32 = CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS;
 
+// TODO(devrandom) replace this with HolderCommitmentTransaction
 #[derive(Clone, PartialEq)]
 struct HolderSignedTx {
        /// txid of the transaction in tx, just used to make comparison faster
@@ -493,7 +493,7 @@ pub(crate) enum ChannelMonitorUpdateStep {
                htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
        },
        LatestCounterpartyCommitmentTXInfo {
-               unsigned_commitment_tx: Transaction, // TODO: We should actually only need the txid here
+               commitment_txid: Txid,
                htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
                commitment_number: u64,
                their_revocation_point: PublicKey,
@@ -527,9 +527,9 @@ impl Writeable for ChannelMonitorUpdateStep {
                                        source.write(w)?;
                                }
                        }
-                       &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { ref unsigned_commitment_tx, ref htlc_outputs, ref commitment_number, ref their_revocation_point } => {
+                       &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, ref htlc_outputs, ref commitment_number, ref their_revocation_point } => {
                                1u8.write(w)?;
-                               unsigned_commitment_tx.write(w)?;
+                               commitment_txid.write(w)?;
                                commitment_number.write(w)?;
                                their_revocation_point.write(w)?;
                                (htlc_outputs.len() as u64).write(w)?;
@@ -573,7 +573,7 @@ impl Readable for ChannelMonitorUpdateStep {
                        },
                        1u8 => {
                                Ok(ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
-                                       unsigned_commitment_tx: Readable::read(r)?,
+                                       commitment_txid: Readable::read(r)?,
                                        commitment_number: Readable::read(r)?,
                                        their_revocation_point: Readable::read(r)?,
                                        htlc_outputs: {
@@ -948,11 +948,11 @@ impl<ChanSigner: ChannelKeys + Writeable> ChannelMonitor<ChanSigner> {
 
 impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        pub(crate) fn new(keys: ChanSigner, shutdown_pubkey: &PublicKey,
-                       on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
-                       counterparty_htlc_base_key: &PublicKey, counterparty_delayed_payment_base_key: &PublicKey,
-                       on_holder_tx_csv: u16, funding_redeemscript: Script, channel_value_satoshis: u64,
-                       commitment_transaction_number_obscure_factor: u64,
-                       initial_holder_commitment_tx: HolderCommitmentTransaction) -> ChannelMonitor<ChanSigner> {
+                         on_counterparty_tx_csv: u16, destination_script: &Script, funding_info: (OutPoint, Script),
+                         channel_parameters: &ChannelTransactionParameters,
+                         funding_redeemscript: Script, channel_value_satoshis: u64,
+                         commitment_transaction_number_obscure_factor: u64,
+                         initial_holder_commitment_tx: HolderCommitmentTransaction) -> ChannelMonitor<ChanSigner> {
 
                assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
                let our_channel_close_key_hash = WPubkeyHash::hash(&shutdown_pubkey.serialize());
@@ -960,21 +960,32 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                let payment_key_hash = WPubkeyHash::hash(&keys.pubkeys().payment_point.serialize());
                let counterparty_payment_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_key_hash[..]).into_script();
 
-               let counterparty_tx_cache = CounterpartyCommitmentTransaction { counterparty_delayed_payment_base_key: *counterparty_delayed_payment_base_key, counterparty_htlc_base_key: *counterparty_htlc_base_key, on_counterparty_tx_csv, per_htlc: HashMap::new() };
-
-               let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), on_holder_tx_csv);
-
-               let holder_tx_sequence = initial_holder_commitment_tx.unsigned_tx.input[0].sequence as u64;
-               let holder_tx_locktime = initial_holder_commitment_tx.unsigned_tx.lock_time as u64;
-               let holder_commitment_tx = HolderSignedTx {
-                       txid: initial_holder_commitment_tx.txid(),
-                       revocation_key: initial_holder_commitment_tx.keys.revocation_key,
-                       a_htlc_key: initial_holder_commitment_tx.keys.broadcaster_htlc_key,
-                       b_htlc_key: initial_holder_commitment_tx.keys.countersignatory_htlc_key,
-                       delayed_payment_key: initial_holder_commitment_tx.keys.broadcaster_delayed_payment_key,
-                       per_commitment_point: initial_holder_commitment_tx.keys.per_commitment_point,
-                       feerate_per_kw: initial_holder_commitment_tx.feerate_per_kw,
-                       htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
+               let counterparty_channel_parameters = channel_parameters.counterparty_parameters.as_ref().unwrap();
+               let counterparty_delayed_payment_base_key = counterparty_channel_parameters.pubkeys.delayed_payment_basepoint;
+               let counterparty_htlc_base_key = counterparty_channel_parameters.pubkeys.htlc_basepoint;
+               let counterparty_tx_cache = CounterpartyCommitmentTransaction { counterparty_delayed_payment_base_key, counterparty_htlc_base_key, on_counterparty_tx_csv, per_htlc: HashMap::new() };
+
+               let mut onchain_tx_handler = OnchainTxHandler::new(destination_script.clone(), keys.clone(), channel_parameters.clone());
+
+               let secp_ctx = Secp256k1::new();
+
+               // block for Rust 1.34 compat
+               let (holder_commitment_tx, current_holder_commitment_number) = {
+                       let trusted_tx = initial_holder_commitment_tx.trust();
+                       let txid = trusted_tx.txid();
+
+                       let tx_keys = trusted_tx.keys();
+                       let holder_commitment_tx = HolderSignedTx {
+                               txid,
+                               revocation_key: tx_keys.revocation_key,
+                               a_htlc_key: tx_keys.broadcaster_htlc_key,
+                               b_htlc_key: tx_keys.countersignatory_htlc_key,
+                               delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
+                               per_commitment_point: tx_keys.per_commitment_point,
+                               feerate_per_kw: trusted_tx.feerate_per_kw(),
+                               htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
+                       };
+                       (holder_commitment_tx, trusted_tx.commitment_number())
                };
                onchain_tx_handler.provide_latest_holder_tx(initial_holder_commitment_tx);
 
@@ -1000,7 +1011,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        channel_value_satoshis,
                        their_cur_revocation_points: None,
 
-                       on_holder_tx_csv,
+                       on_holder_tx_csv: counterparty_channel_parameters.selected_contest_delay,
 
                        commitment_secrets: CounterpartyCommitmentSecrets::new(),
                        counterparty_claimable_outpoints: HashMap::new(),
@@ -1010,7 +1021,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        prev_holder_signed_commitment_tx: None,
                        current_holder_commitment_tx: holder_commitment_tx,
                        current_counterparty_commitment_number: 1 << 48,
-                       current_holder_commitment_number: 0xffff_ffff_ffff - ((((holder_tx_sequence & 0xffffff) << 3*8) | (holder_tx_locktime as u64 & 0xffffff)) ^ commitment_transaction_number_obscure_factor),
+                       current_holder_commitment_number,
 
                        payment_preimages: HashMap::new(),
                        pending_monitor_events: Vec::new(),
@@ -1025,7 +1036,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        holder_tx_signed: false,
 
                        last_block_hash: Default::default(),
-                       secp_ctx: Secp256k1::new(),
+                       secp_ctx,
                }
        }
 
@@ -1084,7 +1095,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        /// The monitor watches for it to be broadcasted and then uses the HTLC information (and
        /// possibly future revocation/preimage information) to claim outputs where possible.
        /// We cache also the mapping hash:commitment number to lighten pruning of old preimages by watchtowers.
-       pub(crate) fn provide_latest_counterparty_commitment_tx_info<L: Deref>(&mut self, unsigned_commitment_tx: &Transaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>, commitment_number: u64, their_revocation_point: PublicKey, logger: &L) where L::Target: Logger {
+       pub(crate) fn provide_latest_counterparty_commitment_tx<L: Deref>(&mut self, txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>, commitment_number: u64, their_revocation_point: PublicKey, logger: &L) where L::Target: Logger {
                // TODO: Encrypt the htlc_outputs data with the single-hash of the commitment transaction
                // so that a remote monitor doesn't learn anything unless there is a malicious close.
                // (only maybe, sadly we cant do the same for local info, as we need to be aware of
@@ -1093,12 +1104,10 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                        self.counterparty_hash_commitment_number.insert(htlc.payment_hash, commitment_number);
                }
 
-               let new_txid = unsigned_commitment_tx.txid();
-               log_trace!(logger, "Tracking new counterparty commitment transaction with txid {} at commitment number {} with {} HTLC outputs", new_txid, commitment_number, htlc_outputs.len());
-               log_trace!(logger, "New potential counterparty commitment transaction: {}", encode::serialize_hex(unsigned_commitment_tx));
+               log_trace!(logger, "Tracking new counterparty commitment transaction with txid {} at commitment number {} with {} HTLC outputs", txid, commitment_number, htlc_outputs.len());
                self.prev_counterparty_commitment_txid = self.current_counterparty_commitment_txid.take();
-               self.current_counterparty_commitment_txid = Some(new_txid);
-               self.counterparty_claimable_outpoints.insert(new_txid, htlc_outputs.clone());
+               self.current_counterparty_commitment_txid = Some(txid);
+               self.counterparty_claimable_outpoints.insert(txid, htlc_outputs.clone());
                self.current_counterparty_commitment_number = commitment_number;
                //TODO: Merge this into the other per-counterparty-transaction output storage stuff
                match self.their_cur_revocation_points {
@@ -1125,7 +1134,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                htlcs.push(htlc.0);
                        }
                }
-               self.counterparty_tx_cache.per_htlc.insert(new_txid, htlcs);
+               self.counterparty_tx_cache.per_htlc.insert(txid, htlcs);
        }
 
        /// Informs this monitor of the latest holder (ie broadcastable) commitment transaction. The
@@ -1133,22 +1142,25 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
        /// is important that any clones of this channel monitor (including remote clones) by kept
        /// up-to-date as our holder commitment transaction is updated.
        /// Panics if set_on_holder_tx_csv has never been called.
-       fn provide_latest_holder_commitment_tx_info(&mut self, commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
-               let txid = commitment_tx.txid();
-               let sequence = commitment_tx.unsigned_tx.input[0].sequence as u64;
-               let locktime = commitment_tx.unsigned_tx.lock_time as u64;
-               let mut new_holder_commitment_tx = HolderSignedTx {
-                       txid,
-                       revocation_key: commitment_tx.keys.revocation_key,
-                       a_htlc_key: commitment_tx.keys.broadcaster_htlc_key,
-                       b_htlc_key: commitment_tx.keys.countersignatory_htlc_key,
-                       delayed_payment_key: commitment_tx.keys.broadcaster_delayed_payment_key,
-                       per_commitment_point: commitment_tx.keys.per_commitment_point,
-                       feerate_per_kw: commitment_tx.feerate_per_kw,
-                       htlc_outputs,
+       fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
+               // block for Rust 1.34 compat
+               let mut new_holder_commitment_tx = {
+                       let trusted_tx = holder_commitment_tx.trust();
+                       let txid = trusted_tx.txid();
+                       let tx_keys = trusted_tx.keys();
+                       self.current_holder_commitment_number = trusted_tx.commitment_number();
+                       HolderSignedTx {
+                               txid,
+                               revocation_key: tx_keys.revocation_key,
+                               a_htlc_key: tx_keys.broadcaster_htlc_key,
+                               b_htlc_key: tx_keys.countersignatory_htlc_key,
+                               delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
+                               per_commitment_point: tx_keys.per_commitment_point,
+                               feerate_per_kw: trusted_tx.feerate_per_kw(),
+                               htlc_outputs,
+                       }
                };
-               self.onchain_tx_handler.provide_latest_holder_tx(commitment_tx);
-               self.current_holder_commitment_number = 0xffff_ffff_ffff - ((((sequence & 0xffffff) << 3*8) | (locktime as u64 & 0xffffff)) ^ self.commitment_transaction_number_obscure_factor);
+               self.onchain_tx_handler.provide_latest_holder_tx(holder_commitment_tx);
                mem::swap(&mut new_holder_commitment_tx, &mut self.current_holder_commitment_tx);
                self.prev_holder_signed_commitment_tx = Some(new_holder_commitment_tx);
                if self.holder_tx_signed {
@@ -1239,11 +1251,11 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
                                ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs } => {
                                        log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
                                        if self.lockdown_from_offchain { panic!(); }
-                                       self.provide_latest_holder_commitment_tx_info(commitment_tx.clone(), htlc_outputs.clone())?
-                               },
-                               ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { unsigned_commitment_tx, htlc_outputs, commitment_number, their_revocation_point } => {
+                                       self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone())?
+                               }
+                               ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_revocation_point } => {
                                        log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
-                                       self.provide_latest_counterparty_commitment_tx_info(&unsigned_commitment_tx, htlc_outputs.clone(), *commitment_number, *their_revocation_point, logger)
+                                       self.provide_latest_counterparty_commitment_tx(*commitment_txid, htlc_outputs.clone(), *commitment_number, *their_revocation_point, logger)
                                },
                                ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage } => {
                                        log_trace!(logger, "Updating ChannelMonitor with payment preimage");
@@ -2589,7 +2601,7 @@ mod tests {
        use ln::channelmanager::{PaymentPreimage, PaymentHash};
        use ln::onchaintx::{OnchainTxHandler, InputDescriptors};
        use ln::chan_utils;
-       use ln::chan_utils::{HTLCOutputInCommitment, HolderCommitmentTransaction};
+       use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
        use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
        use bitcoin::secp256k1::key::{SecretKey,PublicKey};
        use bitcoin::secp256k1::Secp256k1;
@@ -2662,20 +2674,39 @@ mod tests {
                        (0, 0)
                );
 
+               let counterparty_pubkeys = ChannelPublicKeys {
+                       funding_pubkey: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()),
+                       revocation_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()),
+                       payment_point: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[46; 32]).unwrap()),
+                       delayed_payment_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[47; 32]).unwrap()),
+                       htlc_basepoint: PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[48; 32]).unwrap())
+               };
+               let funding_outpoint = OutPoint { txid: Default::default(), index: u16::max_value() };
+               let channel_parameters = ChannelTransactionParameters {
+                       holder_pubkeys: keys.holder_channel_pubkeys.clone(),
+                       holder_selected_contest_delay: 66,
+                       is_outbound_from_holder: true,
+                       counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
+                               pubkeys: counterparty_pubkeys,
+                               selected_contest_delay: 67,
+                       }),
+                       funding_outpoint: Some(funding_outpoint),
+               };
                // Prune with one old state and a holder commitment tx holding a few overlaps with the
                // old state.
                let mut monitor = ChannelMonitor::new(keys,
-                       &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
-                       (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
-                       &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[44; 32]).unwrap()),
-                       &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[45; 32]).unwrap()),
-                       10, Script::new(), 46, 0, HolderCommitmentTransaction::dummy());
-
-               monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..10])).unwrap();
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654, dummy_key, &logger);
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger);
-               monitor.provide_latest_counterparty_commitment_tx_info(&dummy_tx, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger);
+                                                     &PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()), 0, &Script::new(),
+                                                     (OutPoint { txid: Txid::from_slice(&[43; 32]).unwrap(), index: 0 }, Script::new()),
+                                                     &channel_parameters,
+                                                     Script::new(), 46, 0,
+                                                     HolderCommitmentTransaction::dummy());
+
+               monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..10])).unwrap();
+               let dummy_txid = dummy_tx.txid();
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger);
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654, dummy_key, &logger);
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger);
+               monitor.provide_latest_counterparty_commitment_tx(dummy_txid, preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger);
                for &(ref preimage, ref hash) in preimages.iter() {
                        monitor.provide_payment_preimage(hash, preimage, &broadcaster, &fee_estimator, &logger);
                }
@@ -2697,7 +2728,7 @@ mod tests {
 
                // Now update holder commitment tx info, pruning only element 18 as we still care about the
                // previous commitment tx's preimages too
-               monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..5])).unwrap();
+               monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..5])).unwrap();
                secret[0..32].clone_from_slice(&hex::decode("2273e227a5b7449b6e70f1fb4652864038b1cbf9cd7c043a7d6456b7fc275ad8").unwrap());
                monitor.provide_secret(281474976710653, secret.clone()).unwrap();
                assert_eq!(monitor.payment_preimages.len(), 12);
@@ -2705,7 +2736,7 @@ mod tests {
                test_preimages_exist!(&preimages[18..20], monitor);
 
                // But if we do it again, we'll prune 5-10
-               monitor.provide_latest_holder_commitment_tx_info(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..3])).unwrap();
+               monitor.provide_latest_holder_commitment_tx(HolderCommitmentTransaction::dummy(), preimages_to_holder_htlcs!(preimages[0..3])).unwrap();
                secret[0..32].clone_from_slice(&hex::decode("27cddaa5624534cb6cb9d7da077cf2b22ab21e9b506fd4998a51d54502e99116").unwrap());
                monitor.provide_secret(281474976710652, secret.clone()).unwrap();
                assert_eq!(monitor.payment_preimages.len(), 5);
index 5599bfbb9fd763e88c58dd475735ace5a86a0079..085a2f87154b1b5a09c9cd9a32757bf8f164dc49 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};
@@ -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
@@ -231,40 +231,34 @@ 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>), ()>;
+       fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &CommitmentTransaction, 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
+       /// the same commitment_tx (or a copy thereof), though there are currently no guarantees
        /// that it will not be called multiple times.
        /// An external signer implementation should check that the commitment has not been revoked.
        //
        // 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, ()>;
+       fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<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, ()>;
+       fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, 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
+       /// commitment_tx values. While this will never be called with a revoked
+       /// commitment_tx, it is possible that it is called with the second-latest
+       /// 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>>, ()>;
+       /// commitment_tx.
+       fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<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 +313,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.
@@ -348,27 +346,11 @@ pub trait KeysInterface: Send + Sync {
        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,
-}
-
 #[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,7 +367,7 @@ 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
@@ -417,7 +399,7 @@ impl InMemoryChannelKeys {
                        commitment_seed,
                        channel_value_satoshis,
                        holder_channel_pubkeys,
-                       accepted_channel_data: None,
+                       channel_parameters: None,
                        key_derivation_params,
                }
        }
@@ -439,21 +421,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 {
@@ -469,56 +466,50 @@ 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.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<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_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 sig = commitment_tx.trust().built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+               Ok(sig)
        }
 
        #[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<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_holder_sig(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, 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_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)
+       fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()> {
+               let channel_parameters = self.get_channel_parameters();
+               let trusted_tx = commitment_tx.trust();
+               trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), 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, ()> {
@@ -575,8 +566,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 +578,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,7 +593,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 +629,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),
                })
        }
index 7b7d511d653e0553e2fb41d44cd97c2a31adf445..8037de64eb562f4ef854b511a382625b77f0e035 100644 (file)
@@ -14,8 +14,6 @@
 use bitcoin::blockdata::script::{Script,Builder};
 use bitcoin::blockdata::opcodes;
 use bitcoin::blockdata::transaction::{TxIn,TxOut,OutPoint,Transaction, SigHashType};
-use bitcoin::consensus::encode::{Decodable, Encodable};
-use bitcoin::consensus::encode;
 use bitcoin::util::bip143;
 
 use bitcoin::hashes::{Hash, HashEngine};
@@ -25,17 +23,29 @@ use bitcoin::hash_types::{Txid, PubkeyHash};
 
 use ln::channelmanager::{PaymentHash, PaymentPreimage};
 use ln::msgs::DecodeError;
-use util::ser::{Readable, Writeable, Writer, WriterWriteAdaptor};
+use util::ser::{Readable, Writeable, Writer, MAX_BUF_SIZE};
 use util::byte_utils;
 
+use bitcoin::hash_types::WPubkeyHash;
 use bitcoin::secp256k1::key::{SecretKey, PublicKey};
-use bitcoin::secp256k1::{Secp256k1, Signature};
+use bitcoin::secp256k1::{Secp256k1, Signature, Message};
 use bitcoin::secp256k1::Error as SecpError;
 use bitcoin::secp256k1;
 
-use std::{cmp, mem};
+use std::cmp;
+use ln::chan_utils;
+use util::transaction_utils::sort_outputs;
+use ln::channel::INITIAL_COMMITMENT_NUMBER;
+use std::io::Read;
+use std::ops::Deref;
+use chain;
 
-const MAX_ALLOC_SIZE: usize = 64*1024;
+const HTLC_OUTPUT_IN_COMMITMENT_SIZE: usize = 1 + 8 + 4 + 32 + 5;
+
+pub(crate) const MAX_HTLCS: u16 = 483;
+
+// This checks that the buffer size is greater than the maximum possible size for serialized HTLCS
+const _EXCESS_BUFFER_SIZE: usize = MAX_BUF_SIZE - MAX_HTLCS as usize * HTLC_OUTPUT_IN_COMMITMENT_SIZE;
 
 pub(super) const HTLC_SUCCESS_TX_WEIGHT: u64 = 703;
 pub(super) const HTLC_TIMEOUT_TX_WEIGHT: u64 = 663;
@@ -294,7 +304,7 @@ pub fn derive_public_revocation_key<T: secp256k1::Verification>(secp_ctx: &Secp2
 ///
 /// These keys are assumed to be good, either because the code derived them from
 /// channel basepoints via the new function, or they were obtained via
-/// PreCalculatedTxCreationKeys.trust_key_derivation because we trusted the source of the
+/// CommitmentTransaction.trust().keys() because we trusted the source of the
 /// pre-calculated keys.
 #[derive(PartialEq, Clone)]
 pub struct TxCreationKeys {
@@ -314,31 +324,6 @@ pub struct TxCreationKeys {
 impl_writeable!(TxCreationKeys, 33*6,
        { per_commitment_point, revocation_key, broadcaster_htlc_key, countersignatory_htlc_key, broadcaster_delayed_payment_key });
 
-/// The per-commitment point and a set of pre-calculated public keys used for transaction creation
-/// in the signer.
-/// The pre-calculated keys are an optimization, because ChannelKeys has enough
-/// information to re-derive them.
-#[derive(PartialEq, Clone)]
-pub struct PreCalculatedTxCreationKeys(TxCreationKeys);
-
-impl PreCalculatedTxCreationKeys {
-       /// Create a new PreCalculatedTxCreationKeys from TxCreationKeys
-       pub fn new(keys: TxCreationKeys) -> Self {
-               PreCalculatedTxCreationKeys(keys)
-       }
-
-       /// The pre-calculated transaction creation public keys.
-       /// An external validating signer should not trust these keys.
-       pub fn trust_key_derivation(&self) -> &TxCreationKeys {
-               &self.0
-       }
-
-       /// The transaction per-commitment point
-       pub fn per_commitment_point(&self) -> &PublicKey {
-               &self.0.per_commitment_point
-       }
-}
-
 /// One counterparty's public keys which do not change over the life of a channel.
 #[derive(Clone, PartialEq)]
 pub struct ChannelPublicKeys {
@@ -373,7 +358,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(),
@@ -383,6 +369,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 from_channel_static_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
@@ -422,7 +421,7 @@ pub struct HTLCOutputInCommitment {
        pub transaction_output_index: Option<u32>,
 }
 
-impl_writeable!(HTLCOutputInCommitment, 1 + 8 + 4 + 32 + 5, {
+impl_writeable!(HTLCOutputInCommitment, HTLC_OUTPUT_IN_COMMITMENT_SIZE, {
        offered,
        amount_msat,
        cltv_expiry,
@@ -551,130 +550,216 @@ pub fn build_htlc_transaction(prev_hash: &Txid, feerate_per_kw: u32, contest_del
        }
 }
 
+/// Per-channel data used to build transactions in conjunction with the per-commitment data (CommitmentTransaction).
+/// The fields are organized by holder/counterparty.
+///
+/// Normally, this is converted to the broadcaster/countersignatory-organized DirectedChannelTransactionParameters
+/// before use, via the as_holder_broadcastable and as_counterparty_broadcastable functions.
+#[derive(Clone)]
+pub struct ChannelTransactionParameters {
+       /// Holder public keys
+       pub holder_pubkeys: ChannelPublicKeys,
+       /// The contest delay selected by the holder, which applies to counterparty-broadcast transactions
+       pub holder_selected_contest_delay: u16,
+       /// Whether the holder is the initiator of this channel.
+       /// This is an input to the commitment number obscure factor computation.
+       pub is_outbound_from_holder: bool,
+       /// The late-bound counterparty channel transaction parameters.
+       /// These parameters are populated at the point in the protocol where the counterparty provides them.
+       pub counterparty_parameters: Option<CounterpartyChannelTransactionParameters>,
+       /// The late-bound funding outpoint
+       pub funding_outpoint: Option<chain::transaction::OutPoint>,
+}
+
+/// Late-bound per-channel counterparty data used to build transactions.
+#[derive(Clone)]
+pub struct CounterpartyChannelTransactionParameters {
+       /// Counter-party public keys
+       pub pubkeys: ChannelPublicKeys,
+       /// The contest delay selected by the counterparty, which applies to holder-broadcast transactions
+       pub selected_contest_delay: u16,
+}
+
+impl ChannelTransactionParameters {
+       /// Whether the late bound parameters are populated.
+       pub fn is_populated(&self) -> bool {
+               self.counterparty_parameters.is_some() && self.funding_outpoint.is_some()
+       }
+
+       /// Convert the holder/counterparty parameters to broadcaster/countersignatory-organized parameters,
+       /// given that the holder is the broadcaster.
+       ///
+       /// self.is_populated() must be true before calling this function.
+       pub fn as_holder_broadcastable(&self) -> DirectedChannelTransactionParameters {
+               assert!(self.is_populated(), "self.late_parameters must be set before using as_holder_broadcastable");
+               DirectedChannelTransactionParameters {
+                       inner: self,
+                       holder_is_broadcaster: true
+               }
+       }
+
+       /// Convert the holder/counterparty parameters to broadcaster/countersignatory-organized parameters,
+       /// given that the counterparty is the broadcaster.
+       ///
+       /// self.is_populated() must be true before calling this function.
+       pub fn as_counterparty_broadcastable(&self) -> DirectedChannelTransactionParameters {
+               assert!(self.is_populated(), "self.late_parameters must be set before using as_counterparty_broadcastable");
+               DirectedChannelTransactionParameters {
+                       inner: self,
+                       holder_is_broadcaster: false
+               }
+       }
+}
+
+impl_writeable!(CounterpartyChannelTransactionParameters, 0, {
+       pubkeys,
+       selected_contest_delay
+});
+
+impl_writeable!(ChannelTransactionParameters, 0, {
+       holder_pubkeys,
+       holder_selected_contest_delay,
+       is_outbound_from_holder,
+       counterparty_parameters,
+       funding_outpoint
+});
+
+/// Static channel fields used to build transactions given per-commitment fields, organized by
+/// broadcaster/countersignatory.
+///
+/// This is derived from the holder/counterparty-organized ChannelTransactionParameters via the
+/// as_holder_broadcastable and as_counterparty_broadcastable functions.
+pub struct DirectedChannelTransactionParameters<'a> {
+       /// The holder's channel static parameters
+       inner: &'a ChannelTransactionParameters,
+       /// Whether the holder is the broadcaster
+       holder_is_broadcaster: bool,
+}
+
+impl<'a> DirectedChannelTransactionParameters<'a> {
+       /// Get the channel pubkeys for the broadcaster
+       pub fn broadcaster_pubkeys(&self) -> &ChannelPublicKeys {
+               if self.holder_is_broadcaster {
+                       &self.inner.holder_pubkeys
+               } else {
+                       &self.inner.counterparty_parameters.as_ref().unwrap().pubkeys
+               }
+       }
+
+       /// Get the channel pubkeys for the countersignatory
+       pub fn countersignatory_pubkeys(&self) -> &ChannelPublicKeys {
+               if self.holder_is_broadcaster {
+                       &self.inner.counterparty_parameters.as_ref().unwrap().pubkeys
+               } else {
+                       &self.inner.holder_pubkeys
+               }
+       }
+
+       /// Get the contest delay applicable to the transactions.
+       /// Note that the contest delay was selected by the countersignatory.
+       pub fn contest_delay(&self) -> u16 {
+               let counterparty_parameters = self.inner.counterparty_parameters.as_ref().unwrap();
+               if self.holder_is_broadcaster { counterparty_parameters.selected_contest_delay } else { self.inner.holder_selected_contest_delay }
+       }
+
+       /// Whether the channel is outbound from the broadcaster.
+       ///
+       /// The boolean representing the side that initiated the channel is
+       /// an input to the commitment number obscure factor computation.
+       pub fn is_outbound(&self) -> bool {
+               if self.holder_is_broadcaster { self.inner.is_outbound_from_holder } else { !self.inner.is_outbound_from_holder }
+       }
+
+       /// The funding outpoint
+       pub fn funding_outpoint(&self) -> OutPoint {
+               self.inner.funding_outpoint.unwrap().into_bitcoin_outpoint()
+       }
+}
+
+/// Information needed to build and sign a holder's commitment transaction.
+///
+/// The transaction is only signed once we are ready to broadcast.
 #[derive(Clone)]
-/// We use this to track holder commitment transactions and put off signing them until we are ready
-/// to broadcast. This class can be used inside a signer implementation to generate a signature
-/// given the relevant secret key.
 pub struct HolderCommitmentTransaction {
-       // TODO: We should migrate away from providing the transaction, instead providing enough to
-       // allow the ChannelKeys to construct it from scratch. Luckily we already have HTLC data here,
-       // so we're probably most of the way there.
-       /// The commitment transaction itself, in unsigned form.
-       pub unsigned_tx: Transaction,
-       /// Our counterparty's signature for the transaction, above.
+       inner: CommitmentTransaction,
+       /// Our counterparty's signature for the transaction
        pub counterparty_sig: Signature,
+       /// All non-dust counterparty HTLC signatures, in the order they appear in the transaction
+       pub counterparty_htlc_sigs: Vec<Signature>,
        // Which order the signatures should go in when constructing the final commitment tx witness.
-       // The user should be able to reconstruc this themselves, so we don't bother to expose it.
+       // The user should be able to reconstruct this themselves, so we don't bother to expose it.
        holder_sig_first: bool,
-       pub(crate) keys: TxCreationKeys,
-       /// The feerate paid per 1000-weight-unit in this commitment transaction. This value is
-       /// controlled by the channel initiator.
-       pub feerate_per_kw: u32,
-       /// The HTLCs and counterparty htlc signatures which were included in this commitment transaction.
-       ///
-       /// Note that this includes all HTLCs, including ones which were considered dust and not
-       /// actually included in the transaction as it appears on-chain, but who's value is burned as
-       /// fees and not included in the to_holder or to_counterparty outputs.
-       ///
-       /// The counterparty HTLC signatures in the second element will always be set for non-dust HTLCs, ie
-       /// those for which transaction_output_index.is_some().
-       pub per_htlc: Vec<(HTLCOutputInCommitment, Option<Signature>)>,
 }
+
+impl Deref for HolderCommitmentTransaction {
+       type Target = CommitmentTransaction;
+
+       fn deref(&self) -> &Self::Target { &self.inner }
+}
+
+impl PartialEq for HolderCommitmentTransaction {
+       // We dont care whether we are signed in equality comparison
+       fn eq(&self, o: &Self) -> bool {
+               self.inner == o.inner
+       }
+}
+
+impl_writeable!(HolderCommitmentTransaction, 0, {
+       inner, counterparty_sig, counterparty_htlc_sigs, holder_sig_first
+});
+
 impl HolderCommitmentTransaction {
        #[cfg(test)]
        pub fn dummy() -> Self {
-               let dummy_input = TxIn {
-                       previous_output: OutPoint {
-                               txid: Default::default(),
-                               vout: 0,
-                       },
-                       script_sig: Default::default(),
-                       sequence: 0,
-                       witness: vec![]
+               let secp_ctx = Secp256k1::new();
+               let dummy_key = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
+               let dummy_sig = secp_ctx.sign(&secp256k1::Message::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[42; 32]).unwrap());
+
+               let keys = TxCreationKeys {
+                       per_commitment_point: dummy_key.clone(),
+                       revocation_key: dummy_key.clone(),
+                       broadcaster_htlc_key: dummy_key.clone(),
+                       countersignatory_htlc_key: dummy_key.clone(),
+                       broadcaster_delayed_payment_key: dummy_key.clone(),
                };
-               let dummy_key = PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&[42; 32]).unwrap());
-               let dummy_sig = Secp256k1::new().sign(&secp256k1::Message::from_slice(&[42; 32]).unwrap(), &SecretKey::from_slice(&[42; 32]).unwrap());
-               Self {
-                       unsigned_tx: Transaction {
-                               version: 2,
-                               input: vec![dummy_input],
-                               output: Vec::new(),
-                               lock_time: 0,
-                       },
+               let channel_pubkeys = ChannelPublicKeys {
+                       funding_pubkey: dummy_key.clone(),
+                       revocation_basepoint: dummy_key.clone(),
+                       payment_point: dummy_key.clone(),
+                       delayed_payment_basepoint: dummy_key.clone(),
+                       htlc_basepoint: dummy_key.clone()
+               };
+               let channel_parameters = ChannelTransactionParameters {
+                       holder_pubkeys: channel_pubkeys.clone(),
+                       holder_selected_contest_delay: 0,
+                       is_outbound_from_holder: false,
+                       counterparty_parameters: Some(CounterpartyChannelTransactionParameters { pubkeys: channel_pubkeys.clone(), selected_contest_delay: 0 }),
+                       funding_outpoint: Some(chain::transaction::OutPoint { txid: Default::default(), index: 0 })
+               };
+               let mut htlcs_with_aux: Vec<(_, ())> = Vec::new();
+               let inner = CommitmentTransaction::new_with_auxiliary_htlc_data(0, 0, 0, keys, 0, &mut htlcs_with_aux, &channel_parameters.as_counterparty_broadcastable());
+               HolderCommitmentTransaction {
+                       inner,
                        counterparty_sig: dummy_sig,
-                       holder_sig_first: false,
-                       keys: TxCreationKeys {
-                                       per_commitment_point: dummy_key.clone(),
-                                       revocation_key: dummy_key.clone(),
-                                       broadcaster_htlc_key: dummy_key.clone(),
-                                       countersignatory_htlc_key: dummy_key.clone(),
-                                       broadcaster_delayed_payment_key: dummy_key.clone(),
-                               },
-                       feerate_per_kw: 0,
-                       per_htlc: Vec::new()
+                       counterparty_htlc_sigs: Vec::new(),
+                       holder_sig_first: false
                }
        }
 
-       /// Generate a new HolderCommitmentTransaction based on a raw commitment transaction,
-       /// counterparty signature and both parties keys.
-       ///
-       /// The unsigned transaction outputs must be consistent with htlc_data.  This function
-       /// only checks that the shape and amounts are consistent, but does not check the scriptPubkey.
-       pub fn new_missing_holder_sig(unsigned_tx: Transaction, counterparty_sig: Signature, holder_funding_key: &PublicKey, counterparty_funding_key: &PublicKey, keys: TxCreationKeys, feerate_per_kw: u32, htlc_data: Vec<(HTLCOutputInCommitment, Option<Signature>)>) -> HolderCommitmentTransaction {
-               if unsigned_tx.input.len() != 1 { panic!("Tried to store a commitment transaction that had input count != 1!"); }
-               if unsigned_tx.input[0].witness.len() != 0 { panic!("Tried to store a signed commitment transaction?"); }
-
-               for htlc in &htlc_data {
-                       if let Some(index) = htlc.0.transaction_output_index {
-                               let out = &unsigned_tx.output[index as usize];
-                               if out.value != htlc.0.amount_msat / 1000 {
-                                       panic!("HTLC at index {} has incorrect amount", index);
-                               }
-                               if !out.script_pubkey.is_v0_p2wsh() {
-                                       panic!("HTLC at index {} doesn't have p2wsh scriptPubkey", index);
-                               }
-                       }
-               }
-
+       /// Create a new holder transaction with the given counterparty signatures.
+       /// The funding keys are used to figure out which signature should go first when building the transaction for broadcast.
+       pub fn new(commitment_tx: CommitmentTransaction, counterparty_sig: Signature, counterparty_htlc_sigs: Vec<Signature>, holder_funding_key: &PublicKey, counterparty_funding_key: &PublicKey) -> Self {
                Self {
-                       unsigned_tx,
+                       inner: commitment_tx,
                        counterparty_sig,
+                       counterparty_htlc_sigs,
                        holder_sig_first: holder_funding_key.serialize()[..] < counterparty_funding_key.serialize()[..],
-                       keys,
-                       feerate_per_kw,
-                       per_htlc: htlc_data,
                }
        }
 
-       /// The pre-calculated transaction creation public keys.
-       /// An external validating signer should not trust these keys.
-       pub fn trust_key_derivation(&self) -> &TxCreationKeys {
-               &self.keys
-       }
-
-       /// Get the txid of the holder commitment transaction contained in this
-       /// HolderCommitmentTransaction
-       pub fn txid(&self) -> Txid {
-               self.unsigned_tx.txid()
-       }
-
-       /// Gets holder signature for the contained commitment transaction given holder funding private key.
-       ///
-       /// Funding key is your key included in the 2-2 funding_outpoint lock. Should be provided
-       /// by your ChannelKeys.
-       /// Funding redeemscript is script locking funding_outpoint. This is the mutlsig script
-       /// between your own funding key and your counterparty's. Currently, this is provided in
-       /// ChannelKeys::sign_holder_commitment() calls directly.
-       /// Channel value is amount locked in funding_outpoint.
-       pub fn get_holder_sig<T: secp256k1::Signing>(&self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>) -> Signature {
-               let sighash = hash_to_message!(&bip143::SigHashCache::new(&self.unsigned_tx)
-                       .signature_hash(0, funding_redeemscript, channel_value_satoshis, SigHashType::All)[..]);
-               secp_ctx.sign(&sighash, funding_key)
-       }
-
        pub(crate) fn add_holder_sig(&self, funding_redeemscript: &Script, holder_sig: Signature) -> Transaction {
-               let mut tx = self.unsigned_tx.clone();
                // First push the multisig dummy, note that due to BIP147 (NULLDUMMY) it must be a zero-length element.
+               let mut tx = self.inner.built.transaction.clone();
                tx.input[0].witness.push(Vec::new());
 
                if self.holder_sig_first {
@@ -690,59 +775,402 @@ impl HolderCommitmentTransaction {
                tx.input[0].witness.push(funding_redeemscript.as_bytes().to_vec());
                tx
        }
+}
+
+/// A pre-built Bitcoin commitment transaction and its txid.
+#[derive(Clone)]
+pub struct BuiltCommitmentTransaction {
+       /// The commitment transaction
+       pub transaction: Transaction,
+       /// The txid for the commitment transaction.
+       ///
+       /// This is provided as a performance optimization, instead of calling transaction.txid()
+       /// multiple times.
+       pub txid: Txid,
+}
+
+impl_writeable!(BuiltCommitmentTransaction, 0, { transaction, txid });
+
+impl BuiltCommitmentTransaction {
+       /// Get the SIGHASH_ALL sighash value of the transaction.
+       ///
+       /// This can be used to verify a signature.
+       pub fn get_sighash_all(&self, funding_redeemscript: &Script, channel_value_satoshis: u64) -> Message {
+               let sighash = &bip143::SigHashCache::new(&self.transaction).signature_hash(0, funding_redeemscript, channel_value_satoshis, SigHashType::All)[..];
+               hash_to_message!(sighash)
+       }
+
+       /// Sign a transaction, either because we are counter-signing the counterparty's transaction or
+       /// because we are about to broadcast a holder transaction.
+       pub fn sign<T: secp256k1::Signing>(&self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>) -> Signature {
+               let sighash = self.get_sighash_all(funding_redeemscript, channel_value_satoshis);
+               secp_ctx.sign(&sighash, funding_key)
+       }
+}
+
+/// This class tracks the per-transaction information needed to build a commitment transaction and to
+/// actually build it and sign.  It is used for holder transactions that we sign only when needed
+/// and for transactions we sign for the counterparty.
+///
+/// This class can be used inside a signer implementation to generate a signature given the relevant
+/// secret key.
+#[derive(Clone)]
+pub struct CommitmentTransaction {
+       commitment_number: u64,
+       to_broadcaster_value_sat: u64,
+       to_countersignatory_value_sat: u64,
+       feerate_per_kw: u32,
+       htlcs: Vec<HTLCOutputInCommitment>,
+       // A cache of the parties' pubkeys required to construct the transaction, see doc for trust()
+       keys: TxCreationKeys,
+       // For access to the pre-built transaction, see doc for trust()
+       built: BuiltCommitmentTransaction,
+}
+
+impl PartialEq for CommitmentTransaction {
+       fn eq(&self, o: &Self) -> bool {
+               let eq = self.commitment_number == o.commitment_number &&
+                       self.to_broadcaster_value_sat == o.to_broadcaster_value_sat &&
+                       self.to_countersignatory_value_sat == o.to_countersignatory_value_sat &&
+                       self.feerate_per_kw == o.feerate_per_kw &&
+                       self.htlcs == o.htlcs &&
+                       self.keys == o.keys;
+               if eq {
+                       debug_assert_eq!(self.built.transaction, o.built.transaction);
+                       debug_assert_eq!(self.built.txid, o.built.txid);
+               }
+               eq
+       }
+}
+
+impl Writeable for Vec<HTLCOutputInCommitment> {
+       #[inline]
+       fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+               (self.len() as u16).write(w)?;
+               for e in self.iter() {
+                       e.write(w)?;
+               }
+               Ok(())
+       }
+}
+
+impl Readable for Vec<HTLCOutputInCommitment> {
+       #[inline]
+       fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
+               let len: u16 = Readable::read(r)?;
+               let byte_size = (len as usize)
+                       .checked_mul(HTLC_OUTPUT_IN_COMMITMENT_SIZE)
+                       .ok_or(DecodeError::BadLengthDescriptor)?;
+               if byte_size > MAX_BUF_SIZE {
+                       return Err(DecodeError::BadLengthDescriptor);
+               }
+               let mut ret = Vec::with_capacity(len as usize);
+               for _ in 0..len { ret.push(HTLCOutputInCommitment::read(r)?); }
+               Ok(ret)
+       }
+}
+
+impl_writeable!(CommitmentTransaction, 0, {
+       commitment_number,
+       to_broadcaster_value_sat,
+       to_countersignatory_value_sat,
+       feerate_per_kw,
+       htlcs,
+       keys,
+       built
+});
+
+impl CommitmentTransaction {
+       /// Construct an object of the class while assigning transaction output indices to HTLCs.
+       ///
+       /// Populates HTLCOutputInCommitment.transaction_output_index in htlcs_with_aux.
+       ///
+       /// The generic T allows the caller to match the HTLC output index with auxiliary data.
+       /// This auxiliary data is not stored in this object.
+       ///
+       /// Only include HTLCs that are above the dust limit for the channel.
+       pub fn new_with_auxiliary_htlc_data<T>(commitment_number: u64, to_broadcaster_value_sat: u64, to_countersignatory_value_sat: u64, keys: TxCreationKeys, feerate_per_kw: u32, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters) -> CommitmentTransaction {
+               // Sort outputs and populate output indices while keeping track of the auxiliary data
+               let (outputs, htlcs) = Self::internal_build_outputs(&keys, to_broadcaster_value_sat, to_countersignatory_value_sat, htlcs_with_aux, channel_parameters).unwrap();
+
+               let (obscured_commitment_transaction_number, txins) = Self::internal_build_inputs(commitment_number, channel_parameters);
+               let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs);
+               let txid = transaction.txid();
+               CommitmentTransaction {
+                       commitment_number,
+                       to_broadcaster_value_sat,
+                       to_countersignatory_value_sat,
+                       feerate_per_kw,
+                       htlcs,
+                       keys,
+                       built: BuiltCommitmentTransaction {
+                               transaction,
+                               txid
+                       },
+               }
+       }
+
+       fn internal_rebuild_transaction(&self, keys: &TxCreationKeys, channel_parameters: &DirectedChannelTransactionParameters) -> Result<BuiltCommitmentTransaction, ()> {
+               let (obscured_commitment_transaction_number, txins) = Self::internal_build_inputs(self.commitment_number, channel_parameters);
+
+               let mut htlcs_with_aux = self.htlcs.iter().map(|h| (h.clone(), ())).collect();
+               let (outputs, _) = Self::internal_build_outputs(keys, self.to_broadcaster_value_sat, self.to_countersignatory_value_sat, &mut htlcs_with_aux, channel_parameters)?;
+
+               let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs);
+               let txid = transaction.txid();
+               let built_transaction = BuiltCommitmentTransaction {
+                       transaction,
+                       txid
+               };
+               Ok(built_transaction)
+       }
+
+       fn make_transaction(obscured_commitment_transaction_number: u64, txins: Vec<TxIn>, outputs: Vec<TxOut>) -> Transaction {
+               Transaction {
+                       version: 2,
+                       lock_time: ((0x20 as u32) << 8 * 3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32),
+                       input: txins,
+                       output: outputs,
+               }
+       }
+
+       // This is used in two cases:
+       // - initial sorting of outputs / HTLCs in the constructor, in which case T is auxiliary data the
+       //   caller needs to have sorted together with the HTLCs so it can keep track of the output index
+       // - building of a bitcoin transaction during a verify() call, in which case T is just ()
+       fn internal_build_outputs<T>(keys: &TxCreationKeys, to_broadcaster_value_sat: u64, to_countersignatory_value_sat: u64, htlcs_with_aux: &mut Vec<(HTLCOutputInCommitment, T)>, channel_parameters: &DirectedChannelTransactionParameters) -> Result<(Vec<TxOut>, Vec<HTLCOutputInCommitment>), ()> {
+               let countersignatory_pubkeys = channel_parameters.countersignatory_pubkeys();
+               let contest_delay = channel_parameters.contest_delay();
+
+               let mut txouts: Vec<(TxOut, Option<&mut HTLCOutputInCommitment>)> = Vec::new();
+
+               if to_countersignatory_value_sat > 0 {
+                       let script = script_for_p2wpkh(&countersignatory_pubkeys.payment_point);
+                       txouts.push((
+                               TxOut {
+                                       script_pubkey: script.clone(),
+                                       value: to_countersignatory_value_sat,
+                               },
+                               None,
+                       ))
+               }
+
+               if to_broadcaster_value_sat > 0 {
+                       let redeem_script = get_revokeable_redeemscript(
+                               &keys.revocation_key,
+                               contest_delay,
+                               &keys.broadcaster_delayed_payment_key,
+                       );
+                       txouts.push((
+                               TxOut {
+                                       script_pubkey: redeem_script.to_v0_p2wsh(),
+                                       value: to_broadcaster_value_sat,
+                               },
+                               None,
+                       ));
+               }
+
+               let mut htlcs = Vec::with_capacity(htlcs_with_aux.len());
+               for (htlc, _) in htlcs_with_aux {
+                       let script = chan_utils::get_htlc_redeemscript(&htlc, &keys);
+                       let txout = TxOut {
+                               script_pubkey: script.to_v0_p2wsh(),
+                               value: htlc.amount_msat / 1000,
+                       };
+                       txouts.push((txout, Some(htlc)));
+               }
+
+               // Sort output in BIP-69 order (amount, scriptPubkey).  Tie-breaks based on HTLC
+               // CLTV expiration height.
+               sort_outputs(&mut txouts, |a, b| {
+                       if let &Some(ref a_htlcout) = a {
+                               if let &Some(ref b_htlcout) = b {
+                                       a_htlcout.cltv_expiry.cmp(&b_htlcout.cltv_expiry)
+                                               // Note that due to hash collisions, we have to have a fallback comparison
+                                               // here for fuzztarget mode (otherwise at least chanmon_fail_consistency
+                                               // may fail)!
+                                               .then(a_htlcout.payment_hash.0.cmp(&b_htlcout.payment_hash.0))
+                               // For non-HTLC outputs, if they're copying our SPK we don't really care if we
+                               // close the channel due to mismatches - they're doing something dumb:
+                               } else { cmp::Ordering::Equal }
+                       } else { cmp::Ordering::Equal }
+               });
+
+               let mut outputs = Vec::with_capacity(txouts.len());
+               for (idx, out) in txouts.drain(..).enumerate() {
+                       if let Some(htlc) = out.1 {
+                               htlc.transaction_output_index = Some(idx as u32);
+                               htlcs.push(htlc.clone());
+                       }
+                       outputs.push(out.0);
+               }
+               Ok((outputs, htlcs))
+       }
+
+       fn internal_build_inputs(commitment_number: u64, channel_parameters: &DirectedChannelTransactionParameters) -> (u64, Vec<TxIn>) {
+               let broadcaster_pubkeys = channel_parameters.broadcaster_pubkeys();
+               let countersignatory_pubkeys = channel_parameters.countersignatory_pubkeys();
+               let commitment_transaction_number_obscure_factor = get_commitment_transaction_number_obscure_factor(
+                       &broadcaster_pubkeys.payment_point,
+                       &countersignatory_pubkeys.payment_point,
+                       channel_parameters.is_outbound(),
+               );
+
+               let obscured_commitment_transaction_number =
+                       commitment_transaction_number_obscure_factor ^ (INITIAL_COMMITMENT_NUMBER - commitment_number);
+
+               let txins = {
+                       let mut ins: Vec<TxIn> = Vec::new();
+                       ins.push(TxIn {
+                               previous_output: channel_parameters.funding_outpoint(),
+                               script_sig: Script::new(),
+                               sequence: ((0x80 as u32) << 8 * 3)
+                                       | ((obscured_commitment_transaction_number >> 3 * 8) as u32),
+                               witness: Vec::new(),
+                       });
+                       ins
+               };
+               (obscured_commitment_transaction_number, txins)
+       }
+
+       /// The backwards-counting commitment number
+       pub fn commitment_number(&self) -> u64 {
+               self.commitment_number
+       }
+
+       /// The value to be sent to the broadcaster
+       pub fn to_broadcaster_value_sat(&self) -> u64 {
+               self.to_broadcaster_value_sat
+       }
+
+       /// The value to be sent to the counterparty
+       pub fn to_countersignatory_value_sat(&self) -> u64 {
+               self.to_countersignatory_value_sat
+       }
+
+       /// The feerate paid per 1000-weight-unit in this commitment transaction.
+       pub fn feerate_per_kw(&self) -> u32 {
+               self.feerate_per_kw
+       }
+
+       /// The non-dust HTLCs (direction, amt, height expiration, hash, transaction output index)
+       /// which were included in this commitment transaction in output order.
+       /// The transaction index is always populated.
+       pub fn htlcs(&self) -> &Vec<HTLCOutputInCommitment> {
+               &self.htlcs
+       }
+
+       /// Trust our pre-built transaction and derived transaction creation public keys.
+       ///
+       /// Applies a wrapper which allows access to these fields.
+       ///
+       /// This should only be used if you fully trust the builder of this object.  It should not
+       ///     be used by an external signer - instead use the verify function.
+       pub fn trust(&self) -> TrustedCommitmentTransaction {
+               TrustedCommitmentTransaction { inner: self }
+       }
+
+       /// Verify our pre-built transaction and derived transaction creation public keys.
+       ///
+       /// Applies a wrapper which allows access to these fields.
+       ///
+       /// An external validating signer must call this method before signing
+       /// or using the built transaction.
+       pub fn verify<T: secp256k1::Signing + secp256k1::Verification>(&self, channel_parameters: &DirectedChannelTransactionParameters, broadcaster_keys: &ChannelPublicKeys, countersignatory_keys: &ChannelPublicKeys, secp_ctx: &Secp256k1<T>) -> Result<TrustedCommitmentTransaction, ()> {
+               // This is the only field of the key cache that we trust
+               let per_commitment_point = self.keys.per_commitment_point;
+               let keys = TxCreationKeys::from_channel_static_keys(&per_commitment_point, broadcaster_keys, countersignatory_keys, secp_ctx).unwrap();
+               if keys != self.keys {
+                       return Err(());
+               }
+               let tx = self.internal_rebuild_transaction(&keys, channel_parameters)?;
+               if self.built.transaction != tx.transaction || self.built.txid != tx.txid {
+                       return Err(());
+               }
+               Ok(TrustedCommitmentTransaction { inner: self })
+       }
+}
+
+/// A wrapper on CommitmentTransaction indicating that the derived fields (the built bitcoin
+/// transaction and the transaction creation keys) are trusted.
+///
+/// See trust() and verify() functions on CommitmentTransaction.
+///
+/// This structure implements Deref.
+pub struct TrustedCommitmentTransaction<'a> {
+       inner: &'a CommitmentTransaction,
+}
+
+impl<'a> Deref for TrustedCommitmentTransaction<'a> {
+       type Target = CommitmentTransaction;
+
+       fn deref(&self) -> &Self::Target { self.inner }
+}
+
+impl<'a> TrustedCommitmentTransaction<'a> {
+       /// The transaction ID of the built Bitcoin transaction
+       pub fn txid(&self) -> Txid {
+               self.inner.built.txid
+       }
+
+       /// The pre-built Bitcoin commitment transaction
+       pub fn built_transaction(&self) -> &BuiltCommitmentTransaction {
+               &self.inner.built
+       }
+
+       /// The pre-calculated transaction creation public keys.
+       pub fn keys(&self) -> &TxCreationKeys {
+               &self.inner.keys
+       }
 
        /// Get a signature for each HTLC which was included in the commitment transaction (ie for
        /// which HTLCOutputInCommitment::transaction_output_index.is_some()).
        ///
-       /// The returned Vec has one entry for each HTLC, and in the same order. For HTLCs which were
-       /// considered dust and not included, a None entry exists, for all others a signature is
-       /// included.
-       pub fn get_htlc_sigs<T: secp256k1::Signing + secp256k1::Verification>(&self, htlc_base_key: &SecretKey, counterparty_selected_contest_delay: u16, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
-               let txid = self.txid();
-               let mut ret = Vec::with_capacity(self.per_htlc.len());
-               let holder_htlc_key = derive_private_key(secp_ctx, &self.keys.per_commitment_point, htlc_base_key).map_err(|_| ())?;
-
-               for this_htlc in self.per_htlc.iter() {
-                       if this_htlc.0.transaction_output_index.is_some() {
-                               let htlc_tx = build_htlc_transaction(&txid, self.feerate_per_kw, counterparty_selected_contest_delay, &this_htlc.0, &self.keys.broadcaster_delayed_payment_key, &self.keys.revocation_key);
-
-                               let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc.0, &self.keys.broadcaster_htlc_key, &self.keys.countersignatory_htlc_key, &self.keys.revocation_key);
-
-                               let sighash = hash_to_message!(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, this_htlc.0.amount_msat / 1000, SigHashType::All)[..]);
-                               ret.push(Some(secp_ctx.sign(&sighash, &holder_htlc_key)));
-                       } else {
-                               ret.push(None);
-                       }
+       /// The returned Vec has one entry for each HTLC, and in the same order.
+       pub fn get_htlc_sigs<T: secp256k1::Signing>(&self, htlc_base_key: &SecretKey, channel_parameters: &DirectedChannelTransactionParameters, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()> {
+               let inner = self.inner;
+               let keys = &inner.keys;
+               let txid = inner.built.txid;
+               let mut ret = Vec::with_capacity(inner.htlcs.len());
+               let holder_htlc_key = derive_private_key(secp_ctx, &inner.keys.per_commitment_point, htlc_base_key).map_err(|_| ())?;
+
+               for this_htlc in inner.htlcs.iter() {
+                       assert!(this_htlc.transaction_output_index.is_some());
+                       let htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
+
+                       let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc, &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key);
+
+                       let sighash = hash_to_message!(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, this_htlc.amount_msat / 1000, SigHashType::All)[..]);
+                       ret.push(secp_ctx.sign(&sighash, &holder_htlc_key));
                }
                Ok(ret)
        }
 
        /// Gets a signed HTLC transaction given a preimage (for !htlc.offered) and the holder HTLC transaction signature.
-       pub(crate) fn get_signed_htlc_tx(&self, htlc_index: usize, signature: &Signature, preimage: &Option<PaymentPreimage>, counterparty_selected_contest_delay: u16) -> Transaction {
-               let txid = self.txid();
-               let this_htlc = &self.per_htlc[htlc_index];
-               assert!(this_htlc.0.transaction_output_index.is_some());
+       pub(crate) fn get_signed_htlc_tx(&self, channel_parameters: &DirectedChannelTransactionParameters, htlc_index: usize, counterparty_signature: &Signature, signature: &Signature, preimage: &Option<PaymentPreimage>) -> Transaction {
+               let inner = self.inner;
+               let keys = &inner.keys;
+               let txid = inner.built.txid;
+               let this_htlc = &inner.htlcs[htlc_index];
+               assert!(this_htlc.transaction_output_index.is_some());
                // if we don't have preimage for an HTLC-Success, we can't generate an HTLC transaction.
-               if !this_htlc.0.offered && preimage.is_none() { unreachable!(); }
+               if !this_htlc.offered && preimage.is_none() { unreachable!(); }
                // Further, we should never be provided the preimage for an HTLC-Timeout transaction.
-               if  this_htlc.0.offered && preimage.is_some() { unreachable!(); }
+               if  this_htlc.offered && preimage.is_some() { unreachable!(); }
 
-               let mut htlc_tx = build_htlc_transaction(&txid, self.feerate_per_kw, counterparty_selected_contest_delay, &this_htlc.0, &self.keys.broadcaster_delayed_payment_key, &self.keys.revocation_key);
-               // Channel should have checked that we have a counterparty signature for this HTLC at
-               // creation, and we should have a sensible htlc transaction:
-               assert!(this_htlc.1.is_some());
+               let mut htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
 
-               let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc.0, &self.keys.broadcaster_htlc_key, &self.keys.countersignatory_htlc_key, &self.keys.revocation_key);
+               let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc, &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key);
 
                // First push the multisig dummy, note that due to BIP147 (NULLDUMMY) it must be a zero-length element.
                htlc_tx.input[0].witness.push(Vec::new());
 
-               htlc_tx.input[0].witness.push(this_htlc.1.unwrap().serialize_der().to_vec());
+               htlc_tx.input[0].witness.push(counterparty_signature.serialize_der().to_vec());
                htlc_tx.input[0].witness.push(signature.serialize_der().to_vec());
                htlc_tx.input[0].witness[1].push(SigHashType::All as u8);
                htlc_tx.input[0].witness[2].push(SigHashType::All as u8);
 
-               if this_htlc.0.offered {
+               if this_htlc.offered {
                        // Due to BIP146 (MINIMALIF) this must be a zero-length element to relay.
                        htlc_tx.input[0].witness.push(Vec::new());
                } else {
@@ -753,66 +1181,36 @@ impl HolderCommitmentTransaction {
                htlc_tx
        }
 }
-impl PartialEq for HolderCommitmentTransaction {
-       // We dont care whether we are signed in equality comparison
-       fn eq(&self, o: &Self) -> bool {
-               self.txid() == o.txid()
-       }
-}
-impl Writeable for HolderCommitmentTransaction {
-       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
-               if let Err(e) = self.unsigned_tx.consensus_encode(&mut WriterWriteAdaptor(writer)) {
-                       match e {
-                               encode::Error::Io(e) => return Err(e),
-                               _ => panic!("holder tx must have been well-formed!"),
-                       }
-               }
-               self.counterparty_sig.write(writer)?;
-               self.holder_sig_first.write(writer)?;
-               self.keys.write(writer)?;
-               self.feerate_per_kw.write(writer)?;
-               writer.write_all(&byte_utils::be64_to_array(self.per_htlc.len() as u64))?;
-               for &(ref htlc, ref sig) in self.per_htlc.iter() {
-                       htlc.write(writer)?;
-                       sig.write(writer)?;
-               }
-               Ok(())
+
+/// Get the transaction number obscure factor
+pub fn get_commitment_transaction_number_obscure_factor(
+       broadcaster_payment_basepoint: &PublicKey,
+       countersignatory_payment_basepoint: &PublicKey,
+       outbound_from_broadcaster: bool,
+) -> u64 {
+       let mut sha = Sha256::engine();
+
+       if outbound_from_broadcaster {
+               sha.input(&broadcaster_payment_basepoint.serialize());
+               sha.input(&countersignatory_payment_basepoint.serialize());
+       } else {
+               sha.input(&countersignatory_payment_basepoint.serialize());
+               sha.input(&broadcaster_payment_basepoint.serialize());
        }
+       let res = Sha256::from_engine(sha).into_inner();
+
+       ((res[26] as u64) << 5 * 8)
+               | ((res[27] as u64) << 4 * 8)
+               | ((res[28] as u64) << 3 * 8)
+               | ((res[29] as u64) << 2 * 8)
+               | ((res[30] as u64) << 1 * 8)
+               | ((res[31] as u64) << 0 * 8)
 }
-impl Readable for HolderCommitmentTransaction {
-       fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
-               let unsigned_tx = match Transaction::consensus_decode(reader.by_ref()) {
-                       Ok(tx) => tx,
-                       Err(e) => match e {
-                               encode::Error::Io(ioe) => return Err(DecodeError::Io(ioe)),
-                               _ => return Err(DecodeError::InvalidValue),
-                       },
-               };
-               let counterparty_sig = Readable::read(reader)?;
-               let holder_sig_first = Readable::read(reader)?;
-               let keys = Readable::read(reader)?;
-               let feerate_per_kw = Readable::read(reader)?;
-               let htlcs_count: u64 = Readable::read(reader)?;
-               let mut per_htlc = Vec::with_capacity(cmp::min(htlcs_count as usize, MAX_ALLOC_SIZE / mem::size_of::<(HTLCOutputInCommitment, Option<Signature>)>()));
-               for _ in 0..htlcs_count {
-                       let htlc: HTLCOutputInCommitment = Readable::read(reader)?;
-                       let sigs = Readable::read(reader)?;
-                       per_htlc.push((htlc, sigs));
-               }
 
-               if unsigned_tx.input.len() != 1 {
-                       // Ensure tx didn't hit the 0-input ambiguity case.
-                       return Err(DecodeError::InvalidValue);
-               }
-               Ok(Self {
-                       unsigned_tx,
-                       counterparty_sig,
-                       holder_sig_first,
-                       keys,
-                       feerate_per_kw,
-                       per_htlc,
-               })
-       }
+fn script_for_p2wpkh(key: &PublicKey) -> Script {
+       Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0)
+               .push_slice(&WPubkeyHash::hash(&key.serialize())[..])
+               .into_script()
 }
 
 #[cfg(test)]
index 594ef38e7224cfaeb416784ca43f4973cb2cf241..32ca3d30b91fd1cd076a7169bab9b0062e9fc3c3 100644 (file)
@@ -26,7 +26,7 @@ use ln::features::{ChannelFeatures, InitFeatures};
 use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
 use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentPreimage, PaymentHash, BREAKDOWN_TIMEOUT, MAX_LOCAL_BREAKDOWN_TIMEOUT};
-use ln::chan_utils::{CounterpartyCommitmentSecrets, HolderCommitmentTransaction, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, PreCalculatedTxCreationKeys};
+use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS};
 use ln::chan_utils;
 use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER};
@@ -111,7 +111,7 @@ enum InboundHTLCState {
        /// anyway). That said, ChannelMonitor does this for us (see
        /// ChannelMonitor::would_broadcast_at_height) so we actually remove the HTLC from our own
        /// local state before then, once we're sure that the next commitment_signed and
-       /// ChannelMonitor::provide_latest_local_commitment_tx_info will not include this HTLC.
+       /// ChannelMonitor::provide_latest_local_commitment_tx will not include this HTLC.
        LocalRemoved(InboundHTLCRemovalReason),
 }
 
@@ -242,7 +242,7 @@ enum ChannelState {
 const BOTH_SIDES_SHUTDOWN_MASK: u32 = ChannelState::LocalShutdownSent as u32 | ChannelState::RemoteShutdownSent as u32;
 const MULTI_STATE_FLAGS: u32 = BOTH_SIDES_SHUTDOWN_MASK | ChannelState::PeerDisconnected as u32 | ChannelState::MonitorUpdateFailed as u32;
 
-const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
+pub const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
 
 /// Liveness is called to fluctuate given peer disconnecton/monitor failures/closing.
 /// If channel is public, network should have a liveness view announced by us on a
@@ -272,7 +272,6 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
 
        channel_id: [u8; 32],
        channel_state: u32,
-       channel_outbound: bool,
        secp_ctx: Secp256k1<secp256k1::All>,
        channel_value_satoshis: u64,
 
@@ -342,8 +341,6 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
 
        last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, holder_sig)
 
-       funding_txo: Option<OutPoint>,
-
        /// The hash of the block in which the funding transaction reached our CONF_TARGET. We use this
        /// to detect unconfirmation after a serialize-unserialize roundtrip where we may not see a full
        /// series of block_connected/block_disconnected calls. Obviously this is not a guarantee as we
@@ -370,8 +367,6 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        // get_holder_selected_channel_reserve_satoshis(channel_value_sats: u64): u64
        counterparty_htlc_minimum_msat: u64,
        holder_htlc_minimum_msat: u64,
-       counterparty_selected_contest_delay: u16,
-       holder_selected_contest_delay: u16,
        #[cfg(test)]
        pub counterparty_max_accepted_htlcs: u16,
        #[cfg(not(test))]
@@ -379,7 +374,7 @@ pub(super) struct Channel<ChanSigner: ChannelKeys> {
        //implied by OUR_MAX_HTLCS: max_accepted_htlcs: u16,
        minimum_depth: u32,
 
-       counterparty_pubkeys: Option<ChannelPublicKeys>,
+       pub(crate) channel_transaction_parameters: ChannelTransactionParameters,
 
        counterparty_cur_commitment_point: Option<PublicKey>,
 
@@ -468,6 +463,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        {
                let holder_selected_contest_delay = config.own_channel_config.our_to_self_delay;
                let chan_keys = keys_provider.get_channel_keys(false, channel_value_satoshis);
+               let pubkeys = chan_keys.pubkeys().clone();
 
                if channel_value_satoshis >= MAX_FUNDING_SATOSHIS {
                        return Err(APIError::APIMisuseError{err: format!("funding_value must be smaller than {}, it was {}", MAX_FUNDING_SATOSHIS, channel_value_satoshis)});
@@ -492,7 +488,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                        channel_id: keys_provider.get_secure_random_bytes(),
                        channel_state: ChannelState::OurInitSent as u32,
-                       channel_outbound: true,
                        secp_ctx: Secp256k1::new(),
                        channel_value_satoshis,
 
@@ -530,7 +525,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                        last_sent_closing_fee: None,
 
-                       funding_txo: None,
                        funding_tx_confirmed_in: None,
                        short_channel_id: None,
                        last_block_connected: Default::default(),
@@ -543,12 +537,16 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        counterparty_selected_channel_reserve_satoshis: 0,
                        counterparty_htlc_minimum_msat: 0,
                        holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
-                       counterparty_selected_contest_delay: 0,
-                       holder_selected_contest_delay,
                        counterparty_max_accepted_htlcs: 0,
                        minimum_depth: 0, // Filled in in accept_channel
 
-                       counterparty_pubkeys: None,
+                       channel_transaction_parameters: ChannelTransactionParameters {
+                               holder_pubkeys: pubkeys,
+                               holder_selected_contest_delay: config.own_channel_config.our_to_self_delay,
+                               is_outbound_from_holder: true,
+                               counterparty_parameters: None,
+                               funding_outpoint: None
+                       },
                        counterparty_cur_commitment_point: None,
 
                        counterparty_prev_commitment_point: None,
@@ -582,7 +580,8 @@ 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 pubkeys = chan_keys.pubkeys().clone();
                let counterparty_pubkeys = ChannelPublicKeys {
                        funding_pubkey: msg.funding_pubkey,
                        revocation_basepoint: msg.revocation_basepoint,
@@ -590,7 +589,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        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 {
@@ -627,8 +625,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if msg.max_accepted_htlcs < 1 {
                        return Err(ChannelError::Close("0 max_accepted_htlcs makes for a useless channel".to_owned()));
                }
-               if msg.max_accepted_htlcs > 483 {
-                       return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than 483", msg.max_accepted_htlcs)));
+               if msg.max_accepted_htlcs > MAX_HTLCS {
+                       return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than {}", msg.max_accepted_htlcs, MAX_HTLCS)));
                }
 
                // Now check against optional parameters as set by config...
@@ -720,7 +718,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                        channel_id: msg.temporary_channel_id,
                        channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
-                       channel_outbound: false,
                        secp_ctx: Secp256k1::new(),
 
                        latest_monitor_update_id: 0,
@@ -757,7 +754,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                        last_sent_closing_fee: None,
 
-                       funding_txo: None,
                        funding_tx_confirmed_in: None,
                        short_channel_id: None,
                        last_block_connected: Default::default(),
@@ -771,12 +767,19 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        counterparty_selected_channel_reserve_satoshis: msg.channel_reserve_satoshis,
                        counterparty_htlc_minimum_msat: msg.htlc_minimum_msat,
                        holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
-                       counterparty_selected_contest_delay: msg.to_self_delay,
-                       holder_selected_contest_delay: config.own_channel_config.our_to_self_delay,
                        counterparty_max_accepted_htlcs: msg.max_accepted_htlcs,
                        minimum_depth: config.own_channel_config.minimum_depth,
 
-                       counterparty_pubkeys: Some(counterparty_pubkeys),
+                       channel_transaction_parameters: ChannelTransactionParameters {
+                               holder_pubkeys: pubkeys,
+                               holder_selected_contest_delay: config.own_channel_config.our_to_self_delay,
+                               is_outbound_from_holder: false,
+                               counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
+                                       selected_contest_delay: msg.to_self_delay,
+                                       pubkeys: counterparty_pubkeys,
+                               }),
+                               funding_outpoint: None
+                       },
                        counterparty_cur_commitment_point: Some(msg.first_per_commitment_point),
 
                        counterparty_prev_commitment_point: None,
@@ -797,13 +800,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        fn get_commitment_transaction_number_obscure_factor(&self) -> u64 {
                let mut sha = Sha256::engine();
 
-               let counterparty_payment_point = &self.counterparty_pubkeys.as_ref().unwrap().payment_point.serialize();
-               if self.channel_outbound {
-                       sha.input(&self.holder_keys.pubkeys().payment_point.serialize());
+               let counterparty_payment_point = &self.get_counterparty_pubkeys().payment_point.serialize();
+               if self.is_outbound() {
+                       sha.input(&self.get_holder_pubkeys().payment_point.serialize());
                        sha.input(counterparty_payment_point);
                } else {
                        sha.input(counterparty_payment_point);
-                       sha.input(&self.holder_keys.pubkeys().payment_point.serialize());
+                       sha.input(&self.get_holder_pubkeys().payment_point.serialize());
                }
                let res = Sha256::from_engine(sha).into_inner();
 
@@ -828,27 +831,15 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// have not yet committed it. Such HTLCs will only be included in transactions which are being
        /// generated by the peer which proposed adding the HTLCs, and thus we need to understand both
        /// which peer generated this transaction and "to whom" this transaction flows.
-       /// Returns (the transaction built, the number of HTLC outputs which were present in the
+       /// Returns (the transaction info, the number of HTLC outputs which were present in the
        /// transaction, the list of HTLCs which were not ignored when building the transaction).
        /// Note that below-dust HTLCs are included in the third return value, but not the second, and
        /// sources are provided only for outbound HTLCs in the third return value.
        #[inline]
-       fn build_commitment_transaction<L: Deref>(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u32, logger: &L) -> (Transaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger {
-               let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ (INITIAL_COMMITMENT_NUMBER - commitment_number);
-
-               let txins = {
-                       let mut ins: Vec<TxIn> = Vec::new();
-                       ins.push(TxIn {
-                               previous_output: self.funding_txo.unwrap().into_bitcoin_outpoint(),
-                               script_sig: Script::new(),
-                               sequence: ((0x80 as u32) << 8*3) | ((obscured_commitment_transaction_number >> 3*8) as u32),
-                               witness: Vec::new(),
-                       });
-                       ins
-               };
-
-               let mut txouts: Vec<(TxOut, Option<(HTLCOutputInCommitment, Option<&HTLCSource>)>)> = Vec::with_capacity(self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len() + 2);
+       fn build_commitment_transaction<L: Deref>(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u32, logger: &L) -> (CommitmentTransaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger {
                let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new();
+               let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len();
+               let mut included_non_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(num_htlcs);
 
                let broadcaster_dust_limit_satoshis = if local { self.holder_dust_limit_satoshis } else { self.counterparty_dust_limit_satoshis };
                let mut remote_htlc_total_msat = 0;
@@ -875,10 +866,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
                                        if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + (feerate_per_kw as u64 * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
                                                log_trace!(logger, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
-                                               txouts.push((TxOut {
-                                                       script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
-                                                       value: $htlc.amount_msat / 1000
-                                               }, Some((htlc_in_tx, $source))));
+                                               included_non_dust_htlcs.push((htlc_in_tx, $source));
                                        } else {
                                                log_trace!(logger, "   ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                included_dust_htlcs.push((htlc_in_tx, $source));
@@ -887,10 +875,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                        let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
                                        if $htlc.amount_msat / 1000 >= broadcaster_dust_limit_satoshis + (feerate_per_kw as u64 * HTLC_SUCCESS_TX_WEIGHT / 1000) {
                                                log_trace!(logger, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
-                                               txouts.push((TxOut { // "received HTLC output"
-                                                       script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
-                                                       value: $htlc.amount_msat / 1000
-                                               }, Some((htlc_in_tx, $source))));
+                                               included_non_dust_htlcs.push((htlc_in_tx, $source));
                                        } else {
                                                log_trace!(logger, "   ...including {} {} dust HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
                                                included_dust_htlcs.push((htlc_in_tx, $source));
@@ -978,73 +963,47 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64);
                }
 
-               let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (txouts.len() as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
-               let (value_to_self, value_to_remote) = if self.channel_outbound {
+               let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (included_non_dust_htlcs.len() as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
+               let (value_to_self, value_to_remote) = if self.is_outbound() {
                        (value_to_self_msat / 1000 - total_fee as i64, value_to_remote_msat / 1000)
                } else {
                        (value_to_self_msat / 1000, value_to_remote_msat / 1000 - total_fee as i64)
                };
 
-               let value_to_a = if local { value_to_self } else { value_to_remote };
-               let value_to_b = if local { value_to_remote } else { value_to_self };
+               let mut value_to_a = if local { value_to_self } else { value_to_remote };
+               let mut value_to_b = if local { value_to_remote } else { value_to_self };
 
                if value_to_a >= (broadcaster_dust_limit_satoshis as i64) {
                        log_trace!(logger, "   ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a);
-                       txouts.push((TxOut {
-                               script_pubkey: chan_utils::get_revokeable_redeemscript(&keys.revocation_key,
-                                                                                      if local { self.counterparty_selected_contest_delay } else { self.holder_selected_contest_delay },
-                                                                                      &keys.broadcaster_delayed_payment_key).to_v0_p2wsh(),
-                               value: value_to_a as u64
-                       }, None));
+               } else {
+                       value_to_a = 0;
                }
 
                if value_to_b >= (broadcaster_dust_limit_satoshis as i64) {
                        log_trace!(logger, "   ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
-                       let static_payment_pk = if local {
-                               self.counterparty_pubkeys.as_ref().unwrap().payment_point
-                       } else {
-                               self.holder_keys.pubkeys().payment_point
-                       }.serialize();
-                       txouts.push((TxOut {
-                               script_pubkey: Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0)
-                                                            .push_slice(&WPubkeyHash::hash(&static_payment_pk)[..])
-                                                            .into_script(),
-                               value: value_to_b as u64
-                       }, None));
-               }
-
-               transaction_utils::sort_outputs(&mut txouts, |a, b| {
-                       if let &Some(ref a_htlc) = a {
-                               if let &Some(ref b_htlc) = b {
-                                       a_htlc.0.cltv_expiry.cmp(&b_htlc.0.cltv_expiry)
-                                               // Note that due to hash collisions, we have to have a fallback comparison
-                                               // here for fuzztarget mode (otherwise at least chanmon_fail_consistency
-                                               // may fail)!
-                                               .then(a_htlc.0.payment_hash.0.cmp(&b_htlc.0.payment_hash.0))
-                               // For non-HTLC outputs, if they're copying our SPK we don't really care if we
-                               // close the channel due to mismatches - they're doing something dumb:
-                               } else { cmp::Ordering::Equal }
-                       } else { cmp::Ordering::Equal }
-               });
-
-               let mut outputs: Vec<TxOut> = Vec::with_capacity(txouts.len());
-               let mut htlcs_included: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(txouts.len() + included_dust_htlcs.len());
-               for (idx, mut out) in txouts.drain(..).enumerate() {
-                       outputs.push(out.0);
-                       if let Some((mut htlc, source_option)) = out.1.take() {
-                               htlc.transaction_output_index = Some(idx as u32);
-                               htlcs_included.push((htlc, source_option));
-                       }
+               } else {
+                       value_to_b = 0;
                }
-               let non_dust_htlc_count = htlcs_included.len();
+
+               let num_nondust_htlcs = included_non_dust_htlcs.len();
+
+               let channel_parameters =
+                       if local { self.channel_transaction_parameters.as_holder_broadcastable() }
+                       else { self.channel_transaction_parameters.as_counterparty_broadcastable() };
+               let tx = CommitmentTransaction::new_with_auxiliary_htlc_data(commitment_number,
+                                                                            value_to_a as u64,
+                                                                            value_to_b as u64,
+                                                                            keys.clone(),
+                                                                            feerate_per_kw,
+                                                                            &mut included_non_dust_htlcs,
+                                                                            &channel_parameters
+               );
+               let mut htlcs_included = included_non_dust_htlcs;
+               // The unwrap is safe, because all non-dust HTLCs have been assigned an output index
+               htlcs_included.sort_unstable_by_key(|h| h.0.transaction_output_index.unwrap());
                htlcs_included.append(&mut included_dust_htlcs);
 
-               (Transaction {
-                       version: 2,
-                       lock_time: ((0x20 as u32) << 8*3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32),
-                       input: txins,
-                       output: outputs,
-               }, non_dust_htlc_count, htlcs_included)
+               (tx, num_nondust_htlcs, htlcs_included)
        }
 
        #[inline]
@@ -1085,7 +1044,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let txins = {
                        let mut ins: Vec<TxIn> = Vec::new();
                        ins.push(TxIn {
-                               previous_output: self.funding_txo.unwrap().into_bitcoin_outpoint(),
+                               previous_output: self.funding_outpoint().into_bitcoin_outpoint(),
                                script_sig: Script::new(),
                                sequence: 0xffffffff,
                                witness: Vec::new(),
@@ -1098,14 +1057,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let mut txouts: Vec<(TxOut, ())> = Vec::new();
 
                let mut total_fee_satoshis = proposed_total_fee_satoshis;
-               let value_to_self: i64 = (self.value_to_self_msat as i64) / 1000 - if self.channel_outbound { total_fee_satoshis as i64 } else { 0 };
-               let value_to_remote: i64 = ((self.channel_value_satoshis * 1000 - self.value_to_self_msat) as i64 / 1000) - if self.channel_outbound { 0 } else { total_fee_satoshis as i64 };
+               let value_to_self: i64 = (self.value_to_self_msat as i64) / 1000 - if self.is_outbound() { total_fee_satoshis as i64 } else { 0 };
+               let value_to_remote: i64 = ((self.channel_value_satoshis * 1000 - self.value_to_self_msat) as i64 / 1000) - if self.is_outbound() { 0 } else { total_fee_satoshis as i64 };
 
                if value_to_self < 0 {
-                       assert!(self.channel_outbound);
+                       assert!(self.is_outbound());
                        total_fee_satoshis += (-value_to_self) as u64;
                } else if value_to_remote < 0 {
-                       assert!(!self.channel_outbound);
+                       assert!(!self.is_outbound());
                        total_fee_satoshis += (-value_to_remote) as u64;
                }
 
@@ -1138,6 +1097,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }, total_fee_satoshis)
        }
 
+       fn funding_outpoint(&self) -> OutPoint {
+               self.channel_transaction_parameters.funding_outpoint.unwrap()
+       }
+
        #[inline]
        /// Creates a set of keys for build_commitment_transaction to generate a transaction which our
        /// counterparty will sign (ie DO NOT send signatures over a transaction created by this to
@@ -1146,9 +1109,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// TODO Some magic rust shit to compile-time check this?
        fn build_holder_transaction_keys(&self, commitment_number: u64) -> Result<TxCreationKeys, ChannelError> {
                let per_commitment_point = self.holder_keys.get_per_commitment_point(commitment_number, &self.secp_ctx);
-               let delayed_payment_base = &self.holder_keys.pubkeys().delayed_payment_basepoint;
-               let htlc_basepoint = &self.holder_keys.pubkeys().htlc_basepoint;
-               let counterparty_pubkeys = self.counterparty_pubkeys.as_ref().unwrap();
+               let delayed_payment_base = &self.get_holder_pubkeys().delayed_payment_basepoint;
+               let htlc_basepoint = &self.get_holder_pubkeys().htlc_basepoint;
+               let counterparty_pubkeys = self.get_counterparty_pubkeys();
 
                Ok(secp_check!(TxCreationKeys::derive_new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys".to_owned()))
        }
@@ -1160,9 +1123,9 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        fn build_remote_transaction_keys(&self) -> Result<TxCreationKeys, ChannelError> {
                //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we
                //may see payments to it!
-               let revocation_basepoint = &self.holder_keys.pubkeys().revocation_basepoint;
-               let htlc_basepoint = &self.holder_keys.pubkeys().htlc_basepoint;
-               let counterparty_pubkeys = self.counterparty_pubkeys.as_ref().unwrap();
+               let revocation_basepoint = &self.get_holder_pubkeys().revocation_basepoint;
+               let htlc_basepoint = &self.get_holder_pubkeys().htlc_basepoint;
+               let counterparty_pubkeys = self.get_counterparty_pubkeys();
 
                Ok(secp_check!(TxCreationKeys::derive_new(&self.secp_ctx, &self.counterparty_cur_commitment_point.unwrap(), &counterparty_pubkeys.delayed_payment_basepoint, &counterparty_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys".to_owned()))
        }
@@ -1171,14 +1134,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// pays to get_funding_redeemscript().to_v0_p2wsh()).
        /// Panics if called before accept_channel/new_from_req
        pub fn get_funding_redeemscript(&self) -> Script {
-               make_funding_redeemscript(&self.holder_keys.pubkeys().funding_pubkey, self.counterparty_funding_pubkey())
+               make_funding_redeemscript(&self.get_holder_pubkeys().funding_pubkey, self.counterparty_funding_pubkey())
        }
 
        /// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output
        /// @local is used only to convert relevant internal structures which refer to remote vs local
        /// to decide value of outputs and direction of HTLCs.
        fn build_htlc_transaction(&self, prev_hash: &Txid, htlc: &HTLCOutputInCommitment, local: bool, keys: &TxCreationKeys, feerate_per_kw: u32) -> Transaction {
-               chan_utils::build_htlc_transaction(prev_hash, feerate_per_kw, if local { self.counterparty_selected_contest_delay } else { self.holder_selected_contest_delay }, htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key)
+               chan_utils::build_htlc_transaction(prev_hash, feerate_per_kw, if local { self.get_counterparty_selected_contest_delay() } else { self.get_holder_selected_contest_delay() }, htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key)
        }
 
        /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
@@ -1388,7 +1351,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: InitFeatures) -> Result<(), ChannelError> {
                // Check sanity of message fields:
-               if !self.channel_outbound {
+               if !self.is_outbound() {
                        return Err(ChannelError::Close("Got an accept_channel message from an inbound peer".to_owned()));
                }
                if self.channel_state != ChannelState::OurInitSent as u32 {
@@ -1421,8 +1384,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                if msg.max_accepted_htlcs < 1 {
                        return Err(ChannelError::Close("0 max_accepted_htlcs makes for a useless channel".to_owned()));
                }
-               if msg.max_accepted_htlcs > 483 {
-                       return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than 483", msg.max_accepted_htlcs)));
+               if msg.max_accepted_htlcs > MAX_HTLCS {
+                       return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than {}", msg.max_accepted_htlcs, MAX_HTLCS)));
                }
 
                // Now check against optional parameters as set by config...
@@ -1473,7 +1436,6 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                self.counterparty_max_htlc_value_in_flight_msat = cmp::min(msg.max_htlc_value_in_flight_msat, self.channel_value_satoshis * 1000);
                self.counterparty_selected_channel_reserve_satoshis = msg.channel_reserve_satoshis;
                self.counterparty_htlc_minimum_msat = msg.htlc_minimum_msat;
-               self.counterparty_selected_contest_delay = msg.to_self_delay;
                self.counterparty_max_accepted_htlcs = msg.max_accepted_htlcs;
                self.minimum_depth = msg.minimum_depth;
 
@@ -1485,8 +1447,10 @@ 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.channel_transaction_parameters.counterparty_parameters = Some(CounterpartyChannelTransactionParameters {
+                       selected_contest_delay: msg.to_self_delay,
+                       pubkeys: counterparty_pubkeys,
+               });
 
                self.counterparty_cur_commitment_point = Some(msg.first_per_commitment_point);
                self.counterparty_shutdown_scriptpubkey = counterparty_shutdown_scriptpubkey;
@@ -1496,35 +1460,40 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                Ok(())
        }
 
-       fn funding_created_signature<L: Deref>(&mut self, sig: &Signature, logger: &L) -> Result<(Transaction, HolderCommitmentTransaction, Signature), ChannelError> where L::Target: Logger {
+       fn funding_created_signature<L: Deref>(&mut self, sig: &Signature, logger: &L) -> Result<(Txid, CommitmentTransaction, Signature), ChannelError> where L::Target: Logger {
                let funding_script = self.get_funding_redeemscript();
 
                let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number)?;
                let initial_commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, false, self.feerate_per_kw, logger).0;
-               let sighash = hash_to_message!(&bip143::SigHashCache::new(&initial_commitment_tx).signature_hash(0, &funding_script, self.channel_value_satoshis, SigHashType::All)[..]);
-
-               // They sign the "our" commitment transaction...
-               log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.counterparty_funding_pubkey().serialize()), encode::serialize_hex(&initial_commitment_tx), log_bytes!(sighash[..]), encode::serialize_hex(&funding_script));
-               secp_check!(self.secp_ctx.verify(&sighash, &sig, self.counterparty_funding_pubkey()), "Invalid funding_created signature from peer".to_owned());
-
-               let tx = HolderCommitmentTransaction::new_missing_holder_sig(initial_commitment_tx, sig.clone(), &self.holder_keys.pubkeys().funding_pubkey, self.counterparty_funding_pubkey(), keys, self.feerate_per_kw, Vec::new());
+               {
+                       let trusted_tx = initial_commitment_tx.trust();
+                       let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
+                       let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.channel_value_satoshis);
+                       // They sign the holder commitment transaction...
+                       log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.counterparty_funding_pubkey().serialize()), encode::serialize_hex(&initial_commitment_bitcoin_tx.transaction), log_bytes!(sighash[..]), encode::serialize_hex(&funding_script));
+                       secp_check!(self.secp_ctx.verify(&sighash, &sig, self.counterparty_funding_pubkey()), "Invalid funding_created signature from peer".to_owned());
+               }
 
                let counterparty_keys = self.build_remote_transaction_keys()?;
                let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, self.feerate_per_kw, logger).0;
-               let pre_remote_keys = PreCalculatedTxCreationKeys::new(counterparty_keys);
-               let counterparty_signature = self.holder_keys.sign_counterparty_commitment(self.feerate_per_kw, &counterparty_initial_commitment_tx, &pre_remote_keys, &Vec::new(), &self.secp_ctx)
+
+               let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
+               let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
+               log_trace!(logger, "Initial counterparty ID {} tx {}", counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
+
+               let counterparty_signature = self.holder_keys.sign_counterparty_commitment(&counterparty_initial_commitment_tx, &self.secp_ctx)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0;
 
                // We sign "counterparty" commitment transaction, allowing them to broadcast the tx if they wish.
-               Ok((counterparty_initial_commitment_tx, tx, counterparty_signature))
+               Ok((counterparty_initial_bitcoin_tx.txid, initial_commitment_tx, counterparty_signature))
        }
 
        fn counterparty_funding_pubkey(&self) -> &PublicKey {
-               &self.counterparty_pubkeys.as_ref().expect("funding_pubkey() only allowed after accept_channel").funding_pubkey
+               &self.get_counterparty_pubkeys().funding_pubkey
        }
 
        pub fn funding_created<L: Deref>(&mut self, msg: &msgs::FundingCreated, logger: &L) -> Result<(msgs::FundingSigned, ChannelMonitor<ChanSigner>), ChannelError> where L::Target: Logger {
-               if self.channel_outbound {
+               if self.is_outbound() {
                        return Err(ChannelError::Close("Received funding_created for an outbound channel?".to_owned()));
                }
                if self.channel_state != (ChannelState::OurInitSent as u32 | ChannelState::TheirInitSent as u32) {
@@ -1539,32 +1508,46 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
                }
 
-               let funding_txo = OutPoint{ txid: msg.funding_txid, index: msg.funding_output_index };
-               self.funding_txo = Some(funding_txo.clone());
+               let funding_txo = OutPoint { txid: msg.funding_txid, index: msg.funding_output_index };
+               self.channel_transaction_parameters.funding_outpoint = Some(funding_txo);
+               // This is an externally observable change before we finish all our checks.  In particular
+               // funding_created_signature may fail.
+               self.holder_keys.ready_channel(&self.channel_transaction_parameters);
 
-               let (counterparty_initial_commitment_tx, initial_commitment_tx, signature) = match self.funding_created_signature(&msg.signature, logger) {
+               let (counterparty_initial_commitment_txid, initial_commitment_tx, signature) = match self.funding_created_signature(&msg.signature, logger) {
                        Ok(res) => res,
+                       Err(ChannelError::Close(e)) => {
+                               self.channel_transaction_parameters.funding_outpoint = None;
+                               return Err(ChannelError::Close(e));
+                       },
                        Err(e) => {
-                               self.funding_txo = None;
-                               return Err(e);
+                               // The only error we know how to handle is ChannelError::Close, so we fall over here
+                               // to make sure we don't continue with an inconsistent state.
+                               panic!("unexpected error type from funding_created_signature {:?}", e);
                        }
                };
 
+               let holder_commitment_tx = HolderCommitmentTransaction::new(
+                       initial_commitment_tx,
+                       msg.signature,
+                       Vec::new(),
+                       &self.get_holder_pubkeys().funding_pubkey,
+                       self.counterparty_funding_pubkey()
+               );
+
                // Now that we're past error-generating stuff, update our local state:
 
-               let counterparty_pubkeys = self.counterparty_pubkeys.as_ref().unwrap();
                let funding_redeemscript = self.get_funding_redeemscript();
                let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
                let mut channel_monitor = ChannelMonitor::new(self.holder_keys.clone(),
-                                                             &self.shutdown_pubkey, self.holder_selected_contest_delay,
+                                                             &self.shutdown_pubkey, self.get_holder_selected_contest_delay(),
                                                              &self.destination_script, (funding_txo, funding_txo_script.clone()),
-                                                             &counterparty_pubkeys.htlc_basepoint, &counterparty_pubkeys.delayed_payment_basepoint,
-                                                             self.counterparty_selected_contest_delay, funding_redeemscript.clone(), self.channel_value_satoshis,
+                                                             &self.channel_transaction_parameters,
+                                                             funding_redeemscript.clone(), self.channel_value_satoshis,
                                                              self.get_commitment_transaction_number_obscure_factor(),
+                                                             holder_commitment_tx);
 
-                                                             initial_commitment_tx.clone());
-
-               channel_monitor.provide_latest_counterparty_commitment_tx_info(&counterparty_initial_commitment_tx, Vec::new(), self.cur_counterparty_commitment_transaction_number, self.counterparty_cur_commitment_point.unwrap(), logger);
+               channel_monitor.provide_latest_counterparty_commitment_tx(counterparty_initial_commitment_txid, Vec::new(), self.cur_counterparty_commitment_transaction_number, self.counterparty_cur_commitment_point.unwrap(), logger);
 
                self.channel_state = ChannelState::FundingSent as u32;
                self.channel_id = funding_txo.to_channel_id();
@@ -1580,7 +1563,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Handles a funding_signed message from the remote end.
        /// If this call is successful, broadcast the funding transaction (and not before!)
        pub fn funding_signed<L: Deref>(&mut self, msg: &msgs::FundingSigned, logger: &L) -> Result<ChannelMonitor<ChanSigner>, ChannelError> where L::Target: Logger {
-               if !self.channel_outbound {
+               if !self.is_outbound() {
                        return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned()));
                }
                if self.channel_state & !(ChannelState::MonitorUpdateFailed as u32) != ChannelState::FundingCreated as u32 {
@@ -1596,33 +1579,44 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                let counterparty_keys = self.build_remote_transaction_keys()?;
                let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, self.feerate_per_kw, logger).0;
+               let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
+               let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
+
+               log_trace!(logger, "Initial counterparty ID {} tx {}", counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
 
                let holder_keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number)?;
                let initial_commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &holder_keys, true, false, self.feerate_per_kw, logger).0;
-               let sighash = hash_to_message!(&bip143::SigHashCache::new(&initial_commitment_tx).signature_hash(0, &funding_script, self.channel_value_satoshis, SigHashType::All)[..]);
+               {
+                       let trusted_tx = initial_commitment_tx.trust();
+                       let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
+                       let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.channel_value_satoshis);
+                       // They sign our commitment transaction, allowing us to broadcast the tx if we wish.
+                       if let Err(_) = self.secp_ctx.verify(&sighash, &msg.signature, &self.get_counterparty_pubkeys().funding_pubkey) {
+                               return Err(ChannelError::Close("Invalid funding_signed signature from peer".to_owned()));
+                       }
+               }
 
-               let counterparty_funding_pubkey = &self.counterparty_pubkeys.as_ref().unwrap().funding_pubkey;
+               let holder_commitment_tx = HolderCommitmentTransaction::new(
+                       initial_commitment_tx,
+                       msg.signature,
+                       Vec::new(),
+                       &self.get_holder_pubkeys().funding_pubkey,
+                       self.counterparty_funding_pubkey()
+               );
 
-               // They sign our commitment transaction, allowing us to broadcast the tx if we wish.
-               if let Err(_) = self.secp_ctx.verify(&sighash, &msg.signature, counterparty_funding_pubkey) {
-                       return Err(ChannelError::Close("Invalid funding_signed signature from peer".to_owned()));
-               }
 
-               let counterparty_pubkeys = self.counterparty_pubkeys.as_ref().unwrap();
                let funding_redeemscript = self.get_funding_redeemscript();
-               let funding_txo = self.funding_txo.as_ref().unwrap();
+               let funding_txo = self.get_funding_txo().unwrap();
                let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
-               let commitment_tx = HolderCommitmentTransaction::new_missing_holder_sig(initial_commitment_tx.clone(), msg.signature.clone(), &self.holder_keys.pubkeys().funding_pubkey, counterparty_funding_pubkey, holder_keys.clone(), self.feerate_per_kw, Vec::new());
                let mut channel_monitor = ChannelMonitor::new(self.holder_keys.clone(),
-                                                             &self.shutdown_pubkey, self.holder_selected_contest_delay,
-                                                             &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()),
-                                                             &counterparty_pubkeys.htlc_basepoint, &counterparty_pubkeys.delayed_payment_basepoint,
-                                                             self.counterparty_selected_contest_delay, funding_redeemscript.clone(), self.channel_value_satoshis,
+                                                             &self.shutdown_pubkey, self.get_holder_selected_contest_delay(),
+                                                             &self.destination_script, (funding_txo, funding_txo_script),
+                                                             &self.channel_transaction_parameters,
+                                                             funding_redeemscript.clone(), self.channel_value_satoshis,
                                                              self.get_commitment_transaction_number_obscure_factor(),
+                                                             holder_commitment_tx);
 
-                                                             commitment_tx);
-
-               channel_monitor.provide_latest_counterparty_commitment_tx_info(&counterparty_initial_commitment_tx, Vec::new(), self.cur_counterparty_commitment_transaction_number, self.counterparty_cur_commitment_point.unwrap(), logger);
+               channel_monitor.provide_latest_counterparty_commitment_tx(counterparty_initial_bitcoin_tx.txid, Vec::new(), self.cur_counterparty_commitment_transaction_number, self.counterparty_cur_commitment_point.unwrap(), logger);
 
                assert_eq!(self.channel_state & (ChannelState::MonitorUpdateFailed as u32), 0); // We have no had any monitor(s) yet to fail update!
                self.channel_state = ChannelState::FundingSent as u32;
@@ -1717,7 +1711,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        // to add a number of additional HTLCs to the calculation. Note that dust
        // HTLCs are excluded.
        fn next_local_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
-               assert!(self.channel_outbound);
+               assert!(self.is_outbound());
 
                let mut their_acked_htlcs = self.pending_inbound_htlcs.len();
                for ref htlc in self.pending_outbound_htlcs.iter() {
@@ -1748,7 +1742,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        // to add a number of additional HTLCs to the calculation. Note that dust HTLCs
        // are excluded.
        fn next_remote_commit_tx_fee_msat(&self, addl_htlcs: usize) -> u64 {
-               assert!(!self.channel_outbound);
+               assert!(!self.is_outbound());
 
                // When calculating the set of HTLCs which will be included in their next
                // commitment_signed, all inbound HTLCs are included (as all states imply it will be
@@ -1834,7 +1828,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                // Check that the remote can afford to pay for this HTLC on-chain at the current
                // feerate_per_kw, while maintaining their channel reserve (as required by the spec).
-               let remote_commit_tx_fee_msat = if self.channel_outbound { 0 } else {
+               let remote_commit_tx_fee_msat = if self.is_outbound() { 0 } else {
                        // +1 for this HTLC.
                        self.next_remote_commit_tx_fee_msat(1)
                };
@@ -1848,7 +1842,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value".to_owned()));
                }
 
-               if !self.channel_outbound {
+               if !self.is_outbound() {
                        // `+1` for this HTLC, `2 *` and `+1` fee spike buffer we keep for the remote. This deviates from the
                        // spec because in the spec, the fee spike buffer requirement doesn't exist on the receiver's side,
                        // only on the sender's.
@@ -1979,28 +1973,32 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let keys = self.build_holder_transaction_keys(self.cur_holder_commitment_transaction_number).map_err(|e| (None, e))?;
 
                let mut update_fee = false;
-               let feerate_per_kw = if !self.channel_outbound && self.pending_update_fee.is_some() {
+               let feerate_per_kw = if !self.is_outbound() && self.pending_update_fee.is_some() {
                        update_fee = true;
                        self.pending_update_fee.unwrap()
                } else {
                        self.feerate_per_kw
                };
 
-               let mut commitment_tx = {
-                       let mut commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, false, feerate_per_kw, logger);
-                       let htlcs_cloned: Vec<_> = commitment_tx.2.drain(..).map(|htlc| (htlc.0, htlc.1.map(|h| h.clone()))).collect();
-                       (commitment_tx.0, commitment_tx.1, htlcs_cloned)
+               let (num_htlcs, mut htlcs_cloned, commitment_tx, commitment_txid) = {
+                       let commitment_tx = self.build_commitment_transaction(self.cur_holder_commitment_transaction_number, &keys, true, false, feerate_per_kw, logger);
+                       let commitment_txid = {
+                               let trusted_tx = commitment_tx.0.trust();
+                               let bitcoin_tx = trusted_tx.built_transaction();
+                               let sighash = bitcoin_tx.get_sighash_all(&funding_script, self.channel_value_satoshis);
+
+                               log_trace!(logger, "Checking commitment tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(msg.signature.serialize_compact()[..]), log_bytes!(self.counterparty_funding_pubkey().serialize()), encode::serialize_hex(&bitcoin_tx.transaction), log_bytes!(sighash[..]), encode::serialize_hex(&funding_script));
+                               if let Err(_) = self.secp_ctx.verify(&sighash, &msg.signature, &self.counterparty_funding_pubkey()) {
+                                       return Err((None, ChannelError::Close("Invalid commitment tx signature from peer".to_owned())));
+                               }
+                               bitcoin_tx.txid
+                       };
+                       let htlcs_cloned: Vec<_> = commitment_tx.2.iter().map(|htlc| (htlc.0.clone(), htlc.1.map(|h| h.clone()))).collect();
+                       (commitment_tx.1, htlcs_cloned, commitment_tx.0, commitment_txid)
                };
-               let commitment_txid = commitment_tx.0.txid();
-               let sighash = hash_to_message!(&bip143::SigHashCache::new(&commitment_tx.0).signature_hash(0, &funding_script, self.channel_value_satoshis, SigHashType::All)[..]);
-               log_trace!(logger, "Checking commitment tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(msg.signature.serialize_compact()[..]), log_bytes!(self.counterparty_funding_pubkey().serialize()), encode::serialize_hex(&commitment_tx.0), log_bytes!(sighash[..]), encode::serialize_hex(&funding_script));
-               if let Err(_) = self.secp_ctx.verify(&sighash, &msg.signature, &self.counterparty_funding_pubkey()) {
-                       return Err((None, ChannelError::Close("Invalid commitment tx signature from peer".to_owned())));
-               }
 
                //If channel fee was updated by funder confirm funder can afford the new fee rate when applied to the current local commitment transaction
                if update_fee {
-                       let num_htlcs = commitment_tx.1;
                        let total_fee = feerate_per_kw as u64 * (COMMITMENT_TX_BASE_WEIGHT + (num_htlcs as u64) * COMMITMENT_TX_WEIGHT_PER_HTLC) / 1000;
 
                        let counterparty_reserve_we_require = Channel::<ChanSigner>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis);
@@ -2009,15 +2007,13 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                }
 
-               if msg.htlc_signatures.len() != commitment_tx.1 {
-                       return Err((None, ChannelError::Close(format!("Got wrong number of HTLC signatures ({}) from remote. It must be {}", msg.htlc_signatures.len(), commitment_tx.1))));
+               if msg.htlc_signatures.len() != num_htlcs {
+                       return Err((None, ChannelError::Close(format!("Got wrong number of HTLC signatures ({}) from remote. It must be {}", msg.htlc_signatures.len(), num_htlcs))));
                }
 
-               // TODO: Merge these two, sadly they are currently both required to be passed separately to
-               // ChannelMonitor:
-               let mut htlcs_without_source = Vec::with_capacity(commitment_tx.2.len());
-               let mut htlcs_and_sigs = Vec::with_capacity(commitment_tx.2.len());
-               for (idx, (htlc, source)) in commitment_tx.2.drain(..).enumerate() {
+               // TODO: Sadly, we pass HTLCs twice to ChannelMonitor: once via the HolderCommitmentTransaction and once via the update
+               let mut htlcs_and_sigs = Vec::with_capacity(htlcs_cloned.len());
+               for (idx, (htlc, source)) in htlcs_cloned.drain(..).enumerate() {
                        if let Some(_) = htlc.transaction_output_index {
                                let htlc_tx = self.build_htlc_transaction(&commitment_txid, &htlc, true, &keys, feerate_per_kw);
                                let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
@@ -2026,20 +2022,26 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                if let Err(_) = self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &keys.countersignatory_htlc_key) {
                                        return Err((None, ChannelError::Close("Invalid HTLC tx signature from peer".to_owned())));
                                }
-                               htlcs_without_source.push((htlc.clone(), Some(msg.htlc_signatures[idx])));
                                htlcs_and_sigs.push((htlc, Some(msg.htlc_signatures[idx]), source));
                        } else {
-                               htlcs_without_source.push((htlc.clone(), None));
                                htlcs_and_sigs.push((htlc, None, source));
                        }
                }
 
+               let holder_commitment_tx = HolderCommitmentTransaction::new(
+                       commitment_tx,
+                       msg.signature,
+                       msg.htlc_signatures.clone(),
+                       &self.get_holder_pubkeys().funding_pubkey,
+                       self.counterparty_funding_pubkey()
+               );
+
                let next_per_commitment_point = self.holder_keys.get_per_commitment_point(self.cur_holder_commitment_transaction_number - 1, &self.secp_ctx);
                let per_commitment_secret = self.holder_keys.release_commitment_secret(self.cur_holder_commitment_transaction_number + 1);
 
                // Update state now that we've passed all the can-fail calls...
                let mut need_commitment = false;
-               if !self.channel_outbound {
+               if !self.is_outbound() {
                        if let Some(fee_update) = self.pending_update_fee {
                                self.feerate_per_kw = fee_update;
                                // We later use the presence of pending_update_fee to indicate we should generate a
@@ -2052,13 +2054,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        }
                }
 
-               let counterparty_funding_pubkey = self.counterparty_pubkeys.as_ref().unwrap().funding_pubkey;
-
                self.latest_monitor_update_id += 1;
                let mut monitor_update = ChannelMonitorUpdate {
                        update_id: self.latest_monitor_update_id,
                        updates: vec![ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo {
-                               commitment_tx: HolderCommitmentTransaction::new_missing_holder_sig(commitment_tx.0, msg.signature.clone(), &self.holder_keys.pubkeys().funding_pubkey, &counterparty_funding_pubkey, keys, self.feerate_per_kw, htlcs_without_source),
+                               commitment_tx: holder_commitment_tx,
                                htlc_outputs: htlcs_and_sigs
                        }]
                };
@@ -2380,7 +2380,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
                self.value_to_self_msat = (self.value_to_self_msat as i64 + value_to_self_msat_diff) as u64;
 
-               if self.channel_outbound {
+               if self.is_outbound() {
                        if let Some(feerate) = self.pending_update_fee.take() {
                                self.feerate_per_kw = feerate;
                        }
@@ -2464,7 +2464,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// further details on the optionness of the return value.
        /// You MUST call send_commitment prior to any other calls on this Channel
        fn send_update_fee(&mut self, feerate_per_kw: u32) -> Option<msgs::UpdateFee> {
-               if !self.channel_outbound {
+               if !self.is_outbound() {
                        panic!("Cannot send fee from inbound channel");
                }
                if !self.is_usable() {
@@ -2596,7 +2596,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32);
                self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32);
 
-               let needs_broadcast_safe = self.channel_state & (ChannelState::FundingSent as u32) != 0 && self.channel_outbound;
+               let needs_broadcast_safe = self.channel_state & (ChannelState::FundingSent as u32) != 0 && self.is_outbound();
 
                // Because we will never generate a FundingBroadcastSafe event when we're in
                // MonitorUpdateFailed, if we assume the user only broadcast the funding transaction when
@@ -2605,7 +2605,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                // monitor on funding_created, and we even got the funding transaction confirmed before the
                // monitor was persisted.
                let funding_locked = if self.monitor_pending_funding_locked {
-                       assert!(!self.channel_outbound, "Funding transaction broadcast without FundingBroadcastSafe!");
+                       assert!(!self.is_outbound(), "Funding transaction broadcast without FundingBroadcastSafe!");
                        self.monitor_pending_funding_locked = false;
                        let next_per_commitment_point = self.holder_keys.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
                        Some(msgs::FundingLocked {
@@ -2646,7 +2646,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        pub fn update_fee<F: Deref>(&mut self, fee_estimator: &F, msg: &msgs::UpdateFee) -> Result<(), ChannelError>
                where F::Target: FeeEstimator
        {
-               if self.channel_outbound {
+               if self.is_outbound() {
                        return Err(ChannelError::Close("Non-funding remote tried to update channel fee".to_owned()));
                }
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
@@ -2879,7 +2879,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        fn maybe_propose_first_closing_signed<F: Deref>(&mut self, fee_estimator: &F) -> Option<msgs::ClosingSigned>
                where F::Target: FeeEstimator
        {
-               if !self.channel_outbound || !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() ||
+               if !self.is_outbound() || !self.pending_inbound_htlcs.is_empty() || !self.pending_outbound_htlcs.is_empty() ||
                                self.channel_state & (BOTH_SIDES_SHUTDOWN_MASK | ChannelState::AwaitingRemoteRevoke as u32) != BOTH_SIDES_SHUTDOWN_MASK ||
                                self.last_sent_closing_fee.is_some() || self.pending_update_fee.is_some() {
                        return None;
@@ -2928,7 +2928,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                // BOLT 2 says we must only send a scriptpubkey of certain standard forms, which are up to
                // 34 bytes in length, so don't let the remote peer feed us some super fee-heavy script.
-               if self.channel_outbound && msg.scriptpubkey.len() > 34 {
+               if self.is_outbound() && msg.scriptpubkey.len() > 34 {
                        return Err(ChannelError::Close(format!("Got counterparty shutdown_scriptpubkey ({}) of absurd length from remote peer", msg.scriptpubkey.to_bytes().to_hex())));
                }
 
@@ -2990,7 +2990,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                tx.input[0].witness.push(Vec::new()); // First is the multisig dummy
 
-               let funding_key = self.holder_keys.pubkeys().funding_pubkey.serialize();
+               let funding_key = self.get_holder_pubkeys().funding_pubkey.serialize();
                let counterparty_funding_key = self.counterparty_funding_pubkey().serialize();
                if funding_key[..] < counterparty_funding_key[..] {
                        tx.input[0].witness.push(sig.serialize_der().to_vec());
@@ -3028,9 +3028,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
                let mut sighash = hash_to_message!(&bip143::SigHashCache::new(&closing_tx).signature_hash(0, &funding_redeemscript, self.channel_value_satoshis, SigHashType::All)[..]);
 
-               let counterparty_funding_pubkey = &self.counterparty_pubkeys.as_ref().unwrap().funding_pubkey;
-
-               match self.secp_ctx.verify(&sighash, &msg.signature, counterparty_funding_pubkey) {
+               match self.secp_ctx.verify(&sighash, &msg.signature, &self.get_counterparty_pubkeys().funding_pubkey) {
                        Ok(_) => {},
                        Err(_e) => {
                                // The remote end may have decided to revoke their output due to inconsistent dust
@@ -3073,7 +3071,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                let mut min_feerate = 253;
-               if self.channel_outbound {
+               if self.is_outbound() {
                        let max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
                        if (msg.fee_satoshis as u64) > max_feerate as u64 * closing_tx_max_weight / 1000 {
                                if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
@@ -3134,7 +3132,23 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Returns the funding_txo we either got from our peer, or were given by
        /// get_outbound_funding_created.
        pub fn get_funding_txo(&self) -> Option<OutPoint> {
-               self.funding_txo
+               self.channel_transaction_parameters.funding_outpoint
+       }
+
+       fn get_holder_selected_contest_delay(&self) -> u16 {
+               self.channel_transaction_parameters.holder_selected_contest_delay
+       }
+
+       fn get_holder_pubkeys(&self) -> &ChannelPublicKeys {
+               &self.channel_transaction_parameters.holder_pubkeys
+       }
+
+       fn get_counterparty_selected_contest_delay(&self) -> u16 {
+               self.channel_transaction_parameters.counterparty_parameters.as_ref().unwrap().selected_contest_delay
+       }
+
+       fn get_counterparty_pubkeys(&self) -> &ChannelPublicKeys {
+               &self.channel_transaction_parameters.counterparty_parameters.as_ref().unwrap().pubkeys
        }
 
        /// Allowed in any state (including after shutdown)
@@ -3234,7 +3248,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        pub fn is_outbound(&self) -> bool {
-               self.channel_outbound
+               self.channel_transaction_parameters.is_outbound_from_holder
        }
 
        /// Gets the fee we'd want to charge for adding an HTLC output to this Channel
@@ -3248,7 +3262,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                // the fee cost of the HTLC-Success/HTLC-Timeout transaction:
                let mut res = self.feerate_per_kw as u64 * cmp::max(HTLC_TIMEOUT_TX_WEIGHT, HTLC_SUCCESS_TX_WEIGHT) / 1000;
 
-               if self.channel_outbound {
+               if self.is_outbound() {
                        // + the marginal fee increase cost to us in the commitment transaction:
                        res += self.feerate_per_kw as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC / 1000;
                }
@@ -3354,11 +3368,12 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
                if non_shutdown_state & !(ChannelState::TheirFundingLocked as u32) == ChannelState::FundingSent as u32 {
                        for &(index_in_block, tx) in txdata.iter() {
-                               if tx.txid() == self.funding_txo.unwrap().txid {
-                                       let txo_idx = self.funding_txo.unwrap().index as usize;
+                               let funding_txo = self.get_funding_txo().unwrap();
+                               if tx.txid() == funding_txo.txid {
+                                       let txo_idx = funding_txo.index as usize;
                                        if txo_idx >= tx.output.len() || tx.output[txo_idx].script_pubkey != self.get_funding_redeemscript().to_v0_p2wsh() ||
                                                        tx.output[txo_idx].value != self.channel_value_satoshis {
-                                               if self.channel_outbound {
+                                               if self.is_outbound() {
                                                        // If we generated the funding transaction and it doesn't match what it
                                                        // should, the client is really broken and we should just panic and
                                                        // tell them off. That said, because hash collisions happen with high
@@ -3374,7 +3389,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                                        data: "funding tx had wrong script/value".to_owned()
                                                });
                                        } else {
-                                               if self.channel_outbound {
+                                               if self.is_outbound() {
                                                        for input in tx.input.iter() {
                                                                if input.witness.is_empty() {
                                                                        // We generated a malleable funding transaction, implying we've
@@ -3464,7 +3479,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        // something in the handler for the message that prompted this message):
 
        pub fn get_open_channel(&self, chain_hash: BlockHash) -> msgs::OpenChannel {
-               if !self.channel_outbound {
+               if !self.is_outbound() {
                        panic!("Tried to open a channel for an inbound channel?");
                }
                if self.channel_state != ChannelState::OurInitSent as u32 {
@@ -3476,7 +3491,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                let first_per_commitment_point = self.holder_keys.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
-               let keys = self.holder_keys.pubkeys();
+               let keys = self.get_holder_pubkeys();
 
                msgs::OpenChannel {
                        chain_hash,
@@ -3488,7 +3503,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        channel_reserve_satoshis: Channel::<ChanSigner>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis),
                        htlc_minimum_msat: self.holder_htlc_minimum_msat,
                        feerate_per_kw: self.feerate_per_kw as u32,
-                       to_self_delay: self.holder_selected_contest_delay,
+                       to_self_delay: self.get_holder_selected_contest_delay(),
                        max_accepted_htlcs: OUR_MAX_HTLCS,
                        funding_pubkey: keys.funding_pubkey,
                        revocation_basepoint: keys.revocation_basepoint,
@@ -3502,7 +3517,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        }
 
        pub fn get_accept_channel(&self) -> msgs::AcceptChannel {
-               if self.channel_outbound {
+               if self.is_outbound() {
                        panic!("Tried to send accept_channel for an outbound channel?");
                }
                if self.channel_state != (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32) {
@@ -3513,7 +3528,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
 
                let first_per_commitment_point = self.holder_keys.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
-               let keys = self.holder_keys.pubkeys();
+               let keys = self.get_holder_pubkeys();
 
                msgs::AcceptChannel {
                        temporary_channel_id: self.channel_id,
@@ -3522,7 +3537,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        channel_reserve_satoshis: Channel::<ChanSigner>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis),
                        htlc_minimum_msat: self.holder_htlc_minimum_msat,
                        minimum_depth: self.minimum_depth,
-                       to_self_delay: self.holder_selected_contest_delay,
+                       to_self_delay: self.get_holder_selected_contest_delay(),
                        max_accepted_htlcs: OUR_MAX_HTLCS,
                        funding_pubkey: keys.funding_pubkey,
                        revocation_basepoint: keys.revocation_basepoint,
@@ -3538,8 +3553,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        fn get_outbound_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
                let counterparty_keys = self.build_remote_transaction_keys()?;
                let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, self.feerate_per_kw, logger).0;
-               let pre_remote_keys = PreCalculatedTxCreationKeys::new(counterparty_keys);
-               Ok(self.holder_keys.sign_counterparty_commitment(self.feerate_per_kw, &counterparty_initial_commitment_tx, &pre_remote_keys, &Vec::new(), &self.secp_ctx)
+               Ok(self.holder_keys.sign_counterparty_commitment(&counterparty_initial_commitment_tx, &self.secp_ctx)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0)
        }
 
@@ -3551,7 +3565,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
        /// Do NOT broadcast the funding transaction until after a successful funding_signed call!
        /// If an Err is returned, it is a ChannelError::Close.
        pub fn get_outbound_funding_created<L: Deref>(&mut self, funding_txo: OutPoint, logger: &L) -> Result<msgs::FundingCreated, ChannelError> where L::Target: Logger {
-               if !self.channel_outbound {
+               if !self.is_outbound() {
                        panic!("Tried to create outbound funding_created message on an inbound channel!");
                }
                if self.channel_state != (ChannelState::OurInitSent as u32 | ChannelState::TheirInitSent as u32) {
@@ -3563,12 +3577,14 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
                }
 
-               self.funding_txo = Some(funding_txo.clone());
+               self.channel_transaction_parameters.funding_outpoint = Some(funding_txo);
+               self.holder_keys.ready_channel(&self.channel_transaction_parameters);
+
                let signature = match self.get_outbound_funding_created_signature(logger) {
                        Ok(res) => res,
                        Err(e) => {
                                log_error!(logger, "Got bad signatures: {:?}!", e);
-                               self.funding_txo = None;
+                               self.channel_transaction_parameters.funding_outpoint = None;
                                return Err(e);
                        }
                };
@@ -3615,8 +3631,8 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        short_channel_id: self.get_short_channel_id().unwrap(),
                        node_id_1: if were_node_one { node_id } else { self.get_counterparty_node_id() },
                        node_id_2: if were_node_one { self.get_counterparty_node_id() } else { node_id },
-                       bitcoin_key_1: if were_node_one { self.holder_keys.pubkeys().funding_pubkey } else { self.counterparty_funding_pubkey().clone() },
-                       bitcoin_key_2: if were_node_one { self.counterparty_funding_pubkey().clone() } else { self.holder_keys.pubkeys().funding_pubkey },
+                       bitcoin_key_1: if were_node_one { self.get_holder_pubkeys().funding_pubkey } else { self.counterparty_funding_pubkey().clone() },
+                       bitcoin_key_2: if were_node_one { self.counterparty_funding_pubkey().clone() } else { self.get_holder_pubkeys().funding_pubkey },
                        excess_data: Vec::new(),
                };
 
@@ -3723,7 +3739,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        return Err(ChannelError::Ignore(format!("Cannot send value that would put us over the max HTLC value in flight our peer will accept ({})", self.counterparty_max_htlc_value_in_flight_msat)));
                }
 
-               if !self.channel_outbound {
+               if !self.is_outbound() {
                        // Check that we won't violate the remote channel reserve by adding this HTLC.
 
                        let counterparty_balance_msat = self.channel_value_satoshis * 1000 - self.value_to_self_msat;
@@ -3742,7 +3758,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
                // The `+1` is for the HTLC currently being added to the commitment tx and
                // the `2 *` and `+1` are for the fee spike buffer.
-               let commit_tx_fee_msat = if self.channel_outbound {
+               let commit_tx_fee_msat = if self.is_outbound() {
                        2 * self.next_local_commit_tx_fee_msat(1 + 1)
                } else { 0 };
                if pending_value_to_self_msat - amount_msat < commit_tx_fee_msat {
@@ -3847,7 +3863,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                }
                self.resend_order = RAACommitmentOrder::RevokeAndACKFirst;
 
-               let (res, counterparty_commitment_tx, htlcs) = match self.send_commitment_no_state_update(logger) {
+               let (res, counterparty_commitment_txid, htlcs) = match self.send_commitment_no_state_update(logger) {
                        Ok((res, (counterparty_commitment_tx, mut htlcs))) => {
                                // Update state now that we've passed all the can-fail calls...
                                let htlcs_no_ref: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)> =
@@ -3861,7 +3877,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                let monitor_update = ChannelMonitorUpdate {
                        update_id: self.latest_monitor_update_id,
                        updates: vec![ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
-                               unsigned_commitment_tx: counterparty_commitment_tx.clone(),
+                               commitment_txid: counterparty_commitment_txid,
                                htlc_outputs: htlcs.clone(),
                                commitment_number: self.cur_counterparty_commitment_transaction_number,
                                their_revocation_point: self.counterparty_cur_commitment_point.unwrap()
@@ -3873,16 +3889,17 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
        /// Only fails in case of bad keys. Used for channel_reestablish commitment_signed generation
        /// when we shouldn't change HTLC/channel state.
-       fn send_commitment_no_state_update<L: Deref>(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Transaction, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger {
+       fn send_commitment_no_state_update<L: Deref>(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger {
                let mut feerate_per_kw = self.feerate_per_kw;
                if let Some(feerate) = self.pending_update_fee {
-                       if self.channel_outbound {
+                       if self.is_outbound() {
                                feerate_per_kw = feerate;
                        }
                }
 
                let counterparty_keys = self.build_remote_transaction_keys()?;
                let counterparty_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, feerate_per_kw, logger);
+               let counterparty_commitment_txid = counterparty_commitment_tx.0.trust().txid();
                let (signature, htlc_signatures);
 
                {
@@ -3891,22 +3908,20 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                htlcs.push(htlc);
                        }
 
-                       let pre_remote_keys = PreCalculatedTxCreationKeys::new(counterparty_keys);
-                       let res = self.holder_keys.sign_counterparty_commitment(feerate_per_kw, &counterparty_commitment_tx.0, &pre_remote_keys, &htlcs, &self.secp_ctx)
+                       let res = self.holder_keys.sign_counterparty_commitment(&counterparty_commitment_tx.0, &self.secp_ctx)
                                .map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?;
                        signature = res.0;
                        htlc_signatures = res.1;
-                       let counterparty_keys = pre_remote_keys.trust_key_derivation();
 
                        log_trace!(logger, "Signed remote commitment tx {} with redeemscript {} -> {}",
-                               encode::serialize_hex(&counterparty_commitment_tx.0),
+                               &counterparty_commitment_txid,
                                encode::serialize_hex(&self.get_funding_redeemscript()),
                                log_bytes!(signature.serialize_compact()[..]));
 
                        for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) {
                                log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {}",
-                                       encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_tx.0.txid(), feerate_per_kw, self.holder_selected_contest_delay, htlc, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)),
-                                       encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, counterparty_keys)),
+                                       encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, feerate_per_kw, self.get_holder_selected_contest_delay(), htlc, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)),
+                                       encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, &counterparty_keys)),
                                        log_bytes!(counterparty_keys.broadcaster_htlc_key.serialize()),
                                        log_bytes!(htlc_sig.serialize_compact()[..]));
                        }
@@ -3916,7 +3931,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                        channel_id: self.channel_id,
                        signature,
                        htlc_signatures,
-               }, (counterparty_commitment_tx.0, counterparty_commitment_tx.2)))
+               }, (counterparty_commitment_txid, counterparty_commitment_tx.2)))
        }
 
        /// Adds a pending outbound HTLC to this channel, and creates a signed commitment transaction
@@ -4003,7 +4018,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
                                _ => {}
                        }
                }
-               let funding_txo = if let Some(funding_txo) = self.funding_txo {
+               let funding_txo = if let Some(funding_txo) = self.get_funding_txo() {
                        // If we haven't yet exchanged funding signatures (ie channel_state < FundingSent),
                        // returning a channel monitor update here would imply a channel monitor update before
                        // we even registered the channel monitor to begin with, which is invalid.
@@ -4074,7 +4089,6 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
 
                self.channel_id.write(writer)?;
                (self.channel_state | ChannelState::PeerDisconnected as u32).write(writer)?;
-               self.channel_outbound.write(writer)?;
                self.channel_value_satoshis.write(writer)?;
 
                self.latest_monitor_update_id.write(writer)?;
@@ -4216,7 +4230,6 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                        None => 0u8.write(writer)?,
                }
 
-               self.funding_txo.write(writer)?;
                self.funding_tx_confirmed_in.write(writer)?;
                self.short_channel_id.write(writer)?;
 
@@ -4229,12 +4242,10 @@ impl<ChanSigner: ChannelKeys + Writeable> Writeable for Channel<ChanSigner> {
                self.counterparty_selected_channel_reserve_satoshis.write(writer)?;
                self.counterparty_htlc_minimum_msat.write(writer)?;
                self.holder_htlc_minimum_msat.write(writer)?;
-               self.counterparty_selected_contest_delay.write(writer)?;
-               self.holder_selected_contest_delay.write(writer)?;
                self.counterparty_max_accepted_htlcs.write(writer)?;
                self.minimum_depth.write(writer)?;
 
-               self.counterparty_pubkeys.write(writer)?;
+               self.channel_transaction_parameters.write(writer)?;
                self.counterparty_cur_commitment_point.write(writer)?;
 
                self.counterparty_prev_commitment_point.write(writer)?;
@@ -4260,7 +4271,6 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
 
                let channel_id = Readable::read(reader)?;
                let channel_state = Readable::read(reader)?;
-               let channel_outbound = Readable::read(reader)?;
                let channel_value_satoshis = Readable::read(reader)?;
 
                let latest_monitor_update_id = Readable::read(reader)?;
@@ -4370,7 +4380,6 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
                        _ => return Err(DecodeError::InvalidValue),
                };
 
-               let funding_txo = Readable::read(reader)?;
                let funding_tx_confirmed_in = Readable::read(reader)?;
                let short_channel_id = Readable::read(reader)?;
 
@@ -4383,12 +4392,10 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
                let counterparty_selected_channel_reserve_satoshis = Readable::read(reader)?;
                let counterparty_htlc_minimum_msat = Readable::read(reader)?;
                let holder_htlc_minimum_msat = Readable::read(reader)?;
-               let counterparty_selected_contest_delay = Readable::read(reader)?;
-               let holder_selected_contest_delay = Readable::read(reader)?;
                let counterparty_max_accepted_htlcs = Readable::read(reader)?;
                let minimum_depth = Readable::read(reader)?;
 
-               let counterparty_pubkeys = Readable::read(reader)?;
+               let channel_parameters = Readable::read(reader)?;
                let counterparty_cur_commitment_point = Readable::read(reader)?;
 
                let counterparty_prev_commitment_point = Readable::read(reader)?;
@@ -4403,7 +4410,6 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
                        config,
                        channel_id,
                        channel_state,
-                       channel_outbound,
                        secp_ctx: Secp256k1::new(),
                        channel_value_satoshis,
 
@@ -4443,7 +4449,6 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
 
                        last_sent_closing_fee,
 
-                       funding_txo,
                        funding_tx_confirmed_in,
                        short_channel_id,
                        last_block_connected,
@@ -4455,12 +4460,10 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for Channel<ChanSigner> {
                        counterparty_selected_channel_reserve_satoshis,
                        counterparty_htlc_minimum_msat,
                        holder_htlc_minimum_msat,
-                       counterparty_selected_contest_delay,
-                       holder_selected_contest_delay,
                        counterparty_max_accepted_htlcs,
                        minimum_depth,
 
-                       counterparty_pubkeys,
+                       channel_transaction_parameters: channel_parameters,
                        counterparty_cur_commitment_point,
 
                        counterparty_prev_commitment_point,
@@ -4492,7 +4495,7 @@ mod tests {
        use ln::features::InitFeatures;
        use ln::msgs::{OptionalField, DataLossProtect};
        use ln::chan_utils;
-       use ln::chan_utils::{HolderCommitmentTransaction, ChannelPublicKeys};
+       use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
        use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
        use chain::keysinterface::{InMemoryChannelKeys, KeysInterface};
        use chain::transaction::OutPoint;
@@ -4667,11 +4670,9 @@ mod tests {
                let mut config = UserConfig::default();
                config.channel_options.announced_channel = false;
                let mut chan = Channel::<InMemoryChannelKeys>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test
-               chan.counterparty_selected_contest_delay = 144;
                chan.holder_dust_limit_satoshis = 546;
 
                let funding_info = OutPoint{ txid: Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), index: 0 };
-               chan.funding_txo = Some(funding_info);
 
                let counterparty_pubkeys = ChannelPublicKeys {
                        funding_pubkey: public_from_secret_hex(&secp_ctx, "1552dfba4f6cf29a62a0af13c8d6981d36d0ef8d61ba10fb0fe90da7634d7e13"),
@@ -4680,7 +4681,13 @@ 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.channel_transaction_parameters.counterparty_parameters = Some(
+                       CounterpartyChannelTransactionParameters {
+                               pubkeys: counterparty_pubkeys.clone(),
+                               selected_contest_delay: 144
+                       });
+               chan.channel_transaction_parameters.funding_outpoint = Some(funding_info);
+               chan_keys.ready_channel(&chan.channel_transaction_parameters);
 
                assert_eq!(counterparty_pubkeys.payment_point.serialize()[..],
                           hex::decode("032c0b7cf95324a07d05398b240174dc0c2be444d96b159aa6c7f7b1e668680991").unwrap()[..]);
@@ -4700,50 +4707,60 @@ mod tests {
                let htlc_basepoint = &chan.holder_keys.pubkeys().htlc_basepoint;
                let keys = TxCreationKeys::derive_new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint).unwrap();
 
-               chan.counterparty_pubkeys = Some(counterparty_pubkeys);
-
-               let mut unsigned_tx: (Transaction, Vec<HTLCOutputInCommitment>);
-
-               let mut holdertx;
                macro_rules! test_commitment {
                        ( $counterparty_sig_hex: expr, $sig_hex: expr, $tx_hex: expr, {
                                $( { $htlc_idx: expr, $counterparty_htlc_sig_hex: expr, $htlc_sig_hex: expr, $htlc_tx_hex: expr } ), *
                        } ) => { {
-                               unsigned_tx = {
+                               let (commitment_tx, htlcs): (_, Vec<HTLCOutputInCommitment>) = {
                                        let mut res = chan.build_commitment_transaction(0xffffffffffff - 42, &keys, true, false, chan.feerate_per_kw, &logger);
+
                                        let htlcs = res.2.drain(..)
                                                .filter_map(|(htlc, _)| if htlc.transaction_output_index.is_some() { Some(htlc) } else { None })
                                                .collect();
                                        (res.0, htlcs)
                                };
+                               let trusted_tx = commitment_tx.trust();
+                               let unsigned_tx = trusted_tx.built_transaction();
                                let redeemscript = chan.get_funding_redeemscript();
                                let counterparty_signature = Signature::from_der(&hex::decode($counterparty_sig_hex).unwrap()[..]).unwrap();
-                               let sighash = Message::from_slice(&bip143::SigHashCache::new(&unsigned_tx.0).signature_hash(0, &redeemscript, chan.channel_value_satoshis, SigHashType::All)[..]).unwrap();
+                               let sighash = unsigned_tx.get_sighash_all(&redeemscript, chan.channel_value_satoshis);
                                secp_ctx.verify(&sighash, &counterparty_signature, chan.counterparty_funding_pubkey()).unwrap();
 
-                               let mut per_htlc = Vec::new();
+                               let mut per_htlc: Vec<(HTLCOutputInCommitment, Option<Signature>)> = Vec::new();
                                per_htlc.clear(); // Don't warn about excess mut for no-HTLC calls
+                               let mut counterparty_htlc_sigs = Vec::new();
+                               counterparty_htlc_sigs.clear(); // Don't warn about excess mut for no-HTLC calls
                                $({
                                        let remote_signature = Signature::from_der(&hex::decode($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
-                                       per_htlc.push((unsigned_tx.1[$htlc_idx].clone(), Some(remote_signature)));
+                                       per_htlc.push((htlcs[$htlc_idx].clone(), Some(remote_signature)));
+                                       counterparty_htlc_sigs.push(remote_signature);
                                })*
-                               assert_eq!(unsigned_tx.1.len(), per_htlc.len());
+                               assert_eq!(htlcs.len(), per_htlc.len());
 
-                               holdertx = HolderCommitmentTransaction::new_missing_holder_sig(unsigned_tx.0.clone(), counterparty_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.counterparty_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc);
-                               let holder_sig = chan_keys.sign_holder_commitment(&holdertx, &chan.secp_ctx).unwrap();
-                               assert_eq!(Signature::from_der(&hex::decode($sig_hex).unwrap()[..]).unwrap(), holder_sig);
+                               let holder_commitment_tx = HolderCommitmentTransaction::new(
+                                       commitment_tx.clone(),
+                                       counterparty_signature,
+                                       counterparty_htlc_sigs,
+                                       &chan.holder_keys.pubkeys().funding_pubkey,
+                                       chan.counterparty_funding_pubkey()
+                               );
+                               let holder_sig = chan_keys.sign_holder_commitment(&holder_commitment_tx, &secp_ctx).unwrap();
+                               assert_eq!(Signature::from_der(&hex::decode($sig_hex).unwrap()[..]).unwrap(), holder_sig, "holder_sig");
 
-                               assert_eq!(serialize(&holdertx.add_holder_sig(&redeemscript, holder_sig))[..],
-                                               hex::decode($tx_hex).unwrap()[..]);
+                               let funding_redeemscript = chan.get_funding_redeemscript();
+                               let tx = holder_commitment_tx.add_holder_sig(&funding_redeemscript, holder_sig);
+                               assert_eq!(serialize(&tx)[..], hex::decode($tx_hex).unwrap()[..], "tx");
 
-                               let htlc_sigs = chan_keys.sign_holder_commitment_htlc_transactions(&holdertx, &chan.secp_ctx).unwrap();
-                               let mut htlc_sig_iter = holdertx.per_htlc.iter().zip(htlc_sigs.iter().enumerate());
+                               let htlc_sigs = chan_keys.sign_holder_commitment_htlc_transactions(&holder_commitment_tx, &secp_ctx).unwrap();
+
+                               // ((htlc, counterparty_sig), (index, holder_sig))
+                               let mut htlc_sig_iter = holder_commitment_tx.htlcs().iter().zip(&holder_commitment_tx.counterparty_htlc_sigs).zip(htlc_sigs.iter().enumerate());
 
                                $({
                                        let remote_signature = Signature::from_der(&hex::decode($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
 
-                                       let ref htlc = unsigned_tx.1[$htlc_idx];
-                                       let htlc_tx = chan.build_htlc_transaction(&unsigned_tx.0.txid(), &htlc, true, &keys, chan.feerate_per_kw);
+                                       let ref htlc = htlcs[$htlc_idx];
+                                       let htlc_tx = chan.build_htlc_transaction(&unsigned_tx.txid, &htlc, true, &keys, chan.feerate_per_kw);
                                        let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys);
                                        let htlc_sighash = Message::from_slice(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, htlc.amount_msat / 1000, SigHashType::All)[..]).unwrap();
                                        secp_ctx.verify(&htlc_sighash, &remote_signature, &keys.countersignatory_htlc_key).unwrap();
@@ -4760,20 +4777,18 @@ mod tests {
                                                assert!(preimage.is_some());
                                        }
 
-                                       let mut htlc_sig = htlc_sig_iter.next().unwrap();
-                                       while (htlc_sig.1).1.is_none() { htlc_sig = htlc_sig_iter.next().unwrap(); }
-                                       assert_eq!((htlc_sig.0).0.transaction_output_index, Some($htlc_idx));
+                                       let htlc_sig = htlc_sig_iter.next().unwrap();
+                                       assert_eq!((htlc_sig.0).0.transaction_output_index, Some($htlc_idx), "output index");
 
                                        let signature = Signature::from_der(&hex::decode($htlc_sig_hex).unwrap()[..]).unwrap();
-                                       assert_eq!(Some(signature), *(htlc_sig.1).1);
-                                       assert_eq!(serialize(&holdertx.get_signed_htlc_tx((htlc_sig.1).0, &(htlc_sig.1).1.unwrap(), &preimage, chan.counterparty_selected_contest_delay))[..],
-                                                       hex::decode($htlc_tx_hex).unwrap()[..]);
+                                       assert_eq!(signature, *(htlc_sig.1).1, "htlc sig");
+                                       let index = (htlc_sig.1).0;
+                                       let channel_parameters = chan.channel_transaction_parameters.as_holder_broadcastable();
+                                       let trusted_tx = holder_commitment_tx.trust();
+                                       assert_eq!(serialize(&trusted_tx.get_signed_htlc_tx(&channel_parameters, index, &(htlc_sig.0).1, (htlc_sig.1).1, &preimage))[..],
+                                                       hex::decode($htlc_tx_hex).unwrap()[..], "htlc tx");
                                })*
-                               loop {
-                                       let htlc_sig = htlc_sig_iter.next();
-                                       if htlc_sig.is_none() { break; }
-                                       assert!((htlc_sig.unwrap().1).1.is_none());
-                               }
+                               assert!(htlc_sig_iter.next().is_none());
                        } }
                }
 
index 85dae76987dc669424486e2a5d0f03387d469dcf..36709062a6d3b8e4d06a7542dee627e920b621cd 100644 (file)
@@ -32,13 +32,12 @@ use util::ser::{Writeable, ReadableArgs, Readable};
 use util::config::UserConfig;
 
 use bitcoin::hashes::sha256d::Hash as Sha256dHash;
-use bitcoin::hashes::HashEngine;
-use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
+use bitcoin::hash_types::{Txid, BlockHash};
 use bitcoin::util::bip143;
 use bitcoin::util::address::Address;
 use bitcoin::util::bip32::{ChildNumber, ExtendedPubKey, ExtendedPrivKey};
 use bitcoin::blockdata::block::{Block, BlockHeader};
-use bitcoin::blockdata::transaction::{Transaction, TxOut, TxIn, SigHashType, OutPoint as BitcoinOutPoint};
+use bitcoin::blockdata::transaction::{Transaction, TxOut, TxIn, SigHashType};
 use bitcoin::blockdata::script::{Builder, Script};
 use bitcoin::blockdata::opcodes;
 use bitcoin::blockdata::constants::genesis_block;
@@ -59,7 +58,7 @@ use std::sync::atomic::Ordering;
 use std::mem;
 
 use ln::functional_test_utils::*;
-use ln::chan_utils::PreCalculatedTxCreationKeys;
+use ln::chan_utils::CommitmentTransaction;
 
 #[test]
 fn test_insane_channel_opens() {
@@ -1617,20 +1616,20 @@ fn test_fee_spike_violation_fails_htlc() {
 
        // Get the EnforcingChannelKeys for each channel, which will be used to (1) get the keys
        // needed to sign the new commitment tx and (2) sign the new commitment tx.
-       let (local_revocation_basepoint, local_htlc_basepoint, local_payment_point, local_secret, local_secret2) = {
+       let (local_revocation_basepoint, local_htlc_basepoint, local_secret, local_secret2) = {
                let chan_lock = nodes[0].node.channel_state.lock().unwrap();
                let local_chan = chan_lock.by_id.get(&chan.2).unwrap();
                let chan_keys = local_chan.get_keys();
                let pubkeys = chan_keys.pubkeys();
-               (pubkeys.revocation_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
+               (pubkeys.revocation_basepoint, pubkeys.htlc_basepoint,
                 chan_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER), chan_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2))
        };
-       let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_payment_point, remote_secret1) = {
+       let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_secret1) = {
                let chan_lock = nodes[1].node.channel_state.lock().unwrap();
                let remote_chan = chan_lock.by_id.get(&chan.2).unwrap();
                let chan_keys = remote_chan.get_keys();
                let pubkeys = chan_keys.pubkeys();
-               (pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint, pubkeys.payment_point,
+               (pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint,
                 chan_keys.release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1))
        };
 
@@ -1643,70 +1642,31 @@ fn test_fee_spike_violation_fails_htlc() {
        // Build the remote commitment transaction so we can sign it, and then later use the
        // signature for the commitment_signed message.
        let local_chan_balance = 1313;
-       let static_payment_pk = local_payment_point.serialize();
-       let remote_commit_tx_output = TxOut {
-                               script_pubkey: Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0)
-                                                            .push_slice(&WPubkeyHash::hash(&static_payment_pk)[..])
-                                                            .into_script(),
-               value: local_chan_balance as u64
-       };
-
-       let local_commit_tx_output = TxOut {
-               script_pubkey: chan_utils::get_revokeable_redeemscript(&commit_tx_keys.revocation_key,
-                                                                              BREAKDOWN_TIMEOUT,
-                                                                              &commit_tx_keys.broadcaster_delayed_payment_key).to_v0_p2wsh(),
-                               value: 95000,
-       };
 
        let accepted_htlc_info = chan_utils::HTLCOutputInCommitment {
                offered: false,
                amount_msat: 3460001,
                cltv_expiry: htlc_cltv,
-               payment_hash: payment_hash,
+               payment_hash,
                transaction_output_index: Some(1),
        };
 
-       let htlc_output = TxOut {
-               script_pubkey: chan_utils::get_htlc_redeemscript(&accepted_htlc_info, &commit_tx_keys).to_v0_p2wsh(),
-               value: 3460001 / 1000
-       };
-
-       let commit_tx_obscure_factor = {
-               let mut sha = Sha256::engine();
-               let remote_payment_point = &remote_payment_point.serialize();
-               sha.input(&local_payment_point.serialize());
-               sha.input(remote_payment_point);
-               let res = Sha256::from_engine(sha).into_inner();
-
-               ((res[26] as u64) << 5*8) |
-               ((res[27] as u64) << 4*8) |
-               ((res[28] as u64) << 3*8) |
-               ((res[29] as u64) << 2*8) |
-               ((res[30] as u64) << 1*8) |
-               ((res[31] as u64) << 0*8)
-       };
-       let commitment_number = 1;
-       let obscured_commitment_transaction_number = commit_tx_obscure_factor ^ commitment_number;
-       let lock_time = ((0x20 as u32) << 8*3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32);
-       let input = TxIn {
-               previous_output: BitcoinOutPoint { txid: chan.3.txid(), vout: 0 },
-               script_sig: Script::new(),
-               sequence: ((0x80 as u32) << 8*3) | ((obscured_commitment_transaction_number >> 3*8) as u32),
-               witness: Vec::new(),
-       };
+       let commitment_number = INITIAL_COMMITMENT_NUMBER - 1;
 
-       let commit_tx = Transaction {
-               version: 2,
-               lock_time,
-               input: vec![input],
-               output: vec![remote_commit_tx_output, htlc_output, local_commit_tx_output],
-       };
        let res = {
                let local_chan_lock = nodes[0].node.channel_state.lock().unwrap();
                let local_chan = local_chan_lock.by_id.get(&chan.2).unwrap();
                let local_chan_keys = local_chan.get_keys();
-               let pre_commit_tx_keys = PreCalculatedTxCreationKeys::new(commit_tx_keys);
-               local_chan_keys.sign_counterparty_commitment(feerate_per_kw, &commit_tx, &pre_commit_tx_keys, &[&accepted_htlc_info], &secp_ctx).unwrap()
+               let commitment_tx = CommitmentTransaction::new_with_auxiliary_htlc_data(
+                       commitment_number,
+                       95000,
+                       local_chan_balance,
+                       commit_tx_keys.clone(),
+                       feerate_per_kw,
+                       &mut vec![(accepted_htlc_info, ())],
+                       &local_chan.channel_transaction_parameters.as_counterparty_broadcastable()
+               );
+               local_chan_keys.sign_counterparty_commitment(&commitment_tx, &secp_ctx).unwrap()
        };
 
        let commit_signed_msg = msgs::CommitmentSigned {
index 3484d8983f60fc0e9367993b5b5de3b4979dc8ff..146a4d57c8b27d68bef808a45e3ec6789c464a9b 100644 (file)
@@ -9,7 +9,7 @@
 
 //! The logic to build claims and bump in-flight transactions until confirmations.
 //!
-//! OnchainTxHandler objetcs are fully-part of ChannelMonitor and encapsulates all
+//! OnchainTxHandler objects are fully-part of ChannelMonitor and encapsulates all
 //! building, tracking, bumping and notifications functions.
 
 use bitcoin::blockdata::transaction::{Transaction, TxIn, TxOut, SigHashType};
@@ -24,7 +24,7 @@ use bitcoin::secp256k1;
 use ln::msgs::DecodeError;
 use ln::channelmanager::PaymentPreimage;
 use ln::chan_utils;
-use ln::chan_utils::{TxCreationKeys, HolderCommitmentTransaction};
+use ln::chan_utils::{TxCreationKeys, ChannelTransactionParameters, HolderCommitmentTransaction};
 use chain::chaininterface::{FeeEstimator, BroadcasterInterface, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
 use chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER, InputMaterial, ClaimRequest};
 use chain::keysinterface::ChannelKeys;
@@ -244,14 +244,13 @@ pub struct OnchainTxHandler<ChanSigner: ChannelKeys> {
        holder_commitment: Option<HolderCommitmentTransaction>,
        // holder_htlc_sigs and prev_holder_htlc_sigs are in the order as they appear in the commitment
        // transaction outputs (hence the Option<>s inside the Vec). The first usize is the index in
-       // the set of HTLCs in the HolderCommitmentTransaction (including those which do not appear in
-       // the commitment transaction).
+       // the set of HTLCs in the HolderCommitmentTransaction.
        holder_htlc_sigs: Option<Vec<Option<(usize, Signature)>>>,
        prev_holder_commitment: Option<HolderCommitmentTransaction>,
        prev_holder_htlc_sigs: Option<Vec<Option<(usize, Signature)>>>,
-       on_holder_tx_csv: u16,
 
        key_storage: ChanSigner,
+       pub(crate) channel_transaction_parameters: ChannelTransactionParameters,
 
        // Used to track claiming requests. If claim tx doesn't confirm before height timer expiration we need to bump
        // it (RBF or CPFP). If an input has been part of an aggregate tx at first claim try, we need to keep it within
@@ -295,9 +294,8 @@ impl<ChanSigner: ChannelKeys + Writeable> OnchainTxHandler<ChanSigner> {
                self.prev_holder_commitment.write(writer)?;
                self.prev_holder_htlc_sigs.write(writer)?;
 
-               self.on_holder_tx_csv.write(writer)?;
-
                self.key_storage.write(writer)?;
+               self.channel_transaction_parameters.write(writer)?;
 
                writer.write_all(&byte_utils::be64_to_array(self.pending_claim_requests.len() as u64))?;
                for (ref ancestor_claim_txid, claim_tx_data) in self.pending_claim_requests.iter() {
@@ -344,9 +342,8 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for OnchainTxHandler<ChanSigne
                let prev_holder_commitment = Readable::read(reader)?;
                let prev_holder_htlc_sigs = Readable::read(reader)?;
 
-               let on_holder_tx_csv = Readable::read(reader)?;
-
                let key_storage = Readable::read(reader)?;
+               let channel_parameters = Readable::read(reader)?;
 
                let pending_claim_requests_len: u64 = Readable::read(reader)?;
                let mut pending_claim_requests = HashMap::with_capacity(cmp::min(pending_claim_requests_len as usize, MAX_ALLOC_SIZE / 128));
@@ -398,8 +395,8 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for OnchainTxHandler<ChanSigne
                        holder_htlc_sigs,
                        prev_holder_commitment,
                        prev_holder_htlc_sigs,
-                       on_holder_tx_csv,
                        key_storage,
+                       channel_transaction_parameters: channel_parameters,
                        claimable_outpoints,
                        pending_claim_requests,
                        onchain_events_waiting_threshold_conf,
@@ -410,7 +407,7 @@ impl<ChanSigner: ChannelKeys + Readable> Readable for OnchainTxHandler<ChanSigne
 }
 
 impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
-       pub(crate) fn new(destination_script: Script, keys: ChanSigner, on_holder_tx_csv: u16) -> Self {
+       pub(crate) fn new(destination_script: Script, keys: ChanSigner, channel_parameters: ChannelTransactionParameters) -> Self {
 
                let key_storage = keys;
 
@@ -420,8 +417,8 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
                        holder_htlc_sigs: None,
                        prev_holder_commitment: None,
                        prev_holder_htlc_sigs: None,
-                       on_holder_tx_csv,
                        key_storage,
+                       channel_transaction_parameters: channel_parameters,
                        pending_claim_requests: HashMap::new(),
                        claimable_outpoints: HashMap::new(),
                        onchain_events_waiting_threshold_conf: HashMap::new(),
@@ -654,7 +651,7 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
                                                let signed_tx = self.get_fully_signed_holder_tx(funding_redeemscript).unwrap();
                                                // Timer set to $NEVER given we can't bump tx without anchor outputs
                                                log_trace!(logger, "Going to broadcast Holder Transaction {} claiming funding output {} from {}...", signed_tx.txid(), outp.vout, outp.txid);
-                                               return Some((None, self.holder_commitment.as_ref().unwrap().feerate_per_kw, signed_tx));
+                                               return Some((None, self.holder_commitment.as_ref().unwrap().feerate_per_kw(), signed_tx));
                                        }
                                        _ => unreachable!()
                                }
@@ -899,44 +896,39 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
        fn sign_latest_holder_htlcs(&mut self) {
                if let Some(ref holder_commitment) = self.holder_commitment {
                        if let Ok(sigs) = self.key_storage.sign_holder_commitment_htlc_transactions(holder_commitment, &self.secp_ctx) {
-                               self.holder_htlc_sigs = Some(Vec::new());
-                               let ret = self.holder_htlc_sigs.as_mut().unwrap();
-                               for (htlc_idx, (holder_sig, &(ref htlc, _))) in sigs.iter().zip(holder_commitment.per_htlc.iter()).enumerate() {
-                                       if let Some(tx_idx) = htlc.transaction_output_index {
-                                               if ret.len() <= tx_idx as usize { ret.resize(tx_idx as usize + 1, None); }
-                                               ret[tx_idx as usize] = Some((htlc_idx, holder_sig.expect("Did not receive a signature for a non-dust HTLC")));
-                                       } else {
-                                               assert!(holder_sig.is_none(), "Received a signature for a dust HTLC");
-                                       }
-                               }
+                               self.holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs));
                        }
                }
        }
+
        fn sign_prev_holder_htlcs(&mut self) {
                if let Some(ref holder_commitment) = self.prev_holder_commitment {
                        if let Ok(sigs) = self.key_storage.sign_holder_commitment_htlc_transactions(holder_commitment, &self.secp_ctx) {
-                               self.prev_holder_htlc_sigs = Some(Vec::new());
-                               let ret = self.prev_holder_htlc_sigs.as_mut().unwrap();
-                               for (htlc_idx, (holder_sig, &(ref htlc, _))) in sigs.iter().zip(holder_commitment.per_htlc.iter()).enumerate() {
-                                       if let Some(tx_idx) = htlc.transaction_output_index {
-                                               if ret.len() <= tx_idx as usize { ret.resize(tx_idx as usize + 1, None); }
-                                               ret[tx_idx as usize] = Some((htlc_idx, holder_sig.expect("Did not receive a signature for a non-dust HTLC")));
-                                       } else {
-                                               assert!(holder_sig.is_none(), "Received a signature for a dust HTLC");
-                                       }
-                               }
+                               self.prev_holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs));
                        }
                }
        }
 
-       //TODO: getting lastest holder transactions should be infaillible and result in us "force-closing the channel", but we may
+       fn extract_holder_sigs(holder_commitment: &HolderCommitmentTransaction, sigs: Vec<Signature>) -> Vec<Option<(usize, Signature)>> {
+               let mut ret = Vec::new();
+               for (htlc_idx, (holder_sig, htlc)) in sigs.iter().zip(holder_commitment.htlcs().iter()).enumerate() {
+                       let tx_idx = htlc.transaction_output_index.unwrap();
+                       if ret.len() <= tx_idx as usize { ret.resize(tx_idx as usize + 1, None); }
+                       ret[tx_idx as usize] = Some((htlc_idx, holder_sig.clone()));
+               }
+               ret
+       }
+
+       //TODO: getting lastest holder transactions should be infallible and result in us "force-closing the channel", but we may
        // have empty holder commitment transaction if a ChannelMonitor is asked to force-close just after Channel::get_outbound_funding_created,
        // before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing
        // to monitor before.
        pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Option<Transaction> {
                if let Some(ref mut holder_commitment) = self.holder_commitment {
-                       match self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx) {
-                               Ok(sig) => Some(holder_commitment.add_holder_sig(funding_redeemscript, sig)),
+                       match self.key_storage.sign_holder_commitment(&holder_commitment, &self.secp_ctx) {
+                               Ok(sig) => {
+                                       Some(holder_commitment.add_holder_sig(funding_redeemscript, sig))
+                               },
                                Err(_) => return None,
                        }
                } else {
@@ -947,9 +939,10 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
        #[cfg(any(test, feature="unsafe_revoked_tx_signing"))]
        pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Option<Transaction> {
                if let Some(ref mut holder_commitment) = self.holder_commitment {
-                       let holder_commitment = holder_commitment.clone();
-                       match self.key_storage.sign_holder_commitment(&holder_commitment, &self.secp_ctx) {
-                               Ok(sig) => Some(holder_commitment.add_holder_sig(funding_redeemscript, sig)),
+                       match self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx) {
+                               Ok(sig) => {
+                                       Some(holder_commitment.add_holder_sig(funding_redeemscript, sig))
+                               },
                                Err(_) => return None,
                        }
                } else {
@@ -960,24 +953,30 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
        pub(crate) fn get_fully_signed_htlc_tx(&mut self, outp: &::bitcoin::OutPoint, preimage: &Option<PaymentPreimage>) -> Option<Transaction> {
                let mut htlc_tx = None;
                if self.holder_commitment.is_some() {
-                       let commitment_txid = self.holder_commitment.as_ref().unwrap().txid();
+                       let commitment_txid = self.holder_commitment.as_ref().unwrap().trust().txid();
                        if commitment_txid == outp.txid {
                                self.sign_latest_holder_htlcs();
                                if let &Some(ref htlc_sigs) = &self.holder_htlc_sigs {
                                        let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap();
-                                       htlc_tx = Some(self.holder_commitment.as_ref().unwrap()
-                                               .get_signed_htlc_tx(*htlc_idx, htlc_sig, preimage, self.on_holder_tx_csv));
+                                       let holder_commitment = self.holder_commitment.as_ref().unwrap();
+                                       let trusted_tx = holder_commitment.trust();
+                                       let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[*htlc_idx];
+                                       htlc_tx = Some(trusted_tx
+                                               .get_signed_htlc_tx(&self.channel_transaction_parameters.as_holder_broadcastable(), *htlc_idx, &counterparty_htlc_sig, htlc_sig, preimage));
                                }
                        }
                }
                if self.prev_holder_commitment.is_some() {
-                       let commitment_txid = self.prev_holder_commitment.as_ref().unwrap().txid();
+                       let commitment_txid = self.prev_holder_commitment.as_ref().unwrap().trust().txid();
                        if commitment_txid == outp.txid {
                                self.sign_prev_holder_htlcs();
                                if let &Some(ref htlc_sigs) = &self.prev_holder_htlc_sigs {
                                        let &(ref htlc_idx, ref htlc_sig) = htlc_sigs[outp.vout as usize].as_ref().unwrap();
-                                       htlc_tx = Some(self.prev_holder_commitment.as_ref().unwrap()
-                                               .get_signed_htlc_tx(*htlc_idx, htlc_sig, preimage, self.on_holder_tx_csv));
+                                       let holder_commitment = self.prev_holder_commitment.as_ref().unwrap();
+                                       let trusted_tx = holder_commitment.trust();
+                                       let counterparty_htlc_sig = holder_commitment.counterparty_htlc_sigs[*htlc_idx];
+                                       htlc_tx = Some(trusted_tx
+                                               .get_signed_htlc_tx(&self.channel_transaction_parameters.as_holder_broadcastable(), *htlc_idx, &counterparty_htlc_sig, htlc_sig, preimage));
                                }
                        }
                }
index 5d0a196f31fb27d956ae4c314dc302e243a0ae33..92cf178c8c5d912a61e21a6e0d24f778ece99e7c 100644 (file)
@@ -7,7 +7,7 @@
 // You may not use this file except in accordance with one or both of these
 // licenses.
 
-use ln::chan_utils::{HTLCOutputInCommitment, TxCreationKeys, ChannelPublicKeys, HolderCommitmentTransaction, PreCalculatedTxCreationKeys};
+use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, HolderCommitmentTransaction, CommitmentTransaction, ChannelTransactionParameters, TrustedCommitmentTransaction};
 use ln::{chan_utils, msgs};
 use chain::keysinterface::{ChannelKeys, InMemoryChannelKeys};
 
@@ -24,38 +24,25 @@ use util::ser::{Writeable, Writer, Readable};
 use std::io::Error;
 use ln::msgs::DecodeError;
 
-/// Enforces some rules on ChannelKeys calls. Eventually we will probably want to expose a variant
-/// of this which would essentially be what you'd want to run on a hardware wallet.
+/// An implementation of ChannelKeys that enforces some policy checks.
+///
+/// Eventually we will probably want to expose a variant of this which would essentially
+/// be what you'd want to run on a hardware wallet.
 #[derive(Clone)]
 pub struct EnforcingChannelKeys {
        pub inner: InMemoryChannelKeys,
-       commitment_number_obscure_and_last: Arc<Mutex<(Option<u64>, u64)>>,
+       last_commitment_number: Arc<Mutex<Option<u64>>>,
 }
 
 impl EnforcingChannelKeys {
        pub fn new(inner: InMemoryChannelKeys) -> Self {
                Self {
                        inner,
-                       commitment_number_obscure_and_last: Arc::new(Mutex::new((None, 0))),
+                       last_commitment_number: Arc::new(Mutex::new(None)),
                }
        }
 }
 
-impl EnforcingChannelKeys {
-       fn check_keys<T: secp256k1::Signing + secp256k1::Verification>(&self, secp_ctx: &Secp256k1<T>,
-                                                                      keys: &TxCreationKeys) {
-               let remote_points = self.inner.counterparty_pubkeys();
-
-               let keys_expected = TxCreationKeys::derive_new(secp_ctx,
-                                                              &keys.per_commitment_point,
-                                                              &remote_points.delayed_payment_basepoint,
-                                                              &remote_points.htlc_basepoint,
-                                                              &self.inner.pubkeys().revocation_basepoint,
-                                                              &self.inner.pubkeys().htlc_basepoint).unwrap();
-               if keys != &keys_expected { panic!("derived different per-tx keys") }
-       }
-}
-
 impl ChannelKeys for EnforcingChannelKeys {
        fn get_per_commitment_point<T: secp256k1::Signing + secp256k1::Verification>(&self, idx: u64, secp_ctx: &Secp256k1<T>) -> PublicKey {
                self.inner.get_per_commitment_point(idx, secp_ctx)
@@ -69,51 +56,52 @@ impl ChannelKeys for EnforcingChannelKeys {
        fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() }
        fn key_derivation_params(&self) -> (u64, u64) { self.inner.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 { panic!("lightning commitment transactions have a single input"); }
-               self.check_keys(secp_ctx, pre_keys.trust_key_derivation());
-               let obscured_commitment_transaction_number = (commitment_tx.lock_time & 0xffffff) as u64 | ((commitment_tx.input[0].sequence as u64 & 0xffffff) << 3*8);
+       fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
+               self.verify_counterparty_commitment_tx(commitment_tx, secp_ctx);
 
                {
-                       let mut commitment_data = self.commitment_number_obscure_and_last.lock().unwrap();
-                       if commitment_data.0.is_none() {
-                               commitment_data.0 = Some(obscured_commitment_transaction_number ^ commitment_data.1);
-                       }
-                       let commitment_number = obscured_commitment_transaction_number ^ commitment_data.0.unwrap();
-                       assert!(commitment_number == commitment_data.1 || commitment_number == commitment_data.1 + 1);
-                       commitment_data.1 = cmp::max(commitment_number, commitment_data.1)
+                       let mut last_commitment_number_guard = self.last_commitment_number.lock().unwrap();
+                       let actual_commitment_number = commitment_tx.commitment_number();
+                       let last_commitment_number = last_commitment_number_guard.unwrap_or(actual_commitment_number);
+                       // These commitment numbers are backwards counting.  We expect either the same as the previously encountered,
+                       // or the next one.
+                       assert!(last_commitment_number == actual_commitment_number || last_commitment_number - 1 == actual_commitment_number, "{} doesn't come after {}", actual_commitment_number, last_commitment_number);
+                       *last_commitment_number_guard = Some(cmp::min(last_commitment_number, actual_commitment_number))
                }
 
-               Ok(self.inner.sign_counterparty_commitment(feerate_per_kw, commitment_tx, pre_keys, htlcs, secp_ctx).unwrap())
+               Ok(self.inner.sign_counterparty_commitment(commitment_tx, secp_ctx).unwrap())
        }
 
-       fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
+       fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
+               self.verify_holder_commitment_tx(commitment_tx, secp_ctx);
+
                // TODO: enforce the ChannelKeys contract - error if this commitment was already revoked
                // TODO: need the commitment number
-               Ok(self.inner.sign_holder_commitment(holder_commitment_tx, secp_ctx).unwrap())
+               Ok(self.inner.sign_holder_commitment(commitment_tx, secp_ctx).unwrap())
        }
 
        #[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, ()> {
-               Ok(self.inner.unsafe_sign_holder_commitment(holder_commitment_tx, secp_ctx).unwrap())
+       fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
+               Ok(self.inner.unsafe_sign_holder_commitment(commitment_tx, secp_ctx).unwrap())
        }
 
-       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 commitment_txid = holder_commitment_tx.txid();
+       fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()> {
+               let trusted_tx = self.verify_holder_commitment_tx(commitment_tx, secp_ctx);
+               let commitment_txid = trusted_tx.txid();
                let holder_csv = self.inner.counterparty_selected_contest_delay();
 
-               for this_htlc in holder_commitment_tx.per_htlc.iter() {
-                       if this_htlc.0.transaction_output_index.is_some() {
-                               let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, holder_commitment_tx.feerate_per_kw, holder_csv, &this_htlc.0, &holder_commitment_tx.keys.broadcaster_delayed_payment_key, &holder_commitment_tx.keys.revocation_key);
+               for (this_htlc, sig) in trusted_tx.htlcs().iter().zip(&commitment_tx.counterparty_htlc_sigs) {
+                       assert!(this_htlc.transaction_output_index.is_some());
+                       let keys = trusted_tx.keys();
+                       let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, trusted_tx.feerate_per_kw(), holder_csv, &this_htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
 
-                               let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&this_htlc.0, &holder_commitment_tx.keys);
+                       let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&this_htlc, &keys);
 
-                               let sighash = hash_to_message!(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, this_htlc.0.amount_msat / 1000, SigHashType::All)[..]);
-                               secp_ctx.verify(&sighash, this_htlc.1.as_ref().unwrap(), &holder_commitment_tx.keys.countersignatory_htlc_key).unwrap();
-                       }
+                       let sighash = hash_to_message!(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, this_htlc.amount_msat / 1000, SigHashType::All)[..]);
+                       secp_ctx.verify(&sighash, sig, &keys.countersignatory_htlc_key).unwrap();
                }
 
-               Ok(self.inner.sign_holder_commitment_htlc_transactions(holder_commitment_tx, secp_ctx).unwrap())
+               Ok(self.inner.sign_holder_commitment_htlc_transactions(commitment_tx, secp_ctx).unwrap())
        }
 
        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, ()> {
@@ -132,16 +120,16 @@ impl ChannelKeys for EnforcingChannelKeys {
                self.inner.sign_channel_announcement(msg, secp_ctx)
        }
 
-       fn on_accept(&mut self, channel_pubkeys: &ChannelPublicKeys, counterparty_selected_delay: u16, holder_selected_delay: u16) {
-               self.inner.on_accept(channel_pubkeys, counterparty_selected_delay, holder_selected_delay)
+       fn ready_channel(&mut self, channel_parameters: &ChannelTransactionParameters) {
+               self.inner.ready_channel(channel_parameters)
        }
 }
 
+
 impl Writeable for EnforcingChannelKeys {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
                self.inner.write(writer)?;
-               let (obscure, last) = *self.commitment_number_obscure_and_last.lock().unwrap();
-               obscure.write(writer)?;
+               let last = *self.last_commitment_number.lock().unwrap();
                last.write(writer)?;
                Ok(())
        }
@@ -150,10 +138,24 @@ impl Writeable for EnforcingChannelKeys {
 impl Readable for EnforcingChannelKeys {
        fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
                let inner = Readable::read(reader)?;
-               let obscure_and_last = Readable::read(reader)?;
+               let last_commitment_number = Readable::read(reader)?;
                Ok(EnforcingChannelKeys {
-                       inner: inner,
-                       commitment_number_obscure_and_last: Arc::new(Mutex::new(obscure_and_last))
+                       inner,
+                       last_commitment_number: Arc::new(Mutex::new(last_commitment_number))
                })
        }
 }
+
+impl EnforcingChannelKeys {
+       fn verify_counterparty_commitment_tx<'a, T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &'a CommitmentTransaction, secp_ctx: &Secp256k1<T>) -> TrustedCommitmentTransaction<'a> {
+               commitment_tx.verify(&self.inner.get_channel_parameters().as_counterparty_broadcastable(),
+                                    self.inner.counterparty_pubkeys(), self.inner.pubkeys(), secp_ctx)
+                       .expect("derived different per-tx keys or built transaction")
+       }
+
+       fn verify_holder_commitment_tx<'a, T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &'a CommitmentTransaction, secp_ctx: &Secp256k1<T>) -> TrustedCommitmentTransaction<'a> {
+               commitment_tx.verify(&self.inner.get_channel_parameters().as_holder_broadcastable(),
+                                    self.inner.pubkeys(), self.inner.counterparty_pubkeys(), secp_ctx)
+                       .expect("derived different per-tx keys or built transaction")
+       }
+}
index 961d4f1020073f8d76553374ef0cec394e217828..581f228ba7574323180cf662581425e301d74aab 100644 (file)
@@ -31,7 +31,8 @@ use util::byte_utils;
 
 use util::byte_utils::{be64_to_array, be48_to_array, be32_to_array, be16_to_array, slice_to_be16, slice_to_be32, slice_to_be48, slice_to_be64};
 
-const MAX_BUF_SIZE: usize = 64 * 1024;
+/// serialization buffer size
+pub const MAX_BUF_SIZE: usize = 64 * 1024;
 
 /// A trait that is similar to std::io::Write but has one extra function which can be used to size
 /// buffers being written into.