From 8d8d94293ba67b198fa731a31becc7f728eb77a0 Mon Sep 17 00:00:00 2001 From: Alexander Lukin Date: Tue, 21 Nov 2023 16:46:17 +0400 Subject: [PATCH 1/2] refactor!: environment variables validation Refactor environment variable validation rules. Apply consistent patterns to env validation. The main motivation for this change was as follows: 1. Make env validation more strict and consistent. Stick to the same validation patterns in different projects. 2. Provide consistent fallback options for optional variables. Here are the main changes: 1. Previously if a user set an empty string as the variable name in the .env file like this: ``` VARIABLE_NAME= ``` not all variable initializers assigned a correct default value to the variable in this case. Now it is fixed. For the purpose of this improvement, now all optional variable initializers always have the `@Transform` decorator that provides a default value for the variable. Add the two special functions (`toNumber` and `toBoolean`) that support this change. 2. Make validation of boolean variables more strict. CAUTION: This might be a breaking change for users who use values like "yes" to initialize boolean variables. 3. Now the `ETH_NETWORK` variable is validated not only when the validator source is "Lido". Make validation of this variable more strict. CAUTION: Currently the EVM doesn't officially support testnets other than those listed in the `Network` interface. But this might be a breaking change if some users use EVM for other testnets. 4. Implement the new `toArrayOfUrls()` function to provide a reasonable default value for variables that should have a list of URLs in their values. This function also fixes a bug with default values for such variables. Previously, if a user set an empty string as a value of such variables, the validation worked incorrectly. 5. Now we validate that values in `EL_RPC_URLS`, `CL_API_URLS`, and `VALIDATOR_REGISTRY_KEYSAPI_SOURCE_URLS` are indeed URLs. 6. The validation engine now removes trailing slashes from URL values. 7. More consistent validation for the `HTTP_PORT` and `DB_PORT` values. 8. Add `@Optional` decorator to all optional variables. 9. Change `@IsNumber` to `@IsInt`. 10. Move enums from the `env.validation.ts` to `environment.interface.ts`. --- src/common/config/env.validation.ts | 320 +++++++++++------- .../interfaces/environment.interface.ts | 18 + 2 files changed, 219 insertions(+), 119 deletions(-) diff --git a/src/common/config/env.validation.ts b/src/common/config/env.validation.ts index b3eb83cc..80efc416 100644 --- a/src/common/config/env.validation.ts +++ b/src/common/config/env.validation.ts @@ -6,213 +6,230 @@ import { IsEnum, IsInt, IsNotEmpty, - IsNumber, IsObject, + IsOptional, IsPort, + IsPositive, IsString, + IsUrl, Max, Min, - MinLength, ValidateIf, validateSync, } from 'class-validator'; -import { Environment, LogFormat, LogLevel } from './interfaces'; - -export enum Network { - Mainnet = 1, - Goerli = 5, - Holesky = 17000, - Kintsugi = 1337702, -} - -export enum ValidatorRegistrySource { - Lido = 'lido', - File = 'file', - KeysAPI = 'keysapi', -} - -export enum WorkingMode { - Finalized = 'finalized', - Head = 'head', -} - -const toBoolean = (value: any): boolean => { - if (typeof value === 'boolean') { - return value; - } - - if (typeof value === 'number') { - return !!value; - } - - if (!(typeof value === 'string')) { - return false; - } - - switch (value.toLowerCase().trim()) { - case 'true': - case 'yes': - case '1': - return true; - case 'false': - case 'no': - case '0': - case null: - return false; - default: - return false; - } -}; +import { Environment, LogFormat, LogLevel, Network, ValidatorRegistrySource, WorkingMode } from './interfaces'; export class EnvironmentVariables { + @IsOptional() @IsEnum(Environment) - NODE_ENV: Environment = Environment.development; + @Transform(({ value }) => value || Environment.development) + public NODE_ENV: Environment = Environment.development; - @IsNumber() - @Min(1025) - @Max(65535) - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) - public HTTP_PORT = 8080; + @IsOptional() + @IsPort() + @Transform(({ value }) => value || '8080') + public HTTP_PORT = '8080'; + @IsOptional() @IsEnum(LogLevel) - LOG_LEVEL: LogLevel = LogLevel.info; + @Transform(({ value }) => value || LogLevel.info) + public LOG_LEVEL: LogLevel = LogLevel.info; + @IsOptional() @IsEnum(LogFormat) - LOG_FORMAT: LogFormat = LogFormat.json; + @Transform(({ value }) => value || LogFormat.json) + public LOG_FORMAT: LogFormat = LogFormat.json; + @IsOptional() @IsBoolean() - @Transform(({ value }) => toBoolean(value), { toClassOnly: true }) + @Transform(toBoolean({ defaultValue: false })) public DRY_RUN = false; + @ValidateIf((vars) => vars.NODE_ENV !== Environment.test) @IsNotEmpty() @IsString() - @MinLength(2) - @ValidateIf((vars) => vars.NODE_ENV != Environment.test) public DB_HOST!: string; + @ValidateIf((vars) => vars.NODE_ENV !== Environment.test) + @IsNotEmpty() @IsString() - @MinLength(3) - @ValidateIf((vars) => vars.NODE_ENV != Environment.test) public DB_USER!: string; + @ValidateIf((vars) => vars.NODE_ENV !== Environment.test) + @IsOptional() @IsString() - @MinLength(0) - @ValidateIf((vars) => vars.NODE_ENV != Environment.test) - public DB_PASSWORD!: string; + public DB_PASSWORD = ''; + @ValidateIf((vars) => vars.NODE_ENV !== Environment.test) @IsNotEmpty() - @MinLength(1) - @ValidateIf((vars) => vars.NODE_ENV != Environment.test) + @IsString() public DB_NAME!: string; + @IsOptional() @IsPort() + @Transform(({ value }) => value || '8123') public DB_PORT = '8123'; - @IsNumber() - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsOptional() + @IsInt() + @IsPositive() + @Transform(toNumber({ defaultValue: 10 })) public DB_MAX_RETRIES = 10; - @IsNumber() - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsOptional() + @IsInt() + @IsPositive() + @Transform(toNumber({ defaultValue: 1 })) public DB_MIN_BACKOFF_SEC = 1; - @IsNumber() - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsOptional() + @IsInt() + @IsPositive() + @Transform(toNumber({ defaultValue: 120 })) public DB_MAX_BACKOFF_SEC = 120; + @ValidateIf((vars) => vars.NODE_ENV !== Environment.test) @IsNotEmpty() - @IsInt() - @Min(1) - @Max(5000000) - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) - @ValidateIf((vars) => vars.VALIDATOR_REGISTRY_SOURCE == ValidatorRegistrySource.Lido && vars.NODE_ENV != Environment.test) + @IsEnum(Network) + @Transform(({ value }) => parseInt(value, 10)) public ETH_NETWORK!: Network; + @ValidateIf((vars) => vars.VALIDATOR_REGISTRY_SOURCE === ValidatorRegistrySource.Lido && vars.NODE_ENV !== Environment.test) + @IsNotEmpty() @IsArray() @ArrayMinSize(1) - @Transform(({ value }) => value.split(',')) - @ValidateIf((vars) => vars.VALIDATOR_REGISTRY_SOURCE == ValidatorRegistrySource.Lido && vars.NODE_ENV != Environment.test) + @IsUrl( + { + require_protocol: true, + }, + { + each: true, + }, + ) + @Transform(({ value }) => toArrayOfUrls(value)) public EL_RPC_URLS: string[] = []; + @ValidateIf((vars) => vars.NODE_ENV !== Environment.test) + @IsNotEmpty() @IsArray() @ArrayMinSize(1) - @Transform(({ value }) => value.split(',')) - @ValidateIf((vars) => vars.NODE_ENV != Environment.test) - public CL_API_URLS!: string[]; - + @IsUrl( + { + require_protocol: true, + }, + { + each: true, + }, + ) + @Transform(({ value }) => toArrayOfUrls(value)) + public CL_API_URLS: string[] = []; + + @IsOptional() @IsInt() - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsPositive() + @Transform(toNumber({ defaultValue: 500 })) public CL_API_RETRY_DELAY_MS = 500; - @IsNumber() + @IsOptional() + @IsInt() @Min(5000) - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @Transform(toNumber({ defaultValue: 15000 })) public CL_API_GET_RESPONSE_TIMEOUT = 15000; - @IsNumber() - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsOptional() + @IsInt() + @IsPositive() + @Transform(toNumber({ defaultValue: 1 })) public CL_API_MAX_RETRIES = 1; - @IsNumber() - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsOptional() + @IsInt() + @IsPositive() + @Transform(toNumber({ defaultValue: 1 })) public CL_API_GET_BLOCK_INFO_MAX_RETRIES = 1; - @IsNumber() - @Min(74240) // Altair - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) @ValidateIf((vars) => vars.ETH_NETWORK === Network.Mainnet) + @IsOptional() + @IsInt() + @Min(74240) // Altair + @Transform(toNumber({ defaultValue: 155000 })) public START_EPOCH = 155000; - @IsNumber() + @IsOptional() + @IsInt() @Min(32) - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @Transform(toNumber({ defaultValue: 32 })) public FETCH_INTERVAL_SLOTS = 32; + @IsOptional() @IsInt() - @Min(1) - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsPositive() + @Transform(toNumber({ defaultValue: 12 })) public CHAIN_SLOT_TIME_SECONDS = 12; + @IsOptional() @IsEnum(ValidatorRegistrySource) + @Transform(({ value }) => value || ValidatorRegistrySource.Lido) public VALIDATOR_REGISTRY_SOURCE: ValidatorRegistrySource = ValidatorRegistrySource.Lido; + @IsOptional() @IsString() + @Transform(({ value }) => value || './docker/validators/custom_mainnet.yaml') public VALIDATOR_REGISTRY_FILE_SOURCE_PATH = './docker/validators/custom_mainnet.yaml'; + @IsOptional() @IsString() + @Transform(({ value }) => value || './docker/validators/lido_mainnet.db') public VALIDATOR_REGISTRY_LIDO_SOURCE_SQLITE_CACHE_PATH = './docker/validators/lido_mainnet.db'; + @ValidateIf((vars) => vars.VALIDATOR_REGISTRY_SOURCE === ValidatorRegistrySource.KeysAPI && vars.NODE_ENV !== Environment.test) + @IsNotEmpty() @IsArray() @ArrayMinSize(1) - @Transform(({ value }) => value.split(',')) - @ValidateIf((vars) => vars.VALIDATOR_REGISTRY_SOURCE == ValidatorRegistrySource.KeysAPI && vars.NODE_ENV != Environment.test) - public VALIDATOR_REGISTRY_KEYSAPI_SOURCE_URLS!: string[]; - + @IsUrl( + { + require_protocol: true, + }, + { + each: true, + }, + ) + @Transform(({ value }) => toArrayOfUrls(value)) + public VALIDATOR_REGISTRY_KEYSAPI_SOURCE_URLS: string[] = []; + + @IsOptional() @IsInt() - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsPositive() + @Transform(toNumber({ defaultValue: 500 })) public VALIDATOR_REGISTRY_KEYSAPI_SOURCE_RETRY_DELAY_MS = 500; - @IsNumber() + @IsOptional() + @IsInt() @Min(5000) - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @Transform(toNumber({ defaultValue: 30000 })) public VALIDATOR_REGISTRY_KEYSAPI_SOURCE_RESPONSE_TIMEOUT = 30000; - @IsNumber() - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsOptional() + @IsInt() + @Min(0) + @Transform(toNumber({ defaultValue: 2 })) public VALIDATOR_REGISTRY_KEYSAPI_SOURCE_MAX_RETRIES = 2; /** * Use a file with list of validators that are stuck and should be excluded from the monitoring metrics */ + @IsOptional() @IsBoolean() - @Transform(({ value }) => toBoolean(value), { toClassOnly: true }) + @Transform(toBoolean({ defaultValue: false })) public VALIDATOR_USE_STUCK_KEYS_FILE = false; /** * Path to file with list of validators that are stuck and should be excluded from the monitoring metrics */ + @IsOptional() @IsString() + @Transform(({ value }) => value || './docker/validators/stuck_keys.yaml') public VALIDATOR_STUCK_KEYS_FILE_PATH = './docker/validators/stuck_keys.yaml'; /** @@ -225,10 +242,11 @@ export class EnvironmentVariables { * Validator 1 participation is bad, because 78 < (99 - 10) * Validator 2 participation is ok, because 98 > (99 - 10) */ - @IsNumber() + @IsOptional() + @IsInt() @Min(0) @Max(100) - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @Transform(toNumber({ defaultValue: 0 })) public SYNC_PARTICIPATION_DISTANCE_DOWN_FROM_CHAIN_AVG = 0; /** @@ -238,10 +256,11 @@ export class EnvironmentVariables { * SYNC_PARTICIPATION_EPOCHS_LESS_THAN_CHAIN_AVG = 3 * Then we alert about that */ - @IsNumber() - @Min(1) + @IsOptional() + @IsInt() + @IsPositive() @Max(10) - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @Transform(toNumber({ defaultValue: 3 })) public SYNC_PARTICIPATION_EPOCHS_LESS_THAN_CHAIN_AVG = 3; /** @@ -251,19 +270,23 @@ export class EnvironmentVariables { * BAD_ATTESTATION_EPOCHS = 3 * Then we alert about that */ - @IsNumber() - @Min(1) + @IsOptional() + @IsInt() + @IsPositive() @Max(10) - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @Transform(toNumber({ defaultValue: 3 })) public BAD_ATTESTATION_EPOCHS = 3; /** * Critical alerts will be sent for NOs with validators count greater this value */ - @IsNumber() - @Transform(({ value }) => parseInt(value, 10), { toClassOnly: true }) + @IsOptional() + @IsInt() + @IsPositive() + @Transform(toNumber({ defaultValue: 100 })) public CRITICAL_ALERTS_MIN_VAL_COUNT = 100; + @IsOptional() @IsString() public CRITICAL_ALERTS_ALERTMANAGER_URL = ''; @@ -271,11 +294,14 @@ export class EnvironmentVariables { * Additional labels for critical alerts. Must be in JSON string format. * For example - '{"a":"valueA","b":"valueB"}' */ + @IsOptional() @IsObject() - @Transform(({ value }) => JSON.parse(value), { toClassOnly: true }) + @Transform(({ value }) => toObject(value)) public CRITICAL_ALERTS_ALERTMANAGER_LABELS = {}; + @IsOptional() @IsEnum(WorkingMode) + @Transform(({ value }) => value || WorkingMode.Finalized) public WORKING_MODE = WorkingMode.Finalized; } @@ -292,3 +318,59 @@ export function validate(config: Record) { return validatedConfig; } + +// ==================================================================================================================== +// PRIVATE FUNCTIONS +// ==================================================================================================================== +function toNumber({ defaultValue }) { + return function ({ value }) { + if (value == null || value === '') { + return defaultValue; + } + return Number(value); + }; +} + +function toBoolean({ defaultValue }) { + return function ({ value }) { + if (value == null || value === '') { + return defaultValue; + } + + if (typeof value === 'boolean') { + return value; + } + + const str = value.toString().toLowerCase().trim(); + + if (str === 'true') { + return true; + } + + if (str === 'false') { + return false; + } + + return value; + }; +} + +function toArrayOfUrls(url: string | null): string[] { + if (url == null || url === '') { + return []; + } + + return url.split(',').map((str) => str.trim().replace(/\/$/, '')); +} + +function toObject(str: string | null): Object | string { + if (str == null || str === '') { + return {}; + } + + try { + return JSON.parse(str); + } catch (e) { + return str; + } +} diff --git a/src/common/config/interfaces/environment.interface.ts b/src/common/config/interfaces/environment.interface.ts index 58dc31f1..96445a3e 100644 --- a/src/common/config/interfaces/environment.interface.ts +++ b/src/common/config/interfaces/environment.interface.ts @@ -18,3 +18,21 @@ export enum LogFormat { json = 'json', simple = 'simple', } + +export enum Network { + Mainnet = 1, + Goerli = 5, + Holesky = 17000, + Kintsugi = 1337702, +} + +export enum ValidatorRegistrySource { + Lido = 'lido', + File = 'file', + KeysAPI = 'keysapi', +} + +export enum WorkingMode { + Finalized = 'finalized', + Head = 'head', +} From 2ee699d0a56ed920975a3cbaddde8126770be5ee Mon Sep 17 00:00:00 2001 From: Alexander Lukin Date: Sat, 25 Nov 2023 16:06:55 +0400 Subject: [PATCH 2/2] refactor!: environment variables validation Update validation rules for some variables. The main changes are: 1. Loosen validation rules for boolean variables. Now numbers "1" and "0" and constants "yes" and "no" are accepted as valid boolean values. 2. Rename the `Network` interface to `Chain`. --- src/common/config/env.validation.ts | 27 +++++++++++-------- .../interfaces/environment.interface.ts | 2 +- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/common/config/env.validation.ts b/src/common/config/env.validation.ts index 80efc416..ababd113 100644 --- a/src/common/config/env.validation.ts +++ b/src/common/config/env.validation.ts @@ -18,7 +18,7 @@ import { validateSync, } from 'class-validator'; -import { Environment, LogFormat, LogLevel, Network, ValidatorRegistrySource, WorkingMode } from './interfaces'; +import { Environment, LogFormat, LogLevel, Chain, ValidatorRegistrySource, WorkingMode } from './interfaces'; export class EnvironmentVariables { @IsOptional() @@ -91,9 +91,9 @@ export class EnvironmentVariables { @ValidateIf((vars) => vars.NODE_ENV !== Environment.test) @IsNotEmpty() - @IsEnum(Network) + @IsEnum(Chain) @Transform(({ value }) => parseInt(value, 10)) - public ETH_NETWORK!: Network; + public ETH_NETWORK!: Chain; @ValidateIf((vars) => vars.VALIDATOR_REGISTRY_SOURCE === ValidatorRegistrySource.Lido && vars.NODE_ENV !== Environment.test) @IsNotEmpty() @@ -149,7 +149,7 @@ export class EnvironmentVariables { @Transform(toNumber({ defaultValue: 1 })) public CL_API_GET_BLOCK_INFO_MAX_RETRIES = 1; - @ValidateIf((vars) => vars.ETH_NETWORK === Network.Mainnet) + @ValidateIf((vars) => vars.ETH_NETWORK === Chain.Mainnet) @IsOptional() @IsInt() @Min(74240) // Altair @@ -343,15 +343,20 @@ function toBoolean({ defaultValue }) { const str = value.toString().toLowerCase().trim(); - if (str === 'true') { - return true; - } + switch (str) { + case 'true': + case 'yes': + case '1': + return true; - if (str === 'false') { - return false; - } + case 'false': + case 'no': + case '0': + return false; - return value; + default: + return value; + } }; } diff --git a/src/common/config/interfaces/environment.interface.ts b/src/common/config/interfaces/environment.interface.ts index 96445a3e..7dcc10aa 100644 --- a/src/common/config/interfaces/environment.interface.ts +++ b/src/common/config/interfaces/environment.interface.ts @@ -19,7 +19,7 @@ export enum LogFormat { simple = 'simple', } -export enum Network { +export enum Chain { Mainnet = 1, Goerli = 5, Holesky = 17000,