From 47d1cbb11d5b424c346503aa4a56245c8df953d0 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 12 Mar 2024 15:49:57 -0600 Subject: [PATCH 1/7] Restore previous behavior of toChecksumHexAddress Prior to 40acc6c81bfcf2b3995a9c5e94cce069f41f95f6, `toChecksumHexAddress` in `@metamask/controller-utils` would not throw when given `undefined` or `null`. Now it does, because `addHexPrefix` from `ethereumjs-util` has been replaced with `add0x` from `@metamask/utils`, and the latter is more strict when it comes to input. This change is causing some tests on the extension side to fail. Granted, these tests are likely creating an incomplete state object, so they ought to be fixed so that they pass a string to `toChecksumHexAddress`. However, these test failures serve as a reminder that there may be other parts of the extension codebase not covered by tests which are using `toChecksumHexAddress` incorrectly. If the extension were using TypeScript throughout, finding these problem areas would be trivial, because we would have seen type errors already. But because the extension codebase is still primarily written in JavaScript, we cannot guarantee that `toChecksumHexAddress` won't throw for some particular use case even after we fix the obvious usages. Therefore, to prevent unexpected runtime errors, this commit restores the existing behavior of `toChecksumHexAddress`. --- packages/controller-utils/src/util.test.ts | 32 ++++++++++++++++++---- packages/controller-utils/src/util.ts | 10 ++++++- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/controller-utils/src/util.test.ts b/packages/controller-utils/src/util.test.ts index bbf993af5cc..da50a643125 100644 --- a/packages/controller-utils/src/util.test.ts +++ b/packages/controller-utils/src/util.test.ts @@ -277,13 +277,35 @@ describe('util', () => { }); describe('toChecksumHexAddress', () => { - const fullAddress = `0x${VALID}`; - it('should return address for valid address', () => { - expect(util.toChecksumHexAddress(fullAddress)).toBe(fullAddress); + it('should return an 0x-prefixed checksum address untouched', () => { + const address = '0x4e1fF7229BDdAf0A73DF183a88d9c3a04cc975e0'; + expect(util.toChecksumHexAddress(address)).toBe(address); }); - it('should return address for non prefix address', () => { - expect(util.toChecksumHexAddress(VALID)).toBe(fullAddress); + it('should prefix a non-0x-prefixed checksum address with 0x', () => { + expect( + util.toChecksumHexAddress('4e1fF7229BDdAf0A73DF183a88d9c3a04cc975e0'), + ).toBe('0x4e1fF7229BDdAf0A73DF183a88d9c3a04cc975e0'); + }); + + it('should convert a non-checksum address to a checksum address', () => { + expect( + util.toChecksumHexAddress('0x4e1ff7229bddaf0a73df183a88d9c3a04cc975e0'), + ).toBe('0x4e1fF7229BDdAf0A73DF183a88d9c3a04cc975e0'); + }); + + it('should return "0x" if given an empty string', () => { + expect(util.toChecksumHexAddress('')).toBe('0x'); + }); + + it('should return the input untouched if it is undefined', () => { + // @ts-expect-error We are intentionally passing invalid input. + expect(util.toChecksumHexAddress(undefined)).toBeUndefined(); + }); + + it('should return the input untouched if it is null', () => { + // @ts-expect-error We are intentionally passing invalid input. + expect(util.toChecksumHexAddress(null)).toBeNull(); }); }); diff --git a/packages/controller-utils/src/util.ts b/packages/controller-utils/src/util.ts index ed2503cd087..1ee638e34db 100644 --- a/packages/controller-utils/src/util.ts +++ b/packages/controller-utils/src/util.ts @@ -257,14 +257,22 @@ export async function safelyExecuteWithTimeout( * @returns A 0x-prefixed hexidecimal checksummed address, if address is valid. Otherwise original input 0x-prefixe, if address is valid. Otherwise original input 0x-prefixed. */ export function toChecksumHexAddress(address: string) { + if (typeof address !== 'string') { + // Mimic behavior of `addHexPrefix` from `ethereumjs-util` (which this + // function was previously using) for backward compatibility + return address; + } + const hexPrefixed = add0x(address); + if (!isHexString(hexPrefixed)) { - // Version 5.1 of ethereumjs-utils would have returned '0xY' for input 'y' + // Version 5.1 of ethereumjs-util would have returned '0xY' for input 'y' // but we shouldn't waste effort trying to change case on a clearly invalid // string. Instead just return the hex prefixed original string which most // closely mimics the original behavior. return hexPrefixed; } + return toChecksumAddress(hexPrefixed); } From 2c548fe3943166aae831f50e180e0d3a9ca63bc6 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 13 Mar 2024 10:19:31 -0600 Subject: [PATCH 2/7] Use unknown for type --- packages/controller-utils/src/util.test.ts | 2 -- packages/controller-utils/src/util.ts | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/controller-utils/src/util.test.ts b/packages/controller-utils/src/util.test.ts index da50a643125..26e90b9da51 100644 --- a/packages/controller-utils/src/util.test.ts +++ b/packages/controller-utils/src/util.test.ts @@ -299,12 +299,10 @@ describe('util', () => { }); it('should return the input untouched if it is undefined', () => { - // @ts-expect-error We are intentionally passing invalid input. expect(util.toChecksumHexAddress(undefined)).toBeUndefined(); }); it('should return the input untouched if it is null', () => { - // @ts-expect-error We are intentionally passing invalid input. expect(util.toChecksumHexAddress(null)).toBeNull(); }); }); diff --git a/packages/controller-utils/src/util.ts b/packages/controller-utils/src/util.ts index 1ee638e34db..1f86ccf23d8 100644 --- a/packages/controller-utils/src/util.ts +++ b/packages/controller-utils/src/util.ts @@ -254,9 +254,9 @@ export async function safelyExecuteWithTimeout( * Convert an address to a checksummed hexidecimal address. * * @param address - The address to convert. - * @returns A 0x-prefixed hexidecimal checksummed address, if address is valid. Otherwise original input 0x-prefixe, if address is valid. Otherwise original input 0x-prefixed. + * @returns A 0x-prefixed hexadecimal checksummed address, if address is valid. Otherwise returns input 0x-prefixed. */ -export function toChecksumHexAddress(address: string) { +export function toChecksumHexAddress(address: unknown) { if (typeof address !== 'string') { // Mimic behavior of `addHexPrefix` from `ethereumjs-util` (which this // function was previously using) for backward compatibility From 13121266c8416353a97177cfdd5e4a8d70ea9b11 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 13 Mar 2024 10:25:13 -0600 Subject: [PATCH 3/7] Update JSDoc --- packages/controller-utils/src/util.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/controller-utils/src/util.ts b/packages/controller-utils/src/util.ts index 1f86ccf23d8..133e788f1d2 100644 --- a/packages/controller-utils/src/util.ts +++ b/packages/controller-utils/src/util.ts @@ -251,15 +251,15 @@ export async function safelyExecuteWithTimeout( } /** - * Convert an address to a checksummed hexidecimal address. + * Convert an address to a checksummed hexadecimal address. * * @param address - The address to convert. - * @returns A 0x-prefixed hexadecimal checksummed address, if address is valid. Otherwise returns input 0x-prefixed. + * @returns The address in 0x-prefixed hexadecimal checksummed form if it is valid, or untouched otherwise. */ export function toChecksumHexAddress(address: unknown) { if (typeof address !== 'string') { // Mimic behavior of `addHexPrefix` from `ethereumjs-util` (which this - // function was previously using) for backward compatibility + // function was previously using) for backward compatibility. return address; } From ca5dc82e759363132ca80801ff9c53563cf8e7a9 Mon Sep 17 00:00:00 2001 From: legobeat <109787230+legobeat@users.noreply.github.com> Date: Thu, 14 Mar 2024 21:47:54 +0900 Subject: [PATCH 4/7] Make toCheckSumAddress signature generic --- packages/controller-utils/src/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/controller-utils/src/util.ts b/packages/controller-utils/src/util.ts index 133e788f1d2..a5003291072 100644 --- a/packages/controller-utils/src/util.ts +++ b/packages/controller-utils/src/util.ts @@ -256,7 +256,7 @@ export async function safelyExecuteWithTimeout( * @param address - The address to convert. * @returns The address in 0x-prefixed hexadecimal checksummed form if it is valid, or untouched otherwise. */ -export function toChecksumHexAddress(address: unknown) { +export function toChecksumHexAddress(address: T) { if (typeof address !== 'string') { // Mimic behavior of `addHexPrefix` from `ethereumjs-util` (which this // function was previously using) for backward compatibility. From cbb1acd8c5e003c575853a17a3e0262a87970eba Mon Sep 17 00:00:00 2001 From: legobt <6wbvkn0j@anonaddy.me> Date: Thu, 14 Mar 2024 12:53:47 +0000 Subject: [PATCH 5/7] fix toChecksumAddress return type --- packages/controller-utils/src/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/controller-utils/src/util.ts b/packages/controller-utils/src/util.ts index a5003291072..f06cf63919a 100644 --- a/packages/controller-utils/src/util.ts +++ b/packages/controller-utils/src/util.ts @@ -256,7 +256,7 @@ export async function safelyExecuteWithTimeout( * @param address - The address to convert. * @returns The address in 0x-prefixed hexadecimal checksummed form if it is valid, or untouched otherwise. */ -export function toChecksumHexAddress(address: T) { +export function toChecksumHexAddress(address: T) { if (typeof address !== 'string') { // Mimic behavior of `addHexPrefix` from `ethereumjs-util` (which this // function was previously using) for backward compatibility. From 3eae5bfb45d84fd9a8b6087605adc62a732e2be1 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 14 Mar 2024 11:01:55 -0600 Subject: [PATCH 6/7] Add an overload to handle non-string input --- packages/controller-utils/src/util.ts | 21 +++++++++++++++++-- .../src/PreferencesController.ts | 4 +++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/controller-utils/src/util.ts b/packages/controller-utils/src/util.ts index f06cf63919a..3cba5d272ad 100644 --- a/packages/controller-utils/src/util.ts +++ b/packages/controller-utils/src/util.ts @@ -254,9 +254,26 @@ export async function safelyExecuteWithTimeout( * Convert an address to a checksummed hexadecimal address. * * @param address - The address to convert. - * @returns The address in 0x-prefixed hexadecimal checksummed form if it is valid, or untouched otherwise. + * @returns The address in 0x-prefixed hexadecimal checksummed form if it is valid. */ -export function toChecksumHexAddress(address: T) { +export function toChecksumHexAddress(address: string): Hex; + +/** + * Convert an address to a checksummed hexadecimal address. + * + * Note that this particular overload does nothing. + * + * @param address - A value that is not a string (e.g. `undefined` or `null`). + * @returns The `address` untouched. + * @deprecated This overload is designed to gracefully handle an invalid input + * and is only present for backward compatibility. It may be removed in a future + * major version. Please pass a string to `toChecksumHexAddress` instead. + */ +export function toChecksumHexAddress(address: T): T; + +// Tools only see JSDocs for overloads and ignore them for the implementation. +// eslint-disable-next-line jsdoc/require-jsdoc +export function toChecksumHexAddress(address: unknown) { if (typeof address !== 'string') { // Mimic behavior of `addHexPrefix` from `ethereumjs-util` (which this // function was previously using) for backward compatibility. diff --git a/packages/preferences-controller/src/PreferencesController.ts b/packages/preferences-controller/src/PreferencesController.ts index 2bf761950a4..b7451b2c27f 100644 --- a/packages/preferences-controller/src/PreferencesController.ts +++ b/packages/preferences-controller/src/PreferencesController.ts @@ -254,7 +254,9 @@ export class PreferencesController extends BaseController< * @param addresses - List of addresses to use to generate new identities. */ addIdentities(addresses: string[]) { - const checksummedAddresses = addresses.map(toChecksumHexAddress); + const checksummedAddresses = addresses.map((address) => + toChecksumHexAddress(address), + ); this.update((state) => { const { identities } = state; for (const address of checksummedAddresses) { From 3d1305310850e3de35f70999354d49ada72f2c63 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 14 Mar 2024 11:08:00 -0600 Subject: [PATCH 7/7] Change 'normal' overload back to string from Hex (as this was the previous type) --- packages/controller-utils/src/util.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/controller-utils/src/util.ts b/packages/controller-utils/src/util.ts index 3cba5d272ad..c9bf11c9b2a 100644 --- a/packages/controller-utils/src/util.ts +++ b/packages/controller-utils/src/util.ts @@ -256,7 +256,7 @@ export async function safelyExecuteWithTimeout( * @param address - The address to convert. * @returns The address in 0x-prefixed hexadecimal checksummed form if it is valid. */ -export function toChecksumHexAddress(address: string): Hex; +export function toChecksumHexAddress(address: string): string; /** * Convert an address to a checksummed hexadecimal address.