internal/ethapi: unified estimateGasErrors, simplified logic

Author: MariusVanDerWijden

accounts/abi/bind/backends/simulated_test.go

@@ -950,6 +950,31 @@ func TestSimulatedBackend_PendingAndCallContract(t *testing.T) {
 	}
 }
 
+// This test is based on the following contract:
+/*
+contract Reverter {
+    function revertString() public pure{
+        require(false, "some error");
+    }
+    function revertNoString() public pure {
+        require(false, "");
+    }
+    function revertASM() public pure {
+        assembly {
+            revert(0x0, 0x0)
+        }
+    }
+    function noRevert() public pure {
+        assembly {
+            // Assembles something that looks like require(false, "some error") but is not reverted
+            mstore(0x0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
+            mstore(0x4, 0x0000000000000000000000000000000000000000000000000000000000000020)
+            mstore(0x24, 0x000000000000000000000000000000000000000000000000000000000000000a)
+            mstore(0x44, 0x736f6d65206572726f7200000000000000000000000000000000000000000000)
+            return(0x0, 0x64)
+        }
+    }
+}*/
 func TestSimulatedBackend_CallContractRevert(t *testing.T) {
 	testAddr := crypto.PubkeyToAddress(testKey.PublicKey)
 	sim := simTestBackend(testAddr)

console/bridge.go

@@ -425,19 +425,19 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) {
 				}
 				resultVal, err := parse(goja.Null(), call.VM.ToValue(string(result)))
 				if err != nil {
-					setError(resp, -32603, err.Error())
+					setError(resp, -32603, err.Error(), nil)
 				} else {
 					resp.Set("result", resultVal)
 				}
 			}
 		case rpc.Error:
-			errMap := map[string]interface{}{"code": err.ErrorCode(), "message": err.Error()}
 			if dataErr, ok := err.(rpc.DataError); ok {
-				errMap["data"] = dataErr.ErrorData()
+				setError(resp, err.ErrorCode(), err.Error(), dataErr.ErrorData())
+			} else {
+				setError(resp, err.ErrorCode(), err.Error(), nil)
 			}
-			resp.Set("error", errMap)
 		default:
-			setError(resp, -32603, err.Error())
+			setError(resp, -32603, err.Error(), nil)
 		}
 		resps = append(resps, resp)
 	}
@@ -457,8 +457,14 @@ func (b *bridge) Send(call jsre.Call) (goja.Value, error) {
 	return result, nil
 }
 
-func setError(resp *goja.Object, code int, msg string) {
-	resp.Set("error", map[string]interface{}{"code": code, "message": msg})
+func setError(resp *goja.Object, code int, msg string, data interface{}) {
+	err := make(map[string]interface{})
+	err["code"] = code
+	err["message"] = msg
+	if data != nil {
+		err["data"] = data
+	}
+	resp.Set("error", err)
 }
 
 // isNumber returns true if input value is a JS number.

graphql/graphql.go

@@ -811,7 +811,7 @@ func (b *Block) Call(ctx context.Context, args struct {
 	if result.Failed() {
 		status = 0
 	}
-	//TODO(rjl493456442, MariusVanDerWijden) return revert reason here once the spec supports an error reason.
+
 	return &CallResult{
 		data:    result.ReturnData,
 		gasUsed: hexutil.Uint64(result.UsedGas),
@@ -881,7 +881,7 @@ func (p *Pending) Call(ctx context.Context, args struct {
 	if result.Failed() {
 		status = 0
 	}
-	//TODO(rjl493456442, MariusVanDerWijden) return revert reason here once the spec supports an error reason.
+
 	return &CallResult{
 		data:    result.ReturnData,
 		gasUsed: hexutil.Uint64(result.UsedGas),

internal/ethapi/api.go

@@ -864,17 +864,11 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo
 	return result, err
 }
 
-var _ rpc.DataError = (*revertError)(nil)
-
 type revertError struct {
-	err     string      // The error string
+	error
 	errData interface{} // additional data
 }
 
-func (e revertError) Error() string {
-	return e.err
-}
-
 func (e revertError) ErrorCode() int {
 	// revert errors are execution errors.
 	// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal
@@ -905,37 +899,14 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr
 		reason, err := abi.UnpackRevert(result.Revert())
 		if err == nil {
 			return nil, &revertError{
-				err:     "execution reverted",
+				error:   errors.New("execution reverted"),
 				errData: reason,
 			}
 		}
 	}
 	return result.Return(), result.Err
 }
 
-var _ rpc.DataError = (*estimateGasError)(nil)
-
-type estimateGasError struct {
-	error  string // Concrete error type if it's failed to estimate gas usage
-	vmerr  error  // Additional field, it's non-nil if the given transaction is invalid
-	revert string // Additional field, it's non-empty if the transaction is reverted and reason is provided
-}
-
-func (e estimateGasError) Error() string {
-	errMsg := e.error
-	if e.vmerr != nil {
-		errMsg += fmt.Sprintf(" (%v)", e.vmerr)
-	}
-	if e.revert != "" {
-		errMsg += fmt.Sprintf(" (%s)", e.revert)
-	}
-	return errMsg
-}
-
-func (e estimateGasError) ErrorData() interface{} {
-	return e.revert
-}
-
 func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) {
 	// Binary search the gas requirement, as it may be higher than the amount used
 	var (
@@ -1037,14 +1008,13 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash
 						revert = ret
 					}
 				}
-				return 0, estimateGasError{
-					error:  "always failing transaction",
-					vmerr:  result.Err,
-					revert: revert,
+				return 0, revertError{
+					error:   errors.New("always failing transaction"),
+					errData: revert,
 				}
 			}
 			// Otherwise, the specified gas cap is too low
-			return 0, estimateGasError{error: fmt.Sprintf("gas required exceeds allowance (%d)", cap)}
+			return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap)
 		}
 	}
 	return hexutil.Uint64(hi), nil