Skip to content

Commit 07b4924

Browse files
authored
refactor: Side-effect free utils (#3322)
This pull request refactors and improves utility modules in the codebase, focusing on the `Logger` and `SigningUtil` classes, as well as minor optimizations in binary utilities and error handling. The main changes include a redesign of the logger initialization for better encapsulation, simplification of the signing utility instantiation logic, and minor performance and correctness tweaks. > [!NOTE] > This is just the first step (alongside the recent `utils` refactoring). Other packages will follow to make the whole thing much more convenient to use. ## Changes **Logger improvements:** - Refactored the `Logger` class to encapsulate the root logger initialization in a static method, removing the global `rootLogger` variable and improving encapsulation. The logger now lazily initializes the root logger as needed. [[1]](diffhunk://#diff-312c84735c8ba55bd72f753e86af89e6c8f1b058ac23ac2290d447c64b7fd6d4L33-R68) [[2]](diffhunk://#diff-312c84735c8ba55bd72f753e86af89e6c8f1b058ac23ac2290d447c64b7fd6d4L54-L78) [[3]](diffhunk://#diff-312c84735c8ba55bd72f753e86af89e6c8f1b058ac23ac2290d447c64b7fd6d4L91-R99) - Improved the logger method wrapping logic to ensure consistent logging behavior in browser environments. [[1]](diffhunk://#diff-312c84735c8ba55bd72f753e86af89e6c8f1b058ac23ac2290d447c64b7fd6d4L33-R68) [[2]](diffhunk://#diff-312c84735c8ba55bd72f753e86af89e6c8f1b058ac23ac2290d447c64b7fd6d4L54-L78) **Signing utility simplification:** - Simplified the `SigningUtil.getInstance` method by replacing the static `keyTypeToInstance` map with a switch statement that directly instantiates the correct class for each key type. This removes unnecessary static instances and clarifies the instantiation logic. [[1]](diffhunk://#diff-264c1a909477860ce2ed2c73f193874422c99397476f8d74ec602ccd11b80512L34-L38) [[2]](diffhunk://#diff-264c1a909477860ce2ed2c73f193874422c99397476f8d74ec602ccd11b80512L318-L326) **Utility optimizations:** - Updated `binaryUtils.ts` to create new `TextEncoder` and `TextDecoder` instances on each call instead of using shared instances, preventing potential state issues and improving reliability. **Error handling improvements:** - Moved the creation of the logger instance inside the `executeSafePromise` function to ensure a fresh logger is used for each invocation, and removed the unnecessary top-level logger instance. ## Future steps * Propagate the same technique throughout the other packages.
1 parent 240478c commit 07b4924

File tree

4 files changed

+47
-46
lines changed

4 files changed

+47
-46
lines changed

packages/utils/src/Logger.ts

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -30,31 +30,6 @@ function isJestRunning(): boolean {
3030
return env.JEST_WORKER_ID !== undefined
3131
}
3232

33-
const rootLogger = pino({
34-
name: 'rootLogger',
35-
enabled: !env.NOLOG,
36-
level: env.LOG_LEVEL ?? 'info',
37-
formatters: {
38-
level: (label) => {
39-
return { level: label } // log level as string instead of number
40-
}
41-
},
42-
transport: isPrettyPrintDisabled() ? undefined : {
43-
target: 'pino-pretty',
44-
options: {
45-
colorize: parseBoolean(env.LOG_COLORS) ?? true,
46-
singleLine: true,
47-
translateTime: 'yyyy-mm-dd"T"HH:MM:ss.l',
48-
ignore: 'pid,hostname',
49-
levelFirst: true,
50-
sync: isJestRunning(),
51-
},
52-
},
53-
browser: {
54-
asObject: true
55-
}
56-
})
57-
5833
/**
5934
* This whole monstrosity exists only because pino in browser environment will not print a log message
6035
* when invoking `logger.info(undefined, 'msg') instead you need to call `logger.info(msg)`.
@@ -76,6 +51,39 @@ export type LoggerModule = string | { id: string }
7651
export class Logger {
7752
static NAME_LENGTH = 25
7853

54+
private static rootLogger: pino.Logger | undefined
55+
56+
private static getRootLogger(): pino.Logger {
57+
Logger.rootLogger ??= pino({
58+
name: 'rootLogger',
59+
enabled: !env.NOLOG,
60+
level: env.LOG_LEVEL ?? 'info',
61+
formatters: {
62+
level: (label) => {
63+
return { level: label } // log level as string instead of number
64+
},
65+
},
66+
transport: isPrettyPrintDisabled()
67+
? undefined
68+
: {
69+
target: 'pino-pretty',
70+
options: {
71+
colorize: parseBoolean(env.LOG_COLORS) ?? true,
72+
singleLine: true,
73+
translateTime: 'yyyy-mm-dd"T"HH:MM:ss.l',
74+
ignore: 'pid,hostname',
75+
levelFirst: true,
76+
sync: isJestRunning(),
77+
},
78+
},
79+
browser: {
80+
asObject: true,
81+
},
82+
})
83+
84+
return Logger.rootLogger
85+
}
86+
7987
private readonly logger: pino.Logger
8088
fatal: (msg: string, metadata?: Record<string, unknown>) => void
8189
error: (msg: string, metadata?: Record<string, unknown>) => void
@@ -88,7 +96,7 @@ export class Logger {
8896
loggerModule: LoggerModule,
8997
contextBindings?: Record<string, unknown>,
9098
defaultLogLevel: LogLevel = 'info',
91-
parentLogger: pino.Logger = rootLogger
99+
parentLogger: pino.Logger = Logger.getRootLogger()
92100
) {
93101
this.logger = parentLogger.child({
94102
name: Logger.createName(loggerModule),

packages/utils/src/SigningUtil.ts

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,16 @@ export abstract class SigningUtil {
3131
abstract assertValidKeyPair(publicKey: UserIDRaw, privateKey: Uint8Array): void
3232

3333
static getInstance(type: KeyType): SigningUtil {
34-
const util = keyTypeToInstance[type]
35-
if (!util) {
36-
throw new Error(`Unknown key pair type: ${type}`)
34+
switch (type) {
35+
case 'ECDSA_SECP256K1_EVM':
36+
return new EcdsaSecp256k1Evm()
37+
case 'ECDSA_SECP256R1':
38+
return new EcdsaSecp256r1()
39+
case 'ML_DSA_87':
40+
return new MlDsa87()
41+
default:
42+
throw new Error(`Unknown key pair type: ${type}`)
3743
}
38-
return util
3944
}
4045
}
4146

@@ -315,13 +320,4 @@ export class MlDsa87 extends SigningUtil {
315320
throw new Error(`The given ML-DSA public key and private key don't match!`)
316321
}
317322
}
318-
319-
}
320-
321-
// Declared at the bottom of the file because the classes need to be
322-
// declared first. TS makes sure all KeyPairTypes are present.
323-
const keyTypeToInstance: Record<KeyType, SigningUtil> = {
324-
ECDSA_SECP256K1_EVM: new EcdsaSecp256k1Evm(),
325-
ECDSA_SECP256R1: new EcdsaSecp256r1(),
326-
ML_DSA_87: new MlDsa87()
327323
}

packages/utils/src/binaryUtils.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
1-
const textEncoder = new TextEncoder()
2-
const textDecoder = new TextDecoder()
3-
41
export const binaryToUtf8 = (bytes: Uint8Array): string => {
5-
return textDecoder.decode(bytes)
2+
return new TextDecoder().decode(bytes)
63
}
74

85
export const utf8ToBinary = (utf8: string): Uint8Array => {
9-
return textEncoder.encode(utf8)
6+
return new TextEncoder().encode(utf8)
107
}
118

129
export const binaryToHex = (bytes: Uint8Array, addPrefix = false): string => {

packages/utils/src/executeSafePromise.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
import { Logger } from './Logger'
22

3-
const logger = new Logger('executeSafePromise')
4-
53
/**
64
* Execute a promise that should never reject. If it does, log the error and exit the process
75
* (in Node/Electron) or throw an unhandled error (in browsers).
86
* To be used in places where we want to "annotate" that the intention of a promise is never
97
* to reject (unless something is really wrong).
108
*/
119
export const executeSafePromise = async <T>(createPromise: () => Promise<T>): Promise<T> => {
10+
1211
try {
1312
return await createPromise()
1413
} catch (err: any) {
14+
const logger = new Logger('executeSafePromise')
1515
logger.fatal('Assertion failure!', { message: err?.message, err })
1616

1717
// Check if we're in a Node/Electron environment

0 commit comments

Comments
 (0)