Don't sign holder HTLCs along with holder commitments

`sign_holder_commitment_and_htlcs` never really made sense. Unlike
`sign_counterparty_commitment`, the signatures for holder HTLC
transactions may be required much later than the commitment
transaction's. While it was nice for us to only reach the signer once to
obtain all holder signatures, it's not really ideal anymore as we want
our signatures to be random and not reused.

We no longer return all holder HTLC signatures and instead defer to
obtaining them via `EcdsaChannelSigner::sign_holder_htlc_transaction`.

diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs
index 5d44ea640f03ecbef942febb5d9f5536e918953f..95824f910db7f8667066eb0aa387992977514ecd 100644 (file)
--- a/lightning/src/chain/onchaintx.rs
+++ b/lightning/src/chain/onchaintx.rs
@@ -1101,13 +1101,13 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
        // before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing
        // to monitor before.
        pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {
        // before providing a initial commitment transaction. For outbound channel, init ChannelMonitor at Channel::funding_signed, there is nothing
        // to monitor before.
        pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {

-               let (sig, _) = self.signer.sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment");
+               let sig = self.signer.sign_holder_commitment(&self.holder_commitment, &self.secp_ctx).expect("signing holder commitment");

                self.holder_commitment.add_holder_sig(funding_redeemscript, sig)
        }
 
        #[cfg(any(test, feature="unsafe_revoked_tx_signing"))]
        pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {
                self.holder_commitment.add_holder_sig(funding_redeemscript, sig)
        }
 
        #[cfg(any(test, feature="unsafe_revoked_tx_signing"))]
        pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Transaction {

-               let (sig, _) = self.signer.unsafe_sign_holder_commitment_and_htlcs(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment");
+               let sig = self.signer.unsafe_sign_holder_commitment(&self.holder_commitment, &self.secp_ctx).expect("sign holder commitment");

                self.holder_commitment.add_holder_sig(funding_redeemscript, sig)
        }
 
                self.holder_commitment.add_holder_sig(funding_redeemscript, sig)
        }
 

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index a61a8de82debecd5906c0ee95bdcdb420aae48f4..7b14573a1a49cf227f0ff46fbd261f4f156e49af 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -8184,6 +8184,7 @@ mod tests {
                use bitcoin::hashes::hex::FromHex;
                use bitcoin::hash_types::Txid;
                use bitcoin::secp256k1::Message;
                use bitcoin::hashes::hex::FromHex;
                use bitcoin::hash_types::Txid;
                use bitcoin::secp256k1::Message;

+               use crate::events::bump_transaction::{ChannelDerivationParameters, HTLCDescriptor};

                use crate::sign::EcdsaChannelSigner;
                use crate::ln::PaymentPreimage;
                use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys};
                use crate::sign::EcdsaChannelSigner;
                use crate::ln::PaymentPreimage;
                use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys};


@@ -8309,7 +8310,7 @@ mod tests {
                                        &chan.context.holder_signer.as_ref().pubkeys().funding_pubkey,
                                        chan.context.counterparty_funding_pubkey()
                                );
                                        &chan.context.holder_signer.as_ref().pubkeys().funding_pubkey,
                                        chan.context.counterparty_funding_pubkey()
                                );

-                               let (holder_sig, htlc_sigs) = signer.sign_holder_commitment_and_htlcs(&holder_commitment_tx, &secp_ctx).unwrap();
+                               let holder_sig = signer.sign_holder_commitment(&holder_commitment_tx, &secp_ctx).unwrap();

                                assert_eq!(Signature::from_der(&hex::decode($sig_hex).unwrap()[..]).unwrap(), holder_sig, "holder_sig");
 
                                let funding_redeemscript = chan.context.get_funding_redeemscript();
                                assert_eq!(Signature::from_der(&hex::decode($sig_hex).unwrap()[..]).unwrap(), holder_sig, "holder_sig");
 
                                let funding_redeemscript = chan.context.get_funding_redeemscript();


@@ -8317,14 +8318,14 @@ mod tests {
                                assert_eq!(serialize(&tx)[..], hex::decode($tx_hex).unwrap()[..], "tx");
 
                                // ((htlc, counterparty_sig), (index, holder_sig))
                                assert_eq!(serialize(&tx)[..], hex::decode($tx_hex).unwrap()[..], "tx");
 
                                // ((htlc, counterparty_sig), (index, holder_sig))

-                               let mut htlc_sig_iter = holder_commitment_tx.htlcs().iter().zip(&holder_commitment_tx.counterparty_htlc_sigs).zip(htlc_sigs.iter().enumerate());
+                               let mut htlc_counterparty_sig_iter = holder_commitment_tx.counterparty_htlc_sigs.iter();

 
                                $({
                                        log_trace!(logger, "verifying htlc {}", $htlc_idx);
                                        let remote_signature = Signature::from_der(&hex::decode($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
 
                                        let ref htlc = htlcs[$htlc_idx];
 
                                $({
                                        log_trace!(logger, "verifying htlc {}", $htlc_idx);
                                        let remote_signature = Signature::from_der(&hex::decode($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
 
                                        let ref htlc = htlcs[$htlc_idx];

-                                       let htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw,
+                                       let mut htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.context.feerate_per_kw,

                                                chan.context.get_counterparty_selected_contest_delay().unwrap(),
                                                &htlc, $opt_anchors, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
                                        let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, $opt_anchors, &keys);
                                                chan.context.get_counterparty_selected_contest_delay().unwrap(),
                                                &htlc, $opt_anchors, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
                                        let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, $opt_anchors, &keys);


@@ -8344,20 +8345,32 @@ mod tests {
                                                assert!(preimage.is_some());
                                        }
 
                                                assert!(preimage.is_some());
                                        }
 

-                                       let htlc_sig = htlc_sig_iter.next().unwrap();
+                                       let htlc_counterparty_sig = htlc_counterparty_sig_iter.next().unwrap();
+                                       let htlc_holder_sig = signer.sign_holder_htlc_transaction(&htlc_tx, 0, &HTLCDescriptor {
+                                               channel_derivation_parameters: ChannelDerivationParameters {
+                                                       value_satoshis: chan.context.channel_value_satoshis,
+                                                       keys_id: chan.context.channel_keys_id,
+                                                       transaction_parameters: chan.context.channel_transaction_parameters.clone(),
+                                               },
+                                               commitment_txid: trusted_tx.txid(),
+                                               per_commitment_number: trusted_tx.commitment_number(),
+                                               per_commitment_point: trusted_tx.per_commitment_point(),
+                                               feerate_per_kw: trusted_tx.feerate_per_kw(),
+                                               htlc: htlc.clone(),
+                                               preimage: preimage.clone(),
+                                               counterparty_sig: *htlc_counterparty_sig,
+                                       }, &secp_ctx).unwrap();

                                        let num_anchors = if $opt_anchors.supports_anchors_zero_fee_htlc_tx() { 2 } else { 0 };
                                        let num_anchors = if $opt_anchors.supports_anchors_zero_fee_htlc_tx() { 2 } else { 0 };

-                                       assert_eq!((htlc_sig.0).0.transaction_output_index, Some($htlc_idx + num_anchors), "output index");
+                                       assert_eq!(htlc.transaction_output_index, Some($htlc_idx + num_anchors), "output index");

 
                                        let signature = Signature::from_der(&hex::decode($htlc_sig_hex).unwrap()[..]).unwrap();
 
                                        let signature = Signature::from_der(&hex::decode($htlc_sig_hex).unwrap()[..]).unwrap();

-                                       assert_eq!(signature, *(htlc_sig.1).1, "htlc sig");
-                                       let index = (htlc_sig.1).0;
-                                       let channel_parameters = chan.context.channel_transaction_parameters.as_holder_broadcastable();
+                                       assert_eq!(signature, htlc_holder_sig, "htlc sig");

                                        let trusted_tx = holder_commitment_tx.trust();
                                        let trusted_tx = holder_commitment_tx.trust();

-                                       log_trace!(logger, "htlc_tx = {}", hex::encode(serialize(&trusted_tx.get_signed_htlc_tx(&channel_parameters, index, &(htlc_sig.0).1, (htlc_sig.1).1, &preimage))));
-                                       assert_eq!(serialize(&trusted_tx.get_signed_htlc_tx(&channel_parameters, index, &(htlc_sig.0).1, (htlc_sig.1).1, &preimage))[..],
-                                                       hex::decode($htlc_tx_hex).unwrap()[..], "htlc tx");
+                                       htlc_tx.input[0].witness = trusted_tx.build_htlc_input_witness($htlc_idx, htlc_counterparty_sig, &htlc_holder_sig, &preimage);
+                                       log_trace!(logger, "htlc_tx = {}", hex::encode(serialize(&htlc_tx)));
+                                       assert_eq!(serialize(&htlc_tx)[..], hex::decode($htlc_tx_hex).unwrap()[..], "htlc tx");

                                })*
                                })*

-                               assert!(htlc_sig_iter.next().is_none());
+                               assert!(htlc_counterparty_sig_iter.next().is_none());

                        } }
                }
 
                        } }
                }
 

diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index 5be498e918b1227a153ca7d9a44d90db1e651cc9..37fdc647a4cc8f3373e80e0cc330e93ffaa1fa34 100644 (file)
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -1680,7 +1680,7 @@ fn do_test_revoked_counterparty_aggregated_claims(anchors: bool) {
        // secret to the counterparty. However, because we always immediately take the revocation
        // secret from the keys_manager, we would panic at broadcast as we're trying to sign a
        // transaction which, from the point of view of our keys_manager, is revoked.
        // secret to the counterparty. However, because we always immediately take the revocation
        // secret from the keys_manager, we would panic at broadcast as we're trying to sign a
        // transaction which, from the point of view of our keys_manager, is revoked.

-       chanmon_cfgs[1].keys_manager.disable_revocation_policy_check = true;
+       chanmon_cfgs[0].keys_manager.disable_revocation_policy_check = true;

        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
        let mut user_config = test_default_channel_config();
        if anchors {
        let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
        let mut user_config = test_default_channel_config();
        if anchors {

diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
index d9992585819db48a9ec2b799681d1b2fef0ceaee..6abf4bd254cb062250128280fd28c9c0a6367170 100644 (file)
--- a/lightning/src/sign/mod.rs
+++ b/lightning/src/sign/mod.rs
@@ -487,31 +487,26 @@ pub trait EcdsaChannelSigner: ChannelSigner {
        /// This is required in order for the signer to make sure that the state has moved
        /// forward and it is safe to sign the next counterparty commitment.
        fn validate_counterparty_revocation(&self, idx: u64, secret: &SecretKey) -> Result<(), ()>;
        /// This is required in order for the signer to make sure that the state has moved
        /// forward and it is safe to sign the next counterparty commitment.
        fn validate_counterparty_revocation(&self, idx: u64, secret: &SecretKey) -> Result<(), ()>;

-       /// Creates a signature for a holder's commitment transaction and its claiming HTLC transactions.
+       /// Creates a signature for a holder's commitment transaction.

        ///
        /// This will be called
        /// - with a non-revoked `commitment_tx`.
        /// - with the latest `commitment_tx` when we initiate a force-close.
        ///
        /// This will be called
        /// - with a non-revoked `commitment_tx`.
        /// - with the latest `commitment_tx` when we initiate a force-close.

-       /// - with the previous `commitment_tx`, just to get claiming HTLC
-       ///   signatures, if we are reacting to a [`ChannelMonitor`]
-       ///   [replica](https://github.com/lightningdevkit/rust-lightning/blob/main/GLOSSARY.md#monitor-replicas)
-       ///   that decided to broadcast before it had been updated to the latest `commitment_tx`.

        ///
        /// This may be called multiple times for the same transaction.
        ///
        /// An external signer implementation should check that the commitment has not been revoked.
        ///
        /// This may be called multiple times for the same transaction.
        ///
        /// An external signer implementation should check that the commitment has not been revoked.

-       ///
-       /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor
+       //

        // TODO: Document the things someone using this interface should enforce before signing.
        // TODO: Document the things someone using this interface should enforce before signing.

-       fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction,
-               secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()>;
-       /// Same as [`sign_holder_commitment_and_htlcs`], but exists only for tests to get access to
-       /// holder commitment transactions which will be broadcasted later, after the channel has moved
-       /// on to a newer state. Thus, needs its own method as [`sign_holder_commitment_and_htlcs`] may
-       /// enforce that we only ever get called once.
+       fn sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction,
+               secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;
+       /// Same as [`sign_holder_commitment`], but exists only for tests to get access to holder
+       /// commitment transactions which will be broadcasted later, after the channel has moved on to a
+       /// newer state. Thus, needs its own method as [`sign_holder_commitment`] may enforce that we
+       /// only ever get called once.

        #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
        #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]

-       fn unsafe_sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction,
-               secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()>;
+       fn unsafe_sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction,
+               secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>;

        /// Create a signature for the given input in a transaction spending an HTLC transaction output
        /// or a commitment transaction `to_local` output when our counterparty broadcasts an old state.
        ///
        /// Create a signature for the given input in a transaction spending an HTLC transaction output
        /// or a commitment transaction `to_local` output when our counterparty broadcasts an old state.
        ///


@@ -554,7 +549,12 @@ pub trait EcdsaChannelSigner: ChannelSigner {
        /// `htlc_tx`, which spends the commitment transaction at index `input`. The signature returned
        /// must be be computed using [`EcdsaSighashType::All`].
        ///
        /// `htlc_tx`, which spends the commitment transaction at index `input`. The signature returned
        /// must be be computed using [`EcdsaSighashType::All`].
        ///

+       /// Note that this may be called for HTLCs in the penultimate commitment transaction if a
+       /// [`ChannelMonitor`] [replica](https://github.com/lightningdevkit/rust-lightning/blob/main/GLOSSARY.md#monitor-replicas)
+       /// broadcasts it before receiving the update for the latest commitment transaction.
+       ///

        /// [`EcdsaSighashType::All`]: bitcoin::blockdata::transaction::EcdsaSighashType::All
        /// [`EcdsaSighashType::All`]: bitcoin::blockdata::transaction::EcdsaSighashType::All

+       /// [`ChannelMonitor`]: crate::chain::channelmonitor::ChannelMonitor

        fn sign_holder_htlc_transaction(&self, htlc_tx: &Transaction, input: usize,
                htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1<secp256k1::All>
        ) -> Result<Signature, ()>;
        fn sign_holder_htlc_transaction(&self, htlc_tx: &Transaction, input: usize,
                htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1<secp256k1::All>
        ) -> Result<Signature, ()>;


@@ -1118,27 +1118,21 @@ impl EcdsaChannelSigner for InMemorySigner {
                Ok(())
        }
 
                Ok(())
        }
 

-       fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
+       fn sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {

                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
                let counterparty_keys = self.counterparty_pubkeys().expect(MISSING_PARAMS_ERR);
                let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_keys.funding_pubkey);
                let trusted_tx = commitment_tx.trust();
                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
                let counterparty_keys = self.counterparty_pubkeys().expect(MISSING_PARAMS_ERR);
                let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_keys.funding_pubkey);
                let trusted_tx = commitment_tx.trust();

-               let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx);
-               let channel_parameters = self.get_channel_parameters().expect(MISSING_PARAMS_ERR);
-               let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?;
-               Ok((sig, htlc_sigs))
+               Ok(trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx))

        }
 
        #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
        }
 
        #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]

-       fn unsafe_sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
+       fn unsafe_sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {

                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
                let counterparty_keys = self.counterparty_pubkeys().expect(MISSING_PARAMS_ERR);
                let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_keys.funding_pubkey);
                let trusted_tx = commitment_tx.trust();
                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
                let counterparty_keys = self.counterparty_pubkeys().expect(MISSING_PARAMS_ERR);
                let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &counterparty_keys.funding_pubkey);
                let trusted_tx = commitment_tx.trust();

-               let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx);
-               let channel_parameters = self.get_channel_parameters().expect(MISSING_PARAMS_ERR);
-               let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?;
-               Ok((sig, htlc_sigs))
+               Ok(trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx))

        }
 
        fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
        }
 
        fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {

diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs
index c57c5a9d6fd12b2330f587ed0ea8949cf6124eb0..99240873f326a860e99c67a27c948381c36d380c 100644 (file)
--- a/lightning/src/util/test_channel_signer.rs
+++ b/lightning/src/util/test_channel_signer.rs
@@ -155,11 +155,8 @@ impl EcdsaChannelSigner for TestChannelSigner {
                Ok(())
        }
 
                Ok(())
        }
 

-       fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
+       fn sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {

                let trusted_tx = self.verify_holder_commitment_tx(commitment_tx, secp_ctx);
                let trusted_tx = self.verify_holder_commitment_tx(commitment_tx, secp_ctx);

-               let commitment_txid = trusted_tx.txid();
-               let holder_csv = self.inner.counterparty_selected_contest_delay().unwrap();
-

                let state = self.state.lock().unwrap();
                let commitment_number = trusted_tx.commitment_number();
                if state.last_holder_revoked_commitment - 1 != commitment_number && state.last_holder_revoked_commitment - 2 != commitment_number {
                let state = self.state.lock().unwrap();
                let commitment_number = trusted_tx.commitment_number();
                if state.last_holder_revoked_commitment - 1 != commitment_number && state.last_holder_revoked_commitment - 2 != commitment_number {


@@ -168,33 +165,12 @@ impl EcdsaChannelSigner for TestChannelSigner {
                                       state.last_holder_revoked_commitment, commitment_number, self.inner.commitment_seed[0])
                        }
                }
                                       state.last_holder_revoked_commitment, commitment_number, self.inner.commitment_seed[0])
                        }
                }

-
-               for (this_htlc, sig) in trusted_tx.htlcs().iter().zip(&commitment_tx.counterparty_htlc_sigs) {
-                       assert!(this_htlc.transaction_output_index.is_some());
-                       let keys = trusted_tx.keys();
-                       let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, trusted_tx.feerate_per_kw(), holder_csv, &this_htlc, self.channel_type_features(), &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
-
-                       let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&this_htlc, self.channel_type_features(), &keys);
-
-                       let sighash_type = if self.channel_type_features().supports_anchors_zero_fee_htlc_tx() {
-                               EcdsaSighashType::SinglePlusAnyoneCanPay
-                       } else {
-                               EcdsaSighashType::All
-                       };
-                       let sighash = hash_to_message!(
-                               &sighash::SighashCache::new(&htlc_tx).segwit_signature_hash(
-                                       0, &htlc_redeemscript, this_htlc.amount_msat / 1000, sighash_type,
-                               ).unwrap()[..]
-                       );
-                       secp_ctx.verify_ecdsa(&sighash, sig, &keys.countersignatory_htlc_key).unwrap();
-               }
-
-               Ok(self.inner.sign_holder_commitment_and_htlcs(commitment_tx, secp_ctx).unwrap())
+               Ok(self.inner.sign_holder_commitment(commitment_tx, secp_ctx).unwrap())

        }
 
        #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
        }
 
        #[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]

-       fn unsafe_sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
-               Ok(self.inner.unsafe_sign_holder_commitment_and_htlcs(commitment_tx, secp_ctx).unwrap())
+       fn unsafe_sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
+               Ok(self.inner.unsafe_sign_holder_commitment(commitment_tx, secp_ctx).unwrap())

        }
 
        fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
        }
 
        fn sign_justice_revoked_output(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {


@@ -209,8 +185,34 @@ impl EcdsaChannelSigner for TestChannelSigner {
                &self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
                secp_ctx: &Secp256k1<secp256k1::All>
        ) -> Result<Signature, ()> {
                &self, htlc_tx: &Transaction, input: usize, htlc_descriptor: &HTLCDescriptor,
                secp_ctx: &Secp256k1<secp256k1::All>
        ) -> Result<Signature, ()> {

+               let state = self.state.lock().unwrap();
+               if state.last_holder_revoked_commitment - 1 != htlc_descriptor.per_commitment_number &&
+                       state.last_holder_revoked_commitment - 2 != htlc_descriptor.per_commitment_number
+               {
+                       if !self.disable_revocation_policy_check {
+                               panic!("can only sign the next two unrevoked commitment numbers, revoked={} vs requested={} for {}",
+                                      state.last_holder_revoked_commitment, htlc_descriptor.per_commitment_number, self.inner.commitment_seed[0])
+                       }
+               }

                assert_eq!(htlc_tx.input[input], htlc_descriptor.unsigned_tx_input());
                assert_eq!(htlc_tx.output[input], htlc_descriptor.tx_output(secp_ctx));
                assert_eq!(htlc_tx.input[input], htlc_descriptor.unsigned_tx_input());
                assert_eq!(htlc_tx.output[input], htlc_descriptor.tx_output(secp_ctx));

+               {
+                       let witness_script = htlc_descriptor.witness_script(secp_ctx);
+                       let sighash_type = if self.channel_type_features().supports_anchors_zero_fee_htlc_tx() {
+                               EcdsaSighashType::SinglePlusAnyoneCanPay
+                       } else {
+                               EcdsaSighashType::All
+                       };
+                       let sighash = &sighash::SighashCache::new(&*htlc_tx).segwit_signature_hash(
+                               input, &witness_script, htlc_descriptor.htlc.amount_msat / 1000, sighash_type
+                       ).unwrap();
+                       let countersignatory_htlc_key = chan_utils::derive_public_key(
+                               &secp_ctx, &htlc_descriptor.per_commitment_point, &self.inner.counterparty_pubkeys().unwrap().htlc_basepoint
+                       );
+                       secp_ctx.verify_ecdsa(
+                               &hash_to_message!(&sighash), &htlc_descriptor.counterparty_sig, &countersignatory_htlc_key
+                       ).unwrap();
+               }

                Ok(self.inner.sign_holder_htlc_transaction(htlc_tx, input, htlc_descriptor, secp_ctx).unwrap())
        }
 
                Ok(self.inner.sign_holder_htlc_transaction(htlc_tx, input, htlc_descriptor, secp_ctx).unwrap())
        }