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.

Author: Antoine Riard

lightning/src/chain/channelmonitor.rs

@@ -2511,8 +2511,10 @@ impl<Signer: Sign> 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(), true);
+					// 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));
@@ -2695,7 +2697,8 @@ impl<Signer: Sign> 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,
+					self.onchain_tx_handler.opt_anchors(), false
 				);
 				let justice_package = PackageTemplate::build_package(
 					htlc_txid, idx as u32, PackageSolvingData::RevokedOutput(revk_outp),

lightning/src/chain/package.rs

@@ -98,30 +98,36 @@ pub(crate) struct RevokedOutput {
 	weight: u64,
 	amount: u64,
 	on_counterparty_tx_csv: u16,
+	opt_anchors: Option<()>,
+	is_counterparty_balance_or_non_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, opt_anchors: bool, is_counterparty_balance: 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,
+			opt_anchors: if opt_anchors { Some(()) } else { None },
+			is_counterparty_balance_or_non_anchors: if is_counterparty_balance { Some(()) } else { None },
 		}
 	}
 }
 
 impl_writeable_tlv_based!(RevokedOutput, {
 	(0, per_commitment_point, required),
+	(1, is_counterparty_balance_or_non_anchors, option),
 	(2, counterparty_delayed_payment_base_key, required),
 	(4, counterparty_htlc_base_key, required),
 	(6, per_commitment_key, required),
 	(8, weight, required),
 	(10, amount, required),
 	(12, on_counterparty_tx_csv, required),
+	(14, opt_anchors, option),
 });
 
 /// A struct to describe a revoked offered output and corresponding information to generate a
@@ -785,18 +791,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 {
@@ -809,6 +804,20 @@ 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_or_non_anchors: None, .. }) => { (PackageMalleability::Malleable, true) },
+			PackageSolvingData::RevokedOutput(RevokedOutput { opt_anchors: Some(..), .. }) => { (PackageMalleability::Malleable, false) },
+			PackageSolvingData::RevokedOutput(RevokedOutput { opt_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 {
@@ -838,18 +847,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;
@@ -973,7 +971,7 @@ mod tests {
 			{
 				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, false, false))
 			}
 		}
 	}