X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Ffunctional_tests.rs;h=c83b19e7e571e95569c7e11d1acaaa09a03d65cd;hb=f70c113fd813cff4d31481a9200e5f7121224a81;hp=98bb83a7f9d466f802499328dcabd9452528f9a9;hpb=bf395070ddf0dd10527232f9ef51211c71d45195;p=rust-lightning diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 98bb83a7..c83b19e7 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -17,7 +17,7 @@ use crate::chain::chaininterface::LowerBoundedFeeEstimator; use crate::chain::channelmonitor; use crate::chain::channelmonitor::{CLOSED_CHANNEL_UPDATE_ID, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; use crate::chain::transaction::OutPoint; -use crate::sign::{EcdsaChannelSigner, EntropySource, SignerProvider}; +use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, SignerProvider}; use crate::events::{Event, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason}; use crate::ln::{ChannelId, PaymentPreimage, PaymentSecret, PaymentHash}; use crate::ln::channel::{commitment_tx_base_weight, COMMITMENT_TX_WEIGHT_PER_HTLC, CONCURRENT_INBOUND_HTLC_FEE_BUFFER, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, MIN_AFFORDABLE_HTLC_COUNT, get_holder_selected_channel_reserve_satoshis, OutboundV1Channel, InboundV1Channel, COINBASE_MATURITY, ChannelPhase}; @@ -38,11 +38,12 @@ use crate::util::string::UntrustedString; use crate::util::config::{UserConfig, MaxDustHTLCExposure}; use bitcoin::hash_types::BlockHash; -use bitcoin::blockdata::script::{Builder, Script}; +use bitcoin::blockdata::locktime::absolute::LockTime; +use bitcoin::blockdata::script::{Builder, ScriptBuf}; use bitcoin::blockdata::opcodes; use bitcoin::blockdata::constants::ChainHash; use bitcoin::network::constants::Network; -use bitcoin::{PackedLockTime, Sequence, Transaction, TxIn, TxOut, Witness}; +use bitcoin::{Sequence, Transaction, TxIn, TxOut, Witness}; use bitcoin::OutPoint as BitcoinOutPoint; use bitcoin::secp256k1::Secp256k1; @@ -692,7 +693,7 @@ fn test_update_fee_that_funder_cannot_afford() { *feerate_lock += 4; } nodes[0].node.timer_tick_occurred(); - nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot afford to send new feerate at {}", feerate + 4), 1); + nodes[0].logger.assert_log("lightning::ln::channel", format!("Cannot afford to send new feerate at {}", feerate + 4), 1); check_added_monitors!(nodes[0], 0); const INITIAL_COMMITMENT_NUMBER: u64 = 281474976710654; @@ -745,7 +746,7 @@ fn test_update_fee_that_funder_cannot_afford() { &mut htlcs, &local_chan.context.channel_transaction_parameters.as_counterparty_broadcastable() ); - local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), &secp_ctx).unwrap() + local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), Vec::new(), &secp_ctx).unwrap() }; let commit_signed_msg = msgs::CommitmentSigned { @@ -767,7 +768,7 @@ fn test_update_fee_that_funder_cannot_afford() { //check to see if the funder, who sent the update_fee request, can afford the new fee (funder_balance >= fee+channel_reserve) //Should produce and error. nodes[1].node.handle_commitment_signed(&nodes[0].node.get_our_node_id(), &commit_signed_msg); - nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Funding remote cannot afford proposed new fee".to_string(), 1); + nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Funding remote cannot afford proposed new fee", 3); check_added_monitors!(nodes[1], 1); check_closed_broadcast!(nodes[1], true); check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: String::from("Funding remote cannot afford proposed new fee") }, @@ -870,8 +871,8 @@ fn test_update_fee_with_fundee_update_add_htlc() { send_payment(&nodes[1], &vec!(&nodes[0])[..], 800000); send_payment(&nodes[0], &vec!(&nodes[1])[..], 800000); close_channel(&nodes[0], &nodes[1], &chan.2, chan.3, true); - check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); - check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); + check_closed_event!(nodes[0], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[1], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); } #[test] @@ -984,8 +985,8 @@ fn test_update_fee() { assert_eq!(get_feerate!(nodes[0], nodes[1], channel_id), feerate + 30); assert_eq!(get_feerate!(nodes[1], nodes[0], channel_id), feerate + 30); close_channel(&nodes[0], &nodes[1], &chan.2, chan.3, true); - check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); - check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); + check_closed_event!(nodes[0], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[1], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); } #[test] @@ -1103,17 +1104,17 @@ fn fake_network_test() { // Close down the channels... close_channel(&nodes[0], &nodes[1], &chan_1.2, chan_1.3, true); - check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); - check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); + check_closed_event!(nodes[0], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[1], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); close_channel(&nodes[1], &nodes[2], &chan_2.2, chan_2.3, false); - check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[2].node.get_our_node_id()], 100000); - check_closed_event!(nodes[2], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[1], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[2].node.get_our_node_id()], 100000); + check_closed_event!(nodes[2], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); close_channel(&nodes[2], &nodes[3], &chan_3.2, chan_3.3, true); - check_closed_event!(nodes[2], 1, ClosureReason::CooperativeClosure, [nodes[3].node.get_our_node_id()], 100000); - check_closed_event!(nodes[3], 1, ClosureReason::CooperativeClosure, [nodes[2].node.get_our_node_id()], 100000); + check_closed_event!(nodes[2], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[3].node.get_our_node_id()], 100000); + check_closed_event!(nodes[3], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[2].node.get_our_node_id()], 100000); close_channel(&nodes[1], &nodes[3], &chan_4.2, chan_4.3, false); - check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[3].node.get_our_node_id()], 100000); - check_closed_event!(nodes[3], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[1], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[3].node.get_our_node_id()], 100000); + check_closed_event!(nodes[3], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); } #[test] @@ -1414,6 +1415,7 @@ fn test_fee_spike_violation_fails_htlc() { cltv_expiry: htlc_cltv, onion_routing_packet: onion_packet, skimmed_fee_msat: None, + blinding_point: None, }; nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg); @@ -1492,7 +1494,7 @@ fn test_fee_spike_violation_fails_htlc() { &mut vec![(accepted_htlc_info, ())], &local_chan.context.channel_transaction_parameters.as_counterparty_broadcastable() ); - local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), &secp_ctx).unwrap() + local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), Vec::new(), &secp_ctx).unwrap() }; let commit_signed_msg = msgs::CommitmentSigned { @@ -1527,7 +1529,7 @@ fn test_fee_spike_violation_fails_htlc() { }, _ => panic!("Unexpected event"), }; - nodes[1].logger.assert_log("lightning::ln::channel".to_string(), + nodes[1].logger.assert_log("lightning::ln::channel", format!("Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", raa_msg.channel_id), 1); check_added_monitors!(nodes[1], 2); @@ -1610,11 +1612,12 @@ fn test_chan_reserve_violation_inbound_htlc_outbound_channel() { cltv_expiry: htlc_cltv, onion_routing_packet: onion_packet, skimmed_fee_msat: None, + blinding_point: None, }; nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &msg); // Check that the payment failed and the channel is closed in response to the malicious UpdateAdd. - nodes[0].logger.assert_log("lightning::ln::channelmanager".to_string(), "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_string(), 1); + nodes[0].logger.assert_log_contains("lightning::ln::channelmanager", "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value", 3); assert_eq!(nodes[0].node.list_channels().len(), 0); let err_msg = check_closed_broadcast!(nodes[0], true).unwrap(); assert_eq!(err_msg.data, "Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value"); @@ -1788,11 +1791,12 @@ fn test_chan_reserve_violation_inbound_htlc_inbound_chan() { cltv_expiry: htlc_cltv, onion_routing_packet: onion_packet, skimmed_fee_msat: None, + blinding_point: None, }; nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg); // Check that the payment failed and the channel is closed in response to the malicious UpdateAdd. - nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Remote HTLC add would put them under remote reserve value".to_string(), 1); + nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Remote HTLC add would put them under remote reserve value", 3); assert_eq!(nodes[1].node.list_channels().len(), 1); let err_msg = check_closed_broadcast!(nodes[1], true).unwrap(); assert_eq!(err_msg.data, "Remote HTLC add would put them under remote reserve value"); @@ -2269,9 +2273,15 @@ fn channel_monitor_network_test() { nodes[1].node.force_close_broadcasting_latest_txn(&chan_1.2, &nodes[0].node.get_our_node_id()).unwrap(); check_added_monitors!(nodes[1], 1); check_closed_broadcast!(nodes[1], true); + check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000); { let mut node_txn = test_txn_broadcast(&nodes[1], &chan_1, None, HTLCType::NONE); assert_eq!(node_txn.len(), 1); + mine_transaction(&nodes[1], &node_txn[0]); + if nodes[1].connect_style.borrow().updates_best_block_first() { + let _ = nodes[1].tx_broadcaster.txn_broadcast(); + } + mine_transaction(&nodes[0], &node_txn[0]); check_added_monitors!(nodes[0], 1); test_txn_broadcast(&nodes[0], &chan_1, Some(node_txn[0].clone()), HTLCType::NONE); @@ -2280,7 +2290,6 @@ fn channel_monitor_network_test() { assert_eq!(nodes[0].node.list_channels().len(), 0); assert_eq!(nodes[1].node.list_channels().len(), 1); check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed, [nodes[1].node.get_our_node_id()], 100000); - check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000); // One pending HTLC is discarded by the force-close: let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[1], &[&nodes[2], &nodes[3]], 3_000_000); @@ -2589,8 +2598,8 @@ fn do_test_forming_justice_tx_from_monitor_updates(broadcast_initial_commitment: // that a revoked commitment transaction is broadcasted // (Similar to `revoked_output_claim` test but we get the justice tx + broadcast manually) let chanmon_cfgs = create_chanmon_cfgs(2); - let destination_script0 = chanmon_cfgs[0].keys_manager.get_destination_script().unwrap(); - let destination_script1 = chanmon_cfgs[1].keys_manager.get_destination_script().unwrap(); + let destination_script0 = chanmon_cfgs[0].keys_manager.get_destination_script([0; 32]).unwrap(); + let destination_script1 = chanmon_cfgs[1].keys_manager.get_destination_script([0; 32]).unwrap(); let persisters = vec![WatchtowerPersister::new(destination_script0), WatchtowerPersister::new(destination_script1)]; let node_cfgs = create_node_cfgs_with_persisters(2, &chanmon_cfgs, persisters.iter().collect()); @@ -2860,8 +2869,8 @@ fn test_htlc_on_chain_success() { assert_eq!(node_txn[1].input[0].witness.clone().last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); assert!(node_txn[0].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output assert!(node_txn[1].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output - assert_eq!(node_txn[0].lock_time.0, 0); - assert_eq!(node_txn[1].lock_time.0, 0); + assert_eq!(node_txn[0].lock_time, LockTime::ZERO); + assert_eq!(node_txn[1].lock_time, LockTime::ZERO); // Verify that B's ChannelManager is able to extract preimage from HTLC Success tx and pass it backward connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![commitment_tx[0].clone(), node_txn[0].clone(), node_txn[1].clone()])); @@ -2880,8 +2889,10 @@ fn test_htlc_on_chain_success() { } let chan_id = Some(chan_1.2); match forwarded_events[1] { - Event::PaymentForwarded { fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id, outbound_amount_forwarded_msat } => { - assert_eq!(fee_earned_msat, Some(1000)); + Event::PaymentForwarded { total_fee_earned_msat, prev_channel_id, claim_from_onchain_tx, + next_channel_id, outbound_amount_forwarded_msat, .. + } => { + assert_eq!(total_fee_earned_msat, Some(1000)); assert_eq!(prev_channel_id, chan_id); assert_eq!(claim_from_onchain_tx, true); assert_eq!(next_channel_id, Some(chan_2.2)); @@ -2890,8 +2901,10 @@ fn test_htlc_on_chain_success() { _ => panic!() } match forwarded_events[2] { - Event::PaymentForwarded { fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id, outbound_amount_forwarded_msat } => { - assert_eq!(fee_earned_msat, Some(1000)); + Event::PaymentForwarded { total_fee_earned_msat, prev_channel_id, claim_from_onchain_tx, + next_channel_id, outbound_amount_forwarded_msat, .. + } => { + assert_eq!(total_fee_earned_msat, Some(1000)); assert_eq!(prev_channel_id, chan_id); assert_eq!(claim_from_onchain_tx, true); assert_eq!(next_channel_id, Some(chan_2.2)); @@ -2942,8 +2955,8 @@ fn test_htlc_on_chain_success() { // Node[0]: 2 * HTLC-timeout tx check_spends!(node_txn[0], $commitment_tx); check_spends!(node_txn[1], $commitment_tx); - assert_ne!(node_txn[0].lock_time.0, 0); - assert_ne!(node_txn[1].lock_time.0, 0); + assert_ne!(node_txn[0].lock_time, LockTime::ZERO); + assert_ne!(node_txn[1].lock_time, LockTime::ZERO); if $htlc_offered { assert_eq!(node_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); assert_eq!(node_txn[1].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); @@ -2994,7 +3007,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); + assert_eq!(commitment_spend.lock_time.to_consensus_u32(), 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. @@ -3319,22 +3332,18 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), if deliver_bs_raa { 3 + nodes.len() - 1 } else { 4 + nodes.len() }); - match events[0] { - Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } => { }, - _ => panic!("Unexepected event"), - } - match events[1] { - Event::PaymentPathFailed { ref payment_hash, .. } => { - assert_eq!(*payment_hash, fourth_payment_hash); - }, - _ => panic!("Unexpected event"), - } - match events[2] { - Event::PaymentFailed { ref payment_hash, .. } => { - assert_eq!(*payment_hash, fourth_payment_hash); - }, - _ => panic!("Unexpected event"), - } + assert!(events.iter().any(|ev| matches!( + ev, + Event::ChannelClosed { reason: ClosureReason::CommitmentTxConfirmed, .. } + ))); + assert!(events.iter().any(|ev| matches!( + ev, + Event::PaymentPathFailed { ref payment_hash, .. } if *payment_hash == fourth_payment_hash + ))); + assert!(events.iter().any(|ev| matches!( + ev, + Event::PaymentFailed { ref payment_hash, .. } if *payment_hash == fourth_payment_hash + ))); nodes[1].node.process_pending_htlc_forwards(); check_added_monitors!(nodes[1], 1); @@ -3509,6 +3518,7 @@ fn fail_backward_pending_htlc_upon_channel_failure() { cltv_expiry, onion_routing_packet, skimmed_fee_msat: None, + blinding_point: None, }; nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &update_add_htlc); } @@ -3551,7 +3561,7 @@ fn test_htlc_ignore_latest_remote_commitment() { // connect_style. return; } - create_announced_chan_between_nodes(&nodes, 0, 1); + let funding_tx = create_announced_chan_between_nodes(&nodes, 0, 1).3; route_payment(&nodes[0], &[&nodes[1]], 10000000); nodes[0].node.force_close_broadcasting_latest_txn(&nodes[0].node.list_channels()[0].channel_id, &nodes[1].node.get_our_node_id()).unwrap(); @@ -3560,11 +3570,12 @@ fn test_htlc_ignore_latest_remote_commitment() { check_added_monitors!(nodes[0], 1); check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000); - let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); - assert_eq!(node_txn.len(), 3); - assert_eq!(node_txn[0].txid(), node_txn[1].txid()); + let node_txn = nodes[0].tx_broadcaster.unique_txn_broadcast(); + assert_eq!(node_txn.len(), 2); + check_spends!(node_txn[0], funding_tx); + check_spends!(node_txn[1], node_txn[0]); - let block = create_dummy_block(nodes[1].best_block_hash(), 42, vec![node_txn[0].clone(), node_txn[1].clone()]); + let block = create_dummy_block(nodes[1].best_block_hash(), 42, vec![node_txn[0].clone()]); connect_block(&nodes[1], &block); check_closed_broadcast!(nodes[1], true); check_added_monitors!(nodes[1], 1); @@ -3621,7 +3632,7 @@ fn test_force_close_fail_back() { check_closed_broadcast!(nodes[2], true); check_added_monitors!(nodes[2], 1); check_closed_event!(nodes[2], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000); - let tx = { + let commitment_tx = { let mut node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap(); // Note that we don't bother broadcasting the HTLC-Success transaction here as we don't // have a use for it unless nodes[2] learns the preimage somehow, the funds will go @@ -3630,7 +3641,7 @@ fn test_force_close_fail_back() { node_txn.remove(0) }; - mine_transaction(&nodes[1], &tx); + mine_transaction(&nodes[1], &commitment_tx); // Note no UpdateHTLCs event here from nodes[1] to nodes[0]! check_closed_broadcast!(nodes[1], true); @@ -3642,15 +3653,16 @@ fn test_force_close_fail_back() { get_monitor!(nodes[2], payment_event.commitment_msg.channel_id) .provide_payment_preimage(&our_payment_hash, &our_payment_preimage, &node_cfgs[2].tx_broadcaster, &LowerBoundedFeeEstimator::new(node_cfgs[2].fee_estimator), &node_cfgs[2].logger); } - mine_transaction(&nodes[2], &tx); - let node_txn = nodes[2].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(node_txn.len(), 1); - assert_eq!(node_txn[0].input.len(), 1); - assert_eq!(node_txn[0].input[0].previous_output.txid, tx.txid()); - assert_eq!(node_txn[0].lock_time.0, 0); // Must be an HTLC-Success - assert_eq!(node_txn[0].input[0].witness.len(), 5); // Must be an HTLC-Success + mine_transaction(&nodes[2], &commitment_tx); + let mut node_txn = nodes[2].tx_broadcaster.txn_broadcast(); + assert_eq!(node_txn.len(), if nodes[2].connect_style.borrow().updates_best_block_first() { 2 } else { 1 }); + let htlc_tx = node_txn.pop().unwrap(); + assert_eq!(htlc_tx.input.len(), 1); + assert_eq!(htlc_tx.input[0].previous_output.txid, commitment_tx.txid()); + assert_eq!(htlc_tx.lock_time, LockTime::ZERO); // Must be an HTLC-Success + assert_eq!(htlc_tx.input[0].witness.len(), 5); // Must be an HTLC-Success - check_spends!(node_txn[0], tx); + check_spends!(htlc_tx, commitment_tx); } #[test] @@ -4729,7 +4741,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() { assert_eq!(revoked_htlc_txn[0].input.len(), 1); assert_eq!(revoked_htlc_txn[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); check_spends!(revoked_htlc_txn[0], revoked_local_txn[0]); - assert_ne!(revoked_htlc_txn[0].lock_time.0, 0); // HTLC-Timeout + assert_ne!(revoked_htlc_txn[0].lock_time, LockTime::ZERO); // HTLC-Timeout // B will generate justice tx from A's revoked commitment/HTLC tx connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![revoked_local_txn[0].clone(), revoked_htlc_txn[0].clone()])); @@ -4892,7 +4904,7 @@ fn test_onchain_to_onchain_claim() { check_spends!(c_txn[0], commitment_tx[0]); assert_eq!(c_txn[0].input[0].witness.clone().last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); assert!(c_txn[0].output[0].script_pubkey.is_v0_p2wsh()); // revokeable output - assert_eq!(c_txn[0].lock_time.0, 0); // Success tx + assert_eq!(c_txn[0].lock_time, LockTime::ZERO); // Success tx // So we broadcast C's commitment tx and HTLC-Success on B's chain, we should successfully be able to extract preimage and update downstream monitor connect_block(&nodes[1], &create_dummy_block(nodes[1].best_block_hash(), 42, vec![commitment_tx[0].clone(), c_txn[0].clone()])); @@ -4904,8 +4916,10 @@ fn test_onchain_to_onchain_claim() { _ => panic!("Unexpected event"), } match events[1] { - Event::PaymentForwarded { fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id, outbound_amount_forwarded_msat } => { - assert_eq!(fee_earned_msat, Some(1000)); + Event::PaymentForwarded { total_fee_earned_msat, prev_channel_id, claim_from_onchain_tx, + next_channel_id, outbound_amount_forwarded_msat, .. + } => { + assert_eq!(total_fee_earned_msat, Some(1000)); assert_eq!(prev_channel_id, Some(chan_1.2)); assert_eq!(claim_from_onchain_tx, true); assert_eq!(next_channel_id, Some(chan_2.2)); @@ -4951,7 +4965,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); // Success tx + assert_eq!(b_txn[0].lock_time.to_consensus_u32(), nodes[1].best_block_info().1); // Success tx check_closed_broadcast!(nodes[1], true); check_added_monitors!(nodes[1], 1); @@ -5525,8 +5539,9 @@ fn test_key_derivation_params() { let chain_monitor = test_utils::TestChainMonitor::new(Some(&chanmon_cfgs[0].chain_source), &chanmon_cfgs[0].tx_broadcaster, &chanmon_cfgs[0].logger, &chanmon_cfgs[0].fee_estimator, &chanmon_cfgs[0].persister, &keys_manager); let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &chanmon_cfgs[0].logger)); let scorer = RwLock::new(test_utils::TestScorer::new()); - let router = test_utils::TestRouter::new(network_graph.clone(), &scorer); - let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, tx_broadcaster: &chanmon_cfgs[0].tx_broadcaster, fee_estimator: &chanmon_cfgs[0].fee_estimator, router, chain_monitor, keys_manager: &keys_manager, network_graph, node_seed: seed, override_init_features: alloc::rc::Rc::new(core::cell::RefCell::new(None)) }; + let router = test_utils::TestRouter::new(network_graph.clone(), &chanmon_cfgs[0].logger, &scorer); + let message_router = test_utils::TestMessageRouter::new(network_graph.clone()); + let node = NodeCfg { chain_source: &chanmon_cfgs[0].chain_source, logger: &chanmon_cfgs[0].logger, tx_broadcaster: &chanmon_cfgs[0].tx_broadcaster, fee_estimator: &chanmon_cfgs[0].fee_estimator, router, message_router, chain_monitor, keys_manager: &keys_manager, network_graph, node_seed: seed, override_init_features: alloc::rc::Rc::new(core::cell::RefCell::new(None)) }; let mut node_cfgs = create_node_cfgs(3, &chanmon_cfgs); node_cfgs.remove(0); node_cfgs.insert(0, node); @@ -5610,7 +5625,7 @@ fn test_static_output_closing_tx() { let closing_tx = close_channel(&nodes[0], &nodes[1], &chan.2, chan.3, true).2; mine_transaction(&nodes[0], &closing_tx); - check_closed_event!(nodes[0], 1, ClosureReason::CooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); + check_closed_event!(nodes[0], 1, ClosureReason::CounterpartyInitiatedCooperativeClosure, [nodes[1].node.get_our_node_id()], 100000); connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1); let spend_txn = check_spendable_outputs!(nodes[0], node_cfgs[0].keys_manager); @@ -5618,7 +5633,7 @@ fn test_static_output_closing_tx() { check_spends!(spend_txn[0], closing_tx); mine_transaction(&nodes[1], &closing_tx); - check_closed_event!(nodes[1], 1, ClosureReason::CooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); + check_closed_event!(nodes[1], 1, ClosureReason::LocallyInitiatedCooperativeClosure, [nodes[0].node.get_our_node_id()], 100000); connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1); let spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager); @@ -5925,7 +5940,7 @@ fn test_fail_holding_cell_htlc_upon_free() { // us to surface its failure to the user. chan_stat = get_channel_value_stat!(nodes[0], nodes[1], chan.2); assert_eq!(chan_stat.holding_cell_outbound_amount_msat, 0); - nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Freeing holding cell with 1 HTLC updates in channel {}", chan.2), 1); + nodes[0].logger.assert_log("lightning::ln::channel", format!("Freeing holding cell with 1 HTLC updates in channel {}", chan.2), 1); // Check that the payment failed to be sent out. let events = nodes[0].node.get_and_clear_pending_events(); @@ -6013,7 +6028,7 @@ fn test_free_and_fail_holding_cell_htlcs() { // to surface its failure to the user. The first payment should succeed. chan_stat = get_channel_value_stat!(nodes[0], nodes[1], chan.2); assert_eq!(chan_stat.holding_cell_outbound_amount_msat, 0); - nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Freeing holding cell with 2 HTLC updates in channel {}", chan.2), 1); + nodes[0].logger.assert_log("lightning::ln::channel", format!("Freeing holding cell with 2 HTLC updates in channel {}", chan.2), 1); // Check that the second payment failed to be sent out. let events = nodes[0].node.get_and_clear_pending_events(); @@ -6204,6 +6219,30 @@ fn test_fail_holding_cell_htlc_upon_free_multihop() { check_added_monitors!(nodes[0], 1); } +#[test] +fn test_payment_route_reaching_same_channel_twice() { + //A route should not go through the same channel twice + //It is enforced when constructing a route. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let _chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1000000, 0); + + let payment_params = PaymentParameters::from_node_id(nodes[1].node.get_our_node_id(), 0) + .with_bolt11_features(nodes[1].node.bolt11_invoice_features()).unwrap(); + let (mut route, our_payment_hash, _, our_payment_secret) = get_route_and_payment_hash!(nodes[0], nodes[1], payment_params, 100000000); + + // Extend the path by itself, essentially simulating route going through same channel twice + let cloned_hops = route.paths[0].hops.clone(); + route.paths[0].hops.extend_from_slice(&cloned_hops); + + unwrap_send_err!(nodes[0].node.send_payment_with_route(&route, our_payment_hash, + RecipientOnionFields::secret_only(our_payment_secret), PaymentId(our_payment_hash.0) + ), false, APIError::InvalidRoute { ref err }, + assert_eq!(err, &"Path went through the same channel twice")); +} + // BOLT 2 Requirements for the Sender when constructing and sending an update_add_htlc message. // BOLT 2 Requirement: MUST NOT offer amount_msat it cannot pay for in the remote commitment transaction at the current feerate_per_kw (see "Updating Fees") while maintaining its channel reserve. //TODO: I don't believe this is explicitly enforced when sending an HTLC but as the Fee aspect of the BOLT specs is in flux leaving this as a TODO. @@ -6263,7 +6302,7 @@ fn test_update_add_htlc_bolt2_receiver_zero_value_msat() { updates.update_add_htlcs[0].amount_msat = 0; nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]); - nodes[1].logger.assert_log("lightning::ln::channelmanager".to_string(), "Remote side tried to send a 0-msat HTLC".to_string(), 1); + nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Remote side tried to send a 0-msat HTLC", 3); check_closed_broadcast!(nodes[1], true).unwrap(); check_added_monitors!(nodes[1], 1); check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "Remote side tried to send a 0-msat HTLC".to_string() }, @@ -6456,6 +6495,7 @@ fn test_update_add_htlc_bolt2_receiver_check_max_htlc_limit() { cltv_expiry: htlc_cltv, onion_routing_packet: onion_packet.clone(), skimmed_fee_msat: None, + blinding_point: None, }; for i in 0..50 { @@ -7122,7 +7162,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, 11); + assert_eq!(timeout_tx[0].lock_time.to_consensus_u32(), 11); } // We fail non-dust-HTLC 2 by broadcast of local timeout/revocation-claim tx mine_transaction(&nodes[0], &timeout_tx[0]); @@ -7416,7 +7456,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() { assert_eq!(node_txn[0].output.len(), 1); check_spends!(node_txn[0], revoked_txn[0]); let fee_1 = penalty_sum - node_txn[0].output[0].value; - feerate_1 = fee_1 * 1000 / node_txn[0].weight() as u64; + feerate_1 = fee_1 * 1000 / node_txn[0].weight().to_wu(); penalty_1 = node_txn[0].txid(); node_txn.clear(); }; @@ -7436,7 +7476,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() { // Verify new bumped tx is different from last claiming transaction, we don't want spurrious rebroadcast assert_ne!(penalty_2, penalty_1); let fee_2 = penalty_sum - node_txn[0].output[0].value; - feerate_2 = fee_2 * 1000 / node_txn[0].weight() as u64; + feerate_2 = fee_2 * 1000 / node_txn[0].weight().to_wu(); // Verify 25% bump heuristic assert!(feerate_2 * 100 >= feerate_1 * 125); node_txn.clear(); @@ -7459,7 +7499,7 @@ fn test_bump_penalty_txn_on_revoked_commitment() { // Verify new bumped tx is different from last claiming transaction, we don't want spurrious rebroadcast assert_ne!(penalty_3, penalty_2); let fee_3 = penalty_sum - node_txn[0].output[0].value; - feerate_3 = fee_3 * 1000 / node_txn[0].weight() as u64; + feerate_3 = fee_3 * 1000 / node_txn[0].weight().to_wu(); // Verify 25% bump heuristic assert!(feerate_3 * 100 >= feerate_2 * 125); node_txn.clear(); @@ -7578,7 +7618,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() { first = node_txn[3].txid(); // Store both feerates for later comparison let fee_1 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[1].output[0].value - node_txn[3].output[0].value; - feerate_1 = fee_1 * 1000 / node_txn[3].weight() as u64; + feerate_1 = fee_1 * 1000 / node_txn[3].weight().to_wu(); penalty_txn = vec![node_txn[2].clone()]; node_txn.clear(); } @@ -7602,7 +7642,7 @@ fn test_bump_penalty_txn_on_revoked_htlcs() { // Verify bumped tx is different and 25% bump heuristic assert_ne!(first, node_txn[0].txid()); let fee_2 = revoked_htlc_txn[0].output[0].value + revoked_htlc_txn[1].output[0].value - node_txn[0].output[0].value; - let feerate_2 = fee_2 * 1000 / node_txn[0].weight() as u64; + let feerate_2 = fee_2 * 1000 / node_txn[0].weight().to_wu(); assert!(feerate_2 * 100 > feerate_1 * 125); let txn = vec![node_txn[0].clone()]; node_txn.clear(); @@ -7678,7 +7718,7 @@ fn test_bump_penalty_txn_on_remote_commitment() { preimage = node_txn[0].txid(); let index = node_txn[0].input[0].previous_output.vout; let fee = remote_txn[0].output[index as usize].value - node_txn[0].output[0].value; - feerate_preimage = fee * 1000 / node_txn[0].weight() as u64; + feerate_preimage = fee * 1000 / node_txn[0].weight().to_wu(); let (preimage_bump_tx, timeout_tx) = if node_txn[2].input[0].previous_output == node_txn[0].input[0].previous_output { (node_txn[2].clone(), node_txn[1].clone()) @@ -7693,7 +7733,7 @@ fn test_bump_penalty_txn_on_remote_commitment() { timeout = timeout_tx.txid(); let index = timeout_tx.input[0].previous_output.vout; let fee = remote_txn[0].output[index as usize].value - timeout_tx.output[0].value; - feerate_timeout = fee * 1000 / timeout_tx.weight() as u64; + feerate_timeout = fee * 1000 / timeout_tx.weight().to_wu(); node_txn.clear(); }; @@ -7712,13 +7752,13 @@ fn test_bump_penalty_txn_on_remote_commitment() { let index = preimage_bump.input[0].previous_output.vout; let fee = remote_txn[0].output[index as usize].value - preimage_bump.output[0].value; - let new_feerate = fee * 1000 / preimage_bump.weight() as u64; + let new_feerate = fee * 1000 / preimage_bump.weight().to_wu(); assert!(new_feerate * 100 > feerate_timeout * 125); assert_ne!(timeout, preimage_bump.txid()); let index = node_txn[0].input[0].previous_output.vout; let fee = remote_txn[0].output[index as usize].value - node_txn[0].output[0].value; - let new_feerate = fee * 1000 / node_txn[0].weight() as u64; + let new_feerate = fee * 1000 / node_txn[0].weight().to_wu(); assert!(new_feerate * 100 > feerate_preimage * 125); assert_ne!(preimage, node_txn[0].txid()); @@ -8179,8 +8219,10 @@ fn test_onion_value_mpp_set_calculation() { RecipientOnionFields::secret_only(our_payment_secret), height + 1, &None).unwrap(); // Edit amt_to_forward to simulate the sender having set // the final amount and the routing node taking less fee - if let msgs::OutboundOnionPayload::Receive { ref mut amt_msat, .. } = onion_payloads[1] { - *amt_msat = 99_000; + if let msgs::OutboundOnionPayload::Receive { + ref mut sender_intended_htlc_amt_msat, .. + } = onion_payloads[1] { + *sender_intended_htlc_amt_msat = 99_000; } else { panic!() } let new_onion_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &our_payment_hash).unwrap(); payment_event.msgs[0].onion_routing_packet = new_onion_packet; @@ -8538,10 +8580,11 @@ fn test_concurrent_monitor_claim() { watchtower_alice.chain_monitor.block_connected(&block, HTLC_TIMEOUT_BROADCAST); // Watchtower Alice should have broadcast a commitment/HTLC-timeout - let alice_state = { + { let mut txn = alice_broadcaster.txn_broadcast(); assert_eq!(txn.len(), 2); - txn.remove(0) + check_spends!(txn[0], chan_1.3); + check_spends!(txn[1], txn[0]); }; // Copy ChainMonitor to simulate watchtower Bob and make it receive a commitment update first. @@ -8610,11 +8653,8 @@ fn test_concurrent_monitor_claim() { check_added_monitors(&nodes[0], 1); { let htlc_txn = alice_broadcaster.txn_broadcast(); - assert_eq!(htlc_txn.len(), 2); + assert_eq!(htlc_txn.len(), 1); check_spends!(htlc_txn[0], bob_state_y); - // Alice doesn't clean up the old HTLC claim since it hasn't seen a conflicting spend for - // it. However, she should, because it now has an invalid parent. - check_spends!(htlc_txn[1], alice_state); } } @@ -8650,7 +8690,7 @@ fn test_pre_lockin_no_chan_closed_update() { check_added_monitors!(nodes[0], 0); let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); - let channel_id = crate::chain::transaction::OutPoint { txid: funding_created_msg.funding_txid, index: funding_created_msg.funding_output_index }.to_channel_id(); + let channel_id = ChannelId::v1_from_funding_outpoint(crate::chain::transaction::OutPoint { txid: funding_created_msg.funding_txid, index: funding_created_msg.funding_output_index }); nodes[0].node.handle_error(&nodes[1].node.get_our_node_id(), &msgs::ErrorMessage { channel_id, data: "Hi".to_owned() }); assert!(nodes[0].chain_monitor.added_monitors.lock().unwrap().is_empty()); check_closed_event!(nodes[0], 2, ClosureReason::CounterpartyForceClosed { peer_msg: UntrustedString("Hi".to_string()) }, true, @@ -8853,7 +8893,12 @@ fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain assert_eq!(bob_txn.len(), 1); check_spends!(bob_txn[0], txn_to_broadcast[0]); } else { - assert_eq!(bob_txn.len(), 2); + if nodes[1].connect_style.borrow().updates_best_block_first() { + assert_eq!(bob_txn.len(), 3); + assert_eq!(bob_txn[0].txid(), bob_txn[1].txid()); + } else { + assert_eq!(bob_txn.len(), 2); + } check_spends!(bob_txn[0], chan_ab.3); } } @@ -8869,15 +8914,16 @@ fn do_test_onchain_htlc_settlement_after_close(broadcast_alice: bool, go_onchain // If Alice force-closed, Bob only broadcasts a HTLC-output-claiming transaction. Otherwise, // Bob force-closed and broadcasts the commitment transaction along with a // HTLC-output-claiming transaction. - let bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); + let mut bob_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone(); if broadcast_alice { assert_eq!(bob_txn.len(), 1); check_spends!(bob_txn[0], txn_to_broadcast[0]); assert_eq!(bob_txn[0].input[0].witness.last().unwrap().len(), script_weight); } else { - assert_eq!(bob_txn.len(), 2); - check_spends!(bob_txn[1], txn_to_broadcast[0]); - assert_eq!(bob_txn[1].input[0].witness.last().unwrap().len(), script_weight); + assert_eq!(bob_txn.len(), if nodes[1].connect_style.borrow().updates_best_block_first() { 3 } else { 2 }); + let htlc_tx = bob_txn.pop().unwrap(); + check_spends!(htlc_tx, txn_to_broadcast[0]); + assert_eq!(htlc_tx.input[0].witness.last().unwrap().len(), script_weight); } } } @@ -8940,6 +8986,149 @@ fn test_duplicate_temporary_channel_id_from_different_peers() { } } +#[test] +fn test_peer_funding_sidechannel() { + // Test that if a peer somehow learns which txid we'll use for our channel funding before we + // receive `funding_transaction_generated` the peer cannot cause us to crash. We'd previously + // assumed that LDK would receive `funding_transaction_generated` prior to our peer learning + // the txid and panicked if the peer tried to open a redundant channel to us with the same + // funding outpoint. + // + // While this assumption is generally safe, some users may have out-of-band protocols where + // they notify their LSP about a funding outpoint first, or this may be violated in the future + // with collaborative transaction construction protocols, i.e. dual-funding. + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + let temp_chan_id_ab = exchange_open_accept_chan(&nodes[0], &nodes[1], 1_000_000, 0); + let temp_chan_id_ca = exchange_open_accept_chan(&nodes[2], &nodes[0], 1_000_000, 0); + + let (_, tx, funding_output) = + create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 1_000_000, 42); + + let cs_funding_events = nodes[2].node.get_and_clear_pending_events(); + assert_eq!(cs_funding_events.len(), 1); + match cs_funding_events[0] { + Event::FundingGenerationReady { .. } => {} + _ => panic!("Unexpected event {:?}", cs_funding_events), + } + + nodes[2].node.funding_transaction_generated_unchecked(&temp_chan_id_ca, &nodes[0].node.get_our_node_id(), tx.clone(), funding_output.index).unwrap(); + let funding_created_msg = get_event_msg!(nodes[2], MessageSendEvent::SendFundingCreated, nodes[0].node.get_our_node_id()); + nodes[0].node.handle_funding_created(&nodes[2].node.get_our_node_id(), &funding_created_msg); + get_event_msg!(nodes[0], MessageSendEvent::SendFundingSigned, nodes[2].node.get_our_node_id()); + expect_channel_pending_event(&nodes[0], &nodes[2].node.get_our_node_id()); + check_added_monitors!(nodes[0], 1); + + let res = nodes[0].node.funding_transaction_generated(&temp_chan_id_ab, &nodes[1].node.get_our_node_id(), tx.clone()); + let err_msg = format!("{:?}", res.unwrap_err()); + assert!(err_msg.contains("An existing channel using outpoint ")); + assert!(err_msg.contains(" is open with peer")); + // Even though the last funding_transaction_generated errored, it still generated a + // SendFundingCreated. However, when the peer responds with a funding_signed it will send the + // appropriate error message. + let as_funding_created = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &as_funding_created); + check_added_monitors!(nodes[1], 1); + expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id()); + let reason = ClosureReason::ProcessingError { err: format!("An existing channel using outpoint {} is open with peer {}", funding_output, nodes[2].node.get_our_node_id()), }; + check_closed_events(&nodes[0], &[ExpectedCloseEvent::from_id_reason(ChannelId::v1_from_funding_outpoint(funding_output), true, reason)]); + + let funding_signed = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()); + nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed); + get_err_msg(&nodes[0], &nodes[1].node.get_our_node_id()); +} + +#[test] +fn test_duplicate_conflicting_funding_from_second_peer() { + // Test that if a user tries to fund a channel with a funding outpoint they'd previously used + // we don't try to remove the previous ChannelMonitor. This is largely a test to ensure we + // don't regress in the fuzzer, as such funding getting passed our outpoint-matches checks + // implies the user (and our counterparty) has reused cryptographic keys across channels, which + // we require the user not do. + let chanmon_cfgs = create_chanmon_cfgs(4); + let node_cfgs = create_node_cfgs(4, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]); + let nodes = create_network(4, &node_cfgs, &node_chanmgrs); + + let temp_chan_id = exchange_open_accept_chan(&nodes[0], &nodes[1], 1_000_000, 0); + + let (_, tx, funding_output) = + create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 1_000_000, 42); + + // Now that we have a funding outpoint, create a dummy `ChannelMonitor` and insert it into + // nodes[0]'s ChainMonitor so that the initial `ChannelMonitor` write fails. + let dummy_chan_id = create_chan_between_nodes(&nodes[2], &nodes[3]).3; + let dummy_monitor = get_monitor!(nodes[2], dummy_chan_id).clone(); + nodes[0].chain_monitor.chain_monitor.watch_channel(funding_output, dummy_monitor).unwrap(); + + nodes[0].node.funding_transaction_generated(&temp_chan_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap(); + + let mut funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg); + let funding_signed_msg = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()); + check_added_monitors!(nodes[1], 1); + expect_channel_pending_event(&nodes[1], &nodes[0].node.get_our_node_id()); + + nodes[0].node.handle_funding_signed(&nodes[1].node.get_our_node_id(), &funding_signed_msg); + // At this point, the channel should be closed, after having generated one monitor write (the + // watch_channel call which failed), but zero monitor updates. + check_added_monitors!(nodes[0], 1); + get_err_msg(&nodes[0], &nodes[1].node.get_our_node_id()); + let err_reason = ClosureReason::ProcessingError { err: "Channel funding outpoint was a duplicate".to_owned() }; + check_closed_events(&nodes[0], &[ExpectedCloseEvent::from_id_reason(funding_signed_msg.channel_id, true, err_reason)]); +} + +#[test] +fn test_duplicate_funding_err_in_funding() { + // Test that if we have a live channel with one peer, then another peer comes along and tries + // to create a second channel with the same txid we'll fail and not overwrite the + // outpoint_to_peer map in `ChannelManager`. + // + // This was previously broken. + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + + let (_, _, _, real_channel_id, funding_tx) = create_chan_between_nodes(&nodes[0], &nodes[1]); + let real_chan_funding_txo = chain::transaction::OutPoint { txid: funding_tx.txid(), index: 0 }; + assert_eq!(ChannelId::v1_from_funding_outpoint(real_chan_funding_txo), real_channel_id); + + nodes[2].node.create_channel(nodes[1].node.get_our_node_id(), 100_000, 0, 42, None, None).unwrap(); + let mut open_chan_msg = get_event_msg!(nodes[2], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); + let node_c_temp_chan_id = open_chan_msg.temporary_channel_id; + open_chan_msg.temporary_channel_id = real_channel_id; + nodes[1].node.handle_open_channel(&nodes[2].node.get_our_node_id(), &open_chan_msg); + let mut accept_chan_msg = get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[2].node.get_our_node_id()); + accept_chan_msg.temporary_channel_id = node_c_temp_chan_id; + nodes[2].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_chan_msg); + + // Now that we have a second channel with the same funding txo, send a bogus funding message + // and let nodes[1] remove the inbound channel. + let (_, funding_tx, _) = create_funding_transaction(&nodes[2], &nodes[1].node.get_our_node_id(), 100_000, 42); + + nodes[2].node.funding_transaction_generated(&node_c_temp_chan_id, &nodes[1].node.get_our_node_id(), funding_tx).unwrap(); + + let mut funding_created_msg = get_event_msg!(nodes[2], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); + funding_created_msg.temporary_channel_id = real_channel_id; + // Make the signature invalid by changing the funding output + funding_created_msg.funding_output_index += 10; + nodes[1].node.handle_funding_created(&nodes[2].node.get_our_node_id(), &funding_created_msg); + get_err_msg(&nodes[1], &nodes[2].node.get_our_node_id()); + let err = "Invalid funding_created signature from peer".to_owned(); + let reason = ClosureReason::ProcessingError { err }; + let expected_closing = ExpectedCloseEvent::from_id_reason(real_channel_id, false, reason); + check_closed_events(&nodes[1], &[expected_closing]); + + assert_eq!( + *nodes[1].node.outpoint_to_peer.lock().unwrap().get(&real_chan_funding_txo).unwrap(), + nodes[0].node.get_our_node_id() + ); +} + #[test] fn test_duplicate_chan_id() { // Test that if a given peer tries to open a channel with the same channel_id as one that is @@ -8998,7 +9187,7 @@ fn test_duplicate_chan_id() { let funding_signed_msg = get_event_msg!(nodes[1], MessageSendEvent::SendFundingSigned, nodes[0].node.get_our_node_id()); let funding_outpoint = crate::chain::transaction::OutPoint { txid: funding_created_msg.funding_txid, index: funding_created_msg.funding_output_index }; - let channel_id = funding_outpoint.to_channel_id(); + let channel_id = ChannelId::v1_from_funding_outpoint(funding_outpoint); // Now we have the first channel past funding_created (ie it has a txid-based channel_id, not a // temporary one). @@ -9029,7 +9218,7 @@ fn test_duplicate_chan_id() { nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, nodes[0].node.get_our_node_id())); create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42); // Get and check the FundingGenerationReady event - let (_, funding_created) = { + let funding_created = { let per_peer_state = nodes[0].node.per_peer_state.read().unwrap(); let mut a_peer_state = per_peer_state.get(&nodes[1].node.get_our_node_id()).unwrap().lock().unwrap(); // Once we call `get_funding_created` the channel has a duplicate channel_id as @@ -9037,19 +9226,25 @@ fn test_duplicate_chan_id() { // try to create another channel. Instead, we drop the channel entirely here (leaving the // channelmanager in a possibly nonsense state instead). match a_peer_state.channel_by_id.remove(&open_chan_2_msg.temporary_channel_id).unwrap() { - ChannelPhase::UnfundedOutboundV1(chan) => { + ChannelPhase::UnfundedOutboundV1(mut chan) => { let logger = test_utils::TestLogger::new(); chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap() }, _ => panic!("Unexpected ChannelPhase variant"), - } + }.unwrap() }; check_added_monitors!(nodes[0], 0); - nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created.unwrap()); + nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created); // At this point we'll look up if the channel_id is present and immediately fail the channel // without trying to persist the `ChannelMonitor`. check_added_monitors!(nodes[1], 0); + check_closed_events(&nodes[1], &[ + ExpectedCloseEvent::from_id_reason(funding_created.temporary_channel_id, false, ClosureReason::ProcessingError { + err: "Already had channel with the new channel_id".to_owned() + }) + ]); + // ...still, nodes[1] will reject the duplicate channel. { let events = nodes[1].node.get_and_clear_pending_msg_events(); @@ -9182,10 +9377,10 @@ fn test_invalid_funding_tx() { // a panic as we'd try to extract a 32 byte preimage from a witness element without checking // its length. let mut wit_program: Vec = channelmonitor::deliberately_bogus_accepted_htlc_witness_program(); - let wit_program_script: Script = wit_program.into(); + let wit_program_script: ScriptBuf = wit_program.into(); for output in tx.output.iter_mut() { // Make the confirmed funding transaction have a bogus script_pubkey - output.script_pubkey = Script::new_v0_p2wsh(&wit_program_script.wscript_hash()); + output.script_pubkey = ScriptBuf::new_v0_p2wsh(&wit_program_script.wscript_hash()); } nodes[0].node.funding_transaction_generated_unchecked(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone(), 0).unwrap(); @@ -9223,19 +9418,19 @@ fn test_invalid_funding_tx() { // long the ChannelMonitor will try to read 32 bytes from the second-to-last element, panicing // as its not 32 bytes long. let mut spend_tx = Transaction { - version: 2i32, lock_time: PackedLockTime::ZERO, + version: 2i32, lock_time: LockTime::ZERO, input: tx.output.iter().enumerate().map(|(idx, _)| TxIn { previous_output: BitcoinOutPoint { txid: tx.txid(), vout: idx as u32, }, - script_sig: Script::new(), + script_sig: ScriptBuf::new(), sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Witness::from_vec(channelmonitor::deliberately_bogus_accepted_htlc_witness()) + witness: Witness::from_slice(&channelmonitor::deliberately_bogus_accepted_htlc_witness()) }).collect(), output: vec![TxOut { value: 1000, - script_pubkey: Script::new(), + script_pubkey: ScriptBuf::new(), }] }; check_spends!(spend_tx, tx); @@ -9353,8 +9548,12 @@ fn do_test_tx_confirmed_skipping_blocks_immediate_broadcast(test_height_before_t // We should broadcast an HTLC transaction spending our funding transaction first let spending_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0); assert_eq!(spending_txn.len(), 2); - assert_eq!(spending_txn[0].txid(), node_txn[0].txid()); - check_spends!(spending_txn[1], node_txn[0]); + let htlc_tx = if spending_txn[0].txid() == node_txn[0].txid() { + &spending_txn[1] + } else { + &spending_txn[0] + }; + check_spends!(htlc_tx, node_txn[0]); // We should also generate a SpendableOutputs event with the to_self output (as its // timelock is up). let descriptor_spend_txn = check_spendable_outputs!(nodes[1], node_cfgs[1].keys_manager); @@ -9364,7 +9563,7 @@ fn do_test_tx_confirmed_skipping_blocks_immediate_broadcast(test_height_before_t // should immediately fail-backwards the HTLC to the previous hop, without waiting for an // additional block built on top of the current chain. nodes[1].chain_monitor.chain_monitor.transactions_confirmed( - &nodes[1].get_block_header(conf_height + 1), &[(0, &spending_txn[1])], conf_height + 1); + &nodes[1].get_block_header(conf_height + 1), &[(0, htlc_tx)], conf_height + 1); expect_pending_htlcs_forwardable_and_htlc_handling_failed!(nodes[1], vec![HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: channel_id }]); check_added_monitors!(nodes[1], 1); @@ -9813,10 +10012,10 @@ fn do_test_max_dust_htlc_exposure(dust_outbound_balance: bool, exposure_breach_e // Outbound dust balance: 6399 sats let dust_inbound_overflow = dust_inbound_htlc_on_holder_tx_msat * (dust_inbound_htlc_on_holder_tx + 1); let dust_outbound_overflow = dust_outbound_htlc_on_holder_tx_msat * dust_outbound_htlc_on_holder_tx + dust_inbound_htlc_on_holder_tx_msat; - nodes[0].logger.assert_log("lightning::ln::channel".to_string(), format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", if dust_outbound_balance { dust_outbound_overflow } else { dust_inbound_overflow }, max_dust_htlc_exposure_msat), 1); + nodes[0].logger.assert_log("lightning::ln::channel", format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on holder commitment tx", if dust_outbound_balance { dust_outbound_overflow } else { dust_inbound_overflow }, max_dust_htlc_exposure_msat), 1); } else { // Outbound dust balance: 5200 sats - nodes[0].logger.assert_log("lightning::ln::channel".to_string(), + nodes[0].logger.assert_log("lightning::ln::channel", format!("Cannot accept value that would put our exposure to dust HTLCs at {} over the limit {} on counterparty commitment tx", dust_htlc_on_counterparty_tx_msat * (dust_htlc_on_counterparty_tx - 1) + dust_htlc_on_counterparty_tx_msat + 4, max_dust_htlc_exposure_msat), 1); @@ -9883,12 +10082,12 @@ fn test_non_final_funding_tx() { let chan_id = *nodes[0].network_chan_count.borrow(); let events = nodes[0].node.get_and_clear_pending_events(); - let input = TxIn { previous_output: BitcoinOutPoint::null(), script_sig: bitcoin::Script::new(), sequence: Sequence(1), witness: Witness::from_vec(vec!(vec!(1))) }; + let input = TxIn { previous_output: BitcoinOutPoint::null(), script_sig: bitcoin::ScriptBuf::new(), sequence: Sequence(1), witness: Witness::from_slice(&[&[1]]) }; 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 + 1. - Transaction { version: chan_id as i32, lock_time: PackedLockTime(best_height + 2), input: vec![input], output: vec![TxOut { + Transaction { version: chan_id as i32, lock_time: LockTime::from_height(best_height + 2).unwrap(), input: vec![input], output: vec![TxOut { value: *channel_value_satoshis, script_pubkey: output_script.clone(), }]} }, @@ -9928,12 +10127,12 @@ fn test_non_final_funding_tx_within_headroom() { let chan_id = *nodes[0].network_chan_count.borrow(); let events = nodes[0].node.get_and_clear_pending_events(); - let input = TxIn { previous_output: BitcoinOutPoint::null(), script_sig: bitcoin::Script::new(), sequence: Sequence(1), witness: Witness::from_vec(vec!(vec!(1))) }; + let input = TxIn { previous_output: BitcoinOutPoint::null(), script_sig: bitcoin::ScriptBuf::new(), sequence: Sequence(1), witness: Witness::from_slice(&[[1]]) }; assert_eq!(events.len(), 1); let mut tx = match events[0] { Event::FundingGenerationReady { ref channel_value_satoshis, ref output_script, .. } => { // Timelock the transaction within a +1 headroom from the best block. - Transaction { version: chan_id as i32, lock_time: PackedLockTime(best_height + 1), input: vec![input], output: vec![TxOut { + Transaction { version: chan_id as i32, lock_time: LockTime::from_consensus(best_height + 1), input: vec![input], output: vec![TxOut { value: *channel_value_satoshis, script_pubkey: output_script.clone(), }]} }, @@ -10442,7 +10641,7 @@ fn test_batch_channel_open() { // Complete the persistence of the monitor. nodes[0].chain_monitor.complete_sole_pending_chan_update( - &OutPoint { txid: tx.txid(), index: 1 }.to_channel_id() + &ChannelId::v1_from_funding_outpoint(OutPoint { txid: tx.txid(), index: 1 }) ); let events = nodes[0].node.get_and_clear_pending_events(); @@ -10497,17 +10696,23 @@ fn test_disconnect_in_funding_batch() { nodes[0].node.peer_disconnected(&nodes[2].node.get_our_node_id()); // The channels in the batch will close immediately. - let channel_id_1 = OutPoint { txid: tx.txid(), index: 0 }.to_channel_id(); - let channel_id_2 = OutPoint { txid: tx.txid(), index: 1 }.to_channel_id(); + let funding_txo_1 = OutPoint { txid: tx.txid(), index: 0 }; + let funding_txo_2 = OutPoint { txid: tx.txid(), index: 1 }; + let channel_id_1 = ChannelId::v1_from_funding_outpoint(funding_txo_1); + let channel_id_2 = ChannelId::v1_from_funding_outpoint(funding_txo_2); check_closed_events(&nodes[0], &[ ExpectedCloseEvent { channel_id: Some(channel_id_1), discard_funding: true, + channel_funding_txo: Some(funding_txo_1), + user_channel_id: Some(42), ..Default::default() }, ExpectedCloseEvent { channel_id: Some(channel_id_2), discard_funding: true, + channel_funding_txo: Some(funding_txo_2), + user_channel_id: Some(43), ..Default::default() }, ]); @@ -10565,8 +10770,10 @@ fn test_batch_funding_close_after_funding_signed() { assert_eq!(nodes[0].tx_broadcaster.txn_broadcast().len(), 0); // Force-close the channel for which we've completed the initial monitor. - let channel_id_1 = OutPoint { txid: tx.txid(), index: 0 }.to_channel_id(); - let channel_id_2 = OutPoint { txid: tx.txid(), index: 1 }.to_channel_id(); + let funding_txo_1 = OutPoint { txid: tx.txid(), index: 0 }; + let funding_txo_2 = OutPoint { txid: tx.txid(), index: 1 }; + let channel_id_1 = ChannelId::v1_from_funding_outpoint(funding_txo_1); + let channel_id_2 = ChannelId::v1_from_funding_outpoint(funding_txo_2); nodes[0].node.force_close_broadcasting_latest_txn(&channel_id_1, &nodes[1].node.get_our_node_id()).unwrap(); check_added_monitors(&nodes[0], 2); { @@ -10598,11 +10805,15 @@ fn test_batch_funding_close_after_funding_signed() { ExpectedCloseEvent { channel_id: Some(channel_id_1), discard_funding: true, + channel_funding_txo: Some(funding_txo_1), + user_channel_id: Some(42), ..Default::default() }, ExpectedCloseEvent { channel_id: Some(channel_id_2), discard_funding: true, + channel_funding_txo: Some(funding_txo_2), + user_channel_id: Some(43), ..Default::default() }, ]); @@ -10622,7 +10833,7 @@ fn do_test_funding_and_commitment_tx_confirm_same_block(confirm_remote_commitmen let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); let funding_tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 1_000_000, 0); - let chan_id = chain::transaction::OutPoint { txid: funding_tx.txid(), index: 0 }.to_channel_id(); + let chan_id = ChannelId::v1_from_funding_outpoint(chain::transaction::OutPoint { txid: funding_tx.txid(), index: 0 }); assert_eq!(nodes[0].node.list_channels().len(), 1); assert_eq!(nodes[1].node.list_channels().len(), 1);