Test that `height` is included for incorrect payment details
authorFranck Royer <franck@royer.one>
Sun, 19 Apr 2020 21:30:16 +0000 (07:30 +1000)
committerFranck Royer <franck@royer.one>
Sun, 19 Apr 2020 22:30:47 +0000 (08:30 +1000)
Ensure that the best know blockchain height is included in the
data of `incorrect_or_unknown_payment_details` message failure.

lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/onion_utils.rs
lightning/src/util/events.rs

index c59560944d8bc6bb02be2565dee22a89e22d040b..600702dfcb6413e9ef29383fff1515dd7836bca8 100644 (file)
@@ -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!
@@ -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
@@ -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()),
                                                        }
                                                );
                                        }
index 1be967d939db4ceacf4fc8f0f078b4974bf47e65..21d8686033059386d40fcc3777b1988563dbaa1e 100644 (file)
@@ -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 {
@@ -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");
        }
index ff89782bed3863681ae4d51176db3ad3af56ffae..dc3b2f9022542556e9cc01bba9aa0d757fc85fe8 100644 (file)
@@ -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;
 
@@ -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);
 
@@ -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!(); }
 }
index 2c00a133575d60d9923f1e9afb0501b9cc4fa706..8f34605675c4ee2992a9f045bc8b49d19e9ffd81 100644 (file)
@@ -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.
@@ -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)?;
@@ -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)