Fix Nil pointer error in TXM stuck tx detector (#14685)

* add nil ptr check for findBroadcastedAttempts

* add changeset

* fix loop variable lint

* remove comment

* add nil ptr check for BroadcastBeforeBlockNum

* add error log

* update log fmt

* break logs

* update sanity check

* add unit test coverage for empty BroadcastBeforeBlockNum

* update comments

* fix test name

* fix make

Author: huangzhen1997

.changeset/chilled-plants-clap.md

@@ -0,0 +1,5 @@
+---
+"chainlink": patch
+---
+
+The findBroadcastedAttempts in detectStuckTransactionsHeuristic can returns uninitialized struct that potentially cause nil pointer error. Changed the return type of findBroadcastedAttempts to be pointers and added nil pointer check. #bugfix

core/chains/evm/txmgr/stuck_tx_detector.go

@@ -217,10 +217,25 @@ func (d *stuckTxDetector) detectStuckTransactionsHeuristic(ctx context.Context,
 		}
 		// Tx attempts are loaded from newest to oldest
 		oldestBroadcastAttempt, newestBroadcastAttempt, broadcastedAttemptsCount := findBroadcastedAttempts(tx)
+		d.lggr.Debugf("found %d broadcasted attempts for tx id %d in stuck transaction heuristic", broadcastedAttemptsCount, tx.ID)
+
+		// attempt shouldn't be nil as we validated in FindUnconfirmedTxWithLowestNonce, but added anyway for a "belts and braces" approach
+		if oldestBroadcastAttempt == nil || newestBroadcastAttempt == nil {
+			d.lggr.Debugw("failed to find broadcast attempt for tx in stuck transaction heuristic", "tx", tx)
+			continue
+		}
+
+		// sanity check
+		if oldestBroadcastAttempt.BroadcastBeforeBlockNum == nil {
+			d.lggr.Debugw("BroadcastBeforeBlockNum was not set for broadcast attempt in stuck transaction heuristic", "attempt", oldestBroadcastAttempt)
+			continue
+		}
+
 		// 2. Check if Threshold amount of blocks have passed since the oldest attempt's broadcast block num
 		if *oldestBroadcastAttempt.BroadcastBeforeBlockNum > blockNum-int64(*d.cfg.Threshold()) {
 			continue
 		}
+
 		// 3. Check if the transaction has at least MinAttempts amount of broadcasted attempts
 		if broadcastedAttemptsCount < *d.cfg.MinAttempts() {
 			continue
@@ -246,17 +261,18 @@ func compareGasFees(attemptGas gas.EvmFee, marketGas gas.EvmFee) int {
 }
 
 // Assumes tx attempts are loaded newest to oldest
-func findBroadcastedAttempts(tx Tx) (oldestAttempt TxAttempt, newestAttempt TxAttempt, broadcastedCount uint32) {
+func findBroadcastedAttempts(tx Tx) (oldestAttempt *TxAttempt, newestAttempt *TxAttempt, broadcastedCount uint32) {
 	foundNewest := false
-	for _, attempt := range tx.TxAttempts {
+	for i := range tx.TxAttempts {
+		attempt := tx.TxAttempts[i]
 		if attempt.State != types.TxAttemptBroadcast {
 			continue
 		}
 		if !foundNewest {
-			newestAttempt = attempt
+			newestAttempt = &attempt
 			foundNewest = true
 		}
-		oldestAttempt = attempt
+		oldestAttempt = &attempt
 		broadcastedCount++
 	}
 	return

core/chains/evm/txmgr/stuck_tx_detector_test.go

@@ -278,6 +278,15 @@ func TestStuckTxDetector_DetectStuckTransactionsHeuristic(t *testing.T) {
 		require.NoError(t, err)
 		require.Len(t, txs, 1)
 	})
+
+	t.Run("detects stuck transaction with empty BroadcastBeforeBlockNum in attempts will be skipped without panic", func(t *testing.T) {
+		_, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore)
+		enabledAddresses := []common.Address{fromAddress}
+		mustInsertUnconfirmedTxWithBroadcastAttemptsContainsEmptyBroadcastBeforeBlockNum(t, txStore, 0, fromAddress, autoPurgeMinAttempts, marketGasPrice.Add(oneGwei))
+		txs, err := stuckTxDetector.DetectStuckTransactions(ctx, enabledAddresses, blockNum)
+		require.NoError(t, err)
+		require.Len(t, txs, 0)
+	})
 }
 
 func TestStuckTxDetector_DetectStuckTransactionsZircuit(t *testing.T) {
@@ -537,6 +546,23 @@ func mustInsertUnconfirmedTxWithBroadcastAttempts(t *testing.T, txStore txmgr.Te
 	return etx
 }
 
+// helper function for edge case where broadcast attempt contains empty pointer
+func mustInsertUnconfirmedTxWithBroadcastAttemptsContainsEmptyBroadcastBeforeBlockNum(t *testing.T, txStore txmgr.TestEvmTxStore, nonce int64, fromAddress common.Address, numAttempts uint32, latestGasPrice *assets.Wei) txmgr.Tx {
+	ctx := tests.Context(t)
+	etx := cltest.MustInsertUnconfirmedEthTx(t, txStore, nonce, fromAddress)
+	// Insert attempts from oldest to newest
+	for i := int64(numAttempts - 1); i >= 0; i-- {
+		attempt := cltest.NewLegacyEthTxAttempt(t, etx.ID)
+		attempt.State = txmgrtypes.TxAttemptBroadcast
+		attempt.BroadcastBeforeBlockNum = nil
+		attempt.TxFee = gas.EvmFee{GasPrice: latestGasPrice.Sub(assets.NewWeiI(i))}
+		require.NoError(t, txStore.InsertTxAttempt(ctx, &attempt))
+	}
+	etx, err := txStore.FindTxWithAttempts(ctx, etx.ID)
+	require.NoError(t, err)
+	return etx
+}
+
 func mustInsertFatalErrorTxWithError(t *testing.T, txStore txmgr.TestEvmTxStore, nonce int64, fromAddress common.Address, blockNum int64) txmgr.Tx {
 	etx := cltest.NewEthTx(fromAddress)
 	etx.State = txmgrcommon.TxFatalError