Skip to content

Commit 6f9adfa

Browse files
author
Antoine Riard
committed
Remove aggregable flag from PackageTemplate constructor
1 parent b9aa902 commit 6f9adfa

File tree

3 files changed

+73
-83
lines changed

3 files changed

+73
-83
lines changed

‎lightning/src/chain/channelmonitor.rs

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2422,7 +2422,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
24222422
let commitment_package = PackageTemplate::build_package(
24232423
self.funding_info.0.txid.clone(), self.funding_info.0.index as u32,
24242424
PackageSolvingData::HolderFundingOutput(funding_output),
2425-
best_block_height, false, best_block_height,
2425+
best_block_height, best_block_height
24262426
);
24272427
self.onchain_tx_handler.update_claims_view_from_requests(
24282428
vec![commitment_package], best_block_height, best_block_height,
@@ -2604,9 +2604,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
26042604
for (idx, outp) in tx.output.iter().enumerate() {
26052605
if outp.script_pubkey == revokeable_p2wsh {
26062606
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);
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, height);
26102608
claimable_outpoints.push(justice_package);
26112609
to_counterparty_output_info =
26122610
Some((idx.try_into().expect("Txn can't have more than 2^32 outputs"), outp.value));
@@ -2624,7 +2622,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
26242622
to_counterparty_output_info);
26252623
}
26262624
let revk_htlc_outp = RevokedHTLCOutput::build(per_commitment_point, self.counterparty_commitment_params.counterparty_delayed_payment_base_key, self.counterparty_commitment_params.counterparty_htlc_base_key, per_commitment_key, htlc.amount_msat / 1000, htlc.clone(), self.onchain_tx_handler.channel_transaction_parameters.opt_anchors.is_some());
2627-
let justice_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp), htlc.cltv_expiry, true, height);
2625+
let justice_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, PackageSolvingData::RevokedHTLCOutput(revk_htlc_outp), htlc.cltv_expiry, height);
26282626
claimable_outpoints.push(justice_package);
26292627
}
26302628
}
@@ -2749,8 +2747,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
27492747
self.counterparty_commitment_params.counterparty_htlc_base_key,
27502748
htlc.clone(), self.onchain_tx_handler.opt_anchors()))
27512749
};
2752-
let aggregation = if !htlc.offered { false } else { true };
2753-
let counterparty_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, counterparty_htlc_outp, htlc.cltv_expiry,aggregation, 0);
2750+
let counterparty_package = PackageTemplate::build_package(commitment_txid, transaction_output_index, counterparty_htlc_outp, htlc.cltv_expiry, 0);
27542751
claimable_outpoints.push(counterparty_package);
27552752
}
27562753
}
@@ -2794,7 +2791,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
27942791
);
27952792
let justice_package = PackageTemplate::build_package(
27962793
htlc_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp),
2797-
height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, true, height
2794+
height + self.counterparty_commitment_params.on_counterparty_tx_csv as u32, height
27982795
);
27992796
claimable_outpoints.push(justice_package);
28002797
if outputs_to_watch.is_none() {
@@ -2817,11 +2814,11 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
28172814

28182815
for &(ref htlc, _, _) in holder_tx.htlc_outputs.iter() {
28192816
if let Some(transaction_output_index) = htlc.transaction_output_index {
2820-
let (htlc_output, aggregable) = if htlc.offered {
2817+
let htlc_output = if htlc.offered {
28212818
let htlc_output = HolderHTLCOutput::build_offered(
28222819
htlc.amount_msat, htlc.cltv_expiry, self.onchain_tx_handler.opt_anchors()
28232820
);
2824-
(htlc_output, false)
2821+
htlc_output
28252822
} else {
28262823
let payment_preimage = if let Some(preimage) = self.payment_preimages.get(&htlc.payment_hash) {
28272824
preimage.clone()
@@ -2832,12 +2829,12 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
28322829
let htlc_output = HolderHTLCOutput::build_accepted(
28332830
payment_preimage, htlc.amount_msat, self.onchain_tx_handler.opt_anchors()
28342831
);
2835-
(htlc_output, self.onchain_tx_handler.opt_anchors())
2832+
htlc_output
28362833
};
28372834
let htlc_package = PackageTemplate::build_package(
28382835
holder_tx.txid, transaction_output_index,
28392836
PackageSolvingData::HolderHTLCOutput(htlc_output),
2840-
htlc.cltv_expiry, aggregable, conf_height
2837+
htlc.cltv_expiry, conf_height
28412838
);
28422839
claim_requests.push(htlc_package);
28432840
}
@@ -3177,7 +3174,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
31773174
let should_broadcast = self.should_broadcast_holder_commitment_txn(logger);
31783175
if should_broadcast {
31793176
let funding_outp = HolderFundingOutput::build(self.funding_redeemscript.clone(), self.channel_value_satoshis, self.onchain_tx_handler.opt_anchors());
3180-
let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), self.best_block.height(), false, self.best_block.height());
3177+
let commitment_package = PackageTemplate::build_package(self.funding_info.0.txid.clone(), self.funding_info.0.index as u32, PackageSolvingData::HolderFundingOutput(funding_outp), self.best_block.height(), self.best_block.height());
31813178
claimable_outpoints.push(commitment_package);
31823179
self.pending_monitor_events.push(MonitorEvent::CommitmentTxConfirmed(self.funding_info.0));
31833180
let commitment_tx = self.onchain_tx_handler.get_fully_signed_holder_tx(&self.funding_redeemscript);

‎lightning/src/chain/package.rs

Lines changed: 49 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,24 @@ impl PackageSolvingData {
482482
};
483483
absolute_timelock
484484
}
485+
486+
fn map_output_type_flags(&self) -> (PackageMalleability, bool) {
487+
// Post-anchor, aggregation of outputs of different types is unsafe. See https://github.com/lightning/bolts/pull/803.
488+
let (malleability, aggregable) = match self {
489+
PackageSolvingData::RevokedOutput(RevokedOutput { is_counterparty_balance_on_anchors: Some(()), .. }) => { (PackageMalleability::Malleable, false) },
490+
PackageSolvingData::RevokedOutput(RevokedOutput { is_counterparty_balance_on_anchors: None, .. }) => { (PackageMalleability::Malleable, true) },
491+
PackageSolvingData::RevokedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
492+
PackageSolvingData::CounterpartyOfferedHTLCOutput(..) => { (PackageMalleability::Malleable, true) },
493+
PackageSolvingData::CounterpartyReceivedHTLCOutput(..) => { (PackageMalleability::Malleable, false) },
494+
PackageSolvingData::HolderHTLCOutput(ref outp) => if outp.opt_anchors() {
495+
(PackageMalleability::Malleable, outp.preimage.is_some())
496+
} else {
497+
(PackageMalleability::Untractable, false)
498+
},
499+
PackageSolvingData::HolderFundingOutput(..) => { (PackageMalleability::Untractable, false) },
500+
};
501+
(malleability, aggregable)
502+
}
485503
}
486504

487505
impl_writeable_tlv_based_enum!(PackageSolvingData, ;
@@ -494,8 +512,7 @@ impl_writeable_tlv_based_enum!(PackageSolvingData, ;
494512
);
495513

496514
/// A malleable package might be aggregated with other packages to save on fees.
497-
/// A untractable package has been counter-signed and aggregable will break cached counterparty
498-
/// signatures.
515+
/// A untractable package has been counter-signed and aggregable will break cached counterparty signatures.
499516
#[derive(Clone, PartialEq, Eq)]
500517
pub(crate) enum PackageMalleability {
501518
Malleable,
@@ -829,8 +846,8 @@ impl PackageTemplate {
829846
}).is_some()
830847
}
831848

832-
pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, soonest_conf_deadline: u32, aggregable: bool, height_original: u32) -> Self {
833-
let (malleability, aggregable) = Self::map_output_type_flags(&input_solving_data);
849+
pub (crate) fn build_package(txid: Txid, vout: u32, input_solving_data: PackageSolvingData, soonest_conf_deadline: u32, height_original: u32) -> Self {
850+
let (malleability, aggregable) = PackageSolvingData::map_output_type_flags(&input_solving_data);
834851
let mut inputs = Vec::with_capacity(1);
835852
inputs.push((BitcoinOutPoint { txid, vout }, input_solving_data));
836853
PackageTemplate {
@@ -843,19 +860,6 @@ impl PackageTemplate {
843860
height_original,
844861
}
845862
}
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-
}
859863
}
860864

861865
impl Writeable for PackageTemplate {
@@ -885,7 +889,7 @@ impl Readable for PackageTemplate {
885889
inputs.push((outpoint, rev_outp));
886890
}
887891
let (malleability, aggregable) = if let Some((_, lead_input)) = inputs.first() {
888-
Self::map_output_type_flags(&lead_input)
892+
PackageSolvingData::map_output_type_flags(&lead_input)
889893
} else { return Err(DecodeError::InvalidValue); };
890894
let mut soonest_conf_deadline = 0;
891895
let mut feerate_previous = 0;
@@ -1073,8 +1077,8 @@ mod tests {
10731077
let secp_ctx = Secp256k1::new();
10741078
let revk_outp = dumb_revk_output!(secp_ctx, false);
10751079

1076-
let mut package_one_hundred = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, true, 100);
1077-
let package_two_hundred = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, true, 200);
1080+
let mut package_one_hundred = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, 100);
1081+
let package_two_hundred = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, 200);
10781082
package_one_hundred.merge_package(package_two_hundred);
10791083
}
10801084

@@ -1086,8 +1090,8 @@ mod tests {
10861090
let revk_outp = dumb_revk_output!(secp_ctx, false);
10871091
let htlc_outp = dumb_htlc_output!();
10881092

1089-
let mut untractable_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, true, 100);
1090-
let malleable_package = PackageTemplate::build_package(txid, 1, htlc_outp.clone(), 1000, true, 100);
1093+
let mut untractable_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, 100);
1094+
let malleable_package = PackageTemplate::build_package(txid, 1, htlc_outp.clone(), 1000, 100);
10911095
untractable_package.merge_package(malleable_package);
10921096
}
10931097

@@ -1099,8 +1103,8 @@ mod tests {
10991103
let htlc_outp = dumb_htlc_output!();
11001104
let revk_outp = dumb_revk_output!(secp_ctx, false);
11011105

1102-
let mut malleable_package = PackageTemplate::build_package(txid, 0, htlc_outp.clone(), 1000, true, 100);
1103-
let untractable_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, true, 100);
1106+
let mut malleable_package = PackageTemplate::build_package(txid, 0, htlc_outp.clone(), 1000, 100);
1107+
let untractable_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, 100);
11041108
malleable_package.merge_package(untractable_package);
11051109
}
11061110

@@ -1109,10 +1113,11 @@ mod tests {
11091113
fn test_package_noaggregation_to() {
11101114
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
11111115
let secp_ctx = Secp256k1::new();
1112-
let revk_outp = dumb_revk_output!(secp_ctx, true);
1116+
let revk_outp = dumb_revk_output!(secp_ctx, false);
1117+
let revk_outp_counterparty_balance = dumb_revk_output!(secp_ctx, true);
11131118

1114-
let mut noaggregation_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, false, 100);
1115-
let aggregation_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, true, 100);
1119+
let mut noaggregation_package = PackageTemplate::build_package(txid, 0, revk_outp_counterparty_balance.clone(), 1000, 100);
1120+
let aggregation_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, 100);
11161121
noaggregation_package.merge_package(aggregation_package);
11171122
}
11181123

@@ -1121,10 +1126,11 @@ mod tests {
11211126
fn test_package_noaggregation_from() {
11221127
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
11231128
let secp_ctx = Secp256k1::new();
1124-
let revk_outp = dumb_revk_output!(secp_ctx, true);
1129+
let revk_outp = dumb_revk_output!(secp_ctx, false);
1130+
let revk_outp_counterparty_balance = dumb_revk_output!(secp_ctx, true);
11251131

1126-
let mut aggregation_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, true, 100);
1127-
let noaggregation_package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, false, 100);
1132+
let mut aggregation_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, 100);
1133+
let noaggregation_package = PackageTemplate::build_package(txid, 1, revk_outp_counterparty_balance.clone(), 1000, 100);
11281134
aggregation_package.merge_package(noaggregation_package);
11291135
}
11301136

@@ -1135,9 +1141,9 @@ mod tests {
11351141
let secp_ctx = Secp256k1::new();
11361142
let revk_outp = dumb_revk_output!(secp_ctx, false);
11371143

1138-
let mut empty_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, true, 100);
1144+
let mut empty_package = PackageTemplate::build_package(txid, 0, revk_outp.clone(), 1000, 100);
11391145
empty_package.inputs = vec![];
1140-
let package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, true, 100);
1146+
let package = PackageTemplate::build_package(txid, 1, revk_outp.clone(), 1000, 100);
11411147
empty_package.merge_package(package);
11421148
}
11431149

@@ -1149,8 +1155,8 @@ mod tests {
11491155
let revk_outp = dumb_revk_output!(secp_ctx, false);
11501156
let counterparty_outp = dumb_counterparty_output!(secp_ctx, 0, false);
11511157

1152-
let mut revoked_package = PackageTemplate::build_package(txid, 0, revk_outp, 1000, true, 100);
1153-
let counterparty_package = PackageTemplate::build_package(txid, 1, counterparty_outp, 1000, true, 100);
1158+
let mut revoked_package = PackageTemplate::build_package(txid, 0, revk_outp, 1000, 100);
1159+
let counterparty_package = PackageTemplate::build_package(txid, 1, counterparty_outp, 1000, 100);
11541160
revoked_package.merge_package(counterparty_package);
11551161
}
11561162

@@ -1162,9 +1168,9 @@ mod tests {
11621168
let revk_outp_two = dumb_revk_output!(secp_ctx, false);
11631169
let revk_outp_three = dumb_revk_output!(secp_ctx, false);
11641170

1165-
let mut package_one = PackageTemplate::build_package(txid, 0, revk_outp_one, 1000, true, 100);
1166-
let package_two = PackageTemplate::build_package(txid, 1, revk_outp_two, 1000, true, 100);
1167-
let package_three = PackageTemplate::build_package(txid, 2, revk_outp_three, 1000, true, 100);
1171+
let mut package_one = PackageTemplate::build_package(txid, 0, revk_outp_one, 1000, 100);
1172+
let package_two = PackageTemplate::build_package(txid, 1, revk_outp_two, 1000, 100);
1173+
let package_three = PackageTemplate::build_package(txid, 2, revk_outp_three, 1000, 100);
11681174

11691175
package_one.merge_package(package_two);
11701176
package_one.merge_package(package_three);
@@ -1187,7 +1193,7 @@ mod tests {
11871193
let txid = Txid::from_hex("c2d4449afa8d26140898dd54d3390b057ba2a5afcf03ba29d7dc0d8b9ffe966e").unwrap();
11881194
let htlc_outp_one = dumb_htlc_output!();
11891195

1190-
let mut package_one = PackageTemplate::build_package(txid, 0, htlc_outp_one, 1000, true, 100);
1196+
let mut package_one = PackageTemplate::build_package(txid, 0, htlc_outp_one, 1000, 100);
11911197
let ret_split = package_one.split_package(&BitcoinOutPoint { txid, vout: 0});
11921198
assert!(ret_split.is_none());
11931199
}
@@ -1198,7 +1204,7 @@ mod tests {
11981204
let secp_ctx = Secp256k1::new();
11991205
let revk_outp = dumb_revk_output!(secp_ctx, false);
12001206

1201-
let mut package = PackageTemplate::build_package(txid, 0, revk_outp, 1000, true, 100);
1207+
let mut package = PackageTemplate::build_package(txid, 0, revk_outp, 1000, 100);
12021208
assert_eq!(package.timer(), 100);
12031209
package.set_timer(101);
12041210
assert_eq!(package.timer(), 101);
@@ -1210,7 +1216,7 @@ mod tests {
12101216
let secp_ctx = Secp256k1::new();
12111217
let counterparty_outp = dumb_counterparty_output!(secp_ctx, 1_000_000, false);
12121218

1213-
let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000, true, 100);
1219+
let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000, 100);
12141220
assert_eq!(package.package_amount(), 1000);
12151221
}
12161222

@@ -1224,22 +1230,22 @@ mod tests {
12241230

12251231
{
12261232
let revk_outp = dumb_revk_output!(secp_ctx, false);
1227-
let package = PackageTemplate::build_package(txid, 0, revk_outp, 0, true, 100);
1233+
let package = PackageTemplate::build_package(txid, 0, revk_outp, 0, 100);
12281234
assert_eq!(package.package_weight(&Script::new()), weight_sans_output + WEIGHT_REVOKED_OUTPUT as usize);
12291235
}
12301236

12311237
{
12321238
for &opt_anchors in [false, true].iter() {
12331239
let counterparty_outp = dumb_counterparty_output!(secp_ctx, 1_000_000, opt_anchors);
1234-
let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000, true, 100);
1240+
let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000, 100);
12351241
assert_eq!(package.package_weight(&Script::new()), weight_sans_output + weight_received_htlc(opt_anchors) as usize);
12361242
}
12371243
}
12381244

12391245
{
12401246
for &opt_anchors in [false, true].iter() {
12411247
let counterparty_outp = dumb_counterparty_offered_output!(secp_ctx, 1_000_000, opt_anchors);
1242-
let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000, true, 100);
1248+
let package = PackageTemplate::build_package(txid, 0, counterparty_outp, 1000, 100);
12431249
assert_eq!(package.package_weight(&Script::new()), weight_sans_output + weight_offered_htlc(opt_anchors) as usize);
12441250
}
12451251
}

0 commit comments

Comments
 (0)