From 9e44d2b8723dbbe83a66f6b53f36711e81e1c972 Mon Sep 17 00:00:00 2001 From: optout <13562139+optout21@users.noreply.github.com> Date: Fri, 6 Dec 2024 17:40:43 +0100 Subject: [PATCH] Rename method, review --- lightning/src/ln/channel.rs | 11 +++++------ lightning/src/ln/interactivetxs.rs | 28 +++++++++++++++------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index fe84e4ebee6..57d70a5f2f7 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -31,7 +31,7 @@ use crate::ln::types::ChannelId; use crate::types::payment::{PaymentPreimage, PaymentHash}; use crate::types::features::{ChannelTypeFeatures, InitFeatures}; use crate::ln::interactivetxs::{ - get_output_weight, need_to_add_funding_change_output, HandleTxCompleteValue, HandleTxCompleteResult, InteractiveTxConstructor, + get_output_weight, calculate_change_output_value, HandleTxCompleteValue, HandleTxCompleteResult, InteractiveTxConstructor, InteractiveTxConstructorArgs, InteractiveTxMessageSend, InteractiveTxSigningSession, InteractiveTxMessageSendResult, OutputOwned, SharedOwnedOutput, TX_COMMON_FIELDS_WEIGHT, }; @@ -1747,7 +1747,7 @@ pub(super) trait InteractivelyFunded where SP::Target: SignerProvider }; // Optionally add change output - if let Some(change_value) = need_to_add_funding_change_output( + if let Some(change_value) = calculate_change_output_value( self.is_initiator(), self.dual_funding_context().our_funding_satoshis, &funding_inputs_prev_outputs, &funding_outputs, self.dual_funding_context().funding_feerate_sat_per_1000_weight, @@ -1757,8 +1757,8 @@ pub(super) trait InteractivelyFunded where SP::Target: SignerProvider |err| APIError::APIMisuseError { err: format!("Failed to get change script as new destination script, {:?}", err), })?; - let _res = add_funding_change_output( - change_value, change_script, &mut funding_outputs, self.dual_funding_context().funding_feerate_sat_per_1000_weight); + add_funding_change_output(change_value, change_script, + &mut funding_outputs, self.dual_funding_context().funding_feerate_sat_per_1000_weight); } let constructor_args = InteractiveTxConstructorArgs { @@ -4268,7 +4268,7 @@ fn get_v2_channel_reserve_satoshis(channel_value_satoshis: u64, dust_limit_satos fn add_funding_change_output( change_value: u64, change_script: ScriptBuf, funding_outputs: &mut Vec, funding_feerate_sat_per_1000_weight: u32, -) -> TxOut { +) { let mut change_output = TxOut { value: Amount::from_sat(change_value), script_pubkey: change_script, @@ -4277,7 +4277,6 @@ fn add_funding_change_output( 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())); - change_output } pub(super) fn calculate_our_funding_satoshis( diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 32536f050af..63ca011de7f 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -1665,8 +1665,10 @@ impl InteractiveTxConstructor { /// Determine whether a change output should be added or not, and if so, of what size, /// considering our given inputs, outputs, and intended contribution. /// Computes and takes into account fees. +/// Return value is the value computed for the change output (in satoshis), +/// 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 need_to_add_funding_change_output( +pub(super) fn calculate_change_output_value( is_initiator: bool, our_contribution: u64, funding_inputs_prev_outputs: &Vec, funding_outputs: &Vec, funding_feerate_sat_per_1000_weight: u32, holder_dust_limit_satoshis: u64, @@ -1707,7 +1709,7 @@ mod tests { use crate::chain::chaininterface::{fee_for_weight, FEERATE_FLOOR_SATS_PER_KW}; use crate::ln::channel::TOTAL_BITCOIN_SUPPLY_SATOSHIS; use crate::ln::interactivetxs::{ - generate_holder_serial_id, need_to_add_funding_change_output, AbortReason, + 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, @@ -2637,7 +2639,7 @@ mod tests { } #[test] - fn test_need_to_add_funding_change_output_open() { + fn test_calculate_change_output_value_open() { let input_prevouts = vec![ TxOut { value: Amount::from_sat(70_000), script_pubkey: ScriptBuf::new() }, TxOut { value: Amount::from_sat(60_000), script_pubkey: ScriptBuf::new() }, @@ -2653,7 +2655,7 @@ mod tests { let common_fees = 126; { // There is leftover for change - let res = need_to_add_funding_change_output( + let res = calculate_change_output_value( true, our_contributed, &input_prevouts, @@ -2665,7 +2667,7 @@ mod tests { } { // There is leftover for change, without common fees - let res = need_to_add_funding_change_output( + let res = calculate_change_output_value( false, our_contributed, &input_prevouts, @@ -2677,7 +2679,7 @@ mod tests { } { // Larger fee, smaller change - let res = need_to_add_funding_change_output( + let res = calculate_change_output_value( true, our_contributed, &input_prevouts, @@ -2689,7 +2691,7 @@ mod tests { } { // Insufficient inputs, no leftover - let res = need_to_add_funding_change_output( + let res = calculate_change_output_value( false, 130_000, &input_prevouts, @@ -2701,7 +2703,7 @@ mod tests { } { // Very small leftover - let res = need_to_add_funding_change_output( + let res = calculate_change_output_value( false, 128_100, &input_prevouts, @@ -2713,7 +2715,7 @@ mod tests { } { // Small leftover, but not dust - let res = need_to_add_funding_change_output( + let res = calculate_change_output_value( false, 128_100, &input_prevouts, @@ -2726,7 +2728,7 @@ mod tests { } #[test] - fn test_need_to_add_funding_change_output_splice() { + fn test_calculate_change_output_value_splice() { let input_prevouts = vec![ TxOut { value: Amount::from_sat(70_000), script_pubkey: ScriptBuf::new() }, TxOut { value: Amount::from_sat(60_000), script_pubkey: ScriptBuf::new() }, @@ -2742,7 +2744,7 @@ mod tests { let common_fees = 126; { // There is leftover for change - let res = need_to_add_funding_change_output( + let res = calculate_change_output_value( true, our_contributed, &input_prevouts, @@ -2754,7 +2756,7 @@ mod tests { } { // Very small leftover - let res = need_to_add_funding_change_output( + let res = calculate_change_output_value( false, 128_100, &input_prevouts, @@ -2766,7 +2768,7 @@ mod tests { } { // Small leftover, but not dust - let res = need_to_add_funding_change_output( + let res = calculate_change_output_value( false, 128_100, &input_prevouts,