Merge pull request #2319 from valentinewallace/2023-05-forward-less-than-onion
authorElias Rohrer <dev@tnull.de>
Wed, 21 Jun 2023 07:25:07 +0000 (09:25 +0200)
committerGitHub <noreply@github.com>
Wed, 21 Jun 2023 07:25:07 +0000 (09:25 +0200)
Allow forwarding less than the amount in the onion

1  2 
lightning/src/events/mod.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_tests.rs

index fa4c86cd0ddea84b6970536455737517756edf52,3fe17475fff490357283b1198e694ff104973f0d..f4c4af7e90ff02bf5cf59c22994a7728ded4fcf5
@@@ -33,6 -33,8 +33,6 @@@ use crate::util::string::UntrustedStrin
  use crate::routing::router::{BlindedTail, Path, RouteHop, RouteParameters};
  
  use bitcoin::{PackedLockTime, Transaction, OutPoint};
 -#[cfg(anchors)]
 -use bitcoin::{Txid, TxIn, TxOut, Witness};
  use bitcoin::blockdata::script::Script;
  use bitcoin::hashes::Hash;
  use bitcoin::hashes::sha256::Hash as Sha256;
@@@ -385,8 -387,25 +385,25 @@@ pub enum Event 
                ///
                /// Payments received on LDK versions prior to 0.0.115 will have this field unset.
                onion_fields: Option<RecipientOnionFields>,
-               /// The value, in thousandths of a satoshi, that this payment is for.
+               /// The value, in thousandths of a satoshi, that this payment is claimable for. May be greater
+               /// than the invoice amount.
+               ///
+               /// May be less than the invoice amount if [`ChannelConfig::accept_underpaying_htlcs`] is set
+               /// and the previous hop took an extra fee.
+               ///
+               /// # Note
+               /// If [`ChannelConfig::accept_underpaying_htlcs`] is set and you claim without verifying this
+               /// field, you may lose money!
+               ///
+               /// [`ChannelConfig::accept_underpaying_htlcs`]: crate::util::config::ChannelConfig::accept_underpaying_htlcs
                amount_msat: u64,
+               /// The value, in thousands of a satoshi, that was skimmed off of this payment as an extra fee
+               /// taken by our channel counterparty.
+               ///
+               /// Will always be 0 unless [`ChannelConfig::accept_underpaying_htlcs`] is set.
+               ///
+               /// [`ChannelConfig::accept_underpaying_htlcs`]: crate::util::config::ChannelConfig::accept_underpaying_htlcs
+               counterparty_skimmed_fee_msat: u64,
                /// Information for claiming this received payment, based on whether the purpose of the
                /// payment is to pay an invoice or to send a spontaneous payment.
                purpose: PaymentPurpose,
                /// The payment hash of the claimed payment. Note that LDK will not stop you from
                /// registering duplicate payment hashes for inbound payments.
                payment_hash: PaymentHash,
-               /// The value, in thousandths of a satoshi, that this payment is for.
+               /// The value, in thousandths of a satoshi, that this payment is for. May be greater than the
+               /// invoice amount.
                amount_msat: u64,
                /// The purpose of the claimed payment, i.e. whether the payment was for an invoice or a
                /// spontaneous payment.
                inbound_amount_msat: u64,
                /// How many msats the payer intended to route to the next node. Depending on the reason you are
                /// intercepting this payment, you might take a fee by forwarding less than this amount.
+               /// Forwarding less than this amount may break compatibility with LDK versions prior to 0.0.116.
                ///
                /// Note that LDK will NOT check that expected fees were factored into this value. You MUST
                /// check that whatever fee you want has been included here or subtract it as required. Further,
@@@ -830,8 -851,8 +849,8 @@@ impl Writeable for Event 
                                // We never write out FundingGenerationReady events as, upon disconnection, peers
                                // drop any channels which have not yet exchanged funding_signed.
                        },
-                       &Event::PaymentClaimable { ref payment_hash, ref amount_msat, ref purpose,
-                               ref receiver_node_id, ref via_channel_id, ref via_user_channel_id,
+                       &Event::PaymentClaimable { ref payment_hash, ref amount_msat, counterparty_skimmed_fee_msat,
+                               ref purpose, ref receiver_node_id, ref via_channel_id, ref via_user_channel_id,
                                ref claim_deadline, ref onion_fields
                        } => {
                                1u8.write(writer)?;
                                                payment_preimage = Some(*preimage);
                                        }
                                }
+                               let skimmed_fee_opt = if counterparty_skimmed_fee_msat == 0 { None }
+                                       else { Some(counterparty_skimmed_fee_msat) };
                                write_tlv_fields!(writer, {
                                        (0, payment_hash, required),
                                        (1, receiver_node_id, option),
                                        (3, via_channel_id, option),
                                        (4, amount_msat, required),
                                        (5, via_user_channel_id, option),
 -                                      (6, 0u64, required), // user_payment_id required for compatibility with 0.0.103 and earlier
 +                                      // Type 6 was `user_payment_id` on 0.0.103 and earlier
                                        (7, claim_deadline, option),
                                        (8, payment_preimage, option),
                                        (9, onion_fields, option),
+                                       (10, skimmed_fee_opt, option),
                                });
                        },
                        &Event::PaymentSent { ref payment_id, ref payment_preimage, ref payment_hash, ref fee_paid_msat } => {
@@@ -1056,8 -1080,9 +1078,9 @@@ impl MaybeReadable for Event 
                                        let mut payment_preimage = None;
                                        let mut payment_secret = None;
                                        let mut amount_msat = 0;
+                                       let mut counterparty_skimmed_fee_msat_opt = None;
                                        let mut receiver_node_id = None;
 -                                      let mut _user_payment_id = None::<u64>; // For compatibility with 0.0.103 and earlier
 +                                      let mut _user_payment_id = None::<u64>; // Used in 0.0.103 and earlier, no longer written in 0.0.116+.
                                        let mut via_channel_id = None;
                                        let mut claim_deadline = None;
                                        let mut via_user_channel_id = None;
                                                (7, claim_deadline, option),
                                                (8, payment_preimage, option),
                                                (9, onion_fields, option),
+                                               (10, counterparty_skimmed_fee_msat_opt, option),
                                        });
                                        let purpose = match payment_secret {
                                                Some(secret) => PaymentPurpose::InvoicePayment {
                                                receiver_node_id,
                                                payment_hash,
                                                amount_msat,
+                                               counterparty_skimmed_fee_msat: counterparty_skimmed_fee_msat_opt.unwrap_or(0),
                                                purpose,
                                                via_channel_id,
                                                via_user_channel_id,
index 595fad60ff2a321d773455c4a9cb54b8c4471bf8,aef1abe307fd741bf6dd76b8030ae7e001a8284c..fcc672a867c04419ae33c1a4bacc01d22f49dbfe
@@@ -50,7 -50,7 +50,7 @@@ use crate::routing::scoring::{Probabili
  use crate::ln::msgs;
  use crate::ln::onion_utils;
  use crate::ln::onion_utils::HTLCFailReason;
 -use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError, MAX_VALUE_MSAT};
 +use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError};
  #[cfg(test)]
  use crate::ln::outbound_payment;
  use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment};
@@@ -131,6 -131,9 +131,9 @@@ pub(super) struct PendingHTLCInfo 
        /// may overshoot this in either case)
        pub(super) outgoing_amt_msat: u64,
        pub(super) outgoing_cltv_value: u32,
+       /// The fee being skimmed off the top of this HTLC. If this is a forward, it'll be the fee we are
+       /// skimming. If we're receiving this HTLC, it's the fee that our counterparty skimmed.
+       pub(super) skimmed_fee_msat: Option<u64>,
  }
  
  #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
@@@ -210,6 -213,8 +213,8 @@@ struct ClaimableHTLC 
        total_value_received: Option<u64>,
        /// The sender intended sum total of all MPP parts specified in the onion
        total_msat: u64,
+       /// The extra fee our counterparty skimmed off the top of this HTLC.
+       counterparty_skimmed_fee_msat: Option<u64>,
  }
  
  /// A payment identifier used to uniquely identify a payment to LDK.
@@@ -2521,9 -2526,11 +2526,11 @@@ wher
                }
        }
  
-       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]>) -> Result<PendingHTLCInfo, ReceiveError>
-       {
+       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,
+               counterparty_skimmed_fee_msat: Option<u64>,
+       ) -> Result<PendingHTLCInfo, ReceiveError> {
                // final_incorrect_cltv_expiry
                if hop_data.outgoing_cltv_value > cltv_expiry {
                        return Err(ReceiveError {
                                msg: "The final CLTV expiry is too soon to handle",
                        });
                }
-               if 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(),
                        incoming_amt_msat: Some(amt_msat),
                        outgoing_amt_msat: hop_data.amt_to_forward,
                        outgoing_cltv_value: hop_data.outgoing_cltv_value,
+                       skimmed_fee_msat: counterparty_skimmed_fee_msat,
                })
        }
  
-       fn decode_update_add_htlc_onion(&self, msg: &msgs::UpdateAddHTLC) -> PendingHTLCStatus {
+       fn decode_update_add_htlc_onion(
+               &self, msg: &msgs::UpdateAddHTLC
+       ) -> Result<(onion_utils::Hop, [u8; 32], Option<Result<PublicKey, secp256k1::Error>>), HTLCFailureMsg> {
                macro_rules! return_malformed_err {
                        ($msg: expr, $err_code: expr) => {
                                {
                                        log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg);
-                                       return PendingHTLCStatus::Fail(HTLCFailureMsg::Malformed(msgs::UpdateFailMalformedHTLC {
+                                       return Err(HTLCFailureMsg::Malformed(msgs::UpdateFailMalformedHTLC {
                                                channel_id: msg.channel_id,
                                                htlc_id: msg.htlc_id,
                                                sha256_of_onion: Sha256::hash(&msg.onion_routing_packet.hop_data).into_inner(),
                        ($msg: expr, $err_code: expr, $data: expr) => {
                                {
                                        log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg);
-                                       return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
+                                       return Err(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
                                                channel_id: msg.channel_id,
                                                htlc_id: msg.htlc_id,
                                                reason: HTLCFailReason::reason($err_code, $data.to_vec())
                                return_err!(err_msg, err_code, &[0; 0]);
                        },
                };
+               let (outgoing_scid, outgoing_amt_msat, outgoing_cltv_value, next_packet_pk_opt) = match next_hop {
+                       onion_utils::Hop::Forward {
+                               next_hop_data: msgs::OnionHopData {
+                                       format: msgs::OnionHopDataFormat::NonFinalNode { short_channel_id }, amt_to_forward,
+                                       outgoing_cltv_value,
+                               }, ..
+                       } => {
+                               let next_pk = onion_utils::next_hop_packet_pubkey(&self.secp_ctx,
+                                       msg.onion_routing_packet.public_key.unwrap(), &shared_secret);
+                               (short_channel_id, amt_to_forward, outgoing_cltv_value, Some(next_pk))
+                       },
+                       // We'll do receive checks in [`Self::construct_pending_htlc_info`] so we have access to the
+                       // inbound channel's state.
+                       onion_utils::Hop::Receive { .. } => return Ok((next_hop, shared_secret, None)),
+                       onion_utils::Hop::Forward {
+                               next_hop_data: msgs::OnionHopData { format: msgs::OnionHopDataFormat::FinalNode { .. }, .. }, ..
+                       } => {
+                               return_err!("Final Node OnionHopData provided for us as an intermediary node", 0x4000 | 22, &[0; 0]);
+                       }
+               };
  
-               let pending_forward_info = match next_hop {
+               // Perform outbound checks here instead of in [`Self::construct_pending_htlc_info`] because we
+               // can't hold the outbound peer state lock at the same time as the inbound peer state lock.
+               if let Some((err, mut code, chan_update)) = loop {
+                       let id_option = self.short_to_chan_info.read().unwrap().get(&outgoing_scid).cloned();
+                       let forwarding_chan_info_opt = match id_option {
+                               None => { // unknown_next_peer
+                                       // Note that this is likely a timing oracle for detecting whether an scid is a
+                                       // phantom or an intercept.
+                                       if (self.default_configuration.accept_intercept_htlcs &&
+                                               fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, outgoing_scid, &self.genesis_hash)) ||
+                                               fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, outgoing_scid, &self.genesis_hash)
+                                       {
+                                               None
+                                       } else {
+                                               break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
+                                       }
+                               },
+                               Some((cp_id, id)) => Some((cp_id.clone(), id.clone())),
+                       };
+                       let chan_update_opt = if let Some((counterparty_node_id, forwarding_id)) = forwarding_chan_info_opt {
+                               let per_peer_state = self.per_peer_state.read().unwrap();
+                               let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id);
+                               if peer_state_mutex_opt.is_none() {
+                                       break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
+                               }
+                               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
+                               let peer_state = &mut *peer_state_lock;
+                               let chan = match peer_state.channel_by_id.get_mut(&forwarding_id) {
+                                       None => {
+                                               // Channel was removed. The short_to_chan_info and channel_by_id maps
+                                               // have no consistency guarantees.
+                                               break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
+                                       },
+                                       Some(chan) => chan
+                               };
+                               if !chan.context.should_announce() && !self.default_configuration.accept_forwards_to_priv_channels {
+                                       // Note that the behavior here should be identical to the above block - we
+                                       // should NOT reveal the existence or non-existence of a private channel if
+                                       // we don't allow forwards outbound over them.
+                                       break Some(("Refusing to forward to a private channel based on our config.", 0x4000 | 10, None));
+                               }
+                               if chan.context.get_channel_type().supports_scid_privacy() && outgoing_scid != chan.context.outbound_scid_alias() {
+                                       // `option_scid_alias` (referred to in LDK as `scid_privacy`) means
+                                       // "refuse to forward unless the SCID alias was used", so we pretend
+                                       // we don't have the channel here.
+                                       break Some(("Refusing to forward over real channel SCID as our counterparty requested.", 0x4000 | 10, None));
+                               }
+                               let chan_update_opt = self.get_channel_update_for_onion(outgoing_scid, chan).ok();
+                               // Note that we could technically not return an error yet here and just hope
+                               // that the connection is reestablished or monitor updated by the time we get
+                               // around to doing the actual forward, but better to fail early if we can and
+                               // hopefully an attacker trying to path-trace payments cannot make this occur
+                               // on a small/per-node/per-channel scale.
+                               if !chan.context.is_live() { // channel_disabled
+                                       // If the channel_update we're going to return is disabled (i.e. the
+                                       // peer has been disabled for some time), return `channel_disabled`,
+                                       // otherwise return `temporary_channel_failure`.
+                                       if chan_update_opt.as_ref().map(|u| u.contents.flags & 2 == 2).unwrap_or(false) {
+                                               break Some(("Forwarding channel has been disconnected for some time.", 0x1000 | 20, chan_update_opt));
+                                       } else {
+                                               break Some(("Forwarding channel is not in a ready state.", 0x1000 | 7, chan_update_opt));
+                                       }
+                               }
+                               if outgoing_amt_msat < chan.context.get_counterparty_htlc_minimum_msat() { // amount_below_minimum
+                                       break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, chan_update_opt));
+                               }
+                               if let Err((err, code)) = chan.htlc_satisfies_config(&msg, outgoing_amt_msat, outgoing_cltv_value) {
+                                       break Some((err, code, chan_update_opt));
+                               }
+                               chan_update_opt
+                       } else {
+                               if (msg.cltv_expiry as u64) < (outgoing_cltv_value) as u64 + MIN_CLTV_EXPIRY_DELTA as u64 {
+                                       // We really should set `incorrect_cltv_expiry` here but as we're not
+                                       // forwarding over a real channel we can't generate a channel_update
+                                       // for it. Instead we just return a generic temporary_node_failure.
+                                       break Some((
+                                                       "Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta",
+                                                       0x2000 | 2, None,
+                                       ));
+                               }
+                               None
+                       };
+                       let cur_height = self.best_block.read().unwrap().height() + 1;
+                       // Theoretically, channel counterparty shouldn't send us a HTLC expiring now,
+                       // but we want to be robust wrt to counterparty packet sanitization (see
+                       // HTLC_FAIL_BACK_BUFFER rationale).
+                       if msg.cltv_expiry <= cur_height + HTLC_FAIL_BACK_BUFFER as u32 { // expiry_too_soon
+                               break Some(("CLTV expiry is too close", 0x1000 | 14, chan_update_opt));
+                       }
+                       if msg.cltv_expiry > cur_height + CLTV_FAR_FAR_AWAY as u32 { // expiry_too_far
+                               break Some(("CLTV expiry is too far in the future", 21, None));
+                       }
+                       // If the HTLC expires ~now, don't bother trying to forward it to our
+                       // counterparty. They should fail it anyway, but we don't want to bother with
+                       // the round-trips or risk them deciding they definitely want the HTLC and
+                       // force-closing to ensure they get it if we're offline.
+                       // We previously had a much more aggressive check here which tried to ensure
+                       // our counterparty receives an HTLC which has *our* risk threshold met on it,
+                       // but there is no need to do that, and since we're a bit conservative with our
+                       // risk threshold it just results in failing to forward payments.
+                       if (outgoing_cltv_value) as u64 <= (cur_height + LATENCY_GRACE_PERIOD_BLOCKS) as u64 {
+                               break Some(("Outgoing CLTV value is too soon", 0x1000 | 14, chan_update_opt));
+                       }
+                       break None;
+               }
+               {
+                       let mut res = VecWriter(Vec::with_capacity(chan_update.serialized_length() + 2 + 8 + 2));
+                       if let Some(chan_update) = chan_update {
+                               if code == 0x1000 | 11 || code == 0x1000 | 12 {
+                                       msg.amount_msat.write(&mut res).expect("Writes cannot fail");
+                               }
+                               else if code == 0x1000 | 13 {
+                                       msg.cltv_expiry.write(&mut res).expect("Writes cannot fail");
+                               }
+                               else if code == 0x1000 | 20 {
+                                       // TODO: underspecified, follow https://github.com/lightning/bolts/issues/791
+                                       0u16.write(&mut res).expect("Writes cannot fail");
+                               }
+                               (chan_update.serialized_length() as u16 + 2).write(&mut res).expect("Writes cannot fail");
+                               msgs::ChannelUpdate::TYPE.write(&mut res).expect("Writes cannot fail");
+                               chan_update.write(&mut res).expect("Writes cannot fail");
+                       } else if code & 0x1000 == 0x1000 {
+                               // If we're trying to return an error that requires a `channel_update` but
+                               // we're forwarding to a phantom or intercept "channel" (i.e. cannot
+                               // generate an update), just use the generic "temporary_node_failure"
+                               // instead.
+                               code = 0x2000 | 2;
+                       }
+                       return_err!(err, code, &res.0[..]);
+               }
+               Ok((next_hop, shared_secret, next_packet_pk_opt))
+       }
+       fn construct_pending_htlc_status<'a>(
+               &self, msg: &msgs::UpdateAddHTLC, shared_secret: [u8; 32], decoded_hop: onion_utils::Hop,
+               allow_underpay: bool, next_packet_pubkey_opt: Option<Result<PublicKey, secp256k1::Error>>
+       ) -> PendingHTLCStatus {
+               macro_rules! return_err {
+                       ($msg: expr, $err_code: expr, $data: expr) => {
+                               {
+                                       log_info!(self.logger, "Failed to accept/forward incoming HTLC: {}", $msg);
+                                       return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
+                                               channel_id: msg.channel_id,
+                                               htlc_id: msg.htlc_id,
+                                               reason: HTLCFailReason::reason($err_code, $data.to_vec())
+                                                       .get_encrypted_failure_packet(&shared_secret, &None),
+                                       }));
+                               }
+                       }
+               }
+               match decoded_hop {
                        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) {
+                               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.skimmed_fee_msat)
+                               {
                                        Ok(info) => {
                                                // Note that we could obviously respond immediately with an update_fulfill_htlc
                                                // message, however that would leak that we are the recipient of this payment, so
                                }
                        },
                        onion_utils::Hop::Forward { next_hop_data, next_hop_hmac, new_packet_bytes } => {
-                               let new_pubkey = msg.onion_routing_packet.public_key.unwrap();
+                               debug_assert!(next_packet_pubkey_opt.is_some());
                                let outgoing_packet = msgs::OnionPacket {
                                        version: 0,
-                                       public_key: onion_utils::next_hop_packet_pubkey(&self.secp_ctx, new_pubkey, &shared_secret),
+                                       public_key: next_packet_pubkey_opt.unwrap_or(Err(secp256k1::Error::InvalidPublicKey)),
                                        hop_data: new_packet_bytes,
                                        hmac: next_hop_hmac.clone(),
                                };
                                        incoming_amt_msat: Some(msg.amount_msat),
                                        outgoing_amt_msat: next_hop_data.amt_to_forward,
                                        outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
+                                       skimmed_fee_msat: None,
                                })
                        }
-               };
-               if let &PendingHTLCStatus::Forward(PendingHTLCInfo { ref routing, ref outgoing_amt_msat, ref outgoing_cltv_value, .. }) = &pending_forward_info {
-                       // If short_channel_id is 0 here, we'll reject the HTLC as there cannot be a channel
-                       // with a short_channel_id of 0. This is important as various things later assume
-                       // short_channel_id is non-0 in any ::Forward.
-                       if let &PendingHTLCRouting::Forward { ref short_channel_id, .. } = routing {
-                               if let Some((err, mut code, chan_update)) = loop {
-                                       let id_option = self.short_to_chan_info.read().unwrap().get(short_channel_id).cloned();
-                                       let forwarding_chan_info_opt = match id_option {
-                                               None => { // unknown_next_peer
-                                                       // Note that this is likely a timing oracle for detecting whether an scid is a
-                                                       // phantom or an intercept.
-                                                       if (self.default_configuration.accept_intercept_htlcs &&
-                                                          fake_scid::is_valid_intercept(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash)) ||
-                                                          fake_scid::is_valid_phantom(&self.fake_scid_rand_bytes, *short_channel_id, &self.genesis_hash)
-                                                       {
-                                                               None
-                                                       } else {
-                                                               break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
-                                                       }
-                                               },
-                                               Some((cp_id, id)) => Some((cp_id.clone(), id.clone())),
-                                       };
-                                       let chan_update_opt = if let Some((counterparty_node_id, forwarding_id)) = forwarding_chan_info_opt {
-                                               let per_peer_state = self.per_peer_state.read().unwrap();
-                                               let peer_state_mutex_opt = per_peer_state.get(&counterparty_node_id);
-                                               if peer_state_mutex_opt.is_none() {
-                                                       break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
-                                               }
-                                               let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap();
-                                               let peer_state = &mut *peer_state_lock;
-                                               let chan = match peer_state.channel_by_id.get_mut(&forwarding_id) {
-                                                       None => {
-                                                               // Channel was removed. The short_to_chan_info and channel_by_id maps
-                                                               // have no consistency guarantees.
-                                                               break Some(("Don't have available channel for forwarding as requested.", 0x4000 | 10, None));
-                                                       },
-                                                       Some(chan) => chan
-                                               };
-                                               if !chan.context.should_announce() && !self.default_configuration.accept_forwards_to_priv_channels {
-                                                       // Note that the behavior here should be identical to the above block - we
-                                                       // should NOT reveal the existence or non-existence of a private channel if
-                                                       // we don't allow forwards outbound over them.
-                                                       break Some(("Refusing to forward to a private channel based on our config.", 0x4000 | 10, None));
-                                               }
-                                               if chan.context.get_channel_type().supports_scid_privacy() && *short_channel_id != chan.context.outbound_scid_alias() {
-                                                       // `option_scid_alias` (referred to in LDK as `scid_privacy`) means
-                                                       // "refuse to forward unless the SCID alias was used", so we pretend
-                                                       // we don't have the channel here.
-                                                       break Some(("Refusing to forward over real channel SCID as our counterparty requested.", 0x4000 | 10, None));
-                                               }
-                                               let chan_update_opt = self.get_channel_update_for_onion(*short_channel_id, chan).ok();
-                                               // Note that we could technically not return an error yet here and just hope
-                                               // that the connection is reestablished or monitor updated by the time we get
-                                               // around to doing the actual forward, but better to fail early if we can and
-                                               // hopefully an attacker trying to path-trace payments cannot make this occur
-                                               // on a small/per-node/per-channel scale.
-                                               if !chan.context.is_live() { // channel_disabled
-                                                       // If the channel_update we're going to return is disabled (i.e. the
-                                                       // peer has been disabled for some time), return `channel_disabled`,
-                                                       // otherwise return `temporary_channel_failure`.
-                                                       if chan_update_opt.as_ref().map(|u| u.contents.flags & 2 == 2).unwrap_or(false) {
-                                                               break Some(("Forwarding channel has been disconnected for some time.", 0x1000 | 20, chan_update_opt));
-                                                       } else {
-                                                               break Some(("Forwarding channel is not in a ready state.", 0x1000 | 7, chan_update_opt));
-                                                       }
-                                               }
-                                               if *outgoing_amt_msat < chan.context.get_counterparty_htlc_minimum_msat() { // amount_below_minimum
-                                                       break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, chan_update_opt));
-                                               }
-                                               if let Err((err, code)) = chan.htlc_satisfies_config(&msg, *outgoing_amt_msat, *outgoing_cltv_value) {
-                                                       break Some((err, code, chan_update_opt));
-                                               }
-                                               chan_update_opt
-                                       } else {
-                                               if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + MIN_CLTV_EXPIRY_DELTA as u64 {
-                                                       // We really should set `incorrect_cltv_expiry` here but as we're not
-                                                       // forwarding over a real channel we can't generate a channel_update
-                                                       // for it. Instead we just return a generic temporary_node_failure.
-                                                       break Some((
-                                                               "Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta",
-                                                               0x2000 | 2, None,
-                                                       ));
-                                               }
-                                               None
-                                       };
-                                       let cur_height = self.best_block.read().unwrap().height() + 1;
-                                       // Theoretically, channel counterparty shouldn't send us a HTLC expiring now,
-                                       // but we want to be robust wrt to counterparty packet sanitization (see
-                                       // HTLC_FAIL_BACK_BUFFER rationale).
-                                       if msg.cltv_expiry <= cur_height + HTLC_FAIL_BACK_BUFFER as u32 { // expiry_too_soon
-                                               break Some(("CLTV expiry is too close", 0x1000 | 14, chan_update_opt));
-                                       }
-                                       if msg.cltv_expiry > cur_height + CLTV_FAR_FAR_AWAY as u32 { // expiry_too_far
-                                               break Some(("CLTV expiry is too far in the future", 21, None));
-                                       }
-                                       // If the HTLC expires ~now, don't bother trying to forward it to our
-                                       // counterparty. They should fail it anyway, but we don't want to bother with
-                                       // the round-trips or risk them deciding they definitely want the HTLC and
-                                       // force-closing to ensure they get it if we're offline.
-                                       // We previously had a much more aggressive check here which tried to ensure
-                                       // our counterparty receives an HTLC which has *our* risk threshold met on it,
-                                       // but there is no need to do that, and since we're a bit conservative with our
-                                       // risk threshold it just results in failing to forward payments.
-                                       if (*outgoing_cltv_value) as u64 <= (cur_height + LATENCY_GRACE_PERIOD_BLOCKS) as u64 {
-                                               break Some(("Outgoing CLTV value is too soon", 0x1000 | 14, chan_update_opt));
-                                       }
-                                       break None;
-                               }
-                               {
-                                       let mut res = VecWriter(Vec::with_capacity(chan_update.serialized_length() + 2 + 8 + 2));
-                                       if let Some(chan_update) = chan_update {
-                                               if code == 0x1000 | 11 || code == 0x1000 | 12 {
-                                                       msg.amount_msat.write(&mut res).expect("Writes cannot fail");
-                                               }
-                                               else if code == 0x1000 | 13 {
-                                                       msg.cltv_expiry.write(&mut res).expect("Writes cannot fail");
-                                               }
-                                               else if code == 0x1000 | 20 {
-                                                       // TODO: underspecified, follow https://github.com/lightning/bolts/issues/791
-                                                       0u16.write(&mut res).expect("Writes cannot fail");
-                                               }
-                                               (chan_update.serialized_length() as u16 + 2).write(&mut res).expect("Writes cannot fail");
-                                               msgs::ChannelUpdate::TYPE.write(&mut res).expect("Writes cannot fail");
-                                               chan_update.write(&mut res).expect("Writes cannot fail");
-                                       } else if code & 0x1000 == 0x1000 {
-                                               // If we're trying to return an error that requires a `channel_update` but
-                                               // we're forwarding to a phantom or intercept "channel" (i.e. cannot
-                                               // generate an update), just use the generic "temporary_node_failure"
-                                               // instead.
-                                               code = 0x2000 | 2;
-                                       }
-                                       return_err!(err, code, &res.0[..]);
-                               }
-                       }
                }
-               pending_forward_info
        }
  
        /// Gets the current [`channel_update`] for the given channel. This first checks if the channel is
                                                session_priv: session_priv.clone(),
                                                first_hop_htlc_msat: htlc_msat,
                                                payment_id,
-                                       }, onion_packet, &self.logger);
+                                       }, onion_packet, None, &self.logger);
                                match break_chan_entry!(self, send_res, chan) {
                                        Some(monitor_update) => {
                                                let update_id = monitor_update.update_id;
        /// [`ChannelManager::fail_intercepted_htlc`] MUST be called in response to the event.
        ///
        /// Note that LDK does not enforce fee requirements in `amt_to_forward_msat`, and will not stop
-       /// you from forwarding more than you received.
+       /// you from forwarding more than you received. See
+       /// [`HTLCIntercepted::expected_outbound_amount_msat`] for more on forwarding a different amount
+       /// than expected.
        ///
        /// Errors if the event was not handled in time, in which case the HTLC was automatically failed
        /// backwards.
        ///
        /// [`UserConfig::accept_intercept_htlcs`]: crate::util::config::UserConfig::accept_intercept_htlcs
        /// [`HTLCIntercepted`]: events::Event::HTLCIntercepted
+       /// [`HTLCIntercepted::expected_outbound_amount_msat`]: events::Event::HTLCIntercepted::expected_outbound_amount_msat
        // TODO: when we move to deciding the best outbound channel at forward time, only take
        // `next_node_id` and not `next_hop_channel_id`
        pub fn forward_intercepted_htlc(&self, intercept_id: InterceptId, next_hop_channel_id: &[u8; 32], next_node_id: PublicKey, amt_to_forward_msat: u64) -> Result<(), APIError> {
                        },
                        _ => unreachable!() // Only `PendingHTLCRouting::Forward`s are intercepted
                };
+               let skimmed_fee_msat =
+                       payment.forward_info.outgoing_amt_msat.saturating_sub(amt_to_forward_msat);
                let pending_htlc_info = PendingHTLCInfo {
+                       skimmed_fee_msat: if skimmed_fee_msat == 0 { None } else { Some(skimmed_fee_msat) },
                        outgoing_amt_msat: amt_to_forward_msat, routing, ..payment.forward_info
                };
  
                                                                                prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id,
                                                                                forward_info: PendingHTLCInfo {
                                                                                        routing, incoming_shared_secret, payment_hash, outgoing_amt_msat,
-                                                                                       outgoing_cltv_value, incoming_amt_msat: _
+                                                                                       outgoing_cltv_value, ..
                                                                                }
                                                                        }) => {
                                                                                macro_rules! failure_handler {
                                                                                                };
                                                                                                match next_hop {
                                                                                                        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)) {
+                                                                                                               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, 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))
                                                                                                                }
                                                                                prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id: _,
                                                                                forward_info: PendingHTLCInfo {
                                                                                        incoming_shared_secret, payment_hash, outgoing_amt_msat, outgoing_cltv_value,
-                                                                                       routing: PendingHTLCRouting::Forward { onion_packet, .. }, incoming_amt_msat: _,
+                                                                                       routing: PendingHTLCRouting::Forward { onion_packet, .. }, skimmed_fee_msat, ..
                                                                                },
                                                                        }) => {
                                                                                log_trace!(self.logger, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", prev_short_channel_id, log_bytes!(payment_hash.0), short_chan_id);
                                                                                });
                                                                                if let Err(e) = chan.get_mut().queue_add_htlc(outgoing_amt_msat,
                                                                                        payment_hash, outgoing_cltv_value, htlc_source.clone(),
-                                                                                       onion_packet, &self.logger)
+                                                                                       onion_packet, skimmed_fee_msat, &self.logger)
                                                                                {
                                                                                        if let ChannelError::Ignore(msg) = e {
                                                                                                log_trace!(self.logger, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(payment_hash.0), msg);
                                                        HTLCForwardInfo::AddHTLC(PendingAddHTLCInfo {
                                                                prev_short_channel_id, prev_htlc_id, prev_funding_outpoint, prev_user_channel_id,
                                                                forward_info: PendingHTLCInfo {
-                                                                       routing, incoming_shared_secret, payment_hash, incoming_amt_msat, outgoing_amt_msat, ..
+                                                                       routing, incoming_shared_secret, payment_hash, incoming_amt_msat, outgoing_amt_msat,
+                                                                       skimmed_fee_msat, ..
                                                                }
                                                        }) => {
                                                                let (cltv_expiry, onion_payload, payment_data, phantom_shared_secret, mut onion_fields) = match routing {
                                                                        total_msat: if let Some(data) = &payment_data { data.total_msat } else { outgoing_amt_msat },
                                                                        cltv_expiry,
                                                                        onion_payload,
+                                                                       counterparty_skimmed_fee_msat: skimmed_fee_msat,
                                                                };
  
                                                                let mut committed_to_claimable = false;
                                                                                        htlcs.push(claimable_htlc);
                                                                                        let amount_msat = htlcs.iter().map(|htlc| htlc.value).sum();
                                                                                        htlcs.iter_mut().for_each(|htlc| htlc.total_value_received = Some(amount_msat));
+                                                                                       let counterparty_skimmed_fee_msat = htlcs.iter()
+                                                                                               .map(|htlc| htlc.counterparty_skimmed_fee_msat.unwrap_or(0)).sum();
+                                                                                       debug_assert!(total_value.saturating_sub(amount_msat) <=
+                                                                                               counterparty_skimmed_fee_msat);
                                                                                        new_events.push_back((events::Event::PaymentClaimable {
                                                                                                receiver_node_id: Some(receiver_node_id),
                                                                                                payment_hash,
                                                                                                purpose: $purpose,
                                                                                                amount_msat,
+                                                                                               counterparty_skimmed_fee_msat,
                                                                                                via_channel_id: Some(prev_channel_id),
                                                                                                via_user_channel_id: Some(prev_user_channel_id),
                                                                                                claim_deadline: Some(earliest_expiry - HTLC_FAIL_BACK_BUFFER),
                //encrypted with the same key. It's not immediately obvious how to usefully exploit that,
                //but we should prevent it anyway.
  
-               let pending_forward_info = self.decode_update_add_htlc_onion(msg);
+               let decoded_hop_res = self.decode_update_add_htlc_onion(msg);
                let per_peer_state = self.per_peer_state.read().unwrap();
                let peer_state_mutex = per_peer_state.get(counterparty_node_id)
                        .ok_or_else(|| {
                match peer_state.channel_by_id.entry(msg.channel_id) {
                        hash_map::Entry::Occupied(mut chan) => {
  
+                               let pending_forward_info = match decoded_hop_res {
+                                       Ok((next_hop, shared_secret, next_packet_pk_opt)) =>
+                                               self.construct_pending_htlc_status(msg, shared_secret, next_hop,
+                                                       chan.get().context.config().accept_underpaying_htlcs, next_packet_pk_opt),
+                                       Err(e) => PendingHTLCStatus::Fail(e)
+                               };
                                let create_pending_htlc_status = |chan: &Channel<<SP::Target as SignerProvider>::Signer>, pending_forward_info: PendingHTLCStatus, error_code: u16| {
                                        // If the update_add is completely bogus, the call will Err and we will close,
                                        // but if we've sent a shutdown and they haven't acknowledged it yet, we just
                }
        }
  
 -      fn set_payment_hash_secret_map(&self, payment_hash: PaymentHash, payment_preimage: Option<PaymentPreimage>, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
 -              assert!(invoice_expiry_delta_secs <= 60*60*24*365); // Sadly bitcoin timestamps are u32s, so panic before 2106
 -
 -              if min_value_msat.is_some() && min_value_msat.unwrap() > MAX_VALUE_MSAT {
 -                      return Err(APIError::APIMisuseError { err: format!("min_value_msat of {} greater than total 21 million bitcoin supply", min_value_msat.unwrap()) });
 -              }
 -
 -              let payment_secret = PaymentSecret(self.entropy_source.get_secure_random_bytes());
 -
 -              let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 -              let mut payment_secrets = self.pending_inbound_payments.lock().unwrap();
 -              match payment_secrets.entry(payment_hash) {
 -                      hash_map::Entry::Vacant(e) => {
 -                              e.insert(PendingInboundPayment {
 -                                      payment_secret, min_value_msat, payment_preimage,
 -                                      user_payment_id: 0, // For compatibility with version 0.0.103 and earlier
 -                                      // We assume that highest_seen_timestamp is pretty close to the current time -
 -                                      // it's updated when we receive a new block with the maximum time we've seen in
 -                                      // a header. It should never be more than two hours in the future.
 -                                      // Thus, we add two hours here as a buffer to ensure we absolutely
 -                                      // never fail a payment too early.
 -                                      // Note that we assume that received blocks have reasonably up-to-date
 -                                      // timestamps.
 -                                      expiry_time: self.highest_seen_timestamp.load(Ordering::Acquire) as u64 + invoice_expiry_delta_secs as u64 + 7200,
 -                              });
 -                      },
 -                      hash_map::Entry::Occupied(_) => return Err(APIError::APIMisuseError { err: "Duplicate payment hash".to_owned() }),
 -              }
 -              Ok(payment_secret)
 -      }
 -
        /// Gets a payment secret and payment hash for use in an invoice given to a third party wishing
        /// to pay us.
        ///
                        min_final_cltv_expiry_delta)
        }
  
 -      /// Legacy version of [`create_inbound_payment`]. Use this method if you wish to share
 -      /// serialized state with LDK node(s) running 0.0.103 and earlier.
 -      ///
 -      /// May panic if `invoice_expiry_delta_secs` is greater than one year.
 -      ///
 -      /// # Note
 -      /// This method is deprecated and will be removed soon.
 -      ///
 -      /// [`create_inbound_payment`]: Self::create_inbound_payment
 -      #[deprecated]
 -      pub fn create_inbound_payment_legacy(&self, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<(PaymentHash, PaymentSecret), APIError> {
 -              let payment_preimage = PaymentPreimage(self.entropy_source.get_secure_random_bytes());
 -              let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
 -              let payment_secret = self.set_payment_hash_secret_map(payment_hash, Some(payment_preimage), min_value_msat, invoice_expiry_delta_secs)?;
 -              Ok((payment_hash, payment_secret))
 -      }
 -
        /// Gets a [`PaymentSecret`] for a given [`PaymentHash`], for which the payment preimage is
        /// stored external to LDK.
        ///
                        min_final_cltv_expiry)
        }
  
 -      /// Legacy version of [`create_inbound_payment_for_hash`]. Use this method if you wish to share
 -      /// serialized state with LDK node(s) running 0.0.103 and earlier.
 -      ///
 -      /// May panic if `invoice_expiry_delta_secs` is greater than one year.
 -      ///
 -      /// # Note
 -      /// This method is deprecated and will be removed soon.
 -      ///
 -      /// [`create_inbound_payment_for_hash`]: Self::create_inbound_payment_for_hash
 -      #[deprecated]
 -      pub fn create_inbound_payment_for_hash_legacy(&self, payment_hash: PaymentHash, min_value_msat: Option<u64>, invoice_expiry_delta_secs: u32) -> Result<PaymentSecret, APIError> {
 -              self.set_payment_hash_secret_map(payment_hash, None, min_value_msat, invoice_expiry_delta_secs)
 -      }
 -
        /// Gets an LDK-generated payment preimage from a payment hash and payment secret that were
        /// previously returned from [`create_inbound_payment`].
        ///
@@@ -7347,6 -7479,7 +7417,7 @@@ impl_writeable_tlv_based!(PendingHTLCIn
        (6, outgoing_amt_msat, required),
        (8, outgoing_cltv_value, required),
        (9, incoming_amt_msat, option),
+       (10, skimmed_fee_msat, option),
  });
  
  
@@@ -7445,6 -7578,7 +7516,7 @@@ impl Writeable for ClaimableHTLC 
                        (5, self.total_value_received, option),
                        (6, self.cltv_expiry, required),
                        (8, keysend_preimage, option),
+                       (10, self.counterparty_skimmed_fee_msat, option),
                });
                Ok(())
        }
  
  impl Readable for ClaimableHTLC {
        fn read<R: Read>(reader: &mut R) -> Result<Self, DecodeError> {
-               let mut prev_hop = crate::util::ser::RequiredWrapper(None);
-               let mut value = 0;
-               let mut sender_intended_value = None;
-               let mut payment_data: Option<msgs::FinalOnionHopData> = None;
-               let mut cltv_expiry = 0;
-               let mut total_value_received = None;
-               let mut total_msat = None;
-               let mut keysend_preimage: Option<PaymentPreimage> = None;
-               read_tlv_fields!(reader, {
+               _init_and_read_tlv_fields!(reader, {
                        (0, prev_hop, required),
                        (1, total_msat, option),
-                       (2, value, required),
+                       (2, value_ser, required),
                        (3, sender_intended_value, option),
-                       (4, payment_data, option),
+                       (4, payment_data_opt, option),
                        (5, total_value_received, option),
                        (6, cltv_expiry, required),
-                       (8, keysend_preimage, option)
+                       (8, keysend_preimage, option),
+                       (10, counterparty_skimmed_fee_msat, option),
                });
+               let payment_data: Option<msgs::FinalOnionHopData> = payment_data_opt;
+               let value = value_ser.0.unwrap();
                let onion_payload = match keysend_preimage {
                        Some(p) => {
                                if payment_data.is_some() {
                        total_value_received,
                        total_msat: total_msat.unwrap(),
                        onion_payload,
-                       cltv_expiry,
+                       cltv_expiry: cltv_expiry.0.unwrap(),
+                       counterparty_skimmed_fee_msat,
                })
        }
  }
@@@ -9619,6 -9749,50 +9687,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() {
index 8f093f2a92237984ca81751ed8be2c40a3972952,63b3b2bb0a49d9ba661f00050ddb7c2c031ed098..9d894a7ac3ffc859013a7cc4e9237630d8ba8c19
@@@ -1393,6 -1393,7 +1393,7 @@@ fn test_fee_spike_violation_fails_htlc(
                payment_hash: payment_hash,
                cltv_expiry: htlc_cltv,
                onion_routing_packet: onion_packet,
+               skimmed_fee_msat: None,
        };
  
        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg);
@@@ -1582,6 -1583,7 +1583,7 @@@ fn test_chan_reserve_violation_inbound_
                payment_hash: payment_hash,
                cltv_expiry: htlc_cltv,
                onion_routing_packet: onion_packet,
+               skimmed_fee_msat: None,
        };
  
        nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &msg);
@@@ -1758,6 -1760,7 +1760,7 @@@ fn test_chan_reserve_violation_inbound_
                payment_hash: our_payment_hash_1,
                cltv_expiry: htlc_cltv,
                onion_routing_packet: onion_packet,
+               skimmed_fee_msat: None,
        };
  
        nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &msg);
@@@ -3410,6 -3413,7 +3413,7 @@@ fn fail_backward_pending_htlc_upon_chan
                        payment_hash,
                        cltv_expiry,
                        onion_routing_packet,
+                       skimmed_fee_msat: None,
                };
                nodes[0].node.handle_update_add_htlc(&nodes[1].node.get_our_node_id(), &update_add_htlc);
        }
@@@ -6259,6 -6263,7 +6263,7 @@@ fn test_update_add_htlc_bolt2_receiver_
                payment_hash: our_payment_hash,
                cltv_expiry: htlc_cltv,
                onion_routing_packet: onion_packet.clone(),
+               skimmed_fee_msat: None,
        };
  
        for i in 0..50 {
@@@ -8202,6 -8207,67 +8207,6 @@@ fn test_preimage_storage() 
        }
  }
  
 -#[test]
 -#[allow(deprecated)]
 -fn test_secret_timeout() {
 -      // Simple test of payment secret storage time outs. After
 -      // `create_inbound_payment(_for_hash)_legacy` is removed, this test will be removed as well.
 -      let chanmon_cfgs = create_chanmon_cfgs(2);
 -      let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 -      let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 -      let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 -
 -      create_announced_chan_between_nodes(&nodes, 0, 1).0.contents.short_channel_id;
 -
 -      let (payment_hash, payment_secret_1) = nodes[1].node.create_inbound_payment_legacy(Some(100_000), 2).unwrap();
 -
 -      // We should fail to register the same payment hash twice, at least until we've connected a
 -      // block with time 7200 + CHAN_CONFIRM_DEPTH + 1.
 -      if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash_legacy(payment_hash, Some(100_000), 2) {
 -              assert_eq!(err, "Duplicate payment hash");
 -      } else { panic!(); }
 -      let mut block = {
 -              let node_1_blocks = nodes[1].blocks.lock().unwrap();
 -              create_dummy_block(node_1_blocks.last().unwrap().0.block_hash(), node_1_blocks.len() as u32 + 7200, Vec::new())
 -      };
 -      connect_block(&nodes[1], &block);
 -      if let Err(APIError::APIMisuseError { err }) = nodes[1].node.create_inbound_payment_for_hash_legacy(payment_hash, Some(100_000), 2) {
 -              assert_eq!(err, "Duplicate payment hash");
 -      } else { panic!(); }
 -
 -      // If we then connect the second block, we should be able to register the same payment hash
 -      // again (this time getting a new payment secret).
 -      block.header.prev_blockhash = block.header.block_hash();
 -      block.header.time += 1;
 -      connect_block(&nodes[1], &block);
 -      let our_payment_secret = nodes[1].node.create_inbound_payment_for_hash_legacy(payment_hash, Some(100_000), 2).unwrap();
 -      assert_ne!(payment_secret_1, our_payment_secret);
 -
 -      {
 -              let (route, _, _, _) = get_route_and_payment_hash!(nodes[0], nodes[1], 100_000);
 -              nodes[0].node.send_payment_with_route(&route, payment_hash,
 -                      RecipientOnionFields::secret_only(our_payment_secret), PaymentId(payment_hash.0)).unwrap();
 -              check_added_monitors!(nodes[0], 1);
 -              let mut events = nodes[0].node.get_and_clear_pending_msg_events();
 -              let mut payment_event = SendEvent::from_event(events.pop().unwrap());
 -              nodes[1].node.handle_update_add_htlc(&nodes[0].node.get_our_node_id(), &payment_event.msgs[0]);
 -              commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false);
 -      }
 -      // Note that after leaving the above scope we have no knowledge of any arguments or return
 -      // values from previous calls.
 -      expect_pending_htlcs_forwardable!(nodes[1]);
 -      let events = nodes[1].node.get_and_clear_pending_events();
 -      assert_eq!(events.len(), 1);
 -      match events[0] {
 -              Event::PaymentClaimable { purpose: PaymentPurpose::InvoicePayment { payment_preimage, payment_secret }, .. } => {
 -                      assert!(payment_preimage.is_none());
 -                      assert_eq!(payment_secret, our_payment_secret);
 -                      // We don't actually have the payment preimage with which to claim this payment!
 -              },
 -              _ => panic!("Unexpected event"),
 -      }
 -}
 -
  #[test]
  fn test_bad_secret_hash() {
        // Simple test of unregistered payment hash/invalid payment secret handling