Merge pull request #2935 from valentinewallace/2024-03-keysend-to-blinded
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Wed, 20 Mar 2024 19:20:11 +0000 (19:20 +0000)
committerGitHub <noreply@github.com>
Wed, 20 Mar 2024 19:20:11 +0000 (19:20 +0000)
Support keysend to blinded paths

21 files changed:
ci/ci-tests.sh
lightning/src/blinded_path/payment.rs
lightning/src/chain/channelmonitor.rs
lightning/src/events/bump_transaction.rs
lightning/src/events/mod.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/interactivetxs.rs [new file with mode: 0644]
lightning/src/ln/mod.rs
lightning/src/ln/monitor_tests.rs
lightning/src/ln/msgs.rs
lightning/src/ln/reorg_tests.rs
lightning/src/onion_message/functional_tests.rs
lightning/src/onion_message/messenger.rs
lightning/src/routing/gossip.rs
lightning/src/routing/mod.rs
lightning/src/routing/test_utils.rs
lightning/src/util/ser.rs
pending_changelog/blinded-hop-features-optional.txt [new file with mode: 0644]
pending_changelog/relay-constraints-ser.txt [new file with mode: 0644]

index 59acb833f1f2d4fe5d7b35a03ef835bb1f682e85..5cae6d45de5f56a778fc95b2e78b5c5fd9f5ffb6 100755 (executable)
@@ -173,5 +173,7 @@ fi
 
 echo -e "\n\nTest cfg-flag builds"
 RUSTFLAGS="--cfg=taproot" cargo test --verbose --color always -p lightning
+[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
 RUSTFLAGS="--cfg=async_signing" cargo test --verbose --color always -p lightning
+[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
 RUSTFLAGS="--cfg=dual_funding" cargo test --verbose --color always -p lightning
index 106cd802caac2abb860a19ab0366eea69370bfd2..6467af568886bd1891a7e8c52ae6b5956736f6bd 100644 (file)
@@ -13,7 +13,7 @@ use crate::ln::features::BlindedHopFeatures;
 use crate::ln::msgs::DecodeError;
 use crate::offers::invoice::BlindedPayInfo;
 use crate::prelude::*;
-use crate::util::ser::{Readable, Writeable, Writer};
+use crate::util::ser::{HighZeroBytesDroppedBigSize, Readable, Writeable, Writer};
 
 use core::convert::TryFrom;
 
@@ -120,11 +120,14 @@ impl TryFrom<CounterpartyForwardingInfo> for PaymentRelay {
 
 impl Writeable for ForwardTlvs {
        fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+               let features_opt =
+                       if self.features == BlindedHopFeatures::empty() { None }
+                       else { Some(&self.features) };
                encode_tlv_stream!(w, {
                        (2, self.short_channel_id, required),
                        (10, self.payment_relay, required),
                        (12, self.payment_constraints, required),
-                       (14, self.features, required)
+                       (14, features_opt, option)
                });
                Ok(())
        }
@@ -169,7 +172,7 @@ impl Readable for BlindedPaymentTlvs {
                                short_channel_id,
                                payment_relay: payment_relay.ok_or(DecodeError::InvalidValue)?,
                                payment_constraints: payment_constraints.0.unwrap(),
-                               features: features.ok_or(DecodeError::InvalidValue)?,
+                               features: features.unwrap_or_else(BlindedHopFeatures::empty),
                        }))
                } else {
                        if payment_relay.is_some() || features.is_some() { return Err(DecodeError::InvalidValue) }
@@ -276,16 +279,35 @@ pub(super) fn compute_payinfo(
        })
 }
 
-impl_writeable_msg!(PaymentRelay, {
-       cltv_expiry_delta,
-       fee_proportional_millionths,
-       fee_base_msat
-}, {});
+impl Writeable for PaymentRelay {
+       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+               self.cltv_expiry_delta.write(w)?;
+               self.fee_proportional_millionths.write(w)?;
+               HighZeroBytesDroppedBigSize(self.fee_base_msat).write(w)
+       }
+}
+impl Readable for PaymentRelay {
+       fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+               let cltv_expiry_delta: u16 = Readable::read(r)?;
+               let fee_proportional_millionths: u32 = Readable::read(r)?;
+               let fee_base_msat: HighZeroBytesDroppedBigSize<u32> = Readable::read(r)?;
+               Ok(Self { cltv_expiry_delta, fee_proportional_millionths, fee_base_msat: fee_base_msat.0 })
+       }
+}
 
-impl_writeable_msg!(PaymentConstraints, {
-       max_cltv_expiry,
-       htlc_minimum_msat
-}, {});
+impl Writeable for PaymentConstraints {
+       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
+               self.max_cltv_expiry.write(w)?;
+               HighZeroBytesDroppedBigSize(self.htlc_minimum_msat).write(w)
+       }
+}
+impl Readable for PaymentConstraints {
+       fn read<R: io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+               let max_cltv_expiry: u32 = Readable::read(r)?;
+               let htlc_minimum_msat: HighZeroBytesDroppedBigSize<u64> = Readable::read(r)?;
+               Ok(Self { max_cltv_expiry, htlc_minimum_msat: htlc_minimum_msat.0 })
+       }
+}
 
 #[cfg(test)]
 mod tests {
index 5c81a513713f2d79d1b18a42dcc84789f9016f40..4352076e94d009928689dfd524c6214e5c3d06de 100644 (file)
@@ -50,7 +50,7 @@ use crate::chain::Filter;
 use crate::util::logger::{Logger, Record};
 use crate::util::ser::{Readable, ReadableArgs, RequiredWrapper, MaybeReadable, UpgradableRequired, Writer, Writeable, U48};
 use crate::util::byte_utils;
-use crate::events::{Event, EventHandler};
+use crate::events::{ClosureReason, Event, EventHandler};
 use crate::events::bump_transaction::{AnchorDescriptor, BumpTransactionEvent};
 
 use crate::prelude::*;
@@ -155,6 +155,17 @@ pub enum MonitorEvent {
        /// A monitor event containing an HTLCUpdate.
        HTLCEvent(HTLCUpdate),
 
+       /// Indicates we broadcasted the channel's latest commitment transaction and thus closed the
+       /// channel. Holds information about the channel and why it was closed.
+       HolderForceClosedWithInfo {
+               /// The reason the channel was closed.
+               reason: ClosureReason,
+               /// The funding outpoint of the channel.
+               outpoint: OutPoint,
+               /// The channel ID of the channel.
+               channel_id: ChannelId,
+       },
+
        /// Indicates we broadcasted the channel's latest commitment transaction and thus closed the
        /// channel.
        HolderForceClosed(OutPoint),
@@ -184,6 +195,11 @@ impl_writeable_tlv_based_enum_upgradable!(MonitorEvent,
                (2, monitor_update_id, required),
                (4, channel_id, required),
        },
+       (5, HolderForceClosedWithInfo) => {
+               (0, reason, upgradable_required),
+               (2, outpoint, required),
+               (4, channel_id, required),
+       },
 ;
        (2, HTLCEvent),
        (4, HolderForceClosed),
@@ -1059,6 +1075,7 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signe
                writer.write_all(&(self.pending_monitor_events.iter().filter(|ev| match ev {
                        MonitorEvent::HTLCEvent(_) => true,
                        MonitorEvent::HolderForceClosed(_) => true,
+                       MonitorEvent::HolderForceClosedWithInfo { .. } => true,
                        _ => false,
                }).count() as u64).to_be_bytes())?;
                for event in self.pending_monitor_events.iter() {
@@ -1068,6 +1085,10 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signe
                                        upd.write(writer)?;
                                },
                                MonitorEvent::HolderForceClosed(_) => 1u8.write(writer)?,
+                               // `HolderForceClosedWithInfo` replaced `HolderForceClosed` in v0.0.122. To keep
+                               // backwards compatibility, we write a `HolderForceClosed` event along with the
+                               // `HolderForceClosedWithInfo` event. This is deduplicated in the reader.
+                               MonitorEvent::HolderForceClosedWithInfo { .. } => 1u8.write(writer)?,
                                _ => {}, // Covered in the TLV writes below
                        }
                }
@@ -1099,10 +1120,23 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signe
                self.lockdown_from_offchain.write(writer)?;
                self.holder_tx_signed.write(writer)?;
 
+               // If we have a `HolderForceClosedWithInfo` event, we need to write the `HolderForceClosed` for backwards compatibility.
+               let pending_monitor_events = match self.pending_monitor_events.iter().find(|ev| match ev {
+                       MonitorEvent::HolderForceClosedWithInfo { .. } => true,
+                       _ => false,
+               }) {
+                       Some(MonitorEvent::HolderForceClosedWithInfo { outpoint, .. }) => {
+                               let mut pending_monitor_events = self.pending_monitor_events.clone();
+                               pending_monitor_events.push(MonitorEvent::HolderForceClosed(*outpoint));
+                               pending_monitor_events
+                       }
+                       _ => self.pending_monitor_events.clone(),
+               };
+
                write_tlv_fields!(writer, {
                        (1, self.funding_spend_confirmed, option),
                        (3, self.htlcs_resolved_on_chain, required_vec),
-                       (5, self.pending_monitor_events, required_vec),
+                       (5, pending_monitor_events, required_vec),
                        (7, self.funding_spend_seen, required),
                        (9, self.counterparty_node_id, option),
                        (11, self.confirmed_commitment_tx_counterparty_output, option),
@@ -2727,7 +2761,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                }
        }
 
-       fn generate_claimable_outpoints_and_watch_outputs(&mut self) -> (Vec<PackageTemplate>, Vec<TransactionOutputs>) {
+       fn generate_claimable_outpoints_and_watch_outputs(&mut self, reason: ClosureReason) -> (Vec<PackageTemplate>, Vec<TransactionOutputs>) {
                let funding_outp = HolderFundingOutput::build(
                        self.funding_redeemscript.clone(),
                        self.channel_value_satoshis,
@@ -2739,7 +2773,13 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                        self.best_block.height, self.best_block.height
                );
                let mut claimable_outpoints = vec![commitment_package];
-               self.pending_monitor_events.push(MonitorEvent::HolderForceClosed(self.funding_info.0));
+               let event = MonitorEvent::HolderForceClosedWithInfo {
+                       reason,
+                       outpoint: self.funding_info.0,
+                       channel_id: self.channel_id,
+               };
+               self.pending_monitor_events.push(event);
+
                // Although we aren't signing the transaction directly here, the transaction will be signed
                // in the claim that is queued to OnchainTxHandler. We set holder_tx_signed here to reject
                // new channel updates.
@@ -2775,7 +2815,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
                F::Target: FeeEstimator,
                L::Target: Logger,
        {
-               let (claimable_outpoints, _) = self.generate_claimable_outpoints_and_watch_outputs();
+               let (claimable_outpoints, _) = self.generate_claimable_outpoints_and_watch_outputs(ClosureReason::HolderForceClosed);
                self.onchain_tx_handler.update_claims_view_from_requests(
                        claimable_outpoints, self.best_block.height, self.best_block.height, broadcaster,
                        fee_estimator, logger
@@ -3778,7 +3818,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
                let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);
                if should_broadcast {
-                       let (mut new_outpoints, mut new_outputs) = self.generate_claimable_outpoints_and_watch_outputs();
+                       let (mut new_outpoints, mut new_outputs) = self.generate_claimable_outpoints_and_watch_outputs(ClosureReason::HTLCsTimedOut);
                        claimable_outpoints.append(&mut new_outpoints);
                        watch_outputs.append(&mut new_outputs);
                }
@@ -4605,6 +4645,16 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
                        (19, channel_id, option),
                });
 
+               // `HolderForceClosedWithInfo` replaced `HolderForceClosed` in v0.0.122. If we have both
+               // events, we can remove the `HolderForceClosed` event and just keep the `HolderForceClosedWithInfo`.
+               if let Some(ref mut pending_monitor_events) = pending_monitor_events {
+                       if pending_monitor_events.iter().any(|e| matches!(e, MonitorEvent::HolderForceClosed(_))) &&
+                               pending_monitor_events.iter().any(|e| matches!(e, MonitorEvent::HolderForceClosedWithInfo { .. }))
+                       {
+                               pending_monitor_events.retain(|e| !matches!(e, MonitorEvent::HolderForceClosed(_)));
+                       }
+               }
+
                // Monitors for anchor outputs channels opened in v0.0.116 suffered from a bug in which the
                // wrong `counterparty_payment_script` was being tracked. Fix it now on deserialization to
                // give them a chance to recognize the spendable output.
index 44f44dd31fc7d488bc420417e26c2c16236c2d72..8b52af69fe331527a43ae38e0a67cd94e4ea4139 100644 (file)
@@ -41,11 +41,11 @@ use bitcoin::secp256k1;
 use bitcoin::secp256k1::{PublicKey, Secp256k1};
 use bitcoin::secp256k1::ecdsa::Signature;
 
-const EMPTY_SCRIPT_SIG_WEIGHT: u64 = 1 /* empty script_sig */ * WITNESS_SCALE_FACTOR as u64;
+pub(crate) const EMPTY_SCRIPT_SIG_WEIGHT: u64 = 1 /* empty script_sig */ * WITNESS_SCALE_FACTOR as u64;
 
 const BASE_INPUT_SIZE: u64 = 32 /* txid */ + 4 /* vout */ + 4 /* sequence */;
 
-const BASE_INPUT_WEIGHT: u64 = BASE_INPUT_SIZE * WITNESS_SCALE_FACTOR as u64;
+pub(crate) const BASE_INPUT_WEIGHT: u64 = BASE_INPUT_SIZE * WITNESS_SCALE_FACTOR as u64;
 
 /// A descriptor used to sign for a commitment transaction's anchor output.
 #[derive(Clone, Debug, PartialEq, Eq)]
index 485a23e0292e520c28d47a226fe7de51abdd9690..d4fa90c6a73e0c6ab200292d6764f1a6e5443f3a 100644 (file)
@@ -232,6 +232,8 @@ pub enum ClosureReason {
        /// Another channel in the same funding batch closed before the funding transaction
        /// was ready to be broadcast.
        FundingBatchClosure,
+       /// One of our HTLCs timed out in a channel, causing us to force close the channel.
+       HTLCsTimedOut,
 }
 
 impl core::fmt::Display for ClosureReason {
@@ -241,7 +243,7 @@ impl core::fmt::Display for ClosureReason {
                        ClosureReason::CounterpartyForceClosed { peer_msg } => {
                                f.write_fmt(format_args!("counterparty force-closed with message: {}", peer_msg))
                        },
-                       ClosureReason::HolderForceClosed => f.write_str("user manually force-closed the channel"),
+                       ClosureReason::HolderForceClosed => f.write_str("user force-closed the channel"),
                        ClosureReason::LegacyCooperativeClosure => f.write_str("the channel was cooperatively closed"),
                        ClosureReason::CounterpartyInitiatedCooperativeClosure => f.write_str("the channel was cooperatively closed by our peer"),
                        ClosureReason::LocallyInitiatedCooperativeClosure => f.write_str("the channel was cooperatively closed by us"),
@@ -255,6 +257,7 @@ impl core::fmt::Display for ClosureReason {
                        ClosureReason::OutdatedChannelManager => f.write_str("the ChannelManager read from disk was stale compared to ChannelMonitor(s)"),
                        ClosureReason::CounterpartyCoopClosedUnfundedChannel => f.write_str("the peer requested the unfunded channel be closed"),
                        ClosureReason::FundingBatchClosure => f.write_str("another channel in the same funding batch closed"),
+                       ClosureReason::HTLCsTimedOut => f.write_str("htlcs on the channel timed out"),
                }
        }
 }
@@ -272,6 +275,7 @@ impl_writeable_tlv_based_enum_upgradable!(ClosureReason,
        (15, FundingBatchClosure) => {},
        (17, CounterpartyInitiatedCooperativeClosure) => {},
        (19, LocallyInitiatedCooperativeClosure) => {},
+       (21, HTLCsTimedOut) => {},
 );
 
 /// Intended destination of a failed HTLC as indicated in [`Event::HTLCHandlingFailed`].
@@ -797,12 +801,24 @@ pub enum Event {
        /// This event is generated when a payment has been successfully forwarded through us and a
        /// forwarding fee earned.
        PaymentForwarded {
-               /// The incoming channel between the previous node and us. This is only `None` for events
-               /// generated or serialized by versions prior to 0.0.107.
+               /// The channel id of the incoming channel between the previous node and us.
+               ///
+               /// This is only `None` for events generated or serialized by versions prior to 0.0.107.
                prev_channel_id: Option<ChannelId>,
-               /// The outgoing channel between the next node and us. This is only `None` for events
-               /// generated or serialized by versions prior to 0.0.107.
+               /// The channel id of the outgoing channel between the next node and us.
+               ///
+               /// This is only `None` for events generated or serialized by versions prior to 0.0.107.
                next_channel_id: Option<ChannelId>,
+               /// The `user_channel_id` of the incoming channel between the previous node and us.
+               ///
+               /// This is only `None` for events generated or serialized by versions prior to 0.0.122.
+               prev_user_channel_id: Option<u128>,
+               /// The `user_channel_id` of the outgoing channel between the next node and us.
+               ///
+               /// This will be `None` if the payment was settled via an on-chain transaction. See the
+               /// caveat described for the `total_fee_earned_msat` field. Moreover it will be `None` for
+               /// events generated or serialized by versions prior to 0.0.122.
+               next_user_channel_id: Option<u128>,
                /// The total fee, in milli-satoshis, which was earned as a result of the payment.
                ///
                /// Note that if we force-closed the channel over which we forwarded an HTLC while the HTLC
@@ -1121,8 +1137,9 @@ impl Writeable for Event {
                                });
                        }
                        &Event::PaymentForwarded {
-                               total_fee_earned_msat, prev_channel_id, claim_from_onchain_tx,
-                               next_channel_id, outbound_amount_forwarded_msat, skimmed_fee_msat,
+                               prev_channel_id, next_channel_id, prev_user_channel_id, next_user_channel_id,
+                               total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx,
+                               outbound_amount_forwarded_msat,
                        } => {
                                7u8.write(writer)?;
                                write_tlv_fields!(writer, {
@@ -1132,6 +1149,8 @@ impl Writeable for Event {
                                        (3, next_channel_id, option),
                                        (5, outbound_amount_forwarded_msat, option),
                                        (7, skimmed_fee_msat, option),
+                                       (9, prev_user_channel_id, option),
+                                       (11, next_user_channel_id, option),
                                });
                        },
                        &Event::ChannelClosed { ref channel_id, ref user_channel_id, ref reason,
@@ -1427,12 +1446,14 @@ impl MaybeReadable for Event {
                        },
                        7u8 => {
                                let f = || {
-                                       let mut total_fee_earned_msat = None;
                                        let mut prev_channel_id = None;
-                                       let mut claim_from_onchain_tx = false;
                                        let mut next_channel_id = None;
-                                       let mut outbound_amount_forwarded_msat = None;
+                                       let mut prev_user_channel_id = None;
+                                       let mut next_user_channel_id = None;
+                                       let mut total_fee_earned_msat = None;
                                        let mut skimmed_fee_msat = None;
+                                       let mut claim_from_onchain_tx = false;
+                                       let mut outbound_amount_forwarded_msat = None;
                                        read_tlv_fields!(reader, {
                                                (0, total_fee_earned_msat, option),
                                                (1, prev_channel_id, option),
@@ -1440,10 +1461,13 @@ impl MaybeReadable for Event {
                                                (3, next_channel_id, option),
                                                (5, outbound_amount_forwarded_msat, option),
                                                (7, skimmed_fee_msat, option),
+                                               (9, prev_user_channel_id, option),
+                                               (11, next_user_channel_id, option),
                                        });
                                        Ok(Some(Event::PaymentForwarded {
-                                               total_fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id,
-                                               outbound_amount_forwarded_msat, skimmed_fee_msat,
+                                               prev_channel_id, next_channel_id, prev_user_channel_id,
+                                               next_user_channel_id, total_fee_earned_msat, skimmed_fee_msat,
+                                               claim_from_onchain_tx, outbound_amount_forwarded_msat,
                                        }))
                                };
                                f()
index f1c67a8706e430bc70a531f324f140507155c50d..546b317ee26ddf65b04ab64488c53520353c2ba0 100644 (file)
@@ -5746,7 +5746,7 @@ where
        fn claim_funds_internal(&self, source: HTLCSource, payment_preimage: PaymentPreimage,
                forwarded_htlc_value_msat: Option<u64>, skimmed_fee_msat: Option<u64>, from_onchain: bool,
                startup_replay: bool, next_channel_counterparty_node_id: Option<PublicKey>,
-               next_channel_outpoint: OutPoint, next_channel_id: ChannelId,
+               next_channel_outpoint: OutPoint, next_channel_id: ChannelId, next_user_channel_id: Option<u128>,
        ) {
                match source {
                        HTLCSource::OutboundRoute { session_priv, payment_id, path, .. } => {
@@ -5765,11 +5765,10 @@ where
                        },
                        HTLCSource::PreviousHopData(hop_data) => {
                                let prev_channel_id = hop_data.channel_id;
+                               let prev_user_channel_id = hop_data.user_channel_id;
                                let completed_blocker = RAAMonitorUpdateBlockingAction::from_prev_hop_data(&hop_data);
                                #[cfg(debug_assertions)]
                                let claiming_chan_funding_outpoint = hop_data.outpoint;
-                               #[cfg(debug_assertions)]
-                               let claiming_channel_id = hop_data.channel_id;
                                let res = self.claim_funds_from_hop(hop_data, payment_preimage,
                                        |htlc_claim_value_msat, definitely_duplicate| {
                                                let chan_to_release =
@@ -5827,7 +5826,7 @@ where
                                                                                BackgroundEvent::MonitorUpdatesComplete {
                                                                                        channel_id, ..
                                                                                } =>
-                                                                                       *channel_id == claiming_channel_id,
+                                                                                       *channel_id == prev_channel_id,
                                                                        }
                                                                }), "{:?}", *background_events);
                                                        }
@@ -5851,12 +5850,14 @@ where
                                                                "skimmed_fee_msat must always be included in total_fee_earned_msat");
                                                        Some(MonitorUpdateCompletionAction::EmitEventAndFreeOtherChannel {
                                                                event: events::Event::PaymentForwarded {
-                                                                       total_fee_earned_msat,
-                                                                       claim_from_onchain_tx: from_onchain,
                                                                        prev_channel_id: Some(prev_channel_id),
                                                                        next_channel_id: Some(next_channel_id),
-                                                                       outbound_amount_forwarded_msat: forwarded_htlc_value_msat,
+                                                                       prev_user_channel_id,
+                                                                       next_user_channel_id,
+                                                                       total_fee_earned_msat,
                                                                        skimmed_fee_msat,
+                                                                       claim_from_onchain_tx: from_onchain,
+                                                                       outbound_amount_forwarded_msat: forwarded_htlc_value_msat,
                                                                },
                                                                downstream_counterparty_and_funding_outpoint: chan_to_release,
                                                        })
@@ -6830,6 +6831,7 @@ where
 
        fn internal_update_fulfill_htlc(&self, counterparty_node_id: &PublicKey, msg: &msgs::UpdateFulfillHTLC) -> Result<(), MsgHandleErrInternal> {
                let funding_txo;
+               let next_user_channel_id;
                let (htlc_source, forwarded_htlc_value, skimmed_fee_msat) = {
                        let per_peer_state = self.per_peer_state.read().unwrap();
                        let peer_state_mutex = per_peer_state.get(counterparty_node_id)
@@ -6859,6 +6861,7 @@ where
                                                // outbound HTLC is claimed. This is guaranteed to all complete before we
                                                // process the RAA as messages are processed from single peers serially.
                                                funding_txo = chan.context.get_funding_txo().expect("We won't accept a fulfill until funded");
+                                               next_user_channel_id = chan.context.get_user_id();
                                                res
                                        } else {
                                                return try_chan_phase_entry!(self, Err(ChannelError::Close(
@@ -6870,7 +6873,7 @@ where
                };
                self.claim_funds_internal(htlc_source, msg.payment_preimage.clone(),
                        Some(forwarded_htlc_value), skimmed_fee_msat, false, false, Some(*counterparty_node_id),
-                       funding_txo, msg.channel_id
+                       funding_txo, msg.channel_id, Some(next_user_channel_id),
                );
 
                Ok(())
@@ -7372,7 +7375,7 @@ where
                                                        log_trace!(logger, "Claiming HTLC with preimage {} from our monitor", preimage);
                                                        self.claim_funds_internal(htlc_update.source, preimage,
                                                                htlc_update.htlc_value_satoshis.map(|v| v * 1000), None, true,
-                                                               false, counterparty_node_id, funding_outpoint, channel_id);
+                                                               false, counterparty_node_id, funding_outpoint, channel_id, None);
                                                } else {
                                                        log_trace!(logger, "Failing HTLC with hash {} from our monitor", &htlc_update.payment_hash);
                                                        let receiver = HTLCDestination::NextHopChannel { node_id: counterparty_node_id, channel_id };
@@ -7380,7 +7383,7 @@ where
                                                        self.fail_htlc_backwards_internal(&htlc_update.source, &htlc_update.payment_hash, &reason, receiver);
                                                }
                                        },
-                                       MonitorEvent::HolderForceClosed(_funding_outpoint) => {
+                                       MonitorEvent::HolderForceClosed(_) | MonitorEvent::HolderForceClosedWithInfo { .. } => {
                                                let counterparty_node_id_opt = match counterparty_node_id {
                                                        Some(cp_id) => Some(cp_id),
                                                        None => {
@@ -7398,7 +7401,12 @@ where
                                                                let pending_msg_events = &mut peer_state.pending_msg_events;
                                                                if let hash_map::Entry::Occupied(chan_phase_entry) = peer_state.channel_by_id.entry(channel_id) {
                                                                        if let ChannelPhase::Funded(mut chan) = remove_channel_phase!(self, chan_phase_entry) {
-                                                                               failed_channels.push(chan.context.force_shutdown(false, ClosureReason::HolderForceClosed));
+                                                                               let reason = if let MonitorEvent::HolderForceClosedWithInfo { reason, .. } = monitor_event {
+                                                                                       reason
+                                                                               } else {
+                                                                                       ClosureReason::HolderForceClosed
+                                                                               };
+                                                                               failed_channels.push(chan.context.force_shutdown(false, reason.clone()));
                                                                                if let Ok(update) = self.get_channel_update_for_broadcast(&chan) {
                                                                                        pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
                                                                                                msg: update
@@ -7407,7 +7415,7 @@ where
                                                                                pending_msg_events.push(events::MessageSendEvent::HandleError {
                                                                                        node_id: chan.context.get_counterparty_node_id(),
                                                                                        action: msgs::ErrorAction::DisconnectPeer {
-                                                                                               msg: Some(msgs::ErrorMessage { channel_id: chan.context.channel_id(), data: "Channel force-closed".to_owned() })
+                                                                                               msg: Some(msgs::ErrorMessage { channel_id: chan.context.channel_id(), data: reason.to_string() })
                                                                                        },
                                                                                });
                                                                        }
@@ -9248,8 +9256,6 @@ where
        }
 
        fn handle_error(&self, counterparty_node_id: &PublicKey, msg: &msgs::ErrorMessage) {
-               let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
-
                match &msg.data as &str {
                        "cannot co-op close channel w/ active htlcs"|
                        "link failed to shutdown" =>
@@ -9262,34 +9268,45 @@ where
                                // We're not going to bother handling this in a sensible way, instead simply
                                // repeating the Shutdown message on repeat until morale improves.
                                if !msg.channel_id.is_zero() {
-                                       let per_peer_state = self.per_peer_state.read().unwrap();
-                                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
-                                       if peer_state_mutex_opt.is_none() { return; }
-                                       let mut peer_state = peer_state_mutex_opt.unwrap().lock().unwrap();
-                                       if let Some(ChannelPhase::Funded(chan)) = peer_state.channel_by_id.get(&msg.channel_id) {
-                                               if let Some(msg) = chan.get_outbound_shutdown() {
-                                                       peer_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
-                                                               node_id: *counterparty_node_id,
-                                                               msg,
-                                                       });
-                                               }
-                                               peer_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
-                                                       node_id: *counterparty_node_id,
-                                                       action: msgs::ErrorAction::SendWarningMessage {
-                                                               msg: msgs::WarningMessage {
-                                                                       channel_id: msg.channel_id,
-                                                                       data: "You appear to be exhibiting LND bug 6039, we'll keep sending you shutdown messages until you handle them correctly".to_owned()
-                                                               },
-                                                               log_level: Level::Trace,
+                                       PersistenceNotifierGuard::optionally_notify(
+                                               self,
+                                               || -> NotifyOption {
+                                                       let per_peer_state = self.per_peer_state.read().unwrap();
+                                                       let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id);
+                                                       if peer_state_mutex_opt.is_none() { return NotifyOption::SkipPersistNoEvents; }
+                                                       let mut peer_state = peer_state_mutex_opt.unwrap().lock().unwrap();
+                                                       if let Some(ChannelPhase::Funded(chan)) = peer_state.channel_by_id.get(&msg.channel_id) {
+                                                               if let Some(msg) = chan.get_outbound_shutdown() {
+                                                                       peer_state.pending_msg_events.push(events::MessageSendEvent::SendShutdown {
+                                                                               node_id: *counterparty_node_id,
+                                                                               msg,
+                                                                       });
+                                                               }
+                                                               peer_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
+                                                                       node_id: *counterparty_node_id,
+                                                                       action: msgs::ErrorAction::SendWarningMessage {
+                                                                               msg: msgs::WarningMessage {
+                                                                                       channel_id: msg.channel_id,
+                                                                                       data: "You appear to be exhibiting LND bug 6039, we'll keep sending you shutdown messages until you handle them correctly".to_owned()
+                                                                               },
+                                                                               log_level: Level::Trace,
+                                                                       }
+                                                               });
+                                                               // This can happen in a fairly tight loop, so we absolutely cannot trigger
+                                                               // a `ChannelManager` write here.
+                                                               return NotifyOption::SkipPersistHandleEvents;
                                                        }
-                                               });
-                                       }
+                                                       NotifyOption::SkipPersistNoEvents
+                                               }
+                                       );
                                }
                                return;
                        }
                        _ => {}
                }
 
+               let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
+
                if msg.channel_id.is_zero() {
                        let channel_ids: Vec<ChannelId> = {
                                let per_peer_state = self.per_peer_state.read().unwrap();
@@ -11384,7 +11401,9 @@ where
                        // don't remember in the `ChannelMonitor` where we got a preimage from, but if the
                        // channel is closed we just assume that it probably came from an on-chain claim.
                        channel_manager.claim_funds_internal(source, preimage, Some(downstream_value), None,
-                               downstream_closed, true, downstream_node_id, downstream_funding, downstream_channel_id);
+                               downstream_closed, true, downstream_node_id, downstream_funding,
+                               downstream_channel_id, None
+                       );
                }
 
                //TODO: Broadcast channel update for closed channels, but only after we've made a
index cb310ec95398a38309840a3177966102da3eeebb..5840fead943049a6ac2a26eb4e784253ad3bcffb 100644 (file)
@@ -2231,8 +2231,8 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM=CM>>(
 ) -> Option<u64> {
        match event {
                Event::PaymentForwarded {
-                       total_fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id,
-                       outbound_amount_forwarded_msat: _, skimmed_fee_msat
+                       prev_channel_id, next_channel_id, prev_user_channel_id, next_user_channel_id,
+                       total_fee_earned_msat, skimmed_fee_msat, claim_from_onchain_tx, ..
                } => {
                        if allow_1_msat_fee_overpay {
                                // Aggregating fees for blinded paths may result in a rounding error, causing slight
@@ -2249,12 +2249,31 @@ pub fn expect_payment_forwarded<CM: AChannelManager, H: NodeHolder<CM=CM>>(
                        assert!(skimmed_fee_msat == expected_extra_fees_msat);
                        if !upstream_force_closed {
                                // Is the event prev_channel_id in one of the channels between the two nodes?
-                               assert!(node.node().list_channels().iter().any(|x| x.counterparty.node_id == prev_node.node().get_our_node_id() && x.channel_id == prev_channel_id.unwrap()));
+                               assert!(node.node().list_channels().iter().any(|x|
+                                       x.counterparty.node_id == prev_node.node().get_our_node_id() &&
+                                       x.channel_id == prev_channel_id.unwrap() &&
+                                       x.user_channel_id == prev_user_channel_id.unwrap()
+                               ));
                        }
                        // We check for force closures since a force closed channel is removed from the
                        // node's channel list
                        if !downstream_force_closed {
-                               assert!(node.node().list_channels().iter().any(|x| x.counterparty.node_id == next_node.node().get_our_node_id() && x.channel_id == next_channel_id.unwrap()));
+                               // As documented, `next_user_channel_id` will only be `Some` if we didn't settle via an
+                               // onchain transaction, just as the `total_fee_earned_msat` field. Rather than
+                               // introducing yet another variable, we use the latter's state as a flag to detect
+                               // this and only check if it's `Some`.
+                               if total_fee_earned_msat.is_none() {
+                                       assert!(node.node().list_channels().iter().any(|x|
+                                               x.counterparty.node_id == next_node.node().get_our_node_id() &&
+                                               x.channel_id == next_channel_id.unwrap()
+                                       ));
+                               } else {
+                                       assert!(node.node().list_channels().iter().any(|x|
+                                               x.counterparty.node_id == next_node.node().get_our_node_id() &&
+                                               x.channel_id == next_channel_id.unwrap() &&
+                                               x.user_channel_id == next_user_channel_id.unwrap()
+                                       ));
+                               }
                        }
                        assert_eq!(claim_from_onchain_tx, downstream_force_closed);
                        total_fee_earned_msat
index c5ea582b27f81a95abd50bc760e26b8f30e363e5..8c8be0b837bb05fc6cd49fbdf6a3b8b97209e72a 100644 (file)
@@ -2417,7 +2417,7 @@ fn channel_monitor_network_test() {
                }
                check_added_monitors!(nodes[4], 1);
                test_txn_broadcast(&nodes[4], &chan_4, None, HTLCType::SUCCESS);
-               check_closed_event!(nodes[4], 1, ClosureReason::HolderForceClosed, [nodes[3].node.get_our_node_id()], 100000);
+               check_closed_event!(nodes[4], 1, ClosureReason::HTLCsTimedOut, [nodes[3].node.get_our_node_id()], 100000);
 
                mine_transaction(&nodes[4], &node_txn[0]);
                check_preimage_claim(&nodes[4], &node_txn);
@@ -2430,7 +2430,7 @@ fn channel_monitor_network_test() {
 
        assert_eq!(nodes[3].chain_monitor.chain_monitor.watch_channel(OutPoint { txid: chan_3.3.txid(), index: 0 }, chan_3_mon),
                Ok(ChannelMonitorUpdateStatus::Completed));
-       check_closed_event!(nodes[3], 1, ClosureReason::HolderForceClosed, [nodes[4].node.get_our_node_id()], 100000);
+       check_closed_event!(nodes[3], 1, ClosureReason::HTLCsTimedOut, [nodes[4].node.get_our_node_id()], 100000);
 }
 
 #[test]
@@ -5682,7 +5682,7 @@ fn do_htlc_claim_local_commitment_only(use_dust: bool) {
        test_txn_broadcast(&nodes[1], &chan, None, if use_dust { HTLCType::NONE } else { HTLCType::SUCCESS });
        check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
-       check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000);
+       check_closed_event!(nodes[1], 1, ClosureReason::HTLCsTimedOut, [nodes[0].node.get_our_node_id()], 100000);
 }
 
 fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
@@ -5713,7 +5713,7 @@ fn do_htlc_claim_current_remote_commitment_only(use_dust: bool) {
        test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
        check_closed_broadcast!(nodes[0], true);
        check_added_monitors!(nodes[0], 1);
-       check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000);
+       check_closed_event!(nodes[0], 1, ClosureReason::HTLCsTimedOut, [nodes[1].node.get_our_node_id()], 100000);
 }
 
 fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no_close: bool) {
@@ -5759,7 +5759,7 @@ fn do_htlc_claim_previous_remote_commitment_only(use_dust: bool, check_revoke_no
                test_txn_broadcast(&nodes[0], &chan, None, HTLCType::NONE);
                check_closed_broadcast!(nodes[0], true);
                check_added_monitors!(nodes[0], 1);
-               check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000);
+               check_closed_event!(nodes[0], 1, ClosureReason::HTLCsTimedOut, [nodes[1].node.get_our_node_id()], 100000);
        } else {
                expect_payment_failed!(nodes[0], our_payment_hash, true);
        }
@@ -8654,7 +8654,7 @@ fn test_concurrent_monitor_claim() {
        let height = HTLC_TIMEOUT_BROADCAST + 1;
        connect_blocks(&nodes[0], height - nodes[0].best_block_info().1);
        check_closed_broadcast(&nodes[0], 1, true);
-       check_closed_event!(&nodes[0], 1, ClosureReason::HolderForceClosed, false,
+       check_closed_event!(&nodes[0], 1, ClosureReason::HTLCsTimedOut, false,
                [nodes[1].node.get_our_node_id()], 100000);
        watchtower_alice.chain_monitor.block_connected(&create_dummy_block(BlockHash::all_zeros(), 42, vec![bob_state_y.clone()]), height);
        check_added_monitors(&nodes[0], 1);
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
new file mode 100644 (file)
index 0000000..94311a3
--- /dev/null
@@ -0,0 +1,1384 @@
+// This file is Copyright its original authors, visible in version control
+// history.
+//
+// This file is licensed under the Apache License, Version 2.0 <LICENSE-APACHE
+// or http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your option.
+// You may not use this file except in accordance with one or both of these
+// licenses.
+
+use crate::io_extras::sink;
+use crate::prelude::*;
+use core::ops::Deref;
+
+use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR;
+use bitcoin::consensus::Encodable;
+use bitcoin::policy::MAX_STANDARD_TX_WEIGHT;
+use bitcoin::{
+       absolute::LockTime as AbsoluteLockTime, OutPoint, Sequence, Transaction, TxIn, TxOut,
+};
+
+use crate::chain::chaininterface::fee_for_weight;
+use crate::events::bump_transaction::{BASE_INPUT_WEIGHT, EMPTY_SCRIPT_SIG_WEIGHT};
+use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS;
+use crate::ln::msgs::SerialId;
+use crate::ln::{msgs, ChannelId};
+use crate::sign::EntropySource;
+use crate::util::ser::TransactionU16LenLimited;
+
+/// The number of received `tx_add_input` messages during a negotiation at which point the
+/// negotiation MUST be failed.
+const MAX_RECEIVED_TX_ADD_INPUT_COUNT: u16 = 4096;
+
+/// The number of received `tx_add_output` messages during a negotiation at which point the
+/// negotiation MUST be failed.
+const MAX_RECEIVED_TX_ADD_OUTPUT_COUNT: u16 = 4096;
+
+/// The number of inputs or outputs that the state machine can have, before it MUST fail the
+/// negotiation.
+const MAX_INPUTS_OUTPUTS_COUNT: usize = 252;
+
+trait SerialIdExt {
+       fn is_for_initiator(&self) -> bool;
+       fn is_for_non_initiator(&self) -> bool;
+}
+
+impl SerialIdExt for SerialId {
+       fn is_for_initiator(&self) -> bool {
+               self % 2 == 0
+       }
+
+       fn is_for_non_initiator(&self) -> bool {
+               !self.is_for_initiator()
+       }
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub enum AbortReason {
+       InvalidStateTransition,
+       UnexpectedCounterpartyMessage,
+       ReceivedTooManyTxAddInputs,
+       ReceivedTooManyTxAddOutputs,
+       IncorrectInputSequenceValue,
+       IncorrectSerialIdParity,
+       SerialIdUnknown,
+       DuplicateSerialId,
+       PrevTxOutInvalid,
+       ExceededMaximumSatsAllowed,
+       ExceededNumberOfInputsOrOutputs,
+       TransactionTooLarge,
+       BelowDustLimit,
+       InvalidOutputScript,
+       InsufficientFees,
+       OutputsValueExceedsInputsValue,
+       InvalidTx,
+}
+
+#[derive(Debug)]
+pub struct TxInputWithPrevOutput {
+       input: TxIn,
+       prev_output: TxOut,
+}
+
+#[derive(Debug)]
+struct NegotiationContext {
+       holder_is_initiator: bool,
+       received_tx_add_input_count: u16,
+       received_tx_add_output_count: u16,
+       inputs: HashMap<SerialId, TxInputWithPrevOutput>,
+       prevtx_outpoints: HashSet<OutPoint>,
+       outputs: HashMap<SerialId, TxOut>,
+       tx_locktime: AbsoluteLockTime,
+       feerate_sat_per_kw: u32,
+       to_remote_value_satoshis: u64,
+}
+
+impl NegotiationContext {
+       fn is_serial_id_valid_for_counterparty(&self, serial_id: &SerialId) -> bool {
+               // A received `SerialId`'s parity must match the role of the counterparty.
+               self.holder_is_initiator == serial_id.is_for_non_initiator()
+       }
+
+       fn total_input_and_output_count(&self) -> usize {
+               self.inputs.len().saturating_add(self.outputs.len())
+       }
+
+       fn counterparty_inputs_contributed(
+               &self,
+       ) -> impl Iterator<Item = &TxInputWithPrevOutput> + Clone {
+               self.inputs
+                       .iter()
+                       .filter(move |(serial_id, _)| self.is_serial_id_valid_for_counterparty(serial_id))
+                       .map(|(_, input_with_prevout)| input_with_prevout)
+       }
+
+       fn counterparty_outputs_contributed(&self) -> impl Iterator<Item = &TxOut> + Clone {
+               self.outputs
+                       .iter()
+                       .filter(move |(serial_id, _)| self.is_serial_id_valid_for_counterparty(serial_id))
+                       .map(|(_, output)| output)
+       }
+
+       fn received_tx_add_input(&mut self, msg: &msgs::TxAddInput) -> Result<(), AbortReason> {
+               // The interactive-txs spec calls for us to fail negotiation if the `prevtx` we receive is
+               // invalid. However, we would not need to account for this explicit negotiation failure
+               // mode here since `PeerManager` would already disconnect the peer if the `prevtx` is
+               // invalid; implicitly ending the negotiation.
+
+               if !self.is_serial_id_valid_for_counterparty(&msg.serial_id) {
+                       // The receiving node:
+                       //  - MUST fail the negotiation if:
+                       //     - the `serial_id` has the wrong parity
+                       return Err(AbortReason::IncorrectSerialIdParity);
+               }
+
+               self.received_tx_add_input_count += 1;
+               if self.received_tx_add_input_count > MAX_RECEIVED_TX_ADD_INPUT_COUNT {
+                       // The receiving node:
+                       //  - MUST fail the negotiation if:
+                       //     - if has received 4096 `tx_add_input` messages during this negotiation
+                       return Err(AbortReason::ReceivedTooManyTxAddInputs);
+               }
+
+               if msg.sequence >= 0xFFFFFFFE {
+                       // The receiving node:
+                       //  - MUST fail the negotiation if:
+                       //    - `sequence` is set to `0xFFFFFFFE` or `0xFFFFFFFF`
+                       return Err(AbortReason::IncorrectInputSequenceValue);
+               }
+
+               let transaction = msg.prevtx.as_transaction();
+               let txid = transaction.txid();
+
+               if let Some(tx_out) = transaction.output.get(msg.prevtx_out as usize) {
+                       if !tx_out.script_pubkey.is_witness_program() {
+                               // The receiving node:
+                               //  - MUST fail the negotiation if:
+                               //     - the `scriptPubKey` is not a witness program
+                               return Err(AbortReason::PrevTxOutInvalid);
+                       }
+
+                       if !self.prevtx_outpoints.insert(OutPoint { txid, vout: msg.prevtx_out }) {
+                               // The receiving node:
+                               //  - MUST fail the negotiation if:
+                               //     - the `prevtx` and `prevtx_vout` are identical to a previously added
+                               //       (and not removed) input's
+                               return Err(AbortReason::PrevTxOutInvalid);
+                       }
+               } else {
+                       // The receiving node:
+                       //  - MUST fail the negotiation if:
+                       //     - `prevtx_vout` is greater or equal to the number of outputs on `prevtx`
+                       return Err(AbortReason::PrevTxOutInvalid);
+               }
+
+               let prev_out = if let Some(prev_out) = transaction.output.get(msg.prevtx_out as usize) {
+                       prev_out.clone()
+               } else {
+                       return Err(AbortReason::PrevTxOutInvalid);
+               };
+               if self.inputs.iter().any(|(serial_id, _)| *serial_id == msg.serial_id) {
+                       // The receiving node:
+                       //  - MUST fail the negotiation if:
+                       //    - the `serial_id` is already included in the transaction
+                       return Err(AbortReason::DuplicateSerialId);
+               }
+               let prev_outpoint = OutPoint { txid, vout: msg.prevtx_out };
+               self.inputs.entry(msg.serial_id).or_insert_with(|| TxInputWithPrevOutput {
+                       input: TxIn {
+                               previous_output: prev_outpoint.clone(),
+                               sequence: Sequence(msg.sequence),
+                               ..Default::default()
+                       },
+                       prev_output: prev_out,
+               });
+               self.prevtx_outpoints.insert(prev_outpoint);
+               Ok(())
+       }
+
+       fn received_tx_remove_input(&mut self, msg: &msgs::TxRemoveInput) -> Result<(), AbortReason> {
+               if !self.is_serial_id_valid_for_counterparty(&msg.serial_id) {
+                       return Err(AbortReason::IncorrectSerialIdParity);
+               }
+
+               self.inputs
+                       .remove(&msg.serial_id)
+                       // The receiving node:
+                       //  - MUST fail the negotiation if:
+                       //    - the input or output identified by the `serial_id` was not added by the sender
+                       //    - the `serial_id` does not correspond to a currently added input
+                       .ok_or(AbortReason::SerialIdUnknown)
+                       .map(|_| ())
+       }
+
+       fn received_tx_add_output(&mut self, msg: &msgs::TxAddOutput) -> Result<(), AbortReason> {
+               // The receiving node:
+               //  - MUST fail the negotiation if:
+               //     - the serial_id has the wrong parity
+               if !self.is_serial_id_valid_for_counterparty(&msg.serial_id) {
+                       return Err(AbortReason::IncorrectSerialIdParity);
+               }
+
+               self.received_tx_add_output_count += 1;
+               if self.received_tx_add_output_count > MAX_RECEIVED_TX_ADD_OUTPUT_COUNT {
+                       // The receiving node:
+                       //  - MUST fail the negotiation if:
+                       //     - if has received 4096 `tx_add_output` messages during this negotiation
+                       return Err(AbortReason::ReceivedTooManyTxAddOutputs);
+               }
+
+               if msg.sats < msg.script.dust_value().to_sat() {
+                       // The receiving node:
+                       // - MUST fail the negotiation if:
+                       //              - the sats amount is less than the dust_limit
+                       return Err(AbortReason::BelowDustLimit);
+               }
+
+               // Check that adding this output would not cause the total output value to exceed the total
+               // bitcoin supply.
+               let mut outputs_value: u64 = 0;
+               for output in self.outputs.iter() {
+                       outputs_value = outputs_value.saturating_add(output.1.value);
+               }
+               if outputs_value.saturating_add(msg.sats) > TOTAL_BITCOIN_SUPPLY_SATOSHIS {
+                       // The receiving node:
+                       // - MUST fail the negotiation if:
+                       //              - the sats amount is greater than 2,100,000,000,000,000 (TOTAL_BITCOIN_SUPPLY_SATOSHIS)
+                       return Err(AbortReason::ExceededMaximumSatsAllowed);
+               }
+
+               // The receiving node:
+               //   - MUST accept P2WSH, P2WPKH, P2TR scripts
+               //   - MAY fail the negotiation if script is non-standard
+               //
+               // We can actually be a bit looser than the above as only witness version 0 has special
+               // length-based standardness constraints to match similar consensus rules. All witness scripts
+               // with witness versions V1 and up are always considered standard. Yes, the scripts can be
+               // anyone-can-spend-able, but if our counterparty wants to add an output like that then it's none
+               // of our concern really Â¯\_(ツ)_/¯
+               if !msg.script.is_v0_p2wpkh()
+                       && !msg.script.is_v0_p2wsh()
+                       && msg.script.witness_version().map(|v| v.to_num() < 1).unwrap_or(true)
+               {
+                       return Err(AbortReason::InvalidOutputScript);
+               }
+
+               if self.outputs.iter().any(|(serial_id, _)| *serial_id == msg.serial_id) {
+                       // The receiving node:
+                       //  - MUST fail the negotiation if:
+                       //    - the `serial_id` is already included in the transaction
+                       return Err(AbortReason::DuplicateSerialId);
+               }
+
+               let output = TxOut { value: msg.sats, script_pubkey: msg.script.clone() };
+               self.outputs.entry(msg.serial_id).or_insert(output);
+               Ok(())
+       }
+
+       fn received_tx_remove_output(&mut self, msg: &msgs::TxRemoveOutput) -> Result<(), AbortReason> {
+               if !self.is_serial_id_valid_for_counterparty(&msg.serial_id) {
+                       return Err(AbortReason::IncorrectSerialIdParity);
+               }
+               if let Some(_) = self.outputs.remove(&msg.serial_id) {
+                       Ok(())
+               } else {
+                       // The receiving node:
+                       //  - MUST fail the negotiation if:
+                       //    - the input or output identified by the `serial_id` was not added by the sender
+                       //    - the `serial_id` does not correspond to a currently added input
+                       Err(AbortReason::SerialIdUnknown)
+               }
+       }
+
+       fn sent_tx_add_input(&mut self, msg: &msgs::TxAddInput) {
+               let tx = msg.prevtx.as_transaction();
+               let input = TxIn {
+                       previous_output: OutPoint { txid: tx.txid(), vout: msg.prevtx_out },
+                       sequence: Sequence(msg.sequence),
+                       ..Default::default()
+               };
+               debug_assert!((msg.prevtx_out as usize) < tx.output.len());
+               let prev_output = &tx.output[msg.prevtx_out as usize];
+               self.prevtx_outpoints.insert(input.previous_output.clone());
+               self.inputs.insert(
+                       msg.serial_id,
+                       TxInputWithPrevOutput { input, prev_output: prev_output.clone() },
+               );
+       }
+
+       fn sent_tx_add_output(&mut self, msg: &msgs::TxAddOutput) {
+               self.outputs
+                       .insert(msg.serial_id, TxOut { value: msg.sats, script_pubkey: msg.script.clone() });
+       }
+
+       fn sent_tx_remove_input(&mut self, msg: &msgs::TxRemoveInput) {
+               self.inputs.remove(&msg.serial_id);
+       }
+
+       fn sent_tx_remove_output(&mut self, msg: &msgs::TxRemoveOutput) {
+               self.outputs.remove(&msg.serial_id);
+       }
+
+       fn build_transaction(self) -> Result<Transaction, AbortReason> {
+               // The receiving node:
+               // MUST fail the negotiation if:
+
+               // - the peer's total input satoshis is less than their outputs
+               let mut counterparty_inputs_value: u64 = 0;
+               let mut counterparty_outputs_value: u64 = 0;
+               for input in self.counterparty_inputs_contributed() {
+                       counterparty_inputs_value =
+                               counterparty_inputs_value.saturating_add(input.prev_output.value);
+               }
+               for output in self.counterparty_outputs_contributed() {
+                       counterparty_outputs_value = counterparty_outputs_value.saturating_add(output.value);
+               }
+               // ...actually the counterparty might be splicing out, so that their balance also contributes
+               // to the total input value.
+               if counterparty_inputs_value.saturating_add(self.to_remote_value_satoshis)
+                       < counterparty_outputs_value
+               {
+                       return Err(AbortReason::OutputsValueExceedsInputsValue);
+               }
+
+               // - there are more than 252 inputs
+               // - there are more than 252 outputs
+               if self.inputs.len() > MAX_INPUTS_OUTPUTS_COUNT
+                       || self.outputs.len() > MAX_INPUTS_OUTPUTS_COUNT
+               {
+                       return Err(AbortReason::ExceededNumberOfInputsOrOutputs);
+               }
+
+               // TODO: How do we enforce their fees cover the witness without knowing its expected length?
+               const INPUT_WEIGHT: u64 = BASE_INPUT_WEIGHT + EMPTY_SCRIPT_SIG_WEIGHT;
+
+               // - the peer's paid feerate does not meet or exceed the agreed feerate (based on the minimum fee).
+               let counterparty_output_weight_contributed: u64 = self
+                       .counterparty_outputs_contributed()
+                       .map(|output| {
+                               (8 /* value */ + output.script_pubkey.consensus_encode(&mut sink()).unwrap() as u64)
+                                       * WITNESS_SCALE_FACTOR as u64
+                       })
+                       .sum();
+               let counterparty_weight_contributed = counterparty_output_weight_contributed
+                       + self.counterparty_inputs_contributed().count() as u64 * INPUT_WEIGHT;
+               let counterparty_fees_contributed =
+                       counterparty_inputs_value.saturating_sub(counterparty_outputs_value);
+               let mut required_counterparty_contribution_fee =
+                       fee_for_weight(self.feerate_sat_per_kw, counterparty_weight_contributed);
+               if !self.holder_is_initiator {
+                       // if is the non-initiator:
+                       //      - the initiator's fees do not cover the common fields (version, segwit marker + flag,
+                       //              input count, output count, locktime)
+                       let tx_common_fields_weight =
+                       (4 /* version */ + 4 /* locktime */ + 1 /* input count */ + 1 /* output count */) *
+                           WITNESS_SCALE_FACTOR as u64 + 2 /* segwit marker + flag */;
+                       let tx_common_fields_fee =
+                               fee_for_weight(self.feerate_sat_per_kw, tx_common_fields_weight);
+                       required_counterparty_contribution_fee += tx_common_fields_fee;
+               }
+               if counterparty_fees_contributed < required_counterparty_contribution_fee {
+                       return Err(AbortReason::InsufficientFees);
+               }
+
+               // Inputs and outputs must be sorted by serial_id
+               let mut inputs = self.inputs.into_iter().collect::<Vec<_>>();
+               let mut outputs = self.outputs.into_iter().collect::<Vec<_>>();
+               inputs.sort_unstable_by_key(|(serial_id, _)| *serial_id);
+               outputs.sort_unstable_by_key(|(serial_id, _)| *serial_id);
+
+               let tx_to_validate = Transaction {
+                       version: 2,
+                       lock_time: self.tx_locktime,
+                       input: inputs.into_iter().map(|(_, input)| input.input).collect(),
+                       output: outputs.into_iter().map(|(_, output)| output).collect(),
+               };
+               if tx_to_validate.weight().to_wu() > MAX_STANDARD_TX_WEIGHT as u64 {
+                       return Err(AbortReason::TransactionTooLarge);
+               }
+
+               Ok(tx_to_validate)
+       }
+}
+
+// The interactive transaction construction protocol allows two peers to collaboratively build a
+// transaction for broadcast.
+//
+// The protocol is turn-based, so we define different states here that we store depending on whose
+// turn it is to send the next message. The states are defined so that their types ensure we only
+// perform actions (only send messages) via defined state transitions that do not violate the
+// protocol.
+//
+// An example of a full negotiation and associated states follows:
+//
+//     +------------+                         +------------------+---- Holder state after message sent/received ----+
+//     |            |--(1)- tx_add_input ---->|                  |                  SentChangeMsg                   +
+//     |            |<-(2)- tx_complete ------|                  |                ReceivedTxComplete                +
+//     |            |--(3)- tx_add_output --->|                  |                  SentChangeMsg                   +
+//     |            |<-(4)- tx_complete ------|                  |                ReceivedTxComplete                +
+//     |            |--(5)- tx_add_input ---->|                  |                  SentChangeMsg                   +
+//     |   Holder   |<-(6)- tx_add_input -----|   Counterparty   |                ReceivedChangeMsg                 +
+//     |            |--(7)- tx_remove_output >|                  |                  SentChangeMsg                   +
+//     |            |<-(8)- tx_add_output ----|                  |                ReceivedChangeMsg                 +
+//     |            |--(9)- tx_complete ----->|                  |                  SentTxComplete                  +
+//     |            |<-(10) tx_complete ------|                  |                NegotiationComplete               +
+//     +------------+                         +------------------+--------------------------------------------------+
+
+/// Negotiation states that can send & receive `tx_(add|remove)_(input|output)` and `tx_complete`
+trait State {}
+
+/// Category of states where we have sent some message to the counterparty, and we are waiting for
+/// a response.
+trait SentMsgState: State {
+       fn into_negotiation_context(self) -> NegotiationContext;
+}
+
+/// Category of states that our counterparty has put us in after we receive a message from them.
+trait ReceivedMsgState: State {
+       fn into_negotiation_context(self) -> NegotiationContext;
+}
+
+// This macro is a helper for implementing the above state traits for various states subsequently
+// defined below the macro.
+macro_rules! define_state {
+       (SENT_MSG_STATE, $state: ident, $doc: expr) => {
+               define_state!($state, NegotiationContext, $doc);
+               impl SentMsgState for $state {
+                       fn into_negotiation_context(self) -> NegotiationContext {
+                               self.0
+                       }
+               }
+       };
+       (RECEIVED_MSG_STATE, $state: ident, $doc: expr) => {
+               define_state!($state, NegotiationContext, $doc);
+               impl ReceivedMsgState for $state {
+                       fn into_negotiation_context(self) -> NegotiationContext {
+                               self.0
+                       }
+               }
+       };
+       ($state: ident, $inner: ident, $doc: expr) => {
+               #[doc = $doc]
+               #[derive(Debug)]
+               struct $state($inner);
+               impl State for $state {}
+       };
+}
+
+define_state!(
+       SENT_MSG_STATE,
+       SentChangeMsg,
+       "We have sent a message to the counterparty that has affected our negotiation state."
+);
+define_state!(
+       SENT_MSG_STATE,
+       SentTxComplete,
+       "We have sent a `tx_complete` message and are awaiting the counterparty's."
+);
+define_state!(
+       RECEIVED_MSG_STATE,
+       ReceivedChangeMsg,
+       "We have received a message from the counterparty that has affected our negotiation state."
+);
+define_state!(
+       RECEIVED_MSG_STATE,
+       ReceivedTxComplete,
+       "We have received a `tx_complete` message and the counterparty is awaiting ours."
+);
+define_state!(NegotiationComplete, Transaction, "We have exchanged consecutive `tx_complete` messages with the counterparty and the transaction negotiation is complete.");
+define_state!(
+       NegotiationAborted,
+       AbortReason,
+       "The negotiation has failed and cannot be continued."
+);
+
+type StateTransitionResult<S> = Result<S, AbortReason>;
+
+trait StateTransition<NewState: State, TransitionData> {
+       fn transition(self, data: TransitionData) -> StateTransitionResult<NewState>;
+}
+
+// This macro helps define the legal transitions between the states above by implementing
+// the `StateTransition` trait for each of the states that follow this declaration.
+macro_rules! define_state_transitions {
+       (SENT_MSG_STATE, [$(DATA $data: ty, TRANSITION $transition: ident),+]) => {
+               $(
+                       impl<S: SentMsgState> StateTransition<ReceivedChangeMsg, $data> for S {
+                               fn transition(self, data: $data) -> StateTransitionResult<ReceivedChangeMsg> {
+                                       let mut context = self.into_negotiation_context();
+                                       context.$transition(data)?;
+                                       Ok(ReceivedChangeMsg(context))
+                               }
+                       }
+                )*
+       };
+       (RECEIVED_MSG_STATE, [$(DATA $data: ty, TRANSITION $transition: ident),+]) => {
+               $(
+                       impl<S: ReceivedMsgState> StateTransition<SentChangeMsg, $data> for S {
+                               fn transition(self, data: $data) -> StateTransitionResult<SentChangeMsg> {
+                                       let mut context = self.into_negotiation_context();
+                                       context.$transition(data);
+                                       Ok(SentChangeMsg(context))
+                               }
+                       }
+                )*
+       };
+       (TX_COMPLETE, $from_state: ident, $tx_complete_state: ident) => {
+               impl StateTransition<NegotiationComplete, &msgs::TxComplete> for $tx_complete_state {
+                       fn transition(self, _data: &msgs::TxComplete) -> StateTransitionResult<NegotiationComplete> {
+                               let context = self.into_negotiation_context();
+                               let tx = context.build_transaction()?;
+                               Ok(NegotiationComplete(tx))
+                       }
+               }
+
+               impl StateTransition<$tx_complete_state, &msgs::TxComplete> for $from_state {
+                       fn transition(self, _data: &msgs::TxComplete) -> StateTransitionResult<$tx_complete_state> {
+                               Ok($tx_complete_state(self.into_negotiation_context()))
+                       }
+               }
+       };
+}
+
+// State transitions when we have sent our counterparty some messages and are waiting for them
+// to respond.
+define_state_transitions!(SENT_MSG_STATE, [
+       DATA &msgs::TxAddInput, TRANSITION received_tx_add_input,
+       DATA &msgs::TxRemoveInput, TRANSITION received_tx_remove_input,
+       DATA &msgs::TxAddOutput, TRANSITION received_tx_add_output,
+       DATA &msgs::TxRemoveOutput, TRANSITION received_tx_remove_output
+]);
+// State transitions when we have received some messages from our counterparty and we should
+// respond.
+define_state_transitions!(RECEIVED_MSG_STATE, [
+       DATA &msgs::TxAddInput, TRANSITION sent_tx_add_input,
+       DATA &msgs::TxRemoveInput, TRANSITION sent_tx_remove_input,
+       DATA &msgs::TxAddOutput, TRANSITION sent_tx_add_output,
+       DATA &msgs::TxRemoveOutput, TRANSITION sent_tx_remove_output
+]);
+define_state_transitions!(TX_COMPLETE, SentChangeMsg, ReceivedTxComplete);
+define_state_transitions!(TX_COMPLETE, ReceivedChangeMsg, SentTxComplete);
+
+#[derive(Debug)]
+enum StateMachine {
+       Indeterminate,
+       SentChangeMsg(SentChangeMsg),
+       ReceivedChangeMsg(ReceivedChangeMsg),
+       SentTxComplete(SentTxComplete),
+       ReceivedTxComplete(ReceivedTxComplete),
+       NegotiationComplete(NegotiationComplete),
+       NegotiationAborted(NegotiationAborted),
+}
+
+impl Default for StateMachine {
+       fn default() -> Self {
+               Self::Indeterminate
+       }
+}
+
+// The `StateMachine` internally executes the actual transition between two states and keeps
+// track of the current state. This macro defines _how_ those state transitions happen to
+// update the internal state.
+macro_rules! define_state_machine_transitions {
+       ($transition: ident, $msg: ty, [$(FROM $from_state: ident, TO $to_state: ident),+]) => {
+               fn $transition(self, msg: $msg) -> StateMachine {
+                       match self {
+                               $(
+                                       Self::$from_state(s) => match s.transition(msg) {
+                                               Ok(new_state) => StateMachine::$to_state(new_state),
+                                               Err(abort_reason) => StateMachine::NegotiationAborted(NegotiationAborted(abort_reason)),
+                                       }
+                                )*
+                               _ => StateMachine::NegotiationAborted(NegotiationAborted(AbortReason::UnexpectedCounterpartyMessage)),
+                       }
+               }
+       };
+}
+
+impl StateMachine {
+       fn new(
+               feerate_sat_per_kw: u32, is_initiator: bool, tx_locktime: AbsoluteLockTime,
+               to_remote_value_satoshis: u64,
+       ) -> Self {
+               let context = NegotiationContext {
+                       tx_locktime,
+                       holder_is_initiator: is_initiator,
+                       received_tx_add_input_count: 0,
+                       received_tx_add_output_count: 0,
+                       inputs: new_hash_map(),
+                       prevtx_outpoints: new_hash_set(),
+                       outputs: new_hash_map(),
+                       feerate_sat_per_kw,
+                       to_remote_value_satoshis,
+               };
+               if is_initiator {
+                       Self::ReceivedChangeMsg(ReceivedChangeMsg(context))
+               } else {
+                       Self::SentChangeMsg(SentChangeMsg(context))
+               }
+       }
+
+       // TxAddInput
+       define_state_machine_transitions!(sent_tx_add_input, &msgs::TxAddInput, [
+               FROM ReceivedChangeMsg, TO SentChangeMsg,
+               FROM ReceivedTxComplete, TO SentChangeMsg
+       ]);
+       define_state_machine_transitions!(received_tx_add_input, &msgs::TxAddInput, [
+               FROM SentChangeMsg, TO ReceivedChangeMsg,
+               FROM SentTxComplete, TO ReceivedChangeMsg
+       ]);
+
+       // TxAddOutput
+       define_state_machine_transitions!(sent_tx_add_output, &msgs::TxAddOutput, [
+               FROM ReceivedChangeMsg, TO SentChangeMsg,
+               FROM ReceivedTxComplete, TO SentChangeMsg
+       ]);
+       define_state_machine_transitions!(received_tx_add_output, &msgs::TxAddOutput, [
+               FROM SentChangeMsg, TO ReceivedChangeMsg,
+               FROM SentTxComplete, TO ReceivedChangeMsg
+       ]);
+
+       // TxRemoveInput
+       define_state_machine_transitions!(sent_tx_remove_input, &msgs::TxRemoveInput, [
+               FROM ReceivedChangeMsg, TO SentChangeMsg,
+               FROM ReceivedTxComplete, TO SentChangeMsg
+       ]);
+       define_state_machine_transitions!(received_tx_remove_input, &msgs::TxRemoveInput, [
+               FROM SentChangeMsg, TO ReceivedChangeMsg,
+               FROM SentTxComplete, TO ReceivedChangeMsg
+       ]);
+
+       // TxRemoveOutput
+       define_state_machine_transitions!(sent_tx_remove_output, &msgs::TxRemoveOutput, [
+               FROM ReceivedChangeMsg, TO SentChangeMsg,
+               FROM ReceivedTxComplete, TO SentChangeMsg
+       ]);
+       define_state_machine_transitions!(received_tx_remove_output, &msgs::TxRemoveOutput, [
+               FROM SentChangeMsg, TO ReceivedChangeMsg,
+               FROM SentTxComplete, TO ReceivedChangeMsg
+       ]);
+
+       // TxComplete
+       define_state_machine_transitions!(sent_tx_complete, &msgs::TxComplete, [
+               FROM ReceivedChangeMsg, TO SentTxComplete,
+               FROM ReceivedTxComplete, TO NegotiationComplete
+       ]);
+       define_state_machine_transitions!(received_tx_complete, &msgs::TxComplete, [
+               FROM SentChangeMsg, TO ReceivedTxComplete,
+               FROM SentTxComplete, TO NegotiationComplete
+       ]);
+}
+
+pub struct InteractiveTxConstructor {
+       state_machine: StateMachine,
+       channel_id: ChannelId,
+       inputs_to_contribute: Vec<(SerialId, TxIn, TransactionU16LenLimited)>,
+       outputs_to_contribute: Vec<(SerialId, TxOut)>,
+}
+
+pub enum InteractiveTxMessageSend {
+       TxAddInput(msgs::TxAddInput),
+       TxAddOutput(msgs::TxAddOutput),
+       TxComplete(msgs::TxComplete),
+}
+
+// This macro executes a state machine transition based on a provided action.
+macro_rules! do_state_transition {
+       ($self: ident, $transition: ident, $msg: expr) => {{
+               let state_machine = core::mem::take(&mut $self.state_machine);
+               $self.state_machine = state_machine.$transition($msg);
+               match &$self.state_machine {
+                       StateMachine::NegotiationAborted(state) => Err(state.0.clone()),
+                       _ => Ok(()),
+               }
+       }};
+}
+
+fn generate_holder_serial_id<ES: Deref>(entropy_source: &ES, is_initiator: bool) -> SerialId
+where
+       ES::Target: EntropySource,
+{
+       let rand_bytes = entropy_source.get_secure_random_bytes();
+       let mut serial_id_bytes = [0u8; 8];
+       serial_id_bytes.copy_from_slice(&rand_bytes[..8]);
+       let mut serial_id = u64::from_be_bytes(serial_id_bytes);
+       if serial_id.is_for_initiator() != is_initiator {
+               serial_id ^= 1;
+       }
+       serial_id
+}
+
+pub enum HandleTxCompleteValue {
+       SendTxMessage(InteractiveTxMessageSend),
+       SendTxComplete(InteractiveTxMessageSend, Transaction),
+       NegotiationComplete(Transaction),
+}
+
+impl InteractiveTxConstructor {
+       /// Instantiates a new `InteractiveTxConstructor`.
+       ///
+       /// If this is for a dual_funded channel then the `to_remote_value_satoshis` parameter should be set
+       /// to zero.
+       ///
+       /// A tuple is returned containing the newly instantiate `InteractiveTxConstructor` and optionally
+       /// an initial wrapped `Tx_` message which the holder needs to send to the counterparty.
+       pub fn new<ES: Deref>(
+               entropy_source: &ES, channel_id: ChannelId, feerate_sat_per_kw: u32, is_initiator: bool,
+               funding_tx_locktime: AbsoluteLockTime,
+               inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited)>,
+               outputs_to_contribute: Vec<TxOut>, to_remote_value_satoshis: u64,
+       ) -> (Self, Option<InteractiveTxMessageSend>)
+       where
+               ES::Target: EntropySource,
+       {
+               let state_machine = StateMachine::new(
+                       feerate_sat_per_kw,
+                       is_initiator,
+                       funding_tx_locktime,
+                       to_remote_value_satoshis,
+               );
+               let mut inputs_to_contribute: Vec<(SerialId, TxIn, TransactionU16LenLimited)> =
+                       inputs_to_contribute
+                               .into_iter()
+                               .map(|(input, tx)| {
+                                       let serial_id = generate_holder_serial_id(entropy_source, is_initiator);
+                                       (serial_id, input, tx)
+                               })
+                               .collect();
+               // We'll sort by the randomly generated serial IDs, effectively shuffling the order of the inputs
+               // as the user passed them to us to avoid leaking any potential categorization of transactions
+               // before we pass any of the inputs to the counterparty.
+               inputs_to_contribute.sort_unstable_by_key(|(serial_id, _, _)| *serial_id);
+               let mut outputs_to_contribute: Vec<(SerialId, TxOut)> = outputs_to_contribute
+                       .into_iter()
+                       .map(|output| {
+                               let serial_id = generate_holder_serial_id(entropy_source, is_initiator);
+                               (serial_id, output)
+                       })
+                       .collect();
+               // In the same manner and for the same rationale as the inputs above, we'll shuffle the outputs.
+               outputs_to_contribute.sort_unstable_by_key(|(serial_id, _)| *serial_id);
+               let mut constructor =
+                       Self { state_machine, channel_id, inputs_to_contribute, outputs_to_contribute };
+               let message_send = if is_initiator {
+                       match constructor.maybe_send_message() {
+                               Ok(msg_send) => Some(msg_send),
+                               Err(_) => {
+                                       debug_assert!(
+                                               false,
+                                               "We should always be able to start our state machine successfully"
+                                       );
+                                       None
+                               },
+                       }
+               } else {
+                       None
+               };
+               (constructor, message_send)
+       }
+
+       fn maybe_send_message(&mut self) -> Result<InteractiveTxMessageSend, AbortReason> {
+               // We first attempt to send inputs we want to add, then outputs. Once we are done sending
+               // them both, then we always send tx_complete.
+               if let Some((serial_id, input, prevtx)) = self.inputs_to_contribute.pop() {
+                       let msg = msgs::TxAddInput {
+                               channel_id: self.channel_id,
+                               serial_id,
+                               prevtx,
+                               prevtx_out: input.previous_output.vout,
+                               sequence: input.sequence.to_consensus_u32(),
+                       };
+                       do_state_transition!(self, sent_tx_add_input, &msg)?;
+                       Ok(InteractiveTxMessageSend::TxAddInput(msg))
+               } else if let Some((serial_id, output)) = self.outputs_to_contribute.pop() {
+                       let msg = msgs::TxAddOutput {
+                               channel_id: self.channel_id,
+                               serial_id,
+                               sats: output.value,
+                               script: output.script_pubkey,
+                       };
+                       do_state_transition!(self, sent_tx_add_output, &msg)?;
+                       Ok(InteractiveTxMessageSend::TxAddOutput(msg))
+               } else {
+                       let msg = msgs::TxComplete { channel_id: self.channel_id };
+                       do_state_transition!(self, sent_tx_complete, &msg)?;
+                       Ok(InteractiveTxMessageSend::TxComplete(msg))
+               }
+       }
+
+       pub fn handle_tx_add_input(
+               &mut self, msg: &msgs::TxAddInput,
+       ) -> Result<InteractiveTxMessageSend, AbortReason> {
+               do_state_transition!(self, received_tx_add_input, msg)?;
+               self.maybe_send_message()
+       }
+
+       pub fn handle_tx_remove_input(
+               &mut self, msg: &msgs::TxRemoveInput,
+       ) -> Result<InteractiveTxMessageSend, AbortReason> {
+               do_state_transition!(self, received_tx_remove_input, msg)?;
+               self.maybe_send_message()
+       }
+
+       pub fn handle_tx_add_output(
+               &mut self, msg: &msgs::TxAddOutput,
+       ) -> Result<InteractiveTxMessageSend, AbortReason> {
+               do_state_transition!(self, received_tx_add_output, msg)?;
+               self.maybe_send_message()
+       }
+
+       pub fn handle_tx_remove_output(
+               &mut self, msg: &msgs::TxRemoveOutput,
+       ) -> Result<InteractiveTxMessageSend, AbortReason> {
+               do_state_transition!(self, received_tx_remove_output, msg)?;
+               self.maybe_send_message()
+       }
+
+       pub fn handle_tx_complete(
+               &mut self, msg: &msgs::TxComplete,
+       ) -> Result<HandleTxCompleteValue, AbortReason> {
+               do_state_transition!(self, received_tx_complete, msg)?;
+               match &self.state_machine {
+                       StateMachine::ReceivedTxComplete(_) => {
+                               let msg_send = self.maybe_send_message()?;
+                               return match &self.state_machine {
+                                       StateMachine::NegotiationComplete(s) => {
+                                               Ok(HandleTxCompleteValue::SendTxComplete(msg_send, s.0.clone()))
+                                       },
+                                       StateMachine::SentChangeMsg(_) => {
+                                               Ok(HandleTxCompleteValue::SendTxMessage(msg_send))
+                                       }, // We either had an input or output to contribute.
+                                       _ => {
+                                               debug_assert!(false, "We cannot transition to any other states after receiving `tx_complete` and responding");
+                                               return Err(AbortReason::InvalidStateTransition);
+                                       },
+                               };
+                       },
+                       StateMachine::NegotiationComplete(s) => {
+                               Ok(HandleTxCompleteValue::NegotiationComplete(s.0.clone()))
+                       },
+                       _ => {
+                               debug_assert!(
+                                       false,
+                                       "We cannot transition to any other states after receiving `tx_complete`"
+                               );
+                               Err(AbortReason::InvalidStateTransition)
+                       },
+               }
+       }
+}
+
+#[cfg(test)]
+mod tests {
+       use crate::chain::chaininterface::FEERATE_FLOOR_SATS_PER_KW;
+       use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS;
+       use crate::ln::interactivetxs::{
+               generate_holder_serial_id, AbortReason, HandleTxCompleteValue, InteractiveTxConstructor,
+               InteractiveTxMessageSend, MAX_INPUTS_OUTPUTS_COUNT, MAX_RECEIVED_TX_ADD_INPUT_COUNT,
+               MAX_RECEIVED_TX_ADD_OUTPUT_COUNT,
+       };
+       use crate::ln::ChannelId;
+       use crate::sign::EntropySource;
+       use crate::util::atomic_counter::AtomicCounter;
+       use crate::util::ser::TransactionU16LenLimited;
+       use bitcoin::blockdata::opcodes;
+       use bitcoin::blockdata::script::Builder;
+       use bitcoin::{
+               absolute::LockTime as AbsoluteLockTime, OutPoint, Sequence, Transaction, TxIn, TxOut,
+       };
+       use core::ops::Deref;
+
+       // A simple entropy source that works based on an atomic counter.
+       struct TestEntropySource(AtomicCounter);
+       impl EntropySource for TestEntropySource {
+               fn get_secure_random_bytes(&self) -> [u8; 32] {
+                       let mut res = [0u8; 32];
+                       let increment = self.0.get_increment();
+                       for i in 0..32 {
+                               // Rotate the increment value by 'i' bits to the right, to avoid clashes
+                               // when `generate_local_serial_id` does a parity flip on consecutive calls for the
+                               // same party.
+                               let rotated_increment = increment.rotate_right(i as u32);
+                               res[i] = (rotated_increment & 0xff) as u8;
+                       }
+                       res
+               }
+       }
+
+       // An entropy source that deliberately returns you the same seed every time. We use this
+       // to test if the constructor would catch inputs/outputs that are attempting to be added
+       // with duplicate serial ids.
+       struct DuplicateEntropySource;
+       impl EntropySource for DuplicateEntropySource {
+               fn get_secure_random_bytes(&self) -> [u8; 32] {
+                       let mut res = [0u8; 32];
+                       let count = 1u64;
+                       res[0..8].copy_from_slice(&count.to_be_bytes());
+                       res
+               }
+       }
+
+       #[derive(Debug, PartialEq, Eq)]
+       enum ErrorCulprit {
+               NodeA,
+               NodeB,
+               // Some error values are only checked at the end of the negotiation and are not easy to attribute
+               // to a particular party. Both parties would indicate an `AbortReason` in this case.
+               // e.g. Exceeded max inputs and outputs after negotiation.
+               Indeterminate,
+       }
+
+       struct TestSession {
+               inputs_a: Vec<(TxIn, TransactionU16LenLimited)>,
+               outputs_a: Vec<TxOut>,
+               inputs_b: Vec<(TxIn, TransactionU16LenLimited)>,
+               outputs_b: Vec<TxOut>,
+               expect_error: Option<(AbortReason, ErrorCulprit)>,
+       }
+
+       fn do_test_interactive_tx_constructor(session: TestSession) {
+               let entropy_source = TestEntropySource(AtomicCounter::new());
+               do_test_interactive_tx_constructor_internal(session, &&entropy_source);
+       }
+
+       fn do_test_interactive_tx_constructor_with_entropy_source<ES: Deref>(
+               session: TestSession, entropy_source: ES,
+       ) where
+               ES::Target: EntropySource,
+       {
+               do_test_interactive_tx_constructor_internal(session, &entropy_source);
+       }
+
+       fn do_test_interactive_tx_constructor_internal<ES: Deref>(
+               session: TestSession, entropy_source: &ES,
+       ) where
+               ES::Target: EntropySource,
+       {
+               let channel_id = ChannelId(entropy_source.get_secure_random_bytes());
+               let tx_locktime = AbsoluteLockTime::from_height(1337).unwrap();
+
+               let (mut constructor_a, first_message_a) = InteractiveTxConstructor::new(
+                       entropy_source,
+                       channel_id,
+                       FEERATE_FLOOR_SATS_PER_KW * 10,
+                       true,
+                       tx_locktime,
+                       session.inputs_a,
+                       session.outputs_a,
+                       0,
+               );
+               let (mut constructor_b, first_message_b) = InteractiveTxConstructor::new(
+                       entropy_source,
+                       channel_id,
+                       FEERATE_FLOOR_SATS_PER_KW * 10,
+                       false,
+                       tx_locktime,
+                       session.inputs_b,
+                       session.outputs_b,
+                       0,
+               );
+
+               let handle_message_send =
+                       |msg: InteractiveTxMessageSend, for_constructor: &mut InteractiveTxConstructor| {
+                               match msg {
+                                       InteractiveTxMessageSend::TxAddInput(msg) => for_constructor
+                                               .handle_tx_add_input(&msg)
+                                               .map(|msg_send| (Some(msg_send), None)),
+                                       InteractiveTxMessageSend::TxAddOutput(msg) => for_constructor
+                                               .handle_tx_add_output(&msg)
+                                               .map(|msg_send| (Some(msg_send), None)),
+                                       InteractiveTxMessageSend::TxComplete(msg) => {
+                                               for_constructor.handle_tx_complete(&msg).map(|value| match value {
+                                                       HandleTxCompleteValue::SendTxMessage(msg_send) => {
+                                                               (Some(msg_send), None)
+                                                       },
+                                                       HandleTxCompleteValue::SendTxComplete(msg_send, tx) => {
+                                                               (Some(msg_send), Some(tx))
+                                                       },
+                                                       HandleTxCompleteValue::NegotiationComplete(tx) => (None, Some(tx)),
+                                               })
+                                       },
+                               }
+                       };
+
+               assert!(first_message_b.is_none());
+               let mut message_send_a = first_message_a;
+               let mut message_send_b = None;
+               let mut final_tx_a = None;
+               let mut final_tx_b = None;
+               while final_tx_a.is_none() || final_tx_b.is_none() {
+                       if let Some(message_send_a) = message_send_a.take() {
+                               match handle_message_send(message_send_a, &mut constructor_b) {
+                                       Ok((msg_send, final_tx)) => {
+                                               message_send_b = msg_send;
+                                               final_tx_b = final_tx;
+                                       },
+                                       Err(abort_reason) => {
+                                               let error_culprit = match abort_reason {
+                                                       AbortReason::ExceededNumberOfInputsOrOutputs => {
+                                                               ErrorCulprit::Indeterminate
+                                                       },
+                                                       _ => ErrorCulprit::NodeA,
+                                               };
+                                               assert_eq!(Some((abort_reason, error_culprit)), session.expect_error);
+                                               assert!(message_send_b.is_none());
+                                               return;
+                                       },
+                               }
+                       }
+                       if let Some(message_send_b) = message_send_b.take() {
+                               match handle_message_send(message_send_b, &mut constructor_a) {
+                                       Ok((msg_send, final_tx)) => {
+                                               message_send_a = msg_send;
+                                               final_tx_a = final_tx;
+                                       },
+                                       Err(abort_reason) => {
+                                               let error_culprit = match abort_reason {
+                                                       AbortReason::ExceededNumberOfInputsOrOutputs => {
+                                                               ErrorCulprit::Indeterminate
+                                                       },
+                                                       _ => ErrorCulprit::NodeB,
+                                               };
+                                               assert_eq!(Some((abort_reason, error_culprit)), session.expect_error);
+                                               assert!(message_send_a.is_none());
+                                               return;
+                                       },
+                               }
+                       }
+               }
+               assert!(message_send_a.is_none());
+               assert!(message_send_b.is_none());
+               assert_eq!(final_tx_a, final_tx_b);
+               assert!(session.expect_error.is_none());
+       }
+
+       fn generate_tx(values: &[u64]) -> Transaction {
+               generate_tx_with_locktime(values, 1337)
+       }
+
+       fn generate_tx_with_locktime(values: &[u64], locktime: u32) -> Transaction {
+               Transaction {
+                       version: 2,
+                       lock_time: AbsoluteLockTime::from_height(locktime).unwrap(),
+                       input: vec![TxIn { ..Default::default() }],
+                       output: values
+                               .iter()
+                               .map(|value| TxOut {
+                                       value: *value,
+                                       script_pubkey: Builder::new()
+                                               .push_opcode(opcodes::OP_TRUE)
+                                               .into_script()
+                                               .to_v0_p2wsh(),
+                               })
+                               .collect(),
+               }
+       }
+
+       fn generate_inputs(values: &[u64]) -> Vec<(TxIn, TransactionU16LenLimited)> {
+               let tx = generate_tx(values);
+               let txid = tx.txid();
+               tx.output
+                       .iter()
+                       .enumerate()
+                       .map(|(idx, _)| {
+                               let input = TxIn {
+                                       previous_output: OutPoint { txid, vout: idx as u32 },
+                                       script_sig: Default::default(),
+                                       sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
+                                       witness: Default::default(),
+                               };
+                               (input, TransactionU16LenLimited::new(tx.clone()).unwrap())
+                       })
+                       .collect()
+       }
+
+       fn generate_outputs(values: &[u64]) -> Vec<TxOut> {
+               values
+                       .iter()
+                       .map(|value| TxOut {
+                               value: *value,
+                               script_pubkey: Builder::new()
+                                       .push_opcode(opcodes::OP_TRUE)
+                                       .into_script()
+                                       .to_v0_p2wsh(),
+                       })
+                       .collect()
+       }
+
+       fn generate_fixed_number_of_inputs(count: u16) -> Vec<(TxIn, TransactionU16LenLimited)> {
+               // Generate transactions with a total `count` number of outputs such that no transaction has a
+               // serialized length greater than u16::MAX.
+               let max_outputs_per_prevtx = 1_500;
+               let mut remaining = count;
+               let mut inputs: Vec<(TxIn, TransactionU16LenLimited)> = Vec::with_capacity(count as usize);
+
+               while remaining > 0 {
+                       let tx_output_count = remaining.min(max_outputs_per_prevtx);
+                       remaining -= tx_output_count;
+
+                       // Use unique locktime for each tx so outpoints are different across transactions
+                       let tx = generate_tx_with_locktime(
+                               &vec![1_000_000; tx_output_count as usize],
+                               (1337 + remaining).into(),
+                       );
+                       let txid = tx.txid();
+
+                       let mut temp: Vec<(TxIn, TransactionU16LenLimited)> = tx
+                               .output
+                               .iter()
+                               .enumerate()
+                               .map(|(idx, _)| {
+                                       let input = TxIn {
+                                               previous_output: OutPoint { txid, vout: idx as u32 },
+                                               script_sig: Default::default(),
+                                               sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
+                                               witness: Default::default(),
+                                       };
+                                       (input, TransactionU16LenLimited::new(tx.clone()).unwrap())
+                               })
+                               .collect();
+
+                       inputs.append(&mut temp);
+               }
+
+               inputs
+       }
+
+       fn generate_fixed_number_of_outputs(count: u16) -> Vec<TxOut> {
+               // Set a constant value for each TxOut
+               generate_outputs(&vec![1_000_000; count as usize])
+       }
+
+       fn generate_non_witness_output(value: u64) -> TxOut {
+               TxOut {
+                       value,
+                       script_pubkey: Builder::new().push_opcode(opcodes::OP_TRUE).into_script().to_p2sh(),
+               }
+       }
+
+       #[test]
+       fn test_interactive_tx_constructor() {
+               // No contributions.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![],
+                       outputs_a: vec![],
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)),
+               });
+               // Single contribution, no initiator inputs.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![],
+                       outputs_a: generate_outputs(&[1_000_000]),
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)),
+               });
+               // Single contribution, no initiator outputs.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: generate_inputs(&[1_000_000]),
+                       outputs_a: vec![],
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: None,
+               });
+               // Single contribution, insufficient fees.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: generate_inputs(&[1_000_000]),
+                       outputs_a: generate_outputs(&[1_000_000]),
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)),
+               });
+               // Initiator contributes sufficient fees, but non-initiator does not.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: generate_inputs(&[1_000_000]),
+                       outputs_a: vec![],
+                       inputs_b: generate_inputs(&[100_000]),
+                       outputs_b: generate_outputs(&[100_000]),
+                       expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeB)),
+               });
+               // Multi-input-output contributions from both sides.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: generate_inputs(&[1_000_000, 1_000_000]),
+                       outputs_a: generate_outputs(&[1_000_000, 200_000]),
+                       inputs_b: generate_inputs(&[1_000_000, 500_000]),
+                       outputs_b: generate_outputs(&[1_000_000, 400_000]),
+                       expect_error: None,
+               });
+
+               // Prevout from initiator is not a witness program
+               let non_segwit_output_tx = {
+                       let mut tx = generate_tx(&[1_000_000]);
+                       tx.output.push(TxOut {
+                               script_pubkey: Builder::new()
+                                       .push_opcode(opcodes::all::OP_RETURN)
+                                       .into_script()
+                                       .to_p2sh(),
+                               ..Default::default()
+                       });
+
+                       TransactionU16LenLimited::new(tx).unwrap()
+               };
+               let non_segwit_input = TxIn {
+                       previous_output: OutPoint {
+                               txid: non_segwit_output_tx.as_transaction().txid(),
+                               vout: 1,
+                       },
+                       sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
+                       ..Default::default()
+               };
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![(non_segwit_input, non_segwit_output_tx)],
+                       outputs_a: vec![],
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)),
+               });
+
+               // Invalid input sequence from initiator.
+               let tx = TransactionU16LenLimited::new(generate_tx(&[1_000_000])).unwrap();
+               let invalid_sequence_input = TxIn {
+                       previous_output: OutPoint { txid: tx.as_transaction().txid(), vout: 0 },
+                       ..Default::default()
+               };
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![(invalid_sequence_input, tx.clone())],
+                       outputs_a: generate_outputs(&[1_000_000]),
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::IncorrectInputSequenceValue, ErrorCulprit::NodeA)),
+               });
+               // Duplicate prevout from initiator.
+               let duplicate_input = TxIn {
+                       previous_output: OutPoint { txid: tx.as_transaction().txid(), vout: 0 },
+                       sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
+                       ..Default::default()
+               };
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![(duplicate_input.clone(), tx.clone()), (duplicate_input, tx.clone())],
+                       outputs_a: generate_outputs(&[1_000_000]),
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)),
+               });
+               // Non-initiator uses same prevout as initiator.
+               let duplicate_input = TxIn {
+                       previous_output: OutPoint { txid: tx.as_transaction().txid(), vout: 0 },
+                       sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
+                       ..Default::default()
+               };
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![(duplicate_input.clone(), tx.clone())],
+                       outputs_a: generate_outputs(&[1_000_000]),
+                       inputs_b: vec![(duplicate_input.clone(), tx.clone())],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeB)),
+               });
+               // Initiator sends too many TxAddInputs
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: generate_fixed_number_of_inputs(MAX_RECEIVED_TX_ADD_INPUT_COUNT + 1),
+                       outputs_a: vec![],
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::ReceivedTooManyTxAddInputs, ErrorCulprit::NodeA)),
+               });
+               // Attempt to queue up two inputs with duplicate serial ids. We use a deliberately bad
+               // entropy source, `DuplicateEntropySource` to simulate this.
+               do_test_interactive_tx_constructor_with_entropy_source(
+                       TestSession {
+                               inputs_a: generate_fixed_number_of_inputs(2),
+                               outputs_a: vec![],
+                               inputs_b: vec![],
+                               outputs_b: vec![],
+                               expect_error: Some((AbortReason::DuplicateSerialId, ErrorCulprit::NodeA)),
+                       },
+                       &DuplicateEntropySource,
+               );
+               // Initiator sends too many TxAddOutputs.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![],
+                       outputs_a: generate_fixed_number_of_outputs(MAX_RECEIVED_TX_ADD_OUTPUT_COUNT + 1),
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::ReceivedTooManyTxAddOutputs, ErrorCulprit::NodeA)),
+               });
+               // Initiator sends an output below dust value.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![],
+                       outputs_a: generate_outputs(&[1]),
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::BelowDustLimit, ErrorCulprit::NodeA)),
+               });
+               // Initiator sends an output above maximum sats allowed.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![],
+                       outputs_a: generate_outputs(&[TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1]),
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::ExceededMaximumSatsAllowed, ErrorCulprit::NodeA)),
+               });
+               // Initiator sends an output without a witness program.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: vec![],
+                       outputs_a: vec![generate_non_witness_output(1_000_000)],
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::InvalidOutputScript, ErrorCulprit::NodeA)),
+               });
+               // Attempt to queue up two outputs with duplicate serial ids. We use a deliberately bad
+               // entropy source, `DuplicateEntropySource` to simulate this.
+               do_test_interactive_tx_constructor_with_entropy_source(
+                       TestSession {
+                               inputs_a: vec![],
+                               outputs_a: generate_fixed_number_of_outputs(2),
+                               inputs_b: vec![],
+                               outputs_b: vec![],
+                               expect_error: Some((AbortReason::DuplicateSerialId, ErrorCulprit::NodeA)),
+                       },
+                       &DuplicateEntropySource,
+               );
+
+               // Peer contributed more output value than inputs
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: generate_inputs(&[100_000]),
+                       outputs_a: generate_outputs(&[1_000_000]),
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)),
+               });
+
+               // Peer contributed more than allowed number of inputs.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: generate_fixed_number_of_inputs(MAX_INPUTS_OUTPUTS_COUNT as u16 + 1),
+                       outputs_a: vec![],
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((
+                               AbortReason::ExceededNumberOfInputsOrOutputs,
+                               ErrorCulprit::Indeterminate,
+                       )),
+               });
+               // Peer contributed more than allowed number of outputs.
+               do_test_interactive_tx_constructor(TestSession {
+                       inputs_a: generate_inputs(&[TOTAL_BITCOIN_SUPPLY_SATOSHIS]),
+                       outputs_a: generate_fixed_number_of_outputs(MAX_INPUTS_OUTPUTS_COUNT as u16 + 1),
+                       inputs_b: vec![],
+                       outputs_b: vec![],
+                       expect_error: Some((
+                               AbortReason::ExceededNumberOfInputsOrOutputs,
+                               ErrorCulprit::Indeterminate,
+                       )),
+               });
+       }
+
+       #[test]
+       fn test_generate_local_serial_id() {
+               let entropy_source = TestEntropySource(AtomicCounter::new());
+
+               // Initiators should have even serial id, non-initiators should have odd serial id.
+               assert_eq!(generate_holder_serial_id(&&entropy_source, true) % 2, 0);
+               assert_eq!(generate_holder_serial_id(&&entropy_source, false) % 2, 1)
+       }
+}
index 71b73390d1c80a9590afedecfbee6f542822a076..25b32c4c79dda078d3daaac73b3ece86e9414c48 100644 (file)
@@ -82,6 +82,8 @@ mod async_signer_tests;
 #[cfg(test)]
 #[allow(unused_mut)]
 mod offers_tests;
+#[allow(dead_code)] // TODO(dual_funding): Exchange for dual_funding cfg
+pub(crate) mod interactivetxs;
 
 pub use self::peer_channel_encryptor::LN_MAX_MSG_LEN;
 
index 25084689fb73ff6c817fca706638a25689e84d7c..d5f0dc153fc1a1ba9c544c4cfeb552708f860e04 100644 (file)
@@ -1188,14 +1188,14 @@ fn do_test_revoked_counterparty_commitment_balances(anchors: bool, confirm_htlc_
        assert!(failed_payments.is_empty());
        if let Event::PendingHTLCsForwardable { .. } = events[0] {} else { panic!(); }
        match &events[1] {
-               Event::ChannelClosed { reason: ClosureReason::HolderForceClosed, .. } => {},
+               Event::ChannelClosed { reason: ClosureReason::HTLCsTimedOut, .. } => {},
                _ => panic!(),
        }
 
        connect_blocks(&nodes[1], htlc_cltv_timeout + 1 - 10);
        check_closed_broadcast!(nodes[1], true);
        check_added_monitors!(nodes[1], 1);
-       check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 1000000);
+       check_closed_event!(nodes[1], 1, ClosureReason::HTLCsTimedOut, [nodes[0].node.get_our_node_id()], 1000000);
 
        // Prior to channel closure, B considers the preimage HTLC as its own, and otherwise only
        // lists the two on-chain timeout-able HTLCs as claimable balances.
index 1958a352622d9d32e19aa4b0076d7f68de3d4f73..034b2451693975f04da3a310b432be6b2ea4de85 100644 (file)
@@ -396,6 +396,10 @@ pub struct ChannelReady {
        pub short_channel_id_alias: Option<u64>,
 }
 
+/// A randomly chosen number that is used to identify inputs within an interactive transaction
+/// construction.
+pub type SerialId = u64;
+
 /// An stfu (quiescence) message to be sent by or received from the stfu initiator.
 // TODO(splicing): Add spec link for `stfu`; still in draft, using from https://github.com/lightning/bolts/pull/863
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -459,7 +463,7 @@ pub struct TxAddInput {
        pub channel_id: ChannelId,
        /// A randomly chosen unique identifier for this input, which is even for initiators and odd for
        /// non-initiators.
-       pub serial_id: u64,
+       pub serial_id: SerialId,
        /// Serialized transaction that contains the output this input spends to verify that it is non
        /// malleable.
        pub prevtx: TransactionU16LenLimited,
@@ -478,7 +482,7 @@ pub struct TxAddOutput {
        pub channel_id: ChannelId,
        /// A randomly chosen unique identifier for this output, which is even for initiators and odd for
        /// non-initiators.
-       pub serial_id: u64,
+       pub serial_id: SerialId,
        /// The satoshi value of the output
        pub sats: u64,
        /// The scriptPubKey for the output
@@ -493,7 +497,7 @@ pub struct TxRemoveInput {
        /// The channel ID
        pub channel_id: ChannelId,
        /// The serial ID of the input to be removed
-       pub serial_id: u64,
+       pub serial_id: SerialId,
 }
 
 /// A tx_remove_output message for removing an output during interactive transaction construction.
@@ -504,7 +508,7 @@ pub struct TxRemoveOutput {
        /// The channel ID
        pub channel_id: ChannelId,
        /// The serial ID of the output to be removed
-       pub serial_id: u64,
+       pub serial_id: SerialId,
 }
 
 /// A tx_complete message signalling the conclusion of a peer's transaction contributions during
@@ -1136,8 +1140,16 @@ pub struct UnsignedNodeAnnouncement {
        pub alias: NodeAlias,
        /// List of addresses on which this node is reachable
        pub addresses: Vec<SocketAddress>,
-       pub(crate) excess_address_data: Vec<u8>,
-       pub(crate) excess_data: Vec<u8>,
+       /// Excess address data which was signed as a part of the message which we do not (yet) understand how
+       /// to decode.
+       ///
+       /// This is stored to ensure forward-compatibility as new address types are added to the lightning gossip protocol.
+       pub excess_address_data: Vec<u8>,
+       /// Excess data which was signed as a part of the message which we do not (yet) understand how
+       /// to decode.
+       ///
+       /// This is stored to ensure forward-compatibility as new fields are added to the lightning gossip protocol.
+       pub excess_data: Vec<u8>,
 }
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
 /// A [`node_announcement`] message to be sent to or received from a peer.
index a31d9520dad392d94f4956e0f8546926cb917668..62c82b01f59d7a36edd81cefb1cd45d407788d85 100644 (file)
@@ -465,7 +465,7 @@ fn test_set_outpoints_partial_claiming() {
        // Connect blocks on node B
        connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
        check_closed_broadcast!(nodes[1], true);
-       check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 1000000);
+       check_closed_event!(nodes[1], 1, ClosureReason::HTLCsTimedOut, [nodes[0].node.get_our_node_id()], 1000000);
        check_added_monitors!(nodes[1], 1);
        // Verify node B broadcast 2 HTLC-timeout txn
        let partial_claim_tx = {
@@ -807,7 +807,7 @@ fn do_test_retries_own_commitment_broadcast_after_reorg(anchors: bool, revoked_c
        connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
        check_closed_broadcast(&nodes[0], 1, true);
        check_added_monitors(&nodes[0], 1);
-       check_closed_event(&nodes[0], 1, ClosureReason::HolderForceClosed, false, &[nodes[1].node.get_our_node_id()], 100_000);
+       check_closed_event(&nodes[0], 1, ClosureReason::HTLCsTimedOut, false, &[nodes[1].node.get_our_node_id()], 100_000);
 
        {
                let mut txn = nodes[0].tx_broadcaster.txn_broadcast();
index 271b56dc0c92a7a2de07e7d3b6c652777eb608d0..16f0babe3612e365accffd4c6a7ee110e65880ba 100644 (file)
 
 use crate::blinded_path::BlindedPath;
 use crate::events::{Event, EventsProvider};
-use crate::ln::features::InitFeatures;
-use crate::ln::msgs::{self, DecodeError, OnionMessageHandler, SocketAddress};
+use crate::ln::features::{ChannelFeatures, InitFeatures};
+use crate::ln::msgs::{self, DecodeError, OnionMessageHandler};
+use crate::routing::gossip::{NetworkGraph, P2PGossipSync};
+use crate::routing::test_utils::{add_channel, add_or_update_node};
 use crate::sign::{NodeSigner, Recipient};
 use crate::util::ser::{FixedLengthReader, LengthReadable, Writeable, Writer};
 use crate::util::test_utils;
-use super::messenger::{CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger, PendingOnionMessage, SendError};
+use super::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, OnionMessagePath, OnionMessenger, PendingOnionMessage, SendError};
 use super::offers::{OffersMessage, OffersMessageHandler};
 use super::packet::{OnionMessageContents, Packet};
 
 use bitcoin::network::constants::Network;
 use bitcoin::hashes::hex::FromHex;
-use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey, self};
+use bitcoin::secp256k1::{All, PublicKey, Secp256k1, SecretKey};
 
 use crate::io;
 use crate::io_extras::read_to_end;
 use crate::sync::{Arc, Mutex};
 
+use core::ops::Deref;
+
 use crate::prelude::*;
 
 struct MessengerNode {
        node_id: PublicKey,
+       privkey: SecretKey,
        entropy_source: Arc<test_utils::TestKeysInterface>,
        messenger: OnionMessenger<
                Arc<test_utils::TestKeysInterface>,
                Arc<test_utils::TestNodeSigner>,
                Arc<test_utils::TestLogger>,
-               Arc<TestMessageRouter>,
+               Arc<DefaultMessageRouter<
+                       Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
+                       Arc<test_utils::TestLogger>,
+                       Arc<test_utils::TestKeysInterface>
+               >>,
                Arc<TestOffersMessageHandler>,
                Arc<TestCustomMessageHandler>
        >,
        custom_message_handler: Arc<TestCustomMessageHandler>,
-}
-
-struct TestMessageRouter {}
-
-impl MessageRouter for TestMessageRouter {
-       fn find_path(
-               &self, _sender: PublicKey, _peers: Vec<PublicKey>, destination: Destination
-       ) -> Result<OnionMessagePath, ()> {
-               Ok(OnionMessagePath {
-                       intermediate_nodes: vec![],
-                       destination,
-                       first_node_addresses:
-                               Some(vec![SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 1000 }]),
-               })
-       }
-
-       fn create_blinded_paths<
-               T: secp256k1::Signing + secp256k1::Verification
-       >(
-               &self, _recipient: PublicKey, _peers: Vec<PublicKey>, _secp_ctx: &Secp256k1<T>,
-       ) -> Result<Vec<BlindedPath>, ()> {
-               unreachable!()
-       }
+       gossip_sync: Arc<P2PGossipSync<
+               Arc<NetworkGraph<Arc<test_utils::TestLogger>>>,
+               Arc<test_utils::TestChainSource>,
+               Arc<test_utils::TestLogger>
+       >>
 }
 
 struct TestOffersMessageHandler {}
@@ -171,6 +162,12 @@ fn create_nodes(num_messengers: u8) -> Vec<MessengerNode> {
 }
 
 fn create_nodes_using_secrets(secrets: Vec<SecretKey>) -> Vec<MessengerNode> {
+       let gossip_logger = Arc::new(test_utils::TestLogger::with_id("gossip".to_string()));
+       let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, gossip_logger.clone()));
+       let gossip_sync = Arc::new(
+               P2PGossipSync::new(network_graph.clone(), None, gossip_logger)
+       );
+
        let mut nodes = Vec::new();
        for (i, secret_key) in secrets.into_iter().enumerate() {
                let logger = Arc::new(test_utils::TestLogger::with_id(format!("node {}", i)));
@@ -178,10 +175,13 @@ fn create_nodes_using_secrets(secrets: Vec<SecretKey>) -> Vec<MessengerNode> {
                let entropy_source = Arc::new(test_utils::TestKeysInterface::new(&seed, Network::Testnet));
                let node_signer = Arc::new(test_utils::TestNodeSigner::new(secret_key));
 
-               let message_router = Arc::new(TestMessageRouter {});
+               let message_router = Arc::new(
+                       DefaultMessageRouter::new(network_graph.clone(), entropy_source.clone())
+               );
                let offers_message_handler = Arc::new(TestOffersMessageHandler {});
                let custom_message_handler = Arc::new(TestCustomMessageHandler::new());
                nodes.push(MessengerNode {
+                       privkey: secret_key,
                        node_id: node_signer.get_node_id(Recipient::Node).unwrap(),
                        entropy_source: entropy_source.clone(),
                        messenger: OnionMessenger::new(
@@ -189,6 +189,7 @@ fn create_nodes_using_secrets(secrets: Vec<SecretKey>) -> Vec<MessengerNode> {
                                offers_message_handler, custom_message_handler.clone()
                        ),
                        custom_message_handler,
+                       gossip_sync: gossip_sync.clone(),
                });
        }
        for i in 0..nodes.len() - 1 {
@@ -216,6 +217,20 @@ fn release_events(node: &MessengerNode) -> Vec<Event> {
        events.into_inner()
 }
 
+fn add_channel_to_graph(
+       node_a: &MessengerNode, node_b: &MessengerNode, secp_ctx: &Secp256k1<All>, short_channel_id: u64
+) {
+       let gossip_sync = node_a.gossip_sync.deref();
+       let privkey_a = &node_a.privkey;
+       let privkey_b = &node_b.privkey;
+       let channel_features = ChannelFeatures::empty();
+       let node_features_a = node_a.messenger.provided_node_features();
+       let node_features_b = node_b.messenger.provided_node_features();
+       add_channel(gossip_sync, secp_ctx, privkey_a, privkey_b, channel_features, short_channel_id);
+       add_or_update_node(gossip_sync, secp_ctx, privkey_a, node_features_a, 1);
+       add_or_update_node(gossip_sync, secp_ctx, privkey_b, node_features_b, 1);
+}
+
 fn pass_along_path(path: &Vec<MessengerNode>) {
        let mut prev_node = &path[0];
        for node in path.into_iter().skip(1) {
@@ -235,12 +250,8 @@ fn one_unblinded_hop() {
        let nodes = create_nodes(2);
        let test_msg = TestCustomMessage::Response;
 
-       let path = OnionMessagePath {
-               intermediate_nodes: vec![],
-               destination: Destination::Node(nodes[1].node_id),
-               first_node_addresses: None,
-       };
-       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
+       let destination = Destination::Node(nodes[1].node_id);
+       nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap();
        nodes[1].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 }
@@ -255,6 +266,7 @@ fn two_unblinded_hops() {
                destination: Destination::Node(nodes[2].node_id),
                first_node_addresses: None,
        };
+
        nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
        nodes[2].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
@@ -267,12 +279,8 @@ fn one_blinded_hop() {
 
        let secp_ctx = Secp256k1::new();
        let blinded_path = BlindedPath::new_for_message(&[nodes[1].node_id], &*nodes[1].entropy_source, &secp_ctx).unwrap();
-       let path = OnionMessagePath {
-               intermediate_nodes: vec![],
-               destination: Destination::BlindedPath(blinded_path),
-               first_node_addresses: None,
-       };
-       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
+       let destination = Destination::BlindedPath(blinded_path);
+       nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap();
        nodes[1].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 }
@@ -302,13 +310,9 @@ fn three_blinded_hops() {
 
        let secp_ctx = Secp256k1::new();
        let blinded_path = BlindedPath::new_for_message(&[nodes[1].node_id, nodes[2].node_id, nodes[3].node_id], &*nodes[3].entropy_source, &secp_ctx).unwrap();
-       let path = OnionMessagePath {
-               intermediate_nodes: vec![],
-               destination: Destination::BlindedPath(blinded_path),
-               first_node_addresses: None,
-       };
+       let destination = Destination::BlindedPath(blinded_path);
 
-       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
+       nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap();
        nodes[3].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 }
@@ -339,24 +343,16 @@ fn we_are_intro_node() {
 
        let secp_ctx = Secp256k1::new();
        let blinded_path = BlindedPath::new_for_message(&[nodes[0].node_id, nodes[1].node_id, nodes[2].node_id], &*nodes[2].entropy_source, &secp_ctx).unwrap();
-       let path = OnionMessagePath {
-               intermediate_nodes: vec![],
-               destination: Destination::BlindedPath(blinded_path),
-               first_node_addresses: None,
-       };
+       let destination = Destination::BlindedPath(blinded_path);
 
-       nodes[0].messenger.send_onion_message_using_path(path, test_msg.clone(), None).unwrap();
+       nodes[0].messenger.send_onion_message(test_msg.clone(), destination, None).unwrap();
        nodes[2].custom_message_handler.expect_message(TestCustomMessage::Response);
        pass_along_path(&nodes);
 
        // Try with a two-hop blinded path where we are the introduction node.
        let blinded_path = BlindedPath::new_for_message(&[nodes[0].node_id, nodes[1].node_id], &*nodes[1].entropy_source, &secp_ctx).unwrap();
-       let path = OnionMessagePath {
-               intermediate_nodes: vec![],
-               destination: Destination::BlindedPath(blinded_path),
-               first_node_addresses: None,
-       };
-       nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap();
+       let destination = Destination::BlindedPath(blinded_path);
+       nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap();
        nodes[1].custom_message_handler.expect_message(TestCustomMessage::Response);
        nodes.remove(2);
        pass_along_path(&nodes);
@@ -372,12 +368,8 @@ fn invalid_blinded_path_error() {
        let secp_ctx = Secp256k1::new();
        let mut blinded_path = BlindedPath::new_for_message(&[nodes[1].node_id, nodes[2].node_id], &*nodes[2].entropy_source, &secp_ctx).unwrap();
        blinded_path.blinded_hops.clear();
-       let path = OnionMessagePath {
-               intermediate_nodes: vec![],
-               destination: Destination::BlindedPath(blinded_path),
-               first_node_addresses: None,
-       };
-       let err = nodes[0].messenger.send_onion_message_using_path(path, test_msg.clone(), None).unwrap_err();
+       let destination = Destination::BlindedPath(blinded_path);
+       let err = nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap_err();
        assert_eq!(err, SendError::TooFewBlindedHops);
 }
 
@@ -404,14 +396,10 @@ fn reply_path() {
 
        // Destination::BlindedPath
        let blinded_path = BlindedPath::new_for_message(&[nodes[1].node_id, nodes[2].node_id, nodes[3].node_id], &*nodes[3].entropy_source, &secp_ctx).unwrap();
-       let path = OnionMessagePath {
-               intermediate_nodes: vec![],
-               destination: Destination::BlindedPath(blinded_path),
-               first_node_addresses: None,
-       };
+       let destination = Destination::BlindedPath(blinded_path);
        let reply_path = BlindedPath::new_for_message(&[nodes[2].node_id, nodes[1].node_id, nodes[0].node_id], &*nodes[0].entropy_source, &secp_ctx).unwrap();
 
-       nodes[0].messenger.send_onion_message_using_path(path, test_msg, Some(reply_path)).unwrap();
+       nodes[0].messenger.send_onion_message(test_msg, destination, Some(reply_path)).unwrap();
        nodes[3].custom_message_handler.expect_message(TestCustomMessage::Request);
        pass_along_path(&nodes);
 
@@ -439,12 +427,8 @@ fn invalid_custom_message_type() {
        }
 
        let test_msg = InvalidCustomMessage {};
-       let path = OnionMessagePath {
-               intermediate_nodes: vec![],
-               destination: Destination::Node(nodes[1].node_id),
-               first_node_addresses: None,
-       };
-       let err = nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap_err();
+       let destination = Destination::Node(nodes[1].node_id);
+       let err = nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap_err();
        assert_eq!(err, SendError::InvalidMessage);
 }
 
@@ -452,15 +436,11 @@ fn invalid_custom_message_type() {
 fn peer_buffer_full() {
        let nodes = create_nodes(2);
        let test_msg = TestCustomMessage::Request;
-       let path = OnionMessagePath {
-               intermediate_nodes: vec![],
-               destination: Destination::Node(nodes[1].node_id),
-               first_node_addresses: None,
-       };
+       let destination = Destination::Node(nodes[1].node_id);
        for _ in 0..188 { // Based on MAX_PER_PEER_BUFFER_SIZE in OnionMessenger
-               nodes[0].messenger.send_onion_message_using_path(path.clone(), test_msg.clone(), None).unwrap();
+               nodes[0].messenger.send_onion_message(test_msg.clone(), destination.clone(), None).unwrap();
        }
-       let err = nodes[0].messenger.send_onion_message_using_path(path, test_msg, None).unwrap_err();
+       let err = nodes[0].messenger.send_onion_message(test_msg, destination, None).unwrap_err();
        assert_eq!(err, SendError::BufferFull);
 }
 
@@ -492,6 +472,8 @@ fn requests_peer_connection_for_buffered_messages() {
        let nodes = create_nodes(3);
        let message = TestCustomMessage::Request;
        let secp_ctx = Secp256k1::new();
+       add_channel_to_graph(&nodes[0], &nodes[1], &secp_ctx, 42);
+
        let blinded_path = BlindedPath::new_for_message(
                &[nodes[1].node_id, nodes[2].node_id], &*nodes[0].entropy_source, &secp_ctx
        ).unwrap();
@@ -527,6 +509,8 @@ fn drops_buffered_messages_waiting_for_peer_connection() {
        let nodes = create_nodes(3);
        let message = TestCustomMessage::Request;
        let secp_ctx = Secp256k1::new();
+       add_channel_to_graph(&nodes[0], &nodes[1], &secp_ctx, 42);
+
        let blinded_path = BlindedPath::new_for_message(
                &[nodes[1].node_id, nodes[2].node_id], &*nodes[0].entropy_source, &secp_ctx
        ).unwrap();
index 9f6c5cb435276ffbc4a76793f1f9fea624986de4..e213bcbb0e1dc50d9fda8983f164e82611db26d9 100644 (file)
@@ -318,10 +318,10 @@ where
        ES::Target: EntropySource,
 {
        fn find_path(
-               &self, _sender: PublicKey, peers: Vec<PublicKey>, destination: Destination
+               &self, sender: PublicKey, peers: Vec<PublicKey>, destination: Destination
        ) -> Result<OnionMessagePath, ()> {
                let first_node = destination.first_node();
-               if peers.contains(&first_node) {
+               if peers.contains(&first_node) || sender == first_node {
                        Ok(OnionMessagePath {
                                intermediate_nodes: vec![], destination, first_node_addresses: None
                        })
index 806aadd583c02491df9b87b1076a4166b65d71c1..045772486ba7aa150636a1e7b5420a606e87ce64 100644 (file)
@@ -74,6 +74,16 @@ impl NodeId {
                NodeId(pubkey.serialize())
        }
 
+       /// Create a new NodeId from a slice of bytes
+       pub fn from_slice(bytes: &[u8]) -> Result<Self, DecodeError> {
+               if bytes.len() != PUBLIC_KEY_SIZE {
+                       return Err(DecodeError::InvalidValue);
+               }
+               let mut data = [0; PUBLIC_KEY_SIZE];
+               data.copy_from_slice(bytes);
+               Ok(NodeId(data))
+       }
+
        /// Get the public key slice from this NodeId
        pub fn as_slice(&self) -> &[u8] {
                &self.0
index 7fff856345c84c8f8fadadfe262992a4a4e8f6bc..9e6d8e2afd2f763f90ccff2e2eab517d1f30a573 100644 (file)
@@ -14,4 +14,4 @@ pub mod gossip;
 pub mod router;
 pub mod scoring;
 #[cfg(test)]
-mod test_utils;
+pub(crate) mod test_utils;
index 3c0ef85fd7e3815696a669bc34203d411535f337..9f03b4451eff025665517769934b9a62fd09be47 100644 (file)
@@ -9,8 +9,7 @@
 
 use crate::routing::gossip::{NetworkGraph, NodeAlias, P2PGossipSync};
 use crate::ln::features::{ChannelFeatures, NodeFeatures};
-use crate::ln::msgs::{UnsignedChannelAnnouncement, ChannelAnnouncement, RoutingMessageHandler,
-       NodeAnnouncement, UnsignedNodeAnnouncement, ChannelUpdate, UnsignedChannelUpdate, MAX_VALUE_MSAT};
+use crate::ln::msgs::{ChannelAnnouncement, ChannelUpdate, MAX_VALUE_MSAT, NodeAnnouncement, RoutingMessageHandler, SocketAddress, UnsignedChannelAnnouncement, UnsignedChannelUpdate, UnsignedNodeAnnouncement};
 use crate::util::test_utils;
 use crate::util::ser::Writeable;
 
@@ -28,7 +27,7 @@ use crate::sync::{self, Arc};
 use crate::routing::gossip::NodeId;
 
 // Using the same keys for LN and BTC ids
-pub(super) fn add_channel(
+pub(crate) fn add_channel(
        gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
        secp_ctx: &Secp256k1<All>, node_1_privkey: &SecretKey, node_2_privkey: &SecretKey, features: ChannelFeatures, short_channel_id: u64
 ) {
@@ -60,7 +59,7 @@ pub(super) fn add_channel(
        };
 }
 
-pub(super) fn add_or_update_node(
+pub(crate) fn add_or_update_node(
        gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
        secp_ctx: &Secp256k1<All>, node_privkey: &SecretKey, features: NodeFeatures, timestamp: u32
 ) {
@@ -71,7 +70,7 @@ pub(super) fn add_or_update_node(
                node_id,
                rgb: [0; 3],
                alias: NodeAlias([0; 32]),
-               addresses: Vec::new(),
+               addresses: vec![SocketAddress::TcpIpV4 { addr: [127, 0, 0, 1], port: 1000 }],
                excess_address_data: Vec::new(),
                excess_data: Vec::new(),
        };
@@ -87,7 +86,7 @@ pub(super) fn add_or_update_node(
        };
 }
 
-pub(super) fn update_channel(
+pub(crate) fn update_channel(
        gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
        secp_ctx: &Secp256k1<All>, node_privkey: &SecretKey, update: UnsignedChannelUpdate
 ) {
index 7447c92d4be8b61abc23b0527d458796687bd29e..0611e6ffb385b66459f93d3cee1fc46ec1f46223 100644 (file)
@@ -1408,6 +1408,11 @@ impl TransactionU16LenLimited {
        pub fn into_transaction(self) -> Transaction {
                self.0
        }
+
+       /// Returns a reference to the contained `Transaction`
+       pub fn as_transaction(&self) -> &Transaction {
+               &self.0
+       }
 }
 
 impl Writeable for TransactionU16LenLimited {
diff --git a/pending_changelog/blinded-hop-features-optional.txt b/pending_changelog/blinded-hop-features-optional.txt
new file mode 100644 (file)
index 0000000..f8967f1
--- /dev/null
@@ -0,0 +1,5 @@
+## Bug Fixes
+
+* LDK previously would fail to forward an intermediate blinded payment
+       if the blinded hop features were absent, potentially breaking
+       interoperability.
diff --git a/pending_changelog/relay-constraints-ser.txt b/pending_changelog/relay-constraints-ser.txt
new file mode 100644 (file)
index 0000000..f0da509
--- /dev/null
@@ -0,0 +1,7 @@
+## Bug fixes
+
+* LDK previously serialized `PaymentRelay::fee_base_msat` as a u32 when it
+       should have been serialized as a tu32. Similarly, we were serializing
+       `PaymentConstraints::htlc_minimum_msat` as a u64 when we should have been
+       serializing it as tu64. This caused lack of interoperability when using other
+       implementations as forwarding nodes along blinded payment paths.