Check splice contributions against SignedAmount::MAX_MONEY
What changed, and why it matters
This commit fixes a potential integer overflow in the experimental splicing feature of the Lightning Dev Kit. When a user or peer tried to splice a channel with a contribution larger than the total Bitcoin supply (about 21 million BTC), the code could overflow while converting the amount from satoshis to millisatoshis. The patch adds explicit checks that reject contributions above SignedAmount::MAX_MONEY and begins storing contributions using the safer SignedAmount type instead of raw i64 values. The bug is only reachable through the still-experimental splicing code path.
Treat this as a security-relevant hardening fix for the experimental splicing feature. Users building from source should update to a revision containing this commit. Because splicing is still behind a feature flag and not yet productionized, broad immediate impact is limited, but downstream integrators testing splicing should apply the patch promptly.
Security signals we found
Integer overflow prevention in satoshi-to-millisatoshi conversion
Input validation against MAX_MONEY for splice contributions
Type migration from raw i64 to SignedAmount for monetary values
Defense-in-depth: debug_asserts plus runtime error returns
Evidence from the diff
The change hardens splicing contribution handling in channel.rs, channelmanager.rs, and interactivetxs.rs. Previously, our_funding_contribution_satoshis was an i64 and was multiplied by 1000 to compute post_value_to_self_msat via checked_add_signed. The patch introduces SignedAmount for these values and adds runtime checks (returning API misuse or WarnAndDisconnect errors) that reject contributions whose absolute value exceeds SignedAmount::MAX_MONEY (21_000_000 BTC). This prevents a possible overflow in the satoshi-to-millisatoshi conversion and in subsequent arithmetic. The checks are added in both locally-initiated splices (splice_init) and counterparty-initiated splices (validate_splice_init / handle_splice_ack).
Changed components
lightning/src/ln/channel.rslightning/src/ln/channelmanager.rslightning/src/ln/interactivetxs.rsExperimental splicing protocol implementationInspect captured patch +91 / −51
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 56d38d5..4d675f9 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -8,7 +8,7 @@
// licenses.
use bitcoin::absolute::LockTime;
-use bitcoin::amount::Amount;
+use bitcoin::amount::{Amount, SignedAmount};
use bitcoin::consensus::encode;
use bitcoin::constants::ChainHash;
use bitcoin::script::{Builder, Script, ScriptBuf, WScriptHash};
@@ -2244,20 +2244,23 @@ impl FundingScope {
/// Constructs a `FundingScope` for splicing a channel.
#[cfg(splicing)]
fn for_splice<SP: Deref>(
- prev_funding: &Self, context: &ChannelContext<SP>, our_funding_contribution_sats: i64,
- their_funding_contribution_sats: i64, counterparty_funding_pubkey: PublicKey,
+ prev_funding: &Self, context: &ChannelContext<SP>, our_funding_contribution: SignedAmount,
+ their_funding_contribution: SignedAmount, counterparty_funding_pubkey: PublicKey,
) -> Result<Self, ChannelError>
where
SP::Target: SignerProvider,
{
+ debug_assert!(our_funding_contribution.abs() <= SignedAmount::MAX_MONEY);
+ debug_assert!(their_funding_contribution.abs() <= SignedAmount::MAX_MONEY);
+
let post_channel_value = prev_funding.compute_post_splice_value(
- our_funding_contribution_sats,
- their_funding_contribution_sats,
+ our_funding_contribution.to_sat(),
+ their_funding_contribution.to_sat(),
);
let post_value_to_self_msat = AddSigned::checked_add_signed(
prev_funding.value_to_self_msat,
- our_funding_contribution_sats * 1000,
+ our_funding_contribution.to_sat() * 1000,
);
debug_assert!(post_value_to_self_msat.is_some());
let post_value_to_self_msat = post_value_to_self_msat.unwrap();
@@ -5964,7 +5967,7 @@ pub(super) struct FundingNegotiationContext {
/// Whether we initiated the funding negotiation.
pub is_initiator: bool,
/// The amount in satoshis we will be contributing to the channel.
- pub our_funding_contribution_satoshis: i64,
+ pub our_funding_contribution: SignedAmount,
/// The amount in satoshis our counterparty will be contributing to the channel.
#[allow(dead_code)] // TODO(dual_funding): Remove once contribution to V2 channels is enabled.
pub their_funding_contribution_satoshis: Option<i64>,
@@ -6020,7 +6023,7 @@ impl FundingNegotiationContext {
};
// Optionally add change output
- if self.our_funding_contribution_satoshis > 0 {
+ if self.our_funding_contribution > SignedAmount::ZERO {
let change_value_opt = calculate_change_output_value(
&self,
self.shared_funding_input.is_some(),
@@ -10628,11 +10631,22 @@ where
// TODO(splicing): check for quiescence
- if our_funding_contribution_satoshis < 0 {
+ let our_funding_contribution = SignedAmount::from_sat(our_funding_contribution_satoshis);
+ if our_funding_contribution > SignedAmount::MAX_MONEY {
+ return Err(APIError::APIMisuseError {
+ err: format!(
+ "Channel {} cannot be spliced; contribution exceeds total bitcoin supply: {}",
+ self.context.channel_id(),
+ our_funding_contribution,
+ ),
+ });
+ }
+
+ if our_funding_contribution < SignedAmount::ZERO {
return Err(APIError::APIMisuseError {
err: format!(
"TODO(splicing): Splice-out not supported, only splice in; channel ID {}, contribution {}",
- self.context.channel_id(), our_funding_contribution_satoshis,
+ self.context.channel_id(), our_funding_contribution,
),
});
}
@@ -10645,7 +10659,7 @@ where
// Check that inputs are sufficient to cover our contribution.
let _fee = check_v2_funding_inputs_sufficient(
- our_funding_contribution_satoshis,
+ our_funding_contribution.to_sat(),
&our_funding_inputs,
true,
true,
@@ -10669,7 +10683,7 @@ where
let prev_funding_input = self.funding.to_splice_funding_input();
let funding_negotiation_context = FundingNegotiationContext {
is_initiator: true,
- our_funding_contribution_satoshis,
+ our_funding_contribution,
their_funding_contribution_satoshis: None,
funding_tx_locktime: LockTime::from_consensus(locktime),
funding_feerate_sat_per_1000_weight: funding_feerate_per_kw,
@@ -10690,7 +10704,7 @@ where
Ok(msgs::SpliceInit {
channel_id: self.context.channel_id,
- funding_contribution_satoshis: our_funding_contribution_satoshis,
+ funding_contribution_satoshis: our_funding_contribution.to_sat(),
funding_feerate_per_kw,
locktime,
funding_pubkey,
@@ -10701,10 +10715,8 @@ where
/// Checks during handling splice_init
#[cfg(splicing)]
pub fn validate_splice_init(
- &self, msg: &msgs::SpliceInit, our_funding_contribution_satoshis: i64,
+ &self, msg: &msgs::SpliceInit, our_funding_contribution: SignedAmount,
) -> Result<FundingScope, ChannelError> {
- let their_funding_contribution_satoshis = msg.funding_contribution_satoshis;
-
// TODO(splicing): Add check that we are the quiescence acceptor
// Check if a splice has been initiated already.
@@ -10724,21 +10736,40 @@ where
)));
}
- if their_funding_contribution_satoshis.saturating_add(our_funding_contribution_satoshis) < 0
- {
+ // TODO(splicing): Move this check once user-provided contributions are supported for
+ // counterparty-initiated splices.
+ if our_funding_contribution > SignedAmount::MAX_MONEY {
+ return Err(ChannelError::WarnAndDisconnect(format!(
+ "Channel {} cannot be spliced; our contribution exceeds total bitcoin supply: {}",
+ self.context.channel_id(),
+ our_funding_contribution,
+ )));
+ }
+
+ let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis);
+ if their_funding_contribution > SignedAmount::MAX_MONEY {
+ return Err(ChannelError::WarnAndDisconnect(format!(
+ "Channel {} cannot be spliced; their contribution exceeds total bitcoin supply: {}",
+ self.context.channel_id(),
+ their_funding_contribution,
+ )));
+ }
+
+ debug_assert_eq!(our_funding_contribution, SignedAmount::ZERO);
+ if their_funding_contribution < SignedAmount::ZERO {
return Err(ChannelError::WarnAndDisconnect(format!(
"Splice-out not supported, only splice in, contribution is {} ({} + {})",
- their_funding_contribution_satoshis + our_funding_contribution_satoshis,
- their_funding_contribution_satoshis,
- our_funding_contribution_satoshis,
+ their_funding_contribution + our_funding_contribution,
+ their_funding_contribution,
+ our_funding_contribution,
)));
}
let splice_funding = FundingScope::for_splice(
&self.funding,
&self.context,
- our_funding_contribution_satoshis,
- their_funding_contribution_satoshis,
+ our_funding_contribution,
+ their_funding_contribution,
msg.funding_pubkey,
)?;
@@ -10763,7 +10794,8 @@ where
ES::Target: EntropySource,
L::Target: Logger,
{
- let splice_funding = self.validate_splice_init(msg, our_funding_contribution_satoshis)?;
+ let our_funding_contribution = SignedAmount::from_sat(our_funding_contribution_satoshis);
+ let splice_funding = self.validate_splice_init(msg, our_funding_contribution)?;
log_info!(
logger,
@@ -10777,7 +10809,7 @@ where
let prev_funding_input = self.funding.to_splice_funding_input();
let funding_negotiation_context = FundingNegotiationContext {
is_initiator: false,
- our_funding_contribution_satoshis,
+ our_funding_contribution,
their_funding_contribution_satoshis: Some(their_funding_contribution_satoshis),
funding_tx_locktime: LockTime::from_consensus(msg.locktime),
funding_feerate_sat_per_1000_weight: msg.funding_feerate_per_kw,
@@ -10815,7 +10847,7 @@ where
Ok(msgs::SpliceAck {
channel_id: self.context.channel_id,
- funding_contribution_satoshis: our_funding_contribution_satoshis,
+ funding_contribution_satoshis: our_funding_contribution.to_sat(),
funding_pubkey,
require_confirmed_inputs: None,
})
@@ -10862,15 +10894,23 @@ where
},
};
- let our_funding_contribution_satoshis =
- funding_negotiation_context.our_funding_contribution_satoshis;
- let their_funding_contribution_satoshis = msg.funding_contribution_satoshis;
+ let our_funding_contribution = funding_negotiation_context.our_funding_contribution;
+ debug_assert!(our_funding_contribution <= SignedAmount::MAX_MONEY);
+
+ let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis);
+ if their_funding_contribution > SignedAmount::MAX_MONEY {
+ return Err(ChannelError::Warn(format!(
+ "Channel {} cannot be spliced; their contribution exceeds total bitcoin supply: {}",
+ self.context.channel_id(),
+ their_funding_contribution,
+ )));
+ }
let splice_funding = FundingScope::for_splice(
&self.funding,
&self.context,
- our_funding_contribution_satoshis,
- their_funding_contribution_satoshis,
+ our_funding_contribution,
+ their_funding_contribution,
msg.funding_pubkey,
)?;
@@ -12468,7 +12508,7 @@ where
};
let funding_negotiation_context = FundingNegotiationContext {
is_initiator: true,
- our_funding_contribution_satoshis: funding_satoshis as i64,
+ our_funding_contribution: SignedAmount::from_sat(funding_satoshis as i64),
// TODO(dual_funding) TODO(splicing) Include counterparty contribution, once that's enabled
their_funding_contribution_satoshis: None,
funding_tx_locktime,
@@ -12578,10 +12618,11 @@ where
L::Target: Logger,
{
// TODO(dual_funding): Take these as input once supported
- let our_funding_satoshis = 0u64;
+ let (our_funding_contribution, our_funding_contribution_sats) = (SignedAmount::ZERO, 0u64);
let our_funding_inputs = Vec::new();
- let channel_value_satoshis = our_funding_satoshis.saturating_add(msg.common_fields.funding_satoshis);
+ let channel_value_satoshis =
+ our_funding_contribution_sats.saturating_add(msg.common_fields.funding_satoshis);
let counterparty_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
channel_value_satoshis, msg.common_fields.dust_limit_satoshis);
let holder_selected_channel_reserve_satoshis = get_v2_channel_reserve_satoshis(
@@ -12608,9 +12649,7 @@ where
current_chain_height,
logger,
false,
-
- our_funding_satoshis,
-
+ our_funding_contribution_sats,
counterparty_pubkeys,
channel_type,
holder_selected_channel_reserve_satoshis,
@@ -12625,7 +12664,7 @@ where
let funding_negotiation_context = FundingNegotiationContext {
is_initiator: false,
- our_funding_contribution_satoshis: our_funding_satoshis as i64,
+ our_funding_contribution,
their_funding_contribution_satoshis: Some(msg.common_fields.funding_satoshis as i64),
funding_tx_locktime: LockTime::from_consensus(msg.locktime),
funding_feerate_sat_per_1000_weight: msg.funding_feerate_sat_per_1000_weight,
@@ -12649,7 +12688,7 @@ where
is_initiator: false,
inputs_to_contribute: our_funding_inputs,
shared_funding_input: None,
- shared_funding_output: SharedOwnedOutput::new(shared_funding_output, our_funding_satoshis),
+ shared_funding_output: SharedOwnedOutput::new(shared_funding_output, our_funding_contribution_sats),
outputs_to_contribute: Vec::new(),
}
).map_err(|err| {
@@ -12730,7 +12769,7 @@ where
}),
channel_type: Some(self.funding.get_channel_type().clone()),
},
- funding_satoshis: self.funding_negotiation_context.our_funding_contribution_satoshis
+ funding_satoshis: self.funding_negotiation_context.our_funding_contribution.to_sat()
as u64,
second_per_commitment_point,
require_confirmed_inputs: None,
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 9d1c629..724fc2e 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -30,7 +30,7 @@ use bitcoin::hashes::{Hash, HashEngine, HmacEngine};
use bitcoin::secp256k1::Secp256k1;
use bitcoin::secp256k1::{PublicKey, SecretKey};
-use bitcoin::{secp256k1, Sequence};
+use bitcoin::{secp256k1, Sequence, SignedAmount};
#[cfg(splicing)]
use bitcoin::{ScriptBuf, TxIn, Weight};
@@ -9401,7 +9401,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
// Inbound V2 channels with contributed inputs are not considered unfunded.
if let Some(unfunded_chan) = chan.as_unfunded_v2() {
- if unfunded_chan.funding_negotiation_context.our_funding_contribution_satoshis > 0 {
+ if unfunded_chan.funding_negotiation_context.our_funding_contribution > SignedAmount::ZERO {
continue;
}
}
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index b3b3d6d..c4d2ed9 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -11,7 +11,7 @@ use crate::io_extras::sink;
use crate::prelude::*;
use bitcoin::absolute::LockTime as AbsoluteLockTime;
-use bitcoin::amount::Amount;
+use bitcoin::amount::{Amount, SignedAmount};
use bitcoin::consensus::Encodable;
use bitcoin::constants::WITNESS_SCALE_FACTOR;
use bitcoin::key::Secp256k1;
@@ -2077,8 +2077,8 @@ pub(super) fn calculate_change_output_value(
context: &FundingNegotiationContext, is_splice: bool, shared_output_funding_script: &ScriptBuf,
funding_outputs: &Vec<TxOut>, change_output_dust_limit: u64,
) -> Result<Option<u64>, AbortReason> {
- assert!(context.our_funding_contribution_satoshis > 0);
- let our_funding_contribution_satoshis = context.our_funding_contribution_satoshis as u64;
+ assert!(context.our_funding_contribution > SignedAmount::ZERO);
+ let our_funding_contribution_satoshis = context.our_funding_contribution.to_sat() as u64;
let mut total_input_satoshis = 0u64;
let mut our_funding_inputs_weight = 0u64;
@@ -2156,7 +2156,8 @@ mod tests {
use bitcoin::transaction::Version;
use bitcoin::{opcodes, WScriptHash, Weight, XOnlyPublicKey};
use bitcoin::{
- OutPoint, PubkeyHash, ScriptBuf, Sequence, Transaction, TxIn, TxOut, WPubkeyHash, Witness,
+ OutPoint, PubkeyHash, ScriptBuf, Sequence, SignedAmount, Transaction, TxIn, TxOut,
+ WPubkeyHash, Witness,
};
use core::ops::Deref;
@@ -3186,7 +3187,7 @@ mod tests {
// There is leftover for change
let context = FundingNegotiationContext {
is_initiator: true,
- our_funding_contribution_satoshis: our_contributed as i64,
+ our_funding_contribution: SignedAmount::from_sat(our_contributed as i64),
their_funding_contribution_satoshis: None,
funding_tx_locktime: AbsoluteLockTime::ZERO,
funding_feerate_sat_per_1000_weight,
@@ -3209,7 +3210,7 @@ mod tests {
// Insufficient inputs, no leftover
let context = FundingNegotiationContext {
is_initiator: false,
- our_funding_contribution_satoshis: 130_000,
+ our_funding_contribution: SignedAmount::from_sat(130_000),
..context
};
assert_eq!(
@@ -3220,7 +3221,7 @@ mod tests {
// Very small leftover
let context = FundingNegotiationContext {
is_initiator: false,
- our_funding_contribution_satoshis: 118_000,
+ our_funding_contribution: SignedAmount::from_sat(118_000),
..context
};
assert_eq!(
@@ -3231,7 +3232,7 @@ mod tests {
// Small leftover, but not dust
let context = FundingNegotiationContext {
is_initiator: false,
- our_funding_contribution_satoshis: 117_992,
+ our_funding_contribution: SignedAmount::from_sat(117_992),
..context
};
assert_eq!(
@@ -3242,7 +3243,7 @@ mod tests {
// Larger fee, smaller change
let context = FundingNegotiationContext {
is_initiator: true,
- our_funding_contribution_satoshis: our_contributed as i64,
+ our_funding_contribution: SignedAmount::from_sat(our_contributed as i64),
funding_feerate_sat_per_1000_weight: funding_feerate_sat_per_1000_weight * 3,
..context
};
Why this scored 60/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.