Skip to content

Commit

Permalink
Merge pull request #128 from kaleido-io/receipt-revert-reason
Browse files Browse the repository at this point in the history 
If the revert reason is present in a receipt, skip the call to `debug_traceTransaction`
  • Loading branch information
@Chengxuan
Chengxuan committed Apr 8, 2024
2 parents 3685bcb + 50b17d6 commit 29020f8
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 33 deletions.
79 changes: 49 additions & 30 deletions internal/ethereum/get_receipt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,26 @@ import (

"github.com/hyperledger/firefly-common/pkg/fftypes"
"github.com/hyperledger/firefly-common/pkg/i18n"
"github.com/hyperledger/firefly-common/pkg/log"
"github.com/hyperledger/firefly-evmconnect/internal/msgs"
"github.com/hyperledger/firefly-signer/pkg/ethtypes"
"github.com/hyperledger/firefly-transaction-manager/pkg/ffcapi"
)

// txReceiptJSONRPC is the receipt obtained over JSON/RPC from the ethereum client, with gas used, logs and contract address
type txReceiptJSONRPC struct {
BlockHash ethtypes.HexBytes0xPrefix `json:"blockHash"`
BlockNumber *ethtypes.HexInteger `json:"blockNumber"`
ContractAddress *ethtypes.Address0xHex `json:"contractAddress"`
CumulativeGasUsed *ethtypes.HexInteger `json:"cumulativeGasUsed"`
From *ethtypes.Address0xHex `json:"from"`
GasUsed *ethtypes.HexInteger `json:"gasUsed"`
Logs []*logJSONRPC `json:"logs"`
Status *ethtypes.HexInteger `json:"status"`
To *ethtypes.Address0xHex `json:"to"`
TransactionHash ethtypes.HexBytes0xPrefix `json:"transactionHash"`
TransactionIndex *ethtypes.HexInteger `json:"transactionIndex"`
BlockHash ethtypes.HexBytes0xPrefix `json:"blockHash"`
BlockNumber *ethtypes.HexInteger `json:"blockNumber"`
ContractAddress *ethtypes.Address0xHex `json:"contractAddress"`
CumulativeGasUsed *ethtypes.HexInteger `json:"cumulativeGasUsed"`
From *ethtypes.Address0xHex `json:"from"`
GasUsed *ethtypes.HexInteger `json:"gasUsed"`
Logs []*logJSONRPC `json:"logs"`
Status *ethtypes.HexInteger `json:"status"`
To *ethtypes.Address0xHex `json:"to"`
TransactionHash ethtypes.HexBytes0xPrefix `json:"transactionHash"`
TransactionIndex *ethtypes.HexInteger `json:"transactionIndex"`
RevertReason *ethtypes.HexBytes0xPrefix `json:"revertReason"`
}

// receiptExtraInfo is the version of the receipt we store under the TX.
Expand Down Expand Up @@ -119,30 +121,47 @@ func ProtocolIDForReceipt(blockNumber, transactionIndex *fftypes.FFBigInt) strin
return ""
}

func (c *ethConnector) getErrorInfo(ctx context.Context, transactionHash string) (pReturnValue *string, pErrorMessage *string) {

// Attempt to get the return value of the transaction - not possible on all RPC endpoints
var debugTrace *txDebugTrace
traceErr := c.backend.CallRPC(ctx, &debugTrace, "debug_traceTransaction", transactionHash)
if traceErr != nil {
msg := i18n.NewError(ctx, msgs.MsgUnableToCallDebug, traceErr).Error()
return nil, &msg
func padHexData(hexString string) string {
hexString = strings.TrimPrefix(hexString, "0x")
if len(hexString)%2 == 1 {
hexString = "0" + hexString
}

returnValue := debugTrace.ReturnValue
if returnValue == "" {
// some clients (e.g. Besu) include the error reason on the final struct log
if len(debugTrace.StructLogs) > 0 {
finalStructLog := debugTrace.StructLogs[len(debugTrace.StructLogs)-1]
if *finalStructLog.Op == "REVERT" && finalStructLog.Reason != nil {
returnValue = *finalStructLog.Reason
return hexString
}

func (c *ethConnector) getErrorInfo(ctx context.Context, transactionHash string, revertFromReceipt *ethtypes.HexBytes0xPrefix) (pReturnValue *string, pErrorMessage *string) {

var revertReason string
if revertFromReceipt == nil {
log.L(ctx).Trace("No revert reason for the failed transaction found in the receipt. Calling debug_traceTransaction to retrieve it.")
// Attempt to get the return value of the transaction - not possible on all RPC endpoints
var debugTrace *txDebugTrace
traceErr := c.backend.CallRPC(ctx, &debugTrace, "debug_traceTransaction", transactionHash)
if traceErr != nil {
msg := i18n.NewError(ctx, msgs.MsgUnableToCallDebug, traceErr).Error()
return nil, &msg
}

revertReason = debugTrace.ReturnValue
log.L(ctx).Debugf("Revert reason from debug_traceTransaction: '%v'", revertReason)
if revertReason == "" {
// some clients (e.g. Besu) include the error reason on the final struct log
if len(debugTrace.StructLogs) > 0 {
finalStructLog := debugTrace.StructLogs[len(debugTrace.StructLogs)-1]
if *finalStructLog.Op == "REVERT" && finalStructLog.Reason != nil {
revertReason = *finalStructLog.Reason
}
}
}
} else {
log.L(ctx).Trace("Revert reason is set in the receipt. Skipping call to debug_traceTransaction.")
revertReason = revertFromReceipt.String()
}

// See if the return value is using the default error you get from "revert"
var errorMessage string
returnDataBytes, _ := hex.DecodeString(strings.TrimPrefix(returnValue, "0x"))
returnDataBytes, _ := hex.DecodeString(padHexData(revertReason))
if len(returnDataBytes) > 4 && bytes.Equal(returnDataBytes[0:4], defaultErrorID) {
value, err := defaultError.DecodeCallDataCtx(ctx, returnDataBytes)
if err == nil {
Expand All @@ -153,12 +172,12 @@ func (c *ethConnector) getErrorInfo(ctx context.Context, transactionHash string)
// Otherwise we can't decode it, so put it directly in the error
if errorMessage == "" {
if len(returnDataBytes) > 0 {
errorMessage = i18n.NewError(ctx, msgs.MsgReturnValueNotDecoded, returnValue).Error()
errorMessage = i18n.NewError(ctx, msgs.MsgReturnValueNotDecoded, revertReason).Error()
} else {
errorMessage = i18n.NewError(ctx, msgs.MsgReturnValueNotAvailable).Error()
}
}
return &returnValue, &errorMessage
return &revertReason, &errorMessage
}

func (c *ethConnector) TransactionReceipt(ctx context.Context, req *ffcapi.TransactionReceiptRequest) (*ffcapi.TransactionReceiptResponse, ffcapi.ErrorReason, error) {
Expand All @@ -178,7 +197,7 @@ func (c *ethConnector) TransactionReceipt(ctx context.Context, req *ffcapi.Trans
var transactionErrorMessage *string

if !isSuccess {
returnDataString, transactionErrorMessage = c.getErrorInfo(ctx, req.TransactionHash)
returnDataString, transactionErrorMessage = c.getErrorInfo(ctx, req.TransactionHash, ethReceipt.RevertReason)
}

ethReceipt.Logs = nil
Expand Down
47 changes: 44 additions & 3 deletions internal/ethereum/get_receipt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,24 @@ const sampleJSONRPCReceiptFailed = `{
"type": "0x0"
}`

const sampleJSONRPCReceiptFailedWithRevertReason = `{
"blockHash": "0x6197ef1a58a2a592bb447efb651f0db7945de21aa8048801b250bd7b7431f9b6",
"blockNumber": "0x7b9",
"contractAddress": "0x87ae94ab290932c4e6269648bb47c86978af4436",
"cumulativeGasUsed": "0x8414",
"effectiveGasPrice": "0x0",
"from": "0x2b1c769ef5ad304a4889f2a07a6617cd935849ae",
"gasUsed": "0x8414",
"logs": [],
"logsBloom": "0x00000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000100000000000000000002000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000",
"status": "0",
"to": "0x302259069aaa5b10dc6f29a9a3f72a8e52837cc3",
"transactionHash": "0x7d48ae971faf089878b57e3c28e3035540d34f38af395958d2c73c36c57c83a2",
"transactionIndex": "0x1e",
"revertReason": "0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001d5468652073746f7265642076616c756520697320746f6f20736d616c6c000000",
"type": "0x0"
}`

const sampleTransactionTraceGeth = `{
"gas": 23512,
"failed": true,
Expand Down Expand Up @@ -174,7 +192,7 @@ const sampleTransactionTraceBesu = `{
"0000000000000000000000000000000000000000000000000000000000000000"
],
"storage": {},
"reason": "08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000114e6f7420656e6f75676820746f6b656e73000000000000000000000000000000"
"reason": "8c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000114e6f7420656e6f75676820746f6b656e73000000000000000000000000000000"
}
]
}`
Expand Down Expand Up @@ -391,7 +409,6 @@ func TestGetReceiptErrorReasonErrorFromHexDecode(t *testing.T) {
assert.Empty(t, reason)

assert.False(t, res.Success)

}

func TestGetReceiptErrorReasonErrorFromTrace(t *testing.T) {
Expand Down Expand Up @@ -428,7 +445,6 @@ func TestGetReceiptErrorReasonErrorFromTrace(t *testing.T) {
assert.Empty(t, reason)

assert.False(t, res.Success)

}

func TestGetReceiptErrorReasonErrorFromDecodeCallData(t *testing.T) {
Expand Down Expand Up @@ -464,7 +480,32 @@ func TestGetReceiptErrorReasonErrorFromDecodeCallData(t *testing.T) {
assert.Empty(t, reason)

assert.False(t, res.Success)
}

func TestGetReceiptErrorReasonErrorFromReceiptRevert(t *testing.T) {

ctx, c, mRPC, done := newTestConnector(t)
defer done()

mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_getTransactionReceipt",
mock.MatchedBy(func(txHash string) bool {
assert.Equal(t, "0x7d48ae971faf089878b57e3c28e3035540d34f38af395958d2c73c36c57c83a2", txHash)
return true
})).
Return(nil).
Run(func(args mock.Arguments) {
err := json.Unmarshal([]byte(sampleJSONRPCReceiptFailedWithRevertReason), args[1])
assert.NoError(t, err)
})
mRPC.AssertNotCalled(t, "CallRPC", mock.Anything, mock.Anything, "debug_traceTransaction", mock.Anything)
var req ffcapi.TransactionReceiptRequest
err := json.Unmarshal([]byte(sampleGetReceipt), &req)
assert.NoError(t, err)
res, reason, err := c.TransactionReceipt(ctx, &req)
assert.NoError(t, err)
assert.Empty(t, reason)
assert.Contains(t, res.ExtraInfo.String(), "The stored value is too small") // Check the decoded revert reason string is present in extra-info
assert.False(t, res.Success)
}

func TestProtocolIDForReceipt(t *testing.T) {
Expand Down

0 comments on commit 29020f8

Please sign in to comment.