From: Matt Corallo <649246+TheBlueMatt@users.noreply.github.com> Date: Mon, 20 Apr 2020 21:54:35 +0000 (+0000) Subject: Merge pull request #594 from TheBlueMatt/2020-04-cleanups X-Git-Tag: v0.0.12~81 X-Git-Url: http://git.bitcoin.ninja/index.cgi?p=rust-lightning;a=commitdiff_plain;h=5a2ed0324738e170e0b3b4dab3431242c7221a13;hp=c89514c37c8c2632cb32f1fb00764f9d7f2ce7f8 Merge pull request #594 from TheBlueMatt/2020-04-cleanups Trivial Cleanups --- diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 62ac9866..5d52e4a8 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1733,12 +1733,19 @@ impl 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, + ), + ); 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: byte_utils::be64_to_array(htlc.value).to_vec() } + HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data } )); } } else if total_value == data.total_msat { @@ -1819,9 +1826,13 @@ impl 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, + )); self.fail_htlc_backwards_internal(channel_state.take().unwrap(), HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash, - HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: byte_utils::be64_to_array(htlc.value).to_vec() }); + HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data }); } true } else { false } @@ -1845,9 +1856,9 @@ impl 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! @@ -1863,13 +1874,17 @@ impl 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 @@ -1884,6 +1899,8 @@ impl ChannelMan rejected_by_dest: path.len() == 1, #[cfg(test)] error_code: Some(*failure_code), +#[cfg(test)] + error_data: Some(data.clone()), } ); } @@ -1982,12 +1999,13 @@ impl 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_data = byte_utils::be64_to_array(htlc.value).to_vec(); - let mut height_data = byte_utils::be32_to_array(self.latest_block_height.load(Ordering::Acquire) as u32).to_vec(); - htlc_msat_data.append(&mut height_data); + 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, + )); 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_data }); + HTLCFailReason::Reason { failure_code: 0x4000|15, data: htlc_msat_height_data }); } else { match self.claim_funds_from_hop(channel_state.as_mut().unwrap(), htlc.prev_hop, payment_preimage) { Err(Some(e)) => { diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index add73e4b..11cf69b4 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -5326,7 +5326,7 @@ fn run_onion_failure_test_with_fail_intercept(_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"); } diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index ff89782b..dc3b2f90 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -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(secp_ctx: &Secp256k1, logger: &Arc, htlc_source: &HTLCSource, mut packet_decrypted: Vec) -> (Option, bool, Option) { +#[inline] +pub(super) fn process_onion_failure(secp_ctx: &Secp256k1, logger: &Arc, htlc_source: &HTLCSource, mut packet_decrypted: Vec) -> (Option, bool, Option, Option>) { 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(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(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!(); } } diff --git a/lightning/src/util/events.rs b/lightning/src/util/events.rs index 2c00a133..8f346056 100644 --- a/lightning/src/util/events.rs +++ b/lightning/src/util/events.rs @@ -96,6 +96,8 @@ pub enum Event { rejected_by_dest: bool, #[cfg(test)] error_code: Option, +#[cfg(test)] + error_data: Option>, }, /// 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)