diff --git a/yarn-project/bot/src/config.ts b/yarn-project/bot/src/config.ts index 95ae4173a4f0..843ff39ba3a2 100644 --- a/yarn-project/bot/src/config.ts +++ b/yarn-project/bot/src/config.ts @@ -2,6 +2,7 @@ import { type ConfigMappingsType, SecretValue, booleanConfigHelper, + floatConfigHelper, getConfigFromMappings, getDefaultConfig, numberConfigHelper, @@ -103,7 +104,7 @@ export const BotConfigSchema = zodFor()( privateTransfersPerTx: z.number().int().nonnegative(), publicTransfersPerTx: z.number().int().nonnegative(), feePaymentMethod: z.literal('fee_juice'), - minFeePadding: z.number().int().nonnegative(), + minFeePadding: z.number().nonnegative(), noStart: z.boolean(), txMinedWaitSeconds: z.number(), followChain: z.enum(BotFollowChain), @@ -204,7 +205,7 @@ export const botConfigMappings: ConfigMappingsType = { minFeePadding: { env: 'BOT_MIN_FEE_PADDING', description: 'How much is the bot willing to overpay vs. the current base fee', - ...numberConfigHelper(3), + ...floatConfigHelper(3), }, noStart: { env: 'BOT_NO_START', diff --git a/yarn-project/ethereum/src/l1_tx_utils/config.ts b/yarn-project/ethereum/src/l1_tx_utils/config.ts index 531dc6579f4f..92a35b8c48bb 100644 --- a/yarn-project/ethereum/src/l1_tx_utils/config.ts +++ b/yarn-project/ethereum/src/l1_tx_utils/config.ts @@ -73,7 +73,7 @@ export const l1TxUtilsConfigMappings: ConfigMappingsType = { gasLimitBufferPercentage: { description: 'How much to increase calculated gas limit by (percentage)', env: 'L1_GAS_LIMIT_BUFFER_PERCENTAGE', - ...numberConfigHelper(20), + ...floatConfigHelper(20), }, maxGwei: { description: 'Maximum gas price in gwei to be used for transactions.', @@ -90,12 +90,12 @@ export const l1TxUtilsConfigMappings: ConfigMappingsType = { priorityFeeBumpPercentage: { description: 'How much to increase priority fee by each attempt (percentage)', env: 'L1_PRIORITY_FEE_BUMP_PERCENTAGE', - ...numberConfigHelper(20), + ...floatConfigHelper(20), }, priorityFeeRetryBumpPercentage: { description: 'How much to increase priority fee by each retry attempt (percentage)', env: 'L1_PRIORITY_FEE_RETRY_BUMP_PERCENTAGE', - ...numberConfigHelper(50), + ...floatConfigHelper(50), }, minimumPriorityFeePerGas: { description: diff --git a/yarn-project/foundation/src/config/config.test.ts b/yarn-project/foundation/src/config/config.test.ts index 8c125c8ebba9..3b7e36f86e60 100644 --- a/yarn-project/foundation/src/config/config.test.ts +++ b/yarn-project/foundation/src/config/config.test.ts @@ -1,6 +1,14 @@ import { jest } from '@jest/globals'; -import { type ConfigMappingsType, bigintConfigHelper, getConfigFromMappings, numberConfigHelper } from './index.js'; +import { + type ConfigMappingsType, + bigintConfigHelper, + floatConfigHelper, + getConfigFromMappings, + numberConfigHelper, + optionalNumberConfigHelper, + percentageConfigHelper, +} from './index.js'; describe('Config', () => { describe('getConfigFromMappings', () => { @@ -132,6 +140,124 @@ describe('Config', () => { }); }); + describe('numberConfigHelper', () => { + it('parses integer strings', () => { + const { parseEnv } = numberConfigHelper(5); + expect(parseEnv!('42')).toBe(42); + expect(parseEnv!('0')).toBe(0); + expect(parseEnv!('-7')).toBe(-7); + }); + + it('throws for non-numeric input instead of falling back to the default', () => { + const { parseEnv } = numberConfigHelper(5); + expect(() => parseEnv!('not-a-number')).toThrow(); + }); + + it('throws instead of silently truncating a decimal value', () => { + const { parseEnv } = numberConfigHelper(5); + expect(() => parseEnv!('0.8')).toThrow(); + expect(() => parseEnv!('3.14')).toThrow(); + }); + + it('throws for values that are not safe integers', () => { + const { parseEnv } = numberConfigHelper(5); + expect(() => parseEnv!('1e30')).toThrow(); + expect(() => parseEnv!('Infinity')).toThrow(); + }); + + it('applies the default only when the env var is unset, not when it is invalid', () => { + const originalEnv = process.env; + const envVar = 'L1_MINIMUM_PRIORITY_FEE_PER_GAS_GWEI'; + try { + interface TestConfig { + value: number; + } + const mappings: ConfigMappingsType = { + value: { env: envVar, description: 'test', ...numberConfigHelper(5) }, + }; + + // Unset -> default + process.env = { ...originalEnv }; + delete process.env[envVar]; + expect(getConfigFromMappings(mappings).value).toBe(5); + + // Empty -> default + process.env = { ...originalEnv, [envVar]: '' }; + expect(getConfigFromMappings(mappings).value).toBe(5); + + // Set but invalid -> throws + process.env = { ...originalEnv, [envVar]: 'not-a-number' }; + expect(() => getConfigFromMappings(mappings)).toThrow(); + } finally { + process.env = originalEnv; + } + }); + }); + + describe('floatConfigHelper', () => { + it('parses floating-point strings', () => { + const { parseEnv } = floatConfigHelper(1.5); + expect(parseEnv!('0.8')).toBe(0.8); + expect(parseEnv!('42')).toBe(42); + expect(parseEnv!('-2.5')).toBe(-2.5); + }); + + it('throws for invalid input instead of falling back to the default', () => { + const { parseEnv } = floatConfigHelper(1.5); + expect(() => parseEnv!('not-a-number')).toThrow(); + expect(() => parseEnv!('Infinity')).toThrow(); + }); + + it('runs the validation function on the parsed value', () => { + const { parseEnv } = floatConfigHelper(1.5, val => { + if (val < 0) { + throw new Error('must be non-negative'); + } + }); + expect(parseEnv!('2.5')).toBe(2.5); + expect(() => parseEnv!('-1')).toThrow('must be non-negative'); + }); + }); + + describe('percentageConfigHelper', () => { + it('parses 0-1 values', () => { + const { parseEnv } = percentageConfigHelper(0.5); + expect(parseEnv!('0.25')).toBe(0.25); + expect(parseEnv!('0')).toBe(0); + expect(parseEnv!('1')).toBe(1); + }); + + it('throws for out-of-range values', () => { + const { parseEnv } = percentageConfigHelper(0.5); + expect(() => parseEnv!('1.5')).toThrow(); + expect(() => parseEnv!('-0.1')).toThrow(); + }); + + it('throws for invalid input instead of falling back to the default', () => { + const { parseEnv } = percentageConfigHelper(0.5); + expect(() => parseEnv!('not-a-number')).toThrow(); + }); + }); + + describe('optionalNumberConfigHelper', () => { + it('parses integer strings', () => { + const { parseEnv } = optionalNumberConfigHelper(); + expect(parseEnv!('42')).toBe(42); + expect(parseEnv!('0')).toBe(0); + }); + + it('throws instead of silently truncating a decimal value', () => { + const { parseEnv } = optionalNumberConfigHelper(); + expect(() => parseEnv!('0.5')).toThrow(); + expect(() => parseEnv!('3.14')).toThrow(); + }); + + it('throws for non-numeric input', () => { + const { parseEnv } = optionalNumberConfigHelper(); + expect(() => parseEnv!('not-a-number')).toThrow(); + }); + }); + describe('bigintConfigHelper', () => { it('parses plain integer strings', () => { const { parseEnv } = bigintConfigHelper(); diff --git a/yarn-project/foundation/src/config/index.ts b/yarn-project/foundation/src/config/index.ts index e11130b3ca30..2c4542a52b89 100644 --- a/yarn-project/foundation/src/config/index.ts +++ b/yarn-project/foundation/src/config/index.ts @@ -119,21 +119,29 @@ export function omitConfigMappings( } /** - * Generates parseEnv and default values for a numerical config value. - * @param defaultVal - The default numerical value to use if the environment variable is not set or is invalid - * @returns Object with parseEnv and default values for a numerical config value + * Generates parseEnv and default values for an integer config value. + * + * The default is used only when the environment variable is unset or empty. A set-but-invalid + * value (non-numeric, fractional, or outside the safe-integer range) throws rather than silently + * falling back to the default. + * @param defaultVal - The default value to use when the environment variable is unset + * @returns Object with parseEnv and default values for an integer config value */ export function numberConfigHelper(defaultVal: number): Pick, 'parseEnv' | 'defaultValue'> { return { - parseEnv: (val: string) => safeParseNumber(val, defaultVal), + parseEnv: (val: string) => parseSafeInteger(val), defaultValue: defaultVal, }; } /** - * Generates parseEnv and default values for a numerical config value. - * @param defaultVal - The default numerical value to use if the environment variable is not set or is invalid - * @returns Object with parseEnv and default values for a numerical config value + * Generates parseEnv and default values for a floating-point config value. + * + * The default is used only when the environment variable is unset or empty. A set-but-invalid + * value (not a finite number) throws rather than silently falling back to the default. + * @param defaultVal - The default value to use when the environment variable is unset + * @param validationFn - Optional extra validation applied to the parsed value; should throw on invalid input + * @returns Object with parseEnv and default values for a floating-point config value */ export function floatConfigHelper( defaultVal: number, @@ -141,7 +149,7 @@ export function floatConfigHelper( ): Pick, 'parseEnv' | 'defaultValue'> { return { parseEnv: (val: string): number => { - const parsed = safeParseFloat(val, defaultVal); + const parsed = parseFiniteNumber(val); validationFn?.(parsed); return parsed; }, @@ -150,12 +158,17 @@ export function floatConfigHelper( } /** - * Parses an environment variable to a 0-1 percentage value + * Generates parseEnv and default values for a 0-1 percentage config value. + * + * The default is used only when the environment variable is unset or empty. A set-but-invalid + * value (not a finite number, or outside the 0-1 range) throws rather than silently falling back + * to the default. + * @param defaultVal - The default value to use when the environment variable is unset */ export function percentageConfigHelper(defaultVal: number): Pick, 'parseEnv' | 'defaultValue'> { return { parseEnv: (val: string): number => { - const parsed = safeParseFloat(val, defaultVal); + const parsed = parseFiniteNumber(val); if (parsed < 0 || parsed > 1) { throw new TypeError(`Invalid percentage value: ${parsed} should be between 0 and 1`); } @@ -205,13 +218,7 @@ export function bigintConfigHelper( */ export function optionalNumberConfigHelper(): Pick, 'parseEnv'> { return { - parseEnv: (val: string) => { - const parsedValue = parseInt(val); - if (!Number.isSafeInteger(parsedValue)) { - throw new Error(`Invalid number: ${val}`); - } - return parsedValue; - }, + parseEnv: (val: string) => parseSafeInteger(val), }; } @@ -344,27 +351,36 @@ export function secretFqConfigHelper(defaultValue?: Fq): { } /** - * Safely parses a number from a string. - * If the value is not a number or is not a safe integer, the default value is returned. + * Parses a string into a safe integer, throwing if the value is numeric but not an integer. + * + * Unlike `parseInt`, this does not silently truncate decimals: `'0.8'` throws rather than + * becoming `0`. A fractional value indicates the caller intended a non-integer where only + * integers are supported (use `floatConfigHelper`/`percentageConfigHelper` for those). * @param value - The string value to parse - * @param defaultValue - The default value to return - * @returns Either parsed value or default value + * @returns The parsed integer value */ -function safeParseNumber(value: string, defaultValue: number): number { - const parsedValue = parseInt(value, 10); - return Number.isSafeInteger(parsedValue) ? parsedValue : defaultValue; +function parseSafeInteger(value: string): number { + const parsedValue = parseFloat(value); + if (!Number.isSafeInteger(parsedValue)) { + throw new Error(`Invalid integer config value '${value}'; expected a whole number`); + } + return parsedValue; } /** - * Safely parses a floating point number from a string. - * If the value is not a number, the default value is returned. + * Parses a string into a finite floating-point number, throwing if the value is not a finite number. + * + * Invalid input (e.g. `'abc'`, or an overflow to `Infinity`) throws rather than being silently + * swallowed, so a misconfigured value fails loudly instead of falling back to the config default. * @param value - The string value to parse - * @param defaultValue - The default value to return - * @returns Either parsed value or default value + * @returns The parsed number */ -function safeParseFloat(value: string, defaultValue: number): number { +function parseFiniteNumber(value: string): number { const parsedValue = parseFloat(value); - return Number.isNaN(parsedValue) ? defaultValue : parsedValue; + if (!Number.isFinite(parsedValue)) { + throw new Error(`Invalid number config value '${value}'`); + } + return parsedValue; } /** diff --git a/yarn-project/p2p/src/config.ts b/yarn-project/p2p/src/config.ts index cd34de88bade..9c3019c7f22e 100644 --- a/yarn-project/p2p/src/config.ts +++ b/yarn-project/p2p/src/config.ts @@ -3,6 +3,7 @@ import { SecretValue, bigintConfigHelper, booleanConfigHelper, + floatConfigHelper, getConfigFromMappings, getDefaultConfig, numberConfigHelper, @@ -435,17 +436,17 @@ export const p2pConfigMappings: ConfigMappingsType = { gossipsubTxTopicWeight: { env: 'P2P_GOSSIPSUB_TX_TOPIC_WEIGHT', description: 'The weight of the tx topic for the gossipsub protocol.', - ...numberConfigHelper(1), + ...floatConfigHelper(1), }, gossipsubTxInvalidMessageDeliveriesWeight: { env: 'P2P_GOSSIPSUB_TX_INVALID_MESSAGE_DELIVERIES_WEIGHT', description: 'The weight of the tx invalid message deliveries for the gossipsub protocol.', - ...numberConfigHelper(-20), + ...floatConfigHelper(-20), }, gossipsubTxInvalidMessageDeliveriesDecay: { env: 'P2P_GOSSIPSUB_TX_INVALID_MESSAGE_DELIVERIES_DECAY', description: 'Determines how quickly the penalty for invalid message deliveries decays over time. Between 0 and 1.', - ...numberConfigHelper(0.5), + ...floatConfigHelper(0.5), }, peerPenaltyValues: { env: 'P2P_PEER_PENALTY_VALUES', diff --git a/yarn-project/sequencer-client/src/config.ts b/yarn-project/sequencer-client/src/config.ts index 2dd82b2b0d1a..f0ddd4b9dd50 100644 --- a/yarn-project/sequencer-client/src/config.ts +++ b/yarn-project/sequencer-client/src/config.ts @@ -129,7 +129,7 @@ export const sequencerConfigMappings: ConfigMappingsType = { description: 'Per-block budget multiplier applied to DA gas and blob fields in place of perBlockAllocationMultiplier.' + ' Defaults higher than the general multiplier so the largest contract class deploy fits a single block.', - ...numberConfigHelper(DefaultSequencerConfig.perBlockDAAllocationMultiplier), + ...floatConfigHelper(DefaultSequencerConfig.perBlockDAAllocationMultiplier), }, redistributeCheckpointBudget: { env: 'SEQ_REDISTRIBUTE_CHECKPOINT_BUDGET',