Fix issues with empty responses in ERC20Standard (#927)

* Fix an issue with empty ERC20 responses in ERC20Standard * Add comment * One more comment
MetaMask · Dec 8, 2022 · ec10534 · ec10534
1 parent 20d8e7c
commit ec10534
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 6 deletions.
diff --git a/src/assets/Standards/ERC20Standard.test.ts b/src/assets/Standards/ERC20Standard.test.ts
@@ -8,6 +8,7 @@ const MAINNET_PROVIDER = new HttpProvider(
 );
 const ERC20_MATIC_ADDRESS = '0x7d1afa7b718fb893db30a3abc0cfc608aacfebb0';
 const MKR_ADDRESS = '0x9f8F72aA9304c8B593d555F12eF6589cC3A579A2';
+const AMBIRE_ADDRESS = '0xa07D75aacEFd11b425AF7181958F0F85c312f143';
 
 describe('ERC20Standard', () => {
   let erc20Standard: ERC20Standard;
@@ -116,4 +117,43 @@ describe('ERC20Standard', () => {
     expect(decimals).toBe('18');
     expect(symbol).toBe('MKR');
   });
+
+  it('should fail on on empty responses', async () => {
+    nock('https://mainnet.infura.io:443', { encodedQueryParams: true })
+      .post('/v3/341eacb578dd44a1a049cbc5f6fd4035', {
+        jsonrpc: '2.0',
+        id: 5,
+        method: 'eth_call',
+        params: [
+          {
+            to: AMBIRE_ADDRESS,
+            data: '0x95d89b41',
+          },
+          'latest',
+        ],
+      })
+      .reply(200, { jsonrpc: '2.0', id: 5, result: '0x' })
+      .post('/v3/341eacb578dd44a1a049cbc5f6fd4035', {
+        jsonrpc: '2.0',
+        id: 6,
+        method: 'eth_call',
+        params: [
+          {
+            to: AMBIRE_ADDRESS,
+            data: '0x313ce567',
+          },
+          'latest',
+        ],
+      })
+      .reply(200, { jsonrpc: '2.0', id: 6, result: '0x' });
+
+    // Some proxy contracts don't revert when requesting symbol() and decimals(), this test makes sure we handle those cases.
+    await expect(erc20Standard.getTokenSymbol(AMBIRE_ADDRESS)).rejects.toThrow(
+      'Failed to parse token symbol',
+    );
+
+    await expect(
+      erc20Standard.getTokenDecimals(AMBIRE_ADDRESS),
+    ).rejects.toThrow('Failed to parse token decimals');
+  });
 });
diff --git a/src/assets/Standards/ERC20Standard.ts b/src/assets/Standards/ERC20Standard.ts
@@ -47,7 +47,14 @@ export class ERC20Standard {
           reject(error);
           return;
         }
-        resolve(result.toString());
+
+        const resultString = result.toString();
+        // We treat empty string or 0 as invalid
+        if (resultString.length > 0 && resultString !== '0') {
+          resolve(resultString);
+        }
+
+        reject(new Error('Failed to parse token decimals'));
       });
     });
   }
@@ -71,22 +78,24 @@ export class ERC20Standard {
 
         const abiCoder = new AbiCoder();
 
-        // Parse as string
+        // Parse as string - treat empty string as failure
         try {
           const decoded = abiCoder.decode(['string'], result)[0];
-          if (decoded) {
+          if (decoded?.length > 0) {
             resolve(decoded);
             return;
           }
         } catch {
           // Ignore error
         }
 
-        // Parse as bytes
+        // Parse as bytes - treat empty string as failure
         try {
           const utf8 = toUtf8(result);
-          resolve(utf8);
-          return;
+          if (utf8.length > 0) {
+            resolve(utf8);
+            return;
+          }
         } catch {
           // Ignore error
         }