f - Check BOLT2 compatibility in close_channel
[rust-lightning] / lightning / src / ln / channel.rs
index 4051ebf3fbf6cc0d07dd29c4132c2dab3510bb43..94ce779ff895f15ffc3836025d03b9fb607d1f65 100644 (file)
@@ -9,15 +9,15 @@
 
 use bitcoin::blockdata::script::{Script,Builder};
 use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType};
-use bitcoin::blockdata::opcodes;
 use bitcoin::util::bip143;
 use bitcoin::consensus::encode;
 
 use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::sha256d::Hash as Sha256d;
-use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
+use bitcoin::hash_types::{Txid, BlockHash};
 
+use bitcoin::secp256k1::constants::PUBLIC_KEY_SIZE;
 use bitcoin::secp256k1::key::{PublicKey,SecretKey};
 use bitcoin::secp256k1::{Secp256k1,Signature};
 use bitcoin::secp256k1;
@@ -26,9 +26,11 @@ use ln::{PaymentPreimage, PaymentHash};
 use ln::features::{ChannelFeatures, InitFeatures};
 use ln::msgs;
 use ln::msgs::{DecodeError, OptionalField, DataLossProtect};
-use ln::channelmanager::{BestBlock, PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
+use ln::script::ShutdownScript;
+use ln::channelmanager::{PendingHTLCStatus, HTLCSource, HTLCFailReason, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
 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, get_commitment_transaction_number_obscure_factor};
 use ln::chan_utils;
+use chain::BestBlock;
 use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER};
 use chain::transaction::{OutPoint, TransactionData};
@@ -42,11 +44,11 @@ use util::scid_utils::scid_from_parts;
 
 use prelude::*;
 use core::{cmp,mem,fmt};
+use core::convert::TryFrom;
 use core::ops::Deref;
-#[cfg(any(test, feature = "fuzztarget"))]
-use std::sync::Mutex;
+#[cfg(any(test, feature = "fuzztarget", debug_assertions))]
+use sync::Mutex;
 use bitcoin::hashes::hex::ToHex;
-use bitcoin::blockdata::opcodes::all::OP_PUSHBYTES_0;
 
 #[cfg(test)]
 pub struct ChannelValueStat {
@@ -300,6 +302,33 @@ pub struct CounterpartyForwardingInfo {
        pub cltv_expiry_delta: u16,
 }
 
+/// A return value enum for get_update_fulfill_htlc. See UpdateFulfillCommitFetch variants for
+/// description
+enum UpdateFulfillFetch {
+       NewClaim {
+               monitor_update: ChannelMonitorUpdate,
+               msg: Option<msgs::UpdateFulfillHTLC>,
+       },
+       DuplicateClaim {},
+}
+
+/// The return type of get_update_fulfill_htlc_and_commit.
+pub enum UpdateFulfillCommitFetch {
+       /// Indicates the HTLC fulfill is new, and either generated an update_fulfill message, placed
+       /// it in the holding cell, or re-generated the update_fulfill message after the same claim was
+       /// previously placed in the holding cell (and has since been removed).
+       NewClaim {
+               /// The ChannelMonitorUpdate which places the new payment preimage in the channel monitor
+               monitor_update: ChannelMonitorUpdate,
+               /// The update_fulfill message and commitment_signed message (if the claim was not placed
+               /// in the holding cell).
+               msgs: Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>,
+       },
+       /// Indicates the HTLC fulfill is duplicative and already existed either in the holding cell
+       /// or has been forgotten (presumably previously claimed).
+       DuplicateClaim {},
+}
+
 // TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking
 // has been completed, and then turn into a Channel to get compiler-time enforcement of things like
 // calling channel_id() before we're set up or things like get_outbound_funding_signed on an
@@ -320,7 +349,7 @@ pub(super) struct Channel<Signer: Sign> {
        latest_monitor_update_id: u64,
 
        holder_signer: Signer,
-       shutdown_pubkey: PublicKey,
+       shutdown_scriptpubkey: Option<ShutdownScript>,
        destination_script: Script,
 
        // Our commitment numbers start at 2^48-1 and count down, whereas the ones used in transaction
@@ -373,10 +402,10 @@ pub(super) struct Channel<Signer: Sign> {
 
        #[cfg(debug_assertions)]
        /// Max to_local and to_remote outputs in a locally-generated commitment transaction
-       holder_max_commitment_tx_output: ::std::sync::Mutex<(u64, u64)>,
+       holder_max_commitment_tx_output: Mutex<(u64, u64)>,
        #[cfg(debug_assertions)]
        /// Max to_local and to_remote outputs in a remote-generated commitment transaction
-       counterparty_max_commitment_tx_output: ::std::sync::Mutex<(u64, u64)>,
+       counterparty_max_commitment_tx_output: Mutex<(u64, u64)>,
 
        last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, holder_sig)
 
@@ -396,7 +425,7 @@ pub(super) struct Channel<Signer: Sign> {
        counterparty_max_htlc_value_in_flight_msat: u64,
        //get_holder_max_htlc_value_in_flight_msat(): u64,
        /// minimum channel reserve for self to maintain - set by them.
-       counterparty_selected_channel_reserve_satoshis: u64,
+       counterparty_selected_channel_reserve_satoshis: Option<u64>,
        // get_holder_selected_channel_reserve_satoshis(channel_value_sats: u64): u64
        counterparty_htlc_minimum_msat: u64,
        holder_htlc_minimum_msat: u64,
@@ -405,7 +434,7 @@ pub(super) struct Channel<Signer: Sign> {
        #[cfg(not(test))]
        counterparty_max_accepted_htlcs: u16,
        //implied by OUR_MAX_HTLCS: max_accepted_htlcs: u16,
-       minimum_depth: u32,
+       minimum_depth: Option<u32>,
 
        counterparty_forwarding_info: Option<CounterpartyForwardingInfo>,
 
@@ -443,6 +472,15 @@ pub(super) struct Channel<Signer: Sign> {
        ///
        /// See-also <https://github.com/lightningnetwork/lnd/issues/4006>
        pub workaround_lnd_bug_4006: Option<msgs::FundingLocked>,
+
+       #[cfg(any(test, feature = "fuzztarget"))]
+       // When we receive an HTLC fulfill on an outbound path, we may immediately fulfill the
+       // corresponding HTLC on the inbound path. If, then, the outbound path channel is
+       // disconnected and reconnected (before we've exchange commitment_signed and revoke_and_ack
+       // messages), they may re-broadcast their update_fulfill_htlc, causing a duplicate claim. This
+       // is fine, but as a sanity check in our failure to generate the second claim, we check here
+       // that the original was a claim, and that we aren't now trying to fulfill a failed HTLC.
+       historical_inbound_htlc_fulfills: HashSet<u64>,
 }
 
 #[cfg(any(test, feature = "fuzztarget"))]
@@ -455,7 +493,6 @@ struct CommitmentTxInfoCached {
 }
 
 pub const OUR_MAX_HTLCS: u16 = 50; //TODO
-const SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT: u64 = 79; // prevout: 36, nSequence: 4, script len: 1, witness lengths: (3+1)/4, sig: 73/4, if-selector: 1, redeemScript: (6 ops + 2*33 pubkeys + 1*2 delay)/4
 
 #[cfg(not(test))]
 const COMMITMENT_TX_BASE_WEIGHT: u64 = 724;
@@ -530,7 +567,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
 
        // Constructors:
-       pub fn new_outbound<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, APIError>
+       pub fn new_outbound<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: InitFeatures, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, APIError>
        where K::Target: KeysInterface<Signer = Signer>,
              F::Target: FeeEstimator,
        {
@@ -558,6 +595,16 @@ impl<Signer: Sign> Channel<Signer> {
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());
 
+               let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey {
+                       Some(keys_provider.get_shutdown_scriptpubkey())
+               } else { None };
+
+               if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey {
+                       if !shutdown_scriptpubkey.is_compatible(&their_features) {
+                               return Err(APIError::APIMisuseError { err: format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex()) });
+                       }
+               }
+
                Ok(Channel {
                        user_id,
                        config: config.channel_options.clone(),
@@ -570,7 +617,7 @@ impl<Signer: Sign> Channel<Signer> {
                        latest_monitor_update_id: 0,
 
                        holder_signer,
-                       shutdown_pubkey: keys_provider.get_shutdown_pubkey(),
+                       shutdown_scriptpubkey,
                        destination_script: keys_provider.get_destination_script(),
 
                        cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
@@ -595,9 +642,9 @@ impl<Signer: Sign> Channel<Signer> {
                        monitor_pending_failures: Vec::new(),
 
                        #[cfg(debug_assertions)]
-                       holder_max_commitment_tx_output: ::std::sync::Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
+                       holder_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
                        #[cfg(debug_assertions)]
-                       counterparty_max_commitment_tx_output: ::std::sync::Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
+                       counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
 
                        last_sent_closing_fee: None,
 
@@ -609,11 +656,11 @@ impl<Signer: Sign> Channel<Signer> {
                        counterparty_dust_limit_satoshis: 0,
                        holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS,
                        counterparty_max_htlc_value_in_flight_msat: 0,
-                       counterparty_selected_channel_reserve_satoshis: 0,
+                       counterparty_selected_channel_reserve_satoshis: None, // Filled in in accept_channel
                        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_max_accepted_htlcs: 0,
-                       minimum_depth: 0, // Filled in in accept_channel
+                       minimum_depth: None, // Filled in in accept_channel
 
                        counterparty_forwarding_info: None,
 
@@ -644,6 +691,9 @@ impl<Signer: Sign> Channel<Signer> {
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
                        workaround_lnd_bug_4006: None,
+
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       historical_inbound_htlc_fulfills: HashSet::new(),
                })
        }
 
@@ -783,11 +833,11 @@ impl<Signer: Sign> Channel<Signer> {
                                        // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything
                                        if script.len() == 0 {
                                                None
-                                       // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel
-                                       } else if is_unsupported_shutdown_script(&their_features, script) {
-                                               return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex())));
                                        } else {
-                                               Some(script.clone())
+                                               match ShutdownScript::try_from((script.clone(), &their_features)) {
+                                                       Ok(shutdown_script) => Some(shutdown_script.into_inner()),
+                                                       Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))),
+                                               }
                                        }
                                },
                                // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel
@@ -797,6 +847,16 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                } else { None };
 
+               let shutdown_scriptpubkey = if config.channel_options.commit_upfront_shutdown_pubkey {
+                       Some(keys_provider.get_shutdown_scriptpubkey())
+               } else { None };
+
+               if let Some(shutdown_scriptpubkey) = &shutdown_scriptpubkey {
+                       if !shutdown_scriptpubkey.is_compatible(&their_features) {
+                               return Err(ChannelError::Close(format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex())));
+                       }
+               }
+
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());
 
@@ -811,7 +871,7 @@ impl<Signer: Sign> Channel<Signer> {
                        latest_monitor_update_id: 0,
 
                        holder_signer,
-                       shutdown_pubkey: keys_provider.get_shutdown_pubkey(),
+                       shutdown_scriptpubkey,
                        destination_script: keys_provider.get_destination_script(),
 
                        cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
@@ -836,9 +896,9 @@ impl<Signer: Sign> Channel<Signer> {
                        monitor_pending_failures: Vec::new(),
 
                        #[cfg(debug_assertions)]
-                       holder_max_commitment_tx_output: ::std::sync::Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
+                       holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
                        #[cfg(debug_assertions)]
-                       counterparty_max_commitment_tx_output: ::std::sync::Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
+                       counterparty_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
 
                        last_sent_closing_fee: None,
 
@@ -851,11 +911,11 @@ impl<Signer: Sign> Channel<Signer> {
                        counterparty_dust_limit_satoshis: msg.dust_limit_satoshis,
                        holder_dust_limit_satoshis: MIN_DUST_LIMIT_SATOSHIS,
                        counterparty_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000),
-                       counterparty_selected_channel_reserve_satoshis: msg.channel_reserve_satoshis,
+                       counterparty_selected_channel_reserve_satoshis: Some(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_max_accepted_htlcs: msg.max_accepted_htlcs,
-                       minimum_depth: config.own_channel_config.minimum_depth,
+                       minimum_depth: Some(config.own_channel_config.minimum_depth),
 
                        counterparty_forwarding_info: None,
 
@@ -889,6 +949,9 @@ impl<Signer: Sign> Channel<Signer> {
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
                        workaround_lnd_bug_4006: None,
+
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       historical_inbound_htlc_fulfills: HashSet::new(),
                };
 
                Ok(chan)
@@ -1036,7 +1099,7 @@ impl<Signer: Sign> Channel<Signer> {
                        } else {
                                self.counterparty_max_commitment_tx_output.lock().unwrap()
                        };
-                       debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.counterparty_selected_channel_reserve_satoshis as i64);
+                       debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.counterparty_selected_channel_reserve_satoshis.unwrap() as i64);
                        broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat as u64);
                        debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
                        broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64);
@@ -1087,8 +1150,7 @@ impl<Signer: Sign> Channel<Signer> {
 
        #[inline]
        fn get_closing_scriptpubkey(&self) -> Script {
-               let channel_close_key_hash = WPubkeyHash::hash(&self.shutdown_pubkey.serialize());
-               Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_close_key_hash[..]).into_script()
+               self.shutdown_scriptpubkey.clone().unwrap().into_inner()
        }
 
        #[inline]
@@ -1154,6 +1216,7 @@ impl<Signer: Sign> Channel<Signer> {
                        }, ()));
                }
 
+               assert!(self.shutdown_scriptpubkey.is_some());
                if value_to_self as u64 > self.holder_dust_limit_satoshis {
                        txouts.push((TxOut {
                                script_pubkey: self.get_closing_scriptpubkey(),
@@ -1216,20 +1279,7 @@ impl<Signer: Sign> Channel<Signer> {
                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.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.
-       /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
-       /// return Ok(_) if debug assertions are turned on or preconditions are met.
-       ///
-       /// Note that it is still possible to hit these assertions in case we find a preimage on-chain
-       /// but then have a reorg which settles on an HTLC-failure on chain.
-       fn get_update_fulfill_htlc<L: Deref>(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage, logger: &L) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> where L::Target: Logger {
+       fn get_update_fulfill_htlc<L: Deref>(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage, logger: &L) -> UpdateFulfillFetch where L::Target: Logger {
                // Either ChannelFunded got set (which means it won't be unset) or there is no way any
                // caller thought we could have something claimed (cause we wouldn't have accepted in an
                // incoming HTLC anyway). If we got to ShutdownComplete, callers aren't allowed to call us,
@@ -1255,9 +1305,9 @@ impl<Signer: Sign> Channel<Signer> {
                                                if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
                                                } else {
                                                        log_warn!(logger, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id()));
+                                                       debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
                                                }
-                                               debug_assert!(false, "Tried to fulfill an HTLC that was already fail/fulfilled");
-                                               return Ok((None, None));
+                                               return UpdateFulfillFetch::DuplicateClaim {};
                                        },
                                        _ => {
                                                debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to");
@@ -1269,7 +1319,11 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
                if pending_idx == core::usize::MAX {
-                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       // If we failed to find an HTLC to fulfill, make sure it was previously fulfilled and
+                       // this is simply a duplicate claim, not previously failed and we lost funds.
+                       debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
+                       return UpdateFulfillFetch::DuplicateClaim {};
                }
 
                // Now update local state:
@@ -1291,8 +1345,9 @@ impl<Signer: Sign> Channel<Signer> {
                                                if htlc_id_arg == htlc_id {
                                                        // Make sure we don't leave latest_monitor_update_id incremented here:
                                                        self.latest_monitor_update_id -= 1;
-                                                       debug_assert!(false, "Tried to fulfill an HTLC that was already fulfilled");
-                                                       return Ok((None, None));
+                                                       #[cfg(any(test, feature = "fuzztarget"))]
+                                                       debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
+                                                       return UpdateFulfillFetch::DuplicateClaim {};
                                                }
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
@@ -1301,7 +1356,7 @@ impl<Signer: Sign> Channel<Signer> {
                                                        // TODO: We may actually be able to switch to a fulfill here, though its
                                                        // rare enough it may not be worth the complexity burden.
                                                        debug_assert!(false, "Tried to fulfill an HTLC that was already failed");
-                                                       return Ok((None, Some(monitor_update)));
+                                                       return UpdateFulfillFetch::NewClaim { monitor_update, msg: None };
                                                }
                                        },
                                        _ => {}
@@ -1311,52 +1366,58 @@ impl<Signer: Sign> Channel<Signer> {
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
                                payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg,
                        });
-                       return Ok((None, Some(monitor_update)));
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       self.historical_inbound_htlc_fulfills.insert(htlc_id_arg);
+                       return UpdateFulfillFetch::NewClaim { monitor_update, msg: None };
                }
+               #[cfg(any(test, feature = "fuzztarget"))]
+               self.historical_inbound_htlc_fulfills.insert(htlc_id_arg);
 
                {
                        let htlc = &mut self.pending_inbound_htlcs[pending_idx];
                        if let InboundHTLCState::Committed = htlc.state {
                        } else {
                                debug_assert!(false, "Have an inbound HTLC we tried to claim before it was fully committed to");
-                               return Ok((None, Some(monitor_update)));
+                               return UpdateFulfillFetch::NewClaim { monitor_update, msg: None };
                        }
                        log_trace!(logger, "Upgrading HTLC {} to LocalRemoved with a Fulfill in channel {}!", log_bytes!(htlc.payment_hash.0), log_bytes!(self.channel_id));
                        htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(payment_preimage_arg.clone()));
                }
 
-               Ok((Some(msgs::UpdateFulfillHTLC {
-                       channel_id: self.channel_id(),
-                       htlc_id: htlc_id_arg,
-                       payment_preimage: payment_preimage_arg,
-               }), Some(monitor_update)))
+               UpdateFulfillFetch::NewClaim {
+                       monitor_update,
+                       msg: Some(msgs::UpdateFulfillHTLC {
+                               channel_id: self.channel_id(),
+                               htlc_id: htlc_id_arg,
+                               payment_preimage: payment_preimage_arg,
+                       }),
+               }
        }
 
-       pub fn get_update_fulfill_htlc_and_commit<L: Deref>(&mut self, htlc_id: u64, payment_preimage: PaymentPreimage, logger: &L) -> Result<(Option<(msgs::UpdateFulfillHTLC, msgs::CommitmentSigned)>, Option<ChannelMonitorUpdate>), ChannelError> where L::Target: Logger {
-               match self.get_update_fulfill_htlc(htlc_id, payment_preimage, logger)? {
-                       (Some(update_fulfill_htlc), Some(mut monitor_update)) => {
-                               let (commitment, mut additional_update) = self.send_commitment_no_status_check(logger)?;
+       pub fn get_update_fulfill_htlc_and_commit<L: Deref>(&mut self, htlc_id: u64, payment_preimage: PaymentPreimage, logger: &L) -> Result<UpdateFulfillCommitFetch, (ChannelError, ChannelMonitorUpdate)> where L::Target: Logger {
+               match self.get_update_fulfill_htlc(htlc_id, payment_preimage, logger) {
+                       UpdateFulfillFetch::NewClaim { mut monitor_update, msg: Some(update_fulfill_htlc) } => {
+                               let (commitment, mut additional_update) = match self.send_commitment_no_status_check(logger) {
+                                       Err(e) => return Err((e, monitor_update)),
+                                       Ok(res) => res
+                               };
                                // send_commitment_no_status_check may bump latest_monitor_id but we want them to be
                                // strictly increasing by one, so decrement it here.
                                self.latest_monitor_update_id = monitor_update.update_id;
                                monitor_update.updates.append(&mut additional_update.updates);
-                               Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update)))
-                       },
-                       (Some(update_fulfill_htlc), None) => {
-                               let (commitment, monitor_update) = self.send_commitment_no_status_check(logger)?;
-                               Ok((Some((update_fulfill_htlc, commitment)), Some(monitor_update)))
+                               Ok(UpdateFulfillCommitFetch::NewClaim { monitor_update, msgs: Some((update_fulfill_htlc, commitment)) })
                        },
-                       (None, Some(monitor_update)) => Ok((None, Some(monitor_update))),
-                       (None, None) => Ok((None, None))
+                       UpdateFulfillFetch::NewClaim { monitor_update, msg: None } => Ok(UpdateFulfillCommitFetch::NewClaim { monitor_update, msgs: None }),
+                       UpdateFulfillFetch::DuplicateClaim {} => Ok(UpdateFulfillCommitFetch::DuplicateClaim {}),
                }
        }
 
-       /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
-       /// In such cases we debug_assert!(false) and return a ChannelError::Ignore. Thus, will always
-       /// return Ok(_) if debug assertions are turned on or preconditions are met.
-       ///
-       /// Note that it is still possible to hit these assertions in case we find a preimage on-chain
-       /// but then have a reorg which settles on an HTLC-failure on chain.
+       /// We can only have one resolution per HTLC. In some cases around reconnect, we may fulfill
+       /// an HTLC more than once or fulfill once and then attempt to fail after reconnect. We cannot,
+       /// however, fail more than once as we wait for an upstream failure to be irrevocably committed
+       /// before we fail backwards.
+       /// If we do fail twice, we debug_assert!(false) and return Ok(None). Thus, will always return
+       /// Ok(_) if debug assertions are turned on or preconditions are met.
        pub fn get_update_fail_htlc<L: Deref>(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket, logger: &L) -> Result<Option<msgs::UpdateFailHTLC>, ChannelError> where L::Target: Logger {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        panic!("Was asked to fail an HTLC when channel was not in an operational state");
@@ -1372,8 +1433,11 @@ impl<Signer: Sign> Channel<Signer> {
                        if htlc.htlc_id == htlc_id_arg {
                                match htlc.state {
                                        InboundHTLCState::Committed => {},
-                                       InboundHTLCState::LocalRemoved(_) => {
-                                               debug_assert!(false, "Tried to fail an HTLC that was already fail/fulfilled");
+                                       InboundHTLCState::LocalRemoved(ref reason) => {
+                                               if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
+                                               } else {
+                                                       debug_assert!(false, "Tried to fail an HTLC that was already failed");
+                                               }
                                                return Ok(None);
                                        },
                                        _ => {
@@ -1385,7 +1449,11 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
                if pending_idx == core::usize::MAX {
-                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       // If we failed to find an HTLC to fail, make sure it was previously fulfilled and this
+                       // is simply a duplicate fail, not previously failed and we failed-back too early.
+                       debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
+                       return Ok(None);
                }
 
                // Now update local state:
@@ -1394,8 +1462,9 @@ impl<Signer: Sign> Channel<Signer> {
                                match pending_update {
                                        &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
                                                if htlc_id_arg == htlc_id {
-                                                       debug_assert!(false, "Tried to fail an HTLC that was already fulfilled");
-                                                       return Err(ChannelError::Ignore("Unable to find a pending HTLC which matched the given HTLC ID".to_owned()));
+                                                       #[cfg(any(test, feature = "fuzztarget"))]
+                                                       debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
+                                                       return Ok(None);
                                                }
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
@@ -1488,6 +1557,12 @@ impl<Signer: Sign> Channel<Signer> {
                if msg.minimum_depth > config.peer_channel_config_limits.max_minimum_depth {
                        return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", config.peer_channel_config_limits.max_minimum_depth, msg.minimum_depth)));
                }
+               if msg.minimum_depth == 0 {
+                       // Note that if this changes we should update the serialization minimum version to
+                       // indicate to older clients that they don't understand some features of the current
+                       // channel.
+                       return Err(ChannelError::Close("Minimum confirmation depth must be at least 1".to_owned()));
+               }
 
                let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
                        match &msg.shutdown_scriptpubkey {
@@ -1495,11 +1570,11 @@ impl<Signer: Sign> Channel<Signer> {
                                        // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything
                                        if script.len() == 0 {
                                                None
-                                       // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel
-                                       } else if is_unsupported_shutdown_script(&their_features, script) {
-                                               return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex())));
                                        } else {
-                                               Some(script.clone())
+                                               match ShutdownScript::try_from((script.clone(), &their_features)) {
+                                                       Ok(shutdown_script) => Some(shutdown_script.into_inner()),
+                                                       Err(_) => return Err(ChannelError::Close(format!("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format. script: ({})", script.to_bytes().to_hex()))),
+                                               }
                                        }
                                },
                                // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel
@@ -1511,10 +1586,10 @@ impl<Signer: Sign> Channel<Signer> {
 
                self.counterparty_dust_limit_satoshis = msg.dust_limit_satoshis;
                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_selected_channel_reserve_satoshis = Some(msg.channel_reserve_satoshis);
                self.counterparty_htlc_minimum_msat = msg.htlc_minimum_msat;
                self.counterparty_max_accepted_htlcs = msg.max_accepted_htlcs;
-               self.minimum_depth = msg.minimum_depth;
+               self.minimum_depth = Some(msg.minimum_depth);
 
                let counterparty_pubkeys = ChannelPublicKeys {
                        funding_pubkey: msg.funding_pubkey,
@@ -1621,8 +1696,9 @@ impl<Signer: Sign> Channel<Signer> {
                let funding_redeemscript = self.get_funding_redeemscript();
                let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
                let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound());
+               let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
                let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(),
-                                                         &self.shutdown_pubkey, self.get_holder_selected_contest_delay(),
+                                                         shutdown_script, self.get_holder_selected_contest_delay(),
                                                          &self.destination_script, (funding_txo, funding_txo_script.clone()),
                                                          &self.channel_transaction_parameters,
                                                          funding_redeemscript.clone(), self.channel_value_satoshis,
@@ -1694,8 +1770,9 @@ impl<Signer: Sign> Channel<Signer> {
                let funding_txo = self.get_funding_txo().unwrap();
                let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
                let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.get_holder_pubkeys().payment_point, &self.get_counterparty_pubkeys().payment_point, self.is_outbound());
+               let shutdown_script = self.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
                let channel_monitor = ChannelMonitor::new(self.secp_ctx.clone(), self.holder_signer.clone(),
-                                                         &self.shutdown_pubkey, self.get_holder_selected_contest_delay(),
+                                                         shutdown_script, 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,
@@ -1785,8 +1862,22 @@ impl<Signer: Sign> Channel<Signer> {
        /// corner case properly.
        pub fn get_inbound_outbound_available_balance_msat(&self) -> (u64, u64) {
                // Note that we have to handle overflow due to the above case.
-               (cmp::max(self.channel_value_satoshis as i64 * 1000 - self.value_to_self_msat as i64 - self.get_inbound_pending_htlc_stats().1 as i64, 0) as u64,
-               cmp::max(self.value_to_self_msat as i64 - self.get_outbound_pending_htlc_stats().1 as i64, 0) as u64)
+               (
+                       cmp::max(self.channel_value_satoshis as i64 * 1000
+                               - self.value_to_self_msat as i64
+                               - self.get_inbound_pending_htlc_stats().1 as i64
+                               - Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64 * 1000,
+                       0) as u64,
+                       cmp::max(self.value_to_self_msat as i64
+                               - self.get_outbound_pending_htlc_stats().1 as i64
+                               - self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0) as i64 * 1000,
+                       0) as u64
+               )
+       }
+
+       pub fn get_holder_counterparty_selected_channel_reserve_satoshis(&self) -> (u64, Option<u64>) {
+               (Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis),
+               self.counterparty_selected_channel_reserve_satoshis)
        }
 
        // Get the fee cost of a commitment tx with a given number of HTLC outputs.
@@ -2064,7 +2155,7 @@ impl<Signer: Sign> Channel<Signer> {
                        // Check that they won't violate our local required channel reserve by adding this HTLC.
                        let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
                        let local_commit_tx_fee_msat = self.next_local_commit_tx_fee_msat(htlc_candidate, None);
-                       if self.value_to_self_msat < self.counterparty_selected_channel_reserve_satoshis * 1000 + local_commit_tx_fee_msat {
+                       if self.value_to_self_msat < self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat {
                                return Err(ChannelError::Close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned()));
                        }
                }
@@ -2236,7 +2327,10 @@ impl<Signer: Sign> Channel<Signer> {
                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_tx = chan_utils::build_htlc_transaction(&commitment_txid, feerate_per_kw,
+                                       self.get_counterparty_selected_contest_delay().unwrap(), &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)[..]);
                                log_trace!(logger, "Checking HTLC tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} in channel {}.",
@@ -2419,24 +2513,28 @@ impl<Signer: Sign> Channel<Signer> {
                                                }
                                        },
                                        &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => {
-                                               match self.get_update_fulfill_htlc(htlc_id, *payment_preimage, logger) {
-                                                       Ok((update_fulfill_msg_option, additional_monitor_update_opt)) => {
-                                                               update_fulfill_htlcs.push(update_fulfill_msg_option.unwrap());
-                                                               if let Some(mut additional_monitor_update) = additional_monitor_update_opt {
-                                                                       monitor_update.updates.append(&mut additional_monitor_update.updates);
-                                                               }
-                                                       },
-                                                       Err(e) => {
-                                                               if let ChannelError::Ignore(_) = e {}
-                                                               else {
-                                                                       panic!("Got a non-IgnoreError action trying to fulfill holding cell HTLC");
-                                                               }
-                                                       }
-                                               }
+                                               // If an HTLC claim was previously added to the holding cell (via
+                                               // `get_update_fulfill_htlc`, then generating the claim message itself must
+                                               // not fail - any in between attempts to claim the HTLC will have resulted
+                                               // in it hitting the holding cell again and we cannot change the state of a
+                                               // holding cell HTLC from fulfill to anything else.
+                                               let (update_fulfill_msg_option, mut additional_monitor_update) =
+                                                       if let UpdateFulfillFetch::NewClaim { msg, monitor_update } = self.get_update_fulfill_htlc(htlc_id, *payment_preimage, logger) {
+                                                               (msg, monitor_update)
+                                                       } else { unreachable!() };
+                                               update_fulfill_htlcs.push(update_fulfill_msg_option.unwrap());
+                                               monitor_update.updates.append(&mut additional_monitor_update.updates);
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
                                                match self.get_update_fail_htlc(htlc_id, err_packet.clone(), logger) {
-                                                       Ok(update_fail_msg_option) => update_fail_htlcs.push(update_fail_msg_option.unwrap()),
+                                                       Ok(update_fail_msg_option) => {
+                                                               // If an HTLC failure was previously added to the holding cell (via
+                                                               // `get_update_fail_htlc`) then generating the fail message itself
+                                                               // must not fail - we should never end up in a state where we
+                                                               // double-fail an HTLC or fail-then-claim an HTLC as it indicates
+                                                               // we didn't wait for a full revocation before failing.
+                                                               update_fail_htlcs.push(update_fail_msg_option.unwrap())
+                                                       },
                                                        Err(e) => {
                                                                if let ChannelError::Ignore(_) = e {}
                                                                else {
@@ -3005,6 +3103,7 @@ impl<Signer: Sign> Channel<Signer> {
                self.channel_state &= !(ChannelState::PeerDisconnected as u32);
 
                let shutdown_msg = if self.channel_state & (ChannelState::LocalShutdownSent as u32) != 0 {
+                       assert!(self.shutdown_scriptpubkey.is_some());
                        Some(msgs::Shutdown {
                                channel_id: self.channel_id,
                                scriptpubkey: self.get_closing_scriptpubkey(),
@@ -3116,6 +3215,7 @@ impl<Signer: Sign> Channel<Signer> {
                if self.feerate_per_kw > proposed_feerate {
                        proposed_feerate = self.feerate_per_kw;
                }
+               assert!(self.shutdown_scriptpubkey.is_some());
                let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap()));
                let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000;
 
@@ -3134,8 +3234,12 @@ impl<Signer: Sign> Channel<Signer> {
                })
        }
 
-       pub fn shutdown<F: Deref>(&mut self, fee_estimator: &F, their_features: &InitFeatures, msg: &msgs::Shutdown) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>, Vec<(HTLCSource, PaymentHash)>), ChannelError>
-               where F::Target: FeeEstimator
+       pub fn shutdown<F: Deref, K: Deref>(
+               &mut self, fee_estimator: &F, keys_provider: &K, their_features: &InitFeatures, msg: &msgs::Shutdown
+       ) -> Result<(Option<msgs::Shutdown>, Option<msgs::ClosingSigned>, Option<ChannelMonitorUpdate>, Vec<(HTLCSource, PaymentHash)>), ChannelError>
+       where
+               F::Target: FeeEstimator,
+               K::Target: KeysInterface<Signer = Signer>
        {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
                        return Err(ChannelError::Close("Peer sent shutdown when we needed a channel_reestablish".to_owned()));
@@ -3153,18 +3257,36 @@ impl<Signer: Sign> Channel<Signer> {
                }
                assert_eq!(self.channel_state & ChannelState::ShutdownComplete as u32, 0);
 
-               if is_unsupported_shutdown_script(&their_features, &msg.scriptpubkey) {
-                       return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex())));
-               }
+               let shutdown_scriptpubkey = match ShutdownScript::try_from((msg.scriptpubkey.clone(), their_features)) {
+                       Ok(script) => script.into_inner(),
+                       Err(_) => return Err(ChannelError::Close(format!("Got a nonstandard scriptpubkey ({}) from remote peer", msg.scriptpubkey.to_bytes().to_hex()))),
+               };
 
                if self.counterparty_shutdown_scriptpubkey.is_some() {
-                       if Some(&msg.scriptpubkey) != self.counterparty_shutdown_scriptpubkey.as_ref() {
-                               return Err(ChannelError::Close(format!("Got shutdown request with a scriptpubkey ({}) which did not match their previous scriptpubkey.", msg.scriptpubkey.to_bytes().to_hex())));
+                       if Some(&shutdown_scriptpubkey) != self.counterparty_shutdown_scriptpubkey.as_ref() {
+                               return Err(ChannelError::Close(format!("Got shutdown request with a scriptpubkey ({}) which did not match their previous scriptpubkey.", shutdown_scriptpubkey.to_bytes().to_hex())));
                        }
                } else {
-                       self.counterparty_shutdown_scriptpubkey = Some(msg.scriptpubkey.clone());
+                       self.counterparty_shutdown_scriptpubkey = Some(shutdown_scriptpubkey);
                }
 
+               // If we have any LocalAnnounced updates we'll probably just get back a update_fail_htlc
+               // immediately after the commitment dance, but we can send a Shutdown cause we won't send
+               // any further commitment updates after we set LocalShutdownSent.
+               let send_shutdown = (self.channel_state & ChannelState::LocalShutdownSent as u32) != ChannelState::LocalShutdownSent as u32;
+
+               let shutdown_scriptpubkey = match self.shutdown_scriptpubkey {
+                       Some(_) => None,
+                       None => {
+                               assert!(send_shutdown);
+                               let shutdown_scriptpubkey = keys_provider.get_shutdown_scriptpubkey();
+                               if !shutdown_scriptpubkey.is_compatible(their_features) {
+                                       return Err(ChannelError::Close(format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex())));
+                               }
+                               Some(shutdown_scriptpubkey)
+                       },
+               };
+
                // From here on out, we may not fail!
 
                self.channel_state |= ChannelState::RemoteShutdownSent as u32;
@@ -3184,23 +3306,31 @@ impl<Signer: Sign> Channel<Signer> {
                                _ => true
                        }
                });
-               // If we have any LocalAnnounced updates we'll probably just get back a update_fail_htlc
-               // immediately after the commitment dance, but we can send a Shutdown cause we won't send
-               // any further commitment updates after we set LocalShutdownSent.
 
-               let shutdown = if (self.channel_state & ChannelState::LocalShutdownSent as u32) == ChannelState::LocalShutdownSent as u32 {
-                       None
-               } else {
+               let monitor_update = match shutdown_scriptpubkey {
+                       Some(shutdown_scriptpubkey) => {
+                               self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey);
+                               self.latest_monitor_update_id += 1;
+                               Some(ChannelMonitorUpdate {
+                                       update_id: self.latest_monitor_update_id,
+                                       updates: vec![ChannelMonitorUpdateStep::ShutdownScript {
+                                               scriptpubkey: self.get_closing_scriptpubkey(),
+                                       }],
+                               })
+                       },
+                       None => None,
+               };
+               let shutdown = if send_shutdown {
                        Some(msgs::Shutdown {
                                channel_id: self.channel_id,
                                scriptpubkey: self.get_closing_scriptpubkey(),
                        })
-               };
+               } else { None };
 
                self.channel_state |= ChannelState::LocalShutdownSent as u32;
                self.update_time_counter += 1;
 
-               Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), dropped_outbound_htlcs))
+               Ok((shutdown, self.maybe_propose_first_closing_signed(fee_estimator), monitor_update, dropped_outbound_htlcs))
        }
 
        fn build_signed_closing_transaction(&self, tx: &mut Transaction, counterparty_sig: &Signature, sig: &Signature) {
@@ -3275,6 +3405,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                macro_rules! propose_new_feerate {
                        ($new_feerate: expr) => {
+                               assert!(self.shutdown_scriptpubkey.is_some());
                                let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap()));
                                let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * tx_weight / 1000, false);
                                let sig = self.holder_signer
@@ -3336,6 +3467,10 @@ impl<Signer: Sign> Channel<Signer> {
                self.channel_id
        }
 
+       pub fn minimum_depth(&self) -> Option<u32> {
+               self.minimum_depth
+       }
+
        /// Gets the "user_id" value passed into the construction of this channel. It has no special
        /// meaning and exists only to allow users to have a persistent identifier of a channel.
        pub fn get_user_id(&self) -> u64 {
@@ -3363,8 +3498,9 @@ impl<Signer: Sign> Channel<Signer> {
                &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
+       pub fn get_counterparty_selected_contest_delay(&self) -> Option<u16> {
+               self.channel_transaction_parameters.counterparty_parameters
+                       .as_ref().map(|params| params.selected_contest_delay)
        }
 
        fn get_counterparty_pubkeys(&self) -> &ChannelPublicKeys {
@@ -3404,7 +3540,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
 
        pub fn get_fee_proportional_millionths(&self) -> u32 {
-               self.config.fee_proportional_millionths
+               self.config.forwarding_fee_proportional_millionths
        }
 
        pub fn get_cltv_expiry_delta(&self) -> u16 {
@@ -3438,7 +3574,7 @@ impl<Signer: Sign> Channel<Signer> {
                ChannelValueStat {
                        value_to_self_msat: self.value_to_self_msat,
                        channel_value_msat: self.channel_value_satoshis * 1000,
-                       channel_reserve_msat: self.counterparty_selected_channel_reserve_satoshis * 1000,
+                       channel_reserve_msat: self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000,
                        pending_outbound_htlcs_amount_msat: self.pending_outbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
                        pending_inbound_htlcs_amount_msat: self.pending_inbound_htlcs.iter().map(|ref h| h.amount_msat).sum::<u64>(),
                        holding_cell_outbound_amount_msat: {
@@ -3477,24 +3613,8 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Gets the fee we'd want to charge for adding an HTLC output to this Channel
        /// Allowed in any state (including after shutdown)
-       pub fn get_holder_fee_base_msat<F: Deref>(&self, fee_estimator: &F) -> u32
-               where F::Target: FeeEstimator
-       {
-               // For lack of a better metric, we calculate what it would cost to consolidate the new HTLC
-               // output value back into a transaction with the regular channel output:
-
-               // 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.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;
-               }
-
-               // + the marginal cost of an input which spends the HTLC-Success/HTLC-Timeout output:
-               res += fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal) as u64 * SPENDING_INPUT_FOR_A_OUTPUT_WEIGHT / 1000;
-
-               res as u32
+       pub fn get_outbound_forwarding_fee_base_msat(&self) -> u32 {
+               self.config.forwarding_fee_base_msat
        }
 
        /// Returns true if we've ever received a message from the remote end for this Channel
@@ -3555,7 +3675,7 @@ impl<Signer: Sign> Channel<Signer> {
                        self.funding_tx_confirmation_height = 0;
                }
 
-               if funding_tx_confirmations < self.minimum_depth as i64 {
+               if funding_tx_confirmations < self.minimum_depth.unwrap_or(0) as i64 {
                        return None;
                }
 
@@ -3710,10 +3830,10 @@ impl<Signer: Sign> Channel<Signer> {
                        // the funding transaction's confirmation count has dipped below minimum_depth / 2,
                        // close the channel and hope we can get the latest state on chain (because presumably
                        // the funding transaction is at least still in the mempool of most nodes).
-                       if funding_tx_confirmations < self.minimum_depth as i64 / 2 {
+                       if funding_tx_confirmations < self.minimum_depth.unwrap() as i64 / 2 {
                                return Err(msgs::ErrorMessage {
                                        channel_id: self.channel_id(),
-                                       data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth, funding_tx_confirmations),
+                                       data: format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.", self.minimum_depth.unwrap(), funding_tx_confirmations),
                                });
                        }
                }
@@ -3784,7 +3904,10 @@ impl<Signer: Sign> Channel<Signer> {
                        htlc_basepoint: keys.htlc_basepoint,
                        first_per_commitment_point,
                        channel_flags: if self.config.announced_channel {1} else {0},
-                       shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
+                       shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey {
+                               Some(script) => script.clone().into_inner(),
+                               None => Builder::new().into_script(),
+                       }),
                }
        }
 
@@ -3808,7 +3931,7 @@ impl<Signer: Sign> Channel<Signer> {
                        max_htlc_value_in_flight_msat: Channel::<Signer>::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
                        channel_reserve_satoshis: Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis),
                        htlc_minimum_msat: self.holder_htlc_minimum_msat,
-                       minimum_depth: self.minimum_depth,
+                       minimum_depth: self.minimum_depth.unwrap(),
                        to_self_delay: self.get_holder_selected_contest_delay(),
                        max_accepted_htlcs: OUR_MAX_HTLCS,
                        funding_pubkey: keys.funding_pubkey,
@@ -3817,7 +3940,10 @@ impl<Signer: Sign> Channel<Signer> {
                        delayed_payment_basepoint: keys.delayed_payment_basepoint,
                        htlc_basepoint: keys.htlc_basepoint,
                        first_per_commitment_point,
-                       shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() })
+                       shutdown_scriptpubkey: OptionalField::Present(match &self.shutdown_scriptpubkey {
+                               Some(script) => script.clone().into_inner(),
+                               None => Builder::new().into_script(),
+                       }),
                }
        }
 
@@ -4082,7 +4208,7 @@ impl<Signer: Sign> Channel<Signer> {
                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;
-                       let holder_selected_chan_reserve_msat = Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis);
+                       let holder_selected_chan_reserve_msat = Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
                        let htlc_candidate = HTLCCandidate::new(amount_msat, HTLCInitiator::LocalOffered);
                        let counterparty_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(htlc_candidate, None);
                        if counterparty_balance_msat < holder_selected_chan_reserve_msat + counterparty_commit_tx_fee_msat {
@@ -4106,7 +4232,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                // Check self.counterparty_selected_channel_reserve_satoshis (the amount we must keep as
                // reserve for the remote to have something to claim if we misbehave)
-               let chan_reserve_msat = self.counterparty_selected_channel_reserve_satoshis * 1000;
+               let chan_reserve_msat = self.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000;
                if pending_value_to_self_msat - amount_msat - commit_tx_fee_msat < chan_reserve_msat {
                        return Err(ChannelError::Ignore(format!("Cannot send value that would put our balance under counterparty-announced channel reserve value ({})", chan_reserve_msat)));
                }
@@ -4311,8 +4437,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
 
        pub fn channel_update(&mut self, msg: &msgs::ChannelUpdate) -> Result<(), ChannelError> {
-               let usable_channel_value_msat = (self.channel_value_satoshis - self.counterparty_selected_channel_reserve_satoshis) * 1000;
-               if msg.contents.htlc_minimum_msat >= usable_channel_value_msat {
+               if msg.contents.htlc_minimum_msat >= self.channel_value_satoshis * 1000 {
                        return Err(ChannelError::Close("Minimum htlc value is greater than channel value".to_string()));
                }
                self.counterparty_forwarding_info = Some(CounterpartyForwardingInfo {
@@ -4326,7 +4451,8 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Begins the shutdown process, getting a message for the remote peer and returning all
        /// holding cell HTLCs for payment failure.
-       pub fn get_shutdown(&mut self) -> Result<(msgs::Shutdown, Vec<(HTLCSource, PaymentHash)>), APIError> {
+       pub fn get_shutdown<K: Deref>(&mut self, keys_provider: &K, their_features: &InitFeatures) -> Result<(msgs::Shutdown, Option<ChannelMonitorUpdate>, Vec<(HTLCSource, PaymentHash)>), APIError>
+       where K::Target: KeysInterface<Signer = Signer> {
                for htlc in self.pending_outbound_htlcs.iter() {
                        if let OutboundHTLCState::LocalAnnounced(_) = htlc.state {
                                return Err(APIError::APIMisuseError{err: "Cannot begin shutdown with pending HTLCs. Process pending events first".to_owned()});
@@ -4345,7 +4471,30 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(APIError::ChannelUnavailable{err: "Cannot begin shutdown while peer is disconnected or we're waiting on a monitor update, maybe force-close instead?".to_owned()});
                }
 
-               let closing_script = self.get_closing_scriptpubkey();
+               let shutdown_scriptpubkey = match self.shutdown_scriptpubkey {
+                       Some(_) => None,
+                       None => {
+                               let shutdown_scriptpubkey = keys_provider.get_shutdown_scriptpubkey();
+                               if !shutdown_scriptpubkey.is_compatible(their_features) {
+                                       return Err(APIError::APIMisuseError { err: format!("Provided a scriptpubkey format not accepted by peer. script: ({})", shutdown_scriptpubkey.clone().into_inner().to_bytes().to_hex()) });
+                               }
+                               Some(shutdown_scriptpubkey)
+                       },
+               };
+
+               let monitor_update = match shutdown_scriptpubkey {
+                       Some(shutdown_scriptpubkey) => {
+                               self.shutdown_scriptpubkey = Some(shutdown_scriptpubkey);
+                               self.latest_monitor_update_id += 1;
+                               Some(ChannelMonitorUpdate {
+                                       update_id: self.latest_monitor_update_id,
+                                       updates: vec![ChannelMonitorUpdateStep::ShutdownScript {
+                                               scriptpubkey: self.get_closing_scriptpubkey(),
+                                       }],
+                               })
+                       },
+                       None => None,
+               };
 
                // From here on out, we may not fail!
                if self.channel_state < ChannelState::FundingSent as u32 {
@@ -4371,8 +4520,8 @@ impl<Signer: Sign> Channel<Signer> {
 
                Ok((msgs::Shutdown {
                        channel_id: self.channel_id,
-                       scriptpubkey: closing_script,
-               }, dropped_outbound_htlcs))
+                       scriptpubkey: self.get_closing_scriptpubkey(),
+               }, monitor_update, dropped_outbound_htlcs))
        }
 
        /// Gets the latest commitment transaction and any dependent transactions for relay (forcing
@@ -4421,25 +4570,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
 }
 
-fn is_unsupported_shutdown_script(their_features: &InitFeatures, script: &Script) -> bool {
-       // We restrain shutdown scripts to standards forms to avoid transactions not propagating on the p2p tx-relay network
-
-       // BOLT 2 says we must only send a scriptpubkey of certain standard forms,
-       // which for a a BIP-141-compliant witness program is at max 42 bytes in length.
-       // So don't let the remote peer feed us some super fee-heavy script.
-       let is_script_too_long = script.len() > 42;
-       if is_script_too_long {
-               return true;
-       }
-
-       if their_features.supports_shutdown_anysegwit() && script.is_witness_program() && script.as_bytes()[0] != OP_PUSHBYTES_0.into_u8() {
-               return false;
-       }
-
-       return !script.is_p2pkh() && !script.is_p2sh() && !script.is_v0_p2wpkh() && !script.is_v0_p2wsh()
-}
-
-const SERIALIZATION_VERSION: u8 = 1;
+const SERIALIZATION_VERSION: u8 = 2;
 const MIN_SERIALIZATION_VERSION: u8 = 1;
 
 impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,;
@@ -4481,7 +4612,13 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
 
                self.user_id.write(writer)?;
-               self.config.write(writer)?;
+
+               // Write out the old serialization for the config object. This is read by version-1
+               // deserializers, but we will read the version in the TLV at the end instead.
+               self.config.forwarding_fee_proportional_millionths.write(writer)?;
+               self.config.cltv_expiry_delta.write(writer)?;
+               self.config.announced_channel.write(writer)?;
+               self.config.commit_upfront_shutdown_pubkey.write(writer)?;
 
                self.channel_id.write(writer)?;
                (self.channel_state | ChannelState::PeerDisconnected as u32).write(writer)?;
@@ -4496,7 +4633,12 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                (key_data.0.len() as u32).write(writer)?;
                writer.write_all(&key_data.0[..])?;
 
-               self.shutdown_pubkey.write(writer)?;
+               // Write out the old serialization for shutdown_pubkey for backwards compatibility, if
+               // deserialized from that format.
+               match self.shutdown_scriptpubkey.as_ref().and_then(|script| script.as_legacy_pubkey()) {
+                       Some(shutdown_pubkey) => shutdown_pubkey.write(writer)?,
+                       None => [0u8; PUBLIC_KEY_SIZE].write(writer)?,
+               }
                self.destination_script.write(writer)?;
 
                self.cur_holder_commitment_transaction_number.write(writer)?;
@@ -4640,11 +4782,16 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                self.counterparty_dust_limit_satoshis.write(writer)?;
                self.holder_dust_limit_satoshis.write(writer)?;
                self.counterparty_max_htlc_value_in_flight_msat.write(writer)?;
-               self.counterparty_selected_channel_reserve_satoshis.write(writer)?;
+
+               // Note that this field is ignored by 0.0.99+ as the TLV Optional variant is used instead.
+               self.counterparty_selected_channel_reserve_satoshis.unwrap_or(0).write(writer)?;
+
                self.counterparty_htlc_minimum_msat.write(writer)?;
                self.holder_htlc_minimum_msat.write(writer)?;
                self.counterparty_max_accepted_htlcs.write(writer)?;
-               self.minimum_depth.write(writer)?;
+
+               // Note that this field is ignored by 0.0.99+ as the TLV Optional variant is used instead.
+               self.minimum_depth.unwrap_or(0).write(writer)?;
 
                match &self.counterparty_forwarding_info {
                        Some(info) => {
@@ -4669,7 +4816,26 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
                self.channel_update_status.write(writer)?;
 
-               write_tlv_fields!(writer, {(0, self.announcement_sigs, option)});
+               #[cfg(any(test, feature = "fuzztarget"))]
+               (self.historical_inbound_htlc_fulfills.len() as u64).write(writer)?;
+               #[cfg(any(test, feature = "fuzztarget"))]
+               for htlc in self.historical_inbound_htlc_fulfills.iter() {
+                       htlc.write(writer)?;
+               }
+
+               write_tlv_fields!(writer, {
+                       (0, self.announcement_sigs, option),
+                       // minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
+                       // default value instead of being Option<>al. Thus, to maintain compatibility we write
+                       // them twice, once with their original default values above, and once as an option
+                       // here. On the read side, old versions will simply ignore the odd-type entries here,
+                       // and new versions map the default values to None and allow the TLV entries here to
+                       // override that.
+                       (1, self.minimum_depth, option),
+                       (3, self.counterparty_selected_channel_reserve_satoshis, option),
+                       (5, self.config, required),
+                       (7, self.shutdown_scriptpubkey, option),
+               });
 
                Ok(())
        }
@@ -4679,10 +4845,21 @@ const MAX_ALLOC_SIZE: usize = 64*1024;
 impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                where K::Target: KeysInterface<Signer = Signer> {
        fn read<R : ::std::io::Read>(reader: &mut R, keys_source: &'a K) -> Result<Self, DecodeError> {
-               let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
+               let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
                let user_id = Readable::read(reader)?;
-               let config: ChannelConfig = Readable::read(reader)?;
+
+               let mut config = Some(ChannelConfig::default());
+               if ver == 1 {
+                       // Read the old serialization of the ChannelConfig from version 0.0.98.
+                       config.as_mut().unwrap().forwarding_fee_proportional_millionths = Readable::read(reader)?;
+                       config.as_mut().unwrap().cltv_expiry_delta = Readable::read(reader)?;
+                       config.as_mut().unwrap().announced_channel = Readable::read(reader)?;
+                       config.as_mut().unwrap().commit_upfront_shutdown_pubkey = Readable::read(reader)?;
+               } else {
+                       // Read the 8 bytes of backwards-compatibility ChannelConfig data.
+                       let mut _val: u64 = Readable::read(reader)?;
+               }
 
                let channel_id = Readable::read(reader)?;
                let channel_state = Readable::read(reader)?;
@@ -4701,7 +4878,12 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                }
                let holder_signer = keys_source.read_chan_signer(&keys_data)?;
 
-               let shutdown_pubkey = Readable::read(reader)?;
+               // Read the old serialization for shutdown_pubkey, preferring it for shutdown_scriptpubkey
+               // over the TLV if valid.
+               let mut shutdown_scriptpubkey = match <PublicKey as Readable>::read(reader) {
+                       Ok(pubkey) => Some(ShutdownScript::new_p2wpkh_from_pubkey(pubkey)),
+                       Err(_) => None,
+               };
                let destination_script = Readable::read(reader)?;
 
                let cur_holder_commitment_transaction_number = Readable::read(reader)?;
@@ -4812,11 +4994,26 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                let counterparty_dust_limit_satoshis = Readable::read(reader)?;
                let holder_dust_limit_satoshis = Readable::read(reader)?;
                let counterparty_max_htlc_value_in_flight_msat = Readable::read(reader)?;
-               let counterparty_selected_channel_reserve_satoshis = Readable::read(reader)?;
+               let mut counterparty_selected_channel_reserve_satoshis = None;
+               if ver == 1 {
+                       // Read the old serialization from version 0.0.98.
+                       counterparty_selected_channel_reserve_satoshis = Some(Readable::read(reader)?);
+               } else {
+                       // Read the 8 bytes of backwards-compatibility data.
+                       let _dummy: u64 = Readable::read(reader)?;
+               }
                let counterparty_htlc_minimum_msat = Readable::read(reader)?;
                let holder_htlc_minimum_msat = Readable::read(reader)?;
                let counterparty_max_accepted_htlcs = Readable::read(reader)?;
-               let minimum_depth = Readable::read(reader)?;
+
+               let mut minimum_depth = None;
+               if ver == 1 {
+                       // Read the old serialization from version 0.0.98.
+                       minimum_depth = Some(Readable::read(reader)?);
+               } else {
+                       // Read the 4 bytes of backwards-compatibility data.
+                       let _dummy: u32 = Readable::read(reader)?;
+               }
 
                let counterparty_forwarding_info = match <u8 as Readable>::read(reader)? {
                        0 => None,
@@ -4841,8 +5038,24 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 
                let channel_update_status = Readable::read(reader)?;
 
+               #[cfg(any(test, feature = "fuzztarget"))]
+               let mut historical_inbound_htlc_fulfills = HashSet::new();
+               #[cfg(any(test, feature = "fuzztarget"))]
+               {
+                       let htlc_fulfills_len: u64 = Readable::read(reader)?;
+                       for _ in 0..htlc_fulfills_len {
+                               assert!(historical_inbound_htlc_fulfills.insert(Readable::read(reader)?));
+                       }
+               }
+
                let mut announcement_sigs = None;
-               read_tlv_fields!(reader, {(0, announcement_sigs, option)});
+               read_tlv_fields!(reader, {
+                       (0, announcement_sigs, option),
+                       (1, minimum_depth, option),
+                       (3, counterparty_selected_channel_reserve_satoshis, option),
+                       (5, config, option), // Note that if none is provided we will *not* overwrite the existing one.
+                       (7, shutdown_scriptpubkey, option),
+               });
 
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());
@@ -4850,7 +5063,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                Ok(Channel {
                        user_id,
 
-                       config,
+                       config: config.unwrap(),
                        channel_id,
                        channel_state,
                        secp_ctx,
@@ -4859,7 +5072,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                        latest_monitor_update_id,
 
                        holder_signer,
-                       shutdown_pubkey,
+                       shutdown_scriptpubkey,
                        destination_script,
 
                        cur_holder_commitment_transaction_number,
@@ -4886,9 +5099,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                        feerate_per_kw,
 
                        #[cfg(debug_assertions)]
-                       holder_max_commitment_tx_output: ::std::sync::Mutex::new((0, 0)),
+                       holder_max_commitment_tx_output: Mutex::new((0, 0)),
                        #[cfg(debug_assertions)]
-                       counterparty_max_commitment_tx_output: ::std::sync::Mutex::new((0, 0)),
+                       counterparty_max_commitment_tx_output: Mutex::new((0, 0)),
 
                        last_sent_closing_fee,
 
@@ -4928,6 +5141,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
                        workaround_lnd_bug_4006: None,
+
+                       #[cfg(any(test, feature = "fuzztarget"))]
+                       historical_inbound_htlc_fulfills,
                })
        }
 }
@@ -4944,13 +5160,15 @@ mod tests {
        use bitcoin::hashes::hex::FromHex;
        use hex;
        use ln::{PaymentPreimage, PaymentHash};
-       use ln::channelmanager::{BestBlock, HTLCSource};
+       use ln::channelmanager::HTLCSource;
        use ln::channel::{Channel,InboundHTLCOutput,OutboundHTLCOutput,InboundHTLCState,OutboundHTLCState,HTLCOutputInCommitment,HTLCCandidate,HTLCInitiator,TxCreationKeys};
        use ln::channel::MAX_FUNDING_SATOSHIS;
        use ln::features::InitFeatures;
        use ln::msgs::{ChannelUpdate, DataLossProtect, DecodeError, OptionalField, UnsignedChannelUpdate};
+       use ln::script::ShutdownScript;
        use ln::chan_utils;
        use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, HTLC_SUCCESS_TX_WEIGHT, HTLC_TIMEOUT_TX_WEIGHT};
+       use chain::BestBlock;
        use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
        use chain::keysinterface::{InMemorySigner, KeysInterface, BaseSign};
        use chain::transaction::OutPoint;
@@ -4965,7 +5183,7 @@ mod tests {
        use bitcoin::hashes::sha256::Hash as Sha256;
        use bitcoin::hashes::Hash;
        use bitcoin::hash_types::{Txid, WPubkeyHash};
-       use std::sync::Arc;
+       use sync::Arc;
        use prelude::*;
 
        struct TestFeeEstimator {
@@ -4997,10 +5215,10 @@ mod tests {
                        Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&channel_monitor_claim_key_hash[..]).into_script()
                }
 
-               fn get_shutdown_pubkey(&self) -> PublicKey {
+               fn get_shutdown_scriptpubkey(&self) -> ShutdownScript {
                        let secp_ctx = Secp256k1::signing_only();
                        let channel_close_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap();
-                       PublicKey::from_secret_key(&secp_ctx, &channel_close_key)
+                       ShutdownScript::new_p2wpkh_from_pubkey(PublicKey::from_secret_key(&secp_ctx, &channel_close_key))
                }
 
                fn get_channel_signer(&self, _inbound: bool, _channel_value_satoshis: u64) -> InMemorySigner {
@@ -5028,7 +5246,7 @@ mod tests {
 
                let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let config = UserConfig::default();
-               let node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, 10000000, 100000, 42, &config).unwrap();
+               let node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
 
                // Now change the fee so we can check that the fee in the open_channel message is the
                // same as the old fee.
@@ -5053,7 +5271,7 @@ mod tests {
                // Create Node A's channel pointing to Node B's pubkey
                let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let config = UserConfig::default();
-               let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap();
+               let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
 
                // Create Node B's channel by receiving Node A's open_channel message
                // Make sure A's dust limit is as we expect.
@@ -5120,7 +5338,7 @@ mod tests {
 
                let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let config = UserConfig::default();
-               let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, 10000000, 100000, 42, &config).unwrap();
+               let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
 
                let commitment_tx_fee_0_htlcs = chan.commit_tx_fee_msat(0);
                let commitment_tx_fee_1_htlc = chan.commit_tx_fee_msat(1);
@@ -5169,7 +5387,7 @@ mod tests {
                // Create Node A's channel pointing to Node B's pubkey
                let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let config = UserConfig::default();
-               let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap();
+               let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
 
                // Create Node B's channel by receiving Node A's open_channel message
                let open_channel_msg = node_a_chan.get_open_channel(chain_hash);
@@ -5231,7 +5449,7 @@ mod tests {
                // Create a channel.
                let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let config = UserConfig::default();
-               let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, 10000000, 100000, 42, &config).unwrap();
+               let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
                assert!(node_a_chan.counterparty_forwarding_info.is_none());
                assert_eq!(node_a_chan.holder_htlc_minimum_msat, 1); // the default
                assert!(node_a_chan.counterparty_forwarding_info().is_none());
@@ -5295,8 +5513,9 @@ mod tests {
                let counterparty_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let mut config = UserConfig::default();
                config.channel_options.announced_channel = false;
-               let mut chan = Channel::<InMemorySigner>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test
+               let mut chan = Channel::<InMemorySigner>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, InitFeatures::known(), 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test
                chan.holder_dust_limit_satoshis = 546;
+               chan.counterparty_selected_channel_reserve_satoshis = Some(0); // Filled in in accept_channel
 
                let funding_info = OutPoint{ txid: Txid::from_hex("8984484a580b825b9972d7adb15050b3ab624ccd731946b3eeddb92f4e7ef6be").unwrap(), index: 0 };
 
@@ -5384,7 +5603,9 @@ mod tests {
                                        let remote_signature = Signature::from_der(&hex::decode($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
 
                                        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_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.feerate_per_kw,
+                                               chan.get_counterparty_selected_contest_delay().unwrap(),
+                                               &htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
                                        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();