Ensure gas estimate errors are not call exceptions in disguise (#2954).

Author: ricmoo

packages/providers/src.ts/json-rpc-provider.ts

@@ -22,18 +22,21 @@ import { BaseProvider, Event } from "./base-provider";
 
 const errorGas = [ "call", "estimateGas" ];
 
-function spelunk(value: any): null | { message: string, data: string } {
+function spelunk(value: any, requireData: boolean): null | { message: string, data: null | string } {
     if (value == null) { return null; }
 
     // These *are* the droids we're looking for.
-    if (typeof(value.message) === "string" && value.message.match("reverted") && isHexString(value.data)) {
-        return { message: value.message, data: value.data };
+    if (typeof(value.message) === "string" && value.message.match("reverted")) {
+        const data = isHexString(value.data) ? value.data: null;
+        if (!requireData || data) {
+            return { message: value.message, data };
+        }
     }
 
     // Spelunk further...
     if (typeof(value) === "object") {
         for (const key in value) {
-            const result = spelunk(value[key]);
+            const result = spelunk(value[key], requireData);
             if (result) { return result; }
         }
         return null;
@@ -42,7 +45,7 @@ function spelunk(value: any): null | { message: string, data: string } {
     // Might be a JSON string we can further descend...
     if (typeof(value) === "string") {
         try {
-            return spelunk(JSON.parse(value));
+            return spelunk(JSON.parse(value), requireData);
         } catch (error) { }
     }
 
@@ -51,17 +54,33 @@ function spelunk(value: any): null | { message: string, data: string } {
 
 function checkError(method: string, error: any, params: any): any {
 
+    const transaction = params.transaction || params.signedTransaction;
+
     // Undo the "convenience" some nodes are attempting to prevent backwards
     // incompatibility; maybe for v6 consider forwarding reverts as errors
     if (method === "call") {
-        const result = spelunk(error);
+        const result = spelunk(error, true);
         if (result) { return result.data; }
 
+        // Nothing descriptive..
         logger.throwError("missing revert data in call exception; Transaction reverted without a reason string", Logger.errors.CALL_EXCEPTION, {
-            error, data: "0x"
+            data: "0x", transaction, error
         });
     }
 
+    if (method === "estimateGas") {
+        // Try to find something, with a preference on SERVER_ERROR body
+        let result = spelunk(error.body, false);
+        if (result == null) { result = spelunk(error, false); }
+
+        // Found "reverted", this is a CALL_EXCEPTION
+        if (result) {
+            logger.throwError("cannot estimate gas; transaction may fail or may require manual gas limit", Logger.errors.UNPREDICTABLE_GAS_LIMIT, {
+                reason: result.message, method, transaction, error
+            });
+        }
+    }
+
     // @TODO: Should we spelunk for message too?
 
     let message = error.message;
@@ -74,8 +93,6 @@ function checkError(method: string, error: any, params: any): any {
     }
     message = (message || "").toLowerCase();
 
-    const transaction = params.transaction || params.signedTransaction;
-
     // "insufficient funds for gas * price + value + cost(data)"
     if (message.match(/insufficient funds|base fee exceeds gas limit/i)) {
         logger.throwError("insufficient funds for intrinsic transaction cost", Logger.errors.INSUFFICIENT_FUNDS, {