Derive FundingContribution::net_value implicitly
What changed, and why it matters
This commit refactors how a Lightning channel funding contribution calculates the amount being added to or removed from a channel. Instead of storing a separate 'value_added' field, the code now derives it from the inputs, outputs, change, and fees. The change removes an explicit input-sufficiency check and rewrites fee-buffer logic. It appears to be a code-cleanup and correctness refactor rather than a clear security fix, but it alters validation boundaries around fees and change in splicing transactions.
Review the new derivation for off-by-one/fee-deduction errors, ensure the removed input-sufficiency check is compensated elsewhere in the interactive transaction protocol, and verify TLV backward compatibility/upgrade behavior for serialized FundingContribution objects.
Security signals we found
Removed explicit input-sufficiency validation in FundingContribution::validate()
Changed fee-buffer surplus accounting from stored value to derived value
TLV field renumbering for FundingContribution serialization
Refactored net_value and value_added calculations for splicing/funding contributions
Fuzz corpus updated for changed splice funding tx values
Evidence from the diff
The patch removes the stored value_added field from FundingContribution and derives value_added() and net_value() from inputs, outputs, change_output, and estimated_fee. It renumbers the TLV serialization, removes the validate() input-sufficiency check for splice-in contributions, and changes the fee-buffer/surplus accounting so that surplus no longer mutates a stored value_added. Fuzz and unit tests are updated to match new derived values and txids. The change touches consensus-adjacent accounting for interactive transaction construction and splicing.
Changed components
lightning/src/ln/funding.rslightning/src/ln/splicing_tests.rsfuzz/src/full_stack.rsInspect captured patch +98 / −398
diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
index f300ded..1f1cf42 100644
--- a/fuzz/src/full_stack.rs
+++ b/fuzz/src/full_stack.rs
@@ -1886,8 +1886,8 @@ fn splice_seed() -> Vec<u8> {
// CommitmentSigned message with proper signature (r=f7, s=01...) and funding_txid TLV
// signature r encodes sighash first byte f7, s follows the pattern from funding_created
// TLV type 1 (odd/optional) for funding_txid as per impl_writeable_msg!(CommitmentSigned, ...)
- // Note: txid is encoded in reverse byte order (Bitcoin standard), so to get display 0000...0033, encode 3300...0000
- ext_from_hex("0084 c000000000000000000000000000000000000000000000000000000000000000 00000000000000000000000000000000000000000000000000000000000000f7 0100000000000000000000000000000000000000000000000000000000000000 0000 01 20 3300000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
+ // Note: txid is encoded in reverse byte order (Bitcoin standard), so to get display 0000...0031, encode 3100...0000
+ ext_from_hex("0084 c000000000000000000000000000000000000000000000000000000000000000 00000000000000000000000000000000000000000000000000000000000000f7 0100000000000000000000000000000000000000000000000000000000000000 0000 01 20 3100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
// After commitment_signed exchange, we need to exchange tx_signatures.
// Message type IDs: TxSignatures = 71 (0x0047)
@@ -1900,19 +1900,19 @@ fn splice_seed() -> Vec<u8> {
// inbound read from peer id 0 of len 150 (134 message + 16 MAC)
ext_from_hex("030096", &mut test);
// TxSignatures message with shared_input_signature TLV (type 0)
- // txid must match the splice funding txid (0x33 in reverse byte order)
+ // txid must match the splice funding txid (0x31 in reverse byte order)
// shared_input_signature: 64-byte fuzz signature for the shared input
- ext_from_hex("0047 c000000000000000000000000000000000000000000000000000000000000000 3300000000000000000000000000000000000000000000000000000000000000 0000 00 40 00000000000000000000000000000000000000000000000000000000000000dc 0100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
+ ext_from_hex("0047 c000000000000000000000000000000000000000000000000000000000000000 3100000000000000000000000000000000000000000000000000000000000000 0000 00 40 00000000000000000000000000000000000000000000000000000000000000dc 0100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
// Connect a block with the splice funding transaction to confirm it
// The splice funding tx: version(4) + input_count(1) + txid(32) + vout(4) + script_len(1) + sequence(4)
// + output_count(1) + value(8) + script_len(1) + script(34) + locktime(4) = 94 bytes = 0x5e
// Transaction structure from FundingTransactionReadyForSigning:
// - Input: spending c000...00:0 with sequence 0xfffffffd
- // - Output: 115536 sats to OP_0 PUSH32 6e00...00
+ // - Output: 115538 sats to OP_0 PUSH32 6e00...00
// - Locktime: 13
ext_from_hex("0c005e", &mut test);
- ext_from_hex("02000000 01 c000000000000000000000000000000000000000000000000000000000000000 00000000 00 fdffffff 01 50c3010000000000 22 00206e00000000000000000000000000000000000000000000000000000000000000 0d000000", &mut test);
+ ext_from_hex("02000000 01 c000000000000000000000000000000000000000000000000000000000000000 00000000 00 fdffffff 01 52c3010000000000 22 00206e00000000000000000000000000000000000000000000000000000000000000 0d000000", &mut test);
// Connect additional blocks to reach minimum_depth confirmations
for _ in 0..5 {
@@ -1929,8 +1929,8 @@ fn splice_seed() -> Vec<u8> {
// inbound read from peer id 0 of len 82 (66 message + 16 MAC)
ext_from_hex("030052", &mut test);
// SpliceLocked message (type 77 = 0x004d): channel_id + splice_txid + mac
- // splice_txid must match the splice funding txid (0x33 in reverse byte order)
- ext_from_hex("004d c000000000000000000000000000000000000000000000000000000000000000 3300000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
+ // splice_txid must match the splice funding txid (0x31 in reverse byte order)
+ ext_from_hex("004d c000000000000000000000000000000000000000000000000000000000000000 3100000000000000000000000000000000000000000000000000000000000000 03000000000000000000000000000000", &mut test);
test
}
@@ -2060,6 +2060,6 @@ mod tests {
// Splice locked
assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendSpliceLocked event in peer_handler for node 030000000000000000000000000000000000000000000000000000000000000002 for channel c000000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1));
- assert_eq!(log_entries.get(&("lightning::ln::channel".to_string(), "Promoting splice funding txid 0000000000000000000000000000000000000000000000000000000000000033".to_string())), Some(&1));
+ assert_eq!(log_entries.get(&("lightning::ln::channel".to_string(), "Promoting splice funding txid 0000000000000000000000000000000000000000000000000000000000000031".to_string())), Some(&1));
}
}
diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs
index 6341b10..80c1217 100644
--- a/lightning/src/ln/funding.rs
+++ b/lightning/src/ln/funding.rs
@@ -334,7 +334,6 @@ macro_rules! build_funding_contribution {
let CoinSelection { confirmed_utxos: inputs, change_output } = coin_selection;
Ok(FundingContribution::new(
- value_added,
outputs,
inputs,
change_output,
@@ -455,7 +454,6 @@ impl FundingTemplate {
max_feerate,
)?;
Ok(FundingContribution::new(
- Amount::ZERO,
outputs,
vec![],
None,
@@ -572,7 +570,7 @@ impl FundingTemplate {
return Ok(adjusted);
}
build_funding_contribution!(
- contribution.value_added,
+ contribution.value_added(),
contribution.outputs,
shared_input,
min_rbf_feerate,
@@ -628,7 +626,7 @@ impl FundingTemplate {
return Ok(adjusted);
}
build_funding_contribution!(
- contribution.value_added,
+ contribution.value_added(),
contribution.outputs,
shared_input,
min_rbf_feerate,
@@ -707,12 +705,6 @@ fn estimate_transaction_fee(
/// The components of a funding transaction contributed by one party.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct FundingContribution {
- /// The amount to contribute to the channel.
- ///
- /// If `value_added` is [`Amount::ZERO`], then any fees will be deducted from the channel
- /// balance instead of paid by `inputs`.
- value_added: Amount,
-
/// The estimate fees responsible to be paid for the contribution.
estimated_fee: Amount,
@@ -738,20 +730,19 @@ pub struct FundingContribution {
}
impl_writeable_tlv_based!(FundingContribution, {
- (1, value_added, required),
- (3, estimated_fee, required),
- (5, inputs, optional_vec),
- (7, outputs, optional_vec),
- (9, change_output, option),
- (11, feerate, required),
- (13, max_feerate, required),
- (15, is_splice, required),
+ (1, estimated_fee, required),
+ (3, inputs, optional_vec),
+ (5, outputs, optional_vec),
+ (7, change_output, option),
+ (9, feerate, required),
+ (11, max_feerate, required),
+ (13, is_splice, required),
});
impl FundingContribution {
fn new(
- value_added: Amount, outputs: Vec<TxOut>, inputs: Vec<FundingTxInput>,
- change_output: Option<TxOut>, feerate: FeeRate, max_feerate: FeeRate, is_splice: bool,
+ outputs: Vec<TxOut>, inputs: Vec<FundingTxInput>, change_output: Option<TxOut>,
+ feerate: FeeRate, max_feerate: FeeRate, is_splice: bool,
) -> Self {
// The caller creating a FundingContribution is always the initiator for fee estimation
// purposes — this is conservative, overestimating rather than underestimating fees if the
@@ -766,16 +757,7 @@ impl FundingContribution {
);
debug_assert!(estimated_fee <= Amount::MAX_MONEY);
- Self {
- value_added,
- estimated_fee,
- inputs,
- outputs,
- change_output,
- feerate,
- max_feerate,
- is_splice,
- }
+ Self { estimated_fee, inputs, outputs, change_output, feerate, max_feerate, is_splice }
}
pub(super) fn feerate(&self) -> FeeRate {
@@ -794,9 +776,20 @@ impl FundingContribution {
self.outputs.iter().chain(self.change_output.iter())
}
- /// Returns the amount added to the channel by this contribution.
+ /// The value that will be added to the channel after fees. See [`Self::net_value`] for the net
+ /// value contribution to the channel.
pub fn value_added(&self) -> Amount {
- self.value_added
+ let total_input_value = self.inputs.iter().map(|i| i.utxo.output.value).sum::<Amount>();
+ let total_output_value = self.outputs.iter().map(|output| output.value).sum::<Amount>();
+ total_input_value
+ .checked_sub(total_output_value)
+ .and_then(|v| v.checked_sub(self.estimated_fee))
+ .and_then(|v| {
+ v.checked_sub(
+ self.change_output.as_ref().map_or(Amount::ZERO, |output| output.value),
+ )
+ })
+ .unwrap_or(Amount::ZERO)
}
/// Returns the outputs (e.g., withdrawal destinations) included in this contribution.
@@ -849,7 +842,7 @@ impl FundingContribution {
}
/// Validates that the funding inputs are suitable for use in the interactive transaction
- /// protocol, checking prevtx sizes and input sufficiency.
+ /// protocol, checking prevtx sizes.
pub fn validate(&self) -> Result<(), String> {
for FundingTxInput { utxo, prevtx, .. } in self.inputs.iter() {
use crate::util::ser::Writeable;
@@ -871,38 +864,6 @@ impl FundingContribution {
}
}
- // Fees for splice-out are paid from the channel balance whereas fees for splice-in
- // are paid by the funding inputs. Therefore, in the case of splice-out, we add the
- // fees on top of the user-specified contribution. We leave the user-specified
- // contribution as-is for splice-ins.
- if !self.inputs.is_empty() {
- let mut total_input_value = Amount::ZERO;
- for FundingTxInput { utxo, .. } in self.inputs.iter() {
- total_input_value = total_input_value
- .checked_add(utxo.output.value)
- .ok_or("Sum of input values is greater than the total bitcoin supply")?;
- }
-
- // If the inputs are enough to cover intended contribution amount plus fees (which
- // include the change output weight when present), we are fine.
- // If the inputs are less, but enough to cover intended contribution amount with
- // (lower) fees without change, we are also fine (change will not be generated).
- // Since estimated_fee includes change weight, this check is conservative.
- //
- // Note: dust limit is not relevant in this check.
-
- let contributed_input_value = self.value_added;
- let estimated_fee = self.estimated_fee;
- let minimal_input_amount_needed = contributed_input_value
- .checked_add(estimated_fee)
- .ok_or(format!("{contributed_input_value} contribution plus {estimated_fee} fee estimate exceeds the total bitcoin supply"))?;
- if total_input_value < minimal_input_amount_needed {
- return Err(format!(
- "Total input amount {total_input_value} is lower than needed for splice-in contribution {contributed_input_value}, considering fees of {estimated_fee}. Need more inputs.",
- ));
- }
- }
-
Ok(())
}
@@ -1006,21 +967,10 @@ impl FundingContribution {
self.is_splice,
target_feerate,
);
- // The fee buffer is total input value minus value_added and output values.
- // This is estimated_fee plus the coin selection surplus (dust burned to
- // fees), ensuring we never silently reduce value_added beyond the small
- // surplus from coin selection.
- let total_input_value: Amount =
- self.inputs.iter().map(|i| i.utxo.output.value).sum();
- let output_values: Amount = self.outputs.iter().map(|o| o.value).sum();
- let fee_buffer = total_input_value
- .checked_sub(self.value_added)
- .and_then(|v| v.checked_sub(output_values))
- .ok_or(FeeRateAdjustmentError::FeeBufferOverflow)?;
- if target_fee > fee_buffer {
+ if target_fee > self.estimated_fee {
return Err(FeeRateAdjustmentError::FeeBufferInsufficient {
- source: "estimated fee + coin selection surplus",
- available: fee_buffer,
+ source: "estimated fee",
+ available: self.estimated_fee,
required: target_fee,
});
}
@@ -1062,12 +1012,10 @@ impl FundingContribution {
) -> Result<Self, FeeRateAdjustmentError> {
let (new_estimated_fee, new_change) =
self.compute_feerate_adjustment(feerate, holder_balance, is_initiator)?;
- let surplus = self.fee_buffer_surplus(new_estimated_fee, &new_change);
match new_change {
Some(value) => self.change_output.as_mut().unwrap().value = value,
None => self.change_output = None,
}
- self.value_added += surplus;
self.estimated_fee = new_estimated_fee;
self.feerate = feerate;
Ok(self)
@@ -1106,15 +1054,28 @@ impl FundingContribution {
) -> Result<SignedAmount, FeeRateAdjustmentError> {
let (new_estimated_fee, new_change) =
self.compute_feerate_adjustment(target_feerate, holder_balance, is_initiator)?;
- let surplus = self
- .fee_buffer_surplus(new_estimated_fee, &new_change)
+
+ let prev_fee = self
+ .estimated_fee
+ .to_signed()
+ .expect("total input amount cannot exceed Amount::MAX_MONEY");
+ let prev_change = self
+ .change_output
+ .as_ref()
+ .map_or(Amount::ZERO, |output| output.value)
+ .to_signed()
+ .expect("total input amount cannot exceed Amount::MAX_MONEY");
+
+ let new_fee = new_estimated_fee
.to_signed()
- .expect("surplus does not exceed Amount::MAX_MONEY");
- let net_value = self
- .net_value_with_fee(new_estimated_fee)
- .checked_add(surplus)
- .expect("net_value + surplus does not overflow");
- Ok(net_value)
+ .expect("total input amount cannot exceed Amount::MAX_MONEY");
+ let new_change = new_change
+ .unwrap_or(Amount::ZERO)
+ .to_signed()
+ .expect("total input amount cannot exceed Amount::MAX_MONEY");
+
+ let prev_net_value = self.net_value();
+ Ok(prev_net_value + prev_fee + prev_change - new_fee - new_change)
}
/// Returns the net value at the given target feerate without mutating `self`,
@@ -1133,55 +1094,35 @@ impl FundingContribution {
self.net_value_at_feerate(target_feerate, holder_balance, true)
}
- /// Returns the fee buffer surplus when a change output is removed.
- ///
- /// The fee buffer is the actual amount available for fees from inputs: total input value
- /// minus value_added and output values. This includes both the weight-based estimated_fee
- /// and any coin selection surplus (dust burned to fees). When the change output is removed,
- /// the fee buffer may exceed the new fee; the surplus is returned so it can be redirected
- /// to value_added rather than being burned as excess fees.
- ///
- /// Returns [`Amount::ZERO`] when there are no inputs or the change output is kept.
- fn fee_buffer_surplus(&self, new_estimated_fee: Amount, new_change: &Option<Amount>) -> Amount {
- if !self.inputs.is_empty() && new_change.is_none() {
- let total_input_value: Amount = self.inputs.iter().map(|i| i.utxo.output.value).sum();
- let output_values: Amount = self.outputs.iter().map(|o| o.value).sum();
- let fee_buffer = total_input_value - self.value_added - output_values;
- debug_assert!(fee_buffer >= new_estimated_fee);
- fee_buffer - new_estimated_fee
- } else {
- Amount::ZERO
- }
- }
-
- /// The net value contributed to a channel by the splice. If negative, more value will be
- /// spliced out than spliced in. Fees will be deducted from the expected splice-out amount
- /// if no inputs were included.
+ /// The net value contributed to a channel by the splice.
pub fn net_value(&self) -> SignedAmount {
- self.net_value_with_fee(self.estimated_fee)
+ let estimated_fee = self
+ .estimated_fee
+ .to_signed()
+ .expect("total_input_value is validated to not exceed Amount::MAX_MONEY");
+ self.net_value_without_fee()
+ .checked_sub(estimated_fee)
+ .expect("all amounts are validated to not exceed Amount::MAX_MONEY")
}
- /// Computes the net value using the given `estimated_fee` for the splice-out (no inputs)
- /// case. For splice-in, fees are paid by inputs so `estimated_fee` is not deducted.
- fn net_value_with_fee(&self, estimated_fee: Amount) -> SignedAmount {
- let unpaid_fees = if self.inputs.is_empty() { estimated_fee } else { Amount::ZERO }
- .to_signed()
- .expect("estimated_fee is validated to not exceed Amount::MAX_MONEY");
- let value_added = self
- .value_added
+ fn net_value_without_fee(&self) -> SignedAmount {
+ let total_input_value = self
+ .inputs
+ .iter()
+ .map(|input| input.utxo.output.value)
+ .sum::<Amount>()
.to_signed()
- .expect("value_added is validated to not exceed Amount::MAX_MONEY");
- let value_removed = self
+ .expect("total_input_value is validated to not exceed Amount::MAX_MONEY");
+ let total_output_value = self
.outputs
.iter()
+ .chain(self.change_output.iter())
.map(|txout| txout.value)
.sum::<Amount>()
.to_signed()
- .expect("value_removed is validated to not exceed Amount::MAX_MONEY");
-
- let contribution_amount = value_added - value_removed;
- contribution_amount
- .checked_sub(unpaid_fees)
+ .expect("total_output_value is validated to not exceed Amount::MAX_MONEY");
+ total_input_value
+ .checked_sub(total_output_value)
.expect("all amounts are validated to not exceed Amount::MAX_MONEY")
}
}
@@ -1298,190 +1239,6 @@ mod tests {
}
}
- #[test]
- #[rustfmt::skip]
- fn test_check_v2_funding_inputs_sufficient() {
- // positive case, inputs well over intended contribution
- {
- let expected_fee = if cfg!(feature = "grind_signatures") { 2278 } else { 2284 };
- let contribution = FundingContribution {
- value_added: Amount::from_sat(220_000),
- estimated_fee: Amount::from_sat(expected_fee),
- inputs: vec![
- funding_input_sats(200_000),
- funding_input_sats(100_000),
- ],
- outputs: vec![],
- change_output: None,
- is_splice: true,
- feerate: FeeRate::from_sat_per_kwu(2000),
- max_feerate: FeeRate::MAX,
- };
- assert!(contribution.validate().is_ok());
- assert_eq!(contribution.net_value(), contribution.value_added.to_signed().unwrap());
- }
-
- // Net splice-in
- {
- let expected_fee = if cfg!(feature = "grind_signatures") { 2526 } else { 2532 };
- let contribution = FundingContribution {
- value_added: Amount::from_sat(220_000),
- estimated_fee: Amount::from_sat(expected_fee),
- inputs: vec![
- funding_input_sats(200_000),
- funding_input_sats(100_000),
- ],
- outputs: vec![
- funding_output_sats(200_000),
- ],
- change_output: None,
- is_splice: true,
- feerate: FeeRate::from_sat_per_kwu(2000),
- max_feerate: FeeRate::MAX,
- };
- assert!(contribution.validate().is_ok());
- assert_eq!(contribution.net_value(), SignedAmount::from_sat(220_000 - 200_000));
- }
-
- // Net splice-out
- {
- let expected_fee = if cfg!(feature = "grind_signatures") { 2526 } else { 2532 };
- let contribution = FundingContribution {
- value_added: Amount::from_sat(220_000),
- estimated_fee: Amount::from_sat(expected_fee),
- inputs: vec![
- funding_input_sats(200_000),
- funding_input_sats(100_000),
- ],
- outputs: vec![
- funding_output_sats(400_000),
- ],
- change_output: None,
- is_splice: true,
- feerate: FeeRate::from_sat_per_kwu(2000),
- max_feerate: FeeRate::MAX,
- };
- assert!(contribution.validate().is_ok());
- assert_eq!(contribution.net_value(), SignedAmount::from_sat(220_000 - 400_000));
- }
-
- // Net splice-out, inputs insufficient to cover fees
- {
- let expected_fee = if cfg!(feature = "grind_signatures") { 113670 } else { 113940 };
- let contribution = FundingContribution {
- value_added: Amount::from_sat(220_000),
- estimated_fee: Amount::from_sat(expected_fee),
- inputs: vec![
- funding_input_sats(200_000),
- funding_input_sats(100_000),
- ],
- outputs: vec![
- funding_output_sats(400_000),
- ],
- change_output: None,
- is_splice: true,
- feerate: FeeRate::from_sat_per_kwu(90000),
- max_feerate: FeeRate::MAX,
- };
- assert_eq!(
- contribution.validate(),
- Err(format!(
- "Total input amount 0.00300000 BTC is lower than needed for splice-in contribution 0.00220000 BTC, considering fees of {}. Need more inputs.",
- Amount::from_sat(expected_fee),
- )),
- );
- }
-
- // negative case, inputs clearly insufficient
- {
- let expected_fee = if cfg!(feature = "grind_signatures") { 1736 } else { 1740 };
- let contribution = FundingContribution {
- value_added: Amount::from_sat(220_000),
- estimated_fee: Amount::from_sat(expected_fee),
- inputs: vec![
- funding_input_sats(100_000),
- ],
- outputs: vec![],
- change_output: None,
- is_splice: true,
- feerate: FeeRate::from_sat_per_kwu(2000),
- max_feerate: FeeRate::MAX,
- };
- assert_eq!(
- contribution.validate(),
- Err(format!(
- "Total input amount 0.00100000 BTC is lower than needed for splice-in contribution 0.00220000 BTC, considering fees of {}. Need more inputs.",
- Amount::from_sat(expected_fee),
- )),
- );
- }
-
- // barely covers
- {
- let expected_fee = if cfg!(feature = "grind_signatures") { 2278 } else { 2284 };
- let contribution = FundingContribution {
- value_added: Amount::from_sat(300_000 - expected_fee - 20),
- estimated_fee: Amount::from_sat(expected_fee),
- inputs: vec![
- funding_input_sats(200_000),
- funding_input_sats(100_000),
- ],
- outputs: vec![],
- change_output: None,
- is_splice: true,
- feerate: FeeRate::from_sat_per_kwu(2000),
- max_feerate: FeeRate::MAX,
- };
- assert!(contribution.validate().is_ok());
- assert_eq!(contribution.net_value(), contribution.value_added.to_signed().unwrap());
- }
-
- // higher fee rate, does not cover
- {
- let expected_fee = if cfg!(feature = "grind_signatures") { 2506 } else { 2513 };
- let contribution = FundingContribution {
- value_added: Amount::from_sat(298032),
- estimated_fee: Amount::from_sat(expected_fee),
- inputs: vec![
- funding_input_sats(200_000),
- funding_input_sats(100_000),
- ],
- outputs: vec![],
- change_output: None,
- is_splice: true,
- feerate: FeeRate::from_sat_per_kwu(2200),
- max_feerate: FeeRate::MAX,
- };
- assert_eq!(
- contribution.validate(),
- Err(format!(
- "Total input amount 0.00300000 BTC is lower than needed for splice-in contribution 0.00298032 BTC, considering fees of {}. Need more inputs.",
- Amount::from_sat(expected_fee),
- )),
- );
- }
-
- // barely covers, less fees (not a splice)
- {
- let expected_fee = if cfg!(feature = "grind_signatures") { 1512 } else { 1516 };
- let contribution = FundingContribution {
- value_added: Amount::from_sat(300_000 - expected_fee - 20),
- estimated_fee: Amount::from_sat(expected_fee),
- inputs: vec![
- funding_input_sats(200_000),
- funding_input_sats(100_000),
- ],
- outputs: vec![],
- change_output: None,
- is_splice: false,
- feerate: FeeRate::from_sat_per_kwu(2000),
- max_feerate: FeeRate::MAX,
- };
- assert!(contribution.validate().is_ok());
- assert_eq!(contribution.net_value(), contribution.value_added.to_signed().unwrap());
- }
- }
-
struct UnreachableWallet;
impl CoinSelectionSourceSync for UnreachableWallet {
@@ -1612,7 +1369,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs: inputs.clone(),
outputs: vec![],
@@ -1650,7 +1406,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -1691,7 +1446,6 @@ mod tests {
let change = funding_output_sats(change_value.to_sat());
let contribution = FundingContribution {
- value_added,
estimated_fee,
inputs: inputs.clone(),
outputs: vec![],
@@ -1727,7 +1481,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -1753,7 +1506,6 @@ mod tests {
estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::ZERO,
estimated_fee,
inputs: vec![],
outputs: outputs.clone(),
@@ -1783,7 +1535,6 @@ mod tests {
estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::ZERO,
estimated_fee,
inputs: vec![],
outputs,
@@ -1807,12 +1558,12 @@ mod tests {
let target_feerate = FeeRate::from_sat_per_kwu(3000);
let inputs = vec![funding_input_sats(100_000)];
let change = funding_output_sats(10_000);
+ let change_value = change.value;
let estimated_fee =
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -1827,7 +1578,10 @@ mod tests {
let net_at_feerate =
contribution.net_value_for_acceptor_at_feerate(target_feerate, Amount::MAX).unwrap();
assert_eq!(net_at_feerate, contribution.net_value());
- assert_eq!(net_at_feerate, Amount::from_sat(50_000).to_signed().unwrap());
+ assert_eq!(
+ net_at_feerate,
+ (Amount::from_sat(100_000) - estimated_fee - change_value).to_signed().unwrap(),
+ );
}
#[test]
@@ -1842,7 +1596,6 @@ mod tests {
estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::ZERO,
estimated_fee,
inputs: vec![],
outputs: outputs.clone(),
@@ -1878,7 +1631,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -1912,7 +1664,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -1940,7 +1691,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -1972,7 +1722,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -2007,7 +1756,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -2050,7 +1798,6 @@ mod tests {
assert!(target_fee > estimated_fee);
let contribution = FundingContribution {
- value_added,
estimated_fee,
inputs,
outputs: vec![],
@@ -2083,7 +1830,6 @@ mod tests {
assert!(target_fee > estimated_fee);
let contribution = FundingContribution {
- value_added,
estimated_fee,
inputs,
outputs: vec![],
@@ -2122,7 +1868,6 @@ mod tests {
assert!(estimated_fee - target_fee < dust_limit);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -2142,8 +1887,8 @@ mod tests {
#[test]
fn test_for_acceptor_at_feerate_no_change_surplus_absorbed() {
// Inputs, no change. The estimated_fee (is_initiator=true) far exceeds the acceptor's
- // target fee (is_initiator=false). The surplus stays in the channel balance rather than
- // being burned as excess fees.
+ // target fee (is_initiator=false). The surplus stays in the channel contribution rather
+ // than being burned as excess fees.
let feerate = FeeRate::from_sat_per_kwu(2000);
let value_added = Amount::from_sat(50_000);
@@ -2159,7 +1904,6 @@ mod tests {
let target_fee = estimate_transaction_fee(&inputs, &[], None, false, true, feerate);
let contribution = FundingContribution {
- value_added,
estimated_fee,
inputs,
outputs: vec![],
@@ -2178,20 +1922,17 @@ mod tests {
assert!(adjusted.change_output.is_none());
assert_eq!(adjusted.estimated_fee, target_fee);
let surplus = estimated_fee - target_fee;
- assert_eq!(adjusted.value_added, value_added + surplus);
+ assert_eq!(adjusted.value_added(), value_added + surplus);
assert_eq!(adjusted.net_value(), net_value_before + surplus.to_signed().unwrap());
}
#[test]
- fn test_for_acceptor_at_feerate_fee_buffer_overflow() {
- // Construct a contribution with estimated_fee and change values that overflow Amount.
+ fn test_for_acceptor_at_feerate_fee_buffer_overflow_with_change() {
+ // Overflow in estimated_fee + change value should surface as FeeBufferOverflow.
let feerate = FeeRate::from_sat_per_kwu(2000);
- let inputs = vec![funding_input_sats(100_000)];
-
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee: Amount::MAX,
- inputs,
+ inputs: vec![funding_input_sats(100_000)],
outputs: vec![],
change_output: Some(funding_output_sats(1)),
feerate,
@@ -2214,7 +1955,6 @@ mod tests {
estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::ZERO,
estimated_fee,
inputs: vec![],
outputs: outputs.clone(),
@@ -2241,7 +1981,6 @@ mod tests {
estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::ZERO,
estimated_fee,
inputs: vec![],
outputs: outputs.clone(),
@@ -2272,7 +2011,6 @@ mod tests {
estimate_transaction_fee(&[], &outputs, None, true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::ZERO,
estimated_fee,
inputs: vec![],
outputs,
@@ -2301,7 +2039,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, original_feerate);
let contribution = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -2336,7 +2073,6 @@ mod tests {
let max_feerate = FeeRate::from_sat_per_kwu(2020);
let prior = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee: Amount::from_sat(1_000),
inputs: vec![funding_input_sats(100_000)],
outputs: vec![],
@@ -2372,7 +2108,6 @@ mod tests {
estimate_transaction_fee(&inputs, &[], Some(&change), true, true, prior_feerate);
let prior = FundingContribution {
- value_added: Amount::from_sat(50_000),
estimated_fee,
inputs,
outputs: vec![],
@@ -2434,7 +2169,6 @@ mod tests {
let withdrawal = funding_output_sats(20_000);
let prior = FundingContribution {
- value_added: Amount::ZERO,
estimated_fee: Amount::from_sat(500),
inputs: vec![],
outputs: vec![withdrawal.clone()],
@@ -2452,13 +2186,13 @@ mod tests {
let wallet = SingleUtxoWallet {
utxo: funding_input_sats(50_000),
- change_output: Some(funding_output_sats(40_000)),
+ change_output: Some(funding_output_sats(25_000)),
};
// rbf_sync should succeed and the contribution should have inputs from coin selection.
let contribution = template.rbf_sync(FeeRate::MAX, &wallet).unwrap();
- assert_eq!(contribution.value_added, Amount::ZERO);
assert!(!contribution.inputs.is_empty(), "coin selection should have added inputs");
+ assert!(contribution.value_added() > Amount::ZERO);
assert_eq!(contribution.outputs, vec![withdrawal]);
assert_eq!(contribution.feerate, min_rbf_feerate);
}
@@ -2478,8 +2212,8 @@ mod tests {
};
let contribution = template.rbf_sync(FeeRate::MAX, &wallet).unwrap();
- assert_eq!(contribution.value_added, Amount::ZERO);
assert!(!contribution.inputs.is_empty(), "coin selection should have added inputs");
+ assert!(contribution.value_added() > Amount::ZERO);
assert!(contribution.outputs.is_empty());
assert_eq!(contribution.feerate, min_rbf_feerate);
}
@@ -2495,7 +2229,6 @@ mod tests {
let withdrawal = funding_output_sats(20_000);
let prior = FundingContribution {
- value_added: Amount::ZERO,
estimated_fee: Amount::from_sat(500),
inputs: vec![],
outputs: vec![withdrawal.clone()],
@@ -2513,7 +2246,7 @@ mod tests {
let wallet = SingleUtxoWallet {
utxo: funding_input_sats(50_000),
- change_output: Some(funding_output_sats(40_000)),
+ change_output: Some(funding_output_sats(25_000)),
};
let contribution = template.rbf_sync(callers_max_feerate, &wallet).unwrap();
@@ -2537,7 +2270,7 @@ mod tests {
let contribution =
template.splice_out(vec![withdrawal.clone()], feerate, FeeRate::MAX).unwrap();
- assert_eq!(contribution.value_added, Amount::ZERO);
+ assert_eq!(contribution.value_added(), Amount::ZERO);
assert!(contribution.inputs.is_empty());
assert!(contribution.change_output.is_none());
assert_eq!(contribution.outputs, vec![withdrawal]);
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index 5492921..3004c76 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -158,40 +158,6 @@ impl CoinSelectionSourceSync for TightBudgetWallet {
}
}
-#[test]
-fn test_validate_accounts_for_change_output_weight() {
- // Demonstrates that estimated_fee includes the change output's weight when building a
- // FundingContribution. A mock wallet returns a single input whose value is between
- // estimated_fee_without_change (1736/1740 sats) and estimated_fee_with_change (1984/1988
- // sats) above value_added. The validate() check correctly catches that the inputs are
- // insufficient when the change output weight is included. Without accounting for the change
- // output weight, the check would incorrectly pass.
- let chanmon_cfgs = create_chanmon_cfgs(2);
- let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
- let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
- let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-
- let (_, _, channel_id, _) =
- create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0);
-
- let feerate = FeeRate::from_sat_per_kwu(2000);
- let funding_template =
- nodes[0].node.splice_channel(&channel_id, &nodes[1].node.get_our_node_id()).unwrap();
-
- // Input value = value_added + 1800: above 1736/1740 (fee without change), below 1984/1988
- // (fee with change).
- let value_added = Amount::from_sat(20_000);
- let wallet = TightBudgetWallet {
- utxo_value: value_added + Amount::from_sat(1800),
- change_value: Amount::from_sat(1000),
- };
- let contribution =
- funding_template.splice_in_sync(value_added, feerate, FeeRate::MAX, &wallet).unwrap();
-
- assert!(contribution.change_output().is_some());
- assert!(contribution.validate().is_err());
-}
-
pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>(
initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId,
funding_contribution: FundingContribution,
@@ -1862,7 +1828,8 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs:
let splice_in_amount = initial_channel_capacity / 2;
let initiator_contribution =
do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, Amount::from_sat(splice_in_amount));
- let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution);
+ let (splice_tx, _) =
+ splice_channel(&nodes[0], &nodes[1], channel_id, initiator_contribution.clone());
let (preimage2, payment_hash2, ..) = route_payment(&nodes[0], &[&nodes[1]], payment_amount);
let htlc_expiry = nodes[0].best_block_info().1 + TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS;
@@ -1913,7 +1880,7 @@ fn do_test_splice_commitment_broadcast(splice_status: SpliceStatus, claim_htlcs:
message: "test".to_owned(),
};
let closed_channel_capacity = if splice_status == SpliceStatus::Locked {
- initial_channel_capacity + splice_in_amount
+ initial_channel_capacity + initiator_contribution.net_value().to_sat() as u64
} else {
initial_channel_capacity
};
Why this scored 32/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.