internal/common/trace.go (1)

60-78: ⚠️ Potential issue

BlockTimestamp is never serialized

TraceModel includes a BlockTimestamp field (uint64) but Serialize() omits it. Downstream consumers will receive zero values.

-		BlockHash:        t.BlockHash,
+		BlockHash:        t.BlockHash,
+		BlockTimestamp:   uint64(t.BlockTimestamp.Unix()),

Please verify the desired unit (seconds vs milliseconds) and update tests accordingly.

internal/validation/cursor.go (1)

37-50: Consider making the cursor file location configurable.

The current implementation writes cursor files to the working directory, which might be problematic in containerized environments with restricted filesystem access.

+// CursorFilePath is the directory where cursor files are stored
+var CursorFilePath string = "."
+
 func (c *Cursor) Update(blockNumber *big.Int) error {
-	cursorFile := fmt.Sprintf("validation_cursor_%s.json", c.ChainId.String())
+	cursorFile := fmt.Sprintf("%s/validation_cursor_%s.json", CursorFilePath, c.ChainId.String())

The same change would need to be applied to the getLastScannedBlock function.

internal/validation/validation.go (1)

163-166: isContractCreation misses the “empty string” case

A contract-creation tx coming from go-ethereum has To == nil, which your DB
may serialise as an empty string. The current check only treats the all-zero
address as creation:

isContractCreation := tx.ToAddress == "0x0000…0000" && tx.Data != "0x"

Consider:

isContractCreation := (tx.ToAddress == "" ||
-	 tx.ToAddress == "0x0000000000000000000000000000000000000000") && tx.Data != "0x"
+	 tx.ToAddress == "0x0000000000000000000000000000000000000000") &&
+	 len(tx.Data) > 2 // guards against meaningless “0x”

cmd/validate.go (2)

78-86: Harden TLS configuration

The explicit tls.Config{} inherits Go’s default (TLS 1.2 for clients). For a CLI tool that only talks to a modern ClickHouse server you can safely raise the floor:

-		TLS:      &tls.Config{},
+		TLS: &tls.Config{
+			MinVersion: tls.VersionTLS13,
+		},

Not critical but improves security posture.

---

50-52: Off-by-one adjustment is opaque

batchSize = batchSize - 1 relies on readers knowing the inclusive semantics later. Consider renaming the variable or adding a short comment to avoid future errors.

// range end is inclusive, adjust once so we fetch exactly N blocks
batchSizeInclusive := new(big.Int).Sub(batchSize, big.NewInt(1))

internal/validation/validation.go (1)

46-54: *Misleading debug message when printing big.Int values

log.Debug().Msgf("Processing batch of blocks %d to %d", …) prints the decimal
representation of the pointers, not the block numbers, because %d formats
a numeric value, not an object implementing Stringer.
Prefer %s (or %v) to leverage (*big.Int).String():

- log.Debug().Msgf("Processing batch of blocks %d to %d", invalidBlocks[i], invalidBlocks[end-1])
+ log.Debug().Msgf("Processing batch of blocks %s to %s",
+ 	invalidBlocks[i].String(), invalidBlocks[end-1].String())

internal/validation/db.go (1)

120-124: Potential goroutine leak if one query errors early

Each goroutine writes exactly once to its channel, but if an early return
occurs before the send, the receiver blocks forever. Although you currently
return only after sending, future edits could break the guarantee.

A safer pattern is to close the channel once the value is sent or use a
sync.WaitGroup.
Example with a WaitGroup:

var (
   blocksResult blockResult
   …
)
wg.Add(3)
go func() { defer wg.Done(); blocksResult.blocks, blocksResult.err = … }()
…
wg.Wait()

internal/validation/duplicates.go (1)

161-199: removeDuplicateTransactions may exceed ClickHouse parameter limits

The batched deletion is great, but each loop aggregates all hashes in one
IN clause. If a batch nears 1000 items, ClickHouse can hit the default
max_query_size. Consider:

• Lowering batchSize (e.g., 100).
• Or chunking hashes too: execute the same query multiple times with
  smaller slices.

This keeps memory and network utilisation predictable.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

internal/validation/validation.go (2)

286-295: ⚠️ Potential issue

*Handle nil big.Int inputs to uint256.FromBig to prevent panics

The code doesn't check for nil values before converting big.Int to uint256.Int, which could cause panics for older blocks or special transactions where some fields may be nil.

Implement a helper function at the beginning of the file:

// safeUint256 converts a *big.Int to *uint256.Int safely, handling nil values
func safeUint256(b *big.Int) *uint256.Int {
    if b == nil {
        return new(uint256.Int)
    }
    out, _ := uint256.FromBig(b)
    return out
}

Then replace all the conversions:

-chainID, _ := uint256.FromBig(tx.ChainId)
-gasTipCap, _ := uint256.FromBig(tx.MaxPriorityFeePerGas)
-gasFeeCap, _ := uint256.FromBig(tx.MaxFeePerGas)
-value, _ := uint256.FromBig(tx.Value)
-blobFeeCap, _ := uint256.FromBig(tx.MaxFeePerBlobGas)
-v, _ := uint256.FromBig(tx.V)
-r, _ := uint256.FromBig(tx.R)
-s, _ := uint256.FromBig(tx.S)
+chainID := safeUint256(tx.ChainId)
+gasTipCap := safeUint256(tx.MaxPriorityFeePerGas)
+gasFeeCap := safeUint256(tx.MaxFeePerGas)
+value := safeUint256(tx.Value)
+blobFeeCap := safeUint256(tx.MaxFeePerBlobGas)
+v := safeUint256(tx.V)
+r := safeUint256(tx.R)
+s := safeUint256(tx.S)

Also apply the same pattern to lines 332-338.

---

56-58: ⚠️ Potential issue

Check error return from poller.Poll

The Poll method likely returns an error that is not being checked. This could silently ignore failures in the polling process.

poller := orchestrator.NewBoundlessPoller(rpcClient, s)
-poller.Poll(context.Background(), batch)
+if err := poller.Poll(context.Background(), batch); err != nil {
+    return fmt.Errorf("failed to poll blocks %v: %w", batch, err)
+}
log.Debug().Msgf("Batch of invalid blocks polled: %d to %d", invalidBlocks[i], invalidBlocks[end-1])

internal/validation/duplicates.go (1)

213-229: 🛠️ Refactor suggestion

String-built IN clause risks gigantic queries & SQL-injection

Building the IN clause directly with fmt.Sprintf could lead to oversized queries if there are many tuples, and potentially allows SQL injection vulnerabilities even though the values come from the database.

Consider one of these approaches:

1. Use ClickHouse array parameters instead:

-tuples := make([]string, 0, len(batch))
+txHashes := make([]string, 0, len(batch))
+logIndices := make([]uint64, 0, len(batch))

for _, duplicateLog := range batch {
  blockNumbers = append(blockNumbers, duplicateLog.BlockNumber)
-  tuples = append(tuples, fmt.Sprintf("('%s', %d)", duplicateLog.TxHash, duplicateLog.LogIndex))
+  txHashes = append(txHashes, duplicateLog.TxHash)
+  logIndices = append(logIndices, duplicateLog.LogIndex)
}

-query := fmt.Sprintf(`WITH
+query := `WITH
  to_be_inserted AS (
    SELECT chain_id, block_number, block_hash, block_timestamp, transaction_hash, transaction_index, log_index, address,
      data, topic_0, topic_1, topic_2, topic_3, insert_timestamp, -sign as sign
    FROM default.logs FINAL
-    WHERE chain_id = ? AND block_number IN (?) AND (transaction_hash, log_index) IN (%s)
+    WHERE chain_id = ? AND block_number IN (?) AND (transaction_hash, log_index) IN ZIP(?, ?)
  )
  INSERT INTO logs (
    chain_id, block_number, block_hash, block_timestamp, transaction_hash, transaction_index, log_index, address,
    data, topic_0, topic_1, topic_2, topic_3, insert_timestamp, sign
  ) SELECT * from to_be_inserted
-`, strings.Join(tuples, ","))
+`

-err := conn.Exec(context.Background(), query, chainId, blockNumbers)
+err := conn.Exec(context.Background(), query, chainId, blockNumbers, txHashes, logIndices)

cmd/validate.go (1)

19-26: Update the TBD command documentation

The command's Short and Long descriptions are set to "TBD" which should be updated with meaningful descriptions to help users understand the purpose and functionality of the validate command.

var (
	validateCmd = &cobra.Command{
-		Use:   "validate",
-		Short: "TBD",
-		Long:  "TBD",
+		Use:   "validate [batchSize] [fixBatchSize]",
+		Short: "Validates and fixes blockchain data integrity",
+		Long:  "Validates blockchain data integrity by detecting and removing duplicates, filling gaps, and verifying block data. Takes optional batch size (default: 1000) and fix batch size (default: 0) arguments.",
		Run: func(cmd *cobra.Command, args []string) {
			RunValidate(cmd, args)
		},
	}
)

internal/validation/validation.go (2)

164-166: Better contract creation detection

The contract creation detection logic relies on checking for the zero address string and non-empty data, which could be error-prone. Consider using a more robust approach.

-isContractCreation := tx.ToAddress == "0x0000000000000000000000000000000000000000" && tx.Data != "0x"
+// Contract creation transactions have empty "to" field, but in the database they might be
+// stored with the zero address. Check for both empty address and zero address.
+isContractCreation := (tx.ToAddress == "" || tx.ToAddress == "0x0000000000000000000000000000000000000000") && tx.Data != "0x"

---

98-104: Add comments explaining transaction type checking

The code skips transaction root validation for transaction types > 4, but doesn't explain why. Add comments to clarify which transaction types are supported and why others are skipped.

// TODO: remove this once we know how to validate all tx types
for _, tx := range blockData.Transactions {
-    if tx.TransactionType > 4 { // Currently supported types are 0-4
+    // Currently supported transaction types:
+    // 0: Legacy
+    // 1: Access List (EIP-2930)
+    // 2: Dynamic Fee (EIP-1559)
+    // 3: Blob (EIP-4844)
+    // 4: Set Code (EIP-7702)
+    // Skip validation for newer, unsupported transaction types
+    if tx.TransactionType > 4 {
        log.Warn().Msgf("Skipping transaction root validation for block %s due to unsupported transaction type %d", blockData.Block.Number, tx.TransactionType)
        return nil
    }
}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cmd/validate.go (2)

69-87: Set TLS MinVersion for better security

The TLS configuration is missing a MinVersion setting, which could allow connections with insecure TLS protocol versions. This was previously identified and should be addressed by adding MinVersion: tls.VersionTLS13 to enhance connection security.

🧰 Tools

🪛 ast-grep (0.38.1)

[warning] 71-71: MinVersionis missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. AddMinVersion: tls.VersionTLS13' to the TLS configuration to bump the minimum version to TLS 1.3.
Context: tls.Config{}
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
https://owasp.org/Top10/A02_2021-Cryptographic_Failures

(missing-ssl-minversion-go)

---

115-133: Improve error handling in validateAndFixRange

The function uses log.Fatal() which terminates the program immediately without allowing cleanup or graceful shutdown. As previously suggested, consider returning errors instead and handling them at a higher level to allow for proper cleanup and graceful error handling.

internal/validation/validation.go (2)

287-294: *Handle nil big.Int inputs to uint256.FromBig to prevent panics

The fields tx.MaxPriorityFeePerGas, tx.MaxFeePerBlobGas, tx.Value, tx.R, tx.S, etc. may be nil for certain transaction types. Calling uint256.FromBig(nil) will panic. A helper function safeUint256 should be introduced to handle nil inputs safely as previously suggested.

---

332-338: *Handle nil big.Int inputs to uint256.FromBig to prevent panics

Same issue as lines 287-294 - the conversion to uint256 needs null-safety to prevent runtime panics when big.Int fields are nil.

internal/validation/duplicates.go (4)

32-34: Error from removeDuplicateBlocks is silently ignored

The call to removeDuplicateBlocks does not check the returned error.
If the deduplication query fails you will continue as if the data were clean, potentially cascading bad state into gap-filling and validation steps.

-        removeDuplicateBlocks(conn, chainId, duplicateBlockNumbers)
+        if err := removeDuplicateBlocks(conn, chainId, duplicateBlockNumbers); err != nil {
+            return fmt.Errorf("failed to remove duplicate blocks: %w", err)
+        }

🧰 Tools

🪛 golangci-lint (1.64.8)

33-33: Error return value is not checked

(errcheck)

---

43-45: Error from removeDuplicateTransactions is silently ignored

Same issue as above for transaction duplicates. Propagate or at least log the failure so that the caller can abort or retry.

-        removeDuplicateTransactions(conn, chainId, duplicateTransactions)
+        if err := removeDuplicateTransactions(conn, chainId, duplicateTransactions); err != nil {
+            return fmt.Errorf("failed to remove duplicate transactions: %w", err)
+        }

🧰 Tools

🪛 golangci-lint (1.64.8)

44-44: Error return value is not checked

(errcheck)

---

54-56: Error from removeDuplicateLogs is silently ignored

Please handle the error to avoid continuing with an inconsistent database state.

-        removeDuplicateLogs(conn, chainId, duplicateLogs)
+        if err := removeDuplicateLogs(conn, chainId, duplicateLogs); err != nil {
+            return fmt.Errorf("failed to remove duplicate logs: %w", err)
+        }

🧰 Tools

🪛 golangci-lint (1.64.8)

55-55: Error return value is not checked

(errcheck)

---

218-230: String-built tuple list risks gigantic queries & SQL-injection

fmt.Sprintf concatenates every (txHash, logIndex) tuple straight into the SQL.
Drawbacks:

1. Large duplicate batches can blow ClickHouse’s max query length.
2. Although data originates from the DB, handcrafted SQL bypasses driver-level sanitisation.

Prefer parameterised arrays or smaller, parameterised batches, e.g.:

- WHERE chain_id = ? AND block_number IN (?) AND (transaction_hash, log_index) IN (%s)
+ WHERE chain_id = ? AND block_number IN (?) AND (transaction_hash, log_index) IN (?)

Then pass []tuple(String, UInt64) as the third argument, or fall back to the existing 1 000-row batching but build the tuples slice as a proper driver parameter instead of raw SQL.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cmd/validate.go (2)

69-87: TLS configuration missing MinVersion setting.

This security issue has already been identified in previous reviews. The TLS configuration should specify a minimum version for enhanced security.

🧰 Tools

🪛 ast-grep (0.38.1)

[warning] 71-71: MinVersionis missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. AddMinVersion: tls.VersionTLS13' to the TLS configuration to bump the minimum version to TLS 1.3.
Context: tls.Config{}
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
https://owasp.org/Top10/A02_2021-Cryptographic_Failures

(missing-ssl-minversion-go)

---

115-134: Error handling uses log.Fatal() preventing graceful shutdown.

This error handling issue has already been identified in previous reviews. The function should return errors instead of calling log.Fatal() to allow proper cleanup and graceful error handling.

internal/validation/validation.go (1)

286-296: *Handle nil big.Int inputs to prevent panics.

This nil pointer risk with uint256.FromBig() has already been identified in previous reviews. Fields like MaxPriorityFeePerGas, MaxFeePerBlobGas, Value, R, S can be nil and need safe handling to prevent runtime panics.

Also applies to: 332-340

internal/validation/duplicates.go (3)

33-34: Error return values not handled.

The calls to removeDuplicateBlocks, removeDuplicateTransactions, and removeDuplicateLogs return errors that are not being checked, as already identified in previous reviews. These should be handled to prevent silent failures.

Also applies to: 44-45, 55-56

🧰 Tools

🪛 golangci-lint (1.64.8)

33-33: Error return value is not checked

(errcheck)

---

62-67: HAVING clause may miss certain duplicate scenarios.

The HAVING sum(sign) != 1 condition has already been identified as potentially problematic in previous reviews, as it could miss cases where duplicates exist but sum to 1.

---

218-232: SQL injection risk with string interpolation.

The use of fmt.Sprintf to build the IN clause has already been identified as a security risk in previous reviews. This should use parameterized queries instead.

internal/validation/validation.go (1)

97-103: Consider removing or refining the transaction type check.

The current check skips validation for transaction types > 4, but this hardcoded limit may become outdated as new transaction types are introduced. Consider either:

• Making this configurable
• Adding a more explicit comment about which types are supported
• Implementing a more robust unsupported type detection

- for _, tx := range blockData.Transactions {
-     if tx.TransactionType > 4 { // Currently supported types are 0-4
-         log.Warn().Msgf("Skipping transaction root validation for block %s due to unsupported transaction type %d", blockData.Block.Number, tx.TransactionType)
-         return nil
-     }
- }
+ supportedTypes := map[uint8]bool{0: true, 1: true, 2: true, 3: true, 4: true}
+ for _, tx := range blockData.Transactions {
+     if !supportedTypes[tx.TransactionType] {
+         log.Warn().Msgf("Skipping transaction root validation for block %s due to unsupported transaction type %d", blockData.Block.Number, tx.TransactionType)
+         return nil
+     }
+ }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cmd/validate.go (2)

69-87: ⚠️ Potential issue

Set TLS MinVersion for enhanced security.

The TLS configuration lacks a MinVersion setting, potentially allowing insecure TLS protocol versions. This was flagged in previous reviews and by static analysis.

conn, err := clickhouse.Open(&clickhouse.Options{
	Addr:     []string{fmt.Sprintf("%s:%d", config.Cfg.Storage.Main.Clickhouse.Host, config.Cfg.Storage.Main.Clickhouse.Port)},
	Protocol: clickhouse.Native,
-	TLS:      &tls.Config{},
+	TLS: &tls.Config{
+		MinVersion: tls.VersionTLS12,
+	},
	Auth: clickhouse.Auth{
		Username: config.Cfg.Storage.Main.Clickhouse.Username,
		Password: config.Cfg.Storage.Main.Clickhouse.Password,
	},

🧰 Tools

🪛 ast-grep (0.38.1)

[warning] 71-71: MinVersionis missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported an insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. AddMinVersion: tls.VersionTLS13' to the TLS configuration to bump the minimum version to TLS 1.3.
Context: tls.Config{}
Note: [CWE-327]: Use of a Broken or Risky Cryptographic Algorithm [OWASP A03:2017]: Sensitive Data Exposure [OWASP A02:2021]: Cryptographic Failures [REFERENCES]
https://owasp.org/Top10/A02_2021-Cryptographic_Failures

(missing-ssl-minversion-go)

---

115-133: 🛠️ Refactor suggestion

Replace log.Fatal() with proper error handling.

The function uses log.Fatal() which terminates the program immediately, preventing graceful error handling and cleanup. This was flagged in previous reviews.

-func validateAndFixRange(rpcClient rpc.IRPCClient, s storage.IStorage, conn clickhouse.Conn, startBlock *big.Int, endBlock *big.Int, fixBatchSize int) error {
+func validateAndFixRange(rpcClient rpc.IRPCClient, s storage.IStorage, conn clickhouse.Conn, startBlock *big.Int, endBlock *big.Int, fixBatchSize int) error {
	chainId := rpcClient.GetChainID()
	err := validation.FindAndRemoveDuplicates(conn, chainId, startBlock, endBlock)
	if err != nil {
-		log.Fatal().Err(err).Msg("Failed to find and fix duplicates")
+		return fmt.Errorf("failed to find and fix duplicates: %w", err)
	}

	err = validation.FindAndFixGaps(rpcClient, s, conn, chainId, startBlock, endBlock)
	if err != nil {
-		log.Fatal().Err(err).Msg("Failed to find and fix gaps")
+		return fmt.Errorf("failed to find and fix gaps: %w", err)
	}

	err = validation.ValidateAndFixBlocks(rpcClient, s, conn, startBlock, endBlock, fixBatchSize)
	if err != nil {
-		log.Fatal().Err(err).Msg("Failed to validate and fix blocks")
+		return fmt.Errorf("failed to validate and fix blocks: %w", err)
	}

	log.Debug().Msgf("Validation complete for range %v-%v", startBlock, endBlock)
	return nil
}

internal/validation/validation.go (3)

288-296: ⚠️ Potential issue

*Prevent panics from nil big.Int inputs to uint256.FromBig.

Fields like tx.MaxPriorityFeePerGas, tx.MaxFeePerBlobGas, tx.Value, etc. may be nil for certain transaction types, causing uint256.FromBig(nil) to panic.

Add a helper function and use it for safe conversions:

+func safeUint256(b *big.Int) *uint256.Int {
+	if b == nil {
+		return new(uint256.Int)
+	}
+	out, _ := uint256.FromBig(b)
+	return out
+}

Then replace the direct calls:

-		chainID, _ := uint256.FromBig(tx.ChainId)
-		gasTipCap, _ := uint256.FromBig(tx.MaxPriorityFeePerGas)
-		gasFeeCap, _ := uint256.FromBig(tx.MaxFeePerGas)
-		value, _ := uint256.FromBig(tx.Value)
-		blobFeeCap, _ := uint256.FromBig(tx.MaxFeePerBlobGas)
-		v, _ := uint256.FromBig(tx.V)
-		r, _ := uint256.FromBig(tx.R)
-		s, _ := uint256.FromBig(tx.S)
+		chainID := safeUint256(tx.ChainId)
+		gasTipCap := safeUint256(tx.MaxPriorityFeePerGas)
+		gasFeeCap := safeUint256(tx.MaxFeePerGas)
+		value := safeUint256(tx.Value)
+		blobFeeCap := safeUint256(tx.MaxFeePerBlobGas)
+		v := safeUint256(tx.V)
+		r := safeUint256(tx.R)
+		s := safeUint256(tx.S)

---

333-340: ⚠️ Potential issue

Apply the same nil-safety fix for EIP-7702 transactions.

The same nil pointer risk exists here for the EIP-7702 transaction type conversion.

-		chainID, _ := uint256.FromBig(tx.ChainId)
-		gasTipCap, _ := uint256.FromBig(tx.MaxPriorityFeePerGas)
-		gasFeeCap, _ := uint256.FromBig(tx.MaxFeePerGas)
-		value, _ := uint256.FromBig(tx.Value)
-		v, _ := uint256.FromBig(tx.V)
-		r, _ := uint256.FromBig(tx.R)
-		s, _ := uint256.FromBig(tx.S)
+		chainID := safeUint256(tx.ChainId)
+		gasTipCap := safeUint256(tx.MaxPriorityFeePerGas)
+		gasFeeCap := safeUint256(tx.MaxFeePerGas)
+		value := safeUint256(tx.Value)
+		v := safeUint256(tx.V)
+		r := safeUint256(tx.R)
+		s := safeUint256(tx.S)

---

56-58: ⚠️ Potential issue

Handle error return from poller.Poll.

The error returned by poller.Poll is not checked, which could silently ignore polling failures for invalid blocks.

poller := orchestrator.NewBoundlessPoller(rpcClient, s)
-poller.Poll(context.Background(), batch)
+if err := poller.Poll(context.Background(), batch); err != nil {
+	log.Error().Err(err).Msgf("Failed to poll batch of blocks %s-%s", batch[0], batch[len(batch)-1])
+	return fmt.Errorf("failed to poll invalid blocks: %w", err)
+}

internal/validation/duplicates.go (4)

33-34: ⚠️ Potential issue

Handle error return from removeDuplicateBlocks.

The error returned by removeDuplicateBlocks is not checked, potentially ignoring failures in duplicate removal.

log.Debug().Msgf("Found %d duplicate blocks in range %v-%v: %v", len(duplicateBlockNumbers), startBlock, endBlock, duplicateBlockNumbers)
-removeDuplicateBlocks(conn, chainId, duplicateBlockNumbers)
+if err := removeDuplicateBlocks(conn, chainId, duplicateBlockNumbers); err != nil {
+	return fmt.Errorf("failed to remove duplicate blocks: %w", err)
+}

🧰 Tools

🪛 golangci-lint (1.64.8)

33-33: Error return value is not checked

(errcheck)

---

44-45: ⚠️ Potential issue

Handle error return from removeDuplicateTransactions.

The error returned by removeDuplicateTransactions is not checked, potentially ignoring failures in duplicate removal.

log.Debug().Msgf("Found %d duplicate transactions in range %v-%v: %v", len(duplicateTransactions), startBlock, endBlock, duplicateTransactions)
-removeDuplicateTransactions(conn, chainId, duplicateTransactions)
+if err := removeDuplicateTransactions(conn, chainId, duplicateTransactions); err != nil {
+	return fmt.Errorf("failed to remove duplicate transactions: %w", err)
+}

🧰 Tools

🪛 golangci-lint (1.64.8)

44-44: Error return value is not checked

(errcheck)

---

55-56: ⚠️ Potential issue

Handle error return from removeDuplicateLogs.

The error returned by removeDuplicateLogs is not checked, potentially ignoring failures in duplicate removal.

log.Debug().Msgf("Found %d duplicate logs in range %v-%v: %v", len(duplicateLogs), startBlock, endBlock, duplicateLogs)
-removeDuplicateLogs(conn, chainId, duplicateLogs)
+if err := removeDuplicateLogs(conn, chainId, duplicateLogs); err != nil {
+	return fmt.Errorf("failed to remove duplicate logs: %w", err)
+}

🧰 Tools

🪛 golangci-lint (1.64.8)

55-55: Error return value is not checked

(errcheck)

---

218-229: 🛠️ Refactor suggestion

Avoid string interpolation in SQL queries for security.

The SQL query uses fmt.Sprintf to embed tuples directly, which poses security risks and could create oversized queries.

Consider using parameterized queries or batching with smaller queries. If ClickHouse supports tuple arrays as parameters, use those instead:

-		query := fmt.Sprintf(`WITH
-			to_be_inserted AS (
-				SELECT chain_id, block_number, block_hash, block_timestamp, transaction_hash, transaction_index, log_index, address,
-					data, topic_0, topic_1, topic_2, topic_3, insert_timestamp, -sign as sign
-				FROM default.logs FINAL
-				WHERE chain_id = ? AND block_number IN (?) AND (transaction_hash, log_index) IN (%s)
-			)
-			INSERT INTO logs (
-				chain_id, block_number, block_hash, block_timestamp, transaction_hash, transaction_index, log_index, address,
-				data, topic_0, topic_1, topic_2, topic_3, insert_timestamp, sign
-			) SELECT * from to_be_inserted
-		`, strings.Join(tuples, ","))
+		// Alternative approach: process each log individually or use smaller batches
+		for _, duplicateLog := range batch {
+			query := `WITH
+				to_be_inserted AS (
+					SELECT chain_id, block_number, block_hash, block_timestamp, transaction_hash, transaction_index, log_index, address,
+						data, topic_0, topic_1, topic_2, topic_3, insert_timestamp, -sign as sign
+					FROM default.logs FINAL
+					WHERE chain_id = ? AND block_number = ? AND transaction_hash = ? AND log_index = ?
+				)
+				INSERT INTO logs (
+					chain_id, block_number, block_hash, block_timestamp, transaction_hash, transaction_index, log_index, address,
+					data, topic_0, topic_1, topic_2, topic_3, insert_timestamp, sign
+				) SELECT * from to_be_inserted`
+			
+			err := conn.Exec(context.Background(), query, chainId, duplicateLog.BlockNumber, duplicateLog.TxHash, duplicateLog.LogIndex)
+			if err != nil {
+				return err
+			}
+		}

cmd/validate.go (1)

19-26: Complete the command descriptions.

The Short and Long descriptions are marked as "TBD". Please provide meaningful descriptions for the validate command to improve CLI usability.

var (
	validateCmd = &cobra.Command{
		Use:   "validate",
-		Short: "TBD",
-		Long:  "TBD",
+		Short: "Validate and fix blockchain data integrity",
+		Long:  `Validate and fix blockchain data integrity by checking for duplicates, gaps, and block validation errors. Supports custom batch sizes for processing.`,
		Run: func(cmd *cobra.Command, args []string) {
			RunValidate(cmd, args)
		},
	}
)

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cmd/validate.go (2)

69-87: Set TLS MinVersion for better security.

The TLS configuration lacks a MinVersion setting, potentially allowing insecure TLS protocol versions.

TLS: &tls.Config{
+   MinVersion: tls.VersionTLS12,
},

---

115-134: Improve error handling by returning errors instead of using log.Fatal().

The function uses log.Fatal() which terminates the program immediately without cleanup. Consider returning errors for better error handling.

func validateAndFixRange(rpcClient rpc.IRPCClient, s storage.IStorage, conn clickhouse.Conn, startBlock *big.Int, endBlock *big.Int, fixBatchSize int) error {
    chainId := rpcClient.GetChainID()
    err := validation.FindAndRemoveDuplicates(conn, chainId, startBlock, endBlock)
    if err != nil {
-       log.Fatal().Err(err).Msg("Failed to find and fix duplicates")
+       return fmt.Errorf("failed to find and fix duplicates: %w", err)
    }

    err = validation.FindAndFixGaps(rpcClient, s, conn, chainId, startBlock, endBlock)
    if err != nil {
-       log.Fatal().Err(err).Msg("Failed to find and fix gaps")
+       return fmt.Errorf("failed to find and fix gaps: %w", err)
    }

    err = validation.ValidateAndFixBlocks(rpcClient, s, conn, startBlock, endBlock, fixBatchSize)
    if err != nil {
-       log.Fatal().Err(err).Msg("Failed to validate and fix blocks")
+       return fmt.Errorf("failed to validate and fix blocks: %w", err)
    }

    log.Debug().Msgf("Validation complete for range %v-%v", startBlock, endBlock)
    return nil
}

internal/validation/validation.go (4)

56-58: Fix the poller.Poll call signature.

Based on past review comments, the poller.Poll(batch) call appears to have incorrect arity and is missing required arguments.

poller := orchestrator.NewBoundlessPoller(rpcClient, s)
-poller.Poll(context.Background(), batch)
+if err := poller.Poll(context.Background(), batch); err != nil {
+    return fmt.Errorf("polling failed for blocks %s-%s: %w", batch[0], batch[len(batch)-1], err)
+}

---

164-166: Handle contract creation transactions correctly.

Based on past review feedback, the current implementation sets 0x0000000000000000000000000000000000000000 for contract creation transactions instead of properly handling the nil To field.

-// Convert to address
-to := common.HexToAddress(tx.ToAddress)
-isContractCreation := tx.ToAddress == ""
+// Handle contract creation (nil To address)
+var toPtr *common.Address
+isContractCreation := tx.ToAddress == ""
+if !isContractCreation {
+    addr := common.HexToAddress(tx.ToAddress)
+    toPtr = &addr
+}

Then update the transaction creation logic to use toPtr instead of to for non-legacy transactions.

---

288-296: Add nil checks for big.Int to uint256 conversions to prevent panics.

The uint256.FromBig() function will panic if passed a nil pointer. Fields like MaxPriorityFeePerGas, MaxFeePerBlobGas, Value, R, S, etc. can be NULL in the database.

+func safeUint256(b *big.Int) *uint256.Int {
+    if b == nil {
+        return new(uint256.Int)
+    }
+    out, _ := uint256.FromBig(b)
+    return out
+}

Then replace the direct calls:

-chainID, _ := uint256.FromBig(tx.ChainId)
-gasTipCap, _ := uint256.FromBig(tx.MaxPriorityFeePerGas)
-gasFeeCap, _ := uint256.FromBig(tx.MaxFeePerGas)
-value, _ := uint256.FromBig(tx.Value)
-blobFeeCap, _ := uint256.FromBig(tx.MaxFeePerBlobGas)
-v, _ := uint256.FromBig(tx.V)
-r, _ := uint256.FromBig(tx.R)
-s, _ := uint256.FromBig(tx.S)
+chainID := safeUint256(tx.ChainId)
+gasTipCap := safeUint256(tx.MaxPriorityFeePerGas)
+gasFeeCap := safeUint256(tx.MaxFeePerGas)
+value := safeUint256(tx.Value)
+blobFeeCap := safeUint256(tx.MaxFeePerBlobGas)
+v := safeUint256(tx.V)
+r := safeUint256(tx.R)
+s := safeUint256(tx.S)

---

334-340: Apply the same nil check fix for EIP-7702 transaction uint256 conversions.

Similar to the blob transaction case, these conversions also need protection against nil pointers.

-chainID, _ := uint256.FromBig(tx.ChainId)
-gasTipCap, _ := uint256.FromBig(tx.MaxPriorityFeePerGas)
-gasFeeCap, _ := uint256.FromBig(tx.MaxFeePerGas)
-value, _ := uint256.FromBig(tx.Value)
-v, _ := uint256.FromBig(tx.V)
-r, _ := uint256.FromBig(tx.R)
-s, _ := uint256.FromBig(tx.S)
+chainID := safeUint256(tx.ChainId)
+gasTipCap := safeUint256(tx.MaxPriorityFeePerGas)
+gasFeeCap := safeUint256(tx.MaxFeePerGas)
+value := safeUint256(tx.Value)
+v := safeUint256(tx.V)
+r := safeUint256(tx.R)
+s := safeUint256(tx.S)

cmd/validate.go (1)

18-27: Add description for the validate command.

The command currently has placeholder "TBD" values for Short and Long descriptions. Consider adding meaningful descriptions to help users understand the command's purpose.

validateCmd = &cobra.Command{
    Use:   "validate",
-   Short: "TBD",
-   Long:  "TBD",
+   Short: "Validate and repair blockchain data integrity",
+   Long:  `Validate and repair blockchain data integrity by checking for duplicates, gaps, and block validation errors. Processes data in configurable batches.`,
    Run: func(cmd *cobra.Command, args []string) {
        RunValidate(cmd, args)
    },
}

internal/validation/validation.go (1)

97-103: Consider making unsupported transaction type handling configurable.

The current implementation skips transaction root validation entirely when encountering unsupported transaction types. This might mask validation issues in blocks with mixed transaction types.

Consider adding a flag to control this behavior or validate only the supported transactions:

// TODO: remove this once we know how to validate all tx types
+unsupportedTxCount := 0
for _, tx := range blockData.Transactions {
    if tx.TransactionType > 4 { // Currently supported types are 0-4
-       log.Warn().Msgf("Skipping transaction root validation for block %s due to unsupported transaction type %d", blockData.Block.Number, tx.TransactionType)
-       return nil
+       unsupportedTxCount++
    }
}
+if unsupportedTxCount > 0 {
+    log.Warn().Msgf("Block %s contains %d unsupported transaction types, skipping transaction root validation", blockData.Block.Number, unsupportedTxCount)
+    return nil
+}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cmd/validate.go (1)

31-76: Consider improving error handling instead of using log.Fatal().

The function uses log.Fatal() which terminates the program immediately without cleanup. This issue was previously raised for similar code.

internal/validation/duplicates.go (3)

71-76: Consider using count() > 1 for more robust duplicate detection.

As previously noted, HAVING sum(sign) != 1 may miss duplicate scenarios where rows have signs like (+1, +1, -1) that sum to 1. Using count() > 1 would be more reliable for detecting any group with multiple live rows.

-HAVING sum(sign) != 1
+HAVING count() > 1

---

97-101: Apply consistent duplicate detection logic across all find functions.

The same duplicate detection issue applies to transactions and logs queries. Consider using count() > 1 consistently across all three functions.

Also applies to: 123-127

---

227-241: String-built IN clause still poses risks for large queries and bypasses sanitization.

The use of fmt.Sprintf to embed tuples directly into SQL remains problematic:

1. Large duplicate sets can exceed server query length limits
2. String concatenation bypasses ClickHouse's query parameter sanitization

Consider using parameterized queries with array inputs or batching with smaller, fully parameterized queries.

Does ClickHouse support array of tuples as query parameters for IN clauses?

internal/storage/connector.go (1)

106-109: Enhance the method documentation for clarity.

The comment "Gets only the data required for validation" is vague. Consider specifying what subset of data is retrieved (e.g., blocks with transactions and logs, but without traces).

 /**
- * Gets only the data required for validation.
+ * Gets block data required for validation, including blocks, transactions, and logs
+ * for the specified block range. Traces are excluded as they are not needed for validation.
  */

internal/validation/validator.go (4)

29-34: Use idiomatic Go documentation format.

Replace the JSDoc-style comment with Go's standard documentation format.

-/**
- * Validate blocks in the range of startBlock to endBlock
- * @param startBlock - The start block number (inclusive)
- * @param endBlock - The end block number (inclusive)
- * @return error - An error if the validation fails
- */
+// ValidateBlockRange validates blocks in the range of startBlock to endBlock (inclusive).
+// It returns slices of valid and invalid blocks, and an error if the validation fails.

---

40-45: Remove redundant return statement.

The explicit return at line 44 is unnecessary since the same values are already being returned.

 validBlocks, invalidBlocks, err = v.ValidateBlocks(dbData)
 if err != nil {
     return nil, nil, err
 }
-return validBlocks, invalidBlocks, nil
+return validBlocks, invalidBlocks, err

---

48-49: Simplify slice initialization.

Use nil initialization for empty slices instead of make with 0 capacity.

-invalidBlocks = make([]common.BlockData, 0)
-validBlocks = make([]common.BlockData, 0)
+var invalidBlocks []common.BlockData
+var validBlocks []common.BlockData

---

78-84: Track the TODO for unsupported transaction types.

The TODO comment indicates that transaction root validation is skipped for certain transaction types. This should be tracked to ensure complete validation coverage.

Would you like me to create an issue to track the implementation of validation for all transaction types?

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cmd/validate_and_fix.go (2)

57-57: Fix typo in log message.

The log message contains a typo in the function name.

-	log.Info().Msgf("Running validationAndFix for chain %d", rpcClient.GetChainID())
+	log.Info().Msgf("Running validateAndFix for chain %d", rpcClient.GetChainID())

---

114-116: Use Go-style comments.

The function comment uses C-style comment syntax instead of Go conventions.

-/**
- * Validates a range of blocks (end and start are inclusive) for a given chain and fixes any problems it finds
- */
+// validateAndFixRange validates a range of blocks (end and start are inclusive) for a given chain and fixes any problems it finds

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cmd/validate_and_fix.go (1)

70-94: Consider reusing storage abstraction for ClickHouse connection.

The ClickHouse connection is being established directly here with hardcoded configuration, which duplicates the connection logic that likely exists in the storage layer. This creates maintenance overhead and potential configuration drift.

Consider extracting a method from the storage layer to provide the ClickHouse connection, or add a method to the storage interface to get the underlying connection.

internal/storage/clickhouse.go (1)

914-920: getBlockNumbersStringArray generates invalid SQL with trailing comma.

The function unconditionally appends a comma after every element, resulting in invalid SQL syntax like block_number IN (123,456,). ClickHouse will reject the extra comma.

Apply the fix suggested in the previous review:

-func getBlockNumbersStringArray(blockNumbers []*big.Int) string {
-    blockNumbersString := ""
-    for _, blockNumber := range blockNumbers {
-        blockNumbersString += fmt.Sprintf("%s,", blockNumber.String())
-    }
-    return blockNumbersString
-}
+func getBlockNumbersStringArray(blockNumbers []*big.Int) string {
+    if len(blockNumbers) == 0 {
+        return ""
+    }
+    var b strings.Builder
+    for i, n := range blockNumbers {
+        if i > 0 {
+            b.WriteByte(',')
+        }
+        b.WriteString(n.String())
+    }
+    return b.String()
+}

internal/validation/duplicates.go (2)

71-76: HAVING sum(sign) != 1 may miss certain duplicate scenarios.

If rows exist with signs (+1, +1, -1) the sum equals +1, therefore the group is not flagged, yet you still have two live (+1) rows – a duplicate situation. Using count() > 1 (or abs(sum(sign)) != 1) is safer.

Consider using a more robust duplicate detection:

-HAVING sum(sign) != 1
+HAVING count(*) > 1 AND sum(sign) > 0

---

227-238: String-built IN clause risks gigantic queries & SQL injection.

fmt.Sprintf embeds a comma-separated list of tuples directly into the SQL string. This can create excessively large queries and bypasses ClickHouse query-parameter sanitization.

Consider using parameterized queries or batching with smaller, safer queries:

-WHERE chain_id = ? AND block_number IN (?) AND (transaction_hash, log_index) IN (%s)
+// Split into smaller batches and use individual parameter binding
+WHERE chain_id = ? AND block_number = ? AND transaction_hash = ? AND log_index = ?

internal/orchestrator/validator.go (1)

125-127: Consider continuing with remaining batches on partial failures.

The current implementation stops processing on the first failed batch, potentially leaving many fixable blocks unprocessed. Consider collecting errors and continuing with remaining batches to maximize the number of fixed blocks.

+var allErrors []error
 for i := 0; i < len(invalidBlocks); i += fixBatchSize {
     // ... existing batch logic ...
     if len(failedBlocks) > 0 {
         log.Error().Msgf("Failed to poll %d blocks: %v", len(failedBlocks), failedBlocks)
-        return fmt.Errorf("failed to poll %d blocks: %v", len(failedBlocks), failedBlocks)
+        allErrors = append(allErrors, fmt.Errorf("failed to poll %d blocks: %v", len(failedBlocks), failedBlocks))
+        continue
     }
     // ... rest of processing ...
 }
+if len(allErrors) > 0 {
+    return fmt.Errorf("encountered %d errors during fixing: %v", len(allErrors), allErrors)
+}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

cmd/validate_and_fix.go (2)

70-94: Consider reusing storage abstraction for ClickHouse connection.

The ClickHouse connection is being established directly here with hardcoded configuration, which duplicates the connection logic that likely exists in the storage layer. This creates maintenance overhead and potential configuration drift.

Consider extracting a method from the storage layer to provide the ClickHouse connection, or add a method to the storage interface to get the underlying connection.

---

96-96: Add nil check for cursor fields before use.

The code directly accesses cursor.LastScannedBlockNumber and cursor.MaxBlockNumber without verifying they are initialized. If cursor initialization fails to set these values, this could cause a nil pointer dereference.

internal/storage/clickhouse.go (1)

483-486: createBlockNumbersClause may generate invalid SQL because of a trailing comma.

getBlockNumbersStringArray unconditionally appends a comma after every element. When the slice is non-empty, the resulting clause becomes e.g. block_number IN (123,456,) which ClickHouse will reject.

-func getBlockNumbersStringArray(blockNumbers []*big.Int) string {
-    blockNumbersString := ""
-    for _, blockNumber := range blockNumbers {
-        blockNumbersString += fmt.Sprintf("%s,", blockNumber.String())
-    }
-    return blockNumbersString
-}
+func getBlockNumbersStringArray(blockNumbers []*big.Int) string {
+    if len(blockNumbers) == 0 {
+        return ""
+    }
+    var b strings.Builder
+    for i, n := range blockNumbers {
+        if i > 0 {
+            b.WriteByte(',')
+        }
+        b.WriteString(n.String())
+    }
+    return b.String()
+}

Without this fix, every query that relies on block-number filtering (including the new validation CLI) will fail.

Also applies to: 914-920

internal/orchestrator/validator.go (1)

80-85: Consider returning a distinct validation status for unsupported transaction types.

Currently, blocks with unsupported transaction types (> 4) are marked as valid without actual validation. This could mask data integrity issues.

internal/validation/duplicates.go (2)

71-76: HAVING sum(sign) != 1 may miss certain duplicate scenarios.

If rows exist with signs (+1, +1, -1) the sum equals +1, therefore the group is not flagged, yet you still have two live (+1) rows – a duplicate situation. Using count() > 1 (or abs(sum(sign)) != 1) is safer.

Also applies to: 100-101, 126-127

---

227-241: String-built IN clause risks gigantic queries & SQL-injection.

fmt.Sprintf embeds a comma-separated list of tuples directly into the SQL string. This can create excessively large queries and bypasses ClickHouse query-parameter sanitization.

Prefer using parameterized queries or batching with smaller queries.

cmd/validate_and_fix.go (1)

115-117: Use standard Go documentation format.

Use the standard Go documentation format with // instead of /** */.

-/**
- * Validates a range of blocks (end and start are inclusive) for a given chain and fixes any problems it finds
- */
+// validateAndFixRange validates a range of blocks (end and start are inclusive) for a given chain and fixes any problems it finds

internal/storage/clickhouse.go (1)

1867-1912: Consider clarifying the NULL check condition.

The query logic is clever, using ClickHouse's numbers() function to generate the expected sequence. However, the condition e.block_number = 0 might be confusing as it's actually checking for NULL values after the LEFT JOIN.

Consider adding a comment or using IS NULL for clarity:

WHERE 
-    e.block_number = 0
+    e.block_number IS NULL  -- No matching block found in existing_blocks

internal/orchestrator/validator.go (2)

29-34: Use standard Go documentation format.

-/**
- * Validate blocks in the range of startBlock to endBlock
- * @param startBlock - The start block number (inclusive)
- * @param endBlock - The end block number (inclusive)
- * @return error - An error if the validation fails
- */
+// ValidateBlockRange validates blocks in the range of startBlock to endBlock.
+// Both startBlock and endBlock are inclusive.

---

47-63: Consider collecting all validation errors instead of stopping on first error.

The current implementation stops validation on the first error, which might make debugging difficult when multiple blocks have issues. Consider validating all blocks and returning a summary of errors.

 func (v *Validator) ValidateBlocks(blocks []common.BlockData) (validBlocks []common.BlockData, invalidBlocks []common.BlockData, err error) {
     invalidBlocks = make([]common.BlockData, 0)
     validBlocks = make([]common.BlockData, 0)
+    var validationErrors []error
     for _, blockData := range blocks {
         valid, err := ValidateBlock(blockData)
         if err != nil {
             log.Error().Err(err).Msgf("Block verification failed for block %s", blockData.Block.Number)
-            return nil, nil, err
+            validationErrors = append(validationErrors, fmt.Errorf("block %s: %w", blockData.Block.Number, err))
+            continue
         }
         if valid {
             validBlocks = append(validBlocks, blockData)
         } else {
             invalidBlocks = append(invalidBlocks, blockData)
         }
     }
+    if len(validationErrors) > 0 {
+        return validBlocks, invalidBlocks, fmt.Errorf("validation errors: %v", validationErrors)
+    }
     return validBlocks, invalidBlocks, nil
 }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

internal/orchestrator/validator.go (1)

80-85: Consider returning a distinct validation status for unsupported transaction types.

Currently, blocks with unsupported transaction types (> 4) are marked as valid without actual validation. This could mask data integrity issues.

internal/validation/root_calculator.go (1)

221-224: Fix contract creation handling for EIP-7702 transactions.

Similar to blob transactions, the To field handling for contract creation is incorrect.

internal/orchestrator/validator.go (1)

47-63: Consider preserving partial validation results on error.

When ValidateBlock returns an error, the function discards all previously validated blocks by returning nil slices. This could lead to loss of validation progress if an error occurs partway through a large batch.

Consider returning the partial results along with the error:

 func (v *Validator) ValidateBlocks(blocks []common.BlockData) (validBlocks []common.BlockData, invalidBlocks []common.BlockData, err error) {
 	invalidBlocks = make([]common.BlockData, 0)
 	validBlocks = make([]common.BlockData, 0)
 	for _, blockData := range blocks {
 		valid, err := ValidateBlock(blockData)
 		if err != nil {
 			log.Error().Err(err).Msgf("Block verification failed for block %s", blockData.Block.Number)
-			return nil, nil, err
+			return validBlocks, invalidBlocks, err
 		}
 		if valid {
 			validBlocks = append(validBlocks, blockData)
 		} else {
 			invalidBlocks = append(invalidBlocks, blockData)
 		}
 	}
 	return validBlocks, invalidBlocks, nil
 }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro