Merge pull request #596 from D4nte/586-update-htlc-error-code-content

Include `height` to `incorrect_or_unknown_payment_details` failure
This commit is contained in:
Matt Corallo 2020-04-20 05:14:18 +00:00 committed by GitHub
commit 900d900035
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 54 additions and 14 deletions

View file

@ -1733,12 +1733,19 @@ 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,
),
);
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<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,
));
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<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!
@ -1863,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
@ -1884,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()),
}
);
}
@ -1982,12 +1999,13 @@ 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_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)) => {

View 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");
}

View 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!(); }
}

View 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)