Skip to content

Commit

Permalink
Anchor: do not aggregate claim of revoked output
Browse files Browse the repository at this point in the history 
See lightning/bolts#803

This protect the justice claim of counterparty revoked output. As
otherwise if the all the revoked outputs claims are batched in a
single transaction, low-feerate HTLCs transactions can delay our
honest justice claim transaction until BREAKDOWN_TIMEOUT expires.
  • Loading branch information
@optout21
Antoine Riard authored and optout21 committed Jul 24, 2023
1 parent 8676b5c commit a3c32d6
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 43 deletions.
9 changes: 6 additions & 3 deletions lightning/src/chain/channelmonitor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2614,8 +2614,10 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
// First, process non-htlc outputs (to_holder & to_counterparty)
for (idx, outp) in tx.output.iter().enumerate() {
if outp.script_pubkey == revokeable_p2wsh {
let revk_outp = RevokedOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, outp.value, self.counterparty_commitment_params.on_counterparty_tx_csv);
let justice_package = PackageTemplate::build_package(commitment_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp), height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, true, height);
let revk_outp = RevokedOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, outp.value, self.counterparty_commitment_params.on_counterparty_tx_csv, self.onchain_tx_handler.opt_anchors());
// Post-anchor, aggregation of outputs of different types is unsafe. See https://github.com/lightning/bolts/pull/803.
let aggregation = if self.onchain_tx_handler.opt_anchors() { false } else { true };
let justice_package = PackageTemplate::build_package(commitment_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp), height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, aggregation, height);
claimable_outpoints.push(justice_package);
to_counterparty_output_info =
Some((idx.try_into().expect("Txn can't have more than 2^32 outputs"), outp.value));
Expand Down Expand Up @@ -2798,7 +2800,8 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
let revk_outp = RevokedOutput::build(
per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key,
self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key,
tx.output[idx].value, self.counterparty_commitment_params.on_counterparty_tx_csv
tx.output[idx].value, self.counterparty_commitment_params.on_counterparty_tx_csv,
false
);
let justice_package = PackageTemplate::build_package(
htlc_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp),
Expand Down
74 changes: 34 additions & 40 deletions lightning/src/chain/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,20 @@ pub(crate) struct RevokedOutput {
weight: u64,
amount: u64,
on_counterparty_tx_csv: u16,
is_counterparty_balance_on_anchors: Option<()>,
}

impl RevokedOutput {
pub(crate) fn build(per_commitment_point: PublicKey, counterparty_delayed_payment_base_key: PublicKey, counterparty_htlc_base_key: PublicKey, per_commitment_key: SecretKey, amount: u64, on_counterparty_tx_csv: u16) -> Self {
pub(crate) fn build(per_commitment_point: PublicKey, counterparty_delayed_payment_base_key: PublicKey, counterparty_htlc_base_key: PublicKey, per_commitment_key: SecretKey, amount: u64, on_counterparty_tx_csv: u16, is_counterparty_balance_on_anchors: bool) -> Self {
RevokedOutput {
per_commitment_point,
counterparty_delayed_payment_base_key,
counterparty_htlc_base_key,
per_commitment_key,
weight: WEIGHT_REVOKED_OUTPUT,
amount,
on_counterparty_tx_csv
on_counterparty_tx_csv,
is_counterparty_balance_on_anchors: if is_counterparty_balance_on_anchors { Some(()) } else { None }
}
}
}
Expand All @@ -122,6 +124,7 @@ impl_writeable_tlv_based!(RevokedOutput, {
(8, weight, required),
(10, amount, required),
(12, on_counterparty_tx_csv, required),
(14, is_counterparty_balance_on_anchors, option)
});

/// A struct to describe a revoked offered output and corresponding information to generate a
Expand Down Expand Up @@ -827,18 +830,7 @@ impl PackageTemplate {
}

pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, soonest_conf_deadline: u32, aggregable: bool, height_original: u32) -> Self {
let malleability = match input_solving_data {
PackageSolvingData::RevokedOutput(..) => PackageMalleability::Malleable,
PackageSolvingData::RevokedHTLCOutput(..) => PackageMalleability::Malleable,
PackageSolvingData::CounterpartyOfferedHTLCOutput(..) => PackageMalleability::Malleable,
PackageSolvingData::CounterpartyReceivedHTLCOutput(..) => PackageMalleability::Malleable,
PackageSolvingData::HolderHTLCOutput(ref outp) => if outp.opt_anchors() {
PackageMalleability::Malleable
} else {
PackageMalleability::Untractable
},
PackageSolvingData::HolderFundingOutput(..) => PackageMalleability::Untractable,
};
let (malleability, aggregable) = Self::map_output_type_flags(&input_solving_data);
let mut inputs = Vec::with_capacity(1);
inputs.push((BitcoinOutPoint { txid, vout }, input_solving_data));
PackageTemplate {
Expand All @@ -851,6 +843,19 @@ impl PackageTemplate {
height_original,
}
}

fn map_output_type_flags(input_solving_data: &PackageSolvingData) -> (PackageMalleability, bool) {
let (malleability, aggregable) = match input_solving_data {
PackageSolvingData::RevokedOutput(RevokedOutput { is_counterparty_balance_on_anchors: Some(()), .. }) => { (PackageMalleability::Malleable, false) },
PackageSolvingData::RevokedOutput(RevokedOutput { is_counterparty_balance_on_anchors: None, .. }) => { (PackageMalleability::Malleable, true) },
PackageSolvingData::RevokedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
PackageSolvingData::CounterpartyOfferedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
PackageSolvingData::CounterpartyReceivedHTLCOutput(..) => { (PackageMalleability::Malleable, false) },
PackageSolvingData::HolderHTLCOutput(..) => { (PackageMalleability::Untractable, false) },
PackageSolvingData::HolderFundingOutput(..) => { (PackageMalleability::Untractable, false) },
};
(malleability, aggregable)
}
}

impl Writeable for PackageTemplate {
Expand Down Expand Up @@ -880,18 +885,7 @@ impl Readable for PackageTemplate {
inputs.push((outpoint, rev_outp));
}
let (malleability, aggregable) = if let Some((_, lead_input)) = inputs.first() {
match lead_input {
PackageSolvingData::RevokedOutput(..) => { (PackageMalleability::Malleable, true) },
PackageSolvingData::RevokedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
PackageSolvingData::CounterpartyOfferedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
PackageSolvingData::CounterpartyReceivedHTLCOutput(..) => { (PackageMalleability::Malleable, false) },
PackageSolvingData::HolderHTLCOutput(ref outp) => if outp.opt_anchors() {
(PackageMalleability::Malleable, outp.preimage.is_some())
} else {
(PackageMalleability::Untractable, false)
},
PackageSolvingData::HolderFundingOutput(..) => { (PackageMalleability::Untractable, false) },
}
Self::map_output_type_flags(&lead_input)
} else { return Err(DecodeError::InvalidValue); };
let mut soonest_conf_deadline = 0;
let mut feerate_previous = 0;
Expand Down Expand Up @@ -1029,11 +1023,11 @@ mod tests {
use bitcoin::secp256k1::Secp256k1;

macro_rules! dumb_revk_output {
($secp_ctx: expr) => {
($secp_ctx: expr, $is_counterparty_balance_on_anchors: expr) => {
{
let dumb_scalar = SecretKey::from_slice(&hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap();
let dumb_point = PublicKey::from_secret_key(&$secp_ctx, &dumb_scalar);
PackageSolvingData::RevokedOutput(RevokedOutput::build(dumb_point, dumb_point, dumb_point, dumb_scalar, 0, 0))
PackageSolvingData::RevokedOutput(RevokedOutput::build(dumb_point, dumb_point, dumb_point, dumb_scalar, 0, 0, $is_counterparty_balance_on_anchors))
}
}
}
Expand Down Expand Up @@ -1077,7 +1071,7 @@ mod tests {
fn test_package_differing_heights() {
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
let secp_ctx = Secp256k1::new();
let revk_outp = dumb_revk_output!(secp_ctx);
let revk_outp = dumb_revk_output!(secp_ctx, false);

let mut package_one_hundred = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, true, 100);
let package_two_hundred = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, true, 200);
Expand All @@ -1089,7 +1083,7 @@ mod tests {
fn test_package_untractable_merge_to() {
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
let secp_ctx = Secp256k1::new();
let revk_outp = dumb_revk_output!(secp_ctx);
let revk_outp = dumb_revk_output!(secp_ctx, false);
let htlc_outp = dumb_htlc_output!();

let mut untractable_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, true, 100);
Expand All @@ -1103,7 +1097,7 @@ mod tests {
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
let secp_ctx = Secp256k1::new();
let htlc_outp = dumb_htlc_output!();
let revk_outp = dumb_revk_output!(secp_ctx);
let revk_outp = dumb_revk_output!(secp_ctx, false);

let mut malleable_package = PackageTemplate::build_package(txid, 0, htlc_outp.clone(), 1000, true, 100);
let untractable_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, true, 100);
Expand All @@ -1115,7 +1109,7 @@ mod tests {
fn test_package_noaggregation_to() {
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
let secp_ctx = Secp256k1::new();
let revk_outp = dumb_revk_output!(secp_ctx);
let revk_outp = dumb_revk_output!(secp_ctx, true);

let mut noaggregation_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, false, 100);
let aggregation_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, true, 100);
Expand All @@ -1127,7 +1121,7 @@ mod tests {
fn test_package_noaggregation_from() {
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
let secp_ctx = Secp256k1::new();
let revk_outp = dumb_revk_output!(secp_ctx);
let revk_outp = dumb_revk_output!(secp_ctx, true);

let mut aggregation_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, true, 100);
let noaggregation_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, false, 100);
Expand All @@ -1139,7 +1133,7 @@ mod tests {
fn test_package_empty() {
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
let secp_ctx = Secp256k1::new();
let revk_outp = dumb_revk_output!(secp_ctx);
let revk_outp = dumb_revk_output!(secp_ctx, false);

let mut empty_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, true, 100);
empty_package.inputs = vec![];
Expand All @@ -1152,7 +1146,7 @@ mod tests {
fn test_package_differing_categories() {
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
let secp_ctx = Secp256k1::new();
let revk_outp = dumb_revk_output!(secp_ctx);
let revk_outp = dumb_revk_output!(secp_ctx, false);
let counterparty_outp = dumb_counterparty_output!(secp_ctx, 0, false);

let mut revoked_package = PackageTemplate::build_package(txid, 0, revk_outp, 1000, true, 100);
Expand All @@ -1164,9 +1158,9 @@ mod tests {
fn test_package_split_malleable() {
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
let secp_ctx = Secp256k1::new();
let revk_outp_one = dumb_revk_output!(secp_ctx);
let revk_outp_two = dumb_revk_output!(secp_ctx);
let revk_outp_three = dumb_revk_output!(secp_ctx);
let revk_outp_one = dumb_revk_output!(secp_ctx, false);
let revk_outp_two = dumb_revk_output!(secp_ctx, false);
let revk_outp_three = dumb_revk_output!(secp_ctx, false);

let mut package_one = PackageTemplate::build_package(txid, 0, revk_outp_one, 1000, true, 100);
let package_two = PackageTemplate::build_package(txid, 1, revk_outp_two, 1000, true, 100);
Expand Down Expand Up @@ -1202,7 +1196,7 @@ mod tests {
fn test_package_timer() {
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
let secp_ctx = Secp256k1::new();
let revk_outp = dumb_revk_output!(secp_ctx);
let revk_outp = dumb_revk_output!(secp_ctx, false);

let mut package = PackageTemplate::build_package(txid, 0, revk_outp, 1000, true, 100);
assert_eq!(package.timer(), 100);
Expand All @@ -1229,7 +1223,7 @@ mod tests {
let weight_sans_output = (4 + 4 + 1 + 36 + 4 + 1 + 1 + 8 + 1) * WITNESS_SCALE_FACTOR + 2;

{
let revk_outp = dumb_revk_output!(secp_ctx);
let revk_outp = dumb_revk_output!(secp_ctx, false);
let package = PackageTemplate::build_package(txid, 0, revk_outp, 0, true, 100);
assert_eq!(package.package_weight(&Script::new()), weight_sans_output + WEIGHT_REVOKED_OUTPUT as usize);
}
Expand Down

0 comments on commit a3c32d6

Please sign in to comment.