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 Sep 30, 2021
1 parent 483cde4 commit a150a5e
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 16 deletions.
54 changes: 43 additions & 11 deletions lightning/src/ln/channel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,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.
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 @@ -452,6 +457,9 @@ pub(super) struct Channel<Signer: Sign> {
funding_tx_confirmed_in: Option<BlockHash>,
funding_tx_confirmation_height: u32,
short_channel_id: Option<u64>,
/// If this channel was created in recent software, this will be set to the height at which
/// channel creation was intiated. This is used to close if funding is never broadcasted.
channel_creation_height: Option<u32>,

counterparty_dust_limit_satoshis: u64,
#[cfg(test)]
Expand Down Expand Up @@ -620,7 +628,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_heigt: u32
) -> Result<Channel<Signer>, APIError>
where K::Target: KeysInterface<Signer = Signer>,
F::Target: FeeEstimator,
{
Expand Down Expand Up @@ -707,6 +718,7 @@ impl<Signer: Sign> Channel<Signer> {
funding_tx_confirmed_in: None,
funding_tx_confirmation_height: 0,
short_channel_id: None,
channel_creation_height: Some(current_chain_heigt),

feerate_per_kw: feerate,
counterparty_dust_limit_satoshis: 0,
Expand Down Expand Up @@ -775,7 +787,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_heigt: u32
) -> Result<Channel<Signer>, ChannelError>
where K::Target: KeysInterface<Signer = Signer>,
F::Target: FeeEstimator
{
Expand Down Expand Up @@ -970,6 +985,7 @@ impl<Signer: Sign> Channel<Signer> {
funding_tx_confirmed_in: None,
funding_tx_confirmation_height: 0,
short_channel_id: None,
channel_creation_height: Some(current_chain_heigt),

feerate_per_kw: msg.feerate_per_kw,
channel_value_satoshis: msg.funding_satoshis,
Expand Down Expand Up @@ -4145,6 +4161,18 @@ impl<Signer: Sign> Channel<Signer> {
data: err_reason.clone(),
}, ClosureReason::ProcessingError { err: err_reason }));
}
} else if let Some(creation_height) = self.channel_creation_height {
if !self.is_outbound() && self.funding_tx_confirmed_in.is_none() &&
creation_height + FUNDING_CONF_DEADLINE_BLOCKS <= height {
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(msgs::ErrorMessage {
channel_id: self.channel_id(),
data: format!("Funding transaction failed to confirm in {} blocks", FUNDING_CONF_DEADLINE_BLOCKS),
});
}
}

Ok((None, timed_out_htlcs))
Expand Down Expand Up @@ -5174,6 +5202,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
(5, self.config, required),
(7, self.shutdown_scriptpubkey, option),
(9, self.target_closing_feerate_sats_per_kw, option),
(11, self.channel_creation_height, option),
});

Ok(())
Expand Down Expand Up @@ -5407,13 +5436,15 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>

let mut announcement_sigs = None;
let mut target_closing_feerate_sats_per_kw = None;
let mut channel_creation_height = None;
read_tlv_fields!(reader, {
(0, announcement_sigs, option),
(1, minimum_depth, option),
(3, counterparty_selected_channel_reserve_satoshis, option),
(5, config, option), // Note that if none is provided we will *not* overwrite the existing one.
(7, shutdown_scriptpubkey, option),
(9, target_closing_feerate_sats_per_kw, option),
(11, channel_creation_height, option),
});

let mut secp_ctx = Secp256k1::new();
Expand Down Expand Up @@ -5470,6 +5501,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,

counterparty_dust_limit_satoshis,
holder_dust_limit_satoshis,
Expand Down Expand Up @@ -5616,7 +5648,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 @@ -5638,7 +5670,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 @@ -5663,13 +5695,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 @@ -5731,7 +5763,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 @@ -5780,12 +5812,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 @@ -5842,7 +5874,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 @@ -5906,7 +5938,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
6 changes: 4 additions & 2 deletions lightning/src/ln/channelmanager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1236,7 +1236,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_id, config)?
Channel::new_outbound(&self.fee_estimator, &self.keys_manager, their_network_key, their_features,
channel_value_satoshis, push_msat, user_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 @@ -3317,7 +3318,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
39 changes: 36 additions & 3 deletions lightning/src/ln/functional_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7241,7 +7241,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 @@ -7252,7 +7252,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 @@ -7281,7 +7281,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 @@ -8171,6 +8171,39 @@ 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:
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);
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, "Funding transaction failed to confirm in 2016 blocks");
},
_ => panic!("Unexpected event"),
}
}

#[test]
fn test_override_channel_config() {
let chanmon_cfgs = create_chanmon_cfgs(2);
Expand Down

0 comments on commit a150a5e

Please sign in to comment.