From 3c837838006670f0e1a6c61608596f9d40e06059 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Mon, 25 Sep 2023 16:57:04 -0700 Subject: [PATCH] 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. --- lightning/src/chain/package.rs | 35 ++++++++++++---- lightning/src/ln/monitor_tests.rs | 70 ++++++++++++++++++++++++------- 2 files changed, 82 insertions(+), 23 deletions(-) diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 382ffac3..c888118b 100644 --- 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(&self, bumped_tx: &mut Transaction, i: usize, onchain_handler: &mut OnchainTxHandler) -> 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 cb78cda7..9cf3f4ca 100644 --- 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] -- 2.30.2