Rename HTLC `onchain_value_satoshis` to `htlc_value_satoshis`
[rust-lightning] / lightning / src / chain / channelmonitor.rs
index 1699e4827236c89ba6e1dc6fef6b7ff726db6366..7e721f6307160b8a9d7db20d552932b0789e6875 100644 (file)
@@ -20,7 +20,7 @@
 //! security-domain-separated system design, you should consider having multiple paths for
 //! ChannelMonitors to get out of the HSM and onto monitoring devices.
 
-use bitcoin::blockdata::block::{Block, BlockHeader};
+use bitcoin::blockdata::block::BlockHeader;
 use bitcoin::blockdata::transaction::{TxOut,Transaction};
 use bitcoin::blockdata::script::{Script, Builder};
 use bitcoin::blockdata::opcodes;
@@ -29,8 +29,8 @@ use bitcoin::hashes::Hash;
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
 
-use bitcoin::secp256k1::{Secp256k1,Signature};
-use bitcoin::secp256k1::key::{SecretKey,PublicKey};
+use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature};
+use bitcoin::secp256k1::{SecretKey, PublicKey};
 use bitcoin::secp256k1;
 
 use ln::{PaymentHash, PaymentPreimage};
@@ -59,7 +59,7 @@ use sync::Mutex;
 
 /// An update generated by the underlying Channel itself which contains some new information the
 /// ChannelMonitor should be made aware of.
-#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
+#[cfg_attr(any(test, fuzzing, feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
 #[must_use]
 pub struct ChannelMonitorUpdate {
@@ -115,14 +115,6 @@ impl Readable for ChannelMonitorUpdate {
        }
 }
 
-/// General Err type for ChannelMonitor actions. Generally, this implies that the data provided is
-/// inconsistent with the ChannelMonitor being called. eg for ChannelMonitor::update_monitor this
-/// means you tried to update a monitor for a different channel or the ChannelMonitorUpdate was
-/// corrupted.
-/// Contains a developer-readable error message.
-#[derive(Clone, Debug)]
-pub struct MonitorUpdateError(pub &'static str);
-
 /// An event to be processed by the ChannelManager.
 #[derive(Clone, PartialEq)]
 pub enum MonitorEvent {
@@ -174,11 +166,11 @@ pub struct HTLCUpdate {
        pub(crate) payment_hash: PaymentHash,
        pub(crate) payment_preimage: Option<PaymentPreimage>,
        pub(crate) source: HTLCSource,
-       pub(crate) onchain_value_satoshis: Option<u64>,
+       pub(crate) htlc_value_satoshis: Option<u64>,
 }
 impl_writeable_tlv_based!(HTLCUpdate, {
        (0, payment_hash, required),
-       (1, onchain_value_satoshis, option),
+       (1, htlc_value_satoshis, option),
        (2, source, required),
        (4, payment_preimage, option),
 });
@@ -365,10 +357,10 @@ enum OnchainEvent {
        HTLCUpdate {
                source: HTLCSource,
                payment_hash: PaymentHash,
-               onchain_value_satoshis: Option<u64>,
+               htlc_value_satoshis: Option<u64>,
                /// None in the second case, above, ie when there is no relevant output in the commitment
                /// transaction which appeared on chain.
-               input_idx: Option<u32>,
+               commitment_tx_output_idx: Option<u32>,
        },
        MaturingOutput {
                descriptor: SpendableOutputDescriptor,
@@ -389,7 +381,7 @@ enum OnchainEvent {
        ///  * a revoked-state HTLC transaction was broadcasted, which was claimed by the revocation
        ///    signature.
        HTLCSpendConfirmation {
-               input_idx: u32,
+               commitment_tx_output_idx: u32,
                /// If the claim was made by either party with a preimage, this is filled in
                preimage: Option<PaymentPreimage>,
                /// If the claim was made by us on an inbound HTLC against a local commitment transaction,
@@ -431,9 +423,9 @@ impl MaybeReadable for OnchainEventEntry {
 impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
        (0, HTLCUpdate) => {
                (0, source, required),
-               (1, onchain_value_satoshis, option),
+               (1, htlc_value_satoshis, option),
                (2, payment_hash, required),
-               (3, input_idx, option),
+               (3, commitment_tx_output_idx, option),
        },
        (1, MaturingOutput) => {
                (0, descriptor, required),
@@ -442,14 +434,14 @@ impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
                (0, on_local_output_csv, option),
        },
        (5, HTLCSpendConfirmation) => {
-               (0, input_idx, required),
+               (0, commitment_tx_output_idx, required),
                (2, preimage, option),
                (4, on_to_local_output_csv, option),
        },
 
 );
 
-#[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
+#[cfg_attr(any(test, fuzzing, feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
 pub(crate) enum ChannelMonitorUpdateStep {
        LatestHolderCommitmentTXInfo {
@@ -481,6 +473,19 @@ pub(crate) enum ChannelMonitorUpdateStep {
        },
 }
 
+impl ChannelMonitorUpdateStep {
+       fn variant_name(&self) -> &'static str {
+               match self {
+                       ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } => "LatestHolderCommitmentTXInfo",
+                       ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } => "LatestCounterpartyCommitmentTXInfo",
+                       ChannelMonitorUpdateStep::PaymentPreimage { .. } => "PaymentPreimage",
+                       ChannelMonitorUpdateStep::CommitmentSecret { .. } => "CommitmentSecret",
+                       ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed",
+                       ChannelMonitorUpdateStep::ShutdownScript { .. } => "ShutdownScript",
+               }
+       }
+}
+
 impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
        (0, LatestHolderCommitmentTXInfo) => {
                (0, commitment_tx, required),
@@ -563,13 +568,13 @@ pub enum Balance {
 /// An HTLC which has been irrevocably resolved on-chain, and has reached ANTI_REORG_DELAY.
 #[derive(PartialEq)]
 struct IrrevocablyResolvedHTLC {
-       input_idx: u32,
+       commitment_tx_output_idx: u32,
        /// Only set if the HTLC claim was ours using a payment preimage
        payment_preimage: Option<PaymentPreimage>,
 }
 
 impl_writeable_tlv_based!(IrrevocablyResolvedHTLC, {
-       (0, input_idx, required),
+       (0, commitment_tx_output_idx, required),
        (2, payment_preimage, option),
 });
 
@@ -695,6 +700,11 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
        // remote monitor out-of-order with regards to the block view.
        holder_tx_signed: bool,
 
+       // If a spend of the funding output is seen, we set this to true and reject any further
+       // updates. This prevents any further changes in the offchain state no matter the order
+       // of block connection between ChannelMonitors and the ChannelManager.
+       funding_spend_seen: bool,
+
        funding_spend_confirmed: Option<Txid>,
        /// The set of HTLCs which have been either claimed or failed on chain and have reached
        /// the requisite confirmations on the claim/fail transaction (either ANTI_REORG_DELAY or the
@@ -714,9 +724,9 @@ pub(crate) struct ChannelMonitorImpl<Signer: Sign> {
 /// Transaction outputs to watch for on-chain spends.
 pub type TransactionOutputs = (Txid, Vec<(u32, TxOut)>);
 
-#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
-/// Used only in testing and fuzztarget to check serialization roundtrips don't change the
-/// underlying object
+#[cfg(any(test, fuzzing, feature = "_test_utils"))]
+/// Used only in testing and fuzzing to check serialization roundtrips don't change the underlying
+/// object
 impl<Signer: Sign> PartialEq for ChannelMonitor<Signer> {
        fn eq(&self, other: &Self) -> bool {
                let inner = self.inner.lock().unwrap();
@@ -725,9 +735,9 @@ impl<Signer: Sign> PartialEq for ChannelMonitor<Signer> {
        }
 }
 
-#[cfg(any(test, feature = "fuzztarget", feature = "_test_utils"))]
-/// Used only in testing and fuzztarget to check serialization roundtrips don't change the
-/// underlying object
+#[cfg(any(test, fuzzing, feature = "_test_utils"))]
+/// Used only in testing and fuzzing to check serialization roundtrips don't change the underlying
+/// object
 impl<Signer: Sign> PartialEq for ChannelMonitorImpl<Signer> {
        fn eq(&self, other: &Self) -> bool {
                if self.latest_update_id != other.latest_update_id ||
@@ -760,6 +770,7 @@ impl<Signer: Sign> PartialEq for ChannelMonitorImpl<Signer> {
                        self.outputs_to_watch != other.outputs_to_watch ||
                        self.lockdown_from_offchain != other.lockdown_from_offchain ||
                        self.holder_tx_signed != other.holder_tx_signed ||
+                       self.funding_spend_seen != other.funding_spend_seen ||
                        self.funding_spend_confirmed != other.funding_spend_confirmed ||
                        self.htlcs_resolved_on_chain != other.htlcs_resolved_on_chain
                {
@@ -935,6 +946,7 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
                        (1, self.funding_spend_confirmed, option),
                        (3, self.htlcs_resolved_on_chain, vec_type),
                        (5, self.pending_monitor_events, vec_type),
+                       (7, self.funding_spend_seen, required),
                });
 
                Ok(())
@@ -1033,6 +1045,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 
                                lockdown_from_offchain: false,
                                holder_tx_signed: false,
+                               funding_spend_seen: false,
                                funding_spend_confirmed: None,
                                htlcs_resolved_on_chain: Vec::new(),
 
@@ -1044,7 +1057,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
        }
 
        #[cfg(test)]
-       fn provide_secret(&self, idx: u64, secret: [u8; 32]) -> Result<(), MonitorUpdateError> {
+       fn provide_secret(&self, idx: u64, secret: [u8; 32]) -> Result<(), &'static str> {
                self.inner.lock().unwrap().provide_secret(idx, secret)
        }
 
@@ -1066,12 +1079,10 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
 
        #[cfg(test)]
        fn provide_latest_holder_commitment_tx(
-               &self,
-               holder_commitment_tx: HolderCommitmentTransaction,
+               &self, holder_commitment_tx: HolderCommitmentTransaction,
                htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>,
-       ) -> Result<(), MonitorUpdateError> {
-               self.inner.lock().unwrap().provide_latest_holder_commitment_tx(
-                       holder_commitment_tx, htlc_outputs)
+       ) -> Result<(), ()> {
+               self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs).map_err(|_| ())
        }
 
        #[cfg(test)]
@@ -1112,7 +1123,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                broadcaster: &B,
                fee_estimator: &F,
                logger: &L,
-       ) -> Result<(), MonitorUpdateError>
+       ) -> Result<(), ()>
        where
                B::Target: BroadcasterInterface,
                F::Target: FeeEstimator,
@@ -1380,16 +1391,32 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                macro_rules! walk_htlcs {
                        ($holder_commitment: expr, $htlc_iter: expr) => {
                                for htlc in $htlc_iter {
-                                       if let Some(htlc_input_idx) = htlc.transaction_output_index {
-                                               if us.htlcs_resolved_on_chain.iter().any(|v| v.input_idx == htlc_input_idx) {
-                                                       assert!(us.funding_spend_confirmed.is_some());
+                                       if let Some(htlc_commitment_tx_output_idx) = htlc.transaction_output_index {
+                                               if let Some(conf_thresh) = us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
+                                                       if let OnchainEvent::MaturingOutput { descriptor: SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) } = &event.event {
+                                                               if descriptor.outpoint.index as u32 == htlc_commitment_tx_output_idx { Some(event.confirmation_threshold()) } else { None }
+                                                       } else { None }
+                                               }) {
+                                                       debug_assert!($holder_commitment);
+                                                       res.push(Balance::ClaimableAwaitingConfirmations {
+                                                               claimable_amount_satoshis: htlc.amount_msat / 1000,
+                                                               confirmation_height: conf_thresh,
+                                                       });
+                                               } else if us.htlcs_resolved_on_chain.iter().any(|v| v.commitment_tx_output_idx == htlc_commitment_tx_output_idx) {
+                                                       // Funding transaction spends should be fully confirmed by the time any
+                                                       // HTLC transactions are resolved, unless we're talking about a holder
+                                                       // commitment tx, whose resolution is delayed until the CSV timeout is
+                                                       // reached, even though HTLCs may be resolved after only
+                                                       // ANTI_REORG_DELAY confirmations.
+                                                       debug_assert!($holder_commitment || us.funding_spend_confirmed.is_some());
                                                } else if htlc.offered == $holder_commitment {
                                                        // If the payment was outbound, check if there's an HTLCUpdate
                                                        // indicating we have spent this HTLC with a timeout, claiming it back
                                                        // and awaiting confirmations on it.
                                                        let htlc_update_pending = us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
-                                                               if let OnchainEvent::HTLCUpdate { input_idx: Some(input_idx), .. } = event.event {
-                                                                       if input_idx == htlc_input_idx { Some(event.confirmation_threshold()) } else { None }
+                                                               if let OnchainEvent::HTLCUpdate { commitment_tx_output_idx: Some(commitment_tx_output_idx), .. } = event.event {
+                                                                       if commitment_tx_output_idx == htlc_commitment_tx_output_idx {
+                                                                               Some(event.confirmation_threshold()) } else { None }
                                                                } else { None }
                                                        });
                                                        if let Some(conf_thresh) = htlc_update_pending {
@@ -1410,8 +1437,8 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                                                        // preimage, we lost funds to our counterparty! We will then continue
                                                        // to show it as ContentiousClaimable until ANTI_REORG_DELAY.
                                                        let htlc_spend_pending = us.onchain_events_awaiting_threshold_conf.iter().find_map(|event| {
-                                                               if let OnchainEvent::HTLCSpendConfirmation { input_idx, preimage, .. } = event.event {
-                                                                       if input_idx == htlc_input_idx {
+                                                               if let OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, preimage, .. } = event.event {
+                                                                       if commitment_tx_output_idx == htlc_commitment_tx_output_idx {
                                                                                Some((event.confirmation_threshold(), preimage.is_some()))
                                                                        } else { None }
                                                                } else { None }
@@ -1520,7 +1547,7 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                macro_rules! walk_htlcs {
                        ($holder_commitment: expr, $htlc_iter: expr) => {
                                for (htlc, source) in $htlc_iter {
-                                       if us.htlcs_resolved_on_chain.iter().any(|v| Some(v.input_idx) == htlc.transaction_output_index) {
+                                       if us.htlcs_resolved_on_chain.iter().any(|v| Some(v.commitment_tx_output_idx) == htlc.transaction_output_index) {
                                                // We should assert that funding_spend_confirmed is_some() here, but we
                                                // have some unit tests which violate HTLC transaction CSVs entirely and
                                                // would fail.
@@ -1531,17 +1558,17 @@ impl<Signer: Sign> ChannelMonitor<Signer> {
                                                // indicating we have spent this HTLC with a timeout, claiming it back
                                                // and awaiting confirmations on it.
                                                let htlc_update_confd = us.onchain_events_awaiting_threshold_conf.iter().any(|event| {
-                                                       if let OnchainEvent::HTLCUpdate { input_idx: Some(input_idx), .. } = event.event {
+                                                       if let OnchainEvent::HTLCUpdate { commitment_tx_output_idx: Some(commitment_tx_output_idx), .. } = event.event {
                                                                // If the HTLC was timed out, we wait for ANTI_REORG_DELAY blocks
                                                                // before considering it "no longer pending" - this matches when we
                                                                // provide the ChannelManager an HTLC failure event.
-                                                               Some(input_idx) == htlc.transaction_output_index &&
+                                                               Some(commitment_tx_output_idx) == htlc.transaction_output_index &&
                                                                        us.best_block.height() >= event.height + ANTI_REORG_DELAY - 1
-                                                       } else if let OnchainEvent::HTLCSpendConfirmation { input_idx, .. } = event.event {
+                                                       } else if let OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, .. } = event.event {
                                                                // If the HTLC was fulfilled with a preimage, we consider the HTLC
                                                                // immediately non-pending, matching when we provide ChannelManager
                                                                // the preimage.
-                                                               Some(input_idx) == htlc.transaction_output_index
+                                                               Some(commitment_tx_output_idx) == htlc.transaction_output_index
                                                        } else { false }
                                                });
                                                if !htlc_update_confd {
@@ -1662,8 +1689,8 @@ macro_rules! fail_unbroadcast_htlcs {
                                                                event: OnchainEvent::HTLCUpdate {
                                                                        source: (**source).clone(),
                                                                        payment_hash: htlc.payment_hash.clone(),
-                                                                       onchain_value_satoshis: Some(htlc.amount_msat / 1000),
-                                                                       input_idx: None,
+                                                                       htlc_value_satoshis: Some(htlc.amount_msat / 1000),
+                                                                       commitment_tx_output_idx: None,
                                                                },
                                                        };
                                                        log_trace!($logger, "Failing HTLC with payment_hash {} from {} counterparty commitment tx due to broadcast of {} commitment transaction, waiting for confirmation (at height {})",
@@ -1687,9 +1714,9 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither
        /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen
        /// counterparty commitment transaction's secret, they are de facto pruned (we can use revocation key).
-       fn provide_secret(&mut self, idx: u64, secret: [u8; 32]) -> Result<(), MonitorUpdateError> {
+       fn provide_secret(&mut self, idx: u64, secret: [u8; 32]) -> Result<(), &'static str> {
                if let Err(()) = self.commitment_secrets.provide_secret(idx, secret) {
-                       return Err(MonitorUpdateError("Previous secret did not match new one"));
+                       return Err("Previous secret did not match new one");
                }
 
                // Prune HTLCs from the previous counterparty commitment tx so we don't generate failure/fulfill
@@ -1781,7 +1808,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        /// 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(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
+       fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), &'static str> {
                // block for Rust 1.34 compat
                let mut new_holder_commitment_tx = {
                        let trusted_tx = holder_commitment_tx.trust();
@@ -1804,7 +1831,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                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 {
-                       return Err(MonitorUpdateError("Latest holder commitment signed has already been signed, update is rejected"));
+                       return Err("Latest holder commitment signed has already been signed, update is rejected");
                }
                Ok(())
        }
@@ -1868,30 +1895,40 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
        }
 
-       pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), MonitorUpdateError>
+       pub fn update_monitor<B: Deref, F: Deref, L: Deref>(&mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &L) -> Result<(), ()>
        where B::Target: BroadcasterInterface,
                    F::Target: FeeEstimator,
                    L::Target: Logger,
        {
+               log_info!(logger, "Applying update to monitor {}, bringing update_id from {} to {} with {} changes.",
+                       log_funding_info!(self), self.latest_update_id, updates.update_id, updates.updates.len());
                // ChannelMonitor updates may be applied after force close if we receive a
                // preimage for a broadcasted commitment transaction HTLC output that we'd
                // like to claim on-chain. If this is the case, we no longer have guaranteed
                // access to the monitor's update ID, so we use a sentinel value instead.
                if updates.update_id == CLOSED_CHANNEL_UPDATE_ID {
+                       assert_eq!(updates.updates.len(), 1);
                        match updates.updates[0] {
                                ChannelMonitorUpdateStep::PaymentPreimage { .. } => {},
-                               _ => panic!("Attempted to apply post-force-close ChannelMonitorUpdate that wasn't providing a payment preimage"),
+                               _ => {
+                                       log_error!(logger, "Attempted to apply post-force-close ChannelMonitorUpdate of type {}", updates.updates[0].variant_name());
+                                       panic!("Attempted to apply post-force-close ChannelMonitorUpdate that wasn't providing a payment preimage");
+                               },
                        }
-                       assert_eq!(updates.updates.len(), 1);
                } else if self.latest_update_id + 1 != updates.update_id {
                        panic!("Attempted to apply ChannelMonitorUpdates out of order, check the update_id before passing an update to update_monitor!");
                }
+               let mut ret = Ok(());
                for update in updates.updates.iter() {
                        match update {
                                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(commitment_tx.clone(), htlc_outputs.clone())?
+                                       if let Err(e) = self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone()) {
+                                               log_error!(logger, "Providing latest holder commitment transaction failed/was refused:");
+                                               log_error!(logger, "    {}", e);
+                                               ret = Err(());
+                                       }
                                }
                                ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_revocation_point } => {
                                        log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
@@ -1903,7 +1940,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                },
                                ChannelMonitorUpdateStep::CommitmentSecret { idx, secret } => {
                                        log_trace!(logger, "Updating ChannelMonitor with commitment secret");
-                                       self.provide_secret(*idx, *secret)?
+                                       if let Err(e) = self.provide_secret(*idx, *secret) {
+                                               log_error!(logger, "Providing latest counterparty commitment secret failed/was refused:");
+                                               log_error!(logger, "    {}", e);
+                                               ret = Err(());
+                                       }
                                },
                                ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => {
                                        log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast);
@@ -1928,7 +1969,11 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                        }
                }
                self.latest_update_id = updates.update_id;
-               Ok(())
+
+               if ret.is_ok() && self.funding_spend_seen {
+                       log_error!(logger, "Refusing Channel Monitor Update as counterparty attempted to update commitment after funding was spent");
+                       Err(())
+               } else { ret }
        }
 
        pub fn get_latest_update_id(&self) -> u64 {
@@ -2030,7 +2075,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                                tx.output[transaction_output_index as usize].value != htlc.amount_msat / 1000 {
                                                        return (claimable_outpoints, (commitment_txid, watch_outputs)); // Corrupted per_commitment_data, fuck this user
                                                }
-                                               let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone());
+                                               let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), self.onchain_tx_handler.channel_transaction_parameters.opt_anchors.is_some());
                                                let justice_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp), htlc.cltv_expiry, true, height);
                                                claimable_outpoints.push(justice_package);
                                        }
@@ -2357,6 +2402,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                        let mut balance_spendable_csv = None;
                                        log_info!(logger, "Channel {} closed by funding output spend in txid {}.",
                                                log_bytes!(self.funding_info.0.to_channel_id()), tx.txid());
+                                       self.funding_spend_seen = true;
                                        if (tx.input[0].sequence >> 8*3) as u8 == 0x80 && (tx.lock_time >> 8*3) as u8 == 0x20 {
                                                let (mut new_outpoints, new_outputs) = self.check_spend_counterparty_transaction(&tx, height, &logger);
                                                if !new_outputs.1.is_empty() {
@@ -2477,7 +2523,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                // Produce actionable events from on-chain events having reached their threshold.
                for entry in onchain_events_reaching_threshold_conf.drain(..) {
                        match entry.event {
-                               OnchainEvent::HTLCUpdate { ref source, payment_hash, onchain_value_satoshis, input_idx } => {
+                               OnchainEvent::HTLCUpdate { ref source, payment_hash, htlc_value_satoshis, commitment_tx_output_idx } => {
                                        // Check for duplicate HTLC resolutions.
                                        #[cfg(debug_assertions)]
                                        {
@@ -2499,10 +2545,10 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                payment_hash,
                                                payment_preimage: None,
                                                source: source.clone(),
-                                               onchain_value_satoshis,
+                                               htlc_value_satoshis,
                                        }));
-                                       if let Some(idx) = input_idx {
-                                               self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC { input_idx: idx, payment_preimage: None });
+                                       if let Some(idx) = commitment_tx_output_idx {
+                                               self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC { commitment_tx_output_idx: idx, payment_preimage: None });
                                        }
                                },
                                OnchainEvent::MaturingOutput { descriptor } => {
@@ -2511,8 +2557,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                outputs: vec![descriptor]
                                        });
                                },
-                               OnchainEvent::HTLCSpendConfirmation { input_idx, preimage, .. } => {
-                                       self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC { input_idx, payment_preimage: preimage });
+                               OnchainEvent::HTLCSpendConfirmation { commitment_tx_output_idx, preimage, .. } => {
+                                       self.htlcs_resolved_on_chain.push(IrrevocablyResolvedHTLC { commitment_tx_output_idx, payment_preimage: preimage });
                                },
                                OnchainEvent::FundingSpendConfirmation { .. } => {
                                        self.funding_spend_confirmed = Some(entry.txid);
@@ -2608,7 +2654,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                        // appears to be spending the correct type (ie that the match would
                                                        // actually succeed in BIP 158/159-style filters).
                                                        if _script_pubkey.is_v0_p2wsh() {
-                                                               assert_eq!(&bitcoin::Address::p2wsh(&Script::from(input.witness.last().unwrap().clone()), bitcoin::Network::Bitcoin).script_pubkey(), _script_pubkey);
+                                                               assert_eq!(&bitcoin::Address::p2wsh(&Script::from(input.witness.last().unwrap().to_vec()), bitcoin::Network::Bitcoin).script_pubkey(), _script_pubkey);
                                                        } else if _script_pubkey.is_v0_p2wpkh() {
                                                                assert_eq!(&bitcoin::Address::p2wpkh(&bitcoin::PublicKey::from_slice(&input.witness.last().unwrap()).unwrap(), bitcoin::Network::Bitcoin).unwrap().script_pubkey(), _script_pubkey);
                                                        } else { panic!(); }
@@ -2691,20 +2737,23 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
        fn is_resolving_htlc_output<L: Deref>(&mut self, tx: &Transaction, height: u32, logger: &L) where L::Target: Logger {
                'outer_loop: for input in &tx.input {
                        let mut payment_data = None;
-                       let revocation_sig_claim = (input.witness.len() == 3 && HTLCType::scriptlen_to_htlctype(input.witness[2].len()) == Some(HTLCType::OfferedHTLC) && input.witness[1].len() == 33)
-                               || (input.witness.len() == 3 && HTLCType::scriptlen_to_htlctype(input.witness[2].len()) == Some(HTLCType::AcceptedHTLC) && input.witness[1].len() == 33);
-                       let accepted_preimage_claim = input.witness.len() == 5 && HTLCType::scriptlen_to_htlctype(input.witness[4].len()) == Some(HTLCType::AcceptedHTLC);
+                       let witness_items = input.witness.len();
+                       let htlctype = input.witness.last().map(|w| w.len()).and_then(HTLCType::scriptlen_to_htlctype);
+                       let prev_last_witness_len = input.witness.second_to_last().map(|w| w.len()).unwrap_or(0);
+                       let revocation_sig_claim = (witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && prev_last_witness_len == 33)
+                               || (witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && prev_last_witness_len == 33);
+                       let accepted_preimage_claim = witness_items == 5 && htlctype == Some(HTLCType::AcceptedHTLC);
                        #[cfg(not(fuzzing))]
-                       let accepted_timeout_claim = input.witness.len() == 3 && HTLCType::scriptlen_to_htlctype(input.witness[2].len()) == Some(HTLCType::AcceptedHTLC) && !revocation_sig_claim;
-                       let offered_preimage_claim = input.witness.len() == 3 && HTLCType::scriptlen_to_htlctype(input.witness[2].len()) == Some(HTLCType::OfferedHTLC) && !revocation_sig_claim;
+                       let accepted_timeout_claim = witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && !revocation_sig_claim;
+                       let offered_preimage_claim = witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && !revocation_sig_claim;
                        #[cfg(not(fuzzing))]
-                       let offered_timeout_claim = input.witness.len() == 5 && HTLCType::scriptlen_to_htlctype(input.witness[4].len()) == Some(HTLCType::OfferedHTLC);
+                       let offered_timeout_claim = witness_items == 5 && htlctype == Some(HTLCType::OfferedHTLC);
 
                        let mut payment_preimage = PaymentPreimage([0; 32]);
                        if accepted_preimage_claim {
-                               payment_preimage.0.copy_from_slice(&input.witness[3]);
+                               payment_preimage.0.copy_from_slice(input.witness.second_to_last().unwrap());
                        } else if offered_preimage_claim {
-                               payment_preimage.0.copy_from_slice(&input.witness[1]);
+                               payment_preimage.0.copy_from_slice(input.witness.second_to_last().unwrap());
                        }
 
                        macro_rules! log_claim {
@@ -2778,7 +2827,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                                        self.onchain_events_awaiting_threshold_conf.push(OnchainEventEntry {
                                                                                txid: tx.txid(), height,
                                                                                event: OnchainEvent::HTLCSpendConfirmation {
-                                                                                       input_idx: input.previous_output.vout,
+                                                                                       commitment_tx_output_idx: input.previous_output.vout,
                                                                                        preimage: if accepted_preimage_claim || offered_preimage_claim {
                                                                                                Some(payment_preimage) } else { None },
                                                                                        // If this is a payment to us (!outbound_htlc, above),
@@ -2829,7 +2878,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                        txid: tx.txid(),
                                                        height,
                                                        event: OnchainEvent::HTLCSpendConfirmation {
-                                                               input_idx: input.previous_output.vout,
+                                                               commitment_tx_output_idx: input.previous_output.vout,
                                                                preimage: Some(payment_preimage),
                                                                on_to_local_output_csv: None,
                                                        },
@@ -2838,7 +2887,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                        source,
                                                        payment_preimage: Some(payment_preimage),
                                                        payment_hash,
-                                                       onchain_value_satoshis: Some(amount_msat / 1000),
+                                                       htlc_value_satoshis: Some(amount_msat / 1000),
                                                }));
                                        }
                                } else if offered_preimage_claim {
@@ -2850,7 +2899,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                        txid: tx.txid(),
                                                        height,
                                                        event: OnchainEvent::HTLCSpendConfirmation {
-                                                               input_idx: input.previous_output.vout,
+                                                               commitment_tx_output_idx: input.previous_output.vout,
                                                                preimage: Some(payment_preimage),
                                                                on_to_local_output_csv: None,
                                                        },
@@ -2859,7 +2908,7 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                        source,
                                                        payment_preimage: Some(payment_preimage),
                                                        payment_hash,
-                                                       onchain_value_satoshis: Some(amount_msat / 1000),
+                                                       htlc_value_satoshis: Some(amount_msat / 1000),
                                                }));
                                        }
                                } else {
@@ -2877,8 +2926,8 @@ impl<Signer: Sign> ChannelMonitorImpl<Signer> {
                                                height,
                                                event: OnchainEvent::HTLCUpdate {
                                                        source, payment_hash,
-                                                       onchain_value_satoshis: Some(amount_msat / 1000),
-                                                       input_idx: Some(input.previous_output.vout),
+                                                       htlc_value_satoshis: Some(amount_msat / 1000),
+                                                       commitment_tx_output_idx: Some(input.previous_output.vout),
                                                },
                                        };
                                        log_info!(logger, "Failing HTLC with payment_hash {} timeout by a spend tx, waiting for confirmation (at height {})", log_bytes!(payment_hash.0), entry.confirmation_threshold());
@@ -2962,9 +3011,8 @@ where
        F::Target: FeeEstimator,
        L::Target: Logger,
 {
-       fn block_connected(&self, block: &Block, height: u32) {
-               let txdata: Vec<_> = block.txdata.iter().enumerate().collect();
-               self.0.block_connected(&block.header, &txdata, height, &*self.1, &*self.2, &*self.3);
+       fn filtered_block_connected(&self, header: &BlockHeader, txdata: &TransactionData, height: u32) {
+               self.0.block_connected(header, txdata, height, &*self.1, &*self.2, &*self.3);
        }
 
        fn block_disconnected(&self, header: &BlockHeader, height: u32) {
@@ -3206,10 +3254,12 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 
                let mut funding_spend_confirmed = None;
                let mut htlcs_resolved_on_chain = Some(Vec::new());
+               let mut funding_spend_seen = Some(false);
                read_tlv_fields!(reader, {
                        (1, funding_spend_confirmed, option),
                        (3, htlcs_resolved_on_chain, vec_type),
                        (5, pending_monitor_events, vec_type),
+                       (7, funding_spend_seen, option),
                });
 
                let mut secp_ctx = Secp256k1::new();
@@ -3259,6 +3309,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 
                                lockdown_from_offchain,
                                holder_tx_signed,
+                               funding_spend_seen: funding_spend_seen.unwrap(),
                                funding_spend_confirmed,
                                htlcs_resolved_on_chain: htlcs_resolved_on_chain.unwrap(),
 
@@ -3272,32 +3323,138 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
 
 #[cfg(test)]
 mod tests {
+       use bitcoin::blockdata::block::BlockHeader;
        use bitcoin::blockdata::script::{Script, Builder};
        use bitcoin::blockdata::opcodes;
-       use bitcoin::blockdata::transaction::{Transaction, TxIn, TxOut, SigHashType};
+       use bitcoin::blockdata::transaction::{Transaction, TxIn, TxOut, EcdsaSighashType};
        use bitcoin::blockdata::transaction::OutPoint as BitcoinOutPoint;
-       use bitcoin::util::bip143;
+       use bitcoin::util::sighash;
        use bitcoin::hashes::Hash;
        use bitcoin::hashes::sha256::Hash as Sha256;
        use bitcoin::hashes::hex::FromHex;
-       use bitcoin::hash_types::Txid;
+       use bitcoin::hash_types::{BlockHash, Txid};
        use bitcoin::network::constants::Network;
+       use bitcoin::secp256k1::{SecretKey,PublicKey};
+       use bitcoin::secp256k1::Secp256k1;
+
        use hex;
-       use chain::BestBlock;
+
+       use super::ChannelMonitorUpdateStep;
+       use ::{check_added_monitors, check_closed_broadcast, check_closed_event, check_spends, get_local_commitment_txn, get_monitor, get_route_and_payment_hash, unwrap_send_err};
+       use chain::{BestBlock, Confirm};
        use chain::channelmonitor::ChannelMonitor;
-       use chain::package::{WEIGHT_OFFERED_HTLC, WEIGHT_RECEIVED_HTLC, WEIGHT_REVOKED_OFFERED_HTLC, WEIGHT_REVOKED_RECEIVED_HTLC, WEIGHT_REVOKED_OUTPUT};
+       use chain::package::{weight_offered_htlc, weight_received_htlc, weight_revoked_offered_htlc, weight_revoked_received_htlc, WEIGHT_REVOKED_OUTPUT};
        use chain::transaction::OutPoint;
+       use chain::keysinterface::InMemorySigner;
        use ln::{PaymentPreimage, PaymentHash};
        use ln::chan_utils;
        use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
+       use ln::channelmanager::PaymentSendFailure;
+       use ln::features::InitFeatures;
+       use ln::functional_test_utils::*;
        use ln::script::ShutdownScript;
+       use util::errors::APIError;
+       use util::events::{ClosureReason, MessageSendEventsProvider};
        use util::test_utils::{TestLogger, TestBroadcaster, TestFeeEstimator};
-       use bitcoin::secp256k1::key::{SecretKey,PublicKey};
-       use bitcoin::secp256k1::Secp256k1;
+       use util::ser::{ReadableArgs, Writeable};
        use sync::{Arc, Mutex};
-       use chain::keysinterface::InMemorySigner;
+       use io;
+       use bitcoin::Witness;
        use prelude::*;
 
+       fn do_test_funding_spend_refuses_updates(use_local_txn: bool) {
+               // Previously, monitor updates were allowed freely even after a funding-spend transaction
+               // confirmed. This would allow a race condition where we could receive a payment (including
+               // the counterparty revoking their broadcasted state!) and accept it without recourse as
+               // long as the ChannelMonitor receives the block first, the full commitment update dance
+               // occurs after the block is connected, and before the ChannelManager receives the block.
+               // Obviously this is an incredibly contrived race given the counterparty would be risking
+               // their full channel balance for it, but its worth fixing nonetheless as it makes the
+               // potential ChannelMonitor states simpler to reason about.
+               //
+               // This test checks said behavior, as well as ensuring a ChannelMonitorUpdate with multiple
+               // updates is handled correctly in such conditions.
+               let chanmon_cfgs = create_chanmon_cfgs(3);
+               let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+               let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+               let nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+               let channel = create_announced_chan_between_nodes(
+                       &nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+               create_announced_chan_between_nodes(
+                       &nodes, 1, 2, InitFeatures::known(), InitFeatures::known());
+
+               // Rebalance somewhat
+               send_payment(&nodes[0], &[&nodes[1]], 10_000_000);
+
+               // First route two payments for testing at the end
+               let payment_preimage_1 = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000).0;
+               let payment_preimage_2 = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000).0;
+
+               let local_txn = get_local_commitment_txn!(nodes[1], channel.2);
+               assert_eq!(local_txn.len(), 1);
+               let remote_txn = get_local_commitment_txn!(nodes[0], channel.2);
+               assert_eq!(remote_txn.len(), 3); // Commitment and two HTLC-Timeouts
+               check_spends!(remote_txn[1], remote_txn[0]);
+               check_spends!(remote_txn[2], remote_txn[0]);
+               let broadcast_tx = if use_local_txn { &local_txn[0] } else { &remote_txn[0] };
+
+               // Connect a commitment transaction, but only to the ChainMonitor/ChannelMonitor. The
+               // channel is now closed, but the ChannelManager doesn't know that yet.
+               let new_header = BlockHeader {
+                       version: 2, time: 0, bits: 0, nonce: 0,
+                       prev_blockhash: nodes[0].best_block_info().0,
+                       merkle_root: Default::default() };
+               let conf_height = nodes[0].best_block_info().1 + 1;
+               nodes[1].chain_monitor.chain_monitor.transactions_confirmed(&new_header,
+                       &[(0, broadcast_tx)], conf_height);
+
+               let (_, pre_update_monitor) = <(BlockHash, ChannelMonitor<InMemorySigner>)>::read(
+                                               &mut io::Cursor::new(&get_monitor!(nodes[1], channel.2).encode()),
+                                               &nodes[1].keys_manager.backing).unwrap();
+
+               // If the ChannelManager tries to update the channel, however, the ChainMonitor will pass
+               // the update through to the ChannelMonitor which will refuse it (as the channel is closed).
+               let (route, payment_hash, _, payment_secret) = get_route_and_payment_hash!(nodes[1], nodes[0], 100_000);
+               unwrap_send_err!(nodes[1].node.send_payment(&route, payment_hash, &Some(payment_secret)),
+                       true, APIError::ChannelUnavailable { ref err },
+                       assert!(err.contains("ChannelMonitor storage failure")));
+               check_added_monitors!(nodes[1], 2); // After the failure we generate a close-channel monitor update
+               check_closed_broadcast!(nodes[1], true);
+               check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "ChannelMonitor storage failure".to_string() });
+
+               // Build a new ChannelMonitorUpdate which contains both the failing commitment tx update
+               // and provides the claim preimages for the two pending HTLCs. The first update generates
+               // an error, but the point of this test is to ensure the later updates are still applied.
+               let monitor_updates = nodes[1].chain_monitor.monitor_updates.lock().unwrap();
+               let mut replay_update = monitor_updates.get(&channel.2).unwrap().iter().rev().skip(1).next().unwrap().clone();
+               assert_eq!(replay_update.updates.len(), 1);
+               if let ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } = replay_update.updates[0] {
+               } else { panic!(); }
+               replay_update.updates.push(ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage: payment_preimage_1 });
+               replay_update.updates.push(ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage: payment_preimage_2 });
+
+               let broadcaster = TestBroadcaster::new(Arc::clone(&nodes[1].blocks));
+               assert!(
+                       pre_update_monitor.update_monitor(&replay_update, &&broadcaster, &&chanmon_cfgs[1].fee_estimator, &nodes[1].logger)
+                       .is_err());
+               // Even though we error'd on the first update, we should still have generated an HTLC claim
+               // transaction
+               let txn_broadcasted = broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
+               assert!(txn_broadcasted.len() >= 2);
+               let htlc_txn = txn_broadcasted.iter().filter(|tx| {
+                       assert_eq!(tx.input.len(), 1);
+                       tx.input[0].previous_output.txid == broadcast_tx.txid()
+               }).collect::<Vec<_>>();
+               assert_eq!(htlc_txn.len(), 2);
+               check_spends!(htlc_txn[0], broadcast_tx);
+               check_spends!(htlc_txn[1], broadcast_tx);
+       }
+       #[test]
+       fn test_funding_spend_refuses_updates() {
+               do_test_funding_spend_refuses_updates(true);
+               do_test_funding_spend_refuses_updates(false);
+       }
+
        #[test]
        fn test_prune_preimages() {
                let secp_ctx = Secp256k1::new();
@@ -3359,6 +3516,7 @@ mod tests {
                        SecretKey::from_slice(&[41; 32]).unwrap(),
                        SecretKey::from_slice(&[41; 32]).unwrap(),
                        SecretKey::from_slice(&[41; 32]).unwrap(),
+                       SecretKey::from_slice(&[41; 32]).unwrap(),
                        [41; 32],
                        0,
                        [0; 32]
@@ -3444,36 +3602,38 @@ mod tests {
                let secp_ctx = Secp256k1::new();
                let privkey = SecretKey::from_slice(&hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap();
                let pubkey = PublicKey::from_secret_key(&secp_ctx, &privkey);
-               let mut sum_actual_sigs = 0;
 
                macro_rules! sign_input {
                        ($sighash_parts: expr, $idx: expr, $amount: expr, $weight: expr, $sum_actual_sigs: expr, $opt_anchors: expr) => {
                                let htlc = HTLCOutputInCommitment {
-                                       offered: if *$weight == WEIGHT_REVOKED_OFFERED_HTLC || *$weight == WEIGHT_OFFERED_HTLC { true } else { false },
+                                       offered: if *$weight == weight_revoked_offered_htlc($opt_anchors) || *$weight == weight_offered_htlc($opt_anchors) { true } else { false },
                                        amount_msat: 0,
                                        cltv_expiry: 2 << 16,
                                        payment_hash: PaymentHash([1; 32]),
                                        transaction_output_index: Some($idx as u32),
                                };
                                let redeem_script = if *$weight == WEIGHT_REVOKED_OUTPUT { chan_utils::get_revokeable_redeemscript(&pubkey, 256, &pubkey) } else { chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, $opt_anchors, &pubkey, &pubkey, &pubkey) };
-                               let sighash = hash_to_message!(&$sighash_parts.signature_hash($idx, &redeem_script, $amount, SigHashType::All)[..]);
-                               let sig = secp_ctx.sign(&sighash, &privkey);
-                               $sighash_parts.access_witness($idx).push(sig.serialize_der().to_vec());
-                               $sighash_parts.access_witness($idx)[0].push(SigHashType::All as u8);
-                               sum_actual_sigs += $sighash_parts.access_witness($idx)[0].len();
+                               let sighash = hash_to_message!(&$sighash_parts.segwit_signature_hash($idx, &redeem_script, $amount, EcdsaSighashType::All).unwrap()[..]);
+                               let sig = secp_ctx.sign_ecdsa(&sighash, &privkey);
+                               let mut ser_sig = sig.serialize_der().to_vec();
+                               ser_sig.push(EcdsaSighashType::All as u8);
+                               $sum_actual_sigs += ser_sig.len();
+                               let witness = $sighash_parts.witness_mut($idx).unwrap();
+                               witness.push(ser_sig);
                                if *$weight == WEIGHT_REVOKED_OUTPUT {
-                                       $sighash_parts.access_witness($idx).push(vec!(1));
-                               } else if *$weight == WEIGHT_REVOKED_OFFERED_HTLC || *$weight == WEIGHT_REVOKED_RECEIVED_HTLC {
-                                       $sighash_parts.access_witness($idx).push(pubkey.clone().serialize().to_vec());
-                               } else if *$weight == WEIGHT_RECEIVED_HTLC {
-                                       $sighash_parts.access_witness($idx).push(vec![0]);
+                                       witness.push(vec!(1));
+                               } else if *$weight == weight_revoked_offered_htlc($opt_anchors) || *$weight == weight_revoked_received_htlc($opt_anchors) {
+                                       witness.push(pubkey.clone().serialize().to_vec());
+                               } else if *$weight == weight_received_htlc($opt_anchors) {
+                                       witness.push(vec![0]);
                                } else {
-                                       $sighash_parts.access_witness($idx).push(PaymentPreimage([1; 32]).0.to_vec());
+                                       witness.push(PaymentPreimage([1; 32]).0.to_vec());
                                }
-                               $sighash_parts.access_witness($idx).push(redeem_script.into_bytes());
-                               println!("witness[0] {}", $sighash_parts.access_witness($idx)[0].len());
-                               println!("witness[1] {}", $sighash_parts.access_witness($idx)[1].len());
-                               println!("witness[2] {}", $sighash_parts.access_witness($idx)[2].len());
+                               witness.push(redeem_script.into_bytes());
+                               let witness = witness.to_vec();
+                               println!("witness[0] {}", witness[0].len());
+                               println!("witness[1] {}", witness[1].len());
+                               println!("witness[2] {}", witness[2].len());
                        }
                }
 
@@ -3481,83 +3641,98 @@ mod tests {
                let txid = Txid::from_hex("56944c5d3f98413ef45cf54545538103cc9f298e0575820ad3591376e2e0f65d").unwrap();
 
                // Justice tx with 1 to_holder, 2 revoked offered HTLCs, 1 revoked received HTLCs
-               let mut claim_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };
-               for i in 0..4 {
-                       claim_tx.input.push(TxIn {
-                               previous_output: BitcoinOutPoint {
-                                       txid,
-                                       vout: i,
-                               },
-                               script_sig: Script::new(),
-                               sequence: 0xfffffffd,
-                               witness: Vec::new(),
+               for &opt_anchors in [false, true].iter() {
+                       let mut claim_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };
+                       let mut sum_actual_sigs = 0;
+                       for i in 0..4 {
+                               claim_tx.input.push(TxIn {
+                                       previous_output: BitcoinOutPoint {
+                                               txid,
+                                               vout: i,
+                                       },
+                                       script_sig: Script::new(),
+                                       sequence: 0xfffffffd,
+                                       witness: Witness::new(),
+                               });
+                       }
+                       claim_tx.output.push(TxOut {
+                               script_pubkey: script_pubkey.clone(),
+                               value: 0,
                        });
-               }
-               claim_tx.output.push(TxOut {
-                       script_pubkey: script_pubkey.clone(),
-                       value: 0,
-               });
-               let base_weight = claim_tx.get_weight();
-               let inputs_weight = vec![WEIGHT_REVOKED_OUTPUT, WEIGHT_REVOKED_OFFERED_HTLC, WEIGHT_REVOKED_OFFERED_HTLC, WEIGHT_REVOKED_RECEIVED_HTLC];
-               let mut inputs_total_weight = 2; // count segwit flags
-               {
-                       let mut sighash_parts = bip143::SigHashCache::new(&mut claim_tx);
-                       for (idx, inp) in inputs_weight.iter().enumerate() {
-                               sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, false);
-                               inputs_total_weight += inp;
+                       let base_weight = claim_tx.weight();
+                       let inputs_weight = vec![WEIGHT_REVOKED_OUTPUT, weight_revoked_offered_htlc(opt_anchors), weight_revoked_offered_htlc(opt_anchors), weight_revoked_received_htlc(opt_anchors)];
+                       let mut inputs_total_weight = 2; // count segwit flags
+                       {
+                               let mut sighash_parts = sighash::SighashCache::new(&mut claim_tx);
+                               for (idx, inp) in inputs_weight.iter().enumerate() {
+                                       sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, opt_anchors);
+                                       inputs_total_weight += inp;
+                               }
                        }
+                       assert_eq!(base_weight + inputs_total_weight as usize,  claim_tx.weight() + /* max_length_sig */ (73 * inputs_weight.len() - sum_actual_sigs));
                }
-               assert_eq!(base_weight + inputs_total_weight as usize,  claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_weight.len() - sum_actual_sigs));
 
                // Claim tx with 1 offered HTLCs, 3 received HTLCs
-               claim_tx.input.clear();
-               sum_actual_sigs = 0;
-               for i in 0..4 {
+               for &opt_anchors in [false, true].iter() {
+                       let mut claim_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };
+                       let mut sum_actual_sigs = 0;
+                       for i in 0..4 {
+                               claim_tx.input.push(TxIn {
+                                       previous_output: BitcoinOutPoint {
+                                               txid,
+                                               vout: i,
+                                       },
+                                       script_sig: Script::new(),
+                                       sequence: 0xfffffffd,
+                                       witness: Witness::new(),
+                               });
+                       }
+                       claim_tx.output.push(TxOut {
+                               script_pubkey: script_pubkey.clone(),
+                               value: 0,
+                       });
+                       let base_weight = claim_tx.weight();
+                       let inputs_weight = vec![weight_offered_htlc(opt_anchors), weight_received_htlc(opt_anchors), weight_received_htlc(opt_anchors), weight_received_htlc(opt_anchors)];
+                       let mut inputs_total_weight = 2; // count segwit flags
+                       {
+                               let mut sighash_parts = sighash::SighashCache::new(&mut claim_tx);
+                               for (idx, inp) in inputs_weight.iter().enumerate() {
+                                       sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, opt_anchors);
+                                       inputs_total_weight += inp;
+                               }
+                       }
+                       assert_eq!(base_weight + inputs_total_weight as usize,  claim_tx.weight() + /* max_length_sig */ (73 * inputs_weight.len() - sum_actual_sigs));
+               }
+
+               // Justice tx with 1 revoked HTLC-Success tx output
+               for &opt_anchors in [false, true].iter() {
+                       let mut claim_tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() };
+                       let mut sum_actual_sigs = 0;
                        claim_tx.input.push(TxIn {
                                previous_output: BitcoinOutPoint {
                                        txid,
-                                       vout: i,
+                                       vout: 0,
                                },
                                script_sig: Script::new(),
                                sequence: 0xfffffffd,
-                               witness: Vec::new(),
+                               witness: Witness::new(),
                        });
-               }
-               let base_weight = claim_tx.get_weight();
-               let inputs_weight = vec![WEIGHT_OFFERED_HTLC, WEIGHT_RECEIVED_HTLC, WEIGHT_RECEIVED_HTLC, WEIGHT_RECEIVED_HTLC];
-               let mut inputs_total_weight = 2; // count segwit flags
-               {
-                       let mut sighash_parts = bip143::SigHashCache::new(&mut claim_tx);
-                       for (idx, inp) in inputs_weight.iter().enumerate() {
-                               sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, false);
-                               inputs_total_weight += inp;
-                       }
-               }
-               assert_eq!(base_weight + inputs_total_weight as usize,  claim_tx.get_weight() + /* max_length_sig */ (73 * inputs_weight.len() - sum_actual_sigs));
-
-               // Justice tx with 1 revoked HTLC-Success tx output
-               claim_tx.input.clear();
-               sum_actual_sigs = 0;
-               claim_tx.input.push(TxIn {
-                       previous_output: BitcoinOutPoint {
-                               txid,
-                               vout: 0,
-                       },
-                       script_sig: Script::new(),
-                       sequence: 0xfffffffd,
-                       witness: Vec::new(),
-               });
-               let base_weight = claim_tx.get_weight();
-               let inputs_weight = vec![WEIGHT_REVOKED_OUTPUT];
-               let mut inputs_total_weight = 2; // count segwit flags
-               {
-                       let mut sighash_parts = bip143::SigHashCache::new(&mut claim_tx);
-                       for (idx, inp) in inputs_weight.iter().enumerate() {
-                               sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, false);
-                               inputs_total_weight += inp;
+                       claim_tx.output.push(TxOut {
+                               script_pubkey: script_pubkey.clone(),
+                               value: 0,
+                       });
+                       let base_weight = claim_tx.weight();
+                       let inputs_weight = vec![WEIGHT_REVOKED_OUTPUT];
+                       let mut inputs_total_weight = 2; // count segwit flags
+                       {
+                               let mut sighash_parts = sighash::SighashCache::new(&mut claim_tx);
+                               for (idx, inp) in inputs_weight.iter().enumerate() {
+                                       sign_input!(sighash_parts, idx, 0, inp, sum_actual_sigs, opt_anchors);
+                                       inputs_total_weight += inp;
+                               }
                        }
+                       assert_eq!(base_weight + inputs_total_weight as usize, claim_tx.weight() + /* max_length_isg */ (73 * inputs_weight.len() - sum_actual_sigs));
                }
-               assert_eq!(base_weight + inputs_total_weight as usize, claim_tx.get_weight() + /* max_length_isg */ (73 * inputs_weight.len() - sum_actual_sigs));
        }
 
        // Further testing is done in the ChannelManager integration tests.