op-service: use binary search instead of walkback for checkRecentTxs (#11232)

* use binary search instead of walkback for checkRecentTxs

* account for multiple txs from same sender in same block

* use recursion if reorg detected

* limit condition to return false

op-service/eth/transactions.go

@@ -50,14 +50,16 @@ func TransactionsToHashes(elems []*types.Transaction) []common.Hash {
 	return out
 }
 
-// CheckRecentTxs checks the depth recent blocks for transactions from the account with address addr
-// and returns the most recent block and true, if any was found, or the oldest block checked and false, if not.
+// CheckRecentTxs checks the depth recent blocks for txs from the account with address addr
+// and returns either:
+//   - blockNum containing the last tx and true if any was found
+//   - the oldest block checked and false if no nonce change was found
 func CheckRecentTxs(
 	ctx context.Context,
 	l1 L1Client,
 	depth int,
 	addr common.Address,
-) (recentBlock uint64, found bool, err error) {
+) (blockNum uint64, found bool, err error) {
 	blockHeader, err := l1.HeaderByNumber(ctx, nil)
 	if err != nil {
 		return 0, false, fmt.Errorf("failed to retrieve current block header: %w", err)
@@ -69,25 +71,47 @@ func CheckRecentTxs(
 		return 0, false, fmt.Errorf("failed to retrieve current nonce: %w", err)
 	}
 
-	oldestBlock := new(big.Int)
-	oldestBlock.Sub(currentBlock, big.NewInt(int64(depth)))
+	oldestBlock := new(big.Int).Sub(currentBlock, big.NewInt(int64(depth)))
 	previousNonce, err := l1.NonceAt(ctx, addr, oldestBlock)
 	if err != nil {
 		return 0, false, fmt.Errorf("failed to retrieve previous nonce: %w", err)
 	}
 
 	if currentNonce == previousNonce {
+		// Most recent tx is older than the given depth
 		return oldestBlock.Uint64(), false, nil
 	}
 
-	// Decrease block num until we find the block before the most recent batcher tx was sent
-	targetNonce := currentNonce - 1
-	for currentNonce > targetNonce && currentBlock.Cmp(oldestBlock) != -1 {
-		currentBlock.Sub(currentBlock, big.NewInt(1))
-		currentNonce, err = l1.NonceAt(ctx, addr, currentBlock)
+	// Use binary search to find the block where the nonce changed
+	low := oldestBlock.Uint64()
+	high := currentBlock.Uint64()
+
+	for low < high {
+		mid := (low + high) / 2
+		midNonce, err := l1.NonceAt(ctx, addr, new(big.Int).SetUint64(mid))
 		if err != nil {
-			return 0, false, fmt.Errorf("failed to retrieve nonce: %w", err)
+			return 0, false, fmt.Errorf("failed to retrieve nonce at block %d: %w", mid, err)
+		}
+
+		if midNonce > currentNonce {
+			// Catch a reorg that causes inconsistent nonce
+			return CheckRecentTxs(ctx, l1, depth, addr)
+		} else if midNonce == currentNonce {
+			high = mid
+		} else {
+			// midNonce < currentNonce: check the next block to see if we've found the
+			// spot where the nonce transitions to the currentNonce
+			nextBlockNum := mid + 1
+			nextBlockNonce, err := l1.NonceAt(ctx, addr, new(big.Int).SetUint64(nextBlockNum))
+			if err != nil {
+				return 0, false, fmt.Errorf("failed to retrieve nonce at block %d: %w", mid, err)
+			}
+
+			if nextBlockNonce == currentNonce {
+				return nextBlockNum, true, nil
+			}
+			low = mid + 1
 		}
 	}
-	return currentBlock.Uint64() + 1, true, nil
+	return oldestBlock.Uint64(), false, nil
 }

op-service/eth/transactions_test.go

@@ -30,111 +30,98 @@ func (m *MockL1Client) HeaderByNumber(ctx context.Context, number *big.Int) (*ty
 
 func TestTransactions_checkRecentTxs(t *testing.T) {
 	tests := []struct {
-		name               string
-		currentBlock       uint64
-		blockConfirms      uint64
-		previousNonceBlock uint64
-		expectedBlockNum   uint64
-		expectedFound      bool
+		name             string
+		currentBlock     int64
+		blockConfirms    uint64
+		expectedBlockNum uint64
+		expectedFound    bool
+		blocks           map[int64][]uint64 // maps blockNum --> nonceVal (one for each stubbed call)
 	}{
 		{
-			// Blocks       495 496 497 498 499 500
-			// Nonce          5   5   5   6   6   6
-			// call NonceAt   x   -   x   x   x   x
-			name:               "NonceDiff_3Blocks",
-			currentBlock:       500,
-			blockConfirms:      5,
-			previousNonceBlock: 497,
-			expectedBlockNum:   498,
-			expectedFound:      true,
+			name:             "nonceDiff_lowerBound",
+			currentBlock:     500,
+			blockConfirms:    5,
+			expectedBlockNum: 496,
+			expectedFound:    true,
+			blocks: map[int64][]uint64{
+				495: {5, 5},
+				496: {6, 6},
+				497: {6},
+				500: {6},
+			},
 		},
 		{
-			// Blocks       495 496 497 498 499 500
-			// Nonce          5   5   5   5   5   6
-			// call NonceAt   x   -   -   -   x   x
-			name:               "NonceDiff_1Block",
-			currentBlock:       500,
-			blockConfirms:      5,
-			previousNonceBlock: 499,
-			expectedBlockNum:   500,
-			expectedFound:      true,
+			name:             "nonceDiff_midRange",
+			currentBlock:     500,
+			blockConfirms:    5,
+			expectedBlockNum: 497,
+			expectedFound:    true,
+			blocks: map[int64][]uint64{
+				495: {5},
+				496: {5},
+				497: {6, 6},
+				500: {6},
+			},
 		},
 		{
-			// Blocks       495 496 497 498 499 500
-			// Nonce          6   6   6   6   6   6
-			// call NonceAt   x   -   -   -   -   x
-			name:               "NonceUnchanged",
-			currentBlock:       500,
-			blockConfirms:      5,
-			previousNonceBlock: 400,
-			expectedBlockNum:   495,
-			expectedFound:      false,
+			name:             "nonceDiff_upperBound",
+			currentBlock:     500,
+			blockConfirms:    5,
+			expectedBlockNum: 500,
+			expectedFound:    true,
+			blocks: map[int64][]uint64{
+				495: {5},
+				497: {5},
+				498: {5},
+				499: {5},
+				500: {6, 6},
+			},
+		},
+		{
+			name:             "nonce_unchanged",
+			currentBlock:     500,
+			blockConfirms:    5,
+			expectedBlockNum: 495,
+			expectedFound:    false,
+			blocks: map[int64][]uint64{
+				495: {6},
+				500: {6},
+			},
+		},
+		{
+			name:             "reorg",
+			currentBlock:     500,
+			blockConfirms:    5,
+			expectedBlockNum: 496,
+			expectedFound:    true,
+			blocks: map[int64][]uint64{
+				495: {5, 5, 5},
+				496: {7, 7, 7},
+				497: {6, 7},
+				500: {6, 7},
+			},
 		},
 	}
 
 	for _, tt := range tests {
-
 		t.Run(tt.name, func(t *testing.T) {
 			l1Client := new(MockL1Client)
 			ctx := context.Background()
 
-			currentNonce := uint64(6)
-			previousNonce := uint64(5)
-
-			l1Client.On("HeaderByNumber", ctx, (*big.Int)(nil)).Return(&types.Header{Number: big.NewInt(int64(tt.currentBlock))}, nil)
-
-			// Setup mock calls for NonceAt, depending on how many times its expected to be called
-			if tt.previousNonceBlock < tt.currentBlock-tt.blockConfirms {
-				l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(tt.currentBlock))).Return(currentNonce, nil)
-				l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(tt.currentBlock-tt.blockConfirms))).Return(currentNonce, nil)
-			} else {
-				for block := tt.currentBlock; block >= (tt.currentBlock - tt.blockConfirms); block-- {
-					blockBig := big.NewInt(int64(block))
-					if block > (tt.currentBlock-tt.blockConfirms) && block < tt.previousNonceBlock {
-						t.Log("skipped block: ", block)
-						continue
-					} else if block <= tt.previousNonceBlock {
-						t.Log("previousNonce set at block: ", block)
-						l1Client.On("NonceAt", ctx, common.Address{}, blockBig).Return(previousNonce, nil)
-					} else {
-						t.Log("currentNonce set at block: ", block)
-						l1Client.On("NonceAt", ctx, common.Address{}, blockBig).Return(currentNonce, nil)
-					}
+			// Setup mock responses
+			l1Client.On("HeaderByNumber", ctx, (*big.Int)(nil)).Return(&types.Header{Number: big.NewInt(tt.currentBlock)}, nil)
+			for blockNum, block := range tt.blocks {
+				for _, nonce := range block {
+					l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(blockNum)).Return(nonce, nil).Once()
 				}
 			}
 
 			blockNum, found, err := CheckRecentTxs(ctx, l1Client, 5, common.Address{})
 			require.NoError(t, err)
-			require.Equal(t, tt.expectedBlockNum, blockNum)
 			require.Equal(t, tt.expectedFound, found)
+			require.Equal(t, tt.expectedBlockNum, blockNum)
 
 			l1Client.AssertExpectations(t)
 		})
 	}
 }
-func TestTransactions_checkRecentTxs_reorg(t *testing.T) {
-	l1Client := new(MockL1Client)
-	ctx := context.Background()
-
-	currentNonce := uint64(6)
-	currentBlock := uint64(500)
-	blockConfirms := uint64(5)
-
-	l1Client.On("HeaderByNumber", ctx, (*big.Int)(nil)).Return(&types.Header{Number: big.NewInt(int64(currentBlock))}, nil)
-	l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(currentBlock))).Return(currentNonce, nil)
-
-	l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(currentBlock-blockConfirms))).Return(currentNonce+1, nil)
-	l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(currentBlock-1))).Return(currentNonce, nil)
-	l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(currentBlock-2))).Return(currentNonce, nil)
-	l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(currentBlock-3))).Return(currentNonce, nil)
-	l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(currentBlock-4))).Return(currentNonce, nil)
-	l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(currentBlock-5))).Return(currentNonce, nil)
-	l1Client.On("NonceAt", ctx, common.Address{}, big.NewInt(int64(currentBlock-6))).Return(currentNonce, nil)
-
-	blockNum, found, err := CheckRecentTxs(ctx, l1Client, 5, common.Address{})
-	require.NoError(t, err)
-	require.Equal(t, uint64(495), blockNum)
-	require.Equal(t, true, found)
-
-	l1Client.AssertExpectations(t)
-}