mirror of
https://github.com/ZmnSCPxj/clboss.git
synced 2026-08-17 13:07:24 +02:00
8 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Ken Sedgwick
|
5939f7cb2f
|
XMoveFunds: wire the askrene call + plan response
Replaces the scaffold's echo-only stub with the real per-request
flow. clboss-xmovefunds now:
1. Manifests an idempotent startup-time create of the
persistent askrene layer named "clboss-xrebalance"
(created with persistent=true so it survives CLN restart
and accumulates probe knowledge across calls).
2. Per request:
a. Parses params (unchanged from the scaffold commit).
b. Waits for layer-ready.
c. Lists peer channels via listpeerchannels.
d. Generates a fresh transient layer name
clboss-xrebalance-tmp-<uuid> and creates it
(persistent=false).
e. Writes the per-direction masks to the transient layer:
every us->peer not listed in source_scid is disabled,
every peer->us not listed in dest_scid is disabled,
via askrene-update-channel enabled=false. Direction is
computed from BOLT 7 canonical id ordering.
f. Calls getroutes with source=self_id, destination=self_id,
layers=["auto.localchans", "clboss-xrebalance",
<transient>], amount_msat, maxfee_msat, final_cltv=14,
maxparts. Patched askrene (circular-askrene4 branch of
ksedgwic/lightning) interprets source=destination as
circular self-rebalance routing; stock CLN crashes here
with "child died with signal 6", which is the operator's
signal to apply the patch.
g. Removes the transient layer (best-effort -- swallows
errors on the cleanup path).
h. Replies with the original parsed plan plus the askrene
response embedded under "askrene". Status is "planned"
when execute=false, "ready" when execute=true (sendpay
path comes in a subsequent commit; for now execute=true
still falls through to the plan reply with the same
shape, just a different status string).
Architecture decisions
- Persistent xrebalance layer for accumulated knowledge;
transient layer per request for ephemeral masks. See
DEVSTATE/XREBALANCE-PLAN-2026-05-30.org section
"Two-layer pattern per getroutes call".
- AskreneLayer helpers reused -- they already take a layer
name parameter and live at the neutral Boss::Mod::AskreneLayer
namespace. This commit adds the constant
xrebalance_layer_name = "clboss-xrebalance" alongside the
existing clboss_layer_name = "clboss" so both subsystems
coexist without commingling their layer state.
- The patched-askrene requirement is intentional and
opt-in: clboss-xmovefunds is a manual RPC trigger, no
autonomous code path will exercise circular routing until
the periodic xrebalance (Layer 3) and JIT xrebalance
(Layer 4) code paths land. At that point we will need a
startup feature-detection probe; deferred until then.
|
||
|
Ken Sedgwick
|
e7604b0876
|
FundsMover: drop the per-Runner transient askrene layer
The transient "clboss-attempt-<uuid>" layer was created at the
start of each Runner::core_run, shared across all the Runner's
split-Attempters, and removed in Runner::finish. It held
three things:
- bias_channel writes (removed in the previous commit)
- update_channel policy refreshes parsed from BOLT 04 onion
errors
- inform_channel_constrained(amount=1) hard-exclude fallback
for non-0x1007 failures whose channel_update could not be
parsed
Two of those three (bias, max_msat=0 fallback) were
within-Runner-scoped on purpose: we did not want a single
failure to embed in the persistent clboss layer and bias every
future Runner against the channel. update_channel was scoped
to the transient layer for an analogous reason: full
fee/cltv/htlc-bounds overrides could freeze a stale view if
they survived too long.
Two related changes (this commit + the next) together justify
collapsing the transient layer into the persistent clboss
layer:
- bias is gone (previous commit), so half the
over-pessimism concern disappears.
- The next commit shortens the clboss layer's aging from 24h
to 1h, matching xpay's aging window. At a 1h horizon
update_channel overrides and max_msat=0 writes age out
quickly enough that they cannot lock the routing model
into a stale view.
With both of those, a single layer is a cleaner mental model:
the clboss layer is the only place FundsMover's learning lives,
aging is uniform, and we no longer need create_transient_layer /
remove_layer setup-teardown for every Runner.
(The follow-up commit also recomputes per-hop amount and delay
locally during sendpay-route translation, so the update_channel
writes finally do what they were supposed to do -- via our own
translation lookup rather than via askrene's getroutes, which
empirically does not honour layer cltv_expiry_delta overrides
when computing per-hop delay.)
Files
-----
Boss/Mod/AskreneLayer.{hpp,cpp}
Delete create_transient_layer and remove_layer wrappers and
their doc comments. Drop the no-longer-needed Uuid.hpp
include in the .cpp.
Boss/Mod/FundsMover/Attempter.{hpp,cpp}
Remove the runner_layer_name parameter from Attempter::run
and from the Impl constructor. Remove the
runner_layer_name member and its doc. At the two writes in
the 204 handler (update_channel parse-success branch and
inform_channel_constrained parse-failure fallback), switch
the layer name from runner_layer_name to
Boss::Mod::AskreneLayer::clboss_layer_name and drop the
surrounding !runner_layer_name.empty() guards. Rewrite the
"Non-NODE 204 failure feedback policy" doc comment to
reflect the single-layer story. Update the per-write log
prefix from "feedback: runner ..." to "feedback: clboss
..." so the layer name in the log matches reality.
At the getroutes call site, drop the conditional
`la.entry(runner_layer_name)` -- the clboss layer is already
in the layers array on the line above.
Boss/Mod/FundsMover/Runner.{hpp,cpp}
Remove the runner_layer_name member and its doc. Drop the
create_transient_layer call from core_run and the
remove_layer call from finish. Drop the runner_layer_name
argument from the Attempter::run invocation in attempt().
Observable effect
-----------------
Writes that previously went to the per-Runner transient layer
now go to the persistent clboss layer instead. Behaviour on
fresh restarts is unchanged in the immediate term because the
chan_update writes are made just before the next getroutes,
which sees the same writes regardless of which layer carried
them. Over multiple Runners the persistent layer accumulates
slightly more entries than before -- the next commit's 1h
aging change is the bound that keeps the layer from growing
unboundedly with policy-correction writes.
Subsequent commits
------------------
Stack position 2 of 4. Following commits:
3: shorten clboss layer aging from 24h to 1h; re-add
disable_node(self_id) per aging cycle so the self-loop
guard survives the shorter window.
4: regenerate per-hop amount and delay locally during
sendpay-route translation (the actual functional fix).
|
||
|
Ken Sedgwick
|
ba92b05565
|
FundsMover: drop the bias_channel hedge
After a sendpay 204 carrying a parsed channel_update payload,
the Attempter's 204 handler was issuing two writes to the
runner-scoped askrene layer: an update_channel with the
refreshed policy (cltv/fee/min/max) and a bias_channel with
relative=-1. The bias was an xpay-pattern hedge: even after
refreshing the published policy, gently nudge askrene to
prefer alternative routes in case the channel keeps failing
on the wire (fee-rounding interop, peers signing one policy
and enforcing another, etc).
In practice the hedge does not work for circular rebalance.
Empirically on prod1, bias accumulates to the askrene
param_s8_hundred saturation point of -100 within a single
Attempter's retry chain on a hot hub channel, and askrene
keeps choosing the same channel anyway, because bias is a
soft cost adder rather than an exclusion: when the failing
channel is the only viable path through some segment of the
graph (or is dramatically cheaper than alternatives), even
maxed-out negative bias does not displace it.
The hedge is also the wrong shape conceptually. For circular
rebalance the channel is not bad; our route was wrong about
how to use it. We should be correcting the data we feed
askrene, not nudging askrene to avoid the corrected data.
This change removes the bias_channel call from the 204
handler's parse-success branch, and removes the bias_channel
wrapper from AskreneLayer.{hpp,cpp} (it had no other
callers). The update_channel write stays; the fallback
inform_channel_constrained(amount=1) on parse-failure stays;
the 0x1007 persistent capacity write stays.
Files
-----
Boss/Mod/AskreneLayer.{hpp,cpp}
Delete the bias_channel wrapper and its doc comment.
Boss/Mod/FundsMover/Attempter.cpp
In the 204 handler's parse-success branch, delete the
bias_channel call and its accompanying log line. Preserve
the refreshed_policy = true; flag-set that follows.
Observable effect
-----------------
None expected. Bias was already saturated at -100 on the
hot channels (verified via askrene-listlayers on prod1) and
askrene was already ignoring it. Removing the call just
stops writing a value that was not changing behaviour.
Subsequent commits
------------------
This is the first of four in a stack that pivots FundsMover
from "let askrene interpret our learned policy via layer
overrides" to "interpret it ourselves during route
translation" (route values regenerated locally from
authoritative gossmap-or-layer policy with ceiling-rounded
proportional fees and direct cltv_expiry_delta application).
The other three commits drop the per-Runner transient layer,
shorten the persistent clboss layer's aging from 24h to 1h,
and add the local route-value recomputation that is the
actual functional fix.
|
||
|
Ken Sedgwick
|
510bd7f316
|
FundsMover: dedup self-exclude against clboss layer's disabled_nodes
The self-loop guard added in PR9 (commit
|
||
|
Ken Sedgwick
|
0b4bcb89bd
|
FundsMover: per-Runner channel_update + xpay-style negative bias
Mainnet observation (prod1, 19:53-19:54 UTC) showed the
update_channel-only design landing without effect for one
peer's chan: every sendpay 204 carrying a channel_update
parsed to the EXACT SAME values gossip already had
(base=0msat prop=1ppm cltv=80 min=1000msat max=19.8G msat),
so every layer write was a no-op refresh of correct data
and askrene kept picking the same chan because it remained
the cheapest viable route. The underlying problem looked
like a signed-vs-enforced policy mismatch -- likely
fee-rounding interop on a 1ppm channel where some
implementations floor and others ceiling the per-hop fee
calculation -- producing a 1msat-short FEE_INSUFFICIENT
loop our update_channel write cannot break.
xpay (cln/plugins/xpay/xpay.c:process_channel_update_from_
onion_error) handles this by pairing each update_channel
write with an `askrene-bias-channel bias=-1 relative=true`
call. The bias adds a small cost penalty to the channel in
askrene's MCF cost model without absolute-excluding it;
after a few repeats the penalty accumulates enough that
askrene routes around the chan even though its computed
fee is still the lowest in the topology.
This change mirrors that.
Files
-----
Boss/Mod/AskreneLayer.{hpp,cpp}
Add bias_channel(rpc, layer, scid, direction, bias,
relative, description=""). Thin wrapper around the
askrene-bias-channel RPC. bias is std::int8_t mapped to
askrene's param_s8_hundred (clamped to [-100,100] at the
server). description is optional (skipped from the JSON
if empty). Same direction-validity guard and silent-on-
RpcError degraded-learning posture as the other wrappers.
Boss/Mod/FundsMover/Attempter.cpp
Inside the 204 non-NODE branch, in the block guarded by
parse_chan_update success, add a second feedback write
after the existing update_channel call:
AskreneLayer::bias_channel(
rpc, runner_layer_name, echan, edir,
/*bias=*/-1, /*relative=*/true,
/*description=*/"FundsMover sendpay 204"
)
+ a "FundsMover[<tag>]: feedback: runner bias=-1 on
<scid>/<dir>" log line.
The bias write is conditional on the same gate as
update_channel (raw_message present, runner_layer_name
present, fail != 0x1007, parse_chan_update returned
true), so it never fires for cases where the
refreshed-policy path is skipped. In particular:
- 0x1007 (capacity TCF): bias not written; relies on
the existing max_msat=0 in runner layer +
max_msat=amount in persistent layer.
- NODE-level (failcode & 0x2000): bias not written;
relies on the existing persistent disable_node.
- non-NODE non-0x1007 with unparseable raw_message:
bias not written; falls back to max_msat=0 in
runner layer.
Accumulation bound
------------------
bias=-1 relative=true with one call per sendpay 204 means
each chan accumulates -N after N failures in the same
Runner. Askrene's param_s8_hundred clamps at -100. The
Runner's maximum_attempts is 30, so worst case is -30
across one Runner -- well within range and well past the
threshold at which askrene starts disfavouring the chan in
its cost model.
After the Runner ends, the per-Runner layer (and all bias
accumulation in it) is destroyed. Next Runner starts
fresh. No persistent staleness.
|
||
|
Ken Sedgwick
|
d96ad8093c
|
FundsMover: per-Runner private layer + xpay-style channel_update refresh
When askrene routes a payment and a forwarder rejects it at
runtime with a BOLT 04 onion failcode that carries a
channel_update payload (0x1007 TEMPORARY_CHANNEL_FAILURE,
0x100b AMOUNT_BELOW_MINIMUM, 0x100c FEE_INSUFFICIENT,
0x100d INCORRECT_CLTV_EXPIRY, 0x100e EXPIRY_TOO_SOON), the
forwarder is telling us "your idea of my fee/cltv/htlc-bounds
on this channel is stale; here is the current one signed by
me." Prior to this change FundsMover/Attempter extracted the
failcode and wrote max_msat=0 into a per-Attempter transient
layer (PR-B), but threw away the embedded channel_update
entirely.
Recent prod1 observation: in a 60-minute window across 9 log
files, sendpay 204 failcodes were 279 FEE_INSUFFICIENT + 198
INCORRECT_CLTV_EXPIRY + 15 TEMPORARY_CHANNEL_FAILURE. That is
97% of wire-level rejections carrying a channel_update we were
ignoring -- gossmap divergence from real-time policy is
rampant on the network and we had no path to learn it short
of waiting for the next gossip refresh.
This change mirrors what xpay does in
cln/plugins/xpay/xpay.c:process_channel_update_from_onion_
error: parse the channel_update out of raw_message and call
askrene-update-channel to override the gossmap policy for
that one channel-direction inside a transient askrene layer.
Subsequent getroutes calls that include the layer see the
corrected fee/cltv/min/max rather than stale gossip.
Layer architecture
------------------
Two layers consulted by every FundsMover getroutes call (plus
the built-in auto.localchans / auto.sourcefree):
clboss (persistent, name shared across all CLBOSS):
Long-term knowledge: NODE-level disables, and capacity
signals (max_msat=amount) for 0x1007 only. Channel-
policy refreshes do NOT go here -- askrene-update-channel
is a full per-field OVERRIDE that dominates the gossmap
until explicitly removed (confirmed by reading
askrene/layer.c gossmap_local_updatechan), so persisting
these writes would accumulate hashtable entries forever
and freeze our fee view even after gossip catches up.
clboss-attempt-<uuid> (per-Runner private):
Within-Runner discoveries. Holds two flavours of write:
(a) askrene-update-channel: refreshed channel-policy
parsed from BOLT 04 onion-error channel_update
payloads. Lets askrene reconsider the channel
with corrected fee/cltv in the next split-Attempter
(xpay-equivalent).
(b) askrene-inform-channel max_msat=0: absolute within-
Runner exclusion of a channel-direction. Used as a
sledgehammer when (a) is not applicable.
The layer is created by Runner::core_run and destroyed by
Runner::finish. Shared by every split-Attempter the Runner
spawns, so discoveries by one Attempter benefit the next.
Lives ~120s (Runner's maximum_time), then gone -- no
accumulation, no staleness across rebalance requests.
This replaces the prior per-Attempter transient layer. PR-B's
ratchet-by-1-msat retry-storm guard (max_msat=0 dominates the
persistent max_msat=amount) is preserved; the only change is
that the guard now spans all split-Attempters in a Runner
instead of one Attempter.
Why those layer-write rules in particular
------------------------------------------
The two per-Runner writes CONFLICT on the same channel-
direction -- askrene takes min across layers, so any
max_msat=0 dominates whatever update_channel said about
fee/cltv. Writing both for the same chan-dir defeats the
update_channel. Therefore the 204 handler picks one,
conditional on the failcode and whether we could parse the
embedded channel_update:
NODE-level (failcode & 0x2000):
persistent disable_node (unchanged)
non-NODE, NOT 0x1007, channel_update parseable:
per-Runner update_channel only
-- the failure was a policy/cltv/htlc-bound mismatch and
the channel_update tells us the correct values; let
askrene reconsider this channel.
non-NODE, NOT 0x1007, channel_update absent/unparseable:
per-Runner max_msat=0
-- older CLN, or malformed payload; we cannot refresh
policy, so excluding the channel for the remainder of
this Runner is the best we can do.
0x1007 (capacity TCF):
per-Runner max_msat=0 (sledgehammer; channel_update
payload is policy not capacity,
so refreshing fee/cltv would not
address "channel can't push this
amount right now")
AND persistent max_msat=amount (long-term capacity signal,
survives this Runner)
Files
-----
Boss/Mod/AskreneLayer.{hpp,cpp}
Add update_channel(rpc, layer, scid, direction, enabled,
htlc_min, htlc_max, base_fee, prop_fee, cltv_delta). Thin
wrapper around the askrene-update-channel RPC; same
direction-validity guard and silent-on-RpcError degraded-
learning posture as the existing inform_channel functions.
Doc-comment update on create_transient_layer: explicitly
note the typical caller is now Runner-scope (was Attempter
in PR-B), and add the channel_update-override rationale to
the "should NOT accumulate in persistent" reasoning.
Boss/Mod/FundsMover/Runner.{hpp,cpp}
New member runner_layer_name. core_run() creates the
private layer (via the existing AskreneLayer::create_
transient_layer helper, which handles RpcError gracefully
by returning ""). finish() removes the layer (no-op on
empty name). attempt() passes runner_layer_name through
to Attempter::run.
Boss/Mod/FundsMover/Attempter.{hpp,cpp}
- run() / Impl ctor / Impl member: thread runner_layer_name
through to the per-Attempter Impl.
- getroute(): include runner_layer_name in the "layers"
array of askrene-getroutes (skip-if-empty). REMOVED the
per-Attempter transient_layer_name from the array (and
the member, the create call, and the remove call) --
obsoleted by the per-Runner layer.
- attempt_tag(): now sourced from a standalone
Uuid::random() generated in the Impl ctor, stored as
attempt_uuid. Was previously sliced from the per-
Attempter transient layer name (which no longer exists).
- 204 handler: rewritten non-NODE branch with the conditional
write policy described above. raw_msg_hex is extracted
alongside the existing eidx/echan/edir/enode/fail.
Anonymous namespace (added in earlier pass, retained here):
ChanUpdate struct, read_be helper, parse_chan_update()
that consumes the raw_message hex from sendpay_failure
data, sniffs the failcode, skips the per-failcode
variable header (htlc_msat for 0x100b/0x100c,
cltv_expiry for 0x100d), reads the length-prefixed
channel_update blob, optionally skips the 2-byte 0x0102
type prefix (CLN-issued carries it; LND pre-v0.18 omits),
and pulls out the six BOLT 07 policy fields askrene-
update-channel takes.
Effects vs prior PR-B design
----------------------------
+ update_channel ACTUALLY WORKS. In the design I had two
passes ago, both update_channel (per-Runner) and
max_msat=0 (per-Attempter transient) were written for
the same chan-dir, and max_msat=0 dominated -- the
refreshed policy was a no-op. Now writes are mutually
exclusive, so when we successfully parse a channel_update
askrene can re-pick the chan with corrected fee.
+ Cross-Attempter learning. Sibling splits within a Runner
now see each other's discoveries via the shared layer
instead of independently re-exploring the same dead end.
- Sibling Attempter at smaller amount cannot probe a chan
that another Attempter failed at a larger amount with
0x1007. Per-Runner max_msat=0 wins over persistent
max_msat=amount for the duration of this Runner. Likely
a net win (saves a redundant probe) but is a behaviour
change worth noting.
Backward compatibility
----------------------
CLN versions without raw_message in sendpay_failure (older
than v23.05): parse_chan_update is never called; fallback
path writes max_msat=0 to the runner layer (same effective
behaviour as PR-B's per-Attempter transient).
CLN versions without askrene-update-channel (older than
v24.11): the RPC fails, AskreneLayer::update_channel swallows
the error silently. No write lands. We still write
max_msat=0 in the fallback path, so the runner layer still
has an absolute exclusion.
CLN versions without askrene-create-layer (older than v24.11):
Runner can't create the private layer, runner_layer_name
stays empty, getroutes layers array skips it, all per-Runner
writes are skipped. Persistent disable_node and persistent
max_msat=amount (for 0x1007) still happen. Degraded
learning but functional.
Verification
------------
cd clboss && make -j: clean build, -Werror passes, no new
warnings.
End-to-end verification will come from prod1 observation
after deploy. Two new log signals to watch for:
pattern: "FundsMover.*feedback: runner update_channel"
expected: one line per parseable non-0x1007 sendpay 204.
At current prod1 rates (279 FEE_INSUFFICIENT + 198 CLTV /
hour), this should be the dominant feedback line within
the first minute of FundsMover activity.
pattern: "FundsMover.*feedback: runner max_msat=0"
expected: 0x1007 failures + any non-parseable cases. At
~3% TCF rate, this should be much rarer than the
update_channel signal.
The hoped-for behaviour change: subsequent split-Attempters
in the same Runner picking different (or sometimes the
same-with-corrected-fee) routes, leading to actual
transferred>0 outcomes in DONE lines. If the dominant cause
of 0-msat-transferred Runners on prod1 was gossip-stale fee
mismatches, this should move the needle. If not, we are
seeing a different problem (budget-vs-network-policy
mismatch, addressed separately by clboss-max-rebalance-fee-
ppm tuning).
Upstream note
-------------
The channel_update parsing duplicates code that already
exists in CLN's xpay. Long-term fix is to ask CLN to surface
a helper RPC (e.g. decodechannelupdate or parseonionerror)
that callers like CLBOSS could invoke instead of re-
implementing BOLT 07 wire parsing. Filed as a TODO; not
blocking this change.
|
||
|
Ken Sedgwick
|
c5cecd2869
|
FundsMover: stop writing fee-skew failures into the liquidity-shaped clboss layer
Most non-NODE sendpay onion failures (FEE_INSUFFICIENT, INCORRECT_CLTV_ EXPIRY, EXPIRY_TOO_SOON, AMOUNT_BELOW_MINIMUM, ...) are NOT capacity problems. They are gossip-staleness problems: the peer's channel_update changed between when we last synced gossip and when we tried to route, and the HTLC's fee/CLTV/min allocation no longer matches the peer's current parameters. |
||
|
Ken Sedgwick
|
17ba012f9b
|
AskreneLayer: extract shared helpers for clboss layer writes
Move the askrene-inform-channel and askrene-disable-node helpers from Boss::Mod::FundsMover::Attempter::Impl (where they were private member functions) into a new shared module Boss::Mod::AskreneLayer. Lift the "clboss" layer name from a FundsMover-local constant into a shared constant. Pure refactor, no behavioral change. Existing tests still pass (79/79). Done now because PR4 (ActiveProber feeding the same layer) will be the second producer; sharing avoids duplicating the JSON construction, the layer-name string, and the silent-RpcError-catch logic. The new module exposes three free functions: - inform_channel_constrained(rpc, layer, scid, dir, amount): same shape and semantics as Attempter's prior private method. - inform_channel_succeeded(rpc, layer, scid, dir, amount): new variant emitting inform=succeeded instead of inform=constrained. Not used by FundsMover (sendpay failures only carry negative signals). Added in this commit because the implementation is one line over the shared internal helper, and PR4's open-question may have ActiveProber call it on probe-success to record positive lower-bound observations on probed channels. - disable_node(rpc, layer, node_id): same shape and semantics as Attempter's prior private method. All three functions silently swallow RpcError, matching the prior behavior in Attempter. The rationale: if the layer is unavailable (e.g. CLN < v24.11 where askrene layers do not exist), the call gracefully degrades to no-op rather than crashing the caller. Better degraded learning than crashed plugin. The layer name "clboss" follows the xpay convention: persistent shared-knowledge layer = owning plugin name. All CLBOSS-internal writers (FundsMover today, ActiveProber in PR4, possibly others later) target the same single layer so the accumulated knowledge benefits every downstream getroutes call. |