Skip to content

Commit

Permalink
Check UpdateAddHTLC::skimmed_fee_msat on receive
Browse files Browse the repository at this point in the history
Make sure the penultimate hop took the amount of fee that they claimed to take.
Without checking this TLV, we're heavily relying on the receiving wallet code
to correctly implement logic to calculate that that the fee is as expected.
  • Loading branch information
valentinewallace committed Jun 8, 2023
1 parent b70e34c commit a1cbf67
Showing 1 changed file with 52 additions and 4 deletions.
56 changes: 52 additions & 4 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2410,7 +2410,8 @@ where

fn construct_recv_pending_htlc_info(
&self, hop_data: msgs::OnionHopData, shared_secret: [u8; 32], payment_hash: PaymentHash,
amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>, allow_underpay: bool
amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>, allow_underpay: bool,
counterparty_skimmed_fee_msat: Option<u64>,
) -> Result<PendingHTLCInfo, ReceiveError> {
// final_incorrect_cltv_expiry
if hop_data.outgoing_cltv_value > cltv_expiry {
Expand All @@ -2437,7 +2438,10 @@ where
msg: "The final CLTV expiry is too soon to handle",
});
}
if !allow_underpay && hop_data.amt_to_forward > amt_msat {
if (!allow_underpay && hop_data.amt_to_forward > amt_msat) ||
(allow_underpay && hop_data.amt_to_forward >
amt_msat.saturating_add(counterparty_skimmed_fee_msat.unwrap_or(0)))
{
return Err(ReceiveError {
err_code: 19,
err_data: amt_msat.to_be_bytes().to_vec(),
Expand Down Expand Up @@ -2735,7 +2739,7 @@ where
onion_utils::Hop::Receive(next_hop_data) => {
// OUR PAYMENT!
match self.construct_recv_pending_htlc_info(next_hop_data, shared_secret, msg.payment_hash,
msg.amount_msat, msg.cltv_expiry, None, allow_underpay)
msg.amount_msat, msg.cltv_expiry, None, allow_underpay, msg.skimmed_fee_msat)
{
Ok(info) => {
// Note that we could obviously respond immediately with an update_fulfill_htlc
Expand Down Expand Up @@ -3553,7 +3557,7 @@ where
onion_utils::Hop::Receive(hop_data) => {
match self.construct_recv_pending_htlc_info(hop_data,
incoming_shared_secret, payment_hash, outgoing_amt_msat,
outgoing_cltv_value, Some(phantom_shared_secret), false)
outgoing_cltv_value, Some(phantom_shared_secret), false, None)
{
Ok(info) => phantom_receives.push((prev_short_channel_id, prev_funding_outpoint, prev_user_channel_id, vec![(info, prev_htlc_id)])),
Err(ReceiveError { err_code, err_data, msg }) => failed_payment!(msg, err_code, err_data, Some(phantom_shared_secret))
Expand Down Expand Up @@ -9555,6 +9559,50 @@ mod tests {
get_event_msg!(nodes[1], MessageSendEvent::SendAcceptChannel, last_random_pk);
}

#[test]
fn reject_excessively_underpaying_htlcs() {
let chanmon_cfg = create_chanmon_cfgs(1);
let node_cfg = create_node_cfgs(1, &chanmon_cfg);
let node_chanmgr = create_node_chanmgrs(1, &node_cfg, &[None]);
let node = create_network(1, &node_cfg, &node_chanmgr);
let sender_intended_amt_msat = 100;
let extra_fee_msat = 10;
let hop_data = msgs::OnionHopData {
amt_to_forward: 100,
outgoing_cltv_value: 42,
format: msgs::OnionHopDataFormat::FinalNode {
keysend_preimage: None,
payment_metadata: None,
payment_data: Some(msgs::FinalOnionHopData {
payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat,
}),
}
};
// Check that if the amount we received + the penultimate hop extra fee is less than the sender
// intended amount, we fail the payment.
if let Err(crate::ln::channelmanager::ReceiveError { err_code, .. }) =
node[0].node.construct_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
sender_intended_amt_msat - extra_fee_msat - 1, 42, None, true, Some(extra_fee_msat))
{
assert_eq!(err_code, 19);
} else { panic!(); }

// If amt_received + extra_fee is equal to the sender intended amount, we're fine.
let hop_data = msgs::OnionHopData { // This is the same hop_data as above, OnionHopData doesn't implement Clone
amt_to_forward: 100,
outgoing_cltv_value: 42,
format: msgs::OnionHopDataFormat::FinalNode {
keysend_preimage: None,
payment_metadata: None,
payment_data: Some(msgs::FinalOnionHopData {
payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat,
}),
}
};
assert!(node[0].node.construct_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
sender_intended_amt_msat - extra_fee_msat, 42, None, true, Some(extra_fee_msat)).is_ok());
}

#[cfg(anchors)]
#[test]
fn test_anchors_zero_fee_htlc_tx_fallback() {
Expand Down

0 comments on commit a1cbf67

Please sign in to comment.