From 5d881338371807ca75a27aa158a9ecfd1e131963 Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Wed, 24 Jun 2026 14:00:21 -0400 Subject: [PATCH 1/3] feat(config): per-network shared-value inheritance + resolve #570 review findings Per-network env vars now resolve `_` -> unprefixed `` -> default, so shared tuning is set once and overridden per network only where it differs. A single source of truth (network-fields.ts) classifies chain-specific vars (credentials, endpoints, blocklists) that never inherit, and drives both the loader resolver and the Joi schema. Validation: shared unprefixed keys are validated against the same rules as their prefixed overrides (not waved through by allowUnknown). Per-network defaults moved out of Joi into the loader so NestJS-injected prefixed defaults can't shadow a shared value. Also resolves prior-PR review findings on this branch: - TARGET_ (per-network override) +# -> (unprefixed shared value, applies to all active networks) +# -> default +# +# So set a shared value once and override only where a network differs: +# DEAL_JOB_TIMEOUT_SECONDS=360 # shared by both networks +# MAINNET_DEALS_PER_SP_PER_HOUR=1 # mainnet-only override +# +# Chain-specific variables never inherit the unprefixed slot and MUST be +# prefixed: WALLET_ADDRESS, WALLET_PRIVATE_KEY, SESSION_KEY_PRIVATE_KEY, RPC_URL, +# PDP_SUBGRAPH_ENDPOINT, DEALBOT_DATASET_VERSION, BLOCKED_SP_IDS, BLOCKED_SP_ADDRESSES. +# +# Process globals (database, HTTP ports, ClickHouse, pg-boss) have no per-network +# form. Variables for INACTIVE networks are ignored; only active networks are +# validated at startup. # ============================================================================= # ----------------------------------------------------------------------------- diff --git a/apps/backend/src/config/config-pipeline.spec.ts b/apps/backend/src/config/config-pipeline.spec.ts new file mode 100644 index 00000000..d7c49ee0 --- /dev/null +++ b/apps/backend/src/config/config-pipeline.spec.ts @@ -0,0 +1,101 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { validateConfig } from "./env.schema.js"; +import { loadConfig } from "./loader.js"; + +/** + * End-to-end of the config pipeline as NestJS runs it: `validateConfig` first + * (legacy/rename translation + Joi), then `loadConfig` reading `process.env`. + * Both touch the real `process.env`, so each case snapshots and restores it. + */ +const TOUCHED = [ + "NETWORK", + "NETWORKS", + "DATABASE_HOST", + "DATABASE_USER", + "DATABASE_PASSWORD", + "DATABASE_NAME", + "WALLET_ADDRESS", + "SESSION_KEY_PRIVATE_KEY", + "WALLET_PRIVATE_KEY", + "RPC_URL", + "DEALBOT_MAINTENANCE_WINDOWS_UTC", + "MAINTENANCE_WINDOWS_UTC", + "DEAL_JOB_TIMEOUT_SECONDS", + "CALIBRATION_WALLET_ADDRESS", + "CALIBRATION_SESSION_KEY_PRIVATE_KEY", + "CALIBRATION_WALLET_PRIVATE_KEY", + "CALIBRATION_RPC_URL", + "CALIBRATION_MAINTENANCE_WINDOWS_UTC", + "MAINNET_WALLET_PRIVATE_KEY", + "MAINNET_WALLET_ADDRESS", + "MAINNET_DEAL_JOB_TIMEOUT_SECONDS", +]; + +const snapshot: Record = {}; + +beforeEach(() => { + for (const key of TOUCHED) { + snapshot[key] = process.env[key]; + delete process.env[key]; + } + process.env.DATABASE_HOST = "localhost"; + process.env.DATABASE_USER = "test"; + process.env.DATABASE_PASSWORD = "test"; + process.env.DATABASE_NAME = "test"; +}); + +afterEach(() => { + for (const key of TOUCHED) { + if (snapshot[key] === undefined) delete process.env[key]; + else process.env[key] = snapshot[key]; + } +}); + +describe("config pipeline (validateConfig -> loadConfig)", () => { + it("rolls a legacy single-network deployment forward, including renamed vars", () => { + // Staging-style: legacy NETWORK + session-key mode + an old maintenance name. + process.env.NETWORK = "calibration"; + process.env.WALLET_ADDRESS = "0xabc"; + process.env.SESSION_KEY_PRIVATE_KEY = "0xsession"; + process.env.RPC_URL = "http://erpc.local/main/evm/314159"; + process.env.DEALBOT_MAINTENANCE_WINDOWS_UTC = "01:00,13:00"; + + expect(() => validateConfig(process.env as Record)).not.toThrow(); + const cfg = loadConfig(); + + expect(cfg.activeNetworks).toEqual(["calibration"]); + expect(cfg.networks.calibration.rpcUrl).toBe("http://erpc.local/main/evm/314159"); + expect(cfg.networks.calibration.maintenanceWindowsUtc).toEqual(["01:00", "13:00"]); + if ("sessionKeyPrivateKey" in cfg.networks.calibration) { + expect(cfg.networks.calibration.sessionKeyPrivateKey).toBe("0xsession"); + } else { + throw new Error("expected session-key variant"); + } + }); + + it("validates and loads a shared value with a per-network override across both networks", () => { + process.env.NETWORKS = "calibration,mainnet"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + process.env.CALIBRATION_WALLET_ADDRESS = "0xcaladdr"; + process.env.MAINNET_WALLET_PRIVATE_KEY = "0xmain"; + process.env.MAINNET_WALLET_ADDRESS = "0xmainaddr"; + process.env.DEAL_JOB_TIMEOUT_SECONDS = "500"; // shared, validated once + process.env.MAINNET_DEAL_JOB_TIMEOUT_SECONDS = "900"; // override + + expect(() => validateConfig(process.env as Record)).not.toThrow(); + const cfg = loadConfig(); + + expect(cfg.activeNetworks).toEqual(["calibration", "mainnet"]); + expect(cfg.networks.calibration.dealJobTimeoutSeconds).toBe(500); + expect(cfg.networks.mainnet.dealJobTimeoutSeconds).toBe(900); + }); + + it("rejects an out-of-range shared value before load", () => { + process.env.NETWORKS = "calibration"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + process.env.CALIBRATION_WALLET_ADDRESS = "0xcaladdr"; + process.env.DEAL_JOB_TIMEOUT_SECONDS = "1"; // below min(120) + + expect(() => validateConfig(process.env as Record)).toThrow(/DEAL_JOB_TIMEOUT_SECONDS/); + }); +}); diff --git a/apps/backend/src/config/env.helpers.spec.ts b/apps/backend/src/config/env.helpers.spec.ts new file mode 100644 index 00000000..49d27ae9 --- /dev/null +++ b/apps/backend/src/config/env.helpers.spec.ts @@ -0,0 +1,36 @@ +import { describe, expect, it } from "vitest"; +import { coerceBoolean, coerceFloat, coerceNumber } from "./env.helpers.js"; + +describe("coerceBoolean", () => { + it("returns the fallback when the value is absent", () => { + expect(coerceBoolean(undefined, true)).toBe(true); + expect(coerceBoolean(undefined, false)).toBe(false); + }); + + it("parses case-insensitive true/false", () => { + expect(coerceBoolean("true", false)).toBe(true); + expect(coerceBoolean("TRUE", false)).toBe(true); + expect(coerceBoolean("False", true)).toBe(false); + expect(coerceBoolean(" false ", true)).toBe(false); + }); + + it("does NOT treat 0/no/off as true (the old footgun)", () => { + // Old behaviour returned `value !== "false"`, so these were all true. + expect(coerceBoolean("0", true)).toBe(true); // unrecognised -> fallback, not forced true + expect(coerceBoolean("0", false)).toBe(false); + expect(coerceBoolean("no", false)).toBe(false); + }); +}); + +describe("coerceNumber / coerceFloat", () => { + it("parses numeric strings and floats", () => { + expect(coerceNumber("42", 0)).toBe(42); + expect(coerceFloat("0.25", 0)).toBe(0.25); + }); + + it("returns the fallback for empty/absent values", () => { + expect(coerceNumber(undefined, 7)).toBe(7); + expect(coerceNumber("", 7)).toBe(7); + expect(coerceFloat(undefined, 1.5)).toBe(1.5); + }); +}); diff --git a/apps/backend/src/config/env.helpers.ts b/apps/backend/src/config/env.helpers.ts index 6718de13..413d75e7 100644 --- a/apps/backend/src/config/env.helpers.ts +++ b/apps/backend/src/config/env.helpers.ts @@ -1,24 +1,41 @@ /** * Raw environment variable accessors. * - * These are purely mechanical: they read a key from `process.env` (or any - * `NodeJS.ProcessEnv` map) and coerce it to the requested primitive type, - * falling back to a default when the key is absent or empty. + * Two layers: + * - `coerce*` take an already-resolved string value and coerce it to a + * primitive, falling back to a default when absent/empty. The per-network + * loader uses these because it resolves the value itself (override → shared). + * - `get*Env` read a single key from a `NodeJS.ProcessEnv` map and coerce it. + * Global (non per-network) config uses these. */ -export const getStringEnv = (env: NodeJS.ProcessEnv, key: string, fallback: string): string => env[key] || fallback; +export const coerceNumber = (value: string | undefined, fallback: number): number => + value ? Number.parseInt(value, 10) : fallback; -export const getNumberEnv = (env: NodeJS.ProcessEnv, key: string, fallback: number): number => { - const value = env[key]; - return value ? Number.parseInt(value, 10) : fallback; -}; +export const coerceFloat = (value: string | undefined, fallback: number): number => + value ? Number.parseFloat(value) : fallback; -export const getFloatEnv = (env: NodeJS.ProcessEnv, key: string, fallback: number): number => { - const value = env[key]; - return value ? Number.parseFloat(value) : fallback; +/** + * Coerces a boolean env value, matching Joi's case-insensitive `true`/`false`. + * Anything unrecognized returns the fallback (Joi rejects such values at boot + * for registered keys, so the loader never sees them there). Notably `"0"`, + * `"no"`, and `"False"` are NOT treated as `true`. + */ +export const coerceBoolean = (value: string | undefined, fallback: boolean): boolean => { + if (value === undefined) return fallback; + const normalized = value.trim().toLowerCase(); + if (normalized === "true") return true; + if (normalized === "false") return false; + return fallback; }; -export const getBooleanEnv = (env: NodeJS.ProcessEnv, key: string, fallback: boolean): boolean => { - const value = env[key]; - return value !== undefined ? value !== "false" : fallback; -}; +export const getStringEnv = (env: NodeJS.ProcessEnv, key: string, fallback: string): string => env[key] || fallback; + +export const getNumberEnv = (env: NodeJS.ProcessEnv, key: string, fallback: number): number => + coerceNumber(env[key], fallback); + +export const getFloatEnv = (env: NodeJS.ProcessEnv, key: string, fallback: number): number => + coerceFloat(env[key], fallback); + +export const getBooleanEnv = (env: NodeJS.ProcessEnv, key: string, fallback: boolean): boolean => + coerceBoolean(env[key], fallback); diff --git a/apps/backend/src/config/env.schema.spec.ts b/apps/backend/src/config/env.schema.spec.ts index 7c276b99..503fe621 100644 --- a/apps/backend/src/config/env.schema.spec.ts +++ b/apps/backend/src/config/env.schema.spec.ts @@ -506,4 +506,70 @@ describe("createConfigValidationSchema", () => { expect(a).not.toBe(b); }); }); + + describe("shared (unprefixed) override validation", () => { + it("accepts an in-range shared value", () => { + const { error } = validate(schemaFor("calibration"), { + ...baseEnv, + NETWORKS: "calibration", + ...withWalletKey("CALIBRATION"), + DEALS_PER_SP_PER_HOUR: 4, + }); + expect(error).toBeUndefined(); + }); + + it("rejects an out-of-range shared value (not waved through by allowUnknown)", () => { + const { error } = validate(schemaFor("calibration"), { + ...baseEnv, + NETWORKS: "calibration", + ...withWalletKey("CALIBRATION"), + DEALS_PER_SP_PER_HOUR: 999, + }); + expect(error).toBeDefined(); + expect(error?.message).toMatch(/DEALS_PER_SP_PER_HOUR/); + }); + + it("rejects an invalid shared maintenance-window format", () => { + const { error } = validate(schemaFor("calibration"), { + ...baseEnv, + NETWORKS: "calibration", + ...withWalletKey("CALIBRATION"), + MAINTENANCE_WINDOWS_UTC: "not-a-time", + }); + expect(error).toBeDefined(); + }); + + it("does not register a bare key for a chain-specific var", () => { + // BLOCKED_SP_IDS is chain-specific; an unprefixed value is unknown to the + // schema and only allowed because allowUnknown is on (never validated as shared). + const { error } = validate(schemaFor("calibration"), { + ...baseEnv, + NETWORKS: "calibration", + ...withWalletKey("CALIBRATION"), + BLOCKED_SP_IDS: "1,2,3", + }); + expect(error).toBeUndefined(); + }); + }); + + describe("NETWORKS validation", () => { + it("rejects a present-but-empty NETWORKS (whitespace/commas)", () => { + const { error } = validate(schemaFor("calibration"), { + ...baseEnv, + NETWORKS: " , ,", + ...withWalletKey("CALIBRATION"), + }); + expect(error).toBeDefined(); + expect(error?.message).toMatch(/NETWORKS must list at least one supported network/); + }); + + it("rejects an unsupported network name", () => { + const { error } = validate(schemaFor("calibration"), { + ...baseEnv, + NETWORKS: "calibration,moonbase", + ...withWalletKey("CALIBRATION"), + }); + expect(error).toBeDefined(); + }); + }); }); diff --git a/apps/backend/src/config/env.schema.ts b/apps/backend/src/config/env.schema.ts index 0f5574f0..aa87b6c1 100644 --- a/apps/backend/src/config/env.schema.ts +++ b/apps/backend/src/config/env.schema.ts @@ -25,6 +25,7 @@ import type { Network } from "../common/types.js"; import { NETWORK_ENV_PREFIXES } from "./constants.js"; import { parseActiveNetworks } from "./env.parsers.js"; import { applyLegacyEnvCompat, logLegacyEnvCompatResult } from "./legacy-env-compat.js"; +import { INHERITABLE_NETWORK_VARS, type PerNetworkVar } from "./network-fields.js"; // --------------------------------------------------------------------------- // Custom Joi validators @@ -36,15 +37,23 @@ const validateNetworksEnv = (value: string, helpers: Joi.CustomHelpers) => { .map((s) => s.trim()) .filter((s) => s.length > 0); + if (parts.length === 0) { + // A present-but-empty NETWORKS (whitespace/commas) would otherwise load zero + // active networks and boot an idle process. Reject it explicitly. + return helpers.message({ + custom: `NETWORKS must list at least one supported network: ${SUPPORTED_NETWORKS.join(", ")}.`, + }); + } + for (const part of parts) { if (!SUPPORTED_NETWORKS.includes(part as Network)) { - return helpers.error("any.invalid", { - message: `Invalid network "${part}". Supported: ${SUPPORTED_NETWORKS.join(", ")}.`, + return helpers.message({ + custom: `Invalid network "${part}". Supported: ${SUPPORTED_NETWORKS.join(", ")}.`, }); } } - return parts.length === 0 ? SUPPORTED_NETWORKS[0] : value; + return value; }; const validateMaintenanceWindowsEnv = (value: string, helpers: Joi.CustomHelpers) => { @@ -135,79 +144,75 @@ export const retrievalEnvSchema = { // --------------------------------------------------------------------------- /** - * Returns the Joi field rules for a single network prefix (e.g. `"CALIBRATION"`). - * Fields are defined as optional here; active-network enforcement is handled - * in `createConfigValidationSchema`. + * Validation rules for each per-network var, keyed by base name (no prefix). + * + * These carry NO `.default()` on purpose. NestJS assigns Joi-applied defaults + * back into `process.env` for absent keys; an injected prefixed default + * (`CALIBRATION_DEAL_JOB_TIMEOUT_SECONDS`) would then shadow an unprefixed + * shared value in the loader's resolver and break inheritance. All per-network + * defaults live in the loader (`networkDefaults`) instead. Cross-field checks + * (e.g. TARGET < MAX) also live in the loader, post-resolution. */ -export const createPerNetworkEnvSchema = (prefix: Uppercase | "") => { - const k = (key: string) => `${prefix}_${key}`; - return { - [k("WALLET_ADDRESS")]: Joi.string().required(), - [k("WALLET_PRIVATE_KEY")]: Joi.string().optional().empty(""), - [k("SESSION_KEY_PRIVATE_KEY")]: Joi.string().optional().empty(""), - [k("RPC_URL")]: Joi.string() - .uri({ scheme: ["http", "https"] }) - .optional() - .allow(""), - [k("RPC_REQUEST_TIMEOUT_MS")]: Joi.number().integer().min(1000).default(30000), - [k("PDP_SUBGRAPH_ENDPOINT")]: Joi.string().uri().optional().allow(""), - [k("CHECK_DATASET_CREATION_FEES")]: Joi.boolean().optional(), - [k("USE_ONLY_APPROVED_PROVIDERS")]: Joi.boolean().optional(), - [k("DEALBOT_DATASET_VERSION")]: Joi.string().optional(), - [k("MIN_NUM_DATASETS_FOR_CHECKS")]: Joi.number().integer().min(1).optional(), - [k("DEALS_PER_SP_PER_HOUR")]: Joi.number().min(0.001).max(20).optional(), - [k("RETRIEVALS_PER_SP_PER_HOUR")]: Joi.number().min(0.001).max(20).optional(), - [k("DATASET_CREATIONS_PER_SP_PER_HOUR")]: Joi.number().min(0.001).max(20).optional(), - [k("DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR")]: Joi.number().min(0.001).max(20).default(1), - // Enables the data_set_lifecycle_check canary job. Left optional so the loader can - // apply the network-dependent default (enabled off mainnet, disabled on mainnet). - [k("DATASET_LIFECYCLE_CHECK_ENABLED")]: Joi.boolean().optional(), - [k("DEAL_JOB_TIMEOUT_SECONDS")]: Joi.number().min(120).default(360), - [k("RETRIEVAL_JOB_TIMEOUT_SECONDS")]: Joi.number().min(60).default(60), - [k("DATA_SET_CREATION_JOB_TIMEOUT_SECONDS")]: Joi.number().min(60).default(300), - [k("DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS")]: Joi.number().min(60).default(600), - [k("DATA_RETENTION_POLL_INTERVAL_SECONDS")]: Joi.number().optional(), - [k("PROVIDERS_REFRESH_INTERVAL_SECONDS")]: Joi.number().optional(), - [k("MAINTENANCE_WINDOWS_UTC")]: Joi.string().default("07:00,22:00").custom(validateMaintenanceWindowsEnv), - [k("MAINTENANCE_WINDOW_MINUTES")]: Joi.number().min(20).max(360).default(20), - [k("BLOCKED_SP_IDS")]: Joi.string().optional().allow(""), - [k("BLOCKED_SP_ADDRESSES")]: Joi.string().optional().allow(""), - - [k("PIECE_CLEANUP_PER_SP_PER_HOUR")]: Joi.number() - .min(0.001) - .max(20) - .default(1 / 24), - [k("MAX_PIECE_CLEANUP_RUNTIME_SECONDS")]: Joi.number().min(60).default(300), - [k("MAX_DATASET_STORAGE_SIZE_BYTES")]: Joi.number() - .integer() - .min(1) - .default(24 * 1024 * 1024 * 1024), - [k("TARGET_DATASET_STORAGE_SIZE_BYTES")]: Joi.number() - .integer() - .min(1) - .default(20 * 1024 * 1024 * 1024) // 20 GiB per SP - .custom((value, helpers) => { - const max = helpers.state.ancestors?.[0]?.MAX_DATASET_STORAGE_SIZE_BYTES; - if (max != null && value >= max) { - return helpers.error("any.invalid", { - message: `TARGET_DATASET_STORAGE_SIZE_BYTES (${value}) must be less than MAX_DATASET_STORAGE_SIZE_BYTES (${max})`, - }); - } - return value; - }, "target < max validation"), - - [k("PULL_CHECKS_PER_SP_PER_HOUR")]: Joi.number().min(0.001).max(20).default(1), - [k("PULL_CHECK_JOB_TIMEOUT_SECONDS")]: Joi.number().min(60).default(300), - [k("PULL_CHECK_POLL_INTERVAL_SECONDS")]: Joi.number().min(1).default(2), - [k("PULL_CHECK_PIECE_SIZE_BYTES")]: Joi.number() - .integer() - .min(1024) - .default(10 * 1024 * 1024), - [k("PULL_PIECE_CLEANUP_INTERVAL_SECONDS")]: Joi.number() - .integer() - .min(3600) - .default(7 * 24 * 3600), - }; +const perNetworkFieldRules = (): Record => ({ + WALLET_ADDRESS: Joi.string().required(), + WALLET_PRIVATE_KEY: Joi.string().optional().empty(""), + SESSION_KEY_PRIVATE_KEY: Joi.string().optional().empty(""), + RPC_URL: Joi.string() + .uri({ scheme: ["http", "https"] }) + .optional() + .allow(""), + RPC_REQUEST_TIMEOUT_MS: Joi.number().integer().min(1000).optional(), + PDP_SUBGRAPH_ENDPOINT: Joi.string().uri().optional().allow(""), + CHECK_DATASET_CREATION_FEES: Joi.boolean().optional(), + USE_ONLY_APPROVED_PROVIDERS: Joi.boolean().optional(), + DEALBOT_DATASET_VERSION: Joi.string().optional(), + MIN_NUM_DATASETS_FOR_CHECKS: Joi.number().integer().min(1).optional(), + DEALS_PER_SP_PER_HOUR: Joi.number().min(0.001).max(20).optional(), + RETRIEVALS_PER_SP_PER_HOUR: Joi.number().min(0.001).max(20).optional(), + DATASET_CREATIONS_PER_SP_PER_HOUR: Joi.number().min(0.001).max(20).optional(), + DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR: Joi.number().min(0.001).max(20).optional(), + DATASET_LIFECYCLE_CHECK_ENABLED: Joi.boolean().optional(), + DEAL_JOB_TIMEOUT_SECONDS: Joi.number().min(120).optional(), + RETRIEVAL_JOB_TIMEOUT_SECONDS: Joi.number().min(60).optional(), + DATA_SET_CREATION_JOB_TIMEOUT_SECONDS: Joi.number().min(60).optional(), + DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS: Joi.number().min(60).optional(), + DATA_RETENTION_POLL_INTERVAL_SECONDS: Joi.number().optional(), + PROVIDERS_REFRESH_INTERVAL_SECONDS: Joi.number().optional(), + MAINTENANCE_WINDOWS_UTC: Joi.string().custom(validateMaintenanceWindowsEnv).optional(), + MAINTENANCE_WINDOW_MINUTES: Joi.number().min(20).max(360).optional(), + BLOCKED_SP_IDS: Joi.string().optional().allow(""), + BLOCKED_SP_ADDRESSES: Joi.string().optional().allow(""), + PIECE_CLEANUP_PER_SP_PER_HOUR: Joi.number().min(0.001).max(20).optional(), + MAX_PIECE_CLEANUP_RUNTIME_SECONDS: Joi.number().min(60).optional(), + MAX_DATASET_STORAGE_SIZE_BYTES: Joi.number().integer().min(1).optional(), + TARGET_DATASET_STORAGE_SIZE_BYTES: Joi.number().integer().min(1).optional(), + PULL_CHECKS_PER_SP_PER_HOUR: Joi.number().min(0.001).max(20).optional(), + PULL_CHECK_JOB_TIMEOUT_SECONDS: Joi.number().min(60).optional(), + PULL_CHECK_POLL_INTERVAL_SECONDS: Joi.number().min(1).optional(), + PULL_CHECK_PIECE_SIZE_BYTES: Joi.number().integer().min(1024).optional(), + PULL_PIECE_CLEANUP_INTERVAL_SECONDS: Joi.number().integer().min(3600).optional(), +}); + +/** + * Returns the Joi field rules for a single network prefix (e.g. `"CALIBRATION"`), + * keyed by `_`. Active-network enforcement (required wallet, wallet + * key `.or()`) is applied in `createConfigValidationSchema`. + */ +export const createPerNetworkEnvSchema = (prefix: Uppercase): Record => + Object.fromEntries(Object.entries(perNetworkFieldRules()).map(([key, rule]) => [`${prefix}_${key}`, rule])); + +/** + * Bare (unprefixed) shared overrides for inheritable vars. Registered once + * globally so a shared value (e.g. `DEAL_JOB_TIMEOUT_SECONDS`) is validated with + * the same rule as its prefixed override, not waved through by `allowUnknown`. + * `.strip()` keeps the value out of the validated output so it is never assigned + * back into `process.env`; the loader reads the operator's original value. + */ +export const createSharedNetworkEnvSchema = (): Record => { + const rules = perNetworkFieldRules(); + return Object.fromEntries( + INHERITABLE_NETWORK_VARS.map((key) => [key, (rules[key] as Joi.AnySchema).optional().strip()]), + ); }; // --------------------------------------------------------------------------- @@ -231,6 +236,8 @@ export function createConfigValidationSchema(processEnv: NodeJS.ProcessEnv = pro ...datasetEnvSchema, ...timeoutEnvSchema, ...retrievalEnvSchema, + // Unprefixed shared overrides for inheritable per-network vars, validated once. + ...createSharedNetworkEnvSchema(), }; const walletKeyOrConditions: [string, string][] = []; diff --git a/apps/backend/src/config/legacy-env-compat.spec.ts b/apps/backend/src/config/legacy-env-compat.spec.ts index 9cb759e6..1b1f8532 100644 --- a/apps/backend/src/config/legacy-env-compat.spec.ts +++ b/apps/backend/src/config/legacy-env-compat.spec.ts @@ -108,4 +108,50 @@ describe("applyLegacyEnvCompat", () => { expect(b.CALIBRATION_WALLET_PRIVATE_KEY).toBeUndefined(); }); }); + + describe("renamed legacy vars", () => { + it("promotes a renamed var to its current unprefixed name (legacy mode)", () => { + const env = envOf({ + NETWORK: "calibration", + WALLET_PRIVATE_KEY: "0xkey", + DEALBOT_MAINTENANCE_WINDOWS_UTC: "01:00,13:00", + JOB_PIECE_CLEANUP_PER_SP_PER_HOUR: "0.5", + }); + const result = applyLegacyEnvCompat(env); + + expect(result.renamedVars).toEqual( + expect.arrayContaining([ + "DEALBOT_MAINTENANCE_WINDOWS_UTC->MAINTENANCE_WINDOWS_UTC", + "JOB_PIECE_CLEANUP_PER_SP_PER_HOUR->PIECE_CLEANUP_PER_SP_PER_HOUR", + ]), + ); + expect(env.MAINTENANCE_WINDOWS_UTC).toBe("01:00,13:00"); + expect(env.PIECE_CLEANUP_PER_SP_PER_HOUR).toBe("0.5"); + }); + + it("promotes renamed vars even when NETWORKS is already set (multi-network mode)", () => { + const env = envOf({ + NETWORKS: "calibration,mainnet", + DEALBOT_MAINTENANCE_WINDOW_MINUTES: "45", + }); + const result = applyLegacyEnvCompat(env); + + expect(result.applied).toBe(true); + expect(result.skipReason).toBe("networks_already_set"); + expect(result.renamedVars).toContain("DEALBOT_MAINTENANCE_WINDOW_MINUTES->MAINTENANCE_WINDOW_MINUTES"); + expect(env.MAINTENANCE_WINDOW_MINUTES).toBe("45"); + }); + + it("does not overwrite an already-set current name (explicit wins)", () => { + const env = envOf({ + NETWORKS: "calibration", + DEALBOT_MAINTENANCE_WINDOWS_UTC: "01:00", + MAINTENANCE_WINDOWS_UTC: "07:00,22:00", + }); + const result = applyLegacyEnvCompat(env); + + expect(env.MAINTENANCE_WINDOWS_UTC).toBe("07:00,22:00"); + expect(result.renamedVars).not.toContain("DEALBOT_MAINTENANCE_WINDOWS_UTC->MAINTENANCE_WINDOWS_UTC"); + }); + }); }); diff --git a/apps/backend/src/config/legacy-env-compat.ts b/apps/backend/src/config/legacy-env-compat.ts index 63166802..89a45bb1 100644 --- a/apps/backend/src/config/legacy-env-compat.ts +++ b/apps/backend/src/config/legacy-env-compat.ts @@ -29,61 +29,58 @@ import { SUPPORTED_NETWORKS } from "../common/constants.js"; import { createPinoExitLogger } from "../common/pino.config.js"; import type { Network } from "../common/types.js"; +import { PER_NETWORK_VARS } from "./network-fields.js"; /** * Env var names (unprefixed) that were moved into a per-network namespace. - * Each corresponds to a `_` variable in the new layout. - * - * Keep this list in sync with `createPerNetworkEnvSchema` in `env.schema.ts`. + * Each corresponds to a `_` variable in the new layout. Sourced + * from the per-network var catalog so this can't drift from the schema/loader. */ -const LEGACY_PER_NETWORK_VARS = [ - "WALLET_ADDRESS", - "WALLET_PRIVATE_KEY", - "SESSION_KEY_PRIVATE_KEY", - "RPC_URL", - "RPC_REQUEST_TIMEOUT_MS", - "PDP_SUBGRAPH_ENDPOINT", - "CHECK_DATASET_CREATION_FEES", - "USE_ONLY_APPROVED_PROVIDERS", - "DEALBOT_DATASET_VERSION", - "MIN_NUM_DATASETS_FOR_CHECKS", - "DEALS_PER_SP_PER_HOUR", - "RETRIEVALS_PER_SP_PER_HOUR", - "DATASET_CREATIONS_PER_SP_PER_HOUR", - "DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR", - "DATASET_LIFECYCLE_CHECK_ENABLED", - "DEAL_JOB_TIMEOUT_SECONDS", - "RETRIEVAL_JOB_TIMEOUT_SECONDS", - "DATA_SET_CREATION_JOB_TIMEOUT_SECONDS", - "DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS", - "DATA_RETENTION_POLL_INTERVAL_SECONDS", - "PROVIDERS_REFRESH_INTERVAL_SECONDS", - "MAINTENANCE_WINDOWS_UTC", - "MAINTENANCE_WINDOW_MINUTES", - "BLOCKED_SP_IDS", - "BLOCKED_SP_ADDRESSES", - "PIECE_CLEANUP_PER_SP_PER_HOUR", - "MAX_PIECE_CLEANUP_RUNTIME_SECONDS", - "MAX_DATASET_STORAGE_SIZE_BYTES", - "TARGET_DATASET_STORAGE_SIZE_BYTES", - "PULL_CHECKS_PER_SP_PER_HOUR", - "PULL_CHECK_JOB_TIMEOUT_SECONDS", - "PULL_CHECK_POLL_INTERVAL_SECONDS", - "PULL_CHECK_PIECE_SIZE_BYTES", - "PULL_PIECE_CLEANUP_INTERVAL_SECONDS", -] as const; +const LEGACY_PER_NETWORK_VARS = PER_NETWORK_VARS; + +/** + * Per-network vars whose unprefixed name changed in the move to the new layout. + * Maps the operator's existing (old) name to the current unprefixed name so a + * deployment that still sets the old name keeps its configured value instead of + * silently reverting to a default. Applied in both legacy and multi-network + * mode; the current name, once populated, is inherited per network by the loader. + */ +const RENAMED_LEGACY_VARS: Record = { + DEALBOT_MAINTENANCE_WINDOWS_UTC: "MAINTENANCE_WINDOWS_UTC", + DEALBOT_MAINTENANCE_WINDOW_MINUTES: "MAINTENANCE_WINDOW_MINUTES", + JOB_PIECE_CLEANUP_PER_SP_PER_HOUR: "PIECE_CLEANUP_PER_SP_PER_HOUR", +}; export interface LegacyEnvCompatResult { - /** True if legacy translation was applied. */ + /** True if any legacy translation (rename and/or single-network copy) was applied. */ applied: boolean; - /** The network the legacy env was resolved to (only set when applied). */ + /** The network the legacy env was resolved to (only set for single-network translation). */ network?: Network; /** Legacy var names that were copied into the new prefixed slot. */ translatedVars: string[]; - /** Reason translation was skipped, for diagnostics. */ + /** Renamed vars promoted to their current unprefixed name (`OLD->NEW`). */ + renamedVars: string[]; + /** Reason single-network translation was skipped, for diagnostics. */ skipReason?: "networks_already_set" | "no_legacy_network" | "invalid_legacy_network"; } +/** + * Promotes renamed legacy vars to their current unprefixed name when the current + * name is unset. Returns the `OLD->NEW` pairs that were applied. + */ +function applyRenamedLegacyVars(env: NodeJS.ProcessEnv): string[] { + const renamed: string[] = []; + for (const [oldName, currentName] of Object.entries(RENAMED_LEGACY_VARS)) { + const oldValue = env[oldName]; + if (typeof oldValue !== "string" || oldValue.length === 0) continue; + const existing = env[currentName]; + if (typeof existing === "string" && existing.length > 0) continue; // current name wins + env[currentName] = oldValue; + renamed.push(`${oldName}->${currentName}`); + } + return renamed; +} + /** * Translates legacy single-network env vars into the new prefixed layout. * Mutates `env` in place and returns a summary describing what happened. @@ -97,18 +94,20 @@ export interface LegacyEnvCompatResult { * missing-config error with its normal diagnostics. */ export function applyLegacyEnvCompat(env: NodeJS.ProcessEnv): LegacyEnvCompatResult { + const renamedVars = applyRenamedLegacyVars(env); + if (typeof env.NETWORKS === "string" && env.NETWORKS.trim().length > 0) { - return { applied: false, translatedVars: [], skipReason: "networks_already_set" }; + return { applied: renamedVars.length > 0, translatedVars: [], renamedVars, skipReason: "networks_already_set" }; } const legacyRaw = env.NETWORK; if (typeof legacyRaw !== "string" || legacyRaw.trim().length === 0) { - return { applied: false, translatedVars: [], skipReason: "no_legacy_network" }; + return { applied: renamedVars.length > 0, translatedVars: [], renamedVars, skipReason: "no_legacy_network" }; } const legacyNetwork = legacyRaw.trim().toLowerCase(); if (!SUPPORTED_NETWORKS.includes(legacyNetwork as Network)) { - return { applied: false, translatedVars: [], skipReason: "invalid_legacy_network" }; + return { applied: renamedVars.length > 0, translatedVars: [], renamedVars, skipReason: "invalid_legacy_network" }; } const network = legacyNetwork as Network; @@ -129,7 +128,7 @@ export function applyLegacyEnvCompat(env: NodeJS.ProcessEnv): LegacyEnvCompatRes translatedVars.push(key); } - return { applied: true, network, translatedVars }; + return { applied: true, network, translatedVars, renamedVars }; } /** @@ -143,9 +142,10 @@ export function logLegacyEnvCompatResult(result: LegacyEnvCompatResult): void { level: "warn", event: "config_legacy_env_detected", message: - "Legacy single-network env vars detected; translated into per-network prefixed vars. " + - "Update your ConfigMap/Secrets to the prefixed names before the next release.", + "Legacy env vars detected and translated. Update your ConfigMap/Secrets to the current " + + "names (per-network prefixed, or unprefixed shared) before the next release.", network: result.network, translatedVars: result.translatedVars, + renamedVars: result.renamedVars, }); } diff --git a/apps/backend/src/config/loader.spec.ts b/apps/backend/src/config/loader.spec.ts index 95d4baa5..8d5879a8 100644 --- a/apps/backend/src/config/loader.spec.ts +++ b/apps/backend/src/config/loader.spec.ts @@ -10,19 +10,34 @@ const KEYS_TO_RESET = [ "NETWORKS", "NETWORK", "CALIBRATION_WALLET_PRIVATE_KEY", + "CALIBRATION_SESSION_KEY_PRIVATE_KEY", "CALIBRATION_WALLET_ADDRESS", "CALIBRATION_RPC_URL", "CALIBRATION_DEALS_PER_SP_PER_HOUR", "CALIBRATION_BLOCKED_SP_IDS", + "CALIBRATION_DEAL_JOB_TIMEOUT_SECONDS", + "CALIBRATION_MAINTENANCE_WINDOWS_UTC", + "CALIBRATION_CHECK_DATASET_CREATION_FEES", + "CALIBRATION_TARGET_DATASET_STORAGE_SIZE_BYTES", + "CALIBRATION_MAX_DATASET_STORAGE_SIZE_BYTES", + "CALIBRATION_PDP_SUBGRAPH_ENDPOINT", "MAINNET_WALLET_PRIVATE_KEY", + "MAINNET_SESSION_KEY_PRIVATE_KEY", "MAINNET_WALLET_ADDRESS", "MAINNET_RPC_URL", + "MAINNET_DEAL_JOB_TIMEOUT_SECONDS", "WALLET_PRIVATE_KEY", "WALLET_ADDRESS", "RPC_URL", "DEALS_PER_SP_PER_HOUR", "BLOCKED_SP_IDS", "SESSION_KEY_PRIVATE_KEY", + "DEAL_JOB_TIMEOUT_SECONDS", + "MAINTENANCE_WINDOWS_UTC", + "CHECK_DATASET_CREATION_FEES", + "PDP_SUBGRAPH_ENDPOINT", + "MAX_DATASET_STORAGE_SIZE_BYTES", + "TARGET_DATASET_STORAGE_SIZE_BYTES", ]; const snapshot: Record = {}; @@ -87,3 +102,84 @@ describe("loadConfig", () => { expect(() => loadConfig()).not.toThrow(); }); }); + +describe("loadConfig per-network inheritance", () => { + it("inherits an unprefixed shared value on every active network", () => { + process.env.NETWORKS = "calibration,mainnet"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + process.env.MAINNET_WALLET_PRIVATE_KEY = "0xmain"; + process.env.DEAL_JOB_TIMEOUT_SECONDS = "500"; + + const cfg = loadConfig(); + + expect(cfg.networks.calibration.dealJobTimeoutSeconds).toBe(500); + expect(cfg.networks.mainnet.dealJobTimeoutSeconds).toBe(500); + }); + + it("lets a per-network override beat the shared value", () => { + process.env.NETWORKS = "calibration,mainnet"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + process.env.MAINNET_WALLET_PRIVATE_KEY = "0xmain"; + process.env.DEAL_JOB_TIMEOUT_SECONDS = "500"; // shared + process.env.MAINNET_DEAL_JOB_TIMEOUT_SECONDS = "900"; // override + + const cfg = loadConfig(); + + expect(cfg.networks.calibration.dealJobTimeoutSeconds).toBe(500); + expect(cfg.networks.mainnet.dealJobTimeoutSeconds).toBe(900); + }); + + it("falls back to the default when neither override nor shared is set", () => { + process.env.NETWORKS = "calibration"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + + const cfg = loadConfig(); + + expect(cfg.networks.calibration.dealJobTimeoutSeconds).toBe(360); // networkDefaults + }); + + it("does NOT inherit chain-specific vars from the unprefixed slot", () => { + process.env.NETWORKS = "calibration"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + // Unprefixed (shared) PDP_SUBGRAPH_ENDPOINT must be ignored for the network. + process.env.PDP_SUBGRAPH_ENDPOINT = "https://shared.example/subgraph"; + + const cfg = loadConfig(); + + expect(cfg.networks.calibration.pdpSubgraphEndpoint).toBeUndefined(); + }); + + it("throws when TARGET storage size is not below MAX (post-resolution)", () => { + process.env.NETWORKS = "calibration"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + process.env.CALIBRATION_MAX_DATASET_STORAGE_SIZE_BYTES = "1000"; + process.env.CALIBRATION_TARGET_DATASET_STORAGE_SIZE_BYTES = "1000"; + + expect(() => loadConfig()).toThrow(/TARGET_DATASET_STORAGE_SIZE_BYTES/); + }); + + it("catches TARGET>=MAX even when the two come from different precedence tiers", () => { + process.env.NETWORKS = "calibration"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + process.env.MAX_DATASET_STORAGE_SIZE_BYTES = "1000"; // shared + process.env.CALIBRATION_TARGET_DATASET_STORAGE_SIZE_BYTES = "2000"; // override + + expect(() => loadConfig()).toThrow(/must be less than MAX_DATASET_STORAGE_SIZE_BYTES/); + }); + + it("throws when NETWORKS resolves to no supported networks", () => { + process.env.NETWORKS = " , ,"; + + expect(() => loadConfig()).toThrow(/NETWORKS resolved to no supported networks/); + }); + + it("reads case-variant boolean env without forcing true", () => { + process.env.NETWORKS = "calibration"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + process.env.CHECK_DATASET_CREATION_FEES = "False"; // shared, mixed case + + const cfg = loadConfig(); + + expect(cfg.networks.calibration.checkDatasetCreationFees).toBe(false); + }); +}); diff --git a/apps/backend/src/config/loader.ts b/apps/backend/src/config/loader.ts index 628d71c8..7f588d72 100644 --- a/apps/backend/src/config/loader.ts +++ b/apps/backend/src/config/loader.ts @@ -10,7 +10,7 @@ import { DEFAULT_LOCAL_DATASETS_PATH, ZERO_ADDRESS } from "../common/constants.js"; import type { Network } from "../common/types.js"; import { networkDefaults } from "./constants.js"; -import { getBooleanEnv, getFloatEnv, getNumberEnv, getStringEnv } from "./env.helpers.js"; +import { coerceBoolean, coerceFloat, coerceNumber, getBooleanEnv, getNumberEnv, getStringEnv } from "./env.helpers.js"; import { parseActiveNetworks, parseAddressList, @@ -18,6 +18,7 @@ import { parseRandomDatasetSizes, parseRunMode, } from "./env.parsers.js"; +import { inheritsUnprefixed, type PerNetworkVar } from "./network-fields.js"; import type { BaseNetworkConfig, IAppConfig, @@ -114,140 +115,134 @@ type DistributiveOmit = T extends any ? Omit : nev * Reads all per-network env vars for one network's prefix * (e.g. `"CALIBRATION"` → reads `CALIBRATION_RPC_URL`, `CALIBRATION_WALLET_ADDRESS`, ...). * - * Legacy (unprefixed) envs are translated to this prefixed form by - * `applyLegacyEnvCompat` before `loadConfig` runs, so this function only - * needs to handle the prefixed scheme. + * Inheritable vars resolve with precedence `_` → unprefixed `` + * → default, so a shared value can be set once and overridden per network. + * Chain-specific vars (see `network-fields.ts`) read the prefixed slot only. + * Legacy unprefixed single-network envs are translated to the prefixed form by + * `applyLegacyEnvCompat` before `loadConfig` runs. */ function loadNetworkEnvPrefix( prefix: Uppercase, env: NodeJS.ProcessEnv, ): DistributiveOmit { - const k = (key: string) => `${prefix}_${key}`; - const get = (key: string) => env[k(key)]; const network = prefix.toLowerCase() as Network; + // Resolve one per-network var: prefixed override wins, inheritable vars then + // fall back to the unprefixed shared slot, else undefined (caller defaults). + const resolve = (key: PerNetworkVar): string | undefined => { + const prefixed = env[`${prefix}_${key}`]; + if (prefixed) return prefixed; + if (inheritsUnprefixed(key)) { + const shared = env[key]; + if (shared) return shared; + } + return undefined; + }; + const base = { - walletAddress: get("WALLET_ADDRESS") || ZERO_ADDRESS, - rpcUrl: get("RPC_URL") || undefined, - rpcRequestTimeoutMs: getNumberEnv(env, k("RPC_REQUEST_TIMEOUT_MS"), networkDefaults.rpcRequestTimeoutMs), - pdpSubgraphEndpoint: get("PDP_SUBGRAPH_ENDPOINT") || undefined, - checkDatasetCreationFees: getBooleanEnv( - env, - k("CHECK_DATASET_CREATION_FEES"), + walletAddress: resolve("WALLET_ADDRESS") ?? ZERO_ADDRESS, + rpcUrl: resolve("RPC_URL"), + rpcRequestTimeoutMs: coerceNumber(resolve("RPC_REQUEST_TIMEOUT_MS"), networkDefaults.rpcRequestTimeoutMs), + pdpSubgraphEndpoint: resolve("PDP_SUBGRAPH_ENDPOINT"), + checkDatasetCreationFees: coerceBoolean( + resolve("CHECK_DATASET_CREATION_FEES"), networkDefaults.checkDatasetCreationFees, ), - useOnlyApprovedProviders: getBooleanEnv( - env, - k("USE_ONLY_APPROVED_PROVIDERS"), + useOnlyApprovedProviders: coerceBoolean( + resolve("USE_ONLY_APPROVED_PROVIDERS"), networkDefaults.useOnlyApprovedProviders, ), - dealbotDataSetVersion: get("DEALBOT_DATASET_VERSION") || undefined, - minNumDataSetsForChecks: getNumberEnv( - env, - k("MIN_NUM_DATASETS_FOR_CHECKS"), + dealbotDataSetVersion: resolve("DEALBOT_DATASET_VERSION"), + minNumDataSetsForChecks: coerceNumber( + resolve("MIN_NUM_DATASETS_FOR_CHECKS"), networkDefaults.minNumDataSetsForChecks, ), - dealsPerSpPerHour: getFloatEnv(env, k("DEALS_PER_SP_PER_HOUR"), networkDefaults.dealsPerSpPerHour), - dealJobTimeoutSeconds: getNumberEnv(env, k("DEAL_JOB_TIMEOUT_SECONDS"), networkDefaults.dealJobTimeoutSeconds), - retrievalsPerSpPerHour: getFloatEnv(env, k("RETRIEVALS_PER_SP_PER_HOUR"), networkDefaults.retrievalsPerSpPerHour), - retrievalJobTimeoutSeconds: getNumberEnv( - env, - k("RETRIEVAL_JOB_TIMEOUT_SECONDS"), + dealsPerSpPerHour: coerceFloat(resolve("DEALS_PER_SP_PER_HOUR"), networkDefaults.dealsPerSpPerHour), + dealJobTimeoutSeconds: coerceNumber(resolve("DEAL_JOB_TIMEOUT_SECONDS"), networkDefaults.dealJobTimeoutSeconds), + retrievalsPerSpPerHour: coerceFloat(resolve("RETRIEVALS_PER_SP_PER_HOUR"), networkDefaults.retrievalsPerSpPerHour), + retrievalJobTimeoutSeconds: coerceNumber( + resolve("RETRIEVAL_JOB_TIMEOUT_SECONDS"), networkDefaults.retrievalJobTimeoutSeconds, ), - dataSetCreationsPerSpPerHour: getFloatEnv( - env, - k("DATASET_CREATIONS_PER_SP_PER_HOUR"), + dataSetCreationsPerSpPerHour: coerceFloat( + resolve("DATASET_CREATIONS_PER_SP_PER_HOUR"), networkDefaults.dataSetCreationsPerSpPerHour, ), - dataSetCreationJobTimeoutSeconds: getNumberEnv( - env, - k("DATA_SET_CREATION_JOB_TIMEOUT_SECONDS"), + dataSetCreationJobTimeoutSeconds: coerceNumber( + resolve("DATA_SET_CREATION_JOB_TIMEOUT_SECONDS"), networkDefaults.dataSetCreationJobTimeoutSeconds, ), // Network-dependent default: enabled on every network except mainnet. - dataSetLifecycleCheckEnabled: getBooleanEnv(env, k("DATASET_LIFECYCLE_CHECK_ENABLED"), network !== "mainnet"), - dataSetLifecycleChecksPerSpPerHour: getFloatEnv( - env, - k("DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR"), + dataSetLifecycleCheckEnabled: coerceBoolean(resolve("DATASET_LIFECYCLE_CHECK_ENABLED"), network !== "mainnet"), + dataSetLifecycleChecksPerSpPerHour: coerceFloat( + resolve("DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR"), networkDefaults.dataSetLifecycleChecksPerSpPerHour, ), - dataSetLifecycleCheckJobTimeoutSeconds: getNumberEnv( - env, - k("DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS"), + dataSetLifecycleCheckJobTimeoutSeconds: coerceNumber( + resolve("DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS"), networkDefaults.dataSetLifecycleCheckJobTimeoutSeconds, ), - dataRetentionPollIntervalSeconds: getNumberEnv( - env, - k("DATA_RETENTION_POLL_INTERVAL_SECONDS"), + dataRetentionPollIntervalSeconds: coerceNumber( + resolve("DATA_RETENTION_POLL_INTERVAL_SECONDS"), networkDefaults.dataRetentionPollIntervalSeconds, ), - providersRefreshIntervalSeconds: getNumberEnv( - env, - k("PROVIDERS_REFRESH_INTERVAL_SECONDS"), + providersRefreshIntervalSeconds: coerceNumber( + resolve("PROVIDERS_REFRESH_INTERVAL_SECONDS"), networkDefaults.providersRefreshIntervalSeconds, ), - pieceCleanupPerSpPerHour: getFloatEnv( - env, - k("PIECE_CLEANUP_PER_SP_PER_HOUR"), + pieceCleanupPerSpPerHour: coerceFloat( + resolve("PIECE_CLEANUP_PER_SP_PER_HOUR"), networkDefaults.pieceCleanupPerSpPerHour, ), - maxPieceCleanupRuntimeSeconds: getNumberEnv( - env, - k("MAX_PIECE_CLEANUP_RUNTIME_SECONDS"), + maxPieceCleanupRuntimeSeconds: coerceNumber( + resolve("MAX_PIECE_CLEANUP_RUNTIME_SECONDS"), networkDefaults.maxPieceCleanupRuntimeSeconds, ), - maxDatasetStorageSizeBytes: getNumberEnv( - env, - k("MAX_DATASET_STORAGE_SIZE_BYTES"), + maxDatasetStorageSizeBytes: coerceNumber( + resolve("MAX_DATASET_STORAGE_SIZE_BYTES"), networkDefaults.maxDatasetStorageSizeBytes, ), - targetDatasetStorageSizeBytes: getNumberEnv( - env, - k("TARGET_DATASET_STORAGE_SIZE_BYTES"), + targetDatasetStorageSizeBytes: coerceNumber( + resolve("TARGET_DATASET_STORAGE_SIZE_BYTES"), networkDefaults.targetDatasetStorageSizeBytes, ), - maintenanceWindowsUtc: get("MAINTENANCE_WINDOWS_UTC") - ? get("MAINTENANCE_WINDOWS_UTC")! - .split(",") - .map((v) => v.trim()) - .filter((v) => v.length > 0) - : networkDefaults.maintenanceWindowsUtc, - maintenanceWindowMinutes: getNumberEnv( - env, - k("MAINTENANCE_WINDOW_MINUTES"), + maintenanceWindowsUtc: ((raw) => + raw + ? raw + .split(",") + .map((v) => v.trim()) + .filter((v) => v.length > 0) + : networkDefaults.maintenanceWindowsUtc)(resolve("MAINTENANCE_WINDOWS_UTC")), + maintenanceWindowMinutes: coerceNumber( + resolve("MAINTENANCE_WINDOW_MINUTES"), networkDefaults.maintenanceWindowMinutes, ), - blockedSpIds: parseIdList(get("BLOCKED_SP_IDS")), - blockedSpAddresses: parseAddressList(get("BLOCKED_SP_ADDRESSES")), + blockedSpIds: parseIdList(resolve("BLOCKED_SP_IDS")), + blockedSpAddresses: parseAddressList(resolve("BLOCKED_SP_ADDRESSES")), - pullChecksPerSpPerHour: getFloatEnv(env, k("PULL_CHECKS_PER_SP_PER_HOUR"), networkDefaults.pullChecksPerSpPerHour), - pullCheckJobTimeoutSeconds: getNumberEnv( - env, - k("PULL_CHECK_JOB_TIMEOUT_SECONDS"), + pullChecksPerSpPerHour: coerceFloat(resolve("PULL_CHECKS_PER_SP_PER_HOUR"), networkDefaults.pullChecksPerSpPerHour), + pullCheckJobTimeoutSeconds: coerceNumber( + resolve("PULL_CHECK_JOB_TIMEOUT_SECONDS"), networkDefaults.pullCheckJobTimeoutSeconds, ), - pullCheckPollIntervalSeconds: getNumberEnv( - env, - k("PULL_CHECK_POLL_INTERVAL_SECONDS"), + pullCheckPollIntervalSeconds: coerceNumber( + resolve("PULL_CHECK_POLL_INTERVAL_SECONDS"), networkDefaults.pullCheckPollIntervalSeconds, ), - pullCheckPieceSizeBytes: getNumberEnv( - env, - k("PULL_CHECK_PIECE_SIZE_BYTES"), + pullCheckPieceSizeBytes: coerceNumber( + resolve("PULL_CHECK_PIECE_SIZE_BYTES"), networkDefaults.pullCheckPieceSizeBytes, ), - pullPieceCleanupIntervalSeconds: getNumberEnv( - env, - k("PULL_PIECE_CLEANUP_INTERVAL_SECONDS"), + pullPieceCleanupIntervalSeconds: coerceNumber( + resolve("PULL_PIECE_CLEANUP_INTERVAL_SECONDS"), networkDefaults.pullPieceCleanupIntervalSeconds, ), } satisfies Omit; - const walletPrivateKey = (get("WALLET_PRIVATE_KEY") || undefined) as `0x${string}` | undefined; - const sessionKeyPrivateKey = (get("SESSION_KEY_PRIVATE_KEY") || undefined) as `0x${string}` | undefined; + const walletPrivateKey = resolve("WALLET_PRIVATE_KEY") as `0x${string}` | undefined; + const sessionKeyPrivateKey = resolve("SESSION_KEY_PRIVATE_KEY") as `0x${string}` | undefined; if (sessionKeyPrivateKey) return { ...base, sessionKeyPrivateKey }; if (walletPrivateKey) return { ...base, walletPrivateKey }; @@ -259,11 +254,27 @@ function loadNetworkEnvPrefix( const loadNetworkConfigs = (env: NodeJS.ProcessEnv): Pick => { const activeNetworks = parseActiveNetworks(env); + if (activeNetworks.length === 0) { + // parseActiveNetworks already falls back to a default network for an absent + // NETWORKS; reaching here means NETWORKS was set but resolved to nothing + // (e.g. whitespace/commas). Fail fast rather than boot an idle process. + throw new Error("[config] NETWORKS resolved to no supported networks. Set NETWORKS to a comma-separated list."); + } const networks = {} as Record; for (const network of activeNetworks) { const prefix = network.toUpperCase() as Uppercase; - networks[network] = { network, ...loadNetworkEnvPrefix(prefix, env) }; + const networkConfig: INetworkConfig = { network, ...loadNetworkEnvPrefix(prefix, env) }; + // Cross-field invariant: the eviction target must sit below the hard cap, or + // piece cleanup has no headroom to act. Asserted post-resolution because the + // two values can come from different precedence tiers (override vs shared). + if (networkConfig.targetDatasetStorageSizeBytes >= networkConfig.maxDatasetStorageSizeBytes) { + throw new Error( + `[config] ${network}: TARGET_DATASET_STORAGE_SIZE_BYTES (${networkConfig.targetDatasetStorageSizeBytes}) ` + + `must be less than MAX_DATASET_STORAGE_SIZE_BYTES (${networkConfig.maxDatasetStorageSizeBytes})`, + ); + } + networks[network] = networkConfig; } return { networks, activeNetworks }; diff --git a/apps/backend/src/config/network-fields.spec.ts b/apps/backend/src/config/network-fields.spec.ts new file mode 100644 index 00000000..ae4c651c --- /dev/null +++ b/apps/backend/src/config/network-fields.spec.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; +import { + CHAIN_SPECIFIC_NETWORK_VARS, + INHERITABLE_NETWORK_VARS, + inheritsUnprefixed, + PER_NETWORK_VARS, +} from "./network-fields.js"; + +describe("network-fields classification", () => { + it("partitions every per-network var into exactly one bucket", () => { + const inheritable = new Set(INHERITABLE_NETWORK_VARS); + for (const key of PER_NETWORK_VARS) { + const isChainSpecific = CHAIN_SPECIFIC_NETWORK_VARS.has(key); + expect(isChainSpecific).toBe(!inheritable.has(key)); + } + expect(inheritable.size + CHAIN_SPECIFIC_NETWORK_VARS.size).toBe(PER_NETWORK_VARS.length); + }); + + it("keeps credentials and chain endpoints chain-specific (never inherited)", () => { + for (const key of [ + "WALLET_ADDRESS", + "WALLET_PRIVATE_KEY", + "SESSION_KEY_PRIVATE_KEY", + "RPC_URL", + "PDP_SUBGRAPH_ENDPOINT", + "BLOCKED_SP_IDS", + "BLOCKED_SP_ADDRESSES", + ] as const) { + expect(inheritsUnprefixed(key)).toBe(false); + } + }); + + it("marks tuning/timeout vars as inheritable", () => { + for (const key of [ + "DEAL_JOB_TIMEOUT_SECONDS", + "DEALS_PER_SP_PER_HOUR", + "MAINTENANCE_WINDOWS_UTC", + "MAX_DATASET_STORAGE_SIZE_BYTES", + ] as const) { + expect(inheritsUnprefixed(key)).toBe(true); + } + }); +}); diff --git a/apps/backend/src/config/network-fields.ts b/apps/backend/src/config/network-fields.ts new file mode 100644 index 00000000..d9cdbdb7 --- /dev/null +++ b/apps/backend/src/config/network-fields.ts @@ -0,0 +1,81 @@ +/** + * Per-network env var catalog and shared/chain-specific classification. + * + * Single source of truth for which per-network vars inherit an unprefixed + * "shared" value. The loader (resolution precedence) and the Joi schema (which + * keys carry validation rules) both read this, so they can't drift on whether a + * var is shared or chain-specific. + * + * Resolution precedence for an inheritable var, evaluated per active network: + * _ (per-network override) + * (unprefixed shared value, inheritable vars only) + * default (applied by the loader) + * + * Chain-specific vars never read the unprefixed slot: credentials and chain + * endpoints obviously differ per network, and provider ids/addresses registered + * on one chain are meaningless on another, so a shared blocklist would be wrong. + */ + +/** Per-network env var base names (the part after the `_` prefix). */ +export const PER_NETWORK_VARS = [ + "WALLET_ADDRESS", + "WALLET_PRIVATE_KEY", + "SESSION_KEY_PRIVATE_KEY", + "RPC_URL", + "RPC_REQUEST_TIMEOUT_MS", + "PDP_SUBGRAPH_ENDPOINT", + "CHECK_DATASET_CREATION_FEES", + "USE_ONLY_APPROVED_PROVIDERS", + "DEALBOT_DATASET_VERSION", + "MIN_NUM_DATASETS_FOR_CHECKS", + "DEALS_PER_SP_PER_HOUR", + "RETRIEVALS_PER_SP_PER_HOUR", + "DATASET_CREATIONS_PER_SP_PER_HOUR", + "DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR", + "DATASET_LIFECYCLE_CHECK_ENABLED", + "DEAL_JOB_TIMEOUT_SECONDS", + "RETRIEVAL_JOB_TIMEOUT_SECONDS", + "DATA_SET_CREATION_JOB_TIMEOUT_SECONDS", + "DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS", + "DATA_RETENTION_POLL_INTERVAL_SECONDS", + "PROVIDERS_REFRESH_INTERVAL_SECONDS", + "MAINTENANCE_WINDOWS_UTC", + "MAINTENANCE_WINDOW_MINUTES", + "BLOCKED_SP_IDS", + "BLOCKED_SP_ADDRESSES", + "PIECE_CLEANUP_PER_SP_PER_HOUR", + "MAX_PIECE_CLEANUP_RUNTIME_SECONDS", + "MAX_DATASET_STORAGE_SIZE_BYTES", + "TARGET_DATASET_STORAGE_SIZE_BYTES", + "PULL_CHECKS_PER_SP_PER_HOUR", + "PULL_CHECK_JOB_TIMEOUT_SECONDS", + "PULL_CHECK_POLL_INTERVAL_SECONDS", + "PULL_CHECK_PIECE_SIZE_BYTES", + "PULL_PIECE_CLEANUP_INTERVAL_SECONDS", +] as const; + +export type PerNetworkVar = (typeof PER_NETWORK_VARS)[number]; + +/** + * Vars that must differ per network and never inherit an unprefixed value: + * credentials, chain endpoints, and chain-local identifiers (provider ids and + * addresses registered on one chain don't carry to another). + */ +export const CHAIN_SPECIFIC_NETWORK_VARS = new Set([ + "WALLET_ADDRESS", + "WALLET_PRIVATE_KEY", + "SESSION_KEY_PRIVATE_KEY", + "RPC_URL", + "PDP_SUBGRAPH_ENDPOINT", + "DEALBOT_DATASET_VERSION", + "BLOCKED_SP_IDS", + "BLOCKED_SP_ADDRESSES", +]); + +/** Inheritable vars read the unprefixed shared slot; chain-specific ones don't. */ +export function inheritsUnprefixed(varName: PerNetworkVar): boolean { + return !CHAIN_SPECIFIC_NETWORK_VARS.has(varName); +} + +/** Base names that inherit an unprefixed shared value (override → shared → default). */ +export const INHERITABLE_NETWORK_VARS: PerNetworkVar[] = PER_NETWORK_VARS.filter(inheritsUnprefixed); diff --git a/apps/backend/src/deal/deal.service.spec.ts b/apps/backend/src/deal/deal.service.spec.ts index 2218eef9..41dad07b 100644 --- a/apps/backend/src/deal/deal.service.spec.ts +++ b/apps/backend/src/deal/deal.service.spec.ts @@ -145,7 +145,7 @@ describe("DealService", () => { paymentsService: {}, warmStorageService: mockWarmStorageService, }), - getSynapse: vi.fn().mockReturnValue({ + tryGetSynapse: vi.fn().mockReturnValue({ storage: { createContext: vi.fn().mockResolvedValue({ deletePiece: vi.fn(), @@ -1310,7 +1310,7 @@ describe("DealService", () => { createContext: vi.fn().mockResolvedValue({ dataSetId: 7n }), }, }; - mockWalletSdkService.getSynapse.mockReturnValue(synapseMock as unknown as Synapse); + mockWalletSdkService.tryGetSynapse.mockReturnValue(synapseMock as unknown as Synapse); mockWarmStorageService.validateDataSet.mockResolvedValueOnce(undefined); const result = await service.getDataSetProvisioningStatus("0xprovider", { dealbotDS: "1" }, DEFAULT_NETWORK); @@ -1323,7 +1323,7 @@ describe("DealService", () => { createContext: vi.fn().mockResolvedValue({ dataSetId: 9n }), }, }; - mockWalletSdkService.getSynapse.mockReturnValue(synapseMock as unknown as Synapse); + mockWalletSdkService.tryGetSynapse.mockReturnValue(synapseMock as unknown as Synapse); mockDatasetLivenessService.isDataSetLive.mockResolvedValueOnce(false); const result = await service.getDataSetProvisioningStatus("0xprovider", { dealbotDS: "1" }, DEFAULT_NETWORK); @@ -1452,7 +1452,7 @@ describe("DealService", () => { storage: { terminateDataSet: terminateMock }, client: { waitForTransactionReceipt: waitForReceiptMock }, }; - mockWalletSdkService.getSynapse.mockReturnValue(synapseMock); + mockWalletSdkService.tryGetSynapse.mockReturnValue(synapseMock); mockWarmStorageService.getDataSet.mockResolvedValueOnce({ pdpEndEpoch: 0n }); mockWarmStorageService.getDataSet.mockResolvedValueOnce({ pdpEndEpoch: 12345n }); @@ -1482,7 +1482,7 @@ describe("DealService", () => { storage: { terminateDataSet: terminateMock }, client: { waitForTransactionReceipt: vi.fn() }, }; - mockWalletSdkService.getSynapse.mockReturnValue(synapseMock); + mockWalletSdkService.tryGetSynapse.mockReturnValue(synapseMock); mockWarmStorageService.getDataSet.mockResolvedValueOnce({ pdpEndEpoch: 999n }); const updateFn = vi.fn().mockResolvedValue({ affected: 1 }); @@ -1505,7 +1505,7 @@ describe("DealService", () => { storage: { terminateDataSet: terminateMock }, client: { waitForTransactionReceipt: vi.fn() }, }; - mockWalletSdkService.getSynapse.mockReturnValue(synapseMock); + mockWalletSdkService.tryGetSynapse.mockReturnValue(synapseMock); mockWarmStorageService.getDataSet.mockResolvedValueOnce({ pdpEndEpoch: 0n }); mockWarmStorageService.getDataSet.mockResolvedValueOnce({ pdpEndEpoch: 7n }); @@ -1637,7 +1637,7 @@ describe("DealService", () => { const synapseMock = { storage: { createContext: createContextMock }, } as unknown as Synapse; - mockWalletSdkService.getSynapse.mockReturnValue(synapseMock); + mockWalletSdkService.tryGetSynapse.mockReturnValue(synapseMock); (executeUpload as Mock).mockImplementation(async (_service, _data, _rootCid, options) => { await triggerUploadProgress(options?.onProgress); @@ -1676,7 +1676,7 @@ describe("DealService", () => { it("does not invoke data-storage-check metrics or Deal persistence", async () => { vi.spyOn(mockWalletSdkService, "getProviderInfo").mockReturnValue(mockProviderInfo); const createContextMock = vi.fn().mockResolvedValue({ dataSetId: 1 }); - mockWalletSdkService.getSynapse.mockReturnValue({ + mockWalletSdkService.tryGetSynapse.mockReturnValue({ storage: { createContext: createContextMock }, }); (executeUpload as Mock).mockImplementation(async (_s, _d, _r, opts) => { @@ -1695,7 +1695,7 @@ describe("DealService", () => { it("fails when upload completes without a pieceCid", async () => { vi.spyOn(mockWalletSdkService, "getProviderInfo").mockReturnValue(mockProviderInfo); - mockWalletSdkService.getSynapse.mockReturnValue({ + mockWalletSdkService.tryGetSynapse.mockReturnValue({ storage: { createContext: vi.fn().mockResolvedValue({ dataSetId: 1 }) }, }); diff --git a/apps/backend/src/deal/deal.service.ts b/apps/backend/src/deal/deal.service.ts index 1f4dde6d..9c2a7f94 100644 --- a/apps/backend/src/deal/deal.service.ts +++ b/apps/backend/src/deal/deal.service.ts @@ -97,7 +97,7 @@ export class DealService { try { const synapse = - this.walletSdkService.getSynapse(options.network) ?? (await this.createSynapseInstance(options.network)); + this.walletSdkService.tryGetSynapse(options.network) ?? (await this.createSynapseInstance(options.network)); const uploadPayload = await this.prepareUploadPayload(preprocessed, options.signal); return await this.createDeal( synapse, @@ -714,7 +714,7 @@ export class DealService { { status: "missing" } | { status: "live"; dataSetId: bigint } | { status: "terminated"; dataSetId: bigint } > { signal?.throwIfAborted(); - const synapse = this.walletSdkService.getSynapse(network) ?? (await this.createSynapseInstance(network)); + const synapse = this.walletSdkService.tryGetSynapse(network) ?? (await this.createSynapseInstance(network)); const providerInfo = this.walletSdkService.getProviderInfo(providerAddress, network); if (!providerInfo) { throw new Error(`Provider ${providerAddress} not found in registry`); @@ -769,7 +769,7 @@ export class DealService { pollTimeoutMs = 60_000, ): Promise<{ dealsAffected: number; pdpEndEpoch: bigint }> { signal?.throwIfAborted(); - const synapse = this.walletSdkService.getSynapse(network) ?? (await this.createSynapseInstance(network)); + const synapse = this.walletSdkService.tryGetSynapse(network) ?? (await this.createSynapseInstance(network)); const providerInfo = this.walletSdkService.getProviderInfo(providerAddress, network); const { warmStorageService } = this.walletSdkService.getWalletServices(network); @@ -913,7 +913,7 @@ export class DealService { let transactionHash: string | undefined; try { - const synapse = this.walletSdkService.getSynapse(network) ?? (await this.createSynapseInstance(network)); + const synapse = this.walletSdkService.tryGetSynapse(network) ?? (await this.createSynapseInstance(network)); signal?.throwIfAborted(); const DATA_SET_CREATION_PIECE_SIZE = 200 * 1024; // 200 KiB diff --git a/apps/backend/src/jobs/jobs.service.ts b/apps/backend/src/jobs/jobs.service.ts index 964cac47..a6f186f2 100644 --- a/apps/backend/src/jobs/jobs.service.ts +++ b/apps/backend/src/jobs/jobs.service.ts @@ -227,6 +227,9 @@ export class JobsService implements OnModuleInit, OnApplicationShutdown { cfg.dataSetCreationJobTimeoutSeconds, cfg.dataSetLifecycleCheckJobTimeoutSeconds, cfg.pullCheckJobTimeoutSeconds, + // piece_cleanup runs on the same SP_WORK_QUEUE with its own abort + // bound to this timeout; include it so drain doesn't force-fail it. + cfg.maxPieceCleanupRuntimeSeconds, ); if (maxNetworkTimeout > longestJobTimeoutSec) { diff --git a/apps/backend/src/metrics-prometheus/metrics-prometheus.module.ts b/apps/backend/src/metrics-prometheus/metrics-prometheus.module.ts index b6a5ebf7..330b1884 100644 --- a/apps/backend/src/metrics-prometheus/metrics-prometheus.module.ts +++ b/apps/backend/src/metrics-prometheus/metrics-prometheus.module.ts @@ -378,9 +378,11 @@ const metricProviders = [ enabled: true, }, path: "/metrics", + // Network is applied per-metric where it is meaningful. A process runs + // multiple networks, so a single global `network` default label would + // mislabel process/default metrics and any series that forgot to set it. defaultLabels: { app: "dealbot", - network: process.env.NETWORK || "calibration", }, }), ], diff --git a/apps/backend/src/piece-cleanup/piece-cleanup.service.spec.ts b/apps/backend/src/piece-cleanup/piece-cleanup.service.spec.ts index 154fb1c6..d733f2a3 100644 --- a/apps/backend/src/piece-cleanup/piece-cleanup.service.spec.ts +++ b/apps/backend/src/piece-cleanup/piece-cleanup.service.spec.ts @@ -71,7 +71,7 @@ describe("PieceCleanupService", () => { function createWalletSdkMock() { return { getProviderInfo: vi.fn().mockReturnValue({ id: 9, name: "Test SP" }), - getSynapse: vi.fn().mockReturnValue({ + tryGetSynapse: vi.fn().mockReturnValue({ storage: { createContext: vi.fn().mockResolvedValue({ deletePiece: vi.fn(), @@ -533,7 +533,7 @@ describe("PieceCleanupService", () => { const createContextMock = vi.fn().mockResolvedValue({ deletePiece: deletePieceMock, }); - walletSdkMock.getSynapse.mockReturnValue({ + walletSdkMock.tryGetSynapse.mockReturnValue({ storage: { createContext: createContextMock, }, @@ -560,7 +560,7 @@ describe("PieceCleanupService", () => { const createContextMock = vi.fn().mockResolvedValue({ deletePiece: deletePieceMock, }); - walletSdkMock.getSynapse.mockReturnValue(null); + walletSdkMock.tryGetSynapse.mockReturnValue(null); (createSynapseFromConfig as ReturnType).mockResolvedValue({ synapse: { storage: { @@ -589,7 +589,7 @@ describe("PieceCleanupService", () => { const createContextMock = vi.fn().mockResolvedValue({ deletePiece: deletePieceMock, }); - walletSdkMock.getSynapse.mockReturnValue({ + walletSdkMock.tryGetSynapse.mockReturnValue({ storage: { createContext: createContextMock, }, @@ -609,7 +609,7 @@ describe("PieceCleanupService", () => { const createContextMock = vi.fn().mockResolvedValue({ deletePiece: deletePieceMock, }); - walletSdkMock.getSynapse.mockReturnValue({ + walletSdkMock.tryGetSynapse.mockReturnValue({ storage: { createContext: createContextMock, }, diff --git a/apps/backend/src/piece-cleanup/piece-cleanup.service.ts b/apps/backend/src/piece-cleanup/piece-cleanup.service.ts index 999dd8d9..b6d9c861 100644 --- a/apps/backend/src/piece-cleanup/piece-cleanup.service.ts +++ b/apps/backend/src/piece-cleanup/piece-cleanup.service.ts @@ -165,7 +165,7 @@ export class PieceCleanupService { let failed = 0; let bytesRemoved = 0; - const synapse = this.walletSdkService.getSynapse(network) ?? (await this.createSynapseInstance(network)); + const synapse = this.walletSdkService.tryGetSynapse(network) ?? (await this.createSynapseInstance(network)); // Fetch candidates in batches. Keep deleting until back under quota or runtime cap. while (bytesRemoved < excessBytes) { @@ -256,7 +256,7 @@ export class PieceCleanupService { * Query the provider's actual storage via filecoin-pin. */ async getLiveStoredBytesForProvider(spAddress: string, network: Network, signal?: AbortSignal): Promise { - const synapse = this.walletSdkService.getSynapse(network) ?? (await this.createSynapseInstance(network)); + const synapse = this.walletSdkService.tryGetSynapse(network) ?? (await this.createSynapseInstance(network)); const datasets = await this.awaitWithAbort( listDataSets(synapse, { @@ -363,7 +363,7 @@ export class PieceCleanupService { throw new Error(`Provider ID not found for SP address ${deal.spAddress}`); } const synapse = - existingSynapse ?? this.walletSdkService.getSynapse(network) ?? (await this.createSynapseInstance(network)); + existingSynapse ?? this.walletSdkService.tryGetSynapse(network) ?? (await this.createSynapseInstance(network)); const storage = await synapse.storage.createContext({ providerId, dataSetId: deal.dataSetId, diff --git a/apps/backend/src/retrieval/retrieval.service.ts b/apps/backend/src/retrieval/retrieval.service.ts index 15ac75ad..de8086e1 100644 --- a/apps/backend/src/retrieval/retrieval.service.ts +++ b/apps/backend/src/retrieval/retrieval.service.ts @@ -95,7 +95,7 @@ export class RetrievalService { ): Promise { signal?.throwIfAborted(); - const provider = await this.findStorageProvider(deal.spAddress); + const provider = await this.findStorageProvider(deal.spAddress, deal.network); if (!provider) { throw new Error(`Storage provider ${deal.spAddress} not found`); } @@ -460,8 +460,11 @@ export class RetrievalService { } } - private async findStorageProvider(address: string): Promise { - return this.spRepository.findOne({ where: { address } }); + private async findStorageProvider(address: string, network: Network): Promise { + // storage_providers is keyed by (address, network); the same SP address can + // exist on both chains, so the network filter is required to avoid returning + // the wrong row. + return this.spRepository.findOne({ where: { address, network } }); } /** diff --git a/apps/backend/src/wallet-sdk/wallet-sdk.service.ts b/apps/backend/src/wallet-sdk/wallet-sdk.service.ts index 36bb6f8e..9ec4b708 100644 --- a/apps/backend/src/wallet-sdk/wallet-sdk.service.ts +++ b/apps/backend/src/wallet-sdk/wallet-sdk.service.ts @@ -364,6 +364,16 @@ export class WalletSdkService implements OnModuleInit { return this.getNetworkState(network).synapse; } + /** + * Returns the initialized Synapse for a network, or `undefined` when the + * network has no initialized state (chain integration disabled, or the network + * isn't active). Unlike {@link getSynapse}, this never throws, so callers can + * fall back to on-demand creation. + */ + tryGetSynapse(network: Network): Synapse | undefined { + return this.networkStates.get(network)?.synapse; + } + /** * Ensure wallet has sufficient allowances for operations. * Skipped in session key mode, deposits and operator approvals must be diff --git a/docs/environment-variables.md b/docs/environment-variables.md index c17e143d..ae7b28bb 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -10,19 +10,27 @@ Dealbot drives one or more Filecoin networks from a **single process**. The acti # Run Dealbot against both networks from the same instance NETWORKS=calibration,mainnet +# Credentials and endpoints are per-network (never shared) CALIBRATION_WALLET_PRIVATE_KEY=0xabc... CALIBRATION_RPC_URL=https://api.calibration.node.glif.io/rpc/v1 -CALIBRATION_DEALS_PER_SP_PER_HOUR=2 - MAINNET_WALLET_PRIVATE_KEY=0xdef... MAINNET_RPC_URL=https://api.node.glif.io/rpc/v1 + +# Shared tuning: set once unprefixed, applies to every network +DEAL_JOB_TIMEOUT_SECONDS=360 +MAINTENANCE_WINDOWS_UTC=07:00,22:00 + +# ...and override per network only where they differ +CALIBRATION_DEALS_PER_SP_PER_HOUR=2 MAINNET_DEALS_PER_SP_PER_HOUR=1 ``` **Rules** -- **Global vs. per-network.** Unprefixed variables (database, HTTP ports, job timeouts, etc.) apply to the whole process. Prefixed variables configure a specific network. -- **Active-network validation.** Only networks listed in `NETWORKS` are validated at startup. Variables for inactive networks are ignored, so you can keep a `MAINNET_*` block commented out until you are ready. +- **Resolution precedence (per-network vars).** Each network resolves a variable as `_` (per-network override) → `` (unprefixed shared value) → built-in default. Set a value once unprefixed to share it across every active network, and add a `_` override only where a network differs. +- **Chain-specific vars never inherit.** Credentials, chain endpoints, and chain-local identifiers must be set with a prefix and do not read the unprefixed slot: `WALLET_ADDRESS`, `WALLET_PRIVATE_KEY`, `SESSION_KEY_PRIVATE_KEY`, `RPC_URL`, `PDP_SUBGRAPH_ENDPOINT`, `DEALBOT_DATASET_VERSION`, `BLOCKED_SP_IDS`, `BLOCKED_SP_ADDRESSES`. +- **Process-global vars.** Database, HTTP ports, ClickHouse, and pg-boss scheduler settings apply to the whole process and have no per-network form. +- **Validation.** Both the unprefixed shared value and each `_` override are validated against the same rules at startup. Only networks listed in `NETWORKS` are required; variables for inactive networks are ignored, so you can keep a `MAINNET_*` block commented out until you are ready. - **Wallet vs. session key.** Each active network must provide either `_WALLET_PRIVATE_KEY` or `_SESSION_KEY_PRIVATE_KEY`. When both are present the session key takes precedence (see [`docs/runbooks/wallet-and-session-keys.md`](./runbooks/wallet-and-session-keys.md)). - **Supported prefixes.** `CALIBRATION_*`, `MAINNET_*`. Additional networks can be added by extending `SUPPORTED_NETWORKS` in the codebase. From 89fe230fdb28d33e608badfe5c172255b368643a Mon Sep 17 00:00:00 2001 From: Russell Dempsey <1173416+SgtPooki@users.noreply.github.com> Date: Wed, 24 Jun 2026 14:11:04 -0400 Subject: [PATCH 2/3] fix(config): sync shared values via NestJS validated env; harden classification Peer review surfaced that `.optional().strip()` on shared keys dropped them from the validated object NestJS assigns back into process.env, so a shared value sourced from a `.env` file (or a renamed legacy var promoted on NestJS's env copy) never reached the loader. Remove `.strip()`: with per-network Joi defaults already gone, returning present shared keys can't reshadow anything, and it lets file-sourced shared values reach process.env. - DATASET_LIFECYCLE_CHECK_ENABLED is now chain-specific: its default is network-dependent (off on mainnet, dealbot#546), so a shared `=true` must not silently enable the canary on mainnet. - DATA_RETENTION_POLL_INTERVAL_SECONDS / PROVIDERS_REFRESH_INTERVAL_SECONDS gain .integer().min(1) so a 0/negative interval can't busy-loop. - Add Nest-copy-faithful pipeline tests (file-sourced shared value, multi-network rename promotion, no prefixed-default injection) + lifecycle-safety and shared-only TARGET_METRICS_PER_HOUR from the env docs (not in the catalog). --- .../src/config/config-pipeline.spec.ts | 83 +++++++++++++++++++ apps/backend/src/config/env.schema.ts | 16 ++-- apps/backend/src/config/loader.spec.ts | 25 ++++++ apps/backend/src/config/network-fields.ts | 4 + docs/environment-variables.md | 13 +-- 5 files changed, 122 insertions(+), 19 deletions(-) diff --git a/apps/backend/src/config/config-pipeline.spec.ts b/apps/backend/src/config/config-pipeline.spec.ts index d7c49ee0..484b569f 100644 --- a/apps/backend/src/config/config-pipeline.spec.ts +++ b/apps/backend/src/config/config-pipeline.spec.ts @@ -20,15 +20,21 @@ const TOUCHED = [ "RPC_URL", "DEALBOT_MAINTENANCE_WINDOWS_UTC", "MAINTENANCE_WINDOWS_UTC", + "DEALBOT_MAINTENANCE_WINDOW_MINUTES", + "MAINTENANCE_WINDOW_MINUTES", "DEAL_JOB_TIMEOUT_SECONDS", + "DATASET_LIFECYCLE_CHECK_ENABLED", "CALIBRATION_WALLET_ADDRESS", "CALIBRATION_SESSION_KEY_PRIVATE_KEY", "CALIBRATION_WALLET_PRIVATE_KEY", "CALIBRATION_RPC_URL", "CALIBRATION_MAINTENANCE_WINDOWS_UTC", + "CALIBRATION_DEAL_JOB_TIMEOUT_SECONDS", + "CALIBRATION_DATASET_LIFECYCLE_CHECK_ENABLED", "MAINNET_WALLET_PRIVATE_KEY", "MAINNET_WALLET_ADDRESS", "MAINNET_DEAL_JOB_TIMEOUT_SECONDS", + "MAINNET_DATASET_LIFECYCLE_CHECK_ENABLED", ]; const snapshot: Record = {}; @@ -98,4 +104,81 @@ describe("config pipeline (validateConfig -> loadConfig)", () => { expect(() => validateConfig(process.env as Record)).toThrow(/DEAL_JOB_TIMEOUT_SECONDS/); }); + + it("carries a shared value sourced only from a .env file (not the OS env) through to the loader", () => { + // Faithful to @nestjs/config: it validates a merged object, then assigns + // ONLY the returned keys back into process.env (and only when not already + // present). A `.env`-file value lives in that object, not in process.env, so + // the loader sees it only if validateConfig keeps it in its output. + const envFile: Record = { + NETWORKS: "calibration,mainnet", + DATABASE_HOST: "localhost", + DATABASE_USER: "test", + DATABASE_PASSWORD: "test", + DATABASE_NAME: "test", + CALIBRATION_WALLET_PRIVATE_KEY: "0xcal", + CALIBRATION_WALLET_ADDRESS: "0xcaladdr", + MAINNET_WALLET_PRIVATE_KEY: "0xmain", + MAINNET_WALLET_ADDRESS: "0xmainaddr", + DEAL_JOB_TIMEOUT_SECONDS: "500", // shared, file-only + }; + + const config = { ...envFile, ...process.env }; + const validated = validateConfig(config) as Record; + for (const key of Object.keys(validated)) { + if (!(key in process.env)) process.env[key] = String(validated[key]); + } + + const cfg = loadConfig(); + expect(cfg.networks.calibration.dealJobTimeoutSeconds).toBe(500); + expect(cfg.networks.mainnet.dealJobTimeoutSeconds).toBe(500); + }); + + it("promotes a renamed legacy var through the Nest copy path to both networks", () => { + // Multi-network + only the OLD name set. The rename happens on Nest's copy; + // the promoted current name must survive validation and be assigned to + // process.env so the loader inherits it (regression for the strip bug). + const envFile: Record = { + NETWORKS: "calibration,mainnet", + DATABASE_HOST: "localhost", + DATABASE_USER: "test", + DATABASE_PASSWORD: "test", + DATABASE_NAME: "test", + CALIBRATION_WALLET_PRIVATE_KEY: "0xcal", + CALIBRATION_WALLET_ADDRESS: "0xcaladdr", + MAINNET_WALLET_PRIVATE_KEY: "0xmain", + MAINNET_WALLET_ADDRESS: "0xmainaddr", + DEALBOT_MAINTENANCE_WINDOW_MINUTES: "45", // old name only + }; + + const config = { ...envFile, ...process.env }; + const validated = validateConfig(config) as Record; + for (const key of Object.keys(validated)) { + if (!(key in process.env)) process.env[key] = String(validated[key]); + } + + const cfg = loadConfig(); + expect(cfg.networks.calibration.maintenanceWindowMinutes).toBe(45); + expect(cfg.networks.mainnet.maintenanceWindowMinutes).toBe(45); + }); + + it("does not inject a prefixed per-network Joi default into process.env", () => { + // The core inheritance fix: prefixed keys carry no Joi default, so an absent + // override is NOT written back to process.env where it would shadow a shared value. + const config = { + NETWORKS: "calibration", + DATABASE_HOST: "localhost", + DATABASE_USER: "test", + DATABASE_PASSWORD: "test", + DATABASE_NAME: "test", + CALIBRATION_WALLET_PRIVATE_KEY: "0xcal", + CALIBRATION_WALLET_ADDRESS: "0xcaladdr", + }; + const validated = validateConfig(config) as Record; + for (const key of Object.keys(validated)) { + if (!(key in process.env)) process.env[key] = String(validated[key]); + } + + expect(process.env.CALIBRATION_DEAL_JOB_TIMEOUT_SECONDS).toBeUndefined(); + }); }); diff --git a/apps/backend/src/config/env.schema.ts b/apps/backend/src/config/env.schema.ts index aa87b6c1..47f3223c 100644 --- a/apps/backend/src/config/env.schema.ts +++ b/apps/backend/src/config/env.schema.ts @@ -176,8 +176,8 @@ const perNetworkFieldRules = (): Record => ({ RETRIEVAL_JOB_TIMEOUT_SECONDS: Joi.number().min(60).optional(), DATA_SET_CREATION_JOB_TIMEOUT_SECONDS: Joi.number().min(60).optional(), DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS: Joi.number().min(60).optional(), - DATA_RETENTION_POLL_INTERVAL_SECONDS: Joi.number().optional(), - PROVIDERS_REFRESH_INTERVAL_SECONDS: Joi.number().optional(), + DATA_RETENTION_POLL_INTERVAL_SECONDS: Joi.number().integer().min(1).optional(), + PROVIDERS_REFRESH_INTERVAL_SECONDS: Joi.number().integer().min(1).optional(), MAINTENANCE_WINDOWS_UTC: Joi.string().custom(validateMaintenanceWindowsEnv).optional(), MAINTENANCE_WINDOW_MINUTES: Joi.number().min(20).max(360).optional(), BLOCKED_SP_IDS: Joi.string().optional().allow(""), @@ -205,14 +205,16 @@ export const createPerNetworkEnvSchema = (prefix: Uppercase): Record => { const rules = perNetworkFieldRules(); - return Object.fromEntries( - INHERITABLE_NETWORK_VARS.map((key) => [key, (rules[key] as Joi.AnySchema).optional().strip()]), - ); + return Object.fromEntries(INHERITABLE_NETWORK_VARS.map((key) => [key, (rules[key] as Joi.AnySchema).optional()])); }; // --------------------------------------------------------------------------- diff --git a/apps/backend/src/config/loader.spec.ts b/apps/backend/src/config/loader.spec.ts index 8d5879a8..ef93cb7c 100644 --- a/apps/backend/src/config/loader.spec.ts +++ b/apps/backend/src/config/loader.spec.ts @@ -38,6 +38,9 @@ const KEYS_TO_RESET = [ "PDP_SUBGRAPH_ENDPOINT", "MAX_DATASET_STORAGE_SIZE_BYTES", "TARGET_DATASET_STORAGE_SIZE_BYTES", + "DATASET_LIFECYCLE_CHECK_ENABLED", + "CALIBRATION_DATASET_LIFECYCLE_CHECK_ENABLED", + "MAINNET_DATASET_LIFECYCLE_CHECK_ENABLED", ]; const snapshot: Record = {}; @@ -167,6 +170,28 @@ describe("loadConfig per-network inheritance", () => { expect(() => loadConfig()).toThrow(/must be less than MAX_DATASET_STORAGE_SIZE_BYTES/); }); + it("catches TARGET>=MAX when both come from shared unprefixed vars", () => { + process.env.NETWORKS = "calibration"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + process.env.MAX_DATASET_STORAGE_SIZE_BYTES = "1000"; + process.env.TARGET_DATASET_STORAGE_SIZE_BYTES = "1500"; + + expect(() => loadConfig()).toThrow(/TARGET_DATASET_STORAGE_SIZE_BYTES/); + }); + + it("does not let a shared DATASET_LIFECYCLE_CHECK_ENABLED enable the canary on mainnet", () => { + process.env.NETWORKS = "calibration,mainnet"; + process.env.CALIBRATION_WALLET_PRIVATE_KEY = "0xcal"; + process.env.MAINNET_WALLET_PRIVATE_KEY = "0xmain"; + // Shared (unprefixed) value: chain-specific var must NOT inherit it. + process.env.DATASET_LIFECYCLE_CHECK_ENABLED = "true"; + + const cfg = loadConfig(); + + expect(cfg.networks.calibration.dataSetLifecycleCheckEnabled).toBe(true); // network default (off mainnet only) + expect(cfg.networks.mainnet.dataSetLifecycleCheckEnabled).toBe(false); // safety default preserved + }); + it("throws when NETWORKS resolves to no supported networks", () => { process.env.NETWORKS = " , ,"; diff --git a/apps/backend/src/config/network-fields.ts b/apps/backend/src/config/network-fields.ts index d9cdbdb7..cce835bd 100644 --- a/apps/backend/src/config/network-fields.ts +++ b/apps/backend/src/config/network-fields.ts @@ -70,6 +70,10 @@ export const CHAIN_SPECIFIC_NETWORK_VARS = new Set([ "DEALBOT_DATASET_VERSION", "BLOCKED_SP_IDS", "BLOCKED_SP_ADDRESSES", + // Has a network-dependent default (off on mainnet) because the canary can't + // auto-terminate PDP data sets when the payer is a Safe multisig (dealbot#546). + // Kept chain-specific so a shared `=true` can't accidentally enable it on mainnet. + "DATASET_LIFECYCLE_CHECK_ENABLED", ]); /** Inheritable vars read the unprefixed shared slot; chain-specific ones don't. */ diff --git a/docs/environment-variables.md b/docs/environment-variables.md index ae7b28bb..27887130 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -41,7 +41,7 @@ MAINNET_DEALS_PER_SP_PER_HOUR=1 | [Application](#application-configuration) | `NODE_ENV`, `DEALBOT_PORT`, `DEALBOT_HOST`, `DEALBOT_RUN_MODE`, `DEALBOT_METRICS_PORT`, `DEALBOT_METRICS_HOST`, `DEALBOT_ALLOWED_ORIGINS`, `ENABLE_DEV_MODE`, `DEALBOT_API_PUBLIC_URL`, `DEALBOT_PROBE_LOCATION` | | [Database](#database-configuration) | `DATABASE_HOST`, `DATABASE_PORT`, `DATABASE_POOL_MAX`, `DATABASE_USER`, `DATABASE_PASSWORD`, `DATABASE_NAME` | | [Per-Network](#per-network-configuration) | `_WALLET_ADDRESS`, `_WALLET_PRIVATE_KEY`, `_SESSION_KEY_PRIVATE_KEY`, `_RPC_URL`, `_RPC_REQUEST_TIMEOUT_MS`, `_CHECK_DATASET_CREATION_FEES`, `_USE_ONLY_APPROVED_PROVIDERS`, `_PDP_SUBGRAPH_ENDPOINT`, `_DEALBOT_DATASET_VERSION`, `_MIN_NUM_DATASETS_FOR_CHECKS` | -| [Per-Network Scheduling](#per-network-scheduling) | `_DEALS_PER_SP_PER_HOUR`, `_DEAL_JOB_TIMEOUT_SECONDS`, `_RETRIEVALS_PER_SP_PER_HOUR`, `_RETRIEVAL_JOB_TIMEOUT_SECONDS`, `_DATASET_CREATIONS_PER_SP_PER_HOUR`, `_DATA_SET_CREATION_JOB_TIMEOUT_SECONDS`, `_DATASET_LIFECYCLE_CHECK_ENABLED`, `_DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR`, `_DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS`, `_PULL_CHECKS_PER_SP_PER_HOUR`, `_PULL_CHECK_JOB_TIMEOUT_SECONDS`, `_PULL_CHECK_POLL_INTERVAL_SECONDS`, `_PULL_PIECE_CLEANUP_INTERVAL_SECONDS`, `_METRICS_PER_HOUR`, `_PROVIDERS_REFRESH_INTERVAL_SECONDS`, `_DATA_RETENTION_POLL_INTERVAL_SECONDS`, `_MAINTENANCE_WINDOWS_UTC`, `_MAINTENANCE_WINDOW_MINUTES`, `_BLOCKED_SP_IDS`, `_BLOCKED_SP_ADDRESSES`, `_MAX_DATASET_STORAGE_SIZE_BYTES`, `_TARGET_DATASET_STORAGE_SIZE_BYTES`, `_PIECE_CLEANUP_PER_SP_PER_HOUR`, `_MAX_PIECE_CLEANUP_RUNTIME_SECONDS` | +| [Per-Network Scheduling](#per-network-scheduling) | `_DEALS_PER_SP_PER_HOUR`, `_DEAL_JOB_TIMEOUT_SECONDS`, `_RETRIEVALS_PER_SP_PER_HOUR`, `_RETRIEVAL_JOB_TIMEOUT_SECONDS`, `_DATASET_CREATIONS_PER_SP_PER_HOUR`, `_DATA_SET_CREATION_JOB_TIMEOUT_SECONDS`, `_DATASET_LIFECYCLE_CHECK_ENABLED`, `_DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR`, `_DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS`, `_PULL_CHECKS_PER_SP_PER_HOUR`, `_PULL_CHECK_JOB_TIMEOUT_SECONDS`, `_PULL_CHECK_POLL_INTERVAL_SECONDS`, `_PULL_PIECE_CLEANUP_INTERVAL_SECONDS`, `_PROVIDERS_REFRESH_INTERVAL_SECONDS`, `_DATA_RETENTION_POLL_INTERVAL_SECONDS`, `_MAINTENANCE_WINDOWS_UTC`, `_MAINTENANCE_WINDOW_MINUTES`, `_BLOCKED_SP_IDS`, `_BLOCKED_SP_ADDRESSES`, `_MAX_DATASET_STORAGE_SIZE_BYTES`, `_TARGET_DATASET_STORAGE_SIZE_BYTES`, `_PIECE_CLEANUP_PER_SP_PER_HOUR`, `_MAX_PIECE_CLEANUP_RUNTIME_SECONDS` | | [Jobs (pg-boss)](#jobs-pg-boss) | `DEALBOT_PGBOSS_SCHEDULER_ENABLED`, `DEALBOT_PGBOSS_POOL_MAX`, `JOB_SCHEDULER_POLL_SECONDS`, `JOB_WORKER_POLL_SECONDS`, `PG_BOSS_LOCAL_CONCURRENCY`, `JOB_CATCHUP_MAX_ENQUEUE`, `JOB_SCHEDULE_PHASE_SECONDS`, `JOB_ENQUEUE_JITTER_SECONDS`, `SHUTDOWN_FINAL_SCRAPE_DELAY_SECONDS`, `IPFS_BLOCK_FETCH_CONCURRENCY` | | [Pull Check](#pull-check-configuration) | `PULL_CHECK_PIECE_SIZE_BYTES`, `PULL_PIECE_MAX_CONCURRENT_STREAMS`, `PULL_PIECE_MAX_STREAMS_PER_CID` | | [ClickHouse](#clickhouse-configuration) | `CLICKHOUSE_URL`, `CLICKHOUSE_BATCH_SIZE`, `CLICKHOUSE_FLUSH_INTERVAL_MS`, `CLICKHOUSE_MAX_BUFFER_SIZE` | @@ -605,17 +605,6 @@ Dealbot uses pg-boss for rate-based scheduling — see [Jobs (pg-boss)](#jobs-pg --- -### `_METRICS_PER_HOUR` - -- **Type**: `number` -- **Required**: No -- **Default**: `0.1` -- **Limits**: capped at `3` to limit database load from materialized-view refreshes. - -**Role**: Frequency of metrics aggregation runs per hour on this network. - ---- - ### `_PROVIDERS_REFRESH_INTERVAL_SECONDS` - **Type**: `number` From 4274b2d12adcc329a1fe3bac532e7b8c5a8aaf65 Mon Sep 17 00:00:00 2001 From: silent-cipher Date: Fri, 26 Jun 2026 20:57:41 +0530 Subject: [PATCH 3/3] chore: address copilot review comments --- apps/backend/.env.example | 3 +- apps/backend/src/config/env.helpers.ts | 11 ++++-- apps/backend/src/config/network-fields.ts | 2 - .../src/retrieval/retrieval.service.ts | 2 +- docs/environment-variables.md | 38 +++++++++---------- 5 files changed, 29 insertions(+), 27 deletions(-) diff --git a/apps/backend/.env.example b/apps/backend/.env.example index 8e787330..c1bc9fce 100644 --- a/apps/backend/.env.example +++ b/apps/backend/.env.example @@ -22,7 +22,8 @@ # Chain-specific variables never inherit the unprefixed slot and MUST be # prefixed: WALLET_ADDRESS, WALLET_PRIVATE_KEY, SESSION_KEY_PRIVATE_KEY, RPC_URL, # PDP_SUBGRAPH_ENDPOINT, SUBGRAPH_ENDPOINT, DEALBOT_DATASET_VERSION, BLOCKED_SP_IDS, -# BLOCKED_SP_ADDRESSES. +# BLOCKED_SP_ADDRESSES, DATASET_LIFECYCLE_CHECK_ENABLED (network-dependent default: +# off on mainnet, so a shared =true must not enable the canary there). # # Process globals (database, HTTP ports, ClickHouse, pg-boss) have no per-network # form. Variables for INACTIVE networks are ignored; only active networks are diff --git a/apps/backend/src/config/env.helpers.ts b/apps/backend/src/config/env.helpers.ts index 413d75e7..db056419 100644 --- a/apps/backend/src/config/env.helpers.ts +++ b/apps/backend/src/config/env.helpers.ts @@ -16,10 +16,13 @@ export const coerceFloat = (value: string | undefined, fallback: number): number value ? Number.parseFloat(value) : fallback; /** - * Coerces a boolean env value, matching Joi's case-insensitive `true`/`false`. - * Anything unrecognized returns the fallback (Joi rejects such values at boot - * for registered keys, so the loader never sees them there). Notably `"0"`, - * `"no"`, and `"False"` are NOT treated as `true`. + * Coerces a boolean env value, matching Joi's case-insensitive `true`/`false` + * (so `"true"`, `"TRUE"`, `"false"`, `"False"` all parse to the obvious value). + * Any other string — `"0"`, `"1"`, `"no"`, `"yes"`, etc. — is unrecognized and + * returns the fallback rather than parsing as `false`: for a key whose default + * is `true` (e.g. `CHECK_DATASET_CREATION_FEES`), setting `"0"`/`"no"` does NOT + * disable it. (Joi rejects unrecognized values at boot for registered keys, so + * the loader never sees them there.) */ export const coerceBoolean = (value: string | undefined, fallback: boolean): boolean => { if (value === undefined) return fallback; diff --git a/apps/backend/src/config/network-fields.ts b/apps/backend/src/config/network-fields.ts index 86fc5e27..2553807d 100644 --- a/apps/backend/src/config/network-fields.ts +++ b/apps/backend/src/config/network-fields.ts @@ -74,8 +74,6 @@ export const CHAIN_SPECIFIC_NETWORK_VARS = new Set([ "DEALBOT_DATASET_VERSION", "BLOCKED_SP_IDS", "BLOCKED_SP_ADDRESSES", - // Has a network-dependent default (off on mainnet) because the canary can't - // auto-terminate PDP data sets when the payer is a Safe multisig (dealbot#546). // Kept chain-specific so a shared `=true` can't accidentally enable it on mainnet. "DATASET_LIFECYCLE_CHECK_ENABLED", ]); diff --git a/apps/backend/src/retrieval/retrieval.service.ts b/apps/backend/src/retrieval/retrieval.service.ts index de8086e1..1e1f8ecd 100644 --- a/apps/backend/src/retrieval/retrieval.service.ts +++ b/apps/backend/src/retrieval/retrieval.service.ts @@ -97,7 +97,7 @@ export class RetrievalService { const provider = await this.findStorageProvider(deal.spAddress, deal.network); if (!provider) { - throw new Error(`Storage provider ${deal.spAddress} not found`); + throw new Error(`Storage provider ${deal.spAddress} not found on network ${deal.network}`); } const providerLabels = buildCheckMetricLabels({ checkType: "retrieval", diff --git a/docs/environment-variables.md b/docs/environment-variables.md index 09ac3e58..eb85cc21 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -28,7 +28,7 @@ MAINNET_DEALS_PER_SP_PER_HOUR=1 **Rules** - **Resolution precedence (per-network vars).** Each network resolves a variable as `_` (per-network override) → `` (unprefixed shared value) → built-in default. Set a value once unprefixed to share it across every active network, and add a `_` override only where a network differs. -- **Chain-specific vars never inherit.** Credentials, chain endpoints, and chain-local identifiers must be set with a prefix and do not read the unprefixed slot: `WALLET_ADDRESS`, `WALLET_PRIVATE_KEY`, `SESSION_KEY_PRIVATE_KEY`, `RPC_URL`, `PDP_SUBGRAPH_ENDPOINT`, `SUBGRAPH_ENDPOINT`, `DEALBOT_DATASET_VERSION`, `BLOCKED_SP_IDS`, `BLOCKED_SP_ADDRESSES`. +- **Chain-specific vars never inherit.** Credentials, chain endpoints, and chain-local identifiers must be set with a prefix and do not read the unprefixed slot: `WALLET_ADDRESS`, `WALLET_PRIVATE_KEY`, `SESSION_KEY_PRIVATE_KEY`, `RPC_URL`, `PDP_SUBGRAPH_ENDPOINT`, `SUBGRAPH_ENDPOINT`, `DEALBOT_DATASET_VERSION`, `BLOCKED_SP_IDS`, `BLOCKED_SP_ADDRESSES`, `DATASET_LIFECYCLE_CHECK_ENABLED`. (`DATASET_LIFECYCLE_CHECK_ENABLED` is chain-specific because its default is network-dependent — off on mainnet — so a shared `=true` must not silently enable the canary there.) - **Process-global vars.** Database, HTTP ports, ClickHouse, and pg-boss scheduler settings apply to the whole process and have no per-network form. - **Validation.** Both the unprefixed shared value and each `_` override are validated against the same rules at startup. Only networks listed in `NETWORKS` are required; variables for inactive networks are ignored, so you can keep a `MAINNET_*` block commented out until you are ready. - **Wallet vs. session key.** Each active network must provide either `_WALLET_PRIVATE_KEY` or `_SESSION_KEY_PRIVATE_KEY`. When both are present the session key takes precedence (see [`docs/runbooks/wallet-and-session-keys.md`](./runbooks/wallet-and-session-keys.md)). @@ -41,9 +41,9 @@ MAINNET_DEALS_PER_SP_PER_HOUR=1 | [Application](#application-configuration) | `NODE_ENV`, `DEALBOT_PORT`, `DEALBOT_HOST`, `DEALBOT_RUN_MODE`, `DEALBOT_METRICS_PORT`, `DEALBOT_METRICS_HOST`, `DEALBOT_ALLOWED_ORIGINS`, `ENABLE_DEV_MODE`, `DEALBOT_API_PUBLIC_URL`, `DEALBOT_PROBE_LOCATION` | | [Database](#database-configuration) | `DATABASE_HOST`, `DATABASE_PORT`, `DATABASE_POOL_MAX`, `DATABASE_USER`, `DATABASE_PASSWORD`, `DATABASE_NAME` | | [Per-Network](#per-network-configuration) | `_WALLET_ADDRESS`, `_WALLET_PRIVATE_KEY`, `_SESSION_KEY_PRIVATE_KEY`, `_RPC_URL`, `_RPC_REQUEST_TIMEOUT_MS`, `_CHECK_DATASET_CREATION_FEES`, `_USE_ONLY_APPROVED_PROVIDERS`, `_PDP_SUBGRAPH_ENDPOINT`, `_SUBGRAPH_ENDPOINT`, `_DEALBOT_DATASET_VERSION`, `_MIN_NUM_DATASETS_FOR_CHECKS` | -| [Per-Network Scheduling](#per-network-scheduling) | `_DEALS_PER_SP_PER_HOUR`, `_DEAL_JOB_TIMEOUT_SECONDS`, `_RETRIEVALS_PER_SP_PER_HOUR`, `_RETRIEVAL_JOB_TIMEOUT_SECONDS`, `_SAMPLED_RETRIEVALS_PER_SP_PER_HOUR`, `_SAMPLED_RETRIEVAL_JOB_TIMEOUT_SECONDS`, `_DATASET_CREATIONS_PER_SP_PER_HOUR`, `_DATA_SET_CREATION_JOB_TIMEOUT_SECONDS`, `_DATASET_LIFECYCLE_CHECK_ENABLED`, `_DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR`, `_DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS`, `_PULL_CHECKS_PER_SP_PER_HOUR`, `_PULL_CHECK_JOB_TIMEOUT_SECONDS`, `_PULL_CHECK_POLL_INTERVAL_SECONDS`, `_PULL_PIECE_CLEANUP_INTERVAL_SECONDS`, `_PROVIDERS_REFRESH_INTERVAL_SECONDS`, `_DATA_RETENTION_POLL_INTERVAL_SECONDS`, `_MAINTENANCE_WINDOWS_UTC`, `_MAINTENANCE_WINDOW_MINUTES`, `_BLOCKED_SP_IDS`, `_BLOCKED_SP_ADDRESSES`, `_MAX_DATASET_STORAGE_SIZE_BYTES`, `_TARGET_DATASET_STORAGE_SIZE_BYTES`, `_PIECE_CLEANUP_PER_SP_PER_HOUR`, `_MAX_PIECE_CLEANUP_RUNTIME_SECONDS` | +| [Per-Network Scheduling](#per-network-scheduling) | `_DEALS_PER_SP_PER_HOUR`, `_DEAL_JOB_TIMEOUT_SECONDS`, `_RETRIEVALS_PER_SP_PER_HOUR`, `_RETRIEVAL_JOB_TIMEOUT_SECONDS`, `_SAMPLED_RETRIEVALS_PER_SP_PER_HOUR`, `_SAMPLED_RETRIEVAL_JOB_TIMEOUT_SECONDS`, `_DATASET_CREATIONS_PER_SP_PER_HOUR`, `_DATA_SET_CREATION_JOB_TIMEOUT_SECONDS`, `_DATASET_LIFECYCLE_CHECK_ENABLED`, `_DATASET_LIFECYCLE_CHECKS_PER_SP_PER_HOUR`, `_DATA_SET_LIFECYCLE_CHECK_JOB_TIMEOUT_SECONDS`, `_PULL_CHECKS_PER_SP_PER_HOUR`, `_PULL_CHECK_JOB_TIMEOUT_SECONDS`, `_PULL_CHECK_POLL_INTERVAL_SECONDS`, `_PULL_CHECK_PIECE_SIZE_BYTES`, `_PULL_PIECE_CLEANUP_INTERVAL_SECONDS`, `_PROVIDERS_REFRESH_INTERVAL_SECONDS`, `_DATA_RETENTION_POLL_INTERVAL_SECONDS`, `_MAINTENANCE_WINDOWS_UTC`, `_MAINTENANCE_WINDOW_MINUTES`, `_BLOCKED_SP_IDS`, `_BLOCKED_SP_ADDRESSES`, `_MAX_DATASET_STORAGE_SIZE_BYTES`, `_TARGET_DATASET_STORAGE_SIZE_BYTES`, `_PIECE_CLEANUP_PER_SP_PER_HOUR`, `_MAX_PIECE_CLEANUP_RUNTIME_SECONDS` | | [Jobs (pg-boss)](#jobs-pg-boss) | `DEALBOT_PGBOSS_SCHEDULER_ENABLED`, `DEALBOT_PGBOSS_POOL_MAX`, `JOB_SCHEDULER_POLL_SECONDS`, `JOB_WORKER_POLL_SECONDS`, `PG_BOSS_LOCAL_CONCURRENCY`, `JOB_CATCHUP_MAX_ENQUEUE`, `JOB_SCHEDULE_PHASE_SECONDS`, `JOB_ENQUEUE_JITTER_SECONDS`, `SHUTDOWN_FINAL_SCRAPE_DELAY_SECONDS`, `IPFS_BLOCK_FETCH_CONCURRENCY`, `SAMPLED_RETRIEVAL_BLOCK_SAMPLE_COUNT` | -| [Pull Check](#pull-check-configuration) | `PULL_CHECK_PIECE_SIZE_BYTES`, `PULL_PIECE_MAX_CONCURRENT_STREAMS`, `PULL_PIECE_MAX_STREAMS_PER_CID` | +| [Pull Check](#pull-check-configuration) | `PULL_PIECE_MAX_CONCURRENT_STREAMS`, `PULL_PIECE_MAX_STREAMS_PER_CID` | | [ClickHouse](#clickhouse-configuration) | `CLICKHOUSE_URL`, `CLICKHOUSE_BATCH_SIZE`, `CLICKHOUSE_FLUSH_INTERVAL_MS`, `CLICKHOUSE_MAX_BUFFER_SIZE` | | [Dataset](#dataset-configuration) | `DEALBOT_LOCAL_DATASETS_PATH`, `RANDOM_PIECE_SIZES` | | [Timeouts](#timeout-configuration) | `CONNECT_TIMEOUT_MS`, `HTTP_REQUEST_TIMEOUT_MS`, `HTTP2_REQUEST_TIMEOUT_MS`, `IPNI_VERIFICATION_TIMEOUT_MS`, `IPNI_VERIFICATION_POLLING_MS` | @@ -850,6 +850,21 @@ check types (data-storage, retrieval, and data-retention). Matching is case-inse --- +### `_PULL_CHECK_PIECE_SIZE_BYTES` + +- **Type**: `number` (bytes) +- **Required**: No +- **Default**: `10485760` (10 MiB) + +**Role**: Size (bytes) of the synthetic test piece Dealbot generates per pull-check run on this network. Set once unprefixed (`PULL_CHECK_PIECE_SIZE_BYTES`) to share across networks, or override per network with the `_` prefix. + +**When to update**: + +- Increase for more realistic large-piece pull tests +- Decrease for faster jobs in low-bandwidth environments + +--- + ### `_PULL_PIECE_CLEANUP_INTERVAL_SECONDS` - **Type**: `number` (seconds) @@ -1227,22 +1242,7 @@ For each sampled CID, dealbot: ## Pull Check Configuration -These variables tune global aspects of the pull-check subsystem — piece size and server-side streaming limits. Scheduling rates and timeouts are configured per network in the [Per-Network Scheduling](#per-network-scheduling) section. - -### `PULL_CHECK_PIECE_SIZE_BYTES` - -- **Type**: `number` (bytes) -- **Required**: No -- **Default**: `10485760` (10 MiB) - -**Role**: Size (bytes) of the synthetic test piece Dealbot generates per pull-check run. - -**When to update**: - -- Increase for more realistic large-piece pull tests -- Decrease for faster jobs in low-bandwidth environments - ---- +These variables tune global, server-side aspects of the pull-check subsystem — the streaming limits. Piece size (`_PULL_CHECK_PIECE_SIZE_BYTES`) and scheduling rates/timeouts are configured per network in the [Per-Network Scheduling](#per-network-scheduling) section. ### `PULL_PIECE_MAX_CONCURRENT_STREAMS`