Skip to content

Commit 53501e9

Browse files
mmsqemmsqe
authored
fix: align revert reason with go ethereum that expects hex-encoded result (cosmos#289)
* fix: align revert reason with go ethereum that expects hex-encoded result * cleanup * cleanup * Apply suggestions from code review
1 parent 54672be commit 53501e9

File tree

7 files changed

+109
-18
lines changed

7 files changed

+109
-18
lines changed

‎evmd/tests/ibc/ics20_precompile_transfer_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ import (
1616
"github.com/cosmos/evm/evmd"
1717
"github.com/cosmos/evm/evmd/tests/integration"
1818
"github.com/cosmos/evm/precompiles/ics20"
19+
chainutil "github.com/cosmos/evm/testutil"
1920
evmibctesting "github.com/cosmos/evm/testutil/ibc"
2021
evmante "github.com/cosmos/evm/x/vm/ante"
21-
evmtypes "github.com/cosmos/evm/x/vm/types"
2222
transfertypes "github.com/cosmos/ibc-go/v10/modules/apps/transfer/types"
2323
clienttypes "github.com/cosmos/ibc-go/v10/modules/core/02-client/types"
2424

@@ -299,8 +299,8 @@ func (suite *ICS20TransferTestSuite) TestHandleMsgTransfer() {
299299
"INVALID-DENOM-HASH",
300300
)
301301
suite.Require().ErrorContains(err, vm.ErrExecutionReverted.Error())
302-
revertErr := evmtypes.NewExecErrorWithReason(evmRes.Ret)
303-
suite.Require().Contains(revertErr.ErrorData(), "invalid denom trace hash")
302+
revertErr := chainutil.DecodeRevertReason(*evmRes)
303+
suite.Require().Contains(revertErr.Error(), "invalid denom trace hash")
304304

305305
// denomHash query method
306306
evmRes, err = evmAppB.EVMKeeper.CallEVM(
@@ -347,8 +347,8 @@ func (suite *ICS20TransferTestSuite) TestHandleMsgTransfer() {
347347
"",
348348
)
349349
suite.Require().ErrorContains(err, vm.ErrExecutionReverted.Error())
350-
revertErr = evmtypes.NewExecErrorWithReason(evmRes.Ret)
351-
suite.Require().Contains(revertErr.ErrorData(), "invalid denomination for cross-chain transfer")
350+
revertErr = chainutil.DecodeRevertReason(*evmRes)
351+
suite.Require().Contains(revertErr.Error(), "invalid denomination for cross-chain transfer")
352352
})
353353
}
354354
}

‎evmd/tests/ibc/v2_ics20_precompile_transfer_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ import (
1717
"github.com/cosmos/evm/evmd"
1818
"github.com/cosmos/evm/evmd/tests/integration"
1919
"github.com/cosmos/evm/precompiles/ics20"
20+
chainutil "github.com/cosmos/evm/testutil"
2021
evmibctesting "github.com/cosmos/evm/testutil/ibc"
2122
evmante "github.com/cosmos/evm/x/vm/ante"
22-
evmtypes "github.com/cosmos/evm/x/vm/types"
2323
transfertypes "github.com/cosmos/ibc-go/v10/modules/apps/transfer/types"
2424
clienttypes "github.com/cosmos/ibc-go/v10/modules/core/02-client/types"
2525

@@ -305,8 +305,8 @@ func (suite *ICS20TransferV2TestSuite) TestHandleMsgTransfer() {
305305
"INVALID-DENOM-HASH",
306306
)
307307
suite.Require().ErrorContains(err, vm.ErrExecutionReverted.Error())
308-
revertErr := evmtypes.NewExecErrorWithReason(evmRes.Ret)
309-
suite.Require().Contains(revertErr.ErrorData(), "invalid denom trace hash")
308+
revertErr := chainutil.DecodeRevertReason(*evmRes)
309+
suite.Require().Contains(revertErr.Error(), "invalid denom trace hash")
310310

311311
// denomHash query method
312312
evmRes, err = evmAppB.EVMKeeper.CallEVM(
@@ -353,8 +353,8 @@ func (suite *ICS20TransferV2TestSuite) TestHandleMsgTransfer() {
353353
"",
354354
)
355355
suite.Require().ErrorContains(err, vm.ErrExecutionReverted.Error())
356-
revertErr = evmtypes.NewExecErrorWithReason(evmRes.Ret)
357-
suite.Require().Contains(revertErr.ErrorData(), "invalid denomination for cross-chain transfer")
356+
revertErr = chainutil.DecodeRevertReason(*evmRes)
357+
suite.Require().Contains(revertErr.Error(), "invalid denomination for cross-chain transfer")
358358
})
359359
}
360360
}

‎testutil/integration/evm/factory/factory.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
abcitypes "github.com/cometbft/cometbft/abci/types"
1313

1414
"github.com/cosmos/evm/precompiles/testutil"
15+
chainutil "github.com/cosmos/evm/testutil"
1516
basefactory "github.com/cosmos/evm/testutil/integration/base/factory"
1617
"github.com/cosmos/evm/testutil/integration/evm/grpc"
1718
"github.com/cosmos/evm/testutil/integration/evm/network"
@@ -209,8 +210,7 @@ func (tf *IntegrationTxFactory) checkEthTxResponse(res *abcitypes.ExecTxResult)
209210
}
210211

211212
if evmRes.Failed() {
212-
revertErr := evmtypes.NewExecErrorWithReason(evmRes.Ret)
213-
return fmt.Errorf("tx failed with VmError: %v: %s", evmRes.VmError, revertErr.ErrorData())
213+
return chainutil.DecodeRevertReason(evmRes)
214214
}
215215
return nil
216216
}

‎testutil/util.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
11
package testutil
22

33
import (
4+
"bytes"
45
"context"
6+
"fmt"
57
"time"
68

9+
"github.com/ethereum/go-ethereum/accounts/abi"
10+
"github.com/ethereum/go-ethereum/common/hexutil"
11+
712
"github.com/cosmos/evm/crypto/ethsecp256k1"
13+
evmtypes "github.com/cosmos/evm/x/vm/types"
14+
15+
errorsmod "cosmossdk.io/errors"
816

917
"github.com/cosmos/cosmos-sdk/client"
1018
"github.com/cosmos/cosmos-sdk/client/tx"
@@ -108,3 +116,39 @@ func CreateTx(ctx context.Context, txCfg client.TxConfig, priv cryptotypes.PrivK
108116

109117
return txBuilder.GetTx(), nil
110118
}
119+
120+
// DecodeRevertReason extracts and decodes the human-readable revert reason from an EVM transaction response.
121+
// It processes the raw return data (Ret field) from a failed EVM transaction and attempts to decode
122+
// any ABI-encoded revert messages into readable error strings.
123+
//
124+
// Returns:
125+
// - error: A formatted error containing either:
126+
// - "tx failed with VmError: <vmError>: <decoded_reason>" for successfully decoded reverts
127+
// - "tx failed with VmError: <vmError>: <hex_data>" for non-decodable data
128+
// - "failed to decode revert data: <decode_error>" if decoding fails
129+
//
130+
// Example usage:
131+
//
132+
// res, err := executeTransaction(...)
133+
// if res.VmError != "" {
134+
// decodedErr := DecodeRevertReason(res)
135+
// // decodedErr might be: "tx failed with VmError: execution reverted: ERC20: insufficient allowance"
136+
// }
137+
func DecodeRevertReason(evmRes evmtypes.MsgEthereumTxResponse) error {
138+
revertErr := evmtypes.NewExecErrorWithReason(evmRes.Ret)
139+
hexData, ok := revertErr.ErrorData().(string)
140+
if ok {
141+
decodedBytes, err := hexutil.Decode(hexData)
142+
if err == nil {
143+
if len(decodedBytes) >= 4 && bytes.Equal(decodedBytes[:4], evmtypes.RevertSelector) {
144+
var reason string
145+
reason, err = abi.UnpackRevert(decodedBytes)
146+
if err == nil {
147+
return fmt.Errorf("tx failed with VmError: %v: %s", evmRes.VmError, reason)
148+
}
149+
}
150+
}
151+
return errorsmod.Wrap(err, "failed to decode revert data")
152+
}
153+
return fmt.Errorf("tx failed with VmError: %v: %s", evmRes.VmError, revertErr.ErrorData())
154+
}

‎x/vm/keeper/state_transition.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -444,11 +444,6 @@ func (k *Keeper) ApplyMessageWithConfig(
444444
// reset leftoverGas, to be used by the tracer
445445
leftoverGas = msg.GasLimit - gasUsed
446446

447-
// if the execution reverted, we return the revert reason as the return data
448-
if vmError == vm.ErrExecutionReverted.Error() {
449-
ret = evm.Interpreter().ReturnData()
450-
}
451-
452447
return &types.MsgEthereumTxResponse{
453448
GasUsed: gasUsed,
454449
VmError: vmError,

‎x/vm/types/errors.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/ethereum/go-ethereum/accounts/abi"
88
"github.com/ethereum/go-ethereum/common"
9+
"github.com/ethereum/go-ethereum/common/hexutil"
910
"github.com/ethereum/go-ethereum/crypto"
1011

1112
errorsmod "cosmossdk.io/errors"
@@ -122,7 +123,7 @@ func NewExecErrorWithReason(revertReason []byte) *RevertError {
122123
}
123124
return &RevertError{
124125
error: err,
125-
reason: reason,
126+
reason: hexutil.Encode(result),
126127
}
127128
}
128129

‎x/vm/types/errors_test.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package types_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/ethereum/go-ethereum/common/hexutil"
7+
"github.com/stretchr/testify/require"
8+
9+
"github.com/cosmos/evm/x/vm/types"
10+
)
11+
12+
func TestNewExecErrorWithReason(t *testing.T) {
13+
testCases := []struct {
14+
name string
15+
errorMessage string
16+
revertReason []byte
17+
data string
18+
}{
19+
{
20+
"Empty reason",
21+
"execution reverted",
22+
nil,
23+
"0x",
24+
},
25+
{
26+
"With unpackable reason",
27+
"execution reverted",
28+
[]byte("a"),
29+
"0x61",
30+
},
31+
{
32+
"With packable reason but empty reason",
33+
"execution reverted",
34+
types.RevertSelector,
35+
"0x08c379a0",
36+
},
37+
{
38+
"With packable reason with reason",
39+
"execution reverted: COUNTER_TOO_LOW",
40+
hexutil.MustDecode("0x08C379A00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000F434F554E5445525F544F4F5F4C4F570000000000000000000000000000000000"),
41+
"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000f434f554e5445525f544f4f5f4c4f570000000000000000000000000000000000",
42+
},
43+
}
44+
45+
for _, tc := range testCases {
46+
errWithReason := types.NewExecErrorWithReason(tc.revertReason)
47+
require.Equal(t, tc.errorMessage, errWithReason.Error())
48+
require.Equal(t, tc.data, errWithReason.ErrorData())
49+
require.Equal(t, 3, errWithReason.ErrorCode())
50+
}
51+
}

0 commit comments

Comments
 (0)