Skip to content

Commit

Permalink
[LIVE-13893] Bugfix- Fix destructTypeFromString not splitting types…
Browse files Browse the repository at this point in the history
… correctly (#7775)

* Fix `destructTypeFromString` not splitting types correctly

* changeset
  • Loading branch information
lambertkevin authored Sep 9, 2024
1 parent 2f7277a commit 9a732c6
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 35 deletions.
5 changes: 5 additions & 0 deletions .changeset/tame-years-perform.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ledgerhq/hw-app-eth": patch
---

Fix `destructTypeFromString` not splitting types correctly when they contained a number and weren't native types (Struct with numbers in the name)
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ const makeRecursiveFieldStructImplem = ({
value: {
data,
type: typeDescription?.name || "",
sizeInBits: typeDescription?.bits,
sizeInBits: typeDescription?.size,
},
});
}
Expand Down
48 changes: 24 additions & 24 deletions libs/ledgerjs/packages/hw-app-eth/src/modules/EIP712/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,36 +27,36 @@ export const EIP712_TYPE_PROPERTIES: Record<
string,
{
key: (size?: number) => number;
sizeInBits: (size?: number) => number | null;
size: (size?: number) => number | null;
}
> = {
CUSTOM: {
key: () => 0,
sizeInBits: () => null,
size: () => null,
},
INT: {
key: () => 1,
sizeInBits: size => Number(size) / 8,
size: size => Number(size) / 8,
},
UINT: {
key: () => 2,
sizeInBits: size => Number(size) / 8,
size: size => Number(size) / 8,
},
ADDRESS: {
key: () => 3,
sizeInBits: () => null,
size: () => null,
},
BOOL: {
key: () => 4,
sizeInBits: () => null,
size: () => null,
},
STRING: {
key: () => 5,
sizeInBits: () => null,
size: () => null,
},
BYTES: {
key: size => (typeof size !== "undefined" ? 6 : 7),
sizeInBits: size => (typeof size !== "undefined" ? Number(size) : null),
size: size => (typeof size !== "undefined" ? Number(size) : null),
},
};

Expand All @@ -66,7 +66,7 @@ export const EIP712_TYPE_PROPERTIES: Record<
* A Map of encoders to transform a value to formatted buffer
*/
export const EIP712_TYPE_ENCODERS = {
INT(value: string | null, sizeInBits = 256): Buffer {
INT(value: string | null, size = 256): Buffer {
const failSafeValue = value ?? "0";

if (typeof failSafeValue === "string" && failSafeValue?.startsWith("0x")) {
Expand All @@ -78,7 +78,7 @@ export const EIP712_TYPE_ENCODERS = {
// "reversibly convert a positive binary number into a negative binary number with equivalent (but negative) value".
// thx wikipedia
if (valueAsBN.lt(0)) {
const sizeInBytes = sizeInBits / 8;
const sizeInBytes = size / 8;
// Creates BN from a buffer serving as a mask filled by maximum value 0xff
const maskAsBN = new BigNumber(`0x${Buffer.alloc(sizeInBytes, 0xff).toString("hex")}`);

Expand Down Expand Up @@ -109,10 +109,10 @@ export const EIP712_TYPE_ENCODERS = {
return Buffer.from(value ?? "", "utf-8");
},

BYTES(value: string | null, sizeInBits?: number): Buffer {
BYTES(value: string | null, size?: number): Buffer {
const failSafeValue = value ?? "";
// Why slice again ?
return hexBuffer(failSafeValue).slice(0, sizeInBits ?? (failSafeValue?.length - 2) / 2);
return hexBuffer(failSafeValue).slice(0, size ?? (failSafeValue?.length - 2) / 2);
},
};

Expand All @@ -132,23 +132,23 @@ export const EIP712_TYPE_ENCODERS = {
*/
export const destructTypeFromString = (
typeName?: string,
): [{ name: string; bits: number | undefined } | null, Array<number | null>] => {
): [{ name: string; size: number | undefined } | null, Array<number | null>] => {
// Will split "any[][1][10]" in "any", "[][1][10]"
const splitNameAndArraysRegex = new RegExp(/^([^[\]]*)(\[.*\])*/g);
// Will match all numbers (or null) inside each array. [0][10][] => [0,10,null]
const splitArraysRegex = new RegExp(/\[(\d*)\]/g);
// Will separate the the name from the potential bits allocation. uint8 => [uint,8]
const splitNameAndNumberRegex = new RegExp(/(\D*)(\d*)/);
// Will separate the the name from the potential bits/bytes allocation. uint8 => [uint,8]
const splitNameAndNumberRegex = new RegExp(/(?=u?int|bytes)([a-zA-Z-0-9]+?)(\d{1,3})$/g);

const [, type, maybeArrays] = splitNameAndArraysRegex.exec(typeName || "") || [];
const [, name, bits] = splitNameAndNumberRegex.exec(type || "") || [];
const typeDescription = name ? { name, bits: bits ? Number(bits) : undefined } : null;
const [, name = type, size] = splitNameAndNumberRegex.exec(type || "") || [];
const typeDescription = name ? { name, size: size ? Number(size) : undefined } : null;

const arrays = maybeArrays ? [...maybeArrays.matchAll(splitArraysRegex)] : [];
// Parse each size to either a Number or null
const arraySizes = arrays.map(([, size]) => (size ? Number(size) : null));
const arrayLengths = arrays.map(([, arrayLength]) => (arrayLength ? Number(arrayLength) : null));

return [typeDescription, arraySizes];
return [typeDescription, arrayLengths];
};

/**
Expand Down Expand Up @@ -200,10 +200,10 @@ export const makeTypeEntryStructBuffer = ({ name, type }: EIP712MessageTypesEntr
EIP712_TYPE_PROPERTIES[typeDescription?.name?.toUpperCase() || ""] ||
EIP712_TYPE_PROPERTIES.CUSTOM;

const typeKey = typeProperties.key(typeDescription?.bits);
const typeSizeInBits = typeProperties.sizeInBits(typeDescription?.bits);
const typeKey = typeProperties.key(typeDescription?.size);
const typeSize = typeProperties.size(typeDescription?.size);

const typeDescData = constructTypeDescByteString(isTypeAnArray, typeSizeInBits, typeKey);
const typeDescData = constructTypeDescByteString(isTypeAnArray, typeSize, typeKey);

const bufferArray: Buffer[] = [Buffer.from(typeDescData, "hex")];

Expand All @@ -212,8 +212,8 @@ export const makeTypeEntryStructBuffer = ({ name, type }: EIP712MessageTypesEntr
bufferArray.push(Buffer.from(typeDescription?.name ?? "", "utf-8"));
}

if (typeof typeSizeInBits === "number") {
bufferArray.push(Buffer.from(intAsHexBytes(typeSizeInBits, 1), "hex"));
if (typeof typeSize === "number") {
bufferArray.push(Buffer.from(intAsHexBytes(typeSize, 1), "hex"));
}

if (isTypeAnArray) {
Expand Down
30 changes: 20 additions & 10 deletions libs/ledgerjs/packages/hw-app-eth/tests/EIP712/utils.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
getPayloadForFilterV2,
makeTypeEntryStructBuffer,
} from "../../src/modules/EIP712/utils";
import { sign } from "crypto";

const convertTwosComplementToDecimalString = (hex: string, initialValue: string) => {
if (!initialValue?.startsWith("-")) {
Expand All @@ -26,35 +25,46 @@ const convertTwosComplementToDecimalString = (hex: string, initialValue: string)
describe("EIP712", () => {
describe("Utils", () => {
describe("destructTypeFromString", () => {
test("'string[]' should return [{name: 'string', bits: undefined}, [null]]", () => {
test("'string[]' should return [{name: 'string', size: undefined}, [null]]", () => {
expect(destructTypeFromString("string[]")).toEqual([
{ name: "string", bits: undefined },
{ name: "string", size: undefined },
[null],
]);
});

test("'uint8[2][][4]' should return [{name: 'uint', bits: 8}, [2, null, 4]]", () => {
test("'uint8[2][][4]' should return [{name: 'uint', size: 8}, [2, null, 4]]", () => {
expect(destructTypeFromString("uint8[2][][4]")).toEqual([
{ name: "uint", bits: 8 },
{ name: "uint", size: 8 },
[2, null, 4],
]);
});

test("'bytes64' should return [{ name: 'bytes', bits: 64 }, []]", () => {
expect(destructTypeFromString("bytes64")).toEqual([{ name: "bytes", bits: 64 }, []]);
test("'bytes64' should return [{ name: 'bytes', size: 64 }, []]", () => {
expect(destructTypeFromString("bytes64")).toEqual([{ name: "bytes", size: 64 }, []]);
});

test("'bool' should return [{ name: 'bool', bits: undefined }, []]", () => {
expect(destructTypeFromString("bool")).toEqual([{ name: "bool", bits: undefined }, []]);
test("'bool' should return [{ name: 'bool', size: undefined }, []]", () => {
expect(destructTypeFromString("bool")).toEqual([{ name: "bool", size: undefined }, []]);
});

test("'bool[any]' should not throw and return ['bool', []]", () => {
expect(destructTypeFromString("bool[any]")).toEqual([
{ name: "bool", bits: undefined },
{ name: "bool", size: undefined },
[],
]);
});

test("V2DutchOrder should not be splitted even though it contains a number not related to the size", () => {
expect(destructTypeFromString("V2DutchOrder")).toEqual([
{ name: "V2DutchOrder", size: undefined },
[],
]);
});

test("KvnV2 should not be splitted even though it contains a number at the end not related to the size", () => {
expect(destructTypeFromString("KvnV2")).toEqual([{ name: "KvnV2", size: undefined }, []]);
});

test("should not throw with undefined", () => {
expect(destructTypeFromString(undefined)).toEqual([null, []]);
});
Expand Down

0 comments on commit 9a732c6

Please sign in to comment.