From 57fe40e4251e777b9ed8984d60d745aa4b0ba27b Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Mon, 4 Nov 2024 13:47:06 +1100 Subject: [PATCH] fix(eth): present revert "data" as plain bytes decode the cbor return value for reverts and present that, as is expected by Ethereum tooling --- api/api_errors.go | 7 ++++--- itests/fevm_test.go | 26 ++++++++++++------------ node/impl/full/eth.go | 13 +++++++++--- node/impl/full/eth_utils.go | 40 ++++++++++++++----------------------- 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/api/api_errors.go b/api/api_errors.go index b7fa2b25661..ffb01289d7b 100644 --- a/api/api_errors.go +++ b/api/api_errors.go @@ -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") @@ -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), } } diff --git a/itests/fevm_test.go b/itests/fevm_test.go index 7ffa4d51e6c..83db6ade91f 100644 --- a/itests/fevm_test.go +++ b/itests/fevm_test.go @@ -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 @@ -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()'") }) } @@ -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 { @@ -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) } }) } diff --git a/node/impl/full/eth.go b/node/impl/full/eth.go index 4e20e0ca534..3c090398614 100644 --- a/node/impl/full/eth.go +++ b/node/impl/full/eth.go @@ -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 diff --git a/node/impl/full/eth_utils.go b/node/impl/full/eth_utils.go index a4f276095d5..00f3de90163 100644 --- a/node/impl/full/eth_utils.go +++ b/node/impl/full/eth_utils.go @@ -331,28 +331,18 @@ 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 { @@ -360,33 +350,33 @@ func parseEthRevert(ret []byte) string { } 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