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

fix(eth): present revert "data" as plain bytes #12675

Merged
merged 1 commit into from
Nov 7, 2024
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
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