From 68122bd09de5feca5538247f2533ec3b89ccf242 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 28 Mar 2023 12:09:13 -0700 Subject: [PATCH] Set transaction locktime on malleable packages to discourage fee sniping This only applies to all malleable packages on channels pre-dating anchors and malleables packages for counterparty commitments post-anchors. Malleables packages for holder commitments post-anchors should have their transaction locktime applied manually by the consumer of `BumpTransactionEvent::HTLCResolution` events. --- lightning/src/chain/onchaintx.rs | 4 +++- lightning/src/chain/package.rs | 6 +++--- lightning/src/ln/functional_tests.rs | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 0ff0df59c..c95aec977 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -558,7 +558,9 @@ impl OnchainTxHandler ) { assert!(new_feerate != 0); - let transaction = cached_request.finalize_malleable_package(self, output_value, self.destination_script.clone(), logger).unwrap(); + let transaction = cached_request.finalize_malleable_package( + cur_height, self, output_value, self.destination_script.clone(), logger + ).unwrap(); log_trace!(logger, "...with timer {} and feerate {}", new_timer.unwrap(), new_feerate); assert!(predicted_weight >= transaction.weight()); return Some((new_timer, new_feerate, OnchainClaim::Tx(transaction))); diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 2ab374841..e8886e5a2 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -434,7 +434,6 @@ impl PackageSolvingData { let chan_keys = TxCreationKeys::derive_new(&onchain_handler.secp_ctx, &outp.per_commitment_point, &outp.counterparty_delayed_payment_base_key, &outp.counterparty_htlc_base_key, &onchain_handler.signer.pubkeys().revocation_basepoint, &onchain_handler.signer.pubkeys().htlc_basepoint); let witness_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&outp.htlc, onchain_handler.opt_anchors(), &chan_keys.broadcaster_htlc_key, &chan_keys.countersignatory_htlc_key, &chan_keys.revocation_key); - bumped_tx.lock_time = PackedLockTime(outp.htlc.cltv_expiry); // Right now we don't aggregate time-locked transaction, if we do we should set lock_time before to avoid breaking hash computation if let Ok(sig) = onchain_handler.signer.sign_counterparty_htlc_transaction(&bumped_tx, i, &outp.htlc.amount_msat / 1000, &outp.per_commitment_point, &outp.htlc, &onchain_handler.secp_ctx) { let mut ser_sig = sig.serialize_der().to_vec(); ser_sig.push(EcdsaSighashType::All as u8); @@ -708,12 +707,13 @@ impl PackageTemplate { htlcs } pub(crate) fn finalize_malleable_package( - &self, onchain_handler: &mut OnchainTxHandler, value: u64, destination_script: Script, logger: &L + &self, current_height: u32, onchain_handler: &mut OnchainTxHandler, value: u64, + destination_script: Script, logger: &L ) -> Option where L::Target: Logger { debug_assert!(self.is_malleable()); let mut bumped_tx = Transaction { version: 2, - lock_time: PackedLockTime::ZERO, + lock_time: PackedLockTime(self.package_locktime(current_height)), input: vec![], output: vec![TxOut { script_pubkey: destination_script, diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 265bd49ac..38a78ab65 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -2843,7 +2843,7 @@ fn test_htlc_on_chain_success() { assert_eq!(commitment_spend.input.len(), 2); assert_eq!(commitment_spend.input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); assert_eq!(commitment_spend.input[1].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); - assert_eq!(commitment_spend.lock_time.0, 0); + assert_eq!(commitment_spend.lock_time.0, nodes[1].best_block_info().1 + 1); assert!(commitment_spend.output[0].script_pubkey.is_v0_p2wpkh()); // direct payment // We don't bother to check that B can claim the HTLC output on its commitment tx here as // we already checked the same situation with A. @@ -4699,7 +4699,7 @@ fn test_onchain_to_onchain_claim() { check_spends!(b_txn[0], commitment_tx[0]); assert_eq!(b_txn[0].input[0].witness.clone().last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); assert!(b_txn[0].output[0].script_pubkey.is_v0_p2wpkh()); // direct payment - assert_eq!(b_txn[0].lock_time.0, 0); // Success tx + assert_eq!(b_txn[0].lock_time.0, nodes[1].best_block_info().1 + 1); // Success tx check_closed_broadcast!(nodes[1], true); check_added_monitors!(nodes[1], 1); @@ -6860,7 +6860,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { if !revoked { assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); } else { - assert_eq!(timeout_tx[0].lock_time.0, 0); + assert_eq!(timeout_tx[0].lock_time.0, 12); } // We fail non-dust-HTLC 2 by broadcast of local timeout/revocation-claim tx mine_transaction(&nodes[0], &timeout_tx[0]); -- 2.39.5