Skip to content

Improve type safety for message failures #600

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.

Sign up for GitHub

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
wants to merge 6 commits into from
Closed

Improve type safety for message failures #600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
08bb0b3
New enum to type message failures
D4nte Apr 21, 2020
61d45d4
Use TypedReason for `incorrect_or_unknown_payment_details`
D4nte Apr 21, 2020
e5645a6
Use TypedReason for `permanent_channel_failure`
D4nte Apr 21, 2020
fb82ff5
Use TypedReason for `unknown_next_peer`
D4nte Apr 21, 2020
930a325
Use TypedReason for `temporary_channel_failure`
D4nte Apr 21, 2020
b608426
Set top bytes of `failure_code` in constants
D4nte Apr 21, 2020
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
136 changes: 100 additions & 36 deletions lightning/src/ln/channelmanager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpd
use ln::features::{InitFeatures, NodeFeatures};
use ln::router::{Route, RouteHop};
use ln::msgs;
use ln::onion_utils;
use ln::onion_utils::{self, MessageFailure};
use ln::msgs::{ChannelMessageHandler, DecodeError, LightningError};
use chain::keysinterface::{ChannelKeys, KeysInterface, KeysManager, InMemoryChannelKeys};
use util::config::UserConfig;
Expand Down Expand Up @@ -162,7 +162,8 @@ pub(super) enum HTLCFailReason {
Reason {
failure_code: u16,
data: Vec<u8>,
}
},
TypedReason(onion_utils::MessageFailure)
}

/// payment_hash type, use to cross-lock hop
Expand Down Expand Up @@ -867,7 +868,13 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
}
};
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() });
self.fail_htlc_backwards_internal(
self.channel_state.lock().unwrap(),
htlc_source.0,
&htlc_source.1,
HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure),
);

}
let chan_update = if let Some(chan) = chan_option {
if let Ok(update) = self.get_channel_update(&chan) {
Expand All @@ -890,7 +897,12 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
let (funding_txo_option, monitor_update, mut failed_htlcs) = shutdown_res;
log_trace!(self, "Finishing force-closure of channel {} HTLCs to fail", 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() });
self.fail_htlc_backwards_internal(
self.channel_state.lock().unwrap(),
htlc_source.0,
&htlc_source.1,
HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure)
);
}
if let Some(funding_txo) = funding_txo_option {
// There isn't anything we can do if we get an update failure - we're already
Expand Down Expand Up @@ -1565,9 +1577,14 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
htlc_id: prev_htlc_id,
incoming_packet_shared_secret: forward_info.incoming_shared_secret,
});
failed_forwards.push((htlc_source, forward_info.payment_hash,
HTLCFailReason::Reason { failure_code: 0x4000 | 10, data: Vec::new() }
failed_forwards.push((
htlc_source,
forward_info.payment_hash,
HTLCFailReason::TypedReason(
MessageFailure::UnknownNextPeer,
),
));

},
HTLCForwardInfo::FailHTLC { .. } => {
// Channel went away before we could fail it. This implies
Expand Down Expand Up @@ -1602,9 +1619,15 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
} else {
panic!("Stated return value requirements in send_htlc() were not met");
}
let chan_update = self.get_channel_update(chan.get()).unwrap();
failed_forwards.push((htlc_source, payment_hash,
HTLCFailReason::Reason { failure_code: 0x1000 | 7, data: chan_update.encode_with_len() }
let channel_update = self.get_channel_update(chan.get()).unwrap();
failed_forwards.push((
htlc_source,
payment_hash,
HTLCFailReason::TypedReason(
MessageFailure::TemporaryChannelFailure {
channel_update,
},
),
));
continue;
},
Expand Down Expand Up @@ -1733,19 +1756,14 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
}
if total_value >= msgs::MAX_VALUE_MSAT || total_value > data.total_msat {
for htlc in htlcs.iter() {
let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
htlc_msat_height_data.extend_from_slice(
&byte_utils::be32_to_array(
self.latest_block_height.load(Ordering::Acquire)
as u32,
),
);
let htlc_msat = htlc.value;
let height = self.latest_block_height.load(Ordering::Acquire) as u32;
failed_forwards.push((HTLCSource::PreviousHopData(HTLCPreviousHopData {
short_channel_id: htlc.prev_hop.short_channel_id,
htlc_id: htlc.prev_hop.htlc_id,
incoming_packet_shared_secret: htlc.prev_hop.incoming_packet_shared_secret,
}), payment_hash,
HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data }
HTLCFailReason::TypedReason(MessageFailure::IncorrectOrUnknownPaymentDetails { htlc_msat, height})
));
}
} else if total_value == data.total_msat {
Expand Down Expand Up @@ -1826,13 +1844,11 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
if let Some(mut sources) = removed_source {
for htlc in sources.drain(..) {
if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); }
let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
self.latest_block_height.load(Ordering::Acquire) as u32,
));
let htlc_msat = htlc.value;
let height = self.latest_block_height.load(Ordering::Acquire) as u32;
self.fail_htlc_backwards_internal(channel_state.take().unwrap(),
HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash,
HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data });
HTLCFailReason::TypedReason(MessageFailure::IncorrectOrUnknownPaymentDetails { htlc_msat, height}));
}
true
} else { false }
Expand Down Expand Up @@ -1904,15 +1920,36 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
}
);
}
&HTLCFailReason::TypedReason(ref _message_failure) => {
self.pending_events
.lock()
.unwrap()
.push(events::Event::PaymentFailed {
payment_hash: payment_hash.clone(),
rejected_by_dest: path.len() == 1,
#[cfg(test)]
error_code: Some(_message_failure.code()),
#[cfg(test)]
error_data: Some(_message_failure.data()),
});
}
}
},
}

HTLCSource::PreviousHopData(HTLCPreviousHopData { short_channel_id, htlc_id, incoming_packet_shared_secret }) => {
let err_packet = match onion_error {
HTLCFailReason::Reason { failure_code, data } => {
log_trace!(self, "Failing HTLC with payment_hash {} backwards from us with code {}", log_bytes!(payment_hash.0), failure_code);
let packet = onion_utils::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode();
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &packet)
},
}
HTLCFailReason::TypedReason(ref message_failure) => {
let failure_code = message_failure.code();
log_trace!(self, "Failing HTLC with payment_hash {} backwards from us with code {}", log_bytes!(payment_hash.0), failure_code);
let data = message_failure.data();
let packet = onion_utils::build_failure_packet(&incoming_packet_shared_secret, failure_code, &data[..]).encode();
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &packet)
}
HTLCFailReason::LightningError { err } => {
log_trace!(self, "Failing HTLC with payment_hash {} backwards with pre-built LightningError", log_bytes!(payment_hash.0));
onion_utils::encrypt_failure_packet(&incoming_packet_shared_secret, &err.data)
Expand Down Expand Up @@ -1999,13 +2036,11 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
for htlc in sources.drain(..) {
if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); }
if (is_mpp && !valid_mpp) || (!is_mpp && (htlc.value < expected_amount || htlc.value > expected_amount * 2)) {
let mut htlc_msat_height_data = byte_utils::be64_to_array(htlc.value).to_vec();
htlc_msat_height_data.extend_from_slice(&byte_utils::be32_to_array(
self.latest_block_height.load(Ordering::Acquire) as u32,
));
let htlc_msat = htlc.value;
let height = self.latest_block_height.load(Ordering::Acquire) as u32;
self.fail_htlc_backwards_internal(channel_state.take().unwrap(),
HTLCSource::PreviousHopData(htlc.prev_hop), &payment_hash,
HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_height_data });
HTLCFailReason::TypedReason(MessageFailure::IncorrectOrUnknownPaymentDetails { htlc_msat, height }));
} else {
match self.claim_funds_from_hop(channel_state.as_mut().unwrap(), htlc.prev_hop, payment_preimage) {
Err(Some(e)) => {
Expand Down Expand Up @@ -2428,7 +2463,12 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
}
};
for htlc_source in dropped_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() });
self.fail_htlc_backwards_internal(
self.channel_state.lock().unwrap(),
htlc_source.0,
&htlc_source.1,
HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure)
);
}
if let Some(chan) = chan_option {
if let Ok(update) = self.get_channel_update(&chan) {
Expand Down Expand Up @@ -2929,7 +2969,12 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> events::Me
self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage);
} else {
log_trace!(self, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() });
self.fail_htlc_backwards_internal(
self.channel_state.lock().unwrap(),
htlc_update.source,
&htlc_update.payment_hash,
HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure)
);
}
}
}
Expand Down Expand Up @@ -2959,7 +3004,12 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> events::Ev
self.claim_funds_internal(self.channel_state.lock().unwrap(), htlc_update.source, preimage);
} else {
log_trace!(self, "Failing HTLC with hash {} from our monitor", log_bytes!(htlc_update.payment_hash.0));
self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_update.source, &htlc_update.payment_hash, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() });
self.fail_htlc_backwards_internal(
self.channel_state.lock().unwrap(),
htlc_update.source,
&htlc_update.payment_hash,
HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure)
);
}
}
}
Expand Down Expand Up @@ -3227,7 +3277,7 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
let failed_adds = chan.remove_uncommitted_htlcs_and_mark_paused();
chan.to_disabled_marked();
if !failed_adds.is_empty() {
let chan_update = self.get_channel_update(&chan).map(|u| u.encode_with_len()).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe
let chan_update = self.get_channel_update(&chan).unwrap(); // Cannot add/recv HTLCs before we have a short_id so unwrap is safe
failed_payments.push((chan_update, failed_adds));
}
if chan.is_shutdown() {
Expand Down Expand Up @@ -3270,9 +3320,14 @@ impl<ChanSigner: ChannelKeys, M: Deref + Sync + Send, T: Deref + Sync + Send, K:
for failure in failed_channels.drain(..) {
self.finish_force_close_channel(failure);
}
for (chan_update, mut htlc_sources) in failed_payments {
for (channel_update, mut htlc_sources) in failed_payments {
for (htlc_source, payment_hash) in htlc_sources.drain(..) {
self.fail_htlc_backwards_internal(self.channel_state.lock().unwrap(), htlc_source, &payment_hash, HTLCFailReason::Reason { failure_code: 0x1000 | 7, data: chan_update.clone() });
self.fail_htlc_backwards_internal(
self.channel_state.lock().unwrap(),
htlc_source,
&payment_hash,
HTLCFailReason::TypedReason(MessageFailure::TemporaryChannelFailure { channel_update: channel_update.clone() })
);
}
}
}
Expand Down Expand Up @@ -3487,6 +3542,10 @@ impl Writeable for HTLCFailReason {
failure_code.write(writer)?;
data.write(writer)?;
}
&HTLCFailReason::TypedReason (ref message_failure) => {
1u8.write(writer)?;
message_failure.write(writer)?;
}
}
Ok(())
}
Expand Down Expand Up @@ -3810,7 +3869,12 @@ impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: De
};

for htlc_source in failed_htlcs.drain(..) {
channel_manager.fail_htlc_backwards_internal(channel_manager.channel_state.lock().unwrap(), htlc_source.0, &htlc_source.1, HTLCFailReason::Reason { failure_code: 0x4000 | 8, data: Vec::new() });
channel_manager.fail_htlc_backwards_internal(
channel_manager.channel_state.lock().unwrap(),
htlc_source.0,
&htlc_source.1,
HTLCFailReason::TypedReason(MessageFailure::PermanentChannelFailure)
);
}

//TODO: Broadcast channel update for closed channels, but only after we've made a
Expand Down
4 changes: 1 addition & 3 deletions lightning/src/ln/functional_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5408,12 +5408,10 @@ impl Writeable for BogusOnionHopData {
fn test_onion_failure() {
use ln::msgs::ChannelUpdate;
use ln::channelmanager::CLTV_FAR_FAR_AWAY;
use ln::onion_utils::{NODE, PERM, UPDATE};
use secp256k1;

const BADONION: u16 = 0x8000;
const PERM: u16 = 0x4000;
const NODE: u16 = 0x2000;
const UPDATE: u16 = 0x1000;

let chanmon_cfgs = create_chanmon_cfgs(3);
let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
Expand Down
51 changes: 46 additions & 5 deletions lightning/src/ln/onion_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ln::router::RouteHop;
use util::byte_utils;
use util::chacha20::ChaCha20;
use util::errors::{self, APIError};
use util::ser::{Readable, Writeable, LengthCalculatingWriter};
use util::ser::{Readable, Writeable, Writer, LengthCalculatingWriter};
use util::logger::{Logger, LogHolder};

use bitcoin_hashes::{Hash, HashEngine};
Expand All @@ -19,6 +19,7 @@ use secp256k1;

use std::io::Cursor;
use std::sync::Arc;
use ln::msgs::ChannelUpdate;

pub(super) struct OnionKeys {
#[cfg(test)]
Expand Down Expand Up @@ -314,6 +315,50 @@ pub(super) fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type:
encrypt_failure_packet(shared_secret, &failure_packet.encode()[..])
}

pub const UPDATE: u16 = 0x1000;
pub const NODE: u16 = 0x2000;
pub const PERM: u16 = 0x4000;

#[derive(Clone)]
pub(crate) enum MessageFailure {
TemporaryChannelFailure { channel_update: ChannelUpdate },
IncorrectOrUnknownPaymentDetails { htlc_msat: u64, height: u32 },
PermanentChannelFailure,
UnknownNextPeer,
}

impl MessageFailure {
pub fn code(&self) -> u16 {
use ln::onion_utils::MessageFailure::*;
match *self {
TemporaryChannelFailure { .. } => UPDATE | 7,
IncorrectOrUnknownPaymentDetails { .. } => PERM | 15,
PermanentChannelFailure => PERM | 8,
UnknownNextPeer => PERM | 10,
}
}

pub fn data(&self) -> Vec<u8> {
use ln::onion_utils::MessageFailure::*;
match self {
&TemporaryChannelFailure { ref channel_update } => channel_update.encode_with_len(),
&IncorrectOrUnknownPaymentDetails { htlc_msat, height } => {
let mut data = byte_utils::be64_to_array(htlc_msat).to_vec();
data.extend_from_slice(&byte_utils::be32_to_array(height));
data
},
&PermanentChannelFailure => Vec::new(),
&UnknownNextPeer => Vec::new()
}
}
}

impl Writeable for MessageFailure {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
writer.write_all(&self.data())
}
}

/// Process failure we got back from upstream on a payment we sent (implying htlc_source is an
/// OutboundRoute).
/// Returns update, a boolean indicating that the payment itself failed, and the error code.
Expand Down Expand Up @@ -352,10 +397,6 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing>(secp_ctx: &Secp256k1<

if fixed_time_eq(&Hmac::from_engine(hmac).into_inner(), &err_packet.hmac) {
if let Some(error_code_slice) = err_packet.failuremsg.get(0..2) {
const PERM: u16 = 0x4000;
const NODE: u16 = 0x2000;
const UPDATE: u16 = 0x1000;

let error_code = byte_utils::slice_to_be16(&error_code_slice);
error_code_ret = Some(error_code);
error_packet_ret = Some(err_packet.failuremsg[2..].to_vec());
Expand Down