refactor: review pass over the auto-denylist ring

Three things the review turned up, no behaviour change:

Hold recomputed address.ToUInt128() three times where the original hoisted
it to a local. Restored.

Drain cleared _warnedFull on every iteration; it now clears once, after,
and only when something was actually retired. The store was inside the loop
whose cost this whole design exists to keep small.

Grow doubled without a ceiling, so a 324,449 cap reserved 524,288 ring slots
-- 4.8 MB that can never be used. Push only runs below the cap, so the ring
never needs more than maxEntries; growth is capped there. Progress is still
guaranteed: Grow is only reached when capacity == _ringCount, which is at
most maxEntries - 1 at that point, so the capped size is always larger.

Adds a test for that last one, since a cap that is not a power of two is
where a wrapped write would run off the end, and notes the new default in
the configuration table.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs

@@ -103,6 +103,27 @@ public class AutoDenylistTests
         Assert.Equal(0, AutoDenylist.RingCount);
     }
 
+    // The ring grows in doublings but is capped at maxEntries, which is not a power of two. Filling exactly
+    // to it must land on the last slot rather than off the end.
+    [Fact]
+    public void Ring_fills_exactly_to_a_non_power_of_two_cap()
+    {
+        Reset(maxEntries: 100);
+
+        for (var i = 0; i < 120; i++)
+        {
+            AutoDenylist.Hold(IPAddress.Parse($"198.51.100.{i}"), BanReasons.InvalidSeed, Now);
+        }
+
+        Assert.Equal(100, AutoDenylist.Count);
+        Assert.Equal(100, AutoDenylist.RingCount);
+
+        // And the whole ring still drains, so no slot was stranded by a wrapped write.
+        AutoDenylist.Drain(Now + DurationMs + 1);
+        Assert.Equal(0, AutoDenylist.Count);
+        Assert.Equal(0, AutoDenylist.RingCount);
+    }
+
     // Releasing leaves no ring record behind, so a re-detection is not retired by the old one.
     [Fact]
     public void Release_then_re_hold_is_not_retired_by_the_stale_record()

Projects/UOContent/Network/AutoDenylist/AutoDenylist.cs

@@ -96,11 +96,13 @@ public static class AutoDenylist
 
         Drain(nowTicks);
 
+        var key = address.ToUInt128();
+
         // Deliberately not refreshed: the first detection sets the expiry and later ones leave it alone.
-        // That is what keeps insertion order equal to expiry order, which is the whole reason the drain
-        // can stop at the first live record. A flooder whose hold lapses trips the rate limiter on its
-        // next attempt -- which runs before the connection filters -- and is held again immediately.
-        if (!_held.Add(address.ToUInt128()))
+        // That keeps insertion order equal to expiry order, which is why the drain can stop at the first
+        // live record. A flooder whose hold lapses trips the rate limiter on its next attempt -- which
+        // runs ahead of the connection filters -- and is held again.
+        if (!_held.Add(key))
         {
             return true;
         }
@@ -108,7 +110,7 @@ public static class AutoDenylist
         // Drain already reclaimed everything reclaimable, so being over now means genuinely full.
         if (_held.Count > _maxEntries)
         {
-            _held.Remove(address.ToUInt128());
+            _held.Remove(key);
 
             if (!_warnedFull)
             {
@@ -122,7 +124,7 @@ public static class AutoDenylist
             return false;
         }
 
-        Push(address.ToUInt128(), nowTicks + _durationMs);
+        Push(key, nowTicks + _durationMs);
         return true;
     }
 
@@ -130,8 +132,8 @@ public static class AutoDenylist
 
     /// <summary>
     /// The accept-path decision, split out so the policy can be tested without a clock. Drains first: the
-    /// expiry is not stored beside the membership, so there is nothing to expire on read and a lapsed hold
-    /// must be retired here rather than left to deny. Costs one array read when nothing has lapsed.
+    /// expiry lives in the ring, not beside the membership, so a lapsed hold has to be retired here rather
+    /// than expired on read. One array read when nothing has lapsed.
     /// </summary>
     internal static bool IsDenied(IPAddress address, long nowTicks)
     {
@@ -168,12 +170,18 @@ public static class AutoDenylist
     /// </summary>
     internal static void Drain(long nowTicks)
     {
+        var before = _ringCount;
+
         // Subtraction, never a direct compare: tick counts wrap. See dev-docs/tick-counts.md.
         while (_ringCount > 0 && _ringExpiry[_ringHead] - nowTicks <= 0)
         {
             _held.Remove(_ringKeys[_ringHead]);
             _ringHead = _ringHead + 1 == _ringKeys.Length ? 0 : _ringHead + 1;
             _ringCount--;
+        }
+
+        if (_ringCount != before)
+        {
             _warnedFull = false;
         }
     }
@@ -198,7 +206,9 @@ public static class AutoDenylist
 
     private static void Grow()
     {
-        var size = Math.Max(64, _ringKeys.Length * 2);
+        // Capped at the entry cap: Push only runs below it, so the ring never needs more, and doubling
+        // past it would reserve roughly twice the slots it can ever use.
+        var size = Math.Min(Math.Max(64, _ringKeys.Length * 2), _maxEntries);
         var keys = new UInt128[size];
         var expiry = new long[size];
 

dev-docs/ip-bans-and-allowlists.md

@@ -201,7 +201,7 @@ firewalled off. Shortening the 5s handshake window has been tried and broke real
 | `blocklist.json` | `enabled` (default `false`), `file`, `reloadInterval`, `reportHits`, `banDuration`, `promoteSuppression` |
 | `ip-allowlist.json` | `enabled` (default `false`), `files` (wildcards allowed), `reloadInterval` |
 | `login-allowlist.json` | `enabled`, `file`, `ttl`, `flushInterval`, `escalateAfterStrikes`, `strikeWindow` |
-| `auto-denylist.json` | `enabled`, `duration`, `maxEntries` |
+| `auto-denylist.json` | `enabled`, `duration`, `maxEntries` (default `324,449` — sized for the floods seen in practice; see the remark on the setting before raising it) |
 | `crowdsec.json` | `lapiUrl`, `machineId`, `password`, `origin`, `manualBanDuration`, `flushInterval`, `maxQueue` |
 | `firewall.json` | Admin-curated entries |