From: Wilmer Paulino Date: Sat, 15 Apr 2023 00:03:51 +0000 (-0700) Subject: Fix off-by-one finalized transaction locktime X-Git-Tag: v0.0.115~6^2 X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=97e4344bea559b6a808321892b6ccbb9a54c9532;p=rust-lightning Fix off-by-one finalized transaction locktime While these transactions were still valid, we incorrectly assumed that they would propagate with a locktime of `current_height + 1`, when in reality, only those with a locktime strictly lower than the next height in the chain are allowed to enter the mempool. --- diff --git a/lightning/src/chain/onchaintx.rs b/lightning/src/chain/onchaintx.rs index 9141efc0b..2d5c85cd4 100644 --- a/lightning/src/chain/onchaintx.rs +++ b/lightning/src/chain/onchaintx.rs @@ -748,8 +748,8 @@ impl OnchainTxHandler preprocessed_requests.push(req); } - // Claim everything up to and including cur_height + 1 - let remaining_locked_packages = self.locktimed_packages.split_off(&(cur_height + 2)); + // Claim everything up to and including `cur_height` + let remaining_locked_packages = self.locktimed_packages.split_off(&(cur_height + 1)); for (pop_height, mut entry) in self.locktimed_packages.iter_mut() { log_trace!(logger, "Restoring delayed claim of package(s) at their timelock at {}.", pop_height); preprocessed_requests.append(&mut entry); diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index b3f6f50ed..7ea61dc24 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -460,13 +460,13 @@ impl PackageSolvingData { } } fn absolute_tx_timelock(&self, current_height: u32) -> u32 { - // We use `current_height + 1` as our default locktime to discourage fee sniping and because + // We use `current_height` as our default locktime to discourage fee sniping and because // transactions with it always propagate. let absolute_timelock = match self { - PackageSolvingData::RevokedOutput(_) => current_height + 1, - PackageSolvingData::RevokedHTLCOutput(_) => current_height + 1, - PackageSolvingData::CounterpartyOfferedHTLCOutput(_) => current_height + 1, - PackageSolvingData::CounterpartyReceivedHTLCOutput(ref outp) => cmp::max(outp.htlc.cltv_expiry, current_height + 1), + PackageSolvingData::RevokedOutput(_) => current_height, + PackageSolvingData::RevokedHTLCOutput(_) => current_height, + PackageSolvingData::CounterpartyOfferedHTLCOutput(_) => current_height, + PackageSolvingData::CounterpartyReceivedHTLCOutput(ref outp) => cmp::max(outp.htlc.cltv_expiry, current_height), // HTLC timeout/success transactions rely on a fixed timelock due to the counterparty's // signature. PackageSolvingData::HolderHTLCOutput(ref outp) => { @@ -475,7 +475,7 @@ impl PackageSolvingData { } outp.cltv_expiry }, - PackageSolvingData::HolderFundingOutput(_) => current_height + 1, + PackageSolvingData::HolderFundingOutput(_) => current_height, }; absolute_timelock } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5074b7cb7..e8261f686 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3006,10 +3006,11 @@ where } { let height = self.best_block.read().unwrap().height(); - // Transactions are evaluated as final by network mempools at the next block. However, the modules - // constituting our Lightning node might not have perfect sync about their blockchain views. Thus, if - // the wallet module is in advance on the LDK view, allow one more block of headroom. - if !funding_transaction.input.iter().all(|input| input.sequence == Sequence::MAX) && LockTime::from(funding_transaction.lock_time).is_block_height() && funding_transaction.lock_time.0 > height + 2 { + // Transactions are evaluated as final by network mempools if their locktime is strictly + // lower than the next block height. However, the modules constituting our Lightning + // node might not have perfect sync about their blockchain views. Thus, if the wallet + // module is ahead of LDK, only allow one more block of headroom. + if !funding_transaction.input.iter().all(|input| input.sequence == Sequence::MAX) && LockTime::from(funding_transaction.lock_time).is_block_height() && funding_transaction.lock_time.0 > height + 1 { return Err(APIError::APIMisuseError { err: "Funding transaction absolute timelock is non-final".to_owned() }); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index c7c7c7ed6..ca6c1eb29 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -230,6 +230,9 @@ fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: Block, sk #[cfg(feature = "std")] { eprintln!("Connecting block using Block Connection Style: {:?}", *node.connect_style.borrow()); } + // Update the block internally before handing it over to LDK, to ensure our assertions regarding + // transaction broadcast are correct. + node.blocks.lock().unwrap().push((block.clone(), height)); if !skip_intermediaries { let txdata: Vec<_> = block.txdata.iter().enumerate().collect(); match *node.connect_style.borrow() { @@ -279,7 +282,6 @@ fn do_connect_block<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, block: Block, sk } call_claimable_balances(node); node.node.test_process_background_events(); - node.blocks.lock().unwrap().push((block, height)); } pub fn disconnect_blocks<'a, 'b, 'c, 'd>(node: &'a Node<'b, 'c, 'd>, count: u32) { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 948a014a7..53c056ee6 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -1284,7 +1284,7 @@ fn test_duplicate_htlc_different_direction_onchain() { mine_transaction(&nodes[0], &remote_txn[0]); check_added_monitors!(nodes[0], 1); check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed); - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[0], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires let claim_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); assert_eq!(claim_txn.len(), 3); @@ -2438,7 +2438,7 @@ fn test_justice_tx_htlc_timeout() { test_txn_broadcast(&nodes[1], &chan_5, Some(revoked_local_txn[0].clone()), HTLCType::NONE); mine_transaction(&nodes[0], &revoked_local_txn[0]); - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[0], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires // Verify broadcast of revoked HTLC-timeout let node_txn = test_txn_broadcast(&nodes[0], &chan_5, Some(revoked_local_txn[0].clone()), HTLCType::TIMEOUT); check_added_monitors!(nodes[0], 1); @@ -2765,7 +2765,7 @@ fn test_htlc_on_chain_success() { // Verify that B's ChannelManager is able to extract preimage from HTLC Success tx and pass it backward let header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[1].best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42}; connect_block(&nodes[1], &Block { header, txdata: vec![commitment_tx[0].clone(), node_txn[0].clone(), node_txn[1].clone()]}); - connect_blocks(&nodes[1], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[1], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires { let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); assert_eq!(added_monitors.len(), 1); @@ -2894,7 +2894,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, nodes[1].best_block_info().1 + 1); + assert_eq!(commitment_spend.lock_time.0, nodes[1].best_block_info().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. @@ -2902,7 +2902,7 @@ fn test_htlc_on_chain_success() { // Verify that A's ChannelManager is able to extract preimage from preimage tx and generate PaymentSent let mut header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[0].best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42}; connect_block(&nodes[0], &Block { header, txdata: vec![node_a_commitment_tx[0].clone(), commitment_spend.clone()] }); - connect_blocks(&nodes[0], TEST_FINAL_CLTV + MIN_CLTV_EXPIRY_DELTA as u32 - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[0], TEST_FINAL_CLTV + MIN_CLTV_EXPIRY_DELTA as u32); // Confirm blocks until the HTLC expires check_closed_broadcast!(nodes[0], true); check_added_monitors!(nodes[0], 1); let events = nodes[0].node.get_and_clear_pending_events(); @@ -3024,7 +3024,7 @@ fn do_test_htlc_on_chain_timeout(connect_style: ConnectStyle) { check_spends!(commitment_tx[0], chan_1.3); mine_transaction(&nodes[0], &commitment_tx[0]); - connect_blocks(&nodes[0], TEST_FINAL_CLTV + MIN_CLTV_EXPIRY_DELTA as u32 - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[0], TEST_FINAL_CLTV + MIN_CLTV_EXPIRY_DELTA as u32); // Confirm blocks until the HTLC expires check_closed_broadcast!(nodes[0], true); check_added_monitors!(nodes[0], 1); @@ -4446,7 +4446,7 @@ fn test_static_spendable_outputs_timeout_tx() { MessageSendEvent::BroadcastChannelUpdate { .. } => {}, _ => panic!("Unexpected event"), } - connect_blocks(&nodes[1], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[1], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires // Check B's monitor was able to send back output descriptor event for timeout tx on A's commitment tx let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); @@ -4524,7 +4524,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() { check_closed_broadcast!(nodes[0], true); check_added_monitors!(nodes[0], 1); check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed); - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[0], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires let revoked_htlc_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); assert_eq!(revoked_htlc_txn.len(), 1); @@ -4756,7 +4756,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, nodes[1].best_block_info().1 + 1); // Success tx + assert_eq!(b_txn[0].lock_time.0, nodes[1].best_block_info().1); // Success tx check_closed_broadcast!(nodes[1], true); check_added_monitors!(nodes[1], 1); @@ -4807,7 +4807,7 @@ fn test_duplicate_payment_hash_one_failure_one_success() { check_closed_broadcast!(nodes[1], true); check_added_monitors!(nodes[1], 1); check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed); - connect_blocks(&nodes[1], TEST_FINAL_CLTV - 40 + MIN_CLTV_EXPIRY_DELTA as u32 - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[1], TEST_FINAL_CLTV - 40 + MIN_CLTV_EXPIRY_DELTA as u32); // Confirm blocks until the HTLC expires let htlc_timeout_tx; { // Extract one of the two HTLC-Timeout transaction @@ -5287,7 +5287,7 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() { check_closed_broadcast!(nodes[0], true); check_added_monitors!(nodes[0], 1); check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed); - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[0], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires let htlc_timeout = { let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap(); @@ -5370,7 +5370,7 @@ fn test_key_derivation_params() { // Timeout HTLC on A's chain and so it can generate a HTLC-Timeout tx mine_transaction(&nodes[0], &local_txn_1[0]); - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[0], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires check_closed_broadcast!(nodes[0], true); check_added_monitors!(nodes[0], 1); check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed); @@ -6927,7 +6927,7 @@ fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed); assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[0], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires timeout_tx = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().drain(..) .filter(|tx| tx.input[0].previous_output.txid == bs_commitment_tx[0].txid()).collect(); check_spends!(timeout_tx[0], bs_commitment_tx[0]); @@ -6938,7 +6938,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, 12); + assert_eq!(timeout_tx[0].lock_time.0, 11); } // We fail non-dust-HTLC 2 by broadcast of local timeout/revocation-claim tx mine_transaction(&nodes[0], &timeout_tx[0]); @@ -7318,7 +7318,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() { check_closed_broadcast!(nodes[1], true); check_added_monitors!(nodes[1], 1); check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed); - connect_blocks(&nodes[1], 49); // Confirm blocks until the HTLC expires (note CLTV was explicitly 50 above) + connect_blocks(&nodes[1], 50); // Confirm blocks until the HTLC expires (note CLTV was explicitly 50 above) let revoked_htlc_txn = { let txn = nodes[1].tx_broadcaster.unique_txn_broadcast(); @@ -7464,7 +7464,7 @@ fn test_bump_penalty_txn_on_remote_commitment() { expect_payment_claimed!(nodes[1], payment_hash, 3_000_000); mine_transaction(&nodes[1], &remote_txn[0]); check_added_monitors!(nodes[1], 2); - connect_blocks(&nodes[1], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[1], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires // One or more claim tx should have been broadcast, check it let timeout; @@ -8660,7 +8660,7 @@ fn test_htlc_no_detection() { check_closed_broadcast!(nodes[0], true); check_added_monitors!(nodes[0], 1); check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed); - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); + connect_blocks(&nodes[0], TEST_FINAL_CLTV); let htlc_timeout = { let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap(); @@ -9836,8 +9836,8 @@ fn test_non_final_funding_tx() { assert_eq!(events.len(), 1); let mut tx = match events[0] { Event::FundingGenerationReady { ref channel_value_satoshis, ref output_script, .. } => { - // Timelock the transaction _beyond_ the best client height + 2. - Transaction { version: chan_id as i32, lock_time: PackedLockTime(best_height + 3), input: vec![input], output: vec![TxOut { + // Timelock the transaction _beyond_ the best client height + 1. + Transaction { version: chan_id as i32, lock_time: PackedLockTime(best_height + 2), input: vec![input], output: vec![TxOut { value: *channel_value_satoshis, script_pubkey: output_script.clone(), }]} }, @@ -9851,7 +9851,7 @@ fn test_non_final_funding_tx() { _ => panic!() } - // However, transaction should be accepted if it's in a +2 headroom from best block. + // However, transaction should be accepted if it's in a +1 headroom from best block. tx.lock_time = PackedLockTime(tx.lock_time.0 - 1); assert!(nodes[0].node.funding_transaction_generated(&temp_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).is_ok()); get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index 5b2b29dbb..b3f827716 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -497,7 +497,7 @@ fn do_test_claim_value_force_close(prev_commitment_tx: bool) { nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances()); // When the HTLC timeout output is spendable in the next block, A should broadcast it - connect_blocks(&nodes[0], htlc_cltv_timeout - nodes[0].best_block_info().1 - 1); + connect_blocks(&nodes[0], htlc_cltv_timeout - nodes[0].best_block_info().1); let a_broadcast_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); assert_eq!(a_broadcast_txn.len(), 2); assert_eq!(a_broadcast_txn[0].input.len(), 1); @@ -887,7 +887,7 @@ fn test_no_preimage_inbound_htlc_balances() { // HTLC has been spent, even after the HTLC expires. We'll also fail the inbound HTLC, but it // won't do anything as the channel is already closed. - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); + connect_blocks(&nodes[0], TEST_FINAL_CLTV); let as_htlc_timeout_claim = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); assert_eq!(as_htlc_timeout_claim.len(), 1); check_spends!(as_htlc_timeout_claim[0], as_txn[0]); @@ -908,7 +908,7 @@ fn test_no_preimage_inbound_htlc_balances() { // The next few blocks for B look the same as for A, though for the opposite HTLC nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clear(); - connect_blocks(&nodes[1], TEST_FINAL_CLTV - (ANTI_REORG_DELAY - 1) - 1); + connect_blocks(&nodes[1], TEST_FINAL_CLTV - (ANTI_REORG_DELAY - 1)); expect_pending_htlcs_forwardable_conditions!(nodes[1], [HTLCDestination::FailedPayment { payment_hash: to_b_failed_payment_hash }]); let bs_htlc_timeout_claim = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); @@ -1734,7 +1734,7 @@ fn do_test_restored_packages_retry(check_old_monitor_retries_after_upgrade: bool mine_transaction(&nodes[0], &commitment_tx); // Connect blocks until the HTLC's expiration is met, expecting a transaction broadcast. - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1); + connect_blocks(&nodes[0], TEST_FINAL_CLTV); let htlc_timeout_tx = { let mut txn = nodes[0].tx_broadcaster.txn_broadcast(); assert_eq!(txn.len(), 1); @@ -1885,7 +1885,7 @@ fn do_test_monitor_rebroadcast_pending_claims(anchors: bool) { }; // Connect blocks up to one before the HTLC expires. This should not result in a claim/retry. - connect_blocks(&nodes[0], htlc_expiry - nodes[0].best_block_info().1 - 2); + connect_blocks(&nodes[0], htlc_expiry - nodes[0].best_block_info().1 - 1); check_htlc_retry(false, false); // Connect one more block, producing our first claim. diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 51811ec63..0487fc95a 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -562,8 +562,8 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) { mine_transaction(&nodes[0], &bs_commitment_tx[0]); mine_transaction(&nodes[1], &bs_commitment_tx[0]); if !use_dust { - connect_blocks(&nodes[0], TEST_FINAL_CLTV - 1 + (MIN_CLTV_EXPIRY_DELTA as u32)); - connect_blocks(&nodes[1], TEST_FINAL_CLTV - 1 + (MIN_CLTV_EXPIRY_DELTA as u32)); + connect_blocks(&nodes[0], TEST_FINAL_CLTV + (MIN_CLTV_EXPIRY_DELTA as u32)); + connect_blocks(&nodes[1], TEST_FINAL_CLTV + (MIN_CLTV_EXPIRY_DELTA as u32)); let as_htlc_timeout = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); check_spends!(as_htlc_timeout[0], bs_commitment_tx[0]); assert_eq!(as_htlc_timeout.len(), 1); diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs index f7342671a..3bd8b711c 100644 --- a/lightning/src/ln/reload_tests.rs +++ b/lightning/src/ln/reload_tests.rs @@ -948,7 +948,7 @@ fn do_forwarded_payment_no_manager_persistence(use_cs_commitment: bool, claim_ht if claim_htlc { confirm_transaction(&nodes[1], &cs_commitment_tx[1]); } else { - connect_blocks(&nodes[1], htlc_expiry - nodes[1].best_block_info().1); + connect_blocks(&nodes[1], htlc_expiry - nodes[1].best_block_info().1 + 1); let bs_htlc_timeout_tx = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); assert_eq!(bs_htlc_timeout_tx.len(), 1); confirm_transaction(&nodes[1], &bs_htlc_timeout_tx[0]); diff --git a/lightning/src/ln/reorg_tests.rs b/lightning/src/ln/reorg_tests.rs index 14b06457f..e8f0c1259 100644 --- a/lightning/src/ln/reorg_tests.rs +++ b/lightning/src/ln/reorg_tests.rs @@ -103,7 +103,7 @@ fn do_test_onchain_htlc_reorg(local_commitment: bool, claim: bool) { // Give node 1 node 2's commitment transaction and get its response (timing the HTLC out) mine_transaction(&nodes[1], &node_2_commitment_txn[0]); - connect_blocks(&nodes[1], TEST_FINAL_CLTV - 1); // Confirm blocks until the HTLC expires + connect_blocks(&nodes[1], TEST_FINAL_CLTV); // Confirm blocks until the HTLC expires let node_1_commitment_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); assert_eq!(node_1_commitment_txn.len(), 1); // ChannelMonitor: 1 offered HTLC-Timeout check_spends!(node_1_commitment_txn[0], node_2_commitment_txn[0]); diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs index c36c3aab0..5be32a87f 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -335,7 +335,7 @@ impl chaininterface::BroadcasterInterface for TestBroadcaster { fn broadcast_transaction(&self, tx: &Transaction) { let lock_time = tx.lock_time.0; assert!(lock_time < 1_500_000_000); - if lock_time > self.blocks.lock().unwrap().len() as u32 + 1 && lock_time < 500_000_000 { + if bitcoin::LockTime::from(tx.lock_time).is_block_height() && lock_time > self.blocks.lock().unwrap().last().unwrap().1 { for inp in tx.input.iter() { if inp.sequence != Sequence::MAX { panic!("We should never broadcast a transaction before its locktime ({})!", tx.lock_time);