Set transaction locktime on malleable packages to discourage fee sniping
authorWilmer Paulino <wilmer@wilmerpaulino.com>
Tue, 28 Mar 2023 19:09:13 +0000 (12:09 -0700)
committerWilmer Paulino <wilmer@wilmerpaulino.com>
Tue, 28 Mar 2023 19:42:23 +0000 (12:42 -0700)
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
lightning/src/chain/package.rs
lightning/src/ln/functional_tests.rs

index 0ff0df59cf27c91ba39b77bfe5d346586e105e1a..c95aec9774a09481554ba5e89f6c1f29a97570c1 100644 (file)
@@ -558,7 +558,9 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
                        ) {
                                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)));
index 2ab37484148e48402674c68a8b2503961dcebe12..e8886e5a2aa49958fcac012529a3ae4160348230 100644 (file)
@@ -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<L: Deref, Signer: WriteableEcdsaChannelSigner>(
-               &self, onchain_handler: &mut OnchainTxHandler<Signer>, value: u64, destination_script: Script, logger: &L
+               &self, current_height: u32, onchain_handler: &mut OnchainTxHandler<Signer>, value: u64,
+               destination_script: Script, logger: &L
        ) -> Option<Transaction> 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,
index 265bd49ac92ff785ae961872cd6866d0aaaa845e..38a78ab65636ed861f1d3b5707304917f5df6b48 100644 (file)
@@ -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]);