Skip to content

Commit

Permalink
Test that height is included for incorrect payment details
Browse files Browse the repository at this point in the history
Ensure that the best know blockchain height is included in the
data of `incorrect_or_unknown_payment_details` message failure.
  • Loading branch information
D4nte committed Apr 19, 2020
1 parent fae46a0 commit 236887d
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 8 deletions.
12 changes: 9 additions & 3 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1856,9 +1856,9 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
match &onion_error {
&HTLCFailReason::LightningError { ref err } => {
#[cfg(test)]
let (channel_update, payment_retryable, onion_error_code) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
let (channel_update, payment_retryable, onion_error_code, onion_error_data) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
#[cfg(not(test))]
let (channel_update, payment_retryable, _) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
let (channel_update, payment_retryable, _, _) = onion_utils::process_onion_failure(&self.secp_ctx, &self.logger, &source, err.data.clone());
// TODO: If we decided to blame ourselves (or one of our channels) in
// process_onion_failure we should close that channel as it implies our
// next-hop is needlessly blaming us!
Expand All @@ -1874,13 +1874,17 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
payment_hash: payment_hash.clone(),
rejected_by_dest: !payment_retryable,
#[cfg(test)]
error_code: onion_error_code
error_code: onion_error_code,
#[cfg(test)]
error_data: onion_error_data
}
);
},
&HTLCFailReason::Reason {
#[cfg(test)]
ref failure_code,
#[cfg(test)]
ref data,
.. } => {
// we get a fail_malformed_htlc from the first hop
// TODO: We'd like to generate a PaymentFailureNetworkUpdate for temporary
Expand All @@ -1895,6 +1899,8 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
rejected_by_dest: path.len() == 1,
#[cfg(test)]
error_code: Some(*failure_code),
#[cfg(test)]
error_data: Some(data.clone()),
}
);
}
Expand Down
15 changes: 13 additions & 2 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5326,7 +5326,7 @@ fn run_onion_failure_test_with_fail_intercept<F1,F2,F3>(_name: &str, test_case:

let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code } = &events[0] {
if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code, error_data: _ } = &events[0] {
assert_eq!(*rejected_by_dest, !expected_retryable);
assert_eq!(*error_code, expected_error_code);
} else {
Expand Down Expand Up @@ -6914,9 +6914,20 @@ fn test_check_htlc_underpaying() {

let events = nodes[0].node.get_and_clear_pending_events();
assert_eq!(events.len(), 1);
if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code } = &events[0] {
if let &Event::PaymentFailed { payment_hash:_, ref rejected_by_dest, ref error_code, ref error_data } = &events[0] {
assert_eq!(*rejected_by_dest, true);
assert_eq!(error_code.unwrap(), 0x4000|15);
// 10_000 msat as u64, followed by a height of 99 as u32
assert_eq!(&error_data.as_ref().unwrap()[..], &[
((10_000u64 >> 7*8) & 0xff) as u8,
((10_000u64 >> 6*8) & 0xff) as u8,
((10_000u64 >> 5*8) & 0xff) as u8,
((10_000u64 >> 4*8) & 0xff) as u8,
((10_000u64 >> 3*8) & 0xff) as u8,
((10_000u64 >> 2*8) & 0xff) as u8,
((10_000u64 >> 1*8) & 0xff) as u8,
((10_000u64 >> 0*8) & 0xff) as u8,
0, 0, 0, 99]);
} else {
panic!("Unexpected event");
}
Expand Down
9 changes: 6 additions & 3 deletions lightning/src/ln/onion_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,13 @@ pub(super) fn build_first_hop_failure_packet(shared_secret: &[u8], failure_type:
/// 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.
pub(super) fn process_onion_failure<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, logger: &Arc<Logger>, htlc_source: &HTLCSource, mut packet_decrypted: Vec<u8>) -> (Option<msgs::HTLCFailChannelUpdate>, bool, Option<u16>) {
#[inline]
pub(super) fn process_onion_failure<T: secp256k1::Signing>(secp_ctx: &Secp256k1<T>, logger: &Arc<Logger>, htlc_source: &HTLCSource, mut packet_decrypted: Vec<u8>) -> (Option<msgs::HTLCFailChannelUpdate>, bool, Option<u16>, Option<Vec<u8>>) {
if let &HTLCSource::OutboundRoute { ref path, ref session_priv, ref first_hop_htlc_msat } = htlc_source {
let mut res = None;
let mut htlc_msat = *first_hop_htlc_msat;
let mut error_code_ret = None;
let mut error_packet_ret = None;
let mut next_route_hop_ix = 0;
let mut is_from_final_node = false;

Expand Down Expand Up @@ -356,6 +358,7 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing>(secp_ctx: &Secp256k1<

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());

let (debug_field, debug_field_size) = errors::get_onion_debug_field(error_code);

Expand Down Expand Up @@ -456,11 +459,11 @@ pub(super) fn process_onion_failure<T: secp256k1::Signing>(secp_ctx: &Secp256k1<
}
}).expect("Route that we sent via spontaneously grew invalid keys in the middle of it?");
if let Some((channel_update, payment_retryable)) = res {
(channel_update, payment_retryable, error_code_ret)
(channel_update, payment_retryable, error_code_ret, error_packet_ret)
} else {
// only not set either packet unparseable or hmac does not match with any
// payment not retryable only when garbage is from the final node
(None, !is_from_final_node, None)
(None, !is_from_final_node, None, None)
}
} else { unreachable!(); }
}
Expand Down
8 changes: 8 additions & 0 deletions lightning/src/util/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ pub enum Event {
rejected_by_dest: bool,
#[cfg(test)]
error_code: Option<u16>,
#[cfg(test)]
error_data: Option<Vec<u8>>,
},
/// Used to indicate that ChannelManager::process_pending_htlc_forwards should be called at a
/// time in the future.
Expand Down Expand Up @@ -142,12 +144,16 @@ impl Writeable for Event {
&Event::PaymentFailed { ref payment_hash, ref rejected_by_dest,
#[cfg(test)]
ref error_code,
#[cfg(test)]
ref error_data,
} => {
4u8.write(writer)?;
payment_hash.write(writer)?;
rejected_by_dest.write(writer)?;
#[cfg(test)]
error_code.write(writer)?;
#[cfg(test)]
error_data.write(writer)?;
},
&Event::PendingHTLCsForwardable { time_forwardable: _ } => {
5u8.write(writer)?;
Expand Down Expand Up @@ -186,6 +192,8 @@ impl MaybeReadable for Event {
rejected_by_dest: Readable::read(reader)?,
#[cfg(test)]
error_code: Readable::read(reader)?,
#[cfg(test)]
error_data: Readable::read(reader)?,
})),
5u8 => Ok(Some(Event::PendingHTLCsForwardable {
time_forwardable: Duration::from_secs(0)
Expand Down

0 comments on commit 236887d

Please sign in to comment.