Skip to content

Create a single P2A anchor on commitment transactions in 0FC channels #4053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 9, 2025
+192 −64
Merged

Create a single P2A anchor on commitment transactions in 0FC channels #4053

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9008409
Create a single P2A anchor on commitment transactions in 0FC channels
tankyleo Sep 4, 2025
e921bf6
Rename `CommitmentTransaction.feerate_per_kw()`
tankyleo Sep 4, 2025
c944b24
Run fmt on `ChannelContext::validate_commitment_signed`
tankyleo Sep 6, 2025
1cef4c3
Use `SINGLE|ACP` sighash on signatures from peer in 0FC channels
tankyleo Sep 5, 2025
538c4cc
Set version 3 on transactions for HTLC signatures in 0FC channels
tankyleo Sep 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions lightning/src/chain/channelmonitor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ fn write_legacy_holder_commitment_data<W: Writer>(

let txid = trusted_tx.txid();
let to_self_value_sat = commitment_tx.to_broadcaster_value_sat();
let feerate_per_kw = trusted_tx.feerate_per_kw();
let feerate_per_kw = trusted_tx.negotiated_feerate_per_kw();
let revocation_key = &tx_keys.revocation_key;
let a_htlc_key = &tx_keys.broadcaster_htlc_key;
let b_htlc_key = &tx_keys.countersignatory_htlc_key;
Expand Down Expand Up @@ -3455,7 +3455,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
) {
// We populate this field for downgrades
self.initial_counterparty_commitment_info = Some((commitment_tx.per_commitment_point(),
commitment_tx.feerate_per_kw(), commitment_tx.to_broadcaster_value_sat(), commitment_tx.to_countersignatory_value_sat()));
commitment_tx.negotiated_feerate_per_kw(), commitment_tx.to_broadcaster_value_sat(), commitment_tx.to_countersignatory_value_sat()));

#[cfg(debug_assertions)] {
let rebuilt_commitment_tx = self.initial_counterparty_commitment_tx().unwrap();
Expand Down Expand Up @@ -3661,7 +3661,9 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
{
return Err("Per-commitment-point mismatch");
}
if commitment_tx.feerate_per_kw() != other_commitment_tx.feerate_per_kw() {
if commitment_tx.negotiated_feerate_per_kw()
!= other_commitment_tx.negotiated_feerate_per_kw()
{
return Err("Commitment fee rate mismatch");
}
let nondust_htlcs = commitment_tx.nondust_htlcs();
Expand Down Expand Up @@ -4823,7 +4825,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
commitment_txid: tx.txid(),
per_commitment_number: tx.commitment_number(),
per_commitment_point: tx.per_commitment_point(),
feerate_per_kw: tx.feerate_per_kw(),
feerate_per_kw: tx.negotiated_feerate_per_kw(),
htlc: htlc.clone(),
preimage,
counterparty_sig: *counterparty_sig,
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/chain/onchaintx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1397,7 +1397,7 @@ mod tests {
commitment_txid: holder_commit_txid,
per_commitment_number: holder_commit.commitment_number(),
per_commitment_point: holder_commit.per_commitment_point(),
feerate_per_kw: holder_commit.feerate_per_kw(),
feerate_per_kw: holder_commit.negotiated_feerate_per_kw(),
htlc: htlc.clone(),
preimage: None,
counterparty_sig: *counterparty_sig,
Expand Down
6 changes: 3 additions & 3 deletions lightning/src/chain/package.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ impl HolderHTLCOutput {
commitment_txid: trusted_tx.txid(),
per_commitment_number: trusted_tx.commitment_number(),
per_commitment_point: trusted_tx.per_commitment_point(),
feerate_per_kw: trusted_tx.feerate_per_kw(),
feerate_per_kw: trusted_tx.negotiated_feerate_per_kw(),
htlc: htlc.clone(),
preimage: self.preimage.clone(),
counterparty_sig: *counterparty_sig,
Expand Down Expand Up @@ -1893,7 +1893,7 @@ mod tests {
commitment_txid: trusted_tx.txid(),
per_commitment_number: trusted_tx.commitment_number(),
per_commitment_point: trusted_tx.per_commitment_point(),
feerate_per_kw: trusted_tx.feerate_per_kw(),
feerate_per_kw: trusted_tx.negotiated_feerate_per_kw(),
htlc,
preimage: Some(preimage),
counterparty_sig: commitment_tx.counterparty_htlc_sigs[0].clone(),
Expand Down Expand Up @@ -1930,7 +1930,7 @@ mod tests {
commitment_txid: trusted_tx.txid(),
per_commitment_number: trusted_tx.commitment_number(),
per_commitment_point: trusted_tx.per_commitment_point(),
feerate_per_kw: trusted_tx.feerate_per_kw(),
feerate_per_kw: trusted_tx.negotiated_feerate_per_kw(),
htlc,
preimage: None,
counterparty_sig: commitment_tx.counterparty_htlc_sigs[0].clone(),
Expand Down
95 changes: 72 additions & 23 deletions lightning/src/ln/chan_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@ pub const ANCHOR_INPUT_WITNESS_WEIGHT: u64 = 114;
#[cfg(not(feature = "grind_signatures"))]
pub const ANCHOR_INPUT_WITNESS_WEIGHT: u64 = 115;

/// The P2A scriptpubkey
pub const P2A_SCRIPT: &[u8] = &[0x51, 0x02, 0x4e, 0x73];

/// The maximum value of the P2A anchor
pub const P2A_MAX_VALUE: u64 = 240;

/// The upper bound weight of an HTLC timeout input from a commitment transaction with anchor
/// outputs.
pub const HTLC_TIMEOUT_INPUT_ANCHOR_WITNESS_WEIGHT: u64 = 288;
Expand Down Expand Up @@ -804,18 +810,30 @@ pub(crate) fn make_funding_redeemscript_from_slices(broadcaster_funding_key: &[u
///
/// Panics if htlc.transaction_output_index.is_none() (as such HTLCs do not appear in the
/// commitment transaction).
#[rustfmt::skip]
pub fn build_htlc_transaction(commitment_txid: &Txid, feerate_per_kw: u32, contest_delay: u16, htlc: &HTLCOutputInCommitment, channel_type_features: &ChannelTypeFeatures, broadcaster_delayed_payment_key: &DelayedPaymentKey, revocation_key: &RevocationKey) -> Transaction {
let txins= vec![build_htlc_input(commitment_txid, htlc, channel_type_features)];

let mut txouts: Vec<TxOut> = Vec::new();
txouts.push(build_htlc_output(
feerate_per_kw, contest_delay, htlc, channel_type_features,
broadcaster_delayed_payment_key, revocation_key
));
pub fn build_htlc_transaction(
commitment_txid: &Txid, feerate_per_kw: u32, contest_delay: u16, htlc: &HTLCOutputInCommitment,
channel_type_features: &ChannelTypeFeatures,
broadcaster_delayed_payment_key: &DelayedPaymentKey, revocation_key: &RevocationKey,
) -> Transaction {
let txins = vec![build_htlc_input(commitment_txid, htlc, channel_type_features)];

let txouts: Vec<TxOut> = vec![build_htlc_output(
feerate_per_kw,
contest_delay,
htlc,
channel_type_features,
broadcaster_delayed_payment_key,
revocation_key,
)];

let version = if channel_type_features.supports_anchor_zero_fee_commitments() {
Version::non_standard(3)
} else {
Version::TWO
};

Transaction {
version: Version::TWO,
version,
lock_time: LockTime::from_consensus(if htlc.offered { htlc.cltv_expiry } else { 0 }),
input: txins,
output: txouts,
Expand Down Expand Up @@ -859,12 +877,13 @@ pub(crate) fn build_htlc_output(
}

/// Returns the witness required to satisfy and spend a HTLC input.
#[rustfmt::skip]
pub fn build_htlc_input_witness(
local_sig: &Signature, remote_sig: &Signature, preimage: &Option<PaymentPreimage>,
redeem_script: &Script, channel_type_features: &ChannelTypeFeatures,
) -> Witness {
let remote_sighash_type = if channel_type_features.supports_anchors_zero_fee_htlc_tx() {
let remote_sighash_type = if channel_type_features.supports_anchors_zero_fee_htlc_tx()
|| channel_type_features.supports_anchor_zero_fee_commitments()
{
EcdsaSighashType::SinglePlusAnyoneCanPay
} else {
EcdsaSighashType::All
Expand All @@ -873,7 +892,10 @@ pub fn build_htlc_input_witness(
let mut witness = Witness::new();
// First push the multisig dummy, note that due to BIP147 (NULLDUMMY) it must be a zero-length element.
witness.push(vec![]);
witness.push_ecdsa_signature(&BitcoinSignature { signature: *remote_sig, sighash_type: remote_sighash_type });
witness.push_ecdsa_signature(&BitcoinSignature {
signature: *remote_sig,
sighash_type: remote_sighash_type,
});
witness.push_ecdsa_signature(&BitcoinSignature::sighash_all(*local_sig));
if let Some(preimage) = preimage {
witness.push(preimage.0.to_vec());
Expand Down Expand Up @@ -1232,6 +1254,11 @@ impl<'a> DirectedChannelTransactionParameters<'a> {
pub fn channel_type_features(&self) -> &'a ChannelTypeFeatures {
&self.inner.channel_type_features
}

/// The value locked in the channel, denominated in satoshis.
pub fn channel_value_satoshis(&self) -> u64 {
self.inner.channel_value_satoshis
}
}

/// Information needed to build and sign a holder's commitment transaction.
Expand Down Expand Up @@ -1637,7 +1664,7 @@ impl CommitmentTransaction {
let outputs = Self::build_outputs_and_htlcs(&keys, to_broadcaster_value_sat, to_countersignatory_value_sat, &mut nondust_htlcs, channel_parameters);

let (obscured_commitment_transaction_number, txins) = Self::build_inputs(commitment_number, channel_parameters);
let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs);
let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs, channel_parameters);
let txid = transaction.compute_txid();
CommitmentTransaction {
commitment_number,
Expand Down Expand Up @@ -1691,6 +1718,8 @@ impl CommitmentTransaction {
// First rebuild the htlc outputs, note that `outputs` is now the same length as `self.nondust_htlcs`
let mut outputs = Self::build_htlc_outputs(keys, &self.nondust_htlcs, channel_parameters.channel_type_features());

let nondust_htlcs_value_sum_sat = self.nondust_htlcs.iter().map(|htlc| htlc.to_bitcoin_amount()).sum();

// Check that the HTLC outputs are sorted by value, script pubkey, and cltv expiry.
// Note that this only iterates if the length of `outputs` and `self.nondust_htlcs` is >= 2.
if (1..outputs.len()).into_iter().any(|i| Self::is_left_greater(i, &outputs, &self.nondust_htlcs)) {
Expand All @@ -1713,11 +1742,11 @@ impl CommitmentTransaction {
self.to_broadcaster_value_sat,
self.to_countersignatory_value_sat,
channel_parameters,
!self.nondust_htlcs.is_empty(),
nondust_htlcs_value_sum_sat,
insert_non_htlc_output
);

let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs);
let transaction = Self::make_transaction(obscured_commitment_transaction_number, txins, outputs, channel_parameters);
let txid = transaction.compute_txid();
let built_transaction = BuiltCommitmentTransaction {
transaction,
Expand All @@ -1727,9 +1756,14 @@ impl CommitmentTransaction {
}

#[rustfmt::skip]
fn make_transaction(obscured_commitment_transaction_number: u64, txins: Vec<TxIn>, outputs: Vec<TxOut>) -> Transaction {
fn make_transaction(obscured_commitment_transaction_number: u64, txins: Vec<TxIn>, outputs: Vec<TxOut>, channel_parameters: &DirectedChannelTransactionParameters) -> Transaction {
let version = if channel_parameters.channel_type_features().supports_anchor_zero_fee_commitments() {
Version::non_standard(3)
} else {
Version::TWO
};
Transaction {
version: Version::TWO,
version,
lock_time: LockTime::from_consensus(((0x20 as u32) << 8 * 3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32)),
input: txins,
output: outputs,
Expand All @@ -1747,7 +1781,8 @@ impl CommitmentTransaction {
// First build and sort the HTLC outputs.
// Also sort the HTLC output data in `nondust_htlcs` in the same order.
let mut outputs = Self::build_sorted_htlc_outputs(keys, nondust_htlcs, channel_parameters.channel_type_features());
let tx_has_htlc_outputs = !outputs.is_empty();

let nondust_htlcs_value_sum_sat = nondust_htlcs.iter().map(|htlc| htlc.to_bitcoin_amount()).sum();

// Initialize the transaction output indices; we will update them below when we
// add the non-htlc transaction outputs.
Expand Down Expand Up @@ -1784,7 +1819,7 @@ impl CommitmentTransaction {
to_broadcaster_value_sat,
to_countersignatory_value_sat,
channel_parameters,
tx_has_htlc_outputs,
nondust_htlcs_value_sum_sat,
insert_non_htlc_output
);

Expand All @@ -1797,7 +1832,7 @@ impl CommitmentTransaction {
to_broadcaster_value_sat: Amount,
to_countersignatory_value_sat: Amount,
channel_parameters: &DirectedChannelTransactionParameters,
tx_has_htlc_outputs: bool,
nondust_htlcs_value_sum_sat: Amount,
mut insert_non_htlc_output: F,
) where
F: FnMut(TxOut),
Expand All @@ -1807,6 +1842,7 @@ impl CommitmentTransaction {
let broadcaster_funding_key = &channel_parameters.broadcaster_pubkeys().funding_pubkey;
let channel_type = channel_parameters.channel_type_features();
let contest_delay = channel_parameters.contest_delay();
let tx_has_htlc_outputs = nondust_htlcs_value_sum_sat != Amount::ZERO;

if to_countersignatory_value_sat > Amount::ZERO {
let script = if channel_type.supports_anchors_zero_fee_htlc_tx() {
Expand Down Expand Up @@ -1849,6 +1885,16 @@ impl CommitmentTransaction {
});
}
}

if channel_type.supports_anchor_zero_fee_commitments() {
let channel_value_satoshis = Amount::from_sat(channel_parameters.channel_value_satoshis());
// These subtractions panic on underflow, but this should never happen
let trimmed_sum_sat = channel_value_satoshis - nondust_htlcs_value_sum_sat - to_broadcaster_value_sat - to_countersignatory_value_sat;
Comment on lines +1891 to +1892
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subtraction operation here could potentially panic if the sum of the values being subtracted exceeds channel_value_satoshis. While the comment acknowledges this shouldn't happen, it would be safer to use checked arithmetic or validate the constraint before performing the subtraction to prevent a potential DoS vulnerability in edge cases.

Consider using checked_sub or similar methods to handle this gracefully:

let trimmed_sum_sat = channel_value_satoshis
    .checked_sub(nondust_htlcs_value_sum_sat)
    .and_then(|result| result.checked_sub(to_broadcaster_value_sat))
    .and_then(|result| result.checked_sub(to_countersignatory_value_sat))
    .unwrap_or(Amount::ZERO); // Or handle the error case appropriately

This would make the code more robust against unexpected inputs while maintaining the same functionality.

Suggested change
// These subtractions panic on underflow, but this should never happen
let trimmed_sum_sat = channel_value_satoshis - nondust_htlcs_value_sum_sat - to_broadcaster_value_sat - to_countersignatory_value_sat;
// Use checked subtraction to avoid potential panics
let trimmed_sum_sat = channel_value_satoshis
.checked_sub(nondust_htlcs_value_sum_sat)
.and_then(|result| result.checked_sub(to_broadcaster_value_sat))
.and_then(|result| result.checked_sub(to_countersignatory_value_sat))
.unwrap_or_else(|| {
// This should never happen, but we handle it gracefully instead of panicking
debug_assert!(false, "Channel value calculation underflowed");
0
});

Spotted by Diamond

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.

insert_non_htlc_output(TxOut {
script_pubkey: ScriptBuf::from_bytes(P2A_SCRIPT.to_vec()),
value: cmp::min(Amount::from_sat(P2A_MAX_VALUE), trimmed_sum_sat),
});
}
}

#[rustfmt::skip]
Expand Down Expand Up @@ -1950,8 +1996,11 @@ impl CommitmentTransaction {
self.to_countersignatory_value_sat.to_sat()
}

/// The feerate paid per 1000-weight-unit in this commitment transaction.
pub fn feerate_per_kw(&self) -> u32 {
/// The feerate paid per 1000-weight-unit we negotiated with our
/// peer for this commitment transaction. Note that the actual
/// feerate of the commitment transaction may be higher than the
/// negotiated feerate.
pub fn negotiated_feerate_per_kw(&self) -> u32 {
self.feerate_per_kw
}

Expand Down
Loading
Loading