From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Fri, 19 Jul 2019 22:02:25 +0000 (-0400) Subject: Merge pull request #336 from ariard/2019-04-in-flight-txn-tracking-clean X-Git-Tag: v0.0.12~206 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=8470e60415eceecf9abcb518130f08bb06b91640;hp=-c;p=rust-lightning Merge pull request #336 from ariard/2019-04-in-flight-txn-tracking-clean 2019 04 in flight txn tracking clean --- 8470e60415eceecf9abcb518130f08bb06b91640 diff --combined src/ln/channelmanager.rs index e3bd06fd,64a9f2eb..1bf3f0fe --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@@ -28,7 -28,7 +28,7 @@@ use secp256k1 use chain::chaininterface::{BroadcasterInterface,ChainListener,ChainWatchInterface,FeeEstimator}; use chain::transaction::OutPoint; use ln::channel::{Channel, ChannelError}; - use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, HTLC_FAIL_ANTI_REORG_DELAY}; + use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, ManyChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY}; use ln::router::Route; use ln::msgs; use ln::onion_utils; @@@ -46,7 -46,7 +46,7 @@@ use std::collections::{HashMap, hash_ma use std::io::Cursor; use std::sync::{Arc, Mutex, MutexGuard, RwLock}; use std::sync::atomic::{AtomicUsize, Ordering}; -use std::time::{Instant,Duration}; +use std::time::Duration; // We hold various information about HTLC relay in the HTLC objects in Channel itself: // @@@ -247,6 -247,7 +247,6 @@@ pub(super) enum RAACommitmentOrder pub(super) struct ChannelHolder { pub(super) by_id: HashMap<[u8; 32], Channel>, pub(super) short_to_id: HashMap, - pub(super) next_forward: Instant, /// short channel id -> forward infos. Key of 0 means payments received /// Note that while this is held in the same mutex as the channels themselves, no consistency /// guarantees are made about the existence of a channel with the short id here, nor the short @@@ -265,6 -266,7 +265,6 @@@ pub(super) struct MutChannelHolder<'a> { pub(super) by_id: &'a mut HashMap<[u8; 32], Channel>, pub(super) short_to_id: &'a mut HashMap, - pub(super) next_forward: &'a mut Instant, pub(super) forward_htlcs: &'a mut HashMap>, pub(super) claimable_htlcs: &'a mut HashMap>, pub(super) pending_msg_events: &'a mut Vec, @@@ -274,6 -276,7 +274,6 @@@ impl ChannelHolder MutChannelHolder { by_id: &mut self.by_id, short_to_id: &mut self.short_to_id, - next_forward: &mut self.next_forward, forward_htlcs: &mut self.forward_htlcs, claimable_htlcs: &mut self.claimable_htlcs, pending_msg_events: &mut self.pending_msg_events, @@@ -348,20 -351,21 +348,21 @@@ pub struct ChannelManager const CLTV_EXPIRY_DELTA: u16 = 6 * 12; //TODO? pub(super) const CLTV_FAR_FAR_AWAY: u32 = 6 * 24 * 7; //TODO? - // Check that our CLTV_EXPIRY is at least CLTV_CLAIM_BUFFER + 2*HTLC_FAIL_TIMEOUT_BLOCKS + - // HTLC_FAIL_ANTI_REORG_DELAY, ie that if the next-hop peer fails the HTLC within - // HTLC_FAIL_TIMEOUT_BLOCKS then we'll still have HTLC_FAIL_TIMEOUT_BLOCKS left to fail it - // backwards ourselves before hitting the CLTV_CLAIM_BUFFER point and failing the channel - // on-chain to time out the HTLC. + // Check that our CLTV_EXPIRY is at least CLTV_CLAIM_BUFFER + ANTI_REORG_DELAY + LATENCY_GRACE_PERIOD_BLOCKS, + // ie that if the next-hop peer fails the HTLC within + // LATENCY_GRACE_PERIOD_BLOCKS then we'll still have CLTV_CLAIM_BUFFER left to timeout it onchain, + // then waiting ANTI_REORG_DELAY to be reorg-safe on the outbound HLTC and + // failing the corresponding htlc backward, and us now seeing the last block of ANTI_REORG_DELAY before + // LATENCY_GRACE_PERIOD_BLOCKS. #[deny(const_err)] #[allow(dead_code)] - const CHECK_CLTV_EXPIRY_SANITY: u32 = CLTV_EXPIRY_DELTA as u32 - 2*HTLC_FAIL_TIMEOUT_BLOCKS - CLTV_CLAIM_BUFFER - HTLC_FAIL_ANTI_REORG_DELAY; + const CHECK_CLTV_EXPIRY_SANITY: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - CLTV_CLAIM_BUFFER - ANTI_REORG_DELAY - LATENCY_GRACE_PERIOD_BLOCKS; // Check for ability of an attacker to make us fail on-chain by delaying inbound claim. See // ChannelMontior::would_broadcast_at_height for a description of why this is needed. #[deny(const_err)] #[allow(dead_code)] - const CHECK_CLTV_EXPIRY_SANITY_2: u32 = CLTV_EXPIRY_DELTA as u32 - HTLC_FAIL_TIMEOUT_BLOCKS - 2*CLTV_CLAIM_BUFFER; + const CHECK_CLTV_EXPIRY_SANITY_2: u32 = CLTV_EXPIRY_DELTA as u32 - LATENCY_GRACE_PERIOD_BLOCKS - 2*CLTV_CLAIM_BUFFER; macro_rules! secp_call { ( $res: expr, $err: expr ) => { @@@ -388,20 -392,6 +389,20 @@@ pub struct ChannelDetails pub channel_value_satoshis: u64, /// The user_id passed in to create_channel, or 0 if the channel was inbound. pub user_id: u64, + /// The available outbound capacity for sending HTLCs to the remote peer. This does not include + /// any pending HTLCs which are not yet fully resolved (and, thus, who's balance is not + /// available for inclusion in new outbound HTLCs). This further does not include any pending + /// outgoing HTLCs which are awaiting some other resolution to be sent. + pub outbound_capacity_msat: u64, + /// The available inbound capacity for the remote peer to send HTLCs to us. This does not + /// include any pending HTLCs which are not yet fully resolved (and, thus, who's balance is not + /// available for inclusion in new inbound HTLCs). + /// Note that there are some corner cases not fully handled here, so the actual available + /// inbound capacity may be slightly higher than this. + pub inbound_capacity_msat: u64, + /// True if the channel is (a) confirmed and funding_locked messages have been exchanged, (b) + /// the peer is connected, and (c) no monitor update failure is pending resolution. + pub is_live: bool, } macro_rules! handle_error { @@@ -560,6 -550,7 +561,6 @@@ impl ChannelManager channel_state: Mutex::new(ChannelHolder{ by_id: HashMap::new(), short_to_id: HashMap::new(), - next_forward: Instant::now(), forward_htlcs: HashMap::new(), claimable_htlcs: HashMap::new(), pending_msg_events: Vec::new(), @@@ -623,16 -614,12 +624,16 @@@ let channel_state = self.channel_state.lock().unwrap(); let mut res = Vec::with_capacity(channel_state.by_id.len()); for (channel_id, channel) in channel_state.by_id.iter() { + let (inbound_capacity_msat, outbound_capacity_msat) = channel.get_inbound_outbound_available_balance_msat(); res.push(ChannelDetails { channel_id: (*channel_id).clone(), short_channel_id: channel.get_short_channel_id(), remote_network_id: channel.get_their_node_id(), channel_value_satoshis: channel.get_value_satoshis(), + inbound_capacity_msat, + outbound_capacity_msat, user_id: channel.get_user_id(), + is_live: channel.is_live(), }); } res @@@ -640,9 -627,6 +641,9 @@@ /// Gets the list of usable channels, in random order. Useful as an argument to /// Router::get_route to ensure non-announced channels are used. + /// + /// These are guaranteed to have their is_live value set to true, see the documentation for + /// ChannelDetails::is_live for more info on exactly what the criteria are. pub fn list_usable_channels(&self) -> Vec { let channel_state = self.channel_state.lock().unwrap(); let mut res = Vec::with_capacity(channel_state.by_id.len()); @@@ -651,16 -635,12 +652,16 @@@ // internal/external nomenclature, but that's ok cause that's probably what the user // really wanted anyway. if channel.is_live() { + let (inbound_capacity_msat, outbound_capacity_msat) = channel.get_inbound_outbound_available_balance_msat(); res.push(ChannelDetails { channel_id: (*channel_id).clone(), short_channel_id: channel.get_short_channel_id(), remote_network_id: channel.get_their_node_id(), channel_value_satoshis: channel.get_value_satoshis(), + inbound_capacity_msat, + outbound_capacity_msat, user_id: channel.get_user_id(), + is_live: true, }); } } @@@ -841,7 -821,7 +842,7 @@@ let pending_forward_info = if next_hop_data.hmac == [0; 32] { // OUR PAYMENT! // final_expiry_too_soon - if (msg.cltv_expiry as u64) < self.latest_block_height.load(Ordering::Acquire) as u64 + (CLTV_CLAIM_BUFFER + HTLC_FAIL_TIMEOUT_BLOCKS) as u64 { + if (msg.cltv_expiry as u64) < self.latest_block_height.load(Ordering::Acquire) as u64 + (CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS) as u64 { return_err!("The final CLTV expiry is too soon to handle", 17, &[0;0]); } // final_incorrect_htlc_amount @@@ -933,8 -913,8 +934,8 @@@ break Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, Some(self.get_channel_update(chan).unwrap()))); } let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1; - // We want to have at least HTLC_FAIL_TIMEOUT_BLOCKS to fail prior to going on chain CLAIM_BUFFER blocks before expiration - if msg.cltv_expiry <= cur_height + CLTV_CLAIM_BUFFER + HTLC_FAIL_TIMEOUT_BLOCKS as u32 { // expiry_too_soon + // We want to have at least LATENCY_GRACE_PERIOD_BLOCKS to fail prior to going on chain CLAIM_BUFFER blocks before expiration + if msg.cltv_expiry <= cur_height + CLTV_CLAIM_BUFFER + LATENCY_GRACE_PERIOD_BLOCKS as u32 { // expiry_too_soon break Some(("CLTV expiry is too close", 0x1000 | 14, Some(self.get_channel_update(chan).unwrap()))); } if msg.cltv_expiry > cur_height + CLTV_FAR_FAR_AWAY as u32 { // expiry_too_far @@@ -1205,6 -1185,10 +1206,6 @@@ let mut channel_state_lock = self.channel_state.lock().unwrap(); let channel_state = channel_state_lock.borrow_parts(); - if cfg!(not(feature = "fuzztarget")) && Instant::now() < *channel_state.next_forward { - return; - } - for (short_chan_id, mut pending_forwards) in channel_state.forward_htlcs.drain() { if short_chan_id != 0 { let forward_chan_id = match channel_state.short_to_id.get(&short_chan_id) { @@@ -1484,7 -1468,8 +1485,7 @@@ let mut forward_event = None; if channel_state_lock.forward_htlcs.is_empty() { - forward_event = Some(Instant::now() + Duration::from_millis(((rng::rand_f32() * 4.0 + 1.0) * MIN_HTLC_RELAY_HOLDING_CELL_MILLIS as f32) as u64)); - channel_state_lock.next_forward = forward_event.unwrap(); + forward_event = Some(Duration::from_millis(((rng::rand_f32() * 4.0 + 1.0) * MIN_HTLC_RELAY_HOLDING_CELL_MILLIS as f32) as u64)); } match channel_state_lock.forward_htlcs.entry(short_channel_id) { hash_map::Entry::Occupied(mut entry) => { @@@ -2093,7 -2078,8 +2094,7 @@@ if !pending_forwards.is_empty() { let mut channel_state = self.channel_state.lock().unwrap(); if channel_state.forward_htlcs.is_empty() { - forward_event = Some(Instant::now() + Duration::from_millis(((rng::rand_f32() * 4.0 + 1.0) * MIN_HTLC_RELAY_HOLDING_CELL_MILLIS as f32) as u64)); - channel_state.next_forward = forward_event.unwrap(); + forward_event = Some(Duration::from_millis(((rng::rand_f32() * 4.0 + 1.0) * MIN_HTLC_RELAY_HOLDING_CELL_MILLIS as f32) as u64)); } for (forward_info, prev_htlc_id) in pending_forwards.drain(..) { match channel_state.forward_htlcs.entry(forward_info.short_channel_id) { @@@ -2489,7 -2475,7 +2490,7 @@@ impl ChainListener for ChannelManager } /// We force-close the channel without letting our counterparty participate in the shutdown - fn block_disconnected(&self, header: &BlockHeader) { + fn block_disconnected(&self, header: &BlockHeader, _: u32) { let _ = self.total_consistency_lock.read().unwrap(); let mut failed_channels = Vec::new(); { @@@ -3102,6 -3088,7 +3103,6 @@@ impl<'a, R : ::std::io::Read> ReadableA channel_state: Mutex::new(ChannelHolder { by_id, short_to_id, - next_forward: Instant::now(), forward_htlcs, claimable_htlcs, pending_msg_events: Vec::new(), diff --combined src/ln/functional_test_utils.rs index ffa7e7f7,15c372dd..14ba7e5f --- a/src/ln/functional_test_utils.rs +++ b/src/ln/functional_test_utils.rs @@@ -15,11 -15,12 +15,12 @@@ use util::logger::Logger use util::config::UserConfig; use bitcoin::util::hash::BitcoinHash; - use bitcoin::blockdata::block::BlockHeader; + use bitcoin::blockdata::block::{BlockHeader, Block}; use bitcoin::blockdata::transaction::{Transaction, TxOut}; use bitcoin::network::constants::Network; use bitcoin_hashes::sha256::Hash as Sha256; + use bitcoin_hashes::sha256d::Hash as Sha256d; use bitcoin_hashes::Hash; use secp256k1::Secp256k1; @@@ -32,6 -33,7 +33,6 @@@ use std::collections::HashMap use std::default::Default; use std::rc::Rc; use std::sync::{Arc, Mutex}; -use std::time::Instant; use std::mem; pub const CHAN_CONFIRM_DEPTH: u32 = 100; @@@ -45,6 -47,30 +46,30 @@@ pub fn confirm_transaction(chain: &chai } } + pub fn connect_blocks(chain: &chaininterface::ChainWatchInterfaceUtil, depth: u32, height: u32, parent: bool, prev_blockhash: Sha256d) -> Sha256d { + let mut header = BlockHeader { version: 0x2000000, prev_blockhash: if parent { prev_blockhash } else { Default::default() }, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + chain.block_connected_checked(&header, height + 1, &Vec::new(), &Vec::new()); + for i in 2..depth + 1 { + header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + chain.block_connected_checked(&header, height + i, &Vec::new(), &Vec::new()); + } + header.bitcoin_hash() + } + + pub fn disconnect_blocks(chain: &chaininterface::ChainWatchInterfaceUtil, depth: u32, height: u32, parent: bool, prev_blockhash: Sha256d) { + let mut header = BlockHeader { version: 0x2000000, prev_blockhash: if parent { prev_blockhash } else { Default::default() }, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + let mut blocks = Vec::new(); + for _ in 0..depth { + blocks.push(Block { header, txdata: Vec::new() }); + header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + } + let mut height = height; + for block in blocks.pop() { + chain.block_disconnected(&block.header, height); + height -= 1; + } + } + pub struct Node { pub chain_monitor: Arc, pub tx_broadcaster: Arc, @@@ -535,6 -561,8 +560,6 @@@ macro_rules! expect_pending_htlcs_forwa Event::PendingHTLCsForwardable { .. } => { }, _ => panic!("Unexpected event"), }; - let node_ref: &Node = &$node; - node_ref.node.channel_state.lock().unwrap().next_forward = Instant::now(); $node.node.process_pending_htlc_forwards(); }} } diff --combined src/ln/functional_tests.rs index 6580abbc,084b6bc5..210a4e91 --- a/src/ln/functional_tests.rs +++ b/src/ln/functional_tests.rs @@@ -8,7 -8,7 +8,7 @@@ use chain::keysinterface::{KeysInterfac use chain::keysinterface; use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC, BREAKDOWN_TIMEOUT}; use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,HTLCForwardInfo,RAACommitmentOrder, PaymentPreimage, PaymentHash}; - use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, ManyChannelMonitor}; + use ln::channelmonitor::{ChannelMonitor, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ManyChannelMonitor, ANTI_REORG_DELAY}; use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT}; use ln::onion_utils; use ln::router::{Route, RouteHop}; @@@ -27,7 -27,7 +27,7 @@@ use bitcoin::util::bip143 use bitcoin::util::address::Address; use bitcoin::util::bip32::{ChildNumber, ExtendedPubKey, ExtendedPrivKey}; use bitcoin::blockdata::block::{Block, BlockHeader}; - use bitcoin::blockdata::transaction::{Transaction, TxOut, TxIn, SigHashType}; + use bitcoin::blockdata::transaction::{Transaction, TxOut, TxIn, SigHashType, OutPoint as BitcoinOutPoint}; use bitcoin::blockdata::script::{Builder, Script}; use bitcoin::blockdata::opcodes; use bitcoin::blockdata::constants::genesis_block; @@@ -43,6 -43,7 +43,6 @@@ use std::collections::{BTreeSet, HashMa use std::default::Default; use std::sync::Arc; use std::sync::atomic::Ordering; -use std::time::Instant; use std::mem; use ln::functional_test_utils::*; @@@ -1694,7 -1695,7 +1694,7 @@@ fn channel_monitor_network_test() { let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[3].chain_monitor.block_connected_checked(&header, 2, &Vec::new()[..], &[0; 0]); - for i in 3..TEST_FINAL_CLTV + 2 + HTLC_FAIL_TIMEOUT_BLOCKS + 1 { + for i in 3..TEST_FINAL_CLTV + 2 + LATENCY_GRACE_PERIOD_BLOCKS + 1 { header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[3].chain_monitor.block_connected_checked(&header, i, &Vec::new()[..], &[0; 0]); } @@@ -1870,6 -1871,7 +1870,7 @@@ fn claim_htlc_outputs_shared_tx() let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); @@@ -1937,6 -1939,7 +1938,7 @@@ fn claim_htlc_outputs_single_tx() let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[0].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200); nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 200); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash()); let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), 1); @@@ -1948,7 -1951,7 +1950,7 @@@ } let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); - assert_eq!(node_txn.len(), 12); // ChannelManager : 2, ChannelMontitor: 8 (1 standard revoked output, 2 revocation htlc tx, 1 local commitment tx + 1 htlc timeout tx) * 2 (block-rescan) + assert_eq!(node_txn.len(), 22); // ChannelManager : 2, ChannelMontitor: 8 (1 standard revoked output, 2 revocation htlc tx, 1 local commitment tx + 1 htlc timeout tx) * 2 (block-rescan) + 5 * (1 local commitment tx + 1 htlc timeout tx) assert_eq!(node_txn[0], node_txn[7]); assert_eq!(node_txn[1], node_txn[8]); @@@ -1958,6 -1961,10 +1960,10 @@@ assert_eq!(node_txn[3], node_txn[5]); //local commitment tx + htlc timeout tx broadcasted by ChannelManger assert_eq!(node_txn[4], node_txn[6]); + for i in 12..22 { + if i % 2 == 0 { assert_eq!(node_txn[3], node_txn[i]); } else { assert_eq!(node_txn[4], node_txn[i]); } + } + assert_eq!(node_txn[0].input.len(), 1); assert_eq!(node_txn[1].input.len(), 1); assert_eq!(node_txn[2].input.len(), 1); @@@ -2234,6 -2241,7 +2240,7 @@@ fn test_htlc_on_chain_timeout() } nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![timeout_tx]}, 1); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); check_added_monitors!(nodes[1], 0); check_closed_broadcast!(nodes[1]); @@@ -2292,6 -2300,7 +2299,7 @@@ fn test_simple_commitment_revoked_fail_ let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42}; nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); check_added_monitors!(nodes[1], 0); check_closed_broadcast!(nodes[1]); @@@ -2444,6 -2453,7 +2452,7 @@@ fn do_test_commitment_revoked_fail_back let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42}; nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); let events = nodes[1].node.get_and_clear_pending_events(); assert_eq!(events.len(), if deliver_bs_raa { 1 } else { 2 }); @@@ -2459,6 -2469,7 +2468,6 @@@ _ => panic!("Unexpected event"), }; } - nodes[1].node.channel_state.lock().unwrap().next_forward = Instant::now(); nodes[1].node.process_pending_htlc_forwards(); check_added_monitors!(nodes[1], 1); @@@ -2664,8 -2675,10 +2673,10 @@@ fn test_unconf_chan() header = BlockHeader { version: 0x20000000, prev_blockhash: header.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; headers.push(header.clone()); } + let mut height = 99; while !headers.is_empty() { - nodes[0].node.block_disconnected(&headers.pop().unwrap()); + nodes[0].node.block_disconnected(&headers.pop().unwrap(), height); + height -= 1; } check_closed_broadcast!(nodes[0]); let channel_state = nodes[0].node.channel_state.lock().unwrap(); @@@ -2811,6 -2824,7 +2822,6 @@@ fn do_test_drop_messages_peer_disconnec nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false); reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false)); - nodes[1].node.channel_state.lock().unwrap().next_forward = Instant::now(); nodes[1].node.process_pending_htlc_forwards(); let events_2 = nodes[1].node.get_and_clear_pending_events(); @@@ -3885,6 -3899,7 +3896,7 @@@ fn test_duplicate_payment_hash_one_fail check_spends!(htlc_success_txn[1], commitment_txn[0].clone()); nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![htlc_timeout_tx] }, 200); + connect_blocks(&nodes[1].chain_monitor, ANTI_REORG_DELAY - 1, 200, true, header.bitcoin_hash()); expect_pending_htlcs_forwardable!(nodes[1]); let htlc_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); assert!(htlc_updates.update_add_htlcs.is_empty()); @@@ -4103,6 -4118,7 +4115,7 @@@ fn do_test_fail_backwards_unrevoked_rem } else { nodes[2].chain_monitor.block_connected_checked(&header, 1, &[&ds_prev_commitment_tx[0]], &[1; 1]); } + connect_blocks(&nodes[2].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); check_closed_broadcast!(nodes[2]); expect_pending_htlcs_forwardable!(nodes[2]); check_added_monitors!(nodes[2], 2); @@@ -4332,7 -4348,7 +4345,7 @@@ fn do_htlc_claim_current_remote_commitm // to "time out" the HTLC. let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - for i in 1..TEST_FINAL_CLTV + HTLC_FAIL_TIMEOUT_BLOCKS + CHAN_CONFIRM_DEPTH + 1 { + for i in 1..TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + CHAN_CONFIRM_DEPTH + 1 { nodes[0].chain_monitor.block_connected_checked(&header, i, &Vec::new(), &Vec::new()); header.prev_blockhash = header.bitcoin_hash(); } @@@ -4371,7 -4387,7 +4384,7 @@@ fn do_htlc_claim_previous_remote_commit } let mut header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; - for i in 1..TEST_FINAL_CLTV + HTLC_FAIL_TIMEOUT_BLOCKS + CHAN_CONFIRM_DEPTH + 1 { + for i in 1..TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + CHAN_CONFIRM_DEPTH + 1 { nodes[0].chain_monitor.block_connected_checked(&header, i, &Vec::new(), &Vec::new()); header.prev_blockhash = header.bitcoin_hash(); } @@@ -4460,6 -4476,7 +4473,6 @@@ fn run_onion_failure_test_with_fail_int macro_rules! expect_htlc_forward { ($node: expr) => {{ expect_event!($node, Event::PendingHTLCsForwardable); - $node.node.channel_state.lock().unwrap().next_forward = Instant::now(); $node.node.process_pending_htlc_forwards(); }} } @@@ -4774,7 -4791,7 +4787,7 @@@ fn test_onion_failure() }, || {}, true, Some(UPDATE|13), Some(msgs::HTLCFailChannelUpdate::ChannelClosed { short_channel_id: channels[0].0.contents.short_channel_id, is_permanent: true})); run_onion_failure_test("expiry_too_soon", 0, &nodes, &route, &payment_hash, |msg| { - let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - HTLC_FAIL_TIMEOUT_BLOCKS + 1; + let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - LATENCY_GRACE_PERIOD_BLOCKS + 1; let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[1].chain_monitor.block_connected_checked(&header, height, &Vec::new()[..], &[0; 0]); }, ||{}, true, Some(UPDATE|14), Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage{msg: ChannelUpdate::dummy()})); @@@ -4784,7 -4801,7 +4797,7 @@@ }, false, Some(PERM|15), None); run_onion_failure_test("final_expiry_too_soon", 1, &nodes, &route, &payment_hash, |msg| { - let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - HTLC_FAIL_TIMEOUT_BLOCKS + 1; + let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - LATENCY_GRACE_PERIOD_BLOCKS + 1; let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; nodes[2].chain_monitor.block_connected_checked(&header, height, &Vec::new()[..], &[0; 0]); }, || {}, true, Some(17), None); @@@ -5496,3 -5513,272 +5509,272 @@@ fn test_update_fulfill_htlc_bolt2_after check_added_monitors!(nodes[1], 1); } + + fn do_test_failure_delay_dust_htlc_local_commitment(announce_latest: bool) { + // Dust-HTLC failure updates must be delayed until failure-trigger tx (in this case local commitment) reach ANTI_REORG_DELAY + // We can have at most two valid local commitment tx, so both cases must be covered, and both txs must be checked to get them all as + // HTLC could have been removed from lastest local commitment tx but still valid until we get remote RAA + + let nodes = create_network(2); + let chan =create_announced_chan_between_nodes(&nodes, 0, 1); + + let bs_dust_limit = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().our_dust_limit_satoshis; + + // We route 2 dust-HTLCs between A and B + let (_, payment_hash_1) = route_payment(&nodes[0], &[&nodes[1]], bs_dust_limit*1000); + let (_, payment_hash_2) = route_payment(&nodes[0], &[&nodes[1]], bs_dust_limit*1000); + route_payment(&nodes[0], &[&nodes[1]], 1000000); + + // Cache one local commitment tx as previous + let as_prev_commitment_tx = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().last_local_commitment_txn.clone(); + + // Fail one HTLC to prune it in the will-be-latest-local commitment tx + assert!(nodes[1].node.fail_htlc_backwards(&payment_hash_2)); + check_added_monitors!(nodes[1], 0); + expect_pending_htlcs_forwardable!(nodes[1]); + check_added_monitors!(nodes[1], 1); + + let remove = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fail_htlc(&nodes[1].node.get_our_node_id(), &remove.update_fail_htlcs[0]).unwrap(); + nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), &remove.commitment_signed).unwrap(); + check_added_monitors!(nodes[0], 1); + + // Cache one local commitment tx as lastest + let as_last_commitment_tx = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().last_local_commitment_txn.clone(); + + let events = nodes[0].node.get_and_clear_pending_msg_events(); + match events[0] { + MessageSendEvent::SendRevokeAndACK { node_id, .. } => { + assert_eq!(node_id, nodes[1].node.get_our_node_id()); + }, + _ => panic!("Unexpected event"), + } + match events[1] { + MessageSendEvent::UpdateHTLCs { node_id, .. } => { + assert_eq!(node_id, nodes[1].node.get_our_node_id()); + }, + _ => panic!("Unexpected event"), + } + + assert_ne!(as_prev_commitment_tx, as_last_commitment_tx); + // Fail the 2 dust-HTLCs, move their failure in maturation buffer (htlc_updated_waiting_threshold_conf) + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + if announce_latest { + nodes[0].chain_monitor.block_connected_checked(&header, 1, &[&as_last_commitment_tx[0]], &[1; 1]); + } else { + nodes[0].chain_monitor.block_connected_checked(&header, 1, &[&as_prev_commitment_tx[0]], &[1; 1]); + } + + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match events[0] { + MessageSendEvent::BroadcastChannelUpdate { .. } => {}, + _ => panic!("Unexpected event"), + } + + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); + let events = nodes[0].node.get_and_clear_pending_events(); + // Only 2 PaymentFailed events should show up, over-dust HTLC has to be failed by timeout tx + assert_eq!(events.len(), 2); + let mut first_failed = false; + for event in events { + match event { + Event::PaymentFailed { payment_hash, .. } => { + if payment_hash == payment_hash_1 { + assert!(!first_failed); + first_failed = true; + } else { + assert_eq!(payment_hash, payment_hash_2); + } + } + _ => panic!("Unexpected event"), + } + } + } + + #[test] + fn test_failure_delay_dust_htlc_local_commitment() { + do_test_failure_delay_dust_htlc_local_commitment(true); + do_test_failure_delay_dust_htlc_local_commitment(false); + } + + #[test] + fn test_no_failure_dust_htlc_local_commitment() { + // Transaction filters for failing back dust htlc based on local commitment txn infos has been + // prone to error, we test here that a dummy transaction don't fail them. + + let nodes = create_network(2); + let chan = create_announced_chan_between_nodes(&nodes, 0, 1); + + // Rebalance a bit + send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000); + + let as_dust_limit = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().our_dust_limit_satoshis; + let bs_dust_limit = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().our_dust_limit_satoshis; + + // We route 2 dust-HTLCs between A and B + let (preimage_1, _) = route_payment(&nodes[0], &[&nodes[1]], bs_dust_limit*1000); + let (preimage_2, _) = route_payment(&nodes[1], &[&nodes[0]], as_dust_limit*1000); + + // Build a dummy invalid transaction trying to spend a commitment tx + let input = TxIn { + previous_output: BitcoinOutPoint { txid: chan.3.txid(), vout: 0 }, + script_sig: Script::new(), + sequence: 0, + witness: Vec::new(), + }; + + let outp = TxOut { + script_pubkey: Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script(), + value: 10000, + }; + + let dummy_tx = Transaction { + version: 2, + lock_time: 0, + input: vec![input], + output: vec![outp] + }; + + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + nodes[0].chan_monitor.simple_monitor.block_connected(&header, 1, &[&dummy_tx], &[1;1]); + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 0); + // We broadcast a few more block to check everything is all right + connect_blocks(&nodes[0].chain_monitor, 20, 1, true, header.bitcoin_hash()); + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + assert_eq!(nodes[0].node.get_and_clear_pending_msg_events().len(), 0); + + claim_payment(&nodes[0], &vec!(&nodes[1])[..], preimage_1); + claim_payment(&nodes[1], &vec!(&nodes[0])[..], preimage_2); + } + + fn do_test_sweep_outbound_htlc_failure_update(revoked: bool, local: bool) { + // Outbound HTLC-failure updates must be cancelled if we get a reorg before we reach ANTI_REORG_DELAY. + // Broadcast of revoked remote commitment tx, trigger failure-update of dust/non-dust HTLCs + // Broadcast of remote commitment tx, trigger failure-update of dust-HTLCs + // Broadcast of timeout tx on remote commitment tx, trigger failure-udate of non-dust HTLCs + // Broadcast of local commitment tx, trigger failure-update of dust-HTLCs + // Broadcast of HTLC-timeout tx on local commitment tx, trigger failure-update of non-dust HTLCs + + let nodes = create_network(3); + let chan = create_announced_chan_between_nodes(&nodes, 0, 1); + + let bs_dust_limit = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().our_dust_limit_satoshis; + + let (payment_preimage_1, dust_hash) = route_payment(&nodes[0], &[&nodes[1]], bs_dust_limit*1000); + let (payment_preimage_2, non_dust_hash) = route_payment(&nodes[0], &[&nodes[1]], 1000000); + + let as_commitment_tx = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().last_local_commitment_txn.clone(); + let bs_commitment_tx = nodes[1].node.channel_state.lock().unwrap().by_id.get(&chan.2).unwrap().last_local_commitment_txn.clone(); + + // We revoked bs_commitment_tx + if revoked { + let (payment_preimage_3, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000); + claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage_3); + } + + let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + let mut timeout_tx = Vec::new(); + if local { + // We fail dust-HTLC 1 by broadcast of local commitment tx + nodes[0].chain_monitor.block_connected_checked(&header, 1, &[&as_commitment_tx[0]], &[1; 1]); + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match events[0] { + MessageSendEvent::BroadcastChannelUpdate { .. } => {}, + _ => panic!("Unexpected event"), + } + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].clone()); + let parent_hash = connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash()); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + assert_eq!(payment_hash, dust_hash); + }, + _ => panic!("Unexpected event"), + } + assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), OFFERED_HTLC_SCRIPT_WEIGHT); + // We fail non-dust-HTLC 2 by broadcast of local HTLC-timeout tx on local commitment tx + let header_2 = BlockHeader { version: 0x20000000, prev_blockhash: parent_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + nodes[0].chain_monitor.block_connected_checked(&header_2, 7, &[&timeout_tx[0]], &[1; 1]); + let header_3 = BlockHeader { version: 0x20000000, prev_blockhash: header_2.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash()); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + assert_eq!(payment_hash, non_dust_hash); + }, + _ => panic!("Unexpected event"), + } + } else { + // We fail dust-HTLC 1 by broadcast of remote commitment tx. If revoked, fail also non-dust HTLC + nodes[0].chain_monitor.block_connected_checked(&header, 1, &[&bs_commitment_tx[0]], &[1; 1]); + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match events[0] { + MessageSendEvent::BroadcastChannelUpdate { .. } => {}, + _ => panic!("Unexpected event"), + } + timeout_tx.push(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap()[0].clone()); + let parent_hash = connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 2, true, header.bitcoin_hash()); + let header_2 = BlockHeader { version: 0x20000000, prev_blockhash: parent_hash, merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + if !revoked { + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + assert_eq!(payment_hash, dust_hash); + }, + _ => panic!("Unexpected event"), + } + assert_eq!(timeout_tx[0].input[0].witness.last().unwrap().len(), ACCEPTED_HTLC_SCRIPT_WEIGHT); + // We fail non-dust-HTLC 2 by broadcast of local timeout tx on remote commitment tx + nodes[0].chain_monitor.block_connected_checked(&header_2, 7, &[&timeout_tx[0]], &[1; 1]); + assert_eq!(nodes[0].node.get_and_clear_pending_events().len(), 0); + let header_3 = BlockHeader { version: 0x20000000, prev_blockhash: header_2.bitcoin_hash(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; + connect_blocks(&nodes[0].chain_monitor, ANTI_REORG_DELAY - 1, 8, true, header_3.bitcoin_hash()); + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + assert_eq!(payment_hash, non_dust_hash); + }, + _ => panic!("Unexpected event"), + } + } else { + // If revoked, both dust & non-dust HTLCs should have been failed after ANTI_REORG_DELAY confs of revoked + // commitment tx + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + let first; + match events[0] { + Event::PaymentFailed { payment_hash, .. } => { + if payment_hash == dust_hash { first = true; } + else { first = false; } + }, + _ => panic!("Unexpected event"), + } + match events[1] { + Event::PaymentFailed { payment_hash, .. } => { + if first { assert_eq!(payment_hash, non_dust_hash); } + else { assert_eq!(payment_hash, dust_hash); } + }, + _ => panic!("Unexpected event"), + } + } + } + } + + #[test] + fn test_sweep_outbound_htlc_failure_update() { + do_test_sweep_outbound_htlc_failure_update(false, true); + do_test_sweep_outbound_htlc_failure_update(false, false); + do_test_sweep_outbound_htlc_failure_update(true, false); + }