fix: decode any uint256 as number not string (#289)

* fix: return numbers as `number` not `string` when decoding * fix: throw error when literals in `valueContent` doesn't fit in `valueType` * refactor: decodeData hex detection --------- Co-authored-by: Hugo Masclet <hugo@lukso.io>
ERC725Alliance · Aug 3, 2023 · 37203f1 · 37203f1
1 parent a168de2
commit 37203f1
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 34 deletions.
diff --git a/src/lib/decodeData.test.ts b/src/lib/decodeData.test.ts
@@ -102,7 +102,7 @@ describe('decodeData', () => {
       [schema],
     );
 
-    expect(decodedData.value).to.eql(['0x11223344', '12']); // TODO: we may want to return a number instead of a string.
+    expect(decodedData.value).to.eql(['0x11223344', 12]);
   });
 
   it('parses type Array correctly', () => {
@@ -183,7 +183,7 @@ describe('tuple', () => {
         valueContent: '(Bytes4,Number)',
         valueType: '(bytes4,bytes8)',
         encodedValue: '0xdeadbeaf000000000000000c',
-        decodedValue: ['0xdeadbeaf', '12'],
+        decodedValue: ['0xdeadbeaf', 12],
       },
     ]; // TODO: add more cases? Address, etc.
 
@@ -257,13 +257,12 @@ describe('tuple', () => {
         isTuple: false,
         shouldThrow: true, // valueContent is not a valid hex value
       },
-      // TODO: add feature for this test
-      // {
-      //   valueType: '(bytes4,bytes4)',
-      //   valueContent: '(Number,0x1122334455)',
-      //   isTuple: false,
-      //   shouldThrow: true, // valueContent is bytes5 vs bytes4
-      // },
+      {
+        valueType: '(bytes4,bytes4)',
+        valueContent: '(Number,0x1122334455)',
+        isTuple: false,
+        shouldThrow: true, // valueContent is bytes5 vs bytes4
+      },
     ];
 
     testCases.forEach((testCase) => {

diff --git a/src/lib/decodeData.ts b/src/lib/decodeData.ts
@@ -17,10 +17,10 @@
  * @author Robert McLeod <@robertdavid010>
  * @author Hugo Masclet <@Hugoo>
  * @author Callum Grindle <@CallumGrindle>
- * @date 2020
+ * @date 2023
  */
 
-import { isHex } from 'web3-utils';
+import { isHexStrict } from 'web3-utils';
 import { COMPACT_BYTES_ARRAY_STRING } from '../constants/constants';
 
 import { DecodeDataInput, DecodeDataOutput } from '../types/decodeData';
@@ -59,6 +59,7 @@ export const isValidTuple = (valueType: string, valueContent: string) => {
   const valueTypeParts = valueTypeToDecode
     .substring(1, valueTypeToDecode.length - 1)
     .split(',');
+
   const valueContentParts = valueContent
     .substring(1, valueContent.length - 1)
     .split(',');
@@ -85,12 +86,13 @@ export const isValidTuple = (valueType: string, valueContent: string) => {
       );
     }
 
+    const valueTypeBytesLength = valueTypeParts[i].split('bytes')[1];
+
     if (
       valueTypeParts[i].match(tupleValueTypesRegex) &&
       valueContentParts[i].match(valueContentsBytesRegex)
     ) {
-      const valueTypeBytesLength = valueTypeParts[i].slice(4);
-      const valueContentBytesLength = valueContentParts[i].slice(4);
+      const valueContentBytesLength = valueContentParts[i].slice(5);
 
       if (valueTypeBytesLength > valueContentBytesLength) {
         throw new Error(
@@ -109,16 +111,22 @@ export const isValidTuple = (valueType: string, valueContent: string) => {
       );
     }
 
-    if (
-      valueContentParts[i].slice(0, 2) === '0x' &&
-      !isHex(valueContentParts[i])
-    ) {
+    if (isHexStrict(valueContentParts[i])) {
+      // check if length of a hex literal in valueContent (e.g: 0x122334455)
+      // is compatible with the valueType (e.g: bytes4)
+      const hexLiteralLength = valueContentParts[i].length - 2;
+
+      if (parseInt(valueTypeBytesLength, 10) < hexLiteralLength) {
+        throw new Error(
+          `Invalid tuple (${valueType},${valueContent}: ${valueContent[i]} cannot fit in ${valueType[i]}`,
+        );
+      }
+    } else if (valueContentParts[i].startsWith('0x')) {
+      // Value starts with 0x bit is not hex... hmmm... weird :)
       throw new Error(
         `Invalid tuple for valueType: ${valueType} / valueContent: ${valueContent}. valueContent of type: ${valueContentParts[i]} is not a valid hex value`,
       );
     }
-
-    // TODO: check if length of 0x112233 is compatible with of bytesX
   }
 
   return true;

diff --git a/src/lib/encoder.test.ts b/src/lib/encoder.test.ts
@@ -524,7 +524,7 @@ describe('encoder', () => {
         },
         {
           valueType: 'uint256[]',
-          decodedValue: ['1', '99'], // TODO: return them as an array of `number` type, not an array of `string`
+          decodedValue: [1, 99],
           encodedValue:
             '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000063',
         },
@@ -840,7 +840,7 @@ describe('encoder', () => {
       },
       {
         valueContent: 'Number',
-        decodedValue: '876',
+        decodedValue: 876,
         encodedValue:
           '0x000000000000000000000000000000000000000000000000000000000000036c',
       },

diff --git a/src/lib/encoder.ts b/src/lib/encoder.ts
@@ -101,7 +101,7 @@ const encodeToBytesN = (
     valueToEncode = value;
   }
 
-  const numberOfBytesInType = parseInt(bytesN.slice(5), 10);
+  const numberOfBytesInType = parseInt(bytesN.split('bytes')[1], 10);
   const numberOfBytesInValue = countNumberOfBytes(valueToEncode);
 
   if (numberOfBytesInValue > numberOfBytesInType) {
@@ -463,7 +463,12 @@ const valueTypeEncodingMap = {
   'uint256[]': {
     encode: (value: Array<number | string>) =>
       abiCoder.encodeParameter('uint256[]', value),
-    decode: (value: string) => abiCoder.decodeParameter('uint256[]', value),
+    decode: (value: string) => {
+      // we want to return an array of numbers as [1, 2, 3], not an array of strings as [ '1', '2', '3']
+      return abiCoder
+        .decodeParameter('uint256[]', value)
+        .map((numberAsString) => parseInt(numberAsString, 10));
+    },
   },
   'bytes32[]': {
     encode: (value: string[]) => abiCoder.encodeParameter('bytes32[]', value),
@@ -507,7 +512,6 @@ export const valueContentEncodingMap = (valueContent: string) => {
     case 'Number': {
       return {
         type: 'uint256',
-        // TODO: extra logic to handle and always return a string number
         encode: (value: string) => {
           let parsedValue: number;
           try {
@@ -518,7 +522,7 @@ export const valueContentEncodingMap = (valueContent: string) => {
 
           return padLeft(numberToHex(parsedValue), 64);
         },
-        decode: (value) => hexToNumber(value).toString(),
+        decode: (value) => Number(hexToNumber(value)),
       };
     }
     // NOTE: This is not symmetrical, and always returns a checksummed address

diff --git a/src/lib/utils.test.ts b/src/lib/utils.test.ts
@@ -116,7 +116,7 @@ describe('utils', () => {
           valueType: '(bytes4,bytes8)',
           valueContent: '(Bytes4,Number)',
         },
-        decodedValue: ['0xcafecafe', '11'],
+        decodedValue: ['0xcafecafe', 11],
         encodedValue: '0xcafecafe000000000000000b',
       },
       {
@@ -127,7 +127,7 @@ describe('utils', () => {
           valueType: '(bytes4,bytes8,bytes4)',
           valueContent: '(Bytes4,Number,Number)',
         },
-        decodedValue: ['0xcafecafe', '11', '8'],
+        decodedValue: ['0xcafecafe', 11, 8],
         encodedValue: '0xcafecafe000000000000000b00000008',
       },
       {
@@ -245,7 +245,7 @@ describe('utils', () => {
       {
         valueContent: 'Number',
         valueType: 'uint256[]',
-        decodedValue: ['123', '456'],
+        decodedValue: [123, 456],
         encodedValue:
           '0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000007b00000000000000000000000000000000000000000000000000000000000001c8',
       },

diff --git a/src/lib/utils.ts b/src/lib/utils.ts
@@ -235,10 +235,9 @@ export function encodeKey(
   schema: ERC725JSONSchema,
   value:
     | string
-    | string[]
-    | string[][]
     | number
-    | number[]
+    | (string | number)[]
+    | string[][]
     | JSONURLDataToEncode
     | JSONURLDataToEncode[]
     | boolean,

diff --git a/test/mockSchema.ts b/test/mockSchema.ts
@@ -351,7 +351,7 @@ export const mockSchema: (ERC725JSONSchema & {
     ]),
     returnGraphData:
       '0x0000000000000000000000000000000000000000000000000000000000000063',
-    expectedResult: '99',
+    expectedResult: 99,
   },
 
   // Case 14
@@ -478,8 +478,8 @@ export const mockSchema: (ERC725JSONSchema & {
     ]),
     returnGraphData: abiCoder.encodeParameter('uint256[]', [123, 456]),
     expectedResult: [
-      '123', // (firefox metamask key)
-      '456', // {firefox metamask key}
+      123, // (firefox metamask key)
+      456, // {firefox metamask key}
     ],
   },