Use correct input sequence for HTLC claims from counterparty commitments

HTLC outputs, like the `to_remote` output, in commitment transactions
with anchor outputs also have an additional `1 CSV` constraint on the
counterparty. When spending such outputs, their corresponding input
needs to have their sequence set to 1. This was done for HTLC claims
from holder commitments, but unfortunately not for counterparty
commitments as we were lacking test coverage.

diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs
index 382ffac3e8e886c303207798fafe528c78406b0d..c888118bcba4e13b3400027249e8e14eb4e535da 100644 (file)
--- a/lightning/src/chain/package.rs
+++ b/lightning/src/chain/package.rs
@@ -551,6 +551,32 @@ impl PackageSolvingData {
                        _ => { mem::discriminant(self) == mem::discriminant(&input) }
                }
        }
+       fn as_tx_input(&self, previous_output: BitcoinOutPoint) -> TxIn {
+               let sequence = match self {
+                       PackageSolvingData::RevokedOutput(_) => Sequence::ENABLE_RBF_NO_LOCKTIME,
+                       PackageSolvingData::RevokedHTLCOutput(_) => Sequence::ENABLE_RBF_NO_LOCKTIME,
+                       PackageSolvingData::CounterpartyOfferedHTLCOutput(outp) => if outp.channel_type_features.supports_anchors_zero_fee_htlc_tx() {
+                               Sequence::from_consensus(1)
+                       } else {
+                               Sequence::ENABLE_RBF_NO_LOCKTIME
+                       },
+                       PackageSolvingData::CounterpartyReceivedHTLCOutput(outp) => if outp.channel_type_features.supports_anchors_zero_fee_htlc_tx() {
+                               Sequence::from_consensus(1)
+                       } else {
+                               Sequence::ENABLE_RBF_NO_LOCKTIME
+                       },
+                       _ => {
+                               debug_assert!(false, "This should not be reachable by 'untractable' or 'malleable with external funding' packages");
+                               Sequence::ENABLE_RBF_NO_LOCKTIME
+                       },
+               };
+               TxIn {
+                       previous_output,
+                       script_sig: Script::new(),
+                       sequence,
+                       witness: Witness::new(),
+               }
+       }
        fn finalize_input<Signer: WriteableEcdsaChannelSigner>(&self, bumped_tx: &mut Transaction, i: usize, onchain_handler: &mut OnchainTxHandler<Signer>) -> bool {
                match self {
                        PackageSolvingData::RevokedOutput(ref outp) => {
@@ -895,13 +921,8 @@ impl PackageTemplate {
                                value,
                        }],
                };
-               for (outpoint, _) in self.inputs.iter() {
-                       bumped_tx.input.push(TxIn {
-                               previous_output: *outpoint,
-                               script_sig: Script::new(),
-                               sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
-                               witness: Witness::new(),
-                       });
+               for (outpoint, outp) in self.inputs.iter() {
+                       bumped_tx.input.push(outp.as_tx_input(*outpoint));
                }
                for (i, (outpoint, out)) in self.inputs.iter().enumerate() {
                        log_debug!(logger, "Adding claiming input for outpoint {}:{}", outpoint.txid, outpoint.vout);

diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index cb78cda714f83d4edec2b64f44364fc88f55864d..9cf3f4ca29a323b16f5d76114a9a413138f2633f 100644 (file)
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -1867,23 +1867,35 @@ fn test_yield_anchors_events() {
        let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(anchors_config), Some(anchors_config)]);
        let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
-       let chan_id = create_announced_chan_between_nodes_with_value(
+       let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes_with_value(
                &nodes, 0, 1, 1_000_000, 500_000_000
-       ).2;
-       route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
-       let (payment_preimage, payment_hash, ..) = route_payment(&nodes[1], &[&nodes[0]], 1_000_000);
+       );
+       let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+       let (payment_preimage_2, payment_hash_2, ..) = route_payment(&nodes[1], &[&nodes[0]], 2_000_000);
 
        assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
+       assert!(nodes[1].node.get_and_clear_pending_events().is_empty());
 
        *nodes[0].fee_estimator.sat_per_kw.lock().unwrap() *= 2;
+
        connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
-       check_closed_broadcast!(&nodes[0], true);
-       assert!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty());
+       assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty());
+
+       connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
+       {
+               let txn = nodes[1].tx_broadcaster.txn_broadcast();
+               assert_eq!(txn.len(), 1);
+               check_spends!(txn[0], funding_tx);
+       }
 
        get_monitor!(nodes[0], chan_id).provide_payment_preimage(
-               &payment_hash, &payment_preimage, &node_cfgs[0].tx_broadcaster,
+               &payment_hash_2, &payment_preimage_2, &node_cfgs[0].tx_broadcaster,
                &LowerBoundedFeeEstimator::new(node_cfgs[0].fee_estimator), &nodes[0].logger
        );
+       get_monitor!(nodes[1], chan_id).provide_payment_preimage(
+               &payment_hash_1, &payment_preimage_1, &node_cfgs[0].tx_broadcaster,
+               &LowerBoundedFeeEstimator::new(node_cfgs[1].fee_estimator), &nodes[1].logger
+       );
 
        let mut holder_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
        assert_eq!(holder_events.len(), 1);
@@ -1904,27 +1916,50 @@ fn test_yield_anchors_events() {
                        assert_eq!(txn.len(), 2);
                        let anchor_tx = txn.pop().unwrap();
                        let commitment_tx = txn.pop().unwrap();
+                       check_spends!(commitment_tx, funding_tx);
                        check_spends!(anchor_tx, coinbase_tx, commitment_tx);
                        (commitment_tx, anchor_tx)
                },
                _ => panic!("Unexpected event"),
        };
 
+       assert_eq!(commitment_tx.output[2].value, 1_000); // HTLC A -> B
+       assert_eq!(commitment_tx.output[3].value, 2_000); // HTLC B -> A
+
        mine_transactions(&nodes[0], &[&commitment_tx, &anchor_tx]);
        check_added_monitors!(nodes[0], 1);
+       mine_transactions(&nodes[1], &[&commitment_tx, &anchor_tx]);
+       check_added_monitors!(nodes[1], 1);
+
+       {
+               let mut txn = nodes[1].tx_broadcaster.unique_txn_broadcast();
+               assert_eq!(txn.len(), if nodes[1].connect_style.borrow().updates_best_block_first() { 3 } else { 2 });
+
+               let htlc_preimage_tx = txn.pop().unwrap();
+               assert_eq!(htlc_preimage_tx.input.len(), 1);
+               assert_eq!(htlc_preimage_tx.input[0].previous_output.vout, 3);
+               check_spends!(htlc_preimage_tx, commitment_tx);
+
+               let htlc_timeout_tx = txn.pop().unwrap();
+               assert_eq!(htlc_timeout_tx.input.len(), 1);
+               assert_eq!(htlc_timeout_tx.input[0].previous_output.vout, 2);
+               check_spends!(htlc_timeout_tx, commitment_tx);
+
+               if let Some(commitment_tx) = txn.pop() {
+                       check_spends!(commitment_tx, funding_tx);
+               }
+       }
 
        let mut holder_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
        // Certain block `ConnectStyle`s cause an extra `ChannelClose` event to be emitted since the
        // best block is updated before the confirmed transactions are notified.
-       match *nodes[0].connect_style.borrow() {
-               ConnectStyle::BestBlockFirst|ConnectStyle::BestBlockFirstReorgsOnlyTip|ConnectStyle::BestBlockFirstSkippingBlocks => {
-                       assert_eq!(holder_events.len(), 3);
-                       if let Event::BumpTransaction(BumpTransactionEvent::ChannelClose { .. }) = holder_events.remove(0) {}
-                       else { panic!("unexpected event"); }
-
-               },
-               _ => assert_eq!(holder_events.len(), 2),
-       };
+       if nodes[0].connect_style.borrow().updates_best_block_first() {
+               assert_eq!(holder_events.len(), 3);
+               if let Event::BumpTransaction(BumpTransactionEvent::ChannelClose { .. }) = holder_events.remove(0) {}
+               else { panic!("unexpected event"); }
+       } else {
+               assert_eq!(holder_events.len(), 2);
+       }
        let mut htlc_txs = Vec::with_capacity(2);
        for event in holder_events {
                match event {
@@ -1958,6 +1993,9 @@ fn test_yield_anchors_events() {
 
        // Clear the remaining events as they're not relevant to what we're testing.
        nodes[0].node.get_and_clear_pending_events();
+       nodes[1].node.get_and_clear_pending_events();
+       nodes[0].node.get_and_clear_pending_msg_events();
+       nodes[1].node.get_and_clear_pending_msg_events();
 }
 
 #[test]