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

Improve padWithZeroes implementation and add tests #180

Merged
merged 9 commits into from
Aug 16, 2021
10 changes: 2 additions & 8 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import * as ethAbi from 'ethereumjs-abi';
import * as nacl from 'tweetnacl';
import * as naclUtil from 'tweetnacl-util';

import { padWithZeroes } from './utils';

export type TypedData = string | EIP712TypedData | EIP712TypedData[];

interface EIP712TypedData {
Expand Down Expand Up @@ -681,14 +683,6 @@ function getPublicKeyFor<T extends MessageTypes>(
return recoverPublicKey(msgHash, msgParams.sig);
}

function padWithZeroes(number: string, length: number): string {
let myString = `${number}`;
while (myString.length < length) {
myString = `0${myString}`;
Copy link
Member Author

Choose a reason for hiding this comment

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

This computes a new string and stores it in memory length - number.length times.

}
return myString;
}

// converts hex strings to the Uint8Array format used by nacl
function nacl_decodeHex(msgHex: string): Uint8Array {
const msgBase64 = Buffer.from(msgHex, 'hex').toString('base64');
Expand Down
38 changes: 38 additions & 0 deletions src/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { padWithZeroes } from './utils';

describe('padWithZeroes', function () {
it('pads a string shorter than the target length with zeroes', function () {
const input = 'abc';
expect(padWithZeroes(input, 5)).toStrictEqual(`00${input}`);
});

it('pads an empty string', function () {
const input = '';
expect(padWithZeroes(input, 4)).toStrictEqual(`0000`);
});

it('returns a string equal to the target length without modifying it', function () {
const input = 'abc';
expect(padWithZeroes(input, 3)).toStrictEqual(input);
});

it('returns a string longer than the target length without modifying it', function () {
const input = 'abcd';
expect(padWithZeroes(input, 3)).toStrictEqual(input);
});

it('throws an error if passed an invalid hex string', function () {
const inputs = ['0xabc', 'xyz', '-'];
for (const input of inputs) {
expect(() => padWithZeroes(input, 3)).toThrow(
new Error(`Expected an unprefixed hex string. Received: ${input}`),
);
}
});

it('throws an error if passed a negative number', function () {
expect(() => padWithZeroes('abc', -1)).toThrow(
new Error('Expected a non-negative integer target length. Received: -1'),
);
});
});
28 changes: 28 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Pads the front of the given hex string with zeroes until it reaches the
* target length. If the input string is already longer than or equal to the
* target length, it is returned unmodified.
*
* If the input string is "0x"-prefixed or not a hex string, an error will be
* thrown.
*
* @param hexString - The hexadecimal string to pad with zeroes.
* @param targetLength - The target length of the hexadecimal string.
* @returns The input string front-padded with zeroes, or the original string
* if it was already greater than or equal to to the target length.
*/
export function padWithZeroes(hexString: string, targetLength: number): string {
if (hexString !== '' && !/^[a-f0-9]+$/iu.test(hexString)) {
throw new Error(
`Expected an unprefixed hex string. Received: ${hexString}`,
Copy link
Member

Choose a reason for hiding this comment

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

The old function didn't include this check. It would coerce anything it was given into a string. I'm pretty confident it was only ever called with a hex string though, because in the one place it's called, it's given the result of a Buffer.prototype.toString('hex') call. So I'd guess this doesn't change behaviour.

Copy link
Member Author

@rekmarks rekmarks Aug 16, 2021

Choose a reason for hiding this comment

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

Yeah, my thinking was exactly that it's non-breaking for current usage, and we now also won't accidentally shoot ourselves in the foot in the future.

);
}

if (targetLength < 0) {
throw new Error(
`Expected a non-negative integer target length. Received: ${targetLength}`,
);
}

return String.prototype.padStart.call(hexString, targetLength, '0');
}