Skip to content

option_data_loss_protect-on-channel_reestablishment #244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions fuzz/fuzz_targets/full_stack_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ pub fn do_test(data: &[u8], logger: &Arc<Logger>) {
let their_key = get_pubkey!();
let chan_value = slice_to_be24(get_slice!(3)) as u64;
let push_msat_value = slice_to_be24(get_slice!(3)) as u64;
if channelmanager.create_channel(their_key, chan_value, push_msat_value, 0).is_err() { return; }
if channelmanager.create_channel(their_key, chan_value, push_msat_value, 0, &None).is_err() { return; }
},
6 => {
let mut channels = channelmanager.list_channels();
Expand Down Expand Up @@ -501,7 +501,7 @@ pub fn do_test(data: &[u8], logger: &Arc<Logger>) {
let channel_id = get_slice!(1)[0] as usize;
if channel_id >= channels.len() { return; }
channels.sort_by(|a, b| { a.channel_id.cmp(&b.channel_id) });
channelmanager.force_close_channel(&channels[channel_id].channel_id);
channelmanager.force_close_channel(&channels[channel_id].channel_id, false);
},
_ => return,
}
Expand Down
62 changes: 53 additions & 9 deletions src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,8 @@ pub(super) struct Channel {

channel_monitor: ChannelMonitor,

local_feature_flags: Option<msgs::LocalFeatures>,

logger: Arc<Logger>,
}

Expand Down Expand Up @@ -369,6 +371,7 @@ pub const OFFERED_HTLC_SCRIPT_WEIGHT: usize = 133;
pub(super) enum ChannelError {
Ignore(&'static str),
Close(&'static str),
CloseNoPublish(&'static str),
}

macro_rules! secp_check {
Expand All @@ -381,6 +384,15 @@ macro_rules! secp_check {
}

impl Channel {
// get functions
pub fn get_channel_monitor(&self) -> ChannelMonitor{
self.channel_monitor.clone()
}

pub fn get_channel_id(&self) -> [u8; 32] {
self.channel_id
}

// Convert constants + channel value to limits:
fn get_our_max_htlc_value_in_flight_msat(channel_value_satoshis: u64) -> u64 {
channel_value_satoshis * 1000 / 10 //TODO
Expand Down Expand Up @@ -410,7 +422,7 @@ impl Channel {
}

// Constructors:
pub fn new_outbound(fee_estimator: &FeeEstimator, keys_provider: &Arc<KeysInterface>, their_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, logger: Arc<Logger>, config: &UserConfig) -> Result<Channel, APIError> {
pub fn new_outbound(fee_estimator: &FeeEstimator, keys_provider: &Arc<KeysInterface>, their_node_id: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, logger: Arc<Logger>, config: &UserConfig, local_feature_flags: &Option<msgs::LocalFeatures>) -> Result<Channel, APIError> {
let chan_keys = keys_provider.get_channel_keys(false);

if channel_value_satoshis >= MAX_FUNDING_SATOSHIS {
Expand Down Expand Up @@ -504,7 +516,7 @@ impl Channel {
their_shutdown_scriptpubkey: None,

channel_monitor: channel_monitor,

local_feature_flags: local_feature_flags.clone(),
logger,
})
}
Expand All @@ -521,7 +533,7 @@ impl Channel {

/// 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(fee_estimator: &FeeEstimator, keys_provider: &Arc<KeysInterface>, their_node_id: PublicKey, msg: &msgs::OpenChannel, user_id: u64, logger: Arc<Logger>, config: &UserConfig) -> Result<Channel, ChannelError> {
pub fn new_from_req(fee_estimator: &FeeEstimator, keys_provider: &Arc<KeysInterface>, their_node_id: PublicKey, msg: &msgs::OpenChannel, user_id: u64, logger: Arc<Logger>, config: &UserConfig, local_feature_flags: &Option<msgs::LocalFeatures>) -> Result<Channel, ChannelError> {
let chan_keys = keys_provider.get_channel_keys(true);
let mut local_config = (*config).channel_options.clone();

Expand Down Expand Up @@ -694,6 +706,7 @@ impl Channel {
their_shutdown_scriptpubkey: None,

channel_monitor: channel_monitor,
local_feature_flags: local_feature_flags.clone(),

logger,
};
Expand Down Expand Up @@ -2326,6 +2339,21 @@ impl Channel {
return Err(ChannelError::Close("Peer sent a garbage channel_reestablish"));
}

//Check for dataloss fields and fail channel
if msg.next_remote_commitment_number > 0 {
if let Some(ref data_loss) = msg.data_loss_protect {
//check if provided signature is a valid signature from us
if chan_utils::build_commitment_secret(self.local_keys.commitment_seed, INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1) != data_loss.your_last_per_commitment_secret{
return Err(ChannelError::Close("Peer sent a garbage channel_reestablish with secret key not matching the commitment height provided"));
}
//check if we have fallen beind
//We should not broadcast commitment transaction or continue
if msg.next_remote_commitment_number >= INITIAL_COMMITMENT_NUMBER- self.cur_local_commitment_transaction_number{
return Err(ChannelError::CloseNoPublish("We have fallen behind and we cannot catch up, need to close channel but not publish commitment"));
}
}
}

// Go ahead and unmark PeerDisconnected as various calls we may make check for it (and all
// remaining cases either succeed or ErrorMessage-fail).
self.channel_state &= !(ChannelState::PeerDisconnected as u32);
Expand Down Expand Up @@ -2407,6 +2435,7 @@ impl Channel {
// now!
match self.free_holding_cell_htlcs() {
Err(ChannelError::Close(msg)) => return Err(ChannelError::Close(msg)),
Err(ChannelError::CloseNoPublish(msg)) => return Err(ChannelError::CloseNoPublish(msg)),
Err(ChannelError::Ignore(_)) => panic!("Got non-channel-failing result from free_holding_cell_htlcs"),
Ok(Some((commitment_update, channel_monitor))) => return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(channel_monitor), order, shutdown_msg)),
Ok(None) => return Ok((resend_funding_locked, required_revoke, None, None, order, shutdown_msg)),
Expand Down Expand Up @@ -3086,6 +3115,19 @@ impl Channel {
pub fn get_channel_reestablish(&self) -> msgs::ChannelReestablish {
assert_eq!(self.channel_state & ChannelState::PeerDisconnected as u32, ChannelState::PeerDisconnected as u32);
assert_ne!(self.cur_remote_commitment_transaction_number, INITIAL_COMMITMENT_NUMBER);
let data_loss_protect = if let Some(ref local_features) = self.local_feature_flags{
if local_features.supports_data_loss_protect() && self.their_cur_commitment_point.is_some() && self.channel_monitor.get_secret(self.channel_monitor.get_min_seen_secret()).is_some(){
let data_loss = msgs::DataLossProtect{
your_last_per_commitment_secret: self.channel_monitor.get_secret(self.channel_monitor.get_min_seen_secret()).unwrap(),
my_current_per_commitment_point: self.their_cur_commitment_point.unwrap(),
};
Some(data_loss)
} else {
None
}
} else{
None
};
msgs::ChannelReestablish {
channel_id: self.channel_id(),
// The protocol has two different commitment number concepts - the "commitment
Expand All @@ -3106,7 +3148,7 @@ impl Channel {
// dropped this channel on disconnect as it hasn't yet reached FundingSent so we can't
// overflow here.
next_remote_commitment_number: INITIAL_COMMITMENT_NUMBER - self.cur_remote_commitment_transaction_number - 1,
data_loss_protect: None,
data_loss_protect: data_loss_protect,
}
}

Expand Down Expand Up @@ -3381,7 +3423,8 @@ impl Channel {
/// those explicitly stated to be allowed after shutdown completes, eg some simple getters).
/// Also returns the list of payment_hashes for channels which we can safely fail backwards
/// immediately (others we will have to allow to time out).
pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>) {
/// the returning bool, is to indicate that it must write to channel
pub fn force_shutdown(&mut self) -> (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>, bool) {
assert!(self.channel_state != ChannelState::ShutdownComplete as u32);

// We go ahead and "free" any holding cell HTLCs or HTLCs we haven't yet committed to and
Expand All @@ -3406,7 +3449,7 @@ impl Channel {
self.channel_update_count += 1;
let mut res = Vec::new();
mem::swap(&mut res, &mut self.last_local_commitment_txn);
(res, dropped_outbound_htlcs)
(res, dropped_outbound_htlcs, true)
}
}

Expand Down Expand Up @@ -3642,7 +3685,7 @@ impl Writeable for Channel {
self.their_node_id.write(writer)?;

write_option!(self.their_shutdown_scriptpubkey);

write_option!(self.local_feature_flags);
self.channel_monitor.write_for_disk(writer)?;
Ok(())
}
Expand Down Expand Up @@ -3816,6 +3859,7 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
let their_node_id = Readable::read(reader)?;

let their_shutdown_scriptpubkey = read_option!();
let local_feature_flags = read_option!();
let (monitor_last_block, channel_monitor) = ReadableArgs::read(reader, logger.clone())?;
// We drop the ChannelMonitor's last block connected hash cause we don't actually bother
// doing full block connection operations on the internal CHannelMonitor copies
Expand Down Expand Up @@ -3895,7 +3939,7 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
their_shutdown_scriptpubkey,

channel_monitor,

local_feature_flags,
logger,
})
}
Expand Down Expand Up @@ -3987,7 +4031,7 @@ mod tests {
let their_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&secp_ctx, &[42; 32]).unwrap());
let mut config = UserConfig::new();
config.channel_options.announced_channel = false;
let mut chan = Channel::new_outbound(&feeest, &keys_provider, their_node_id, 10000000, 100000, 42, Arc::clone(&logger), &config).unwrap(); // Nothing uses their network key in this test
let mut chan = Channel::new_outbound(&feeest, &keys_provider, their_node_id, 10000000, 100000, 42, Arc::clone(&logger), &config, &None).unwrap(); // Nothing uses their network key in this test
chan.their_to_self_delay = 144;
chan.our_dust_limit_satoshis = 546;

Expand Down
74 changes: 56 additions & 18 deletions src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ pub struct PaymentHash(pub [u8;32]);
#[derive(Hash, Copy, Clone, PartialEq, Eq, Debug)]
pub struct PaymentPreimage(pub [u8;32]);

type ShutdownResult = (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>);
type ShutdownResult = (Vec<Transaction>, Vec<(HTLCSource, PaymentHash)>, bool);

/// Error type returned across the channel_state mutex boundary. When an Err is generated for a
/// Channel, we generally end up with a ChannelError::Close for which we have to close the channel
Expand Down Expand Up @@ -196,6 +196,15 @@ impl MsgHandleErrInternal {
},
}),
},
ChannelError::CloseNoPublish(msg) => HandleError {
err: msg,
action: Some(msgs::ErrorAction::SendErrorMessage {
msg: msgs::ErrorMessage {
channel_id,
data: msg.to_string()
},
}),
},
},
shutdown_finish: None,
}
Expand Down Expand Up @@ -415,6 +424,14 @@ macro_rules! break_chan_entry {
}
break Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, chan.force_shutdown(), $self.get_channel_update(&chan).ok()))
},
Err(ChannelError::CloseNoPublish(msg)) => {
log_trace!($self, "Closing channel {} due to Close-No_publish_required error: {}", log_bytes!($entry.key()[..]), msg);
let (channel_id, mut chan) = $entry.remove_entry();
if let Some(short_id) = chan.get_short_channel_id() {
$channel_state.short_to_id.remove(&short_id);
}
break Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, chan.force_shutdown(), $self.get_channel_update(&chan).ok()))
},
}
}
}
Expand All @@ -434,6 +451,13 @@ macro_rules! try_chan_entry {
}
return Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, chan.force_shutdown(), $self.get_channel_update(&chan).ok()))
},
Err(ChannelError::CloseNoPublish(msg)) => {
let (channel_id, mut chan) = $entry.remove_entry();
if let Some(short_id) = chan.get_short_channel_id() {
$channel_state.short_to_id.remove(&short_id);
}
return Err(MsgHandleErrInternal::from_finish_shutdown(msg, channel_id, chan.force_shutdown(), $self.get_channel_update(&chan).ok()))
},
}
}
}
Expand Down Expand Up @@ -529,7 +553,6 @@ impl ChannelManager {

pending_events: Mutex::new(Vec::new()),
total_consistency_lock: RwLock::new(()),

keys_manager,

logger,
Expand All @@ -551,12 +574,12 @@ impl ChannelManager {
///
/// Raises APIError::APIMisuseError when channel_value_satoshis > 2**24 or push_msat is
/// greater than channel_value_satoshis * 1k or channel_value_satoshis is < 1000.
pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64) -> Result<(), APIError> {
pub fn create_channel(&self, their_network_key: PublicKey, channel_value_satoshis: u64, push_msat: u64, user_id: u64, local_feature_flags : &Option<msgs::LocalFeatures>) -> Result<(), APIError> {
if channel_value_satoshis < 1000 {
return Err(APIError::APIMisuseError { err: "channel_value must be at least 1000 satoshis" });
}

let channel = Channel::new_outbound(&*self.fee_estimator, &self.keys_manager, their_network_key, channel_value_satoshis, push_msat, user_id, Arc::clone(&self.logger), &self.default_configuration)?;
let channel = Channel::new_outbound(&*self.fee_estimator, &self.keys_manager, their_network_key, channel_value_satoshis, push_msat, user_id, Arc::clone(&self.logger), &self.default_configuration, local_feature_flags)?;
let res = channel.get_open_channel(self.genesis_hash.clone(), &*self.fee_estimator);

let _ = self.total_consistency_lock.read().unwrap();
Expand Down Expand Up @@ -666,19 +689,23 @@ impl ChannelManager {

#[inline]
fn finish_force_close_channel(&self, shutdown_res: ShutdownResult) {
let (local_txn, mut failed_htlcs) = shutdown_res;
let (local_txn, mut failed_htlcs, claim_from_chain) = shutdown_res;
log_trace!(self, "Finishing force-closure of channel with {} transactions to broadcast and {} HTLCs to fail", local_txn.len(), failed_htlcs.len());
for htlc_source in failed_htlcs.drain(..) {
self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() });
}
for tx in local_txn {
self.tx_broadcaster.broadcast_transaction(&tx);
}
if claim_from_chain {
for tx in local_txn {
self.tx_broadcaster.broadcast_transaction(&tx);
}
};
}

/// Force closes a channel, immediately broadcasting the latest local commitment transaction to
/// the chain and rejecting new HTLCs on the given channel.
pub fn force_close_channel(&self, channel_id: &[u8; 32]) {
/// delay_tx_broadcast parameter is there to stop the immediate broadcast of the channel TX. This is delayed by one week.
/// Look in bolt spec under data loss protect.
pub fn force_close_channel(&self, channel_id: &[u8; 32], delay_tx_broadcast : bool) {
let _ = self.total_consistency_lock.read().unwrap();

let mut chan = {
Expand All @@ -694,7 +721,18 @@ impl ChannelManager {
}
};
log_trace!(self, "Force-closing channel {}", log_bytes!(channel_id[..]));
self.finish_force_close_channel(chan.force_shutdown());
let mut shutdown_result = chan.force_shutdown();
if delay_tx_broadcast{
shutdown_result.2 = false;
self.finish_force_close_channel(shutdown_result);
let mut chan_monitor = chan.get_channel_monitor();
let broadcast_height = (self.latest_block_height.load(Ordering::Relaxed))*6*24*7; //6 blocks per hour, 24 hours in a day, 7 days in a week = 1 week later
chan_monitor.broadcast_transaction_at_height(broadcast_height as u32);
self.monitor.add_update_monitor(chan_monitor.get_funding_txo().unwrap(), chan_monitor);
} else
{
self.finish_force_close_channel(shutdown_result);
}
if let Ok(update) = self.get_channel_update(&chan) {
let mut channel_state = self.channel_state.lock().unwrap();
channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelUpdate {
Expand All @@ -707,7 +745,7 @@ impl ChannelManager {
/// for each to the chain and rejecting new HTLCs on each.
pub fn force_close_all_channels(&self) {
for chan in self.list_channels() {
self.force_close_channel(&chan.channel_id);
self.force_close_channel(&chan.channel_id, false);
}
}

Expand Down Expand Up @@ -1609,12 +1647,12 @@ impl ChannelManager {
}
}

fn internal_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> {
fn internal_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel, local_feature_flags : &Option<msgs::LocalFeatures>) -> Result<(), MsgHandleErrInternal> {
if msg.chain_hash != self.genesis_hash {
return Err(MsgHandleErrInternal::send_err_msg_no_close("Unknown genesis block hash", msg.temporary_channel_id.clone()));
}

let channel = Channel::new_from_req(&*self.fee_estimator, &self.keys_manager, their_node_id.clone(), msg, 0, Arc::clone(&self.logger), &self.default_configuration)
let channel = Channel::new_from_req(&*self.fee_estimator, &self.keys_manager, their_node_id.clone(), msg, 0, Arc::clone(&self.logger), &self.default_configuration, local_feature_flags)
.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 = channel_state_lock.borrow_parts();
Expand Down Expand Up @@ -2427,9 +2465,9 @@ impl ChainListener for ChannelManager {

impl ChannelMessageHandler for ChannelManager {
//TODO: Handle errors and close channel (or so)
fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), HandleError> {
fn handle_open_channel(&self, their_node_id: &PublicKey, msg: &msgs::OpenChannel, local_feature_flags: &Option<msgs::LocalFeatures>) -> Result<(), HandleError> {
let _ = self.total_consistency_lock.read().unwrap();
handle_error!(self, self.internal_open_channel(their_node_id, msg), their_node_id)
handle_error!(self, self.internal_open_channel(their_node_id, msg, local_feature_flags), their_node_id)
}

fn handle_accept_channel(&self, their_node_id: &PublicKey, msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
Expand Down Expand Up @@ -2598,11 +2636,11 @@ impl ChannelMessageHandler for ChannelManager {
if msg.channel_id == [0; 32] {
for chan in self.list_channels() {
if chan.remote_network_id == *their_node_id {
self.force_close_channel(&chan.channel_id);
self.force_close_channel(&chan.channel_id, false);
}
}
} else {
self.force_close_channel(&msg.channel_id);
self.force_close_channel(&msg.channel_id, false);
}
}
}
Expand Down Expand Up @@ -2953,7 +2991,7 @@ impl<'a, R : ::std::io::Read> ReadableArgs<R, ChannelManagerReadArgs<'a>> for (S

for (ref funding_txo, ref monitor) in args.channel_monitors.iter() {
if !funding_txo_set.contains(funding_txo) {
closed_channels.push((monitor.get_latest_local_commitment_txn(), Vec::new()));
closed_channels.push((monitor.get_latest_local_commitment_txn(), Vec::new(), true));
}
}

Expand Down
Loading