Skip to content

Commit 38284a0

Browse files
arik-soarik-so
committed
Remove invalid state options from Hop
Now that each `InboundOnionPayload` variant corresponds to its own struct, we can reference these same types inside `Hop` and thereby avoid nesting that allowed invalid combinations, and instead store supplemental data as each variant calls for.
1 parent d3981e1 commit 38284a0

File tree

4 files changed

+133
-73
lines changed

4 files changed

+133
-73
lines changed

Diff for: ‎lightning/src/ln/blinded_payment_tests.rs

+20-12
Original file line numberDiff line numberDiff line change
@@ -411,15 +411,23 @@ fn do_forward_checks_failure(check: ForwardCheckFail, intro_fails: bool) {
411411
do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false);
412412
let failed_destination = match check {
413413
ForwardCheckFail::InboundOnionCheck => HTLCDestination::InvalidOnion,
414-
ForwardCheckFail::ForwardPayloadEncodedAsReceive => HTLCDestination::FailedPayment { payment_hash },
414+
ForwardCheckFail::ForwardPayloadEncodedAsReceive => HTLCDestination::InvalidOnion,
415415
ForwardCheckFail::OutboundChannelCheck =>
416416
HTLCDestination::NextHopChannel { node_id: Some(nodes[2].node.get_our_node_id()), channel_id: chan_1_2.2 },
417417
};
418418
expect_htlc_handling_failed_destinations!(
419419
nodes[1].node.get_and_clear_pending_events(), &[failed_destination.clone()]
420420
);
421-
expect_payment_failed_conditions(&nodes[0], payment_hash, false,
422-
PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32]));
421+
match check {
422+
ForwardCheckFail::ForwardPayloadEncodedAsReceive => {
423+
expect_payment_failed_conditions(&nodes[0], payment_hash, false,
424+
PaymentFailedConditions::new().expected_htlc_error_data(0x4000 | 22, &[0; 0]));
425+
}
426+
_ => {
427+
expect_payment_failed_conditions(&nodes[0], payment_hash, false,
428+
PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32]));
429+
}
430+
};
423431
return
424432
}
425433

@@ -1560,10 +1568,10 @@ fn route_blinding_spec_test_vector() {
15601568
Ok(res) => res,
15611569
_ => panic!("Unexpected error")
15621570
};
1563-
let (carol_packet_bytes, carol_hmac) = if let onion_utils::Hop::Forward {
1564-
next_hop_data: msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload {
1571+
let (carol_packet_bytes, carol_hmac) = if let onion_utils::Hop::BlindedForward {
1572+
next_hop_data: msgs::InboundOnionBlindedForwardPayload {
15651573
short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override
1566-
}), next_hop_hmac, new_packet_bytes
1574+
}, next_hop_hmac, new_packet_bytes
15671575
} = bob_peeled_onion {
15681576
assert_eq!(short_channel_id, 1729);
15691577
assert!(next_blinding_override.is_none());
@@ -1594,10 +1602,10 @@ fn route_blinding_spec_test_vector() {
15941602
Ok(res) => res,
15951603
_ => panic!("Unexpected error")
15961604
};
1597-
let (dave_packet_bytes, dave_hmac) = if let onion_utils::Hop::Forward {
1598-
next_hop_data: msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload {
1605+
let (dave_packet_bytes, dave_hmac) = if let onion_utils::Hop::BlindedForward {
1606+
next_hop_data: msgs::InboundOnionBlindedForwardPayload {
15991607
short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override
1600-
}), next_hop_hmac, new_packet_bytes
1608+
}, next_hop_hmac, new_packet_bytes
16011609
} = carol_peeled_onion {
16021610
assert_eq!(short_channel_id, 1105);
16031611
assert_eq!(next_blinding_override, Some(pubkey_from_hex("031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f")));
@@ -1628,10 +1636,10 @@ fn route_blinding_spec_test_vector() {
16281636
Ok(res) => res,
16291637
_ => panic!("Unexpected error")
16301638
};
1631-
let (eve_packet_bytes, eve_hmac) = if let onion_utils::Hop::Forward {
1632-
next_hop_data: msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload {
1639+
let (eve_packet_bytes, eve_hmac) = if let onion_utils::Hop::BlindedForward {
1640+
next_hop_data: msgs::InboundOnionBlindedForwardPayload {
16331641
short_channel_id, payment_relay, payment_constraints, features, intro_node_blinding_point, next_blinding_override
1634-
}), next_hop_hmac, new_packet_bytes
1642+
}, next_hop_hmac, new_packet_bytes
16351643
} = dave_peeled_onion {
16361644
assert_eq!(short_channel_id, 561);
16371645
assert!(next_blinding_override.is_none());

Diff for: ‎lightning/src/ln/channelmanager.rs

+42-18
Original file line numberDiff line numberDiff line change
@@ -4430,7 +4430,24 @@ where
44304430
onion_utils::Hop::Receive(next_hop_data) => {
44314431
// OUR PAYMENT!
44324432
let current_height: u32 = self.best_block.read().unwrap().height;
4433-
match create_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash,
4433+
match create_recv_pending_htlc_info(msgs::InboundOnionPayload::Receive(next_hop_data), shared_secret, msg.payment_hash,
4434+
msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat,
4435+
current_height)
4436+
{
4437+
Ok(info) => {
4438+
// Note that we could obviously respond immediately with an update_fulfill_htlc
4439+
// message, however that would leak that we are the recipient of this payment, so
4440+
// instead we stay symmetric with the forwarding case, only responding (after a
4441+
// delay) once they've send us a commitment_signed!
4442+
PendingHTLCStatus::Forward(info)
4443+
},
4444+
Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
4445+
}
4446+
},
4447+
onion_utils::Hop::BlindedReceive(next_hop_data) => {
4448+
// OUR PAYMENT!
4449+
let current_height: u32 = self.best_block.read().unwrap().height;
4450+
match create_recv_pending_htlc_info(msgs::InboundOnionPayload::BlindedReceive(next_hop_data), shared_secret, msg.payment_hash,
44344451
msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat,
44354452
current_height)
44364453
{
@@ -4445,7 +4462,14 @@ where
44454462
}
44464463
},
44474464
onion_utils::Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes } => {
4448-
match create_fwd_pending_htlc_info(msg, next_hop_data, next_hop_hmac,
4465+
match create_fwd_pending_htlc_info(msg, msgs::InboundOnionPayload::Forward(next_hop_data), next_hop_hmac,
4466+
new_packet_bytes, shared_secret, next_packet_pubkey_opt) {
4467+
Ok(info) => PendingHTLCStatus::Forward(info),
4468+
Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
4469+
}
4470+
},
4471+
onion_utils::Hop::BlindedForward { next_hop_data, next_hop_hmac, new_packet_bytes } => {
4472+
match create_fwd_pending_htlc_info(msg, msgs::InboundOnionPayload::BlindedForward(next_hop_data), next_hop_hmac,
44494473
new_packet_bytes, shared_secret, next_packet_pubkey_opt) {
44504474
Ok(info) => PendingHTLCStatus::Forward(info),
44514475
Err(InboundHTLCErr { err_code, err_data, msg }) => return_err!(msg, err_code, &err_data)
@@ -5860,22 +5884,22 @@ where
58605884
failed_payment!(err_msg, err_code, Vec::new(), Some(phantom_shared_secret));
58615885
},
58625886
};
5863-
match next_hop {
5864-
onion_utils::Hop::Receive(hop_data) => {
5865-
let current_height: u32 = self.best_block.read().unwrap().height;
5866-
match create_recv_pending_htlc_info(hop_data,
5867-
incoming_shared_secret, payment_hash, outgoing_amt_msat,
5868-
outgoing_cltv_value, Some(phantom_shared_secret), false, None,
5869-
current_height)
5870-
{
5871-
Ok(info) => phantom_receives.push((
5872-
prev_short_channel_id, prev_counterparty_node_id, prev_funding_outpoint,
5873-
prev_channel_id, prev_user_channel_id, vec![(info, prev_htlc_id)]
5874-
)),
5875-
Err(InboundHTLCErr { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret))
5876-
}
5877-
},
5878-
_ => panic!(),
5887+
let inbound_onion_payload = match next_hop {
5888+
onion_utils::Hop::Receive(hop_data) => msgs::InboundOnionPayload::Receive(hop_data),
5889+
onion_utils::Hop::BlindedReceive(hop_data) => msgs::InboundOnionPayload::BlindedReceive(hop_data),
5890+
_ => panic!()
5891+
};
5892+
let current_height: u32 = self.best_block.read().unwrap().height;
5893+
match create_recv_pending_htlc_info(inbound_onion_payload,
5894+
incoming_shared_secret, payment_hash, outgoing_amt_msat,
5895+
outgoing_cltv_value, Some(phantom_shared_secret), false, None,
5896+
current_height)
5897+
{
5898+
Ok(info) => phantom_receives.push((
5899+
prev_short_channel_id, prev_counterparty_node_id, prev_funding_outpoint,
5900+
prev_channel_id, prev_user_channel_id, vec![(info, prev_htlc_id)]
5901+
)),
5902+
Err(InboundHTLCErr { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret))
58795903
}
58805904
} else {
58815905
fail_forward!(format!("Unknown short channel id {} for forward HTLC", short_chan_id), 0x4000 | 10, Vec::new(), None);

Diff for: ‎lightning/src/ln/onion_payment.rs

+28-30
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::ln::channelmanager::{BlindedFailure, BlindedForward, CLTV_FAR_FAR_AWA
1616
use crate::types::features::BlindedHopFeatures;
1717
use crate::ln::msgs;
1818
use crate::ln::onion_utils;
19-
use crate::ln::onion_utils::{HTLCFailReason, INVALID_ONION_BLINDING};
19+
use crate::ln::onion_utils::{Hop, HTLCFailReason, INVALID_ONION_BLINDING};
2020
use crate::sign::{NodeSigner, Recipient};
2121
use crate::util::logger::Logger;
2222

@@ -296,7 +296,13 @@ where
296296
InboundHTLCErr { msg, err_code, err_data }
297297
})?;
298298
Ok(match hop {
299-
onion_utils::Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes } => {
299+
onion_utils::Hop::Forward { next_hop_hmac, new_packet_bytes, .. } | onion_utils::Hop::BlindedForward { next_hop_hmac, new_packet_bytes, .. } => {
300+
let inbound_onion_payload = match hop {
301+
onion_utils::Hop::Forward { next_hop_data, .. } => msgs::InboundOnionPayload::Forward(next_hop_data),
302+
onion_utils::Hop::BlindedForward { next_hop_data, .. } => msgs::InboundOnionPayload::BlindedForward(next_hop_data),
303+
_ => unreachable!()
304+
};
305+
300306
let NextPacketDetails {
301307
next_packet_pubkey, outgoing_amt_msat: _, outgoing_scid: _, outgoing_cltv_value
302308
} = match next_packet_details_opt {
@@ -310,7 +316,7 @@ where
310316
};
311317

312318
if let Err((err_msg, code)) = check_incoming_htlc_cltv(
313-
cur_height, outgoing_cltv_value, msg.cltv_expiry
319+
cur_height, outgoing_cltv_value, msg.cltv_expiry,
314320
) {
315321
return Err(InboundHTLCErr {
316322
msg: err_msg,
@@ -321,16 +327,21 @@ where
321327

322328
// TODO: If this is potentially a phantom payment we should decode the phantom payment
323329
// onion here and check it.
324-
325330
create_fwd_pending_htlc_info(
326-
msg, next_hop_data, next_hop_hmac, new_packet_bytes, shared_secret,
327-
Some(next_packet_pubkey)
331+
msg, inbound_onion_payload, next_hop_hmac, new_packet_bytes, shared_secret,
332+
Some(next_packet_pubkey),
328333
)?
329334
},
330335
onion_utils::Hop::Receive(received_data) => {
331336
create_recv_pending_htlc_info(
332-
received_data, shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry,
333-
None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height
337+
msgs::InboundOnionPayload::Receive(received_data), shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry,
338+
None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height,
339+
)?
340+
},
341+
onion_utils::Hop::BlindedReceive(received_data) => {
342+
create_recv_pending_htlc_info(
343+
msgs::InboundOnionPayload::BlindedReceive(received_data), shared_secret, msg.payment_hash, msg.amount_msat, msg.cltv_expiry,
344+
None, allow_skimmed_fees, msg.skimmed_fee_msat, cur_height,
334345
)?
335346
}
336347
})
@@ -424,23 +435,15 @@ where
424435
};
425436

426437
let next_packet_details = match next_hop {
427-
onion_utils::Hop::Forward {
428-
next_hop_data: msgs::InboundOnionPayload::Forward(msgs::InboundOnionForwardPayload {
429-
short_channel_id, amt_to_forward, outgoing_cltv_value
430-
}), ..
431-
} => {
438+
Hop::Forward { next_hop_data: msgs::InboundOnionForwardPayload { short_channel_id, amt_to_forward, outgoing_cltv_value }, .. } => {
432439
let next_packet_pubkey = onion_utils::next_hop_pubkey(secp_ctx,
433440
msg.onion_routing_packet.public_key.unwrap(), &shared_secret);
434-
NextPacketDetails {
441+
Some(NextPacketDetails {
435442
next_packet_pubkey, outgoing_scid: short_channel_id,
436443
outgoing_amt_msat: amt_to_forward, outgoing_cltv_value
437-
}
438-
},
439-
onion_utils::Hop::Forward {
440-
next_hop_data: msgs::InboundOnionPayload::BlindedForward (msgs::InboundOnionBlindedForwardPayload{
441-
short_channel_id, ref payment_relay, ref payment_constraints, ref features, ..
442-
}), ..
443-
} => {
444+
})
445+
}
446+
Hop::BlindedForward { next_hop_data: msgs::InboundOnionBlindedForwardPayload { short_channel_id, ref payment_relay, ref payment_constraints, ref features, .. }, .. } => {
444447
let (amt_to_forward, outgoing_cltv_value) = match check_blinded_forward(
445448
msg.amount_msat, msg.cltv_expiry, &payment_relay, &payment_constraints, &features
446449
) {
@@ -452,20 +455,15 @@ where
452455
};
453456
let next_packet_pubkey = onion_utils::next_hop_pubkey(&secp_ctx,
454457
msg.onion_routing_packet.public_key.unwrap(), &shared_secret);
455-
NextPacketDetails {
458+
Some(NextPacketDetails {
456459
next_packet_pubkey, outgoing_scid: short_channel_id, outgoing_amt_msat: amt_to_forward,
457460
outgoing_cltv_value
458-
}
459-
},
460-
onion_utils::Hop::Receive { .. } => return Ok((next_hop, shared_secret, None)),
461-
onion_utils::Hop::Forward { next_hop_data: msgs::InboundOnionPayload::Receive { .. }, .. } |
462-
onion_utils::Hop::Forward { next_hop_data: msgs::InboundOnionPayload::BlindedReceive { .. }, .. } =>
463-
{
464-
return_err!("Final Node OnionHopData provided for us as an intermediary node", 0x4000 | 22, &[0; 0]);
461+
})
465462
}
463+
_ => None
466464
};
467465

468-
Ok((next_hop, shared_secret, Some(next_packet_details)))
466+
Ok((next_hop, shared_secret, next_packet_details))
469467
}
470468

471469
pub(super) fn check_incoming_htlc_cltv(

Diff for: ‎lightning/src/ln/onion_utils.rs

+43-13
Original file line numberDiff line numberDiff line change
@@ -1415,29 +1415,40 @@ impl NextPacketBytes for Vec<u8> {
14151415

14161416
/// Data decrypted from a payment's onion payload.
14171417
pub(crate) enum Hop {
1418-
/// This onion payload was for us, not for forwarding to a next-hop. Contains information for
1419-
/// verifying the incoming payment.
1420-
Receive(msgs::InboundOnionPayload),
14211418
/// This onion payload needs to be forwarded to a next-hop.
14221419
Forward {
14231420
/// Onion payload data used in forwarding the payment.
1424-
next_hop_data: msgs::InboundOnionPayload,
1421+
next_hop_data: msgs::InboundOnionForwardPayload,
14251422
/// HMAC of the next hop's onion packet.
14261423
next_hop_hmac: [u8; 32],
14271424
/// Bytes of the onion packet we're forwarding.
14281425
new_packet_bytes: [u8; ONION_DATA_LEN],
14291426
},
1427+
/// This onion payload needs to be forwarded to a next-hop.
1428+
BlindedForward {
1429+
/// Onion payload data used in forwarding the payment.
1430+
next_hop_data: msgs::InboundOnionBlindedForwardPayload,
1431+
/// HMAC of the next hop's onion packet.
1432+
next_hop_hmac: [u8; 32],
1433+
/// Bytes of the onion packet we're forwarding.
1434+
new_packet_bytes: [u8; ONION_DATA_LEN],
1435+
},
1436+
/// This onion payload was for us, not for forwarding to a next-hop. Contains information for
1437+
/// verifying the incoming payment.
1438+
Receive(msgs::InboundOnionReceivePayload),
1439+
/// This onion payload was for us, not for forwarding to a next-hop. Contains information for
1440+
/// verifying the incoming payment.
1441+
BlindedReceive(msgs::InboundOnionBlindedReceivePayload),
14301442
}
14311443

14321444
impl Hop {
14331445
pub(crate) fn is_intro_node_blinded_forward(&self) -> bool {
14341446
match self {
1435-
Self::Forward {
1447+
Self::BlindedForward {
14361448
next_hop_data:
1437-
msgs::InboundOnionPayload::BlindedForward(msgs::InboundOnionBlindedForwardPayload {
1438-
intro_node_blinding_point: Some(_),
1439-
..
1440-
}),
1449+
msgs::InboundOnionBlindedForwardPayload {
1450+
intro_node_blinding_point: Some(_), ..
1451+
},
14411452
..
14421453
} => true,
14431454
_ => false,
@@ -1461,16 +1472,35 @@ pub(crate) fn decode_next_payment_hop<NS: Deref>(
14611472
where
14621473
NS::Target: NodeSigner,
14631474
{
1464-
match decode_next_hop(
1475+
let decoded_hop: Result<(msgs::InboundOnionPayload, Option<_>), _> = decode_next_hop(
14651476
shared_secret,
14661477
hop_data,
14671478
hmac_bytes,
14681479
Some(payment_hash),
14691480
(blinding_point, node_signer),
1470-
) {
1471-
Ok((next_hop_data, None)) => Ok(Hop::Receive(next_hop_data)),
1481+
);
1482+
match decoded_hop {
14721483
Ok((next_hop_data, Some((next_hop_hmac, FixedSizeOnionPacket(new_packet_bytes))))) => {
1473-
Ok(Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes })
1484+
match next_hop_data {
1485+
msgs::InboundOnionPayload::Forward(next_hop_data) => {
1486+
Ok(Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes })
1487+
},
1488+
msgs::InboundOnionPayload::BlindedForward(next_hop_data) => {
1489+
Ok(Hop::BlindedForward { next_hop_data, next_hop_hmac, new_packet_bytes })
1490+
},
1491+
_ => Err(OnionDecodeErr::Relay {
1492+
err_msg: "Final Node OnionHopData provided for us as an intermediary node",
1493+
err_code: 0x4000 | 22,
1494+
}),
1495+
}
1496+
},
1497+
Ok((next_hop_data, None)) => match next_hop_data {
1498+
msgs::InboundOnionPayload::Receive(payload) => Ok(Hop::Receive(payload)),
1499+
msgs::InboundOnionPayload::BlindedReceive(payload) => Ok(Hop::BlindedReceive(payload)),
1500+
_ => Err(OnionDecodeErr::Relay {
1501+
err_msg: "Intermediate Node OnionHopData provided for us as a final node",
1502+
err_code: 0x4000 | 22,
1503+
}),
14741504
},
14751505
Err(e) => Err(e),
14761506
}

0 commit comments

Comments
 (0)