Skip to content

Commit 2ec670f

Browse files
authored
fix(config): fail loudly on invalid or fractional numeric config (#24536)
## Human-written summary Invalid numeric values in config were silently ignored and replaced with the default. Also, config entries defined with `numberConfigHelper` accepted non-integer values but silently truncated them. ## Context The numeric config helpers mishandled bad input in two ways: - `numberConfigHelper` / `optionalNumberConfigHelper` parsed with `parseInt`, which silently truncated decimals — an integer config set to `0.8` became `0` with no warning. In the worst case (A-1312) that turned a fractional retention window into `0`, i.e. "delete immediately". - `numberConfigHelper`, `floatConfigHelper` and `percentageConfigHelper` swallowed any unparseable value and returned the config default (the JSDoc even documented "...or is invalid"), so a typo'd env var ran silently with an unexpected value. ## Approach The config default is for an **unset** env var only — empty/unset is already resolved to the default before `parseEnv` runs, so the only thing `parseEnv` returning a default achieved was hiding bad input. The helpers now parse strictly and throw on any set-but-invalid value: - `numberConfigHelper` / `optionalNumberConfigHelper`: parse with `parseFloat` and require a safe integer (no more silent truncation). - `floatConfigHelper` / `percentageConfigHelper`: require a finite number; percentage additionally enforces 0–1. `bigint`, `enum` and the secret helpers already threw on invalid input. `booleanConfigHelper` is intentionally unchanged: `parseBooleanEnv` is a total function (unrecognized tokens map to `false`), it never substitutes the configured default. Auditing all ~150 `numberConfigHelper` call sites then surfaced options that legitimately take fractional values and would now wrongly reject them; these were moved to `floatConfigHelper`: - p2p gossipsub tx scoring — topic weight, invalid-message-delivery weight, and decay (default `0.5`), which feed libp2p's float `TopicScoreParams`. - sequencer `perBlockDAAllocationMultiplier` (default `1.5`); its sibling `perBlockAllocationMultiplier` already used `floatConfigHelper`. - L1 tx fee percentages — `gasLimitBufferPercentage`, `priorityFeeBumpPercentage`, `priorityFeeRetryBumpPercentage`. - bot `minFeePadding`, consumed as the fractional overpay factor `1 + padding`; its zod schema no longer pins it to an integer. ## Impact A node whose numeric config env var (or CLI flag) is set to a non-numeric, fractional (for integer options), or out-of-range value now fails at startup with a clear error instead of silently running with a truncated or default value. Operators relying on the old lenient behavior must correct the value. Fixes A-1398
1 parent c678921 commit 2ec670f

6 files changed

Lines changed: 184 additions & 40 deletions

File tree

yarn-project/bot/src/config.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {
22
type ConfigMappingsType,
33
SecretValue,
44
booleanConfigHelper,
5+
floatConfigHelper,
56
getConfigFromMappings,
67
getDefaultConfig,
78
numberConfigHelper,
@@ -103,7 +104,7 @@ export const BotConfigSchema = zodFor<BotConfig>()(
103104
privateTransfersPerTx: z.number().int().nonnegative(),
104105
publicTransfersPerTx: z.number().int().nonnegative(),
105106
feePaymentMethod: z.literal('fee_juice'),
106-
minFeePadding: z.number().int().nonnegative(),
107+
minFeePadding: z.number().nonnegative(),
107108
noStart: z.boolean(),
108109
txMinedWaitSeconds: z.number(),
109110
followChain: z.enum(BotFollowChain),
@@ -204,7 +205,7 @@ export const botConfigMappings: ConfigMappingsType<BotConfig> = {
204205
minFeePadding: {
205206
env: 'BOT_MIN_FEE_PADDING',
206207
description: 'How much is the bot willing to overpay vs. the current base fee',
207-
...numberConfigHelper(3),
208+
...floatConfigHelper(3),
208209
},
209210
noStart: {
210211
env: 'BOT_NO_START',

yarn-project/ethereum/src/l1_tx_utils/config.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export const l1TxUtilsConfigMappings: ConfigMappingsType<L1TxUtilsConfig> = {
7373
gasLimitBufferPercentage: {
7474
description: 'How much to increase calculated gas limit by (percentage)',
7575
env: 'L1_GAS_LIMIT_BUFFER_PERCENTAGE',
76-
...numberConfigHelper(20),
76+
...floatConfigHelper(20),
7777
},
7878
maxGwei: {
7979
description: 'Maximum gas price in gwei to be used for transactions.',
@@ -90,12 +90,12 @@ export const l1TxUtilsConfigMappings: ConfigMappingsType<L1TxUtilsConfig> = {
9090
priorityFeeBumpPercentage: {
9191
description: 'How much to increase priority fee by each attempt (percentage)',
9292
env: 'L1_PRIORITY_FEE_BUMP_PERCENTAGE',
93-
...numberConfigHelper(20),
93+
...floatConfigHelper(20),
9494
},
9595
priorityFeeRetryBumpPercentage: {
9696
description: 'How much to increase priority fee by each retry attempt (percentage)',
9797
env: 'L1_PRIORITY_FEE_RETRY_BUMP_PERCENTAGE',
98-
...numberConfigHelper(50),
98+
...floatConfigHelper(50),
9999
},
100100
minimumPriorityFeePerGas: {
101101
description:

yarn-project/foundation/src/config/config.test.ts

Lines changed: 127 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
import { jest } from '@jest/globals';
22

3-
import { type ConfigMappingsType, bigintConfigHelper, getConfigFromMappings, numberConfigHelper } from './index.js';
3+
import {
4+
type ConfigMappingsType,
5+
bigintConfigHelper,
6+
floatConfigHelper,
7+
getConfigFromMappings,
8+
numberConfigHelper,
9+
optionalNumberConfigHelper,
10+
percentageConfigHelper,
11+
} from './index.js';
412

513
describe('Config', () => {
614
describe('getConfigFromMappings', () => {
@@ -132,6 +140,124 @@ describe('Config', () => {
132140
});
133141
});
134142

143+
describe('numberConfigHelper', () => {
144+
it('parses integer strings', () => {
145+
const { parseEnv } = numberConfigHelper(5);
146+
expect(parseEnv!('42')).toBe(42);
147+
expect(parseEnv!('0')).toBe(0);
148+
expect(parseEnv!('-7')).toBe(-7);
149+
});
150+
151+
it('throws for non-numeric input instead of falling back to the default', () => {
152+
const { parseEnv } = numberConfigHelper(5);
153+
expect(() => parseEnv!('not-a-number')).toThrow();
154+
});
155+
156+
it('throws instead of silently truncating a decimal value', () => {
157+
const { parseEnv } = numberConfigHelper(5);
158+
expect(() => parseEnv!('0.8')).toThrow();
159+
expect(() => parseEnv!('3.14')).toThrow();
160+
});
161+
162+
it('throws for values that are not safe integers', () => {
163+
const { parseEnv } = numberConfigHelper(5);
164+
expect(() => parseEnv!('1e30')).toThrow();
165+
expect(() => parseEnv!('Infinity')).toThrow();
166+
});
167+
168+
it('applies the default only when the env var is unset, not when it is invalid', () => {
169+
const originalEnv = process.env;
170+
const envVar = 'L1_MINIMUM_PRIORITY_FEE_PER_GAS_GWEI';
171+
try {
172+
interface TestConfig {
173+
value: number;
174+
}
175+
const mappings: ConfigMappingsType<TestConfig> = {
176+
value: { env: envVar, description: 'test', ...numberConfigHelper(5) },
177+
};
178+
179+
// Unset -> default
180+
process.env = { ...originalEnv };
181+
delete process.env[envVar];
182+
expect(getConfigFromMappings(mappings).value).toBe(5);
183+
184+
// Empty -> default
185+
process.env = { ...originalEnv, [envVar]: '' };
186+
expect(getConfigFromMappings(mappings).value).toBe(5);
187+
188+
// Set but invalid -> throws
189+
process.env = { ...originalEnv, [envVar]: 'not-a-number' };
190+
expect(() => getConfigFromMappings(mappings)).toThrow();
191+
} finally {
192+
process.env = originalEnv;
193+
}
194+
});
195+
});
196+
197+
describe('floatConfigHelper', () => {
198+
it('parses floating-point strings', () => {
199+
const { parseEnv } = floatConfigHelper(1.5);
200+
expect(parseEnv!('0.8')).toBe(0.8);
201+
expect(parseEnv!('42')).toBe(42);
202+
expect(parseEnv!('-2.5')).toBe(-2.5);
203+
});
204+
205+
it('throws for invalid input instead of falling back to the default', () => {
206+
const { parseEnv } = floatConfigHelper(1.5);
207+
expect(() => parseEnv!('not-a-number')).toThrow();
208+
expect(() => parseEnv!('Infinity')).toThrow();
209+
});
210+
211+
it('runs the validation function on the parsed value', () => {
212+
const { parseEnv } = floatConfigHelper(1.5, val => {
213+
if (val < 0) {
214+
throw new Error('must be non-negative');
215+
}
216+
});
217+
expect(parseEnv!('2.5')).toBe(2.5);
218+
expect(() => parseEnv!('-1')).toThrow('must be non-negative');
219+
});
220+
});
221+
222+
describe('percentageConfigHelper', () => {
223+
it('parses 0-1 values', () => {
224+
const { parseEnv } = percentageConfigHelper(0.5);
225+
expect(parseEnv!('0.25')).toBe(0.25);
226+
expect(parseEnv!('0')).toBe(0);
227+
expect(parseEnv!('1')).toBe(1);
228+
});
229+
230+
it('throws for out-of-range values', () => {
231+
const { parseEnv } = percentageConfigHelper(0.5);
232+
expect(() => parseEnv!('1.5')).toThrow();
233+
expect(() => parseEnv!('-0.1')).toThrow();
234+
});
235+
236+
it('throws for invalid input instead of falling back to the default', () => {
237+
const { parseEnv } = percentageConfigHelper(0.5);
238+
expect(() => parseEnv!('not-a-number')).toThrow();
239+
});
240+
});
241+
242+
describe('optionalNumberConfigHelper', () => {
243+
it('parses integer strings', () => {
244+
const { parseEnv } = optionalNumberConfigHelper();
245+
expect(parseEnv!('42')).toBe(42);
246+
expect(parseEnv!('0')).toBe(0);
247+
});
248+
249+
it('throws instead of silently truncating a decimal value', () => {
250+
const { parseEnv } = optionalNumberConfigHelper();
251+
expect(() => parseEnv!('0.5')).toThrow();
252+
expect(() => parseEnv!('3.14')).toThrow();
253+
});
254+
255+
it('throws for non-numeric input', () => {
256+
const { parseEnv } = optionalNumberConfigHelper();
257+
expect(() => parseEnv!('not-a-number')).toThrow();
258+
});
259+
});
260+
135261
describe('bigintConfigHelper', () => {
136262
it('parses plain integer strings', () => {
137263
const { parseEnv } = bigintConfigHelper();

yarn-project/foundation/src/config/index.ts

Lines changed: 46 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -119,29 +119,37 @@ export function omitConfigMappings<T, K extends keyof T>(
119119
}
120120

121121
/**
122-
* Generates parseEnv and default values for a numerical config value.
123-
* @param defaultVal - The default numerical value to use if the environment variable is not set or is invalid
124-
* @returns Object with parseEnv and default values for a numerical config value
122+
* Generates parseEnv and default values for an integer config value.
123+
*
124+
* The default is used only when the environment variable is unset or empty. A set-but-invalid
125+
* value (non-numeric, fractional, or outside the safe-integer range) throws rather than silently
126+
* falling back to the default.
127+
* @param defaultVal - The default value to use when the environment variable is unset
128+
* @returns Object with parseEnv and default values for an integer config value
125129
*/
126130
export function numberConfigHelper(defaultVal: number): Pick<ConfigMapping<number>, 'parseEnv' | 'defaultValue'> {
127131
return {
128-
parseEnv: (val: string) => safeParseNumber(val, defaultVal),
132+
parseEnv: (val: string) => parseSafeInteger(val),
129133
defaultValue: defaultVal,
130134
};
131135
}
132136

133137
/**
134-
* Generates parseEnv and default values for a numerical config value.
135-
* @param defaultVal - The default numerical value to use if the environment variable is not set or is invalid
136-
* @returns Object with parseEnv and default values for a numerical config value
138+
* Generates parseEnv and default values for a floating-point config value.
139+
*
140+
* The default is used only when the environment variable is unset or empty. A set-but-invalid
141+
* value (not a finite number) throws rather than silently falling back to the default.
142+
* @param defaultVal - The default value to use when the environment variable is unset
143+
* @param validationFn - Optional extra validation applied to the parsed value; should throw on invalid input
144+
* @returns Object with parseEnv and default values for a floating-point config value
137145
*/
138146
export function floatConfigHelper(
139147
defaultVal: number,
140148
validationFn?: (val: number) => void,
141149
): Pick<ConfigMapping<number>, 'parseEnv' | 'defaultValue'> {
142150
return {
143151
parseEnv: (val: string): number => {
144-
const parsed = safeParseFloat(val, defaultVal);
152+
const parsed = parseFiniteNumber(val);
145153
validationFn?.(parsed);
146154
return parsed;
147155
},
@@ -150,12 +158,17 @@ export function floatConfigHelper(
150158
}
151159

152160
/**
153-
* Parses an environment variable to a 0-1 percentage value
161+
* Generates parseEnv and default values for a 0-1 percentage config value.
162+
*
163+
* The default is used only when the environment variable is unset or empty. A set-but-invalid
164+
* value (not a finite number, or outside the 0-1 range) throws rather than silently falling back
165+
* to the default.
166+
* @param defaultVal - The default value to use when the environment variable is unset
154167
*/
155168
export function percentageConfigHelper(defaultVal: number): Pick<ConfigMapping<number>, 'parseEnv' | 'defaultValue'> {
156169
return {
157170
parseEnv: (val: string): number => {
158-
const parsed = safeParseFloat(val, defaultVal);
171+
const parsed = parseFiniteNumber(val);
159172
if (parsed < 0 || parsed > 1) {
160173
throw new TypeError(`Invalid percentage value: ${parsed} should be between 0 and 1`);
161174
}
@@ -205,13 +218,7 @@ export function bigintConfigHelper(
205218
*/
206219
export function optionalNumberConfigHelper(): Pick<ConfigMapping<number>, 'parseEnv'> {
207220
return {
208-
parseEnv: (val: string) => {
209-
const parsedValue = parseInt(val);
210-
if (!Number.isSafeInteger(parsedValue)) {
211-
throw new Error(`Invalid number: ${val}`);
212-
}
213-
return parsedValue;
214-
},
221+
parseEnv: (val: string) => parseSafeInteger(val),
215222
};
216223
}
217224

@@ -344,27 +351,36 @@ export function secretFqConfigHelper(defaultValue?: Fq): {
344351
}
345352

346353
/**
347-
* Safely parses a number from a string.
348-
* If the value is not a number or is not a safe integer, the default value is returned.
354+
* Parses a string into a safe integer, throwing if the value is numeric but not an integer.
355+
*
356+
* Unlike `parseInt`, this does not silently truncate decimals: `'0.8'` throws rather than
357+
* becoming `0`. A fractional value indicates the caller intended a non-integer where only
358+
* integers are supported (use `floatConfigHelper`/`percentageConfigHelper` for those).
349359
* @param value - The string value to parse
350-
* @param defaultValue - The default value to return
351-
* @returns Either parsed value or default value
360+
* @returns The parsed integer value
352361
*/
353-
function safeParseNumber(value: string, defaultValue: number): number {
354-
const parsedValue = parseInt(value, 10);
355-
return Number.isSafeInteger(parsedValue) ? parsedValue : defaultValue;
362+
function parseSafeInteger(value: string): number {
363+
const parsedValue = parseFloat(value);
364+
if (!Number.isSafeInteger(parsedValue)) {
365+
throw new Error(`Invalid integer config value '${value}'; expected a whole number`);
366+
}
367+
return parsedValue;
356368
}
357369

358370
/**
359-
* Safely parses a floating point number from a string.
360-
* If the value is not a number, the default value is returned.
371+
* Parses a string into a finite floating-point number, throwing if the value is not a finite number.
372+
*
373+
* Invalid input (e.g. `'abc'`, or an overflow to `Infinity`) throws rather than being silently
374+
* swallowed, so a misconfigured value fails loudly instead of falling back to the config default.
361375
* @param value - The string value to parse
362-
* @param defaultValue - The default value to return
363-
* @returns Either parsed value or default value
376+
* @returns The parsed number
364377
*/
365-
function safeParseFloat(value: string, defaultValue: number): number {
378+
function parseFiniteNumber(value: string): number {
366379
const parsedValue = parseFloat(value);
367-
return Number.isNaN(parsedValue) ? defaultValue : parsedValue;
380+
if (!Number.isFinite(parsedValue)) {
381+
throw new Error(`Invalid number config value '${value}'`);
382+
}
383+
return parsedValue;
368384
}
369385

370386
/**

yarn-project/p2p/src/config.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
SecretValue,
44
bigintConfigHelper,
55
booleanConfigHelper,
6+
floatConfigHelper,
67
getConfigFromMappings,
78
getDefaultConfig,
89
numberConfigHelper,
@@ -435,17 +436,17 @@ export const p2pConfigMappings: ConfigMappingsType<P2PConfig> = {
435436
gossipsubTxTopicWeight: {
436437
env: 'P2P_GOSSIPSUB_TX_TOPIC_WEIGHT',
437438
description: 'The weight of the tx topic for the gossipsub protocol.',
438-
...numberConfigHelper(1),
439+
...floatConfigHelper(1),
439440
},
440441
gossipsubTxInvalidMessageDeliveriesWeight: {
441442
env: 'P2P_GOSSIPSUB_TX_INVALID_MESSAGE_DELIVERIES_WEIGHT',
442443
description: 'The weight of the tx invalid message deliveries for the gossipsub protocol.',
443-
...numberConfigHelper(-20),
444+
...floatConfigHelper(-20),
444445
},
445446
gossipsubTxInvalidMessageDeliveriesDecay: {
446447
env: 'P2P_GOSSIPSUB_TX_INVALID_MESSAGE_DELIVERIES_DECAY',
447448
description: 'Determines how quickly the penalty for invalid message deliveries decays over time. Between 0 and 1.',
448-
...numberConfigHelper(0.5),
449+
...floatConfigHelper(0.5),
449450
},
450451
peerPenaltyValues: {
451452
env: 'P2P_PEER_PENALTY_VALUES',

yarn-project/sequencer-client/src/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ export const sequencerConfigMappings: ConfigMappingsType<SequencerConfig> = {
129129
description:
130130
'Per-block budget multiplier applied to DA gas and blob fields in place of perBlockAllocationMultiplier.' +
131131
' Defaults higher than the general multiplier so the largest contract class deploy fits a single block.',
132-
...numberConfigHelper(DefaultSequencerConfig.perBlockDAAllocationMultiplier),
132+
...floatConfigHelper(DefaultSequencerConfig.perBlockDAAllocationMultiplier),
133133
},
134134
redistributeCheckpointBudget: {
135135
env: 'SEQ_REDISTRIBUTE_CHECKPOINT_BUDGET',

0 commit comments

Comments
 (0)