Skip to content

Commit

Permalink
Automatically close channels that go unfunded for 2016 blocks
Browse files Browse the repository at this point in the history 
As recommended by BOLT 2 added in
lightning/bolts#839
  • Loading branch information
@TheBlueMatt
TheBlueMatt committed Nov 16, 2021
1 parent b288a27 commit 3582921
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 20 deletions.
55 changes: 42 additions & 13 deletions lightning/src/ln/channel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,11 @@ pub const FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE: u64 = 2;
#[cfg(not(fuzzing))]
const FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE: u64 = 2;

/// If we fail to see a funding transaction confirmed on-chain within this many blocks after the
/// channel creation on an inbound channel, we simply force-close and move on.
/// This constant is the one suggested in BOLT 2.
pub(crate) const FUNDING_CONF_DEADLINE_BLOCKS: u32 = 2016;

// TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking
// has been completed, and then turn into a Channel to get compiler-time enforcement of things like
// calling channel_id() before we're set up or things like get_outbound_funding_signed on an
Expand Down Expand Up @@ -476,6 +481,10 @@ pub(super) struct Channel<Signer: Sign> {
funding_tx_confirmed_in: Option<BlockHash>,
funding_tx_confirmation_height: u32,
short_channel_id: Option<u64>,
/// Either the height at which this channel was created or the height at which it was last
/// serialized if it was serialized by versions prior to 0.0.103.
/// We use this to close if funding is never broadcasted.
channel_creation_height: u32,

counterparty_dust_limit_satoshis: u64,
#[cfg(test)]
Expand Down Expand Up @@ -647,7 +656,10 @@ impl<Signer: Sign> Channel<Signer> {
}

// Constructors:
pub fn new_outbound<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, APIError>
pub fn new_outbound<K: Deref, F: Deref>(
fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
channel_value_satoshis: u64, push_msat: u64, user_id: u64, config: &UserConfig, current_chain_height: u32
) -> Result<Channel<Signer>, APIError>
where K::Target: KeysInterface<Signer = Signer>,
F::Target: FeeEstimator,
{
Expand Down Expand Up @@ -735,6 +747,7 @@ impl<Signer: Sign> Channel<Signer> {
funding_tx_confirmed_in: None,
funding_tx_confirmation_height: 0,
short_channel_id: None,
channel_creation_height: current_chain_height,

feerate_per_kw: feerate,
counterparty_dust_limit_satoshis: 0,
Expand Down Expand Up @@ -808,7 +821,10 @@ impl<Signer: Sign> Channel<Signer> {

/// Creates a new channel from a remote sides' request for one.
/// Assumes chain_hash has already been checked and corresponds with what we expect!
pub fn new_from_req<K: Deref, F: Deref>(fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig) -> Result<Channel<Signer>, ChannelError>
pub fn new_from_req<K: Deref, F: Deref>(
fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig, current_chain_height: u32
) -> Result<Channel<Signer>, ChannelError>
where K::Target: KeysInterface<Signer = Signer>,
F::Target: FeeEstimator
{
Expand Down Expand Up @@ -1021,6 +1037,7 @@ impl<Signer: Sign> Channel<Signer> {
funding_tx_confirmed_in: None,
funding_tx_confirmation_height: 0,
short_channel_id: None,
channel_creation_height: current_chain_height,

feerate_per_kw: msg.feerate_per_kw,
channel_value_satoshis: msg.funding_satoshis,
Expand Down Expand Up @@ -4236,6 +4253,13 @@ impl<Signer: Sign> Channel<Signer> {
self.minimum_depth.unwrap(), funding_tx_confirmations);
return Err(ClosureReason::ProcessingError { err: err_reason });
}
} else if !self.is_outbound() && self.funding_tx_confirmed_in.is_none() &&
height >= self.channel_creation_height + FUNDING_CONF_DEADLINE_BLOCKS {
log_info!(logger, "Closing channel {} due to funding timeout", log_bytes!(self.channel_id));
// If funding_tx_confirmed_in is unset, the channel must not be active
assert!(non_shutdown_state <= ChannelState::ChannelFunded as u32);
assert_eq!(non_shutdown_state & ChannelState::OurFundingLocked as u32, 0);
return Err(ClosureReason::FundingTimedOut);
}

Ok((None, timed_out_htlcs))
Expand Down Expand Up @@ -5274,16 +5298,18 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
(7, self.shutdown_scriptpubkey, option),
(9, self.target_closing_feerate_sats_per_kw, option),
(11, self.monitor_pending_finalized_fulfills, vec_type),
(13, self.channel_creation_height, required),
});

Ok(())
}
}

const MAX_ALLOC_SIZE: usize = 64*1024;
impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
where K::Target: KeysInterface<Signer = Signer> {
fn read<R : io::Read>(reader: &mut R, keys_source: &'a K) -> Result<Self, DecodeError> {
fn read<R : io::Read>(reader: &mut R, args: (&'a K, u32)) -> Result<Self, DecodeError> {
let (keys_source, serialized_height) = args;
let ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);

let user_id = Readable::read(reader)?;
Expand Down Expand Up @@ -5511,6 +5537,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
// Prior to supporting channel type negotiation, all of our channels were static_remotekey
// only, so we default to that if none was written.
let mut channel_type = Some(ChannelTypeFeatures::only_static_remote_key());
let mut channel_creation_height = Some(serialized_height);
read_tlv_fields!(reader, {
(0, announcement_sigs, option),
(1, minimum_depth, option),
Expand All @@ -5520,6 +5547,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
(7, shutdown_scriptpubkey, option),
(9, target_closing_feerate_sats_per_kw, option),
(11, monitor_pending_finalized_fulfills, vec_type),
(13, channel_creation_height, option),
});

let chan_features = channel_type.as_ref().unwrap();
Expand Down Expand Up @@ -5584,6 +5612,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
funding_tx_confirmed_in,
funding_tx_confirmation_height,
short_channel_id,
channel_creation_height: channel_creation_height.unwrap(),

counterparty_dust_limit_satoshis,
holder_dust_limit_satoshis,
Expand Down Expand Up @@ -5732,7 +5761,7 @@ mod tests {
let secp_ctx = Secp256k1::new();
let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
let config = UserConfig::default();
match Channel::<EnforcingSigner>::new_outbound(&&fee_estimator, &&keys_provider, node_id, &features, 10000000, 100000, 42, &config) {
match Channel::<EnforcingSigner>::new_outbound(&&fee_estimator, &&keys_provider, node_id, &features, 10000000, 100000, 42, &config, 0) {
Err(APIError::IncompatibleShutdownScript { script }) => {
assert_eq!(script.into_inner(), non_v0_segwit_shutdown_script.into_inner());
},
Expand All @@ -5754,7 +5783,7 @@ mod tests {

let node_a_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
let config = UserConfig::default();
let node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
let node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_a_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();

// Now change the fee so we can check that the fee in the open_channel message is the
// same as the old fee.
Expand All @@ -5779,13 +5808,13 @@ mod tests {
// Create Node A's channel pointing to Node B's pubkey
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
let config = UserConfig::default();
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();

// Create Node B's channel by receiving Node A's open_channel message
// Make sure A's dust limit is as we expect.
let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash());
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
let node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config).unwrap();
let node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0).unwrap();

// Node B --> Node A: accept channel, explicitly setting B's dust limit.
let mut accept_channel_msg = node_b_chan.get_accept_channel();
Expand Down Expand Up @@ -5849,7 +5878,7 @@ mod tests {

let node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
let config = UserConfig::default();
let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();

let commitment_tx_fee_0_htlcs = chan.commit_tx_fee_msat(0);
let commitment_tx_fee_1_htlc = chan.commit_tx_fee_msat(1);
Expand Down Expand Up @@ -5898,12 +5927,12 @@ mod tests {
// Create Node A's channel pointing to Node B's pubkey
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
let config = UserConfig::default();
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();

// Create Node B's channel by receiving Node A's open_channel message
let open_channel_msg = node_a_chan.get_open_channel(chain_hash);
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config).unwrap();
let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0).unwrap();

// Node B --> Node A: accept channel
let accept_channel_msg = node_b_chan.get_accept_channel();
Expand Down Expand Up @@ -5960,7 +5989,7 @@ mod tests {
// Create a channel.
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
let config = UserConfig::default();
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config).unwrap();
let mut node_a_chan = Channel::<EnforcingSigner>::new_outbound(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();
assert!(node_a_chan.counterparty_forwarding_info.is_none());
assert_eq!(node_a_chan.holder_htlc_minimum_msat, 1); // the default
assert!(node_a_chan.counterparty_forwarding_info().is_none());
Expand Down Expand Up @@ -6024,7 +6053,7 @@ mod tests {
let counterparty_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap());
let mut config = UserConfig::default();
config.channel_options.announced_channel = false;
let mut chan = Channel::<InMemorySigner>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, &InitFeatures::known(), 10_000_000, 100000, 42, &config).unwrap(); // Nothing uses their network key in this test
let mut chan = Channel::<InMemorySigner>::new_outbound(&&feeest, &&keys_provider, counterparty_node_id, &InitFeatures::known(), 10_000_000, 100000, 42, &config, 0).unwrap(); // Nothing uses their network key in this test
chan.holder_dust_limit_satoshis = 546;
chan.counterparty_selected_channel_reserve_satoshis = Some(0); // Filled in in accept_channel

Expand Down
8 changes: 5 additions & 3 deletions lightning/src/ln/channelmanager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1398,7 +1398,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
let peer_state = peer_state.lock().unwrap();
let their_features = &peer_state.latest_features;
let config = if override_config.is_some() { override_config.as_ref().unwrap() } else { &self.default_configuration };
Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features, channel_value_satoshis, push_msat, user_channel_id, config)?
Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features,
channel_value_satoshis, push_msat, user_channel_id, config, self.best_block.read().unwrap().height())?
},
None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", their_network_key) }),
}
Expand Down Expand Up @@ -3645,7 +3646,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash".to_owned(), msg.temporary_channel_id.clone()));
}

let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(), &their_features, msg, 0, &self.default_configuration)
let channel = Channel::new_from_req(&self.fee_estimator, &self.keys_manager, counterparty_node_id.clone(),
&their_features, msg, 0, &self.default_configuration, self.best_block.read().unwrap().height())
.map_err(|e| MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id))?;
let mut channel_state_lock = self.channel_state.lock().unwrap();
let channel_state = &mut *channel_state_lock;
Expand Down Expand Up @@ -5833,7 +5835,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
let mut short_to_id = HashMap::with_capacity(cmp::min(channel_count as usize, 128));
let mut channel_closures = Vec::new();
for _ in 0..channel_count {
let mut channel: Channel<Signer> = Channel::read(reader, &args.keys_manager)?;
let mut channel: Channel<Signer> = Channel::read(reader, (&args.keys_manager, best_block_height))?;
let funding_txo = channel.get_funding_txo().ok_or(DecodeError::InvalidValue)?;
funding_txo_set.insert(funding_txo.clone());
if let Some(ref mut monitor) = args.channel_monitors.get_mut(&funding_txo) {
Expand Down
42 changes: 39 additions & 3 deletions lightning/src/ln/functional_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6982,7 +6982,7 @@ fn test_user_configurable_csv_delay() {
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

// We test config.our_to_self > BREAKDOWN_TIMEOUT is enforced in Channel::new_outbound()
if let Err(error) = Channel::new_outbound(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), 1000000, 1000000, 0, &low_our_to_self_config) {
if let Err(error) = Channel::new_outbound(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), 1000000, 1000000, 0, &low_our_to_self_config, 0) {
match error {
APIError::APIMisuseError { err } => { assert!(regex::Regex::new(r"Configured with an unreasonable our_to_self_delay \(\d+\) putting user funds at risks").unwrap().is_match(err.as_str())); },
_ => panic!("Unexpected event"),
Expand All @@ -6993,7 +6993,7 @@ fn test_user_configurable_csv_delay() {
nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap();
let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id());
open_channel.to_self_delay = 200;
if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &low_our_to_self_config) {
if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &low_our_to_self_config, 0) {
match error {
ChannelError::Close(err) => { assert!(regex::Regex::new(r"Configured with an unreasonable our_to_self_delay \(\d+\) putting user funds at risks").unwrap().is_match(err.as_str())); },
_ => panic!("Unexpected event"),
Expand Down Expand Up @@ -7022,7 +7022,7 @@ fn test_user_configurable_csv_delay() {
nodes[1].node.create_channel(nodes[0].node.get_our_node_id(), 1000000, 1000000, 42, None).unwrap();
let mut open_channel = get_event_msg!(nodes[1], MessageSendEvent::SendOpenChannel, nodes[0].node.get_our_node_id());
open_channel.to_self_delay = 200;
if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &high_their_to_self_config) {
if let Err(error) = Channel::new_from_req(&&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &InitFeatures::known(), &open_channel, 0, &high_their_to_self_config, 0) {
match error {
ChannelError::Close(err) => { assert!(regex::Regex::new(r"They wanted our payments to be delayed by a needlessly long period\. Upper limit: \d+\. Actual: \d+").unwrap().is_match(err.as_str())); },
_ => panic!("Unexpected event"),
Expand Down Expand Up @@ -7908,6 +7908,42 @@ fn test_bump_txn_sanitize_tracking_maps() {
}
}

#[test]
fn test_channel_conf_timeout() {
// Tests that, for inbound channels, we give up on them if the funding transaction does not
// confirm within 2016 blocks, as recommended by BOLT 2.
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 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, 100_000, InitFeatures::known(), InitFeatures::known());

// The outbound node should wait forever for confirmation:
// This matches `channel::FUNDING_CONF_DEADLINE_BLOCKS` and BOLT 2's suggested timeout, thus is
// copied here instead of directly referencing the constant.
connect_blocks(&nodes[0], 2016);
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());

// The inbound node should fail the channel after exactly 2016 blocks
connect_blocks(&nodes[1], 2015);
check_added_monitors!(nodes[1], 0);
assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());

connect_blocks(&nodes[1], 1);
check_added_monitors!(nodes[1], 1);
check_closed_event!(nodes[1], 1, ClosureReason::FundingTimedOut);
let close_ev = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(close_ev.len(), 1);
match close_ev[0] {
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, ref node_id } => {
assert_eq!(*node_id, nodes[0].node.get_our_node_id());
assert_eq!(msg.data, "Channel closed because funding transaction failed to confirm within 2016 blocks");
},
_ => panic!("Unexpected event"),
}
}

#[test]
fn test_override_channel_config() {
let chanmon_cfgs = create_chanmon_cfgs(2);
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub(crate) mod peer_channel_encryptor;
#[cfg(feature = "fuzztarget")]
pub mod channel;
#[cfg(not(feature = "fuzztarget"))]
mod channel;
pub(crate) mod channel;

mod onion_utils;
pub mod wire;
Expand Down
Loading

0 comments on commit 3582921

Please sign in to comment.