diff --git a/internal/ethereum/get_receipt.go b/internal/ethereum/get_receipt.go index 88d81c0..97a1334 100644 --- a/internal/ethereum/get_receipt.go +++ b/internal/ethereum/get_receipt.go @@ -26,6 +26,7 @@ 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" @@ -33,17 +34,18 @@ import ( // 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. @@ -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 { @@ -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) { @@ -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 diff --git a/internal/ethereum/get_receipt_test.go b/internal/ethereum/get_receipt_test.go index a952833..5aaa2e4 100644 --- a/internal/ethereum/get_receipt_test.go +++ b/internal/ethereum/get_receipt_test.go @@ -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, @@ -174,7 +192,7 @@ const sampleTransactionTraceBesu = `{ "0000000000000000000000000000000000000000000000000000000000000000" ], "storage": {}, - "reason": "08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000114e6f7420656e6f75676820746f6b656e73000000000000000000000000000000" + "reason": "8c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000114e6f7420656e6f75676820746f6b656e73000000000000000000000000000000" } ] }` @@ -391,7 +409,6 @@ func TestGetReceiptErrorReasonErrorFromHexDecode(t *testing.T) { assert.Empty(t, reason) assert.False(t, res.Success) - } func TestGetReceiptErrorReasonErrorFromTrace(t *testing.T) { @@ -428,7 +445,6 @@ func TestGetReceiptErrorReasonErrorFromTrace(t *testing.T) { assert.Empty(t, reason) assert.False(t, res.Success) - } func TestGetReceiptErrorReasonErrorFromDecodeCallData(t *testing.T) { @@ -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) {