Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Resolve Type Warnings for ConfigService.get() #3350

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/config-service/src/commands/printEnvs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
68 changes: 64 additions & 4 deletions packages/config-service/src/services/globalConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -18,15 +18,30 @@
*
*/

type TypeStrToType<Tstr extends string> = Tstr extends 'string'
? string
: Tstr extends 'boolean'
? boolean
: Tstr extends 'number'
? number
: Tstr extends 'array'
? unknown[]
: never;

type GetTypeStrOfKey<K extends string> = K extends keyof typeof _CONFIG
? typeof _CONFIG[K]['type']
: never;

export type TypeOfKey<K extends string> = TypeStrToType<GetTypeStrOfKey<K>>;

export interface ConfigProperty {
envName: string;
type: string;
required: boolean;
defaultValue: string | number | boolean | null;
}

export class GlobalConfig {
public static readonly ENTRIES: Record<string, ConfigProperty> = {
const _CONFIG = {
BATCH_REQUESTS_ENABLED: {
envName: 'BATCH_REQUESTS_ENABLED',
type: 'boolean',
Expand Down Expand Up @@ -568,6 +583,12 @@ export class GlobalConfig {
required: false,
defaultValue: null,
},
SERVER_HOST: {
envName: 'SERVER_HOST',
type: 'string',
required: false,
defaultValue: null,
},
SERVER_PORT: {
envName: 'SERVER_PORT',
type: 'number',
Expand Down Expand Up @@ -742,5 +763,44 @@ export class GlobalConfig {
required: false,
defaultValue: null,
},
};
} satisfies { [key: string]: ConfigProperty };

export type ConfigKey = keyof typeof _CONFIG;

export class ConfigService {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this extra class. The runtime conversion is already done by ValidationService.typeCasting.

private static config: typeof _CONFIG = _CONFIG;

public static get<K extends ConfigKey>(name: K): TypeOfKey<K> | undefined {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This signature should be placed in the other ConfigService.

const configItem = this.config[name];
if (!configItem) {
return undefined;
}

const value = process.env[configItem.envName];

if (value === undefined) {
return configItem.defaultValue as TypeOfKey<K>;
}

switch (configItem.type) {
case 'boolean':
return (value.toLowerCase() === 'true') as TypeOfKey<K>;
case 'number':
return Number(value) as TypeOfKey<K>;
case 'string':
return value as TypeOfKey<K>;
case 'array':
try {
return JSON.parse(value) as TypeOfKey<K>;
} catch {
return undefined;
}
default:
return undefined;
}
}
}

export class GlobalConfig {
public static readonly ENTRIES: Record<ConfigKey, ConfigProperty> = _CONFIG;
}
3 changes: 2 additions & 1 deletion packages/config-service/src/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import findConfig from 'find-config';
import pino from 'pino';
import { LoggerService } from './loggerService';
import { ValidationService } from './validationService';
import type { ConfigKey } from './globalConfig';

const mainLogger = pino({
name: 'hedera-json-rpc-relay',
Expand Down Expand Up @@ -97,7 +98,7 @@ export class ConfigService {
* @param name string
* @returns string | undefined
*/
public static get(name: string): string | number | boolean | null | undefined {
public static get(name: ConfigKey): string | number | boolean | null | undefined {
return this.getInstance().envs[name];
}
Comment on lines +101 to 103
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this still doesn’t resolve the primary issue with the .get() method. There are still instances across the codebase where the error “Argument of type ‘string | number | boolean’ is not assignable to parameter of type…” persists, as shown in the example below. This remains an unresolved task that the ticket is meant to address.

image

Copy link
Member

@quiet-node quiet-node Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Luis added a very good solution which can fix the issue, please refer to his solution here #3350 (comment)

}
2 changes: 1 addition & 1 deletion packages/config-service/src/services/loggerService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's revert back to original

*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -21,6 +21,7 @@
import chai, { expect } from 'chai';
import chaiAsPromised from 'chai-as-promised';
import { ConfigService } from '../../../src/services';
import type { ConfigKey } from '../../../src/services/globalConfig';

chai.use(chaiAsPromised);

Expand Down Expand Up @@ -55,7 +56,7 @@ describe('ConfigService tests', async function () {
});

it('should return undefined for non-existing variable', async () => {
const res = ConfigService.get('NON_EXISTING_VAR');
const res = ConfigService.get('NON_EXISTING_VAR' as ConfigKey);
acuarica marked this conversation as resolved.
Show resolved Hide resolved

expect(res).to.equal(undefined);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ describe('LoggerService tests', async function () {
});

it('should be able to return plain information', async () => {
const envName = GlobalConfig.ENTRIES.CHAIN_ID.envName;
const res = ConfigService.get(envName);
const envName = 'CHAIN_ID';
const res = ConfigService.get(envName) as string | undefined;

expect(LoggerService.maskUpEnv(envName, res)).to.equal(`${envName} = ${res}`);
});
Expand Down
2 changes: 1 addition & 1 deletion packages/relay/src/lib/types/ITracerConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion packages/relay/src/lib/types/ITracerConfigWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion packages/relay/src/lib/types/ITransactionReceipt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion packages/relay/src/lib/types/RequestDetails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion packages/relay/src/lib/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion packages/relay/src/logsBloomUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion packages/relay/src/receiptsRootUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
2 changes: 1 addition & 1 deletion packages/relay/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*
/*-
*
* Hedera JSON RPC Relay
*
Expand Down
5 changes: 3 additions & 2 deletions packages/relay/tests/lib/eth/eth-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
*/

import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';

import { nanOrNumberTo0x,numberTo0x } from '../../../dist/formatters';
import constants from '../../../src/lib/constants';
import {
defaultDetailedContractResultByHash,
defaultEvmAddress,
Expand All @@ -29,8 +32,6 @@ import {
mockData,
toHex,
} from '../../helpers';
import { numberTo0x, nanOrNumberTo0x } from '../../../dist/formatters';
import constants from '../../../src/lib/constants';

export const BLOCK_TRANSACTION_COUNT = 77;
export const GAS_USED_1 = 200000;
Expand Down
5 changes: 3 additions & 2 deletions packages/relay/tests/lib/eth/eth_common.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@

import { ConfigService } from '@hashgraph/json-rpc-config-service/dist/services';
import { expect, use } from 'chai';
import { Registry } from 'prom-client';
import pino from 'pino';
import chaiAsPromised from 'chai-as-promised';
import pino from 'pino';
import { Registry } from 'prom-client';

import { RelayImpl } from '../../../src';
import { RequestDetails } from '../../../src/lib/types';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('@ethSendRawTransaction eth_sendRawTransaction spec', async function ()
},
receiver_sig_required: false,
};
const useAsyncTxProcessing = ConfigService.get('USE_ASYNC_TX_PROCESSING') as boolean;
const useAsyncTxProcessing = ConfigService.get('USE_ASYNC_TX_PROCESSING');

beforeEach(() => {
clock = useFakeTimers();
Expand Down
36 changes: 27 additions & 9 deletions packages/relay/tests/lib/hapiService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ describe('HAPI Service', async function () {
withOverriddenEnvsInMochaTest({ HAPI_CLIENT_TRANSACTION_RESET: 2 }, () => {
it('should be able to reinitialise SDK instance upon reaching transaction limit', async function () {
hapiService = new HAPIService(logger, registry, cacheService, eventEmitter, hbarLimitService);
expect(hapiService.getTransactionCount()).to.eq(parseInt(ConfigService.get('HAPI_CLIENT_TRANSACTION_RESET')!));
expect(hapiService.getTransactionCount()).to.eq(
parseInt(ConfigService.get('HAPI_CLIENT_TRANSACTION_RESET')!),
);

const oldClientInstance = hapiService.getMainClientInstance();
let oldSDKInstance = hapiService.getSDKClient(); // decrease transaction limit by taking the instance
Expand All @@ -105,15 +107,19 @@ describe('HAPI Service', async function () {
withOverriddenEnvsInMochaTest({ HAPI_CLIENT_DURATION_RESET: 100 }, () => {
it('should be able to reinitialise SDK instance upon reaching time limit', async function () {
hapiService = new HAPIService(logger, registry, cacheService, eventEmitter, hbarLimitService);
expect(hapiService.getTimeUntilReset()).to.eq(parseInt(ConfigService.get('HAPI_CLIENT_DURATION_RESET')!));
expect(hapiService.getTimeUntilReset()).to.eq(
parseInt(ConfigService.get('HAPI_CLIENT_DURATION_RESET')!),
);

const oldClientInstance = hapiService.getMainClientInstance();
await new Promise((r) => setTimeout(r, 200)); // await to reach time limit
const oldSDKInstance = hapiService.getSDKClient();
const newSDKInstance = hapiService.getSDKClient();
const newClientInstance = hapiService.getMainClientInstance();

expect(hapiService.getTimeUntilReset()).to.eq(parseInt(ConfigService.get('HAPI_CLIENT_DURATION_RESET')!));
expect(hapiService.getTimeUntilReset()).to.eq(
parseInt(ConfigService.get('HAPI_CLIENT_DURATION_RESET')),
);
expect(oldSDKInstance).to.not.be.equal(newSDKInstance);
expect(oldClientInstance).to.not.be.equal(newClientInstance);
});
Expand All @@ -122,7 +128,9 @@ describe('HAPI Service', async function () {
withOverriddenEnvsInMochaTest({ HAPI_CLIENT_ERROR_RESET: '[50]' }, () => {
it('should be able to reinitialise SDK instance upon error status code encounter', async function () {
hapiService = new HAPIService(logger, registry, cacheService, eventEmitter, hbarLimitService);
expect(hapiService.getErrorCodes()[0]).to.eq(JSON.parse(ConfigService.get('HAPI_CLIENT_ERROR_RESET')!)[0]);
expect(hapiService.getErrorCodes()[0]).to.eq(
JSON.parse(ConfigService.get('HAPI_CLIENT_ERROR_RESET')!)[0],
);

const oldClientInstance = hapiService.getMainClientInstance();
const oldSDKInstance = hapiService.getSDKClient();
Expand All @@ -132,7 +140,9 @@ describe('HAPI Service', async function () {

expect(oldSDKInstance).to.not.be.equal(newSDKInstance);
expect(oldClientInstance).to.not.be.equal(newClientInstance);
expect(hapiService.getErrorCodes()[0]).to.eq(JSON.parse(ConfigService.get('HAPI_CLIENT_ERROR_RESET')!)[0]);
expect(hapiService.getErrorCodes()[0]).to.eq(
JSON.parse(ConfigService.get('HAPI_CLIENT_ERROR_RESET')!)[0],
);
});
});

Expand All @@ -146,15 +156,21 @@ describe('HAPI Service', async function () {
it('should be able to reset all counter upon reinitialization of the SDK Client', async function () {
hapiService = new HAPIService(logger, registry, cacheService, eventEmitter, hbarLimitService);

expect(hapiService.getErrorCodes()[0]).to.eq(JSON.parse(ConfigService.get('HAPI_CLIENT_ERROR_RESET')!)[0]);
expect(hapiService.getErrorCodes()[0]).to.eq(
JSON.parse(ConfigService.get('HAPI_CLIENT_ERROR_RESET')!)[0],
);
const oldClientInstance = hapiService.getMainClientInstance();
const oldSDKInstance = hapiService.getSDKClient();
hapiService.decrementErrorCounter(errorStatus);
const newSDKInstance = hapiService.getSDKClient();
const newClientInstance = hapiService.getMainClientInstance();

expect(hapiService.getTimeUntilReset()).to.eq(parseInt(ConfigService.get('HAPI_CLIENT_DURATION_RESET')!));
expect(hapiService.getErrorCodes()[0]).to.eq(JSON.parse(ConfigService.get('HAPI_CLIENT_ERROR_RESET')!)[0]);
expect(hapiService.getTimeUntilReset()).to.eq(
parseInt(ConfigService.get('HAPI_CLIENT_DURATION_RESET')!),
);
expect(hapiService.getErrorCodes()[0]).to.eq(
JSON.parse(ConfigService.get('HAPI_CLIENT_ERROR_RESET')!)[0],
);
expect(hapiService.getTransactionCount()).to.eq(
parseInt(ConfigService.get('HAPI_CLIENT_TRANSACTION_RESET')!) - 1,
); // one less because we took the instance once and decreased the counter
Expand Down Expand Up @@ -204,7 +220,9 @@ describe('HAPI Service', async function () {
() => {
it('should not be able to reinitialise and decrement counters, if it is disabled', async function () {
hapiService = new HAPIService(logger, registry, cacheService, eventEmitter, hbarLimitService);
expect(hapiService.getTransactionCount()).to.eq(parseInt(ConfigService.get('HAPI_CLIENT_TRANSACTION_RESET')!));
expect(hapiService.getTransactionCount()).to.eq(
parseInt(ConfigService.get('HAPI_CLIENT_TRANSACTION_RESET')!),
);

const oldClientInstance = hapiService.getMainClientInstance();
const oldSDKInstance = hapiService.getSDKClient();
Expand Down
2 changes: 1 addition & 1 deletion packages/relay/tests/lib/logsBloomUtils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
*
* Hedera JSON RPC Relay
*
* Copyright (C) 2024 Hedera Hashgraph, LLC
* Copyright (C) 2022-2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down
Loading
Loading