Skip to content

Commit ba059c5

Browse files
Antoine RiardAntoine Riard
Antoine Riard
authored and
Antoine Riard
committed
Anchor: do not aggregate claim of revoked output
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.
1 parent 56b0c96 commit ba059c5

File tree

2 files changed

+40
-43
lines changed

2 files changed

+40
-43
lines changed

‎lightning/src/chain/channelmonitor.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2603,8 +2603,10 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
26032603
// First, process non-htlc outputs (to_holder & to_counterparty)
26042604
for (idx, outp) in tx.output.iter().enumerate() {
26052605
if outp.script_pubkey == revokeable_p2wsh {
2606-
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);
2607-
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);
2606+
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());
2607+
// Post-anchor, aggregation of outputs of different types is unsafe. See https://github.com/lightning/bolts/pull/803.
2608+
let aggregation = if self.onchain_tx_handler.opt_anchors() { false } else { true };
2609+
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);
26082610
claimable_outpoints.push(justice_package);
26092611
to_counterparty_output_info =
26102612
Some((idx.try_into().expect("Txn can't have more than 2^32 outputs"), outp.value));
@@ -2787,7 +2789,8 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
27872789
let revk_outp = RevokedOutput::build(
27882790
per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key,
27892791
self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key,
2790-
tx.output[idx].value, self.counterparty_commitment_params.on_counterparty_tx_csv
2792+
tx.output[idx].value, self.counterparty_commitment_params.on_counterparty_tx_csv,
2793+
false
27912794
);
27922795
let justice_package = PackageTemplate::build_package(
27932796
htlc_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp),

‎lightning/src/chain/package.rs

Lines changed: 34 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -98,24 +98,27 @@ pub(crate) struct RevokedOutput {
9898
weight: u64,
9999
amount: u64,
100100
on_counterparty_tx_csv: u16,
101+
is_counterparty_balance_on_anchors: Option<()>,
101102
}
102103

103104
impl RevokedOutput {
104-
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 {
105+
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 {
105106
RevokedOutput {
106107
per_commitment_point,
107108
counterparty_delayed_payment_base_key,
108109
counterparty_htlc_base_key,
109110
per_commitment_key,
110111
weight: WEIGHT_REVOKED_OUTPUT,
111112
amount,
112-
on_counterparty_tx_csv
113+
on_counterparty_tx_csv,
114+
is_counterparty_balance_on_anchors: if is_counterparty_balance_on_anchors { Some(()) } else { None }
113115
}
114116
}
115117
}
116118

117119
impl_writeable_tlv_based!(RevokedOutput, {
118120
(0, per_commitment_point, required),
121+
(1, is_counterparty_balance_on_anchors, required),
119122
(2, counterparty_delayed_payment_base_key, required),
120123
(4, counterparty_htlc_base_key, required),
121124
(6, per_commitment_key, required),
@@ -827,18 +830,7 @@ impl PackageTemplate {
827830
}
828831

829832
pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, soonest_conf_deadline: u32, aggregable: bool, height_original: u32) -> Self {
830-
let malleability = match input_solving_data {
831-
PackageSolvingData::RevokedOutput(..) => PackageMalleability::Malleable,
832-
PackageSolvingData::RevokedHTLCOutput(..) => PackageMalleability::Malleable,
833-
PackageSolvingData::CounterpartyOfferedHTLCOutput(..) => PackageMalleability::Malleable,
834-
PackageSolvingData::CounterpartyReceivedHTLCOutput(..) => PackageMalleability::Malleable,
835-
PackageSolvingData::HolderHTLCOutput(ref outp) => if outp.opt_anchors() {
836-
PackageMalleability::Malleable
837-
} else {
838-
PackageMalleability::Untractable
839-
},
840-
PackageSolvingData::HolderFundingOutput(..) => PackageMalleability::Untractable,
841-
};
833+
let (malleability, aggregable) = Self::map_output_type_flags(&input_solving_data);
842834
let mut inputs = Vec::with_capacity(1);
843835
inputs.push((BitcoinOutPoint { txid, vout }, input_solving_data));
844836
PackageTemplate {
@@ -851,6 +843,19 @@ impl PackageTemplate {
851843
height_original,
852844
}
853845
}
846+
847+
fn map_output_type_flags(input_solving_data: &PackageSolvingData) -> (PackageMalleability, bool) {
848+
let (malleability, aggregable) = match input_solving_data {
849+
PackageSolvingData::RevokedOutput(RevokedOutput { is_counterparty_balance_on_anchors: Some(()), .. }) => { (PackageMalleability::Malleable, false) },
850+
PackageSolvingData::RevokedOutput(RevokedOutput { is_counterparty_balance_on_anchors: None, .. }) => { (PackageMalleability::Malleable, true) },
851+
PackageSolvingData::RevokedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
852+
PackageSolvingData::CounterpartyOfferedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
853+
PackageSolvingData::CounterpartyReceivedHTLCOutput(..) => { (PackageMalleability::Malleable, false) },
854+
PackageSolvingData::HolderHTLCOutput(..) => { (PackageMalleability::Untractable, false) },
855+
PackageSolvingData::HolderFundingOutput(..) => { (PackageMalleability::Untractable, false) },
856+
};
857+
(malleability, aggregable)
858+
}
854859
}
855860

856861
impl Writeable for PackageTemplate {
@@ -880,18 +885,7 @@ impl Readable for PackageTemplate {
880885
inputs.push((outpoint, rev_outp));
881886
}
882887
let (malleability, aggregable) = if let Some((_, lead_input)) = inputs.first() {
883-
match lead_input {
884-
PackageSolvingData::RevokedOutput(..) => { (PackageMalleability::Malleable, true) },
885-
PackageSolvingData::RevokedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
886-
PackageSolvingData::CounterpartyOfferedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
887-
PackageSolvingData::CounterpartyReceivedHTLCOutput(..) => { (PackageMalleability::Malleable, false) },
888-
PackageSolvingData::HolderHTLCOutput(ref outp) => if outp.opt_anchors() {
889-
(PackageMalleability::Malleable, outp.preimage.is_some())
890-
} else {
891-
(PackageMalleability::Untractable, false)
892-
},
893-
PackageSolvingData::HolderFundingOutput(..) => { (PackageMalleability::Untractable, false) },
894-
}
888+
Self::map_output_type_flags(&lead_input)
895889
} else { return Err(DecodeError::InvalidValue); };
896890
let mut soonest_conf_deadline = 0;
897891
let mut feerate_previous = 0;
@@ -1029,11 +1023,11 @@ mod tests {
10291023
use bitcoin::secp256k1::Secp256k1;
10301024

10311025
macro_rules! dumb_revk_output {
1032-
($secp_ctx: expr) => {
1026+
($secp_ctx: expr, $is_counterparty_balance_on_anchors: expr) => {
10331027
{
10341028
let dumb_scalar = SecretKey::from_slice(&hex::decode("0101010101010101010101010101010101010101010101010101010101010101").unwrap()[..]).unwrap();
10351029
let dumb_point = PublicKey::from_secret_key(&$secp_ctx, &dumb_scalar);
1036-
PackageSolvingData::RevokedOutput(RevokedOutput::build(dumb_point, dumb_point, dumb_point, dumb_scalar, 0, 0))
1030+
PackageSolvingData::RevokedOutput(RevokedOutput::build(dumb_point, dumb_point, dumb_point, dumb_scalar, 0, 0, $is_counterparty_balance_on_anchors))
10371031
}
10381032
}
10391033
}
@@ -1077,7 +1071,7 @@ mod tests {
10771071
fn test_package_differing_heights() {
10781072
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
10791073
let secp_ctx = Secp256k1::new();
1080-
let revk_outp = dumb_revk_output!(secp_ctx);
1074+
let revk_outp = dumb_revk_output!(secp_ctx, false);
10811075

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

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

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

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

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

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

11581152
let mut revoked_package = PackageTemplate::build_package(txid, 0, revk_outp, 1000, true, 100);
@@ -1164,9 +1158,9 @@ mod tests {
11641158
fn test_package_split_malleable() {
11651159
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
11661160
let secp_ctx = Secp256k1::new();
1167-
let revk_outp_one = dumb_revk_output!(secp_ctx);
1168-
let revk_outp_two = dumb_revk_output!(secp_ctx);
1169-
let revk_outp_three = dumb_revk_output!(secp_ctx);
1161+
let revk_outp_one = dumb_revk_output!(secp_ctx, false);
1162+
let revk_outp_two = dumb_revk_output!(secp_ctx, false);
1163+
let revk_outp_three = dumb_revk_output!(secp_ctx, false);
11701164

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

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

12311225
{
1232-
let revk_outp = dumb_revk_output!(secp_ctx);
1226+
let revk_outp = dumb_revk_output!(secp_ctx, false);
12331227
let package = PackageTemplate::build_package(txid, 0, revk_outp, 0, true, 100);
12341228
assert_eq!(package.package_weight(&Script::new()), weight_sans_output + WEIGHT_REVOKED_OUTPUT as usize);
12351229
}

0 commit comments

Comments
 (0)