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.

Sign up for GitHub

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

Support for logging custom errors on tx execution revert in not only real tx execution but also gas estimation #39

Merged
merged 13 commits into from
Apr 26, 2024
+486 −187
Merged

Support for logging custom errors on tx execution revert in not only real tx execution but also gas estimation #39

Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7e6b78d
Change so that Custom Error can be output to revertReason
dongrie Feb 15, 2024
dc2faf6
fix to the way to get revert reason
siburu Apr 24, 2024
d371625
overhaul ErrorRepository
siburu Apr 24, 2024
c852e9f
fix a bug
siburu Apr 24, 2024
7d9990a
avoid using panicable type assertions
siburu Apr 24, 2024
16a9937
fix a bug
siburu Apr 24, 2024
aa347c2
make errors more informative
siburu Apr 24, 2024
399dd8e
emit an error log that contains the whole error data returned by EVM …
siburu Apr 25, 2024
f6fc68e
add "msg_index" attribute to logging in the loop of `SendMsgs`
siburu Apr 25, 2024
d972fff
change the way to handle default errors (Error and Panic)
siburu Apr 25, 2024
aac5147
add "raw_error_data" and "tx_hash" attributes to log outputs from `Se…
siburu Apr 26, 2024
fcb8da1
output raw_error_data also in logging in GetMsgResult
siburu Apr 26, 2024
99492cd
add tx_hash attribute in logging in GetMsgResult
siburu Apr 26, 2024
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
43 changes: 22 additions & 21 deletions pkg/relay/ethereum/tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ethereum

import (
"context"
"encoding/hex"
"errors"
"fmt"
math "math"
Expand Down Expand Up @@ -52,12 +53,12 @@ func (c *Chain) SendMsgs(msgs []sdk.Msg) ([]core.MsgID, error) {
}
estimatedGas, err := c.client.EstimateGasFromTx(ctx, tx)
if err != nil {
revertReason, err2 := c.getRevertReasonFromEstimateGas(err)
revertReason, rawErrorData, err2 := c.getRevertReasonFromEstimateGas(err)
if err2 != nil {
logger.Error("failed to get revert reason", err2, "msg_index", i)
}

logger.Error("failed to estimate gas", err, "revert_reason", revertReason, "msg_index", i)
logger.Error("failed to estimate gas", err, "revert_reason", revertReason, "raw_error_data", hex.EncodeToString(rawErrorData), "msg_index", i)
return nil, err
}
txGasLimit := estimatedGas * c.Config().GasEstimateRate.Numerator / c.Config().GasEstimateRate.Denominator
Expand All @@ -74,22 +75,22 @@ func (c *Chain) SendMsgs(msgs []sdk.Msg) ([]core.MsgID, error) {
}
receipt, err := c.client.WaitForReceiptAndGet(ctx, tx.Hash())
if err != nil {
logger.Error("failed to get receipt", err, "msg_index", i)
logger.Error("failed to get receipt", err, "msg_index", i, "tx_hash", tx.Hash())
return nil, err
}
if receipt.Status == gethtypes.ReceiptStatusFailed {
revertReason, err2 := c.getRevertReasonFromReceipt(ctx, receipt)
revertReason, rawErrorData, err2 := c.getRevertReasonFromReceipt(ctx, receipt)
if err2 != nil {
logger.Error("failed to get revert reason", err2, "msg_index", i)
logger.Error("failed to get revert reason", err2, "msg_index", i, "tx_hash", tx.Hash())
}

err := fmt.Errorf("tx execution reverted: revertReason=%s, msgIndex=%d", revertReason, i)
logger.Error("tx execution reverted", err, "revert_reason", revertReason, "msg_index", i)
err := fmt.Errorf("tx execution reverted: revertReason=%s, rawErrorData=%x, msgIndex=%d, txHash=%s", revertReason, rawErrorData, i, tx.Hash())
logger.Error("tx execution reverted", err, "revert_reason", revertReason, "raw_error_data", hex.EncodeToString(rawErrorData), "msg_index", i, "tx_hash", tx.Hash())
return nil, err
}
if c.msgEventListener != nil {
if err := c.msgEventListener.OnSentMsg([]sdk.Msg{msg}); err != nil {
logger.Error("failed to OnSendMsg call", err, "msg_index", i)
logger.Error("failed to OnSendMsg call", err, "msg_index", i, "tx_hash", tx.Hash())
}
}
msgIDs = append(msgIDs, NewMsgID(tx.Hash()))
Expand All @@ -113,7 +114,7 @@ func (c *Chain) GetMsgResult(id core.MsgID) (core.MsgResult, error) {
if receipt.Status == gethtypes.ReceiptStatusSuccessful {
return c.makeMsgResultFromReceipt(&receipt.Receipt, "")
}
revertReason, err := c.getRevertReasonFromReceipt(ctx, receipt)
revertReason, _, err := c.getRevertReasonFromReceipt(ctx, receipt)
if err != nil {
logger.Error("failed to get revert reason", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also contain errorData in the error log?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 ... sure. I will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluele fixed fcb8da1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ... should we also add tx_hash here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added 99492cd

}
Expand Down Expand Up @@ -363,42 +364,42 @@ func (c *Chain) SendTx(opts *bind.TransactOpts, msg sdk.Msg, skipUpdateClientCom
return tx, err
}

func (c *Chain) getRevertReasonFromReceipt(ctx context.Context, receipt *client.Receipt) (string, error) {
func (c *Chain) getRevertReasonFromReceipt(ctx context.Context, receipt *client.Receipt) (string, []byte, error) {
var errorData []byte
if receipt.HasRevertReason() {
errorData = receipt.RevertReason
} else if c.config.EnableDebugTrace {
callFrame, err := c.client.DebugTraceTransaction(ctx, receipt.TxHash)
if err != nil {
return "", err
return "", nil, err
} else if len(callFrame.Output) == 0 {
return "", fmt.Errorf("execution reverted without error data")
return "", nil, fmt.Errorf("execution reverted without error data")
}
bluele marked this conversation as resolved.
Show resolved Hide resolved
errorData = callFrame.Output
} else {
return "", fmt.Errorf("no way to get revert reason")
return "", nil, fmt.Errorf("no way to get revert reason")
}

revertReason, err := c.errorRepository.ParseError(errorData)
if err != nil {
return "", fmt.Errorf("failed to parse error: %v", err)
return "", errorData, fmt.Errorf("failed to parse error: %v", err)
}
return revertReason, nil
return revertReason, errorData, nil
}

func (c *Chain) getRevertReasonFromEstimateGas(err error) (string, error) {
func (c *Chain) getRevertReasonFromEstimateGas(err error) (string, []byte, error) {
if de, ok := err.(rpc.DataError); !ok {
return "", fmt.Errorf("eth_estimateGas failed with unexpected error type: errorType=%T", err)
return "", nil, fmt.Errorf("eth_estimateGas failed with unexpected error type: errorType=%T", err)
} else if de.ErrorData() == nil {
return "", fmt.Errorf("eth_estimateGas failed without error data")
return "", nil, fmt.Errorf("eth_estimateGas failed without error data")
} else if errorData, ok := de.ErrorData().(string); !ok {
return "", fmt.Errorf("eth_estimateGas failed with unexpected error data type: errorDataType=%T", de.ErrorData())
return "", nil, fmt.Errorf("eth_estimateGas failed with unexpected error data type: errorDataType=%T", de.ErrorData())
} else {
errorData := common.FromHex(errorData)
revertReason, err := c.errorRepository.ParseError(errorData)
siburu marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return "", fmt.Errorf("failed to parse error: %v", err)
return "", errorData, fmt.Errorf("failed to parse error: %v", err)
}
return revertReason, nil
return revertReason, errorData, nil
}
}