fix(settings): state that rate-limit periods are milliseconds

Every shipped limit counts in milliseconds - 1000, 60000, 900000, 3600000 - but
nothing said so: RateLimit.period is a bare number, so period: 60 for a minute
became a 60 ms window, a thousand times tighter than intended, honoured without
complaint.

The unit is now stated on RateLimit and on each rateLimits block in
resources/default-settings.yaml, and the validator rejects what cannot be a
count of milliseconds: a fraction, a string, a non-number, or below 1.

Author: chappie-daemon

.changeset/rate-limit-period-milliseconds.md

@@ -0,0 +1,9 @@
+---
+"nostream": patch
+---
+
+fix(settings): state that rate-limit periods are milliseconds and reject a period that is not one
+
+Every shipped limit counts in milliseconds — `1000`, `60000`, `900000`, `3600000` — but nothing said so. `RateLimit.period` is a bare `number`, so an operator writing `period: 60` for a minute got a 60 ms window: a limit a thousand times tighter than intended, honoured without complaint, and nothing in review caught it.
+
+The unit is now stated on `RateLimit` and on every `rateLimits` block in `resources/default-settings.yaml`, and `validateRateLimitPeriods` rejects what cannot be a count of milliseconds: a fraction, a string, a non-number, or anything below one millisecond. A whole number is not enough to tell 60 from 60000, which is why the unit is documented where it is set.

resources/default-settings.yaml

@@ -216,10 +216,14 @@ eventStore:
 dvm:
   workers: []
 limits:
+  # Every `period` below is in milliseconds. A window written in seconds -
+  # period: 60 for a minute - is 1000x tighter and is not rejected, because
+  # 60 is a valid number of milliseconds. See RateLimit in @types/settings.
   # strategy selection configuration for rate limiting:
   rateLimiter:
     strategy: ewma
   invoice:
+    # period is milliseconds (60000 = one minute), never seconds
     rateLimits:
       - period: 60000
         rate: 12
@@ -232,11 +236,13 @@ limits:
   invite:
     # Per-pubkey limit on kind 28935 invite requests. Each mint writes a row to
     # invite_codes, so this is the main defence against a code flood.
+    # period is milliseconds (60000 = one minute), never seconds
     rateLimits:
       - description: 5 invite requests per hour per pubkey
         period: 3600000
         rate: 5
   admissionCheck:
+    # period is milliseconds (60000 = one minute), never seconds
     rateLimits:
       - description: 30 admission checks/min or 1 check every 2 seconds
         period: 60000
@@ -246,10 +252,12 @@ limits:
       - "10.10.10.1"
       - "::ffff:10.10.10.1"
   admin:
+    # period is milliseconds (60000 = one minute), never seconds
     rateLimits:
       - description: 30 admin requests/min
         period: 60000
         rate: 30
+    # period is milliseconds (60000 = one minute), never seconds
     loginRateLimits:
       - description: 10 login attempts per 15 minutes
         period: 900000
@@ -259,6 +267,7 @@ limits:
       - "10.10.10.1"
       - "::ffff:10.10.10.1"
   connection:
+    # period is milliseconds (60000 = one minute), never seconds
     rateLimits:
       - period: 1000
         rate: 12
@@ -323,6 +332,7 @@ limits:
             - 39
           - - 50
             - 9007199254740991
+    # period is milliseconds (60000 = one minute), never seconds
     rateLimits:
       - description: 6 events/min for event kinds 0, 3, 40 and 41
         kinds:
@@ -386,6 +396,7 @@ limits:
       maxLimit: 5000
       minPrefixLength: 4
   message:
+    # period is milliseconds (60000 = one minute), never seconds
     rateLimits:
       - description: 240 raw messages/min
         period: 60000

src/@types/settings.ts

@@ -24,6 +24,14 @@ export interface Network {
 
 export interface RateLimit {
   description?: string
+  /**
+   * Length of the window in **milliseconds**, never seconds.
+   *
+   * Every shipped limit uses this unit (`1000`, `60000`, `900000`, `3600000`),
+   * so `period: 60` is not "one minute" — it is a 60 ms window, a thousand
+   * times tighter than intended, and the relay honours it without complaint.
+   * The validator rejects anything that is not a positive whole number.
+   */
   period: number
   rate: number
 }

src/utils/settings-config.ts

@@ -595,8 +595,17 @@ const validateRateLimitPeriods = (
       issues.push({ path: `${pathPrefix}[${index}]`, message: 'rateLimits entry must be an object' })
       return
     }
-    if (!(entry.period > 0)) {
-      issues.push({ path: `${pathPrefix}[${index}].period`, message: 'period must be greater than 0' })
+    // Milliseconds, never seconds: every shipped limit counts in ms, and a
+    // window written in seconds (60 for a minute) is a 1000x tighter limit the
+    // relay would simply honour. A whole number is the only thing this can
+    // check — 60 is as valid as 60000 — so the unit is stated in the type and
+    // in resources/default-settings.yaml as well.
+    if (typeof entry.period !== 'number' || !Number.isFinite(entry.period)) {
+      issues.push({ path: `${pathPrefix}[${index}].period`, message: 'period must be a number of milliseconds' })
+    } else if (!Number.isInteger(entry.period)) {
+      issues.push({ path: `${pathPrefix}[${index}].period`, message: 'period must be a whole number of milliseconds' })
+    } else if (entry.period < 1) {
+      issues.push({ path: `${pathPrefix}[${index}].period`, message: 'period must be at least 1 millisecond' })
     }
   })
 }

test/unit/utils/settings-config.spec.ts

@@ -387,6 +387,34 @@ describe('settings-config', () => {
       expect(issues.some((issue) => issue.path.startsWith('limits.event.rateLimits'))).to.equal(false)
     })
 
+    // The window is milliseconds everywhere, but 60 is as plausible a number as
+    // 60000, so the unit cannot be inferred. What can be refused is a value that
+    // is not a whole count of milliseconds at all: a fraction (0.5 ms), a string
+    // from a hand-edited YAML, and a value that is not a number.
+    const invalidPeriodCases: { label: string; period: unknown }[] = [
+      { label: 'a fractional period', period: 0.5 },
+      { label: 'a string period', period: '60000' },
+      { label: 'a NaN period', period: Number.NaN },
+    ]
+
+    for (const { label, period } of invalidPeriodCases) {
+      it(`rejects ${label} on limits.event.rateLimits`, () => {
+        const settings = baseSettings()
+        settings.limits = { event: { rateLimits: [{ period, rate: 10 }] } }
+
+        const issues = validateSettings(settings)
+        expect(issues.some((issue) => issue.path === 'limits.event.rateLimits[0].period')).to.equal(true)
+      })
+    }
+
+    it('names milliseconds in the period message, so a rejected value is not read as seconds', () => {
+      const settings = baseSettings()
+      settings.limits = { event: { rateLimits: [{ period: 0.5, rate: 10 }] } }
+
+      const message = validateSettings(settings).find((issue) => issue.path === 'limits.event.rateLimits[0].period')?.message
+      expect(message).to.contain('milliseconds')
+    })
+
     it('rejects a null entry in limits.event.rateLimits without throwing', () => {
       const settings = baseSettings()
       settings.limits = { event: { rateLimits: [null] } }