Skip to content

Commit 1613f87

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #3598 from arik-so/arik/trampoline/inbound-prefactors-02
Eliminate invalid Hop/InboundOnionPayload combinations
2 parents a6b8a22 + 38284a0 commit 1613f87

File tree

5 files changed

+205
-135
lines changed

5 files changed

+205
-135
lines changed

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

+17-9
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,8 +1568,8 @@ 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 {
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
15661574
}, next_hop_hmac, new_packet_bytes
15671575
} = bob_peeled_onion {
@@ -1594,8 +1602,8 @@ 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 {
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
16001608
}, next_hop_hmac, new_packet_bytes
16011609
} = carol_peeled_onion {
@@ -1628,8 +1636,8 @@ 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 {
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
16341642
}, next_hop_hmac, new_packet_bytes
16351643
} = dave_peeled_onion {

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

+54-27
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);
@@ -15570,16 +15594,17 @@ mod tests {
1557015594
let node = create_network(1, &node_cfg, &node_chanmgr);
1557115595
let sender_intended_amt_msat = 100;
1557215596
let extra_fee_msat = 10;
15573-
let hop_data = msgs::InboundOnionPayload::Receive {
15597+
let hop_data = msgs::InboundOnionPayload::Receive(msgs::InboundOnionReceivePayload {
1557415598
sender_intended_htlc_amt_msat: 100,
1557515599
cltv_expiry_height: 42,
1557615600
payment_metadata: None,
1557715601
keysend_preimage: None,
1557815602
payment_data: Some(msgs::FinalOnionHopData {
15579-
payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat,
15603+
payment_secret: PaymentSecret([0; 32]),
15604+
total_msat: sender_intended_amt_msat,
1558015605
}),
1558115606
custom_tlvs: Vec::new(),
15582-
};
15607+
});
1558315608
// Check that if the amount we received + the penultimate hop extra fee is less than the sender
1558415609
// intended amount, we fail the payment.
1558515610
let current_height: u32 = node[0].node.best_block.read().unwrap().height;
@@ -15592,16 +15617,17 @@ mod tests {
1559215617
} else { panic!(); }
1559315618

1559415619
// If amt_received + extra_fee is equal to the sender intended amount, we're fine.
15595-
let hop_data = msgs::InboundOnionPayload::Receive { // This is the same payload as above, InboundOnionPayload doesn't implement Clone
15620+
let hop_data = msgs::InboundOnionPayload::Receive(msgs::InboundOnionReceivePayload { // This is the same payload as above, InboundOnionPayload doesn't implement Clone
1559615621
sender_intended_htlc_amt_msat: 100,
1559715622
cltv_expiry_height: 42,
1559815623
payment_metadata: None,
1559915624
keysend_preimage: None,
1560015625
payment_data: Some(msgs::FinalOnionHopData {
15601-
payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat,
15626+
payment_secret: PaymentSecret([0; 32]),
15627+
total_msat: sender_intended_amt_msat,
1560215628
}),
1560315629
custom_tlvs: Vec::new(),
15604-
};
15630+
});
1560515631
let current_height: u32 = node[0].node.best_block.read().unwrap().height;
1560615632
assert!(create_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
1560715633
sender_intended_amt_msat - extra_fee_msat, 42, None, true, Some(extra_fee_msat),
@@ -15616,16 +15642,17 @@ mod tests {
1561615642
let node = create_network(1, &node_cfg, &node_chanmgr);
1561715643

1561815644
let current_height: u32 = node[0].node.best_block.read().unwrap().height;
15619-
let result = create_recv_pending_htlc_info(msgs::InboundOnionPayload::Receive {
15645+
let result = create_recv_pending_htlc_info(msgs::InboundOnionPayload::Receive(msgs::InboundOnionReceivePayload {
1562015646
sender_intended_htlc_amt_msat: 100,
1562115647
cltv_expiry_height: 22,
1562215648
payment_metadata: None,
1562315649
keysend_preimage: None,
1562415650
payment_data: Some(msgs::FinalOnionHopData {
15625-
payment_secret: PaymentSecret([0; 32]), total_msat: 100,
15651+
payment_secret: PaymentSecret([0; 32]),
15652+
total_msat: 100,
1562615653
}),
1562715654
custom_tlvs: Vec::new(),
15628-
}, [0; 32], PaymentHash([0; 32]), 100, 23, None, true, None, current_height);
15655+
}), [0; 32], PaymentHash([0; 32]), 100, 23, None, true, None, current_height);
1562915656

1563015657
// Should not return an error as this condition:
1563115658
// https://github.com/lightning/bolts/blob/4dcc377209509b13cf89a4b91fde7d478f5b46d8/04-onion-routing.md?plain=1#L334

0 commit comments

Comments
 (0)