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

feat(controller-utils): support bn.js v4 input to BN functions #4844

Open
wants to merge 2 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: 2 additions & 0 deletions packages/controller-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@
"@metamask/utils": "^10.0.0",
"@spruceid/siwe-parser": "2.1.0",
"@types/bn.js": "^5.1.5",
"@types/bnjs4": "npm:@types/bn.js@^4.11.6",
"bn.js": "^5.2.1",
"bnjs4": "npm:bn.js@^4.12.0",
"eth-ens-namehash": "^2.0.8",
"fast-deep-equal": "^3.1.3"
},
Expand Down
17 changes: 17 additions & 0 deletions packages/controller-utils/src/util.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import EthQuery from '@metamask/eth-query';
import BigNumber from 'bignumber.js';
import BN from 'bn.js';
import BN4 from 'bnjs4';
import nock from 'nock';

import { FakeProvider } from '../../../tests/fake-provider';
Expand Down Expand Up @@ -32,11 +33,27 @@ describe('util', () => {

it('bNToHex', () => {
expect(util.BNToHex(new BN('1337'))).toBe('0x539');
expect(util.BNToHex(new BN4('1337'))).toBe('0x539');
expect(util.BNToHex(new BigNumber('1337'))).toBe('0x539');
});

it('fractionBN', () => {
expect(util.fractionBN(new BN('1337'), 9, 10).toNumber()).toBe(1203);
expect(util.fractionBN(new BN4('1337'), 9, 10).toNumber()).toBe(1203);
// Ensure return values use the same bn.js implementation as input by detection using non-typed API
/* eslint-disable @typescript-eslint/no-explicit-any */
expect(
(util.fractionBN(new BN4('1337'), 9, 10) as any)._strip,
).toBeUndefined();
expect(
(util.fractionBN(new BN4('1337'), 9, 10) as any).strip,
).toBeDefined();
expect(
(util.fractionBN(new BN('1337'), 9, 10) as any)._strip,
).toBeDefined();
expect(
(util.fractionBN(new BN('1337'), 9, 10) as any).strip,
).toBeUndefined();
});

it('getBuyURL', () => {
Expand Down
46 changes: 37 additions & 9 deletions packages/controller-utils/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '@metamask/utils';
import type { BigNumber } from 'bignumber.js';
import BN from 'bn.js';
import BN4 from 'bnjs4';
import ensNamehash from 'eth-ens-namehash';
import deepEqual from 'fast-deep-equal';

Expand Down Expand Up @@ -69,10 +70,31 @@ export function isSafeChainId(chainId: Hex): boolean {
*/
// TODO: Either fix this lint violation or explain why it's necessary to ignore.
// eslint-disable-next-line @typescript-eslint/naming-convention
export function BNToHex(inputBn: BN | BigNumber) {
export function BNToHex(inputBn: BN | BN4 | BigNumber): string {
return add0x(inputBn.toString(16));
}

function getBNImplementation(targetBN: BN4): typeof BN4;
function getBNImplementation(targetBN: BN): typeof BN;
/**
* Return the bn.js library responsible for the BN in question
* @param targetBN - A BN instance
* @returns A bn.js instance
*/
function getBNImplementation(targetBN: BN | BN4): typeof BN4 | typeof BN {
return Object.keys(targetBN).includes('_strip') ? BN4 : BN;
}

export function fractionBN(
targetBN: BN,
numerator: number | string,
denominator: number | string,
): BN;
export function fractionBN(
targetBN: BN4,
numerator: number | string,
denominator: number | string,
): BN4;
/**
* Used to multiply a BN by a fraction.
*
Expand All @@ -82,12 +104,16 @@ export function BNToHex(inputBn: BN | BigNumber) {
* @returns Product of the multiplication.
*/
export function fractionBN(
targetBN: BN,
targetBN: BN | BN4,
numerator: number | string,
denominator: number | string,
) {
const numBN = new BN(numerator);
const denomBN = new BN(denominator);
): BN | BN4 {
// @ts-expect-error - Signature overload confusion
const BNImplementation = getBNImplementation(targetBN);

const numBN = new BNImplementation(numerator);
const denomBN = new BNImplementation(denominator);
// @ts-expect-error - BNImplementation gets unexpected typed
return targetBN.mul(numBN).div(denomBN);
}

Expand Down Expand Up @@ -192,18 +218,20 @@ export function hexToText(hex: string) {
}
}

export function fromHex(value: string | BN): BN;
export function fromHex(value: BN4): BN4;
/**
* Parses a hex string and converts it into a number that can be operated on in a bignum-safe,
* base-10 way.
*
* @param value - A base-16 number encoded as a string.
* @returns The number as a BN object in base-16 mode.
*/
export function fromHex(value: string | BN): BN {
export function fromHex(value: string | BN | BN4): BN | BN4 {
if (BN.isBN(value)) {
legobeat marked this conversation as resolved.
Show resolved Hide resolved
return value;
}
return new BN(hexToBN(value).toString(10));
return new BN(hexToBN(value as string).toString(10), 10);
}

/**
Expand All @@ -212,14 +240,14 @@ export function fromHex(value: string | BN): BN {
* @param value - An integer, an integer encoded as a base-10 string, or a BN.
* @returns The integer encoded as a hex string.
*/
export function toHex(value: number | bigint | string | BN): Hex {
export function toHex(value: number | bigint | string | BN | BN4): Hex {
if (typeof value === 'string' && isStrictHexString(value)) {
return value;
}
const hexString =
BN.isBN(value) || typeof value === 'bigint'
? value.toString(16)
: new BN(value.toString(), 10).toString(16);
: new BN(value.toString(10), 10).toString(16);
return `0x${hexString}`;
}

Expand Down
13 changes: 12 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2338,9 +2338,11 @@ __metadata:
"@metamask/utils": "npm:^10.0.0"
"@spruceid/siwe-parser": "npm:2.1.0"
"@types/bn.js": "npm:^5.1.5"
"@types/bnjs4": "npm:@types/bn.js@^4.11.6"
"@types/jest": "npm:^27.4.1"
bignumber.js: "npm:^9.1.2"
bn.js: "npm:^5.2.1"
bnjs4: "npm:bn.js@^4.12.0"
deepmerge: "npm:^4.2.2"
eth-ens-namehash: "npm:^2.0.8"
fast-deep-equal: "npm:^3.1.3"
Expand Down Expand Up @@ -4331,6 +4333,15 @@ __metadata:
languageName: node
linkType: hard

"@types/bnjs4@npm:@types/bn.js@^4.11.6":
version: 4.11.6
resolution: "@types/bn.js@npm:4.11.6"
dependencies:
"@types/node": "npm:*"
checksum: 10/9ff3e7a1539a953c381c0d30ea2049162e3cab894cda91ee10f3a84d603f9afa2b2bc2a38fe9b427de94b6e2b7b77aefd217c1c7b07a10ae8d7499f9d6697a41
languageName: node
linkType: hard

"@types/debug@npm:^4.1.7":
version: 4.1.12
resolution: "@types/debug@npm:4.1.12"
Expand Down Expand Up @@ -5419,7 +5430,7 @@ __metadata:
languageName: node
linkType: hard

"bn.js@npm:^4.11.9":
"bn.js@npm:^4.11.9, bnjs4@npm:bn.js@^4.12.0":
version: 4.12.0
resolution: "bn.js@npm:4.12.0"
checksum: 10/10f8db196d3da5adfc3207d35d0a42aa29033eb33685f20ba2c36cadfe2de63dad05df0a20ab5aae01b418d1c4b3d4d205273085262fa020d17e93ff32b67527
Expand Down
Loading