Merge pull request #131 from kaleido-io/revert-trace-conf

Make the use of `debug_traceTransaction` configurable

config.md

@@ -75,6 +75,7 @@
 |passthroughHeadersEnabled|Enable passing through the set of allowed HTTP request headers|`boolean`|`false`
 |requestTimeout|The maximum amount of time that a request is allowed to remain open|[`time.Duration`](https://pkg.go.dev/time#Duration)|`30s`
 |tlsHandshakeTimeout|The maximum amount of time to wait for a successful TLS handshake|[`time.Duration`](https://pkg.go.dev/time#Duration)|`10s`
+|traceTXForRevertReason|Enable the use of transaction trace functions (e.g. debug_traceTransaction) to obtain transaction revert reasons. This can place a high load on the EVM client.|`boolean`|`false`
 |txCacheSize|Maximum of transactions to hold in the transaction info cache|`int`|`250`
 |url|URL of JSON/RPC endpoint for the Ethereum node/gateway|string|`<nil>`
 

internal/ethereum/config.go

@@ -37,6 +37,7 @@ const (
 	RetryFactor                 = "retry.factor"
 	MaxConcurrentRequests       = "maxConcurrentRequests"
 	TxCacheSize                 = "txCacheSize"
+	TraceTXForRevertReason      = "traceTXForRevertReason"
 )
 
 const (
@@ -70,4 +71,5 @@ func InitConfig(conf config.Section) {
 	conf.AddKnownKey(RetryMaxDelay, DefaultRetryMaxDelay)
 	conf.AddKnownKey(MaxConcurrentRequests, 50)
 	conf.AddKnownKey(TxCacheSize, 250)
+	conf.AddKnownKey(TraceTXForRevertReason, false)
 }

internal/ethereum/ethereum.go

@@ -49,6 +49,7 @@ type ethConnector struct {
 	eventBlockTimestamps       bool
 	blockListener              *blockListener
 	eventFilterPollingInterval time.Duration
+	traceTXForRevertReason     bool
 
 	mux          sync.Mutex
 	eventStreams map[fftypes.UUID]*eventStream
@@ -64,6 +65,7 @@ func NewEthereumConnector(ctx context.Context, conf config.Section) (cc ffcapi.A
 		checkpointBlockGap:         conf.GetInt64(EventsCheckpointBlockGap),
 		eventBlockTimestamps:       conf.GetBool(EventsBlockTimestamps),
 		eventFilterPollingInterval: conf.GetDuration(EventsFilterPollingInterval),
+		traceTXForRevertReason:     conf.GetBool(TraceTXForRevertReason),
 		retry: &retry.Retry{
 			InitialDelay: conf.GetDuration(RetryInitDelay),
 			MaximumDelay: conf.GetDuration(RetryMaxDelay),

internal/ethereum/ethereum_test.go

@@ -37,6 +37,7 @@ func newTestConnector(t *testing.T) (context.Context, *ethConnector, *rpcbackend
 	config.RootConfigReset()
 	conf := config.RootSection("unittest")
 	InitConfig(conf)
+	//conf.Set(TraceTXForRevertReason, true)
 	conf.Set(ffresty.HTTPConfigURL, "http://localhost:8545")
 	conf.Set(BlockPollingInterval, "1h") // Disable for tests that are not using it
 	logrus.SetLevel(logrus.DebugLevel)

internal/ethereum/get_receipt.go

@@ -134,23 +134,26 @@ func (c *ethConnector) getErrorInfo(ctx context.Context, transactionHash 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
-		}
+		// Tracing a transaction to get revert information is expensive so it's not enabled by default
+		if c.traceTXForRevertReason {
+			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
+			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
+					}
 				}
 			}
 		}

internal/ethereum/get_receipt_test.go

@@ -269,6 +269,7 @@ func TestGetReceiptError(t *testing.T) {
 func TestGetReceiptErrorReasonGeth(t *testing.T) {
 
 	ctx, c, mRPC, done := newTestConnector(t)
+	c.traceTXForRevertReason = true
 	defer done()
 
 	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_getTransactionReceipt",
@@ -306,6 +307,7 @@ func TestGetReceiptErrorReasonGeth(t *testing.T) {
 func TestGetReceiptErrorReasonBesu(t *testing.T) {
 
 	ctx, c, mRPC, done := newTestConnector(t)
+	c.traceTXForRevertReason = true
 	defer done()
 
 	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_getTransactionReceipt",
@@ -343,6 +345,7 @@ func TestGetReceiptErrorReasonBesu(t *testing.T) {
 func TestGetReceiptErrorReasonNotFound(t *testing.T) {
 
 	ctx, c, mRPC, done := newTestConnector(t)
+	c.traceTXForRevertReason = true
 	defer done()
 
 	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_getTransactionReceipt",
@@ -379,6 +382,7 @@ func TestGetReceiptErrorReasonNotFound(t *testing.T) {
 func TestGetReceiptErrorReasonErrorFromHexDecode(t *testing.T) {
 
 	ctx, c, mRPC, done := newTestConnector(t)
+	c.traceTXForRevertReason = true
 	defer done()
 
 	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_getTransactionReceipt",
@@ -415,6 +419,7 @@ func TestGetReceiptErrorReasonErrorFromTrace(t *testing.T) {
 	// if we get an error tracing the transaction, we ignore it.  Not all nodes support the debug_traceTransaction RPC call
 
 	ctx, c, mRPC, done := newTestConnector(t)
+	c.traceTXForRevertReason = true
 	defer done()
 
 	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_getTransactionReceipt",
@@ -450,6 +455,7 @@ func TestGetReceiptErrorReasonErrorFromTrace(t *testing.T) {
 func TestGetReceiptErrorReasonErrorFromDecodeCallData(t *testing.T) {
 
 	ctx, c, mRPC, done := newTestConnector(t)
+	c.traceTXForRevertReason = true
 	defer done()
 
 	mRPC.On("CallRPC", mock.Anything, mock.Anything, "eth_getTransactionReceipt",
@@ -508,6 +514,32 @@ func TestGetReceiptErrorReasonErrorFromReceiptRevert(t *testing.T) {
 	assert.False(t, res.Success)
 }
 
+func TestGetReceiptNoDebugTraceIfDisabled(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(sampleJSONRPCReceiptFailed), args[1])
+			assert.NoError(t, err)
+		})
+	mRPC.AssertNotCalled(t, "CallRPC", mock.Anything, mock.Anything, "debug_traceTransaction")
+	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.False(t, res.Success)
+	mRPC.AssertExpectations(t)
+}
+
 func TestProtocolIDForReceipt(t *testing.T) {
 	assert.Equal(t, "000000012345/000042", ProtocolIDForReceipt(fftypes.NewFFBigInt(12345), fftypes.NewFFBigInt(42)))
 	assert.Equal(t, "", ProtocolIDForReceipt(nil, nil))

internal/msgs/en_config_descriptions.go

@@ -40,4 +40,5 @@ var (
 	ConfigEventsFilterPollingInterval = ffc("config.connector.events.filterPollingInterval", "The interval between polling calls to a filter, when checking for newly arrived events", i18n.TimeDurationType)
 	ConfigTxCacheSize                 = ffc("config.connector.txCacheSize", "Maximum of transactions to hold in the transaction info cache", i18n.IntType)
 	ConfigMaxConcurrentRequests       = ffc("config.connector.maxConcurrentRequests", "Maximum of concurrent requests to be submitted to the blockchain", i18n.IntType)
+	ConfigTraceTXForRevertReason      = ffc("config.connector.traceTXForRevertReason", "Enable the use of transaction trace functions (e.g. debug_traceTransaction) to obtain transaction revert reasons. This can place a high load on the EVM client.", i18n.BooleanType)
 )