Skip to content

Commit

Permalink
Restore previous behavior of toChecksumHexAddress (#4046)
Browse files Browse the repository at this point in the history
Prior to 40acc6c,
`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`.
  • Loading branch information
mcmire authored Mar 15, 2024
1 parent 675c82c commit 7da8ef7
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 10 deletions.
30 changes: 25 additions & 5 deletions packages/controller-utils/src/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,13 +277,33 @@ 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', () => {
expect(util.toChecksumHexAddress(undefined)).toBeUndefined();
});

it('should return the input untouched if it is null', () => {
expect(util.toChecksumHexAddress(null)).toBeNull();
});
});

Expand Down
33 changes: 29 additions & 4 deletions packages/controller-utils/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,20 +251,45 @@ export async function safelyExecuteWithTimeout<Result>(
}

/**
* 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 hexidecimal checksummed address, if address is valid. Otherwise original input 0x-prefixe, if address is valid. Otherwise original input 0x-prefixed.
* @returns The address in 0x-prefixed hexadecimal checksummed form if it is valid.
*/
export function toChecksumHexAddress(address: string) {
export function toChecksumHexAddress(address: string): string;

/**
* 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<T>(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.
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);
}

Expand Down
4 changes: 3 additions & 1 deletion packages/preferences-controller/src/PreferencesController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 7da8ef7

Please sign in to comment.