Skip to content

Commit

Permalink
fix(eth): present revert "data" as plain bytes
Browse files Browse the repository at this point in the history
decode the cbor return value for reverts and present that, as is expected by
Ethereum tooling
  • Loading branch information
rvagg committed Nov 7, 2024
1 parent 3a34832 commit 57fe40e
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 44 deletions.
7 changes: 4 additions & 3 deletions api/api_errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/filecoin-project/go-jsonrpc"
"github.com/filecoin-project/go-state-types/abi"
"github.com/filecoin-project/go-state-types/exitcode"
)

var invalidExecutionRevertedMsg = xerrors.New("invalid execution reverted error")
Expand Down Expand Up @@ -160,10 +161,10 @@ func (e *ErrExecutionReverted) ToJSONRPCError() (jsonrpc.JSONRPCError, error) {
}

// NewErrExecutionReverted creates a new ErrExecutionReverted with the given reason.
func NewErrExecutionReverted(reason string) *ErrExecutionReverted {
func NewErrExecutionReverted(exitCode exitcode.ExitCode, error, reason string, data []byte) *ErrExecutionReverted {
return &ErrExecutionReverted{
Message: "execution reverted",
Data: reason,
Message: fmt.Sprintf("message execution failed (exit=[%s], revert reason=[%s], vm error=[%s])", exitCode, reason, error),
Data: fmt.Sprintf("0x%x", data),
}
}

Expand Down
26 changes: 13 additions & 13 deletions itests/fevm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1031,10 +1031,10 @@ func TestFEVMErrorParsing(t *testing.T) {
require.NoError(t, err)
customError := ethtypes.EthBytes(kit.CalcFuncSignature("CustomError()")).String()
for sig, expected := range map[string]string{
"failRevertEmpty()": "none",
"failRevertReason()": "Error(my reason)",
"failAssert()": "Assert()",
"failDivZero()": "DivideByZero()",
"failRevertEmpty()": "0x",
"failRevertReason()": fmt.Sprintf("%x", []byte("my reason")),
"failAssert()": "0x4e487b710000000000000000000000000000000000000000000000000000000000000001", // Assert()
"failDivZero()": "0x4e487b710000000000000000000000000000000000000000000000000000000000000012", // DivideByZero()
"failCustom()": customError,
} {
sig := sig
Expand Down Expand Up @@ -1602,10 +1602,10 @@ func TestEthCall(t *testing.T) {

var dataErr *api.ErrExecutionReverted
require.ErrorAs(t, err, &dataErr, "Expected error to be ErrExecutionReverted")
require.Contains(t, dataErr.Message, "execution reverted", "Expected 'execution reverted' message")
require.Regexp(t, `message execution failed [\s\S]+\[DivideByZero\(\)\]`, dataErr.Message)

// Get the error data
require.Equal(t, dataErr.Data, "DivideByZero()", "Expected error data to contain 'DivideByZero()'")
require.Equal(t, dataErr.Data, "0x4e487b710000000000000000000000000000000000000000000000000000000000000012", "Expected error data to contain 'DivideByZero()'")
})
}

Expand All @@ -1627,12 +1627,12 @@ func TestEthEstimateGas(t *testing.T) {
name string
function string
expectedError string
expectedErrMsg string
expectedErrMsg interface{}
}{
{"DivideByZero", "failDivZero()", "DivideByZero()", "execution reverted"},
{"Assert", "failAssert()", "Assert()", "execution reverted"},
{"RevertWithReason", "failRevertReason()", "Error(my reason)", "execution reverted"},
{"RevertEmpty", "failRevertEmpty()", "", "execution reverted"},
{"DivideByZero", "failDivZero()", "0x4e487b710000000000000000000000000000000000000000000000000000000000000012", `message execution failed [\s\S]+\[DivideByZero\(\)\]`},
{"Assert", "failAssert()", "0x4e487b710000000000000000000000000000000000000000000000000000000000000001", `message execution failed [\s\S]+\[Assert\(\)\]`},
{"RevertWithReason", "failRevertReason()", fmt.Sprintf("%x", []byte("my reason")), `message execution failed [\s\S]+\[Error\(my reason\)\]`},
{"RevertEmpty", "failRevertEmpty()", "0x", `message execution failed [\s\S]+\[none\]`},
}

for _, tc := range testCases {
Expand All @@ -1654,8 +1654,8 @@ func TestEthEstimateGas(t *testing.T) {
require.Error(t, err)
var dataErr *api.ErrExecutionReverted
require.ErrorAs(t, err, &dataErr, "Expected error to be ErrExecutionReverted")
require.Equal(t, tc.expectedErrMsg, dataErr.Message)
require.Contains(t, tc.expectedError, dataErr.Data)
require.Regexp(t, tc.expectedErrMsg, dataErr.Message)
require.Contains(t, dataErr.Data, tc.expectedError)
}
})
}
Expand Down
13 changes: 10 additions & 3 deletions node/impl/full/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -1496,9 +1496,16 @@ func (a *EthModule) applyMessage(ctx context.Context, msg *types.Message, tsk ty
}

if res.MsgRct.ExitCode.IsError() {
return nil, api.NewErrExecutionReverted(
parseEthRevert(res.MsgRct.Return),
)
reason := "none"
var cbytes abi.CborBytes
if err := cbytes.UnmarshalCBOR(bytes.NewReader(res.MsgRct.Return)); err != nil {
log.Warnw("failed to unmarshal cbor bytes from message receipt return", "error", err)
reason = "ERROR: revert reason is not cbor encoded bytes"
} // else leave as empty bytes
if len(cbytes) > 0 {
reason = parseEthRevert(cbytes)
}
return nil, api.NewErrExecutionReverted(res.MsgRct.ExitCode, reason, res.Error, cbytes)
}

return res, nil
Expand Down
40 changes: 15 additions & 25 deletions node/impl/full/eth_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,62 +331,52 @@ var panicErrorCodes = map[uint64]string{
//
// See https://docs.soliditylang.org/en/latest/control-structures.html#panic-via-assert-and-error-via-require
func parseEthRevert(ret []byte) string {
if len(ret) == 0 {
return "none"
}
var cbytes abi.CborBytes
if err := cbytes.UnmarshalCBOR(bytes.NewReader(ret)); err != nil {
return "ERROR: revert reason is not cbor encoded bytes"
}
if len(cbytes) == 0 {
return "none"
}
// If it's not long enough to contain an ABI encoded response, return immediately.
if len(cbytes) < 4+32 {
return ethtypes.EthBytes(cbytes).String()
if len(ret) < 4+32 {
return ethtypes.EthBytes(ret).String()
}
switch string(cbytes[:4]) {
switch string(ret[:4]) {
case panicFunctionSelector:
cbytes := cbytes[4 : 4+32]
ret := ret[4 : 4+32]
// Read the and check the code.
code, err := ethtypes.EthUint64FromBytes(cbytes)
code, err := ethtypes.EthUint64FromBytes(ret)
if err != nil {
// If it's too big, just return the raw value.
codeInt := big.PositiveFromUnsignedBytes(cbytes)
codeInt := big.PositiveFromUnsignedBytes(ret)
return fmt.Sprintf("Panic(%s)", ethtypes.EthBigInt(codeInt).String())
}
if s, ok := panicErrorCodes[uint64(code)]; ok {
return s
}
return fmt.Sprintf("Panic(0x%x)", code)
case errorFunctionSelector:
cbytes := cbytes[4:]
cbytesLen := ethtypes.EthUint64(len(cbytes))
ret := ret[4:]
retLen := ethtypes.EthUint64(len(ret))
// Read the and check the offset.
offset, err := ethtypes.EthUint64FromBytes(cbytes[:32])
offset, err := ethtypes.EthUint64FromBytes(ret[:32])
if err != nil {
break
}
if cbytesLen < offset {
if retLen < offset {
break
}

// Read and check the length.
if cbytesLen-offset < 32 {
if retLen-offset < 32 {
break
}
start := offset + 32
length, err := ethtypes.EthUint64FromBytes(cbytes[offset : offset+32])
length, err := ethtypes.EthUint64FromBytes(ret[offset : offset+32])
if err != nil {
break
}
if cbytesLen-start < length {
if retLen-start < length {
break
}
// Slice the error message.
return fmt.Sprintf("Error(%s)", cbytes[start:start+length])
return fmt.Sprintf("Error(%s)", ret[start:start+length])
}
return ethtypes.EthBytes(cbytes).String()
return ethtypes.EthBytes(ret).String()
}

// lookupEthAddress makes its best effort at finding the Ethereum address for a
Expand Down

0 comments on commit 57fe40e

Please sign in to comment.