From a379f1dbed12f4e4f9a8c884e49293a43704fc94 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Wed, 11 Dec 2024 20:37:18 +0100 Subject: [PATCH] Minor changes post review --- lightning/src/ln/channel.rs | 54 +++++++++++++----------------- lightning/src/ln/interactivetxs.rs | 8 +++-- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f370f158506..8ec91510e52 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1696,26 +1696,27 @@ pub(super) trait InteractivelyFunded where SP::Target: SignerProvider ) -> Result, APIError> where ES::Target: EntropySource { - let mut funding_inputs = self.dual_funding_context_mut().our_funding_inputs.take().unwrap_or_else(|| vec![]); + let mut funding_inputs = Vec::new(); + mem::swap(&mut self.dual_funding_context_mut().our_funding_inputs, &mut funding_inputs); if let Some(prev_funding_input) = prev_funding_input { funding_inputs.push(prev_funding_input); } - let mut funding_inputs_prev_outputs: Vec = Vec::with_capacity(funding_inputs.len()); + let mut funding_inputs_prev_outputs: Vec<&TxOut> = Vec::with_capacity(funding_inputs.len()); // Check that vouts exist for each TxIn in provided transactions. - for (idx, input) in funding_inputs.iter().enumerate() { - if let Some(output) = input.1.as_transaction().output.get(input.0.previous_output.vout as usize) { - funding_inputs_prev_outputs.push(output.clone()); + for (idx, (txin, tx)) in funding_inputs.iter().enumerate() { + if let Some(output) = tx.as_transaction().output.get(txin.previous_output.vout as usize) { + funding_inputs_prev_outputs.push(output); } else { return Err(APIError::APIMisuseError { err: format!("Transaction with txid {} does not have an output with vout of {} corresponding to TxIn at funding_inputs[{}]", - input.1.as_transaction().compute_txid(), input.0.previous_output.vout, idx) }); + tx.as_transaction().compute_txid(), txin.previous_output.vout, idx) }); } } let total_input_satoshis: u64 = funding_inputs.iter().map( - |input| input.1.as_transaction().output.get(input.0.previous_output.vout as usize).map(|out| out.value.to_sat()).unwrap_or(0) + |(txin, tx)| tx.as_transaction().output.get(txin.previous_output.vout as usize).map(|out| out.value.to_sat()).unwrap_or(0) ).sum(); if total_input_satoshis < self.dual_funding_context().our_funding_satoshis { return Err(APIError::APIMisuseError { @@ -1757,8 +1758,14 @@ pub(super) trait InteractivelyFunded where SP::Target: SignerProvider |err| APIError::APIMisuseError { err: format!("Failed to get change script as new destination script, {:?}", err), })?; - add_funding_change_output(change_value, change_script, - &mut funding_outputs, self.dual_funding_context().funding_feerate_sat_per_1000_weight); + let mut change_output = TxOut { + value: Amount::from_sat(change_value), + script_pubkey: change_script, + }; + let change_output_weight = get_output_weight(&change_output.script_pubkey).to_wu(); + let change_output_fee = fee_for_weight(self.dual_funding_context().funding_feerate_sat_per_1000_weight, change_output_weight); + change_output.value = Amount::from_sat(change_value.saturating_sub(change_output_fee)); + funding_outputs.push(OutputOwned::Single(change_output)); } let constructor_args = InteractiveTxConstructorArgs { @@ -1766,9 +1773,9 @@ pub(super) trait InteractivelyFunded where SP::Target: SignerProvider holder_node_id, counterparty_node_id: self.context().counterparty_node_id, channel_id: self.context().channel_id(), - feerate_sat_per_kw: self.dual_funding_context_mut().funding_feerate_sat_per_1000_weight, + feerate_sat_per_kw: self.dual_funding_context().funding_feerate_sat_per_1000_weight, is_initiator: self.is_initiator(), - funding_tx_locktime: self.dual_funding_context_mut().funding_tx_locktime, + funding_tx_locktime: self.dual_funding_context().funding_tx_locktime, inputs_to_contribute: funding_inputs, outputs_to_contribute: funding_outputs, expected_remote_shared_funding_output, @@ -1777,7 +1784,7 @@ pub(super) trait InteractivelyFunded where SP::Target: SignerProvider .map_err(|_| APIError::APIMisuseError { err: "Incorrect shared output provided".into() })?; let msg = tx_constructor.take_initiator_first_message(); - self.interactive_tx_constructor_mut().replace(tx_constructor); + *self.interactive_tx_constructor_mut() = Some(tx_constructor); Ok(msg) } @@ -4264,21 +4271,6 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos cmp::min(channel_value_satoshis, cmp::max(q, dust_limit_satoshis)) } -#[allow(dead_code)] // TODO(dual_funding): Remove once begin_interactive_funding_tx_construction() is used -fn add_funding_change_output( - change_value: u64, change_script: ScriptBuf, - funding_outputs: &mut Vec, funding_feerate_sat_per_1000_weight: u32, -) { - let mut change_output = TxOut { - value: Amount::from_sat(change_value), - script_pubkey: change_script, - }; - let change_output_weight = get_output_weight(&change_output.script_pubkey).to_wu(); - let change_output_fee = fee_for_weight(funding_feerate_sat_per_1000_weight, change_output_weight); - change_output.value = Amount::from_sat(change_value.saturating_sub(change_output_fee)); - funding_outputs.push(OutputOwned::Single(change_output.clone())); -} - pub(super) fn calculate_our_funding_satoshis( is_initiator: bool, funding_inputs: &[(TxIn, TransactionU16LenLimited)], total_witness_weight: Weight, funding_feerate_sat_per_1000_weight: u32, @@ -4337,8 +4329,10 @@ pub(super) struct DualFundingChannelContext { /// Note that the `our_funding_satoshis` field is equal to the total value of `our_funding_inputs` /// minus any fees paid for our contributed weight. This means that change will never be generated /// and the maximum value possible will go towards funding the channel. + /// + /// Note that this field may be emptied once the interactive negotiation has been started. #[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled. - pub our_funding_inputs: Option>, + pub our_funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>, } // Holder designates channel data owned for the benefit of the user client. @@ -8920,7 +8914,7 @@ impl OutboundV2Channel where SP::Target: SignerProvider { their_funding_satoshis: None, funding_tx_locktime, funding_feerate_sat_per_1000_weight, - our_funding_inputs: Some(funding_inputs), + our_funding_inputs: funding_inputs, }, interactive_tx_constructor: None, }; @@ -9084,7 +9078,7 @@ impl InboundV2Channel where SP::Target: SignerProvider { their_funding_satoshis: Some(msg.common_fields.funding_satoshis), funding_tx_locktime: LockTime::from_consensus(msg.locktime), funding_feerate_sat_per_1000_weight: msg.funding_feerate_sat_per_1000_weight, - our_funding_inputs: Some(funding_inputs.clone()), + our_funding_inputs: funding_inputs.clone(), }; let interactive_tx_constructor = Some(InteractiveTxConstructor::new( diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 63ca011de7f..accd2ad4c53 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -1669,7 +1669,7 @@ impl InteractiveTxConstructor { /// or None if a change is not needed/possible. #[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_prev_outputs: &Vec, + is_initiator: bool, our_contribution: u64, funding_inputs_prev_outputs: &Vec<&TxOut>, funding_outputs: &Vec, funding_feerate_sat_per_1000_weight: u32, holder_dust_limit_satoshis: u64, ) -> Option { @@ -2640,10 +2640,11 @@ mod tests { #[test] fn test_calculate_change_output_value_open() { - let input_prevouts = vec![ + let input_prevouts_owned = vec![ TxOut { value: Amount::from_sat(70_000), script_pubkey: ScriptBuf::new() }, TxOut { value: Amount::from_sat(60_000), script_pubkey: ScriptBuf::new() }, ]; + let input_prevouts: Vec<&TxOut> = input_prevouts_owned.iter().collect(); let our_contributed = 110_000; let txout = TxOut { value: Amount::from_sat(128_000), script_pubkey: ScriptBuf::new() }; let outputs = vec![OutputOwned::SharedControlFullyOwned(txout)]; @@ -2729,10 +2730,11 @@ mod tests { #[test] fn test_calculate_change_output_value_splice() { - let input_prevouts = vec![ + let input_prevouts_owned = vec![ TxOut { value: Amount::from_sat(70_000), script_pubkey: ScriptBuf::new() }, TxOut { value: Amount::from_sat(60_000), script_pubkey: ScriptBuf::new() }, ]; + let input_prevouts: Vec<&TxOut> = input_prevouts_owned.iter().collect(); let our_contributed = 110_000; let txout = TxOut { value: Amount::from_sat(148_000), script_pubkey: ScriptBuf::new() }; let outputs = vec![OutputOwned::Shared(SharedOwnedOutput::new(txout, our_contributed))];