Address feedback

Author: dimriou

pkg/txm/clientwrappers/dualbroadcast/meta_error_handler.go

@@ -24,15 +24,15 @@ func (e *errorHandler) HandleError(
 	txErr error,
 	txStore txm.TxStore,
 	setNonce func(common.Address, uint64),
-	isFromBroadcastMethod bool,
 ) (noTransmission bool, err error) {
 	// Mark the tx as fatal only if this is the first broadcast. In any other case, other txs might be included on-chain.
 	if (errors.Is(txErr, ErrNoBids) || errors.Is(txErr, ErrAuction)) && tx.AttemptCount == 1 {
 		if err := txStore.MarkTxFatal(ctx, tx, tx.FromAddress); err != nil {
 			return false, err
 		}
 		setNonce(tx.FromAddress, *tx.Nonce)
-		lggr.Infof("transaction with txID: %d marked as fatal", tx.ID)
+		l := logger.Sugared(logger.Named(lggr, "Txm.MetaErrorHandler"))
+		l.Infow("transaction marked as fatal", "txID", tx.ID, "transactionLifecycleID", tx.GetTransactionLifecycleID(l))
 		return true, nil
 	}
 

pkg/txm/clientwrappers/dualbroadcast/meta_error_handler_test.go

@@ -47,10 +47,10 @@ func TestMetaErrorHandler(t *testing.T) {
 		_, err = txStore.AppendAttemptToTransaction(*tx.Nonce, attempt)
 		require.NoError(t, err)
 		tx, _ = txStore.FetchUnconfirmedTransactionAtNonceWithCount(nonce)
-		noTransmission, err := errorHandler.HandleError(t.Context(), lggr, tx, ErrNoBids, txStoreManager, setNonce, false)
+		noTransmission, err := errorHandler.HandleError(t.Context(), lggr, tx, ErrNoBids, txStoreManager, setNonce)
 		require.NoError(t, err)
 		require.True(t, noTransmission)
-		tests.AssertLogEventually(t, observedLogs, "transaction with txID: 0 marked as fatal")
+		tests.AssertLogEventually(t, observedLogs, "transaction marked as fatal")
 		_, unconfirmedCount := txStore.FetchUnconfirmedTransactionAtNonceWithCount(nonce)
 		assert.Equal(t, 0, unconfirmedCount)
 	})
@@ -82,7 +82,7 @@ func TestMetaErrorHandler(t *testing.T) {
 		_, err = txStore.AppendAttemptToTransaction(*tx.Nonce, attempt)
 		require.NoError(t, err)
 		tx, _ = txStore.FetchUnconfirmedTransactionAtNonceWithCount(nonce)
-		noTransmission, err := errorHandler.HandleError(t.Context(), logger.Test(t), tx, txErr, txStoreManager, setNonce, false)
+		noTransmission, err := errorHandler.HandleError(t.Context(), logger.Test(t), tx, txErr, txStoreManager, setNonce)
 		require.NoError(t, err)
 		require.False(t, noTransmission)
 		_, unconfirmedCount := txStore.FetchUnconfirmedTransactionAtNonceWithCount(nonce)
@@ -115,10 +115,10 @@ func TestMetaErrorHandler(t *testing.T) {
 		_, err = txStore.AppendAttemptToTransaction(*tx.Nonce, attempt)
 		require.NoError(t, err)
 		tx, _ = txStore.FetchUnconfirmedTransactionAtNonceWithCount(nonce)
-		noTransmission, err := errorHandler.HandleError(t.Context(), lggr, tx, txErr, txStoreManager, setNonce, false)
+		noTransmission, err := errorHandler.HandleError(t.Context(), lggr, tx, txErr, txStoreManager, setNonce)
 		require.NoError(t, err)
 		require.True(t, noTransmission)
-		tests.AssertLogEventually(t, observedLogs, "transaction with txID: 0 marked as fatal")
+		tests.AssertLogEventually(t, observedLogs, "transaction marked as fatal")
 		_, unconfirmedCount := txStore.FetchUnconfirmedTransactionAtNonceWithCount(nonce)
 		assert.Equal(t, 0, unconfirmedCount)
 	})

pkg/txm/txm.go

@@ -59,7 +59,7 @@ type AttemptBuilder interface {
 type ErrorHandler interface {
 	// HandleError tries to decide if there was a successful transmission by parsing the error message. If it can't decide, it returns control to
 	// the standard execution path.
-	HandleError(context.Context, logger.Logger, *types.Transaction, error, TxStore, func(common.Address, uint64), bool) (noTransmission bool, err error)
+	HandleError(context.Context, logger.Logger, *types.Transaction, error, TxStore, func(common.Address, uint64)) (noTransmission bool, err error)
 }
 
 type StuckTxDetector interface {
@@ -342,7 +342,7 @@ func (t *Txm) sendTransactionWithError(ctx context.Context, tx *types.Transactio
 	if txErr != nil {
 		// If ErrorHandler is set, try to decide if there was a successful transmission by parsing the error message. If there wasn't, we return early.
 		if t.errorHandler != nil {
-			noTransmission, hErr := t.errorHandler.HandleError(ctx, t.lggr, tx, txErr, t.txStore, t.SetNonce, false)
+			noTransmission, hErr := t.errorHandler.HandleError(ctx, t.lggr, tx, txErr, t.txStore, t.SetNonce)
 			if hErr != nil {
 				return hErr
 			}

pkg/txm/txm_test.go

@@ -546,7 +546,7 @@ func TestFlow_ErrorHandler(t *testing.T) {
 	client.On("SendTransaction", mock.Anything, mock.Anything, mock.Anything).Return(dualbroadcast.ErrNoBids).Once()
 	_, err = tm.BroadcastTransaction(t.Context(), address)
 	require.NoError(t, err)
-	tests.AssertLogEventually(t, observedLogs, "transaction with txID: 0 marked as fatal")
+	tests.AssertLogEventually(t, observedLogs, "transaction marked as fatal")
 	_, count, err := txStoreManager.FetchUnconfirmedTransactionAtNonceWithCount(t.Context(), 0, address)
 	require.NoError(t, err)
 	require.Equal(t, 0, count)