From 62c8ecddda2a04471313a1ba5dace33e7cf79f21 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sat, 21 Jun 2025 14:43:18 +0200 Subject: [PATCH 1/3] Refactor shared output support (interactivetxs) This simplifies tracking separately the expected and actual shared output. In the initiator case, we can just provide the shared output separately, instead of including it within other outputs, and marking which one is the output. We can use the same field for the intended shared output in the initiator case, and the expected one in the acceptor case. --- lightning/src/ln/channel.rs | 26 +- lightning/src/ln/interactivetxs.rs | 705 ++++++++++++----------------- 2 files changed, 290 insertions(+), 441 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 60e263f245a..9677e41ac09 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -57,7 +57,7 @@ use crate::ln::channelmanager::{ use crate::ln::interactivetxs::{ calculate_change_output_value, get_output_weight, AbortReason, HandleTxCompleteResult, InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend, - InteractiveTxMessageSendResult, InteractiveTxSigningSession, OutputOwned, SharedOwnedOutput, + InteractiveTxMessageSendResult, InteractiveTxSigningSession, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT, }; use crate::ln::msgs; @@ -2756,24 +2756,12 @@ where // Note: For the error case when the inputs are insufficient, it will be handled after // the `calculate_change_output_value` call below let mut funding_outputs = Vec::new(); - let mut expected_remote_shared_funding_output = None; let shared_funding_output = TxOut { value: Amount::from_sat(self.funding.get_value_satoshis()), script_pubkey: self.funding.get_funding_redeemscript().to_p2wsh(), }; - if self.funding.is_outbound() { - funding_outputs.push( - OutputOwned::Shared(SharedOwnedOutput::new( - shared_funding_output, self.dual_funding_context.our_funding_satoshis, - )) - ); - } else { - let TxOut { value, script_pubkey } = shared_funding_output; - expected_remote_shared_funding_output = Some((script_pubkey, value.to_sat())); - } - // Optionally add change output let change_script = if let Some(script) = change_destination_opt { script @@ -2783,7 +2771,7 @@ where }; let change_value_opt = calculate_change_output_value( self.funding.is_outbound(), self.dual_funding_context.our_funding_satoshis, - &funding_inputs, &funding_outputs, + &shared_funding_output.script_pubkey, &funding_inputs, &funding_outputs, self.dual_funding_context.funding_feerate_sat_per_1000_weight, change_script.minimal_non_dust().to_sat(), )?; @@ -2798,7 +2786,7 @@ where // Check dust limit again if change_value_decreased_with_fee > self.context.holder_dust_limit_satoshis { change_output.value = Amount::from_sat(change_value_decreased_with_fee); - funding_outputs.push(OutputOwned::Single(change_output)); + funding_outputs.push(change_output); } } @@ -2811,8 +2799,8 @@ where is_initiator: self.funding.is_outbound(), funding_tx_locktime: self.dual_funding_context.funding_tx_locktime, inputs_to_contribute: funding_inputs, + shared_funding_output: SharedOwnedOutput::new(shared_funding_output, self.dual_funding_context.our_funding_satoshis), outputs_to_contribute: funding_outputs, - expected_remote_shared_funding_output, }; let mut tx_constructor = InteractiveTxConstructor::new(constructor_args)?; let msg = tx_constructor.take_initiator_first_message(); @@ -11809,6 +11797,10 @@ where funding_feerate_sat_per_1000_weight: msg.funding_feerate_sat_per_1000_weight, our_funding_inputs: our_funding_inputs.clone(), }; + let shared_funding_output = TxOut { + value: Amount::from_sat(funding.get_value_satoshis()), + script_pubkey: funding.get_funding_redeemscript().to_p2wsh(), + }; let interactive_tx_constructor = Some(InteractiveTxConstructor::new( InteractiveTxConstructorArgs { @@ -11820,8 +11812,8 @@ where funding_tx_locktime: dual_funding_context.funding_tx_locktime, is_initiator: false, inputs_to_contribute: our_funding_inputs, + shared_funding_output: SharedOwnedOutput::new(shared_funding_output, our_funding_satoshis), outputs_to_contribute: Vec::new(), - expected_remote_shared_funding_output: Some((funding.get_funding_redeemscript().to_p2wsh(), funding.get_value_satoshis())), } ).map_err(|_| ChannelError::Close(( "V2 channel rejected due to sender error".into(), diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index ee991a0ae8c..4c2544d8f41 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -466,19 +466,13 @@ struct NegotiationContext { received_tx_add_input_count: u16, received_tx_add_output_count: u16, inputs: HashMap, - /// The output script intended to be the new funding output script. - /// The script pubkey is used to determine which output is the funding output. - /// When an output with the same script pubkey is added by any of the nodes, it will be - /// treated as the shared output. - /// The value is the holder's intended contribution to the shared funding output. - /// The rest is the counterparty's contribution. - /// When the funding output is added (recognized by its output script pubkey), it will be marked - /// as shared, and split between the peers according to the local value. - /// If the local value is found to be larger than the actual funding output, an error is generated. - expected_shared_funding_output: (ScriptBuf, u64), - /// The actual new funding output, set only after the output has actually been added. - /// NOTE: this output is also included in `outputs`. - actual_new_funding_output: Option, + /// The intended/expected funding output, potentially co-owned by both peers (shared). + /// - For the initiator: + /// The output intended to be the new funding output. This will be added alongside + /// the provided outputs. + /// - 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, /// The outputs added so far. outputs: HashMap, @@ -515,7 +509,7 @@ fn is_serial_id_valid_for_counterparty(holder_is_initiator: bool, serial_id: Ser impl NegotiationContext { fn new( holder_node_id: PublicKey, counterparty_node_id: PublicKey, holder_is_initiator: bool, - expected_shared_funding_output: (ScriptBuf, u64), tx_locktime: AbsoluteLockTime, + shared_funding_output: SharedOwnedOutput, tx_locktime: AbsoluteLockTime, feerate_sat_per_kw: u32, ) -> Self { NegotiationContext { @@ -525,8 +519,7 @@ impl NegotiationContext { received_tx_add_input_count: 0, received_tx_add_output_count: 0, inputs: new_hash_map(), - expected_shared_funding_output, - actual_new_funding_output: None, + shared_funding_output, prevtx_outpoints: new_hash_set(), outputs: new_hash_map(), tx_locktime, @@ -534,23 +527,6 @@ impl NegotiationContext { } } - fn set_actual_new_funding_output( - &mut self, tx_out: TxOut, - ) -> Result { - if self.actual_new_funding_output.is_some() { - return Err(AbortReason::DuplicateFundingOutput); - } - let value = tx_out.value.to_sat(); - let local_owned = self.expected_shared_funding_output.1; - // Sanity check - if local_owned > value { - return Err(AbortReason::InvalidLowFundingOutputValue); - } - let shared_output = SharedOwnedOutput::new(tx_out, local_owned); - self.actual_new_funding_output = Some(shared_output.clone()); - Ok(shared_output) - } - fn is_serial_id_valid_for_counterparty(&self, serial_id: &SerialId) -> bool { is_serial_id_valid_for_counterparty(self.holder_is_initiator, *serial_id) } @@ -745,22 +721,19 @@ impl NegotiationContext { } let txout = TxOut { value: Amount::from_sat(msg.sats), script_pubkey: msg.script.clone() }; - let is_shared = msg.script == self.expected_shared_funding_output.0; - let output = if is_shared { - // this is a shared funding output - let shared_output = self.set_actual_new_funding_output(txout)?; - InteractiveTxOutput { - serial_id: msg.serial_id, - added_by: AddingRole::Remote, - output: OutputOwned::Shared(shared_output), + let output = if txout == self.shared_funding_output.tx_out { + if self.holder_is_initiator { + return Err(AbortReason::DuplicateFundingOutput); } - } else { - InteractiveTxOutput { - serial_id: msg.serial_id, - added_by: AddingRole::Remote, - output: OutputOwned::Single(txout), + if self.outputs.values().any(|output| matches!(output.output, OutputOwned::Shared(_))) { + return Err(AbortReason::DuplicateFundingOutput); } + OutputOwned::Shared(self.shared_funding_output.clone()) + } else { + OutputOwned::Single(txout) }; + let output = + InteractiveTxOutput { serial_id: msg.serial_id, added_by: AddingRole::Remote, output }; match self.outputs.entry(msg.serial_id) { hash_map::Entry::Occupied(_) => { // The receiving node: @@ -814,22 +787,13 @@ impl NegotiationContext { fn sent_tx_add_output(&mut self, msg: &msgs::TxAddOutput) -> Result<(), AbortReason> { let txout = TxOut { value: Amount::from_sat(msg.sats), script_pubkey: msg.script.clone() }; - let is_shared = msg.script == self.expected_shared_funding_output.0; - let output = if is_shared { - // this is a shared funding output - let shared_output = self.set_actual_new_funding_output(txout)?; - InteractiveTxOutput { - serial_id: msg.serial_id, - added_by: AddingRole::Local, - output: OutputOwned::Shared(shared_output), - } + let output = if txout == self.shared_funding_output.tx_out { + OutputOwned::Shared(self.shared_funding_output.clone()) } else { - InteractiveTxOutput { - serial_id: msg.serial_id, - added_by: AddingRole::Local, - output: OutputOwned::Single(txout), - } + OutputOwned::Single(txout) }; + let output = + InteractiveTxOutput { serial_id: msg.serial_id, added_by: AddingRole::Local, output }; self.outputs.insert(msg.serial_id, output); Ok(()) } @@ -886,15 +850,18 @@ impl NegotiationContext { return Err(AbortReason::ExceededNumberOfInputsOrOutputs); } - if self.actual_new_funding_output.is_none() { - return Err(AbortReason::MissingFundingOutput); - } - // - 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))?; + let shared_funding_output = self.shared_funding_output.clone(); let constructed_tx = ConstructedTransaction::new(self); - + if !constructed_tx + .outputs + .iter() + .any(|output| *output.tx_out() == shared_funding_output.tx_out) + { + return Err(AbortReason::MissingFundingOutput); + } if constructed_tx.weight().to_wu() > MAX_STANDARD_TX_WEIGHT as u64 { return Err(AbortReason::TransactionTooLarge); } @@ -1107,13 +1074,13 @@ impl StateMachine { fn new( holder_node_id: PublicKey, counterparty_node_id: PublicKey, feerate_sat_per_kw: u32, is_initiator: bool, tx_locktime: AbsoluteLockTime, - expected_shared_funding_output: (ScriptBuf, u64), + shared_funding_output: SharedOwnedOutput, ) -> Self { let context = NegotiationContext::new( holder_node_id, counterparty_node_id, is_initiator, - expected_shared_funding_output, + shared_funding_output, tx_locktime, feerate_sat_per_kw, ); @@ -1224,14 +1191,14 @@ impl_writeable_tlv_based!(SharedOwnedOutput, { }); impl SharedOwnedOutput { - pub fn new(tx_out: TxOut, local_owned: u64) -> SharedOwnedOutput { + pub fn new(tx_out: TxOut, local_owned: u64) -> Self { debug_assert!( local_owned <= tx_out.value.to_sat(), "SharedOwnedOutput: Inconsistent local_owned value {}, larger than output value {}", local_owned, - tx_out.value + tx_out.value.to_sat(), ); - SharedOwnedOutput { tx_out, local_owned } + Self { tx_out, local_owned } } fn remote_owned(&self) -> u64 { @@ -1239,11 +1206,9 @@ impl SharedOwnedOutput { } } -/// Represents an output, with information about -/// its control -- exclusive by the adder or shared --, and -/// its ownership -- value fully owned by the adder or jointly +/// A transaction output, differentiated by ownership: exclusive by the adder or shared. #[derive(Clone, Debug, Eq, PartialEq)] -pub(super) enum OutputOwned { +enum OutputOwned { /// Belongs to a single party -- controlled exclusively and fully belonging to a single party Single(TxOut), /// Output with shared control and value split between the two ends (or fully at one side) @@ -1527,21 +1492,13 @@ where pub is_initiator: bool, pub funding_tx_locktime: AbsoluteLockTime, pub inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited)>, - pub outputs_to_contribute: Vec, - pub expected_remote_shared_funding_output: Option<(ScriptBuf, u64)>, + pub shared_funding_output: SharedOwnedOutput, + pub outputs_to_contribute: Vec, } impl InteractiveTxConstructor { /// Instantiates a new `InteractiveTxConstructor`. /// - /// `expected_remote_shared_funding_output`: In the case when the local node doesn't - /// add a shared output, but it expects a shared output to be added by the remote node, - /// it has to specify the script pubkey, used to determine the shared output, - /// and its (local) contribution from the shared output: - /// 0 when the whole value belongs to the remote node, or - /// positive if owned also by local. - /// Note: The local value cannot be larger than the actual shared output. - /// /// If the holder is the initiator, they need to send the first message which is a `TxAddInput` /// message. pub fn new(args: InteractiveTxConstructorArgs) -> Result @@ -1557,81 +1514,61 @@ impl InteractiveTxConstructor { is_initiator, funding_tx_locktime, inputs_to_contribute, + shared_funding_output, outputs_to_contribute, - expected_remote_shared_funding_output, } = args; - // Sanity check: There can be at most one shared output, local-added or remote-added - let mut expected_shared_funding_output: Option<(ScriptBuf, u64)> = None; - for output in &outputs_to_contribute { - let new_output = match output { - OutputOwned::Single(_tx_out) => None, - OutputOwned::Shared(output) => { - // Sanity check - if output.local_owned > output.tx_out.value.to_sat() { - return Err(AbortReason::InvalidLowFundingOutputValue); - } - Some((output.tx_out.script_pubkey.clone(), output.local_owned)) - }, - }; - if new_output.is_some() { - if expected_shared_funding_output.is_some() - || expected_remote_shared_funding_output.is_some() - { - // more than one local-added shared output or - // one local-added and one remote-expected shared output - return Err(AbortReason::DuplicateFundingOutput); - } - expected_shared_funding_output = new_output; - } - } - if let Some(expected_remote_shared_funding_output) = expected_remote_shared_funding_output { - expected_shared_funding_output = Some(expected_remote_shared_funding_output); - } - if let Some(expected_shared_funding_output) = expected_shared_funding_output { - let state_machine = StateMachine::new( - holder_node_id, - counterparty_node_id, - feerate_sat_per_kw, - is_initiator, - funding_tx_locktime, - expected_shared_funding_output, - ); - 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<_> = outputs_to_contribute + + let state_machine = StateMachine::new( + holder_node_id, + counterparty_node_id, + feerate_sat_per_kw, + is_initiator, + funding_tx_locktime, + shared_funding_output.clone(), + ); + + let mut inputs_to_contribute: Vec<(SerialId, TxIn, TransactionU16LenLimited)> = + inputs_to_contribute .into_iter() - .map(|output| { + .map(|(input, tx)| { let serial_id = generate_holder_serial_id(entropy_source, is_initiator); - (serial_id, output) + (serial_id, input, tx) }) .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, - initiator_first_message: None, - channel_id, - inputs_to_contribute, - outputs_to_contribute, - }; - // We'll store the first message for the initiator. - if is_initiator { - constructor.initiator_first_message = Some(constructor.maybe_send_message()?); - } - Ok(constructor) - } else { - Err(AbortReason::MissingFundingOutput) + // 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<_> = outputs_to_contribute + .into_iter() + .map(|output| { + let serial_id = generate_holder_serial_id(entropy_source, is_initiator); + let output = OutputOwned::Single(output); + (serial_id, output) + }) + .collect(); + if is_initiator { + // Add shared funding output + let serial_id = generate_holder_serial_id(entropy_source, is_initiator); + let output = OutputOwned::Shared(shared_funding_output); + outputs_to_contribute.push((serial_id, output)); + } + // 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, + initiator_first_message: None, + channel_id, + inputs_to_contribute, + outputs_to_contribute, + }; + // We'll store the first message for the initiator. + if is_initiator { + constructor.initiator_first_message = Some(constructor.maybe_send_message()?); } + Ok(constructor) } pub fn take_initiator_first_message(&mut self) -> Option { @@ -1740,10 +1677,9 @@ impl InteractiveTxConstructor { /// `Ok(None)` /// - Inputs are not sufficent to cover contribution and fees: /// `Err(AbortReason::InsufficientFees)` -#[allow(dead_code)] // TODO(dual_funding): Remove once begin_interactive_funding_tx_construction() is used pub(super) fn calculate_change_output_value( - is_initiator: bool, our_contribution: u64, - funding_inputs: &Vec<(TxIn, TransactionU16LenLimited)>, funding_outputs: &Vec, + is_initiator: bool, our_contribution: u64, shared_output_funding_script: &ScriptBuf, + funding_inputs: &Vec<(TxIn, TransactionU16LenLimited)>, funding_outputs: &Vec, funding_feerate_sat_per_1000_weight: u32, change_output_dust_limit: u64, ) -> Result, AbortReason> { // Process inputs and their prev txs: @@ -1764,25 +1700,30 @@ pub(super) fn calculate_change_output_value( } } + let total_output_satoshis = + funding_outputs.iter().fold(0u64, |total, out| total.saturating_add(out.value.to_sat())); + let our_funding_outputs_weight = funding_outputs.iter().fold(0u64, |weight, out| { - weight.saturating_add(get_output_weight(&out.tx_out().script_pubkey).to_wu()) + weight.saturating_add(get_output_weight(&out.script_pubkey).to_wu()) }); let mut weight = our_funding_outputs_weight.saturating_add(our_funding_inputs_weight); - // If we are the initiator, we must pay for weight of all common fields in the funding transaction. + // If we are the initiator, we must pay for the weight of the funding output and + // all common fields in the funding transaction. if is_initiator { + weight = weight.saturating_add(get_output_weight(shared_output_funding_script).to_wu()); weight = weight.saturating_add(TX_COMMON_FIELDS_WEIGHT); } let fees_sats = fee_for_weight(funding_feerate_sat_per_1000_weight, weight); - // Note: in case of additional outputs, they will have to be subtracted here - let total_inputs_less_fees = total_input_satoshis.saturating_sub(fees_sats); - if total_inputs_less_fees < our_contribution { + let net_total_less_fees = + total_input_satoshis.saturating_sub(total_output_satoshis).saturating_sub(fees_sats); + if net_total_less_fees < our_contribution { // Not enough to cover contribution plus fees return Err(AbortReason::InsufficientFees); } - let remaining_value = total_inputs_less_fees.saturating_sub(our_contribution); + let remaining_value = net_total_less_fees.saturating_sub(our_contribution); if remaining_value < change_output_dust_limit { // Enough to cover contribution plus fees, but leftover is below dust limit; no change Ok(None) @@ -1799,8 +1740,8 @@ mod tests { use crate::ln::interactivetxs::{ calculate_change_output_value, generate_holder_serial_id, AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, - InteractiveTxMessageSend, MAX_INPUTS_OUTPUTS_COUNT, MAX_RECEIVED_TX_ADD_INPUT_COUNT, - MAX_RECEIVED_TX_ADD_OUTPUT_COUNT, + InteractiveTxMessageSend, SharedOwnedOutput, MAX_INPUTS_OUTPUTS_COUNT, + MAX_RECEIVED_TX_ADD_INPUT_COUNT, MAX_RECEIVED_TX_ADD_OUTPUT_COUNT, }; use crate::ln::types::ChannelId; use crate::sign::EntropySource; @@ -1820,8 +1761,7 @@ mod tests { use core::ops::Deref; use super::{ - get_output_weight, AddingRole, OutputOwned, SharedOwnedOutput, - P2TR_INPUT_WEIGHT_LOWER_BOUND, P2WPKH_INPUT_WEIGHT_LOWER_BOUND, + get_output_weight, P2TR_INPUT_WEIGHT_LOWER_BOUND, P2WPKH_INPUT_WEIGHT_LOWER_BOUND, P2WSH_INPUT_WEIGHT_LOWER_BOUND, TX_COMMON_FIELDS_WEIGHT, }; @@ -1870,14 +1810,14 @@ mod tests { struct TestSession { description: &'static str, inputs_a: Vec<(TxIn, TransactionU16LenLimited)>, - outputs_a: Vec, + /// The funding output, with the value contributed + shared_output_a: (TxOut, u64), + outputs_a: Vec, inputs_b: Vec<(TxIn, TransactionU16LenLimited)>, - outputs_b: Vec, + /// The funding output, with the value contributed + shared_output_b: (TxOut, u64), + outputs_b: Vec, expect_error: Option<(AbortReason, ErrorCulprit)>, - /// A node adds no shared output, but expects the peer to add one, with the specific script pubkey, and local contribution - a_expected_remote_shared_output: Option<(ScriptBuf, u64)>, - /// B node adds no shared output, but expects the peer to add one, with the specific script pubkey, and local contribution - b_expected_remote_shared_output: Option<(ScriptBuf, u64)>, } fn do_test_interactive_tx_constructor(session: TestSession) { @@ -1909,57 +1849,6 @@ mod tests { &SecretKey::from_slice(&[43; 32]).unwrap(), ); - // funding output sanity check - let shared_outputs_by_a: Vec<_> = - session.outputs_a.iter().filter(|o| o.is_shared()).collect(); - if shared_outputs_by_a.len() > 1 { - println!("Test warning: Expected at most one shared output. NodeA"); - } - let shared_output_by_a = if !shared_outputs_by_a.is_empty() { - Some(shared_outputs_by_a[0].value()) - } else { - None - }; - let shared_outputs_by_b: Vec<_> = - session.outputs_b.iter().filter(|o| o.is_shared()).collect(); - if shared_outputs_by_b.len() > 1 { - println!("Test warning: Expected at most one shared output. NodeB"); - } - let shared_output_by_b = if !shared_outputs_by_b.is_empty() { - Some(shared_outputs_by_b[0].value()) - } else { - None - }; - if session.a_expected_remote_shared_output.is_some() - || session.b_expected_remote_shared_output.is_some() - { - let expected_by_a = if let Some(a_expected_remote_shared_output) = - &session.a_expected_remote_shared_output - { - a_expected_remote_shared_output.1 - } else if !shared_outputs_by_a.is_empty() { - shared_outputs_by_a[0].local_value(AddingRole::Local) - } else { - 0 - }; - let expected_by_b = if let Some(b_expected_remote_shared_output) = - &session.b_expected_remote_shared_output - { - b_expected_remote_shared_output.1 - } else if !shared_outputs_by_b.is_empty() { - shared_outputs_by_b[0].local_value(AddingRole::Local) - } else { - 0 - }; - - let expected_sum = expected_by_a + expected_by_b; - let actual_shared_output = - shared_output_by_a.unwrap_or(shared_output_by_b.unwrap_or(0)); - if expected_sum != actual_shared_output { - println!("Test warning: Sum of expected shared output values does not match actual shared output value, {} {} {} {} {} {}", expected_sum, actual_shared_output, expected_by_a, expected_by_b, shared_output_by_a.unwrap_or(0), shared_output_by_b.unwrap_or(0)); - } - } - let mut constructor_a = match InteractiveTxConstructor::new(InteractiveTxConstructorArgs { entropy_source, channel_id, @@ -1969,8 +1858,11 @@ mod tests { is_initiator: true, funding_tx_locktime, inputs_to_contribute: session.inputs_a, - outputs_to_contribute: session.outputs_a.to_vec(), - expected_remote_shared_funding_output: session.a_expected_remote_shared_output, + shared_funding_output: SharedOwnedOutput::new( + session.shared_output_a.0, + session.shared_output_a.1, + ), + outputs_to_contribute: session.outputs_a, }) { Ok(r) => r, Err(abort_reason) => { @@ -1992,8 +1884,11 @@ mod tests { is_initiator: false, funding_tx_locktime, inputs_to_contribute: session.inputs_b, - outputs_to_contribute: session.outputs_b.to_vec(), - expected_remote_shared_funding_output: session.b_expected_remote_shared_output, + shared_funding_output: SharedOwnedOutput::new( + session.shared_output_b.0, + session.shared_output_b.1, + ), + outputs_to_contribute: session.outputs_b, }) { Ok(r) => r, Err(abort_reason) => { @@ -2174,37 +2069,20 @@ mod tests { Builder::new().push_int(33).into_script().to_p2wsh() } - fn generate_output_nonfunding_one(output: &TestOutput) -> OutputOwned { - OutputOwned::Single(generate_txout(output)) + fn generate_output_nonfunding_one(output: &TestOutput) -> TxOut { + generate_txout(output) } - fn generate_outputs(outputs: &[TestOutput]) -> Vec { + fn generate_outputs(outputs: &[TestOutput]) -> Vec { outputs.iter().map(generate_output_nonfunding_one).collect() } - /// Generate a single output that is the funding output - fn generate_output(output: &TestOutput) -> Vec { - let txout = generate_txout(output); - let value = txout.value.to_sat(); - vec![OutputOwned::Shared(SharedOwnedOutput::new(txout, value))] - } - - /// Generate a single P2WSH output that is the funding output - fn generate_funding_output(value: u64) -> Vec { - generate_output(&TestOutput::P2WSH(value)) - } - - /// Generate a single P2WSH output with shared contribution that is the funding output - fn generate_shared_funding_output_one(value: u64, local_value: u64) -> OutputOwned { - OutputOwned::Shared(SharedOwnedOutput { - tx_out: generate_txout(&TestOutput::P2WSH(value)), - local_owned: local_value, - }) - } - - /// Generate a single P2WSH output with shared contribution that is the funding output - fn generate_shared_funding_output(value: u64, local_value: u64) -> Vec { - vec![generate_shared_funding_output_one(value, local_value)] + /// Generate a single P2WSH output that is the funding output, with local contributions + fn generate_funding_txout(value: u64, local_value: u64) -> (TxOut, u64) { + if local_value > value { + println!("Warning: Invalid local value, {} {}", value, local_value); + } + (generate_txout(&TestOutput::P2WSH(value)), local_value) } fn generate_fixed_number_of_inputs(count: u16) -> Vec<(TxIn, TransactionU16LenLimited)> { @@ -2246,7 +2124,7 @@ mod tests { inputs } - fn generate_fixed_number_of_outputs(count: u16) -> Vec { + fn generate_fixed_number_of_outputs(count: u16) -> Vec { // Set a constant value for each TxOut generate_outputs(&vec![TestOutput::P2WPKH(1_000_000); count as usize]) } @@ -2255,111 +2133,99 @@ mod tests { Builder::new().push_opcode(opcodes::OP_TRUE).into_script().to_p2sh() } - fn generate_non_witness_output(value: u64) -> OutputOwned { - OutputOwned::Single(TxOut { - value: Amount::from_sat(value), - script_pubkey: generate_p2sh_script_pubkey(), - }) + fn generate_non_witness_output(value: u64) -> TxOut { + TxOut { value: Amount::from_sat(value), script_pubkey: generate_p2sh_script_pubkey() } } #[test] fn test_interactive_tx_constructor() { - do_test_interactive_tx_constructor(TestSession { - description: "No contributions", - inputs_a: vec![], - outputs_a: vec![], - inputs_b: vec![], - outputs_b: vec![], - expect_error: Some((AbortReason::MissingFundingOutput, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: None, - }); do_test_interactive_tx_constructor(TestSession { description: "Single contribution, no initiator inputs", inputs_a: vec![], - outputs_a: generate_output(&TestOutput::P2WPKH(1_000_000)), - inputs_b: vec![], - outputs_b: vec![], - expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), - }); - do_test_interactive_tx_constructor(TestSession { - description: "Single contribution, no initiator outputs", - inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], - expect_error: Some((AbortReason::MissingFundingOutput, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: None, + expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)), }); + do_test_interactive_tx_constructor(TestSession { description: "Single contribution, no fees", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), - outputs_a: generate_output(&TestOutput::P2WPKH(1_000_000)), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); let p2wpkh_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, P2WPKH_INPUT_WEIGHT_LOWER_BOUND); let outputs_fee = fee_for_weight( TEST_FEERATE_SATS_PER_KW, - get_output_weight(&generate_p2wpkh_script_pubkey()).to_wu(), + get_output_weight(&generate_p2wsh_script_pubkey()).to_wu(), ); let tx_common_fields_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, TX_COMMON_FIELDS_WEIGHT); let amount_adjusted_with_p2wpkh_fee = - 1_000_000 - p2wpkh_fee - outputs_fee - tx_common_fields_fee; + 1_000_000 - p2wpkh_fee - outputs_fee - tx_common_fields_fee + 1; do_test_interactive_tx_constructor(TestSession { description: "Single contribution, with P2WPKH input, insufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), - outputs_a: generate_output(&TestOutput::P2WPKH( - amount_adjusted_with_p2wpkh_fee + 1, /* makes fees insuffcient for initiator */ - )), + // makes initiator inputs insufficient to cover fees + shared_output_a: generate_funding_txout( + amount_adjusted_with_p2wpkh_fee + 1, + amount_adjusted_with_p2wpkh_fee + 1, + ), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(amount_adjusted_with_p2wpkh_fee + 1, 0), outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); do_test_interactive_tx_constructor(TestSession { description: "Single contribution with P2WPKH input, sufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), - outputs_a: generate_output(&TestOutput::P2WPKH(amount_adjusted_with_p2wpkh_fee)), + shared_output_a: generate_funding_txout( + amount_adjusted_with_p2wpkh_fee, + amount_adjusted_with_p2wpkh_fee, + ), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(amount_adjusted_with_p2wpkh_fee, 0), outputs_b: vec![], expect_error: None, - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); let p2wsh_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, P2WSH_INPUT_WEIGHT_LOWER_BOUND); let amount_adjusted_with_p2wsh_fee = - 1_000_000 - p2wsh_fee - outputs_fee - tx_common_fields_fee; + 1_000_000 - p2wsh_fee - outputs_fee - tx_common_fields_fee + 1; do_test_interactive_tx_constructor(TestSession { description: "Single contribution, with P2WSH input, insufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WSH(1_000_000)]), - outputs_a: generate_output(&TestOutput::P2WPKH( - amount_adjusted_with_p2wsh_fee + 1, /* makes fees insuffcient for initiator */ - )), + // makes initiator inputs insufficient to cover fees + shared_output_a: generate_funding_txout( + amount_adjusted_with_p2wsh_fee + 1, + amount_adjusted_with_p2wsh_fee + 1, + ), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(amount_adjusted_with_p2wsh_fee + 1, 0), outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); do_test_interactive_tx_constructor(TestSession { description: "Single contribution with P2WSH input, sufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WSH(1_000_000)]), - outputs_a: generate_output(&TestOutput::P2WPKH(amount_adjusted_with_p2wsh_fee)), + shared_output_a: generate_funding_txout( + amount_adjusted_with_p2wsh_fee, + amount_adjusted_with_p2wsh_fee, + ), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(amount_adjusted_with_p2wsh_fee, 0), outputs_b: vec![], expect_error: None, - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); let p2tr_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, P2TR_INPUT_WEIGHT_LOWER_BOUND); let amount_adjusted_with_p2tr_fee = @@ -2367,61 +2233,63 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Single contribution, with P2TR input, insufficient fees", inputs_a: generate_inputs(&[TestOutput::P2TR(1_000_000)]), - outputs_a: generate_output(&TestOutput::P2WPKH( - amount_adjusted_with_p2tr_fee + 1, /* makes fees insuffcient for initiator */ - )), + // makes initiator inputs insufficient to cover fees + shared_output_a: generate_funding_txout( + amount_adjusted_with_p2tr_fee + 1, + amount_adjusted_with_p2tr_fee + 1, + ), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(amount_adjusted_with_p2tr_fee + 1, 0), outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); do_test_interactive_tx_constructor(TestSession { description: "Single contribution with P2TR input, sufficient fees", inputs_a: generate_inputs(&[TestOutput::P2TR(1_000_000)]), - outputs_a: generate_output(&TestOutput::P2WPKH(amount_adjusted_with_p2tr_fee)), + shared_output_a: generate_funding_txout( + amount_adjusted_with_p2tr_fee, + amount_adjusted_with_p2tr_fee, + ), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(amount_adjusted_with_p2tr_fee, 0), outputs_b: vec![], expect_error: None, - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); do_test_interactive_tx_constructor(TestSession { description: "Initiator contributes sufficient fees, but non-initiator does not", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), + shared_output_a: generate_funding_txout(100_000, 0), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(100_000)]), - outputs_b: generate_output(&TestOutput::P2WPKH(100_000)), + shared_output_b: generate_funding_txout(100_000, 100_000), + outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeB)), - a_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), - b_expected_remote_shared_output: None, }); do_test_interactive_tx_constructor(TestSession { description: "Multi-input-output contributions from both sides", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000); 2]), - outputs_a: vec![ - generate_shared_funding_output_one(1_000_000, 200_000), - generate_output_nonfunding_one(&TestOutput::P2WPKH(200_000)), - ], + shared_output_a: generate_funding_txout(1_000_000, 200_000), + outputs_a: vec![generate_output_nonfunding_one(&TestOutput::P2WPKH(200_000))], inputs_b: generate_inputs(&[ TestOutput::P2WPKH(1_000_000), TestOutput::P2WPKH(500_000), ]), + shared_output_b: generate_funding_txout(1_000_000, 800_000), outputs_b: vec![generate_output_nonfunding_one(&TestOutput::P2WPKH(400_000))], expect_error: None, - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 800_000)), }); do_test_interactive_tx_constructor(TestSession { description: "Prevout from initiator is not a witness program", inputs_a: generate_inputs(&[TestOutput::P2PKH(1_000_000)]), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); let tx = @@ -2433,12 +2301,12 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Invalid input sequence from initiator", inputs_a: vec![(invalid_sequence_input, tx.clone())], - outputs_a: generate_output(&TestOutput::P2WPKH(1_000_000)), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::IncorrectInputSequenceValue, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); let duplicate_input = TxIn { previous_output: OutPoint { txid: tx.as_transaction().compute_txid(), vout: 0 }, @@ -2448,12 +2316,12 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Duplicate prevout from initiator", inputs_a: vec![(duplicate_input.clone(), tx.clone()), (duplicate_input, tx.clone())], - outputs_a: generate_output(&TestOutput::P2WPKH(1_000_000)), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeB)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); // Non-initiator uses same prevout as initiator. let duplicate_input = TxIn { @@ -2464,12 +2332,12 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Non-initiator uses same prevout as initiator", inputs_a: vec![(duplicate_input.clone(), tx.clone())], - outputs_a: generate_shared_funding_output(1_000_000, 905_000), + shared_output_a: generate_funding_txout(1_000_000, 905_000), + outputs_a: vec![], inputs_b: vec![(duplicate_input.clone(), tx.clone())], + shared_output_b: generate_funding_txout(1_000_000, 95_000), outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 95_000)), }); let duplicate_input = TxIn { previous_output: OutPoint { txid: tx.as_transaction().compute_txid(), vout: 0 }, @@ -2479,90 +2347,92 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Non-initiator uses same prevout as initiator", inputs_a: vec![(duplicate_input.clone(), tx.clone())], - outputs_a: generate_output(&TestOutput::P2WPKH(1_000_000)), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), + outputs_a: vec![], inputs_b: vec![(duplicate_input.clone(), tx.clone())], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_p2wpkh_script_pubkey(), 0)), }); do_test_interactive_tx_constructor(TestSession { description: "Initiator sends too many TxAddInputs", inputs_a: generate_fixed_number_of_inputs(MAX_RECEIVED_TX_ADD_INPUT_COUNT + 1), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::ReceivedTooManyTxAddInputs, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); do_test_interactive_tx_constructor_with_entropy_source( TestSession { // We use a deliberately bad entropy source, `DuplicateEntropySource` to simulate this. description: "Attempt to queue up two inputs with duplicate serial ids", inputs_a: generate_fixed_number_of_inputs(2), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::DuplicateSerialId, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }, &DuplicateEntropySource, ); do_test_interactive_tx_constructor(TestSession { description: "Initiator sends too many TxAddOutputs", inputs_a: vec![], - outputs_a: generate_fixed_number_of_outputs(MAX_RECEIVED_TX_ADD_OUTPUT_COUNT + 1), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), + outputs_a: generate_fixed_number_of_outputs(MAX_RECEIVED_TX_ADD_OUTPUT_COUNT), inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::ReceivedTooManyTxAddOutputs, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); + let dust_amount = generate_p2wsh_script_pubkey().minimal_non_dust().to_sat() - 1; do_test_interactive_tx_constructor(TestSession { description: "Initiator sends an output below dust value", inputs_a: vec![], - outputs_a: generate_funding_output( - generate_p2wsh_script_pubkey().minimal_non_dust().to_sat() - 1, - ), + shared_output_a: generate_funding_txout(dust_amount, dust_amount), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(dust_amount, 0), outputs_b: vec![], expect_error: Some((AbortReason::BelowDustLimit, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); do_test_interactive_tx_constructor(TestSession { description: "Initiator sends an output above maximum sats allowed", inputs_a: vec![], - outputs_a: generate_output(&TestOutput::P2WPKH(TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1)), + shared_output_a: generate_funding_txout( + TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1, + TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1, + ), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1, 0), outputs_b: vec![], expect_error: Some((AbortReason::ExceededMaximumSatsAllowed, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); do_test_interactive_tx_constructor(TestSession { description: "Initiator sends an output without a witness program", inputs_a: vec![], + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![generate_non_witness_output(1_000_000)], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::InvalidOutputScript, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); do_test_interactive_tx_constructor_with_entropy_source( TestSession { // We use a deliberately bad entropy source, `DuplicateEntropySource` to simulate this. description: "Attempt to queue up two outputs with duplicate serial ids", inputs_a: vec![], + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: generate_fixed_number_of_outputs(2), inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::DuplicateSerialId, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }, &DuplicateEntropySource, ); @@ -2570,99 +2440,87 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Peer contributed more output value than inputs", inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000)]), - outputs_a: generate_output(&TestOutput::P2WPKH(1_000_000)), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), + outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); do_test_interactive_tx_constructor(TestSession { description: "Peer contributed more than allowed number of inputs", inputs_a: generate_fixed_number_of_inputs(MAX_INPUTS_OUTPUTS_COUNT as u16 + 1), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some(( AbortReason::ExceededNumberOfInputsOrOutputs, ErrorCulprit::Indeterminate, )), - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); do_test_interactive_tx_constructor(TestSession { description: "Peer contributed more than allowed number of outputs", inputs_a: generate_inputs(&[TestOutput::P2WPKH(TOTAL_BITCOIN_SUPPLY_SATOSHIS)]), - outputs_a: generate_fixed_number_of_outputs(MAX_INPUTS_OUTPUTS_COUNT as u16 + 1), + shared_output_a: generate_funding_txout(1_000_000, 1_000_000), + outputs_a: generate_fixed_number_of_outputs(MAX_INPUTS_OUTPUTS_COUNT as u16), inputs_b: vec![], + shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some(( AbortReason::ExceededNumberOfInputsOrOutputs, ErrorCulprit::Indeterminate, )), - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), - }); - - // Adding multiple outputs to the funding output pubkey is an error - do_test_interactive_tx_constructor(TestSession { - description: "Adding two outputs to the funding output pubkey", - inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), - outputs_a: generate_funding_output(100_000), - inputs_b: generate_inputs(&[TestOutput::P2WPKH(1_001_000)]), - outputs_b: generate_funding_output(100_000), - expect_error: Some((AbortReason::DuplicateFundingOutput, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: None, }); // We add the funding output, but we contribute a little do_test_interactive_tx_constructor(TestSession { description: "Funding output by us, small contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(12_000)]), - outputs_a: generate_shared_funding_output(1_000_000, 10_000), + shared_output_a: generate_funding_txout(1_000_000, 10_000), + outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(992_000)]), + shared_output_b: generate_funding_txout(1_000_000, 990_000), outputs_b: vec![], expect_error: None, - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 990_000)), }); // They add the funding output, and we contribute a little do_test_interactive_tx_constructor(TestSession { description: "Funding output by them, small contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(12_000)]), + shared_output_a: generate_funding_txout(1_000_000, 10_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(992_000)]), - outputs_b: generate_shared_funding_output(1_000_000, 990_000), + shared_output_b: generate_funding_txout(1_000_000, 990_000), + outputs_b: vec![], expect_error: None, - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 10_000)), - b_expected_remote_shared_output: None, }); // We add the funding output, and we contribute most do_test_interactive_tx_constructor(TestSession { description: "Funding output by us, large contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(992_000)]), - outputs_a: generate_shared_funding_output(1_000_000, 990_000), + shared_output_a: generate_funding_txout(1_000_000, 990_000), + outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(12_000)]), + shared_output_b: generate_funding_txout(1_000_000, 10_000), outputs_b: vec![], expect_error: None, - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 10_000)), }); // They add the funding output, but we contribute most do_test_interactive_tx_constructor(TestSession { description: "Funding output by them, large contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(992_000)]), + shared_output_a: generate_funding_txout(1_000_000, 990_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(12_000)]), - outputs_b: generate_shared_funding_output(1_000_000, 10_000), + shared_output_b: generate_funding_txout(1_000_000, 10_000), + outputs_b: vec![], expect_error: None, - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 990_000)), - b_expected_remote_shared_output: None, }); // During a splice-out, with peer providing more output value than input value @@ -2671,12 +2529,12 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Splice out with sufficient initiator balance", inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000), TestOutput::P2WPKH(50_000)]), - outputs_a: generate_funding_output(120_000), + shared_output_a: generate_funding_txout(120_000, 120_000), + outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(50_000)]), + shared_output_b: generate_funding_txout(120_000, 0), outputs_b: vec![], expect_error: None, - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); // During a splice-out, with peer providing more output value than input value @@ -2685,37 +2543,24 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Splice out with insufficient initiator balance", inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000), TestOutput::P2WPKH(15_000)]), - outputs_a: generate_funding_output(120_000), + shared_output_a: generate_funding_txout(120_000, 120_000), + outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(85_000)]), + shared_output_b: generate_funding_txout(120_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 0)), }); - // The actual funding output value is lower than the intended local contribution by the same node - do_test_interactive_tx_constructor(TestSession { - description: "Splice in, invalid intended local contribution", - inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000), TestOutput::P2WPKH(15_000)]), - outputs_a: generate_shared_funding_output(100_000, 120_000), // local value is higher than the output value - inputs_b: generate_inputs(&[TestOutput::P2WPKH(85_000)]), - outputs_b: vec![], - expect_error: Some((AbortReason::InvalidLowFundingOutputValue, ErrorCulprit::NodeA)), - a_expected_remote_shared_output: None, - b_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 20_000)), - }); - - // The actual funding output value is lower than the intended local contribution of the other node + // The intended&expected shared output value differ do_test_interactive_tx_constructor(TestSession { description: "Splice in, invalid intended local contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000), TestOutput::P2WPKH(15_000)]), + shared_output_a: generate_funding_txout(100_000, 100_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(85_000)]), - outputs_b: generate_funding_output(100_000), - // The error is caused by NodeA, it occurs when nodeA prepares the message to be sent to NodeB, that's why here it shows up as NodeB - expect_error: Some((AbortReason::InvalidLowFundingOutputValue, ErrorCulprit::NodeB)), - a_expected_remote_shared_output: Some((generate_funding_script_pubkey(), 120_000)), // this is higher than the actual output value - b_expected_remote_shared_output: None, + shared_output_b: generate_funding_txout(120_000, 0), // value different + outputs_b: vec![], + expect_error: Some((AbortReason::MissingFundingOutput, ErrorCulprit::NodeA)), }); } @@ -2754,80 +2599,92 @@ mod tests { }) .collect::>(); let our_contributed = 110_000; - let txout = TxOut { value: Amount::from_sat(128_000), script_pubkey: ScriptBuf::new() }; - let value = txout.value.to_sat(); - let outputs = vec![OutputOwned::Shared(SharedOwnedOutput::new(txout, value))]; + let txout = TxOut { value: Amount::from_sat(10_000), script_pubkey: ScriptBuf::new() }; + let outputs = vec![txout]; let funding_feerate_sat_per_1000_weight = 3000; let total_inputs: u64 = input_prevouts.iter().map(|o| o.value.to_sat()).sum(); - let gross_change = total_inputs - our_contributed; + let total_outputs: u64 = outputs.iter().map(|o| o.value.to_sat()).sum(); + let gross_change = total_inputs - total_outputs - our_contributed; let fees = 1746; - let common_fees = 126; + let common_fees = 234; { // There is leftover for change let res = calculate_change_output_value( true, our_contributed, + &ScriptBuf::new(), &inputs, &outputs, funding_feerate_sat_per_1000_weight, 300, ); - assert_eq!(res.unwrap().unwrap(), gross_change - fees - common_fees); + assert_eq!(res, Ok(Some(gross_change - fees - common_fees))); } { // There is leftover for change, without common fees let res = calculate_change_output_value( false, our_contributed, + &ScriptBuf::new(), &inputs, &outputs, funding_feerate_sat_per_1000_weight, 300, ); - assert_eq!(res.unwrap().unwrap(), gross_change - fees); + assert_eq!(res, Ok(Some(gross_change - fees))); } { // Larger fee, smaller change - let res = - calculate_change_output_value(true, our_contributed, &inputs, &outputs, 9000, 300); - assert_eq!(res.unwrap().unwrap(), 14384); + let res = calculate_change_output_value( + true, + our_contributed, + &ScriptBuf::new(), + &inputs, + &outputs, + funding_feerate_sat_per_1000_weight * 3, + 300, + ); + assert_eq!(res, Ok(Some(4060))); } { // Insufficient inputs, no leftover let res = calculate_change_output_value( false, 130_000, + &ScriptBuf::new(), &inputs, &outputs, funding_feerate_sat_per_1000_weight, 300, ); - assert_eq!(res.err().unwrap(), AbortReason::InsufficientFees); + assert_eq!(res, Err(AbortReason::InsufficientFees)); } { // Very small leftover let res = calculate_change_output_value( false, - 128_100, + 118_000, + &ScriptBuf::new(), &inputs, &outputs, funding_feerate_sat_per_1000_weight, 300, ); - assert!(res.unwrap().is_none()); + assert_eq!(res, Ok(None)); } { // Small leftover, but not dust let res = calculate_change_output_value( false, - 128_100, + 117_992, + &ScriptBuf::new(), &inputs, &outputs, funding_feerate_sat_per_1000_weight, 100, ); - assert_eq!(res.unwrap().unwrap(), 154); + assert_eq!(res, Ok(Some(262))); } } } From c18422d8a4b2798d96d1969b7d3bf211bea0ec38 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Sat, 21 Jun 2025 14:46:12 +0200 Subject: [PATCH 2/3] Add Shared Input support In interactivetxs, add support for shared inputs, similar to shared outputs. A shared input is optional, and is used in case of splicing to add the current funding as an input. --- lightning/src/ln/channel.rs | 5 +- lightning/src/ln/dual_funding_tests.rs | 7 +- lightning/src/ln/interactivetxs.rs | 728 +++++++++++++++++++------ lightning/src/ln/msgs.rs | 32 +- lightning/src/util/ser.rs | 37 +- 5 files changed, 616 insertions(+), 193 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9677e41ac09..321b8700506 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2771,7 +2771,8 @@ where }; let change_value_opt = calculate_change_output_value( self.funding.is_outbound(), self.dual_funding_context.our_funding_satoshis, - &shared_funding_output.script_pubkey, &funding_inputs, &funding_outputs, + &funding_inputs, None, + &shared_funding_output.script_pubkey, &funding_outputs, self.dual_funding_context.funding_feerate_sat_per_1000_weight, change_script.minimal_non_dust().to_sat(), )?; @@ -2799,6 +2800,7 @@ where is_initiator: self.funding.is_outbound(), funding_tx_locktime: self.dual_funding_context.funding_tx_locktime, inputs_to_contribute: funding_inputs, + shared_funding_input: None, shared_funding_output: SharedOwnedOutput::new(shared_funding_output, self.dual_funding_context.our_funding_satoshis), outputs_to_contribute: funding_outputs, }; @@ -11812,6 +11814,7 @@ where funding_tx_locktime: dual_funding_context.funding_tx_locktime, is_initiator: false, inputs_to_contribute: our_funding_inputs, + shared_funding_input: None, shared_funding_output: SharedOwnedOutput::new(shared_funding_output, our_funding_satoshis), outputs_to_contribute: Vec::new(), } diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index ed770d06e6d..39cf6200765 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -89,13 +89,14 @@ fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession) let tx_add_input_msg = TxAddInput { channel_id, serial_id: 2, // Even serial_id from initiator. - prevtx: initiator_funding_inputs[0].1.clone(), + prevtx: Some(initiator_funding_inputs[0].1.clone()), prevtx_out: 0, sequence: initiator_funding_inputs[0].0.sequence.0, shared_input_txid: None, }; - let input_value = - tx_add_input_msg.prevtx.as_transaction().output[tx_add_input_msg.prevtx_out as usize].value; + let input_value = tx_add_input_msg.prevtx.as_ref().unwrap().as_transaction().output + [tx_add_input_msg.prevtx_out as usize] + .value; assert_eq!(input_value.to_sat(), session.initiator_input_value_satoshis); nodes[1].node.handle_tx_add_input(nodes[0].node.get_our_node_id(), &tx_add_input_msg); diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 4c2544d8f41..66ee00c12a5 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -21,6 +21,7 @@ use bitcoin::{OutPoint, ScriptBuf, Sequence, Transaction, TxIn, TxOut, Txid, Wei use crate::chain::chaininterface::fee_for_weight; use crate::events::bump_transaction::{BASE_INPUT_WEIGHT, EMPTY_SCRIPT_SIG_WEIGHT}; +use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT; use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; use crate::ln::msgs; use crate::ln::msgs::{MessageSendEvent, SerialId, TxSignatures}; @@ -106,14 +107,20 @@ pub(crate) enum AbortReason { InsufficientFees, OutputsValueExceedsInputsValue, InvalidTx, + /// No funding (shared) input found. + MissingFundingInput, + /// A funding (shared) input was seen, but we don't expect one + UnexpectedFundingInput, + /// In tx_add_input, the prev_tx field must be filled in case of non-shared input + MissingPrevTx, + /// In tx_add_input, the prev_tx field should not be filled in case of shared input + UnexpectedPrevTx, /// No funding (shared) output found. MissingFundingOutput, /// More than one funding (shared) output found. DuplicateFundingOutput, - /// The intended local part of the funding output is higher than the actual shared funding output, - /// if funding output is provided by the peer this is an interop error, - /// if provided by the same node than internal input consistency error. - InvalidLowFundingOutputValue, + /// More than one funding (shared) input found. + DuplicateFundingInput, /// Internal error InternalError(&'static str), } @@ -158,13 +165,21 @@ impl Display for AbortReason { f.write_str("Total value of outputs exceeds total value of inputs") }, AbortReason::InvalidTx => f.write_str("The transaction is invalid"), + AbortReason::MissingFundingInput => f.write_str("No shared funding input found"), + AbortReason::UnexpectedFundingInput => { + f.write_str("A funding (shared) input was seen, but we don't expect one") + }, + AbortReason::MissingPrevTx => f.write_str( + "In tx_add_input, the prev_tx field must be filled in case of non-shared input", + ), + AbortReason::UnexpectedPrevTx => f.write_str( + "In tx_add_input, the prev_tx should not be filled in case of shared input", + ), AbortReason::MissingFundingOutput => f.write_str("No shared funding output found"), AbortReason::DuplicateFundingOutput => { f.write_str("More than one funding output found") }, - AbortReason::InvalidLowFundingOutputValue => f.write_str( - "Local part of funding output value is greater than the funding output value", - ), + AbortReason::DuplicateFundingInput => f.write_str("More than one funding input found"), AbortReason::InternalError(text) => { f.write_fmt(format_args!("Internal error: {}", text)) }, @@ -176,7 +191,7 @@ impl Display for AbortReason { pub(crate) struct ConstructedTransaction { holder_is_initiator: bool, - inputs: Vec, + inputs: Vec, outputs: Vec, local_inputs_value_satoshis: u64, @@ -189,6 +204,20 @@ pub(crate) struct ConstructedTransaction { holder_sends_tx_signatures_first: bool, } +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct NegotiatedTxInput { + serial_id: SerialId, + txin: TxIn, + // The weight of the input including an estimate of its witness weight. + weight: Weight, +} + +impl_writeable_tlv_based!(NegotiatedTxInput, { + (1, serial_id, required), + (3, txin, required), + (5, weight, required), +}); + impl_writeable_tlv_based!(ConstructedTransaction, { (1, holder_is_initiator, required), (3, inputs, required), @@ -202,7 +231,22 @@ impl_writeable_tlv_based!(ConstructedTransaction, { }); impl ConstructedTransaction { - fn new(context: NegotiationContext) -> Self { + 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 local_inputs_value_satoshis = context .inputs .iter() @@ -215,10 +259,11 @@ impl ConstructedTransaction { let remote_inputs_value_satoshis = context.remote_inputs_value(); let remote_outputs_value_satoshis = context.remote_outputs_value(); - let mut inputs: Vec = context.inputs.into_values().collect(); + let mut inputs: Vec = + context.inputs.into_values().map(|tx_input| tx_input.into_negotiated_input()).collect(); let mut outputs: Vec = context.outputs.into_values().collect(); // Inputs and outputs must be sorted by serial_id - inputs.sort_unstable_by_key(|input| input.serial_id()); + inputs.sort_unstable_by_key(|input| input.serial_id); outputs.sort_unstable_by_key(|output| output.serial_id); // There is a strict ordering for `tx_signatures` exchange to prevent deadlocks. @@ -232,7 +277,7 @@ impl ConstructedTransaction { local_inputs_value_satoshis < remote_inputs_value_satoshis }; - Self { + let constructed_tx = Self { holder_is_initiator: context.holder_is_initiator, local_inputs_value_satoshis, @@ -246,12 +291,18 @@ impl ConstructedTransaction { lock_time: context.tx_locktime, holder_sends_tx_signatures_first, + }; + + if constructed_tx.weight().to_wu() > MAX_STANDARD_TX_WEIGHT as u64 { + return Err(AbortReason::TransactionTooLarge); } + + Ok(constructed_tx) } pub fn weight(&self) -> Weight { let inputs_weight = self.inputs.iter().fold(Weight::from_wu(0), |weight, input| { - weight.checked_add(estimate_input_weight(input.prev_output())).unwrap_or(Weight::MAX) + weight.checked_add(input.weight).unwrap_or(Weight::MAX) }); let outputs_weight = self.outputs.iter().fold(Weight::from_wu(0), |weight, output| { weight.checked_add(get_output_weight(output.script_pubkey())).unwrap_or(Weight::MAX) @@ -265,7 +316,7 @@ impl ConstructedTransaction { pub fn build_unsigned_tx(&self) -> Transaction { let ConstructedTransaction { inputs, outputs, .. } = self; - let input: Vec = inputs.iter().map(|input| input.txin().clone()).collect(); + let input: Vec = inputs.iter().map(|input| input.txin.clone()).collect(); let output: Vec = outputs.iter().map(|output| output.tx_out().clone()).collect(); Transaction { version: Version::TWO, lock_time: self.lock_time, input, output } @@ -275,7 +326,7 @@ impl ConstructedTransaction { self.outputs.iter() } - pub fn inputs(&self) -> impl Iterator { + pub fn inputs(&self) -> impl Iterator { self.inputs.iter() } @@ -290,9 +341,9 @@ impl ConstructedTransaction { self.inputs .iter_mut() .filter(|input| { - !is_serial_id_valid_for_counterparty(self.holder_is_initiator, input.serial_id()) + !is_serial_id_valid_for_counterparty(self.holder_is_initiator, input.serial_id) }) - .map(|input| input.txin_mut()) + .map(|input| &mut input.txin) .zip(witnesses) .for_each(|(input, witness)| input.witness = witness); } @@ -304,9 +355,9 @@ impl ConstructedTransaction { self.inputs .iter_mut() .filter(|input| { - is_serial_id_valid_for_counterparty(self.holder_is_initiator, input.serial_id()) + is_serial_id_valid_for_counterparty(self.holder_is_initiator, input.serial_id) }) - .map(|input| input.txin_mut()) + .map(|input| &mut input.txin) .zip(witnesses) .for_each(|(input, witness)| input.witness = witness); } @@ -419,7 +470,7 @@ impl InteractiveTxSigningSession { .filter(|input| { is_serial_id_valid_for_counterparty( self.unsigned_tx.holder_is_initiator, - input.serial_id(), + input.serial_id, ) }) .count() @@ -432,7 +483,7 @@ impl InteractiveTxSigningSession { .filter(|input| { !is_serial_id_valid_for_counterparty( self.unsigned_tx.holder_is_initiator, - input.serial_id(), + input.serial_id, ) }) .count() @@ -445,7 +496,7 @@ impl InteractiveTxSigningSession { Transaction { version: Version::TWO, lock_time, - input: inputs.iter().cloned().map(|input| input.into_txin()).collect(), + input: inputs.iter().cloned().map(|input| input.txin).collect(), output: outputs.iter().cloned().map(|output| output.into_tx_out()).collect(), } } @@ -466,6 +517,14 @@ struct NegotiationContext { received_tx_add_input_count: u16, received_tx_add_output_count: u16, inputs: HashMap, + /// Optional intended/expected funding input, used during splicing. + /// The funding input is shared, it is usually co-owned by both peers. + /// - For the initiator: + /// The intended previous funding input. This will be added alongside + /// the provided inputs. + /// - For the acceptor: + /// The expected previous funding input. It should be added by the initiator node. + shared_funding_input: Option, /// The intended/expected funding output, potentially co-owned by both peers (shared). /// - For the initiator: /// The output intended to be the new funding output. This will be added alongside @@ -509,8 +568,8 @@ fn is_serial_id_valid_for_counterparty(holder_is_initiator: bool, serial_id: Ser impl NegotiationContext { fn new( holder_node_id: PublicKey, counterparty_node_id: PublicKey, holder_is_initiator: bool, - shared_funding_output: SharedOwnedOutput, tx_locktime: AbsoluteLockTime, - feerate_sat_per_kw: u32, + shared_funding_input: Option, shared_funding_output: SharedOwnedOutput, + tx_locktime: AbsoluteLockTime, feerate_sat_per_kw: u32, ) -> Self { NegotiationContext { holder_node_id, @@ -519,6 +578,7 @@ impl NegotiationContext { received_tx_add_input_count: 0, received_tx_add_output_count: 0, inputs: new_hash_map(), + shared_funding_input, shared_funding_output, prevtx_outpoints: new_hash_set(), outputs: new_hash_map(), @@ -545,7 +605,7 @@ impl NegotiationContext { .iter() .filter(|(serial_id, _)| self.is_serial_id_valid_for_counterparty(serial_id)) .fold(0u64, |weight, (_, input)| { - weight.saturating_add(estimate_input_weight(input.prev_output()).to_wu()) + weight.saturating_add(input.estimate_input_weight().to_wu()) }), ) } @@ -562,9 +622,7 @@ impl NegotiationContext { } fn local_inputs_value(&self) -> u64 { - self.inputs - .iter() - .fold(0u64, |acc, (_, input)| acc.saturating_add(input.prev_output().value.to_sat())) + self.inputs.iter().fold(0u64, |acc, (_, input)| acc.saturating_add(input.value())) } fn received_tx_add_input(&mut self, msg: &msgs::TxAddInput) -> Result<(), AbortReason> { @@ -595,36 +653,64 @@ impl NegotiationContext { return Err(AbortReason::IncorrectInputSequenceValue); } - let transaction = msg.prevtx.as_transaction(); - let txid = transaction.compute_txid(); + // Extract info from msg, check if shared + let (input, prev_outpoint) = if let Some(shared_txid) = &msg.shared_input_txid { + if self.holder_is_initiator { + return Err(AbortReason::DuplicateFundingInput); + } + if msg.prevtx.is_some() { + return Err(AbortReason::UnexpectedPrevTx); + } + if let Some(shared_funding_input) = &self.shared_funding_input { + if self.inputs.values().any(|input| matches!(input.input, InputOwned::Shared(_))) { + return Err(AbortReason::DuplicateFundingInput); + } - 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); + let previous_output = OutPoint { txid: *shared_txid, vout: msg.prevtx_out }; + if previous_output != shared_funding_input.input.previous_output { + return Err(AbortReason::UnexpectedFundingInput); + } + + (InputOwned::Shared(shared_funding_input.clone()), previous_output) + } else { + return Err(AbortReason::UnexpectedFundingInput); } + } else if let Some(prevtx) = &msg.prevtx { + let transaction = prevtx.as_transaction(); + let txid = transaction.compute_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 }) { + let prev_outpoint = OutPoint { txid, vout: msg.prevtx_out }; + let txin = TxIn { + previous_output: prev_outpoint, + sequence: Sequence(msg.sequence), + ..Default::default() + }; + ( + InputOwned::Single(SingleOwnedInput { + input: txin, + prev_tx: prevtx.clone(), + prev_output: tx_out.clone(), + }), + prev_outpoint, + ) + } else { // The receiving node: // - MUST fail the negotiation if: - // - the `prevtx` and `prevtx_vout` are identical to a previously added - // (and not removed) input's + // - `prevtx_vout` is greater or equal to the number of outputs on `prevtx` 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); + return Err(AbortReason::MissingPrevTx); }; + match self.inputs.entry(msg.serial_id) { hash_map::Entry::Occupied(_) => { // The receiving node: @@ -633,17 +719,18 @@ impl NegotiationContext { Err(AbortReason::DuplicateSerialId) }, hash_map::Entry::Vacant(entry) => { - let prev_outpoint = OutPoint { txid, vout: msg.prevtx_out }; - entry.insert(InteractiveTxInput::Remote(LocalOrRemoteInput { + if !self.prevtx_outpoints.insert(prev_outpoint) { + // 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); + } + entry.insert(InteractiveTxInput { serial_id: msg.serial_id, - input: TxIn { - previous_output: prev_outpoint, - sequence: Sequence(msg.sequence), - ..Default::default() - }, - prev_output: prev_out, - })); - self.prevtx_outpoints.insert(prev_outpoint); + added_by: AddingRole::Remote, + input, + }); Ok(()) }, } @@ -764,23 +851,40 @@ impl NegotiationContext { } fn sent_tx_add_input(&mut self, msg: &msgs::TxAddInput) -> Result<(), AbortReason> { - let tx = msg.prevtx.as_transaction(); - let txin = TxIn { - previous_output: OutPoint { txid: tx.compute_txid(), vout: msg.prevtx_out }, - sequence: Sequence(msg.sequence), - ..Default::default() + 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 }; + if let Some(shared_funding_input) = &self.shared_funding_input { + (prev_outpoint, InputOwned::Shared(shared_funding_input.clone())) + } else { + return Err(AbortReason::UnexpectedFundingInput); + } + } else if let Some(prevtx) = &msg.prevtx { + let prev_txid = prevtx.as_transaction().compute_txid(); + let prev_outpoint = OutPoint { txid: prev_txid, vout: msg.prevtx_out }; + let prev_output = prevtx + .as_transaction() + .output + .get(vout) + .ok_or(AbortReason::PrevTxOutInvalid)? + .clone(); + let txin = TxIn { + previous_output: prev_outpoint, + sequence: Sequence(msg.sequence), + ..Default::default() + }; + let single_input = + SingleOwnedInput { input: txin, prev_tx: prevtx.clone(), prev_output }; + (prev_outpoint, InputOwned::Single(single_input)) + } else { + return Err(AbortReason::MissingPrevTx); }; - if !self.prevtx_outpoints.insert(txin.previous_output) { + if !self.prevtx_outpoints.insert(prev_outpoint) { // We have added an input that already exists return Err(AbortReason::PrevTxOutInvalid); } - let vout = txin.previous_output.vout as usize; - let prev_output = tx.output.get(vout).ok_or(AbortReason::PrevTxOutInvalid)?.clone(); - let input = InteractiveTxInput::Local(LocalOrRemoteInput { - serial_id: msg.serial_id, - input: txin, - prev_output, - }); + let input = + InteractiveTxInput { serial_id: msg.serial_id, added_by: AddingRole::Local, input }; self.inputs.insert(msg.serial_id, input); Ok(()) } @@ -853,20 +957,7 @@ impl NegotiationContext { // - 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))?; - let shared_funding_output = self.shared_funding_output.clone(); - let constructed_tx = ConstructedTransaction::new(self); - if !constructed_tx - .outputs - .iter() - .any(|output| *output.tx_out() == shared_funding_output.tx_out) - { - return Err(AbortReason::MissingFundingOutput); - } - if constructed_tx.weight().to_wu() > MAX_STANDARD_TX_WEIGHT as u64 { - return Err(AbortReason::TransactionTooLarge); - } - - Ok(constructed_tx) + ConstructedTransaction::new(self) } } @@ -1074,12 +1165,13 @@ impl StateMachine { fn new( holder_node_id: PublicKey, counterparty_node_id: PublicKey, feerate_sat_per_kw: u32, is_initiator: bool, tx_locktime: AbsoluteLockTime, - shared_funding_output: SharedOwnedOutput, + shared_funding_input: Option, shared_funding_output: SharedOwnedOutput, ) -> Self { let context = NegotiationContext::new( holder_node_id, counterparty_node_id, is_initiator, + shared_funding_input, shared_funding_output, tx_locktime, feerate_sat_per_kw, @@ -1155,29 +1247,118 @@ impl_writeable_tlv_based_enum!(AddingRole, /// Represents an input -- local or remote (both have the same fields) #[derive(Clone, Debug, Eq, PartialEq)] -pub struct LocalOrRemoteInput { - serial_id: SerialId, +struct SingleOwnedInput { input: TxIn, + prev_tx: TransactionU16LenLimited, prev_output: TxOut, } -impl_writeable_tlv_based!(LocalOrRemoteInput, { - (1, serial_id, required), - (3, input, required), - (5, prev_output, required), -}); +#[derive(Clone, Debug, Eq, PartialEq)] +pub(super) struct SharedOwnedInput { + input: TxIn, + prev_output: TxOut, + local_owned: u64, +} + +impl SharedOwnedInput { + pub fn new(input: TxIn, prev_output: TxOut, local_owned: u64) -> Self { + let value = prev_output.value.to_sat(); + debug_assert!( + local_owned <= value, + "SharedOwnedInput: Inconsistent local_owned value {}, larger than prev out value {}", + local_owned, + value, + ); + Self { input, prev_output, local_owned } + } + + fn remote_owned(&self) -> u64 { + self.prev_output.value.to_sat().saturating_sub(self.local_owned) + } +} +/// A transaction input, differentiated by ownership: +/// - exclusive by the adder, or +/// - shared #[derive(Clone, Debug, Eq, PartialEq)] -pub(crate) enum InteractiveTxInput { - Local(LocalOrRemoteInput), - Remote(LocalOrRemoteInput), - // TODO(splicing) SharedInput should be added +enum InputOwned { + /// Belongs to a single party -- controlled exclusively and fully belonging to a single party + /// Includes the input and the previous output + Single(SingleOwnedInput), + /// Input with shared control and value split between the counterparties (or fully by one). + Shared(SharedOwnedInput), } -impl_writeable_tlv_based_enum!(InteractiveTxInput, - {1, Local} => (), - {3, Remote} => (), -); +impl InputOwned { + pub fn tx_in(&self) -> &TxIn { + match &self { + InputOwned::Single(single) => &single.input, + InputOwned::Shared(shared) => &shared.input, + } + } + + pub fn tx_in_mut(&mut self) -> &mut TxIn { + match self { + InputOwned::Single(ref mut single) => &mut single.input, + InputOwned::Shared(shared) => &mut shared.input, + } + } + + pub 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(), + InputOwned::Shared(shared) => shared.prev_output.value.to_sat(), + } + } + + fn is_shared(&self) -> bool { + match self { + InputOwned::Single(_) => false, + InputOwned::Shared(_) => true, + } + } + + fn local_value(&self, local_role: AddingRole) -> u64 { + match self { + InputOwned::Single(single) => match local_role { + AddingRole::Local => single.prev_output.value.to_sat(), + AddingRole::Remote => 0, + }, + InputOwned::Shared(shared) => shared.local_owned, + } + } + + fn remote_value(&self, local_role: AddingRole) -> u64 { + match self { + InputOwned::Single(single) => match local_role { + AddingRole::Local => 0, + AddingRole::Remote => single.prev_output.value.to_sat(), + }, + InputOwned::Shared(shared) => shared.remote_owned(), + } + } + + fn estimate_input_weight(&self) -> Weight { + match self { + InputOwned::Single(single) => estimate_input_weight(&single.prev_output), + InputOwned::Shared(shared) => estimate_input_weight(&shared.prev_output), + } + } +} + +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct InteractiveTxInput { + serial_id: SerialId, + added_by: AddingRole, + input: InputOwned, +} #[derive(Clone, Debug, Eq, PartialEq)] pub(super) struct SharedOwnedOutput { @@ -1308,56 +1489,40 @@ impl InteractiveTxOutput { impl InteractiveTxInput { pub fn serial_id(&self) -> SerialId { - match self { - InteractiveTxInput::Local(input) => input.serial_id, - InteractiveTxInput::Remote(input) => input.serial_id, - } + self.serial_id } pub fn txin(&self) -> &TxIn { - match self { - InteractiveTxInput::Local(input) => &input.input, - InteractiveTxInput::Remote(input) => &input.input, - } + self.input.tx_in() } pub fn txin_mut(&mut self) -> &mut TxIn { - match self { - InteractiveTxInput::Local(input) => &mut input.input, - InteractiveTxInput::Remote(input) => &mut input.input, - } + self.input.tx_in_mut() } pub fn into_txin(self) -> TxIn { - match self { - InteractiveTxInput::Local(input) => input.input, - InteractiveTxInput::Remote(input) => input.input, - } - } - - pub fn prev_output(&self) -> &TxOut { - match self { - InteractiveTxInput::Local(input) => &input.prev_output, - InteractiveTxInput::Remote(input) => &input.prev_output, - } + self.input.into_tx_in() } pub fn value(&self) -> u64 { - self.prev_output().value.to_sat() + self.input.value() } pub fn local_value(&self) -> u64 { - match self { - InteractiveTxInput::Local(input) => input.prev_output.value.to_sat(), - InteractiveTxInput::Remote(_input) => 0, - } + self.input.local_value(self.added_by) } pub fn remote_value(&self) -> u64 { - match self { - InteractiveTxInput::Local(_input) => 0, - InteractiveTxInput::Remote(input) => input.prev_output.value.to_sat(), - } + self.input.remote_value(self.added_by) + } + + pub fn estimate_input_weight(&self) -> Weight { + self.input.estimate_input_weight() + } + + fn into_negotiated_input(self) -> NegotiatedTxInput { + let weight = self.input.estimate_input_weight(); + NegotiatedTxInput { serial_id: self.serial_id, txin: self.input.into_tx_in(), weight } } } @@ -1365,7 +1530,7 @@ pub(super) struct InteractiveTxConstructor { state_machine: StateMachine, initiator_first_message: Option, channel_id: ChannelId, - inputs_to_contribute: Vec<(SerialId, TxIn, TransactionU16LenLimited)>, + inputs_to_contribute: Vec<(SerialId, InputOwned)>, outputs_to_contribute: Vec<(SerialId, OutputOwned)>, } @@ -1492,6 +1657,7 @@ where pub is_initiator: bool, pub funding_tx_locktime: AbsoluteLockTime, pub inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited)>, + pub shared_funding_input: Option, pub shared_funding_output: SharedOwnedOutput, pub outputs_to_contribute: Vec, } @@ -1514,6 +1680,7 @@ impl InteractiveTxConstructor { is_initiator, funding_tx_locktime, inputs_to_contribute, + shared_funding_input, shared_funding_output, outputs_to_contribute, } = args; @@ -1524,21 +1691,40 @@ impl InteractiveTxConstructor { feerate_sat_per_kw, is_initiator, funding_tx_locktime, + shared_funding_input.clone(), shared_funding_output.clone(), ); - 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(); + // Check for the existence of prevouts' + for (txin, tx) in inputs_to_contribute.iter() { + let vout = txin.previous_output.vout as usize; + if tx.as_transaction().output.get(vout).is_none() { + return Err(AbortReason::PrevTxOutInvalid); + } + } + let mut inputs_to_contribute: Vec<(SerialId, InputOwned)> = inputs_to_contribute + .into_iter() + .map(|(txin, tx)| { + let serial_id = generate_holder_serial_id(entropy_source, is_initiator); + let vout = txin.previous_output.vout as usize; + let prev_output = tx.as_transaction().output.get(vout).unwrap().clone(); // checked above + let input = + InputOwned::Single(SingleOwnedInput { input: txin, prev_tx: tx, prev_output }); + (serial_id, input) + }) + .collect(); + if let Some(shared_funding_input) = &shared_funding_input { + if is_initiator { + // Add shared funding input + let serial_id = generate_holder_serial_id(entropy_source, is_initiator); + inputs_to_contribute + .push((serial_id, InputOwned::Shared(shared_funding_input.clone()))); + } + } // 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); + inputs_to_contribute.sort_unstable_by_key(|(serial_id, _)| *serial_id); let mut outputs_to_contribute: Vec<_> = outputs_to_contribute .into_iter() @@ -1578,14 +1764,24 @@ impl InteractiveTxConstructor { fn maybe_send_message(&mut self) -> Result { // 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(), - shared_input_txid: None, + if let Some((serial_id, input)) = self.inputs_to_contribute.pop() { + let msg = match input { + InputOwned::Single(single) => msgs::TxAddInput { + channel_id: self.channel_id, + serial_id, + prevtx: Some(single.prev_tx), + 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, + prevtx: None, + prevtx_out: shared.input.previous_output.vout, + sequence: shared.input.sequence.to_consensus_u32(), + shared_input_txid: Some(shared.input.previous_output.txid), + }, }; do_state_transition!(self, sent_tx_add_input, &msg)?; Ok(InteractiveTxMessageSend::TxAddInput(msg)) @@ -1667,19 +1863,32 @@ impl InteractiveTxConstructor { } } -/// Determine whether a change output should be added, and if yes, of what size, -/// considering our given inputs & outputs, and intended contribution. -/// Computes the fees, takes into account the fees and the dust limit. +/// Determine whether a change output should be added, and if yes, of what size, considering our +/// given inputs and outputs, and intended contribution. Takes into account the fees and the dust +/// limit. +/// /// Three outcomes are possible: /// - Inputs are sufficient for intended contribution, fees, and a larger-than-dust change: /// `Ok(Some(change_amount))` /// - Inputs are sufficient for intended contribution and fees, and a change output isn't needed: /// `Ok(None)` -/// - Inputs are not sufficent to cover contribution and fees: +/// - Inputs are not sufficient to cover contribution and fees: /// `Err(AbortReason::InsufficientFees)` +/// +/// Parameters: +/// - `is_initiator` - Whether we are the negotiation initiator or not (acceptor). +/// - `our_contribution` - The sats amount we intend to contribute to the funding +/// transaction being negotiated. +/// - `funding_inputs` - List of our inputs. It does not include the shared input, if there is one. +/// - `shared_input` - The locally owned amount of the shared input (in sats), if there is one. +/// - `shared_output_funding_script` - The script of the shared output. +/// - `funding_outputs` - Our funding outputs. +/// - `funding_feerate_sat_per_1000_weight` - Fee rate to be used. +/// - `change_output_dust_limit` - The dust limit (in sats) to consider. pub(super) fn calculate_change_output_value( - is_initiator: bool, our_contribution: u64, shared_output_funding_script: &ScriptBuf, - funding_inputs: &Vec<(TxIn, TransactionU16LenLimited)>, funding_outputs: &Vec, + is_initiator: bool, our_contribution: u64, + funding_inputs: &Vec<(TxIn, TransactionU16LenLimited)>, shared_input: Option, + shared_output_funding_script: &ScriptBuf, funding_outputs: &Vec, funding_feerate_sat_per_1000_weight: u32, change_output_dust_limit: u64, ) -> Result, AbortReason> { // Process inputs and their prev txs: @@ -1691,13 +1900,18 @@ pub(super) fn calculate_change_output_value( if txin.previous_output.txid != txid { return Err(AbortReason::PrevTxOutInvalid); } - if let Some(output) = tx.as_transaction().output.get(txin.previous_output.vout as usize) { - total_input_satoshis = total_input_satoshis.saturating_add(output.value.to_sat()); - our_funding_inputs_weight = - our_funding_inputs_weight.saturating_add(estimate_input_weight(output).to_wu()); - } else { - return Err(AbortReason::PrevTxOutInvalid); - } + let output = tx + .as_transaction() + .output + .get(txin.previous_output.vout as usize) + .ok_or(AbortReason::PrevTxOutInvalid)?; + total_input_satoshis = total_input_satoshis.saturating_add(output.value.to_sat()); + let weight = estimate_input_weight(output).to_wu(); + our_funding_inputs_weight = our_funding_inputs_weight.saturating_add(weight); + } + + if let Some(shared_input) = shared_input { + total_input_satoshis = total_input_satoshis.saturating_add(shared_input); } let total_output_satoshis = @@ -1713,6 +1927,10 @@ pub(super) fn calculate_change_output_value( if is_initiator { weight = weight.saturating_add(get_output_weight(shared_output_funding_script).to_wu()); weight = weight.saturating_add(TX_COMMON_FIELDS_WEIGHT); + + if shared_input.is_some() { + weight = weight.saturating_add(FUNDING_TRANSACTION_WITNESS_WEIGHT); + } } let fees_sats = fee_for_weight(funding_feerate_sat_per_1000_weight, weight); @@ -1740,7 +1958,7 @@ mod tests { use crate::ln::interactivetxs::{ calculate_change_output_value, generate_holder_serial_id, AbortReason, HandleTxCompleteValue, InteractiveTxConstructor, InteractiveTxConstructorArgs, - InteractiveTxMessageSend, SharedOwnedOutput, MAX_INPUTS_OUTPUTS_COUNT, + InteractiveTxMessageSend, SharedOwnedInput, SharedOwnedOutput, MAX_INPUTS_OUTPUTS_COUNT, MAX_RECEIVED_TX_ADD_INPUT_COUNT, MAX_RECEIVED_TX_ADD_OUTPUT_COUNT, }; use crate::ln::types::ChannelId; @@ -1810,10 +2028,12 @@ mod tests { struct TestSession { description: &'static str, inputs_a: Vec<(TxIn, TransactionU16LenLimited)>, + a_shared_input: Option<(OutPoint, TxOut, u64)>, /// The funding output, with the value contributed shared_output_a: (TxOut, u64), outputs_a: Vec, inputs_b: Vec<(TxIn, TransactionU16LenLimited)>, + b_shared_input: Option<(OutPoint, TxOut, u64)>, /// The funding output, with the value contributed shared_output_b: (TxOut, u64), outputs_b: Vec, @@ -1858,6 +2078,17 @@ mod tests { is_initiator: true, funding_tx_locktime, inputs_to_contribute: session.inputs_a, + shared_funding_input: session.a_shared_input.map(|(op, prev_output, lo)| { + SharedOwnedInput::new( + TxIn { + previous_output: op, + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + ..Default::default() + }, + prev_output, + lo, + ) + }), shared_funding_output: SharedOwnedOutput::new( session.shared_output_a.0, session.shared_output_a.1, @@ -1884,6 +2115,17 @@ mod tests { is_initiator: false, funding_tx_locktime, inputs_to_contribute: session.inputs_b, + shared_funding_input: session.b_shared_input.map(|(op, prev_output, lo)| { + SharedOwnedInput::new( + TxIn { + previous_output: op, + sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, + ..Default::default() + }, + prev_output, + lo, + ) + }), shared_funding_output: SharedOwnedOutput::new( session.shared_output_b.0, session.shared_output_b.1, @@ -2046,17 +2288,32 @@ mod tests { .iter() .enumerate() .map(|(idx, _)| { - let input = TxIn { + let txin = 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()) + (txin, TransactionU16LenLimited::new(tx.clone()).unwrap()) }) .collect() } + fn generate_shared_input( + prev_funding_tx: &Transaction, vout: u32, local_owned: u64, + ) -> (OutPoint, TxOut, u64) { + let txid = prev_funding_tx.compute_txid(); + let prev_output = prev_funding_tx.output.get(vout as usize).unwrap(); + let value = prev_output.value.to_sat(); + assert!( + local_owned <= value, + "local owned > value for shared input, {} {}", + local_owned, + value, + ); + (OutPoint { txid, vout }, prev_output.clone(), local_owned) + } + fn generate_p2wsh_script_pubkey() -> ScriptBuf { Builder::new().push_opcode(opcodes::OP_TRUE).into_script().to_p2wsh() } @@ -2139,12 +2396,25 @@ mod tests { #[test] fn test_interactive_tx_constructor() { + // A transaction that can be used as a previous funding transaction + let prev_funding_tx_1 = Transaction { + input: Vec::new(), + output: vec![TxOut { + value: Amount::from_sat(60_000), + script_pubkey: ScriptBuf::new(), + }], + lock_time: AbsoluteLockTime::ZERO, + version: Version::TWO, + }; + do_test_interactive_tx_constructor(TestSession { description: "Single contribution, no initiator inputs", inputs_a: vec![], + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)), @@ -2153,9 +2423,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Single contribution, no fees", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)), @@ -2173,6 +2445,7 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Single contribution, with P2WPKH input, insufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), + a_shared_input: None, // makes initiator inputs insufficient to cover fees shared_output_a: generate_funding_txout( amount_adjusted_with_p2wpkh_fee + 1, @@ -2180,6 +2453,7 @@ mod tests { ), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(amount_adjusted_with_p2wpkh_fee + 1, 0), outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)), @@ -2187,12 +2461,14 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Single contribution with P2WPKH input, sufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout( amount_adjusted_with_p2wpkh_fee, amount_adjusted_with_p2wpkh_fee, ), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(amount_adjusted_with_p2wpkh_fee, 0), outputs_b: vec![], expect_error: None, @@ -2203,6 +2479,7 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Single contribution, with P2WSH input, insufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WSH(1_000_000)]), + a_shared_input: None, // makes initiator inputs insufficient to cover fees shared_output_a: generate_funding_txout( amount_adjusted_with_p2wsh_fee + 1, @@ -2210,6 +2487,7 @@ mod tests { ), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(amount_adjusted_with_p2wsh_fee + 1, 0), outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)), @@ -2217,12 +2495,14 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Single contribution with P2WSH input, sufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WSH(1_000_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout( amount_adjusted_with_p2wsh_fee, amount_adjusted_with_p2wsh_fee, ), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(amount_adjusted_with_p2wsh_fee, 0), outputs_b: vec![], expect_error: None, @@ -2233,6 +2513,7 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Single contribution, with P2TR input, insufficient fees", inputs_a: generate_inputs(&[TestOutput::P2TR(1_000_000)]), + a_shared_input: None, // makes initiator inputs insufficient to cover fees shared_output_a: generate_funding_txout( amount_adjusted_with_p2tr_fee + 1, @@ -2240,6 +2521,7 @@ mod tests { ), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(amount_adjusted_with_p2tr_fee + 1, 0), outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)), @@ -2247,12 +2529,14 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Single contribution with P2TR input, sufficient fees", inputs_a: generate_inputs(&[TestOutput::P2TR(1_000_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout( amount_adjusted_with_p2tr_fee, amount_adjusted_with_p2tr_fee, ), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(amount_adjusted_with_p2tr_fee, 0), outputs_b: vec![], expect_error: None, @@ -2260,9 +2544,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Initiator contributes sufficient fees, but non-initiator does not", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(100_000, 0), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(100_000)]), + b_shared_input: None, shared_output_b: generate_funding_txout(100_000, 100_000), outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeB)), @@ -2270,12 +2556,14 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Multi-input-output contributions from both sides", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000); 2]), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 200_000), outputs_a: vec![generate_output_nonfunding_one(&TestOutput::P2WPKH(200_000))], inputs_b: generate_inputs(&[ TestOutput::P2WPKH(1_000_000), TestOutput::P2WPKH(500_000), ]), + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 800_000), outputs_b: vec![generate_output_nonfunding_one(&TestOutput::P2WPKH(400_000))], expect_error: None, @@ -2284,9 +2572,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Prevout from initiator is not a witness program", inputs_a: generate_inputs(&[TestOutput::P2PKH(1_000_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)), @@ -2301,9 +2591,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Invalid input sequence from initiator", inputs_a: vec![(invalid_sequence_input, tx.clone())], + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::IncorrectInputSequenceValue, ErrorCulprit::NodeA)), @@ -2316,9 +2608,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Duplicate prevout from initiator", inputs_a: vec![(duplicate_input.clone(), tx.clone()), (duplicate_input, tx.clone())], + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeB)), @@ -2332,9 +2626,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Non-initiator uses same prevout as initiator", inputs_a: vec![(duplicate_input.clone(), tx.clone())], + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 905_000), outputs_a: vec![], inputs_b: vec![(duplicate_input.clone(), tx.clone())], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 95_000), outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)), @@ -2347,9 +2643,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Non-initiator uses same prevout as initiator", inputs_a: vec![(duplicate_input.clone(), tx.clone())], + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![(duplicate_input.clone(), tx.clone())], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)), @@ -2357,9 +2655,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Initiator sends too many TxAddInputs", inputs_a: generate_fixed_number_of_inputs(MAX_RECEIVED_TX_ADD_INPUT_COUNT + 1), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::ReceivedTooManyTxAddInputs, ErrorCulprit::NodeA)), @@ -2369,9 +2669,11 @@ mod tests { // We use a deliberately bad entropy source, `DuplicateEntropySource` to simulate this. description: "Attempt to queue up two inputs with duplicate serial ids", inputs_a: generate_fixed_number_of_inputs(2), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::DuplicateSerialId, ErrorCulprit::NodeA)), @@ -2381,9 +2683,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Initiator sends too many TxAddOutputs", inputs_a: vec![], + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: generate_fixed_number_of_outputs(MAX_RECEIVED_TX_ADD_OUTPUT_COUNT), inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::ReceivedTooManyTxAddOutputs, ErrorCulprit::NodeA)), @@ -2392,9 +2696,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Initiator sends an output below dust value", inputs_a: vec![], + a_shared_input: None, shared_output_a: generate_funding_txout(dust_amount, dust_amount), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(dust_amount, 0), outputs_b: vec![], expect_error: Some((AbortReason::BelowDustLimit, ErrorCulprit::NodeA)), @@ -2402,12 +2708,14 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Initiator sends an output above maximum sats allowed", inputs_a: vec![], + a_shared_input: None, shared_output_a: generate_funding_txout( TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1, TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1, ), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(TOTAL_BITCOIN_SUPPLY_SATOSHIS + 1, 0), outputs_b: vec![], expect_error: Some((AbortReason::ExceededMaximumSatsAllowed, ErrorCulprit::NodeA)), @@ -2415,9 +2723,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Initiator sends an output without a witness program", inputs_a: vec![], + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![generate_non_witness_output(1_000_000)], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::InvalidOutputScript, ErrorCulprit::NodeA)), @@ -2427,9 +2737,11 @@ mod tests { // We use a deliberately bad entropy source, `DuplicateEntropySource` to simulate this. description: "Attempt to queue up two outputs with duplicate serial ids", inputs_a: vec![], + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: generate_fixed_number_of_outputs(2), inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::DuplicateSerialId, ErrorCulprit::NodeA)), @@ -2440,9 +2752,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Peer contributed more output value than inputs", inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)), @@ -2451,9 +2765,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Peer contributed more than allowed number of inputs", inputs_a: generate_fixed_number_of_inputs(MAX_INPUTS_OUTPUTS_COUNT as u16 + 1), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: vec![], inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some(( @@ -2464,9 +2780,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Peer contributed more than allowed number of outputs", inputs_a: generate_inputs(&[TestOutput::P2WPKH(TOTAL_BITCOIN_SUPPLY_SATOSHIS)]), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 1_000_000), outputs_a: generate_fixed_number_of_outputs(MAX_INPUTS_OUTPUTS_COUNT as u16), inputs_b: vec![], + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 0), outputs_b: vec![], expect_error: Some(( @@ -2479,9 +2797,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Funding output by us, small contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(12_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 10_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(992_000)]), + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 990_000), outputs_b: vec![], expect_error: None, @@ -2491,9 +2811,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Funding output by them, small contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(12_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 10_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(992_000)]), + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 990_000), outputs_b: vec![], expect_error: None, @@ -2503,9 +2825,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Funding output by us, large contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(992_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 990_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(12_000)]), + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 10_000), outputs_b: vec![], expect_error: None, @@ -2515,9 +2839,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Funding output by them, large contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(992_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(1_000_000, 990_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(12_000)]), + b_shared_input: None, shared_output_b: generate_funding_txout(1_000_000, 10_000), outputs_b: vec![], expect_error: None, @@ -2529,9 +2855,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Splice out with sufficient initiator balance", inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000), TestOutput::P2WPKH(50_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(120_000, 120_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(50_000)]), + b_shared_input: None, shared_output_b: generate_funding_txout(120_000, 0), outputs_b: vec![], expect_error: None, @@ -2543,9 +2871,11 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Splice out with insufficient initiator balance", inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000), TestOutput::P2WPKH(15_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(120_000, 120_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(85_000)]), + b_shared_input: None, shared_output_b: generate_funding_txout(120_000, 0), outputs_b: vec![], expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)), @@ -2555,13 +2885,57 @@ mod tests { do_test_interactive_tx_constructor(TestSession { description: "Splice in, invalid intended local contribution", inputs_a: generate_inputs(&[TestOutput::P2WPKH(100_000), TestOutput::P2WPKH(15_000)]), + a_shared_input: None, shared_output_a: generate_funding_txout(100_000, 100_000), outputs_a: vec![], inputs_b: generate_inputs(&[TestOutput::P2WPKH(85_000)]), + b_shared_input: None, shared_output_b: generate_funding_txout(120_000, 0), // value different outputs_b: vec![], expect_error: Some((AbortReason::MissingFundingOutput, ErrorCulprit::NodeA)), }); + + // Provide and expect a shared input + do_test_interactive_tx_constructor(TestSession { + description: "Provide and expect a shared input", + inputs_a: generate_inputs(&[TestOutput::P2WPKH(50_000)]), + a_shared_input: Some(generate_shared_input(&prev_funding_tx_1, 0, 60_000)), + shared_output_a: generate_funding_txout(108_000, 108_000), + outputs_a: vec![], + inputs_b: vec![], + b_shared_input: Some(generate_shared_input(&prev_funding_tx_1, 0, 0)), + shared_output_b: generate_funding_txout(108_000, 0), + outputs_b: vec![], + expect_error: None, + }); + + // Expect a shared input, but it's missing + do_test_interactive_tx_constructor(TestSession { + description: "Expect a shared input, but it's missing", + inputs_a: generate_inputs(&[TestOutput::P2WPKH(110_000)]), + a_shared_input: None, + shared_output_a: generate_funding_txout(108_000, 108_000), + outputs_a: vec![], + inputs_b: vec![], + b_shared_input: Some(generate_shared_input(&prev_funding_tx_1, 0, 0)), + shared_output_b: generate_funding_txout(108_000, 0), + outputs_b: vec![], + expect_error: Some((AbortReason::MissingFundingInput, ErrorCulprit::NodeA)), + }); + + // Provide a shared input, but it's not expected + do_test_interactive_tx_constructor(TestSession { + description: "Provide a shared input, but it's not expected", + inputs_a: generate_inputs(&[TestOutput::P2WPKH(50_000)]), + a_shared_input: Some(generate_shared_input(&prev_funding_tx_1, 0, 60_000)), + shared_output_a: generate_funding_txout(108_000, 108_000), + outputs_a: vec![], + inputs_b: vec![], + b_shared_input: None, + shared_output_b: generate_funding_txout(108_000, 0), + outputs_b: vec![], + expect_error: Some((AbortReason::UnexpectedFundingInput, ErrorCulprit::NodeA)), + }); } #[test] @@ -2613,8 +2987,9 @@ mod tests { let res = calculate_change_output_value( true, our_contributed, - &ScriptBuf::new(), &inputs, + None, + &ScriptBuf::new(), &outputs, funding_feerate_sat_per_1000_weight, 300, @@ -2626,8 +3001,9 @@ mod tests { let res = calculate_change_output_value( false, our_contributed, - &ScriptBuf::new(), &inputs, + None, + &ScriptBuf::new(), &outputs, funding_feerate_sat_per_1000_weight, 300, @@ -2639,8 +3015,9 @@ mod tests { let res = calculate_change_output_value( true, our_contributed, - &ScriptBuf::new(), &inputs, + None, + &ScriptBuf::new(), &outputs, funding_feerate_sat_per_1000_weight * 3, 300, @@ -2652,8 +3029,9 @@ mod tests { let res = calculate_change_output_value( false, 130_000, - &ScriptBuf::new(), &inputs, + None, + &ScriptBuf::new(), &outputs, funding_feerate_sat_per_1000_weight, 300, @@ -2665,8 +3043,9 @@ mod tests { let res = calculate_change_output_value( false, 118_000, - &ScriptBuf::new(), &inputs, + None, + &ScriptBuf::new(), &outputs, funding_feerate_sat_per_1000_weight, 300, @@ -2678,8 +3057,9 @@ mod tests { let res = calculate_change_output_value( false, 117_992, - &ScriptBuf::new(), &inputs, + None, + &ScriptBuf::new(), &outputs, funding_feerate_sat_per_1000_weight, 100, diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 0379397e2fc..42b51bab27f 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -525,9 +525,9 @@ pub struct TxAddInput { /// A randomly chosen unique identifier for this input, which is even for initiators and odd for /// non-initiators. pub serial_id: SerialId, - /// Serialized transaction that contains the output this input spends to verify that it is non - /// malleable. - pub prevtx: TransactionU16LenLimited, + /// Serialized transaction that contains the output this input spends to verify that it is + /// non-malleable. Omitted for shared input. + pub prevtx: Option, /// The index of the output being spent pub prevtx_out: u32, /// The sequence number of this input @@ -5292,7 +5292,7 @@ mod tests { let tx_add_input = msgs::TxAddInput { channel_id: ChannelId::from_bytes([2; 32]), serial_id: 4886718345, - prevtx: TransactionU16LenLimited::new(Transaction { + prevtx: Some(TransactionU16LenLimited::new(Transaction { version: Version::TWO, lock_time: LockTime::ZERO, input: vec![TxIn { @@ -5313,13 +5313,31 @@ mod tests { script_pubkey: Address::from_str("bc1qxmk834g5marzm227dgqvynd23y2nvt2ztwcw2z").unwrap().assume_checked().script_pubkey(), }, ], - }).unwrap(), + }).unwrap()), prevtx_out: 305419896, sequence: 305419896, - shared_input_txid: Some(Txid::from_str("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap()), + shared_input_txid: None, }; let encoded_value = tx_add_input.encode(); - let target_value = "0202020202020202020202020202020202020202020202020202020202020202000000012345678900de02000000000101779ced6c148293f86b60cb222108553d22c89207326bb7b6b897e23e64ab5b300200000000fdffffff0236dbc1000000000016001417d29e4dd454bac3b1cde50d1926da80cfc5287b9cbd03000000000016001436ec78d514df462da95e6a00c24daa8915362d420247304402206af85b7dd67450ad12c979302fac49dfacbc6a8620f49c5da2b5721cf9565ca502207002b32fed9ce1bf095f57aeb10c36928ac60b12e723d97d2964a54640ceefa701210301ab7dc16488303549bfcdd80f6ae5ee4c20bf97ab5410bbd6b1bfa85dcd694400000000123456781234567800206e96fe9f8b0ddcd729ba03cfafa5a27b050b39d354dd980814268dfa9a44d4c2"; + let target_value = "0202020202020202020202020202020202020202020202020202020202020202000000012345678900de02000000000101779ced6c148293f86b60cb222108553d22c89207326bb7b6b897e23e64ab5b300200000000fdffffff0236dbc1000000000016001417d29e4dd454bac3b1cde50d1926da80cfc5287b9cbd03000000000016001436ec78d514df462da95e6a00c24daa8915362d420247304402206af85b7dd67450ad12c979302fac49dfacbc6a8620f49c5da2b5721cf9565ca502207002b32fed9ce1bf095f57aeb10c36928ac60b12e723d97d2964a54640ceefa701210301ab7dc16488303549bfcdd80f6ae5ee4c20bf97ab5410bbd6b1bfa85dcd6944000000001234567812345678"; + assert_eq!(encoded_value.as_hex().to_string(), target_value); + } + + #[test] + fn encoding_tx_add_input_shared() { + let tx_add_input = msgs::TxAddInput { + channel_id: ChannelId::from_bytes([2; 32]), + serial_id: 4886718345, + prevtx: None, + prevtx_out: 305419896, + sequence: 305419896, + shared_input_txid: Some( + Txid::from_str("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e") + .unwrap(), + ), + }; + let encoded_value = tx_add_input.encode(); + let target_value = "020202020202020202020202020202020202020202020202020202020202020200000001234567890000123456781234567800206e96fe9f8b0ddcd729ba03cfafa5a27b050b39d354dd980814268dfa9a44d4c2"; assert_eq!(encoded_value.as_hex().to_string(), target_value); } diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 737a558946e..9069f333bf4 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -15,7 +15,7 @@ use crate::io::{self, BufRead, Read, Write}; use crate::io_extras::{copy, sink}; -use crate::ln::interactivetxs::{InteractiveTxInput, InteractiveTxOutput}; +use crate::ln::interactivetxs::{InteractiveTxOutput, NegotiatedTxInput}; use crate::ln::onion_utils::{HMAC_COUNT, HMAC_LEN, HOLD_TIME_LEN, MAX_HOPS}; use crate::prelude::*; use crate::sync::{Mutex, RwLock}; @@ -41,7 +41,7 @@ use bitcoin::secp256k1::ecdsa; use bitcoin::secp256k1::schnorr; use bitcoin::secp256k1::{PublicKey, SecretKey}; use bitcoin::transaction::{OutPoint, Transaction, TxOut}; -use bitcoin::{consensus, TxIn, Witness}; +use bitcoin::{consensus, TxIn, Weight, Witness}; use dnssec_prover::rr::Name; @@ -1082,7 +1082,7 @@ impl_for_vec!(crate::ln::channelmanager::PaymentClaimDetails); impl_for_vec!(crate::ln::msgs::SocketAddress); impl_for_vec!((A, B), A, B); impl_for_vec!(SerialId); -impl_for_vec!(InteractiveTxInput); +impl_for_vec!(NegotiatedTxInput); impl_for_vec!(InteractiveTxOutput); impl_writeable_for_vec!(&crate::routing::router::BlindedTail); impl_readable_for_vec!(crate::routing::router::BlindedTail); @@ -1381,6 +1381,19 @@ impl Readable for Amount { } } +impl Writeable for Weight { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.to_wu().write(w) + } +} + +impl Readable for Weight { + fn read(r: &mut R) -> Result { + let wu: u64 = Readable::read(r)?; + Ok(Weight::from_wu(wu)) + } +} + impl Writeable for Txid { fn write(&self, w: &mut W) -> Result<(), io::Error> { w.write_all(&self[..]) @@ -1691,22 +1704,30 @@ impl TransactionU16LenLimited { } } -impl Writeable for TransactionU16LenLimited { +impl Writeable for Option { fn write(&self, w: &mut W) -> Result<(), io::Error> { - (self.0.serialized_length() as u16).write(w)?; - self.0.write(w) + match self { + Some(tx) => { + (tx.0.serialized_length() as u16).write(w)?; + tx.0.write(w) + }, + None => 0u16.write(w), + } } } -impl Readable for TransactionU16LenLimited { +impl Readable for Option { fn read(r: &mut R) -> Result { let len = ::read(r)?; + if len == 0 { + return Ok(None); + } let mut tx_reader = FixedLengthReader::new(r, len as u64); let tx: Transaction = Readable::read(&mut tx_reader)?; if tx_reader.bytes_remain() { Err(DecodeError::BadLengthDescriptor) } else { - Ok(Self(tx)) + Ok(Some(TransactionU16LenLimited(tx))) } } } From cd2ffafd69e988aed40ca539e02b38a5e8038190 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 14 Jul 2025 18:46:39 -0500 Subject: [PATCH 3/3] Fix rounding bug when checking counterparty fees Instead of calculating the fee for the entire weight contributed by the counterparty, each portion of the weight was calculated individually before summed. That caused a rounding error that is avoided by summing the weights first. --- lightning/src/ln/interactivetxs.rs | 42 ++++++++++++++---------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 66ee00c12a5..f4252445a3e 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -915,20 +915,18 @@ impl NegotiationContext { fn check_counterparty_fees( &self, counterparty_fees_contributed: u64, ) -> Result<(), AbortReason> { - let counterparty_weight_contributed = self + let mut counterparty_weight_contributed = self .remote_inputs_weight() .to_wu() .saturating_add(self.remote_outputs_weight().to_wu()); - 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_fee = - fee_for_weight(self.feerate_sat_per_kw, TX_COMMON_FIELDS_WEIGHT); - required_counterparty_contribution_fee += tx_common_fields_fee; + 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); } @@ -2432,16 +2430,12 @@ mod tests { outputs_b: vec![], expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)), }); - let p2wpkh_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, P2WPKH_INPUT_WEIGHT_LOWER_BOUND); - let outputs_fee = fee_for_weight( - TEST_FEERATE_SATS_PER_KW, - get_output_weight(&generate_p2wsh_script_pubkey()).to_wu(), - ); - let tx_common_fields_fee = - fee_for_weight(TEST_FEERATE_SATS_PER_KW, TX_COMMON_FIELDS_WEIGHT); - - let amount_adjusted_with_p2wpkh_fee = - 1_000_000 - p2wpkh_fee - outputs_fee - tx_common_fields_fee + 1; + let outputs_weight = get_output_weight(&generate_p2wsh_script_pubkey()).to_wu(); + let amount_adjusted_with_p2wpkh_fee = 1_000_000 + - fee_for_weight( + TEST_FEERATE_SATS_PER_KW, + P2WPKH_INPUT_WEIGHT_LOWER_BOUND + TX_COMMON_FIELDS_WEIGHT + outputs_weight, + ); do_test_interactive_tx_constructor(TestSession { description: "Single contribution, with P2WPKH input, insufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WPKH(1_000_000)]), @@ -2473,9 +2467,11 @@ mod tests { outputs_b: vec![], expect_error: None, }); - let p2wsh_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, P2WSH_INPUT_WEIGHT_LOWER_BOUND); - let amount_adjusted_with_p2wsh_fee = - 1_000_000 - p2wsh_fee - outputs_fee - tx_common_fields_fee + 1; + let amount_adjusted_with_p2wsh_fee = 1_000_000 + - fee_for_weight( + TEST_FEERATE_SATS_PER_KW, + P2WSH_INPUT_WEIGHT_LOWER_BOUND + TX_COMMON_FIELDS_WEIGHT + outputs_weight, + ); do_test_interactive_tx_constructor(TestSession { description: "Single contribution, with P2WSH input, insufficient fees", inputs_a: generate_inputs(&[TestOutput::P2WSH(1_000_000)]), @@ -2507,9 +2503,11 @@ mod tests { outputs_b: vec![], expect_error: None, }); - let p2tr_fee = fee_for_weight(TEST_FEERATE_SATS_PER_KW, P2TR_INPUT_WEIGHT_LOWER_BOUND); - let amount_adjusted_with_p2tr_fee = - 1_000_000 - p2tr_fee - outputs_fee - tx_common_fields_fee; + let amount_adjusted_with_p2tr_fee = 1_000_000 + - fee_for_weight( + TEST_FEERATE_SATS_PER_KW, + P2TR_INPUT_WEIGHT_LOWER_BOUND + TX_COMMON_FIELDS_WEIGHT + outputs_weight, + ); do_test_interactive_tx_constructor(TestSession { description: "Single contribution, with P2TR input, insufficient fees", inputs_a: generate_inputs(&[TestOutput::P2TR(1_000_000)]),