diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1ca067f43f4..585e1ac134e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -60,7 +60,8 @@ use crate::ln::funding::{FundingTxInput, SpliceContribution}; use crate::ln::interactivetxs::{ calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend, - InteractiveTxSigningSession, SharedOwnedInput, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT, + InteractiveTxSigningSession, NegotiationError, SharedOwnedInput, SharedOwnedOutput, + TX_COMMON_FIELDS_WEIGHT, }; use crate::ln::msgs; use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket}; @@ -1794,20 +1795,22 @@ where let (interactive_tx_msg_send, negotiation_complete) = match tx_complete_action { HandleTxCompleteValue::SendTxMessage(interactive_tx_msg_send) => { - (Some(interactive_tx_msg_send), false) + (Some(interactive_tx_msg_send), None) }, - HandleTxCompleteValue::SendTxComplete( + HandleTxCompleteValue::NegotiationComplete( interactive_tx_msg_send, - negotiation_complete, - ) => (Some(interactive_tx_msg_send), negotiation_complete), - HandleTxCompleteValue::NegotiationComplete => (None, true), + funding_outpoint, + ) => (interactive_tx_msg_send, Some(funding_outpoint)), }; - if !negotiation_complete { + + let funding_outpoint = if let Some(funding_outpoint) = negotiation_complete { + funding_outpoint + } else { return Ok((interactive_tx_msg_send, None)); - } + }; let commitment_signed = self - .funding_tx_constructed(logger) + .funding_tx_constructed(funding_outpoint, logger) .map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))?; Ok((interactive_tx_msg_send, Some(commitment_signed))) } @@ -1890,13 +1893,13 @@ where } fn funding_tx_constructed( - &mut self, logger: &L, + &mut self, funding_outpoint: OutPoint, logger: &L, ) -> Result where L::Target: Logger, { let logger = WithChannelContext::from(logger, self.context(), None); - match &mut self.phase { + let (interactive_tx_constructor, commitment_signed) = match &mut self.phase { ChannelPhase::UnfundedV2(chan) => { debug_assert_eq!( chan.context.channel_state, @@ -1906,52 +1909,73 @@ where ), ); - let signing_session = chan + let interactive_tx_constructor = chan .interactive_tx_constructor .take() - .expect("PendingV2Channel::interactive_tx_constructor should be set") - .into_signing_session(); + .expect("PendingV2Channel::interactive_tx_constructor should be set"); let commitment_signed = chan.context.funding_tx_constructed( &mut chan.funding, - signing_session, + funding_outpoint, false, chan.unfunded_context.transaction_number(), &&logger, )?; - return Ok(commitment_signed); + (interactive_tx_constructor, commitment_signed) }, ChannelPhase::Funded(chan) => { if let Some(pending_splice) = chan.pending_splice.as_mut() { - if let Some(funding_negotiation) = pending_splice.funding_negotiation.take() { - if let FundingNegotiation::ConstructingTransaction { - mut funding, - interactive_tx_constructor, - } = funding_negotiation - { - let signing_session = interactive_tx_constructor.into_signing_session(); - let commitment_signed = chan.context.funding_tx_constructed( + pending_splice + .funding_negotiation + .take() + .and_then(|funding_negotiation| { + if let FundingNegotiation::ConstructingTransaction { + funding, + interactive_tx_constructor, + } = funding_negotiation + { + Some((funding, interactive_tx_constructor)) + } else { + // Replace the taken state for later error handling + pending_splice.funding_negotiation = Some(funding_negotiation); + None + } + }) + .ok_or_else(|| { + AbortReason::InternalError( + "Got a tx_complete message in an invalid state", + ) + }) + .and_then(|(mut funding, interactive_tx_constructor)| { + match chan.context.funding_tx_constructed( &mut funding, - signing_session, + funding_outpoint, true, chan.holder_commitment_point.next_transaction_number(), &&logger, - )?; - - pending_splice.funding_negotiation = - Some(FundingNegotiation::AwaitingSignatures { funding }); - - return Ok(commitment_signed); - } else { - // Replace the taken state - pending_splice.funding_negotiation = Some(funding_negotiation); - } - } + ) { + Ok(commitment_signed) => { + // Advance the state + pending_splice.funding_negotiation = + Some(FundingNegotiation::AwaitingSignatures { funding }); + Ok((interactive_tx_constructor, commitment_signed)) + }, + Err(e) => { + // Restore the taken state for later error handling + pending_splice.funding_negotiation = + Some(FundingNegotiation::ConstructingTransaction { + funding, + interactive_tx_constructor, + }); + Err(e) + }, + } + })? + } else { + return Err(AbortReason::InternalError( + "Got a tx_complete message in an invalid state", + )); } - - return Err(AbortReason::InternalError( - "Got a tx_complete message in an invalid state", - )); }, _ => { debug_assert!(false); @@ -1959,7 +1983,11 @@ where "Got a tx_complete message in an invalid phase", )); }, - } + }; + + let signing_session = interactive_tx_constructor.into_signing_session(); + self.context_mut().interactive_tx_signing_session = Some(signing_session); + Ok(commitment_signed) } pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult { @@ -6051,30 +6079,13 @@ where #[rustfmt::skip] fn funding_tx_constructed( - &mut self, funding: &mut FundingScope, signing_session: InteractiveTxSigningSession, - is_splice: bool, holder_commitment_transaction_number: u64, logger: &L + &mut self, funding: &mut FundingScope, funding_outpoint: OutPoint, is_splice: bool, + holder_commitment_transaction_number: u64, logger: &L, ) -> Result where L::Target: Logger { - let mut output_index = None; - let expected_spk = funding.get_funding_redeemscript().to_p2wsh(); - for (idx, outp) in signing_session.unsigned_tx().tx().output.iter().enumerate() { - if outp.script_pubkey == expected_spk && outp.value.to_sat() == funding.get_value_satoshis() { - if output_index.is_some() { - return Err(AbortReason::DuplicateFundingOutput); - } - output_index = Some(idx as u16); - } - } - let outpoint = if let Some(output_index) = output_index { - OutPoint { txid: signing_session.unsigned_tx().compute_txid(), index: output_index } - } else { - return Err(AbortReason::MissingFundingOutput); - }; - funding - .channel_transaction_parameters.funding_outpoint = Some(outpoint); - self.interactive_tx_signing_session = Some(signing_session); + funding.channel_transaction_parameters.funding_outpoint = Some(funding_outpoint); if is_splice { debug_assert_eq!( @@ -6091,7 +6102,6 @@ where Some(commitment_signed) => commitment_signed, // TODO(splicing): Support async signing None => { - funding.channel_transaction_parameters.funding_outpoint = None; return Err(AbortReason::InternalError("Failed to compute commitment_signed signatures")); }, }; @@ -6167,8 +6177,6 @@ where SP::Target: SignerProvider, L::Target: Logger, { - debug_assert!(self.interactive_tx_signing_session.is_some()); - let signatures = self.get_initial_counterparty_commitment_signatures(funding, logger); if let Some((signature, htlc_signatures)) = signatures { log_info!( @@ -6508,9 +6516,9 @@ impl FundingNegotiationContext { /// Prepare and start interactive transaction negotiation. /// If error occurs, it is caused by our side, not the counterparty. fn into_interactive_tx_constructor( - self, context: &ChannelContext, funding: &FundingScope, signer_provider: &SP, + mut self, context: &ChannelContext, funding: &FundingScope, signer_provider: &SP, entropy_source: &ES, holder_node_id: PublicKey, - ) -> Result + ) -> Result where SP::Target: SignerProvider, ES::Target: EntropySource, @@ -6536,25 +6544,32 @@ impl FundingNegotiationContext { // Optionally add change output let change_value_opt = if self.our_funding_contribution > SignedAmount::ZERO { - calculate_change_output_value( + match calculate_change_output_value( &self, self.shared_funding_input.is_some(), &shared_funding_output.script_pubkey, context.holder_dust_limit_satoshis, - )? + ) { + Ok(change_value_opt) => change_value_opt, + Err(reason) => { + return Err(self.into_negotiation_error(reason)); + }, + } } else { None }; - let mut funding_outputs = self.our_funding_outputs; - if let Some(change_value) = change_value_opt { let change_script = if let Some(script) = self.change_script { script } else { - signer_provider - .get_destination_script(context.channel_keys_id) - .map_err(|_err| AbortReason::InternalError("Error getting change script"))? + match signer_provider.get_destination_script(context.channel_keys_id) { + Ok(script) => script, + Err(_) => { + let reason = AbortReason::InternalError("Error getting change script"); + return Err(self.into_negotiation_error(reason)); + }, + } }; let mut change_output = TxOut { value: Amount::from_sat(change_value), script_pubkey: change_script }; @@ -6565,7 +6580,7 @@ impl FundingNegotiationContext { // Check dust limit again if change_value_decreased_with_fee > context.holder_dust_limit_satoshis { change_output.value = Amount::from_sat(change_value_decreased_with_fee); - funding_outputs.push(change_output); + self.our_funding_outputs.push(change_output); } } @@ -6583,10 +6598,19 @@ impl FundingNegotiationContext { shared_funding_output, funding.value_to_self_msat / 1000, ), - outputs_to_contribute: funding_outputs, + outputs_to_contribute: self.our_funding_outputs, }; InteractiveTxConstructor::new(constructor_args) } + + fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { + let contributed_inputs = + self.our_funding_inputs.into_iter().map(|input| input.utxo.outpoint).collect(); + + let contributed_outputs = self.our_funding_outputs; + + NegotiationError { reason, contributed_inputs, contributed_outputs } + } } // Holder designates channel data owned for the benefit of the user client. @@ -13660,8 +13684,8 @@ where outputs_to_contribute: funding_negotiation_context.our_funding_outputs.clone(), } ).map_err(|err| { - let reason = ClosureReason::ProcessingError { err: err.to_string() }; - ChannelError::Close((err.to_string(), reason)) + let reason = ClosureReason::ProcessingError { err: err.reason.to_string() }; + ChannelError::Close((err.reason.to_string(), reason)) })?); let unfunded_context = UnfundedChannelContext { diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 3c683fcc5ee..d1cac891c11 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -7,6 +7,7 @@ // You may not use this file except in accordance with one or both of these // licenses. +use crate::chain::transaction::OutPoint; use crate::io_extras::sink; use crate::prelude::*; @@ -21,8 +22,8 @@ use bitcoin::secp256k1::{Message, PublicKey}; use bitcoin::sighash::SighashCache; use bitcoin::transaction::Version; use bitcoin::{ - sighash, EcdsaSighashType, OutPoint, ScriptBuf, Sequence, TapSighashType, Transaction, TxIn, - TxOut, Txid, Weight, Witness, XOnlyPublicKey, + sighash, EcdsaSighashType, OutPoint as BitcoinOutPoint, ScriptBuf, Sequence, TapSighashType, + Transaction, TxIn, TxOut, Txid, Weight, Witness, XOnlyPublicKey, }; use crate::chain::chaininterface::fee_for_weight; @@ -88,6 +89,13 @@ impl SerialIdExt for SerialId { } } +#[derive(Clone, Debug)] +pub(crate) struct NegotiationError { + pub reason: AbortReason, + pub contributed_inputs: Vec, + pub contributed_outputs: Vec, +} + #[derive(Debug, Clone, Copy, PartialEq)] pub(crate) enum AbortReason { InvalidStateTransition, @@ -199,7 +207,8 @@ pub(crate) struct ConstructedTransaction { input_metadata: Vec, output_metadata: Vec, tx: Transaction, - shared_input_index: Option, + shared_input_index: Option, + shared_output_index: u16, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -244,30 +253,22 @@ impl_writeable_tlv_based!(ConstructedTransaction, { (5, output_metadata, required), (7, tx, required), (9, shared_input_index, option), + (11, shared_output_index, required), }); impl ConstructedTransaction { fn new(context: NegotiationContext) -> Result { - if let Some(shared_funding_input) = &context.shared_funding_input { - if !context.inputs.iter().any(|(_, input)| { - input.txin().previous_output == shared_funding_input.input.previous_output - }) { - return Err(AbortReason::MissingFundingInput); - } - } - if !context - .outputs - .iter() - .any(|(_, output)| *output.tx_out() == context.shared_funding_output.tx_out) - { - return Err(AbortReason::MissingFundingOutput); - } + let remote_inputs_value = context.remote_inputs_value(); + let remote_outputs_value = context.remote_outputs_value(); + let remote_weight_contributed = context.remote_weight_contributed(); let satisfaction_weight = Weight::from_wu(context.inputs.iter().fold(0u64, |value, (_, input)| { value.saturating_add(input.satisfaction_weight().to_wu()) })); + let lock_time = context.tx_locktime; + let mut inputs: Vec<(TxIn, TxInMetadata)> = context.inputs.into_values().map(|input| input.into_txin_and_metadata()).collect(); let mut outputs: Vec<(TxOut, TxOutMetadata)> = @@ -275,35 +276,102 @@ impl ConstructedTransaction { inputs.sort_unstable_by_key(|(_, input)| input.serial_id); outputs.sort_unstable_by_key(|(_, output)| output.serial_id); + let (input, input_metadata): (Vec, Vec) = inputs.into_iter().unzip(); + let (output, output_metadata): (Vec, Vec) = + outputs.into_iter().unzip(); + let shared_input_index = context.shared_funding_input.as_ref().and_then(|shared_funding_input| { - inputs + input .iter() - .position(|(txin, _)| { + .position(|txin| { txin.previous_output == shared_funding_input.input.previous_output }) - .map(|position| position as u32) + .map(|position| position as u16) }); - let (input, input_metadata): (Vec, Vec) = inputs.into_iter().unzip(); - let (output, output_metadata): (Vec, Vec) = - outputs.into_iter().unzip(); + let shared_output_index = output + .iter() + .position(|txout| *txout == context.shared_funding_output.tx_out) + .map(|position| position as u16) + .unwrap_or(u16::MAX); + + let tx = ConstructedTransaction { + holder_is_initiator: context.holder_is_initiator, + input_metadata, + output_metadata, + tx: Transaction { version: Version::TWO, lock_time, input, output }, + shared_input_index, + shared_output_index, + }; - let tx = - Transaction { version: Version::TWO, lock_time: context.tx_locktime, input, output }; + // The receiving node: + // MUST fail the negotiation if: + // - the peer's total input satoshis is less than their outputs + if remote_inputs_value < remote_outputs_value { + return Err(AbortReason::OutputsValueExceedsInputsValue); + } - let tx_weight = tx.weight().checked_add(satisfaction_weight).unwrap_or(Weight::MAX); + // - the peer's paid feerate does not meet or exceed the agreed feerate (based on the minimum fee). + let remote_fees_contributed = remote_inputs_value.saturating_sub(remote_outputs_value); + let required_remote_contribution_fee = + fee_for_weight(context.feerate_sat_per_kw, remote_weight_contributed); + if remote_fees_contributed < required_remote_contribution_fee { + return Err(AbortReason::InsufficientFees); + } + + // - there are more than 252 inputs + // - there are more than 252 outputs + if tx.tx.input.len() > MAX_INPUTS_OUTPUTS_COUNT + || tx.tx.output.len() > MAX_INPUTS_OUTPUTS_COUNT + { + return Err(AbortReason::ExceededNumberOfInputsOrOutputs); + } + + if context.shared_funding_input.is_some() && tx.shared_input_index.is_none() { + return Err(AbortReason::MissingFundingInput); + } + + if tx.shared_output_index == u16::MAX { + return Err(AbortReason::MissingFundingOutput); + } + + let tx_weight = tx.tx.weight().checked_add(satisfaction_weight).unwrap_or(Weight::MAX); if tx_weight > Weight::from_wu(MAX_STANDARD_TX_WEIGHT as u64) { return Err(AbortReason::TransactionTooLarge); } - Ok(Self { - holder_is_initiator: context.holder_is_initiator, - input_metadata, - output_metadata, - tx, - shared_input_index, - }) + Ok(tx) + } + + fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { + let contributed_inputs = self + .tx + .input + .into_iter() + .zip(self.input_metadata.iter()) + .enumerate() + .filter(|(_, (_, input))| input.is_local(self.holder_is_initiator)) + .filter(|(index, _)| { + self.shared_input_index + .map(|shared_index| *index != shared_index as usize) + .unwrap_or(true) + }) + .map(|(_, (txin, _))| txin.previous_output) + .collect(); + + let contributed_outputs = self + .tx + .output + .into_iter() + .zip(self.output_metadata.iter()) + .enumerate() + .filter(|(_, (_, output))| output.is_local(self.holder_is_initiator)) + .filter(|(index, _)| *index != self.shared_output_index as usize) + .map(|(_, (txout, _))| txout) + .collect(); + + NegotiationError { reason, contributed_inputs, contributed_outputs } } pub fn tx(&self) -> &Transaction { @@ -318,6 +386,10 @@ impl ConstructedTransaction { self.tx().compute_txid() } + fn funding_outpoint(&self) -> OutPoint { + OutPoint { txid: self.compute_txid(), index: self.shared_output_index } + } + /// Returns the total input value from all local contributions, including the entire shared /// input value if applicable. fn local_contributed_input_value(&self) -> Amount { @@ -425,7 +497,7 @@ impl ConstructedTransaction { self.holder_is_initiator } - pub fn shared_input_index(&self) -> Option { + pub fn shared_input_index(&self) -> Option { self.shared_input_index } } @@ -776,6 +848,10 @@ impl InteractiveTxSigningSession { Ok(()) } + + pub(crate) fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { + self.unsigned_tx.into_negotiation_error(reason) + } } impl_writeable_tlv_based!(InteractiveTxSigningSession, { @@ -810,7 +886,7 @@ struct NegotiationContext { /// - For the acceptor: /// The output expected as new funding output. It should be added by the initiator node. shared_funding_output: SharedOwnedOutput, - prevtx_outpoints: HashSet, + prevtx_outpoints: HashSet, /// The outputs added so far. outputs: HashMap, /// The locktime of the funding transaction. @@ -892,6 +968,18 @@ impl NegotiationContext { ) } + fn remote_weight_contributed(&self) -> u64 { + self.remote_inputs_weight() + .to_wu() + .saturating_add(self.remote_outputs_weight().to_wu()) + // The receiving node: + // - MUST fail the negotiation if + // - if is the non-initiator: + // - the initiator's fees do not cover the common fields (version, segwit marker + flag, + // input count, output count, locktime) + .saturating_add(if !self.holder_is_initiator { TX_COMMON_FIELDS_WEIGHT } else { 0 }) + } + fn remote_outputs_weight(&self) -> Weight { Weight::from_wu( self.outputs @@ -948,7 +1036,7 @@ impl NegotiationContext { return Err(AbortReason::DuplicateFundingInput); } - let previous_output = OutPoint { txid: *shared_txid, vout: msg.prevtx_out }; + let previous_output = BitcoinOutPoint { txid: *shared_txid, vout: msg.prevtx_out }; if previous_output != shared_funding_input.input.previous_output { return Err(AbortReason::UnexpectedFundingInput); } @@ -968,7 +1056,7 @@ impl NegotiationContext { return Err(AbortReason::PrevTxOutInvalid); } - let prev_outpoint = OutPoint { txid, vout: msg.prevtx_out }; + let prev_outpoint = BitcoinOutPoint { txid, vout: msg.prevtx_out }; let txin = TxIn { previous_output: prev_outpoint, sequence: Sequence(msg.sequence), @@ -1137,7 +1225,7 @@ impl NegotiationContext { ) -> Result<(), AbortReason> { let vout = msg.prevtx_out as usize; let (prev_outpoint, input) = if let Some(shared_input_txid) = msg.shared_input_txid { - let prev_outpoint = OutPoint { txid: shared_input_txid, vout: msg.prevtx_out }; + let prev_outpoint = BitcoinOutPoint { txid: shared_input_txid, vout: msg.prevtx_out }; if let Some(shared_funding_input) = &self.shared_funding_input { (prev_outpoint, InputOwned::Shared(shared_funding_input.clone())) } else { @@ -1145,7 +1233,7 @@ impl NegotiationContext { } } else if let Some(prevtx) = &msg.prevtx { let prev_txid = prevtx.compute_txid(); - let prev_outpoint = OutPoint { txid: prev_txid, vout: msg.prevtx_out }; + let prev_outpoint = BitcoinOutPoint { txid: prev_txid, vout: msg.prevtx_out }; let prev_output = prevtx.output.get(vout).ok_or(AbortReason::PrevTxOutInvalid)?.clone(); let txin = TxIn { previous_output: prev_outpoint, @@ -1194,52 +1282,6 @@ impl NegotiationContext { self.outputs.remove(&msg.serial_id); Ok(()) } - - fn check_counterparty_fees( - &self, counterparty_fees_contributed: u64, - ) -> Result<(), AbortReason> { - let mut counterparty_weight_contributed = self - .remote_inputs_weight() - .to_wu() - .saturating_add(self.remote_outputs_weight().to_wu()); - 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) - counterparty_weight_contributed += TX_COMMON_FIELDS_WEIGHT; - } - let required_counterparty_contribution_fee = - fee_for_weight(self.feerate_sat_per_kw, counterparty_weight_contributed); - if counterparty_fees_contributed < required_counterparty_contribution_fee { - return Err(AbortReason::InsufficientFees); - } - Ok(()) - } - - fn validate_tx(self) -> Result { - // The receiving node: - // MUST fail the negotiation if: - - // - the peer's total input satoshis is less than their outputs - let remote_inputs_value = self.remote_inputs_value(); - let remote_outputs_value = self.remote_outputs_value(); - if remote_inputs_value < remote_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); - } - - // - the peer's paid feerate does not meet or exceed the agreed feerate (based on the minimum fee). - self.check_counterparty_fees(remote_inputs_value.saturating_sub(remote_outputs_value))?; - - ConstructedTransaction::new(self) - } } // The interactive transaction construction protocol allows two peers to collaboratively build a @@ -1378,7 +1420,7 @@ macro_rules! define_state_transitions { let holder_node_id = context.holder_node_id; let counterparty_node_id = context.counterparty_node_id; - let tx = context.validate_tx()?; + let tx = ConstructedTransaction::new(context)?; // Strict ordering prevents deadlocks during tx_signatures exchange let local_contributed_input_value = tx.local_contributed_input_value(); @@ -1614,6 +1656,13 @@ impl InputOwned { } } + fn into_tx_in(self) -> TxIn { + match self { + InputOwned::Single(single) => single.input, + InputOwned::Shared(shared) => shared.input, + } + } + pub fn value(&self) -> u64 { match self { InputOwned::Single(single) => single.prev_output.value.to_sat(), @@ -1840,6 +1889,8 @@ pub(super) struct InteractiveTxConstructor { channel_id: ChannelId, inputs_to_contribute: Vec<(SerialId, InputOwned)>, outputs_to_contribute: Vec<(SerialId, OutputOwned)>, + next_input_index: Option, + next_output_index: Option, } #[allow(clippy::enum_variant_names)] // Clippy doesn't like the repeated `Tx` prefix here @@ -1893,8 +1944,7 @@ where pub(super) enum HandleTxCompleteValue { SendTxMessage(InteractiveTxMessageSend), - SendTxComplete(InteractiveTxMessageSend, bool), - NegotiationComplete, + NegotiationComplete(Option, OutPoint), } pub(super) struct InteractiveTxConstructorArgs<'a, ES: Deref> @@ -1919,7 +1969,7 @@ impl InteractiveTxConstructor { /// /// If the holder is the initiator, they need to send the first message which is a `TxAddInput` /// message. - pub fn new(args: InteractiveTxConstructorArgs) -> Result + pub fn new(args: InteractiveTxConstructorArgs) -> Result where ES::Target: EntropySource, { @@ -1992,41 +2042,73 @@ impl InteractiveTxConstructor { // 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 next_input_index = (!inputs_to_contribute.is_empty()).then_some(0); + let next_output_index = (!outputs_to_contribute.is_empty()).then_some(0); + let mut constructor = Self { state_machine, initiator_first_message: None, channel_id, inputs_to_contribute, outputs_to_contribute, + next_input_index, + next_output_index, }; // We'll store the first message for the initiator. if is_initiator { - constructor.initiator_first_message = Some(constructor.maybe_send_message()?); + match constructor.maybe_send_message() { + Ok(message) => { + constructor.initiator_first_message = Some(message); + }, + Err(reason) => { + return Err(constructor.into_negotiation_error(reason)); + }, + } } Ok(constructor) } + fn into_negotiation_error(self, reason: AbortReason) -> NegotiationError { + NegotiationError { + reason, + contributed_inputs: self + .inputs_to_contribute + .into_iter() + .filter(|(_, input)| !input.is_shared()) + .map(|(_, input)| input.into_tx_in().previous_output) + .collect(), + contributed_outputs: self + .outputs_to_contribute + .into_iter() + .filter(|(_, output)| !output.is_shared()) + .map(|(_, output)| output.into_tx_out()) + .collect(), + } + } + pub fn take_initiator_first_message(&mut self) -> Option { self.initiator_first_message.take() } fn maybe_send_message(&mut self) -> Result { + let channel_id = self.channel_id; + // 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)) = self.inputs_to_contribute.pop() { + if let Some((serial_id, input)) = self.next_input_to_contribute() { let satisfaction_weight = input.satisfaction_weight(); let msg = match input { InputOwned::Single(single) => msgs::TxAddInput { - channel_id: self.channel_id, - serial_id, - prevtx: Some(single.prev_tx), + channel_id, + serial_id: *serial_id, + prevtx: Some(single.prev_tx.clone()), prevtx_out: single.input.previous_output.vout, sequence: single.input.sequence.to_consensus_u32(), shared_input_txid: None, }, InputOwned::Shared(shared) => msgs::TxAddInput { - channel_id: self.channel_id, - serial_id, + channel_id, + serial_id: *serial_id, prevtx: None, prevtx_out: shared.input.previous_output.vout, sequence: shared.input.sequence.to_consensus_u32(), @@ -2035,22 +2117,44 @@ impl InteractiveTxConstructor { }; do_state_transition!(self, sent_tx_add_input, (&msg, satisfaction_weight))?; Ok(InteractiveTxMessageSend::TxAddInput(msg)) - } else if let Some((serial_id, output)) = self.outputs_to_contribute.pop() { + } else if let Some((serial_id, output)) = self.next_output_to_contribute() { let msg = msgs::TxAddOutput { - channel_id: self.channel_id, - serial_id, + channel_id, + serial_id: *serial_id, sats: output.tx_out().value.to_sat(), script: output.tx_out().script_pubkey.clone(), }; do_state_transition!(self, sent_tx_add_output, &msg)?; Ok(InteractiveTxMessageSend::TxAddOutput(msg)) } else { - let msg = msgs::TxComplete { channel_id: self.channel_id }; + let msg = msgs::TxComplete { channel_id }; do_state_transition!(self, sent_tx_complete, &msg)?; Ok(InteractiveTxMessageSend::TxComplete(msg)) } } + fn next_input_to_contribute(&mut self) -> Option<&(SerialId, InputOwned)> { + match self.next_input_index { + Some(index) => { + self.next_input_index = + index.checked_add(1).filter(|index| *index < self.inputs_to_contribute.len()); + self.inputs_to_contribute.get(index) + }, + None => None, + } + } + + fn next_output_to_contribute(&mut self) -> Option<&(SerialId, OutputOwned)> { + match self.next_output_index { + Some(index) => { + self.next_output_index = + index.checked_add(1).filter(|index| *index < self.outputs_to_contribute.len()); + self.outputs_to_contribute.get(index) + }, + None => None, + } + } + pub fn handle_tx_add_input( &mut self, msg: &msgs::TxAddInput, ) -> Result { @@ -2087,8 +2191,13 @@ impl InteractiveTxConstructor { StateMachine::ReceivedTxComplete(_) => { let msg_send = self.maybe_send_message()?; match &self.state_machine { - StateMachine::NegotiationComplete(_) => { - Ok(HandleTxCompleteValue::SendTxComplete(msg_send, true)) + StateMachine::NegotiationComplete(NegotiationComplete(signing_session)) => { + let funding_outpoint = signing_session.unsigned_tx.funding_outpoint(); + debug_assert!(matches!(msg_send, InteractiveTxMessageSend::TxComplete(_))); + Ok(HandleTxCompleteValue::NegotiationComplete( + Some(msg_send), + funding_outpoint, + )) }, StateMachine::SentChangeMsg(_) => { Ok(HandleTxCompleteValue::SendTxMessage(msg_send)) @@ -2099,7 +2208,10 @@ impl InteractiveTxConstructor { }, } }, - StateMachine::NegotiationComplete(_) => Ok(HandleTxCompleteValue::NegotiationComplete), + StateMachine::NegotiationComplete(NegotiationComplete(signing_session)) => { + let funding_outpoint = signing_session.unsigned_tx.funding_outpoint(); + Ok(HandleTxCompleteValue::NegotiationComplete(None, funding_outpoint)) + }, _ => { debug_assert!( false, @@ -2340,9 +2452,9 @@ mod tests { outputs_to_contribute: session.outputs_a, }) { Ok(r) => Some(r), - Err(abort_reason) => { + Err(e) => { assert_eq!( - Some((abort_reason, ErrorCulprit::NodeA)), + Some((e.reason, ErrorCulprit::NodeA)), session.expect_error, "Test: {}", session.description @@ -2379,9 +2491,9 @@ mod tests { outputs_to_contribute: session.outputs_b, }) { Ok(r) => Some(r), - Err(abort_reason) => { + Err(e) => { assert_eq!( - Some((abort_reason, ErrorCulprit::NodeB)), + Some((e.reason, ErrorCulprit::NodeB)), session.expect_error, "Test: {}", session.description @@ -2404,11 +2516,9 @@ mod tests { HandleTxCompleteValue::SendTxMessage(msg_send) => { (Some(msg_send), false) }, - HandleTxCompleteValue::SendTxComplete( - msg_send, - negotiation_complete, - ) => (Some(msg_send), negotiation_complete), - HandleTxCompleteValue::NegotiationComplete => (None, true), + HandleTxCompleteValue::NegotiationComplete(msg_send, _) => { + (msg_send, true) + }, }) }, } @@ -3311,6 +3421,7 @@ mod tests { output_metadata: vec![], // N/A for test tx: transaction.clone(), shared_input_index: None, + shared_output_index: 0, }; let secp_ctx = Secp256k1::new();