Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate custom Solidity errors for eth_estimateGas JSON RPC function #1922

Merged
merged 3 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions jsonrpc/eth_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,23 +652,29 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
}

// Run the transaction with the specified gas value.
// Returns a status indicating if the transaction failed and the accompanying error
testTransaction := func(gas uint64, shouldOmitErr bool) (bool, error) {
// Returns a status indicating if the transaction failed, return value (data), and the accompanying error
testTransaction := func(gas uint64, shouldOmitErr bool) (bool, interface{}, error) {
var data interface{}

transaction.Gas = gas

result, applyErr := e.store.ApplyTxn(header, transaction, nil)

if result != nil {
data = []byte(hex.EncodeToString(result.ReturnValue))
}

if applyErr != nil {
// Check the application error.
// Gas apply errors are valid, and should be ignored
if isGasApplyError(applyErr) && shouldOmitErr {
// Specifying the transaction failed, but not providing an error
// is an indication that a valid error occurred due to low gas,
// which will increase the lower bound for the search
return true, nil
return true, data, nil
}

return true, applyErr
return true, data, applyErr
}

// Check if an out of gas error happened during EVM execution
Expand All @@ -677,30 +683,30 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
// Specifying the transaction failed, but not providing an error
// is an indication that a valid error occurred due to low gas,
// which will increase the lower bound for the search
return true, nil
return true, data, nil
}

if isEVMRevertError(result.Err) {
// The EVM reverted during execution, attempt to extract the
// error message and return it
return true, constructErrorFromRevert(result)
return true, data, constructErrorFromRevert(result)
}

return true, result.Err
return true, data, result.Err
}

return false, nil
return false, nil, nil
}

// Start the binary search for the lowest possible gas price
for lowEnd < highEnd {
mid := lowEnd + ((highEnd - lowEnd) >> 1) // (lowEnd + highEnd) / 2 can overflow

failed, testErr := testTransaction(mid, true)
failed, retVal, testErr := testTransaction(mid, true)
if testErr != nil && !isEVMRevertError(testErr) {
// Reverts are ignored in the binary search, but are checked later on
// during the execution for the optimal gas limit found
return 0, testErr
return retVal, testErr
}

if failed {
Expand All @@ -713,10 +719,10 @@ func (e *Eth) EstimateGas(arg *txnArgs, rawNum *BlockNumber) (interface{}, error
}

// Check if the highEnd is a good value to make the transaction pass
failed, err := testTransaction(highEnd, false)
failed, retVal, err := testTransaction(highEnd, false)
if failed {
// The transaction shouldn't fail, for whatever reason, at highEnd
return 0, fmt.Errorf(
return retVal, fmt.Errorf(
"unable to apply transaction even for the highest gas limit %d: %w",
highEnd,
err,
Expand Down
87 changes: 58 additions & 29 deletions jsonrpc/eth_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ func TestEth_EstimateGas_GasLimit(t *testing.T) {
assert.ErrorIs(t, estimateErr, testCase.expectedError)

// Make sure the estimate is nullified
assert.Equal(t, 0, estimate)
assert.Equal(t, []byte{}, estimate)
} else {
// Make sure no errors occurred
assert.NoError(t, estimateErr)
Expand All @@ -705,42 +705,71 @@ func TestEth_EstimateGas_GasLimit(t *testing.T) {
}

func TestEth_EstimateGas_Reverts(t *testing.T) {
// Example revert data that has the string "revert reason" as the revert reason
exampleReturnData := "08c379a000000000000000000000000000000000000000000000000000000000000000" +
"20000000000000000000000000000000000000000000000000000000000000000d72657665727420726561736f6e" +
"00000000000000000000000000000000000000"
rawReturnData, err := hex.DecodeHex(exampleReturnData)
assert.NoError(t, err)

revertReason := errors.New("revert reason")
testTable := []struct {
name string
exampleReturnData string
revertReason error
err error
}{
{
"revert with string message 'revert reason'",
// Example revert data that has the string "revert reason" as the revert reason
"08c379a000000000000000000000000000000000000000000000000000000000000000" +
"20000000000000000000000000000000000000000000000000000000000000000d72657665727420726561736f6e" +
"00000000000000000000000000000000000000",
errors.New("revert reason"),
runtime.ErrExecutionReverted,
},
{
"revert with custom solidity error",
// First 4 bytes of ABI encoded custom solidity error PermissionedContractAccessDenied()
"8cd01c38",
errors.New(""), // revert reason in this case doesn't exist, reason is decoded in response data
runtime.ErrExecutionReverted,
},
{
"revert with empty response",
"",
errors.New(""),
runtime.ErrExecutionReverted,
},
}

store := getExampleStore()
ethEndpoint := newTestEthEndpoint(store)

// We want to simulate an EVM revert here
store.applyTxnHook = func(
header *types.Header,
txn *types.Transaction,
) (*runtime.ExecutionResult, error) {
return &runtime.ExecutionResult{
ReturnValue: rawReturnData,
Err: runtime.ErrExecutionReverted,
}, nil
}
for _, test := range testTable {
rawReturnData, err := hex.DecodeHex(test.exampleReturnData)
assert.NoError(t, err)

// We want to simulate an EVM revert here
store.applyTxnHook = func(
header *types.Header,
txn *types.Transaction,
) (*runtime.ExecutionResult, error) {
return &runtime.ExecutionResult{
ReturnValue: rawReturnData,
Err: runtime.ErrExecutionReverted,
}, nil
}

// Run the estimation
estimate, estimateErr := ethEndpoint.EstimateGas(
constructMockTx(nil, nil),
nil,
)

// Run the estimation
estimate, estimateErr := ethEndpoint.EstimateGas(
constructMockTx(nil, nil),
nil,
)
responseData, ok := estimate.([]byte)
assert.True(t, ok)

assert.Equal(t, 0, estimate)
assert.Equal(t, test.exampleReturnData, string(responseData))

// Make sure the EVM revert message is contained
assert.ErrorIs(t, estimateErr, runtime.ErrExecutionReverted)
// Make sure the EVM revert message is contained
assert.ErrorIs(t, estimateErr, runtime.ErrExecutionReverted)

// Make sure the EVM revert reason is contained
assert.ErrorAs(t, estimateErr, &revertReason)
// Make sure the EVM revert reason is contained
assert.ErrorAs(t, estimateErr, &test.revertReason)
}
}

func TestEth_EstimateGas_Errors(t *testing.T) {
Expand Down
Loading