Combine common fields of `OpenChannel` & `OpenChannelV2` into struct
[rust-lightning] / lightning / src / ln / channel.rs
index cd98418c5204d369759637fc88667b6d4255dca5..e9017d94107f5c0e761134342669d61ebd58c70e 100644 (file)
@@ -267,7 +267,7 @@ enum HTLCUpdateAwaitingACK {
 }
 
 macro_rules! define_state_flags {
-       ($flag_type_doc: expr, $flag_type: ident, [$(($flag_doc: expr, $flag: ident, $value: expr)),+], $extra_flags: expr) => {
+       ($flag_type_doc: expr, $flag_type: ident, [$(($flag_doc: expr, $flag: ident, $value: expr, $get: ident, $set: ident, $clear: ident)),+], $extra_flags: expr) => {
                #[doc = $flag_type_doc]
                #[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq)]
                struct $flag_type(u32);
@@ -296,15 +296,18 @@ macro_rules! define_state_flags {
 
                        #[allow(unused)]
                        fn is_empty(&self) -> bool { self.0 == 0 }
-
                        #[allow(unused)]
                        fn is_set(&self, flag: Self) -> bool { *self & flag == flag }
+                       #[allow(unused)]
+                       fn set(&mut self, flag: Self) { *self |= flag }
+                       #[allow(unused)]
+                       fn clear(&mut self, flag: Self) -> Self { self.0 &= !flag.0; *self }
                }
 
-               impl core::ops::Not for $flag_type {
-                       type Output = Self;
-                       fn not(self) -> Self::Output { Self(!self.0) }
-               }
+               $(
+                       define_state_flags!($flag_type, Self::$flag, $get, $set, $clear);
+               )*
+
                impl core::ops::BitOr for $flag_type {
                        type Output = Self;
                        fn bitor(self, rhs: Self) -> Self::Output { Self(self.0 | rhs.0) }
@@ -323,8 +326,28 @@ macro_rules! define_state_flags {
        ($flag_type_doc: expr, $flag_type: ident, $flags: tt) => {
                define_state_flags!($flag_type_doc, $flag_type, $flags, 0);
        };
+       ($flag_type: ident, $flag: expr, $get: ident, $set: ident, $clear: ident) => {
+               impl $flag_type {
+                       #[allow(unused)]
+                       fn $get(&self) -> bool { self.is_set($flag_type::new() | $flag) }
+                       #[allow(unused)]
+                       fn $set(&mut self) { self.set($flag_type::new() | $flag) }
+                       #[allow(unused)]
+                       fn $clear(&mut self) -> Self { self.clear($flag_type::new() | $flag) }
+               }
+       };
        ($flag_type_doc: expr, FUNDED_STATE, $flag_type: ident, $flags: tt) => {
                define_state_flags!($flag_type_doc, $flag_type, $flags, FundedStateFlags::ALL.0);
+
+               define_state_flags!($flag_type, FundedStateFlags::PEER_DISCONNECTED,
+                       is_peer_disconnected, set_peer_disconnected, clear_peer_disconnected);
+               define_state_flags!($flag_type, FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS,
+                       is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress);
+               define_state_flags!($flag_type, FundedStateFlags::REMOTE_SHUTDOWN_SENT,
+                       is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent);
+               define_state_flags!($flag_type, FundedStateFlags::LOCAL_SHUTDOWN_SENT,
+                       is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent);
+
                impl core::ops::BitOr<FundedStateFlags> for $flag_type {
                        type Output = Self;
                        fn bitor(self, rhs: FundedStateFlags) -> Self::Output { Self(self.0 | rhs.0) }
@@ -371,15 +394,19 @@ define_state_flags!(
        "Flags that apply to all [`ChannelState`] variants in which the channel is funded.",
        FundedStateFlags, [
                ("Indicates the remote side is considered \"disconnected\" and no updates are allowed \
-                       until after we've done a `channel_reestablish` dance.", PEER_DISCONNECTED, state_flags::PEER_DISCONNECTED),
+                       until after we've done a `channel_reestablish` dance.", PEER_DISCONNECTED, state_flags::PEER_DISCONNECTED,
+                       is_peer_disconnected, set_peer_disconnected, clear_peer_disconnected),
                ("Indicates the user has told us a `ChannelMonitor` update is pending async persistence \
                        somewhere and we should pause sending any outbound messages until they've managed to \
-                       complete it.", MONITOR_UPDATE_IN_PROGRESS, state_flags::MONITOR_UPDATE_IN_PROGRESS),
+                       complete it.", MONITOR_UPDATE_IN_PROGRESS, state_flags::MONITOR_UPDATE_IN_PROGRESS,
+                       is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress),
                ("Indicates we received a `shutdown` message from the remote end. If set, they may not add \
                        any new HTLCs to the channel, and we are expected to respond with our own `shutdown` \
-                       message when possible.", REMOTE_SHUTDOWN_SENT, state_flags::REMOTE_SHUTDOWN_SENT),
+                       message when possible.", REMOTE_SHUTDOWN_SENT, state_flags::REMOTE_SHUTDOWN_SENT,
+                       is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent),
                ("Indicates we sent a `shutdown` message. At this point, we may not add any new HTLCs to \
-                       the channel.", LOCAL_SHUTDOWN_SENT, state_flags::LOCAL_SHUTDOWN_SENT)
+                       the channel.", LOCAL_SHUTDOWN_SENT, state_flags::LOCAL_SHUTDOWN_SENT,
+                       is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent)
        ]
 );
 
@@ -387,9 +414,9 @@ define_state_flags!(
        "Flags that only apply to [`ChannelState::NegotiatingFunding`].",
        NegotiatingFundingFlags, [
                ("Indicates we have (or are prepared to) send our `open_channel`/`accept_channel` message.",
-                       OUR_INIT_SENT, state_flags::OUR_INIT_SENT),
+                       OUR_INIT_SENT, state_flags::OUR_INIT_SENT, is_our_init_sent, set_our_init_sent, clear_our_init_sent),
                ("Indicates we have received their `open_channel`/`accept_channel` message.",
-                       THEIR_INIT_SENT, state_flags::THEIR_INIT_SENT)
+                       THEIR_INIT_SENT, state_flags::THEIR_INIT_SENT, is_their_init_sent, set_their_init_sent, clear_their_init_sent)
        ]
 );
 
@@ -398,13 +425,16 @@ define_state_flags!(
        FUNDED_STATE, AwaitingChannelReadyFlags, [
                ("Indicates they sent us a `channel_ready` message. Once both `THEIR_CHANNEL_READY` and \
                        `OUR_CHANNEL_READY` are set, our state moves on to `ChannelReady`.",
-                       THEIR_CHANNEL_READY, state_flags::THEIR_CHANNEL_READY),
+                       THEIR_CHANNEL_READY, state_flags::THEIR_CHANNEL_READY,
+                       is_their_channel_ready, set_their_channel_ready, clear_their_channel_ready),
                ("Indicates we sent them a `channel_ready` message. Once both `THEIR_CHANNEL_READY` and \
                        `OUR_CHANNEL_READY` are set, our state moves on to `ChannelReady`.",
-                       OUR_CHANNEL_READY, state_flags::OUR_CHANNEL_READY),
+                       OUR_CHANNEL_READY, state_flags::OUR_CHANNEL_READY,
+                       is_our_channel_ready, set_our_channel_ready, clear_our_channel_ready),
                ("Indicates the channel was funded in a batch and the broadcast of the funding transaction \
                        is being held until all channels in the batch have received `funding_signed` and have \
-                       their monitors persisted.", WAITING_FOR_BATCH, state_flags::WAITING_FOR_BATCH)
+                       their monitors persisted.", WAITING_FOR_BATCH, state_flags::WAITING_FOR_BATCH,
+                       is_waiting_for_batch, set_waiting_for_batch, clear_waiting_for_batch)
        ]
 );
 
@@ -415,10 +445,13 @@ define_state_flags!(
                        `revoke_and_ack` message. During this period, we can't generate new `commitment_signed` \
                        messages as we'd be unable to determine which HTLCs they included in their `revoke_and_ack` \
                        implicit ACK, so instead we have to hold them away temporarily to be sent later.",
-                       AWAITING_REMOTE_REVOKE, state_flags::AWAITING_REMOTE_REVOKE)
+                       AWAITING_REMOTE_REVOKE, state_flags::AWAITING_REMOTE_REVOKE,
+                       is_awaiting_remote_revoke, set_awaiting_remote_revoke, clear_awaiting_remote_revoke)
        ]
 );
 
+// Note that the order of this enum is implicitly defined by where each variant is placed. Take this
+// into account when introducing new states and update `test_channel_state_order` accordingly.
 #[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq)]
 enum ChannelState {
        /// We are negotiating the parameters required for the channel prior to funding it.
@@ -439,12 +472,12 @@ enum ChannelState {
 }
 
 macro_rules! impl_state_flag {
-       ($get: ident, $set: ident, $clear: ident, $state_flag: expr, [$($state: ident),+]) => {
+       ($get: ident, $set: ident, $clear: ident, [$($state: ident),+]) => {
                #[allow(unused)]
                fn $get(&self) -> bool {
                        match self {
                                $(
-                                       ChannelState::$state(flags) => flags.is_set($state_flag.into()),
+                                       ChannelState::$state(flags) => flags.$get(),
                                )*
                                _ => false,
                        }
@@ -453,7 +486,7 @@ macro_rules! impl_state_flag {
                fn $set(&mut self) {
                        match self {
                                $(
-                                       ChannelState::$state(flags) => *flags |= $state_flag,
+                                       ChannelState::$state(flags) => flags.$set(),
                                )*
                                _ => debug_assert!(false, "Attempted to set flag on unexpected ChannelState"),
                        }
@@ -462,17 +495,17 @@ macro_rules! impl_state_flag {
                fn $clear(&mut self) {
                        match self {
                                $(
-                                       ChannelState::$state(flags) => *flags &= !($state_flag),
+                                       ChannelState::$state(flags) => { let _ = flags.$clear(); },
                                )*
                                _ => debug_assert!(false, "Attempted to clear flag on unexpected ChannelState"),
                        }
                }
        };
-       ($get: ident, $set: ident, $clear: ident, $state_flag: expr, FUNDED_STATES) => {
-               impl_state_flag!($get, $set, $clear, $state_flag, [AwaitingChannelReady, ChannelReady]);
+       ($get: ident, $set: ident, $clear: ident, FUNDED_STATES) => {
+               impl_state_flag!($get, $set, $clear, [AwaitingChannelReady, ChannelReady]);
        };
-       ($get: ident, $set: ident, $clear: ident, $state_flag: expr, $state: ident) => {
-               impl_state_flag!($get, $set, $clear, $state_flag, [$state]);
+       ($get: ident, $set: ident, $clear: ident, $state: ident) => {
+               impl_state_flag!($get, $set, $clear, [$state]);
        };
 }
 
@@ -523,35 +556,27 @@ impl ChannelState {
                }
        }
 
-       fn should_force_holding_cell(&self) -> bool {
+       fn can_generate_new_commitment(&self) -> bool {
                match self {
                        ChannelState::ChannelReady(flags) =>
-                               flags.is_set(ChannelReadyFlags::AWAITING_REMOTE_REVOKE) ||
-                                       flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into()) ||
-                                       flags.is_set(FundedStateFlags::PEER_DISCONNECTED.into()),
+                               !flags.is_set(ChannelReadyFlags::AWAITING_REMOTE_REVOKE) &&
+                                       !flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into()) &&
+                                       !flags.is_set(FundedStateFlags::PEER_DISCONNECTED.into()),
                        _ => {
-                               debug_assert!(false, "The holding cell is only valid within ChannelReady");
+                               debug_assert!(false, "Can only generate new commitment within ChannelReady");
                                false
                        },
                }
        }
 
-       impl_state_flag!(is_peer_disconnected, set_peer_disconnected, clear_peer_disconnected,
-               FundedStateFlags::PEER_DISCONNECTED, FUNDED_STATES);
-       impl_state_flag!(is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress,
-               FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS, FUNDED_STATES);
-       impl_state_flag!(is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent,
-               FundedStateFlags::LOCAL_SHUTDOWN_SENT, FUNDED_STATES);
-       impl_state_flag!(is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent,
-               FundedStateFlags::REMOTE_SHUTDOWN_SENT, FUNDED_STATES);
-       impl_state_flag!(is_our_channel_ready, set_our_channel_ready, clear_our_channel_ready,
-               AwaitingChannelReadyFlags::OUR_CHANNEL_READY, AwaitingChannelReady);
-       impl_state_flag!(is_their_channel_ready, set_their_channel_ready, clear_their_channel_ready,
-               AwaitingChannelReadyFlags::THEIR_CHANNEL_READY, AwaitingChannelReady);
-       impl_state_flag!(is_waiting_for_batch, set_waiting_for_batch, clear_waiting_for_batch,
-               AwaitingChannelReadyFlags::WAITING_FOR_BATCH, AwaitingChannelReady);
-       impl_state_flag!(is_awaiting_remote_revoke, set_awaiting_remote_revoke, clear_awaiting_remote_revoke,
-               ChannelReadyFlags::AWAITING_REMOTE_REVOKE, ChannelReady);
+       impl_state_flag!(is_peer_disconnected, set_peer_disconnected, clear_peer_disconnected, FUNDED_STATES);
+       impl_state_flag!(is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress, FUNDED_STATES);
+       impl_state_flag!(is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent, FUNDED_STATES);
+       impl_state_flag!(is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent, FUNDED_STATES);
+       impl_state_flag!(is_our_channel_ready, set_our_channel_ready, clear_our_channel_ready, AwaitingChannelReady);
+       impl_state_flag!(is_their_channel_ready, set_their_channel_ready, clear_their_channel_ready, AwaitingChannelReady);
+       impl_state_flag!(is_waiting_for_batch, set_waiting_for_batch, clear_waiting_for_batch, AwaitingChannelReady);
+       impl_state_flag!(is_awaiting_remote_revoke, set_awaiting_remote_revoke, clear_awaiting_remote_revoke, ChannelReady);
 }
 
 pub const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1;
@@ -814,15 +839,20 @@ pub(super) struct ReestablishResponses {
 /// The result of a shutdown that should be handled.
 #[must_use]
 pub(crate) struct ShutdownResult {
+       pub(crate) closure_reason: ClosureReason,
        /// A channel monitor update to apply.
-       pub(crate) monitor_update: Option<(PublicKey, OutPoint, ChannelMonitorUpdate)>,
+       pub(crate) monitor_update: Option<(PublicKey, OutPoint, ChannelId, ChannelMonitorUpdate)>,
        /// A list of dropped outbound HTLCs that can safely be failed backwards immediately.
        pub(crate) dropped_outbound_htlcs: Vec<(HTLCSource, PaymentHash, PublicKey, ChannelId)>,
        /// An unbroadcasted batch funding transaction id. The closure of this channel should be
        /// propagated to the remainder of the batch.
        pub(crate) unbroadcasted_batch_funding_txid: Option<Txid>,
        pub(crate) channel_id: ChannelId,
+       pub(crate) user_channel_id: u128,
+       pub(crate) channel_capacity_satoshis: u64,
        pub(crate) counterparty_node_id: PublicKey,
+       pub(crate) unbroadcasted_funding_tx: Option<Transaction>,
+       pub(crate) channel_funding_txo: Option<OutPoint>,
 }
 
 /// If the majority of the channels funds are to the fundee and the initiator holds only just
@@ -2311,15 +2341,17 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                res
        }
 
-       fn if_unbroadcasted_funding<F, O>(&self, f: F) -> Option<O>
-               where F: Fn() -> Option<O> {
+       fn if_unbroadcasted_funding<F, O>(&self, f: F) -> Option<O> where F: Fn() -> Option<O> {
                match self.channel_state {
                        ChannelState::FundingNegotiated => f(),
-                       ChannelState::AwaitingChannelReady(flags) => if flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH) {
-                               f()
-                       } else {
-                               None
-                       },
+                       ChannelState::AwaitingChannelReady(flags) =>
+                               if flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH) ||
+                                       flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into())
+                               {
+                                       f()
+                               } else {
+                                       None
+                               },
                        _ => None,
                }
        }
@@ -2354,7 +2386,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
        /// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
        /// Also returns the list of payment_hashes for channels which we can safely fail backwards
        /// immediately (others we will have to allow to time out).
-       pub fn force_shutdown(&mut self, should_broadcast: bool) -> ShutdownResult {
+       pub fn force_shutdown(&mut self, should_broadcast: bool, closure_reason: ClosureReason) -> ShutdownResult {
                // Note that we MUST only generate a monitor update that indicates force-closure - we're
                // called during initialization prior to the chain_monitor in the encompassing ChannelManager
                // being fully configured in some cases. Thus, its likely any monitor events we generate will
@@ -2381,29 +2413,32 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
                        // funding transaction, don't return a funding txo (which prevents providing the
                        // monitor update to the user, even if we return one).
                        // See test_duplicate_chan_id and test_pre_lockin_no_chan_closed_update for more.
-                       let generate_monitor_update = match self.channel_state {
-                               ChannelState::AwaitingChannelReady(_)|ChannelState::ChannelReady(_)|ChannelState::ShutdownComplete => true,
-                               _ => false,
-                       };
-                       if generate_monitor_update {
+                       if !self.channel_state.is_pre_funded_state() {
                                self.latest_monitor_update_id = CLOSED_CHANNEL_UPDATE_ID;
-                               Some((self.get_counterparty_node_id(), funding_txo, ChannelMonitorUpdate {
+                               Some((self.get_counterparty_node_id(), funding_txo, self.channel_id(), ChannelMonitorUpdate {
                                        update_id: self.latest_monitor_update_id,
                                        counterparty_node_id: Some(self.counterparty_node_id),
                                        updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }],
+                                       channel_id: Some(self.channel_id()),
                                }))
                        } else { None }
                } else { None };
                let unbroadcasted_batch_funding_txid = self.unbroadcasted_batch_funding_txid();
+               let unbroadcasted_funding_tx = self.unbroadcasted_funding();
 
                self.channel_state = ChannelState::ShutdownComplete;
                self.update_time_counter += 1;
                ShutdownResult {
+                       closure_reason,
                        monitor_update,
                        dropped_outbound_htlcs,
                        unbroadcasted_batch_funding_txid,
                        channel_id: self.channel_id,
+                       user_channel_id: self.user_id,
+                       channel_capacity_satoshis: self.channel_value_satoshis,
                        counterparty_node_id: self.counterparty_node_id,
+                       unbroadcasted_funding_tx,
+                       channel_funding_txo: self.get_funding_txo(),
                }
        }
 
@@ -2544,26 +2579,24 @@ impl FailHTLCContents for msgs::OnionErrorPacket {
                HTLCUpdateAwaitingACK::FailHTLC { htlc_id, err_packet: self }
        }
 }
-impl FailHTLCContents for (u16, [u8; 32]) {
-       type Message = msgs::UpdateFailMalformedHTLC; // (failure_code, sha256_of_onion)
+impl FailHTLCContents for ([u8; 32], u16) {
+       type Message = msgs::UpdateFailMalformedHTLC;
        fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message {
                msgs::UpdateFailMalformedHTLC {
                        htlc_id,
                        channel_id,
-                       failure_code: self.0,
-                       sha256_of_onion: self.1
+                       sha256_of_onion: self.0,
+                       failure_code: self.1
                }
        }
        fn to_inbound_htlc_state(self) -> InboundHTLCState {
-               InboundHTLCState::LocalRemoved(
-                       InboundHTLCRemovalReason::FailMalformed((self.1, self.0))
-               )
+               InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed(self))
        }
        fn to_htlc_update_awaiting_ack(self, htlc_id: u64) -> HTLCUpdateAwaitingACK {
                HTLCUpdateAwaitingACK::FailMalformedHTLC {
                        htlc_id,
-                       failure_code: self.0,
-                       sha256_of_onion: self.1
+                       sha256_of_onion: self.0,
+                       failure_code: self.1
                }
        }
 }
@@ -2698,7 +2731,7 @@ impl<SP: Deref> Channel<SP> where
        where L::Target: Logger {
                // Assert that we'll add the HTLC claim to the holding cell in `get_update_fulfill_htlc`
                // (see equivalent if condition there).
-               assert!(self.context.channel_state.should_force_holding_cell());
+               assert!(!self.context.channel_state.can_generate_new_commitment());
                let mon_update_id = self.context.latest_monitor_update_id; // Forget the ChannelMonitor update
                let fulfill_resp = self.get_update_fulfill_htlc(htlc_id_arg, payment_preimage_arg, logger);
                self.context.latest_monitor_update_id = mon_update_id;
@@ -2766,9 +2799,10 @@ impl<SP: Deref> Channel<SP> where
                        updates: vec![ChannelMonitorUpdateStep::PaymentPreimage {
                                payment_preimage: payment_preimage_arg.clone(),
                        }],
+                       channel_id: Some(self.context.channel_id()),
                };
 
-               if self.context.channel_state.should_force_holding_cell() {
+               if !self.context.channel_state.can_generate_new_commitment() {
                        // Note that this condition is the same as the assertion in
                        // `claim_htlc_while_disconnected_dropping_mon_update` and must match exactly -
                        // `claim_htlc_while_disconnected_dropping_mon_update` would not work correctly if we
@@ -2890,7 +2924,7 @@ impl<SP: Deref> Channel<SP> where
        pub fn queue_fail_malformed_htlc<L: Deref>(
                &mut self, htlc_id_arg: u64, failure_code: u16, sha256_of_onion: [u8; 32], logger: &L
        ) -> Result<(), ChannelError> where L::Target: Logger {
-               self.fail_htlc(htlc_id_arg, (failure_code, sha256_of_onion), true, logger)
+               self.fail_htlc(htlc_id_arg, (sha256_of_onion, failure_code), true, logger)
                        .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?"))
        }
 
@@ -2903,7 +2937,7 @@ impl<SP: Deref> Channel<SP> where
        /// return `Ok(_)` if preconditions are met. In any case, `Err`s will only be
        /// [`ChannelError::Ignore`].
        fn fail_htlc<L: Deref, E: FailHTLCContents + Clone>(
-               &mut self, htlc_id_arg: u64, err_packet: E, mut force_holding_cell: bool,
+               &mut self, htlc_id_arg: u64, err_contents: E, mut force_holding_cell: bool,
                logger: &L
        ) -> Result<Option<E::Message>, ChannelError> where L::Target: Logger {
                if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) {
@@ -2942,7 +2976,7 @@ impl<SP: Deref> Channel<SP> where
                        return Ok(None);
                }
 
-               if self.context.channel_state.should_force_holding_cell() {
+               if !self.context.channel_state.can_generate_new_commitment() {
                        debug_assert!(force_holding_cell, "!force_holding_cell is only called when emptying the holding cell, so we shouldn't end up back in it!");
                        force_holding_cell = true;
                }
@@ -2970,7 +3004,7 @@ impl<SP: Deref> Channel<SP> where
                                }
                        }
                        log_trace!(logger, "Placing failure for HTLC ID {} in holding cell in channel {}.", htlc_id_arg, &self.context.channel_id());
-                       self.context.holding_cell_htlc_updates.push(err_packet.to_htlc_update_awaiting_ack(htlc_id_arg));
+                       self.context.holding_cell_htlc_updates.push(err_contents.to_htlc_update_awaiting_ack(htlc_id_arg));
                        return Ok(None);
                }
 
@@ -2978,10 +3012,10 @@ impl<SP: Deref> Channel<SP> where
                        E::Message::name(), &self.context.channel_id());
                {
                        let htlc = &mut self.context.pending_inbound_htlcs[pending_idx];
-                       htlc.state = err_packet.clone().to_inbound_htlc_state();
+                       htlc.state = err_contents.clone().to_inbound_htlc_state();
                }
 
-               Ok(Some(err_packet.to_message(htlc_id_arg, self.context.channel_id())))
+               Ok(Some(err_contents.to_message(htlc_id_arg, self.context.channel_id())))
        }
 
        // Message handlers:
@@ -3038,12 +3072,12 @@ impl<SP: Deref> Channel<SP> where
                let mut check_reconnection = false;
                match &self.context.channel_state {
                        ChannelState::AwaitingChannelReady(flags) => {
-                               let flags = *flags & !FundedStateFlags::ALL;
+                               let flags = flags.clone().clear(FundedStateFlags::ALL.into());
                                debug_assert!(!flags.is_set(AwaitingChannelReadyFlags::OUR_CHANNEL_READY) || !flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH));
-                               if flags & !AwaitingChannelReadyFlags::WAITING_FOR_BATCH == AwaitingChannelReadyFlags::THEIR_CHANNEL_READY {
+                               if flags.clone().clear(AwaitingChannelReadyFlags::WAITING_FOR_BATCH) == AwaitingChannelReadyFlags::THEIR_CHANNEL_READY {
                                        // If we reconnected before sending our `channel_ready` they may still resend theirs.
                                        check_reconnection = true;
-                               } else if (flags & !AwaitingChannelReadyFlags::WAITING_FOR_BATCH).is_empty() {
+                               } else if flags.clone().clear(AwaitingChannelReadyFlags::WAITING_FOR_BATCH).is_empty() {
                                        self.context.channel_state.set_their_channel_ready();
                                } else if flags == AwaitingChannelReadyFlags::OUR_CHANNEL_READY {
                                        self.context.channel_state = ChannelState::ChannelReady(self.context.channel_state.with_funded_state_flags_mask().into());
@@ -3293,7 +3327,7 @@ impl<SP: Deref> Channel<SP> where
                Err(ChannelError::Close("Remote tried to fulfill/fail an HTLC we couldn't find".to_owned()))
        }
 
-       pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<(HTLCSource, u64), ChannelError> {
+       pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<(HTLCSource, u64, Option<u64>), ChannelError> {
                if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) {
                        return Err(ChannelError::Close("Got fulfill HTLC message when channel was not in an operational state".to_owned()));
                }
@@ -3301,7 +3335,7 @@ impl<SP: Deref> Channel<SP> where
                        return Err(ChannelError::Close("Peer sent update_fulfill_htlc when we needed a channel_reestablish".to_owned()));
                }
 
-               self.mark_outbound_htlc_removed(msg.htlc_id, Some(msg.payment_preimage), None).map(|htlc| (htlc.source.clone(), htlc.amount_msat))
+               self.mark_outbound_htlc_removed(msg.htlc_id, Some(msg.payment_preimage), None).map(|htlc| (htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat))
        }
 
        pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> {
@@ -3504,7 +3538,8 @@ impl<SP: Deref> Channel<SP> where
                                htlc_outputs: htlcs_and_sigs,
                                claimed_htlcs,
                                nondust_htlc_sources,
-                       }]
+                       }],
+                       channel_id: Some(self.context.channel_id()),
                };
 
                self.context.cur_holder_commitment_transaction_number -= 1;
@@ -3559,7 +3594,7 @@ impl<SP: Deref> Channel<SP> where
        ) -> (Option<ChannelMonitorUpdate>, Vec<(HTLCSource, PaymentHash)>)
        where F::Target: FeeEstimator, L::Target: Logger
        {
-               if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) && !self.context.channel_state.should_force_holding_cell() {
+               if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) && self.context.channel_state.can_generate_new_commitment() {
                        self.free_holding_cell_htlcs(fee_estimator, logger)
                } else { (None, Vec::new()) }
        }
@@ -3580,6 +3615,7 @@ impl<SP: Deref> Channel<SP> where
                                update_id: self.context.latest_monitor_update_id + 1, // We don't increment this yet!
                                counterparty_node_id: Some(self.context.counterparty_node_id),
                                updates: Vec::new(),
+                               channel_id: Some(self.context.channel_id()),
                        };
 
                        let mut htlc_updates = Vec::new();
@@ -3594,7 +3630,7 @@ impl<SP: Deref> Channel<SP> where
                                // the limit. In case it's less rare than I anticipate, we may want to revisit
                                // handling this case better and maybe fulfilling some of the HTLCs while attempting
                                // to rebalance channels.
-                               match &htlc_update {
+                               let fail_htlc_res = match &htlc_update {
                                        &HTLCUpdateAwaitingACK::AddHTLC {
                                                amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet,
                                                skimmed_fee_msat, blinding_point, ..
@@ -3622,6 +3658,7 @@ impl<SP: Deref> Channel<SP> where
                                                                }
                                                        }
                                                }
+                                               None
                                        },
                                        &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => {
                                                // If an HTLC claim was previously added to the holding cell (via
@@ -3635,40 +3672,33 @@ impl<SP: Deref> Channel<SP> where
                                                        { monitor_update } else { unreachable!() };
                                                update_fulfill_count += 1;
                                                monitor_update.updates.append(&mut additional_monitor_update.updates);
+                                               None
                                        },
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => {
-                                               match self.fail_htlc(htlc_id, err_packet.clone(), false, logger) {
-                                                       Ok(update_fail_msg_option) => {
-                                                               // If an HTLC failure was previously added to the holding cell (via
-                                                               // `queue_fail_htlc`) then generating the fail message itself must
-                                                               // not fail - we should never end up in a state where we double-fail
-                                                               // an HTLC or fail-then-claim an HTLC as it indicates we didn't wait
-                                                               // for a full revocation before failing.
-                                                               debug_assert!(update_fail_msg_option.is_some());
-                                                               update_fail_count += 1;
-                                                       },
-                                                       Err(e) => {
-                                                               if let ChannelError::Ignore(_) = e {}
-                                                               else {
-                                                                       panic!("Got a non-IgnoreError action trying to fail holding cell HTLC");
-                                                               }
-                                                       }
-                                               }
+                                               Some(self.fail_htlc(htlc_id, err_packet.clone(), false, logger)
+                                                .map(|fail_msg_opt| fail_msg_opt.map(|_| ())))
                                        },
                                        &HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => {
-                                               match self.fail_htlc(htlc_id, (failure_code, sha256_of_onion), false, logger) {
-                                                       Ok(update_fail_malformed_opt) => {
-                                                               debug_assert!(update_fail_malformed_opt.is_some()); // See above comment
-                                                               update_fail_count += 1;
-                                                       },
-                                                       Err(e) => {
-                                                               if let ChannelError::Ignore(_) = e {}
-                                                               else {
-                                                                       panic!("Got a non-IgnoreError action trying to fail holding cell HTLC");
-                                                               }
-                                                       }
-                                               }
-                                       },
+                                               Some(self.fail_htlc(htlc_id, (sha256_of_onion, failure_code), false, logger)
+                                                .map(|fail_msg_opt| fail_msg_opt.map(|_| ())))
+                                       }
+                               };
+                               if let Some(res) = fail_htlc_res {
+                                       match res {
+                                               Ok(fail_msg_opt) => {
+                                                       // If an HTLC failure was previously added to the holding cell (via
+                                                       // `queue_fail_{malformed_}htlc`) then generating the fail message itself must
+                                                       // not fail - we should never end up in a state where we double-fail
+                                                       // an HTLC or fail-then-claim an HTLC as it indicates we didn't wait
+                                                       // for a full revocation before failing.
+                                                       debug_assert!(fail_msg_opt.is_some());
+                                                       update_fail_count += 1;
+                                               },
+                                               Err(ChannelError::Ignore(_)) => {},
+                                               Err(_) => {
+                                                       panic!("Got a non-IgnoreError action trying to fail holding cell HTLC");
+                                               },
+                                       }
                                }
                        }
                        if update_add_count == 0 && update_fulfill_count == 0 && update_fail_count == 0 && self.context.holding_cell_update_fee.is_none() {
@@ -3764,6 +3794,7 @@ impl<SP: Deref> Channel<SP> where
                                idx: self.context.cur_counterparty_commitment_transaction_number + 1,
                                secret: msg.per_commitment_secret,
                        }],
+                       channel_id: Some(self.context.channel_id()),
                };
 
                // Update state now that we've passed all the can-fail calls...
@@ -4175,8 +4206,8 @@ impl<SP: Deref> Channel<SP> where
                // first received the funding_signed.
                let mut funding_broadcastable =
                        if self.context.is_outbound() &&
-                               matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(flags) if !flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH)) ||
-                               matches!(self.context.channel_state, ChannelState::ChannelReady(_))
+                               (matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(flags) if !flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH)) ||
+                               matches!(self.context.channel_state, ChannelState::ChannelReady(_)))
                        {
                                self.context.funding_transaction.take()
                        } else { None };
@@ -4821,6 +4852,7 @@ impl<SP: Deref> Channel<SP> where
                                updates: vec![ChannelMonitorUpdateStep::ShutdownScript {
                                        scriptpubkey: self.get_closing_scriptpubkey(),
                                }],
+                               channel_id: Some(self.context.channel_id()),
                        };
                        self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new());
                        self.push_ret_blockable_mon_update(monitor_update)
@@ -4931,11 +4963,16 @@ impl<SP: Deref> Channel<SP> where
                if let Some((last_fee, sig)) = self.context.last_sent_closing_fee {
                        if last_fee == msg.fee_satoshis {
                                let shutdown_result = ShutdownResult {
+                                       closure_reason: ClosureReason::CooperativeClosure,
                                        monitor_update: None,
                                        dropped_outbound_htlcs: Vec::new(),
                                        unbroadcasted_batch_funding_txid: self.context.unbroadcasted_batch_funding_txid(),
                                        channel_id: self.context.channel_id,
+                                       user_channel_id: self.context.user_id,
+                                       channel_capacity_satoshis: self.context.channel_value_satoshis,
                                        counterparty_node_id: self.context.counterparty_node_id,
+                                       unbroadcasted_funding_tx: self.context.unbroadcasted_funding(),
+                                       channel_funding_txo: self.context.get_funding_txo(),
                                };
                                let tx = self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig);
                                self.context.channel_state = ChannelState::ShutdownComplete;
@@ -4961,11 +4998,16 @@ impl<SP: Deref> Channel<SP> where
                                                        .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?;
                                                let (signed_tx, shutdown_result) = if $new_fee == msg.fee_satoshis {
                                                        let shutdown_result = ShutdownResult {
+                                                               closure_reason: ClosureReason::CooperativeClosure,
                                                                monitor_update: None,
                                                                dropped_outbound_htlcs: Vec::new(),
                                                                unbroadcasted_batch_funding_txid: self.context.unbroadcasted_batch_funding_txid(),
                                                                channel_id: self.context.channel_id,
+                                                               user_channel_id: self.context.user_id,
+                                                               channel_capacity_satoshis: self.context.channel_value_satoshis,
                                                                counterparty_node_id: self.context.counterparty_node_id,
+                                                               unbroadcasted_funding_tx: self.context.unbroadcasted_funding(),
+                                                               channel_funding_txo: self.context.get_funding_txo(),
                                                        };
                                                        self.context.channel_state = ChannelState::ShutdownComplete;
                                                        self.context.update_time_counter += 1;
@@ -5176,7 +5218,7 @@ impl<SP: Deref> Channel<SP> where
                if !self.is_awaiting_monitor_update() { return false; }
                if matches!(
                        self.context.channel_state, ChannelState::AwaitingChannelReady(flags)
-                       if (flags & !(AwaitingChannelReadyFlags::THEIR_CHANNEL_READY | FundedStateFlags::PEER_DISCONNECTED | FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS | AwaitingChannelReadyFlags::WAITING_FOR_BATCH)).is_empty()
+                       if flags.clone().clear(AwaitingChannelReadyFlags::THEIR_CHANNEL_READY | FundedStateFlags::PEER_DISCONNECTED | FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS | AwaitingChannelReadyFlags::WAITING_FOR_BATCH).is_empty()
                ) {
                        // If we're not a 0conf channel, we'll be waiting on a monitor update with only
                        // AwaitingChannelReady set, though our peer could have sent their channel_ready.
@@ -5262,14 +5304,14 @@ impl<SP: Deref> Channel<SP> where
 
                // Note that we don't include ChannelState::WaitingForBatch as we don't want to send
                // channel_ready until the entire batch is ready.
-               let need_commitment_update = if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if (f & !FundedStateFlags::ALL).is_empty()) {
+               let need_commitment_update = if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f.clone().clear(FundedStateFlags::ALL.into()).is_empty()) {
                        self.context.channel_state.set_our_channel_ready();
                        true
-               } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f & !FundedStateFlags::ALL == AwaitingChannelReadyFlags::THEIR_CHANNEL_READY) {
+               } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f.clone().clear(FundedStateFlags::ALL.into()) == AwaitingChannelReadyFlags::THEIR_CHANNEL_READY) {
                        self.context.channel_state = ChannelState::ChannelReady(self.context.channel_state.with_funded_state_flags_mask().into());
                        self.context.update_time_counter += 1;
                        true
-               } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f & !FundedStateFlags::ALL == AwaitingChannelReadyFlags::OUR_CHANNEL_READY) {
+               } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f.clone().clear(FundedStateFlags::ALL.into()) == AwaitingChannelReadyFlags::OUR_CHANNEL_READY) {
                        // We got a reorg but not enough to trigger a force close, just ignore.
                        false
                } else {
@@ -5844,7 +5886,7 @@ impl<SP: Deref> Channel<SP> where
                        return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected from channel counterparty".to_owned()));
                }
 
-               let need_holding_cell = self.context.channel_state.should_force_holding_cell();
+               let need_holding_cell = !self.context.channel_state.can_generate_new_commitment();
                log_debug!(logger, "Pushing new outbound HTLC with hash {} for {} msat {}",
                        payment_hash, amount_msat,
                        if force_holding_cell { "into holding cell" }
@@ -5950,7 +5992,8 @@ impl<SP: Deref> Channel<SP> where
                                feerate_per_kw: Some(counterparty_commitment_tx.feerate_per_kw()),
                                to_broadcaster_value_sat: Some(counterparty_commitment_tx.to_broadcaster_value_sat()),
                                to_countersignatory_value_sat: Some(counterparty_commitment_tx.to_countersignatory_value_sat()),
-                       }]
+                       }],
+                       channel_id: Some(self.context.channel_id()),
                };
                self.context.channel_state.set_awaiting_remote_revoke();
                monitor_update
@@ -6144,6 +6187,7 @@ impl<SP: Deref> Channel<SP> where
                                updates: vec![ChannelMonitorUpdateStep::ShutdownScript {
                                        scriptpubkey: self.get_closing_scriptpubkey(),
                                }],
+                               channel_id: Some(self.context.channel_id()),
                        };
                        self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new());
                        self.push_ret_blockable_mon_update(monitor_update)
@@ -6460,7 +6504,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
                // Now that we're past error-generating stuff, update our local state:
 
                self.context.channel_state = ChannelState::FundingNegotiated;
-               self.context.channel_id = funding_txo.to_channel_id();
+               self.context.channel_id = ChannelId::v1_from_funding_outpoint(funding_txo);
 
                // If the funding transaction is a coinbase transaction, we need to set the minimum depth to 100.
                // We can skip this if it is a zero-conf channel.
@@ -6571,29 +6615,31 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
                let keys = self.context.get_holder_pubkeys();
 
                msgs::OpenChannel {
-                       chain_hash,
-                       temporary_channel_id: self.context.channel_id,
-                       funding_satoshis: self.context.channel_value_satoshis,
+                       common_fields: msgs::CommonOpenChannelFields {
+                               chain_hash,
+                               temporary_channel_id: self.context.channel_id,
+                               funding_satoshis: self.context.channel_value_satoshis,
+                               dust_limit_satoshis: self.context.holder_dust_limit_satoshis,
+                               max_htlc_value_in_flight_msat: self.context.holder_max_htlc_value_in_flight_msat,
+                               htlc_minimum_msat: self.context.holder_htlc_minimum_msat,
+                               commitment_feerate_sat_per_1000_weight: self.context.feerate_per_kw as u32,
+                               to_self_delay: self.context.get_holder_selected_contest_delay(),
+                               max_accepted_htlcs: self.context.holder_max_accepted_htlcs,
+                               funding_pubkey: keys.funding_pubkey,
+                               revocation_basepoint: keys.revocation_basepoint.to_public_key(),
+                               payment_basepoint: keys.payment_point,
+                               delayed_payment_basepoint: keys.delayed_payment_basepoint.to_public_key(),
+                               htlc_basepoint: keys.htlc_basepoint.to_public_key(),
+                               first_per_commitment_point,
+                               channel_flags: if self.context.config.announced_channel {1} else {0},
+                               shutdown_scriptpubkey: Some(match &self.context.shutdown_scriptpubkey {
+                                       Some(script) => script.clone().into_inner(),
+                                       None => Builder::new().into_script(),
+                               }),
+                               channel_type: Some(self.context.channel_type.clone()),
+                       },
                        push_msat: self.context.channel_value_satoshis * 1000 - self.context.value_to_self_msat,
-                       dust_limit_satoshis: self.context.holder_dust_limit_satoshis,
-                       max_htlc_value_in_flight_msat: self.context.holder_max_htlc_value_in_flight_msat,
                        channel_reserve_satoshis: self.context.holder_selected_channel_reserve_satoshis,
-                       htlc_minimum_msat: self.context.holder_htlc_minimum_msat,
-                       feerate_per_kw: self.context.feerate_per_kw as u32,
-                       to_self_delay: self.context.get_holder_selected_contest_delay(),
-                       max_accepted_htlcs: self.context.holder_max_accepted_htlcs,
-                       funding_pubkey: keys.funding_pubkey,
-                       revocation_basepoint: keys.revocation_basepoint.to_public_key(),
-                       payment_point: keys.payment_point,
-                       delayed_payment_basepoint: keys.delayed_payment_basepoint.to_public_key(),
-                       htlc_basepoint: keys.htlc_basepoint.to_public_key(),
-                       first_per_commitment_point,
-                       channel_flags: if self.context.config.announced_channel {1} else {0},
-                       shutdown_scriptpubkey: Some(match &self.context.shutdown_scriptpubkey {
-                               Some(script) => script.clone().into_inner(),
-                               None => Builder::new().into_script(),
-                       }),
-                       channel_type: Some(self.context.channel_type.clone()),
                }
        }
 
@@ -6799,7 +6845,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
                                                          &self.context.channel_transaction_parameters,
                                                          funding_redeemscript.clone(), self.context.channel_value_satoshis,
                                                          obscure_factor,
-                                                         holder_commitment_tx, best_block, self.context.counterparty_node_id);
+                                                         holder_commitment_tx, best_block, self.context.counterparty_node_id, self.context.channel_id());
                channel_monitor.provide_initial_counterparty_commitment_tx(
                        counterparty_initial_bitcoin_tx.txid, Vec::new(),
                        self.context.cur_counterparty_commitment_transaction_number,
@@ -6849,7 +6895,7 @@ pub(super) fn channel_type_from_open_channel(
        msg: &msgs::OpenChannel, their_features: &InitFeatures,
        our_supported_features: &ChannelTypeFeatures
 ) -> Result<ChannelTypeFeatures, ChannelError> {
-       if let Some(channel_type) = &msg.channel_type {
+       if let Some(channel_type) = &msg.common_fields.channel_type {
                if channel_type.supports_any_optional_bits() {
                        return Err(ChannelError::Close("Channel Type field contained optional bits - this is not allowed".to_owned()));
                }
@@ -6864,7 +6910,7 @@ pub(super) fn channel_type_from_open_channel(
                if !channel_type.is_subset(our_supported_features) {
                        return Err(ChannelError::Close("Channel Type contains unsupported features".to_owned()));
                }
-               let announced_channel = if (msg.channel_flags & 1) == 1 { true } else { false };
+               let announced_channel = if (msg.common_fields.channel_flags & 1) == 1 { true } else { false };
                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()));
                }
@@ -6891,22 +6937,22 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                          F::Target: FeeEstimator,
                          L::Target: Logger,
        {
-               let logger = WithContext::from(logger, Some(counterparty_node_id), Some(msg.temporary_channel_id));
-               let announced_channel = if (msg.channel_flags & 1) == 1 { true } else { false };
+               let logger = WithContext::from(logger, Some(counterparty_node_id), Some(msg.common_fields.temporary_channel_id));
+               let announced_channel = if (msg.common_fields.channel_flags & 1) == 1 { true } else { false };
 
                // First check the channel type is known, failing before we do anything else if we don't
                // support this channel type.
                let channel_type = channel_type_from_open_channel(msg, their_features, our_supported_features)?;
 
-               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);
+               let channel_keys_id = signer_provider.generate_channel_keys_id(true, msg.common_fields.funding_satoshis, user_id);
+               let holder_signer = signer_provider.derive_channel_signer(msg.common_fields.funding_satoshis, channel_keys_id);
                let pubkeys = holder_signer.pubkeys().clone();
                let counterparty_pubkeys = ChannelPublicKeys {
-                       funding_pubkey: msg.funding_pubkey,
-                       revocation_basepoint: RevocationBasepoint::from(msg.revocation_basepoint),
-                       payment_point: msg.payment_point,
-                       delayed_payment_basepoint: DelayedPaymentBasepoint::from(msg.delayed_payment_basepoint),
-                       htlc_basepoint: HtlcBasepoint::from(msg.htlc_basepoint)
+                       funding_pubkey: msg.common_fields.funding_pubkey,
+                       revocation_basepoint: RevocationBasepoint::from(msg.common_fields.revocation_basepoint),
+                       payment_point: msg.common_fields.payment_basepoint,
+                       delayed_payment_basepoint: DelayedPaymentBasepoint::from(msg.common_fields.delayed_payment_basepoint),
+                       htlc_basepoint: HtlcBasepoint::from(msg.common_fields.htlc_basepoint)
                };
 
                if config.channel_handshake_config.our_to_self_delay < BREAKDOWN_TIMEOUT {
@@ -6914,59 +6960,59 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                }
 
                // Check sanity of message fields:
-               if msg.funding_satoshis > config.channel_handshake_limits.max_funding_satoshis {
-                       return Err(ChannelError::Close(format!("Per our config, funding must be at most {}. It was {}", config.channel_handshake_limits.max_funding_satoshis, msg.funding_satoshis)));
+               if msg.common_fields.funding_satoshis > config.channel_handshake_limits.max_funding_satoshis {
+                       return Err(ChannelError::Close(format!("Per our config, funding must be at most {}. It was {}", config.channel_handshake_limits.max_funding_satoshis, msg.common_fields.funding_satoshis)));
                }
-               if msg.funding_satoshis >= TOTAL_BITCOIN_SUPPLY_SATOSHIS {
-                       return Err(ChannelError::Close(format!("Funding must be smaller than the total bitcoin supply. It was {}", msg.funding_satoshis)));
+               if msg.common_fields.funding_satoshis >= TOTAL_BITCOIN_SUPPLY_SATOSHIS {
+                       return Err(ChannelError::Close(format!("Funding must be smaller than the total bitcoin supply. It was {}", msg.common_fields.funding_satoshis)));
                }
-               if msg.channel_reserve_satoshis > msg.funding_satoshis {
-                       return Err(ChannelError::Close(format!("Bogus channel_reserve_satoshis ({}). Must be not greater than funding_satoshis: {}", msg.channel_reserve_satoshis, msg.funding_satoshis)));
+               if msg.channel_reserve_satoshis > msg.common_fields.funding_satoshis {
+                       return Err(ChannelError::Close(format!("Bogus channel_reserve_satoshis ({}). Must be not greater than funding_satoshis: {}", msg.channel_reserve_satoshis, msg.common_fields.funding_satoshis)));
                }
-               let full_channel_value_msat = (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000;
+               let full_channel_value_msat = (msg.common_fields.funding_satoshis - msg.channel_reserve_satoshis) * 1000;
                if msg.push_msat > full_channel_value_msat {
                        return Err(ChannelError::Close(format!("push_msat {} was larger than channel amount minus reserve ({})", msg.push_msat, full_channel_value_msat)));
                }
-               if msg.dust_limit_satoshis > msg.funding_satoshis {
-                       return Err(ChannelError::Close(format!("dust_limit_satoshis {} was larger than funding_satoshis {}. Peer never wants payout outputs?", msg.dust_limit_satoshis, msg.funding_satoshis)));
+               if msg.common_fields.dust_limit_satoshis > msg.common_fields.funding_satoshis {
+                       return Err(ChannelError::Close(format!("dust_limit_satoshis {} was larger than funding_satoshis {}. Peer never wants payout outputs?", msg.common_fields.dust_limit_satoshis, msg.common_fields.funding_satoshis)));
                }
-               if msg.htlc_minimum_msat >= full_channel_value_msat {
-                       return Err(ChannelError::Close(format!("Minimum htlc value ({}) was larger than full channel value ({})", msg.htlc_minimum_msat, full_channel_value_msat)));
+               if msg.common_fields.htlc_minimum_msat >= full_channel_value_msat {
+                       return Err(ChannelError::Close(format!("Minimum htlc value ({}) was larger than full channel value ({})", msg.common_fields.htlc_minimum_msat, full_channel_value_msat)));
                }
-               Channel::<SP>::check_remote_fee(&channel_type, fee_estimator, msg.feerate_per_kw, None, &&logger)?;
+               Channel::<SP>::check_remote_fee(&channel_type, fee_estimator, msg.common_fields.commitment_feerate_sat_per_1000_weight, None, &&logger)?;
 
                let max_counterparty_selected_contest_delay = u16::min(config.channel_handshake_limits.their_to_self_delay, MAX_LOCAL_BREAKDOWN_TIMEOUT);
-               if msg.to_self_delay > max_counterparty_selected_contest_delay {
-                       return Err(ChannelError::Close(format!("They wanted our payments to be delayed by a needlessly long period. Upper limit: {}. Actual: {}", max_counterparty_selected_contest_delay, msg.to_self_delay)));
+               if msg.common_fields.to_self_delay > max_counterparty_selected_contest_delay {
+                       return Err(ChannelError::Close(format!("They wanted our payments to be delayed by a needlessly long period. Upper limit: {}. Actual: {}", max_counterparty_selected_contest_delay, msg.common_fields.to_self_delay)));
                }
-               if msg.max_accepted_htlcs < 1 {
+               if msg.common_fields.max_accepted_htlcs < 1 {
                        return Err(ChannelError::Close("0 max_accepted_htlcs makes for a useless channel".to_owned()));
                }
-               if msg.max_accepted_htlcs > MAX_HTLCS {
-                       return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than {}", msg.max_accepted_htlcs, MAX_HTLCS)));
+               if msg.common_fields.max_accepted_htlcs > MAX_HTLCS {
+                       return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than {}", msg.common_fields.max_accepted_htlcs, MAX_HTLCS)));
                }
 
                // Now check against optional parameters as set by config...
-               if msg.funding_satoshis < config.channel_handshake_limits.min_funding_satoshis {
-                       return Err(ChannelError::Close(format!("Funding satoshis ({}) is less than the user specified limit ({})", msg.funding_satoshis, config.channel_handshake_limits.min_funding_satoshis)));
+               if msg.common_fields.funding_satoshis < config.channel_handshake_limits.min_funding_satoshis {
+                       return Err(ChannelError::Close(format!("Funding satoshis ({}) is less than the user specified limit ({})", msg.common_fields.funding_satoshis, config.channel_handshake_limits.min_funding_satoshis)));
                }
-               if msg.htlc_minimum_msat > config.channel_handshake_limits.max_htlc_minimum_msat {
-                       return Err(ChannelError::Close(format!("htlc_minimum_msat ({}) is higher than the user specified limit ({})", msg.htlc_minimum_msat,  config.channel_handshake_limits.max_htlc_minimum_msat)));
+               if msg.common_fields.htlc_minimum_msat > config.channel_handshake_limits.max_htlc_minimum_msat {
+                       return Err(ChannelError::Close(format!("htlc_minimum_msat ({}) is higher than the user specified limit ({})", msg.common_fields.htlc_minimum_msat,  config.channel_handshake_limits.max_htlc_minimum_msat)));
                }
-               if msg.max_htlc_value_in_flight_msat < config.channel_handshake_limits.min_max_htlc_value_in_flight_msat {
-                       return Err(ChannelError::Close(format!("max_htlc_value_in_flight_msat ({}) is less than the user specified limit ({})", msg.max_htlc_value_in_flight_msat, config.channel_handshake_limits.min_max_htlc_value_in_flight_msat)));
+               if msg.common_fields.max_htlc_value_in_flight_msat < config.channel_handshake_limits.min_max_htlc_value_in_flight_msat {
+                       return Err(ChannelError::Close(format!("max_htlc_value_in_flight_msat ({}) is less than the user specified limit ({})", msg.common_fields.max_htlc_value_in_flight_msat, config.channel_handshake_limits.min_max_htlc_value_in_flight_msat)));
                }
                if msg.channel_reserve_satoshis > config.channel_handshake_limits.max_channel_reserve_satoshis {
                        return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is higher than the user specified limit ({})", msg.channel_reserve_satoshis, config.channel_handshake_limits.max_channel_reserve_satoshis)));
                }
-               if msg.max_accepted_htlcs < config.channel_handshake_limits.min_max_accepted_htlcs {
-                       return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, config.channel_handshake_limits.min_max_accepted_htlcs)));
+               if msg.common_fields.max_accepted_htlcs < config.channel_handshake_limits.min_max_accepted_htlcs {
+                       return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.common_fields.max_accepted_htlcs, config.channel_handshake_limits.min_max_accepted_htlcs)));
                }
-               if msg.dust_limit_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS {
-                       return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.dust_limit_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS)));
+               if msg.common_fields.dust_limit_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS {
+                       return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.common_fields.dust_limit_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS)));
                }
-               if msg.dust_limit_satoshis >  MAX_CHAN_DUST_LIMIT_SATOSHIS {
-                       return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.dust_limit_satoshis, MAX_CHAN_DUST_LIMIT_SATOSHIS)));
+               if msg.common_fields.dust_limit_satoshis >  MAX_CHAN_DUST_LIMIT_SATOSHIS {
+                       return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.common_fields.dust_limit_satoshis, MAX_CHAN_DUST_LIMIT_SATOSHIS)));
                }
 
                // Convert things into internal flags and prep our state:
@@ -6977,7 +7023,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                        }
                }
 
-               let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(msg.funding_satoshis, config);
+               let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(msg.common_fields.funding_satoshis, config);
                if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS {
                        // Protocol level safety check in place, although it should never happen because
                        // of `MIN_THEIR_CHAN_RESERVE_SATOSHIS`
@@ -6990,8 +7036,8 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                        log_debug!(logger, "channel_reserve_satoshis ({}) is smaller than our dust limit ({}). We can broadcast stale states without any risk, implying this channel is very insecure for our counterparty.",
                                msg.channel_reserve_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS);
                }
-               if holder_selected_channel_reserve_satoshis < msg.dust_limit_satoshis {
-                       return Err(ChannelError::Close(format!("Dust limit ({}) too high for the channel reserve we require the remote to keep ({})", msg.dust_limit_satoshis, holder_selected_channel_reserve_satoshis)));
+               if holder_selected_channel_reserve_satoshis < msg.common_fields.dust_limit_satoshis {
+                       return Err(ChannelError::Close(format!("Dust limit ({}) too high for the channel reserve we require the remote to keep ({})", msg.common_fields.dust_limit_satoshis, holder_selected_channel_reserve_satoshis)));
                }
 
                // check if the funder's amount for the initial commitment tx is sufficient
@@ -7001,8 +7047,8 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                } else {
                        0
                };
-               let funders_amount_msat = msg.funding_satoshis * 1000 - msg.push_msat;
-               let commitment_tx_fee = commit_tx_fee_msat(msg.feerate_per_kw, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) / 1000;
+               let funders_amount_msat = msg.common_fields.funding_satoshis * 1000 - msg.push_msat;
+               let commitment_tx_fee = commit_tx_fee_msat(msg.common_fields.commitment_feerate_sat_per_1000_weight, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) / 1000;
                if (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value) < commitment_tx_fee {
                        return Err(ChannelError::Close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value), commitment_tx_fee)));
                }
@@ -7015,7 +7061,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                }
 
                let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
-                       match &msg.shutdown_scriptpubkey {
+                       match &msg.common_fields.shutdown_scriptpubkey {
                                &Some(ref script) => {
                                        // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything
                                        if script.len() == 0 {
@@ -7075,8 +7121,8 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 
                                inbound_handshake_limits_override: None,
 
-                               temporary_channel_id: Some(msg.temporary_channel_id),
-                               channel_id: msg.temporary_channel_id,
+                               temporary_channel_id: Some(msg.common_fields.temporary_channel_id),
+                               channel_id: msg.common_fields.temporary_channel_id,
                                channel_state: ChannelState::NegotiatingFunding(
                                        NegotiatingFundingFlags::OUR_INIT_SENT | NegotiatingFundingFlags::THEIR_INIT_SENT
                                ),
@@ -7115,9 +7161,9 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                                signer_pending_funding: false,
 
                                #[cfg(debug_assertions)]
-                               holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
+                               holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.common_fields.funding_satoshis * 1000 - msg.push_msat)),
                                #[cfg(debug_assertions)]
-                               counterparty_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)),
+                               counterparty_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.common_fields.funding_satoshis * 1000 - msg.push_msat)),
 
                                last_sent_closing_fee: None,
                                pending_counterparty_closing_signed: None,
@@ -7130,17 +7176,17 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                                short_channel_id: None,
                                channel_creation_height: current_chain_height,
 
-                               feerate_per_kw: msg.feerate_per_kw,
-                               channel_value_satoshis: msg.funding_satoshis,
-                               counterparty_dust_limit_satoshis: msg.dust_limit_satoshis,
+                               feerate_per_kw: msg.common_fields.commitment_feerate_sat_per_1000_weight,
+                               channel_value_satoshis: msg.common_fields.funding_satoshis,
+                               counterparty_dust_limit_satoshis: msg.common_fields.dust_limit_satoshis,
                                holder_dust_limit_satoshis: MIN_CHAN_DUST_LIMIT_SATOSHIS,
-                               counterparty_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000),
-                               holder_max_htlc_value_in_flight_msat: get_holder_max_htlc_value_in_flight_msat(msg.funding_satoshis, &config.channel_handshake_config),
+                               counterparty_max_htlc_value_in_flight_msat: cmp::min(msg.common_fields.max_htlc_value_in_flight_msat, msg.common_fields.funding_satoshis * 1000),
+                               holder_max_htlc_value_in_flight_msat: get_holder_max_htlc_value_in_flight_msat(msg.common_fields.funding_satoshis, &config.channel_handshake_config),
                                counterparty_selected_channel_reserve_satoshis: Some(msg.channel_reserve_satoshis),
                                holder_selected_channel_reserve_satoshis,
-                               counterparty_htlc_minimum_msat: msg.htlc_minimum_msat,
+                               counterparty_htlc_minimum_msat: msg.common_fields.htlc_minimum_msat,
                                holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat },
-                               counterparty_max_accepted_htlcs: msg.max_accepted_htlcs,
+                               counterparty_max_accepted_htlcs: msg.common_fields.max_accepted_htlcs,
                                holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, MAX_HTLCS),
                                minimum_depth,
 
@@ -7151,7 +7197,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                                        holder_selected_contest_delay: config.channel_handshake_config.our_to_self_delay,
                                        is_outbound_from_holder: false,
                                        counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
-                                               selected_contest_delay: msg.to_self_delay,
+                                               selected_contest_delay: msg.common_fields.to_self_delay,
                                                pubkeys: counterparty_pubkeys,
                                        }),
                                        funding_outpoint: None,
@@ -7160,7 +7206,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                                funding_transaction: None,
                                is_batch_funding: None,
 
-                               counterparty_cur_commitment_point: Some(msg.first_per_commitment_point),
+                               counterparty_cur_commitment_point: Some(msg.common_fields.first_per_commitment_point),
                                counterparty_prev_commitment_point: None,
                                counterparty_node_id,
 
@@ -7341,7 +7387,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                // Now that we're past error-generating stuff, update our local state:
 
                self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
-               self.context.channel_id = funding_txo.to_channel_id();
+               self.context.channel_id = ChannelId::v1_from_funding_outpoint(funding_txo);
                self.context.cur_counterparty_commitment_transaction_number -= 1;
                self.context.cur_holder_commitment_transaction_number -= 1;
 
@@ -7359,7 +7405,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
                                                          &self.context.channel_transaction_parameters,
                                                          funding_redeemscript.clone(), self.context.channel_value_satoshis,
                                                          obscure_factor,
-                                                         holder_commitment_tx, best_block, self.context.counterparty_node_id);
+                                                         holder_commitment_tx, best_block, self.context.counterparty_node_id, self.context.channel_id());
                channel_monitor.provide_initial_counterparty_commitment_tx(
                        counterparty_initial_commitment_tx.trust().txid(), Vec::new(),
                        self.context.cur_counterparty_commitment_transaction_number + 1,
@@ -7462,6 +7508,8 @@ impl<SP: Deref> Writeable for Channel<SP> where SP::Target: SignerProvider {
                        let mut channel_state = self.context.channel_state;
                        if matches!(channel_state, ChannelState::AwaitingChannelReady(_)|ChannelState::ChannelReady(_)) {
                                channel_state.set_peer_disconnected();
+                       } else {
+                               debug_assert!(false, "Pre-funded/shutdown channels should not be written");
                        }
                        channel_state.to_u32().write(writer)?;
                }
@@ -8363,6 +8411,18 @@ mod tests {
        use bitcoin::address::{WitnessProgram, WitnessVersion};
        use crate::prelude::*;
 
+       #[test]
+       fn test_channel_state_order() {
+               use crate::ln::channel::NegotiatingFundingFlags;
+               use crate::ln::channel::AwaitingChannelReadyFlags;
+               use crate::ln::channel::ChannelReadyFlags;
+
+               assert!(ChannelState::NegotiatingFunding(NegotiatingFundingFlags::new()) < ChannelState::FundingNegotiated);
+               assert!(ChannelState::FundingNegotiated < ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()));
+               assert!(ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()) < ChannelState::ChannelReady(ChannelReadyFlags::new()));
+               assert!(ChannelState::ChannelReady(ChannelReadyFlags::new()) < ChannelState::ShutdownComplete);
+       }
+
        struct TestFeeEstimator {
                fee_est: u32
        }
@@ -8467,7 +8527,7 @@ mod tests {
                // same as the old fee.
                fee_est.fee_est = 500;
                let open_channel_msg = node_a_chan.get_open_channel(ChainHash::using_genesis_block(network));
-               assert_eq!(open_channel_msg.feerate_per_kw, original_fee);
+               assert_eq!(open_channel_msg.common_fields.commitment_feerate_sat_per_1000_weight, original_fee);
        }
 
        #[test]
@@ -8870,17 +8930,34 @@ mod tests {
        fn blinding_point_skimmed_fee_malformed_ser() {
                // Ensure that channel blinding points, skimmed fees, and malformed HTLCs are (de)serialized
                // properly.
+               let logger = test_utils::TestLogger::new();
                let feeest = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000});
                let secp_ctx = Secp256k1::new();
                let seed = [42; 32];
                let network = Network::Testnet;
+               let best_block = BestBlock::from_network(network);
                let keys_provider = test_utils::TestKeysInterface::new(&seed, network);
 
                let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
                let config = UserConfig::default();
                let features = channelmanager::provided_init_features(&config);
-               let outbound_chan = OutboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &features, 10000000, 100000, 42, &config, 0, 42, None).unwrap();
-               let mut chan = Channel { context: outbound_chan.context };
+               let mut outbound_chan = OutboundV1Channel::<&TestKeysInterface>::new(
+                       &feeest, &&keys_provider, &&keys_provider, node_b_node_id, &features, 10000000, 100000, 42, &config, 0, 42, None
+               ).unwrap();
+               let inbound_chan = InboundV1Channel::<&TestKeysInterface>::new(
+                       &feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config),
+                       &features, &outbound_chan.get_open_channel(ChainHash::using_genesis_block(network)), 7, &config, 0, &&logger, false
+               ).unwrap();
+               outbound_chan.accept_channel(&inbound_chan.get_accept_channel_message(), &config.channel_handshake_limits, &features).unwrap();
+               let tx = Transaction { version: 1, lock_time: LockTime::ZERO, input: Vec::new(), output: vec![TxOut {
+                       value: 10000000, script_pubkey: outbound_chan.context.get_funding_redeemscript(),
+               }]};
+               let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
+               let funding_created = outbound_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap().unwrap();
+               let mut chan = match inbound_chan.funding_created(&funding_created, best_block, &&keys_provider, &&logger) {
+                       Ok((chan, _, _)) => chan,
+                       Err((_, e)) => panic!("{}", e),
+               };
 
                let dummy_htlc_source = HTLCSource::OutboundRoute {
                        path: Path {
@@ -9772,7 +9849,7 @@ mod tests {
                channel_type_features.set_zero_conf_required();
 
                let mut open_channel_msg = node_a_chan.get_open_channel(ChainHash::using_genesis_block(network));
-               open_channel_msg.channel_type = Some(channel_type_features);
+               open_channel_msg.common_fields.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 = InboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider,
                        node_b_node_id, &channelmanager::provided_channel_type_features(&config),
@@ -9855,7 +9932,7 @@ mod tests {
 
                // Set `channel_type` to `None` to force the implicit feature negotiation.
                let mut open_channel_msg = channel_a.get_open_channel(ChainHash::using_genesis_block(network));
-               open_channel_msg.channel_type = None;
+               open_channel_msg.common_fields.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.
@@ -9901,7 +9978,7 @@ mod tests {
                ).unwrap();
 
                let mut open_channel_msg = channel_a.get_open_channel(ChainHash::using_genesis_block(network));
-               open_channel_msg.channel_type = Some(simple_anchors_channel_type.clone());
+               open_channel_msg.common_fields.channel_type = Some(simple_anchors_channel_type.clone());
 
                let res = InboundV1Channel::<&TestKeysInterface>::new(
                        &fee_estimator, &&keys_provider, &&keys_provider, node_id_a,