Move TODO from `handle_monitor_update_res` into `Channel`
[rust-lightning] / lightning / src / ln / channel.rs
index c1e7c0bcad326da46ea6ae0e88156a98d192a6f8..896c475049b4df56673dfa5e2c3ca7d45760e448 100644 (file)
@@ -35,7 +35,8 @@ use crate::chain::BestBlock;
 use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator};
 use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
 use crate::chain::transaction::{OutPoint, TransactionData};
-use crate::chain::keysinterface::{Sign, EntropySource, BaseSign, NodeSigner, Recipient, SignerProvider};
+use crate::chain::keysinterface::{WriteableEcdsaChannelSigner, EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient};
+use crate::routing::gossip::NodeId;
 use crate::util::events::ClosureReason;
 use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
 use crate::util::logger::Logger;
@@ -498,7 +499,7 @@ pub(crate) const EXPIRE_PREV_CONFIG_TICKS: usize = 5;
 //
 // Holder designates channel data owned for the benefice of the user client.
 // Counterparty designates channel data owned by the another channel participant entity.
-pub(super) struct Channel<Signer: Sign> {
+pub(super) struct Channel<Signer: ChannelSigner> {
        config: LegacyChannelConfig,
 
        // Track the previous `ChannelConfig` so that we can continue forwarding HTLCs that were
@@ -557,6 +558,11 @@ pub(super) struct Channel<Signer: Sign> {
        monitor_pending_channel_ready: bool,
        monitor_pending_revoke_and_ack: bool,
        monitor_pending_commitment_signed: bool,
+
+       // TODO: If a channel is drop'd, we don't know whether the `ChannelMonitor` is ultimately
+       // responsible for some of the HTLCs here or not - we don't know whether the update in question
+       // completed or not. We currently ignore these fields entirely when force-closing a channel,
+       // but need to handle this somehow or we run the risk of losing HTLCs!
        monitor_pending_forwards: Vec<(PendingHTLCInfo, u64)>,
        monitor_pending_failures: Vec<(HTLCSource, PaymentHash, HTLCFailReason)>,
        monitor_pending_finalized_fulfills: Vec<HTLCSource>,
@@ -742,6 +748,12 @@ pub(super) struct Channel<Signer: Sign> {
        /// The unique identifier used to re-derive the private key material for the channel through
        /// [`SignerProvider::derive_channel_signer`].
        channel_keys_id: [u8; 32],
+
+       /// When we generate [`ChannelMonitorUpdate`]s to persist, they may not be persisted immediately.
+       /// If we then persist the [`channelmanager::ChannelManager`] and crash before the persistence
+       /// completes we still need to be able to complete the persistence. Thus, we have to keep a
+       /// copy of the [`ChannelMonitorUpdate`] here until it is complete.
+       pending_monitor_updates: Vec<ChannelMonitorUpdate>,
 }
 
 #[cfg(any(test, fuzzing))]
@@ -832,7 +844,7 @@ macro_rules! secp_check {
        };
 }
 
-impl<Signer: Sign> Channel<Signer> {
+impl<Signer: WriteableEcdsaChannelSigner> Channel<Signer> {
        /// Returns the value to use for `holder_max_htlc_value_in_flight_msat` as a percentage of the
        /// `channel_value_satoshis` in msat, set through
        /// [`ChannelHandshakeConfig::max_inbound_htlc_value_in_flight_percent_of_channel`]
@@ -877,15 +889,29 @@ impl<Signer: Sign> Channel<Signer> {
                self.channel_transaction_parameters.opt_anchors.is_some()
        }
 
-       fn get_initial_channel_type(config: &UserConfig) -> ChannelTypeFeatures {
+       fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures) -> ChannelTypeFeatures {
                // The default channel type (ie the first one we try) depends on whether the channel is
                // public - if it is, we just go with `only_static_remotekey` as it's the only option
                // available. If it's private, we first try `scid_privacy` as it provides better privacy
-               // with no other changes, and fall back to `only_static_remotekey`
+               // with no other changes, and fall back to `only_static_remotekey`.
                let mut ret = ChannelTypeFeatures::only_static_remote_key();
-               if !config.channel_handshake_config.announced_channel && config.channel_handshake_config.negotiate_scid_privacy {
+               if !config.channel_handshake_config.announced_channel &&
+                       config.channel_handshake_config.negotiate_scid_privacy &&
+                       their_features.supports_scid_privacy() {
                        ret.set_scid_privacy_required();
                }
+
+               // Optionally, if the user would like to negotiate the `anchors_zero_fee_htlc_tx` option, we
+               // set it now. If they don't understand it, we'll fall back to our default of
+               // `only_static_remotekey`.
+               #[cfg(anchors)]
+               { // Attributes are not allowed on if expressions on our current MSRV of 1.41.
+                       if config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx &&
+                               their_features.supports_anchors_zero_fee_htlc_tx() {
+                               ret.set_anchors_zero_fee_htlc_tx_required();
+                       }
+               }
+
                ret
        }
 
@@ -898,7 +924,24 @@ impl<Signer: Sign> Channel<Signer> {
                        // We've exhausted our options
                        return Err(());
                }
-               self.channel_type = ChannelTypeFeatures::only_static_remote_key(); // We only currently support two types
+               // We support opening a few different types of channels. Try removing our additional
+               // features one by one until we've either arrived at our default or the counterparty has
+               // accepted one.
+               //
+               // Due to the order below, we may not negotiate `option_anchors_zero_fee_htlc_tx` if the
+               // counterparty doesn't support `option_scid_privacy`. Since `get_initial_channel_type`
+               // checks whether the counterparty supports every feature, this would only happen if the
+               // counterparty is advertising the feature, but rejecting channels proposing the feature for
+               // whatever reason.
+               if self.channel_type.supports_anchors_zero_fee_htlc_tx() {
+                       self.channel_type.clear_anchors_zero_fee_htlc_tx();
+                       assert!(self.channel_transaction_parameters.opt_non_zero_fee_anchors.is_none());
+                       self.channel_transaction_parameters.opt_anchors = None;
+               } else if self.channel_type.supports_scid_privacy() {
+                       self.channel_type.clear_scid_privacy();
+               } else {
+                       self.channel_type = ChannelTypeFeatures::only_static_remote_key();
+               }
                Ok(self.get_open_channel(chain_hash))
        }
 
@@ -912,8 +955,6 @@ impl<Signer: Sign> Channel<Signer> {
              SP::Target: SignerProvider<Signer = Signer>,
              F::Target: FeeEstimator,
        {
-               let opt_anchors = false; // TODO - should be based on features
-
                let holder_selected_contest_delay = config.channel_handshake_config.our_to_self_delay;
                let channel_keys_id = signer_provider.generate_channel_keys_id(false, channel_value_satoshis, user_id);
                let holder_signer = signer_provider.derive_channel_signer(channel_value_satoshis, channel_keys_id);
@@ -939,10 +980,13 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(APIError::APIMisuseError { err: format!("Holder selected channel  reserve below implemention limit dust_limit_satoshis {}", holder_selected_channel_reserve_satoshis) });
                }
 
+               let channel_type = Self::get_initial_channel_type(&config, their_features);
+               debug_assert!(channel_type.is_subset(&channelmanager::provided_channel_type_features(&config)));
+
                let feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal);
 
                let value_to_self_msat = channel_value_satoshis * 1000 - push_msat;
-               let commitment_tx_fee = Self::commit_tx_fee_msat(feerate, MIN_AFFORDABLE_HTLC_COUNT, opt_anchors);
+               let commitment_tx_fee = Self::commit_tx_fee_msat(feerate, MIN_AFFORDABLE_HTLC_COUNT, channel_type.requires_anchors_zero_fee_htlc_tx());
                if value_to_self_msat < commitment_tx_fee {
                        return Err(APIError::APIMisuseError{ err: format!("Funding amount ({}) can't even pay fee for initial commitment transaction fee of {}.", value_to_self_msat / 1000, commitment_tx_fee / 1000) });
                }
@@ -1044,7 +1088,7 @@ impl<Signer: Sign> Channel<Signer> {
                                is_outbound_from_holder: true,
                                counterparty_parameters: None,
                                funding_outpoint: None,
-                               opt_anchors: if opt_anchors { Some(()) } else { None },
+                               opt_anchors: if channel_type.requires_anchors_zero_fee_htlc_tx() { Some(()) } else { None },
                                opt_non_zero_fee_anchors: None
                        },
                        funding_transaction: None,
@@ -1077,8 +1121,10 @@ impl<Signer: Sign> Channel<Signer> {
                        #[cfg(any(test, fuzzing))]
                        historical_inbound_htlc_fulfills: HashSet::new(),
 
-                       channel_type: Self::get_initial_channel_type(&config),
+                       channel_type,
                        channel_keys_id,
+
+                       pending_monitor_updates: Vec::new(),
                })
        }
 
@@ -1117,16 +1163,16 @@ impl<Signer: Sign> Channel<Signer> {
        /// Creates a new channel from a remote sides' request for one.
        /// Assumes chain_hash has already been checked and corresponds with what we expect!
        pub fn new_from_req<ES: Deref, SP: Deref, F: Deref, L: Deref>(
-               fee_estimator: &LowerBoundedFeeEstimator<F>, entropy_source: &ES, signer_provider: &SP, counterparty_node_id: PublicKey, their_features: &InitFeatures,
-               msg: &msgs::OpenChannel, user_id: u128, config: &UserConfig, current_chain_height: u32, logger: &L,
-               outbound_scid_alias: u64
+               fee_estimator: &LowerBoundedFeeEstimator<F>, entropy_source: &ES, signer_provider: &SP,
+               counterparty_node_id: PublicKey, our_supported_features: &ChannelTypeFeatures,
+               their_features: &InitFeatures, msg: &msgs::OpenChannel, user_id: u128, config: &UserConfig,
+               current_chain_height: u32, logger: &L, outbound_scid_alias: u64
        ) -> Result<Channel<Signer>, ChannelError>
                where ES::Target: EntropySource,
                          SP::Target: SignerProvider<Signer = Signer>,
                          F::Target: FeeEstimator,
                          L::Target: Logger,
        {
-               let opt_anchors = false; // TODO - should be based on features
                let announced_channel = if (msg.channel_flags & 1) == 1 { true } else { false };
 
                // First check the channel type is known, failing before we do anything else if we don't
@@ -1136,31 +1182,28 @@ impl<Signer: Sign> Channel<Signer> {
                                return Err(ChannelError::Close("Channel Type field contained optional bits - this is not allowed".to_owned()));
                        }
 
-                       if channel_type.requires_unknown_bits() {
-                               return Err(ChannelError::Close("Channel Type field contains unknown bits".to_owned()));
+                       // We only support the channel types defined by the `ChannelManager` in
+                       // `provided_channel_type_features`. The channel type must always support
+                       // `static_remote_key`.
+                       if !channel_type.requires_static_remote_key() {
+                               return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned()));
                        }
-
-                       // We currently only allow four channel types, so write it all out here - we allow
-                       // `only_static_remote_key` or `static_remote_key | zero_conf` in all contexts, and
-                       // further allow `static_remote_key | scid_privacy` or
-                       // `static_remote_key | scid_privacy | zero_conf`, if the channel is not
-                       // publicly announced.
-                       if *channel_type != ChannelTypeFeatures::only_static_remote_key() {
-                               if !channel_type.requires_scid_privacy() && !channel_type.requires_zero_conf() {
-                                       return Err(ChannelError::Close("Channel Type was not understood".to_owned()));
-                               }
-
-                               if channel_type.requires_scid_privacy() && announced_channel {
-                                       return Err(ChannelError::Close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned()));
-                               }
+                       // Make sure we support all of the features behind the channel type.
+                       if !channel_type.is_subset(our_supported_features) {
+                               return Err(ChannelError::Close("Channel Type contains unsupported features".to_owned()));
+                       }
+                       if channel_type.requires_scid_privacy() && announced_channel {
+                               return Err(ChannelError::Close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned()));
                        }
                        channel_type.clone()
                } else {
-                       ChannelTypeFeatures::from_counterparty_init(&their_features)
+                       let channel_type = ChannelTypeFeatures::from_init(&their_features);
+                       if channel_type != ChannelTypeFeatures::only_static_remote_key() {
+                               return Err(ChannelError::Close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
+                       }
+                       channel_type
                };
-               if !channel_type.supports_static_remote_key() {
-                       return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned()));
-               }
+               let opt_anchors = channel_type.supports_anchors_zero_fee_htlc_tx();
 
                let channel_keys_id = signer_provider.generate_channel_keys_id(true, msg.funding_satoshis, user_id);
                let holder_signer = signer_provider.derive_channel_signer(msg.funding_satoshis, channel_keys_id);
@@ -1428,6 +1471,8 @@ impl<Signer: Sign> Channel<Signer> {
 
                        channel_type,
                        channel_keys_id,
+
+                       pending_monitor_updates: Vec::new(),
                };
 
                Ok(chan)
@@ -2130,7 +2175,11 @@ impl<Signer: Sign> Channel<Signer> {
                } else if their_features.supports_channel_type() {
                        // Assume they've accepted the channel type as they said they understand it.
                } else {
-                       self.channel_type = ChannelTypeFeatures::from_counterparty_init(&their_features)
+                       let channel_type = ChannelTypeFeatures::from_init(&their_features);
+                       if channel_type != ChannelTypeFeatures::only_static_remote_key() {
+                               return Err(ChannelError::Close("Only static_remote_key is supported for non-negotiated channel types".to_owned()));
+                       }
+                       self.channel_type = channel_type;
                }
 
                let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
@@ -4857,6 +4906,10 @@ impl<Signer: Sign> Channel<Signer> {
                (self.channel_state & ChannelState::MonitorUpdateInProgress as u32) != 0
        }
 
+       pub fn get_next_monitor_update(&self) -> Option<&ChannelMonitorUpdate> {
+               self.pending_monitor_updates.first()
+       }
+
        /// Returns true if funding_created was sent/received.
        pub fn is_funding_initiated(&self) -> bool {
                self.channel_state >= ChannelState::FundingSent as u32
@@ -5383,18 +5436,19 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned()));
                }
 
-               let node_id = node_signer.get_node_id(Recipient::Node)
-                       .map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?;
-               let were_node_one = node_id.serialize()[..] < self.counterparty_node_id.serialize()[..];
+               let node_id = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node)
+                       .map_err(|_| ChannelError::Ignore("Failed to retrieve own public key".to_owned()))?);
+               let counterparty_node_id = NodeId::from_pubkey(&self.get_counterparty_node_id());
+               let were_node_one = node_id.as_slice() < counterparty_node_id.as_slice();
 
                let msg = msgs::UnsignedChannelAnnouncement {
                        features: channelmanager::provided_channel_features(&user_config),
                        chain_hash,
                        short_channel_id: self.get_short_channel_id().unwrap(),
-                       node_id_1: if were_node_one { node_id } else { self.get_counterparty_node_id() },
-                       node_id_2: if were_node_one { self.get_counterparty_node_id() } else { node_id },
-                       bitcoin_key_1: if were_node_one { self.get_holder_pubkeys().funding_pubkey } else { self.counterparty_funding_pubkey().clone() },
-                       bitcoin_key_2: if were_node_one { self.counterparty_funding_pubkey().clone() } else { self.get_holder_pubkeys().funding_pubkey },
+                       node_id_1: if were_node_one { node_id } else { counterparty_node_id },
+                       node_id_2: if were_node_one { counterparty_node_id } else { node_id },
+                       bitcoin_key_1: NodeId::from_pubkey(if were_node_one { &self.get_holder_pubkeys().funding_pubkey } else { self.counterparty_funding_pubkey() }),
+                       bitcoin_key_2: NodeId::from_pubkey(if were_node_one { self.counterparty_funding_pubkey() } else { &self.get_holder_pubkeys().funding_pubkey }),
                        excess_data: Vec::new(),
                };
 
@@ -5464,8 +5518,8 @@ impl<Signer: Sign> Channel<Signer> {
                &self, node_signer: &NS, announcement: msgs::UnsignedChannelAnnouncement
        ) -> Result<msgs::ChannelAnnouncement, ChannelError> where NS::Target: NodeSigner {
                if let Some((their_node_sig, their_bitcoin_sig)) = self.announcement_sigs {
-                       let our_node_key = node_signer.get_node_id(Recipient::Node)
-                               .map_err(|_| ChannelError::Ignore("Signer failed to retrieve own public key".to_owned()))?;
+                       let our_node_key = NodeId::from_pubkey(&node_signer.get_node_id(Recipient::Node)
+                               .map_err(|_| ChannelError::Ignore("Signer failed to retrieve own public key".to_owned()))?);
                        let were_node_one = announcement.node_id_1 == our_node_key;
 
                        let our_node_sig = node_signer.sign_gossip_message(msgs::UnsignedGossipMessage::ChannelAnnouncement(&announcement))
@@ -5756,8 +5810,16 @@ impl<Signer: Sign> Channel<Signer> {
                Ok(Some(res))
        }
 
-       /// Only fails in case of bad keys
+       /// Only fails in case of signer rejection.
        fn send_commitment_no_status_check<L: Deref>(&mut self, logger: &L) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> where L::Target: Logger {
+               let monitor_update = self.build_commitment_no_status_check(logger);
+               match self.send_commitment_no_state_update(logger) {
+                       Ok((commitment_signed, _)) => Ok((commitment_signed, monitor_update)),
+                       Err(e) => Err(e),
+               }
+       }
+
+       fn build_commitment_no_status_check<L: Deref>(&mut self, logger: &L) -> ChannelMonitorUpdate where L::Target: Logger {
                log_trace!(logger, "Updating HTLC state for a newly-sent commitment_signed...");
                // We can upgrade the status of some HTLCs that are waiting on a commitment, even if we
                // fail to generate this, we still are at least at a position where upgrading their status
@@ -5790,15 +5852,9 @@ impl<Signer: Sign> Channel<Signer> {
                }
                self.resend_order = RAACommitmentOrder::RevokeAndACKFirst;
 
-               let (res, counterparty_commitment_txid, htlcs) = match self.send_commitment_no_state_update(logger) {
-                       Ok((res, (counterparty_commitment_tx, mut htlcs))) => {
-                               // Update state now that we've passed all the can-fail calls...
-                               let htlcs_no_ref: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)> =
-                                       htlcs.drain(..).map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect();
-                               (res, counterparty_commitment_tx, htlcs_no_ref)
-                       },
-                       Err(e) => return Err(e),
-               };
+               let (counterparty_commitment_txid, mut htlcs_ref) = self.build_commitment_no_state_update(logger);
+               let htlcs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)> =
+                       htlcs_ref.drain(..).map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect();
 
                if self.announcement_sigs_state == AnnouncementSigsState::MessageSent {
                        self.announcement_sigs_state = AnnouncementSigsState::Committed;
@@ -5815,16 +5871,13 @@ impl<Signer: Sign> Channel<Signer> {
                        }]
                };
                self.channel_state |= ChannelState::AwaitingRemoteRevoke as u32;
-               Ok((res, monitor_update))
+               monitor_update
        }
 
-       /// Only fails in case of bad keys. Used for channel_reestablish commitment_signed generation
-       /// when we shouldn't change HTLC/channel state.
-       fn send_commitment_no_state_update<L: Deref>(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger {
+       fn build_commitment_no_state_update<L: Deref>(&self, logger: &L) -> (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) where L::Target: Logger {
                let counterparty_keys = self.build_remote_transaction_keys();
                let commitment_stats = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger);
                let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
-               let (signature, htlc_signatures);
 
                #[cfg(any(test, fuzzing))]
                {
@@ -5844,6 +5897,21 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
 
+               (counterparty_commitment_txid, commitment_stats.htlcs_included)
+       }
+
+       /// Only fails in case of signer rejection. Used for channel_reestablish commitment_signed
+       /// generation when we shouldn't change HTLC/channel state.
+       fn send_commitment_no_state_update<L: Deref>(&self, logger: &L) -> Result<(msgs::CommitmentSigned, (Txid, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>)), ChannelError> where L::Target: Logger {
+               // Get the fee tests from `build_commitment_no_state_update`
+               #[cfg(any(test, fuzzing))]
+               self.build_commitment_no_state_update(logger);
+
+               let counterparty_keys = self.build_remote_transaction_keys();
+               let commitment_stats = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, true, logger);
+               let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
+               let (signature, htlc_signatures);
+
                {
                        let mut htlcs = Vec::with_capacity(commitment_stats.htlcs_included.len());
                        for &(ref htlc, _) in commitment_stats.htlcs_included.iter() {
@@ -6100,7 +6168,7 @@ impl Readable for AnnouncementSigsState {
        }
 }
 
-impl<Signer: Sign> Writeable for Channel<Signer> {
+impl<Signer: WriteableEcdsaChannelSigner> Writeable for Channel<Signer> {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
                // Note that we write out as if remove_uncommitted_htlcs_and_mark_paused had just been
                // called.
@@ -6395,13 +6463,13 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 }
 
 const MAX_ALLOC_SIZE: usize = 64*1024;
-impl<'a, 'b, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32)> for Channel<<SP::Target as SignerProvider>::Signer>
+impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c ChannelTypeFeatures)> for Channel<<SP::Target as SignerProvider>::Signer>
                where
                        ES::Target: EntropySource,
                        SP::Target: SignerProvider
 {
-       fn read<R : io::Read>(reader: &mut R, args: (&'a ES, &'b SP, u32)) -> Result<Self, DecodeError> {
-               let (entropy_source, signer_provider, serialized_height) = args;
+       fn read<R : io::Read>(reader: &mut R, args: (&'a ES, &'b SP, u32, &'c ChannelTypeFeatures)) -> Result<Self, DecodeError> {
+               let (entropy_source, signer_provider, serialized_height, our_supported_features) = args;
                let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
                // `user_id` used to be a single u64 value. In order to remain backwards compatible with
@@ -6717,17 +6785,12 @@ impl<'a, 'b, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32)> for Chann
                }
 
                let chan_features = channel_type.as_ref().unwrap();
-               if chan_features.supports_unknown_bits() || chan_features.requires_unknown_bits() {
+               if !chan_features.is_subset(our_supported_features) {
                        // If the channel was written by a new version and negotiated with features we don't
                        // understand yet, refuse to read it.
                        return Err(DecodeError::UnknownRequiredFeature);
                }
 
-               if channel_parameters.opt_anchors.is_some() {
-                       // Relax this check when ChannelTypeFeatures supports anchors.
-                       return Err(DecodeError::InvalidValue);
-               }
-
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes());
 
@@ -6847,6 +6910,8 @@ impl<'a, 'b, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32)> for Chann
 
                        channel_type: channel_type.unwrap(),
                        channel_keys_id,
+
+                       pending_monitor_updates: Vec::new(),
                })
        }
 }
@@ -6862,6 +6927,8 @@ mod tests {
        use hex;
        use crate::ln::PaymentHash;
        use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
+       #[cfg(anchors)]
+       use crate::ln::channel::InitFeatures;
        use crate::ln::channel::{Channel, InboundHTLCOutput, OutboundHTLCOutput, InboundHTLCState, OutboundHTLCState, HTLCCandidate, HTLCInitiator};
        use crate::ln::channel::{MAX_FUNDING_SATOSHIS_NO_WUMBO, TOTAL_BITCOIN_SUPPLY_SATOSHIS, MIN_THEIR_CHAN_RESERVE_SATOSHIS};
        use crate::ln::features::ChannelTypeFeatures;
@@ -6871,7 +6938,7 @@ mod tests {
        use crate::ln::chan_utils::{htlc_success_tx_weight, htlc_timeout_tx_weight};
        use crate::chain::BestBlock;
        use crate::chain::chaininterface::{FeeEstimator, LowerBoundedFeeEstimator, ConfirmationTarget};
-       use crate::chain::keysinterface::{BaseSign, InMemorySigner, EntropySource, SignerProvider};
+       use crate::chain::keysinterface::{ChannelSigner, InMemorySigner, EntropySource, SignerProvider};
        use crate::chain::transaction::OutPoint;
        use crate::util::config::UserConfig;
        use crate::util::enforcing_trait_impls::EnforcingSigner;
@@ -7025,7 +7092,7 @@ mod tests {
                // Make sure A's dust limit is as we expect.
                let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash());
                let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-               let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();
+               let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();
 
                // Node B --> Node A: accept channel, explicitly setting B's dust limit.
                let mut accept_channel_msg = node_b_chan.accept_inbound_channel(0);
@@ -7143,7 +7210,7 @@ mod tests {
                // Create Node B's channel by receiving Node A's open_channel message
                let open_channel_msg = node_a_chan.get_open_channel(chain_hash);
                let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-               let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();
+               let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();
 
                // Node B --> Node A: accept channel
                let accept_channel_msg = node_b_chan.accept_inbound_channel(0);
@@ -7225,12 +7292,12 @@ mod tests {
                // Test that `new_from_req` creates a channel with the correct value for
                // `holder_max_htlc_value_in_flight_msat`, when configured with a valid percentage value,
                // which is set to the lower bound - 1 (2%) of the `channel_value`.
-               let chan_3 = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_init_features(&config_2_percent), &chan_1_open_channel_msg, 7, &config_2_percent, 0, &&logger, 42).unwrap();
+               let chan_3 = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_2_percent), &channelmanager::provided_init_features(&config_2_percent), &chan_1_open_channel_msg, 7, &config_2_percent, 0, &&logger, 42).unwrap();
                let chan_3_value_msat = chan_3.channel_value_satoshis * 1000;
                assert_eq!(chan_3.holder_max_htlc_value_in_flight_msat, (chan_3_value_msat as f64 * 0.02) as u64);
 
                // Test with the upper bound - 1 of valid values (99%).
-               let chan_4 = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_init_features(&config_99_percent), &chan_1_open_channel_msg, 7, &config_99_percent, 0, &&logger, 42).unwrap();
+               let chan_4 = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_99_percent), &channelmanager::provided_init_features(&config_99_percent), &chan_1_open_channel_msg, 7, &config_99_percent, 0, &&logger, 42).unwrap();
                let chan_4_value_msat = chan_4.channel_value_satoshis * 1000;
                assert_eq!(chan_4.holder_max_htlc_value_in_flight_msat, (chan_4_value_msat as f64 * 0.99) as u64);
 
@@ -7249,14 +7316,14 @@ mod tests {
 
                // Test that `new_from_req` uses the lower bound of the configurable percentage values (1%)
                // if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a value less than 1.
-               let chan_7 = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_init_features(&config_0_percent), &chan_1_open_channel_msg, 7, &config_0_percent, 0, &&logger, 42).unwrap();
+               let chan_7 = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_0_percent), &channelmanager::provided_init_features(&config_0_percent), &chan_1_open_channel_msg, 7, &config_0_percent, 0, &&logger, 42).unwrap();
                let chan_7_value_msat = chan_7.channel_value_satoshis * 1000;
                assert_eq!(chan_7.holder_max_htlc_value_in_flight_msat, (chan_7_value_msat as f64 * 0.01) as u64);
 
                // Test that `new_from_req` uses the upper bound of the configurable percentage values
                // (100%) if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a larger value
                // than 100.
-               let chan_8 = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_init_features(&config_101_percent), &chan_1_open_channel_msg, 7, &config_101_percent, 0, &&logger, 42).unwrap();
+               let chan_8 = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_101_percent), &channelmanager::provided_init_features(&config_101_percent), &chan_1_open_channel_msg, 7, &config_101_percent, 0, &&logger, 42).unwrap();
                let chan_8_value_msat = chan_8.channel_value_satoshis * 1000;
                assert_eq!(chan_8.holder_max_htlc_value_in_flight_msat, chan_8_value_msat);
        }
@@ -7306,7 +7373,7 @@ mod tests {
                inbound_node_config.channel_handshake_config.their_channel_reserve_proportional_millionths = (inbound_selected_channel_reserve_perc * 1_000_000.0) as u32;
 
                if outbound_selected_channel_reserve_perc + inbound_selected_channel_reserve_perc < 1.0 {
-                       let chan_inbound_node = Channel::<EnforcingSigner>::new_from_req(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42).unwrap();
+                       let chan_inbound_node = Channel::<EnforcingSigner>::new_from_req(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42).unwrap();
 
                        let expected_inbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.channel_value_satoshis as f64 * inbound_selected_channel_reserve_perc) as u64);
 
@@ -7314,7 +7381,7 @@ mod tests {
                        assert_eq!(chan_inbound_node.counterparty_selected_channel_reserve_satoshis.unwrap(), expected_outbound_selected_chan_reserve);
                } else {
                        // Channel Negotiations failed
-                       let result = Channel::<EnforcingSigner>::new_from_req(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42);
+                       let result = Channel::<EnforcingSigner>::new_from_req(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42);
                        assert!(result.is_err());
                }
        }
@@ -7376,7 +7443,7 @@ mod tests {
                use bitcoin::hashes::hex::FromHex;
                use bitcoin::hash_types::Txid;
                use bitcoin::secp256k1::Message;
-               use crate::chain::keysinterface::BaseSign;
+               use crate::chain::keysinterface::EcdsaChannelSigner;
                use crate::ln::PaymentPreimage;
                use crate::ln::channel::{HTLCOutputInCommitment ,TxCreationKeys};
                use crate::ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
@@ -7390,7 +7457,6 @@ mod tests {
 
                let mut signer = InMemorySigner::new(
                        &secp_ctx,
-                       SecretKey::from_slice(&hex::decode("4242424242424242424242424242424242424242424242424242424242424242").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("30ff4956bbdd3222d44cc5e8a1261dab1e07957bdac5ae88fe3261ef321f3749").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap(),
@@ -8134,7 +8200,164 @@ mod tests {
                open_channel_msg.channel_type = Some(channel_type_features);
                let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
                let res = Channel::<EnforcingSigner>::new_from_req(&feeest, &&keys_provider, &&keys_provider,
-                       node_b_node_id, &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42);
+                       node_b_node_id, &channelmanager::provided_channel_type_features(&config),
+                       &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42);
                assert!(res.is_ok());
        }
+
+       #[cfg(anchors)]
+       #[test]
+       fn test_supports_anchors_zero_htlc_tx_fee() {
+               // Tests that if both sides support and negotiate `anchors_zero_fee_htlc_tx`, it is the
+               // resulting `channel_type`.
+               let secp_ctx = Secp256k1::new();
+               let fee_estimator = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000});
+               let network = Network::Testnet;
+               let keys_provider = test_utils::TestKeysInterface::new(&[42; 32], network);
+               let logger = test_utils::TestLogger::new();
+
+               let node_id_a = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[1; 32]).unwrap());
+               let node_id_b = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[2; 32]).unwrap());
+
+               let mut config = UserConfig::default();
+               config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
+
+               // It is not enough for just the initiator to signal `option_anchors_zero_fee_htlc_tx`, both
+               // need to signal it.
+               let channel_a = Channel::<EnforcingSigner>::new_outbound(
+                       &fee_estimator, &&keys_provider, &&keys_provider, node_id_b,
+                       &channelmanager::provided_init_features(&UserConfig::default()), 10000000, 100000, 42,
+                       &config, 0, 42
+               ).unwrap();
+               assert!(!channel_a.channel_type.supports_anchors_zero_fee_htlc_tx());
+
+               let mut expected_channel_type = ChannelTypeFeatures::empty();
+               expected_channel_type.set_static_remote_key_required();
+               expected_channel_type.set_anchors_zero_fee_htlc_tx_required();
+
+               let channel_a = Channel::<EnforcingSigner>::new_outbound(
+                       &fee_estimator, &&keys_provider, &&keys_provider, node_id_b,
+                       &channelmanager::provided_init_features(&config), 10000000, 100000, 42, &config, 0, 42
+               ).unwrap();
+
+               let open_channel_msg = channel_a.get_open_channel(genesis_block(network).header.block_hash());
+               let channel_b = Channel::<EnforcingSigner>::new_from_req(
+                       &fee_estimator, &&keys_provider, &&keys_provider, node_id_a,
+                       &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config),
+                       &open_channel_msg, 7, &config, 0, &&logger, 42
+               ).unwrap();
+
+               assert_eq!(channel_a.channel_type, expected_channel_type);
+               assert_eq!(channel_b.channel_type, expected_channel_type);
+       }
+
+       #[cfg(anchors)]
+       #[test]
+       fn test_rejects_implicit_simple_anchors() {
+               // Tests that if `option_anchors` is being negotiated implicitly through the intersection of
+               // each side's `InitFeatures`, it is rejected.
+               let secp_ctx = Secp256k1::new();
+               let fee_estimator = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000});
+               let network = Network::Testnet;
+               let keys_provider = test_utils::TestKeysInterface::new(&[42; 32], network);
+               let logger = test_utils::TestLogger::new();
+
+               let node_id_a = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[1; 32]).unwrap());
+               let node_id_b = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[2; 32]).unwrap());
+
+               let config = UserConfig::default();
+
+               // See feature bit assignments: https://github.com/lightning/bolts/blob/master/09-features.md
+               let static_remote_key_required: u64 = 1 << 12;
+               let simple_anchors_required: u64 = 1 << 20;
+               let raw_init_features = static_remote_key_required | simple_anchors_required;
+               let init_features_with_simple_anchors = InitFeatures::from_le_bytes(raw_init_features.to_le_bytes().to_vec());
+
+               let channel_a = Channel::<EnforcingSigner>::new_outbound(
+                       &fee_estimator, &&keys_provider, &&keys_provider, node_id_b,
+                       &channelmanager::provided_init_features(&config), 10000000, 100000, 42, &config, 0, 42
+               ).unwrap();
+
+               // Set `channel_type` to `None` to force the implicit feature negotiation.
+               let mut open_channel_msg = channel_a.get_open_channel(genesis_block(network).header.block_hash());
+               open_channel_msg.channel_type = None;
+
+               // Since A supports both `static_remote_key` and `option_anchors`, but B only accepts
+               // `static_remote_key`, it will fail the channel.
+               let channel_b = Channel::<EnforcingSigner>::new_from_req(
+                       &fee_estimator, &&keys_provider, &&keys_provider, node_id_a,
+                       &channelmanager::provided_channel_type_features(&config), &init_features_with_simple_anchors,
+                       &open_channel_msg, 7, &config, 0, &&logger, 42
+               );
+               assert!(channel_b.is_err());
+       }
+
+       #[cfg(anchors)]
+       #[test]
+       fn test_rejects_simple_anchors_channel_type() {
+               // Tests that if `option_anchors` is being negotiated through the `channel_type` feature,
+               // it is rejected.
+               let secp_ctx = Secp256k1::new();
+               let fee_estimator = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000});
+               let network = Network::Testnet;
+               let keys_provider = test_utils::TestKeysInterface::new(&[42; 32], network);
+               let logger = test_utils::TestLogger::new();
+
+               let node_id_a = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[1; 32]).unwrap());
+               let node_id_b = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[2; 32]).unwrap());
+
+               let config = UserConfig::default();
+
+               // See feature bit assignments: https://github.com/lightning/bolts/blob/master/09-features.md
+               let static_remote_key_required: u64 = 1 << 12;
+               let simple_anchors_required: u64 = 1 << 20;
+               let simple_anchors_raw_features = static_remote_key_required | simple_anchors_required;
+               let simple_anchors_init = InitFeatures::from_le_bytes(simple_anchors_raw_features.to_le_bytes().to_vec());
+               let simple_anchors_channel_type = ChannelTypeFeatures::from_le_bytes(simple_anchors_raw_features.to_le_bytes().to_vec());
+               assert!(simple_anchors_init.requires_unknown_bits());
+               assert!(simple_anchors_channel_type.requires_unknown_bits());
+
+               // First, we'll try to open a channel between A and B where A requests a channel type for
+               // the original `option_anchors` feature (non zero fee htlc tx). This should be rejected by
+               // B as it's not supported by LDK.
+               let channel_a = Channel::<EnforcingSigner>::new_outbound(
+                       &fee_estimator, &&keys_provider, &&keys_provider, node_id_b,
+                       &channelmanager::provided_init_features(&config), 10000000, 100000, 42, &config, 0, 42
+               ).unwrap();
+
+               let mut open_channel_msg = channel_a.get_open_channel(genesis_block(network).header.block_hash());
+               open_channel_msg.channel_type = Some(simple_anchors_channel_type.clone());
+
+               let res = Channel::<EnforcingSigner>::new_from_req(
+                       &fee_estimator, &&keys_provider, &&keys_provider, node_id_a,
+                       &channelmanager::provided_channel_type_features(&config), &simple_anchors_init,
+                       &open_channel_msg, 7, &config, 0, &&logger, 42
+               );
+               assert!(res.is_err());
+
+               // Then, we'll try to open another channel where A requests a channel type for
+               // `anchors_zero_fee_htlc_tx`. B is malicious and tries to downgrade the channel type to the
+               // original `option_anchors` feature, which should be rejected by A as it's not supported by
+               // LDK.
+               let mut channel_a = Channel::<EnforcingSigner>::new_outbound(
+                       &fee_estimator, &&keys_provider, &&keys_provider, node_id_b, &simple_anchors_init,
+                       10000000, 100000, 42, &config, 0, 42
+               ).unwrap();
+
+               let open_channel_msg = channel_a.get_open_channel(genesis_block(network).header.block_hash());
+
+               let channel_b = Channel::<EnforcingSigner>::new_from_req(
+                       &fee_estimator, &&keys_provider, &&keys_provider, node_id_a,
+                       &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config),
+                       &open_channel_msg, 7, &config, 0, &&logger, 42
+               ).unwrap();
+
+               let mut accept_channel_msg = channel_b.get_accept_channel_message();
+               accept_channel_msg.channel_type = Some(simple_anchors_channel_type.clone());
+
+               let res = channel_a.accept_channel(
+                       &accept_channel_msg, &config.channel_handshake_limits, &simple_anchors_init
+               );
+               assert!(res.is_err());
+       }
 }