diff --git a/docs/fee-engine-algorithm.md b/docs/fee-engine-algorithm.md index 1e9c468b..8cdfe374 100644 --- a/docs/fee-engine-algorithm.md +++ b/docs/fee-engine-algorithm.md @@ -69,7 +69,7 @@ evaluation. | **Source** | `50` | Peer feeds *us* liquidity — keep cheap so the flow keeps coming. | | **Bidirectional** | `1500` | Balanced peer — mid fee. | | **Sink** | `2500` | Peer *drains* us — outbound liquidity here is scarce and valuable, charge a lot. | -| **Uncategorized** | `1500` | No signal yet (too young / not yet evaluated) — same as bidirectional. | +| **Uncategorized** | `1500` | **Unreachable from the job** since the eligibility gate (§7) skips uncategorized channels outright; it remains the `switch`'s default arm, so tuning `ROUTING_ENGINE_FEE_BASELINE_PPM_UNCATEGORIZED` has no effect in production. | | **Idle** | `1500` | Mature but below the volume gate — no flow to read. Defaults to the mid tier so behaviour matches `Uncategorized`; lower it to price idle channels down and try to attract flow. | Because steps scale with `p₀`, a Source channel moves in ~1 ppm increments while a Sink moves in @@ -168,6 +168,7 @@ per node: | `SatsAmount >= ROUTING_ENGINE_FEE_MIN_CHANNEL_SIZE_SATS` (10 M) | Fee moves on tiny channels aren't worth the write | | **Not** a source of a `Pending`/`InFlight` rebalance | Authority split — see below | | Has a `ChannelRoutingState` row for this node | No signal ⇒ no decision | +| `PeerFlowCategory != Uncategorized` | No committed flow verdict ⇒ nothing to price off, so the operator's fees are left alone. Filtered in `OptimizeNode` after the snapshot rather than on the `GetOpenChannels` query, because the category is per (channel, managed node) rather than per channel. `Idle` is a real verdict and stays eligible. | **Authority split with the rebalancer.** A channel currently being drained by a rebalance is excluded outright (`GetPendingInFlightSourceChannelIds`). Its balance is mid-flight, so the EMA is diff --git a/docs/rebalance-algorithm.md b/docs/rebalance-algorithm.md index ec3e007b..21715049 100644 --- a/docs/rebalance-algorithm.md +++ b/docs/rebalance-algorithm.md @@ -56,6 +56,11 @@ routing state, via [`RoutingEngineSnapshotService`](../src/Services/RoutingEngin | `Active` | `ListChannels` | Inactive channels take no part, either side. | | `SourceOptIn` | `Channel.IsAutoRebalanceEnabled` **and** not already being drained | Per-channel opt-in, ANDed with `GetPendingInFlightSourceChannelIds`. **Source side only** — see §8. | +`Classify` knows nothing about `PeerFlowCategory`. The policy is a single filter in +`AutoRebalanceJob`: channels whose `PeerFlowCategory` is `Uncategorized` are dropped before the +signals are built, so they are neither drained nor counted toward any destination peer. `Idle` is a +real verdict and stays eligible. The cost of that simplicity is the mixed-peer case in §8. + The split matters: **direction is decided on the EMA** so a single forward can't trigger a payment, while **sizing uses live balances** because those are the sats that actually move. @@ -72,7 +77,8 @@ Let `T` = `TargetLocalRatio`, `E` = `EmaLocalRatio`, `db` = `ROUTING_ENGINE_REBA ### Source side — per channel -Gated on `Active && SourceOptIn && base > 0`. +Gated on `Active && SourceOptIn && base > 0`. `Uncategorized` channels never reach `Classify` at +all, having been filtered in the job (§3). | Pool | Condition | How much it may give | |---|---|---| @@ -88,8 +94,12 @@ Channels grouped by `PeerPubKey`; `aggEma` and `aggTarget` are weighted by each | `Destinations` | `aggEma − aggTarget < −db` **and** deficit `> 0` | `round(Σ T·base) − peerLocal` — up to its **own target** | | `FallbackDestinations` | otherwise, if absorbable `> min` | `round(min(1, aggTarget + db) · peerBase) − peerLocal` — up to the **high edge of its deadband** | -Note the destination loop groups on `Active` alone — it does **not** filter on `SourceOptIn`. -Opting a channel out stops it being drained, not from receiving (§8). +The loop aggregates over every channel it receives — which excludes uncategorized ones, filtered +upstream (§3). On a peer with a mix of categorized and uncategorized channels that under-reports the +peer's liquidity; see §8. + +The loop groups on `Active` alone — it does **not** filter on `SourceOptIn`. Opting a +channel out stops it being drained, not from receiving (§8). ### Why the fallback pools stop at the deadband edge @@ -269,6 +279,22 @@ at a peer whose previous refill is still pending. Arguably a policy question rather than a bug — "don't drain this" and "don't send traffic here" are different intents — but the current toggle only implements the first. +### A mixed peer is over-refilled while one of its channels is uncategorized + +Uncategorized channels are filtered out in the job, so their balance is invisible to the destination +aggregate. On a peer where some channels are categorized and one is not, the peer reads as holding +less local than it really does, and can be classified as a real destination — with a deficit — while +actually sitting on target. + +The common trigger is opening a **second channel to an existing peer**: the new channel is +`Uncategorized` until it clears the age gate (~21 days) and is typically full, so for that window the +rebalancer can keep buying inbound for a peer that already has plenty. + +Pinned by `AutoRebalanceJobTests.Execute_MixedPeer_OverRefillsBecauseUncategorizedSiblingIsInvisible` +(7M sent where counting both channels would have capped it at 6M). Accepted in exchange for the +policy being one filter in the job rather than a rule threaded through `Classify`. Closing it means +keeping uncategorized channels in the signal list and gating the two sides separately. + ### A slow run can starve the cadence Because dispatch is awaited and serial (§7), a node whose payments time out can occupy the job for diff --git a/docs/routing-engine.md b/docs/routing-engine.md index 3fcdb953..90bb6754 100644 --- a/docs/routing-engine.md +++ b/docs/routing-engine.md @@ -134,6 +134,12 @@ cycles, and comes back the same way when volume returns. Because `ComputeCategory` no longer emits `Uncategorized`, it is now effectively **write-once**: a channel that leaves it never returns short of the row being reset. +`Uncategorized` is also **hands-off** for both actuators: the fee engine leaves the operator's fees +alone and the rebalancer will not drain the channel (nor refill a peer whose channels are all +uncategorized). `Idle` is a committed verdict, so it stays under management. See +[fee-engine-algorithm.md](fee-engine-algorithm.md) §7 and +[rebalance-algorithm.md](rebalance-algorithm.md) §3–4. + Both `Uncategorized` and `Idle` hold the target ratio at a neutral `0.5`, but for **different reasons** — the shared predicate is about the setpoint, not about flow: diff --git a/src/Jobs/AutoRebalanceJob.cs b/src/Jobs/AutoRebalanceJob.cs index 2698f7b5..a40b8535 100644 --- a/src/Jobs/AutoRebalanceJob.cs +++ b/src/Jobs/AutoRebalanceJob.cs @@ -172,17 +172,22 @@ private async Task RebalanceNode( return; } - var signals = owned.Select(oc => new ChannelSignal( - oc.DbChannel.Id, - oc.Lnd.ChanId, - oc.Lnd.RemotePubkey, - oc.Lnd.LocalBalance, - oc.Lnd.RemoteBalance, - oc.RoutingState.EmaLocalRatio, - oc.RoutingState.TargetLocalRatio, - oc.Lnd.Active, - // A channel is a fresh source only if opted in and not already being drained - oc.DbChannel.IsAutoRebalanceEnabled && !inFlightSourceChannelIds.Contains(oc.DbChannel.Id))) + // The engine acts only on channels it has a verdict for: an Uncategorized channel is never + // drained, and never contributes to a destination peer. Note this also hides its balance + // from that peer's aggregate — see the mixed-peer limitation in docs/rebalance-algorithm.md. + var signals = owned + .Where(oc => oc.RoutingState.PeerFlowCategory != PeerFlowCategory.Uncategorized) + .Select(oc => new ChannelSignal( + oc.DbChannel.Id, + oc.Lnd.ChanId, + oc.Lnd.RemotePubkey, + oc.Lnd.LocalBalance, + oc.Lnd.RemoteBalance, + oc.RoutingState.EmaLocalRatio, + oc.RoutingState.TargetLocalRatio, + oc.Lnd.Active, + // A channel is a fresh source only if opted in and not already being drained + oc.DbChannel.IsAutoRebalanceEnabled && !inFlightSourceChannelIds.Contains(oc.DbChannel.Id))) .ToList(); var tunables = RebalanceInitiatorTunables.FromConstants(node); diff --git a/src/Jobs/ChannelFeeOptimizerJob.cs b/src/Jobs/ChannelFeeOptimizerJob.cs index a4d79201..f20c0ba2 100644 --- a/src/Jobs/ChannelFeeOptimizerJob.cs +++ b/src/Jobs/ChannelFeeOptimizerJob.cs @@ -129,7 +129,12 @@ private async Task OptimizeNode( return; } - foreach (var oc in owned) + // We can't yet optimize channels that are still Uncategorized — they have no time verdict to price off + var eligible = owned + .Where(oc => oc.RoutingState.PeerFlowCategory != PeerFlowCategory.Uncategorized) + .ToList(); + + foreach (var oc in eligible) { try { diff --git a/test/NodeGuard.Tests/Jobs/AutoRebalanceJobTests.cs b/test/NodeGuard.Tests/Jobs/AutoRebalanceJobTests.cs index ddecc5d8..d12ea309 100644 --- a/test/NodeGuard.Tests/Jobs/AutoRebalanceJobTests.cs +++ b/test/NodeGuard.Tests/Jobs/AutoRebalanceJobTests.cs @@ -155,6 +155,122 @@ private void ArrangeRebalancePair(bool sourceOptedIn, bool sourceLiquidityFlag = } + [Fact] + public async Task Execute_PeerWithOnlyUncategorizedChannels_IsNeverRefilled() + { + // peerD's only channel is Uncategorized and deeply depleted (ema 0.10 vs target 0.50), + // so without the gate it is a textbook destination with an 8M deficit, funded by the + // categorized source 101. The peer is dropped whole instead, leaving nothing to pair. + ArrangeRebalancePair(sourceOptedIn: true); + _routingStateRepository.Setup(x => x.GetByManagedNodePubKey(NodePubKey)).ReturnsAsync(new List + { + new() { ChannelId = 101, ManagedNodePubKey = NodePubKey, ChanIdLnd = 1001, EmaLocalRatio = 0.75, TargetLocalRatio = 0.50, PeerFlowCategory = PeerFlowCategory.Source }, + new() { ChannelId = 102, ManagedNodePubKey = NodePubKey, ChanIdLnd = 1002, EmaLocalRatio = 0.10, TargetLocalRatio = 0.50, PeerFlowCategory = PeerFlowCategory.Uncategorized }, + }); + + await RoutingEngineSwitch.WithEngine(enabled: true, async () => + { + await BuildJob().Execute(Mock.Of()); + }); + + _rebalanceService.Verify(x => x.RebalanceAsync( + It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public async Task Execute_MixedPeer_OverRefillsBecauseUncategorizedSiblingIsInvisible() + { + // ACCEPTED LIMITATION, pinned deliberately — see docs/rebalance-algorithm.md §8. + // + // Both destination channels face the SAME peer, with equal 20M bases: 1002 is depleted + // (ema 0.10) and categorized, 1003 is full (ema 0.90) and Uncategorized. peerD therefore + // holds 20M of 40M — exactly on target 0.50 — and needs nothing. + // + // Because uncategorized channels are filtered out before Classify, 1003's 18M of local is + // invisible: peerD reads 0.10 vs 0.50, becomes a real destination with an invented 8M + // deficit, and gets refilled with the source's full 7M excess. Counting both channels + // would instead cap it at the deadband edge (0.65 x 40M - 20M = 6M). + ArrangeRebalancePair(sourceOptedIn: true); + + _channelRepository.Setup(x => x.GetOpenChannels()).ReturnsAsync(new List + { + new() + { + Id = 101, ChanId = 1001, Status = Channel.ChannelStatus.Open, + IsDynamicFeeEnabled = true, IsAutoRebalanceEnabled = true, + FundingTx = "txS", FundingTxOutputIndex = 0, + }, + new() + { + Id = 102, ChanId = 1002, Status = Channel.ChannelStatus.Open, + IsDynamicFeeEnabled = true, IsAutoRebalanceEnabled = false, + FundingTx = "txD", FundingTxOutputIndex = 0, + }, + new() + { + Id = 103, ChanId = 1003, Status = Channel.ChannelStatus.Open, + IsDynamicFeeEnabled = true, IsAutoRebalanceEnabled = false, + FundingTx = "txD2", FundingTxOutputIndex = 0, + }, + }); + + _routingStateRepository.Setup(x => x.GetByManagedNodePubKey(NodePubKey)).ReturnsAsync(new List + { + new() { ChannelId = 101, ManagedNodePubKey = NodePubKey, ChanIdLnd = 1001, EmaLocalRatio = 0.85, TargetLocalRatio = 0.50, PeerFlowCategory = PeerFlowCategory.Source }, + new() { ChannelId = 102, ManagedNodePubKey = NodePubKey, ChanIdLnd = 1002, EmaLocalRatio = 0.10, TargetLocalRatio = 0.50, PeerFlowCategory = PeerFlowCategory.Sink }, + new() { ChannelId = 103, ManagedNodePubKey = NodePubKey, ChanIdLnd = 1003, EmaLocalRatio = 0.90, TargetLocalRatio = 0.50, PeerFlowCategory = PeerFlowCategory.Uncategorized }, + }); + + _lightningClientService + .Setup(x => x.ListChannels(It.IsAny(), It.IsAny())) + .ReturnsAsync(new Lnrpc.ListChannelsResponse + { + Channels = + { + // Source excess = 17M - round(0.50 x 20M) = 7M, deliberately above the 6M the + // correct aggregate allows, so the destination cap is what binds. + new Lnrpc.Channel { ChanId = 1001, Capacity = 20_000_000, LocalBalance = 17_000_000, RemoteBalance = 3_000_000, Active = true, Initiator = true, RemotePubkey = "peerS" }, + new Lnrpc.Channel { ChanId = 1002, Capacity = 20_000_000, LocalBalance = 2_000_000, RemoteBalance = 18_000_000, Active = true, Initiator = true, RemotePubkey = "peerD" }, + new Lnrpc.Channel { ChanId = 1003, Capacity = 20_000_000, LocalBalance = 18_000_000, RemoteBalance = 2_000_000, Active = true, Initiator = true, RemotePubkey = "peerD" }, + }, + }); + + _lightningService.Setup(x => x.GetLocalOutboundFeeRatesPpmAsync(It.IsAny())) + .ReturnsAsync(new Dictionary { [1001] = 50, [1002] = 2500, [1003] = 2500 }); + + await RoutingEngineSwitch.WithEngine(enabled: true, async () => + { + await BuildJob().Execute(Mock.Of()); + }); + + _rebalanceService.Verify(x => x.RebalanceAsync( + It.Is(r => r.TargetPubkey == "peerD" && r.AmountSats == 7_000_000), + It.IsAny()), Times.Once, + "the uncategorized sibling's 18M is filtered out, so an on-target peer reads as starved"); + } + + [Fact] + public async Task Execute_UncategorizedSource_DispatchesNothing() + { + ArrangeRebalancePair(sourceOptedIn: true); + // Identical to Execute_DispatchesThePlannedRebalance except the too-local source has no + // committed verdict, which must take it out of the draining pool. Pins the job -> signal + // wiring: hardcoding Categorized:true would leave every other test green. + _routingStateRepository.Setup(x => x.GetByManagedNodePubKey(NodePubKey)).ReturnsAsync(new List + { + new() { ChannelId = 101, ManagedNodePubKey = NodePubKey, ChanIdLnd = 1001, EmaLocalRatio = 0.75, TargetLocalRatio = 0.50, PeerFlowCategory = PeerFlowCategory.Uncategorized }, + new() { ChannelId = 102, ManagedNodePubKey = NodePubKey, ChanIdLnd = 1002, EmaLocalRatio = 0.10, TargetLocalRatio = 0.50, PeerFlowCategory = PeerFlowCategory.Sink }, + }); + + await RoutingEngineSwitch.WithEngine(enabled: true, async () => + { + await BuildJob().Execute(Mock.Of()); + }); + + _rebalanceService.Verify(x => x.RebalanceAsync( + It.IsAny(), It.IsAny()), Times.Never); + } + [Fact] public async Task Execute_DispatchesThePlannedRebalance() { diff --git a/test/NodeGuard.Tests/Jobs/ChannelFeeOptimizerJobTests.cs b/test/NodeGuard.Tests/Jobs/ChannelFeeOptimizerJobTests.cs index 81257407..9e06b8f6 100644 --- a/test/NodeGuard.Tests/Jobs/ChannelFeeOptimizerJobTests.cs +++ b/test/NodeGuard.Tests/Jobs/ChannelFeeOptimizerJobTests.cs @@ -298,6 +298,44 @@ public async Task Execute_ChannelBelowMinSize_SkipsChannel() } } + [Fact] + public async Task Execute_UncategorizedChannel_SkipsWithoutPersistingFeeState() + { + var prevEnabled = Constants.ROUTING_ENGINE_ENABLED; + Constants.ROUTING_ENGINE_ENABLED = true; + try + { + var node = BuildNode(); + ArrangeSingleSinkChannel(node, inFlightRebalance: false); + // Same deviation the Sink arrangement uses (ema 0.40 vs target 0.50 — well outside the + // deadband, so it would otherwise Update), but with no committed flow verdict. + _routingStateRepository.Setup(x => x.GetByManagedNodePubKey(NodePubKey)).ReturnsAsync(new List + { + new() + { + ChannelId = ChannelDbId, + ManagedNodePubKey = NodePubKey, + ChanIdLnd = ChanId, + EmaLocalRatio = 0.40, + TargetLocalRatio = 0.50, + PeerFlowCategory = PeerFlowCategory.Uncategorized, + }, + }); + + await BuildJob().Execute(Mock.Of()); + + _lightningService.Verify(x => x.GetChannelFeePolicy(It.IsAny(), It.IsAny()), Times.Never); + VerifyNoFeeWrite(); + // No fee-state row either, so LastAppliedOutboundPpm stays null and the first managed + // cycle seeds from the category baseline. + _feeStateRepository.Verify(x => x.UpsertByChannelAndNode(It.IsAny()), Times.Never); + } + finally + { + Constants.ROUTING_ENGINE_ENABLED = prevEnabled; + } + } + [Fact] public async Task Execute_InsideDeadband_PersistsStateButDoesNotTouchLnd() {