XMoveFunds: on FEE_INSUFFICIENT, exclude the incoming channel, not the outgoing
Some checks are pending
Code Base Sanity Check / tests (push) Waiting to run
Code Base Sanity Check / coverage (push) Waiting to run
Code Base Sanity Check / build-clang (push) Waiting to run

A FEE_INSUFFICIENT at a forwarding node means the node required more than we
paid, where its required fee is outbound_fee(outgoing channel) +
inbound_fee(incoming channel). The onion error names and carries the policy for
the node's OUTGOING channel only; the inbound fee that actually causes the
shortfall lives on the INCOMING channel (route[erring_index-1]) and is not in
the payload. The previous code keyed off the outgoing channel's own inbound-fee
TLV and excluded the outgoing channel -- the wrong edge -- and only worked by
luck when the node set positive inbound fees broadly or when no channel_update
was present.

Replace that with discriminating logic for failcode 0x100c:
- Compute the fee we allocated at the erring hop (amount_in - amount_out).
- If the error carried a channel_update, compute the outgoing channel's required
  outbound fee (base + prop*amount_out/1e6).
- If we already paid the outbound fee (allocated >= required_out) but still
  failed, the shortfall is the inbound fee on the incoming channel: hard-exclude
  route[erring_index-1] with inform_channel_constrained(amount=1) -> max_msat=0.
  This is a self-aging constraint, so it recovers automatically if the peer
  later drops the inbound fee.
- If we underpaid the outbound fee (allocated < required_out), it is a stale
  outbound policy: refresh the outgoing channel from the channel_update.
- If there is no channel_update to compare, treat it as the inbound case and
  exclude the incoming channel (a plain stale-outbound failure normally carries
  the update). This also covers the terminally-failed-MPP-part case where
  waitsendpay omits raw_message.
- Never self-exclude one of our own channels; fall through if the incoming hop
  is local.

Other policy-carrying failcodes (amount_below_minimum, incorrect_cltv_expiry,
expiry_too_soon) still refresh the outgoing channel, since those genuinely
concern the outgoing channel's published policy.

Needs prod1 retest against the cyberdyne/Tachyon positive-inbound paths.
This commit is contained in:
Ken Sedgwick 2026-06-04 11:14:43 -07:00
parent b844a4f146
commit 118a722b32
No known key found for this signature in database
GPG key ID: DBD2AF0849D711A9

View file

@ -1244,15 +1244,104 @@ private:
return;
}
/* Policy-carrying failcodes: try to refresh
* the forwarder's published policy from the
* embedded channel_update. Applies even to
* our local-channel scids (the failing
* direction is the peer's outbound, gossip-
* derived). */
/* FEE_INSUFFICIENT (0x100c): the required fee at the
* erring node is outbound_fee(erring/OUTGOING channel) +
* inbound_fee(INCOMING channel). The error names and
* carries the policy for the OUTGOING channel only, so we
* must decide which side is actually at fault:
* - if we already paid the outgoing channel's advertised
* outbound fee (allocated >= required_out) yet still
* failed, the shortfall is the INBOUND fee on the
* incoming channel route[eidx-1] -> hard-exclude that
* channel (a max_msat=0 constraint that self-ages, so it
* recovers if the peer later drops the inbound fee);
* - if we underpaid the outbound fee (allocated <
* required_out), it is a stale outbound policy -> refresh
* the outgoing channel from the embedded channel_update;
* - no channel_update to compare -> treat as the inbound
* case (a plain stale-outbound failure normally carries
* the update) and exclude the incoming channel.
* NB: the OUTGOING channel's own inbound-fee TLV is NOT the
* fee that bit us (that is the incoming channel's), so we no
* longer key off it. */
if (fail == 0x100c) {
/* fee we allocated at the erring (outgoing) hop */
auto alloc = std::uint64_t(0);
if (eidx < askrene_path.size()
&& askrene_path[eidx].has("amount_in_msat")
&& askrene_path[eidx].has("amount_out_msat")) {
auto in = Ln::Amount::object(
askrene_path[eidx]["amount_in_msat"]).to_msat();
auto out = Ln::Amount::object(
askrene_path[eidx]["amount_out_msat"]).to_msat();
alloc = (in >= out) ? in - out : std::uint64_t(0);
}
ChanUpdate cu;
auto have_cu = data.has("raw_message")
&& eidx < askrene_path.size()
&& askrene_path[eidx].has("amount_out_msat")
&& parse_chan_update(
std::string(data["raw_message"]), cu);
auto outbound_satisfied = true;
if (have_cu) {
auto out = Ln::Amount::object(
askrene_path[eidx]["amount_out_msat"]).to_msat();
auto required_out =
std::uint64_t(cu.fee_base_msat)
+ std::uint64_t(
cu.fee_proportional_millionths)
* out / 1000000;
outbound_satisfied = (alloc >= required_out);
}
/* Inbound-fee case: exclude the INCOMING channel
* route[eidx-1], unless it is one of our own channels
* (auto.localchans owns those; never self-exclude). */
if (outbound_satisfied && eidx >= 1
&& (eidx - 1) < askrene_path.size()
&& askrene_path[eidx - 1].has("short_channel_id_dir")) {
auto scidd = std::string(
askrene_path[eidx - 1]["short_channel_id_dir"]);
auto slash = scidd.find('/');
auto in_scid = scidd.substr(0, slash);
if (!our_scids.count(in_scid)) {
auto in_dir = std::uint32_t(std::stoul(
scidd.substr(slash + 1)));
actions.push_back(
Boss::Mod::AskreneLayer::
inform_channel_constrained(
*rpc,
Boss::Mod::AskreneLayer::
xrebalance_layer_name,
Ln::Scid(in_scid), in_dir,
Ln::Amount::msat(1)));
return;
}
}
/* Stale outbound fee: refresh the outgoing channel. */
if (have_cu) {
actions.push_back(
Boss::Mod::AskreneLayer::update_channel(
*rpc,
Boss::Mod::AskreneLayer::
xrebalance_layer_name,
Ln::Scid(echan_str), edir,
cu.enabled,
Ln::Amount::msat(cu.htlc_minimum_msat),
Ln::Amount::msat(cu.htlc_maximum_msat),
Ln::Amount::msat(cu.fee_base_msat),
cu.fee_proportional_millionths,
cu.cltv_expiry_delta));
return;
}
/* else: fall through to the capacity constraint. */
}
/* Other policy-carrying failcodes (amount_below_minimum,
* incorrect_cltv_expiry, expiry_too_soon) genuinely concern
* the OUTGOING channel's published policy -- refresh it from
* the embedded channel_update. */
auto policy_carrying =
fail == 0x100b
|| fail == 0x100c
|| fail == 0x100d
|| fail == 0x100e;
if (policy_carrying && data.has("raw_message")) {
@ -1260,22 +1349,6 @@ private:
std::string(data["raw_message"]);
ChanUpdate cu;
if (parse_chan_update(raw, cu)) {
/* Positive inbound fee: askrene cannot
* price it, so applying the update just
* loops. Exclude the channel instead. */
if (cu.has_inbound_fee
&& (cu.inbound_fee_base_msat > 0
|| cu.inbound_fee_proportional_millionths > 0)) {
actions.push_back(
Boss::Mod::AskreneLayer::
inform_channel_constrained(
*rpc,
Boss::Mod::AskreneLayer::
xrebalance_layer_name,
Ln::Scid(echan_str), edir,
Ln::Amount::msat(1)));
return;
}
actions.push_back(
Boss::Mod::AskreneLayer::
update_channel(